ux(settings): add payment option selector to AWS account overrides modal - #72
Merged
cristim merged 1 commit intoApr 27, 2026
Conversation
The Settings → Accounts → Overrides panel previously listed each per-service
override as a static row showing Term / Payment / Coverage with no way to
edit the payment option. AWS teams running mixed payment strategies across
accounts (No Upfront in dev, All Upfront in prod) had no UI path to set this
per account — they had to call the API directly.
This change replaces the read-only Payment cell with a `<select>` (AWS rows
only — Azure/GCP reservations don't expose distinct payment options) bound
to the existing `PUT /api/accounts/:id/service-overrides/:provider/:service`
endpoint. The handler's `applyOverrideScalars` already treats unset request
fields as "leave alone", so sending `{ payment }` preserves term/coverage/etc.
UX:
- "Inherit (default)" is the initial choice when an override has no payment
set; selecting any of {No Upfront, Partial Upfront, All Upfront} writes
the value via PUT.
- When a payment is already set, the Inherit option is disabled because the
backend has no clear-field channel — the user is directed to the existing
Reset action which removes the entire override. This is documented in the
option's title attribute.
- Success/failure surface via toast; the select is rolled back to its prior
value on error and the panel re-renders on success so all rows reflect
fresh server state.
Tests in settings-accounts.test.ts cover render (Inherit + 3 AWS options
present), save (PUT receives only `{ payment }`), the Inherit-disabled
branch when payment was already set, and the AWS-only guard so Azure rows
keep the read-only cell.
Closes #23
Member
Author
|
@coderabbitai review |
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 41 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
Contributor
✅ Actions performedReview triggered.
|
This was referenced Apr 27, 2026
cristim
added a commit
that referenced
this pull request
Apr 27, 2026
#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
added a commit
that referenced
this pull request
Apr 27, 2026
#104) (#106) 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.
This was referenced Apr 27, 2026
Closed
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 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.
Merged
7 tasks
cristim
added a commit
that referenced
this pull request
May 14, 2026
…e rows (closes #110) (#396) V2 of #72. PR #72 made the Payment cell on existing override rows editable; this extends the same sparse-PUT pattern to the remaining editable scalars on `account_service_overrides`. Changes (AWS rows only; Azure/GCP inline editing tracked under the provider-aware modal work): - Term cell: inline <select> (Inherit / 1yr / 3yr) filtered by `getValidTermOptions(provider, service, payment)` so RDS rows with payment=no-upfront do not list term=3 (the one hard AWS rule per commitmentOptions.ts). Inherit is disabled when term is already set (no clear-field channel, same rationale as Payment's Inherit). - Coverage cell: inline numeric <input> (0..100, integer). Fires on change (blur or Enter), not input, so a single PUT per finished edit. Out-of-range or non-integer input reverts to previous value with an error toast; clearing a preset value reverts (no clear-field) with an info toast pointing the user at Delete. - Enabled toggle: per-row checkbox, second-to-last column. Toggling disabled adds `.override-disabled` to the row (CSS `opacity: 0.55`) so the inactive state is visually obvious. Legacy rows with `enabled === undefined` render as ACTIVE; only explicit `enabled: false` renders unchecked + dimmed. For non-AWS rows the new column slot keeps the layout consistent: the Enabled checkbox renders disabled with an explanatory tooltip; Term and Coverage stay as read-only text cells until the provider-aware modal lands. Each handler mirrors `handlePaymentOverrideChange`'s shape verbatim: sparse PUT of just the touched field (backend's `applyOverrideScalars` treats unset request fields as "leave alone"), `loadOverridesPanel` reload + `refreshRecommendationsAfterOverrideChange` on success, revert-and-toast on failure. Tests (15 new in `settings-accounts.test.ts`) cover per-cell render, sparse-PUT payload shape, invalid-input revert paths, Inherit-disabled when value already set, AWS-only guards on non-AWS rows, and one Term sad-path that exercises the shared revert-on-error behaviour. Full frontend suite (46 suites, 1660 tests) green; production webpack build green; npx tsc --noEmit clean. The three new builder/handler pairs share the same template as `buildPaymentOverrideSelect`/`handlePaymentOverrideChange`. Extracting a common helper would also touch the existing Payment code path and expand scope beyond this issue; filing a follow-up refactor after this PR lands. Closes #110
cristim
added a commit
that referenced
this pull request
Jun 19, 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
Jul 10, 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
Jul 17, 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
Jul 19, 2026
) * docs(README): document per-account service overrides 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). * docs(readme): correct stale override edit/delete wording to shipped UI (refs #117) - Replace stale "#110 tracked / delete-and-recreate" limitation with the shipped reality: AWS overrides support inline edit of Term, Payment, Coverage, and Enabled directly on the row; non-AWS providers are read-only. - Rename "Reset" to "Delete" throughout the override-editing section to match the current button label and confirm dialog title (per #114). * docs(README): fix stale provider-boundary and em-dash issues in override section - Replace the stale "AWS-only V1 boundary" callout with accurate text: the override modal is provider-aware and lists services for the account's provider (AWS, Azure, GCP). Issues #109 and #110 shipped and both the create modal and inline-edit work for all providers. - Replace "Non-AWS overrides display field values as read-only text" with the correct statement that all override rows support inline edit (Term, Payment, Coverage, Enabled). References to tracking issue #109 removed as it is closed. - Replace five U+2014 em-dashes in the new section with punctuation that matches the project prose style (semicolons, parentheses, period).
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.
Summary
<select>for AWS rows so users can set the payment option (No Upfront / Partial Upfront / All Upfront) per account, addressing the gap reported in ux(settings): account-level overrides panel missing payment option selector for AWS accounts #23.PUT /api/accounts/:id/service-overrides/:provider/:servicewith{ payment }only, so other override fields (term, coverage, …) are preserved by the backend's existingapplyOverrideScalarslogic.Test plan
npx jest src/__tests__/settings-accounts.test.ts— 4 new tests cover render, save payload, Inherit-disabled when payment was preset, and the AWS-only guard for non-AWS rows.npx jest— full frontend suite (1255 tests) green.npm run typechecknpm run build(production webpack)Closes #23