chore: simplify and fix naming for save settings endpoints

- Renamed `save_dropbox_settings` inside `app/api/google_drive.py` to `save_google_drive_settings` to fix a copy-paste naming error.
- Extracted duplicate `.env` file updating logic from `app/api/google_drive.py`, `app/api/onedrive.py`, and `app/api/dropbox.py` into a new reusable helper function `update_env_file` inside `app/utils/settings_service.py`.
- Refactored the three API endpoints to use the new helper function, significantly reducing complexity and code duplication.
- Updated relevant test files (`tests/test_api_google_drive_final.py`) to reflect the new function name.

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

app/api/dropbox.py

@@ -14,7 +14,7 @@ from app.auth import require_login
 from app.config import settings
 from app.database import get_db
 from app.utils.oauth_helper import exchange_oauth_token
-from app.utils.settings_service import save_setting_to_db
+from app.utils.settings_service import save_setting_to_db, update_env_file
 from app.utils.settings_sync import notify_settings_updated
 
 # Set up logging
@@ -249,44 +249,16 @@ async def save_dropbox_settings(
         # Best-effort .env file write
         try:
             env_path = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(__file__))), ".env")
-            if not os.path.exists(env_path):
+            dropbox_settings = {"DROPBOX_REFRESH_TOKEN": refresh_token}
-                logger.warning(f".env file not found at {env_path}, skipping file write")
+            if app_key:
-            else:
+                dropbox_settings["DROPBOX_APP_KEY"] = app_key
-                logger.info(f"Updating Dropbox settings in {env_path}")
+            if app_secret:
+                dropbox_settings["DROPBOX_APP_SECRET"] = app_secret
+            if folder_path:
+                dropbox_settings["DROPBOX_FOLDER"] = folder_path
 
-                with open(env_path, "r") as f:
+            if not update_env_file(env_path, dropbox_settings):
-                    env_lines = f.readlines()
+                logger.info("Continuing with in-memory update despite .env file update failure or skip")
-
-                dropbox_settings = {"DROPBOX_REFRESH_TOKEN": refresh_token}
-                if app_key:
-                    dropbox_settings["DROPBOX_APP_KEY"] = app_key
-                if app_secret:
-                    dropbox_settings["DROPBOX_APP_SECRET"] = app_secret
-                if folder_path:
-                    dropbox_settings["DROPBOX_FOLDER"] = folder_path
-
-                updated = set()
-                new_env_lines = []
-                for line in env_lines:
-                    stripped_line = line.rstrip()
-                    is_updated = False
-                    for key, value in dropbox_settings.items():
-                        if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="):
-                            new_env_lines.append(f"{key}={value}")
-                            updated.add(key)
-                            is_updated = True
-                            break
-                    if not is_updated:
-                        new_env_lines.append(stripped_line)
-
-                for key, value in dropbox_settings.items():
-                    if key not in updated:
-                        new_env_lines.append(f"{key}={value}")
-
-                with open(env_path, "w") as f:
-                    f.write("\n".join(new_env_lines) + "\n")
-
-                logger.info("Successfully updated Dropbox settings in .env file")
         except Exception as env_err:
             logger.warning(f"Failed to write .env file (non-fatal): {env_err}")
 

app/api/google_drive.py

@@ -14,7 +14,7 @@ from app.auth import require_login
 from app.config import settings
 from app.database import get_db
 from app.utils.oauth_helper import exchange_oauth_token
-from app.utils.settings_service import save_setting_to_db
+from app.utils.settings_service import save_setting_to_db, update_env_file
 from app.utils.settings_sync import notify_settings_updated
 
 # Set up logging
@@ -363,7 +363,7 @@ def format_time_remaining(time_delta):
 
 @router.post("/google-drive/save-settings")
 @require_login
