Skip to content

Commit c39d2bd

Browse files
committed
fix: onProgress wipes uploading state on retry
1 parent 4ecc7cf commit c39d2bd

File tree

2 files changed

+59
-11
lines changed

2 files changed

+59
-11
lines changed

src/messageComposer/attachmentManager.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -538,24 +538,23 @@ export class AttachmentManager {
538538
}
539539

540540
const shouldTrackProgress = this.config.trackUploadProgress;
541-
this.upsertAttachments([
542-
{
543-
...attachment,
544-
localMetadata: {
545-
...attachment.localMetadata,
546-
uploadState: 'uploading',
547-
...(shouldTrackProgress && { uploadProgress: 0 }),
548-
},
541+
const uploadingAttachment: LocalUploadAttachment = {
542+
...attachment,
543+
localMetadata: {
544+
...attachment.localMetadata,
545+
uploadState: 'uploading',
546+
...(shouldTrackProgress && { uploadProgress: 0 }),
549547
},
550-
]);
548+
};
549+
this.upsertAttachments([uploadingAttachment]);
551550

552551
const uploadOptions = shouldTrackProgress
553552
? {
554553
onProgress: (percent: number | undefined) => {
555554
this.updateAttachment({
556-
...attachment,
555+
...uploadingAttachment,
557556
localMetadata: {
558-
...attachment.localMetadata,
557+
...uploadingAttachment.localMetadata,
559558
uploadProgress: percent,
560559
},
561560
});

test/unit/MessageComposer/attachmentManager.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
API_MAX_FILES_ALLOWED_PER_MESSAGE,
44
DEFAULT_UPLOAD_SIZE_LIMIT_BYTES,
55
} from '../../../src/constants';
6+
import type { LocalUploadAttachment } from '../../../src';
67
import {
78
AttachmentManagerConfig,
89
DraftResponse,
@@ -1309,6 +1310,54 @@ describe('AttachmentManager', () => {
13091310
);
13101311
expect(mockChannel.sendImage).not.toHaveBeenCalled();
13111312
});
1313+
1314+
it('on retry from failed state, onProgress keeps updating uploadProgress while uploading', async () => {
1315+
const {
1316+
messageComposer: { attachmentManager },
1317+
mockChannel,
1318+
} = setup();
1319+
1320+
const file = new File([''], 'test.jpg', { type: 'image/jpeg' });
1321+
mockChannel.sendImage.mockRejectedValueOnce(new Error('Upload failed'));
1322+
1323+
const local = await attachmentManager.fileToLocalUploadAttachment(file);
1324+
await attachmentManager.uploadAttachment(local);
1325+
1326+
const failedAttachment = attachmentManager.attachments[0];
1327+
expect(failedAttachment.localMetadata.uploadState).toBe('failed');
1328+
1329+
let releaseUpload!: () => void;
1330+
const uploadGate = new Promise<void>((resolve) => {
1331+
releaseUpload = resolve;
1332+
});
1333+
1334+
const customUploadFn = vi.fn(async (_file, options) => {
1335+
options?.onProgress?.(33);
1336+
await uploadGate;
1337+
return { file: 'retry-url' };
1338+
});
1339+
attachmentManager.setCustomUploadFn(customUploadFn);
1340+
1341+
const uploadPromise = attachmentManager.uploadAttachment(
1342+
failedAttachment as LocalUploadAttachment,
1343+
);
1344+
1345+
await vi.waitFor(() => {
1346+
expect(attachmentManager.attachments[0].localMetadata.uploadProgress).toBe(33);
1347+
expect(attachmentManager.attachments[0].localMetadata.uploadState).toBe(
1348+
'uploading',
1349+
);
1350+
});
1351+
1352+
releaseUpload();
1353+
await uploadPromise;
1354+
1355+
expect(customUploadFn).toHaveBeenCalledWith(
1356+
file,
1357+
expect.objectContaining({ onProgress: expect.any(Function) }),
1358+
);
1359+
expect(attachmentManager.attachments[0].localMetadata.uploadState).toBe('finished');
1360+
});
13121361
});
13131362

13141363
describe('doDefaultUploadRequest', () => {

0 commit comments

Comments
 (0)