Skip to content

fix(mobile): make low-balance alert validation and save errors clear - #8491

Merged
iscekic merged 3 commits into
mainfrom
drain/low-balance-alert-validation-copy
Oct 21, 2026
Merged

iscekic merged 3 commits into
mainfrom
drain/low-balance-alert-validation-copy

Conversation

@iscekic

@iscekic iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

What changed

apps/mobile/src/components/organization/low-balance-alert-sheet.tsx and low-balance-alert-validators.ts:

  • The threshold field no longer looks filled while it is empty. The 10.00 placeholder 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.
  • The recipient error says what is wrong. An empty list and malformed entries now get separate sentences, and the malformed-entry line no longer repeats the comma hint rendered below the field.
  • Client validation matches the procedure's schema. Each entry is checked with the same zod address rule the procedure uses, so an address the server would reject is reported at the field instead of after a round trip.
  • A rejected save is localized and placed. An input rejection renders on the recipient field; any other failure renders next to Save. The raw server message is never shown, editing either field clears the line, and Save stays available after the field is corrected (the draft survives a failure).
  • Removed the now-dead organization.lowBalanceAlert.emailsError key and added five keys to en.json, translated into the other 86 catalogs (check:i18n requires key parity and rejects untranslated keys).

Verification

Local gates:

  • pnpm --filter kilo-app check:i18n → check-catalogs: 87 languages, every catalog matches en.json
  • pnpm --filter kilo-app check:plurals → clean
  • npx vitest run src/components/organization → 16 files, 161 tests pass, including the new low-balance-alert-sheet.mounted.test.tsx and the rewritten validator suite
  • pnpm --filter kilo-app typecheck → clean
  • oxlint --config apps/mobile/.oxlintrc.json on the four changed source files → 0 errors

Android emulator, local dev stack on this VM, app language Hindi, seeded fake account and organization fixture (six stored recipient addresses, no stored threshold):

  1. Enabled the alert: the threshold field shows the Hindi hint (राशि दर्ज करें), its accessibility hint equals the displayed text (field empty), and Save reports enabled="false".
  2. Entered 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.
  3. Entered 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.
  4. Cleared the field: कम से कम एक ईमेल पता दर्ज करें। — the required-copy branch, distinct from the invalid-entry line.
  5. Corrected both fields (amount 25, two valid recipients): no error line, Save enabled="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_REQUEST from 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.

  1. Question: Empty threshold: When the low-balance alert has never been configured, the threshold field is empty but shows a "10.00" placeholder, so the form looks filled while Save is disabled. How should it be fixed?
    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.
  2. Question: Client email rule: The mobile field accepts addresses with the lax pattern /.+@.+\..+/ (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.
  3. Question: Locale catalogs: The brief says new copy in en.json only, but tools/i18n/check-catalogs.mjs (run by pnpm --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.
  4. Question: Other save failures: Besides the input-schema rejection, updateMinimumBalanceAlert can 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).
  5. Question: Invalid-recipient copy: How should the invalid-recipient message be worded (it must not repeat the comma hint)?
    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.

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.
@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Oct 21, 2026
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Low balance alert sheet state in the Hindi UI on the emulator.

Enabled alert sheet with the empty threshold hint

Run the repository formatter over the new mounted suite so the format check
passes.
@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

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.

  1. A malformed entry (broken-address) in a list of valid addresses: the recipient field shows the invalid-entry line once and Save stays disabled; the old combined sentence is gone, and the comma hint appears once below the field.
  2. An address the previous pattern accepted but the server schema rejects (a@b..com): the same localized line appears at the field, so no round trip is needed and Save stays disabled.
  3. Empty recipient field: the required line instead of the invalid-entry line.
  4. Corrected form (a valid amount and valid recipients): no error line and Save enabled; pressing Save closed the sheet and persisted the values to the local database.

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.

@iscekic

iscekic commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Remaining low balance alert sheet states from the same emulator run.

  1. A malformed entry in a list of valid addresses.
  2. An address the previous pattern accepted but the server schema rejects.
  3. Empty recipient field.
  4. Corrected form with Save available.

Malformed entry line at the recipient field

Schema-rejected address reported at the field

Required line for an empty recipient field

Corrected form with Save available

@kilo-code-bot

kilo-code-bot Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The 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: [INFERENCE] Tests were not executed in this read-only review; the clearing behavior is verified by inspection of the harness, the reset mock, and clearSaveError in low-balance-alert-sheet.tsx:73. The previous finding on this test is resolved, not carried forward.

Files Reviewed (1 file in this increment)
  • apps/mobile/src/components/organization/low-balance-alert-sheet.mounted.test.tsx - no issues
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 Summary

The 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: [INFERENCE] saveErrorCopy maps every BAD_REQUEST to the recipient field. The server's schema refine also rejects a non-positive threshold, which the client currently blocks locally, so no reachable save rejection is misattributed today.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/mobile/src/components/organization/low-balance-alert-sheet.mounted.test.tsx 185 "Drops the rejected-save line" test only asserts reset() was called; the mocked mutation keeps isError: true, so the line is never asserted gone.
Files Reviewed (91 files)
  • apps/mobile/src/components/organization/low-balance-alert-sheet.tsx - no issues
  • apps/mobile/src/components/organization/low-balance-alert-sheet.mounted.test.tsx - 1 issue
  • apps/mobile/src/components/organization/low-balance-alert-validators.ts - no issues
  • apps/mobile/src/components/organization/low-balance-alert-validators.test.ts - no issues
  • apps/mobile/src/i18n/locales/*.json (86 catalogs) - key parity and removal of emailsError verified, no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 51.5K · Output: 6K · Cached: 392.2K

Review guidance: REVIEW.md from base branch main

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.
@iscekic iscekic added the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants