Skip to content

fix: RI Exchange active-RI column filters + immediate override-delete Undo toast - #1435

Merged
cristim merged 2 commits into
mainfrom
fix/qa-inline-filters-toast-undo
Jul 17, 2026
Merged

fix: RI Exchange active-RI column filters + immediate override-delete Undo toast#1435
cristim merged 2 commits into
mainfrom
fix/qa-inline-filters-toast-undo

Conversation

@cristim

@cristim cristim commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

Two independent QA regressions were investigated. What I found and changed:

RI Exchange active-RI column filters (refs #1414)

The Active Convertible RIs table (#ri-exchange-instances-list) had no per-column filter buttons -- PRs #789/#791 wired filters only into the Reshape Recommendations table. This PR restores them:

  • Adds ActiveRiColumnId state slice to state.ts
  • Adds a popover state machine + applyActiveRiColumnFilters to riexchange.ts, wiring filter buttons into the Instance Type, AZ, Count, Offering, and Utilization column headers
  • Adds a DOM-level regression test file (riexchange-active-ri-filters.test.ts) with 4 tests (button presence, popover detachment to document.body, categorical narrowing, numeric narrowing)

Override-delete Undo toast (closes #1415)

The Undo toast for deleting a Service Defaults override was awaited behind loadOverridesPanel + refreshRecommendationsAfterOverrideChange (a full page-worth of recommendation API calls). A slow refresh delayed the toast enough that QA saw no Undo affordance. Fix:

  • Show the toast immediately after the server confirms the delete, before the panel reload + recommendations refresh
  • Split the delete into its own try/catch with an early return, so a refresh error can no longer be misreported as "Failed to delete override" when the delete succeeded
  • Two regression tests: Undo toast visible while a refresh is still in flight (fails pre-fix, passes post-fix); a failed delete shows an error toast with no Undo and no refresh

Scope notes on issue #1414 (why not closes)

Issue #1414 covers RI Exchange and Plans tables. I verified the current state of both on main:

  • RI Exchange active-RI filters: genuinely missing -- fixed here.
  • Plans table filters: already present and fully wired on main (plans.ts filterBtn per column + applyPlansColumnFilters + openPlansColumnPopover, covered by plans-column-filters.test.ts). QA's Plans repro (sheet row 534) is most likely stale.

This PR does not touch the Plans table, so it intentionally uses refs #1414 rather than closes #1414 to avoid auto-closing the Plans half before it is re-verified by QA. A QA-reverify follow-up will confirm/close the Plans half.

Verification

  • cd frontend && npm test -- exit 0, 2609 passed, 1 skipped, 0 failed
  • cd frontend && npm run build -- exit 0 (only the pre-existing entrypoint-size warning)
  • New #1415 immediacy test confirmed to FAIL against pre-fix settings.ts (toast undefined) and PASS after

closes #1415
refs #1414

…ble (closes #1414)

Add per-column categorical and numeric filter controls to the Active
Convertible RIs table in the RI Exchange view, matching the existing
behavior of the Reshape Recommendations table (introduced in PRs #789
and #791). The regression left `renderRIsTable` with no filter buttons.

- Add `ActiveRiColumnId`, `ActiveRiColumnFilter`, `ActiveRiColumnFilters`
  types and state slice to `state.ts`
- Add `applyActiveRiColumnFilters` (exported), popover state machine,
  and all helper functions to `riexchange.ts`; wire filter buttons into
  `renderRIsTable` header cells for Instance Type, AZ, Count, Offering,
  and Utilization columns
- Add DOM-level regression test file with 4 tests: button presence,
  popover detachment to body, categorical narrowing, numeric narrowing
- Restore `getActiveRiColumnFilters` mock in existing describe blocks
  that call `jest.resetAllMocks()`, preventing cross-describe contamination
@cristim cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/this-quarter Within the quarter impact/many Affects most users effort/s Hours type/bug Defect labels Jul 17, 2026
@cristim

cristim commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 6 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

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.

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 reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

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, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cf4e9abb-2060-4bc0-b2d9-c95b7b3d82f4

📥 Commits

Reviewing files that changed from the base of the PR and between 25e3e86 and aebaf6c.

📒 Files selected for processing (7)
  • frontend/src/__tests__/riexchange-active-ri-filters.test.ts
  • frontend/src/__tests__/riexchange-permissions.test.ts
  • frontend/src/__tests__/riexchange.test.ts
  • frontend/src/__tests__/settings-accounts.test.ts
  • frontend/src/riexchange.ts
  • frontend/src/settings.ts
  • frontend/src/state.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/qa-inline-filters-toast-undo

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

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
✅ 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.

)

The Undo toast for deleting a Service Defaults override was awaited behind
loadOverridesPanel + refreshRecommendationsAfterOverrideChange, the latter
issuing a full page-worth of recommendation API calls. A slow refresh
delayed the toast long enough that users reported seeing no Undo affordance
at all.

Show the toast immediately after the server confirms the delete, before the
panel reload and recommendations refresh. Split the delete into its own
try/catch that returns early on failure, so a refresh error can no longer be
misreported as "Failed to delete override" when the delete actually
succeeded.

Add two regression tests: one asserting the Undo toast is visible while a
recommendations refresh is still in flight (fails pre-fix, passes post-fix),
and one asserting a failed delete shows an error toast with no Undo and does
not trigger the refresh.
@cristim cristim changed the title fix(frontend): restore inline column filters to Active Convertible RIs table fix: RI Exchange active-RI column filters + immediate override-delete Undo toast Jul 17, 2026
@cristim

cristim commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Addressed the scope BLOCK. Summary of what I verified on main and what changed:

Plans filters (#1414 second half): verified already present and fully wired on main -- plans.ts renders a filterBtn per column (Provider/Service/Resource Type/Count/Term/Payment/Upfront/Est. Savings/Status), applyPlansColumnFilters drives the pipeline, openPlansColumnPopover + rebindOpenPlansPopoverAnchor handle the popover, and plans-column-filters.test.ts covers it. This PR does not touch Plans, so I dropped closes #1414 and left refs #1414. QA should re-verify the Plans repro (sheet row 534) -- it looks stale.

Reset/Delete override toast (#1415): confirmed it WAS gated. On main, showToast was awaited behind loadOverridesPanel + refreshRecommendationsAfterOverrideChange (settings.ts), the latter running loadRecommendations (a full recommendations fetch). A slow refresh delayed the Undo toast, matching QA's "no toast" report. Fixed by moving showToast to fire immediately after the delete resolves, before the panel reload + refresh, and splitting the delete into its own try/catch with an early return so a refresh error can't be misreported as a delete failure. Kept closes #1415.

Verification: npm test exit 0 (2609 passed, 1 skipped, 0 failed); npm run build exit 0. New immediacy test confirmed to fail against pre-fix settings.ts and pass after.

Final closes-links: closes #1415, refs #1414.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@cristim Thanks for the detailed scope clarification and verification notes. I’ll review the updated changes, including the issue-linking scope and the new regression coverage.


✏️ Learnings added
Learnt from: cristim
URL: https://github.com/LeanerCloud/CUDly/pull/1435

Timestamp: 2026-07-17T06:58:45.591Z
Learning: For LeanerCloud/CUDly issue `#1414`, the Plans table already has per-column filters implemented on main in `plans.ts`: `filterBtn` controls per column, `applyPlansColumnFilters`, `openPlansColumnPopover`, and `rebindOpenPlansPopoverAnchor`, with coverage in `plans-column-filters.test.ts`. A PR that only restores RI Exchange active-RI filters should reference, not close, `#1414` until QA re-verifies the Plans report.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ 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.

@cristim
cristim merged commit 89658a3 into main Jul 17, 2026
19 of 21 checks passed
@cristim
cristim deleted the fix/qa-inline-filters-toast-undo branch July 17, 2026 10:37
@cristim

cristim commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Merged on the substantive CI signal (all Lint/Unit/Integration/Security/Build/E2E/Validate-Terraform green; only the non-blocking tflint pre-commit flake red).

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

Labels

effort/s Hours impact/many Affects most users priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(settings): Reset override row toast-undo not appearing (regression after #113/PR #834)

1 participant