feat(settings): create-override modal for AWS account overrides (closes #104)#106
Merged
cristim merged 1 commit intofeat/multicloud-web-frontendfrom Apr 27, 2026
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
#104) Settings → Accounts → Service overrides was a dead end for any account with no overrides yet: the panel rendered "No service overrides set." text with no UI affordance to create one. The only path was a direct PUT /api/accounts/:id/service-overrides/:provider/:service curl. PR #72's inline payment selector inherited the same dead end — it operates on existing rows that nobody can create. This commit adds an "Add override" modal flow: - Panel empty-state for an AWS account auto-opens the create modal so users land directly on the create form. The Add Override button stays in the panel as a fallback if they cancel and want to retry without re-expanding the row. - Populated panels gain an "Add override" button at the top so users can add another override for a different service. - The modal lets users pick service (filtered to AWS_OVERRIDE_SERVICES, excluding services that already have an override for this account so UPSERT doesn't silently overwrite), and set Term / Payment / Coverage via Inherit-by-default selects + a 0–100 numeric input. - Submit sends a sparse PUT — fields left as Inherit are omitted, so the backend's applyOverrideScalars treats them as "use global default". - Submit blocks when no field is set (would persist a no-op override row) and when coverage is out of 0–100 range. Non-AWS accounts (Azure / GCP) keep their existing read-only empty-state text and gain neither the Add button nor the auto-open. Per-product term/payment semantics differ for those providers and a follow-up issue under #104 will track the modal expansion once the AWS UX settles. The modal mirrors the existing account-modal pattern in index.html (role=dialog, aria-modal, .modal-content, .modal-buttons) and the open/close/submit code follows the same shape as openAccountModal / closeAccountModal / handleAccountFormSubmit. Tests: 10 new cases in settings-accounts.test.ts cover empty-state auto-open, populated-state add button, sparse PUT semantics, term/coverage coercion, no-fields rejection, coverage-range validation, save-success panel reload, cancel-without-save, all-services-used disables submit, and non-AWS empty state stays passive. Full suite: 1265 tests green.
cristim
force-pushed
the
feat/account-override-create-modal
branch
from
77ee80f to
2b24e10
Compare
This was referenced Apr 27, 2026
Open
Open
Closed
cristim
added a commit
that referenced
this pull request
Apr 27, 2026
closes #122, #116) (#124) 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.
This was referenced Apr 28, 2026
cristim
added
triaged
cristim
added a commit
that referenced
this pull request
Apr 29, 2026
Closes #117. Per-account service overrides have been a feature since migration 000011_cloud_accounts.up.sql (mid-2024) and now have full UI coverage via #72 (inline payment edit) and #106 (create modal). Adds a new "Per-Account Service Overrides" section to README between "Coverage Percentage" and "Safety Features" covering: - Concept + when to use vs the global Settings → Purchasing card. - Web-UI walkthrough (Settings → Accounts → expand → Service overrides). - AWS-only V1 boundary callout pointing at #109 for Azure/GCP. - How to edit existing overrides — inline Payment per #72, Reset, and the #110 follow-up for inline Term/Coverage/Enabled. - "Inherit" semantics: blank fields are not stored as a sentinel; the PUT request stays sparse and the engine reads the global default at evaluation time. - API parity note: the override modal targets the same endpoint as scripted setups; both write the same row. Also disables MD060 (table-column-style) in .markdownlint.yaml — same rationale as PR #169 (landing here too in case the PRs merge in a different order; both diffs are idempotent).
cristim
added a commit
that referenced
this pull request
Apr 29, 2026
…rules Closes #107. The two override UIs (inline payment selector from #72 and create-modal from #106) accepted any (service, term, payment) combination — including ones the global Settings → Purchasing cards already block as invalid. Concrete example: RDS 3yr no-upfront is hidden in the global RDS card via commitmentOptions.invalidCombinations, but the override modal saved it cleanly and the inline selector let the user pick it. Wired the override surfaces through the same commitmentOptions helpers the global cards use: 1. **Inline payment selector (`buildPaymentOverrideSelect`)**: now calls `getValidPaymentOptions(provider, service, term)` and only renders options that pass. For an RDS row with term=3, no-upfront is gone. When term is unset (override row created without a term), falls back to the full AWS list — can't pre-validate without it. If the row's CURRENT payment somehow isn't in the valid set (e.g. saved via direct API curl before this filter shipped, or AWS tightened the rules), still renders it as a dedicated "(invalid for term Ny — Delete the override and recreate)" option so the row isn't silently mutated. 2. **Override-create modal (`openOverrideModal`)**: new `syncOverridePaymentOptions` helper repopulates the payment dropdown on every service or term change, dropping invalid combinations automatically and snapping the selection back to "Inherit" if the previously-chosen payment is no longer valid for the new (service, term) pair. 3. **Submit-side validation (`submitOverrideForm`)**: when both term and payment are explicitly set, refuses to save invalid combos even if a stale select option leaked through (e.g. browser back-button restoring a stale dropdown state). Mirror of the global Settings → Purchasing card's checkCommitmentOptionCombo. Side change: exported `loadOverridesPanel` and `openOverrideModal` so the new tests can drive the panel and modal directly. Both are coherent units; just needed the export. Drive-by: removed unused `formatRelativeTime` import in `frontend/src/recommendations.ts` (pre-existing TS6133 blocking the pre-commit Run frontend tests hook on this branch — same fix as PR Tests: - `inline payment selector hides invalid options for RDS term=3 row` pins the RDS-no-3yr-no-upfront case. - `inline payment selector shows full list when term is unset` pins the fallback case. - `override-create modal hides invalid payment options on service+term change` pins the dependency loop and the reset- to-Inherit-when-stale snap. - All 40 existing settings-accounts tests still pass; 43 total. Backend save-side guard (the issue's third bullet) is deferred — the frontend gates close the practical bug; a backend mirror via the existing `commitmentopts.Service.Validate` machinery is its own PR-sized change touching `internal/api/handler_accounts.go` and would need probe-data integration tests. Worth filing as a follow-up.
cristim
added a commit
that referenced
this pull request
Apr 30, 2026
…rules (closes #107) (#176) * fix(ux/settings): override surfaces reuse commitmentOptions validity rules Closes #107. The two override UIs (inline payment selector from #72 and create-modal from #106) accepted any (service, term, payment) combination — including ones the global Settings → Purchasing cards already block as invalid. Concrete example: RDS 3yr no-upfront is hidden in the global RDS card via commitmentOptions.invalidCombinations, but the override modal saved it cleanly and the inline selector let the user pick it. Wired the override surfaces through the same commitmentOptions helpers the global cards use: 1. **Inline payment selector (`buildPaymentOverrideSelect`)**: now calls `getValidPaymentOptions(provider, service, term)` and only renders options that pass. For an RDS row with term=3, no-upfront is gone. When term is unset (override row created without a term), falls back to the full AWS list — can't pre-validate without it. If the row's CURRENT payment somehow isn't in the valid set (e.g. saved via direct API curl before this filter shipped, or AWS tightened the rules), still renders it as a dedicated "(invalid for term Ny — Delete the override and recreate)" option so the row isn't silently mutated. 2. **Override-create modal (`openOverrideModal`)**: new `syncOverridePaymentOptions` helper repopulates the payment dropdown on every service or term change, dropping invalid combinations automatically and snapping the selection back to "Inherit" if the previously-chosen payment is no longer valid for the new (service, term) pair. 3. **Submit-side validation (`submitOverrideForm`)**: when both term and payment are explicitly set, refuses to save invalid combos even if a stale select option leaked through (e.g. browser back-button restoring a stale dropdown state). Mirror of the global Settings → Purchasing card's checkCommitmentOptionCombo. Side change: exported `loadOverridesPanel` and `openOverrideModal` so the new tests can drive the panel and modal directly. Both are coherent units; just needed the export. Drive-by: removed unused `formatRelativeTime` import in `frontend/src/recommendations.ts` (pre-existing TS6133 blocking the pre-commit Run frontend tests hook on this branch — same fix as PR Tests: - `inline payment selector hides invalid options for RDS term=3 row` pins the RDS-no-3yr-no-upfront case. - `inline payment selector shows full list when term is unset` pins the fallback case. - `override-create modal hides invalid payment options on service+term change` pins the dependency loop and the reset- to-Inherit-when-stale snap. - All 40 existing settings-accounts tests still pass; 43 total. Backend save-side guard (the issue's third bullet) is deferred — the frontend gates close the practical bug; a backend mirror via the existing `commitmentopts.Service.Validate` machinery is its own PR-sized change touching `internal/api/handler_accounts.go` and would need probe-data integration tests. Worth filing as a follow-up. * fix(frontend): prevent override modal listener stacking Scope override payment-option listeners to an AbortController per modal open so reopening the modal cannot accumulate stale service/term handlers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #104.
Summary
Settings → Accounts → Service overrides had no UI to create an override — empty accounts saw "No service overrides set." with no path forward, and PR #72's inline payment editor inherited the same dead end (it edits existing rows; nobody could create the first row except via direct curl to
PUT /api/accounts/:id/service-overrides/:provider/:service).This PR adds the missing create flow as a modal:
applyOverrideScalarstreats unset fields as "use global default".How it interacts with PR #72
PR #72 makes the Payment column on existing override rows editable via an inline
<select>. This PR creates the rows in the first place. They're complementary — together they give users the full create+edit lifecycle in the UI without needing to know the API shape. Both modifyfrontend/src/settings.tsbut in different code regions (PR #72 insideloadOverridesPanel's table cells; this PR adds the modal handlers + anAdd overridebutton + the empty-state auto-open path), so a clean rebase should produce a small mechanical merge.Test plan
npx jest src/__tests__/settings-accounts.test.ts— 10 new tests + 22 existing = 32/32 pass. Coverage:npx jest— full frontend suite (1265 tests) greennpm run typecheckcleannpm run build(production webpack) clean🤖 Generated with claude-flow