fix(settings): clamp per-service payment options to supported combinations

[cristim]

Summary

Follow-up to #12 (Azure payment defaults, fixed in #27). The Settings → Purchasing form still let users set RDS 3-year + No Upfront (and the same for ElastiCache / OpenSearch / Redshift), which the AWS provider refuses at plan-execution time — see cmd/validators.go:warnRDS3YearNoUpfront. The plan-creation modal already guards against this via commitmentOptions.isValidCombination; this PR wires the same rules into the Settings form so a default that the backend can't honour can't even be typed in.

What changes

frontend/src/settings.ts

• Adds syncPaymentConstraintsForService(field) and syncAllServiceCommitmentConstraints() — for each AWS service with a restricted pairing, hide + disable the invalid <option> rows in the payment dropdown. If the currently-selected payment becomes invalid after a term change, it auto-clamps to the first valid option so the form never holds an unsavable value.
• Wires a change listener on each AWS service term dropdown → re-applies the constraint on that service's payment dropdown.
• Calls syncAllServiceCommitmentConstraints() at the end of loadGlobalSettings so legacy-persisted invalid combos (RDS 3yr + no-upfront, possible on settings saved before this fix) get clamped on load.
• Calls it from inside propagateTermToServices and propagatePaymentToServices so global-default propagation never silently writes invalid per-service combos.

Azure and GCP are intentionally out of scope — Azure has only two payment options (neither restricted by term) and GCP has no payment selector at all, so there are no same-shape bugs to fix there.

Test changes

New describe block per-service term/payment combination constraints:

• RDS 3yr hides "No Upfront", keeps Partial / All Upfront selectable
• RDS 1yr keeps all three payment options visible
• Switching RDS term 1→3 with "No Upfront" selected auto-clamps to a valid payment
• Legacy-persisted invalid combo (RDS 3yr + no-upfront) gets clamped on load
• EC2 3yr keeps all three options visible (no service-level restriction)
• ElastiCache / OpenSearch / Redshift all honour the same 3yr rule (via test.each)
• Propagating global "no-upfront" to all services while term=3 clamps the restricted services and leaves EC2 unchanged

The existing "sets up default payment to propagate to AWS services" test was updated to reflect the new clamping behaviour — previously it asserted that RDS would accept the propagated no-upfront, which was exactly the bug this PR fixes.

Test fixture updated so the AWS payment selects carry all three options (no-upfront, partial-upfront, all-upfront) like production — the previous fixture only had two and couldn't meaningfully exercise the constraint filtering.

Test plan

• npm test — 1243/1243 pass across 35 suites
• npm run typecheck — clean
• npm run build — production bundle builds
• Browser smoke-test in the deployed Lambda preview after merge: open Settings → Purchasing, pick RDS card, flip term between 1yr / 3yr, confirm the "No Upfront" option disappears / reappears appropriately; repeat for ElastiCache / OpenSearch / Redshift; confirm EC2 shows all three options regardless of term.

Refs: #12

@coderabbitai review

Summary by CodeRabbit

Release Notes

• New Features

  • Added support for additional AWS payment options with improved validation across services.

• Bug Fixes

  • Payment options now automatically adjust when changing service terms.
  • Invalid persisted configurations are automatically corrected on startup.
  • Payment option visibility is properly enforced based on selected term combinations.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 761de39d-53f6-4d7c-9e87-3480718ae3e1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR introduces per-service AWS payment/term validity constraints in the settings UI. It adds payment option visibility rules (e.g., RDS hides no-upfront at 3-year term), auto-clamps invalid payments when term selections change, validates persisted configurations on load, and synchronizes constraints across bulk propagation operations.

Changes

Cohort / File(s)	Summary
Test Updates frontend/src/__tests__/settings.test.ts	Updated payment dropdown test fixtures to include partial-upfront option; expanded test suite to validate per-service payment/term constraint visibility, auto-clamping on term change, legacy invalid config handling, and propagation behavior across services with different restrictions.
Constraint Implementation frontend/src/settings.ts	Added syncPaymentConstraintsForService helper to enforce per-service payment validity; wired term change handlers to trigger constraint sync; updated propagateTermToServices and propagatePaymentToServices to re-apply constraints after bulk operations; added full constraint sync to loadGlobalSettings to clamp invalid persisted combinations on initial load.

Sequence Diagram

sequenceDiagram
    actor User
    participant UI as Settings UI
    participant Handler as Term/Payment Handler
    participant Validator as Constraint Validator
    participant DOM as Form DOM

    User->>UI: Change service term dropdown
    UI->>Handler: Trigger term change event
    Handler->>Validator: Call syncPaymentConstraintsForService(service)
    Validator->>Validator: Check isValidCombination for each payment option
    Validator->>DOM: Hide/disable invalid payment options
    Validator->>DOM: If current payment invalid, clamp to first valid option
    DOM->>User: Display updated payment options

    User->>UI: Propagate payment to all services
    UI->>Handler: Call propagatePaymentToServices(payment)
    Handler->>DOM: Update payment value for all service dropdowns
    Handler->>Validator: Sync constraints for each service
    Validator->>DOM: Hide/disable + clamp per-service invalid combinations
    DOM->>User: Display final valid state across all services

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Constraints now reign, payment terms align,
Service by service, invalid options resign,
No more persistence of configs gone wrong,
The dropdown dance is finally strong!
AWS settings, now sage and refined! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing the settings form to properly clamp per-service payment options to valid AWS provider combinations.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/service-term-payment-combos

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[cristim]

