fix: apply Phase 1-2 correctness and constant extraction from PR #273
- 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>
This commit is contained in:
@@ -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
|
||||
"""
|
||||
|
||||
+6
-3
@@ -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"
|
||||
|
||||
@@ -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}
|
||||
|
||||
|
||||
@@ -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>")
|
||||
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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
+7
-4
@@ -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")
|
||||
|
||||
@@ -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})
|
||||
|
||||
Reference in New Issue
Block a user