Merge pull request #281 from christianlouis/copilot/fix-ruff-and-tests

Fix Ruff linting errors and test failures in upload tasks

app/api/dropbox.py

@@ -224,18 +224,17 @@ async def save_dropbox_settings(
         updated = set()
         new_env_lines = []
         for line in env_lines:
-            line = line.rstrip()
+            stripped_line = line.rstrip()
             is_updated = False
             for key, value in dropbox_settings.items():
-                if line.startswith(f"{key}=") or line.startswith(f"# {key}="):
-                    if line.startswith("# "):  # Uncomment if commented out
-                        line = line[2:]
+                if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="):
+                    # Uncomment if commented out - check the original stripped line
                     new_env_lines.append(f"{key}={value}")
                     updated.add(key)
                     is_updated = True
                     break
             if not is_updated:
-                new_env_lines.append(line)
+                new_env_lines.append(stripped_line)
 
         # Add any settings that weren't updated (they weren't in the file)
         for key, value in dropbox_settings.items():

app/api/google_drive.py

@@ -373,18 +373,17 @@ async def save_dropbox_settings(
                 updated = set()
                 new_env_lines = []
                 for line in env_lines:
-                    line = line.rstrip()
+                    stripped_line = line.rstrip()
                     is_updated = False
                     for key, value in drive_settings.items():
-                        if line.startswith(f"{key}=") or line.startswith(f"# {key}="):
-                            if line.startswith("# "):  # Uncomment if commented out
-                                line = line[2:]
+                        if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="):
+                            # Uncomment if commented out - check the original stripped line
                             new_env_lines.append(f"{key}={value}")
                             updated.add(key)
                             is_updated = True
                             break
                     if not is_updated:
-                        new_env_lines.append(line)
+                        new_env_lines.append(stripped_line)
 
                 # Add any settings that weren't updated (they weren't in the file)
                 for key, value in drive_settings.items():

app/api/onedrive.py

@@ -242,18 +242,17 @@ async def save_onedrive_settings(
         updated = set()
         new_env_lines = []
         for line in env_lines:
-            line = line.rstrip()
+            stripped_line = line.rstrip()
             is_updated = False
             for key, value in onedrive_settings.items():
-                if line.startswith(f"{key}=") or line.startswith(f"# {key}="):
-                    if line.startswith("# "):  # Uncomment if commented out
-                        line = line[2:]
+                if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="):
+                    # Uncomment if commented out - check the original stripped line
                     new_env_lines.append(f"{key}={value}")
                     updated.add(key)
                     is_updated = True
                     break
             if not is_updated:
-                new_env_lines.append(line)
+                new_env_lines.append(stripped_line)
 
         # Add any settings that weren't updated (they weren't in the file)
         for key, value in onedrive_settings.items():

app/tasks/upload_to_ftp.py

@@ -53,11 +53,12 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
         # First attempt FTPS (FTP with TLS)
         use_tls = getattr(settings, "ftp_use_tls", True)  # Default to try TLS
         allow_plaintext = getattr(settings, "ftp_allow_plaintext", True)  # Default to allow plaintext fallback
+        used_tls = False  # Track whether we successfully used TLS
 
         if use_tls:
             try:
                 logger.info(f"Attempting FTPS connection to {settings.ftp_host}")
-                ftp = ftplib.FTP_TLS()
+                ftp = ftplib.FTP_TLS()  # noqa: S321
                 ftp.connect(host=settings.ftp_host, port=settings.ftp_port or 21)
 
                 # Login with credentials
@@ -66,6 +67,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
                 # Enable data protection - encrypt the data channel
                 ftp.prot_p()
                 logger.info("Successfully established FTPS connection with TLS")
+                used_tls = True
             except Exception as e:
                 if not allow_plaintext:
                     error_msg = f"FTPS connection failed and plaintext FTP is forbidden: {str(e)}"
@@ -74,7 +76,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
                 else:
                     logger.warning(f"FTPS connection failed, falling back to regular FTP: {str(e)}")
                     # Fall back to regular FTP - only if explicitly allowed by configuration
-                    ftp = ftplib.FTP()  # nosec B321 - Fallback to FTP intentional when configured
+                    ftp = ftplib.FTP()  # nosec B321 - Fallback to FTP intentional when configured  # noqa: S321
                     ftp.connect(host=settings.ftp_host, port=settings.ftp_port or 21)
 
                     # Login with credentials
@@ -88,7 +90,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
 
             # Directly use regular FTP if TLS is explicitly disabled
             logger.warning("Using plaintext FTP - connection is NOT encrypted!")
-            ftp = ftplib.FTP()  # nosec B321 - Plaintext FTP intentional when explicitly configured
+            ftp = ftplib.FTP()  # nosec B321 - Plaintext FTP intentional when explicitly configured  # noqa: S321
             ftp.connect(host=settings.ftp_host, port=settings.ftp_port or 21)
 
             # Login with credentials
@@ -137,7 +139,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None):
             "file": file_path,
             "ftp_host": settings.ftp_host,
             "ftp_path": f"{settings.ftp_folder}/{filename}" if settings.ftp_folder else filename,
-            "used_tls": isinstance(ftp, ftplib.FTP_TLS),
+            "used_tls": used_tls,
         }
 
     except Exception as e:

app/tasks/upload_to_sftp.py

@@ -56,7 +56,7 @@ def upload_to_sftp(self, file_path: str, file_id: int = None):
             "This should only be used in development/testing. For production, remove "
             "SFTP_DISABLE_HOST_KEY_VERIFICATION or set it to False and configure known_hosts."
         )
