refactor(services): one periodic-task shape, one APScheduler job shape, one token hash, one summary rule — the services pass of the shape audit (#2830, milestone 296)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 12s
CI & Build / integration (push) Successful in 24s
CI & Build / Python tests (push) Successful in 55s
CI & Build / Build & push image (push) Successful in 24s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / TypeScript typecheck (push) Successful in 12s
CI & Build / integration (push) Successful in 24s
CI & Build / Python tests (push) Successful in 55s
CI & Build / Build & push image (push) Successful in 24s
- background.start_periodic(interval, work, label=) replaces the three hand-rolled while-True/sleep/try loops in logging, auth and notifications. - services/scheduler.ScheduledJob replaces the four private BackgroundScheduler copies in recurrence/version_pinning/trash/db_maintenance schedulers; public start_/stop_/reschedule_ surfaces unchanged. - api_keys.hash_token is the one sha256 helper; auth.py used to inline it 5x. - auth.is_registration_open reads via settings.get_admin_setting; notification prefs read via settings.get_setting; _fire_share_email uses _get_user_email. - projects.get_project_summary / milestones.get_project_milestone_summary are now the one-id view of their batch siblings instead of a second copy of the queries; sharing.best_permission_by (was _deduplicate_by_permission) is the one rank-dedup, now also used by list_projects_for_user. - backup: the row builders for every section both exporters carry are named functions, so a column added to one export cannot silently miss the other. - iso() from models.base replaces the attr.isoformat()-if-attr-else-None idiom and db_maintenance._iso across services; backup keeps its explicit shape. - trash.py hoists the sql_delete/timedelta imports it re-imported per function. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,17 +3,20 @@
|
||||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
from datetime import date, datetime, time, timezone
|
||||
from datetime import date, datetime, time, timedelta, timezone
|
||||
|
||||
from sqlalchemy import func, select, text
|
||||
from sqlalchemy import delete as sa_delete, func, select, text
|
||||
from sqlalchemy import update as sa_update
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.app_log import AppLog
|
||||
from scribe.models.note import Note
|
||||
from scribe.models.setting import Setting
|
||||
from scribe.models.notification import Notification
|
||||
from scribe.models.user import User
|
||||
from scribe.models.base import iso
|
||||
from scribe.services.email import _email_html, is_smtp_configured, send_email
|
||||
from scribe.services.logging import log_audit
|
||||
from scribe.services.settings import get_setting
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -29,13 +32,7 @@ SECURITY_EVENT_LABELS = {
|
||||
|
||||
async def _get_user_notification_pref(user_id: int, key: str) -> bool:
|
||||
"""Check if a user has a notification preference enabled (default True)."""
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(Setting).where(Setting.user_id == user_id, Setting.key == key)
|
||||
)
|
||||
setting = result.scalar_one_or_none()
|
||||
# Default to enabled
|
||||
return setting.value != "false" if setting else True
|
||||
return await get_setting(user_id, key, "true") != "false"
|
||||
|
||||
|
||||
async def _get_user_email(user_id: int) -> str | None:
|
||||
@@ -222,7 +219,7 @@ async def check_due_tasks() -> None:
|
||||
for task in user_tasks:
|
||||
overdue = task.due_date < today if task.due_date else False
|
||||
date_color = "#ef4444" if overdue else "#374151"
|
||||
date_label = f'<span style="color: {date_color};">{task.due_date.isoformat()}</span>' if task.due_date else ""
|
||||
date_label = f'<span style="color: {date_color};">{iso(task.due_date)}</span>' if task.due_date else ""
|
||||
overdue_badge = ' <span style="color:#ef4444;font-weight:600;font-size:11px;">(overdue)</span>' if overdue else ""
|
||||
task_rows += (
|
||||
f'<tr>'
|
||||
@@ -261,13 +258,10 @@ _NOTIFICATION_RETENTION_DAYS = 30
|
||||
|
||||
async def purge_old_read_notifications(retention_days: int = _NOTIFICATION_RETENTION_DAYS) -> int:
|
||||
"""Delete already-read in-app notifications older than retention_days."""
|
||||
from datetime import timedelta
|
||||
from sqlalchemy import delete
|
||||
from scribe.models.notification import Notification
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(days=retention_days)
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
delete(Notification).where(
|
||||
sa_delete(Notification).where(
|
||||
Notification.read_at.isnot(None),
|
||||
Notification.read_at < cutoff,
|
||||
)
|
||||
@@ -276,25 +270,21 @@ async def purge_old_read_notifications(retention_days: int = _NOTIFICATION_RETEN
|
||||
return result.rowcount or 0
|
||||
|
||||
|
||||
async def _notification_loop() -> None:
|
||||
while True:
|
||||
await asyncio.sleep(3600) # hourly
|
||||
try:
|
||||
await check_due_tasks()
|
||||
except Exception:
|
||||
logger.exception("Error in notification loop")
|
||||
try:
|
||||
removed = await purge_old_read_notifications()
|
||||
if removed:
|
||||
logger.info("Notification retention: deleted %d read notification(s)", removed)
|
||||
except Exception:
|
||||
logger.exception("Error in notification retention cleanup")
|
||||
async def _notification_tick() -> None:
|
||||
try:
|
||||
await check_due_tasks()
|
||||
except Exception:
|
||||
logger.exception("Error in notification loop")
|
||||
removed = await purge_old_read_notifications()
|
||||
if removed:
|
||||
logger.info("Notification retention: deleted %d read notification(s)", removed)
|
||||
|
||||
|
||||
def start_notification_loop() -> None:
|
||||
global _notification_task
|
||||
if _notification_task is None or _notification_task.done():
|
||||
_notification_task = asyncio.create_task(_notification_loop())
|
||||
from scribe.services.background import start_periodic
|
||||
_notification_task = start_periodic(3600, _notification_tick, label="notifications") # hourly
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -303,7 +293,6 @@ def start_notification_loop() -> None:
|
||||
|
||||
async def create_in_app_notification(user_id: int, notif_type: str, payload: dict):
|
||||
"""Create an in-app Notification record."""
|
||||
from scribe.models.notification import Notification
|
||||
async with async_session() as session:
|
||||
n = Notification(user_id=user_id, type=notif_type, payload=payload)
|
||||
session.add(n)
|
||||
@@ -316,11 +305,10 @@ async def _fire_share_email(user_id: int, subject: str, body_text: str) -> None:
|
||||
try:
|
||||
if not await is_smtp_configured():
|
||||
return
|
||||
async with async_session() as session:
|
||||
user = await session.get(User, user_id)
|
||||
if user and user.email:
|
||||
email = await _get_user_email(user_id)
|
||||
if email:
|
||||
html = _email_html(subject, f"<p>{body_text.replace(chr(10), '<br>')}</p>")
|
||||
await send_email(user.email, subject, html)
|
||||
await send_email(email, subject, html)
|
||||
except Exception:
|
||||
logger.exception("Share email notification failed for user %d", user_id)
|
||||
|
||||
@@ -427,7 +415,6 @@ async def notify_group_added(
|
||||
|
||||
|
||||
async def list_in_app_notifications(user_id: int, unread_only: bool = True) -> list[dict]:
|
||||
from scribe.models.notification import Notification
|
||||
async with async_session() as session:
|
||||
q = select(Notification).where(Notification.user_id == user_id)
|
||||
if unread_only:
|
||||
@@ -438,7 +425,6 @@ async def list_in_app_notifications(user_id: int, unread_only: bool = True) -> l
|
||||
|
||||
|
||||
async def unread_notification_count(user_id: int) -> int:
|
||||
from scribe.models.notification import Notification
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(func.count()).where(
|
||||
@@ -450,8 +436,6 @@ async def unread_notification_count(user_id: int) -> int:
|
||||
|
||||
|
||||
async def mark_notification_read(user_id: int, notification_id: int) -> bool:
|
||||
from scribe.models.notification import Notification
|
||||
from datetime import timezone as tz
|
||||
async with async_session() as session:
|
||||
n = (await session.execute(
|
||||
select(Notification).where(
|
||||
@@ -461,21 +445,17 @@ async def mark_notification_read(user_id: int, notification_id: int) -> bool:
|
||||
)).scalar_one_or_none()
|
||||
if not n:
|
||||
return False
|
||||
from datetime import datetime
|
||||
n.read_at = datetime.now(tz.utc)
|
||||
n.read_at = datetime.now(timezone.utc)
|
||||
await session.commit()
|
||||
return True
|
||||
|
||||
|
||||
async def mark_all_notifications_read(user_id: int) -> int:
|
||||
from scribe.models.notification import Notification
|
||||
from datetime import datetime, timezone as tz
|
||||
from sqlalchemy import update as sa_update
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
sa_update(Notification)
|
||||
.where(Notification.user_id == user_id, Notification.read_at.is_(None))
|
||||
.values(read_at=datetime.now(tz.utc))
|
||||
.values(read_at=datetime.now(timezone.utc))
|
||||
.returning(Notification.id)
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
Reference in New Issue
Block a user