diff --git a/core/brief/builder.py b/core/brief/builder.py index bb64183..b39edb8 100644 --- a/core/brief/builder.py +++ b/core/brief/builder.py @@ -2,6 +2,7 @@ from __future__ import annotations +import logging from datetime import date from core.constants import DONE, GRADE_WEIGHT, SUBMITTED, TODO, URGENT, WAITING @@ -10,6 +11,8 @@ fetch_tasks_direct, ) +log = logging.getLogger(__name__) + # Statuses kept OUT of the deterministic brief: work already handed in (SUBMITTED), # resolved (DONE), or handled face-to-face (WAITING = discuss/demonstrate, which we # deliberately don't surface). Everything else — where the student still owes work — @@ -42,9 +45,19 @@ def pending_task_entries(rows: list[dict], today: date, base_url: str) -> list[d try: due = date.fromisoformat(deadline) except ValueError: + log.warning( + "pending_task_entries: invalid deadline %r for task %r — skipping", + deadline, + r.get("name") or r.get("abbreviation"), + ) continue abbrev = r.get("abbreviation") or "" - url = f"{base}/projects/{r['project_id']}/dashboard/{abbrev}" if base and abbrev else None + pid = r.get("project_id") + url = ( + f"{base}/projects/{pid}/dashboard/{abbrev}" + if base and abbrev and pid is not None + else None + ) entries.append( { "task": { diff --git a/core/brief/renderer.py b/core/brief/renderer.py index 5a03e27..5893da3 100644 --- a/core/brief/renderer.py +++ b/core/brief/renderer.py @@ -97,7 +97,7 @@ def _as_of_note(as_of: str | None) -> str: try: stamp = date.fromisoformat(stamp).strftime("%b %d, %Y") except ValueError: - pass + stamp = "recently" return ( f'

' f"Based on your OnTrack data as of {stamp}. Open OnTrack to refresh.

