diff --git a/.changeset/files-anonymous-upload-tokens.md b/.changeset/files-anonymous-upload-tokens.md new file mode 100644 index 000000000..09aa6e83e --- /dev/null +++ b/.changeset/files-anonymous-upload-tokens.md @@ -0,0 +1,11 @@ +--- +'@baseapp-frontend/components': patch +--- + +Support the backend's anonymous uploads and private files. The upload hooks keep the +`uploadToken` an anonymous upload gets at initiation and send it back on complete and abort +(`X-File-Upload-Token`) and on `fileAttachToTarget` (`uploadTokens`); logged-in uploads are +unchanged. `AttachedFileItem` disables Download when the file has no `url`, which private files +return for viewers without access. +With `autoAttach: false`, `onUploadComplete` also receives the upload tokens of the files +(`(fileRelayIds, uploadTokens)`), since the files leave the upload store before the caller attaches them. diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index e53fc87d8..bcecf1e46 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -1,5 +1,26 @@ # @baseapp-frontend/components +## 3.0.1 + +### Patch Changes + +- fee7012: Comment attachments accept any file type (videos included) and any size. The comment composer and + comment edit no longer restrict uploads to PNG/JPG/PDF up to 100MB, and `useFileSelect` / + `FileUploadDropzone` no longer default to a 100MB cap; pass `maxFileSize` / `acceptedFileTypes` to + restrict them. Limits are left to the backend (`MAX_FILE_UPLOAD_SIZE`): when it rejects an upload, + its reason is shown on the file and in an error toast instead of axios' generic "Request failed with + status code 400". This includes a rejected part upload: `uploadChunk` keeps the response body + and `uploadChunks` passes it on after its retries. `FileUploadDropzone` treats only an omitted + `maxFileSize` as unlimited, so an explicit `0` is enforced. `formatFileSize` now reports gigabytes. + A non-JSON error body (e.g. a proxy's HTML error page) shows the generic message instead of its + first character, and nested DRF errors show their first message instead of `[object Object]`. +- 5ee95e2: `File.fileSize` is a `BigInt` in the GraphQL schema, so files over 2 GB report their size. The + shared Relay config maps `BigInt` to `number`, keeping `fileSize` typed as a number. + - @baseapp-frontend/authentication@6.0.1 + - @baseapp-frontend/design-system@2.1.2 + - @baseapp-frontend/graphql@2.0.2 + - @baseapp-frontend/utils@4.2.1 + ## 3.0.0 ### Major Changes diff --git a/packages/components/modules/comments/web/CommentCreate/index.tsx b/packages/components/modules/comments/web/CommentCreate/index.tsx index 509679ab7..2a1e922cb 100644 --- a/packages/components/modules/comments/web/CommentCreate/index.tsx +++ b/packages/components/modules/comments/web/CommentCreate/index.tsx @@ -16,11 +16,6 @@ import CommentFilesUpsertActions from './CommentFilesUpsertActions' import { CommentCreateProps } from './types' const MAX_FILES = 5 -const MAX_FILE_SIZE = 100 * 1024 * 1024 // 100MB -const ACCEPTED_FILE_TYPES = { - 'image/*': ['.png', '.jpg'], - 'application/pdf': ['.pdf'], -} /** * ### CommentCreate Component @@ -153,8 +148,6 @@ const CommentCreate = forwardRef( onFilesSelected: handleFilesSelected, isUploading, maxFiles: MAX_FILES, - maxFileSize: MAX_FILE_SIZE, - acceptedFileTypes: ACCEPTED_FILE_TYPES, }} // Inside the form, not after it: the form is sticky, so a sibling would sit // below the pinned composer and stay off-screen until the list is scrolled. diff --git a/packages/components/modules/comments/web/CommentUpdate/CommentFilesUpsertActions/index.tsx b/packages/components/modules/comments/web/CommentUpdate/CommentFilesUpsertActions/index.tsx index e8d927b44..9b8b18398 100644 --- a/packages/components/modules/comments/web/CommentUpdate/CommentFilesUpsertActions/index.tsx +++ b/packages/components/modules/comments/web/CommentUpdate/CommentFilesUpsertActions/index.tsx @@ -10,11 +10,6 @@ import { ActionsContainer } from './styled' import type { CommentFilesUpsertActionsProps } from './types' const MAX_FILES = 5 -const MAX_FILE_SIZE = 100 * 1024 * 1024 // 100MB -const ACCEPTED_FILE_TYPES = { - 'image/*': ['.png', '.jpg'], - 'application/pdf': ['.pdf'], -} /** * SocialInput upsert-action bar for the comment editor: the attach icon is the @@ -27,8 +22,6 @@ const CommentFilesUpsertActions: FC = ({ target as="button" icon={} maxFiles={MAX_FILES} - maxFileSize={MAX_FILE_SIZE} - acceptedFileTypes={ACCEPTED_FILE_TYPES} autoAttach /> diff --git a/packages/components/modules/files/README.md b/packages/components/modules/files/README.md index 0e4f7ad75..4323aaf0c 100644 --- a/packages/components/modules/files/README.md +++ b/packages/components/modules/files/README.md @@ -124,7 +124,7 @@ const { **Parameters:** - `targetObjectId?: string` - The Relay ID of the target object to attach files to - `autoAttach?: boolean` - Automatically attach files after upload (default: true) -- `onUploadComplete?: (fileRelayIds: string[]) => void` - Called after upload succeeds +- `onUploadComplete?: (fileRelayIds: string[], uploadTokens: string[]) => void` - Called after upload succeeds. `uploadTokens` holds the tokens of anonymous uploads among them; with `autoAttach: false`, pass them as `uploadTokens` to `fileAttachToTarget` when attaching - `onAttachComplete?: () => void` - Called after files are attached - `onError?: (error: Error) => void` - Called on errors @@ -327,11 +327,11 @@ fragment FileItem_file on File { |------|------|---------|-------------| | target | `FilesInterface$key` | required | Relay fragment reference for the target object | | maxFiles | `number` | - | Maximum number of files allowed | -| maxFileSize | `number` | - | Maximum file size in bytes | -| acceptedFileTypes | `string[]` | - | Accepted MIME types (e.g., `['image/*', 'application/pdf']`) | +| maxFileSize | `number` | no limit | Maximum file size in bytes; when omitted the backend's `MAX_FILE_UPLOAD_SIZE` decides and its error is toasted | +| acceptedFileTypes | `string[]` | any type | Accepted MIME types (e.g., `['image/*', 'application/pdf']`) | | disabled | `boolean` | false | Disable file upload | | autoAttach | `boolean` | true | Automatically attach files after upload | -| onUploadComplete | `(fileRelayIds: string[]) => void` | - | Callback when upload completes | +| onUploadComplete | `(fileRelayIds: string[], uploadTokens: string[]) => void` | - | Callback when upload completes; `uploadTokens` are needed to attach anonymous uploads yourself | | onAttachComplete | `() => void` | - | Callback when files are attached | | onError | `(error: Error) => void` | - | Error callback | @@ -454,6 +454,10 @@ File operations respect backend permissions: - Delete button only shows if user has `change_file` permission - Permission checked via `hasPerm` field in FileItem fragment +- With private files on the backend, `url` is a signed, expiring link, or `null` when the viewer + may not see the file; the Download button is then disabled +- Anonymous uploads (when the backend allows them) need no setup: the hooks keep the + `uploadToken` returned at initiation and send it back on complete, abort and attach ## Best Practices diff --git a/packages/components/modules/files/common/hooks/useChunkedUpload/__tests__/useChunkedUpload.test.ts b/packages/components/modules/files/common/hooks/useChunkedUpload/__tests__/useChunkedUpload.test.ts index ad93afd87..b5433eb19 100644 --- a/packages/components/modules/files/common/hooks/useChunkedUpload/__tests__/useChunkedUpload.test.ts +++ b/packages/components/modules/files/common/hooks/useChunkedUpload/__tests__/useChunkedUpload.test.ts @@ -8,11 +8,16 @@ import { useFileUploadStore } from '../../../context/FileUploadProvider' import { uploadChunks } from '../../../utils' import { useChunkedUpload } from '../index' -vi.mock('@baseapp-frontend/utils', () => ({ +const sendToast = vi.fn() +vi.mock('@baseapp-frontend/utils', async () => ({ axios: { post: vi.fn(), delete: vi.fn(), }, + getApiErrorMessage: ( + await vi.importActual('@baseapp-frontend/utils') + ).getApiErrorMessage, + useNotification: () => ({ sendToast }), })) vi.mock('../../../utils', async () => ({ @@ -61,17 +66,41 @@ describe('useChunkedUpload', () => { 'files/uploads', expect.objectContaining({ numParts: 2, partSize: CHUNK_SIZE }), ) - expect(mockAxiosPost).toHaveBeenNthCalledWith(2, 'files/uploads/file-public-id/complete', { - parts: [ - { partNumber: 1, etag: 'etag-0' }, - { partNumber: 2, etag: 'etag-1' }, - ], - }) + expect(mockAxiosPost).toHaveBeenNthCalledWith( + 2, + 'files/uploads/file-public-id/complete', + { + parts: [ + { partNumber: 1, etag: 'etag-0' }, + { partNumber: 2, etag: 'etag-1' }, + ], + }, + undefined, + ) const progress = Array.from(useFileUploadStore.getState().files.values())[0]! expect(progress.status).toBe(FileUploadStatus.COMPLETED) }) + it('sends the upload token of an anonymous upload back when completing it', async () => { + mockAxiosPost + .mockResolvedValueOnce({ ...initiateResponse(1), uploadToken: 'token-1' } as never) + .mockResolvedValueOnce({} as never) + mockUploadChunks.mockResolvedValue(['etag-0']) + + const { result } = renderHook(() => useChunkedUpload()) + await result.current.uploadFile(makeFile(1)) + + expect(mockAxiosPost).toHaveBeenNthCalledWith( + 2, + 'files/uploads/file-public-id/complete', + expect.anything(), + { headers: { 'X-File-Upload-Token': 'token-1' } }, + ) + const progress = Array.from(useFileUploadStore.getState().files.values())[0]! + expect(progress.uploadToken).toBe('token-1') + }) + it('uploadFile marks the file failed and rethrows on error', async () => { mockAxiosPost.mockRejectedValue(new Error('initiate failed')) @@ -83,6 +112,57 @@ describe('useChunkedUpload', () => { expect(progress.error).toBe('initiate failed') }) + it('shows the backend reason for a rejected upload, in the file and in a toast', async () => { + mockAxiosPost.mockRejectedValue( + Object.assign(new Error('Request failed with status code 400'), { + response: { data: ['File is too large. The maximum size is 5.0\u00a0GB.'] }, + }), + ) + + const { result } = renderHook(() => useChunkedUpload()) + await expect(result.current.uploadFile(makeFile(1))).rejects.toThrow() + + const progress = Array.from(useFileUploadStore.getState().files.values())[0]! + expect(progress.error).toBe('File is too large. The maximum size is 5.0\u00a0GB.') + expect(sendToast).toHaveBeenCalledWith( + 'video.mp4: File is too large. The maximum size is 5.0\u00a0GB.', + { type: 'error' }, + ) + }) + + it('shows the backend reason for a rejected part upload', async () => { + mockAxiosPost.mockResolvedValueOnce(initiateResponse(1) as never) + mockUploadChunks.mockRejectedValue( + Object.assign(new Error('Failed to upload chunk 0 after 3 attempts'), { + response: { status: 400, data: { detail: 'Upload has expired.' } }, + }), + ) + + const { result } = renderHook(() => useChunkedUpload()) + await expect(result.current.uploadFile(makeFile(1))).rejects.toThrow() + + const progress = Array.from(useFileUploadStore.getState().files.values())[0]! + expect(progress.error).toBe('Upload has expired.') + expect(sendToast).toHaveBeenCalledWith('video.mp4: Upload has expired.', { type: 'error' }) + }) + + it.each([ + ['an HTML error page', '502 Bad Gateway', 'Upload failed'], + ['a nested DRF error', { parts: [{ etag: ['etag is required'] }] }, 'etag is required'], + ['a detail message', { detail: 'Not allowed.' }, 'Not allowed.'], + ])('shows a readable message for %s', async (_case, data, expected) => { + mockAxiosPost.mockRejectedValue( + Object.assign(new Error('Request failed with status code 400'), { response: { data } }), + ) + + const { result } = renderHook(() => useChunkedUpload()) + await expect(result.current.uploadFile(makeFile(1))).rejects.toThrow() + + const progress = Array.from(useFileUploadStore.getState().files.values())[0]! + expect(progress.error).toBe(expected) + expect(sendToast).toHaveBeenCalledWith(`video.mp4: ${expected}`, { type: 'error' }) + }) + it('resumeUpload uploads only the chunks without recorded ETags', async () => { const file = makeFile(3) const { addFile, updateFileProgress } = useFileUploadStore.getState() @@ -112,13 +192,17 @@ describe('useChunkedUpload', () => { expect(call.chunks).toHaveLength(1) expect(call.presignedUrls).toEqual(['https://s3/part-1']) - expect(mockAxiosPost).toHaveBeenCalledWith('files/uploads/file-public-id/complete', { - parts: [ - { partNumber: 1, etag: 'etag-0' }, - { partNumber: 2, etag: 'etag-1' }, - { partNumber: 3, etag: 'etag-2' }, - ], - }) + expect(mockAxiosPost).toHaveBeenCalledWith( + 'files/uploads/file-public-id/complete', + { + parts: [ + { partNumber: 1, etag: 'etag-0' }, + { partNumber: 2, etag: 'etag-1' }, + { partNumber: 3, etag: 'etag-2' }, + ], + }, + undefined, + ) expect(useFileUploadStore.getState().files.get(fileId)!.status).toBe(FileUploadStatus.COMPLETED) }) @@ -148,15 +232,20 @@ describe('useChunkedUpload', () => { await result.current.resumeUpload(fileId) // Stale upload aborted server-side, then a fresh initiate + full upload. - expect(mockAxiosDelete).toHaveBeenCalledWith('files/uploads/stale-id') + expect(mockAxiosDelete).toHaveBeenCalledWith('files/uploads/stale-id', undefined) expect(mockAxiosPost).toHaveBeenNthCalledWith(1, 'files/uploads', expect.any(Object)) expect(mockUploadChunks.mock.calls[0]![0].chunks).toHaveLength(2) - expect(mockAxiosPost).toHaveBeenNthCalledWith(2, 'files/uploads/file-public-id/complete', { - parts: [ - { partNumber: 1, etag: 'fresh-0' }, - { partNumber: 2, etag: 'fresh-1' }, - ], - }) + expect(mockAxiosPost).toHaveBeenNthCalledWith( + 2, + 'files/uploads/file-public-id/complete', + { + parts: [ + { partNumber: 1, etag: 'fresh-0' }, + { partNumber: 2, etag: 'fresh-1' }, + ], + }, + undefined, + ) }) it('resumeUpload ignores files that are not paused', async () => { @@ -216,12 +305,16 @@ describe('useChunkedUpload', () => { expect(mockUploadChunks).not.toHaveBeenCalled() expect(mockAxiosDelete).not.toHaveBeenCalled() expect(mockAxiosPost).toHaveBeenCalledTimes(1) - expect(mockAxiosPost).toHaveBeenCalledWith('files/uploads/file-public-id/complete', { - parts: [ - { partNumber: 1, etag: 'etag-0' }, - { partNumber: 2, etag: 'etag-1' }, - ], - }) + expect(mockAxiosPost).toHaveBeenCalledWith( + 'files/uploads/file-public-id/complete', + { + parts: [ + { partNumber: 1, etag: 'etag-0' }, + { partNumber: 2, etag: 'etag-1' }, + ], + }, + undefined, + ) expect(useFileUploadStore.getState().files.get(fileId)!.status).toBe(FileUploadStatus.COMPLETED) }) diff --git a/packages/components/modules/files/common/hooks/useChunkedUpload/index.ts b/packages/components/modules/files/common/hooks/useChunkedUpload/index.ts index 6441d0138..39607d7e9 100644 --- a/packages/components/modules/files/common/hooks/useChunkedUpload/index.ts +++ b/packages/components/modules/files/common/hooks/useChunkedUpload/index.ts @@ -1,6 +1,6 @@ import { useCallback } from 'react' -import { axios } from '@baseapp-frontend/utils' +import { axios, getApiErrorMessage, useNotification } from '@baseapp-frontend/utils' import { CHUNK_SIZE, FileUploadStatus, URL_EXPIRY_SAFETY_MARGIN_MS } from '../../constants' import { useFileUploadStore } from '../../context/FileUploadProvider' @@ -13,6 +13,28 @@ const areUrlsExpired = (initiatedAt?: number, expiresIn?: number): boolean => { return Date.now() >= initiatedAt + expiresIn * 1000 - URL_EXPIRY_SAFETY_MARGIN_MS } +/** First string anywhere in a DRF error body, e.g. {"parts": [{"etag": ["..."]}]}. */ +const firstMessage = (value: unknown): string | undefined => { + if (typeof value === 'string') return value + if (Array.isArray(value)) return value.map(firstMessage).find(Boolean) + if (value && typeof value === 'object') + return Object.values(value).map(firstMessage).find(Boolean) + return undefined +} + +/** + * The backend's reason for a failed request (e.g. file too large). A non-JSON body, + * such as a proxy's HTML error page, falls back instead of showing its first character. + */ +const uploadErrorMessage = (error: unknown, fallbackMessage: string): string => { + const data = (error as { response?: { data?: unknown } } | undefined)?.response?.data + if (data === undefined || data === null || data === '') { + return getApiErrorMessage(error, { defaultMessage: fallbackMessage }) + } + if (typeof data !== 'object') return fallbackMessage + return firstMessage(data) ?? fallbackMessage +} + const buildParts = (etags: (string | undefined)[]): CompleteUploadPart[] => etags.map((etag, idx) => { if (!etag) { @@ -21,12 +43,17 @@ const buildParts = (etags: (string | undefined)[]): CompleteUploadPart[] => return { partNumber: idx + 1, etag } }) +// Anonymous uploads prove they started the upload with the token from initiation. +const uploadTokenConfig = (uploadToken?: string) => + uploadToken ? { headers: { 'X-File-Upload-Token': uploadToken } } : undefined + export const useChunkedUpload = (options?: UseChunkedUploadOptions) => { // Actions are picked individually: subscribing to the whole store here would // re-render every consumer of this hook on each progress tick. const addFile = useFileUploadStore((state) => state.addFile) const updateFileProgress = useFileUploadStore((state) => state.updateFileProgress) const updateChunkProgress = useFileUploadStore((state) => state.updateChunkProgress) + const { sendToast } = useNotification() /** * Translate a thrown upload error into store state. A pause aborts the @@ -44,14 +71,16 @@ export const useChunkedUpload = (options?: UseChunkedUploadOptions) => { return } - const errorMessage = error instanceof Error ? error.message : fallbackMessage + // Prefer the backend's reason (e.g. file too large) over axios' "status code 400". + const errorMessage = uploadErrorMessage(error, fallbackMessage) updateFileProgress(fileId, { status: FileUploadStatus.FAILED, error: errorMessage, }) + sendToast(`${current?.fileName ?? 'File'}: ${errorMessage}`, { type: 'error' }) options?.onUploadError?.(fileId, error instanceof Error ? error : new Error(errorMessage)) }, - [updateFileProgress, options], + [updateFileProgress, sendToast, options], ) /** @@ -89,6 +118,7 @@ export const useChunkedUpload = (options?: UseChunkedUploadOptions) => { uploadId: data.uploadId, backendId: data.id, fileRelayId: data.relayId, + uploadToken: data.uploadToken, presignedUrls: data.presignedUrls, initiatedAt: Date.now(), expiresIn: data.expiresIn, @@ -106,9 +136,11 @@ export const useChunkedUpload = (options?: UseChunkedUploadOptions) => { }, }) - await axios.post(`files/uploads/${data.id}/complete`, { - parts: buildParts(etags), - }) + await axios.post( + `files/uploads/${data.id}/complete`, + { parts: buildParts(etags) }, + uploadTokenConfig(data.uploadToken), + ) updateFileProgress(fileId, { status: FileUploadStatus.COMPLETED, @@ -171,7 +203,12 @@ export const useChunkedUpload = (options?: UseChunkedUploadOptions) => { // The presigned part URLs have expired; abort the stale upload and // start fresh, since we cannot re-sign individual parts. if (fileProgress.backendId) { - await axios.delete(`files/uploads/${fileProgress.backendId}`).catch(() => {}) + await axios + .delete( + `files/uploads/${fileProgress.backendId}`, + uploadTokenConfig(fileProgress.uploadToken), + ) + .catch(() => {}) } const relayId = await startUpload(fileId, fileProgress.file) options?.onUploadComplete?.(fileId, relayId) @@ -203,9 +240,11 @@ export const useChunkedUpload = (options?: UseChunkedUploadOptions) => { }) } - await axios.post(`files/uploads/${fileProgress.backendId}/complete`, { - parts: buildParts(allEtags), - }) + await axios.post( + `files/uploads/${fileProgress.backendId}/complete`, + { parts: buildParts(allEtags) }, + uploadTokenConfig(fileProgress.uploadToken), + ) updateFileProgress(fileId, { status: FileUploadStatus.COMPLETED, diff --git a/packages/components/modules/files/common/hooks/useDeferredFileAttachments/index.ts b/packages/components/modules/files/common/hooks/useDeferredFileAttachments/index.ts index deaea8392..6a92d00b7 100644 --- a/packages/components/modules/files/common/hooks/useDeferredFileAttachments/index.ts +++ b/packages/components/modules/files/common/hooks/useDeferredFileAttachments/index.ts @@ -71,6 +71,7 @@ export const useDeferredFileAttachments = (): UseDeferredFileAttachmentsReturn = (file) => file.status === FileUploadStatus.COMPLETED && !!file.fileRelayId, ) const fileRelayIds = scoped.map((file) => file.fileRelayId as string) + const uploadTokens = scoped.flatMap((file) => (file.uploadToken ? [file.uploadToken] : [])) // Drop everything this submit claimed, attached or not. A paused or failed // upload left in scope would otherwise ride along with the next comment. @@ -85,7 +86,7 @@ export const useDeferredFileAttachments = (): UseDeferredFileAttachmentsReturn = const connectionID = ConnectionHandler.getConnectionID(targetObjectId, 'FilesList_files') attachFiles({ variables: { - input: { fileRelayIds, targetObjectId }, + input: { fileRelayIds, targetObjectId, uploadTokens }, connections: [connectionID], }, onCompleted: () => { diff --git a/packages/components/modules/files/common/hooks/useFileSelect/__tests__/useFileSelect.test.ts b/packages/components/modules/files/common/hooks/useFileSelect/__tests__/useFileSelect.test.ts index 98b40810a..4b92ed391 100644 --- a/packages/components/modules/files/common/hooks/useFileSelect/__tests__/useFileSelect.test.ts +++ b/packages/components/modules/files/common/hooks/useFileSelect/__tests__/useFileSelect.test.ts @@ -54,6 +54,19 @@ describe('useFileSelect', () => { expect(onFilesSelected).toHaveBeenCalledWith([ok1, ok2]) }) + it('by default accepts any type and size, leaving limits to the backend', () => { + const onFilesSelected = vi.fn() + const { result } = renderHook(() => useFileSelect({ onFilesSelected })) + + const video = makeFile('clip.mov', 2 * 1024 * 1024 * 1024) + const huge = makeFile('disk.iso', 50 * 1024 * 1024 * 1024) + + fireChange(result.current.getInputProps().onChange, [video, huge]) + + expect(result.current.getInputProps().accept).toBeUndefined() + expect(onFilesSelected).toHaveBeenCalledWith([video, huge]) + }) + it('does not fire the callback when nothing valid was selected, and resets the input', () => { const onFilesSelected = vi.fn() const { result } = renderHook(() => useFileSelect({ onFilesSelected, maxFileSize: 100 })) diff --git a/packages/components/modules/files/common/hooks/useFileSelect/index.ts b/packages/components/modules/files/common/hooks/useFileSelect/index.ts index ec5e0efbe..4bdf0003f 100644 --- a/packages/components/modules/files/common/hooks/useFileSelect/index.ts +++ b/packages/components/modules/files/common/hooks/useFileSelect/index.ts @@ -16,7 +16,7 @@ const buildAcceptAttr = (acceptedFileTypes?: Accept): string | undefined => { export const useFileSelect = ({ onFilesSelected, maxFiles = 10, - maxFileSize = 100 * 1024 * 1024, + maxFileSize, acceptedFileTypes, disabled = false, }: UseFileSelectParams): UseFileSelectReturn => { @@ -41,7 +41,8 @@ export const useFileSelect = ({ onChange: (event) => { const { target } = event const selected = Array.from(target.files ?? []) - .filter((file) => file.size <= maxFileSize) + // No limit by default: the backend enforces MAX_FILE_UPLOAD_SIZE and the error is toasted. + .filter((file) => maxFileSize === undefined || file.size <= maxFileSize) .slice(0, maxFiles) // Reset so selecting the same file again re-triggers onChange. target.value = '' diff --git a/packages/components/modules/files/common/hooks/useFileSelect/types.ts b/packages/components/modules/files/common/hooks/useFileSelect/types.ts index 63c449805..3b6457aaf 100644 --- a/packages/components/modules/files/common/hooks/useFileSelect/types.ts +++ b/packages/components/modules/files/common/hooks/useFileSelect/types.ts @@ -5,7 +5,7 @@ export type Accept = Record export interface UseFileSelectParams { onFilesSelected: (files: File[]) => void maxFiles?: number - maxFileSize?: number // in bytes + maxFileSize?: number // in bytes; unlimited when omitted acceptedFileTypes?: Accept disabled?: boolean } diff --git a/packages/components/modules/files/common/hooks/useFileUploadLogic/__tests__/useFileUploadLogic.uploadTokens.test.ts b/packages/components/modules/files/common/hooks/useFileUploadLogic/__tests__/useFileUploadLogic.uploadTokens.test.ts new file mode 100644 index 000000000..7b4223ef6 --- /dev/null +++ b/packages/components/modules/files/common/hooks/useFileUploadLogic/__tests__/useFileUploadLogic.uploadTokens.test.ts @@ -0,0 +1,71 @@ +import { act, renderHook } from '@testing-library/react' +import { vi } from 'vitest' + +import { FileUploadStatus } from '../../../constants' +import { useFileUploadStore } from '../../../context/FileUploadProvider' +import { useFileUploadLogic } from '../index' + +const mockUploadFile = vi.fn() + +vi.mock('react-relay', () => ({ + ConnectionHandler: { getConnectionID: (id: string) => `connection:${id}` }, +})) + +vi.mock('../../../graphql/mutations/FileAttachToTarget', () => ({ + useFileAttachToTargetMutation: () => [vi.fn(), false], +})) + +vi.mock('../../useChunkedUpload', () => ({ + useChunkedUpload: () => ({ uploadFile: mockUploadFile }), +})) + +const TARGET = 'Comment:1' + +/** Like useChunkedUpload: the finished upload is in the store, with its token if anonymous. */ +const finishUpload = (relayId: string, uploadToken?: string) => (file: File, scope?: string) => { + const { addFile, updateFileProgress } = useFileUploadStore.getState() + const id = addFile(file, scope) + updateFileProgress(id, { + status: FileUploadStatus.COMPLETED, + fileRelayId: relayId, + uploadToken, + }) + return Promise.resolve(relayId) +} + +describe('useFileUploadLogic upload tokens', () => { + beforeEach(() => { + vi.clearAllMocks() + useFileUploadStore.setState({ files: new Map() }) + }) + + it('hands the caller the upload tokens when it attaches files itself', async () => { + mockUploadFile.mockImplementation(finishUpload('File:1', 'token-1')) + const onUploadComplete = vi.fn() + const { result } = renderHook(() => + useFileUploadLogic({ targetObjectId: TARGET, autoAttach: false, onUploadComplete }), + ) + + await act(async () => { + await result.current.handleFilesSelected([new File(['x'], 'doc.pdf')]) + }) + + expect(onUploadComplete).toHaveBeenCalledWith(['File:1'], ['token-1']) + // The store no longer has the file, so the callback is the only place left for the token. + expect(useFileUploadStore.getState().files.size).toBe(0) + }) + + it('passes no tokens for logged-in uploads', async () => { + mockUploadFile.mockImplementation(finishUpload('File:1')) + const onUploadComplete = vi.fn() + const { result } = renderHook(() => + useFileUploadLogic({ targetObjectId: TARGET, autoAttach: false, onUploadComplete }), + ) + + await act(async () => { + await result.current.handleFilesSelected([new File(['x'], 'doc.pdf')]) + }) + + expect(onUploadComplete).toHaveBeenCalledWith(['File:1'], []) + }) +}) diff --git a/packages/components/modules/files/common/hooks/useFileUploadLogic/index.ts b/packages/components/modules/files/common/hooks/useFileUploadLogic/index.ts index 7fde13133..6a4f37fe0 100644 --- a/packages/components/modules/files/common/hooks/useFileUploadLogic/index.ts +++ b/packages/components/modules/files/common/hooks/useFileUploadLogic/index.ts @@ -5,6 +5,7 @@ import { ConnectionHandler } from 'react-relay' import { FileUploadStatus } from '../../constants' import { useFileUploadStore } from '../../context/FileUploadProvider' import { useFileAttachToTargetMutation } from '../../graphql/mutations/FileAttachToTarget' +import { uploadTokensFor } from '../../utils/uploadTokens' import { useChunkedUpload } from '../useChunkedUpload' import type { AttachOptions, UseFileUploadLogicParams, UseFileUploadLogicReturn } from './types' @@ -38,7 +39,7 @@ export const useFileUploadLogic = ({ attachFiles({ variables: { - input: { fileRelayIds: fresh, targetObjectId }, + input: { fileRelayIds: fresh, targetObjectId, uploadTokens: uploadTokensFor(fresh) }, connections: [connectionID], }, onCompleted: () => { @@ -93,7 +94,9 @@ export const useFileUploadLogic = ({ .map((result) => (result as PromiseFulfilledResult).value) if (fileRelayIds.length) { - onUploadComplete?.(fileRelayIds) + // Read before the store drops the files: with autoAttach off, the caller + // needs the tokens to attach anonymous uploads itself. + onUploadComplete?.(fileRelayIds, uploadTokensFor(fileRelayIds)) } if (autoAttach) { diff --git a/packages/components/modules/files/common/hooks/useFileUploadLogic/types.ts b/packages/components/modules/files/common/hooks/useFileUploadLogic/types.ts index deaaa6fc4..44916335e 100644 --- a/packages/components/modules/files/common/hooks/useFileUploadLogic/types.ts +++ b/packages/components/modules/files/common/hooks/useFileUploadLogic/types.ts @@ -1,7 +1,8 @@ export interface UseFileUploadLogicParams { targetObjectId?: string autoAttach?: boolean - onUploadComplete?: (fileRelayIds: string[]) => void + /** `uploadTokens`: tokens of anonymous uploads among them, needed to attach them later (`autoAttach: false`). */ + onUploadComplete?: (fileRelayIds: string[], uploadTokens: string[]) => void onAttachComplete?: () => void onError?: (error: Error) => void } diff --git a/packages/components/modules/files/common/types.ts b/packages/components/modules/files/common/types.ts index 2fd5b2533..4b4634a20 100644 --- a/packages/components/modules/files/common/types.ts +++ b/packages/components/modules/files/common/types.ts @@ -26,6 +26,7 @@ export interface FileUploadProgress { uploadId?: string // Backend upload ID backendId?: string // Backend file id (public_id) for upload endpoints fileRelayId?: string // GraphQL relay ID after completion + uploadToken?: string // Anonymous uploads only: proves ownership on complete, abort and attach etags: (string | undefined)[] // ETags from S3, sparse-indexed by chunk presignedUrls?: PresignedUrl[] initiatedAt?: number // Epoch ms when the upload was initiated @@ -40,6 +41,8 @@ export interface InitiateUploadResponse { uploadStatus: string expiresIn: number presignedUrls: PresignedUrl[] + /** Only for anonymous uploads. */ + uploadToken?: string } export interface CompleteUploadPart { diff --git a/packages/components/modules/files/common/utils/__tests__/formatters.test.ts b/packages/components/modules/files/common/utils/__tests__/formatters.test.ts new file mode 100644 index 000000000..dd1b48956 --- /dev/null +++ b/packages/components/modules/files/common/utils/__tests__/formatters.test.ts @@ -0,0 +1,9 @@ +import { formatFileSize } from '../formatters' + +describe('formatFileSize', () => { + it('scales from bytes up to gigabytes', () => { + expect(formatFileSize(512)).toBe('512 B') + expect(formatFileSize(150 * 1024 * 1024)).toBe('150.00 MB') + expect(formatFileSize(5 * 1024 * 1024 * 1024)).toBe('5.00 GB') + }) +}) diff --git a/packages/components/modules/files/common/utils/__tests__/uploadChunk.test.ts b/packages/components/modules/files/common/utils/__tests__/uploadChunk.test.ts new file mode 100644 index 000000000..05393b126 --- /dev/null +++ b/packages/components/modules/files/common/utils/__tests__/uploadChunk.test.ts @@ -0,0 +1,88 @@ +import { vi } from 'vitest' + +import { type ChunkUploadError, uploadChunk } from '../uploadChunk' + +type Listener = () => void + +class FakeXhr { + static respond: (xhr: FakeXhr) => void + + status = 0 + + statusText = '' + + responseText = '' + + headers: Record = {} + + upload = { addEventListener: vi.fn() } + + private listeners: Record = {} + + addEventListener(event: string, listener: Listener) { + this.listeners[event] = listener + } + + getResponseHeader(name: string) { + return this.headers[name] ?? null + } + + open = vi.fn() + + setRequestHeader = vi.fn() + + abort = vi.fn() + + send() { + FakeXhr.respond(this) + this.listeners.load?.() + } +} + +describe('uploadChunk', () => { + beforeEach(() => { + vi.stubGlobal('XMLHttpRequest', FakeXhr) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('resolves with the ETag of a stored part', async () => { + FakeXhr.respond = (xhr) => { + xhr.status = 200 + xhr.headers.ETag = '"etag-1"' + } + + await expect(uploadChunk(new Blob(['x']), 'https://s3/part-0')).resolves.toBe('etag-1') + }) + + it('keeps the JSON body of a rejected part so its reason can be shown', async () => { + FakeXhr.respond = (xhr) => { + xhr.status = 400 + xhr.statusText = 'Bad Request' + xhr.responseText = JSON.stringify({ detail: 'Upload has expired.' }) + } + + const error = (await uploadChunk(new Blob(['x']), 'https://s3/part-0').catch( + (e) => e, + )) as ChunkUploadError + + expect(error.message).toBe('Chunk upload failed: 400 Bad Request') + expect(error.response).toEqual({ status: 400, data: { detail: 'Upload has expired.' } }) + }) + + it('leaves data empty when the body is not JSON', async () => { + FakeXhr.respond = (xhr) => { + xhr.status = 403 + xhr.statusText = 'Forbidden' + xhr.responseText = 'AccessDenied' + } + + const error = (await uploadChunk(new Blob(['x']), 'https://s3/part-0').catch( + (e) => e, + )) as ChunkUploadError + + expect(error.response).toEqual({ status: 403, data: undefined }) + }) +}) diff --git a/packages/components/modules/files/common/utils/__tests__/uploadChunks.test.ts b/packages/components/modules/files/common/utils/__tests__/uploadChunks.test.ts index 003367b8d..d1f5e50ae 100644 --- a/packages/components/modules/files/common/utils/__tests__/uploadChunks.test.ts +++ b/packages/components/modules/files/common/utils/__tests__/uploadChunks.test.ts @@ -89,6 +89,20 @@ describe('uploadChunks', () => { expect(mockUploadChunk).toHaveBeenCalledTimes(3) }) + it('keeps the response of the last failed attempt', async () => { + const response = { status: 400, data: { detail: 'Upload has expired.' } } + mockUploadChunk.mockRejectedValue(Object.assign(new Error('rejected'), { response })) + + await expect( + uploadChunks({ + chunks: makeChunks(1), + presignedUrls: makeUrls(1), + retryAttempts: 1, + retryDelay: 1, + }), + ).rejects.toMatchObject({ response }) + }) + it('does not retry when the upload was aborted', async () => { const abortController = new AbortController() mockUploadChunk.mockImplementation(() => { diff --git a/packages/components/modules/files/common/utils/__tests__/uploadTokens.test.ts b/packages/components/modules/files/common/utils/__tests__/uploadTokens.test.ts new file mode 100644 index 000000000..a0414917f --- /dev/null +++ b/packages/components/modules/files/common/utils/__tests__/uploadTokens.test.ts @@ -0,0 +1,29 @@ +import { FileUploadStatus } from '../../constants' +import { useFileUploadStore } from '../../context/FileUploadProvider' +import { uploadTokensFor } from '../uploadTokens' + +const addUpload = (fileRelayId: string, uploadToken?: string) => { + const { addFile, updateFileProgress } = useFileUploadStore.getState() + const id = addFile(new File(['x'], `${fileRelayId}.pdf`)) + updateFileProgress(id, { status: FileUploadStatus.COMPLETED, fileRelayId, uploadToken }) +} + +describe('uploadTokensFor', () => { + beforeEach(() => { + useFileUploadStore.setState({ files: new Map() }) + }) + + it('returns the tokens of the requested anonymous uploads only', () => { + addUpload('File:1', 'token-1') + addUpload('File:2', 'token-2') + addUpload('File:3') + + expect(uploadTokensFor(['File:1', 'File:3'])).toEqual(['token-1']) + }) + + it('is empty for logged-in uploads', () => { + addUpload('File:1') + + expect(uploadTokensFor(['File:1'])).toEqual([]) + }) +}) diff --git a/packages/components/modules/files/common/utils/formatters.ts b/packages/components/modules/files/common/utils/formatters.ts index 83a9f3bdf..4799b14ce 100644 --- a/packages/components/modules/files/common/utils/formatters.ts +++ b/packages/components/modules/files/common/utils/formatters.ts @@ -5,7 +5,8 @@ export const formatFileSize = (bytes: number | null | undefined): string => { if (!bytes) return '0 B' if (bytes < 1024) return `${bytes} B` if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(2)} KB` - return `${(bytes / (1024 * 1024)).toFixed(2)} MB` + if (bytes < 1024 * 1024 * 1024) return `${(bytes / (1024 * 1024)).toFixed(2)} MB` + return `${(bytes / (1024 * 1024 * 1024)).toFixed(2)} GB` } /** diff --git a/packages/components/modules/files/common/utils/uploadChunk.ts b/packages/components/modules/files/common/utils/uploadChunk.ts index af9131529..6d1f707fb 100644 --- a/packages/components/modules/files/common/utils/uploadChunk.ts +++ b/packages/components/modules/files/common/utils/uploadChunk.ts @@ -1,3 +1,14 @@ +export type ChunkUploadError = Error & { response?: { status: number; data?: unknown } } + +const parseJsonBody = (text: string): unknown => { + try { + const data = JSON.parse(text) + return data && typeof data === 'object' ? data : undefined + } catch { + return undefined + } +} + export async function uploadChunk( chunk: Blob, presignedUrl: string, @@ -26,7 +37,12 @@ export async function uploadChunk( resolve(etag) } } else { - reject(new Error(`Chunk upload failed: ${xhr.status} ${xhr.statusText}`)) + // Keep the body so the backend's reason reaches the user (getApiErrorMessage). + const error: ChunkUploadError = new Error( + `Chunk upload failed: ${xhr.status} ${xhr.statusText}`, + ) + error.response = { status: xhr.status, data: parseJsonBody(xhr.responseText) } + reject(error) } }) diff --git a/packages/components/modules/files/common/utils/uploadChunks.ts b/packages/components/modules/files/common/utils/uploadChunks.ts index 6f84de0ba..78e150903 100644 --- a/packages/components/modules/files/common/utils/uploadChunks.ts +++ b/packages/components/modules/files/common/utils/uploadChunks.ts @@ -1,5 +1,5 @@ import { MAX_CONCURRENT_CHUNKS, RETRY_ATTEMPTS, RETRY_DELAY } from '../constants' -import { uploadChunk } from './uploadChunk' +import { type ChunkUploadError, uploadChunk } from './uploadChunk' interface UploadChunksOptions { chunks: Blob[] @@ -55,9 +55,11 @@ export async function uploadChunks({ return uploadChunkWithRetry(chunkIndex, chunk, url, attempt + 1) } - throw new Error( + const failure: ChunkUploadError = new Error( `Failed to upload chunk ${chunkIndex} after ${retryAttempts} attempts: ${error}`, ) + failure.response = (error as ChunkUploadError)?.response + throw failure } } diff --git a/packages/components/modules/files/common/utils/uploadTokens.ts b/packages/components/modules/files/common/utils/uploadTokens.ts new file mode 100644 index 000000000..2969e71f5 --- /dev/null +++ b/packages/components/modules/files/common/utils/uploadTokens.ts @@ -0,0 +1,9 @@ +import { useFileUploadStore } from '../context/FileUploadProvider' + +/** Upload tokens of the given files, for attaching anonymous uploads. Empty when logged in. */ +export const uploadTokensFor = (fileRelayIds: string[]): string[] => + Array.from(useFileUploadStore.getState().files.values()).flatMap((file) => + file.uploadToken && file.fileRelayId && fileRelayIds.includes(file.fileRelayId) + ? [file.uploadToken] + : [], + ) diff --git a/packages/components/modules/files/web/AttachedFileItem/index.tsx b/packages/components/modules/files/web/AttachedFileItem/index.tsx index 745f76f37..bdb91bdc1 100644 --- a/packages/components/modules/files/web/AttachedFileItem/index.tsx +++ b/packages/components/modules/files/web/AttachedFileItem/index.tsx @@ -93,6 +93,7 @@ const AttachedFileItem: FC = ({ handleDownload(file.url)} + disabled={!file.url} title="Download" aria-label="Download" > @@ -145,7 +146,12 @@ const AttachedFileItem: FC = ({ - handleDownload(file.url)} title="Download"> + handleDownload(file.url)} + disabled={!file.url} + title="Download" + > diff --git a/packages/components/modules/files/web/FileUpload/types.ts b/packages/components/modules/files/web/FileUpload/types.ts index 5f643d6ff..560bf108a 100644 --- a/packages/components/modules/files/web/FileUpload/types.ts +++ b/packages/components/modules/files/web/FileUpload/types.ts @@ -9,7 +9,8 @@ export interface FileUploadProps { acceptedFileTypes?: Accept disabled?: boolean autoAttach?: boolean - onUploadComplete?: (fileIds: string[]) => void + /** `uploadTokens`: tokens of anonymous uploads among them, needed to attach them later (`autoAttach: false`). */ + onUploadComplete?: (fileIds: string[], uploadTokens: string[]) => void onAttachComplete?: () => void onError?: (error: Error) => void } diff --git a/packages/components/modules/files/web/FileUploadDropzone/index.tsx b/packages/components/modules/files/web/FileUploadDropzone/index.tsx index f995179c9..9c399ca29 100644 --- a/packages/components/modules/files/web/FileUploadDropzone/index.tsx +++ b/packages/components/modules/files/web/FileUploadDropzone/index.tsx @@ -5,12 +5,13 @@ import { useCallback } from 'react' import { Dropzone } from '@baseapp-frontend/design-system/components/web/dropzones' +import { formatFileSize } from '../../common/utils/formatters' import type { FileUploadDropzoneProps } from './types' const FileUploadDropzone: FC = ({ onFilesSelected, maxFiles = 10, - maxFileSize = 100 * 1024 * 1024, // 100MB default + maxFileSize, acceptedFileTypes, disabled, }) => { @@ -28,10 +29,15 @@ const FileUploadDropzone: FC = ({ onSelect={handleSelect} onRemove={() => {}} multiple={maxFiles > 1} - maxFileSize={maxFileSize / (1024 * 1024)} // Convert bytes to MB + // Dropzone takes MB; unlimited when omitted. + maxFileSize={maxFileSize !== undefined ? maxFileSize / (1024 * 1024) : Infinity} asBase64={false} // Keep as File objects for chunked upload title="Upload Files" - subTitle={`Max ${maxFiles} files, ${Math.round(maxFileSize / (1024 * 1024))}MB each`} + subTitle={ + maxFileSize !== undefined + ? `Max ${maxFiles} files, ${formatFileSize(maxFileSize)} each` + : `Max ${maxFiles} files` + } includeActionButton={false} DropzoneOptions={{ disabled, diff --git a/packages/components/modules/files/web/FileUploadDropzone/types.ts b/packages/components/modules/files/web/FileUploadDropzone/types.ts index fe395603d..7ffb0b430 100644 --- a/packages/components/modules/files/web/FileUploadDropzone/types.ts +++ b/packages/components/modules/files/web/FileUploadDropzone/types.ts @@ -3,7 +3,7 @@ type Accept = Record export interface FileUploadDropzoneProps { onFilesSelected: (files: File[]) => void maxFiles?: number - maxFileSize?: number // in bytes + maxFileSize?: number // in bytes; unlimited when omitted acceptedFileTypes?: Accept disabled?: boolean } diff --git a/packages/components/modules/files/web/FileUploadTrigger/types.ts b/packages/components/modules/files/web/FileUploadTrigger/types.ts index 4360893d2..fe8238dde 100644 --- a/packages/components/modules/files/web/FileUploadTrigger/types.ts +++ b/packages/components/modules/files/web/FileUploadTrigger/types.ts @@ -17,7 +17,8 @@ export interface FileUploadTriggerProps { acceptedFileTypes?: Accept disabled?: boolean autoAttach?: boolean - onUploadComplete?: (fileIds: string[]) => void + /** `uploadTokens`: tokens of anonymous uploads among them, needed to attach them later (`autoAttach: false`). */ + onUploadComplete?: (fileIds: string[], uploadTokens: string[]) => void onAttachComplete?: () => void onError?: (error: Error) => void } diff --git a/packages/components/package.json b/packages/components/package.json index bdda0927c..23bf91c7f 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -1,7 +1,7 @@ { "name": "@baseapp-frontend/components", "description": "BaseApp components modules such as comments, notifications, messages, and more.", - "version": "3.0.0", + "version": "3.0.1", "sideEffects": false, "scripts": { "build": "rm -rf dist && pnpm relay && tsc --build tsconfig.build.json", diff --git a/packages/components/schema.graphql b/packages/components/schema.graphql index 8bf79fe0f..5adab35eb 100644 --- a/packages/components/schema.graphql +++ b/packages/components/schema.graphql @@ -43,6 +43,13 @@ interface ActivityLogWithProfileInterface { profile: Profile } +""" +The `BigInt` scalar type represents non-fractional whole numeric values. +`BigInt` is not constrained to 32-bit like the `Int` type and thus is a less +compatible type. +""" +scalar BigInt + type Block implements Node { """The ID of the object""" id: ID! @@ -760,7 +767,7 @@ type File implements Node & PermissionsInterface & FileInterface & CommentsInter fileName: String! """File size in bytes""" - fileSize: Int + fileSize: BigInt """The stored file""" file: String! @@ -818,6 +825,9 @@ type File implements Node & PermissionsInterface & FileInterface & CommentsInter input FileAttachToTargetInput { fileRelayIds: [ID]! targetObjectId: ID! + + """Upload tokens of anonymous uploads, proving the caller started them.""" + uploadTokens: [String!] clientMutationId: String } diff --git a/packages/config/CHANGELOG.md b/packages/config/CHANGELOG.md index d157d175f..97078b1f3 100644 --- a/packages/config/CHANGELOG.md +++ b/packages/config/CHANGELOG.md @@ -1,5 +1,12 @@ # @baseapp-frontend/config +## 2.1.16 + +### Patch Changes + +- 5ee95e2: `File.fileSize` is a `BigInt` in the GraphQL schema, so files over 2 GB report their size. The + shared Relay config maps `BigInt` to `number`, keeping `fileSize` typed as a number. + ## 2.1.15 ### Patch Changes diff --git a/packages/config/package.json b/packages/config/package.json index 59f9bc8ce..1aca29f14 100644 --- a/packages/config/package.json +++ b/packages/config/package.json @@ -1,7 +1,7 @@ { "name": "@baseapp-frontend/config", "description": "Reusable configurations for eslint, prettier, jest and relay", - "version": "2.1.15", + "version": "2.1.16", "files": [ ".eslintrc.js", ".eslintrc-with-restricted-paths.js", diff --git a/packages/config/relay.config.ts b/packages/config/relay.config.ts index 7a4d24c4d..d7653643c 100644 --- a/packages/config/relay.config.ts +++ b/packages/config/relay.config.ts @@ -13,4 +13,7 @@ module.exports = { ], language: 'typescript', artifactDirectory: './__generated__', + customScalarTypes: { + BigInt: 'number', + }, }