Release: develop -> main - #1211
Merged
Merged
Conversation
…result (#1204) * fix(buy): stop the completion mail step from silently swallowing its result MailEdit is rendered by BuyCompletion when the account has no mail yet. Two defects turned that step into a dead end: BuyCompletion passed `onSubmit={(email) => (!email || email.length === 0) && close()}`, which for a non-empty address evaluates to `false && close()` and therefore does nothing. After a successful save the confirmation screen stayed exactly as it was — same address, same enabled Confirm button, no feedback — so the user clicked Confirm again. The account now carried a mail, which the API answers with 403 TFA_REQUIRED, and MailEdit rethrew that into an unhandled promise rejection with no UI change at all, inviting yet another click. In production this produced 1206 TFA_REQUIRED responses on PUT /v2/user/mail over 30 days against 19 successful sets. MailEdit now handles the 403 the way kyc.screen.tsx already does — redirect to /2fa with the Basic level — surfaces every other error through ErrorHint instead of discarding it, and tracks its own in-flight state so Confirm is disabled during the request regardless of the isUserUpdating flag from the react package. BuyCompletion closes on a successful submit, matching what its own Close button does in the branch for accounts that already have a mail. * fix(buy): explain the mail step's failure cases instead of redirecting out of the flow Follow-up to the previous commit, from review: The 2FA redirect it introduced was not an improvement. setRedirect stores only the pathname, while the completion view is component-local state in the buy screens, so returning from /2fa remounts an empty buy form: the typed address is gone, the buy completion screen is unreachable, and nothing explains why. That trades a visible dead end for a silent flow reset. This step can only ever set a FIRST address — changing an existing one also needs a code sent to the new address, and this component has no field to enter one — so it now says exactly that and leaves the user on the completion screen, where Close still works. Two more cases the previous commit routed into the generic error hint: - A 409 "account already exists - account merge request sent" reaches this step on the first-set path too. ErrorHint tells the user to try again, which only mails another merge request; the correct action is to follow the link already sent. It now shows the same hint the account and KYC screens use for this case. - A stale error survived the Change button, so a new address could be presented with the previous address's error still on screen. The new string is added to the German, French and Italian translations. * fix(settings): stop the mail parameter from re-submitting on every user refresh The effect that applies the mail app-parameter compares it against the cached user address with a case-sensitive !==, but the API lowercases the address on write. A parameter such as ?mail=John.Doe@Example.com therefore never satisfies the guard: it fires the update again on every change of the user object, which is one of the ways an account that already carries an address ends up hammering PUT /v2/user/mail and collecting 403 TFA_REQUIRED. Comparing case-insensitively lets the guard close once the address has been applied. * fix(buy): give the "account already has an address" case its own screen and a way out Review found the previous commit inconsistent with its own reasoning. It moved the 409 merge case out of ErrorHint because ErrorHint's fixed opening line tells the user to try again, which is the wrong action there — then routed the 2FA case into ErrorHint, where retrying is just as futile: the account has an address, so every further PUT answers 403 and this component has no field for the emailed code. The user read "Please try again" directly above "you can change it in your account settings", with Confirm still enabled. It also had no exit. BuyCompletion renders its Close button only in the branch for accounts that already carry an address, and both host screens suppress the layout back button while the completion is showing, so this state could only be left through the burger menu. The case now gets the same treatment as the merge hint: its own short screen with an OK button that ends the step. * fix(settings): bound the mail parameter to one submit per address The case-insensitive guard from the previous commit only mirrored half of the server's normaliser. The API applies trim and lowercase, so a padded parameter such as ?mail=%20a@b.com is stored as a@b.com while the guard keeps comparing against the padded value — permanently open, and re-submitting on every change of the user object exactly as before. The parameter is persisted to local storage, so that survives reloads. The guard now normalises the same way the server does, and additionally remembers the address it already submitted. That also covers the two cases normalisation alone cannot close: a value the API rejects outright, and a change that is accepted with 202 and stays pending mail verification, where the stored address legitimately does not move. The normalised address is what gets sent, which is what the server would store anyway. * fix(buy): give the completion screen a real exit when closing cannot do anything close() calls closeServices and then blanks the component. On /buy/info that combination strands the user: the screen passes navigateOnClose={false}, and closeServices only acts when the app runs as a widget, inside an iframe, or with a redirect URI configured — outside those it executes nothing at all. The body then renders empty with the layout back button suppressed, leaving only the burger menu. That state predates this branch, reachable through the existing Close button and the empty-address Finish path, but this PR routes three more outcomes into it. It now falls back to the account screen when closing has nowhere to go. * docs(buy): condense the comments on the mail step Keep why each branch exists — retrying cannot succeed, closing can be a no-op, the parameter guard has to normalise like the API — and drop the narration. * fix(buy): decide the completion exit on a real close, not on host flags isEmbedded and canClose describe the host, not whether closeServices did anything. A widget host that leaves its optional close callback unbound, and a stored redirect URI that is discarded as unsafe, both satisfied the flags while nothing fired -- so the completion blanked itself and, with the back button suppressed, stranded the user. closeServices now reports whether a channel actually fired and the completion falls back to the account screen when none did. Three callers that returned its result from a Promise<void> function are adjusted accordingly. * fix(settings): keep a transient mail-parameter failure retryable The guard was set before the request and never taken back, so a single offline blip or a rate-limit response lost the mail parameter for the rest of the session. It now survives anything transient and keeps blocking only what retrying cannot fix, and the rejection is handled instead of escaping unhandled. * chore(deps): move @dfx.swiss/react off the prerelease to 1.7.0 The lockfile pinned 1.7.0-beta.0, so npm ci installed the prerelease even though the caret range already allowed the stable release. 1.7.0 is the one that carries the updateMail cache refresh this branch builds on. Brings @dfx.swiss/core 0.6.0-beta.0 -> 0.6.0 along as a transitive requirement of react 1.7.0; nothing else in the lockfile moves. --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
…te (#1209) * docs(safe): say why a completed order may fall back to its creation date The fallback inside the completed branch covers a single case - an order completed before the valuta column existed - and the comment did not name it, which is the one thing a reader would want to know there. * docs(safe): describe the valuta fallback as an invariant guard The previous wording claimed a historical data gap that never existed: the migration adding the column backfilled every Completed row, and production holds none without a valuta. What the fallback actually guards is the invariant itself. * docs(safe): call the valuta fallback what it is Saying it guards the invariant overstated it: the fallback does not repair a completed order without a valuta, it hides the anomaly behind a less precise date. That is defensible in a rendered row and not where interest is computed - the backend raises the same case there - and the comment now says so instead of implying protection it does not provide.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 29, 2026 17:25
…1210) * fix(safe): format activity timestamps in Swiss notation The activity list took its date format from the app UI language, so an English interface rendered 1/28/26, 1:31 PM. The date format is a Swiss convention and does not follow the interface language, so the locale is fixed to de-CH regardless of the selected language. Explicit field options instead of dateStyle: 'short', which would shorten the year to two digits. * test(safe): regenerate Safe baselines for the Swiss date format The activity list is visible in every Safe screenshot, so the format change affects all of them. Regenerated locally against the API on the same seed data the previous baselines were taken on, so the diff shows the date format and nothing else. The PDF modal shot is unchanged: it covers the activity list. * test(safe): regenerate the account switcher baselines too safe-accounts.spec.ts renders the same /safe route and therefore the same activity list, so its four screenshots carried the US format as well. They were missed in the previous commit because the spec never names the component — only the route. Regenerated on the same seed data, so the diff again shows the date format and nothing else.
* test: give async waits the time a loaded machine needs transaction-document-error.test.tsx failed 1 in 5 full-suite runs on plain develop, with nothing changed: findByRole ran into DOM Testing Library's 1000ms default while the transaction list had not rendered yet. The test is not at fault. It passes 20 of 20 isolated runs, TransactionList has no timer in its load path, and both mocks resolve immediately. Measuring every findBy* in that file across ten full runs on an 18-core machine gave a median wait of 278ms, a longest wait of 1523ms, and 7 of 50 waits above the 1000ms limit. With that limit as the only changed variable, all ten runs passed. Jest defaults to one worker fewer than the machine has cores, and under that parallelism a worker goes without CPU for well over a second. asyncUtilTimeout now allows 5000ms, three times the longest wait measured. jest.setTimeout follows to 15000ms so it stays above it; otherwise Jest ends the test first and Testing Library's DOM dump naming the element it could not find is replaced by a bare "Exceeded timeout of 5000 ms". react-scripts does not accept testTimeout in the jest section of package.json, so it is set in setupTests. No assertion is weakened: each still waits for exactly the condition it waited for before, and a genuinely broken expectation still fails. The setting is suite-wide because every findBy* and waitFor runs against the same limit; transaction-document-error.test.tsx is only where it surfaced first. * build: keep the test setup out of the library build tsconfig.build.json excludes test code by name pattern alone — **/*.test.ts, **/*spec.ts, **/__tests__/** — and src/setupTests.ts matches none of them. It therefore stayed in the program that build:lib emits to dist/ and that the review bot type-checks, although it is test-only code. That went unnoticed while the file held nothing but an import. jest.setTimeout is the first value access to jest in it, and tsconfig.json sets typeRoots: ["typings"], which keeps node_modules/@types out of scope, so @types/jest never loads and the reference fails with TS2708. The whole test suite hits the same wall, which is why it is excluded from this config to begin with; this file was simply missed. Verified with the command the bot runs: tsc -p tsconfig.build.json --noEmit reports no errors, where it reported one before.
* perf(financial): skip balancesByType on the overview screen The overview screen polls dashboard/financial/log every minute but never reads balancesByType, which makes up ~82% of the response payload. Pass byType=false (now supported server-side) so the overview call skips that subtree; the history screen still requests it for its area chart. * fix(financial): keep the by-type charts type-safe instead of defaulting to zero The optional DTO field leaked into the chart component, turning a loud crash into a silent zero line. A derived, narrower entry type now carries the requirement, so the charts access the field directly again as before. The History screen resolves it once and states plainly when the data is missing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist