fix(comments): allow any backend-allowed file type and size on comment attachments

.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.

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

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<HTMLInputElement, CommentCreateProps>(
           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.

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<CommentFilesUpsertActionsProps> = ({ target
       as="button"
       icon={<AttachmentIcon />}
       maxFiles={MAX_FILES}
-      maxFileSize={MAX_FILE_SIZE}
-      acceptedFileTypes={ACCEPTED_FILE_TYPES}
       autoAttach
     />
     <IconButton disabled aria-label="mention">

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
 

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<typeof import('@baseapp-frontend/utils')>('@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', '<html><body>502 Bad Gateway</body></html>', '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)
   })