Skip to content

Commit db4ad6b

Browse files
yinlianghuiclaude
andauthored
fix(plugin-gantt): tooltip currency re-formats when the tenant currency resolves (#4542) (#4554)
The `tasks` memo builds every tooltip string eagerly inside its callback, and the `'currency'` case resolves its code down to the tenant default via `resolveFieldCurrency(def, tenantCurrency)`. `tenantCurrency` was read but not watched: it was missing from the memo's dependency array, which ESLint's `react-hooks/exhaustive-deps` was already reporting on `origin/main`. The tenant default arrives from `GET /api/v1/auth/me/localization`, which is cosmetic and non-blocking and therefore answers AFTER first paint. The context change re-rendered ObjectGantt, but with no dependency changed the memo handed back its cached task array, so the tooltip kept its pre-resolution rendering. Not covered by the `displayLocale` dependency #4272 (PR #4544) added to this same array: the producer writes currency and locale from one response, so a tenant configuring BOTH re-runs the memo through the locale channel — but a tenant configuring a currency and no locale (the common shape) leaves `displayLocale` untouched and the currency stale. The new test resolves currency alone for exactly that reason. Module-local: the package's nine `.d.ts` files are byte-identical across the change, so the changeset is a patch. Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-authored-by: Claude <noreply@anthropic.com>
1 parent dd3adbd commit db4ad6b

3 files changed

Lines changed: 337 additions & 1 deletion

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@object-ui/plugin-gantt': patch
3+
---
4+
5+
Gantt tooltip currency re-formats when the tenant currency resolves
6+
(objectui#4542).
7+
8+
ObjectGantt's `tasks` memo builds every tooltip string eagerly inside its
9+
callback, and the `'currency'` case resolves its code down to the tenant
10+
default (`resolveFieldCurrency(def, tenantCurrency)`). `tenantCurrency` was
11+
not in the memo's dependency array, so the value was read but never watched.
12+
13+
That default comes from `GET /api/v1/auth/me/localization`, which is cosmetic
14+
and non-blocking and therefore answers AFTER first paint. The context change
15+
re-rendered ObjectGantt, but with none of `data` / `ganttConfig` /
16+
`objectSchema` / `displayLocale` changed the memo handed back its cached task
17+
array — so a tooltip amount kept the pre-resolution rendering (a plain
18+
`1,234.50` instead of `€1,234.50`) until something unrelated invalidated the
19+
memo.
20+
21+
This is the currency twin of objectui#4272, which added `displayLocale` to
22+
this same array for the same reason, and it is not covered by that dep: the
23+
producer writes currency and locale from one response, so a tenant that
24+
configures BOTH re-runs the memo through the locale channel — but a tenant
25+
that configures a currency and no locale (the common shape, since the tenant
26+
locale is frequently unset) leaves `displayLocale` untouched and the currency
27+
stale.
28+
29+
Module-local: the fix is one dependency, the package's `.d.ts` files are
30+
byte-identical, and rendering is unchanged whenever the channel resolves
31+
before first paint or a field carries its own currency code.
Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#4542 — the `tasks` memo formatted tooltip currency with
11+
* `tenantCurrency` but did not depend on it, so a tooltip could keep the
12+
* pre-resolution string forever.
13+
*
14+
* The tooltip strings are built EAGERLY inside the memo:
15+
*
16+
* case 'currency':
17+
* return formatCurrency(Number(value), resolveFieldCurrency(def, tenantCurrency));
18+
*
19+
* `tenantCurrency` comes from `useLocalization()`, which is fed by
20+
* `GET /api/v1/auth/me/localization` — cosmetic and non-blocking, so it
21+
* resolves AFTER first paint (`LocalizationFetchProvider` does one async
22+
* `setValue` post-mount). The context change re-renders ObjectGantt, but with
23+
* `tenantCurrency` absent from the dependency array none of `data` /
24+
* `ganttConfig` / `objectSchema` / `displayLocale` has changed, so the memo
25+
* hands back its cached task array and the tooltip keeps the fallback
26+
* rendering. This is the currency twin of objectui#4272 (PR #4544), which
27+
* added `displayLocale` to this very array for the same reason.
28+
*
29+
* ── Two masking paths this file deliberately steers around ───────────────
30+
* Both were measured before the fix; getting either wrong makes the red case
31+
* pass for the wrong reason.
32+
*
33+
* 1. **The locale channel.** The producer writes currency and locale from one
34+
* response, so a tenant configuring BOTH re-runs the memo through #4544's
35+
* `displayLocale` dep and the currency staleness never shows. The bug is
36+
* observable on a tenant that configures **currency only** — the common
37+
* shape, since `useDisplayLocale` documents the tenant locale as
38+
* "frequently `undefined`", in which case it falls back to the UI language
39+
* and does not change. So the deferred value here is `{ currency }` alone.
40+
*
41+
* 2. **`ganttConfig` identity.** `getGanttConfig` returns a FRESH object
42+
* literal on the flattened top-level (console ListView) path, which
43+
* invalidates this memo on every render all by itself; on the
44+
* `schema.gantt` path it returns `schema.gantt` by reference. Only the
45+
* latter is memoized in practice, so these cases use `schema.gantt` with a
46+
* module-constant schema — the identity a metadata-sourced schema has.
47+
* The `dataSource` is module-constant for the same reason.
48+
*
49+
* ── Directions (predicted in writing before the run, then measured) ──────
50+
* Runner: machine locale en-US. Only "re-formats when the tenant currency
51+
* resolves after first paint" is red against unfixed code. Every other case is
52+
* a must-not-change pin, GREEN ON BOTH SIDES: the pre-resolution rendering,
53+
* the resolved-before-first-paint rendering, the field-level currency
54+
* precedence, and #4544's own locale dep.
55+
*/
56+
57+
import React from 'react';
58+
import { describe, it, expect, vi, afterEach } from 'vitest';
59+
import { render, screen, waitFor, cleanup, act } from '@testing-library/react';
60+
import { I18nProvider, LocalizationProvider, type LocalizationValue } from '@object-ui/i18n';
61+
import { ObjectGantt } from './ObjectGantt';
62+
import type { DataSource } from '@object-ui/types';
63+
64+
// Same GanttView stub idiom as ObjectGantt.test.tsx / ObjectGantt.dateLocale
65+
// .test.tsx: tooltip rows are surfaced as `gv-field-<id>-<i>` handles so their
66+
// formatted text is assertable without rendering the real timeline.
67+
vi.mock('./GanttView', () => ({
68+
GanttView: ({ tasks }: any) => (
69+
<div data-testid="gantt-view">
70+
{tasks.map((t: any) => (
71+
<div key={t.id} data-testid="gantt-task">
72+
<span>{t.title}</span>
73+
{t.fields ? (
74+
<div data-testid={`gv-fields-${t.id}`}>
75+
{t.fields.map((f: any, i: number) => (
76+
<span key={i} data-testid={`gv-field-${t.id}-${i}`}>{f.label}={f.value}</span>
77+
))}
78+
</div>
79+
) : null}
80+
</div>
81+
))}
82+
</div>
83+
),
84+
}));
85+
86+
/**
87+
* `amount` carries no currency code of its own, so it falls through to the
88+
* tenant default — the channel under test. `amount_fixed` pins its own code.
89+
* A fractional amount for the tenant-defaulted one: the symbol AND the minor
90+
* unit both come from the resolved code, so the two renderings differ in more
91+
* than one place.
92+
*/
93+
const ROWS = [
94+
{
95+
id: '1',
96+
name: 'Task 1',
97+
start_date: '2024-01-01',
98+
end_date: '2024-01-10',
99+
amount: 1234.5,
100+
amount_fixed: 1234,
101+
due_date: '2024-01-05',
102+
},
103+
];
104+
105+
const OBJECT_SCHEMA = {
106+
fields: {
107+
name: { type: 'text' },
108+
start_date: { type: 'date' },
109+
end_date: { type: 'date' },
110+
amount: { type: 'currency', label: 'Amount' },
111+
amount_fixed: { type: 'currency', label: 'Fixed', currency: 'JPY' },
112+
due_date: { type: 'date', label: 'Due' },
113+
},
114+
};
115+
116+
// Module-constant so the identity is stable across the re-render the late
117+
// resolution causes — see masking path 2 in the header.
118+
const DATA_SOURCE: DataSource = {
119+
find: vi.fn().mockResolvedValue({ data: ROWS }),
120+
findOne: vi.fn(),
121+
create: vi.fn(),
122+
update: vi.fn(),
123+
delete: vi.fn(),
124+
getObjectSchema: vi.fn().mockResolvedValue(OBJECT_SCHEMA),
125+
} as any;
126+
127+
const SCHEMA: any = {
128+
type: 'gantt',
129+
gantt: {
130+
titleField: 'name',
131+
startDateField: 'start_date',
132+
endDateField: 'end_date',
133+
tooltipFields: ['amount', 'amount_fixed', 'due_date'],
134+
},
135+
data: { provider: 'object', object: 'tasks' },
136+
};
137+
138+
/** Tooltip row handles, in `tooltipFields` order. */
139+
const AMOUNT = 'gv-field-1-0';
140+
const FIXED = 'gv-field-1-1';
141+
const DUE = 'gv-field-1-2';
142+
143+
/**
144+
* The tenant localization channel as the app actually drives it: an initial
145+
* value (usually empty — the endpoint has not answered yet) and one async
146+
* write once a promise the test controls resolves. This is
147+
* `LocalizationFetchProvider` reduced to its timing: cosmetic, non-blocking,
148+
* one `setValue` after mount.
149+
*/
150+
function DeferredLocalization({
151+
initial,
152+
pending,
153+
children,
154+
}: {
155+
initial: LocalizationValue;
156+
pending: Promise<LocalizationValue>;
157+
children: React.ReactNode;
158+
}) {
159+
const [value, setValue] = React.useState<LocalizationValue>(initial);
160+
React.useEffect(() => {
161+
let cancelled = false;
162+
void pending.then((next) => {
163+
if (!cancelled) setValue(next);
164+
});
165+
return () => {
166+
cancelled = true;
167+
};
168+
}, [pending]);
169+
return <LocalizationProvider value={value}>{children}</LocalizationProvider>;
170+
}
171+
172+
function renderSession(opts: {
173+
initial?: LocalizationValue;
174+
pending?: Promise<LocalizationValue>;
175+
language?: string;
176+
} = {}) {
177+
const pending = opts.pending ?? new Promise<LocalizationValue>(() => {});
178+
return render(
179+
<I18nProvider
180+
config={{ defaultLanguage: opts.language ?? 'en', detectBrowserLanguage: false }}
181+
persistLanguage={false}
182+
>
183+
<DeferredLocalization initial={opts.initial ?? {}} pending={pending}>
184+
<ObjectGantt schema={SCHEMA} dataSource={DATA_SOURCE} />
185+
</DeferredLocalization>
186+
</I18nProvider>,
187+
);
188+
}
189+
190+
/** Let the resolved promise's `.then` and the state write flush. */
191+
async function flush() {
192+
await act(async () => {
193+
await Promise.resolve();
194+
await Promise.resolve();
195+
});
196+
}
197+
198+
afterEach(() => cleanup());
199+
200+
describe('ObjectGantt tooltips — the tasks memo depends on the tenant currency (objectui#4542)', () => {
201+
/**
202+
* THE RED CASE. Pre-fix the memo returns its cached array when the context
203+
* changes, so this row keeps `Amount=1,234.50` forever.
204+
*/
205+
it('re-formats when the tenant currency resolves after first paint', async () => {
206+
let resolveLocalization!: (v: LocalizationValue) => void;
207+
const pending = new Promise<LocalizationValue>((res) => {
208+
resolveLocalization = res;
209+
});
210+
renderSession({ pending });
211+
212+
await waitFor(() => expect(screen.getByTestId('gv-fields-1')).toBeDefined());
213+
// First paint: the endpoint has not answered, so there is no code to
214+
// render and the amount is a plain number.
215+
expect(screen.getByTestId(AMOUNT).textContent).toBe('Amount=1,234.50');
216+
217+
// The endpoint answers with a currency and NO locale — the tenant shape
218+
// that isolates this channel from #4544's `displayLocale` dep.
219+
await act(async () => {
220+
resolveLocalization({ currency: 'EUR' });
221+
});
222+
await flush();
223+
224+
expect(screen.getByTestId(AMOUNT).textContent).toBe('Amount=€1,234.50');
225+
});
226+
227+
/**
228+
* PIN, green both sides: the pre-resolution rendering itself is unchanged —
229+
* no tenant default known means a plain number, not a guessed symbol.
230+
*/
231+
it('renders a plain number while the endpoint has not answered (must-not-change)', async () => {
232+
renderSession();
233+
await waitFor(() => expect(screen.getByTestId('gv-fields-1')).toBeDefined());
234+
expect(screen.getByTestId(AMOUNT).textContent).toBe('Amount=1,234.50');
235+
});
236+
237+
/**
238+
* PIN, green both sides: when the channel has already answered before first
239+
* paint the memo never needed to re-run, so this rendering is byte-identical
240+
* across the fix. This is the case the PM ruling asks be pinned explicitly.
241+
*/
242+
it('currency resolved BEFORE first paint renders identically (must-not-change)', async () => {
243+
renderSession({ initial: { currency: 'EUR' } });
244+
await waitFor(() => expect(screen.getByTestId('gv-fields-1')).toBeDefined());
245+
expect(screen.getByTestId(AMOUNT).textContent).toBe('Amount=€1,234.50');
246+
expect(screen.getByTestId(FIXED).textContent).toBe('Fixed=¥1,234');
247+
});
248+
249+
/**
250+
* PIN, green both sides: `resolveFieldCurrency` precedence is untouched — a
251+
* field's own code outranks the tenant default before AND after the late
252+
* resolution, so the added dependency re-formats without re-deciding.
253+
*/
254+
it("a field's explicit currency outranks the tenant default, before and after (must-not-change)", async () => {
255+
let resolveLocalization!: (v: LocalizationValue) => void;
256+
const pending = new Promise<LocalizationValue>((res) => {
257+
resolveLocalization = res;
258+
});
259+
renderSession({ pending });
260+
261+
await waitFor(() => expect(screen.getByTestId('gv-fields-1')).toBeDefined());
262+
expect(screen.getByTestId(FIXED).textContent).toBe('Fixed=¥1,234');
263+
264+
await act(async () => {
265+
resolveLocalization({ currency: 'EUR' });
266+
});
267+
await flush();
268+
269+
expect(screen.getByTestId(FIXED).textContent).toBe('Fixed=¥1,234');
270+
});
271+
272+
/**
273+
* PIN, green both sides: objectui#4272 / PR #4544's `displayLocale` dep on
274+
* this same array still invalidates the memo when the TENANT LOCALE lands
275+
* late. Guards the locale half of the array against a regression from this
276+
* card's edit — and documents masking path 1: it is precisely because this
277+
* works that the red case above must resolve currency alone.
278+
*/
279+
it("#4544's displayLocale dep still re-formats on a late tenant locale (must-not-change)", async () => {
280+
let resolveLocalization!: (v: LocalizationValue) => void;
281+
const pending = new Promise<LocalizationValue>((res) => {
282+
resolveLocalization = res;
283+
});
284+
renderSession({ pending });
285+
286+
await waitFor(() => expect(screen.getByTestId('gv-fields-1')).toBeDefined());
287+
expect(screen.getByTestId(DUE).textContent).toBe('Due=Jan 5, 2024');
288+
289+
await act(async () => {
290+
resolveLocalization({ locale: 'de' });
291+
});
292+
await flush();
293+
294+
expect(screen.getByTestId(DUE).textContent).toBe('Due=5. Jan. 2024');
295+
});
296+
});

‎packages/plugin-gantt/src/ObjectGantt.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,16 @@ export const ObjectGantt: React.FC<ObjectGanttProps> = ({
771771
// `displayLocale` is a dependency because the tooltip strings are FORMATTED
772772
// in here: without it a language switch would leave already-built tooltips
773773
// on the previous locale.
774-
}, [data, ganttConfig, objectSchema, displayLocale]);
774+
//
775+
// `tenantCurrency` is one for exactly the same reason, in the other channel
776+
// (objectui#4542): the `'currency'` case resolves its code down to the
777+
// tenant default eagerly in here. That default arrives from
778+
// `GET /api/v1/auth/me/localization`, which is cosmetic and non-blocking and
779+
// therefore answers AFTER first paint — so the memo has to be able to re-run
780+
// on it alone. It is not covered by `displayLocale`: a tenant that
781+
// configures a currency but no locale (the common shape) leaves that value
782+
// untouched, and the tooltip would keep its pre-resolution rendering.
783+
}, [data, ganttConfig, objectSchema, displayLocale, tenantCurrency]);
775784

776785
// Dynamic Group by accessor (动态 Group by). Resolves each task's grouping
777786
// value off its backing record, mapping select options / lookups to their

0 commit comments

Comments
 (0)