Skip to content

Commit 1400127

Browse files
authored
Merge pull request #70 from ModusCreateOrg/ADE-60
[ADE-60] add Cancel upload functionality
2 parents 6e51c34 + ab9e948 commit 1400127

File tree

6 files changed

+223
-67
lines changed

6 files changed

+223
-67
lines changed

frontend/src/common/api/reportService.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,16 @@ export interface UploadProgressCallback {
1414
/**
1515
* Creates an authenticated request config with bearer token
1616
*/
17-
export const getAuthConfig = async (): Promise<{ headers: { Accept: string, 'Content-Type': string, Authorization: string }, onUploadProgress?: (progressEvent: AxiosProgressEvent) => void }> => {
17+
export const getAuthConfig = async (signal?: AbortSignal): Promise<{ headers: { Accept: string, 'Content-Type': string, Authorization: string }, signal?: AbortSignal, onUploadProgress?: (progressEvent: AxiosProgressEvent) => void }> => {
1818
const session = await fetchAuthSession();
1919
const idToken = session.tokens?.idToken?.toString() || '';
2020
return {
2121
headers: {
2222
Accept: 'application/json',
2323
'Content-Type': 'application/json',
2424
Authorization: idToken ? `Bearer ${idToken}` : ''
25-
}
25+
},
26+
signal
2627
};
2728
};
2829

@@ -40,11 +41,13 @@ export class ReportError extends Error {
4041
* Uploads a medical report file
4142
* @param file - The file to upload
4243
* @param onProgress - Optional callback for tracking upload progress
44+
* @param signal - Optional abort signal for canceling the request
4345
* @returns Promise with the created medical report
4446
*/
4547
export const uploadReport = async (
4648
file: File,
47-
onProgress?: UploadProgressCallback
49+
onProgress?: UploadProgressCallback,
50+
signal?: AbortSignal
4851
): Promise<MedicalReport> => {
4952
try {
5053
// Import s3StorageService dynamically to avoid circular dependency
@@ -54,11 +57,12 @@ export const uploadReport = async (
5457
const s3Key = await s3StorageService.uploadFile(
5558
file,
5659
'reports',
57-
onProgress as (progress: number) => void
60+
onProgress as (progress: number) => void,
61+
signal
5862
);
5963

6064
// Then create the report record with the S3 key
61-
const config = await getAuthConfig();
65+
const config = await getAuthConfig(signal);
6266

6367
// Send the report metadata to the API
6468
const response = await axios.post(
@@ -71,6 +75,11 @@ export const uploadReport = async (
7175

7276
return response.data;
7377
} catch (error) {
78+
// If the request was aborted, propagate the abort error
79+
if (signal?.aborted) {
80+
throw new DOMException('The operation was aborted', 'AbortError');
81+
}
82+
7483
if (axios.isAxiosError(error)) {
7584
console.error('API Error Details:', error.response?.data, error.response?.headers);
7685
throw new ReportError(`Failed to upload report: ${error.message}`);

frontend/src/common/components/Upload/UploadModal.scss

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,29 @@
279279
&__cancel-btn {
280280
margin-top: 1rem;
281281
width: 100%;
282-
max-width: 15rem;
282+
max-width: 12rem; // Match the upload button's max-width
283283
--color: white;
284-
--border-color: white;
285-
--background: transparent;
284+
--border-color: rgba(255, 255, 255, 0.6);
285+
--background: rgba(255, 255, 255, 0.1);
286+
--border-radius: 0.5rem;
287+
--border-width: 1px;
288+
--border-style: solid;
289+
height: 2.5rem; // Exactly match the upload button's height (40px)
290+
font-weight: 500;
291+
text-transform: none;
292+
293+
ion-icon {
294+
margin-right: 0.5rem;
295+
font-size: 1.25rem;
296+
}
297+
}
298+
299+
&__bottom-actions {
300+
margin-top: 20px;
301+
width: 100%;
302+
display: flex;
303+
justify-content: center; // Center the button
304+
padding: 0 1rem;
286305
}
287306

288307
&__file-item {
@@ -338,4 +357,4 @@
338357
--background: rgba(0, 0, 0, 0.1);
339358
--progress-background: var(--ion-color-primary);
340359
}
341-
}
360+
}

frontend/src/common/components/Upload/UploadModal.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,10 @@ const UploadModal = ({ isOpen, onClose, onUploadComplete }: UploadModalProps): J
157157
</div>
158158
</div>
159159
)}
160-
161-
{/* Cancel button */}
160+
</div>
161+
162+
{/* Cancel button - updated to match the size of the upload button */}
163+
<div className="upload-modal__bottom-actions">
162164
<IonButton
163165
expand="block"
164166
fill="outline"
@@ -227,4 +229,4 @@ const UploadModal = ({ isOpen, onClose, onUploadComplete }: UploadModalProps): J
227229
);
228230
};
229231

230-
export default UploadModal;
232+
export default UploadModal;

frontend/src/common/hooks/__tests__/useFileUpload.test.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import { uploadReport } from '../../api/reportService';
3535
// Define mocked function types
3636
type MockedValidateFile = { mockReturnValue: (value: { isValid: boolean, errorKey?: string }) => void };
3737
type MockedUploadReport = {
38-
mockImplementation: (fn: (file: File, progressCallback?: UploadProgressCallback) => Promise<MedicalReport>) => void;
38+
mockImplementation: (fn: (file: File, progressCallback?: UploadProgressCallback, signal?: AbortSignal) => Promise<MedicalReport>) => void;
3939
mockRejectedValue: (err: Error) => void;
4040
};
4141
type MockedPermissionCheck = { mockResolvedValue: (value: boolean) => void };
@@ -48,7 +48,7 @@ describe('useFileUpload hook', () => {
4848
vi.resetAllMocks();
4949
// Default mock implementation with safer type casting
5050
(validateFile as unknown as MockedValidateFile).mockReturnValue({ isValid: true });
51-
(uploadReport as unknown as MockedUploadReport).mockImplementation((file: File, progressCallback?: UploadProgressCallback) => {
51+
(uploadReport as unknown as MockedUploadReport).mockImplementation((file: File, progressCallback?: UploadProgressCallback, _signal?: AbortSignal) => {
5252
if (progressCallback) progressCallback(1);
5353
return Promise.resolve(mockReport as MedicalReport);
5454
});
@@ -114,7 +114,7 @@ describe('useFileUpload hook', () => {
114114
});
115115

116116
expect(checkFilePermissions).toHaveBeenCalled();
117-
expect(uploadReport).toHaveBeenCalledWith(mockFile, expect.any(Function));
117+
expect(uploadReport).toHaveBeenCalledWith(mockFile, expect.any(Function), expect.any(AbortSignal));
118118
expect(result.current.status).toBe(UploadStatus.SUCCESS);
119119
expect(result.current.progress).toBe(1);
120120
expect(onUploadCompleteMock).toHaveBeenCalledWith(mockReport);
@@ -140,7 +140,7 @@ describe('useFileUpload hook', () => {
140140
}
141141
});
142142

143-
expect(uploadReport).toHaveBeenCalledWith(mockFile, expect.any(Function));
143+
expect(uploadReport).toHaveBeenCalledWith(mockFile, expect.any(Function), expect.any(AbortSignal));
144144
expect(result.current.status).toBe(UploadStatus.ERROR);
145145
expect(result.current.error).toBe('Upload failed');
146146
});
@@ -186,4 +186,4 @@ describe('useFileUpload hook', () => {
186186
expect(result.current.progress).toBe(0);
187187
expect(result.current.error).toBeNull();
188188
});
189-
});
189+
});

