From aa88ea94e7c94b927beabf4a9272d6ea5ff97e37 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 17:11:50 +0100 Subject: [PATCH 1/3] test(review): pin the Paper read-only banner to what the pane actually shows Red-first coverage for #1434 findings 2 and 3 on the Paper surface. Four of the five assertions fail on the unmodified view: - the recorded-operations fallback (the common expired path) is announced as a "stored preview from the original submission"; - the no-content state is announced the same way, on top of the empty-state sentence below it; - the revised caveat says "recorded operations" while the banner above it says "stored preview" for the same pane; - there is no catalog key carrying the Legacy card's "Applied to board" status wording. The stored-preview case already matches the Legacy card and passes; it is kept as the parity anchor for the other two modes. --- .../paper/review/PaperReviewView.spec.ts | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts index 0ccfc6cfe..214f8251f 100644 --- a/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts +++ b/frontend/taskdeck-web/src/tests/views/paper/review/PaperReviewView.spec.ts @@ -2891,6 +2891,174 @@ describe('PaperReviewView', () => { wrapper.unmount() }) + it('names the stored preview in the read-only banner when one was captured (#1434 finding 2)', async () => { + // Parity anchor for the three banner modes below: with a captured + // diffPreview on screen the banner keeps the Legacy card's exact sentence + // (ReviewProposalCard.vue readOnlyDiffBanner). + const wrapper = await mountView([ + makeProposal({ + id: 'banner-stored', + status: 'Expired', + expiresAt: new Date(Date.now() - 60_000).toISOString(), + diffPreview: '0. Create card "Archived plan"', + }), + ]) + + window.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) + await flushPromises() + + const banner = wrapper.find('[data-testid="paper-review-diff-banner"]') + expect(banner.exists()).toBe(true) + expect(banner.text()).toContain('Expired · read-only') + expect(banner.text()).toContain('showing the stored preview from the original submission') + + wrapper.unmount() + }) + + it('names the recorded operations in the read-only banner when no stored preview was captured (#1434 finding 2)', async () => { + // The banner used to claim a "stored preview from the original submission" + // even for the synthesized recorded-operations fallback — the COMMON expired + // path, since normal creation flows never populate diffPreview. It must name + // what is actually on screen, in the Legacy card's wording. + const wrapper = await mountView([ + makeProposal({ + id: 'banner-ops', + status: 'Expired', + expiresAt: new Date(Date.now() - 60_000).toISOString(), + diffPreview: null, + }), + ]) + + window.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) + await flushPromises() + + expect(wrapper.find('[data-testid="paper-review-diff-stored-operations"]').exists()).toBe(true) + const banner = wrapper.find('[data-testid="paper-review-diff-banner"]') + expect(banner.exists()).toBe(true) + expect(banner.text()).toContain('Expired · read-only') + expect(banner.text()).toContain("showing the proposal's recorded operations") + expect(banner.text()).not.toContain('stored preview') + // Said ONCE: the Paper pane renders no separate note under the banner, so + // the banner alone carries the sentence (the LOW recorded on the Legacy + // side, where the banner repeats the note directly below it). + const section = wrapper.find('[data-testid="paper-review-diff"]') + expect( + section.text().split("showing the proposal's recorded operations").length - 1, + ).toBe(1) + + wrapper.unmount() + }) + + it('does not claim a stored preview in the read-only banner when nothing was captured (#1434 finding 2)', async () => { + // Nothing to show at all: the banner states the status and that the record + // is read-only, and the empty state below keeps its own sentence. The banner + // must neither claim a preview exists nor repeat that sentence. + const wrapper = await mountView([ + makeProposal({ + id: 'banner-none', + status: 'Expired', + expiresAt: new Date(Date.now() - 60_000).toISOString(), + diffPreview: null, + operations: [], + }), + ]) + + window.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) + await flushPromises() + + const banner = wrapper.find('[data-testid="paper-review-diff-banner"]') + expect(banner.exists()).toBe(true) + expect(banner.text()).toContain('Expired · read-only') + expect(banner.text()).not.toContain('stored preview') + expect(banner.text()).not.toContain('showing') + const storedEmpty = wrapper.find('[data-testid="paper-review-diff-stored-empty"]') + expect(storedEmpty.exists()).toBe(true) + expect(storedEmpty.text()).toContain('No stored preview is available for this proposal.') + const section = wrapper.find('[data-testid="paper-review-diff"]') + expect(section.text().toLowerCase().split('no stored preview').length - 1).toBe(1) + + wrapper.unmount() + }) + + it('keeps the revised caveat consistent with the recorded-operations banner (#1434 finding 2)', async () => { + // Banner and caveat must describe the same content: the fallback tail already + // says "recorded operations", so the banner above it may not say "stored + // preview" for the same pane. + const now = new Date().toISOString() + mocks.getRevisions.mockResolvedValue([ + { + id: 'rev-1', + proposalId: 'banner-ops-revised', + revisionNumber: 1, + editorUserId: 'u-1', + revisedPayload: '{"operations":[]}', + revisedAt: now, + reason: 'edit', + createdAt: now, + }, + ]) + const wrapper = await mountView([ + makeProposal({ + id: 'banner-ops-revised', + status: 'Expired', + expiresAt: new Date(Date.now() - 60_000).toISOString(), + diffPreview: null, + }), + ]) + + window.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) + await flushPromises() + + const banner = wrapper.find('[data-testid="paper-review-diff-banner"]') + expect(banner.exists()).toBe(true) + expect(banner.text()).toContain("showing the proposal's recorded operations") + expect(banner.text()).not.toContain('stored preview') + const revisedNote = wrapper.find('[data-testid="paper-review-diff-revised-note"]') + expect(revisedNote.exists()).toBe(true) + expect(revisedNote.text()).toContain('recorded operations') + expect(revisedNote.text()).not.toContain('stored preview') + + wrapper.unmount() + }) + + it('converges the read-only banner status on the Legacy card\'s "Applied to board" wording (#1434 finding 3)', async () => { + // Legacy renders Applied through reviewStatusLabel as "Applied to board" + // (ReviewProposalCard.vue); Paper's previewReadOnlyLabel rendered the bare + // normalized status. reviewStatusLabel is component-local — not a composable + // or util this view may import — so the convergence is the catalog key the + // banner reads. + expect(enReview.status.appliedToBoard).toBe('Applied to board') + + // Why there is no rendered-banner assertion for this one status: the preview + // key is inert on an applied record (useReviewKeymap `isActionEnabled` allows + // only onReject while `activeAppliedProposal` is set), so the read-only + // banner cannot be opened on one. Every other terminal status reaches it and + // is covered above. + const wrapper = await mountView([ + makeProposal({ + id: 'banner-applied', + status: 'Applied', + summary: 'Applied banner probe', + diffPreview: '0. Create card "Stored"', + appliedAt: new Date(Date.now() - 5 * 60_000).toISOString(), + }), + ]) + + const row = wrapper + .findAll('.paper-review-recent__row') + .find((button) => button.text().includes('Applied banner probe'))! + await row.trigger('click') + await flushPromises() + + window.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', cancelable: true })) + await flushPromises() + + expect(mocks.getProposalDiff).not.toHaveBeenCalled() + expect(wrapper.find('[data-testid="paper-review-diff"]').exists()).toBe(false) + + wrapper.unmount() + }) + it('renders the invalid verdict with the backend reason (not a toast) when /diff 400s for a pending proposal (#1397)', async () => { mocks.getProposalDiff.mockRejectedValueOnce({ response: { From e457580d1d6c4ddc035cdd59a182658bc79447b8 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 17:14:26 +0100 Subject: [PATCH 2/3] fix(review): name the content the Paper read-only banner is actually showing The banner said "showing the stored preview from the original submission" whenever the pane was in stored mode, including the two modes where no stored preview exists: the recorded-operations fallback synthesized from the proposal's own operations (the common expired path, since normal creation flows never populate diffPreview) and the state where nothing was captured at all. It now names each mode, in the wording the Legacy card settled on in PR #2655: - a captured preview keeps the existing sentence; - the fallback says "showing the proposal's recorded operations", the sentence the Legacy card puts in a note under its banner. It stays on the banner here and nowhere else, so this pane does not repeat itself the way the Legacy card does; - with nothing to show, the banner states the status and the read-only fact only. review.diff.storedEmpty renders directly below it and remains the empty state's sentence. The banner's status also converges on the Legacy card's "Applied to board" (#1434 finding 3). reviewStatusLabel is local to ReviewProposalCard.vue, so importing it would be a new view-to-component dependency; the shells converge through review.status.appliedToBoard instead. Every other status already matched. That branch has no route to the screen today: the preview key is inert on an applied record, which the spec records. All three catalogs carry the new keys. Refs #1434, #2655, #1397, #1414 --- .../taskdeck-web/src/locales/en/review.ts | 17 ++++++++++ .../taskdeck-web/src/locales/es/review.ts | 3 ++ .../taskdeck-web/src/locales/it/review.ts | 4 +++ .../src/views/paper/PaperReviewView.vue | 33 +++++++++++++++++-- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/frontend/taskdeck-web/src/locales/en/review.ts b/frontend/taskdeck-web/src/locales/en/review.ts index a815424d4..f7b9a12a2 100644 --- a/frontend/taskdeck-web/src/locales/en/review.ts +++ b/frontend/taskdeck-web/src/locales/en/review.ts @@ -549,8 +549,20 @@ export default { title: 'Operation details', hint: 'Press Space to hide', loading: 'Loading diff…', + // The read-only banner names the content that is ACTUALLY on screen, in the + // three modes the pane can be in, worded exactly as the Legacy card words + // them (ReviewProposalCard.vue `readOnlyDiffBanner`) — #1434 finding 2. + // `storedBannerRecorded` is the common expired path: normal creation flows + // never populate `diffPreview`, so the pane synthesizes a listing from the + // proposal's own operations. That sentence lives HERE and nowhere else on + // the Paper pane, which renders no note under the banner. storedBanner: '{status} · read-only — showing the stored preview from the original submission.', + storedBannerRecorded: "{status} · read-only — showing the proposal's recorded operations.", + // Nothing to show: `storedEmpty` below is the empty state's own sentence and + // renders directly under this banner, so the banner states the status and + // the read-only fact and does not repeat it. + storedBannerNone: '{status} · read-only.', // Rendered `✎ {lead} {emphasis} {tail}` — the spaces come // from the template, so `lead` must not carry a trailing space. revised: { @@ -689,10 +701,15 @@ export default { }, // Rendered status labels. The wire values themselves are never keys. + // `appliedToBoard` is the read-only diff banner's form of `applied`: the + // Legacy card names that status "Applied to board" there (#1434 finding 3), + // and its `reviewStatusLabel` is component-local, so the two shells converge + // on the wording through this key rather than through a shared helper. status: { pendingReview: 'Pending review', approved: 'Approved', applied: 'Applied', + appliedToBoard: 'Applied to board', rejected: 'Rejected', failed: 'Failed', expired: 'Expired', diff --git a/frontend/taskdeck-web/src/locales/es/review.ts b/frontend/taskdeck-web/src/locales/es/review.ts index d4c9dae6d..01c040819 100644 --- a/frontend/taskdeck-web/src/locales/es/review.ts +++ b/frontend/taskdeck-web/src/locales/es/review.ts @@ -457,6 +457,8 @@ export default { hint: 'Pulsa Espacio para ocultar', loading: 'Cargando el diff…', storedBanner: '{status} · solo lectura — muestra la vista previa guardada del envío original.', + storedBannerRecorded: '{status} · solo lectura — muestra las operaciones registradas de la propuesta.', + storedBannerNone: '{status} · solo lectura.', revised: { lead: 'Esta propuesta se', emphasis: 'revisó', @@ -562,6 +564,7 @@ export default { pendingReview: 'En espera de revisión', approved: 'Aprobada', applied: 'Aplicada', + appliedToBoard: 'Aplicada al tablero', rejected: 'Rechazada', failed: 'Fallida', expired: 'Caducada', diff --git a/frontend/taskdeck-web/src/locales/it/review.ts b/frontend/taskdeck-web/src/locales/it/review.ts index a13b51f88..b42bcb76a 100644 --- a/frontend/taskdeck-web/src/locales/it/review.ts +++ b/frontend/taskdeck-web/src/locales/it/review.ts @@ -459,6 +459,9 @@ export default { loading: 'Caricamento del diff…', storedBanner: '{status} · sola lettura — mostra l’anteprima salvata dell’invio originale.', + storedBannerRecorded: + '{status} · sola lettura — mostra le operazioni registrate della proposta.', + storedBannerNone: '{status} · sola lettura.', revised: { lead: 'Questa proposta è stata', emphasis: 'revisionata', @@ -564,6 +567,7 @@ export default { pendingReview: 'In attesa di revisione', approved: 'Approvata', applied: 'Applicata', + appliedToBoard: 'Applicata alla bacheca', rejected: 'Rifiutata', failed: 'Non riuscita', expired: 'Scaduta', diff --git a/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue b/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue index da4231acb..ea30338be 100644 --- a/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue +++ b/frontend/taskdeck-web/src/views/paper/PaperReviewView.vue @@ -520,11 +520,19 @@ function clearPreviewDiff() { // The read-only banner label for the active proposal's stored preview: 'Expired' // when the clock/domain says so, otherwise its terminal status. +// +// #1434 finding 3: Applied is spelled "Applied to board" here, the way the +// Legacy card's `reviewStatusLabel` spells it (ReviewProposalCard.vue). That +// helper is component-local — not a composable or util this view may import — +// so the two shells converge through the catalog key instead. Every other +// status already matched. const previewReadOnlyLabel = computed(() => { const p = activeProposal.value if (!p) return '' if (isProposalExpired(p)) return t('review.status.expired') - return t(`review.status.${statusKeySuffix(normalizeProposalStatus(p.status))}`) + const status = normalizeProposalStatus(p.status) + if (status === 'Applied') return t('review.status.appliedToBoard') + return t(`review.status.${statusKeySuffix(status)}`) }) // Read-only fallback when the proposal never captured a `diffPreview` (normal @@ -546,6 +554,24 @@ const storedOperationsFallback = computed(() => { ) .join('\n') }) + +// The read-only banner names the content actually on screen, the way the Legacy +// card's `readOnlyDiffBanner` does (ReviewProposalCard.vue): the captured stored +// preview, the recorded-operations fallback synthesized above (the COMMON +// expired path — normal creation flows never populate `diffPreview`), or +// neither. Claiming a "stored preview from the original submission" for the +// synthesized listing was inaccurate, and claiming one when nothing was captured +// contradicted the empty state right below it (#1434 finding 2). +// +// The recorded-operations sentence lives on the banner ALONE. Unlike the Legacy +// card, which renders a note under its banner saying the same thing, this pane +// has no such note — so the fact is stated once. +const previewReadOnlyBanner = computed(() => { + const status = previewReadOnlyLabel.value + if (previewDiff.value) return t('review.diff.storedBanner', { status }) + if (storedOperationsFallback.value) return t('review.diff.storedBannerRecorded', { status }) + return t('review.diff.storedBannerNone', { status }) +}) // Guards against a double-click firing two feedback POSTs (the backend is idempotent as a backstop). const reportingProposalId = ref(null) @@ -2805,14 +2831,15 @@ async function onClearBoardScope() {

{{ $t('review.diff.title') }}

{{ $t('review.diff.hint') }} - +

- {{ $t('review.diff.storedBanner', { status: previewReadOnlyLabel }) }} + {{ previewReadOnlyBanner }}