-        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())  # nosec B507 - Configurable, warns user
+        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())  # nosec B507 - Configurable, warns user  # noqa: S507
     else:
         # Use system known_hosts for host key verification (more secure)
         ssh.load_system_host_keys()
@@ -149,8 +149,8 @@ def upload_to_sftp(self, file_path: str, file_id: int = None):
             if "sftp" in locals():
                 sftp.close()
             ssh.close()
-        except Exception:
-            pass
+        except Exception:  # noqa: S110
+            pass  # Ignore errors during cleanup
 
         error_msg = f"Failed to upload {filename} to SFTP server: {str(e)}"
         logger.error(f"[{task_id}] {error_msg}")

app/tasks/upload_with_rclone.py

@@ -57,7 +57,7 @@ def upload_with_rclone(self, file_path: str, destination: str):
         # Ensure the remote path exists (create folders if needed)
         mkdir_cmd = ["rclone", "mkdir", "--config", rclone_config_path, destination]
 
-        subprocess.run(mkdir_cmd, check=True, capture_output=True)
+        subprocess.run(mkdir_cmd, check=True, capture_output=True)  # noqa: S603
 
         # Construct the upload command
         upload_cmd = ["rclone", "copy", "--config", rclone_config_path, file_path, destination, "--progress"]
@@ -65,14 +65,14 @@ def upload_with_rclone(self, file_path: str, destination: str):
         log_task_progress(task_id, "rclone_upload", "in_progress", f"Executing rclone copy to {destination}")
 
         # Execute the upload command
-        result = subprocess.run(upload_cmd, check=True, capture_output=True, text=True)
+        result = subprocess.run(upload_cmd, check=True, capture_output=True, text=True)  # noqa: S603
 
         # Check if upload was successful
         if result.returncode == 0:
             # Try to get a public link if possible
             try:
                 link_cmd = ["rclone", "link", "--config", rclone_config_path, f"{destination}/{filename}"]
-                link_result = subprocess.run(link_cmd, capture_output=True, text=True, check=False)
+                link_result = subprocess.run(link_cmd, capture_output=True, text=True, check=False)  # noqa: S603
                 public_url = link_result.stdout.strip() if link_result.returncode == 0 else None
             except (subprocess.SubprocessError, OSError) as e:
                 logger.warning(f"[{task_id}] Failed to get public link for {filename}: {str(e)}")