frontend/src/common/hooks/useFileUpload.ts

Lines changed: 94 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ export enum UploadStatus {
1515
REQUESTING_PERMISSION = 'requesting_permission',
1616
UPLOADING = 'uploading',
1717
SUCCESS = 'success',
18-
ERROR = 'error'
18+
ERROR = 'error',
19+
CANCELLED = 'cancelled'
1920
}
2021

2122
interface UseFileUploadOptions {
@@ -45,19 +46,34 @@ export const useFileUpload = ({ onUploadComplete }: UseFileUploadOptions = {}):
4546
const [error, setError] = useState<string | null>(null);
4647
// Use a ref to track if upload should be canceled
4748
const cancelRef = useRef<boolean>(false);
49+
// Use a ref to hold the AbortController
50+
const abortControllerRef = useRef<AbortController | null>(null);
4851

4952
const reset = useCallback(() => {
5053
setFile(null);
5154
setStatus(UploadStatus.IDLE);
5255
setProgress(0);
5356
setError(null);
5457
cancelRef.current = false;
58+
59+
// Abort any pending requests from previous uploads
60+
if (abortControllerRef.current) {
61+
abortControllerRef.current.abort();
62+
abortControllerRef.current = null;
63+
}
5564
}, []);
5665

5766
const cancelUpload = useCallback(() => {
67+
cancelRef.current = true;
68+
69+
// Abort the ongoing request if there's one
70+
if (abortControllerRef.current) {
71+
abortControllerRef.current.abort();
72+
abortControllerRef.current = null;
73+
}
74+
5875
if (status === UploadStatus.UPLOADING || status === UploadStatus.REQUESTING_PERMISSION) {
59-
cancelRef.current = true;
60-
setStatus(UploadStatus.IDLE);
76+
setStatus(UploadStatus.CANCELLED);
6177
setProgress(0);
6278
} else {
6379
reset();
@@ -89,78 +105,117 @@ export const useFileUpload = ({ onUploadComplete }: UseFileUploadOptions = {}):
89105
setError(null);
90106
}, [t]);
91107

108+
// Extract the progress callback outside of uploadFile to reduce complexity
109+
const createProgressCallback = useCallback((signal: AbortSignal): UploadProgressCallback => {
110+
return (progress: number) => {
111+
if (!cancelRef.current && !signal.aborted) {
112+
setProgress(progress);
113+
}
114+
};
115+
}, []);
116+
117+
// Helper to check if the upload has been canceled
118+
const isUploadCanceled = useCallback((signal: AbortSignal): boolean => {
119+
return cancelRef.current || signal.aborted;
120+
}, []);
121+
92122
const uploadFile = useCallback(async () => {
123+
// Don't proceed if there's no file or if the status is CANCELLED
93124
if (!file) {
94125
setError(t('upload.error.noFile'));
95126
return;
96127
}
128+
129+
// If currently in CANCELLED state, we need to reset first
130+
if (status === UploadStatus.CANCELLED) {
131+
reset();
132+
return;
133+
}
97134

98135
// Reset cancel flag
99136
cancelRef.current = false;
137+
138+
// Create a new AbortController for this upload request
139+
abortControllerRef.current = new AbortController();
140+
const { signal } = abortControllerRef.current;
100141

101142
try {
102143
setStatus(UploadStatus.REQUESTING_PERMISSION);
103144

104-
// Check for permissions
105-
let hasPermission = await checkFilePermissions();
145+
// Check and request permissions if needed
146+
const hasPermission = await checkPermissions();
106147

107148
if (!hasPermission) {
108-
// Request permissions
109-
hasPermission = await requestFilePermissions();
110-
111-
if (!hasPermission) {
112-
setStatus(UploadStatus.ERROR);
113-
setError(t('upload.error.permissionDenied'));
114-
return;
115-
}
149+
setStatus(UploadStatus.ERROR);
150+
setError(t('upload.error.permissionDenied'));
151+
return;
116152
}
117153

118154
// Check if canceled during permission check
119-
if (cancelRef.current) {
120-
setStatus(UploadStatus.IDLE);
155+
if (isUploadCanceled(signal)) {
156+
setStatus(UploadStatus.CANCELLED);
121157
return;
122158
}
123159

124160
setStatus(UploadStatus.UPLOADING);
125161
setProgress(0);
126162

127-
// Create a progress callback for the upload
128-
const updateProgress: UploadProgressCallback = (progress) => {
129-
// Only update progress if not canceled
130-
if (!cancelRef.current) {
131-
setProgress(progress);
132-
}
133-
};
163+
// Get a progress callback
164+
const updateProgress = createProgressCallback(signal);
134165

135-
// Upload the file using the API service
136-
const result = await uploadReport(file, updateProgress);
166+
// Upload the file
167+
const result = await uploadReport(file, updateProgress, signal);
137168

138169
// Check if canceled during upload
139-
if (cancelRef.current) {
140-
setStatus(UploadStatus.IDLE);
170+
if (isUploadCanceled(signal)) {
171+
setStatus(UploadStatus.CANCELLED);
141172
return;
142173
}
143174

144-
// Set progress to 100% to indicate completion
175+
// Success
145176
setProgress(1);
146177
setStatus(UploadStatus.SUCCESS);
147178

148-
// Notify parent component if callback provided
149179
if (onUploadComplete) {
150180
onUploadComplete(result);
151181
}
152182
} catch (error) {
153-
// Don't show error if canceled
154-
if (!cancelRef.current) {
155-
setStatus(UploadStatus.ERROR);
156-
setError(
157-
error instanceof Error
158-
? error.message
159-
: t('upload.error.unknown')
160-
);
161-
}
183+
handleUploadError(error as Error, signal);
184+
} finally {
185+
cleanupAbortController(signal);
186+
}
187+
}, [file, status, onUploadComplete, t, createProgressCallback, isUploadCanceled, reset]);
188+
189+
// Helper to handle file permissions
190+
const checkPermissions = useCallback(async (): Promise<boolean> => {
191+
let hasPermission = await checkFilePermissions();
192+
193+
if (!hasPermission) {
194+
hasPermission = await requestFilePermissions();
162195
}
163-
}, [file, onUploadComplete, t]);
196+
197+
return hasPermission;
198+
}, []);
199+
200+
// Helper to handle upload errors
201+
const handleUploadError = useCallback((error: Error, signal: AbortSignal) => {
202+
// Don't show error for aborted requests
203+
if (error instanceof DOMException && error.name === 'AbortError') {
204+
return;
205+
}
206+
207+
if (!isUploadCanceled(signal)) {
208+
setStatus(UploadStatus.ERROR);
209+
setError(error instanceof Error ? error.message : t('upload.error.unknown'));
210+
}
211+
}, [t, isUploadCanceled]);
212+
213+
// Helper to clean up the AbortController
214+
const cleanupAbortController = useCallback((signal: AbortSignal) => {
215+
if (abortControllerRef.current?.signal === signal) {
216+
abortControllerRef.current = null;
217+
}
218+
}, []);
164219

165220
return {
166221
file,
@@ -173,4 +228,4 @@ export const useFileUpload = ({ onUploadComplete }: UseFileUploadOptions = {}):
173228
formatFileSize,
174229
cancelUpload
175230
};
176-
};
231+
};

0 commit comments

Comments
 (0)