Skip to content

Commit 64becf0

Browse files
elebouchereleboucher
authored
fix(profile): keep extended profile fields across cache refreshes (#1846)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes # #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings ### AI disclosure: - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. -->
2 parents bccbdc6 + 4a5505b commit 64becf0

5 files changed

Lines changed: 361 additions & 60 deletions

File tree

‎src/app/features/settings/account/AnimalCosmetics.tsx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import type { UserProfile } from '$hooks/useUserProfile';
55
import { useSetting } from '$state/hooks/settings';
66
import { settingsAtom } from '$state/settings';
77
import { profilesCacheAtom } from '$state/userRoomProfile';
8+
import { showToast } from '$state/toast';
9+
import { invalidateUserProfileCache } from '$hooks/useUserProfile';
810
import { Box, IconButton, Input, Text } from 'folds';
911
import { useSetAtom } from 'jotai';
1012
import { useCallback, useEffect, useState, type ChangeEvent } from 'react';
@@ -87,12 +89,15 @@ export function AnimalCosmetics({ profile, userId }: Readonly<AnimalCosmeticsPro
8789

8890
const handleSaveField = useCallback(
8991
async (key: string, value: unknown) => {
90-
await mx.setExtendedProfileProperty?.(key, value);
91-
setGlobalProfiles((prev) => {
92-
const newCache = { ...prev };
93-
delete newCache[userId];
94-
return newCache;
95-
});
92+
try {
93+
await mx.setExtendedProfileProperty?.(key, value);
94+
} catch (error) {
95+
showToast(
96+
`Failed to save profile field: ${error instanceof Error ? error.message : String(error)}`
97+
);
98+
return;
99+
}
100+
invalidateUserProfileCache(mx, userId, setGlobalProfiles);
96101
},
97102
[mx, userId, setGlobalProfiles]
98103
);

‎src/app/features/settings/account/Profile.test.tsx‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1-
import { render, within } from '@testing-library/react';
1+
import { act, render, within } from '@testing-library/react';
22
import { describe, expect, it, vi } from 'vitest';
33
import { ScreenSize, ScreenSizeProvider } from '$hooks/useScreenSize';
44
import { SettingsLinkProvider } from '$features/settings/SettingsLinkContext';
5+
import { showToast } from '$state/toast';
56
import { Profile } from './Profile';
67

8+
vi.mock('$state/toast', () => ({
9+
showToast: vi.fn<(text: string) => void>(),
10+
}));
11+
12+
const timezoneEditorMock = vi.hoisted(() => ({
13+
props: undefined as { current?: string; onSave: (tz: string) => void } | undefined,
14+
}));
15+
716
const mockMatrixClient = {
817
getUserId: () => '@alice:example.org',
918
setAvatarUrl: vi.fn<() => Promise<void>>(),
@@ -72,7 +81,10 @@ vi.mock('jotai', async () => {
7281
});
7382

7483
vi.mock('./TimezoneEditor', () => ({
75-
TimezoneEditor: () => <div>Timezone</div>,
84+
TimezoneEditor: (props: { current?: string; onSave: (tz: string) => void }) => {
85+
timezoneEditorMock.props = props;
86+
return <div>Timezone</div>;
87+
},
7688
}));
7789

7890
vi.mock('./PronounEditor', () => ({
@@ -119,4 +131,25 @@ describe('Profile', () => {
119131
within(customFieldTile as HTMLElement).queryByRole('button', { name: /copy settings link/i })
120132
).not.toBeInTheDocument();
121133
});
134+
135+
it('surfaces a toast when saving the timezone fails', async () => {
136+
mockMatrixClient.setExtendedProfileProperty.mockRejectedValue(
137+
new Error('Server does not support extended profiles')
138+
);
139+
140+
render(
141+
<ScreenSizeProvider value={ScreenSize.Desktop}>
142+
<SettingsLinkProvider value={{ section: 'account', baseUrl: 'https://app.example' }}>
143+
<Profile />
144+
</SettingsLinkProvider>
145+
</ScreenSizeProvider>
146+
);
147+
148+
expect(timezoneEditorMock.props).toBeDefined();
149+
await act(async () => {
150+
timezoneEditorMock.props?.onSave('Europe/Paris');
151+
});
152+
153+
expect(showToast).toHaveBeenCalledWith(expect.stringContaining('Failed to save profile field'));
154+
});
122155
});

‎src/app/features/settings/account/Profile.tsx‎

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { SettingMenuSelector } from '$components/setting-menu-selector';
99
import { SettingTile } from '$components/setting-tile';
1010
import { useMatrixClient } from '$hooks/useMatrixClient';
1111
import type { UserProfile, MSC4440Bio, ColorSet } from '$hooks/useUserProfile';
12-
import { useUserProfile } from '$hooks/useUserProfile';
12+
import { invalidateUserProfileCache, useUserProfile } from '$hooks/useUserProfile';
1313
import { getMxIdLocalPart, mxcUrlToHttp } from '$utils/matrix';
1414
import { UserAvatar } from '$components/user-avatar';
1515
import { useMediaAuthentication } from '$hooks/useMediaAuthentication';
@@ -38,6 +38,7 @@ import { NameColorEditor } from './NameColorEditor';
3838
import { StatusEditor } from './StatusEditor';
3939
import { AnimalCosmetics } from './AnimalCosmetics';
4040
import * as prefix from '$unstable/prefixes';
41+
import { showToast } from '$state/toast';
4142
import { confirm } from '$components/confirm/confirm';
4243
import { AvatarUploadTile } from '$components/avatar-upload-tile/AvatarUploadTile';
4344
import { accessibleColor } from '$plugins/color';
@@ -107,8 +108,9 @@ function ProfileAvatar({ profile, userId, propagateTo }: Readonly<ProfileProps>)
107108
);
108109
}
109110

110-
function ProfileBanner({ profile }: Readonly<Pick<ProfileProps, 'profile'>>) {
111+
function ProfileBanner({ profile, userId }: Readonly<Pick<ProfileProps, 'profile' | 'userId'>>) {
111112
const mx = useMatrixClient();
113+
const setGlobalProfiles = useSetAtom(profilesCacheAtom);
112114
const useAuthentication = useMediaAuthentication();
113115
const [stagedUrl, setStagedUrl] = useState<string>();
114116
const [isRemoving, setIsRemoving] = useState(false);
@@ -144,15 +146,27 @@ function ProfileBanner({ profile }: Readonly<Pick<ProfileProps, 'profile'>>) {
144146
}, []);
145147

146148
const handleUploaded = useCallback(
147-
(upload: UploadSuccess) => {
149+
async (upload: UploadSuccess) => {
148150
const { mxc } = upload;
149151

150152
if (imageFileURL) setStagedUrl(imageFileURL);
151-
152-
mx.setExtendedProfileProperty?.(prefix.MATRIX_UNSTABLE_PROFILE_BANNER_PROPERTY_NAME, mxc);
153153
setImageFile(undefined);
154+
155+
try {
156+
await mx.setExtendedProfileProperty?.(
157+
prefix.MATRIX_UNSTABLE_PROFILE_BANNER_PROPERTY_NAME,
158+
mxc
159+
);
160+
} catch (error) {
161+
showToast(
162+
`Failed to save profile field: ${error instanceof Error ? error.message : String(error)}`
163+
);
164+
setStagedUrl(undefined);
165+
return;
166+
}
167+
invalidateUserProfileCache(mx, userId, setGlobalProfiles);
154168
},
155-
[mx, imageFileURL]
169+
[mx, userId, imageFileURL, setGlobalProfiles]
156170
);
157171

158172
const handleRemoveBanner = async () => {
@@ -166,10 +180,19 @@ function ProfileBanner({ profile }: Readonly<Pick<ProfileProps, 'profile'>>) {
166180
setIsRemoving(true);
167181
setStagedUrl(undefined);
168182
setImageFile(undefined);
169-
await mx.setExtendedProfileProperty?.(
170-
prefix.MATRIX_UNSTABLE_PROFILE_BANNER_PROPERTY_NAME,
171-
null
172-
);
183+
try {
184+
await mx.setExtendedProfileProperty?.(
185+
prefix.MATRIX_UNSTABLE_PROFILE_BANNER_PROPERTY_NAME,
186+
null
187+
);
188+
} catch (error) {
189+
showToast(
190+
`Failed to save profile field: ${error instanceof Error ? error.message : String(error)}`
191+
);
192+
setIsRemoving(false);
193+
return;
194+
}
195+
invalidateUserProfileCache(mx, userId, setGlobalProfiles);
173196
}
174197
};
175198

@@ -406,12 +429,15 @@ function ProfileExtended({ profile, userId }: Readonly<ProfileProps>) {
406429

407430
const handleSaveField = useCallback(
408431
async (key: string, value: unknown) => {
409-
await mx.setExtendedProfileProperty?.(key, value);
410-
setGlobalProfiles((prev) => {
411-
const newCache = { ...prev };
412-
delete newCache[userId];
413-
return newCache;
414-
});
432+
try {
433+
await mx.setExtendedProfileProperty?.(key, value);
434+
} catch (error) {
435+
showToast(
436+
`Failed to save profile field: ${error instanceof Error ? error.message : String(error)}`
437+
);
438+
return;
439+
}
440+
invalidateUserProfileCache(mx, userId, setGlobalProfiles);
415441
},
416442
[mx, userId, setGlobalProfiles]
417443
);
@@ -715,7 +741,7 @@ export function Profile() {
715741
direction="Column"
716742
gap="400"
717743
>
718-
<ProfileBanner profile={profile} />
744+
<ProfileBanner profile={profile} userId={userId} />
719745
</SequenceCard>
720746
<SequenceCard
721747
className={SequenceCardStyle}

‎src/app/hooks/useUserProfile.test.tsx‎

Lines changed: 154 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import { Provider as JotaiProvider, createStore } from 'jotai';
55
import { afterEach, describe, expect, it, vi } from 'vitest';
66

77
import type { MatrixClient } from '$types/matrix-sdk';
8+
import { profilesCacheAtom } from '$state/userRoomProfile';
9+
import { PROFILE_CACHE_FRESH_MS } from '$client/userProfileCache';
810
import { MatrixClientProvider } from './useMatrixClient';
911
import { IsInactivePanelProvider } from './useRoom';
10-
import { useUserProfile } from './useUserProfile';
12+
import { invalidateUserProfileCache, useUserProfile } from './useUserProfile';
1113

1214
vi.mock('$state/hooks/settings', () => ({
1315
useSetting: (_atom: unknown, key: string) => {
@@ -117,4 +119,155 @@ describe('useUserProfile', () => {
117119

118120
expect(requested[4]).toBe(userIds[5]);
119121
});
122+
123+
it('keeps cached profile fields when a refetch response omits them', async () => {
124+
vi.useFakeTimers();
125+
const mx = {
126+
getProfileInfo: vi
127+
.fn<() => Promise<{ displayname: string }>>()
128+
.mockResolvedValue({ displayname: 'Alice' }),
129+
getUser: vi.fn<() => void>(),
130+
getUserId: vi.fn<() => string>().mockReturnValue('@me:example.org'),
131+
} as unknown as MatrixClient;
132+
133+
const store = createStore();
134+
store.set(profilesCacheAtom, {
135+
'@alice:example.org': {
136+
displayName: 'Alice',
137+
timezone: 'Europe/Paris',
138+
_fetched: true,
139+
_fetchedAt: Date.now() - PROFILE_CACHE_FRESH_MS - 1000,
140+
},
141+
});
142+
143+
const wrapper = ({ children }: { children: ReactNode }) =>
144+
createElement(
145+
JotaiProvider,
146+
{ store },
147+
createElement(
148+
MatrixClientProvider,
149+
{ value: mx },
150+
createElement(IsInactivePanelProvider, { value: false }, children)
151+
)
152+
);
153+
154+
const { result } = renderHook(() => useUserProfile('@alice:example.org'), { wrapper });
155+
156+
await act(async () => {
157+
await vi.advanceTimersByTimeAsync(150);
158+
});
159+
await act(async () => {
160+
await vi.advanceTimersByTimeAsync(0);
161+
});
162+
163+
expect(mx.getProfileInfo).toHaveBeenCalledOnce();
164+
expect(result.current.timezone).toBe('Europe/Paris');
165+
});
166+
167+
it('updates cached timezone when a refetch response contains a new value', async () => {
168+
vi.useFakeTimers();
169+
const mx = {
170+
getProfileInfo: vi
171+
.fn<() => Promise<{ displayname: string; 'm.tz': string }>>()
172+
.mockResolvedValue({ displayname: 'Alice', 'm.tz': 'America/New_York' }),
173+
getUser: vi.fn<() => void>(),
174+
getUserId: vi.fn<() => string>().mockReturnValue('@me:example.org'),
175+
} as unknown as MatrixClient;
176+
177+
const store = createStore();
178+
store.set(profilesCacheAtom, {
179+
'@alice:example.org': {
180+
displayName: 'Alice',
181+
timezone: 'Europe/Paris',
182+
_fetched: true,
183+
_fetchedAt: Date.now() - PROFILE_CACHE_FRESH_MS - 1000,
184+
},
185+
});
186+
187+
const wrapper = ({ children }: { children: ReactNode }) =>
188+
createElement(
189+
JotaiProvider,
190+
{ store },
191+
createElement(
192+
MatrixClientProvider,
193+
{ value: mx },
194+
createElement(IsInactivePanelProvider, { value: false }, children)
195+
)
196+
);
197+
198+
const { result } = renderHook(() => useUserProfile('@alice:example.org'), { wrapper });
199+
200+
await act(async () => {
201+
await vi.advanceTimersByTimeAsync(150);
202+
});
203+
await act(async () => {
204+
await vi.advanceTimersByTimeAsync(0);
205+
});
206+
207+
expect(result.current.timezone).toBe('America/New_York');
208+
});
209+
210+
it('discards an in-flight response captured before a profile write', async () => {
211+
vi.useFakeTimers();
212+
213+
let resolveStale: ((info: { displayname: string; 'm.tz': string }) => void) | undefined;
214+
const mx = {
215+
getProfileInfo: vi
216+
.fn<() => Promise<{ displayname: string; 'm.tz': string }>>()
217+
.mockImplementationOnce(
218+
() =>
219+
new Promise((resolve) => {
220+
resolveStale = resolve;
221+
})
222+
)
223+
.mockResolvedValue({ displayname: 'Alice', 'm.tz': 'America/New_York' }),
224+
getUser: vi.fn<() => void>(),
225+
getUserId: vi.fn<() => string>().mockReturnValue('@me:example.org'),
226+
} as unknown as MatrixClient;
227+
228+
const store = createStore();
229+
store.set(profilesCacheAtom, {
230+
'@alice:example.org': {
231+
displayName: 'Alice',
232+
timezone: 'Europe/Paris',
233+
_fetched: true,
234+
_fetchedAt: Date.now() - PROFILE_CACHE_FRESH_MS - 1000,
235+
},
236+
});
237+
238+
const wrapper = ({ children }: { children: ReactNode }) =>
239+
createElement(
240+
JotaiProvider,
241+
{ store },
242+
createElement(
243+
MatrixClientProvider,
244+
{ value: mx },
245+
createElement(IsInactivePanelProvider, { value: false }, children)
246+
)
247+
);
248+
249+
const { result } = renderHook(() => useUserProfile('@alice:example.org'), { wrapper });
250+
251+
await act(async () => {
252+
await vi.advanceTimersByTimeAsync(150);
253+
});
254+
expect(mx.getProfileInfo).toHaveBeenCalledOnce();
255+
256+
await act(async () => {
257+
invalidateUserProfileCache(mx, '@alice:example.org', (update) =>
258+
store.set(profilesCacheAtom, update)
259+
);
260+
});
261+
262+
await act(async () => {
263+
resolveStale?.({ displayname: 'Alice', 'm.tz': 'Europe/Paris' });
264+
await vi.advanceTimersByTimeAsync(0);
265+
});
266+
await act(async () => {
267+
await vi.advanceTimersByTimeAsync(0);
268+
});
269+
270+
expect(mx.getProfileInfo).toHaveBeenCalledTimes(2);
271+
expect(result.current.timezone).toBe('America/New_York');
272+
});
120273
});

0 commit comments

Comments
 (0)