-async def save_dropbox_settings(
+async def save_google_drive_settings(
     request: Request,
     refresh_token: Annotated[str, Form(...)],
     client_id: Annotated[Optional[str], Form()] = None,
@@ -404,46 +404,8 @@ async def save_dropbox_settings(
             drive_settings["GOOGLE_DRIVE_FOLDER_ID"] = folder_id
 
         # Try to update the .env file, but don't fail if it doesn't exist (for Docker containers)
-        if os.path.exists(env_path):
+        if not update_env_file(env_path, drive_settings):
-            try:
+            logger.info("Continuing with in-memory update despite .env file update failure or skip")
-                logger.info(f"Updating Google Drive settings in {env_path}")
-
-                # Read the current .env file
-                with open(env_path, "r") as f:
-                    env_lines = f.readlines()
-
-                # Process each line and update or add settings
-                updated = set()
-                new_env_lines = []
-                for line in env_lines:
-                    stripped_line = line.rstrip()
-                    is_updated = False
-                    for key, value in drive_settings.items():
-                        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(stripped_line)
-
-                # Add any settings that weren't updated (they weren't in the file)
-                for key, value in drive_settings.items():
-                    if key not in updated:
-                        new_env_lines.append(f"{key}={value}")
-
-                # Write the updated .env file
-                with open(env_path, "w") as f:
-                    f.write("\n".join(new_env_lines) + "\n")
-
-                logger.info("Successfully updated Google Drive settings in .env file")
-            except Exception as e:
-                logger.warning(f"Failed to update .env file: {str(e)}, but will continue with in-memory update")
-        else:
-            logger.warning(
-                f".env file not found at {env_path}, skipping file update but continuing with in-memory update"
-            )
 
         # Update the settings in memory (this always happens)
         if refresh_token:

app/api/onedrive.py

@@ -15,7 +15,7 @@ from app.auth import require_login
 from app.config import settings
 from app.database import get_db
 from app.utils.oauth_helper import exchange_oauth_token
-from app.utils.settings_service import save_setting_to_db
+from app.utils.settings_service import save_setting_to_db, update_env_file
 from app.utils.settings_sync import notify_settings_updated
 
 # Set up logging
@@ -249,46 +249,18 @@ async def save_onedrive_settings(
         # Best-effort .env file write
         try:
             env_path = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname(__file__))), ".env")
-            if not os.path.exists(env_path):
+            onedrive_settings = {"ONEDRIVE_REFRESH_TOKEN": refresh_token}
-                logger.warning(f".env file not found at {env_path}, skipping file write")
+            if client_id:
-            else:
+                onedrive_settings["ONEDRIVE_CLIENT_ID"] = client_id
-                logger.info(f"Updating OneDrive settings in {env_path}")
+            if client_secret:
+                onedrive_settings["ONEDRIVE_CLIENT_SECRET"] = client_secret
+            if tenant_id:
+                onedrive_settings["ONEDRIVE_TENANT_ID"] = tenant_id
+            if folder_path:
+                onedrive_settings["ONEDRIVE_FOLDER_PATH"] = folder_path
 
-                with open(env_path, "r") as f:
+            if not update_env_file(env_path, onedrive_settings):
-                    env_lines = f.readlines()
+                logger.info("Continuing with in-memory update despite .env file update failure or skip")
-
-                onedrive_settings = {"ONEDRIVE_REFRESH_TOKEN": refresh_token}
-                if client_id:
-                    onedrive_settings["ONEDRIVE_CLIENT_ID"] = client_id
-                if client_secret:
-                    onedrive_settings["ONEDRIVE_CLIENT_SECRET"] = client_secret
-                if tenant_id:
-                    onedrive_settings["ONEDRIVE_TENANT_ID"] = tenant_id
-                if folder_path:
-                    onedrive_settings["ONEDRIVE_FOLDER_PATH"] = folder_path
-
-                updated = set()
-                new_env_lines = []
-                for line in env_lines:
-                    stripped_line = line.rstrip()
-                    is_updated = False
-                    for key, value in onedrive_settings.items():
-                        if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="):
-                            new_env_lines.append(f"{key}={value}")
-                            updated.add(key)
-                            is_updated = True
-                            break
-                    if not is_updated:
-                        new_env_lines.append(stripped_line)
-
-                for key, value in onedrive_settings.items():
-                    if key not in updated:
-                        new_env_lines.append(f"{key}={value}")
-
-                with open(env_path, "w") as f:
-                    f.write("\n".join(new_env_lines) + "\n")
-
-                logger.info("Successfully updated OneDrive settings in .env file")
         except Exception as env_err:
             logger.warning(f"Failed to write .env file (non-fatal): {env_err}")
 

app/utils/settings_service.py

@@ -8,6 +8,7 @@ This module provides functionality to:
 """
 
 import logging
+import os
 from typing import Any, Dict, List, Optional, Tuple
 
 from sqlalchemy.exc import SQLAlchemyError
@@ -3120,3 +3121,60 @@ def get_settings_for_export(db: Session, source: str = "db") -> Dict[str, str]:
         # DB only
         db_settings = get_all_settings_from_db(db)
         return {k.upper(): v for k, v in sorted(db_settings.items()) if v is not None}
+
+
+def update_env_file(env_path: str, settings_to_update: Dict[str, str]) -> bool:
+    """
+    Update an .env file with new settings.
+
+    Reads the file, updates matching settings (even if commented),
+    appends any that weren't found, and writes the result back.
+
+    Args:
+        env_path: Path to the .env file
+        settings_to_update: Dictionary mapping setting names (e.g. 'GOOGLE_DRIVE_USE_OAUTH') to string values
+
+    Returns:
+        True if the file was successfully updated, False otherwise (e.g. file not found or write error)
+    """
+    if not os.path.exists(env_path):
+        logger.warning(f".env file not found at {env_path}, skipping file update")
+        return False
+
+    try:
+        logger.info(f"Updating settings in {env_path}")
+
+        # Read the current .env file
+        with open(env_path, "r") as f:
+            env_lines = f.readlines()
+
+        # Process each line and update or add settings
+        updated = set()
+        new_env_lines = []
+        for line in env_lines:
+            stripped_line = line.rstrip()
+            is_updated = False
+            for key, value in settings_to_update.items():
+                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(stripped_line)
+
+        # Add any settings that weren't updated (they weren't in the file)
+        for key, value in settings_to_update.items():
+            if key not in updated:
+                new_env_lines.append(f"{key}={value}")
+
+        # Write the updated .env file
+        with open(env_path, "w") as f:
+            f.write("\n".join(new_env_lines) + "\n")
+
+        logger.info(f"Successfully updated settings in {env_path}")
+        return True
+    except Exception as e:
+        logger.warning(f"Failed to update {env_path}: {str(e)}")
+        return False

tests/test_api_google_drive_final.py

@@ -7,9 +7,9 @@ Targets the remaining uncovered branches from the 97.03% baseline:
   - 214     : test_google_drive_token — generic connection error (not token-related)
   - 302->306: get_google_drive_token_info — credentials already valid (no refresh)
   - 307->318: get_google_drive_token_info — credentials have no expiry
-  - 395->397: save_dropbox_settings — refresh_token falsy inside use_oauth block
+  - 395->397: save_google_drive_settings — refresh_token falsy inside use_oauth block
-  - 449->451: save_dropbox_settings — refresh_token falsy in in-memory update
+  - 449->451: save_google_drive_settings — refresh_token falsy in in-memory update
-  - 468->470: save_dropbox_settings — folder_id falsy in db-persist block
+  - 468->470: save_google_drive_settings — folder_id falsy in db-persist block
 """
 
 from datetime import datetime, timedelta
@@ -152,11 +152,7 @@ class TestGetTokenInfoCredentialsBranches:
 
 @pytest.mark.unit
 class TestSaveGoogleDriveSettingsFalsyFields:
-    """Cover branches 395->397, 449->451, 468->470 in save_dropbox_settings.
+    """Cover branches 395->397, 449->451, 468->470 in save_google_drive_settings."""
-
-    Note: the Google Drive save endpoint is named save_dropbox_settings in the
-    source (app/api/google_drive.py) due to an existing naming inconsistency.
-    """
 
     @patch("app.api.google_drive.settings")
     @patch("os.path.exists", return_value=False)
@@ -167,7 +163,7 @@ class TestSaveGoogleDriveSettingsFalsyFields:
 
         from starlette.requests import Request as StarletteRequest
 
-        from app.api.google_drive import save_dropbox_settings
+        from app.api.google_drive import save_google_drive_settings
 
         mock_request = MagicMock(spec=StarletteRequest)
         mock_request.session = {}
@@ -175,7 +171,7 @@ class TestSaveGoogleDriveSettingsFalsyFields:
 
         with patch("app.api.google_drive.save_setting_to_db"):
             with patch("app.api.google_drive.notify_settings_updated"):
-                result = await save_dropbox_settings(
+                result = await save_google_drive_settings(
                     request=mock_request,
                     refresh_token="",  # falsy → branches 395->397 and 449->451
                     client_id="cid",