@@ -135,7 +135,7 @@ def send_to_all_rclone_destinations(self, file_path: str):
     # Get list of configured destinations from rclone
     try:
         remotes_cmd = ["rclone", "listremotes", "--config", rclone_config_path]
-        result = subprocess.run(remotes_cmd, check=True, capture_output=True, text=True)
+        result = subprocess.run(remotes_cmd, check=True, capture_output=True, text=True)  # noqa: S603
 
         if result.returncode == 0:
             # Process the list of remotes

app/utils/config_loader.py

@@ -87,21 +87,21 @@ def convert_setting_value(value: Optional[str], field_type: Any) -> Any:
         field_type = next((arg for arg in args if arg is not type(None)), str)
 
     # Convert based on type
-    if field_type == bool:
+    if field_type is bool:
         return value.lower() in ("true", "1", "yes", "y", "t")
-    elif field_type == int:
+    elif field_type is int:
         try:
             return int(value)
         except ValueError:
             logger.warning(f"Failed to convert '{value}' to int, returning 0")
             return 0
-    elif field_type == float:
+    elif field_type is float:
         try:
             return float(value)
         except ValueError:
             logger.warning(f"Failed to convert '{value}' to float, returning 0.0")
             return 0.0
-    elif field_type == list or getattr(field_type, "__origin__", None) == list:
+    elif field_type is list or getattr(field_type, "__origin__", None) is list:
         # Handle list types - assume comma-separated values
         if isinstance(value, str):
             return [item.strip() for item in value.split(",") if item.strip()]

app/views/status.py

@@ -57,8 +57,8 @@ async def status_dashboard(request: Request):
             # Try to get runtime information
             try:
                 container_info["runtime_info"] = settings.runtime_info
-            except Exception:
-                pass
+            except Exception:  # noqa: S110
+                pass  # Ignore if runtime_info not available
         else:
             container_info["is_docker"] = False
 

tests/test_check_credentials.py

@@ -2,7 +2,7 @@
 
 import json
 import os
-from unittest.mock import MagicMock, Mock, patch
+from unittest.mock import Mock, patch
 
 import pytest
 
@@ -12,9 +12,6 @@ from app.tasks.check_credentials import (
     get_failure_state,
     save_failure_state,
     sync_test_azure_connection,
-    sync_test_dropbox_token,
-    sync_test_google_drive_token,
-    sync_test_onedrive_token,
     sync_test_openai_connection,
     unwrap_decorated_function,
 )

tests/test_e2e_full_stack.py

@@ -449,7 +449,7 @@ class TestFullInfrastructure:
 
         # Create SFTP client
         ssh = paramiko.SSHClient()
