Release: develop -> main - #1229
Merged
Merged
Conversation
…locale (#1222) * fix(format): render dates in Swiss notation outside the Safe Date formatting is a Swiss convention, not a translation of the interface, but every call site outside the Safe passed no locale to toLocaleString(), so the format followed the browser locale: an English UI rendered 6/12/26, 2:30 PM instead of 12.06.2026, 14:30. Adds formatSwissDate() and formatSwissDateTime() to util/utils.ts, next to the existing formatChf()/formatCurrency() helpers, and routes the compliance and RealUnit call sites through them. Explicit field options rather than dateStyle: 'short', which would shorten the year to two digits. The compliance formatDate()/formatDateTime() helpers now delegate to the shared ones, which covers their callers across the compliance components without touching each import. formatDateTimeShort() keeps its own option set: the two-digit year is the point of that variant. It only gains the fixed locale. * fix(format): keep compliance seconds and unify the remaining date sites Review follow-up on the shared Swiss date helpers. A bare toLocaleString() emits seconds, so routing the compliance formatDateTime() through a minute-precision helper silently dropped them across roughly 27 call sites - note lists, call queue comments and issue tables, where events can share a minute and the timestamp is what orders them. Adds formatSwissDateTimeWithSeconds() and uses it for the compliance views, which now keep exactly the precision they had. Folds the Safe formatTimestamp() into formatSwissDateTime(). It carried an inline copy of the same option set since the Safe fix; the output is identical, so the Safe baselines stay valid. Eight compliance sites passed de-CH but no field options and rendered 12.6.2026 next to the padded 12.06.2026 - among them a KYC step's created date shown both ways on adjacent screens. They now use formatSwissDate(). Two more sites followed the UI language on screens this change already pins: the support dashboard bucket labels and the recommendation expiry date on the account screen. Adds unit tests for the three helpers, built from local date components so they hold in any timezone the suite runs in. * fix(format): preserve timestamp precision and fold the last duplicates Second review round. Precision is now preserved everywhere rather than only in compliance. The ten RealUnit and account sites also emitted seconds through a bare toLocaleString() and had silently dropped to minute precision; they use formatSwissDateTimeWithSeconds() again. The rule across the change is that notation is fixed and precision is left exactly as each site had it, so the Safe keeps the minute precision it chose explicitly. The live financial dashboard passed de-CH without field options and showed an unpadded timestamp directly above the padded log table below it. Five compliance screens carried their own formatDate() with an option set identical to the shared helpers; they delegate now and keep their '-' guards for missing values. trendLabel() took no locale for the month branch, which is the branch the default 365-day period actually renders. A month name is a word rather than notation, so it follows the interface language, as the other month labels in the app already do; the numeric day label stays Swiss. The locale is passed in explicitly, so no caller falls back to the browser. * fix(support): let the trend labels follow a language switch Third review round. loadStats() reads the locale but did not list it as a dependency, and the hook functions around it are memoized on a stable identity, so the callback captured the language active when the screen mounted. Switching the interface language left the month labels in the previous language, and a period change did not clear it either - which defeats the point of passing the locale in at all. Neither gate catches this: the lint config does not load the React hooks plugin, and the build runs with CI=false, so the warning never fails anything. Also widens a comment and a test name that still said "compliance": the seconds variant serves every site that emitted seconds before, which since the last round includes the RealUnit and account screens. The previous commit message justified the live-dashboard change with a log table sitting below that card. That table is rendered nowhere in the app - the component has no importers at all - so the justification was wrong, though the change itself is right and matches the rule the rest of the change follows. The PR description carries the corrected wording. * fix(format): convert the reachable transaction screens and guard the stats race Fourth review round. The account list links one click into the transaction detail. Converting only the list left the same timestamp reading two ways a navigation apart, where before this change both sides followed the browser and at least agreed. The three transaction detail sites and the support ticket list are converted too; all four preserve their field set, seconds included. The payment screens and the chat stay out of scope: nothing this change converts navigates into them. Passing the locale into loadStats() also made the language a second trigger for a fetch that had no request guard, so a slow earlier response could land last and put the previous language's labels back - the very bug the locale was threaded in to fix. Only the newest request may write now. The period selector could already race the same way; it is covered by the same guard. * fix(format): drop a site that was never locale-dependent, localize labels at render Fifth review round. The price step timestamp is typed Date but arrives as a raw ISO string: the API layer parses JSON without a reviver, so the old call landed on String.prototype.toLocaleString() and returned the string unchanged - in UTC and identical in every locale. Converting it moved the instant into the viewer's zone, dropped the Z marker and dropped the milliseconds, which is the opposite of what this change promises. It is reverted, with a comment so the next reader does not repeat it. The other two sites on that screen are real Date values and stay converted. Loading the statistics per language turned a relabel into a refetch: the chart blanked behind a spinner on every language switch, and a refetch that failed replaced statistics that were already correct with an error page. The buckets are stored with their stable keys now and localized in a memo at render time, so a language switch only relabels. The request guard stays for the period selector, which can still race itself, and the fallback ticket fetch is no longer issued at all once its request is superseded. * fix(support): keep the statistics fallback reachable, pin the chat clock Sixth review round. Storing the statistics DTO unchecked lost a behaviour the old code had by accident: reading dto.trend threw on an empty or unparseable 200 body - the api layer resolves those as undefined - and that throw is what dropped into the ticket fallback. Without it the view spun forever, with no fallback, no error and no way back short of a reload; a partial DTO was worse, because the render-time memo then threw outside the promise chain. The response is checked explicitly now. The chat message time asked for hour12: false against the browser locale. That is not the same as a 24-hour clock: on locales that default to h24 it renders midnight as 24:30, so an en-US browser disagreed with every converted site by an hour a day, one click from the converted ticket list. It uses a fixed Swiss time now, with a test for the midnight case. * fix(support): check the statistics response shape, not its truthiness Seventh review round. The guard added last round tested truthiness, which is weaker than the behaviour it restored. A 200 body whose trend is truthy but not an array - a serialized map, say - passed it, was stored, and then threw in the render-time memo instead: outside the promise chain, so the ticket fallback never ran and the router error screen replaced the dashboard. Reading dto.trend on develop threw inside the promise and fell back correctly. Array.isArray covers the undefined and null cases too. The two tests added for the Swiss time helper did not actually pin it. The plain 14:30 assertion passes with the old browser-locale implementation in every language the app offers, and the midnight one only fails where the runner's own locale resolves to a 24-hour clock - so it would pass on a European machine and catch nothing. A third test asserts the fixed locale directly; reverting the helper now fails two of the three. * fix(test): make the spy-based time test failure-safe Closing out round 8. Two findings from the last review pass, applied directly rather than starting another round. The spy on Date.prototype.toLocaleTimeString only restored on the happy path; a failing assertion would have left it in place for every later test in the run. It restores in a finally now. The PR body's carve-out for the statistics-tab screenshot gave the wrong reason. That shot does render a changed line - trendLabel's month branch - it just reproduces its old output today, because the default period selects month granularity and the app locale derives from the browser language, so both resolve the same short month name on any machine that took the existing baseline. Worded accurately instead of "wrong tab". * test(e2e): regenerate the one baseline this change can cleanly affect Ran the local API stack per CONTRIBUTING.md and checked all six specs the description lists as touched. Three read `formatDate`/`formatDateTime`/`trendLabel` output through a real backend and need named fixtures (a specific customer, tickets in specific states) that no available seed script creates - compliance.spec.ts, support-dashboard-overview.spec.ts and support-dashboard.spec.ts stay stale for that reason, unrelated to this change. Of the three that mock their feature data and only needed working admin auth, six of their seven shots keep the app under this repo's own 5000-pixel tolerance - the date-string length change is real but small. realunit-compliance's dossier baseline is unusable for a different reason: independent of this change, it already reflects substantial unrelated UI drift (a Checks column, layout and fields the current screen no longer has) against the current app, so regenerating it would bury this change inside a confusing, unrelated diff. Only realunit-support-01-list crosses the tolerance on a clean diff - verified pixel-for-pixel that only the Created/Last Msg columns changed; every other cell is identical.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
August 1, 2026 00:40
* Report the errors customers are shown The error screen is the router error element, so it is the one place where the error that broke the render is still available. It read none of it: useRouteError was never called, the error object was discarded, and the failure existed nowhere but the customer browser console. The screen now reads the error and reports it, and the chunk-reload guard moves to the same place. The guard previously listened on window error and unhandledrejection events, which a failed React.lazy import never reaches, because React hands it to the error boundary instead. So the recovery never ran for the case it was written for. Also reports the deliberate navigations to /error?msg=..., which have no route and therefore surfaced as a bare 404 saying nothing about the actual failure. The comment in public/_redirects claimed real files are matched before the SPA fallback. That holds only while they exist: a build asset that does not is answered with index.html and status 200, which the /static/* header rule then stamps with a year of immutable cache. Corrected to record what a request for an absent asset actually returns. * Stop classifying a JSON response that arrived as HTML as a chunk failure Chunk detection also matched the bare syntax wordings a browser produces while parsing HTML as a script. One of them, Unexpected token '<', is what JSON.parse reports when a response is HTML instead of JSON - an everyday failure whenever a gateway, WAF or login redirect answers an API call. That would have reloaded the page and discarded whatever the customer had typed, which is worse than the failure it was meant to recover from. Verified against the wording current browser engines produce. The bundler wordings identify the real case on their own, so the generic ones are gone and a regression test holds the line. The MIME type refusal stays: a response cannot reach JSON.parse and be refused for its MIME type. Also: an empty message no longer drops the whole report, since the endpoint requires one and the rejection was swallowed silently - the exact blind spot this reporting exists to close. The app now identifies itself, so reports stop arriving as an unknown client. And building the payload moved inside the guarded block, because the fallback to String(error) can throw on a hostile value. * Drop the MIME type branch from chunk detection The branch matched the wording a browser prints when it refuses a script for its MIME type, and the comment presented that as real coverage. The wording is a console diagnostic; nothing establishes that it ever reaches application code as an error message, and the case it was meant to cover is already carried by the branch next to it: a stale chunk answered with the app shell makes the script load without registering the chunk, and the bundler's loader turns that into a ChunkLoadError reading `Loading chunk N failed. (missing: <url>)`. So the branch was an unverifiable claim guarding a case that is already handled. Removed, and the test that asserted the regex against its own assumed string is replaced by one built from the wording the bundler actually produces. * Report the route the customer was on, and cover the widget entry point The reported route came from window.location. That is right for the app, but the widget and library builds run on a memory router, where the browser URL belongs to the host page and never changes as the customer moves between screens. Every report from an embedded deployment would have carried the same wrong route - in the contexts that are hardest to debug from a support ticket. It now comes from the router. The test was hiding this: it forced window.location to match the route before rendering. It now sets it to something else on purpose and asserts the reported route is the router's, which is exactly the divergence the embedded builds live with. The widget build swaps index-widget.tsx in for index.tsx, so the window-level handling wired up in index.tsx never reached that bundle. Extracted into a shared function and called from both. The widget is embedded on third-party pages, which is where a chunk left stale by a deploy has the longest to sit in a cache. Also: the chunk-message test asserted on an error that carried the ChunkLoadError name as well, so it passed on the name check and never pinned the message pattern - it now asserts the message alone. And the comment claiming the field limits are kept in sync with the endpoint now says plainly that nothing enforces that across repositories. * Never reload a host page the app is only embedded in The widget and the library run inside someone else's page, and window belongs to that page - the closed shadow root isolates DOM and CSS, not window. Recovering a stale chunk by reloading would therefore discard state that has nothing to do with us: the host's forms, their cart, their scroll position, over a deploy of ours. The previous commit made this worse by wiring the window listeners into the widget entry point, where the reload had the most reach. Both embedded builds now mark themselves, and recovery there falls back to reporting without reloading. The customer still sees the error screen inside the widget, and we still learn about it - only the host page is left alone. The library entry point also never installed the handling at all: it is imported straight into a consumer's React app and has no entry point of its own, which the comment listing the entry points had missed. It installs it now, reporting only. Also asserts the reported route in the no-match case, which is where the router's own location is least obviously defined. * Give the embedded customer a way out instead of a dead end Suppressing the reload on a host page was right, but it left nothing in its place. The only thing the error screen offers is the support button, and the support screen is lazy-loaded like every other one - so it fails on the same chunk and puts the customer straight back where they were. Meanwhile the text told them to return to the previous page, which does not help either. Embedded, a chunk failure now says to reload the page, in all three languages, and the support button is left out of that case: it would only send them in circles. Every other failure keeps the button, because reloading would not fix those. The window listeners are also no longer installed when embedded. They are page-wide, and the page belongs to the host - the widget is a web component in their window, not an iframe - so on a host that ships its own bundler they would have caught that bundler's chunk failures and filed them as ours. Recovery is off there anyway, and the failures worth seeing come through the router regardless. And installChunkErrorHandling, the wiring all three entry points depend on, had no test of its own: which event property carries the error was covered nowhere. It is now, for both channels. * Drop the calls that mark themselves embedded and then do nothing Both embedded entry points called installChunkErrorHandling after marking themselves embedded, which makes the call return immediately - in Main.lib.tsx the two sit on consecutive lines, and index-widget.tsx imports Main.widget, whose module body runs first. So neither call could ever install anything, while the comments beside them described it as active handling. Removed, with the reason stated where the reader will be looking: the embedded builds do not install page-wide listeners because the page is the host's. The guard inside the function stays, so a future entry point that calls it without checking still gets the right behaviour.
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