-
Notifications
You must be signed in to change notification settings - Fork 512
Delete multiple objects #856
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d4eae3d to
a8f96ff
Compare
dvaldivia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testd. lgtm
| for (var i = 0; i < selectedObjects.length; i++) { | ||
| removeRecord(i); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will trigger multiple delete object calls, and the removeRecord inside has
setDeleteLoading(false);
closeDeleteModalAndRefresh(true);
that will be called multiple times as well, @adfost its better to implement a new endpoint called bulkDelete that receive a list of object IDs and then perform the delete operations on the backend, once all objects are deleted you return operation success to the frontend, that way you will have only 1 call instead of multiple ones
f0a1779 to
283e74a
Compare
| api | ||
| .invoke( | ||
| "DELETE", | ||
| `/api/v1/buckets/${selectedBucket}/delete-multiple?path=${selectedObjects}&recursive=${recursive}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are sending multiple paths to delete, I think it's better so send them in the body of the request as a string array instead of sending them in the query. This may cause troubles in the future.
Please change API accordingly
| `/api/v1/buckets/${selectedBucket}/delete-multiple?path=${selectedObjects}&recursive=${recursive}` | |
| `/api/v1/buckets/${selectedBucket}/delete-multiple?recursive=${recursive}`, {path: selectedObjects} |
| <Button | ||
| variant="contained" | ||
| color="primary" | ||
| startIcon={<DeleteIcon />} | ||
| onClick={() => { | ||
| setDeleteMultipleOpen(true); | ||
| }} | ||
| disabled={selectedObjects.length === 0} | ||
| > | ||
| Delete Selected | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fb51d7a to
5134819
Compare
109bd9c to
9bb6b00
Compare
dvaldivia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested. LGTM



Delete multiple objects using selectors on object list