Skip to content

Commit 1d8ecb8

Browse files
authored
feat(popup): Show resolved state and user avatars in thread (#745)
* feat(popup): Show resolved state and user avatars in thread * fix(popup): Use Authorization header for avatar requests Fetch user avatars via an Authorization header and render them from blob URLs, so the access token is no longer appended to the avatar URL as a query parameter. Blob URLs are cached and revoked on unmount or when credentials change. In-flight requests compare captured credentials against the current ref so a token swap cannot land a stale blob in the cache.
1 parent 4e2c801 commit 1d8ecb8

8 files changed

Lines changed: 199 additions & 16 deletions

File tree

src/@types/model.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ export interface Annotation {
1212
modified_by: User;
1313
permissions: Permissions;
1414
replies?: Array<Reply>;
15+
resolution?: {
16+
resolved_at: string;
17+
resolved_by: User;
18+
} | null;
19+
status?: 'open' | 'resolved';
1520
target: Target;
1621
type: 'annotation';
1722
}

src/common/BaseAnnotator.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,13 @@ export default class BaseAnnotator extends EventEmitter {
9494
},
9595
common: { color: initialColor, mode: initialMode },
9696
options: {
97+
apiHost,
9798
features,
9899
fileId: file.id,
99100
fileVersionId: fileOptionsVersionId ?? fileVersionId,
100101
isCurrentFileVersion: !fileOptionsVersionId || fileOptionsVersionId === currentFileVersionId,
101102
permissions: file.permissions,
103+
token,
102104
viewMode: initialViewMode,
103105
},
104106
};

src/components/Popups/PopupV2.tsx

Lines changed: 124 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
serializeMentionMarkup,
1111
} from '@box/threaded-annotations';
1212
import type { DocumentNodeV2, TextMessageTypeV2 } from '@box/threaded-annotations';
13+
import type { FetchedAvatarUrls, UserContactType } from '@box/user-selector';
1314
import type { JSONContent } from '@tiptap/core';
1415
import FocusTrap from 'box-ui-elements/es/components/focus-trap/FocusTrap';
1516

@@ -21,6 +22,7 @@ import {
2122
updateAnnotationAction,
2223
} from '../../store/annotations/actions';
2324
import { getAnnotation } from '../../store/annotations/selectors';
25+
import { getApiHost, getToken } from '../../store/options';
2426
import { fetchCollaboratorsAction } from '../../store/users/actions';
2527

2628
import type { AppState, AppThunkDispatch } from '../../store/types';
@@ -57,17 +59,65 @@ const createDocumentNode = (content: JSONContent | null): DocumentNodeV2 => {
5759
return { type: 'doc', content: [content] } as DocumentNodeV2;
5860
};
5961

