Skip to content

fix: amount precision, icon upload FormData error, and unsaved-edit guard#41

Merged
darui3018823 merged 8 commits into
mainfrom
fix/edit-form-issues
Jul 6, 2026
Merged

fix: amount precision, icon upload FormData error, and unsaved-edit guard#41
darui3018823 merged 8 commits into
mainfrom
fix/edit-form-issues

Conversation

@darui3018823

Copy link
Copy Markdown
Member

Summary

Three user-reported issues fixed, one commit each:

  1. fix(form): reject amounts that exceed the safe integer range — Amounts above Number.MAX_SAFE_INTEGER were silently rounded by IEEE 754 doubles (e.g. 1145141919364364810 was stored as …800). The add/edit forms now validate against MAX_AMOUNT and show a dedicated error message (en/ja) instead of saving a corrupted value.

  2. fix(upload): replace uri-based FormData part with expo-file-system File — Expo SDK 54+ replaces global fetch with its WinterCG implementation, which rejects React Native's legacy { uri, name, type } FormData parts with "Unsupported FormDataPart implementation", breaking icon upload after cropping. Uploads now append an expo-file-system File on native (Blob on web). Adds expo-file-system as a direct dependency (native module already bundled via the expo package, so no new native build is required); lockfile regenerated with pnpm 10 and verified with frozen installs on both pnpm 10 and 11.

  3. feat(edit): confirm before discarding unsaved changes — Closing the edit screen via the × button / back gesture / swipe-down with unsaved edits now asks for confirmation (beforeRemove guard); saving skips the guard.

Test plan

  • Enter an amount like 1145141919364364810 in add/edit → "amount too large" error, value not saved
  • Edit screen → change icon → crop → ✓ → upload succeeds (no FormDataPart error)
  • Edit screen → change any field → × → discard confirmation appears; "破棄" closes, "編集を続ける" stays
  • Edit without changes → × closes immediately; saving closes without prompt

🤖 Generated with Claude Code

darui3018823 and others added 3 commits July 6, 2026 17:56
Amounts above Number.MAX_SAFE_INTEGER are silently rounded by IEEE 754
doubles (e.g. 1145141919364364810 became ...800), so validate against
MAX_AMOUNT in the add/edit forms and show a dedicated error instead of
storing a corrupted value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Expo SDK 54+ replaces global fetch with its WinterCG implementation,
which rejects React Native's legacy { uri, name, type } FormData parts
with "Unsupported FormDataPart implementation". Icon uploads after
cropping hit this on every submit.

Append an expo-file-system File (native) or a Blob read back from the
picker URI (web) instead, and let the file's own name/mime metadata
replace the manual extension-to-mime mapping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closing the edit screen with the x button (or back gesture) silently
dropped any edits. Intercept beforeRemove while the form differs from
the stored subscription and ask the user to keep editing or discard;
a successful save skips the guard.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

@codex review

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

@claude review

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

@gemini-code-assist review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several key improvements, including a validation check to reject subscription amounts exceeding the safe integer limit (MAX_AMOUNT) and a confirmation dialog in the edit screen to prevent users from accidentally discarding unsaved changes. Additionally, the file upload mechanism in lib/api.ts has been refactored to support both web and native platforms correctly using expo-file-system. The review feedback suggests optimizing the isDirty check in the edit form by using useMemo to cache the initial next payment date timestamp, which avoids redundant Date object instantiations on every keystroke.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread app/edit.tsx Outdated
Comment thread app/edit.tsx
@darui3018823

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19d3b65baf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/add.tsx Outdated
@darui3018823

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown

Claude finished @darui3018823's task in 3m 44s —— View job


Review complete ✅

  • Gather context (diff, changed files, existing review comments)
  • Review app/add.tsx and app/edit.tsx amount validation logic (incl. Codex's fractional-rounding concern)
  • Review lib/api.ts FormData/File upload change
  • Review app/edit.tsx unsaved-changes guard (incl. Gemini's useMemo suggestion)
  • Review lib/currency.ts, i18n changes, package.json/lockfile
  • Post final review

Summary

Good news first: both prior automated review comments are already resolved by follow-up commits on this branch, not just left open:

  • Codex's fractional-rounding concern (9007199254740991.1 slipping past the Number()-coerced comparison) was fixed in 1322177isAmountInputAboveMax (lib/currency.ts:39-60) now compares the raw integer-part string length/lexical order and checks for any nonzero trailing decimal digit, instead of relying on the rounded Number() value. I traced through the boundary cases (9007199254740991.1, ...990.9999, 20-digit overflow) and the logic holds up correctly in each.
  • Gemini's useMemo suggestion for initialNextPaymentTimestamp was applied in 7815ede (app/edit.tsx:79-82).

Amount validation (app/add.tsx:61, app/edit.tsx:126)

Both forms now call isAmountInputAboveMax(amount) on the raw input string before parseAmountInput, so the check runs on the un-coerced value — this is the right order and avoids the original bug class entirely.

One very low-risk edge case I noticed but don't think is worth blocking on: isAmountInputAboveMax's decimal regex ^(\d+)(?:\.(\d*))?$ (lib/currency.ts:47) doesn't match a leading +/-, so an input like +9007199254740991.5 falls through to decimalMatch === null → returns false (not flagged as too large), even though parseAmountInput would go on to coerce/round it silently. Given the field uses keyboardType="numeric" this is hard to trigger from the on-screen keyboard, but a pasted value could hit it. Not asking for a change here, just flagging for awareness.

Upload fix (lib/api.ts:544-556)

The web/native split (Blob via fetch().blob() on web, expo-file-system File on native) matches the documented WinterCG fetch constraint, and the expo-file-system: ~57.0.0 pin is consistent with the other expo-* deps already on SDK 57. I couldn't install dependencies in this sandbox (no node_modules, no network) to actually exercise the upload path, so I'm relying on the PR description's manual test plan for the runtime behavior — worth double-checking the icon still renders correctly post-upload on both a fresh native build and web once merged, per your own checklist.

Unsaved-changes guard (app/edit.tsx:78-119)

Logic looks correct: beforeRemove guard checks isDirtyRef/skipDirtyGuardRef, and handleSave sets skipDirtyGuardRef.current = true before router.back() so saving never prompts. The modal is registered with presentation: 'modal' in app/_layout.tsx:99, so this also covers the swipe-down dismiss gesture as called out in the test plan.

Minor note (not a bug): billingCycle/paymentMethod/currency come from the shared useAddFormStore, which is populated from subscription in a separate useEffect (app/edit.tsx:72-76) rather than at useState init time. There's a theoretical one-render window where isDirty could read stale values left over from a previous add/edit session before that effect runs, but it's not practically reachable by a user action and pre-dates this PR's change.

Environment note

This sandbox has no network access and no node_modules installed, so I wasn't able to run pnpm lint/pnpm install/build to verify formatting or type-check the expo-file-system File usage — the diff reads correctly by inspection, but if CI hasn't run lint/typecheck yet on this branch it'd be worth confirming there.

Overall: this looks solid and ready to merge pending CI. Nice job addressing both bots' feedback proactively in follow-up commits.

@darui3018823 darui3018823 merged commit ca95c21 into main Jul 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant