Skip to content

feat(frontend/plans): inline column filters via shared lib (refs #166) - #791

Merged
cristim merged 4 commits into
feat/multicloud-web-frontendfrom
refactor/plans-column-filters
Jun 3, 2026
Merged

feat(frontend/plans): inline column filters via shared lib (refs #166)#791
cristim merged 4 commits into
feat/multicloud-web-frontendfrom
refactor/plans-column-filters

Conversation

@cristim

@cristim cristim commented May 28, 2026

Copy link
Copy Markdown
Member

Summary

Wires the Planned Purchases table to the shared lib/column-filters primitives extracted in merged PR #570. Each filterable column header carries an inline filter trigger button that opens a popover (multi-select for categorical, free-text expression for numeric). Filters AND together and persist in-memory for the SPA session — matching the canonical Recommendations-tab pattern.

This is one of three parallel follow-up PRs to #570. Siblings cover History and RI Exchange; each owns its own column-id type and filter slice so they can land in any order without merge conflicts.

Columns wired:

  • Categorical: provider, service, resource_type, term, payment, status
  • Numeric: count, upfront_cost, estimated_savings

Numeric predicates compare against the rounded display value (issue #484 contract preserved here too).

Commits

  1. refactor(frontend/state) — adds PlansColumnId + plansColumnFilters slice (separate from RecommendationsColumnId so siblings can land in parallel)
  2. feat(frontend/plans) — table wiring: trigger buttons, popover lifecycle, cell extractors, applyColumnFilters call site
  3. test(frontend/plans) — new plans-column-filters.test.ts regression suite
  4. style(frontend/plans) — match the canonical recommendations filter glyph + em-dash escapes

Test plan

  • npx tsc --noEmit clean
  • npm test -- plans — all 65 suites pass (2148 tests, 1 pre-existing skip)
  • npm test -- column-filters — shared-lib suite still green
  • New plans-column-filters.test.ts covers: header trigger presence, popover opens detached, categorical narrowing (provider=aws), numeric narrowing (count >= 2), stacked filters AND together, invalid expression shows inline error, (All) restores full row set
  • Manual: open Plans tab, click the filter glyph on Provider header → popover opens, untick a value → table re-renders narrowed; type >=100 in the Upfront filter → table narrows; clear via Clear button → full set returns.

Refs #166.

@cristim cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline impact/few Limited audience effort/m Days type/feat New capability labels May 28, 2026
@cristim

cristim commented May 28, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 7 minutes and 20 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4a00cba2-aea6-401b-a85e-1a2c637a7f74

📥 Commits

Reviewing files that changed from the base of the PR and between eb85a50 and d1d4645.

📒 Files selected for processing (7)
  • frontend/src/__tests__/plans-column-filters.test.ts
  • frontend/src/__tests__/plans-permissions.test.ts
  • frontend/src/__tests__/plans-range-validation.test.ts
  • frontend/src/__tests__/plans.test.ts
  • frontend/src/__tests__/xss-purchase-status.test.ts
  • frontend/src/plans.ts
  • frontend/src/state.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/plans-column-filters

Comment @coderabbitai help to get the list of available commands and usage tips.

@cristim

cristim commented May 30, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 54 minutes and 14 seconds before sending another message.

cristim added 4 commits June 3, 2026 16:46
Introduces a Plans-scoped per-column filter slice (PlansColumnId,
PlansColumnFilter, PlansColumnFilters) with get/set/clear accessors,
mirroring the existing Recommendations slice. Kept as a separate slice
so the Plans, History, and RI Exchange follow-ups to PR #570 can land
in parallel without contending on each other's state shape.

In-memory only — survives tab switches within the SPA, resets on full
reload (same lifecycle as recommendationsColumnFilters).

Refs #166.
Wires the Planned Purchases table to the shared lib/column-filters
primitives extracted in PR #570. Each filterable column gets an inline
trigger button in its header; clicking opens a popover with either a
multi-select (categorical) or a numeric-expression input. Filters
AND together, persist in-memory across the SPA session, and are
applied at render time.

Columns wired:
- categorical: provider, service, resource_type, term, payment, status
- numeric: count, upfront_cost, estimated_savings

Numeric predicates compare against the rounded display value
(roundForDisplay + displayPrecisionForPlan) so the issue #484
exact-match contract is preserved here too.

Re-renders go through a cached lastLoadedPurchases module slice so
popover commits never re-fetch from the API. Popover lives on
document.body and is re-anchored after each table re-render.

Mock-state additions in plans*.test.ts and xss-purchase-status.test.ts
mirror the new accessors; legacy assertions continue to pass.

Refs #166. Sibling follow-ups land in parallel for History and RI
Exchange.
Adds plans-column-filters.test.ts covering the Planned Purchases table
integration with lib/column-filters:

- every filterable column header carries a trigger button
- clicking opens a portal popover detached to document.body
- categorical set filter narrows rows (provider=aws)
- numeric expression filter narrows rows (count >= 2)
- stacked filters AND together
- invalid expressions surface the lib's inline error and apply no filter
- (All) tri-state restores the full row set after narrowing

The shared parseNumericFilter + applyColumnFilters primitives keep
their own coverage in column-filters.test.ts; these tests focus on
the Plans-specific wiring.

Refs #166.
Matches the canonical recommendations.ts wiring exactly (⛛ filter
icon, — em-dash inside the aria-label). Keeps the rendered DOM
identical to the Opportunities tab so the two surfaces are
indistinguishable at the screen-reader and visual layers.

No behavioural change.
@cristim
cristim force-pushed the refactor/plans-column-filters branch from 415faec to d1d4645 Compare June 3, 2026 14:49
@cristim

cristim commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Rebased on feat/multicloud-web-frontend.

Resolved conflicts in three test files where the base branch's group-membership authz migration (PR #907) collided with this PR's new per-column filter mock accessors:

  • frontend/src/__tests__/plans.test.ts
  • frontend/src/__tests__/plans-range-validation.test.ts
  • frontend/src/__tests__/xss-purchase-status.test.ts

In each, kept both sides: the base's getCurrentUser mock returning groups: [...] (the old role: 'admin' shape is stale post-#907) plus this PR's getPlansColumnFilters / setPlansColumnFilter / clearAllPlansColumnFilters accessors. This matches the already-migrated sibling plans-permissions.test.ts.

Verified post-rebase: all touched plans/state suites pass, npm run build succeeds. The only failing suite (recommendations-lookback.test.ts) is a pre-existing base-branch failure introduced by PR #911, unrelated to this PR's seven files.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

@cristim Thanks for the detailed rebase notes! The conflict resolution strategy — preserving the base branch's groups: [...] shape from #907 alongside this PR's getPlansColumnFilters / setPlansColumnFilter / clearAllPlansColumnFilters mock accessors — looks like exactly the right approach, and the alignment with plans-permissions.test.ts as the reference sibling is a good consistency anchor.

Kicking off the incremental review now against the rebased commits.


🧠 Learnings used
Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 277
File: frontend/src/__tests__/recommendations.test.ts:2472-2476
Timestamp: 2026-05-05T07:46:01.902Z
Learning: In the CUDly frontend (`frontend/src/recommendations.ts`), `effectiveSavingsPct` intentionally has NO plausibility guard (no null return or warning for reconstructed percentages above per-term ceilings). The maintainer explicitly rejected that approach in PR `#277`. The fix for inflated percentages (issue `#274`) is purely a data-path fix: plumbing the provider's canonical `on_demand_cost` through to the frontend so the denominator is not reconstructed from potentially misleading `monthly_cost + savings + amortized`. Tests that pin the reconstructed >80% behavior are intentional documentation of the pre-fix path, not bugs.
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/few Limited audience priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/feat New capability urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant