From 32e2a228b9893248c8825c39b9c5fc7e74375e58 Mon Sep 17 00:00:00 2001 From: timmy Date: Sun, 23 Aug 2026 07:38:53 +0000 Subject: [PATCH] feat: make Following a changed-first inbox (Closes #1297) --- frontend/dashboard.css | 2 + frontend/following.js | 36 +++++++-- src/following_store.py | 89 ++++++++++++++++++++-- src/main.py | 59 +++++++++++++- tests/e2e/test_mobile_following_release.py | 3 +- tests/test_following_api.py | 71 ++++++++++++++++- tests/test_following_frontend.py | 40 +++++++++- tests/test_following_store.py | 46 ++++++++++- 8 files changed, 325 insertions(+), 21 deletions(-) diff --git a/frontend/dashboard.css b/frontend/dashboard.css index 87c2a80..e59b476 100644 --- a/frontend/dashboard.css +++ b/frontend/dashboard.css @@ -1092,6 +1092,8 @@ textarea { resize: vertical; min-height: 120px; } .following-list { display:grid; gap:8px; min-width:0; max-height:70dvh; overflow:auto; } .following-card { box-sizing:border-box; display:flex; align-items:center; justify-content:space-between; gap:12px; width:100%; min-width:0; min-height:52px; padding:10px 12px; text-align:left; } .following-card span:first-child { min-width:0; display:grid; gap:3px; } +.following-card em { color:var(--accent); font-size:.75rem; font-style:normal; font-weight:700; text-transform:uppercase; letter-spacing:.04em; } +.following-card.has-unseen-change { border-color:var(--accent); } .following-card strong, .following-card small { overflow-wrap:anywhere; } .markdown-content { min-width:0; max-width:100%; overflow-wrap:anywhere; white-space:normal; } .markdown-content > :first-child { margin-top:0; } diff --git a/frontend/following.js b/frontend/following.js index 63bcb03..2bed33f 100644 --- a/frontend/following.js +++ b/frontend/following.js @@ -14,9 +14,12 @@ function publish(status, error) { const state = {status, revision:snapshot.revision, items:[...snapshot.items]}; + state.degraded = snapshot.degraded === true; + state.refreshFailures = Number(snapshot.refreshFailures) || 0; if (error) state.error = error; options.render?.(state); - if (status === 'ready') options.onCount?.(snapshot.items.length); + if (status === 'ready') options.onCount?.( + snapshot.items.filter(item => item.has_unseen_change === true).length); return state; } @@ -29,6 +32,8 @@ snapshot = { revision:Number(result?.revision) || 0, items:Array.isArray(result?.items) ? result.items.slice(0, 50) : [], + degraded:result?.degraded === true, + refreshFailures:Number(result?.refresh_failures) || 0, }; publish('ready'); return snapshot; @@ -38,10 +43,15 @@ } } - function open(index) { + async function open(index) { const item = snapshot.items[Number(index)]; if (!item) return false; - Promise.resolve(options.onOpen?.({...item, kind:'issue'})).catch(() => {}); + await options.onOpen?.({...item, kind:'issue'}); + if (item.has_unseen_change === true && typeof options.onAcknowledge === 'function') { + await options.onAcknowledge(item); + item.has_unseen_change = false; + publish('ready'); + } return true; } @@ -67,17 +77,19 @@ query('#retry-following').hidden = state.status !== 'error'; if (state.status === 'loading') return void (status.textContent = 'Loading watched issues…'); if (state.status === 'error') return void (status.textContent = state.error?.message || 'Following is temporarily unavailable.'); - status.textContent = state.items.length + status.textContent = (state.degraded ? 'Some watched issues could not be refreshed. Showing last known details. ' : '') + (state.items.length ? state.items.length + (state.items.length === 1 ? ' watched issue.' : ' watched issues.') - : 'No watched issues yet. Watch one from Search to keep it here.'; + : 'No watched issues yet. Watch one from Search to keep it here.'); list.innerHTML = state.items.map((item, index) => - '').join(''); list.querySelectorAll('[data-following-index]').forEach(button => button.addEventListener('click', () => { query('#following-sheet').close(); - feature.open(Number(button.dataset.followingIndex)); + feature.open(Number(button.dataset.followingIndex)).catch(() => {}); })); } feature = createFollowing({ @@ -86,9 +98,17 @@ const value = query('[data-mobile-queue-count="following"]'); value.textContent = count; value.closest('button').setAttribute('aria-label', 'Following, ' + count + - (count === 1 ? ' watched issue' : ' watched issues')); + (count === 1 ? ' unseen change' : ' unseen changes')); }, onOpen, + onAcknowledge:item => { + const [owner, repo] = item.repository.split('/'); + return fetchJson('api/v1/following/' + encodeURIComponent(owner) + '/' + + encodeURIComponent(repo) + '/issues/' + item.number + '/seen', { + method:'PUT', headers:{'Content-Type':'application/json', Accept:'application/json'}, + body:JSON.stringify({updated_at:item.updated_at}), + }); + }, }); query('#close-following').addEventListener('click', () => query('#following-sheet').close()); query('#retry-following').addEventListener('click', () => feature.load().catch(() => {})); diff --git a/src/following_store.py b/src/following_store.py index fbb9d21..88a7961 100644 --- a/src/following_store.py +++ b/src/following_store.py @@ -86,6 +86,13 @@ class FollowingStore: url = raw.get("url") if not isinstance(url, str) or not url.startswith(("http://", "https://")) or len(url) > 2048: raise ValueError("url is invalid") + last_seen_updated_at = raw.get("last_seen_updated_at", updated_at) + if ( + not isinstance(last_seen_updated_at, str) + or not last_seen_updated_at + or len(last_seen_updated_at) > 64 + ): + raise ValueError("last seen update is invalid") return { "repository": repository, "number": number, @@ -93,8 +100,22 @@ class FollowingStore: "state": state, "updated_at": updated_at, "url": url, + "last_seen_updated_at": last_seen_updated_at, } + @classmethod + def _present(cls, snapshot: dict) -> dict: + changed = [] + unchanged = [] + for raw in snapshot["items"]: + stored = cls._normalize_item(raw) + unseen = stored["updated_at"] != stored["last_seen_updated_at"] + item = {key: value for key, value in stored.items() if key != "last_seen_updated_at"} + item["has_unseen_change"] = unseen + (changed if unseen else unchanged).append(item) + changed.sort(key=lambda item: item["updated_at"], reverse=True) + return {"revision": snapshot["revision"], "items": changed + unchanged} + @staticmethod def _identity(item: dict) -> tuple[str, int]: return item["repository"].lower(), item["number"] @@ -106,12 +127,13 @@ class FollowingStore: "SELECT revision, items FROM following_issues WHERE login = ?", (login,) ).fetchone() snapshot, legacy = self._snapshot(row, login) + snapshot["items"] = [self._normalize_item(item) for item in snapshot["items"]] if row is not None and legacy: connection.execute( "UPDATE following_issues SET items = ? WHERE login = ? AND items = ?", (self._seal(login, snapshot["items"]), login, row[1]), ) - return snapshot + return self._present(snapshot) def preflight(self, login: str, raw_item: dict, watching: bool) -> dict: """Validate a requested change and capacity without mutating the registry.""" @@ -146,7 +168,7 @@ class FollowingStore: "SELECT revision, items FROM following_issues WHERE login = ?", (login,) ).fetchone() current, _legacy = self._snapshot(row, login) - items = list(current["items"]) + items = [self._normalize_item(candidate) for candidate in current["items"]] index = next( (position for position, candidate in enumerate(items) if self._identity(candidate) == identity), @@ -157,13 +179,14 @@ class FollowingStore: if len(items) >= self.limit: raise ValueError(f"following is limited to {self.limit} issues") items.insert(0, item) - elif items[index] == item: - return current else: + item["last_seen_updated_at"] = items[index]["last_seen_updated_at"] + if items[index] == item: + return self._present({"revision": current["revision"], "items": items}) items.pop(index) items.insert(0, item) elif index is None: - return current + return self._present({"revision": current["revision"], "items": items}) else: items.pop(index) revision = current["revision"] + 1 @@ -172,4 +195,58 @@ class FollowingStore: "ON CONFLICT(login) DO UPDATE SET revision=excluded.revision, items=excluded.items", (login, revision, self._seal(login, items)), ) - return {"revision": revision, "items": items} + return self._present({"revision": revision, "items": items}) + + def refresh(self, login: str, raw_items: list[dict]) -> dict: + """Merge successful upstream snapshots while preserving seen revisions.""" + login = self._login(login) + fresh = {self._identity(item): self._normalize_item(item) for item in raw_items} + with self._connect() as connection: + connection.execute("BEGIN IMMEDIATE") + row = connection.execute( + "SELECT revision, items FROM following_issues WHERE login = ?", (login,) + ).fetchone() + current, _legacy = self._snapshot(row, login) + items = [self._normalize_item(candidate) for candidate in current["items"]] + changed = False + for index, item in enumerate(items): + update = fresh.get(self._identity(item)) + if update is None: + continue + update["last_seen_updated_at"] = item["last_seen_updated_at"] + if update != item: + items[index] = update + changed = True + revision = current["revision"] + if changed: + revision += 1 + connection.execute( + "UPDATE following_issues SET revision = ?, items = ? WHERE login = ?", + (revision, self._seal(login, items), login), + ) + return self._present({"revision": revision, "items": items}) + + def acknowledge(self, login: str, repository: str, number: int, updated_at: str) -> dict: + """Mark only the exact upstream revision successfully opened by the operator.""" + login = self._login(login) + identity = (str(repository).lower(), number) + with self._connect() as connection: + connection.execute("BEGIN IMMEDIATE") + row = connection.execute( + "SELECT revision, items FROM following_issues WHERE login = ?", (login,) + ).fetchone() + current, _legacy = self._snapshot(row, login) + items = [self._normalize_item(candidate) for candidate in current["items"]] + revision = current["revision"] + for item in items: + if self._identity(item) != identity or item["updated_at"] != updated_at: + continue + if item["last_seen_updated_at"] != updated_at: + item["last_seen_updated_at"] = updated_at + revision += 1 + connection.execute( + "UPDATE following_issues SET revision = ?, items = ? WHERE login = ?", + (revision, self._seal(login, items), login), + ) + break + return self._present({"revision": revision, "items": items}) diff --git a/src/main.py b/src/main.py index 4d08757..4ee19e8 100644 --- a/src/main.py +++ b/src/main.py @@ -2564,6 +2564,10 @@ def _following_store() -> FollowingStore: ) +class FollowingSeenRevision(BaseModel): + updated_at: str = Field(min_length=1, max_length=64) + + def _completed_filed_review_store() -> CompletedFiledReviewStore: return CompletedFiledReviewStore( os.getenv( @@ -2611,8 +2615,33 @@ async def _confirmed_login() -> str: @app.get("/api/v1/following") async def get_following(response: Response): login = await _confirmed_login() + store = _following_store() try: - snapshot = await asyncio.to_thread(_following_store().get, login) + snapshot = await asyncio.to_thread(store.get, login) + semaphore = asyncio.Semaphore(5) + + async def refresh_item(item: dict) -> dict: + async with semaphore: + preview = await asyncio.wait_for( + gitea_proxy.work_preview(item["repository"], "issue", item["number"]), + timeout=GLOBAL_SEARCH_TIMEOUT_SECONDS, + ) + return store._normalize_item({ + "repository": item["repository"], + "number": item["number"], + "title": preview.get("title", ""), + "state": preview.get("state", ""), + "updated_at": preview.get("updated_at", ""), + "url": preview.get("url", ""), + }) + + results = await asyncio.gather( + *(refresh_item(item) for item in snapshot["items"]), + return_exceptions=True, + ) + refreshed = [result for result in results if isinstance(result, dict)] + failures = len(results) - len(refreshed) + snapshot = await asyncio.to_thread(store.refresh, login, refreshed) except (OSError, sqlite3.Error, PrivateStateEncryptionError): raise HTTPException( status_code=503, @@ -2620,7 +2649,33 @@ async def get_following(response: Response): headers={"Retry-After": "1"}, ) response.headers["Cache-Control"] = "no-store" - return snapshot + return {**snapshot, "degraded": failures > 0, "refresh_failures": failures} + + +@app.put("/api/v1/following/{owner}/{repo}/issues/{number}/seen") +async def acknowledge_following_revision( + payload: FollowingSeenRevision, + owner: str, + repo: str, + number: int = PathParam(gt=0), +): + login = await _confirmed_login() + try: + return await asyncio.to_thread( + _following_store().acknowledge, + login, + f"{owner}/{repo}", + number, + payload.updated_at, + ) + except ValueError as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + except (OSError, sqlite3.Error, PrivateStateEncryptionError): + raise HTTPException( + status_code=503, + detail="Following synchronization is unavailable", + headers={"Retry-After": "1"}, + ) @app.get("/api/v1/completed-filed-reviews") diff --git a/tests/e2e/test_mobile_following_release.py b/tests/e2e/test_mobile_following_release.py index d5c894e..42e5f5f 100644 --- a/tests/e2e/test_mobile_following_release.py +++ b/tests/e2e/test_mobile_following_release.py @@ -23,12 +23,13 @@ def test_following_queue_is_phone_usable_at_narrow_viewport(): row = page.locator('[data-mobile-queue="following"]') expect(row).to_have_count(1) page.locator("#following-list").evaluate("""node => { - node.innerHTML = ''; + node.innerHTML = ''; }""") page.locator("#following-sheet").evaluate("node => node.showModal()") expect(page.locator("#following-sheet")).to_be_visible() expect(page.locator(".following-card")).to_be_visible() + expect(page.locator(".following-card")).to_contain_text("New activity") assert page.locator(".following-card").bounding_box()["height"] >= 44 assert page.locator("#close-following").bounding_box()["height"] >= 44 overflow = page.evaluate("document.documentElement.scrollWidth > document.documentElement.clientWidth") diff --git a/tests/test_following_api.py b/tests/test_following_api.py index cc0a689..5e5aa70 100644 --- a/tests/test_following_api.py +++ b/tests/test_following_api.py @@ -50,13 +50,14 @@ async def test_confirmed_watch_updates_account_following_collection(monkeypatch, } assert following.status_code == 200 assert following.headers["cache-control"] == "no-store" - assert following.json() == {"revision": 1, "items": [{ + assert following.json() == {"revision": 1, "degraded": False, "refresh_failures": 0, "items": [{ "repository": "stackchain/api", "number": 42, "title": "Make mobile review useful", "state": "open", "updated_at": "2026-08-23T03:00:00Z", "url": "https://forge.example/stackchain/api/issues/42", + "has_unseen_change": False, }]} @@ -180,3 +181,71 @@ async def test_uncompensated_following_write_reports_authoritative_partial_outco "error": "Watching in Gitea, but Following could not sync. Retry this action.", } assert calls == [True, False] + + +@pytest.mark.anyio +async def test_following_refreshes_changed_items_and_preserves_failed_items(monkeypatch, tmp_path): + store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=b"e" * 32) + first = { + "repository": "stackchain/api", "number": 42, "title": "Old title", + "state": "open", "updated_at": "2026-08-23T03:00:00Z", + "url": "https://forge.example/stackchain/api/issues/42", + } + failed = {**first, "number": 43, "title": "Last known", + "url": "https://forge.example/stackchain/api/issues/43"} + store.set_watching("timmy", failed, True) + store.set_watching("timmy", first, True) + + async def preview(repository, kind, number): + assert (repository, kind) == ("stackchain/api", "issue") + if number == 43: + return {**failed, "kind": "issue", "title": ""} + return {**first, "kind": "issue", "title": "Fresh title", "state": "closed", + "updated_at": "2026-08-23T04:00:00Z"} + + async def user(): + return {"login": "timmy"} + + monkeypatch.setattr(main, "_following_store", lambda: store) + monkeypatch.setattr(main.gitea_proxy, "work_preview", preview) + monkeypatch.setattr(main, "current_user", user) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/api/v1/following") + + assert response.status_code == 200 + payload = response.json() + assert payload["degraded"] is True + assert payload["refresh_failures"] == 1 + assert [(item["number"], item["has_unseen_change"]) for item in payload["items"]] == [ + (42, True), (43, False) + ] + assert payload["items"][0]["title"] == "Fresh title" + assert payload["items"][1]["title"] == "Last known" + + +@pytest.mark.anyio +async def test_following_acknowledges_only_the_exact_loaded_revision(monkeypatch, tmp_path): + store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=b"f" * 32) + item = { + "repository": "stackchain/api", "number": 42, "title": "Changed", + "state": "open", "updated_at": "2026-08-23T03:00:00Z", + "url": "https://forge.example/stackchain/api/issues/42", + } + store.set_watching("timmy", item, True) + store.refresh("timmy", [{**item, "updated_at": "2026-08-23T04:00:00Z"}]) + + async def user(): + return {"login": "timmy"} + + monkeypatch.setattr(main, "_following_store", lambda: store) + monkeypatch.setattr(main, "current_user", user) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.put( + "/api/v1/following/stackchain/api/issues/42/seen", + json={"updated_at": "2026-08-23T04:00:00Z"}, + ) + + assert response.status_code == 200 + assert response.json()["items"][0]["has_unseen_change"] is False diff --git a/tests/test_following_frontend.py b/tests/test_following_frontend.py index b8ceaeb..49aa7ab 100644 --- a/tests/test_following_frontend.py +++ b/tests/test_following_frontend.py @@ -14,7 +14,7 @@ const state = {{ renders:[], counts:[], opened:[], requests:[] }}; const feature = createFollowing({{ fetchJson: async path => {{ state.requests.push(path); - return {{revision:3,items:[{{repository:'stackchain/api',number:42,title:'Quiet issue',state:'open',updated_at:'2026-08-23T03:00:00Z',url:'https://forge.example/issue/42'}}]}}; + return {{revision:3,items:[{{repository:'stackchain/api',number:42,title:'Quiet issue',state:'open',updated_at:'2026-08-23T03:00:00Z',url:'https://forge.example/issue/42',has_unseen_change:false}}]}}; }}, render: snapshot => state.renders.push(snapshot), onCount: count => state.counts.push(count), @@ -34,7 +34,7 @@ process.stdout.write(JSON.stringify(state)); """) assert result["requests"] == ["api/v1/following"] - assert result["counts"] == [1] + assert result["counts"] == [0] assert result["renders"][-1]["status"] == "ready" assert result["renders"][-1]["items"][0]["title"] == "Quiet issue" assert result["opened"] == [{ @@ -44,6 +44,7 @@ process.stdout.write(JSON.stringify(state)); "state": "open", "updated_at": "2026-08-23T03:00:00Z", "url": "https://forge.example/issue/42", + "has_unseen_change": False, "kind": "issue", }] @@ -69,3 +70,38 @@ process.stdout.write(JSON.stringify({{opened:feature.open('following'),recommend "recommended": {"name": "find", "count": 0, "label": "Find Work"}, "calls": ["following"], } + + +def test_following_counts_unseen_changes_and_acknowledges_after_preview_loads(): + script = f""" +const createFollowing = require({json.dumps(str(MODULE))}); +const state = {{counts:[], opened:[], acknowledged:[], renders:[]}}; +let finishPreview; +const feature = createFollowing({{ + fetchJson:async () => ({{revision:4,items:[ + {{repository:'stackchain/api',number:42,title:'Changed',state:'open',updated_at:'2026-08-23T04:00:00Z',url:'https://forge/42',has_unseen_change:true}}, + {{repository:'stackchain/api',number:43,title:'Quiet',state:'open',updated_at:'2026-08-23T03:00:00Z',url:'https://forge/43',has_unseen_change:false}} + ]}}), + render:value => state.renders.push(value), + onCount:value => state.counts.push(value), + onOpen:item => new Promise(resolve => {{ finishPreview=() => {{ state.opened.push(item.number); resolve(); }}; }}), + onAcknowledge:async item => state.acknowledged.push(item.updated_at), +}}); +(async () => {{ + await feature.load(); + const opening=feature.open(0); + await new Promise(resolve => setImmediate(resolve)); + state.before={{acknowledged:[...state.acknowledged],counts:[...state.counts]}}; + finishPreview(); + await opening; + process.stdout.write(JSON.stringify(state)); +}})().catch(error => {{ console.error(error); process.exit(1); }}); +""" + result = json.loads(subprocess.run( + ["node", "-e", script], text=True, capture_output=True, check=True + ).stdout) + assert result["before"] == {"acknowledged": [], "counts": [1]} + assert result["opened"] == [42] + assert result["acknowledged"] == ["2026-08-23T04:00:00Z"] + assert result["counts"] == [1, 0] + assert result["renders"][-1]["items"][0]["has_unseen_change"] is False diff --git a/tests/test_following_store.py b/tests/test_following_store.py index 47cf0da..1b58fc1 100644 --- a/tests/test_following_store.py +++ b/tests/test_following_store.py @@ -21,10 +21,54 @@ def test_confirmed_watch_is_account_scoped_idempotent_and_encrypted(tmp_path): first = store.set_watching("Timmy", ITEM, True) repeated = store.set_watching("timmy", ITEM, True) - assert first == repeated == {"revision": 1, "items": [ITEM]} + assert first == repeated == {"revision": 1, "items": [{**ITEM, "has_unseen_change": False}]} assert store.get("other") == {"revision": 0, "items": []} stored = sqlite3.connect(path).execute( "SELECT items FROM following_issues WHERE login = ?", ("timmy",) ).fetchone()[0] assert ITEM["title"] not in stored assert ITEM["repository"] not in stored + + +def test_refresh_marks_changed_items_unseen_and_sorts_them_first(tmp_path): + store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=KEY) + quiet = {**ITEM, "number": 7, "title": "Quiet", "updated_at": "2026-08-23T02:00:00Z"} + store.set_watching("timmy", quiet, True) + store.set_watching("timmy", ITEM, True) + + snapshot = store.refresh("timmy", [{ + **quiet, "title": "Changed upstream", "state": "closed", + "updated_at": "2026-08-23T04:00:00Z", + }]) + + assert [(item["number"], item["has_unseen_change"]) for item in snapshot["items"]] == [ + (7, True), (42, False) + ] + assert snapshot["items"][0]["title"] == "Changed upstream" + assert snapshot["items"][0]["state"] == "closed" + + +def test_acknowledgement_is_revision_conditional_and_later_change_is_unseen(tmp_path): + store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=KEY) + store.set_watching("timmy", ITEM, True) + changed = {**ITEM, "updated_at": "2026-08-23T04:00:00Z"} + store.refresh("timmy", [changed]) + + stale = store.acknowledge("timmy", ITEM["repository"], ITEM["number"], ITEM["updated_at"]) + assert stale["items"][0]["has_unseen_change"] is True + seen = store.acknowledge("timmy", ITEM["repository"], ITEM["number"], changed["updated_at"]) + assert seen["items"][0]["has_unseen_change"] is False + + store.refresh("timmy", [{**ITEM, "updated_at": "2026-08-23T05:00:00Z"}]) + assert store.get("timmy")["items"][0]["has_unseen_change"] is True + + +def test_reconfirmed_watch_does_not_mark_an_unseen_change_as_seen(tmp_path): + store = FollowingStore(tmp_path / "following.sqlite3", encryption_key=KEY) + store.set_watching("timmy", ITEM, True) + changed = {**ITEM, "updated_at": "2026-08-23T04:00:00Z"} + store.refresh("timmy", [changed]) + + snapshot = store.set_watching("timmy", changed, True) + + assert snapshot["items"][0]["has_unseen_change"] is True -- 2.43.0