Repository navigation
fix(mobile): make low-balance alert validation and save errors clear - #8491
Conversation
The threshold field showed a value-shaped placeholder while the alert had no stored amount, so a freshly enabled alert looked filled with Save disabled. It now shows a localized hint, and Save is disabled only while the amount is actually missing. Recipient validation now separates an empty list from malformed entries, and checks each entry against the procedure's own address rule, so an address the server would reject is reported at the field instead of after a round trip. A rejected save renders localized copy where the problem is: the recipient field for an input rejection, and next to Save for any other failure. It no longer shows the server's message, and correcting either field clears it. The dead emailsError key is removed and the five new strings are translated in every catalog.
Run the repository formatter over the new mounted suite so the format check passes.
|
bot: Android emulator evidence for the low balance alert sheet, Hindi UI, local dev stack, seeded fake account and organization fixture (stored recipient addresses, no stored threshold). The enabled-alert state is in the comment above.
The input-rejection branch from the procedure itself is proven by the mounted test, not by the device: with the client rule matching the schema, the device cannot reach it. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit makes the mounted harness re-render from the mocked mutation state and adds the missing pre/post assertions, so the "drops the rejected-save line" test now proves the recipient-field line clears; no new issues in the changed lines. Assumptions: Files Reviewed (1 file in this increment)
Previous Review Summary (commit 7def605)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 7def605)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe threshold-hint, email-validation, and localized save-error changes are correct and covered; the single finding is a mounted test that asserts a harness call instead of the line-clearing behavior its name promises. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (91 files)
Reviewed by deepseek-flash-latest · Input: 51.5K · Output: 6K · Cached: 392.2K Review guidance: REVIEW.md from base branch |
The suite asserted only that reset is called. Make the mocked reset clear the mutation error and re-render, then assert the recipient-field line is gone and Save still usable, which is the behavior the test is named for.





What changed
apps/mobile/src/components/organization/low-balance-alert-sheet.tsxandlow-balance-alert-validators.ts:10.00placeholder is replaced by a localized hint (Enter an amount), so switching the alert on shows an empty field and a disabled Save that agree with each other. No default amount is written.organization.lowBalanceAlert.emailsErrorkey and added five keys toen.json, translated into the other 86 catalogs (check:i18nrequires key parity and rejects untranslated keys).Verification
Local gates:
pnpm --filter kilo-app check:i18n→check-catalogs: 87 languages, every catalog matches en.jsonpnpm --filter kilo-app check:plurals→ cleannpx vitest run src/components/organization→ 16 files, 161 tests pass, including the newlow-balance-alert-sheet.mounted.test.tsxand the rewritten validator suitepnpm --filter kilo-app typecheck→ cleanoxlint --config apps/mobile/.oxlintrc.jsonon the four changed source files → 0 errorsAndroid emulator, local dev stack on this VM, app language Hindi, seeded fake account and organization fixture (six stored recipient addresses, no stored threshold):
राशि दर्ज करें), its accessibilityhintequals the displayed text (field empty), and Save reportsenabled="false".one@example.com, two@example.com, broken-address: the field showsएक से ज़्यादा ईमेल पते मान्य नहीं हैं।once, the comma hint appears once below it, and the old combined sentence is absent. Save stays disabled.one@example.com,a@b..com(accepted by the previous pattern, rejected by the server rule): the same localized line appears locally, Save disabled — the server round trip no longer happens.कम से कम एक ईमेल पता दर्ज करें।— the required-copy branch, distinct from the invalid-entry line.25, two valid recipients): no error line, Saveenabled="true". Pressing Save closed the sheet and persisted{"minimum_balance": 25, "minimum_balance_alert_email": [...]}to the local database, so the save path still works.The input-rejection branch itself (a
BAD_REQUESTfrom the procedure) cannot be reached from the device once the client rule matches the schema; a mounted test drives that branch and asserts the recipient-field copy renders instead of the server's message, and that editing the field resets the mutation. The transient-failure branch is covered by the same suite.Screenshots of states 1–5 are attached in the comment below.
Out of scope
The same class of placeholder on the spend-alerts screen, the web spending-alerts modal's raw mutation message, and the invite sheet's raw mutation message are untouched: different screens, not part of this work item.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Hint placeholder
Confidence: 0.70 · Precedent: Product scope: user-caused costs and settings come from user choice, with a Kilo default the user can override (session d85d895b2835). A prefilled 10 would save an alert the user never picked. The hint fixes the 'looks filled' defect with no hidden value.
/.+@.+\..+/(accepts emoji, combining marks,a@b..com), while the server's zod.email()rejects them, which is what produced the raw server JSON. How strict should the client be?Answer: Match the server rule
Confidence: 0.80 · Precedent: Fix the root cause and the class of the issue (session 7f3470e0ba6f, "fix class of issue preventing progress"). A client/server rule mismatch is the root cause of the raw server error.
tools/i18n/check-catalogs.mjs(run bypnpm --filter kilo-app check:i18n, part of CI) fails when a locale lacks an English key and when a new key equals English in every locale. Recent mobile PRs fill all 86 catalogs in the same commit. What should this PR do?Answer: Fill all 86 catalogs
Confidence: 0.75 · Precedent: Localize every string in the mobile app (session dcb3f9170164, "the mobile app always must respect i18n"). Keep CI checks that catch real errors (session 99e88001c4b4). The Hindi e2e also needs real translations.
updateMinimumBalanceAlertcan fail transiently (network, 5xx) and today renders its raw English message in the same place. How far should this PR go?Answer: Localize both
Confidence: 0.80 · Precedent: If you see a gap, fix it (session 89824a222897, "if you saw a gap, fix it"). Plans cover the retryable-error state (session df491946927a).
Answer: One generic invalid line
Confidence: 0.60 · Precedent: Do the least work possible with no drop in quality (session 7f3470e0ba6f, "do the least amount of work possible, with no drop in quality"). The brief explicitly allows saying that one entry is invalid.