[IAmJSD]

Since #614, the per-service config PUTs need unrestricted account scope, like the global config PUT. A scoped admin can still click Save, which fires all 17 service PUTs; every one returns 403, and the toast read "Settings saved, but 17 services failed:" followed by the same denial 17 times, even though nothing was saved. This PR shows one "Failed to save settings: permission denied: ..." toast in that case, and only says "Settings saved, but ..." when something actually saved.

Closes #617

What changed

• frontend/src/settings.ts: saveGlobalSettings records whether the global PUT was sent (globalSent), and the failure toast text now comes from a new helper, serviceSaveFailureMessage:
  • "Settings saved, but N services failed: ..." only when something saved (global PUT sent, or at least one service PUT fulfilled).
  • Nothing saved and every rejection passes isPermissionDeniedError: "Failed to save settings: ", the same wording as a global-PUT 403.
  • Nothing saved for other reasons: "Failed to save settings: N services failed: ".
• The snapshot and dirty-marker logic from fix(settings): keep propagated default coverage on save and reject an empty field #538/fix(settings): send only changed global fields on Save #570/fix(frontend): keep edits made during a global save dirty #575/fix(config): Settings Save from a stale tab reverts the global default term, payment and coverage #539 is untouched: failed services keep their fields dirty for a retry.
• Three new tests in frontend/src/__tests__/settings-service-payment.test.ts.

Why this approach

The issue offers three options. Mapping permission-denied results to one failure toast is the smallest change and leaves the save and dirty-tracking flow alone. Skipping unchanged service PUTs would change what Save writes, which interacts with coverage propagation and the #539 stale-write handling (and the existing test pins 17 calls). Hiding or disabling settings for scoped users would need the session to expose account scope, which the frontend can't see today. The extra "nothing saved, other errors" branch fixes the same false "Settings saved" claim for non-permission failures.

Screenshots

Before is main (a14dc8f2), after is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API. The scenario is Admin > Purchasing policies with every config PUT mocked to 403, after changing the EC2 payment and clicking Save.

Before	After

• Before: "Settings saved, but 17 services failed:" with the same permission-denied reason repeated 17 times, filling the screen. After: one toast, "Failed to save settings: permission denied: global configuration requires unrestricted account access".

Verification

Run on this branch's head a89a19cd on top of main a14dc8f2:

• Lint: 0 errors (142 existing warnings). Typecheck and build pass.
• Jest: 109 suites, 3489 passed, 1 skipped. New tests: (1) scoped admin, all service PUTs 403 and no global change, gives exactly one toast "Failed to save settings: permission denied: ..." with the edited card still dirty; (2) all PUTs fail with mixed errors gives "Failed to save settings: 17 services failed: ..."; (3) global saved but all services fail still gives "Settings saved, but 17 services failed". Tests 1 and 2 fail on main a14dc8f2 and pass here. Randomized run: 18/18.
• Playwright: 157 passed. No new spec (toast copy only; jest covers it).
• git-secrets scan clean (planted key flagged); allowlist test 137 pass.

Notes for review

• The denied case reuses the backend message verbatim ("permission denied: global configuration requires unrestricted account access"), so it matches the global-PUT toast word for word.
• Root cause left in place: a scoped admin still sees an enabled Save on these settings. Disabling it properly needs the backend to expose the session's account scope (for example in /auth/me).
• Mixed case (some services denied, some fine) still lists each denied service, which is accurate because some services did save.

Overlaps

Other branches edit settings.ts in different regions only (#186 near line 3170, #734 near lines 1348-1509, #731, #728, #748), so no textual conflicts with the saveGlobalSettings hunk (about lines 3550-3700) are expected. #642 also edits settings.ts around Reset/reload and should be checked when combining. Those PRs will be opened alongside this one and merged in sequence, with conflicts resolved on the later branch.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 61be5b6
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8fcec40d-045b-42fb-a66a-092f15256188

Commits

Reviewing files that changed from the base of the PR and between 61be5b6 and a89a19c.

Files selected for processing: 2

• frontend/src/__tests__/settings-service-payment.test.ts
• frontend/src/settings.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE a89a19cd31a509f093163e7a979fa44b5d18f014

Blocking:

• none
  Non-blocking:
• The "global saved, every service denied" branch still lists 17 identical denials. It is unreachable for a scoped admin, because the global PUT 403s first and goes to the catch toast.
  Attacked: Walked saveGlobalSettings with globalSent false or true, all services fulfilled, rejected or mixed, and checked serviceSaveFailureMessage's savedSomething logic. reasons[0] is safe when every result is a rejected permission error (isPermissionDeniedError requires an Error with status 403 and a "permission denied" prefix). Confirmed defaultCoverageChanged implies globalSent. The snapshot and dirty-revert logic is untouched, and the new tests assert one toast with the card still dirty. "Closes fix(settings): a scoped admin's Settings Save shows 'Settings saved, but 17 services failed' after service-config PUTs became scope-gated #617" matches the acceptance criteria (one clear message plus a jest test).

[IAmJSD]

Local verification at a89a19cd merged onto current main (8e9aa9f7; merge-tree 6510d94e; Linux, Node, headless Chromium):

• This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
• The combined tree of every UI PR in this merge wave (tree 89dd34d8, main plus 46 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 3914 passed, 3915 total), and the full Playwright suite (208 passed).
• CI is green on a89a19cd (CI Success, Run pre-commit hooks).
• The independent review above approves this exact head.
• Nothing was applied to any cloud.

[IAmJSD]

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at a89a19cd.