From 4897cb6655b32bdf9053312cf24d791c9cc640c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 11:25:30 +0000 Subject: [PATCH] fix: apply Phase 1-2 correctness and constant extraction from PR #273 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace datetime.utcnow() with datetime.now(timezone.utc) in 4 files - Extract duplicate literals to constants in 5 files - models.py: "files.id" → _FILES_ID_FK - upload_to_email.py: "logo.png" → _LOGO_FILENAME - general.py: "%B %d, %Y" → _DATE_DISPLAY_FORMAT - files.py: "File not found" → _FILE_NOT_FOUND - upload_to_google_drive.py: Google token URL → _GOOGLE_TOKEN_URL Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/diagnostic.py | 2 +- app/models.py | 9 ++++++--- app/tasks/monitor_stalled_steps.py | 6 +++--- app/tasks/upload_to_email.py | 11 +++++++---- app/tasks/upload_to_google_drive.py | 5 ++++- app/utils/logging.py | 4 ++-- app/utils/step_timeout.py | 4 ++-- app/views/files.py | 11 +++++++---- app/views/general.py | 9 ++++++--- 9 files changed, 38 insertions(+), 23 deletions(-) diff --git a/app/api/diagnostic.py b/app/api/diagnostic.py index 7fffbcbd..0ccd1fe4 100644 --- a/app/api/diagnostic.py +++ b/app/api/diagnostic.py @@ -64,7 +64,7 @@ async def test_notification(request: Request): # Add request_time to request.state import datetime - request.state.request_time = datetime.datetime.utcnow().isoformat() + request.state.request_time = datetime.datetime.now(datetime.timezone.utc).isoformat() """ Send a test notification through all configured notification channels """ diff --git a/app/models.py b/app/models.py index ad3b1e59..848f71bf 100644 --- a/app/models.py +++ b/app/models.py @@ -4,6 +4,9 @@ from sqlalchemy import Boolean, Column, DateTime, ForeignKey, Integer, String, T from app.database import Base +# Foreign key constants +_FILES_ID_FK = "files.id" + class DocumentMetadata(Base): __tablename__ = "documents" @@ -50,7 +53,7 @@ class FileRecord(Base): is_duplicate = Column(Boolean, default=False, nullable=False, index=True) # If this is a duplicate, record the ID of the original file for reference - duplicate_of_id = Column(Integer, ForeignKey("files.id"), nullable=True) + duplicate_of_id = Column(Integer, ForeignKey(_FILES_ID_FK), nullable=True) # Timestamp when we inserted this record created_at = Column(DateTime(timezone=True), server_default=func.now()) @@ -65,7 +68,7 @@ class FileProcessingStep(Base): __tablename__ = "file_processing_steps" id = Column(Integer, primary_key=True, index=True) - file_id = Column(Integer, ForeignKey("files.id"), nullable=False, index=True) + file_id = Column(Integer, ForeignKey(_FILES_ID_FK), nullable=False, index=True) step_name = Column(String, nullable=False, index=True) # e.g., "hash_file", "upload_to_dropbox" status = Column(String, nullable=False) # "pending", "in_progress", "success", "failure", "skipped" started_at = Column(DateTime(timezone=True), nullable=True) # When step started @@ -80,7 +83,7 @@ class FileProcessingStep(Base): class ProcessingLog(Base): __tablename__ = "processing_logs" id = Column(Integer, primary_key=True, index=True) - file_id = Column(Integer, ForeignKey("files.id"), nullable=True) # Optional file association + file_id = Column(Integer, ForeignKey(_FILES_ID_FK), nullable=True) # Optional file association task_id = Column(String, index=True) # Celery task ID step_name = Column(String) # e.g., "OCR", "convert_to_pdf", "upload_s3" status = Column(String) # "pending", "in_progress", "success", "failure" diff --git a/app/tasks/monitor_stalled_steps.py b/app/tasks/monitor_stalled_steps.py index 5fbc8dba..e970bd7a 100644 --- a/app/tasks/monitor_stalled_steps.py +++ b/app/tasks/monitor_stalled_steps.py @@ -6,7 +6,7 @@ that have been stuck in "in_progress" state for too long and mark them as failed """ import logging -from datetime import datetime +from datetime import datetime, timezone from app.celery_app import celery from app.database import SessionLocal @@ -37,12 +37,12 @@ def monitor_stalled_steps(): if stalled_count > 0: logger.warning( - f"[{datetime.utcnow().isoformat()}] " + f"[{datetime.now(timezone.utc).isoformat()}] " f"Recovered {stalled_count} stalled step(s). " f"Marked as failed due to timeout." ) else: - logger.debug(f"[{datetime.utcnow().isoformat()}] No stalled steps found.") + logger.debug(f"[{datetime.now(timezone.utc).isoformat()}] No stalled steps found.") return {"recovered": stalled_count} diff --git a/app/tasks/upload_to_email.py b/app/tasks/upload_to_email.py index 9c29d7e8..32e547a5 100644 --- a/app/tasks/upload_to_email.py +++ b/app/tasks/upload_to_email.py @@ -20,6 +20,9 @@ from app.utils import log_task_progress logger = logging.getLogger(__name__) +# Constants +_LOGO_FILENAME = "logo.png" + def get_email_template(template_name="default.html"): """ @@ -87,16 +90,16 @@ def attach_logo(msg): """Attach the DocuElevate logo to the email with proper Content-ID.""" try: # Try to find logo in workdir first (for customization) - custom_logo_path = os.path.join(settings.workdir, "templates", "email", "logo.png") + custom_logo_path = os.path.join(settings.workdir, "templates", "email", _LOGO_FILENAME) if os.path.exists(custom_logo_path): logo_path = custom_logo_path else: # Use built-in logo app_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - logo_path = os.path.join(app_dir, "static", "logo.png") + logo_path = os.path.join(app_dir, "static", _LOGO_FILENAME) # Fallback to logo in frontend/static if app/static doesn't exist if not os.path.exists(logo_path): - logo_path = os.path.join(app_dir, "..", "frontend", "static", "logo.png") + logo_path = os.path.join(app_dir, "..", "frontend", "static", _LOGO_FILENAME) if os.path.exists(logo_path): with open(logo_path, "rb") as img: @@ -106,7 +109,7 @@ def attach_logo(msg): mimetype = "image/svg+xml" if logo_path.endswith(".svg") else "image/png" logo_attach = MIMEImage(logo_data, mimetype) logo_attach.add_header("Content-ID", "") - logo_attach.add_header("Content-Disposition", "inline", filename="logo.png") + logo_attach.add_header("Content-Disposition", "inline", filename=_LOGO_FILENAME) msg.attach(logo_attach) logger.info(f"Logo attached from {logo_path}") return True diff --git a/app/tasks/upload_to_google_drive.py b/app/tasks/upload_to_google_drive.py index 32ed95b3..ace6e9e9 100644 --- a/app/tasks/upload_to_google_drive.py +++ b/app/tasks/upload_to_google_drive.py @@ -20,6 +20,9 @@ from app.utils import log_task_progress logger = logging.getLogger(__name__) +# Google OAuth constants +_GOOGLE_TOKEN_URL = "https://oauth2.googleapis.com/token" + def get_drive_service_oauth(): """ @@ -40,7 +43,7 @@ def get_drive_service_oauth(): credentials = OAuthCredentials( None, # No access token initially, will be refreshed refresh_token=settings.google_drive_refresh_token, - token_uri="https://oauth2.googleapis.com/token", + token_uri=_GOOGLE_TOKEN_URL, client_id=settings.google_drive_client_id, client_secret=settings.google_drive_client_secret, # Use only drive.file scope diff --git a/app/utils/logging.py b/app/utils/logging.py index 0c555891..b7ef29da 100644 --- a/app/utils/logging.py +++ b/app/utils/logging.py @@ -1,7 +1,7 @@ import logging import threading from collections import defaultdict -from datetime import datetime +from datetime import datetime, timezone from app.database import SessionLocal from app.models import FileProcessingStep, ProcessingLog @@ -106,7 +106,7 @@ def log_task_progress(task_id, step_name, status, message=None, file_id=None, de .first() ) - now = datetime.utcnow() + now = datetime.now(timezone.utc) if not step_record: # Create new step record diff --git a/app/utils/step_timeout.py b/app/utils/step_timeout.py index 35961b44..363add72 100644 --- a/app/utils/step_timeout.py +++ b/app/utils/step_timeout.py @@ -6,7 +6,7 @@ This prevents files from getting stuck in "pending" state when processing crashe """ import logging -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from typing import Optional from sqlalchemy.orm import Session @@ -54,7 +54,7 @@ def mark_stalled_steps_as_failed( if timeout_seconds is None: timeout_seconds = get_step_timeout() - now = datetime.utcnow() + now = datetime.now(timezone.utc) cutoff_time = now - timedelta(seconds=timeout_seconds) # Query for stalled steps diff --git a/app/views/files.py b/app/views/files.py index 624a36b4..5d7d0c9f 100644 --- a/app/views/files.py +++ b/app/views/files.py @@ -13,6 +13,9 @@ from app.views.base import APIRouter, get_db, logger, require_login, templates router = APIRouter() +# Error message constants +_FILE_NOT_FOUND = "File not found" + @router.get("/files") @require_login @@ -433,7 +436,7 @@ def preview_original_file(request: Request, file_id: int, db: Session = Depends( file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() if not file_record: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=_FILE_NOT_FOUND) if not file_record.original_file_path or not os.path.exists(file_record.original_file_path): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Original file not found on disk") @@ -460,7 +463,7 @@ def preview_processed_file(request: Request, file_id: int, db: Session = Depends file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() if not file_record: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=_FILE_NOT_FOUND) if not file_record.processed_file_path or not os.path.exists(file_record.processed_file_path): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Processed file not found on disk") @@ -487,7 +490,7 @@ def get_original_text(request: Request, file_id: int, db: Session = Depends(get_ file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() if not file_record: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=_FILE_NOT_FOUND) if not file_record.original_file_path or not os.path.exists(file_record.original_file_path): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Original file not found on disk") @@ -527,7 +530,7 @@ def get_processed_text(request: Request, file_id: int, db: Session = Depends(get file_record = db.query(FileRecord).filter(FileRecord.id == file_id).first() if not file_record: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="File not found") + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=_FILE_NOT_FOUND) if not file_record.processed_file_path or not os.path.exists(file_record.processed_file_path): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Processed file not found on disk") diff --git a/app/views/general.py b/app/views/general.py index e22d86e7..26c45580 100644 --- a/app/views/general.py +++ b/app/views/general.py @@ -14,6 +14,9 @@ from app.views.base import APIRouter, get_db, logger, require_login, templates router = APIRouter() +# Date format constant +_DATE_DISPLAY_FORMAT = "%B %d, %Y" + @router.get("/", include_in_schema=False) async def serve_index(request: Request, db: Session = Depends(get_db)): @@ -82,7 +85,7 @@ async def serve_about(request: Request): async def serve_privacy(request: Request): """Serve the privacy policy page.""" # Pass the current date for the "Last Updated" field - current_date = date.today().strftime("%B %d, %Y") + current_date = date.today().strftime(_DATE_DISPLAY_FORMAT) return templates.TemplateResponse("privacy.html", {"request": request, "current_date": current_date}) @@ -148,12 +151,12 @@ Please visit http://www.apache.org/licenses/LICENSE-2.0 for the complete license @router.get("/cookies", include_in_schema=False) async def serve_cookies(request: Request): """Serve the cookie policy page.""" - current_date = date.today().strftime("%B %d, %Y") + current_date = date.today().strftime(_DATE_DISPLAY_FORMAT) return templates.TemplateResponse("cookies.html", {"request": request, "current_date": current_date}) @router.get("/terms", include_in_schema=False) async def serve_terms(request: Request): """Serve the terms of service page.""" - current_date = date.today().strftime("%B %d, %Y") + current_date = date.today().strftime(_DATE_DISPLAY_FORMAT) return templates.TemplateResponse("terms.html", {"request": request, "current_date": current_date})