From e77f6067fa38b9356d6b7b889a0c8031d5b6c7c6 Mon Sep 17 00:00:00 2001 From: ZdenekSrotyr <139972147+ZdenekSrotyr@users.noreply.github.com> Date: Fri, 15 May 2026 20:05:21 +0200 Subject: [PATCH] feat(memory): bulk-edit batch bar on All Items tab (#129) (#325) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #62 / PR #126 — that PR shipped the bulk-edit batch bar in the Review tab only and deferred the symmetric bar on All Items. This adds it. Scope: - New .batch-bar block in #tab-all with selectAllAll, selectedCountAll, and the five bulk-edit buttons (Move to category / Move to domain / Add tag / Remove tag / Set audience). - renderItemCard signature: third param widened from boolean isReview to a 'review' | 'all' | 'browse' mode enum so the Browse tab's call site can explicitly suppress the row checkbox. The earlier iteration of this PR widened the checkbox condition without auditing other callers, which left the Browse tab with orphan checkboxes that fired updateSelectionCount('all') against an invisible tab. Adversarial-review fix. - updateSelectionCount('all') toggles the *BtnAll set; renderAllItems resets the header checkbox + recomputes counts on every re-render so stale selection state can't survive a list refresh. Approve / Reject stay scoped to Review per the issue's scope decision — status-change actions belong with the per-row action buttons or the keyboard workflow in Review. Existing JS plumbing already assumed tab-aware selection (getSelectedIds(tab), toggleSelectAll(tab), openBulkEditModal reads currentTab); the All-items DOM and the *BtnAll ID set are the only additions. Tests in tests/test_admin_memory_page_all_items_batch_bar.py: - test_admin_page_renders_all_items_batch_bar — all five button IDs + the select-all checkbox + the toggleSelectAll('all') callback are present on the rendered admin page. - test_all_items_bar_omits_approve_reject — Review-only Approve / Reject IDs do not appear with the All suffix (scope guard). - test_browse_tab_omits_row_checkbox — regression guard for the Browse-tab orphan checkbox: confirms the call site uses 'browse' mode and renderItemCard omits the checkbox markup on that branch. Closes #129. --- CHANGELOG.md | 10 +++ app/web/templates/admin_corporate_memory.html | 62 +++++++++++--- ...t_admin_memory_page_all_items_batch_bar.py | 84 +++++++++++++++++++ 3 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 tests/test_admin_memory_page_all_items_batch_bar.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 503a41d..dad6005 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C ## [Unreleased] +### Added +- **Corporate Memory — bulk-edit batch bar on the All Items tab.** + Symmetric to the Review-tab bar shipped in #126; row checkboxes, + "Select all" header, and the five bulk-edit actions (Move to + category / Move to domain / Add tag / Remove tag / Set audience) + now appear on `/corporate-memory-admin` All Items as well. Approve + / Reject stay scoped to Review per #129's scope decision (status + flips belong with the per-row actions or the keyboard workflow). + Closes #129. + ## [0.54.19] — 2026-05-15 ### Changed diff --git a/app/web/templates/admin_corporate_memory.html b/app/web/templates/admin_corporate_memory.html index cedff12..246f1c8 100644 --- a/app/web/templates/admin_corporate_memory.html +++ b/app/web/templates/admin_corporate_memory.html @@ -967,6 +967,25 @@
+ +
+ + +
+ + + + + +
+
` : ''} + ${mode === 'browse' ? '' : ``}
@@ -2085,7 +2118,7 @@ ${item.domain ? `${escapeHtml(item.domain)}` : ''} ${item.confidence ? `${Math.round(item.confidence*100)}%` : ''} ${item.source_type === 'user_verification' ? 'Verified' : ''} - ${!isReview ? `${escapeHtml(status)}` : ''} + ${mode !== 'review' ? `${escapeHtml(status)}` : ''} Added ${escapeHtml(dateStr)}
@@ -2374,15 +2407,20 @@ function updateSelectionCount(tab) { const ids = getSelectedIds(tab); const countEl = document.getElementById(tab === 'review' ? 'selectedCountReview' : 'selectedCountAll'); - const approveBtn = document.getElementById(tab === 'review' ? 'batchApproveBtn' : 'batchApproveBtnAll'); - const rejectBtn = document.getElementById(tab === 'review' ? 'batchRejectBtn' : 'batchRejectBtnAll'); + // Approve / Reject buttons exist only in the Review tab — All Items + // batch bar (issue #129) deliberately omits them (status changes belong + // to per-row buttons or the keyboard workflow in Review). + const approveBtn = tab === 'review' ? document.getElementById('batchApproveBtn') : null; + const rejectBtn = tab === 'review' ? document.getElementById('batchRejectBtn') : null; if (countEl) countEl.textContent = ids.length > 0 ? `${ids.length} selected` : ''; if (approveBtn) approveBtn.disabled = ids.length === 0; if (rejectBtn) rejectBtn.disabled = ids.length === 0; - // Bulk-edit buttons (issue #62) — enable when at least one item ticked. - ['batchMoveCategoryBtn', 'batchMoveDomainBtn', 'batchAddTagBtn', 'batchRemoveTagBtn', 'batchSetAudienceBtn'].forEach(id => { - const btn = document.getElementById(id); + // Bulk-edit buttons (issue #62 + #129) — Review tab uses bare IDs; + // All Items tab adds the "All" suffix. Each tab toggles its own set. + const suffix = tab === 'review' ? '' : 'All'; + ['batchMoveCategoryBtn', 'batchMoveDomainBtn', 'batchAddTagBtn', 'batchRemoveTagBtn', 'batchSetAudienceBtn'].forEach(base => { + const btn = document.getElementById(base + suffix); if (btn) btn.disabled = ids.length === 0; }); } diff --git a/tests/test_admin_memory_page_all_items_batch_bar.py b/tests/test_admin_memory_page_all_items_batch_bar.py new file mode 100644 index 0000000..6e442a1 --- /dev/null +++ b/tests/test_admin_memory_page_all_items_batch_bar.py @@ -0,0 +1,84 @@ +"""GET /admin/corporate-memory page — All Items tab batch bar (issue #129). + +Follow-up to #62 / PR #126 which shipped the bulk-edit batch bar in the +Review tab only. This test guards the symmetric bar on the All Items tab: + +- batch-bar block visible on page render (regardless of pending count) +- the 5 bulk-edit actions ship with distinct ``*BtnAll`` IDs so they don't + collide with the Review tab's bare-ID buttons +- Approve / Reject are intentionally absent — those stay scoped to Review + per the issue's scope decision +""" + +from __future__ import annotations + + +def _auth(token: str) -> dict: + return {"Authorization": f"Bearer {token}"} + + +class TestAllItemsBatchBar: + def test_admin_page_renders_all_items_batch_bar(self, seeded_app): + c = seeded_app["client"] + token = seeded_app["admin_token"] + resp = c.get("/admin/corporate-memory", headers=_auth(token)) + assert resp.status_code == 200 + body = resp.text + + # All five bulk-edit buttons present with the All-suffix IDs the JS + # plumbing (`updateSelectionCount('all')`) toggles. + for btn_id in ( + "batchMoveCategoryBtnAll", + "batchMoveDomainBtnAll", + "batchAddTagBtnAll", + "batchRemoveTagBtnAll", + "batchSetAudienceBtnAll", + ): + assert f'id="{btn_id}"' in body, f"missing button id={btn_id}" + + # Select-all checkbox + count span scoped to All Items. + assert 'id="selectAllAll"' in body + assert 'id="selectedCountAll"' in body + assert "toggleSelectAll('all')" in body + + def test_all_items_bar_omits_approve_reject(self, seeded_app): + """Approve / Reject are Review-only by design (issue #129 scope).""" + c = seeded_app["client"] + token = seeded_app["admin_token"] + resp = c.get("/admin/corporate-memory", headers=_auth(token)) + assert resp.status_code == 200 + body = resp.text + + # Bare-suffix Review buttons stay; *BtnAll variants of approve/reject + # must NOT appear — otherwise the JS in updateSelectionCount('all') + # would silently enable a status-change action the All-tab UX hasn't + # signed off on. + assert 'id="batchApproveBtn"' in body # Review tab still has it + assert 'id="batchApproveBtnAll"' not in body + assert 'id="batchRejectBtnAll"' not in body + + def test_browse_tab_omits_row_checkbox(self, seeded_app): + """Regression guard for the adversarial-review finding: widening + ``renderItemCard``'s checkbox gate from "Review-only" to "Review or + All" must NOT also render the checkbox in the Browse tab — Browse + has no batch bar of its own, so an orphan checkbox there would + fire ``updateSelectionCount('all')`` against an invisible tab and + confuse the UX. + + The renderItemCard signature is now a ``mode`` enum + (``'review' | 'all' | 'browse'``); the Browse path passes + ``'browse'`` and the function early-returns no checkbox markup + for that mode. Both invariants are checked at template level so + a future refactor can't silently regress either. + """ + c = seeded_app["client"] + token = seeded_app["admin_token"] + resp = c.get("/admin/corporate-memory", headers=_auth(token)) + assert resp.status_code == 200 + body = resp.text + + # Browse-tab call site uses the 'browse' mode literal. + assert "renderItemCard(it, idx, 'browse')" in body + # And renderItemCard's checkbox is gated on `mode === 'browse'` so + # the input element is omitted entirely on that branch. + assert "mode === 'browse' ? ''" in body