From a9d586e68912da5bf96654eabbfd54caf8f5b503 Mon Sep 17 00:00:00 2001 From: timmy Date: Thu, 13 Aug 2026 18:25:21 +0000 Subject: [PATCH] feat: load update conversations progressively (Closes #761) --- frontend/dashboard.js | 11 ++++++- frontend/index.html | 1 + frontend/my-work.js | 31 ++++++++++++++++++ src/gitea_proxy.py | 12 +++---- src/main.py | 2 +- tests/test_gitea_notifications.py | 14 +------- tests/test_my_work.py | 54 ++++++++++++++++++++++++++++++- tests/test_notification_detail.py | 31 ++++++++++++------ 8 files changed, 123 insertions(+), 33 deletions(-) diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 7f49d4a..b2ee2d4 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -761,8 +761,9 @@ } async function fetchNotificationConversation(notificationId, page) { + const pageQuery = Number.isInteger(page) ? '&page=' + encodeURIComponent(page) : ''; const response = await fetch('api/v1/notifications/' + encodeURIComponent(notificationId) + - '/conversation?page=' + encodeURIComponent(page) + '&limit=20', { + '/conversation?limit=20' + pageQuery, { headers: { Accept: 'application/json' }, }); const payload = await response.json().catch(() => ({})); @@ -936,6 +937,7 @@ qs('#update-sheet-title').textContent = item.title || 'Unread update'; qs('#update-comments').textContent = ''; qs('#update-conversation-status').textContent = ''; + qs('#retry-update-conversation').hidden = true; qs('#load-older-update-comments').hidden = true; qs('#update-subject-body').textContent = ''; qs('#update-subject-type').textContent = item.subject_type || 'Update'; @@ -975,6 +977,10 @@ qs('#retry-update-load').hidden = true; }, onConversation: renderUpdateConversation, + onConversationStatus: message => { + qs('#update-conversation-status').textContent = message; + qs('#retry-update-conversation').hidden = !message.startsWith('Conversation temporarily unavailable.'); + }, onItems: items => { const readId = selectedUpdate?.notification_id; lastMyWork = items; @@ -5527,6 +5533,9 @@ qs('#retry-update-load').addEventListener('click', () => { if (selectedUpdate) notificationReader.open(selectedUpdate, lastMyWork); }); + qs('#retry-update-conversation').addEventListener('click', () => { + notificationReader.retryConversation(); + }); qs('#load-older-update-comments').addEventListener('click', async () => { const button = qs('#load-older-update-comments'); const panel = qs('#update-sheet .update-sheet-panel'); diff --git a/frontend/index.html b/frontend/index.html index e9a804a..81b5914 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -763,6 +763,7 @@
+

Subject context
diff --git a/frontend/my-work.js b/frontend/my-work.js index acaf87f..5c0f4b0 100644 --- a/frontend/my-work.js +++ b/frontend/my-work.js @@ -379,6 +379,7 @@ function createNotificationReader({ loadSaved = () => null, loadConversation = null, onConversation = () => {}, + onConversationStatus = () => {}, createPager = typeof createConversationPager === 'function' ? createConversationPager : null, getScope = () => '', }) { @@ -390,6 +391,29 @@ function createNotificationReader({ let prefetched = null; let prefetchKey = ''; + async function hydrateConversation(item, version) { + if (!loadConversation || !createPager || !item || offlineHydrated) return false; + onConversationStatus('Loading conversation…'); + try { + const initial = await loadConversation(item.notification_id, null); + if (selected !== item || version !== loadVersion) return false; + conversationPager = createPager({ + loadPage: page => loadConversation(item.notification_id, page), + }); + const state = conversationPager.reset(initial); + onConversation(state); + onConversationStatus(state.comments.length + ' of ' + + Math.max(state.total || 0, state.comments.length) + ' messages loaded.'); + return true; + } catch (_error) { + if (selected === item && version === loadVersion) { + conversationPager = null; + onConversationStatus('Conversation temporarily unavailable. Retry.'); + } + return false; + } + } + function prefetch(item) { if (!item) { prefetched = null; prefetchKey = ''; return false; } const notificationId = item?.notification_id; @@ -446,6 +470,9 @@ function createNotificationReader({ conversationPager = null; } onStatus('Update ready.'); + if (!offlineHydrated && detail.conversation_available) { + void hydrateConversation(item, version); + } return true; } catch (_error) { if (selected === item && version === loadVersion) { @@ -458,6 +485,10 @@ function createNotificationReader({ return { open, prefetch, + retryConversation() { + if (!selected || offlineHydrated) return Promise.resolve(false); + return hydrateConversation(selected, loadVersion); + }, commentPager() { return conversationPager; }, diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 6c42ff9..9424f6d 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -948,19 +948,15 @@ async def notification_detail(thread_id: int) -> dict: ) if supported_conversation: assert conversation_match is not None - subject_detail, comment, conversation, subscription = await asyncio.gather( + subject_detail, comment, subscription = await asyncio.gather( fetch(subject_path), fetch(comment_path) if comment_path else asyncio.sleep(0, result={}), - issue_conversation_page( - conversation_match.group(1), int(conversation_match.group(3)) - ), _notification_subscription(conversation_match.group(1), conversation_match.group(3)), ) else: - subject_detail, comment, conversation, subscription = await asyncio.gather( + subject_detail, comment, subscription = await asyncio.gather( fetch(subject_path) if subject_path else asyncio.sleep(0, result={}), fetch(comment_path) if comment_path else asyncio.sleep(0, result={}), - asyncio.sleep(0, result={"comments": [], "page": 1, "older_page": None, "total": 0}), asyncio.sleep(0, result={}), ) subject_detail = subject_detail if isinstance(subject_detail, dict) else {} @@ -1028,12 +1024,12 @@ async def notification_detail(thread_id: int) -> dict: and subscription.get("subscribed") is True and subscription.get("ignored") is not True ), - "conversation": conversation, + "conversation_available": bool(supported_conversation), } async def notification_conversation_page( - thread_id: int, page: int, limit: int = 20 + thread_id: int, page: int | None, limit: int = 20 ) -> dict: thread = await fetch(f"notifications/threads/{thread_id}") if not isinstance(thread, dict): diff --git a/src/main.py b/src/main.py index c8ca7a1..4352f1f 100644 --- a/src/main.py +++ b/src/main.py @@ -3634,7 +3634,7 @@ async def notification_thread_detail( @app.get("/api/v1/notifications/{thread_id}/conversation") async def notification_thread_conversation( thread_id: int = PathParam(gt=0), - page: int = Query(ge=1), + page: int | None = Query(default=None, ge=1), limit: int = Query(default=20, ge=1, le=50), ) -> JSONResponse: try: diff --git a/tests/test_gitea_notifications.py b/tests/test_gitea_notifications.py index cf5316b..5f06716 100644 --- a/tests/test_gitea_notifications.py +++ b/tests/test_gitea_notifications.py @@ -284,7 +284,6 @@ async def test_notification_detail_loads_subject_and_latest_comment_for_inbox_re "http://127.0.0.1:3000/api/v1/notifications/threads/42", "http://127.0.0.1:3000/api/v1/repos/stackchain/api/issues/7", "http://127.0.0.1:3000/api/v1/repos/stackchain/api/issues/comments/9", - "http://127.0.0.1:3000/api/v1/repos/stackchain/api/issues/7/comments?limit=20&page=1", "http://127.0.0.1:3000/api/v1/repos/stackchain/api/issues/7/subscriptions/check", ] assert result == { @@ -304,18 +303,7 @@ async def test_notification_detail_loads_subject_and_latest_comment_for_inbox_re "issue": {"number": 7, "assignees": [], "claimable": True}, "acknowledge_supported": True, "mute_supported": True, - "conversation": { - "comments": [{ - "id": 9, - "author": "alexander", - "body": "Logs point to the worker timeout.", - "created_at": "2026-08-06T12:30:00Z", - "url": "https://forge.example/stackchain/api/issues/7#issuecomment-9", - }], - "page": 1, - "older_page": None, - "total": 1, - }, + "conversation_available": True, } diff --git a/tests/test_my_work.py b/tests/test_my_work.py index 90bb9ce..f824f3e 100644 --- a/tests/test_my_work.py +++ b/tests/test_my_work.py @@ -5169,6 +5169,56 @@ reader.open(firstItem).then(async () => {{ assert output["states"] == [[41], [90]] +def test_notification_reader_renders_core_before_conversation_and_retries_failure_in_place(): + script = f""" +const build = require({json.dumps(str(MY_WORK))}); +const createPager = require({json.dumps(str(CONVERSATION))}); +const events = []; +let attempts = 0; +let release; +const reader = build.createNotificationReader({{ + load: async id => ({{id, title:'Core ready', conversation_available:true}}), + createPager, + loadConversation: () => {{ + attempts += 1; + if (attempts === 1) return Promise.reject(new Error('comments offline')); + return new Promise(resolve => {{ release = () => resolve({{ + comments:[{{id:47, body:'Newest'}}], page:3, older_page:2, total:47, + }}); }}); + }}, + markRead: async () => {{}}, onOpen: () => events.push('open'), + onDetail: detail => events.push(['detail', detail.title]), + onConversation: state => events.push(['conversation', state.comments.map(item => item.id)]), + onConversationStatus: status => events.push(['conversation-status', status]), + onItems: () => {{}}, onStatus: status => events.push(['status', status]), onClose: () => {{}}, +}}); +(async () => {{ + const opened = await reader.open({{notification_id:42}}); + const retrying = reader.retryConversation(); + await Promise.resolve(); + const beforeRelease = events.slice(); + release(); + const retried = await retrying; + process.stdout.write(JSON.stringify({{opened, retried, attempts, beforeRelease, events}})); +}})(); +""" + output = json.loads(subprocess.run( + ["node", "-e", script], check=True, capture_output=True, text=True + ).stdout) + + assert output["opened"] is True + assert output["retried"] is True + assert output["attempts"] == 2 + assert ["detail", "Core ready"] in output["beforeRelease"] + assert ["status", "Update ready."] in output["beforeRelease"] + assert ["conversation-status", "Conversation temporarily unavailable. Retry."] in output["beforeRelease"] + assert ["conversation", [47]] not in output["beforeRelease"] + assert output["events"][-2:] == [ + ["conversation", [47]], + ["conversation-status", "1 of 47 messages loaded."], + ] + + def test_notification_reader_hydrates_saved_conversation_without_server_state_actions(): script = f""" const build = require({json.dumps(str(MY_WORK))}); @@ -7126,7 +7176,9 @@ async def test_mobile_update_sheet_renders_and_pages_the_complete_conversation() assert 'id="load-older-update-comments"' in update_sheet assert 'id="update-conversation-status"' in update_sheet assert '.conversation-more' in html and 'min-height:44px' in html - assert "'/conversation?page='" in html + assert "'/conversation?limit=20' + pageQuery" in html + assert 'id="retry-update-conversation"' in update_sheet + assert "notificationReader.retryConversation()" in html assert "loadConversation: fetchNotificationConversation" in html assert "onConversation: renderUpdateConversation" in html assert "notificationReader.loadOlder()" in html diff --git a/tests/test_notification_detail.py b/tests/test_notification_detail.py index 1012025..b5b405c 100644 --- a/tests/test_notification_detail.py +++ b/tests/test_notification_detail.py @@ -7,7 +7,7 @@ from src import gitea_proxy, main @pytest.mark.anyio -async def test_notification_detail_opens_the_newest_conversation_page_in_chronological_order(monkeypatch): +async def test_notification_detail_returns_core_without_loading_conversation(monkeypatch): requested_pages = [] def comment(comment_id): @@ -53,14 +53,9 @@ async def test_notification_detail_opens_the_newest_conversation_page_in_chronol finally: await gitea_proxy.stop_client() - assert requested_pages == [1, 3] - assert [item["id"] for item in result["conversation"]["comments"]] == list(range(41, 48)) - assert result["conversation"] == { - "comments": result["conversation"]["comments"], - "page": 3, - "older_page": 2, - "total": 47, - } + assert requested_pages == [] + assert result["conversation_available"] is True + assert "conversation" not in result assert result["issue"] == { "number": 7, "assignees": [], @@ -130,6 +125,24 @@ async def test_notification_conversation_api_loads_one_bounded_older_page(monkey assert calls == [(42, 2, 20)] +@pytest.mark.anyio +async def test_notification_conversation_api_opens_newest_page_when_page_is_omitted(monkeypatch): + calls = [] + + async def conversation(thread_id, page, limit): + calls.append((thread_id, page, limit)) + return {"comments": [{"id": 47}], "page": 3, "older_page": 2, "total": 47} + + monkeypatch.setattr(main.gitea_proxy, "notification_conversation_page", conversation) + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/api/v1/notifications/42/conversation?limit=20") + + assert response.status_code == 200 + assert response.json()["page"] == 3 + assert calls == [(42, None, 20)] + + @pytest.mark.anyio async def test_notification_detail_timeout_is_sanitized_and_retryable(monkeypatch): async def detail(_thread_id):