" diff --git a/core/crypto.py b/core/crypto.py index 244bf27..4cd84cb 100644 --- a/core/crypto.py +++ b/core/crypto.py @@ -88,4 +88,4 @@ def decrypt(value: str | None) -> str | None: return _fernet.decrypt(value[len(_PREFIX) :].encode()).decode() except InvalidToken: log.error("Failed to decrypt token — wrong TOKEN_ENCRYPTION_KEY for this data?") - return value + return None diff --git a/core/db.py b/core/db.py index bab9ba3..9130ae6 100644 --- a/core/db.py +++ b/core/db.py @@ -292,7 +292,10 @@ def upsert_user( clerk_user_id, ), ) - return cur.fetchone()[0] + row = cur.fetchone() + if row is None: + raise RuntimeError(f"upsert_user: RETURNING id returned no row for email={email!r}") + return row[0] else: cur.execute( f""" @@ -321,7 +324,12 @@ def upsert_user( clerk_user_id, ), ) - return cur.execute(f"SELECT id FROM users WHERE email = {_P}", (email,)).fetchone()[0] + row = cur.execute(f"SELECT id FROM users WHERE email = {_P}", (email,)).fetchone() + if row is None: + raise RuntimeError( + f"upsert_user: user row missing after upsert for email={email!r}" + ) + return row[0] @_retry_on_deadlock diff --git a/core/jobs.py b/core/jobs.py index ce11e0c..4af03fd 100644 --- a/core/jobs.py +++ b/core/jobs.py @@ -39,41 +39,52 @@ def run_brief(user_id: int, *, confirm_if_empty: bool = False) -> None: when there's nothing to show, send a one-off confirmation instead of returning silently, so the deliberate click gets feedback. The daily cron leaves it False. """ - user = get_user_by_id(user_id) - if not user: - log.error("run_brief: no user found for id=%s", user_id) - return - if not user.get("subscribed", 1): - # Defensive: a paused user should have no scheduled job, but never email - # someone who has unsubscribed even if a stale job somehow fires. - log.info("run_brief: %s is unsubscribed — skipping", user["email"]) - return - - email = user["email"] - window_days = user.get("brief_days") or _DEFAULT_WINDOW_DAYS - today = date.today() - end = today + timedelta(days=window_days) - - task_count, last_seen = get_capture_meta(user_id) - if task_count == 0: - # Cold start: nothing captured yet (the student hasn't opened OnTrack with - # the extension since subscribing). Don't send an empty brief every day — - # confirm the deliberate enable click, otherwise stay quiet until data lands. - if confirm_if_empty: - log.info("No captured tasks for %s yet — sending briefs-enabled confirmation", email) - send_briefs_enabled_email(email) + try: + user = get_user_by_id(user_id) + if not user: + log.error("run_brief: no user found for id=%s", user_id) + return + if not user.get("subscribed", 1): + # Defensive: a paused user should have no scheduled job, but never email + # someone who has unsubscribed even if a stale job somehow fires. + log.info("run_brief: %s is unsubscribed — skipping", user["email"]) + return + + email = user["email"] + window_days = user.get("brief_days") or _DEFAULT_WINDOW_DAYS + today = date.today() + end = today + timedelta(days=window_days) + + task_count, last_seen = get_capture_meta(user_id) + if task_count == 0: + # Cold start: nothing captured yet (the student hasn't opened OnTrack with + # the extension since subscribing). Don't send an empty brief every day — + # confirm the deliberate enable click, otherwise stay quiet until data lands. + if confirm_if_empty: + log.info( + "No captured tasks for %s yet — sending briefs-enabled confirmation", email + ) + ok = send_briefs_enabled_email(email) + if not ok: + log.error("run_brief: failed to send briefs-enabled confirmation to %s", email) + else: + log.info("run_brief: no captured tasks for %s yet — skipping", email) + return + + rows = get_pending_tasks(user_id, today.isoformat(), end.isoformat()) + entries = pending_task_entries(rows, today, user["base_url"]) + due_this_week = sum(1 for e in entries if (e["due"] - today).days <= _THIS_WEEK_DAYS) + + # The user has captured data, so send the brief even when nothing is due in the + # window — render_html shows the "nothing due" state, same as before. + html = render_html(entries, today, window_days=window_days, as_of=last_seen) + ok = send_brief_to(html, email, today, due_this_week) + if not ok: + log.error("run_brief: email delivery failed for user_id=%s (%s)", user_id, email) else: - log.info("run_brief: no captured tasks for %s yet — skipping", email) - return - - rows = get_pending_tasks(user_id, today.isoformat(), end.isoformat()) - entries = pending_task_entries(rows, today, user["base_url"]) - due_this_week = sum(1 for e in entries if (e["due"] - today).days <= _THIS_WEEK_DAYS) - - # The user has captured data, so send the brief even when nothing is due in the - # window — render_html shows the "nothing due" state, same as before. - html = render_html(entries, today, window_days=window_days, as_of=last_seen) - send_brief_to(html, email, today, due_this_week) + log.info("run_brief: brief sent to %s (%d tasks due this week)", email, due_this_week) + except Exception as exc: + log.error("run_brief failed for user_id=%s: %s", user_id, exc, exc_info=True) # The 20-min token-refresh poll has been retired. run_brief now mints a fresh diff --git a/extension/public/background.js b/extension/public/background.js index 45af830..c0c5b74 100644 --- a/extension/public/background.js +++ b/extension/public/background.js @@ -41,7 +41,7 @@ chrome.cookies.onChanged.addListener(({ cookie, removed }) => { if (removed) return; if (cookie.name !== "refresh_token") return; if (!cookie.domain.includes("ontrack.deakin.edu.au")) return; - chrome.storage.local.get("username", ({ username }) => { + chrome.storage.local.get("username").then(({ username }) => { // username never changes across a re-login, so the stored one is still valid. if (username) pushRefreshToken(username); }); @@ -102,7 +102,8 @@ chrome.runtime.onMessage.addListener((msg, _sender, sendResponse) => { sendResponse({ ok: true, deduped: true }); return true; } - lastIngestHash.set(dedupKey, hash); + // Do NOT cache the hash until the server confirms receipt — premature caching + // would cause a failed push to be silently deduped away on the next retry. fetch(`${APP_URL}/ingest`, { method: "POST", @@ -114,11 +115,12 @@ chrome.runtime.onMessage.addListener((msg, _sender, sendResponse) => { }), }) .then((r) => r.json()) - .then(() => sendResponse({ ok: true })) + .then(() => { + lastIngestHash.set(dedupKey, hash); // only cache once server has accepted it + sendResponse({ ok: true }); + }) .catch(() => { - // Push failed — drop the cached hash so the next identical capture retries - // instead of being silently deduped away. - lastIngestHash.delete(dedupKey); + // Hash was never set, so the next identical capture will retry naturally. sendResponse({ ok: false }); }); diff --git a/extension/public/injected.js b/extension/public/injected.js index 9e32cb4..00089a5 100644 --- a/extension/public/injected.js +++ b/extension/public/injected.js @@ -4,9 +4,10 @@ * a new token on every response, so reading request headers gives a stale value. */ (function () { - let lastToken = null; - let lastUsername = null; - let _swept = false; // only sweep once per page load + let lastToken = null; + let lastUsername = null; + let _swept = false; // only sweep once per page load + let _pendingProjects = null; // projects captured before first token — flushed on emit function emit(token, username) { if (!token || !username) return; @@ -16,6 +17,12 @@ window.dispatchEvent(new CustomEvent("ontrack-auth-captured", { detail: { auth_token: token, username: username } })); + // Flush a sweep that was deferred because the token wasn't available yet. + if (_pendingProjects) { + const projects = _pendingProjects; + _pendingProjects = null; + sweepProjectTasks(projects); + } } // Track username from outgoing request headers (it never rotates) @@ -78,7 +85,11 @@ // needing to click into each unit. Uses the already-captured auth token. // Guard (_swept) prevents re-triggering on subsequent project-list refreshes. function sweepProjectTasks(projects) { - if (_swept || !lastToken || !lastUsername) return; + if (_swept) return; + if (!lastToken || !lastUsername) { + _pendingProjects = projects; // defer until first token is captured via emit() + return; + } _swept = true; const today = new Date().toISOString().slice(0, 10); const headers = { @@ -161,10 +172,9 @@ const username = h["username"] || lastUsername; if (respToken && username) { emit(respToken, username); - } else { - // Fallback: use request token (first page load before any response) - emit(h["auth-token"], h["username"]); } + // No fallback to request headers: the request token is already rotated (stale) + // once the response arrives. Emit only when we have a fresh response token. // Capture the response body for the data endpoints we care about. // Angular's HttpClient sets responseType="json", and reading responseText diff --git a/routes/main.py b/routes/main.py index ec5490a..403d77f 100644 --- a/routes/main.py +++ b/routes/main.py @@ -540,8 +540,11 @@ def api_snapshot(): for r in get_pending_tasks(user_id, today.isoformat(), end.isoformat()): if is_hidden(r): continue + deadline = r.get("deadline") or "" + if not deadline: + continue try: - offset = (date.fromisoformat(r["deadline"]) - today).days + offset = (date.fromisoformat(deadline) - today).days except (ValueError, TypeError): continue if not 0 <= offset < days_count: @@ -553,8 +556,8 @@ def api_snapshot(): "abbreviation": abbrev, "unit": r.get("unit_code") or "", "grade": r.get("target_grade_label") or "P (Pass)", - "due_date": r["deadline"], - "url": f"{base_url}/projects/{r['project_id']}/dashboard/{abbrev}", + "due_date": deadline, + "url": f"{base_url}/projects/{r.get('project_id', '')}/dashboard/{abbrev}", } ) @@ -569,7 +572,7 @@ def api_snapshot(): "unit": r.get("unit_code") or "", "task": r.get("name") or abbrev, "text": trimmed, - "url": f"{base_url}/projects/{r['project_id']}/dashboard/{abbrev}", + "url": f"{base_url}/projects/{r.get('project_id', '')}/dashboard/{abbrev}", } )