From 8b4bd480cc35dc88b08109e2a838033b6407dfa3 Mon Sep 17 00:00:00 2001 From: timmy Date: Tue, 18 Aug 2026 04:41:08 +0000 Subject: [PATCH] feat: correct recent Today updates in place (Closes #1062) --- frontend/conversation-action-hydrator.js | 29 ++++----- frontend/dashboard.js | 8 +-- frontend/today-progress.js | 45 ++++++++++--- tests/e2e/fake_gitea.py | 52 ++++++++++++++- .../e2e/test_mobile_today_handoff_release.py | 49 +++++++++++++- tests/test_conversation_action_hydrator.py | 15 +++++ tests/test_today_progress.py | 65 +++++++++++++++++++ 7 files changed, 231 insertions(+), 32 deletions(-) diff --git a/frontend/conversation-action-hydrator.js b/frontend/conversation-action-hydrator.js index 1b22d6e..4dab18a 100644 --- a/frontend/conversation-action-hydrator.js +++ b/frontend/conversation-action-hydrator.js @@ -1,28 +1,23 @@ function createConversationActionHydrator({ load, activate }) { let actions = null; let pending = null; - const wiredRoots = new WeakSet(); + const wired = new WeakSet(); - function ensure() { - if (actions) return Promise.resolve(actions); - if (!pending) { - pending = load().then(() => { - actions = activate(); - return actions; - }).catch(error => { - pending = null; - throw error; - }); - } + async function ensure() { + if (actions) return actions; + if (!pending) pending = load().then(() => actions = activate()).catch(error => { + pending = null; + throw error; + }); return pending; } function show({ root, state, paint, wire, retry }) { if (actions) { retry.hidden = true; - if (!wiredRoots.has(root)) { + if (!wired.has(root)) { wire(actions); - wiredRoots.add(root); + wired.add(root); } paint(state, actions); return Promise.resolve(true); @@ -31,9 +26,9 @@ function createConversationActionHydrator({ load, activate }) { paint(state, null); retry.hidden = true; return ensure().then(controller => { - if (!wiredRoots.has(root)) { + if (!wired.has(root)) { wire(controller); - wiredRoots.add(root); + wired.add(root); } paint(state, controller); return true; @@ -43,7 +38,7 @@ function createConversationActionHydrator({ load, activate }) { }); } - return { show, ready: () => Boolean(actions) }; + return {show,get:ensure}; } if (typeof module !== 'undefined' && module.exports) module.exports = createConversationActionHydrator; diff --git a/frontend/dashboard.js b/frontend/dashboard.js index 0524aa5..7db6368 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -990,7 +990,7 @@ issueOutbox.reconcileBackground(records); authoredOutbox.reconcileBackground(records); notificationReadOutbox.reconcileBackground(records); - }).catch(() => { /* The foreground localStorage outboxes remain available. */ }); + }).catch(() => {}); } const shareParams = new URLSearchParams(location.search); const appShortcut = mobileAppShortcuts.createController({ @@ -1012,7 +1012,7 @@ 'comment-actions': document.querySelector('meta[name="stackchain-feature-comment-actions"]')?.content || '', }, }); - const commentActionHydrator = createConversationActionHydrator({ + const actionHydrator = createConversationActionHydrator({ load: () => commentActionFeatures.load('comment-actions'), activate: () => { commentActions = createCommentActions({ @@ -1968,7 +1968,7 @@ const todayProgressView = createTodayProgressView({ progress:todayProgress, currentTarget:currentTodayProgressTarget, qs, photos:todayProgressPhotos, voice:todayProgressVoice, - activity:mountTodayProgressActivity(qs, fetchReviewJson), + activity:mountTodayProgressActivity(qs,fetchReviewJson,actionHydrator,renderMarkdown), announce:message => { qs('#my-work-action-status').textContent = message; }, onAdmitted:() => refreshMyWorkView(), }); @@ -3871,7 +3871,7 @@ function showConversationWithActions(kind, state) { const surface = conversationActionSurfaces[kind]; latestConversationStates[kind] = state; - return commentActionHydrator.show({ + return actionHydrator.show({ root:qs(surface.selector), state, paint:surface.paint, retry:qs('#retry-' + kind + '-comment-actions'), wire:controller => wireConversationActions(kind, controller), diff --git a/frontend/today-progress.js b/frontend/today-progress.js index aae2969..aa84984 100644 --- a/frontend/today-progress.js +++ b/frontend/today-progress.js @@ -132,10 +132,11 @@ maxLength = 2000, maxItems = 20 }) { return { load, save, discard:identity => save(identity, ''), post }; } -function createTodayProgressActivity({ fetchJson, createPager, paint = () => {}, setStatus = () => {} }) { +function createTodayProgressActivity({ fetchJson, createPager, getActions, onActions, surfaceStatus, paint = () => {}, setStatus = () => {} }) { let requestToken = 0; let pager = null; let target = null; + let actions = null; const pathFor = (value, page) => { const repository = String(value.repository || '').split('/').map(encodeURIComponent).join('/'); @@ -153,6 +154,9 @@ function createTodayProgressActivity({ fetchJson, createPager, paint = () => {}, const page = await loadPage(); if (token !== requestToken || target.identity !== value.identity) return false; pager = createPager({ loadPage }); + actions = typeof getActions === 'function' ? await getActions() : null; + if (token !== requestToken || target.identity !== value.identity) return false; + onActions?.(actions); paint(pager.reset(page)); setStatus(''); return true; @@ -182,29 +186,51 @@ function createTodayProgressActivity({ fetchJson, createPager, paint = () => {}, open, retry:() => target ? open(target) : Promise.resolve(false), loadOlder, - close:() => { requestToken++; target = null; pager = null; setStatus(''); }, + actionHtml:comment => actions?.actionHtml?.(comment) || '', + surface:() => ({ + context:{ kind:target?.kind, item:target }, pager, + status:surfaceStatus, render:paint, + }), + close:() => { requestToken++; target = null; pager = null; actions = null; setStatus(''); }, }; } -function mountTodayProgressActivity(qs, fetchJson) { +function mountTodayProgressActivity(qs, fetchJson, actionSource = {}, render) { + const options = actionSource.get && !actionSource.getActions ? + { getActions:actionSource.get, isOffline:() => !navigator.onLine } : actionSource; + const escape = options.escape || (value => String(value).replace(/[&<>"']/g, + character => ({'&':'&','<':'<','>':'>','"':'"',"'":'''})[character])); + render ||= options.render; + const list = qs('#today-progress-activity-list'); + const optionsStatus = qs('#today-progress-activity-status'); + let actionsWired = false; const activity = createTodayProgressActivity({ fetchJson, - createPager:createConversationPager, + createPager:options.createPager || createConversationPager, + getActions:options.getActions, + surfaceStatus:optionsStatus, + onActions:actions => { + if (actionsWired || !actions?.wire) return; + actions.wire({ root:list, getSurface:() => activity.surface(), + isOffline:options.isOffline || (() => false), escapeHtml:escape }); + actionsWired = true; + }, paint:state => { - const list = qs('#today-progress-activity-list'); list.innerHTML = (state.comments || []).map(comment => { const author = comment.author || comment.user?.login || 'Unknown author'; const timing = comment.created_at ? ' ยท ' + new Date(comment.created_at).toLocaleString() : ''; - return '
' + - escapeHtml(author) + escapeHtml(timing) + '
' + - renderMarkdown(comment.body || 'No message body provided.') + '
'; + return '
' + + escape(author) + escape(timing) + '
' + + render(comment.body || 'No message body provided.') + '
' + + activity.actionHtml(comment) + '
'; }).join(''); qs('#today-progress-activity-count').textContent = state.total ? String(state.comments.length) + ' of ' + String(state.total) + ' messages' : ''; qs('#load-older-today-progress-activity').hidden = !Number.isInteger(state.older_page); }, setStatus:message => { - qs('#today-progress-activity-status').textContent = message; + optionsStatus.textContent = message; qs('#retry-today-progress-activity').hidden = !message.includes('unavailable'); }, }); @@ -368,4 +394,5 @@ if (typeof module !== 'undefined' && module.exports) { module.exports.createView = createTodayProgressView; module.exports.createPhotos = createTodayProgressPhotos; module.exports.createActivity = createTodayProgressActivity; + module.exports.mountActivity = mountTodayProgressActivity; } diff --git a/tests/e2e/fake_gitea.py b/tests/e2e/fake_gitea.py index 9b6e5f6..b36295a 100644 --- a/tests/e2e/fake_gitea.py +++ b/tests/e2e/fake_gitea.py @@ -55,6 +55,8 @@ class FakeGiteaServer(ThreadingHTTPServer): self.issue_creation_ready = Event() self.assigned_issue_numbers = [issue["number"] for issue in AVAILABLE_ISSUES] self.comments: list[tuple[int, str]] = [] + self.activity_comments: dict[int, list[dict]] = {} + self.edited_comments: list[tuple[int, int, str]] = [] self.requests: list[tuple[str, str]] = [] @@ -103,8 +105,21 @@ class FakeGiteaHandler(BaseHTTPRequestHandler): ] issues = assigned if is_assigned_scan else available if is_available_scan else [] self._json(200, issues, **{"X-Total-Count": str(len(issues))}) + elif path.startswith("/api/v1/repos/acme/mobile/issues/comments/"): + try: + comment_id = int(path.rsplit("/", 1)[-1]) + except ValueError: + comment_id = 0 + comment = next((item for comments in self.server.activity_comments.values() + for item in comments if item.get("id") == comment_id), None) + self._json(200, {**comment, "user": USER}) if comment else self._json(404, {"message": "not found"}) elif path.startswith("/api/v1/repos/acme/mobile/issues/") and path.endswith("/comments"): - self._json(200, [], **{"X-Total-Count": "0"}) + try: + number = int(path.split("/")[-2]) + except ValueError: + number = 0 + comments = self.server.activity_comments.get(number, []) + self._json(200, comments, **{"X-Total-Count": str(len(comments))}) elif path.startswith("/api/v1/repos/acme/mobile/issues/") and path.endswith("/dependencies"): self._json(200, []) elif path.startswith("/api/v1/repos/acme/mobile/issues/"): @@ -175,8 +190,43 @@ class FakeGiteaHandler(BaseHTTPRequestHandler): def do_PATCH(self) -> None: # noqa: N802 - BaseHTTPRequestHandler API path = urlsplit(self.path).path + self.server.requests.append(("PATCH", self.path)) length = int(self.headers.get("Content-Length", "0")) payload = json.loads(self.rfile.read(length) or b"{}") + parts = path.strip("/").split("/") + if len(parts) == 8 and parts[:7] == ["api", "v1", "repos", "acme", "mobile", "issues", "comments"]: + try: + comment_id = int(parts[7]) + except ValueError: + self._json(404, {"message": "not found"}) + return + match = next(((number, item) for number, comments in self.server.activity_comments.items() + for item in comments if item.get("id") == comment_id), None) + if match is None: + self._json(404, {"message": "not found"}) + return + number, comment = match + body = str(payload.get("body", "")) + comment["body"] = body + self.server.edited_comments.append((number, comment_id, body)) + self._json(200, {**comment, "user": USER, "updated_at": "2026-08-18T01:00:00Z"}) + return + if len(parts) == 9 and parts[:5] == ["api", "v1", "repos", "acme", "mobile"] and parts[5] == "issues" and parts[7] == "comments": + try: + number, comment_id = int(parts[6]), int(parts[8]) + except ValueError: + self._json(404, {"message": "not found"}) + return + comments = self.server.activity_comments.get(number, []) + comment = next((item for item in comments if item.get("id") == comment_id), None) + if comment is None: + self._json(404, {"message": "not found"}) + return + body = str(payload.get("body", "")) + comment["body"] = body + self.server.edited_comments.append((number, comment_id, body)) + self._json(200, {**comment, "user": USER, "updated_at": "2026-08-18T01:00:00Z"}) + return if not path.startswith("/api/v1/repos/acme/mobile/issues/"): self._json(404, {"message": "not found"}) return diff --git a/tests/e2e/test_mobile_today_handoff_release.py b/tests/e2e/test_mobile_today_handoff_release.py index d999354..2873ed0 100644 --- a/tests/e2e/test_mobile_today_handoff_release.py +++ b/tests/e2e/test_mobile_today_handoff_release.py @@ -46,7 +46,10 @@ def test_release_artifact_plans_hands_off_and_opens_next_mobile_issue(tmp_path: page.locator("#submit-sign-in").click() page.wait_for_url(origin + "/", wait_until="networkidle") - expect(page.locator("#my-work-status")).to_contain_text("2") + try: + expect(page.locator("#my-work-status")).to_contain_text("2") + except AssertionError as error: + raise AssertionError({"browser_errors": browser_errors, "failed_responses": failed_responses}) from error page.locator('[data-mobile-task="work"]').click() expect(page.locator("#plan-today-sheet")).to_be_visible() expect(page.locator("#plan-today-candidates .plan-today-item")).to_have_count(2) @@ -84,6 +87,50 @@ def test_release_artifact_plans_hands_off_and_opens_next_mobile_issue(tmp_path: if page.locator("#plan-today-sheet").is_visible(): page.locator("#cancel-plan-today").click() expect(page.locator("[data-mobile-today-hud]")).to_be_visible() + fake.activity_comments = { + 41: [{ + "id": 701, + "user": {"login": "timmy"}, + "body": "Wrong voice transcript", + "issue_url": f"{fake_url}/api/v1/repos/acme/mobile/issues/41", + "created_at": "2026-08-18T00:00:00Z", + }] + } + page.locator("[data-mobile-today-update]").click() + expect(page.locator("#today-progress-sheet")).to_be_visible() + owned = page.locator('[data-comment-id="701"]') + try: + expect(owned.locator('[data-comment-action="edit"]')).to_be_visible() + except AssertionError as error: + direct = page.evaluate("""async () => { + const response = await fetch('api/v1/repos/acme/mobile/issues/41/comments?limit=20'); + return [response.status, await response.text()]; + }""") + raise AssertionError({ + "activity": page.locator("#today-progress-activity").inner_html(), + "direct": direct, + "requests": fake.requests[-20:], + "browser_errors": browser_errors, + }) from error + owned.locator('[data-comment-action="edit"]').click() + correction = owned.locator(".comment-edit-textarea") + correction.fill("Corrected voice transcript") + owned.locator("[data-comment-edit-save]").click() + try: + expect(owned.locator(".markdown-content")).to_have_text("Corrected voice transcript") + except AssertionError as error: + raise AssertionError({ + "activity": page.locator("#today-progress-activity").inner_html(), + "edited": fake.edited_comments, + "requests": fake.requests[-10:], + "failed_responses": failed_responses, + }) from error + expect(page.locator("#today-progress-sheet")).to_be_visible() + expect(page.locator("[data-mobile-today-hud]")).to_contain_text("Ship mobile capture") + assert fake.edited_comments == [(41, 701, "Corrected voice transcript")] + assert page.evaluate("document.documentElement.scrollWidth <= window.innerWidth") + page.locator("#cancel-today-progress").click() + expect(page.locator("#today-progress-sheet")).to_be_hidden() take_break = page.locator("[data-today-break-open]") expect(take_break).to_be_visible() take_break.click() diff --git a/tests/test_conversation_action_hydrator.py b/tests/test_conversation_action_hydrator.py index b63e186..688173c 100644 --- a/tests/test_conversation_action_hydrator.py +++ b/tests/test_conversation_action_hydrator.py @@ -96,6 +96,21 @@ const options = {{ } +def test_other_focused_surfaces_can_request_the_same_lazy_action_controller(): + script = f""" +const createConversationActionHydrator = require({json.dumps(str(HYDRATOR))}); +let loads=0; const controller={{name:'shared-actions'}}; +const hydrator=createConversationActionHydrator({{ + load:async()=>{{loads++;}}, activate:()=>controller, +}}); +(async()=>{{ + const [first,second]=await Promise.all([hydrator.get(),hydrator.get()]); + process.stdout.write(JSON.stringify({{loads,same:first===second,name:first.name}})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + assert run_node(script) == {"loads": 1, "same": True, "name": "shared-actions"} + + def test_issue_pull_and_update_conversations_trigger_optional_actions_not_startup(): html = (FRONTEND / "index.html").read_text() javascript = (FRONTEND / "dashboard.js").read_text() diff --git a/tests/test_today_progress.py b/tests/test_today_progress.py index 5ffdf65..e808322 100644 --- a/tests/test_today_progress.py +++ b/tests/test_today_progress.py @@ -338,6 +338,70 @@ const target={{identity:'issue:stackchain/dashboard:1060:',kind:'issue',reposito assert output["paints"][-1]["comments"] == [{"id": 1}] +def test_recent_activity_mount_renders_owned_actions_on_the_exact_comment_card(): + script = f""" +const {{mountActivity}}=require({json.dumps(str(TODAY_PROGRESS))}); +const createPager=require({json.dumps(str(ROOT / 'frontend' / 'conversation.js'))}); +class Element {{ + constructor(){{this.innerHTML='';this.textContent='';this.hidden=false;this.listeners={{}};this.scrollHeight=0;this.scrollTop=0;}} + addEventListener(name,callback){{this.listeners[name]=callback;}} +}} +const selectors=['#today-progress-activity-list','#today-progress-activity-count','#load-older-today-progress-activity', + '#today-progress-activity-status','#retry-today-progress-activity']; +const elements=Object.fromEntries(selectors.map(selector=>[selector,new Element()])); +const actions={{actionHtml:comment=>comment.author==='timmy'?'':''}}; +const activity=mountActivity(selector=>elements[selector],async()=>({{ + comments:[{{id:17,author:'timmy',body:'Needs correction',created_at:'2026-08-18T00:00:00Z'}}],page:1,older_page:null,total:1, +}}),{{getActions:async()=>actions,createPager,escape:value=>String(value),render:value=>String(value)}}); +(async()=>{{ + await activity.open({{identity:'issue:stackchain/dashboard:1062:',kind:'issue',repository:'stackchain/dashboard',number:1062}}); + process.stdout.write(JSON.stringify({{html:elements['#today-progress-activity-list'].innerHTML,count:elements['#today-progress-activity-count'].textContent}})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = run_node(script) + assert 'class="today-progress-activity-item issue-comment"' in output["html"] + assert 'data-comment-id="17"' in output["html"] + assert 'data-comment-action="edit"' in output["html"] + assert output["count"] == "1 of 1 messages" + + +def test_recent_activity_edit_control_preserves_a_failed_correction_for_retry(): + script = f""" +const {{mountActivity}}=require({json.dumps(str(TODAY_PROGRESS))}); +const createPager=require({json.dumps(str(ROOT / 'frontend' / 'conversation.js'))}); +const createActions=require({json.dumps(str(ROOT / 'frontend' / 'comment-actions.js'))}); +class Element {{constructor(){{this.innerHTML='';this.textContent='';this.hidden=false;this.listeners={{}};this.scrollHeight=0;this.scrollTop=0;}}addEventListener(n,c){{this.listeners[n]=c;}}}} +const selectors=['#today-progress-activity-list','#today-progress-activity-count','#load-older-today-progress-activity','#today-progress-activity-status','#retry-today-progress-activity']; +const elements=Object.fromEntries(selectors.map(selector=>[selector,new Element()])); +let state={{comments:[{{id:17,author:'timmy',body:'Wrong'}}],page:1,older_page:null,total:1}}; +let attempted=''; +const actions=createActions({{ + getLogin:()=> 'timmy', + fetchJson:async(_path,options)=>{{attempted=JSON.parse(options.body).body;throw new Error('network unavailable');}}, +}}); +const activity=mountActivity(selector=>elements[selector],async()=>state,{{getActions:async()=>actions,createPager,escape:value=>String(value),render:value=>String(value),isOffline:()=>false}}); +const textarea={{value:'Corrected transcript',focuses:0,focus(){{this.focuses++;}}}}; +const save={{disabled:false,listeners:{{}},addEventListener(n,c){{this.listeners[n]=c;}}}}; +const cancel={{listeners:{{}},addEventListener(n,c){{this.listeners[n]=c;}}}}; +const card={{dataset:{{commentId:'17'}},innerHTML:'',querySelector:selector=>({{'.comment-edit-textarea':textarea,'[data-comment-edit-save]':save,'[data-comment-edit-cancel]':cancel}})[selector]}}; +const button={{dataset:{{commentAction:'edit'}},closest:selector=>selector==='[data-comment-action]'?button:card}}; +(async()=>{{ + await activity.open({{identity:'issue:stackchain/dashboard:1062:',kind:'issue',repository:'stackchain/dashboard',number:1062}}); + await elements['#today-progress-activity-list'].listeners.click({{target:button}}); + await save.listeners.click({{currentTarget:save}}); + process.stdout.write(JSON.stringify({{attempted,value:textarea.value,focuses:textarea.focuses,disabled:save.disabled,status:elements['#today-progress-activity-status'].textContent}})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = run_node(script) + assert output == { + "attempted": "Corrected transcript", + "value": "Corrected transcript", + "focuses": 2, + "disabled": False, + "status": "network unavailable", + } + + def test_mobile_progress_sheet_is_accessible_bundled_and_safe_area_aware(): html = (ROOT / "frontend" / "index.html").read_text() css = (ROOT / "frontend" / "dashboard.css").read_text() @@ -385,6 +449,7 @@ def test_mobile_progress_sheet_is_accessible_bundled_and_safe_area_aware(): assert "voice:todayProgressVoice" in dashboard assert "mountTodayProgressActivity" in dashboard assert "activity:mountTodayProgressActivity" in dashboard + assert "fetchReviewJson,actionHydrator" in dashboard assert ".today-progress-activity-list" in css assert ".today-progress-activity-actions button" in css assert "min-height:44px" in css