Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(trashbin): Allow emptying trash #49171

Merged
merged 10 commits into from
Dec 12, 2024
Merged

feat(trashbin): Allow emptying trash #49171

merged 10 commits into from
Dec 12, 2024

Conversation

Pytal
Copy link
Member

@Pytal Pytal commented Nov 8, 2024 •
edited
Loading

Close #5534

Summary

Adds a button to empty the trashbin.

image

The TODO requries more adjustments so it will be done separately.

Checklist

@Pytal Pytal added this to the Nextcloud 31 milestone Nov 8, 2024
@Pytal Pytal requested review from susnux, artonge and sorbaugh November 8, 2024 23:28
@Pytal Pytal self-assigned this Nov 8, 2024
@Pytal Pytal requested a review from skjnldsv as a code owner November 8, 2024 23:28
@Pytal Pytal requested a review from marcoambrosini November 8, 2024 23:28
@skjnldsv skjnldsv self-assigned this Nov 20, 2024
jancborchardt

This comment was marked as resolved.

Sign in to view

@skjnldsv skjnldsv removed their assignment Dec 4, 2024
@Pytal

This comment was marked as resolved.

Sign in to view
@sorbaugh

This comment was marked as resolved.

Sign in to view
@sorbaugh sorbaugh mentioned this pull request Dec 9, 2024
Open
@jancborchardt

This comment was marked as resolved.

Sign in to view
@sorbaugh

This comment was marked as resolved.

Sign in to view
@Pytal Pytal mentioned this pull request Dec 11, 2024
Merged
@Pytal
Copy link
Member Author

Pytal commented Dec 12, 2024

All design points addressed :)

@skjnldsv skjnldsv force-pushed the feat/empty-trash branch 2 times, most recently from 4a9fca3 to bfbf56b Compare December 12, 2024 10:52
@skjnldsv
Copy link
Member

skjnldsv commented Dec 12, 2024 •
edited
Loading

Added a few things

  • Made sure we try/catch the action methods, otherwise you can have actions breaking the entire app
  • Added cypress tests

A few comments:
1. I suggest we improve the trashbin endpoint to allow to send only ONE delete request on /remote.php/dav/trashbin/ to empty everything. That would be much faster and avoid sending 48651894 requests
2. I also suggest the action API returns a boolean or null from its exec like the other actions. Otherwise we cannot tell the user if something went wrong. Let's stay consistent on how we address actions across the file app please :)
https://nextcloud-libraries.github.io/nextcloud-files/classes/index.FileListAction.html#exec
nextcloud-libraries/nextcloud-files#1145

Both of those two points can be followups

@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv skjnldsv mentioned this pull request Dec 12, 2024
Merged
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 12, 2024
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
Pytal and others added 8 commits December 12, 2024 10:08
@Pytal
feat(files): Right-align list actions
0af875d 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal
feat(trashbin): Allow emptying trash
943023a 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@skjnldsv @Pytal
fix(files_trashbin): update @nextcloud/files and adjust api + l10n fix
9c08d3b 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv @Pytal
fix(files): protect filelist actions with try...catch
34299e6 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv @Pytal
feat(files_trashbin): add cypress tests
46573b7 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv @Pytal
perf(files_trashbin): use empty trashbin endpoint instead of batch de…
e8b0bdc 
…lete requests

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv @Pytal
feat(files_trashbin): improve wordings
1f419c9 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@Pytal
fix(trashbin): Fix infinitely loading empty trash action on close
aa1944e 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal
Copy link
Member Author

Pytal commented Dec 12, 2024

Nice improvements @skjnldsv, thanks!

Pushed a small bugfix and cypress passses locally

@Pytal Pytal force-pushed the feat/empty-trash branch 3 times, most recently from 882af1c to fed2c3b Compare December 12, 2024 19:43
@Pytal
chore: compile assets
415f082 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal
chore(psalm): Update baseline
ad29dd3 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal enabled auto-merge December 12, 2024 20:17
@Pytal Pytal merged commit c8741b4 into master Dec 12, 2024
120 checks passed
@Pytal Pytal deleted the feat/empty-trash branch December 12, 2024 20:18
@skjnldsv
Copy link
Member

Congrats 👏🎉🤩🥳

@sorbaugh
Copy link
Contributor

🤩

@skjnldsv skjnldsv mentioned this pull request Jan 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nimishavijay nimishavijay nimishavijay left review comments

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@susnux susnux Awaiting requested review from susnux

@artonge artonge Awaiting requested review from artonge

@sorbaugh sorbaugh Awaiting requested review from sorbaugh

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

Assignees

@Pytal Pytal

Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: files feature: trashbin
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.

Add empty trash button
6 participants
@Pytal @sorbaugh @jancborchardt @skjnldsv @marcoambrosini @nimishavijay