@coderabbitai review

[coderabbitai]

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

frontend/src/settings.ts (1)

33-33: ⚠️ Potential issue | 🟡 Minor

Service key mismatch: savingsplans vs savings-plans.

SERVICE_FIELDS uses service: 'savingsplans' here, but commitmentOptions.ts registers the config under the key 'savings-plans' (with hyphen). When isValidCombination('aws', 'savingsplans', …) is called, the lookup fails and silently falls back to _default instead of the savings-plans-specific config.

Today both are identical (same terms and payments, no invalidCombinations), so constraint checking still works. However, if someone adds restrictions to the 'savings-plans' config in commitmentOptions.ts, this file will silently skip them. Fix by aligning the key:

Change SERVICE_FIELDS to use hyphenated form

-  { provider: 'aws', service: 'savingsplans', termId: 'aws-savingsplans-term', paymentId: 'aws-savingsplans-payment' },
+  { provider: 'aws', service: 'savings-plans', termId: 'aws-savingsplans-term', paymentId: 'aws-savingsplans-payment' },

The DOM ids (aws-savingsplans-*) can remain unchanged; only the service key needs aligning with commitmentOptions.ts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/settings.ts` at line 33, SERVICE_FIELDS currently uses service:
'savingsplans' which does not match the key 'savings-plans' registered in
commitmentOptions.ts, causing isValidCombination('aws','savingsplans',...) to
miss the specific config; update the SERVICE_FIELDS entry for AWS to use the
hyphenated service key 'savings-plans' (leave DOM ids like
'aws-savingsplans-term' unchanged) so lookups in commitmentOptions.ts and calls
to isValidCombination resolve to the correct config.

🧹 Nitpick comments (1)

frontend/src/__tests__/settings.test.ts (1)

735-862: Solid coverage for the new constraint wiring.

The suite exercises the three behavioral axes the implementation cares about:

• Visibility differences by term (RDS 3yr vs 1yr).
• Event-driven clamp on term change.
• Load-time clamp for legacy-persisted invalid combos.
• Negative case (EC2 is unrestricted).
• .each for the other restricted services (elasticache/opensearch/redshift) avoids repetition.
• Propagation path end-to-end (global default → per-service clamp).

Two minor optional refinements:

1. The await Promise.resolve(); await Promise.resolve(); pump at lines 853–854 is brittle — if confirmAndPropagatePayment ever gains an extra await in its chain the test will silently pass before the assertion is meaningful. Prefer an explicit settle helper such as await new Promise(r => setTimeout(r, 0)); (matching what setupSettingsHandlers tests already use at lines 192/208/230).
2. memorydb has the same { term: 3, payment: 'no-upfront' } restriction in commitmentOptions.ts but no entry in SERVICE_FIELDS, so it's correctly omitted from the .each. If a MemoryDB card is ever added to the Settings UI, extend the .each to cover it.

💚 Proposed test-settling tweak

-      // Allow the async confirmDialog promise to resolve.
-      await Promise.resolve();
-      await Promise.resolve();
+      // Allow the async confirmDialog → propagate chain to settle.
+      await new Promise((r) => setTimeout(r, 0));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/__tests__/settings.test.ts` around lines 735 - 862, Replace the
brittle double Promise.resolve pump after dispatching the defaultPayment change
with an explicit microtask tick (e.g. await new Promise(r => setTimeout(r, 0)))
so the test reliably waits for confirmAndPropagatePayment/confirmDialog promise
chains to settle; also, note that memorydb currently has the same restriction in
commitmentOptions.ts but is absent from SERVICE_FIELDS, so if a MemoryDB
settings card is later added extend the test.each([...]) that enumerates
'elasticache','opensearch','redshift' to include 'memorydb' to keep coverage
consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@frontend/src/settings.ts`:
- Line 33: SERVICE_FIELDS currently uses service: 'savingsplans' which does not
match the key 'savings-plans' registered in commitmentOptions.ts, causing
isValidCombination('aws','savingsplans',...) to miss the specific config; update
the SERVICE_FIELDS entry for AWS to use the hyphenated service key
'savings-plans' (leave DOM ids like 'aws-savingsplans-term' unchanged) so
lookups in commitmentOptions.ts and calls to isValidCombination resolve to the
correct config.

---

Nitpick comments:
In `@frontend/src/__tests__/settings.test.ts`:
- Around line 735-862: Replace the brittle double Promise.resolve pump after
dispatching the defaultPayment change with an explicit microtask tick (e.g.
await new Promise(r => setTimeout(r, 0))) so the test reliably waits for
confirmAndPropagatePayment/confirmDialog promise chains to settle; also, note
that memorydb currently has the same restriction in commitmentOptions.ts but is
absent from SERVICE_FIELDS, so if a MemoryDB settings card is later added extend
the test.each([...]) that enumerates 'elasticache','opensearch','redshift' to
include 'memorydb' to keep coverage consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5dc4bde3-5823-4b60-ab57-9e0290757e42

📥 Commits

Reviewing files that changed from the base of the PR and between f760d4d and a62260c.

📒 Files selected for processing (2)

• frontend/src/__tests__/settings.test.ts
• frontend/src/settings.ts