-        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
+        ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())  # noqa: S507
 
         # Connect to SFTP server
         ssh.connect(

tests/test_security_headers.py

@@ -207,9 +207,10 @@ class TestSecurityHeadersMiddleware:
     @pytest.mark.asyncio
     async def test_dispatch_adds_headers_when_enabled(self):
         """Test dispatch adds security headers when enabled."""
+        from fastapi import Response
+
         from app.config import settings
         from app.middleware.security_headers import SecurityHeadersMiddleware
-        from fastapi import Response
 
         if not settings.security_headers_enabled:
             pytest.skip("Security headers disabled in configuration")
@@ -231,10 +232,10 @@ class TestSecurityHeadersMiddleware:
     @pytest.mark.asyncio
     async def test_dispatch_skips_headers_when_disabled(self):
         """Test dispatch skips headers when disabled."""
-        from app.config import settings
-        from app.middleware.security_headers import SecurityHeadersMiddleware
         from fastapi import Response
 
+        from app.middleware.security_headers import SecurityHeadersMiddleware
+
         # Create a config copy with headers disabled
         mock_config = Mock()
         mock_config.security_headers_enabled = False
@@ -254,9 +255,10 @@ class TestSecurityHeadersMiddleware:
 
     def test_add_security_headers_hsts(self):
         """Test _add_security_headers adds HSTS header."""
-        from app.middleware.security_headers import SecurityHeadersMiddleware
         from fastapi import Response
 
+        from app.middleware.security_headers import SecurityHeadersMiddleware
+
         mock_config = Mock()
         mock_config.security_headers_enabled = True
         mock_config.security_header_hsts_enabled = True
@@ -275,9 +277,10 @@ class TestSecurityHeadersMiddleware:
 
     def test_add_security_headers_csp(self):
         """Test _add_security_headers adds CSP header."""
-        from app.middleware.security_headers import SecurityHeadersMiddleware
         from fastapi import Response
 
+        from app.middleware.security_headers import SecurityHeadersMiddleware
+
         mock_config = Mock()
         mock_config.security_headers_enabled = True
         mock_config.security_header_hsts_enabled = False
@@ -296,9 +299,10 @@ class TestSecurityHeadersMiddleware:
 
     def test_add_security_headers_x_frame_options(self):
         """Test _add_security_headers adds X-Frame-Options header."""
-        from app.middleware.security_headers import SecurityHeadersMiddleware
         from fastapi import Response
 
+        from app.middleware.security_headers import SecurityHeadersMiddleware
+
         mock_config = Mock()
         mock_config.security_headers_enabled = True
         mock_config.security_header_hsts_enabled = False
@@ -317,9 +321,10 @@ class TestSecurityHeadersMiddleware:
 
     def test_add_security_headers_x_content_type_options(self):
         """Test _add_security_headers adds X-Content-Type-Options header."""
-        from app.middleware.security_headers import SecurityHeadersMiddleware
         from fastapi import Response
 
+        from app.middleware.security_headers import SecurityHeadersMiddleware
+
         mock_config = Mock()
         mock_config.security_headers_enabled = True
         mock_config.security_header_hsts_enabled = False
@@ -337,9 +342,10 @@ class TestSecurityHeadersMiddleware:
 
     def test_add_all_security_headers(self):
         """Test _add_security_headers adds all headers when all enabled."""
-        from app.middleware.security_headers import SecurityHeadersMiddleware
         from fastapi import Response
 
+        from app.middleware.security_headers import SecurityHeadersMiddleware
+
         mock_config = Mock()
         mock_config.security_headers_enabled = True
         mock_config.security_header_hsts_enabled = True

tests/test_upload_email.py

@@ -1,11 +1,8 @@
 """Tests for app/tasks/upload_to_email.py module."""
 
 import json
-import os
-import smtplib
 import socket
 from email.mime.multipart import MIMEMultipart
-from pathlib import Path
 from unittest.mock import MagicMock, Mock, mock_open, patch
 
 import pytest

tests/test_upload_ftp_additional.py

@@ -1,7 +1,7 @@
 """Additional tests for upload_to_ftp task."""
 
 import ftplib
-from unittest.mock import MagicMock, Mock, patch
+from unittest.mock import Mock, patch
 
 import pytest
 
@@ -18,14 +18,18 @@ class TestUploadToFtp:
 
         assert callable(upload_to_ftp)
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
     @patch("builtins.open", create=True)
-    def test_uploads_file_with_ftps(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls):
+    def test_uploads_file_with_ftps(
+        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename
+    ):
         """Test uploads file using FTPS (FTP with TLS)."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -47,6 +51,7 @@ class TestUploadToFtp:
         mock_ftp.login.assert_called_once()
         mock_ftp.prot_p.assert_called_once()
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
@@ -54,10 +59,11 @@ class TestUploadToFtp:
     @patch("app.tasks.upload_to_ftp.settings")
     @patch("builtins.open", create=True)
     def test_falls_back_to_plaintext_ftp(
-        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_ftp
+        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_ftp, mock_basename
     ):
         """Test falls back to plaintext FTP when FTPS fails."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -83,15 +89,17 @@ class TestUploadToFtp:
         assert result["status"] == "Completed"
         assert result["used_tls"] is False
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
     def test_raises_error_when_ftps_fails_and_plaintext_forbidden(
-        self, mock_settings, mock_exists, mock_log, mock_ftp_tls
+        self, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename
     ):
         """Test raises error when FTPS fails and plaintext is forbidden."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -110,11 +118,13 @@ class TestUploadToFtp:
         with pytest.raises(Exception, match="FTPS connection failed and plaintext FTP is forbidden"):
             upload_to_ftp(mock_self, "/tmp/test.pdf")
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
-    def test_raises_error_when_file_not_found(self, mock_exists, mock_log):
+    def test_raises_error_when_file_not_found(self, mock_exists, mock_log, mock_basename):
         """Test raises error when file not found."""
         mock_exists.return_value = False
+        mock_basename.return_value = "file.pdf"
 
         mock_self = Mock()
         mock_self.request.id = "test-task-id"
@@ -122,12 +132,14 @@ class TestUploadToFtp:
         with pytest.raises(FileNotFoundError):
             upload_to_ftp(mock_self, "/nonexistent/file.pdf")
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
-    def test_raises_error_when_ftp_host_not_configured(self, mock_settings, mock_exists, mock_log):
+    def test_raises_error_when_ftp_host_not_configured(self, mock_settings, mock_exists, mock_log, mock_basename):
         """Test raises error when FTP host not configured."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = None
 
         mock_self = Mock()
@@ -136,14 +148,18 @@ class TestUploadToFtp:
         with pytest.raises(ValueError, match="FTP host is not configured"):
             upload_to_ftp(mock_self, "/tmp/test.pdf")
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
     @patch("builtins.open", create=True)
-    def test_creates_directory_structure(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls):
+    def test_creates_directory_structure(
+        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename
+    ):
         """Test creates directory structure if it doesn't exist."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -152,7 +168,14 @@ class TestUploadToFtp:
         mock_settings.ftp_use_tls = True
 
         mock_ftp = Mock()
-        mock_ftp.cwd.side_effect = [ftplib.error_perm("No such directory"), None]
+        # First cwd fails (trying full path), then for each subfolder: fail then succeed after mkd
+        mock_ftp.cwd.side_effect = [
+            ftplib.error_perm("No such directory"),  # noqa: S321  # Initial try for full path
+            ftplib.error_perm("No such directory"),  # noqa: S321  # /uploads doesn't exist
+            None,  # /uploads now exists after mkd
+            ftplib.error_perm("No such directory"),  # noqa: S321  # /uploads/documents doesn't exist
+            None,  # /uploads/documents now exists after mkd
+        ]
         mock_ftp_tls.return_value = mock_ftp
 
         mock_self = Mock()
@@ -163,14 +186,18 @@ class TestUploadToFtp:
         assert result["status"] == "Completed"
         mock_ftp.mkd.assert_called()
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
     @patch("builtins.open", create=True)
-    def test_uses_plaintext_ftp_when_tls_disabled(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp):
+    def test_uses_plaintext_ftp_when_tls_disabled(
+        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp, mock_basename
+    ):
         """Test uses plaintext FTP when TLS is explicitly disabled."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -190,12 +217,16 @@ class TestUploadToFtp:
         assert result["status"] == "Completed"
         assert result["used_tls"] is False
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
-    def test_raises_error_when_plaintext_forbidden_and_tls_disabled(self, mock_settings, mock_exists, mock_log):
+    def test_raises_error_when_plaintext_forbidden_and_tls_disabled(
+        self, mock_settings, mock_exists, mock_log, mock_basename
+    ):
         """Test raises error when plaintext is forbidden and TLS is disabled."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_use_tls = False
@@ -207,14 +238,18 @@ class TestUploadToFtp:
         with pytest.raises(Exception, match="Plaintext FTP is forbidden"):
             upload_to_ftp(mock_self, "/tmp/test.pdf")
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
     @patch("builtins.open", create=True)
-    def test_removes_leading_slash_from_folder(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls):
+    def test_removes_leading_slash_from_folder(
+        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename
+    ):
         """Test removes leading slash from folder path."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -233,13 +268,15 @@ class TestUploadToFtp:
         # Verify cwd was called with folder without leading slash
         mock_ftp.cwd.assert_called_with("uploads")
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
-    def test_handles_directory_creation_error(self, mock_settings, mock_exists, mock_log, mock_ftp_tls):
+    def test_handles_directory_creation_error(self, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename):
         """Test handles directory creation error."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"
@@ -248,8 +285,8 @@ class TestUploadToFtp:
         mock_settings.ftp_use_tls = True
 
         mock_ftp = Mock()
-        mock_ftp.cwd.side_effect = ftplib.error_perm("Permission denied")
-        mock_ftp.mkd.side_effect = ftplib.error_perm("Cannot create directory")
+        mock_ftp.cwd.side_effect = ftplib.error_perm("Permission denied")  # noqa: S321
+        mock_ftp.mkd.side_effect = ftplib.error_perm("Cannot create directory")  # noqa: S321
         mock_ftp_tls.return_value = mock_ftp
 
         mock_self = Mock()
@@ -258,14 +295,18 @@ class TestUploadToFtp:
         with pytest.raises(Exception, match="Failed to change/create directory"):
             upload_to_ftp(mock_self, "/tmp/test.pdf")
 
+    @patch("app.tasks.upload_to_ftp.os.path.basename")
     @patch("app.tasks.upload_to_ftp.ftplib.FTP_TLS")
     @patch("app.tasks.upload_to_ftp.log_task_progress")
     @patch("app.tasks.upload_to_ftp.os.path.exists")
     @patch("app.tasks.upload_to_ftp.settings")
     @patch("builtins.open", create=True)
-    def test_returns_ftp_path_in_result(self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls):
+    def test_returns_ftp_path_in_result(
+        self, mock_open, mock_settings, mock_exists, mock_log, mock_ftp_tls, mock_basename
+    ):
         """Test returns FTP path in result."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
         mock_settings.ftp_host = "ftp.example.com"
         mock_settings.ftp_port = 21
         mock_settings.ftp_username = "user"

tests/test_upload_google_drive.py

@@ -1,7 +1,7 @@
 """Tests for app/tasks/upload_to_google_drive.py module."""
 
 import json
-from unittest.mock import MagicMock, Mock, mock_open, patch
+from unittest.mock import Mock, patch
 
 import pytest
 from google.auth.exceptions import RefreshError
@@ -219,6 +219,8 @@ class TestTruncatePropertyValue:
 class TestUploadToGoogleDriveTask:
     """Tests for upload_to_google_drive task."""
 
+    @patch("app.tasks.upload_to_google_drive.os.path.splitext")
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.get_google_drive_service")
     @patch("app.tasks.upload_to_google_drive.extract_metadata_from_file")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
@@ -226,10 +228,12 @@ class TestUploadToGoogleDriveTask:
     @patch("app.tasks.upload_to_google_drive.MediaFileUpload")
     @patch("app.tasks.upload_to_google_drive.settings")
     def test_uploads_file_successfully(
-        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service
+        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service, mock_basename, mock_splitext
     ):
         """Test uploads file to Google Drive successfully."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
+        mock_splitext.return_value = ("/tmp/test", ".pdf")
         mock_settings.google_drive_folder_id = "folder_123"
 
         mock_extract.return_value = {}
@@ -257,13 +261,15 @@ class TestUploadToGoogleDriveTask:
 
         assert result["status"] == "Completed"
         assert result["google_drive_file_id"] == "file_123"
-        assert "webViewLink" in result["google_drive_web_link"]
+        assert result["google_drive_web_link"] == "https://drive.google.com/file/d/file_123"
 
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
     @patch("app.tasks.upload_to_google_drive.os.path.exists")
-    def test_raises_error_when_file_not_found(self, mock_exists, mock_log):
+    def test_raises_error_when_file_not_found(self, mock_exists, mock_log, mock_basename):
         """Test raises error when file not found."""
         mock_exists.return_value = False
+        mock_basename.return_value = "file.pdf"
 
         mock_self = Mock()
         mock_self.request.id = "test-task-id"
@@ -271,12 +277,18 @@ class TestUploadToGoogleDriveTask:
         with pytest.raises(FileNotFoundError):
             upload_to_google_drive(mock_self, "/nonexistent/file.pdf")
 
+    @patch("app.tasks.upload_to_google_drive.os.path.splitext")
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.get_google_drive_service")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
     @patch("app.tasks.upload_to_google_drive.os.path.exists")
-    def test_raises_error_when_service_init_fails(self, mock_exists, mock_log, mock_service):
+    def test_raises_error_when_service_init_fails(
+        self, mock_exists, mock_log, mock_service, mock_basename, mock_splitext
+    ):
         """Test raises error when service initialization fails."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
+        mock_splitext.return_value = ("/tmp/test", ".pdf")
         mock_service.return_value = None
 
         mock_self = Mock()
@@ -285,6 +297,8 @@ class TestUploadToGoogleDriveTask:
         with pytest.raises(Exception, match="Failed to initialize Google Drive service"):
             upload_to_google_drive(mock_self, "/tmp/test.pdf")
 
+    @patch("app.tasks.upload_to_google_drive.os.path.splitext")
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.get_google_drive_service")
     @patch("app.tasks.upload_to_google_drive.extract_metadata_from_file")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
@@ -292,10 +306,12 @@ class TestUploadToGoogleDriveTask:
     @patch("app.tasks.upload_to_google_drive.MediaFileUpload")
     @patch("app.tasks.upload_to_google_drive.settings")
     def test_includes_metadata_in_upload(
-        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service
+        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service, mock_basename, mock_splitext
     ):
         """Test includes metadata in upload."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
+        mock_splitext.return_value = ("/tmp/test", ".pdf")
         mock_settings.google_drive_folder_id = None
 
         metadata = {"document_type": "invoice", "amount": "100.00", "date": "2024-01-01"}
@@ -320,10 +336,12 @@ class TestUploadToGoogleDriveTask:
         mock_self = Mock()
         mock_self.request.id = "test-task-id"
 
-        result = upload_to_google_drive(mock_self, "/tmp/test.pdf", include_metadata=True)
+        result = upload_to_google_drive(mock_self, "/tmp/test.pdf", True)
 
         assert result["metadata_included"] is True
 
+    @patch("app.tasks.upload_to_google_drive.os.path.splitext")
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.get_google_drive_service")
     @patch("app.tasks.upload_to_google_drive.extract_metadata_from_file")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
@@ -331,10 +349,12 @@ class TestUploadToGoogleDriveTask:
     @patch("app.tasks.upload_to_google_drive.MediaFileUpload")
     @patch("app.tasks.upload_to_google_drive.settings")
     def test_skips_nested_metadata_objects(
-        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service
+        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service, mock_basename, mock_splitext
     ):
         """Test skips nested objects in metadata."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
+        mock_splitext.return_value = ("/tmp/test", ".pdf")
         mock_settings.google_drive_folder_id = None
 
         metadata = {"simple_field": "value", "nested_object": {"key": "value"}, "nested_list": [1, 2, 3]}
@@ -358,7 +378,7 @@ class TestUploadToGoogleDriveTask:
         mock_self = Mock()
         mock_self.request.id = "test-task-id"
 
-        result = upload_to_google_drive(mock_self, "/tmp/test.pdf", include_metadata=True)
+        result = upload_to_google_drive(mock_self, "/tmp/test.pdf", True)
 
         # Verify the create call was made
         mock_files.create.assert_called_once()
@@ -370,14 +390,20 @@ class TestUploadToGoogleDriveTask:
             assert "nested_object" not in file_metadata["properties"]
             assert "nested_list" not in file_metadata["properties"]
 
+    @patch("app.tasks.upload_to_google_drive.os.path.splitext")
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.get_google_drive_service")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
     @patch("app.tasks.upload_to_google_drive.os.path.exists")
     @patch("app.tasks.upload_to_google_drive.MediaFileUpload")
     @patch("app.tasks.upload_to_google_drive.settings")
-    def test_handles_upload_exception(self, mock_settings, mock_media, mock_exists, mock_log, mock_service):
+    def test_handles_upload_exception(
+        self, mock_settings, mock_media, mock_exists, mock_log, mock_service, mock_basename, mock_splitext
+    ):
         """Test handles upload exception."""
         mock_exists.return_value = True
+        mock_basename.return_value = "test.pdf"
+        mock_splitext.return_value = ("/tmp/test", ".pdf")
         mock_settings.google_drive_folder_id = None
 
         mock_drive_service = Mock()
@@ -392,6 +418,8 @@ class TestUploadToGoogleDriveTask:
         with pytest.raises(Exception, match="Failed to upload"):
             upload_to_google_drive(mock_self, "/tmp/test.pdf")
 
+    @patch("app.tasks.upload_to_google_drive.os.path.splitext")
+    @patch("app.tasks.upload_to_google_drive.os.path.basename")
     @patch("app.tasks.upload_to_google_drive.get_google_drive_service")
     @patch("app.tasks.upload_to_google_drive.extract_metadata_from_file")
     @patch("app.tasks.upload_to_google_drive.log_task_progress")
@@ -399,9 +427,11 @@ class TestUploadToGoogleDriveTask:
     @patch("app.tasks.upload_to_google_drive.MediaFileUpload")
     @patch("app.tasks.upload_to_google_drive.settings")
     def test_sets_parent_folder_when_configured(
-        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service
+        self, mock_settings, mock_media, mock_exists, mock_log, mock_extract, mock_service, mock_basename, mock_splitext
     ):
         """Test sets parent folder when configured."""
+        mock_basename.return_value = "test.pdf"
+        mock_splitext.return_value = ("/tmp/test", ".pdf")
         mock_exists.return_value = True
         mock_settings.google_drive_folder_id = "parent_folder_123"
         mock_extract.return_value = {}

tests/test_views_status.py

@@ -240,7 +240,6 @@ class TestContainerInfoDetection:
     @patch("builtins.open", new_callable=mock_open, read_data="12:docker:/abc123456789")
     def test_extracts_container_id(self, mock_file, mock_exists):
         """Test extracts container ID from cgroup."""
-        from app.views.status import status_dashboard
 
         mock_exists.return_value = True
 
@@ -250,7 +249,6 @@ class TestContainerInfoDetection:
     @patch("app.views.status.os.path.exists")
     def test_handles_missing_cgroup_file(self, mock_exists):
         """Test handles missing cgroup file gracefully."""
-        from app.views.status import status_dashboard
 
         mock_exists.side_effect = [True, False]  # Docker env exists, but cgroup doesn't
 
@@ -259,7 +257,6 @@ class TestContainerInfoDetection:
     @patch("app.views.status.settings")
     def test_includes_runtime_info_when_available(self, mock_settings):
         """Test includes runtime info when available."""
-        from app.views.status import status_dashboard
 
         mock_settings.runtime_info = "Python 3.11.5 on Linux"