diff --git a/src/@types/model.ts b/src/@types/model.ts index e5bbbb037..49ca4caa2 100644 --- a/src/@types/model.ts +++ b/src/@types/model.ts @@ -12,6 +12,11 @@ export interface Annotation { modified_by: User; permissions: Permissions; replies?: Array; + resolution?: { + resolved_at: string; + resolved_by: User; + } | null; + status?: 'open' | 'resolved'; target: Target; type: 'annotation'; } diff --git a/src/common/BaseAnnotator.ts b/src/common/BaseAnnotator.ts index 7b658298d..1c8a6c0d7 100644 --- a/src/common/BaseAnnotator.ts +++ b/src/common/BaseAnnotator.ts @@ -94,11 +94,13 @@ export default class BaseAnnotator extends EventEmitter { }, common: { color: initialColor, mode: initialMode }, options: { + apiHost, features, fileId: file.id, fileVersionId: fileOptionsVersionId ?? fileVersionId, isCurrentFileVersion: !fileOptionsVersionId || fileOptionsVersionId === currentFileVersionId, permissions: file.permissions, + token, viewMode: initialViewMode, }, }; diff --git a/src/components/Popups/PopupV2.tsx b/src/components/Popups/PopupV2.tsx index 76aa86640..13512f84e 100644 --- a/src/components/Popups/PopupV2.tsx +++ b/src/components/Popups/PopupV2.tsx @@ -10,6 +10,7 @@ import { serializeMentionMarkup, } from '@box/threaded-annotations'; import type { DocumentNodeV2, TextMessageTypeV2 } from '@box/threaded-annotations'; +import type { FetchedAvatarUrls, UserContactType } from '@box/user-selector'; import type { JSONContent } from '@tiptap/core'; import FocusTrap from 'box-ui-elements/es/components/focus-trap/FocusTrap'; @@ -21,6 +22,7 @@ import { updateAnnotationAction, } from '../../store/annotations/actions'; import { getAnnotation } from '../../store/annotations/selectors'; +import { getApiHost, getToken } from '../../store/options'; import { fetchCollaboratorsAction } from '../../store/users/actions'; import type { AppState, AppThunkDispatch } from '../../store/types'; @@ -57,6 +59,21 @@ const createDocumentNode = (content: JSONContent | null): DocumentNodeV2 => { return { type: 'doc', content: [content] } as DocumentNodeV2; }; +// Callers render initials as a fallback on null. +// A persistent null across all users usually indicates a stale token. +const fetchAvatarBlob = async (apiHost: string, token: string, userId: string): Promise => { + try { + const response = await fetch(`${apiHost}/2.0/users/${userId}/avatar?pic_type=large`, { + headers: { Authorization: `Bearer ${token}` }, + }); + if (!response.ok) return null; + const blob = await response.blob(); + return URL.createObjectURL(blob); + } catch { + return null; + } +}; + const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => { const intl = useIntl(); const dispatch = useDispatch(); @@ -64,10 +81,43 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => { const popperRef = React.useRef(); const optionsRef = React.useRef>(getPopupOptions()); + const apiHost = useSelector(getApiHost); + const token = useSelector(getToken); const annotation = useSelector((state: AppState) => annotationId ? getAnnotation(state, annotationId) : undefined, ); + const [avatarBlobs, setAvatarBlobs] = React.useState>({}); + const avatarCacheRef = React.useRef>(new Map()); + const credentialsRef = React.useRef({ apiHost, token }); + credentialsRef.current = { apiHost, token }; + + const getOrFetchAvatarBlob = React.useCallback( + async (userId: string): Promise => { + const cached = avatarCacheRef.current.get(userId); + if (cached) return cached; + const capturedApiHost = apiHost; + const capturedToken = token; + const url = await fetchAvatarBlob(capturedApiHost, capturedToken, userId); + if (!url) return null; + if ( + credentialsRef.current.apiHost !== capturedApiHost || + credentialsRef.current.token !== capturedToken + ) { + URL.revokeObjectURL(url); + return null; + } + const existing = avatarCacheRef.current.get(userId); + if (existing) { + URL.revokeObjectURL(url); + return existing; + } + avatarCacheRef.current.set(userId, url); + return url; + }, + [apiHost, token], + ); + React.useEffect(() => { if (popupRef.current) { popperRef.current = createPopper(reference, popupRef.current, optionsRef.current); @@ -78,20 +128,85 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => { }; }, [reference]); + React.useEffect(() => { + const cache = avatarCacheRef.current; + return () => { + cache.forEach(url => URL.revokeObjectURL(url)); + cache.clear(); + }; + }, []); + + React.useEffect(() => { + avatarCacheRef.current.forEach(url => URL.revokeObjectURL(url)); + avatarCacheRef.current.clear(); + setAvatarBlobs({}); + }, [apiHost, token]); + const handleEvent = React.useCallback((event: React.SyntheticEvent) => { event.stopPropagation(); }, []); - const threadMessages: TextMessageTypeV2[] = React.useMemo( - () => (annotation ? annotationToMessages(annotation) : []), - [annotation], - ); + React.useEffect(() => { + if (!annotation) return undefined; + + const userIds = Array.from( + new Set(annotationToMessages(annotation).map(msg => String(msg.author.id))), + ); + let cancelled = false; + + Promise.all(userIds.map(async id => [id, await getOrFetchAvatarBlob(id)] as const)).then(entries => { + if (cancelled) return; + setAvatarBlobs(prev => { + const next = { ...prev }; + entries.forEach(([id, url]) => { + if (url) next[id] = url; + }); + return next; + }); + }); + + return () => { + cancelled = true; + }; + }, [annotation, getOrFetchAvatarBlob]); + + const threadMessages: TextMessageTypeV2[] = React.useMemo(() => { + if (!annotation) return []; + return annotationToMessages(annotation).map(msg => ({ + ...msg, + author: { + ...msg.author, + avatarUrl: avatarBlobs[String(msg.author.id)], + }, + })); + }, [annotation, avatarBlobs]); + + const isResolved = annotation?.status === 'resolved'; + const resolvedBy = isResolved + ? annotation?.resolution?.resolved_by?.name ?? annotation?.modified_by?.name + : undefined; + const resolvedAtSource = isResolved + ? annotation?.resolution?.resolved_at ?? annotation?.modified_at + : undefined; + const resolvedAt = resolvedAtSource ? new Date(resolvedAtSource).getTime() : undefined; const userSelectorProps = React.useMemo( () => ({ allowEmptyQuery: true, ariaRoleDescription: intl.formatMessage(messages.ariaLabelMentionSelector), - fetchAvatarUrls: async () => ({}), + fetchAvatarUrls: async (userContacts: UserContactType[]): Promise => { + const urls: FetchedAvatarUrls = {}; + await Promise.all( + userContacts.map(async ({ id }) => { + const key = String(id); + const blobUrl = await getOrFetchAvatarBlob(key); + if (blobUrl) { + urls[key] = blobUrl; + } + }), + ); + return urls; + }, fetchUsers: async (query: string) => { const action = await dispatch(fetchCollaboratorsAction(query)); if (fetchCollaboratorsAction.fulfilled.match(action)) { @@ -101,7 +216,7 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => { }, loadingAriaLabel: intl.formatMessage(messages.ariaLabelMentionLoading), }), - [dispatch, intl], + [dispatch, getOrFetchAvatarBlob, intl], ); const handlePost = React.useCallback( @@ -169,6 +284,7 @@ const PopupV2 = ({ annotationId, onSubmit, reference }: Props): JSX.Element => { {annotationId ? ( { onResolve={handleResolve} onThreadDelete={handleThreadDelete} onUnresolve={handleUnresolve} + resolvedAt={resolvedAt} + resolvedBy={resolvedBy} userSelectorProps={userSelectorProps} /> ) : ( diff --git a/src/components/Popups/__tests__/PopupV2-test.tsx b/src/components/Popups/__tests__/PopupV2-test.tsx index 5c6917456..4c6a19d86 100644 --- a/src/components/Popups/__tests__/PopupV2-test.tsx +++ b/src/components/Popups/__tests__/PopupV2-test.tsx @@ -1,7 +1,8 @@ import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import { useDispatch, useSelector } from 'react-redux'; import PopupV2, { Props } from '../PopupV2'; +import { getApiHost, getToken } from '../../../store/options'; jest.mock('react-redux', () => ({ useDispatch: jest.fn(), @@ -67,8 +68,34 @@ jest.mock('../../../store/users/actions', () => ({ const mockUseDispatch = useDispatch as jest.MockedFunction; const mockUseSelector = useSelector as jest.MockedFunction; +const mockSelectorValues = (annotation?: unknown): void => { + mockUseSelector.mockImplementation(selector => { + if (selector === getApiHost) return 'https://api.box.com'; + if (selector === getToken) return 'test-token'; + return annotation; + }); +}; + describe('PopupV2', () => { const mockDispatch = jest.fn(); + const mockFetch = jest.fn(); + const originalFetch = window.fetch; + const originalCreateObjectURL = window.URL.createObjectURL; + const originalRevokeObjectURL = window.URL.revokeObjectURL; + + beforeAll(() => { + window.fetch = mockFetch as unknown as typeof fetch; + window.URL.createObjectURL = jest.fn().mockReturnValue('blob:mock-url'); + window.URL.revokeObjectURL = jest.fn(); + }); + + afterAll(() => { + window.fetch = originalFetch; + window.URL.createObjectURL = originalCreateObjectURL; + window.URL.revokeObjectURL = originalRevokeObjectURL; + }); + + const flushPromises = (): Promise => act(() => new Promise(resolve => { setTimeout(resolve, 0); })); const mockAnnotation = { created_at: '2026-01-01T00:00:00Z', @@ -92,6 +119,10 @@ describe('PopupV2', () => { beforeEach(() => { mockUseDispatch.mockReturnValue(mockDispatch); + mockFetch.mockResolvedValue({ + blob: () => Promise.resolve(new Blob(['avatar'])), + ok: true, + }); }); afterEach(() => { @@ -105,7 +136,7 @@ describe('PopupV2', () => { }; beforeEach(() => { - mockUseSelector.mockReturnValue(undefined); + mockSelectorValues(undefined); }); test('should render MessageEditorV2 with FocusTrap and MentionContextProvider', () => { @@ -139,11 +170,12 @@ describe('PopupV2', () => { }; beforeEach(() => { - mockUseSelector.mockReturnValue(mockAnnotation); + mockSelectorValues(mockAnnotation); }); - test('should render ThreadedAnnotationsV2 with FocusTrap and MentionContextProvider', () => { + test('should render ThreadedAnnotationsV2 with FocusTrap and MentionContextProvider', async () => { render(); + await flushPromises(); expect(screen.getByTestId('focus-trap')).toBeVisible(); expect(screen.getByTestId('mention-context')).toBeVisible(); @@ -151,23 +183,26 @@ describe('PopupV2', () => { expect(screen.queryByTestId('message-editor-v2')).toBeNull(); }); - test('should render with isAnnotations=true and messages from annotation', () => { + test('should render with isAnnotations=true and messages from annotation', async () => { render(); + await flushPromises(); const thread = screen.getByTestId('threaded-annotations-v2'); expect(thread.getAttribute('data-is-annotations')).toBe('true'); expect(thread.getAttribute('data-messages-count')).toBe('1'); }); - test('should render empty messages when annotation is not found', () => { - mockUseSelector.mockReturnValue(undefined); + test('should render empty messages when annotation is not found', async () => { + mockSelectorValues(undefined); render(); + await flushPromises(); expect(screen.getByTestId('threaded-annotations-v2').getAttribute('data-messages-count')).toBe('0'); }); - test('should pass all action callbacks to ThreadedAnnotationsV2', () => { + test('should pass all action callbacks to ThreadedAnnotationsV2', async () => { render(); + await flushPromises(); const thread = screen.getByTestId('threaded-annotations-v2'); expect(thread.getAttribute('data-has-on-post')).toBe('true'); @@ -176,21 +211,36 @@ describe('PopupV2', () => { expect(thread.getAttribute('data-has-on-unresolve')).toBe('true'); }); - test('should set popupThreadV2 as resin component', () => { + test('should set popupThreadV2 as resin component', async () => { render(); + await flushPromises(); const popup = screen.getByRole('presentation'); expect(popup).toHaveAttribute('data-resin-component', 'popupThreadV2'); }); + + test('should fetch avatars with Authorization header and no access_token query param', async () => { + render(); + await flushPromises(); + + expect(mockFetch).toHaveBeenCalledWith( + 'https://api.box.com/2.0/users/100/avatar?pic_type=large', + { headers: { Authorization: 'Bearer test-token' } }, + ); + const [calledUrl] = mockFetch.mock.calls[0]; + expect(calledUrl).not.toContain('access_token'); + }); }); test('should set aria-label on popup container', () => { + mockSelectorValues(undefined); render(); expect(screen.getByRole('presentation')).toHaveAttribute('aria-label', 'Comment'); }); test('should render portal container for threaded-annotations popovers', () => { + mockSelectorValues(undefined); render(); const portal = screen.getByRole('presentation').querySelector('[data-threaded-annotations-portal]'); diff --git a/src/store/options/__tests__/selectors-test.ts b/src/store/options/__tests__/selectors-test.ts index 2f13904d5..3595e968d 100644 --- a/src/store/options/__tests__/selectors-test.ts +++ b/src/store/options/__tests__/selectors-test.ts @@ -11,6 +11,7 @@ import { describe('store/options/selectors', () => { const optionsState = { + apiHost: 'https://api.box.com', features: { enabledFeature: true }, fileId: '12345', fileVersionId: '67890', @@ -21,6 +22,7 @@ describe('store/options/selectors', () => { }, rotation: 0, scale: 1, + token: 'test-token', viewMode: 'annotations' as const, }; diff --git a/src/store/options/reducer.ts b/src/store/options/reducer.ts index c6afb0765..be642d9d7 100644 --- a/src/store/options/reducer.ts +++ b/src/store/options/reducer.ts @@ -9,7 +9,8 @@ import { setViewModeAction, } from './actions'; -export const initialState = { +export const initialState: OptionsState = { + apiHost: '', features: {}, fileId: null, fileVersionId: null, @@ -17,6 +18,7 @@ export const initialState = { permissions: {}, rotation: 0, scale: 1, + token: '', viewMode: 'annotations' as ViewMode, }; diff --git a/src/store/options/selectors.ts b/src/store/options/selectors.ts index 2649128c0..0f7031d87 100644 --- a/src/store/options/selectors.ts +++ b/src/store/options/selectors.ts @@ -6,6 +6,7 @@ import { ViewMode } from './types'; type State = Pick; +export const getApiHost = (state: State): string => state.options.apiHost; export const getFeatures = (state: State): Features => state.options.features; export const getViewMode = (state: State): ViewMode => state.options.viewMode; export const getFileId = (state: State): string | null => state.options.fileId; @@ -14,5 +15,6 @@ export const getIsCurrentFileVersion = (state: State): boolean => state.options. export const getPermissions = (state: State): Permissions => state.options.permissions; export const getRotation = (state: State): number => state.options.rotation; export const getScale = (state: State): number => state.options.scale; +export const getToken = (state: State): string => state.options.token; export const isFeatureEnabled = (state: State, featurename: string): boolean => getProp(getFeatures(state), featurename, false); diff --git a/src/store/options/types.ts b/src/store/options/types.ts index af31957f3..9abfd9ab0 100644 --- a/src/store/options/types.ts +++ b/src/store/options/types.ts @@ -5,6 +5,7 @@ import { Permissions } from '../../@types'; export type ViewMode = 'annotations' | 'boundingBoxes'; export type OptionsState = { + apiHost: string; features: Features; fileId: string | null; fileVersionId: string | null; @@ -12,5 +13,6 @@ export type OptionsState = { permissions: Permissions; rotation: number; scale: number; + token: string; viewMode: ViewMode; };