Merge pull request 'Acknowledge unread updates from the mobile My Work inbox' (#136) from timmy/135-notification-mark-read into main

Merge pull request 'Acknowledge unread updates from the mobile My Work inbox' (#136)

README.md

@@ -14,7 +14,8 @@ python3 -m pip install -r requirements.txt
 ```
 
 Point the dashboard at the Gitea server root (without `/api/v1`) and provide a
-read-scoped access token, then start the API and bundled frontend:
+token that can read dashboard data and update the authenticated user's
+notification threads, then start the API and bundled frontend:
 
 ```bash
 export GITEA_URL='https://forge.example.com'

frontend/index.html

@@ -60,10 +60,12 @@ textarea { resize: vertical; min-height: 120px; }
 .work-filter { min-height: 44px; }
 .work-filter[aria-pressed="true"] { border-color:var(--accent); background:#1d4f7a; }
 .my-work-list { display:grid; grid-template-columns:repeat(auto-fit,minmax(260px,1fr)); gap:10px; }
-.my-work-card { min-height: 44px; display:block; padding:12px; border:1px solid #1f3a5f; border-radius:12px; background:#0f1d33; color:var(--text); }
-.my-work-card.review-trigger { width:100%; text-align:left; font:inherit; }
+.my-work-card { min-height: 44px; display:grid; gap:8px; padding:12px; border:1px solid #1f3a5f; border-radius:12px; background:#0f1d33; color:var(--text); }
+.my-work-card-main { display:block; width:100%; color:var(--text); text-align:left; font:inherit; background:transparent; border:0; padding:0; }
+.my-work-card-main.review-trigger { width:100%; text-align:left; font:inherit; }
 .my-work-card:hover { border-color:var(--accent); }
 .my-work-card-title { display:block; margin:5px 0; font-weight:650; }
+.mark-update-read { min-height:44px; width:100%; }
 .my-work[data-stale="true"] { border-color:#fcd34d; }
 .review-sheet { position:fixed; inset:0; z-index:50; display:none; justify-content:flex-end; background:rgba(5,12,21,.72); backdrop-filter:blur(4px); }
 .review-sheet.open { display:flex; }
@@ -135,6 +137,7 @@ textarea { resize: vertical; min-height: 120px; }
       </div>
     </div>
     <div class="my-work-list" id="my-work-list"></div>
+    <div class="small" id="my-work-action-status" aria-live="assertive"></div>
   </section>
   <aside class="sidebar">
     <details class="panel stack" data-panel-key="context" open>
@@ -332,6 +335,25 @@ textarea { resize: vertical; min-height: 120px; }
     return res.json();
   }
 
+  async function markNotificationRead(notificationId) {
+    const response = await fetch('api/v1/notifications/' + encodeURIComponent(notificationId) + '/read', {
+      method: 'PATCH',
+      headers: { Accept: 'application/json' },
+    });
+    const payload = await response.json().catch(() => ({}));
+    if (!response.ok) throw new Error(payload.error || 'Mark read failed.');
+    return payload;
+  }
+
+  const notificationAcknowledger = createNotificationAcknowledger({
+    markRead: markNotificationRead,
+    onItems: items => {
+      lastMyWork = items;
+      refreshMyWorkView();
+    },
+    onStatus: message => { qs('#my-work-action-status').textContent = message; },
+  });
+
   function renderContextSnapshot(data) {
     liveMode = true;
     hasContextSnapshot = true;
@@ -376,6 +398,10 @@ textarea { resize: vertical; min-height: 120px; }
 
   function paintMyWork(data) {
     lastMyWork = buildMyWork(data);
+    refreshMyWorkView();
+  }
+
+  function refreshMyWorkView() {
     const counts = countMyWork(lastMyWork);
     Object.entries(counts).forEach(([filter, count]) => {
       const element = qs('[data-work-count="' + filter + '"]');
@@ -394,16 +420,32 @@ textarea { resize: vertical; min-height: 120px; }
       '<span class="small">' + escapeHtml(item.key) + ' · ' + escapeHtml(item.kind === 'pull' ? 'PR' : (item.kind === 'update' ? 'Update' : 'Issue')) + '</span>' +
       '<span class="my-work-card-title">' + escapeHtml(item.title) + '</span>' +
       '<span class="pill">' + escapeHtml(item.reason) + '</span>' +
+      (item.has_update ? ' <span class="pill">Unread update</span>' : '') +
       (item.updated_at ? '<span class="small"> · Updated ' + escapeHtml(fmt(item.updated_at)) + '</span>' : '');
+      const markRead = item.has_update && Number.isInteger(item.notification_id) ?
+        '<button class="mark-update-read" data-notification-id="' + item.notification_id + '">Mark read</button>' : '';
       if (item.is_review) {
         const index = lastMyWork.findIndex(candidate => candidate.key === item.key && candidate.kind === item.kind);
-        return '<button class="my-work-card review-trigger" data-review-index="' + index + '">' + contents + '</button>';
+        return '<article class="my-work-card"><button class="my-work-card-main review-trigger" data-review-index="' + index + '">' + contents + '</button>' + markRead + '</article>';
       }
-      return '<a class="my-work-card" href="' + escAttr(item.url) + '" target="_blank" rel="noopener noreferrer">' + contents + '</a>';
+      return '<article class="my-work-card"><a class="my-work-card-main" href="' + escAttr(item.url) + '" target="_blank" rel="noopener noreferrer">' + contents + '</a>' + markRead + '</article>';
     }).join('') : '<div class="muted">No ' + (selectedWorkFilter === 'review' ? 'reviews' : (selectedWorkFilter === 'update' ? 'unread updates' : (selectedWorkFilter === 'all' ? 'work' : selectedWorkFilter + ' items'))) + '.</div>';
     document.querySelectorAll('[data-review-index]').forEach(button => {
       button.addEventListener('click', () => openReviewSheet(lastMyWork[Number(button.dataset.reviewIndex)], button));
     });
+    document.querySelectorAll('[data-notification-id]').forEach(button => {
+      button.addEventListener('click', async () => {
+        const notificationId = Number(button.dataset.notificationId);
+        button.disabled = true;
+        const acknowledged = await notificationAcknowledger.acknowledge(lastMyWork, notificationId);
+        if (acknowledged) {
+          lastNotifications = lastNotifications.filter(item => item.id !== notificationId);
+          (document.querySelector('[data-notification-id]') || qs('[data-work-filter="update"]'))?.focus();
+        } else {
+          document.querySelector('[data-notification-id="' + notificationId + '"]')?.focus();
+        }
+      });
+    });
   }
 
   function reviewFileElement(filename) {

frontend/my-work.js

@@ -30,6 +30,7 @@ function buildMyWork(data) {
     const existing = byKey.get(subjectKind + ':' + key);
     if (existing) {
       existing.has_update = true;
+      existing.notification_id = update.id;
       existing.url = update.url || existing.url;
       existing.updated_at = update.updated_at || existing.updated_at;
       existing._priority = Math.min(existing._priority, 1);
@@ -43,6 +44,7 @@ function buildMyWork(data) {
       is_review: false,
       is_assigned: false,
       has_update: true,
+      notification_id: update.id,
       reason: 'Unread update',
       _priority: 1,
     };
@@ -57,6 +59,38 @@ function buildMyWork(data) {
   ).map(({ _priority, ...item }) => item);
 }
 
+function acknowledgeNotification(items, notificationId) {
+  return items.flatMap((item) => {
+    if (item.notification_id !== notificationId) return [item];
+    if (item.kind === 'update') return [];
+    const { notification_id, ...acknowledged } = item;
+    return [{ ...acknowledged, has_update: false }];
+  });
+}
+
+function createNotificationAcknowledger({ markRead, onItems, onStatus }) {
+  const pending = new Set();
+  return {
+    async acknowledge(items, notificationId) {
+      if (pending.has(notificationId)) return false;
+      pending.add(notificationId);
+      onItems(acknowledgeNotification(items, notificationId));
+      onStatus('Marking update read…');
+      try {
+        await markRead(notificationId);
+        onStatus('Update marked read.');
+        return true;
+      } catch (_error) {
+        onItems(items);
+        onStatus('Could not mark update read. Retry.');
+        return false;
+      } finally {
+        pending.delete(notificationId);
+      }
+    },
+  };
+}
+
 function filterMyWork(items, selectedFilter) {
   if (selectedFilter === 'all') return items;
   if (selectedFilter === 'review') return items.filter((item) => item.is_review);
@@ -88,5 +122,7 @@ if (typeof module !== 'undefined' && module.exports) {
   buildMyWork.filterMyWork = filterMyWork;
   buildMyWork.summarizeMyWork = summarizeMyWork;
   buildMyWork.countMyWork = countMyWork;
+  buildMyWork.acknowledgeNotification = acknowledgeNotification;
+  buildMyWork.createNotificationAcknowledger = createNotificationAcknowledger;
   module.exports = buildMyWork;
 }

src/gitea_proxy.py

@@ -181,6 +181,14 @@ async def notifications() -> list[dict]:
     return normalized
 
 
+async def mark_notification_read(thread_id: int) -> None:
+    response = await _get_client().patch(
+        f"/api/v1/notifications/threads/{thread_id}?to-status=read",
+        headers=_auth(),
+    )
+    response.raise_for_status()
+
+
 async def pull_requests() -> list[dict]:
     assigned, review_requested = await asyncio.gather(
         fetch("repos/issues/search?state=open&assigned=true&type=pulls&limit=50"),

src/main.py

@@ -3,7 +3,7 @@ import math
 from contextlib import asynccontextmanager
 from pathlib import Path
 
-from fastapi import FastAPI, HTTPException
+from fastapi import FastAPI, HTTPException, Path as PathParam
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.responses import JSONResponse
 from fastapi.staticfiles import StaticFiles
@@ -14,6 +14,7 @@ from src.gitea_proxy import (
     current_user,
     is_requested_review,
     issues,
+    mark_notification_read,
     notifications,
     pull_requests,
     pull_review_detail,
@@ -37,6 +38,7 @@ CONTEXT_TIMEOUT_SECONDS = 5.0
 EVENT_STREAM_TIMEOUT_SECONDS = 5.0
 READINESS_TIMEOUT_SECONDS = 5.0
 REVIEW_DETAIL_TIMEOUT_SECONDS = 5.0
+NOTIFICATION_MUTATION_TIMEOUT_SECONDS = 5.0
 FRONTEND_DIR = Path(__file__).resolve().parent.parent / "frontend"
 _live_snapshot_task: asyncio.Task | None = None
 
@@ -112,6 +114,9 @@ async def prevent_live_api_caching(request, call_next):
     if request.url.path in {"/api/v1/context", "/api/v1/events", "/api/v1/live"} or (
         request.url.path.startswith("/api/v1/repos/")
         and request.url.path.endswith("/review")
+    ) or (
+        request.url.path.startswith("/api/v1/notifications/")
+        and request.url.path.endswith("/read")
     ):
         response.headers["Cache-Control"] = "no-store"
     return response
@@ -305,6 +310,27 @@ async def event_stream():
         )
 
 
+@app.patch("/api/v1/notifications/{thread_id}/read")
+async def read_notification(thread_id: int = PathParam(gt=0)) -> JSONResponse:
+    try:
+        await asyncio.wait_for(
+            mark_notification_read(thread_id),
+            timeout=NOTIFICATION_MUTATION_TIMEOUT_SECONDS,
+        )
+    except TimeoutError:
+        return JSONResponse(
+            {"error": "Marking the update read timed out. Please retry."},
+            status_code=503,
+            headers={"Retry-After": "1"},
+        )
+    except Exception:
+        return JSONResponse(
+            {"error": "The update could not be marked read. Please retry."},
+            status_code=503,
+        )
+    return JSONResponse({"id": thread_id, "status": "read"})
+
+
 @app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/review")
 async def review_detail(owner: str, repo: str, number: int):
     async def load_requested_review():

tests/test_api_paths.py

@@ -16,4 +16,5 @@ def test_api_requests_resolve_inside_dashboard_subpath():
         for path in api_paths
     } == {
         "https://forge.alexanderwhitestone.com/dashboard/api/v1/live",
+        "https://forge.alexanderwhitestone.com/dashboard/api/v1/notifications/",
     }

tests/test_my_work.py

@@ -189,6 +189,73 @@ process.stdout.write(JSON.stringify(buildMyWork({json.dumps(payload)})));
     assert pull["has_update"] is False
 
 
+def test_notification_identity_survives_merge_and_acknowledgement_preserves_assigned_work():
+    payload = {
+        "user": {"login": "timmy"},
+        "issues": [{
+            "number": 7, "title": "Assigned issue", "repository": "stackchain/api",
+            "assignees": ["timmy"], "url": "https://forge.example/issues/7",
+        }],
+        "notifications": [
+            {"id": 42, "number": 7, "title": "Assigned issue", "repository": "stackchain/api",
+             "subject_type": "Issue", "unread": True, "url": "https://forge.example/issues/7#comment"},
+            {"id": 43, "number": 8, "title": "Mention", "repository": "stackchain/web",
+             "subject_type": "Issue", "unread": True, "url": "https://forge.example/issues/8"},
+        ],
+    }
+    script = f"""
+const buildMyWork = require({json.dumps(str(MY_WORK))});
+const before = buildMyWork({json.dumps(payload)});
+const afterMerged = buildMyWork.acknowledgeNotification(before, 42);
+const afterStandalone = buildMyWork.acknowledgeNotification(before, 43);
+process.stdout.write(JSON.stringify({{before, afterMerged, afterStandalone}}));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    )
+    output = json.loads(result.stdout)
+
+    assigned = next(item for item in output["before"] if item["kind"] == "issue")
+    assert assigned["notification_id"] == 42
+    assert assigned["has_update"] is True
+    assert len(output["afterMerged"]) == 2
+    acknowledged = next(item for item in output["afterMerged"] if item["kind"] == "issue")
+    assert acknowledged["has_update"] is False
+    assert "notification_id" not in acknowledged
+    assert [item["notification_id"] for item in output["afterStandalone"] if item["has_update"]] == [42]
+
+
+def test_notification_acknowledger_is_single_flight_and_rolls_back_on_failure():
+    script = f"""
+const buildMyWork = require({json.dumps(str(MY_WORK))});
+const original = [{{kind:'update', notification_id:42, has_update:true}}];
+let calls = 0;
+let release;
+const states = [];
+const statuses = [];
+const controller = buildMyWork.createNotificationAcknowledger({{
+  markRead: () => {{ calls += 1; return new Promise((resolve, reject) => {{ release = reject; }}); }},
+  onItems: items => states.push(items),
+  onStatus: status => statuses.push(status),
+}});
+const first = controller.acknowledge(original, 42);
+const duplicate = controller.acknowledge(original, 42);
+release(new Error('offline'));
+Promise.all([first, duplicate]).then(results => process.stdout.write(JSON.stringify({{
+  calls, states, statuses, results
+}})));
+"""
+    result = subprocess.run(
+        ["node", "-e", script], check=True, capture_output=True, text=True
+    )
+    output = json.loads(result.stdout)
+
+    assert output["calls"] == 1
+    assert output["states"] == [[], [{"kind": "update", "notification_id": 42, "has_update": True}]]
+    assert output["statuses"] == ["Marking update read…", "Could not mark update read. Retry."]
+    assert output["results"] == [False, False]
+
+
 @pytest.mark.anyio
 async def test_mobile_dashboard_puts_filterable_my_work_before_auxiliary_panels():
     html = await dashboard()
@@ -207,6 +274,20 @@ async def test_mobile_dashboard_puts_filterable_my_work_before_auxiliary_panels(
     assert "filterMyWork(lastMyWork, selectedWorkFilter)" in html
 
 
+@pytest.mark.anyio
+async def test_unread_cards_offer_accessible_mobile_mark_read_without_nested_actions():
+    html = await dashboard()
+
+    assert '.mark-update-read' in html and 'min-height:44px' in html
+    assert 'data-notification-id' in html
+    assert 'Unread update' in html
+    assert 'id="my-work-action-status"' in html and 'aria-live="assertive"' in html
+    assert "createNotificationAcknowledger" in html
+    assert "method: 'PATCH'" in html
+    assert "api/v1/notifications/" in html
+    assert '<a class="my-work-card"' not in html
+
+
 @pytest.mark.anyio
 async def test_mobile_filters_wrap_show_counts_and_persist_for_the_session():
     html = await dashboard()

tests/test_notification_read.py

@@ -0,0 +1,47 @@
+import httpx
+import pytest
+
+from src import gitea_proxy, main
+
+
+@pytest.mark.anyio
+async def test_mark_notification_read_calls_supported_gitea_thread_endpoint(monkeypatch):
+    calls = []
+
+    class Response:
+        def raise_for_status(self):
+            return None
+
+    class Client:
+        async def patch(self, path, headers):
+            calls.append((path, headers))
+            return Response()
+
+    monkeypatch.setattr(gitea_proxy, "_get_client", lambda: Client())
+    monkeypatch.setattr(gitea_proxy, "_auth", lambda: {"Authorization": "token test"})
+
+    await gitea_proxy.mark_notification_read(42)
+
+    assert calls == [
+        ("/api/v1/notifications/threads/42?to-status=read", {"Authorization": "token test"})
+    ]
+
+
+@pytest.mark.anyio
+async def test_mark_notification_read_api_is_bounded_and_never_cacheable(monkeypatch):
+    marked = []
+
+    async def mark(thread_id):
+        marked.append(thread_id)
+
+    monkeypatch.setattr(main, "mark_notification_read", mark)
+    transport = httpx.ASGITransport(app=main.app)
+    async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
+        response = await client.patch("/api/v1/notifications/42/read")
+        invalid = await client.patch("/api/v1/notifications/0/read")
+
+    assert response.status_code == 200
+    assert response.json() == {"id": 42, "status": "read"}
+    assert response.headers["cache-control"] == "no-store"
+    assert invalid.status_code == 422
+    assert marked == [42]