Merge pull request 'Load mobile update conversations without blocking core detail' (#762) from timmy/761-progressive-update-conversation into main
This commit is contained in:
commit
6528b2e73f
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -763,6 +763,7 @@
|
|||
<button class="update-new-activity-jump" id="jump-update-new-activity" type="button" hidden>Jump to new activity</button>
|
||||
<button class="conversation-more" id="load-older-update-comments" type="button" hidden>Load older messages</button>
|
||||
<div id="update-conversation-status" class="small" aria-live="assertive"></div>
|
||||
<button class="update-retry" id="retry-update-conversation" type="button" hidden>Retry conversation</button>
|
||||
<details>
|
||||
<summary><h2>Subject context</h2></summary>
|
||||
<div class="update-sheet-content muted markdown-content" id="update-subject-body"></div>
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user