SWC-7064 - Hook to prepare uploading a list of files #1354
Conversation
| @@ -119,5 +117,5 @@ export function getUseMutationMock<TData, TError, TVariables>( | |||
| failureReason: null, | |||
| isPending: false, | |||
| submittedAt: 0, | |||
| } | |||
| } satisfies UseMutationResult<TData, TError, TVariables> | |||
There was a problem hiding this comment.
Using satisfies here gives us the type-checking we want while preserving the actual type.
The utility of this is that callers will now see that mutate/mutateAsync/reset properties are Jest mock functions, and can be interacted with as such without a type cast
i.e.
const mockResult = getUseMutationMock('foo')
mockResult.mutateAsync.mockResolvedValue('bar') // this is a type error without this change!
| * In the future, this sequence could be amended to check if the storage location has enough space to accommodate all the | ||
| * new files, and return an error if not. |
There was a problem hiding this comment.
Holding off on adding this functionality for now while I try to achieve feature parity. I think it makes sense to put this check here as the first step.
| // Split the results into new and updated files | ||
| const filesPreparedForUpload = getExistingFilesResults | ||
| // All of these promises are fulfilled -- we use this filter to narrow the type | ||
| .filter(promise => promise.status === 'fulfilled') | ||
| .map(promise => promise.value) | ||
|
|
||
| const newFileEntities = filesPreparedForUpload.filter( | ||
| f => f.existingEntityId == null, | ||
| ) | ||
|
|
||
| const updatedFileEntities = filesPreparedForUpload.filter( | ||
| f => f.existingEntityId != null, | ||
| ) | ||
|
|
||
| return { newFileEntities, updatedFileEntities } |
There was a problem hiding this comment.
This wasn't captured in any design, but I'm slightly changing how we do this in SWC.
In SWC, each file is handled serially, so the process is something like (pseudocode)
for file in files:
createDirectories(file)
const fileEntityExists = checkIfFileEntityExists(file)
if fileEntityExists:
promptUserToUpdateFile()
upload(file)
Where now it will be something like
const userPrompts = []
for file in files:
createDirectories(file)
const fileEntityExists = checkIfFileEntityExists(file)
if fileEntityExists:
userPrompts.push(file)
promptUserToUpdateFiles(userPrompts)
upload(files)
When we prompt the user "Do you want to create a new version of file xyz?" we can give the options "Yes", "Yes to all", "No", "Cancel uploads", so their upload won't be interrupted unless there's an error, and no uploads start until they have gone through each of these prompts.
WDYT about this? I think in the happy-path scenario this is much more user-friendly. A major downside to this approach is that a bunch of directories might be created unnecessarily
| * | ||
| * @param options | ||
| */ | ||
| export function usePrepareFileEntityUpload( |
There was a problem hiding this comment.
This wasn't captured in any design, but I'm slightly changing how we do this compared to SWC.
In SWC, each file is handled serially, so the process is something like (pseudocode)
for file in files:
createDirectories(file)
const fileEntityExists = checkIfFileEntityExists(file)
if fileEntityExists:
promptUserToUpdateFile()
upload(file)
Where now it will be something like
const userPrompts = []
for file in files:
createDirectories(file)
const fileEntityExists = checkIfFileEntityExists(file)
if fileEntityExists:
userPrompts.push(file)
promptUserToUpdateFiles(userPrompts)
upload(files)
When we prompt the user "Do you want to create a new version of file xyz?" we can give the options "Yes", "Yes to all", "No", "Cancel uploads", so their upload won't be interrupted unless there's an error, and no uploads start until they have gone through each of these prompts.
I think in the happy-path scenario this is much more user-friendly. A major downside to this approach is that a bunch of directories might be created unnecessarily. 'Preparing' all files in parallel makes the most sense when we will add logic to check the storage limit, where users will want that warning before they start uploading anything.
@jay-hodgson
WDYT about this? Is there a compelling reason to preserve the current logic that I am unaware of?
There was a problem hiding this comment.
There have been many requests to move away from serially processing, so that users do not have to babysit the upload (with this new approach, the user has the option to say "Yes to all" and walk away!). I think the side effect creating the full directory tree up front is acceptable (given the user has not spent time uploading actual file content, which is the bulk of the operation, and they can always move the entire tree into the trash can if they don't want it).
No description provided.