Skip to content

feat(settings): per-account overrides modal — fixes panel-stacking (closes #122, #116)#124

Merged
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
feat/overrides-modal-rework
Apr 27, 2026
Merged

feat(settings): per-account overrides modal — fixes panel-stacking (closes #122, #116)#124
cristim merged 1 commit into
feat/multicloud-web-frontendfrom
feat/overrides-modal-rework

Conversation

@cristim
Copy link
Copy Markdown
Member

@cristim cristim commented Apr 27, 2026

Closes #122. Closes #116.

The bug, in one screenshot's worth

Two AWS accounts: AWS 540659244915 and CUDly host (909626172446). User clicks Overrides on 540… → empty-state auto-opens the create modal → creates an aws/ec2 override with Partial Upfront / 34% coverage. Modal closes; the inline panel under the accounts table now shows [Add override] and the new row.

User then clicks Overrides on 909…. That account's panel also opens, appended after 540…'s panel. Result: two panels stacked under the table with no titles. The aws/ec2 row visually attaches to whichever account is closer on screen, two unscoped Add override buttons appear, and "No service overrides yet for this account." text floats between them with no indication of which account.

The rework

All four CRUD operations move into a per-account modal keyed by an explicit title (Service overrides for {account.name} ({external_id})). The accounts table loses the inline expandable panel entirely.

renderAccountsList no longer creates per-row panels, no longer collects them in a panels[] array, and no longer appends them after the table. The whole inline-panel scaffolding is gone.

Empty-state UX is preserved: an AWS account with zero overrides still auto-opens the inner create modal, so users land directly on the form in 1 click — no regression vs #106.

A defensive tweak: closeAccountOverridesModal also closes the inner create modal, so a programmatic close (or future ESC-to-close from #115) doesn't leave an orphan modal whose Save would target a hidden parent.

What this closes / pairs with

Test plan

🤖 Generated with claude-flow

closes #122, #116)

The Overrides UX from #72 (inline payment edit) and #106 (create modal)
rendered each account's overrides into an inline expandable panel
appended below the entire accounts table. Two open panels stacked
without per-row attachment, so an override row created for one account
appeared visually adjacent to a different account's empty-state. From
the user's screenshot: an aws/ec2 override created for AWS 540... was
displayed under CUDly host (909...), with two unscoped "Add override"
buttons and a confusing "No service overrides yet for this account."
text in between.

Rework: every Overrides operation now lives inside a per-account modal
keyed by an explicit title. Clicking the row's Overrides button opens
account-overrides-modal whose title reads "Service overrides for
{account.name} ({external_id})". The modal body uses the same
loadOverridesPanel rendering function as before, so all four CRUD
operations carry forward unchanged:

- LIST: the existing overrides-table with Service / Term / Payment /
  Coverage / Reset, including #72's inline payment <select> in the
  Payment column.
- CREATE: the existing override-modal from #106 stacks on top of the
  account-overrides-modal when the user clicks "Add override". Saving
  reloads the parent table; cancelling leaves the parent open.
- EDIT-PAYMENT: same inline <select> as #72, now scoped inside the
  modal body so its loadOverridesPanel reload target is unambiguous.
- DELETE: Reset button + confirmDialog, identical to before.

renderAccountsList no longer creates an inline panel per row, no longer
collects them in a panels[] array, and no longer appends them after the
table — that whole inline-panel scaffolding is gone.

Empty-state UX is preserved: an AWS account with zero overrides still
auto-opens the inner create modal so users land directly on the form
in 1 click instead of 2.

Defensive: closeAccountOverridesModal also closes the inner create
modal so a programmatic close (or future ESC-to-close from #115)
doesn't leave an orphan modal whose Save would target a hidden parent.

Tests: existing #72 + #106 test helpers updated to look for
#account-overrides-modal-body instead of .account-overrides-panel.
Four new tests in a "Account overrides modal" describe block lock in
the behaviour: title-binds-to-account, switching-accounts-swaps-title,
close-clears-body, and a regression guard that .account-overrides-panel
no longer renders.

Full settings-accounts test count: 40/40 (was 36 + 4 new).
Full frontend suite: 1273/1273 green; typecheck + production build
clean.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

Warning

Rate limit exceeded

@cristim has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 42 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 457360ff-27f7-4724-be5d-fbbe0d8bebd8

📥 Commits

Reviewing files that changed from the base of the PR and between 89e503a and 8be0f56.

📒 Files selected for processing (3)
  • frontend/src/__tests__/settings-accounts.test.ts
  • frontend/src/index.html
  • frontend/src/settings.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/overrides-modal-rework

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@cristim
Copy link
Copy Markdown
Member Author

cristim commented Apr 27, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

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

@cristim cristim merged commit a752baf into feat/multicloud-web-frontend Apr 27, 2026
3 checks passed
cristim added a commit that referenced this pull request Apr 27, 2026
…#125)

PR #124 (issue #122) moved per-account overrides into a modal and
deleted the inline `.account-overrides-panel` element from the accounts
table. The cleanup selector at renderAccountsList top still listed it,
which is dead — no such element ever exists now. Drop the class from
the querySelectorAll list and explain in the comment that overrides
moved to the modal so a future reader doesn't restore the reference.

No behaviour change. Tests + typecheck + build still green.
@cristim cristim added triaged Item has been triaged priority/p1 Next up; this sprint severity/high Significant harm urgency/this-sprint Within the current sprint impact/many Affects most users effort/s Hours type/feat New capability labels Apr 28, 2026
@cristim cristim deleted the feat/overrides-modal-rework branch April 29, 2026 10:07
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/p1 Next up; this sprint severity/high Significant harm triaged Item has been triaged type/feat New capability urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant