Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/common/types/upload.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
// @flow
import { STATUS_PENDING, STATUS_IN_PROGRESS, STATUS_STAGED, STATUS_COMPLETE, STATUS_ERROR } from '../../constants';
import {
STATUS_PENDING,
STATUS_IN_PROGRESS,
STATUS_STAGED,
STATUS_COMPLETE,
STATUS_ERROR,
STATUS_CANCELED,
} from '../../constants';
import type { Token, BoxItem } from './core';

// TODO: replace with `UploadItemStatus` from @box/uploads-manager once 'inprogress' is aligned to 'uploading'.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we could add | typeof STATUS_CANCELED to the existing type instead of refactoring the whole union to inline strings. with inline strings the type and constants might drift. unless there is a strong reason to replace these all with inline strings..

i think adding 'canceled' is fine since that's additive. but the TODO mentions replacing 'inprogress' to 'uploading'. that would be a breaking change for consumers using useUploadsManager: true, since onComplete, onUpload, onClickCancel etc. all pass UploadItem with status directly to consumers. So i just want to make sure that's an "add" and not a "replace".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we could add | typeof STATUS_CANCELED to the existing type instead of refactoring the whole union to inline strings. with inline strings the type and constants might drift. unless there is a strong reason to replace these all with inline strings..

agree added back

but the TODO mentions replacing 'inprogress' to 'uploading'. that would be a breaking change for consumers using useUploadsManager: true, since onComplete, onUpload, onClickCancel etc. all pass UploadItem with status directly to consumers.

I was placing a todo because we would need to cleanup old uploads manager

So i just want to make sure that's an "add" and not a "replace".

Currently this is add not the replace — yes

type UploadStatus =
| typeof STATUS_PENDING
| typeof STATUS_IN_PROGRESS
| typeof STATUS_STAGED
| typeof STATUS_COMPLETE
| typeof STATUS_ERROR;
| typeof STATUS_ERROR
| typeof STATUS_CANCELED;

type FileSystemFileEntry = {
createReader: Function,
Expand Down
1 change: 1 addition & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ export const CLIENT_VERSION = __VERSION__;

/* ---------------------- Statuses -------------------------- */
export const STATUS_ACCEPTED: 'accepted' = 'accepted';
export const STATUS_CANCELED: 'canceled' = 'canceled';
export const STATUS_COMPLETE: 'complete' = 'complete';
export const STATUS_ERROR: 'error' = 'error';
export const STATUS_INACTIVE: 'inactive' = 'inactive';
Expand Down
109 changes: 97 additions & 12 deletions src/elements/content-uploader/ContentUploader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
DEFAULT_HOSTNAME_UPLOAD,
ERROR_CODE_ITEM_NAME_IN_USE,
ERROR_CODE_UPLOAD_FILE_LIMIT,
STATUS_CANCELED,
STATUS_COMPLETE,
STATUS_ERROR,
STATUS_IN_PROGRESS,
Expand Down Expand Up @@ -1165,6 +1166,72 @@ class ContentUploader extends Component<ContentUploaderProps, State> {
});
};

/**
* Mark a single in-progress or pending item as canceled without removing
* it from the queue. Used by the modernized uploads manager flow so that
* canceled items remain visible in the list.
*/
markItemCanceled = (item: UploadItem) => {
const { onClickCancel } = this.props;
item.api.cancel();
item.status = STATUS_CANCELED;
onClickCancel(item);
};

/**
* Cancel every pending or in-progress upload at once. Items keep their row
* in the list with the canceled status. Only used by the modernized flow.
*/
handleUploadsManagerCancelAll = () => {
const { onCancel } = this.props;
const cancelable: UploadItem[] = this.itemsRef.current.filter(
item => item.status === STATUS_PENDING || item.status === STATUS_IN_PROGRESS,
);
cancelable.forEach(item => this.markItemCanceled(item));
onCancel(cancelable);
this.updateViewAndCollection([...this.itemsRef.current]);
};

/**
* Retry every errored or canceled item. Resumable items (with a sessionId)
* are resumed; everything else is restarted via resetFile + uploadFile.
* Only used by the modernized flow.
*/
handleUploadsManagerRetryAll = () => {
// Snapshot since removeFileFromUploadQueue mutates itemsRef.current.
const targets = this.itemsRef.current.filter(
item => item.status === STATUS_ERROR || item.status === STATUS_CANCELED,
);
targets.forEach(item => this.retryUploadsManagerItem(item));
};

/**
* Retry a single errored or canceled item: drop name-in-use, resume resumable
* chunked uploads, otherwise restart from scratch. Shared by RetryAll and per-item retry.
*/
retryUploadsManagerItem = (item: UploadItem) => {
const { chunked, isResumableUploadsEnabled, onClickCancel, onClickResume, onClickRetry } = this.props;
const { file, api, error } = item;
// Name-in-use cannot be resolved by retrying — drop the item like the legacy onClick path.
if (error?.code === ERROR_CODE_ITEM_NAME_IN_USE) {
this.removeFileFromUploadQueue(item);
onClickCancel(item);
return;
}
const isChunkedUpload =
chunked && !item.isFolder && file.size > CHUNKED_UPLOAD_MIN_SIZE_BYTES && isMultiputSupported();
const isResumable = isResumableUploadsEnabled && isChunkedUpload && api?.sessionId;
if (isResumable) {
item.bytesUploadedOnLastResume = api.totalUploadedBytes;
this.resumeFile(item);
onClickResume(item);
} else {
this.resetFile(item);
this.uploadFile(item);
onClickRetry(item);
}
};

/**
* Expands the upload manager
*
Expand Down Expand Up @@ -1236,24 +1303,40 @@ class ContentUploader extends Component<ContentUploaderProps, State> {
return this.state.items.find(item => getUploadItemKey(item, rootFolderId) === key);
};

handleModernizedItemAction = (key: string) => {
handleUploadsManagerItemCancel = (key: string) => {
const item = this.findItemByUploadKey(key);
if (item) {
this.onClick(item);
} else {
if (!item) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same concern as #4573. if findItemByModernizedId doesn't find a match, this silently returns. we could catch any unhandled scenarios here with an else. not sure how you want to surface that though.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will use the same console.warn pattern

// eslint-disable-next-line no-console
console.warn(`ContentUploader: no upload item found for key "${key}" on action.`);
console.warn(`ContentUploader: no upload item found for key "${key}" on cancel.`);
return;
}
if (item.status === STATUS_PENDING || item.status === STATUS_IN_PROGRESS) {
this.markItemCanceled(item);
this.updateViewAndCollection([...this.itemsRef.current]);
}
};

handleModernizedItemRemove = (key: string) => {
handleUploadsManagerItemRetry = (key: string) => {
const item = this.findItemByUploadKey(key);
if (item) {
this.removeFileFromUploadQueue(item);
} else {
if (!item) {
// eslint-disable-next-line no-console
console.warn(`ContentUploader: no upload item found for key "${key}" on retry.`);
return;
}
if (item.status !== STATUS_ERROR && item.status !== STATUS_CANCELED) {
return;
}
this.retryUploadsManagerItem(item);
};

handleUploadsManagerItemRemove = (key: string) => {
const item = this.findItemByUploadKey(key);
if (!item) {
// eslint-disable-next-line no-console
console.warn(`ContentUploader: no upload item found for key "${key}" on remove.`);
return;
}
this.removeFileFromUploadQueue(item);
};

/**
Expand Down Expand Up @@ -1345,9 +1428,11 @@ class ContentUploader extends Component<ContentUploaderProps, State> {
items={mapToModernizedUploadItems(items, rootFolderId)}
isExpanded={isUploadsManagerExpanded}
onToggle={this.toggleUploadsManager}
onItemCancel={this.handleModernizedItemAction}
onItemRetry={this.handleModernizedItemAction}
onItemRemove={this.handleModernizedItemRemove}
onItemCancel={this.handleUploadsManagerItemCancel}
onItemRetry={this.handleUploadsManagerItemRetry}
onItemRemove={this.handleUploadsManagerItemRemove}
onCancelAll={this.handleUploadsManagerCancelAll}
onRetryAll={this.handleUploadsManagerRetryAll}
/>
</div>
);
Expand Down
Loading
Loading