feat(settings): Azure and GCP create-override modal (closes #109) - #588
Conversation
Extend the per-account override create/edit flow to Azure and GCP accounts (previously AWS-only, tracked as follow-up to issue #104). - Drop `canCreate = provider === 'aws'` gate: all providers now render the "Add override" button and auto-open the create modal on empty state - Add AZURE_OVERRIDE_SERVICES and GCP_OVERRIDE_SERVICES lists; dispatch via getOverrideServicesForProvider so the service dropdown is provider-aware - Remove `o.provider === 'aws'` guards on term/payment/coverage table cells: all three get inline editing selects/inputs for every provider - Remove the disabled+tooltip guard on the enabled checkbox for non-AWS rows - buildPaymentOverrideSelect and syncOverridePaymentOptions now derive the fallback payment list from getCommitmentConfig(provider) instead of a hardcoded AWS-only constant, so Azure shows upfront/monthly and GCP shows monthly-only - Remove now-unused AWS_PAYMENT_OPTIONS constant (superseded by getCommitmentConfig which already owns the same data) - Tests: update three tests that asserted the old read-only behavior; add coverage for Azure payment options, Azure inline editing, and Azure empty-state modal auto-open with provider-correct service list
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR extends per-account service overrides from AWS-only to be provider-aware: adds Azure/GCP service lists and commitment-config payment fallbacks, enables inline editing and Enabled toggles for all providers, updates the override creation modal and empty-state behavior, and updates tests for Azure scenarios. ChangesProvider-aware service overrides for Azure and GCP
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/settings.ts (1)
1170-1172: ⚡ Quick winStale comment no longer reflects the implementation.
The comment states "AWS-editable, disabled (with tooltip) for non-AWS" but the code now renders an editable checkbox for all providers per issue
#109. Update or remove this comment to avoid confusion.📝 Suggested fix
- // Enabled cell: per-row checkbox toggling account_service_overrides.enabled. - // AWS-editable, disabled (with tooltip) for non-AWS to keep the column - // alignment consistent across provider types. Issue `#110`. + // Enabled cell: per-row checkbox toggling account_service_overrides.enabled. + // Editable for all providers. Issue `#110`, extended to Azure/GCP in `#109`. const enabledTd = tr.insertCell();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/settings.ts` around lines 1170 - 1172, The inline comment describing the "Enabled cell: per-row checkbox toggling account_service_overrides.enabled" is stale (says "AWS-editable, disabled (with tooltip) for non-AWS") and should be updated or removed to match the current behavior where the checkbox is editable for all providers per issue `#109`; edit the comment in frontend/src/settings.ts near the enabled-cell logic that mentions account_service_overrides.enabled (or remove it entirely) so it no longer refers to AWS-only behavior or tooltips and instead accurately reflects that the checkbox is editable for all providers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/settings.ts`:
- Around line 1170-1172: The inline comment describing the "Enabled cell:
per-row checkbox toggling account_service_overrides.enabled" is stale (says
"AWS-editable, disabled (with tooltip) for non-AWS") and should be updated or
removed to match the current behavior where the checkbox is editable for all
providers per issue `#109`; edit the comment in frontend/src/settings.ts near the
enabled-cell logic that mentions account_service_overrides.enabled (or remove it
entirely) so it no longer refers to AWS-only behavior or tooltips and instead
accurately reflects that the checkbox is editable for all providers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53f7cb3e-742a-4190-ab5f-e71bcedc77cd
📒 Files selected for processing (2)
frontend/src/__tests__/settings-accounts.test.tsfrontend/src/settings.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
canCreate = provider === 'aws'gate inloadOverridesPanel; Azure and GCP accounts now get the "Add override" button and auto-open the create modal on empty stateAZURE_OVERRIDE_SERVICESandGCP_OVERRIDE_SERVICESlists;getOverrideServicesForProviderdispatches to the right list so the service dropdown is provider-awareo.provider === 'aws'inline-editing guards for Term, Payment, Coverage, and Enabled cells on existing override rowsgetCommitmentConfig(provider).paymentsas fallback; Azure shows upfront/monthly, GCP shows monthly-onlyAWS_PAYMENT_OPTIONSconstant (superseded bygetCommitmentConfig)Test plan
npx tsc --noEmitpassesnpx jestpasses (1897/1897)Summary by CodeRabbit
New Features
Tests