Bug #22395
closed
Trashing collections reported to be slow
Added by Peter Amstutz 4 months ago.
Updated 11 days ago.
Description
In #22202 the process delete method removeProcessPermanently
was rewritten to do the whole batch of delete requests, then check for errors and refresh the display at the end. This is more efficient (it isn't refreshing the list after every delete request) and also combines the results into a single snackbar instead of 50.
For this ticket, we want to do something similar with the MOVE_TO_TRASH operation: send all the trash requests in a batch, and only update the user interface on completion. This probably involves changing toggleCollectionTrashed
to take a list of uuids.
- Description updated (diff)
- Target version changed from Development 2025-01-29 to Development 2025-02-12
- Target version changed from Development 2025-02-12 to Development 2025-02-26
- Assigned To set to Stephen Smith
- Target version changed from Development 2025-02-26 to Development 2025-03-19
- Status changed from New to In Progress
- Target version changed from Development 2025-03-19 to Development 2025-04-02
Changes at arvados|c3c0caa9eb38feb2f8e9d0fc05ea6028a571846a branch 22395-batch-collection-trash
Tests developer-run-tests-services-workbench2: #1480 
- All agreed upon points are implemented / addressed.
- Converted toggleCollectionTrashed into a batch action - this is mostly a copy of the process trash action. I hard coded isTrashed on some of the calls though I change this later after discovering that some of the context menu items are dual purpose trash/untrash
- Abstracted batch promise result handling into a separate helper function so that remove process and other batch resource API calls can use the same logic
- I thought about making it a thunk so that it could access dispatch, but getting a typed return value from dispatchable functions seemed infeasible, and a main draw of the abstraction is getting a type correct PromiseFulfilledResult so I decided to pass in dispatch for the snackbars instead.
- The helper accepts a map of CommonResourceErrors to messageFuncs, which generate the appropriate error/success message using a count. "NONE" is used for success messages and UNKNOWN errors are grouped with any other errors that aren't present in the messageFunc map
- The success / unknown errors fall back to generic errors if not passed in the map
- I discovered some bugs involving the wrong kind of trash func being called (project instead of collection), and also noticed that trashing groups of mixed resources is allowed but doesn't work. I decided the best solution is to merge the trash functions to handle all trashable resources (project and collection)
- Because the old trash method handled
isTrashed
of each resource individually, routing it to the correct operation, it technically allowed toggling a group of resources with mixed isTrashed
status despite the UI not allowing that to happen. Since I didn't want to build in support for a scenario that shouldn't be possible, I decided to calculate isTrashed
for the whole batch using resources.some(res => res.isTrashed)
. While I think it would be better for the context menu to decide isTrashed in a way that is linked to the display of the text that shows what operation will be performed (ToggleTrashAction) and use that for the whole batch, at least this way if trashed/untrashed get accidentally mixed, it will try to double-trash/untrash some of them and present an error for those items instead of toggling every item and leaving the user in a confusing state where some items were trashed and some were untrashed.
- Anything not implemented (discovered or discussed during work) has a follow-up story.
- Code is tested and passing, both automated and manual, what manual testing was done is described
- passes automated tests
- Tested single/multi, collection+project
- deleting / restoring favorites
- restoring collections refreshes trash panel
- restoring single project navigates to project
- deleting currently viewed collection/project navigates to parent
- Documentation has been updated.
- Behaves appropriately at the intended scale (describe intended scale).
- Should handle ~50 items fine
- Considered backwards and forwards compatibility issues between client and server.
- Follows our coding standards and GUI style guidelines.
Note: The bug where the breadcrumbs switches to Trash after deleting a resource is present, but fixed when I cherry-picked your other branch, so that fix is left out of this branch
- Target version changed from Development 2025-04-02 to Development 2025-04-16
- Status changed from In Progress to Resolved
Also available in: Atom
PDF