Merge pull request 'Mark unread updates read from Web Push' (#562) from timmy/561-web-push-mark-read into main

frontend/service-worker.js

@@ -4,6 +4,7 @@ const CACHE = 'stackchain-dashboard-shell-v88';
 const OFFLINE_LEASE_URL = new URL(BASE + '__offline-session-lease', self.location.origin).href;
 const OUTAGE_STATUSES = new Set([500, 502, 503, 504]);
 const NAVIGATION_TIMEOUT_MS = self.__STACKCHAIN_NAVIGATION_TIMEOUT_MS || 4000;
+const PUSH_ACTION_TIMEOUT_MS = self.__STACKCHAIN_PUSH_ACTION_TIMEOUT_MS || 8000;
 const SHELL = [
   BASE,
   BASE + 'manifest.webmanifest',
@@ -276,26 +277,61 @@ self.addEventListener('push', event => {
   catch (_error) { return; }
   const route = String(payload.route || '');
   const tag = String(payload.tag || '');
+  const notificationId = Number(payload.notification_id);
   if (!/^#\/my-work\/update\/\d+$/.test(route) || !/^stackchain-update-\d+$/.test(tag)) return;
-  event.waitUntil(self.registration.showNotification('New work update', {
+  const options = {
     body: 'Tap to review it in Stackchain.',
     tag,
     data: {route},
-  }));
+  };
+  if (
+    Number.isSafeInteger(notificationId) && notificationId > 0
+    && route === '#/my-work/update/' + notificationId
+    && tag === 'stackchain-update-' + notificationId
+  ) {
+    options.actions = [{ action: 'mark-read', title: 'Mark read' }];
+    options.data.notificationId = notificationId;
+  }
+  event.waitUntil(self.registration.showNotification('New work update', options));
 });
 
-self.addEventListener('notificationclick', event => {
-  event.notification.close();
-  const route = String(event.notification.data?.route || '');
-  if (!route.startsWith('#/my-work/')) return;
+async function openWorkRoute(route) {
   const target = new URL(BASE + route, self.location.origin).href;
-  event.waitUntil((async () => {
-    const windows = await self.clients.matchAll({ type: 'window', includeUncontrolled: true });
-    const client = windows.find(candidate => candidate.url.startsWith(self.location.origin + BASE));
-    if (!client) return self.clients.openWindow(target);
-    if (client.navigate) await client.navigate(target);
-    return client.focus();
-  })());
+  const windows = await self.clients.matchAll({ type: 'window', includeUncontrolled: true });
+  const client = windows.find(candidate => candidate.url.startsWith(self.location.origin + BASE));
+  if (!client) return self.clients.openWindow(target);
+  if (client.navigate) await client.navigate(target);
+  return client.focus();
+}
+
+self.addEventListener('notificationclick', event => {
+  const route = String(event.notification.data?.route || '');
+  if (event.action === 'mark-read') {
+    const notificationId = Number(event.notification.data?.notificationId);
+    if (
+      !Number.isSafeInteger(notificationId) || notificationId <= 0
+      || route !== '#/my-work/update/' + notificationId
+    ) return;
+    event.waitUntil((async () => {
+      const controller = new AbortController();
+      const timeout = setTimeout(() => controller.abort(), PUSH_ACTION_TIMEOUT_MS);
+      try {
+        await fetchJson(BASE + 'api/v1/notifications/' + notificationId + '/read', {
+          method: 'PATCH', headers: { Accept: 'application/json' }, signal: controller.signal,
+        });
+        event.notification.close();
+      } catch (_error) {
+        await openWorkRoute(route);
+        event.notification.close();
+      } finally {
+        clearTimeout(timeout);
+      }
+    })());
+    return;
+  }
+  event.notification.close();
+  if (!route.startsWith('#/my-work/')) return;
+  event.waitUntil(openWorkRoute(route));
 });
 
 self.addEventListener('fetch', event => {

src/push_notifications.py

@@ -103,6 +103,7 @@ async def dispatch_unread_updates(
                             "body": "Tap to review it in Stackchain.",
                             "route": f"#/my-work/update/{thread_id}",
                             "tag": f"stackchain-update-{thread_id}",
+                            "notification_id": thread_id,
                         },
                         separators=(",", ":"),
                     )

tests/test_push_notifications.py

@@ -141,6 +141,7 @@ async def test_dispatch_sends_one_privacy_safe_deep_link_per_new_thread(tmp_path
         "body": "Tap to review it in Stackchain.",
         "route": "#/my-work/update/42",
         "tag": "stackchain-update-42",
+        "notification_id": 42,
     }
     assert "private/repo" not in json.dumps(sent)
     assert "Secret title" not in json.dumps(sent)

tests/test_service_worker.py

@@ -41,6 +41,7 @@ const context = {{
   self: {{
     location: {{ href: 'https://forge.example/dashboard/service-worker.js', origin: 'https://forge.example' }},
     __STACKCHAIN_NAVIGATION_TIMEOUT_MS: 15,
+    __STACKCHAIN_PUSH_ACTION_TIMEOUT_MS: 15,
     addEventListener: (name, handler) => {{ listeners[name] = handler; }},
     skipWaiting: async () => {{ state.skipped = true; }},
     clients: {{
@@ -104,10 +105,11 @@ async function dispatchMessage(data, ports = []) {{
   listeners.message({{ data, ports, waitUntil: promise => {{ pending = promise; }} }});
   if (pending) await pending;
 }}
-async function dispatchNotificationClick(route) {{
+async function dispatchNotificationClick(route, action = '', notificationId = null) {{
   let pending;
   listeners.notificationclick({{
-    notification: {{data: {{route}}, close: () => {{ state.notificationClosed = true; }}}},
+    action,
+    notification: {{data: {{route, notificationId}}, close: () => {{ state.notificationClosed = true; }}}},
     waitUntil: promise => {{ pending = promise; }},
   }});
   if (pending) await pending;
@@ -386,7 +388,7 @@ def test_inbound_push_renders_generic_update_and_tap_opens_existing_workflow():
         """
   await dispatchPush({
     title:'New work update', body:'Tap to review it in Stackchain.',
-    tag:'stackchain-update-42', route:'#/my-work/update/42',
+    tag:'stackchain-update-42', route:'#/my-work/update/42', notification_id:42,
     repository:'must-not-render',
   });
   await dispatchNotificationClick('#/my-work/update/42');
@@ -399,7 +401,11 @@ def test_inbound_push_renders_generic_update_and_tap_opens_existing_workflow():
         "options": {
             "body": "Tap to review it in Stackchain.",
             "tag": "stackchain-update-42",
-            "data": {"route": "#/my-work/update/42"},
+            "actions": [{"action": "mark-read", "title": "Mark read"}],
+            "data": {
+                "route": "#/my-work/update/42",
+                "notificationId": 42,
+            },
         },
     }]
     assert result["opened"] == [
@@ -408,6 +414,132 @@ def test_inbound_push_renders_generic_update_and_tap_opens_existing_workflow():
     assert "must-not-render" not in json.dumps(result["notifications"])
 
 
+def test_push_mark_read_action_confirms_authenticated_mutation_without_opening_app():
+    result = run_worker_scenario(
+        """
+  const calls = [];
+  context.fetch = async (request, options = {}) => {
+    const url = String(request.url || request);
+    const headers = new Headers(options.headers || {});
+    calls.push({url, method:String(options.method || 'GET'), csrf:headers.get('X-CSRF-Token')});
+    if (url.endsWith('/api/v1/session')) {
+      return new Response(JSON.stringify({csrf_token:'session-proof'}), {
+        status:200, headers:{'Content-Type':'application/json'},
+      });
+    }
+    return new Response(JSON.stringify({ok:true}), {
+      status:200, headers:{'Content-Type':'application/json'},
+    });
+  };
+  await dispatchNotificationClick('#/my-work/update/42', 'mark-read', 42);
+  process.stdout.write(JSON.stringify({state,calls}));
+"""
+    )
+
+    assert result["calls"] == [
+        {
+            "url": "https://forge.example/dashboard/api/v1/session",
+            "method": "GET",
+            "csrf": None,
+        },
+        {
+            "url": "https://forge.example/dashboard/api/v1/notifications/42/read",
+            "method": "PATCH",
+            "csrf": "session-proof",
+        },
+    ]
+    assert result["state"]["notificationClosed"] is True
+    assert result["state"]["opened"] == []
+    assert result["state"]["focused"] == []
+
+
+def test_push_mark_read_failure_opens_existing_update_reader():
+    result = run_worker_scenario(
+        """
+  state.clientList = [{
+    url:'https://forge.example/dashboard/',
+    navigate:async function(url){ this.url=url; },
+    focus:async function(){ state.focused.push(this.url); },
+  }];
+  context.fetch = async (request, options = {}) => {
+    const url = String(request.url || request);
+    if (url.endsWith('/api/v1/session')) {
+      return new Response(JSON.stringify({csrf_token:'session-proof'}), {
+        status:200, headers:{'Content-Type':'application/json'},
+      });
+    }
+    return new Response(JSON.stringify({error:'unavailable'}), {
+      status:503, headers:{'Content-Type':'application/json'},
+    });
+  };
+  await dispatchNotificationClick('#/my-work/update/42', 'mark-read', 42);
+  process.stdout.write(JSON.stringify(state));
+"""
+    )
+
+    assert result["focused"] == [
+        "https://forge.example/dashboard/#/my-work/update/42"
+    ]
+    assert result["opened"] == []
+    assert result["notificationClosed"] is True
+
+
+def test_push_mark_read_rejects_mismatched_notification_identity():
+    result = run_worker_scenario(
+        """
+  const calls = [];
+  context.fetch = async (request, options = {}) => {
+    calls.push({url:String(request.url || request), method:String(options.method || 'GET')});
+    return new Response(JSON.stringify({csrf_token:'session-proof'}), {
+      status:200, headers:{'Content-Type':'application/json'},
+    });
+  };
+  await dispatchNotificationClick('#/my-work/update/42', 'mark-read', 43);
+  process.stdout.write(JSON.stringify({state,calls}));
+"""
+    )
+
+    assert result["calls"] == []
+    assert result["state"].get("notificationClosed") is not True
+    assert result["state"]["opened"] == []
+
+
+def test_push_mark_read_timeout_aborts_request_and_opens_update_reader():
+    result = run_worker_scenario(
+        """
+  state.clientList = [{
+    url:'https://forge.example/dashboard/',
+    navigate:async function(url){ this.url=url; },
+    focus:async function(){ state.focused.push(this.url); },
+  }];
+  context.fetch = async (request, options = {}) => {
+    const url = String(request.url || request);
+    if (url.endsWith('/api/v1/session')) {
+      return new Response(JSON.stringify({csrf_token:'session-proof'}), {
+        status:200, headers:{'Content-Type':'application/json'},
+      });
+    }
+    return new Promise((resolve, reject) => {
+      options.signal?.addEventListener('abort', () => {
+        state.actionFetchAborted = true;
+        reject(new Error('aborted'));
+      }, {once:true});
+    });
+  };
+  dispatchNotificationClick('#/my-work/update/42', 'mark-read', 42);
+  await new Promise(resolve => setTimeout(resolve, 50));
+  process.stdout.write(JSON.stringify(state));
+  process.exit(0);
+"""
+    )
+
+    assert result["actionFetchAborted"] is True
+    assert result["focused"] == [
+        "https://forge.example/dashboard/#/my-work/update/42"
+    ]
+    assert result["notificationClosed"] is True
+
+
 def test_background_mutations_obtain_session_bound_csrf_proof():
     source = WORKER.read_text()