Adding privacy request service tests

src/fides/api/api/v1/endpoints/consent_request_endpoints.py

@@ -23,8 +23,8 @@
 
 from fides.api.api.deps import get_config_proxy, get_db, get_messaging_service
 from fides.api.common_exceptions import (
-    RedisNotConfigured,
     IdentityVerificationException,
+    RedisNotConfigured,
 )
 from fides.api.db.seed import DEFAULT_CONSENT_POLICY
 from fides.api.models.messaging import get_messaging_method

src/fides/api/api/v1/endpoints/privacy_request_endpoints.py

@@ -17,7 +17,6 @@
     Union,
 )
 
-from fides.service.messaging.messaging_service import MessagingService
 import sqlalchemy
 from fastapi import Body, Depends, HTTPException, Security
 from fastapi.encoders import jsonable_encoder
@@ -167,6 +166,7 @@
 )
 from fides.config import CONFIG
 from fides.config.config_proxy import ConfigProxy
+from fides.service.messaging.messaging_service import MessagingService
 from fides.service.privacy_request.privacy_request_service import (
     PrivacyRequestService,
     _trigger_pre_approval_webhooks,

src/fides/api/app_setup.py

@@ -23,7 +23,7 @@
 from fides.api.api.v1.endpoints.generic_overrides import GENERIC_OVERRIDES_ROUTER
 from fides.api.api.v1.endpoints.health import HEALTH_ROUTER
 from fides.api.api.v1.exception_handlers import ExceptionHandlers
-from fides.api.common_exceptions import RedisNotConfigured, RedisConnectionError
+from fides.api.common_exceptions import RedisConnectionError, RedisNotConfigured
 from fides.api.db.database import configure_db
 from fides.api.db.seed import create_or_update_parent_user
 from fides.api.models.application_config import ApplicationConfig

src/fides/service/privacy_request/privacy_request_service.py

@@ -5,8 +5,9 @@
 from sqlalchemy.orm import Session
 
 from fides.api.common_exceptions import (
-    RedisNotConfigured,
+    FidesopsException,
     MessageDispatchException,
+    RedisNotConfigured,
 )
 from fides.api.models.audit_log import AuditLog, AuditLogAction
 from fides.api.models.policy import Policy
@@ -22,18 +23,14 @@
 )
 from fides.api.models.property import Property
 from fides.api.schemas.api import BulkUpdateFailed
-from fides.api.schemas.messaging.messaging import (
-    MessagingActionType,
-)
+from fides.api.schemas.messaging.messaging import MessagingActionType
 from fides.api.schemas.privacy_request import (
     BulkPostPrivacyRequests,
     BulkReviewResponse,
     PrivacyRequestCreate,
     PrivacyRequestResponse,
 )
