fix(merge): resolve tests/test_auth.py conflict keeping all tests from both branches

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>

tests/test_auth.py

@@ -174,6 +174,61 @@ class TestRequireLogin:
             assert result["message"] == "sync"
             assert result["param"] == "test_value"
 
+    @pytest.mark.asyncio
+    async def test_returns_401_for_api_paths_when_not_authenticated(self):
+        """Test that require_login returns 401 (not redirect) for /api/* paths.
+
+        This prevents the /api/auth/whoami JS probe from overwriting
+        redirect_after_login with an API URL, which would send the user to a
+        JSON endpoint after login instead of the page they actually wanted.
+        """
+        from fastapi.responses import JSONResponse
+
+        with patch("app.auth.AUTH_ENABLED", True):
+            from app.auth import require_login
+
+            @require_login
+            async def api_endpoint(request: Request):
+                return {"message": "success"}
+
+            mock_request = MagicMock(spec=Request)
+            mock_request.session = {}
+            mock_request.url = MagicMock()
+            mock_request.url.__str__ = MagicMock(return_value="http://test.com/api/auth/whoami")
+
+            result = await api_endpoint(mock_request)
+
+            assert isinstance(result, JSONResponse)
+            assert result.status_code == status.HTTP_401_UNAUTHORIZED
+            # Redirect URL must NOT be stored for API paths
+            assert "redirect_after_login" not in mock_request.session
+
+    @pytest.mark.asyncio
+    async def test_does_not_save_redirect_for_api_paths(self):
+        """Test that redirect_after_login is never set for any /api/* request."""
+        from fastapi.responses import JSONResponse
+
+        with patch("app.auth.AUTH_ENABLED", True):
+            from app.auth import require_login
+
+            @require_login
+            async def api_endpoint(request: Request):
+                return {"data": "ok"}
+
+            for api_path in ["/api/documents/upload", "/api/v1/resource", "/api/users/me"]:
+                mock_request = MagicMock(spec=Request)
+                mock_request.session = {}
+                mock_request.url = MagicMock()
+                mock_request.url.__str__ = MagicMock(return_value=f"http://test.com{api_path}")
+
+                result = await api_endpoint(mock_request)
+
+                assert isinstance(result, JSONResponse), f"Expected JSONResponse for {api_path}"
+                assert result.status_code == status.HTTP_401_UNAUTHORIZED
+                assert "redirect_after_login" not in mock_request.session, (
+                    f"redirect_after_login must not be set for {api_path}"
+                )
+
     @pytest.mark.asyncio
     async def test_path_param_before_request_async(self):
         """Regression: endpoints with a path param before request must not get

tests/test_auth_module.py

@@ -185,6 +185,33 @@ class TestRequireLogin:
             assert isinstance(result, RedirectResponse)
             assert result.status_code == status.HTTP_302_FOUND
 
+    @pytest.mark.asyncio
+    async def test_returns_401_for_api_path_when_not_authenticated(self):
+        """Test returns 401 for /api/* paths instead of redirect-to-login.
+
+        Prevents the common.js /api/auth/whoami probe from overwriting
+        redirect_after_login, which would send the user to a JSON endpoint
+        after login instead of the page they originally requested.
+        """
+        from fastapi.responses import JSONResponse
+
+        with patch("app.auth.AUTH_ENABLED", True):
+
+            @require_login
+            async def test_api_endpoint(request: Request):
+                return {"data": "ok"}
+
+            mock_request = MagicMock(spec=Request)
+            mock_request.session = {}
+            mock_request.url = MagicMock()
+            mock_request.url.__str__ = MagicMock(return_value="http://localhost/api/auth/whoami")
+
+            result = await test_api_endpoint(mock_request)
+
+            assert isinstance(result, JSONResponse)
+            assert result.status_code == status.HTTP_401_UNAUTHORIZED
+            assert "redirect_after_login" not in mock_request.session
+
 
 @pytest.mark.unit
 class TestOAuthConfiguration:

tests/test_config_validators.py

@@ -75,13 +75,13 @@ class TestValidateStorageConfigs:
             assert "Neither SFTP_KEY_PATH nor SFTP_PASSWORD is configured" in result["sftp"]
 
     def test_email_storage_missing_config(self):
-        """Test validation when email storage config is missing."""
+        """Test validation when email destination storage config is missing."""
         with patch("app.utils.config_validator.validators.settings") as mock_settings:
-            mock_settings.email_host = None
-            mock_settings.email_default_recipient = None
+            mock_settings.dest_email_host = None
+            mock_settings.dest_email_default_recipient = None
             result = validate_storage_configs()
-            assert "EMAIL_HOST is not configured" in result["email"]
-            assert "EMAIL_DEFAULT_RECIPIENT is not configured" in result["email"]
+            assert "DEST_EMAIL_HOST is not configured" in result["email"]
+            assert "DEST_EMAIL_DEFAULT_RECIPIENT is not configured" in result["email"]
 
 
 @pytest.mark.unit
@@ -438,8 +438,8 @@ class TestValidateStorageConfigsEdgeCases:
             # Configure all services
             mock_settings.sftp_host = "sftp.example.com"
             mock_settings.sftp_password = "pass"
-            mock_settings.email_host = "smtp.example.com"
-            mock_settings.email_default_recipient = "test@example.com"
+            mock_settings.dest_email_host = "smtp.example.com"
+            mock_settings.dest_email_default_recipient = "test@example.com"
             mock_settings.s3_bucket_name = "my-bucket"
             mock_settings.aws_access_key_id = "key"
             mock_settings.aws_secret_access_key = "secret"

tests/test_coverage_remaining_gaps.py

@@ -620,6 +620,13 @@ def _set_minimal_provider_settings(mock_settings):
         "email_password": None,
         "email_use_tls": True,
         "email_sender": None,
+        "dest_email_host": None,
+        "dest_email_default_recipient": None,
+        "dest_email_port": 587,
+        "dest_email_username": None,
+        "dest_email_password": None,
+        "dest_email_use_tls": True,
+        "dest_email_sender": None,
         "ftp_host": None,
         "ftp_username": None,
         "ftp_password": None,

tests/test_finalize_storage.py

@@ -372,3 +372,69 @@ class TestFinalizeDocumentStorage:
 
                     # Verify send_to_all was called with delete_after=True
                     mock_send_all.delay.assert_called_once_with("/workdir/processed/file.pdf", True, 505)
+
+    @patch("app.tasks.finalize_document_storage.notify_file_processed")
+    @patch("app.tasks.finalize_document_storage.send_to_all_destinations")
+    @patch("app.tasks.finalize_document_storage.get_configured_services_from_validator")
+    @patch("app.tasks.finalize_document_storage.log_task_progress")
+    @patch("app.tasks.finalize_document_storage.SessionLocal")
+    def test_pdfa_enabled_does_not_regress_finalize_step_to_in_progress(
+        self,
+        mock_session_local,
+        mock_log_progress,
+        mock_get_services,
+        mock_send_all,
+        mock_notify,
+    ):
+        """
+        Regression test: when PDF/A conversion is enabled, the finalize_document_storage
+        step must NOT be logged as in_progress after it has already been logged as success.
+
+        Previously, a second log_task_progress call with status="in_progress" was made for
+        "finalize_document_storage" when queueing PDF/A archival conversion, which overwrote
+        the prior success status and caused the overall file status to appear stuck in
+        processing/failed.
+        """
+        mock_get_services.return_value = {"dropbox": True}
+
+        mock_db = MagicMock()
+        mock_session_local.return_value.__enter__.return_value = mock_db
+        mock_db.query.return_value.filter.return_value.first.return_value = None
+
+        with patch("app.tasks.finalize_document_storage.os.path.exists", return_value=True):
+            with patch("app.tasks.finalize_document_storage.os.path.getsize", return_value=1024):
+                with patch("app.tasks.finalize_document_storage.os.path.basename", return_value="doc.pdf"):
+                    with patch("app.tasks.finalize_document_storage.settings") as mock_settings:
+                        mock_settings.workdir = "/tmp"
+                        mock_settings.enable_pdfa_conversion = True
+
+                        mock_convert = MagicMock()
+                        with patch(
+                            "app.tasks.finalize_document_storage.convert_to_pdfa",
+                            mock_convert,
+                            create=True,
+                        ):
+                            finalize_document_storage.request.id = "test-task-id"
+
+                            finalize_document_storage.__wrapped__(
+                                original_file="/tmp/original.pdf",
+                                processed_file="/workdir/processed/doc.pdf",
+                                metadata={"filename": "doc.pdf"},
+                                file_id=606,
+                            )
+
+                        # Collect all (step_name, status) pairs logged for finalize_document_storage
+                        finalize_calls = [
+                            call
+                            for call in mock_log_progress.call_args_list
+                            if call.args[1] == "finalize_document_storage"
+                        ]
+
+                        # After the success log, no in_progress log should follow for this step
+                        statuses = [call.args[2] for call in finalize_calls]
+                        assert "success" in statuses, "finalize_document_storage must be logged as success"
+                        # The last status logged must be success, not in_progress
+                        assert statuses[-1] == "success", (
+                            "finalize_document_storage must not be regressed to in_progress after success; "
+                            f"got statuses: {statuses}"
+                        )

tests/test_send_to_all.py

@@ -120,10 +120,10 @@ class TestShouldUploadFunctions:
     @patch("app.tasks.send_to_all.settings")
     def test_should_upload_to_email_configured(self, mock_settings):
         """Test email upload check."""
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_username = "user"
-        mock_settings.email_password = "pass"
-        mock_settings.email_default_recipient = "recipient@example.com"
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_username = "user"
+        mock_settings.dest_email_password = "pass"
+        mock_settings.dest_email_default_recipient = "recipient@example.com"
 
         assert _should_upload_to_email() is True
 

tests/test_upload_email.py

@@ -229,7 +229,7 @@ class TestPrepareRecipients:
     @patch("app.tasks.upload_to_email.settings")
     def test_uses_default_recipient_when_none_provided(self, mock_settings):
         """Test uses default recipient when none provided."""
-        mock_settings.email_default_recipient = "default@example.com"
+        mock_settings.dest_email_default_recipient = "default@example.com"
 
         result, error = _prepare_recipients(None)
 
@@ -239,7 +239,7 @@ class TestPrepareRecipients:
     @patch("app.tasks.upload_to_email.settings")
     def test_returns_error_when_no_recipients_and_no_default(self, mock_settings):
         """Test returns error when no recipients and no default."""
-        mock_settings.email_default_recipient = None
+        mock_settings.dest_email_default_recipient = None
 
         result, error = _prepare_recipients(None)
 
@@ -256,11 +256,11 @@ class TestSendEmailWithSMTP:
     @patch("app.tasks.upload_to_email.settings")
     def test_sends_email_successfully(self, mock_settings, mock_gethostbyname, mock_smtp):
         """Test sends email successfully."""
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_port = 587
-        mock_settings.email_use_tls = True
-        mock_settings.email_username = "user@example.com"
-        mock_settings.email_password = "password"
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_port = 587
+        mock_settings.dest_email_use_tls = True
+        mock_settings.dest_email_username = "user@example.com"
+        mock_settings.dest_email_password = "password"
 
         mock_server = MagicMock()
         mock_smtp.return_value.__enter__.return_value = mock_server
@@ -292,8 +292,8 @@ class TestSendEmailWithSMTP:
     @patch("app.tasks.upload_to_email.settings")
     def test_handles_connection_refused_error(self, mock_settings, mock_gethostbyname, mock_smtp):
         """Test handles connection refused error."""
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_port = 587
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_port = 587
 
         mock_smtp.return_value.__enter__.side_effect = ConnectionRefusedError("Connection refused")
 
@@ -309,11 +309,11 @@ class TestSendEmailWithSMTP:
     @patch("app.tasks.upload_to_email.settings")
     def test_sends_email_without_tls(self, mock_settings, mock_gethostbyname, mock_smtp):
         """Test sends email without TLS."""
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_port = 25
-        mock_settings.email_use_tls = False
-        mock_settings.email_username = "user@example.com"
-        mock_settings.email_password = "password"
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_port = 25
+        mock_settings.dest_email_use_tls = False
+        mock_settings.dest_email_username = "user@example.com"
+        mock_settings.dest_email_password = "password"
 
         mock_server = MagicMock()
         mock_smtp.return_value.__enter__.return_value = mock_server
@@ -333,11 +333,11 @@ class TestSendEmailWithSMTP:
     @patch("app.tasks.upload_to_email.settings")
     def test_sends_email_without_authentication(self, mock_settings, mock_gethostbyname, mock_smtp):
         """Test sends email without authentication credentials."""
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_port = 25
-        mock_settings.email_use_tls = False
-        mock_settings.email_username = None
-        mock_settings.email_password = None
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_port = 25
+        mock_settings.dest_email_use_tls = False
+        mock_settings.dest_email_username = None
+        mock_settings.dest_email_password = None
 
         mock_server = MagicMock()
         mock_smtp.return_value.__enter__.return_value = mock_server
@@ -356,8 +356,8 @@ class TestSendEmailWithSMTP:
     @patch("app.tasks.upload_to_email.settings")
     def test_handles_timeout_error(self, mock_settings, mock_gethostbyname, mock_smtp):
         """Test handles timeout error."""
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_port = 587
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_port = 587
 
         mock_smtp.return_value.__enter__.side_effect = TimeoutError("Connection timeout")
 
@@ -392,10 +392,10 @@ class TestUploadToEmailTask:
     @patch("app.tasks.upload_to_email.os.path.exists")
     @patch("app.tasks.upload_to_email.settings")
     def test_skips_when_email_host_not_configured(self, mock_settings, mock_exists, mock_log, mock_basename):
-        """Test skips when email host not configured."""
+        """Test skips when email destination host not configured."""
         mock_exists.return_value = True
         mock_basename.return_value = "test.pdf"
-        mock_settings.email_host = None
+        mock_settings.dest_email_host = None
 
         mock_self = Mock()
         mock_self.request.id = "test-task-id"
@@ -403,7 +403,7 @@ class TestUploadToEmailTask:
         result = upload_to_email(mock_self, "/tmp/test.pdf")
 
         assert result["status"] == "Skipped"
-        assert "Email host is not configured" in result["reason"]
+        assert "DEST_EMAIL_HOST" in result["reason"]
 
     @patch("app.tasks.upload_to_email.os.path.basename")
     @patch("app.tasks.upload_to_email._prepare_recipients")
@@ -414,7 +414,7 @@ class TestUploadToEmailTask:
         """Test skips when no valid recipients."""
         mock_exists.return_value = True
         mock_basename.return_value = "test.pdf"
-        mock_settings.email_host = "smtp.example.com"
+        mock_settings.dest_email_host = "smtp.example.com"
         mock_prepare.return_value = (None, "No recipients specified")
 
         mock_self = Mock()

tests/test_upload_tasks.py

@@ -364,12 +364,12 @@ def test_upload_to_email_accepts_file_id(sample_text_file):
         patch("app.tasks.upload_to_email.attach_logo") as mock_logo,
     ):
         # Setup settings
-        mock_settings.email_host = "smtp.example.com"
-        mock_settings.email_port = 587
-        mock_settings.email_username = "test@example.com"
-        mock_settings.email_password = _TEST_CREDENTIAL
-        mock_settings.email_use_tls = True
-        mock_settings.email_sender = "sender@example.com"
+        mock_settings.dest_email_host = "smtp.example.com"
+        mock_settings.dest_email_port = 587
+        mock_settings.dest_email_username = "test@example.com"
+        mock_settings.dest_email_password = _TEST_CREDENTIAL
+        mock_settings.dest_email_use_tls = True
+        mock_settings.dest_email_sender = "sender@example.com"
         mock_settings.external_hostname = "docuelevate.example.com"
 
         # Setup mocks
@@ -503,7 +503,7 @@ def test_send_to_all_calls_upload_tasks_with_keyword_argument():
             mock_settings.webdav_url = None
             mock_settings.ftp_host = None
             mock_settings.sftp_host = None
-            mock_settings.email_host = None
+            mock_settings.dest_email_host = None
             mock_settings.onedrive_client_id = None
             mock_settings.workdir = "/tmp"