diff --git a/apps/frontend/src/app/components/AddExpenseModal.tsx b/apps/frontend/src/app/components/AddExpenseModal.tsx index 15dc707..4888b90 100644 --- a/apps/frontend/src/app/components/AddExpenseModal.tsx +++ b/apps/frontend/src/app/components/AddExpenseModal.tsx @@ -4,9 +4,12 @@ import { useState } from 'react'; import { Button, Dialog, Portal, CloseButton, Stack } from '@chakra-ui/react'; import DropdownSelector from './DropdownSelector'; import { useApi } from '@/hooks/useApi'; -import FileUpload from './FileUpload'; +import FileUpload, { UploadProgress } from './FileUpload'; +import { putWithProgress } from '@/lib/upload'; import { FiDollarSign } from 'react-icons/fi'; import { Project } from '@/types'; + +const RECEIPT_CONTENT_TYPE = 'application/pdf'; interface AddExpenseModalProps { open: boolean; onClose: () => void; @@ -38,6 +41,8 @@ export default function AddExpenseModal({ const [projectError, setProjectError] = useState(false); const [submitError, setSubmitError] = useState(null); const [fileError, setFileError] = useState(null); + const [uploadProgress, setUploadProgress] = useState(null); + const [isSubmitting, setIsSubmitting] = useState(false); function resetForm() { setNewDate(''); @@ -45,6 +50,9 @@ export default function AddExpenseModal({ setNewDescription(''); setNewAmount(''); setNewProject(''); + setNewFile(null); + setUploadProgress(null); + setIsSubmitting(false); setDateError(false); setTypeError(false); setDescError(false); @@ -83,19 +91,38 @@ export default function AddExpenseModal({ return; } + setSubmitError(null); + setIsSubmitting(true); try { + // The receipt goes to S3 first: POST /expenditures stores the resulting + // object URL, so there is nothing to record until the file has landed. + const { uploadUrl, objectUrl } = await api.get<{ uploadUrl: string; objectUrl: string }>( + `/expenditures/upload-url?fileName=${encodeURIComponent(newFile.name)}` + + `&projectId=${selectedProject.project_id}`, + ); + + setUploadProgress({ transferredBytes: 0, totalBytes: newFile.size, fileName: newFile.name }); + await putWithProgress(uploadUrl, newFile, RECEIPT_CONTENT_TYPE, (transferredBytes) => + setUploadProgress({ transferredBytes, totalBytes: newFile.size, fileName: newFile.name }), + ); + setUploadProgress(null); + await api.post('/expenditures', { projectID: selectedProject.project_id, amount: Number(newAmount), category: newType, description: newDescription, spentOn: newDate, + receiptUrl: objectUrl, }); resetForm(); onSuccess(); } catch (err) { + setUploadProgress(null); setSubmitError(err instanceof Error ? err.message : 'Failed to create expense'); + } finally { + setIsSubmitting(false); } } @@ -304,6 +331,7 @@ export default function AddExpenseModal({ setFileError(null); }} onReject={() => setFileError('File type not supported')} + progress={uploadProgress} /> {fileError && ( @@ -332,6 +360,8 @@ export default function AddExpenseModal({ backgroundColor="var(--color-core-green)" color="var(--color-core-white)" onClick={handleSubmit} + loading={isSubmitting} + disabled={isSubmitting} > Submit For Review diff --git a/apps/frontend/src/app/components/FileUpload.tsx b/apps/frontend/src/app/components/FileUpload.tsx index d83390d..32719f3 100644 --- a/apps/frontend/src/app/components/FileUpload.tsx +++ b/apps/frontend/src/app/components/FileUpload.tsx @@ -1,51 +1,32 @@ 'use client'; -import { useCallback, useRef, useState } from 'react'; +import { useCallback } from 'react'; import { FileRejection, useDropzone } from 'react-dropzone'; import UploadProgressBar from './UploadProgressBar'; import FilePreview from './FilePreview'; const MAX_FILE_SIZE_BYTES = 10 * 1024 * 1024; // 10MB +export interface UploadProgress { + transferredBytes: number; + totalBytes: number; + fileName: string; +} + interface FileUploadProps { value: File | null; onChange: (file: File | null) => void; onReject?: () => void; + /** + * Live progress of the real upload, driven by the parent. The file is sent + * when the form is submitted -- not on drop -- because the presigned URL is + * per project, and because a file dropped into an abandoned form would + * otherwise leave an orphaned object in the bucket. + */ + progress?: UploadProgress | null; } -export default function FileUpload({ value, onChange, onReject }: FileUploadProps) { - const [isUploading, setIsUploading] = useState(false); - const [transferredBytes, setTransferredBytes] = useState(0); - const [pendingFile, setPendingFile] = useState(null); - const intervalRef = useRef | null>(null); - - {/*Simulated progress for testing - TODO: update to use real progress of uploaded file */} - const simulateUpload = useCallback( - (file: File) => { - setPendingFile(file); - setIsUploading(true); - setTransferredBytes(0); - - const total = file.size; - const step = total / 15; // ~15 ticks to finish - let transferred = 0; - - intervalRef.current = setInterval(() => { - transferred += step; - if (transferred >= total) { - transferred = total; - if (intervalRef.current) clearInterval(intervalRef.current); - setTransferredBytes(total); - setIsUploading(false); - setPendingFile(null); - onChange(file); // flip to selected state - } else { - setTransferredBytes(transferred); - } - }, 100); - }, - [onChange], - ); +export default function FileUpload({ value, onChange, onReject, progress }: FileUploadProps) { + const isUploading = progress !== null && progress !== undefined; {/*When the file is dropped, check if it's accepted*/} const onDrop = useCallback( @@ -54,9 +35,9 @@ export default function FileUpload({ value, onChange, onReject }: FileUploadProp onReject?.(); return; } - if (accepted[0]) simulateUpload(accepted[0]); + if (accepted[0]) onChange(accepted[0]); }, - [simulateUpload, onReject], + [onChange, onReject], ); {/*Dropzone component to allow user to drop in files*/} @@ -73,12 +54,12 @@ export default function FileUpload({ value, onChange, onReject }: FileUploadProp ? 'var(--color-core-green)' : 'var(--color-black-200)'; - if (isUploading && pendingFile) { + if (isUploading) { return ( ); } diff --git a/apps/frontend/src/lib/upload.ts b/apps/frontend/src/lib/upload.ts new file mode 100644 index 0000000..34efa3f --- /dev/null +++ b/apps/frontend/src/lib/upload.ts @@ -0,0 +1,33 @@ +/** + * Uploads a file to a presigned S3 PUT, reporting real byte progress. + * + * XHR rather than `fetch`, which exposes no upload progress. The request is + * deliberately unauthenticated: the signature is in the URL, and an extra + * Authorization header would not be part of what was signed. + */ +export function putWithProgress( + uploadUrl: string, + file: File, + contentType: string, + onProgress: (transferredBytes: number) => void, +): Promise { + return new Promise((resolve, reject) => { + const xhr = new XMLHttpRequest(); + xhr.open('PUT', uploadUrl); + // Must match the ContentType the URL was signed with, or S3 rejects the + // signature -- so it comes from the caller, not from `file.type`. + xhr.setRequestHeader('Content-Type', contentType); + + xhr.upload.addEventListener('progress', (event) => { + if (event.lengthComputable) onProgress(event.loaded); + }); + xhr.addEventListener('load', () => { + if (xhr.status >= 200 && xhr.status < 300) resolve(); + else reject(new Error(`Upload failed (${xhr.status})`)); + }); + xhr.addEventListener('error', () => reject(new Error('Upload failed'))); + xhr.addEventListener('abort', () => reject(new Error('Upload cancelled'))); + + xhr.send(file); + }); +} diff --git a/apps/frontend/test/components/AddExpenseModal.test.tsx b/apps/frontend/test/components/AddExpenseModal.test.tsx index bfb55d8..65104e1 100644 --- a/apps/frontend/test/components/AddExpenseModal.test.tsx +++ b/apps/frontend/test/components/AddExpenseModal.test.tsx @@ -1,12 +1,33 @@ import { render, screen, fireEvent, waitFor } from '../utils'; import AddExpenseModal from '@/app/components/AddExpenseModal'; import { authedFetch as apiFetch } from '@/lib/authClient'; +import { putWithProgress } from '@/lib/upload'; jest.mock('../../src/lib/authClient', () => ({ ...jest.requireActual('../../src/lib/authClient'), authedFetch: jest.fn(), })); +jest.mock('../../src/lib/upload', () => ({ + putWithProgress: jest.fn(), +})); + +const mockPut = putWithProgress as jest.Mock; + +const UPLOAD_URL = '/expenditures/upload-url?fileName=receipt.pdf&projectId=2'; +const OBJECT_URL = 'https://bucket.s3.us-east-2.amazonaws.com/receipts/2/1-receipt.pdf'; + +/** GET /expenditures/upload-url resolves, POST /expenditures resolves. */ +function mockHappyPath() { + (apiFetch as jest.Mock).mockImplementation((path: string) => { + if (path.startsWith('/expenditures/upload-url')) { + return Promise.resolve({ uploadUrl: 'https://signed.example/put', objectUrl: OBJECT_URL }); + } + return Promise.resolve({}); + }); + mockPut.mockResolvedValue(undefined); +} + // Mock DropdownSelector — render a simple native select so we can drive it jest.mock('../../src/app/components/DropdownSelector', () => { return function MockDropdownSelector({ @@ -72,6 +93,26 @@ beforeEach(() => { jest.clearAllMocks(); }); +/** Fills every required field with a valid value, including the receipt. */ +function fillValidForm() { + fireEvent.change(document.querySelector('input[type="date"]')!, { + target: { value: '2025-05-15' }, + }); + fireEvent.change(screen.getByPlaceholderText('Enter the Amount'), { + target: { value: '12000' }, + }); + fireEvent.change(screen.getByLabelText('Select type'), { + target: { value: 'Travel Foreign' }, + }); + fireEvent.change(screen.getByLabelText('Select a project'), { + target: { value: 'Project Name 2' }, + }); + fireEvent.change(screen.getByPlaceholderText('Placeholder'), { + target: { value: 'A test description' }, + }); + fireEvent.click(screen.getByText('mock-select-file')); +} + describe('AddExpenseModal Component', () => { it('renders the modal title when open', () => { render(); @@ -121,27 +162,10 @@ describe('AddExpenseModal Component', () => { expect(apiFetch).not.toHaveBeenCalled(); }); - it('calls apiFetch with the correct payload when the form is valid', async () => { - (apiFetch as jest.Mock).mockResolvedValueOnce({}); + it('uploads the receipt and posts its object URL as receiptUrl', async () => { + mockHappyPath(); render(); - - fireEvent.change(document.querySelector('input[type="date"]')!, { - target: { value: '2025-05-15' }, - }); - fireEvent.change(screen.getByPlaceholderText('Enter the Amount'), { - target: { value: '12000' }, - }); - fireEvent.change(screen.getByLabelText('Select type'), { - target: { value: 'Travel Foreign' }, - }); - fireEvent.change(screen.getByLabelText('Select a project'), { - target: { value: 'Project Name 2' }, - }); - fireEvent.change(screen.getByPlaceholderText('Placeholder'), { - target: { value: 'A test description' }, - }); - fireEvent.click(screen.getByText('mock-select-file')); - + fillValidForm(); fireEvent.click(screen.getByText('Submit For Review')); await waitFor(() => { @@ -155,32 +179,56 @@ describe('AddExpenseModal Component', () => { category: 'Travel Foreign', description: 'A test description', spentOn: '2025-05-15', + receiptUrl: OBJECT_URL, }), }), ); }); + + // The URL is requested for the chosen project and the dropped filename. + expect(apiFetch).toHaveBeenCalledWith(UPLOAD_URL, { method: 'GET' }); + + // The PUT must be signed for the same content type the backend signed. + expect(mockPut).toHaveBeenCalledWith( + 'https://signed.example/put', + expect.any(File), + 'application/pdf', + expect.any(Function), + ); }); - it('calls onSuccess after a successful submit', async () => { - (apiFetch as jest.Mock).mockResolvedValueOnce({}); + it('uploads the file before recording the expenditure', async () => { + mockHappyPath(); render(); + fillValidForm(); + fireEvent.click(screen.getByText('Submit For Review')); - fireEvent.change(document.querySelector('input[type="date"]')!, { - target: { value: '2025-05-15' }, - }); - fireEvent.change(screen.getByPlaceholderText('Enter the Amount'), { - target: { value: '12000' }, - }); - fireEvent.change(screen.getByLabelText('Select type'), { - target: { value: 'Travel Foreign' }, - }); - fireEvent.change(screen.getByLabelText('Select a project'), { - target: { value: 'Project Name 2' }, - }); - fireEvent.change(screen.getByPlaceholderText('Placeholder'), { - target: { value: 'A test description' }, + await waitFor(() => expect(baseProps.onSuccess).toHaveBeenCalled()); + + const postCall = (apiFetch as jest.Mock).mock.invocationCallOrder[ + (apiFetch as jest.Mock).mock.calls.findIndex(([path]) => path === '/expenditures') + ]; + expect(mockPut.mock.invocationCallOrder[0]).toBeLessThan(postCall); + }); + + it('does not record an expenditure when the receipt upload fails', async () => { + mockHappyPath(); + mockPut.mockRejectedValueOnce(new Error('Upload failed (403)')); + render(); + fillValidForm(); + fireEvent.click(screen.getByText('Submit For Review')); + + await waitFor(() => { + expect(screen.getByText('Upload failed (403)')).toBeInTheDocument(); }); - fireEvent.click(screen.getByText('mock-select-file')); + expect(apiFetch).not.toHaveBeenCalledWith('/expenditures', expect.anything()); + expect(baseProps.onSuccess).not.toHaveBeenCalled(); + }); + + it('calls onSuccess after a successful submit', async () => { + mockHappyPath(); + render(); + fillValidForm(); fireEvent.click(screen.getByText('Submit For Review')); await waitFor(() => { @@ -189,25 +237,9 @@ describe('AddExpenseModal Component', () => { }); it('shows a submit error if apiFetch rejects', async () => { - (apiFetch as jest.Mock).mockRejectedValueOnce(new Error('Server exploded')); + (apiFetch as jest.Mock).mockRejectedValue(new Error('Server exploded')); render(); - - fireEvent.change(document.querySelector('input[type="date"]')!, { - target: { value: '2025-05-15' }, - }); - fireEvent.change(screen.getByPlaceholderText('Enter the Amount'), { - target: { value: '12000' }, - }); - fireEvent.change(screen.getByLabelText('Select type'), { - target: { value: 'Travel Foreign' }, - }); - fireEvent.change(screen.getByLabelText('Select a project'), { - target: { value: 'Project Name 2' }, - }); - fireEvent.change(screen.getByPlaceholderText('Placeholder'), { - target: { value: 'A test description' }, - }); - fireEvent.click(screen.getByText('mock-select-file')); + fillValidForm(); fireEvent.click(screen.getByText('Submit For Review')); await waitFor(() => { diff --git a/apps/frontend/test/components/FileUpload.test.tsx b/apps/frontend/test/components/FileUpload.test.tsx index 4ddc803..82e8b81 100644 --- a/apps/frontend/test/components/FileUpload.test.tsx +++ b/apps/frontend/test/components/FileUpload.test.tsx @@ -41,32 +41,52 @@ describe('FileUpload Component', () => { expect(screen.getByText('Upload Complete')).toBeInTheDocument(); }); - it('shows the upload progress bar after a valid file is dropped', () => { - jest.useFakeTimers(); - render( {}} />); + it('calls onChange as soon as a valid file is dropped', () => { + const onChange = jest.fn(); + render(); act(() => { capturedOnDrop!([makePdf()], []); }); - expect(screen.getByText(/uploading\.\.\./)).toBeInTheDocument(); - jest.useRealTimers(); + expect(onChange.mock.calls[0][0].name).toBe('receipt.pdf'); }); - it('calls onChange with the file once the simulated upload completes', () => { - jest.useFakeTimers(); - const onChange = jest.fn(); - render(); + it('shows no progress bar on drop — the upload happens on submit', () => { + render( {}} />); act(() => { capturedOnDrop!([makePdf()], []); }); - act(() => { - jest.advanceTimersByTime(2000); - }); - expect(onChange.mock.calls[0][0].name).toBe('receipt.pdf'); - jest.useRealTimers(); + expect(screen.queryByText(/uploading\.\.\./)).not.toBeInTheDocument(); + }); + + it('renders the real transferred bytes when the parent reports progress', () => { + render( + {}} + progress={{ transferredBytes: 512, totalBytes: 2048, fileName: 'receipt.pdf' }} + />, + ); + + expect(screen.getByText(/receipt\.pdf uploading\.\.\./)).toBeInTheDocument(); + expect(screen.getByText('25%')).toBeInTheDocument(); + }); + + it('returns to the preview once progress clears', () => { + const { rerender } = render( + {}} + progress={{ transferredBytes: 2048, totalBytes: 2048, fileName: 'receipt.pdf' }} + />, + ); + expect(screen.getByText('100%')).toBeInTheDocument(); + + rerender( {}} progress={null} />); + expect(screen.getByText('Upload Complete')).toBeInTheDocument(); }); it('calls onReject when a rejected file is dropped', () => { diff --git a/apps/frontend/test/lib/upload.test.ts b/apps/frontend/test/lib/upload.test.ts new file mode 100644 index 0000000..61d0427 --- /dev/null +++ b/apps/frontend/test/lib/upload.test.ts @@ -0,0 +1,121 @@ +import { putWithProgress } from '@/lib/upload'; + +/** Minimal XMLHttpRequest stand-in; jsdom's does not run in tests. */ +class MockXhr { + static last: MockXhr; + + status = 200; + method = ''; + url = ''; + headers: Record = {}; + body: unknown = null; + + private listeners: Record void>> = {}; + upload = { + listeners: [] as Array<(event: ProgressEvent) => void>, + addEventListener(_type: string, handler: (event: ProgressEvent) => void) { + this.listeners.push(handler); + }, + }; + + constructor() { + MockXhr.last = this; + } + + open(method: string, url: string) { + this.method = method; + this.url = url; + } + setRequestHeader(key: string, value: string) { + this.headers[key] = value; + } + addEventListener(type: string, handler: () => void) { + (this.listeners[type] ??= []).push(handler); + } + send(body: unknown) { + this.body = body; + } + + emit(type: string) { + this.listeners[type]?.forEach((handler) => handler()); + } + emitProgress(loaded: number, lengthComputable = true) { + this.upload.listeners.forEach((handler) => + handler({ loaded, lengthComputable } as ProgressEvent), + ); + } +} + +beforeEach(() => { + (global as unknown as { XMLHttpRequest: unknown }).XMLHttpRequest = MockXhr; +}); + +function pdf() { + return new File(['x'], 'receipt.pdf', { type: 'application/pdf' }); +} + +describe('putWithProgress', () => { + it('PUTs the file to the presigned URL with the signed content type', async () => { + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', () => {}); + MockXhr.last.emit('load'); + await promise; + + expect(MockXhr.last.method).toBe('PUT'); + expect(MockXhr.last.url).toBe('https://signed.example/put'); + expect(MockXhr.last.headers['Content-Type']).toBe('application/pdf'); + expect(MockXhr.last.body).toBeInstanceOf(File); + }); + + it('does not send an Authorization header — the signature is in the URL', async () => { + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', () => {}); + MockXhr.last.emit('load'); + await promise; + + expect(MockXhr.last.headers.Authorization).toBeUndefined(); + }); + + it('reports transferred bytes as they arrive', async () => { + const onProgress = jest.fn(); + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', onProgress); + + MockXhr.last.emitProgress(256); + MockXhr.last.emitProgress(1024); + MockXhr.last.emit('load'); + await promise; + + expect(onProgress.mock.calls).toEqual([[256], [1024]]); + }); + + it('ignores progress events with no computable length', async () => { + const onProgress = jest.fn(); + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', onProgress); + + MockXhr.last.emitProgress(256, false); + MockXhr.last.emit('load'); + await promise; + + expect(onProgress).not.toHaveBeenCalled(); + }); + + it('rejects on a non-2xx response, carrying the status', async () => { + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', () => {}); + MockXhr.last.status = 403; + MockXhr.last.emit('load'); + + await expect(promise).rejects.toThrow('Upload failed (403)'); + }); + + it('rejects on a network error', async () => { + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', () => {}); + MockXhr.last.emit('error'); + + await expect(promise).rejects.toThrow('Upload failed'); + }); + + it('rejects when the upload is aborted', async () => { + const promise = putWithProgress('https://signed.example/put', pdf(), 'application/pdf', () => {}); + MockXhr.last.emit('abort'); + + await expect(promise).rejects.toThrow('Upload cancelled'); + }); +});