-from fides.api.service.messaging.message_dispatch_service import (
-    message_send_enabled,
-)
+from fides.api.service.messaging.message_dispatch_service import message_send_enabled
 from fides.api.service.privacy_request.request_service import (
     build_required_privacy_request_kwargs,
     cache_data,
@@ -113,7 +110,7 @@ def create_privacy_request(
             )
             if not valid_property:
                 raise PrivacyRequestError(
-                    "Property id must be valid to process",
+                    "Property ID must be valid to process",
                     privacy_request_data.model_dump(mode="json"),
                 )
 
@@ -264,6 +261,9 @@ def resubmit_privacy_request(
         if not existing_privacy_request:
             return None
 
+        if existing_privacy_request.status == PrivacyRequestStatus.complete:
+            raise FidesopsException("Cannot resubmit a completed privacy request")
+
         # Copy all needed data first
         create_data = PrivacyRequestCreate(
             id=privacy_request_id,
@@ -295,7 +295,10 @@ def resubmit_privacy_request(
             authenticated=True,
         )
 
-        self.approve_privacy_requests([privacy_request_id], user_id=user_id)
+        if self.config_proxy.execution.require_manual_request_approval:
+            self.approve_privacy_requests(
+                [privacy_request_id], user_id=user_id, suppress_notification=True
+            )
 
         return privacy_request
 
@@ -316,7 +319,7 @@ def approve_privacy_requests(
             if not privacy_request:
                 failed.append(
                     BulkUpdateFailed(
-                        message=f"No privacy request found with id '{request_id}'",
+                        message=f"No privacy request found with ID '{request_id}'",
                         data={"privacy_request_id": request_id},
                     )
                 )

tests/conftest.py

@@ -40,6 +40,7 @@
 from fides.api.schemas.messaging.messaging import MessagingServiceType
 from fides.api.task.graph_runners import access_runner, consent_runner, erasure_runner
 from fides.api.tasks import celery_app
+from fides.api.tasks.scheduled.scheduler import async_scheduler, scheduler
 from fides.api.util.cache import get_cache
 from fides.api.util.collection_util import Row
 from fides.common.api.scope_registry import SCOPE_REGISTRY
@@ -79,6 +80,33 @@
 TEST_DEPRECATED_CONFIG_PATH = "tests/ctl/test_deprecated_config.toml"
 
 
+@pytest.fixture(scope="session")
+def db(api_client, config):
+    """Return a connection to the test DB"""
+    # Create the test DB engine
+    assert config.test_mode
+    assert requests.post != api_client.post
+    engine = get_db_engine(
+        database_uri=config.database.sqlalchemy_test_database_uri,
+    )
+
+    create_citext_extension(engine)
+
+    if not scheduler.running:
+        scheduler.start()
+    if not async_scheduler.running:
+        async_scheduler.start()
+
+    SessionLocal = get_db_session(config, engine=engine)
+    the_session = SessionLocal()
+    # Setup above...
+
+    yield the_session
+    # Teardown below...
+    the_session.close()
+    engine.dispose()
+
+
 @pytest.fixture(scope="session")
 def test_client():
     """Starlette test client fixture. Easier to use mocks with when testing out API calls"""

tests/lib/conftest.py

@@ -19,27 +19,6 @@ def setup_db(api_client, config):
     yield api_client.post(url=f"{config.cli.server_url}/v1/admin/db/reset")
 
 
-@pytest.fixture(scope="session")
-def db(api_client, config):
-    """Return a connection to the test DB"""
-    # Create the test DB engine
-    assert config.test_mode
-    assert requests.post != api_client.post
-    engine = get_db_engine(
-        database_uri=config.database.sqlalchemy_test_database_uri,
-    )
-
-    create_citext_extension(engine)
-
-    SessionLocal = get_db_session(config, engine=engine)
-    the_session = SessionLocal()
-
-    yield the_session
-
-    the_session.close()
-    engine.dispose()
-
-
 @pytest.fixture(autouse=True)
 def clear_db_tables(db):
     """Clear data from tables between tests.

tests/ops/conftest.py

@@ -23,33 +23,6 @@ def setup_db(api_client, config):
     yield api_client.post(url=f"{config.cli.server_url}/v1/admin/db/reset")
 
 
-@pytest.fixture(scope="session")
-def db(api_client, config):
-    """Return a connection to the test DB"""
-    # Create the test DB engine
-    assert config.test_mode
-    assert requests.post != api_client.post
-    engine = get_db_engine(
-        database_uri=config.database.sqlalchemy_test_database_uri,
-    )
-
-    create_citext_extension(engine)
-
-    if not scheduler.running:
-        scheduler.start()
-    if not async_scheduler.running:
-        async_scheduler.start()
-
-    SessionLocal = get_db_session(config, engine=engine)
-    the_session = SessionLocal()
-    # Setup above...
-
-    yield the_session
-    # Teardown below...
-    the_session.close()
-    engine.dispose()
-
-
 @pytest.fixture(autouse=True)
 def clear_db_tables(db):
     """Clear data from tables between tests.

tests/service/test_privacy_request_service.py

@@ -0,0 +1,112 @@
+from unittest.mock import create_autospec
+
+import pytest
+from sqlalchemy.orm import Session
+
+from fides.api.common_exceptions import FidesopsException
+from fides.api.models.connectionconfig import ConnectionConfig
+from fides.api.models.policy import Policy
+from fides.api.models.privacy_request import (
+    ExecutionLog,
+    ExecutionLogStatus,
+    PrivacyRequestStatus,
+)
+from fides.api.schemas.policy import ActionType
+from fides.api.schemas.privacy_request import PrivacyRequestCreate
+from fides.api.schemas.redis_cache import Identity
+from fides.config.config_proxy import ConfigProxy
+from fides.service.messaging.messaging_service import MessagingService
+from fides.service.privacy_request.privacy_request_service import PrivacyRequestService
+from tests.conftest import wait_for_tasks_to_complete
+
+
+class TestPrivacyRequestService:
+    @pytest.fixture
+    def mock_messaging_service(self) -> MessagingService:
+        return create_autospec(MessagingService)
+
+    @pytest.fixture
+    def privacy_request_service(
+        self, db: Session, mock_messaging_service
+    ) -> PrivacyRequestService:
+        return PrivacyRequestService(db, ConfigProxy(db), mock_messaging_service)
+
+    @pytest.mark.integration
+    @pytest.mark.integration_postgres
+    @pytest.mark.usefixtures(
+        "use_dsr_3_0",
+        "postgres_integration_db",
+        "automatically_approved",
+        "postgres_example_test_dataset_config",
+    )
+    def test_resubmit_complete_privacy_request(
+        self,
+        db: Session,
+        privacy_request_service: PrivacyRequestService,
+        mock_messaging_service: MessagingService,
+        policy: Policy,
+        connection_config: ConnectionConfig,
+    ):
+        # remove the host from the Postgres example connection
+        # to force the privacy request to error
+        secrets = connection_config.secrets
+        host = secrets.pop("host")
+        db.commit()
+
+        privacy_request = privacy_request_service.create_privacy_request(
+            PrivacyRequestCreate(
+                identity=Identity(email="jane@example.com"),
+                policy_key=policy.key,
+            ),
+            authenticated=True,
+        )
+
+        error_logs = privacy_request.execution_logs.filter(
+            ExecutionLog.status == ExecutionLogStatus.error
+        )
+        assert error_logs.count() > 0
+
+        # re-add the host to fix the Postgres connection
+        connection_config.secrets["host"] = host
+        db.commit()
+
+        # resubmit the request and wait for it to complete
+        privacy_request = privacy_request_service.resubmit_privacy_request(
+            privacy_request.id
+        )
+        wait_for_tasks_to_complete(db, privacy_request, ActionType.access)
+        db.refresh(privacy_request)
+        assert privacy_request.status == PrivacyRequestStatus.complete
+        mock_messaging_service.send_request_approved.assert_not_called()
+
+        # verify that the error logs from the original attempt
+        # were deleted as part of the re-submission
+        error_logs = privacy_request.execution_logs.filter(
+            ExecutionLog.status == ExecutionLogStatus.error
+        )
+        assert error_logs.count() == 0
+
+    @pytest.mark.usefixtures("automatically_approved")
+    def test_cannot_resubmit_complete_privacy_request(
+        self,
+        db: Session,
+        privacy_request_service: PrivacyRequestService,
+        policy: Policy,
+    ):
+        privacy_request = privacy_request_service.create_privacy_request(
+            PrivacyRequestCreate(
+                identity=Identity(email="user@example.com"),
+                policy_key=policy.key,
+            ),
+            authenticated=True,
+        )
+        wait_for_tasks_to_complete(db, privacy_request, ActionType.access)
+
+        with pytest.raises(FidesopsException):
+            privacy_request_service.resubmit_privacy_request(privacy_request.id)
+
+    def test_cannot_resubmit_non_existent_privacy_request(
+        self,
+        privacy_request_service: PrivacyRequestService,
+    ):
+        assert privacy_request_service.resubmit_privacy_request("123") is None