62+
// Callers render initials as a fallback on null.
63+
// A persistent null across all users usually indicates a stale token.
64+
const fetchAvatarBlob = async (apiHost: string, token: string, userId: string): Promise<string | null> => {
65+
try {
66+
const response = await fetch(`${apiHost}/2.0/users/${userId}/avatar?pic_type=large`, {
67+
headers: { Authorization: `Bearer ${token}` },
68+
});
69+
if (!response.ok) return null;
70+
const blob = await response.blob();
71+
return URL.createObjectURL(blob);
72+
} catch {
73+
return null;
74+
}
75+
};
76+
6077
const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => {
6178
const intl = useIntl();
6279
const dispatch = useDispatch<AppThunkDispatch>();
6380
const popupRef = React.useRef<HTMLDivElement>(null);
6481
const popperRef = React.useRef<Instance>();
6582
const optionsRef = React.useRef<Partial<Options>>(getPopupOptions());
6683

84+
const apiHost = useSelector(getApiHost);
85+
const token = useSelector(getToken);
6786
const annotation = useSelector((state: AppState) =>
6887
annotationId ? getAnnotation(state, annotationId) : undefined,
6988
);
7089

90+
const [avatarBlobs, setAvatarBlobs] = React.useState<Record<string, string>>({});
91+
const avatarCacheRef = React.useRef<Map<string, string>>(new Map());
92+
const credentialsRef = React.useRef({ apiHost, token });
93+
credentialsRef.current = { apiHost, token };
94+
95+
const getOrFetchAvatarBlob = React.useCallback(
96+
async (userId: string): Promise<string | null> => {
97+
const cached = avatarCacheRef.current.get(userId);
98+
if (cached) return cached;
99+
const capturedApiHost = apiHost;
100+
const capturedToken = token;
101+
const url = await fetchAvatarBlob(capturedApiHost, capturedToken, userId);
102+
if (!url) return null;
103+
if (
104+
credentialsRef.current.apiHost !== capturedApiHost ||
105+
credentialsRef.current.token !== capturedToken
106+
) {
107+
URL.revokeObjectURL(url);
108+
return null;
109+
}
110+
const existing = avatarCacheRef.current.get(userId);
111+
if (existing) {
112+
URL.revokeObjectURL(url);
113+
return existing;
114+
}
115+
avatarCacheRef.current.set(userId, url);
116+
return url;
117+
},
118+
[apiHost, token],
119+
);
120+
71121
React.useEffect(() => {
72122
if (popupRef.current) {
73123
popperRef.current = createPopper(reference, popupRef.current, optionsRef.current);
@@ -78,20 +128,85 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => {
78128
};
79129
}, [reference]);
80130

131+
React.useEffect(() => {
132+
const cache = avatarCacheRef.current;
133+
return () => {
134+
cache.forEach(url => URL.revokeObjectURL(url));
135+
cache.clear();
136+
};
137+
}, []);
138+
139+
React.useEffect(() => {
140+
avatarCacheRef.current.forEach(url => URL.revokeObjectURL(url));
141+
avatarCacheRef.current.clear();
142+
setAvatarBlobs({});
143+
}, [apiHost, token]);
144+
81145
const handleEvent = React.useCallback((event: React.SyntheticEvent) => {
82146
event.stopPropagation();
83147
}, []);
84148

85-
const threadMessages: TextMessageTypeV2[] = React.useMemo(
86-
() => (annotation ? annotationToMessages(annotation) : []),
87-
[annotation],
88-
);
149+
React.useEffect(() => {
150+
if (!annotation) return undefined;
151+
152+
const userIds = Array.from(
153+
new Set(annotationToMessages(annotation).map(msg => String(msg.author.id))),
154+
);
155+
let cancelled = false;
156+
157+
Promise.all(userIds.map(async id => [id, await getOrFetchAvatarBlob(id)] as const)).then(entries => {
158+
if (cancelled) return;
159+
setAvatarBlobs(prev => {
160+
const next = { ...prev };
161+
entries.forEach(([id, url]) => {
162+
if (url) next[id] = url;
163+
});
164+
return next;
165+
});
166+
});
167+
168+
return () => {
169+
cancelled = true;
170+
};
171+
}, [annotation, getOrFetchAvatarBlob]);
172+
173+
const threadMessages: TextMessageTypeV2[] = React.useMemo(() => {
174+
if (!annotation) return [];
175+
return annotationToMessages(annotation).map(msg => ({
176+
...msg,
177+
author: {
178+
...msg.author,
179+
avatarUrl: avatarBlobs[String(msg.author.id)],
180+
},
181+
}));
182+
}, [annotation, avatarBlobs]);
183+
184+
const isResolved = annotation?.status === 'resolved';
185+
const resolvedBy = isResolved
186+
? annotation?.resolution?.resolved_by?.name ?? annotation?.modified_by?.name
187+
: undefined;
188+
const resolvedAtSource = isResolved
189+
? annotation?.resolution?.resolved_at ?? annotation?.modified_at
190+
: undefined;
191+
const resolvedAt = resolvedAtSource ? new Date(resolvedAtSource).getTime() : undefined;
89192

90193
const userSelectorProps = React.useMemo(
91194
() => ({
92195
allowEmptyQuery: true,
93196
ariaRoleDescription: intl.formatMessage(messages.ariaLabelMentionSelector),
94-
fetchAvatarUrls: async () => ({}),
197+
fetchAvatarUrls: async (userContacts: UserContactType[]): Promise<FetchedAvatarUrls> => {
198+
const urls: FetchedAvatarUrls = {};
199+
await Promise.all(
200+
userContacts.map(async ({ id }) => {
201+
const key = String(id);
202+
const blobUrl = await getOrFetchAvatarBlob(key);
203+
if (blobUrl) {
204+
urls[key] = blobUrl;
205+
}
206+
}),
207+
);
208+
return urls;
209+
},
95210
fetchUsers: async (query: string) => {
96211
const action = await dispatch(fetchCollaboratorsAction(query));
97212
if (fetchCollaboratorsAction.fulfilled.match(action)) {
@@ -101,7 +216,7 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => {
101216
},
102217
loadingAriaLabel: intl.formatMessage(messages.ariaLabelMentionLoading),
103218
}),
104-
[dispatch, intl],
219+
[dispatch, getOrFetchAvatarBlob, intl],
105220
);
106221

107222
const handlePost = React.useCallback(
@@ -169,13 +284,16 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => {
169284
{annotationId ? (
170285
<ThreadedAnnotationsV2
171286
isAnnotations
287+
isResolved={isResolved}
172288
messages={threadMessages}
173289
onAvatarClick={noop}
174290
onDelete={noop}
175291
onPost={handlePost}
176292
onResolve={handleResolve}
177293
onThreadDelete={handleThreadDelete}
178294
onUnresolve={handleUnresolve}
295+
resolvedAt={resolvedAt}
296+
resolvedBy={resolvedBy}
179297
userSelectorProps={userSelectorProps}
180298
/>
181299
) : (

src/components/Popups/__tests__/PopupV2-test.tsx

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import React from 'react';
2-
import { render, screen } from '@testing-library/react';
2+
import { act, render, screen } from '@testing-library/react';
33
import { useDispatch, useSelector } from 'react-redux';
44
import PopupV2, { Props } from '../PopupV2';
5+
import { getApiHost, getToken } from '../../../store/options';
56

67
jest.mock('react-redux', () => ({
78
useDispatch: jest.fn(),
@@ -67,8 +68,34 @@ jest.mock('../../../store/users/actions', () => ({
6768
const mockUseDispatch = useDispatch as jest.MockedFunction<typeof useDispatch>;
6869
const mockUseSelector = useSelector as jest.MockedFunction<typeof useSelector>;
6970

71+
const mockSelectorValues = (annotation?: unknown): void => {
72+
mockUseSelector.mockImplementation(selector => {
73+
if (selector === getApiHost) return 'https://api.box.com';
74+
if (selector === getToken) return 'test-token';
75+
return annotation;
76+
});
77+
};
78+
7079
describe('PopupV2', () => {
7180
const mockDispatch = jest.fn();
81+
const mockFetch = jest.fn();
82+
const originalFetch = window.fetch;
83+
const originalCreateObjectURL = window.URL.createObjectURL;
84+
const originalRevokeObjectURL = window.URL.revokeObjectURL;
85+
86+
beforeAll(() => {
87+
window.fetch = mockFetch as unknown as typeof fetch;
88+
window.URL.createObjectURL = jest.fn().mockReturnValue('blob:mock-url');
89+
window.URL.revokeObjectURL = jest.fn();
90+
});
91+
92+
afterAll(() => {
93+
window.fetch = originalFetch;
94+
window.URL.createObjectURL = originalCreateObjectURL;
95+
window.URL.revokeObjectURL = originalRevokeObjectURL;
96+
});
97+
98+
const flushPromises = (): Promise<void> => act(() => new Promise<void>(resolve => { setTimeout(resolve, 0); }));
7299

73100
const mockAnnotation = {
74101
created_at: '2026-01-01T00:00:00Z',
@@ -92,6 +119,10 @@ describe('PopupV2', () => {
92119

93120
beforeEach(() => {
94121
mockUseDispatch.mockReturnValue(mockDispatch);
122+
mockFetch.mockResolvedValue({
123+
blob: () => Promise.resolve(new Blob(['avatar'])),
124+
ok: true,
125+
});
95126
});
96127

97128
afterEach(() => {
@@ -105,7 +136,7 @@ describe('PopupV2', () => {
105136
};
106137

107138
beforeEach(() => {
108-
mockUseSelector.mockReturnValue(undefined);
139+
mockSelectorValues(undefined);
109140
});
110141

111142
test('should render MessageEditorV2 with FocusTrap and MentionContextProvider', () => {
@@ -139,35 +170,39 @@ describe('PopupV2', () => {
139170
};
140171

141172
beforeEach(() => {
142-
mockUseSelector.mockReturnValue(mockAnnotation);
173+
mockSelectorValues(mockAnnotation);
143174
});
144175

145-
test('should render ThreadedAnnotationsV2 with FocusTrap and MentionContextProvider', () => {
176+
test('should render ThreadedAnnotationsV2 with FocusTrap and MentionContextProvider', async () => {
146177
render(<PopupV2 {...defaults} />);
178+
await flushPromises();
147179

148180
expect(screen.getByTestId('focus-trap')).toBeVisible();
149181
expect(screen.getByTestId('mention-context')).toBeVisible();
150182
expect(screen.getByTestId('threaded-annotations-v2')).toBeVisible();
151183
expect(screen.queryByTestId('message-editor-v2')).toBeNull();
152184
});
153185

154-
test('should render with isAnnotations=true and messages from annotation', () => {
186+
test('should render with isAnnotations=true and messages from annotation', async () => {
155187
render(<PopupV2 {...defaults} />);
188+
await flushPromises();
156189

157190
const thread = screen.getByTestId('threaded-annotations-v2');
158191
expect(thread.getAttribute('data-is-annotations')).toBe('true');
159192
expect(thread.getAttribute('data-messages-count')).toBe('1');
160193
});
161194

162-
test('should render empty messages when annotation is not found', () => {
163-
mockUseSelector.mockReturnValue(undefined);
195+
test('should render empty messages when annotation is not found', async () => {
196+
mockSelectorValues(undefined);
164197
render(<PopupV2 {...defaults} />);
198+
await flushPromises();
165199

166200
expect(screen.getByTestId('threaded-annotations-v2').getAttribute('data-messages-count')).toBe('0');
167201
});
168202

169-
test('should pass all action callbacks to ThreadedAnnotationsV2', () => {
203+
test('should pass all action callbacks to ThreadedAnnotationsV2', async () => {
170204
render(<PopupV2 {...defaults} />);
205+
await flushPromises();
171206

172207
const thread = screen.getByTestId('threaded-annotations-v2');
173208
expect(thread.getAttribute('data-has-on-post')).toBe('true');
@@ -176,21 +211,36 @@ describe('PopupV2', () => {
176211
expect(thread.getAttribute('data-has-on-unresolve')).toBe('true');
177212
});
178213

179-
test('should set popupThreadV2 as resin component', () => {
214+
test('should set popupThreadV2 as resin component', async () => {
180215
render(<PopupV2 {...defaults} />);
216+
await flushPromises();
181217

182218
const popup = screen.getByRole('presentation');
183219
expect(popup).toHaveAttribute('data-resin-component', 'popupThreadV2');
184220
});
221+
222+
test('should fetch avatars with Authorization header and no access_token query param', async () => {
223+
render(<PopupV2 {...defaults} />);
224+
await flushPromises();
225+
226+
expect(mockFetch).toHaveBeenCalledWith(
227+
'https://api.box.com/2.0/users/100/avatar?pic_type=large',
228+
{ headers: { Authorization: 'Bearer test-token' } },
229+
);
230+
const [calledUrl] = mockFetch.mock.calls[0];
231+
expect(calledUrl).not.toContain('access_token');
232+
});
185233
});
186234

187235
test('should set aria-label on popup container', () => {
236+
mockSelectorValues(undefined);
188237
render(<PopupV2 onSubmit={jest.fn()} reference={document.createElement('div')} />);
189238

190239
expect(screen.getByRole('presentation')).toHaveAttribute('aria-label', 'Comment');
191240
});
192241

193242
test('should render portal container for threaded-annotations popovers', () => {
243+
mockSelectorValues(undefined);
194244
render(<PopupV2 onSubmit={jest.fn()} reference={document.createElement('div')} />);
195245

196246
const portal = screen.getByRole('presentation').querySelector('[data-threaded-annotations-portal]');

src/store/options/__tests__/selectors-test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111

1212
describe('store/options/selectors', () => {
1313
const optionsState = {
14+
apiHost: 'https://api.box.com',
1415
features: { enabledFeature: true },
1516
fileId: '12345',
1617
fileVersionId: '67890',
@@ -21,6 +22,7 @@ describe('store/options/selectors', () => {
2122
},
2223
rotation: 0,
2324
scale: 1,
25+
token: 'test-token',
2426
viewMode: 'annotations' as const,
2527
};
2628

0 commit comments

Comments
 (0)