feat(frontend/history): inline column filters via shared lib (refs #166)#790
feat(frontend/history): inline column filters via shared lib (refs #166)#790cristim wants to merge 7 commits into
Conversation
cristim
added
triaged
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Warning Review limit reached
More reviews will be available in 8 minutes and 9 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR adds per-column filtering to Purchase History and Approval Queue tables with a shared portal-rendered popover UI. New state management stores filter selections, filter application functions transform row arrays, and filter buttons in table headers open popover controls for numeric expression and categorical set filtering. ChangesHistory Column Filters
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
Rate Limit Exceeded
|
cristim
force-pushed
the
refactor/history-column-filters
branch
from
2f2a298 to
3cec781
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
cristim
force-pushed
the
refactor/history-column-filters
branch
from
3cec781 to
03c7a40
Compare
|
Rebased on Conflict resolution: only Verification on the rebased head: @coderabbitai review |
|
🧠 Learnings used✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review Generated by Claude Code |
|
🧠 Learnings used✅ Action performedFull review finished. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
cristim
force-pushed
the
refactor/history-column-filters
branch
from
03c7a40 to
eb409e1
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/lib/history-filter-popover.ts`:
- Around line 71-84: closeOpenHistoryPopover currently removes the popover DOM
but doesn't reset the trigger button's aria-expanded, leaving stale
accessibility state; update closeOpenHistoryPopover (and any matching close
paths) to locate the trigger using the same selector used when opening (e.g.,
`.history-column-filter-btn[data-column="${CSS.escape(column)}"]`) and set its
aria-expanded attribute to "false" before or after calling trigger?.focus(),
ensuring this runs regardless of restoreFocus so the button state is always
cleared when openPopover is removed; keep existing removal of el, nulling
openPopover, calling detachGlobalListeners, and clearing lastAnchorSelector.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5bf961a8-97fa-42d8-a3e4-24ed6e2cc00b
📒 Files selected for processing (13)
frontend/src/__tests__/allowed-accounts.test.tsfrontend/src/__tests__/approval-queue-column-filters.test.tsfrontend/src/__tests__/history-approval-queue.test.tsfrontend/src/__tests__/history-approve-button.test.tsfrontend/src/__tests__/history-cancel-button.test.tsfrontend/src/__tests__/history-cancel-permissions.test.tsfrontend/src/__tests__/history-column-filters.test.tsfrontend/src/__tests__/history-retry-button.test.tsfrontend/src/__tests__/history.test.tsfrontend/src/__tests__/xss-provider-class.test.tsfrontend/src/history.tsfrontend/src/lib/history-filter-popover.tsfrontend/src/state.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
…ces (refs #166) Adds two new closed column-id enums and matching filter slices for the History page tables: PurchaseHistoryColumnId for the completed-purchases table and ApprovalQueueColumnId for the pending-approvals card. The two tables share Provider/Service/Term/Count/UpfrontCost/MonthlySavings columns but diverge on the queue's Account/Payment/MonthlyCost/CreatedBy vs Purchase History's ResourceType/Region, so each gets its own in-memory slice with set/clear/getAll accessors mirroring the existing recommendations equivalents. In-memory only on this iteration; localStorage persistence stays a follow-up under the same umbrella as the recommendations equivalent.
…se History table (refs #166) Adds per-column filter buttons to the Purchase History table headers. Filter columns: Provider, Service, Type (resource_type), Region, Term (categorical), and Count, Upfront Cost, Monthly Savings (numeric). Status is excluded — the existing status chip-row is the canonical filter for that column. Wires `applyColumnFilters` from lib/column-filters.ts through extractors that match the rendered cell shape: categorical extractors return the raw field; numeric extractors return the value rounded to display precision so typed numbers match the rendered cells (issue #484 contract). Filters compose with the existing status chip filter, and the column-filter popover lists distinct values from rows that survived the status filter (so picking "Failed" then opening Provider only lists providers with failed rows). Introduces a small lib/history-filter-popover.ts module shared by both History tables — keeps the popover DOM/teardown/keyboard wiring out of history.ts itself and reuses the existing .column-filter-popover CSS so the visual matches the recommendations equivalent.
…al Queue table (refs #166) Adds per-column filter buttons to the Approval Queue table headers. Filter columns: Provider, Account, Service, Term, Payment, Created by (categorical), and Count, Monthly Cost, Upfront Cost, Monthly Savings (numeric). Status is excluded — the queue scope is already pending| notified by definition; the broader Status chip-row above is the authoritative status filter for the page. Uses the same lib/history-filter-popover.ts helper introduced for the Purchase History table. Numeric extractors round to display precision (CURRENCY_DEFAULT_DIGITS) so a "$X" filter matches the rendered cell; monthly_cost returns NaN for null so a "= 0" predicate doesn't match rows where the provider didn't report a value. Categorical extractors mirror the cell rendering: account uses account_id with getAccountName as the display label, created_by prefers email then falls back to UUID. Test-suite mocks (history-* + allowed-accounts + xss-provider-class) extended with the new state accessors so they keep passing against the expanded state surface.
Adds two test files exercising the new History column-filter wiring:
* history-column-filters.test.ts — Purchase History table:
numeric expr, categorical set, stacked AND, invalid expr (no-op),
clear, and the term-as-stringified-categorical case.
* approval-queue-column-filters.test.ts — Approval Queue table:
numeric expr (monthly_cost >= N), categorical set (payment in
{…}), stacked AND across provider+created_by, invalid expr (no-op),
the NaN-as-missing contract for null monthly_cost (so "= 0" and
"> 0" don't coincidentally match unreported rows), and clear.
Both suites mock the heavy module transitive deps (api / navigation /
utils / state / confirmDialog / approval-details / toast / skeleton /
recommendations) so the pure column-filter helpers can be exercised
without standing up a DOM.
…ssions suite Rebasing onto feat/multicloud-web-frontend surfaced a gap: the new History/ApprovalQueue per-column-filter accessors added to ../state by this PR (issue #166) were missing from the ../state mock in history-cancel-permissions.test.ts. Without them renderApprovalQueue threw "getApprovalQueueColumnFilters is not a function", suppressing the whole approval-queue render and zeroing out the cancel buttons the permission-gating assertions depend on. Add the same six getter/setter/clear mocks already present in the other history test suites so the render path completes and the cancel-gating assertions exercise real button output again. refs #166
closeOpenHistoryPopover removed the popover DOM but never restored the trigger button's aria-expanded to "false", leaving stale expanded accessibility state after outside-click, Escape, or toggle-close. Reset it on every close path, independent of focus restoration.
…ttrs renderHistoryFilterButton injected `column` and `label` raw into aria-label, title, and data-column attributes via innerHTML template literal. Apply escapeHtmlAttr to both before interpolation and replace the em-dash in the active-state aria-label with a hyphen. Add history-filter-popover-xss.test.ts to assert hostile payloads in label and column are entity-encoded, not executed.
cristim
force-pushed
the
refactor/history-column-filters
branch
from
4d42d02 to
d8caeef
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Wires the History page to the shared
frontend/src/lib/column-filters.tsextracted by merged PR #570. Adds inline per-column filter buttons to both History tables (Purchase History + Approval Queue) using the sameapplyColumnFilters<TRow, TColumnId>shape that recommendations.ts already uses.PurchaseHistoryColumnId+ApprovalQueueColumnId, each with its own in-memory filter record, get/set/clear accessors. Two separate enums because the queue uses Account/Payment/Monthly Cost/Created By while Purchase History uses Resource Type/Region — overlap is too narrow to share a single column-id type without ugly unions.frontend/src/lib/history-filter-popover.ts— small reusable popover helper shared by the two tables; reuses the existing.column-filter-popoverCSS so the visual matches recommendations.roundForDisplay(_, 0)so a typed$Xmatches the rendered cell (matches the issue ux(opportunities): numeric column filter doesn't match exact values (typed value compares against unrounded backend) #484 contract on recommendations).monthly_costreturnsNaNfor null (= 0and> 0filters don't coincidentally match unreported rows).Sibling PRs
Part of the issue #166 follow-up trio:
Each PR touches only its own state slice, so all three rebase cleanly against feat/multicloud-web-frontend.
Test plan
npm test -- history— all history-related suites green.npm test -- approval-queue— new regression suite passes.npm test -- column-filters— shared lib regression unchanged.npx tsc --noEmit— clean.npm test— 67 suites / 2154 tests / 1 skipped — all green.Refs #166.
Summary by CodeRabbit
Release Notes
New Features
Tests