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.

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

Remove unused confirm_bulk_delete functionality #6261

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Conversation

legoktm
Copy link
Member

@legoktm legoktm commented Feb 9, 2022

Status

Ready for review

Description of Changes

This page was originally used as a confirmation interstitial when deleting
files or messages, but in 1e64f36 (#2946) it was replaced with a CSS-only
deletion confirmation on the same page, so users would never hit the
separate confirmation page.

Remove the tests that were still calling this endpoint, the HTML template
and code that was checking for the "confirm_delete" action.

I noticed this when investigating a translator comment about the "Permanently Delete Files" text and was unable to figure out how to take a screenshot of it. We probably don't need to get this into 2.2.0, but maybe we can mark these strings as not requiring translation in the meantime.

Testing

  • Verify that you still see a confirmation dialog in the JI when deleting files/messages

Deployment

Any special considerations for deployment? No

Checklist

  • Linting (make lint) and tests (make test) pass in the development container
  • These changes do not require documentation

This page was originally used as a confirmation interstitial when deleting
files or messages, but in 1e64f36 (#2946) it was replaced with a CSS-only
deletion confirmation on the same page, so users would never hit the
separate confirmation page.

Remove the tests that were still calling this endpoint, the HTML template
and code that was checking for the "confirm_delete" action.
@legoktm legoktm marked this pull request as ready for review February 9, 2022 20:39
@legoktm legoktm requested a review from a team as a code owner February 9, 2022 20:39
@cfm
Copy link
Member

cfm commented Feb 17, 2022

I'll be happy to review this in this sprint, @legoktm. I developed the same suspicion while working on #6165 but didn't have time to prove that this view and logic could indeed be removed. :-)

@cfm cfm self-assigned this Feb 17, 2022
Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

Confirmed all deletion flows appear and function as expected:

  1. /: select one or more sources → DeleteFiles and Messages
  2. /: select one or more sources → DeleteSource AccountsYes, Delete Selected Source Accounts
  3. /col/<filesystem_id>: select one or more entries → Delete SelectedDELETE

Nothing like a good all-deletion pull request. :-)

@cfm cfm merged commit 37f969e into develop Mar 1, 2022
@legoktm legoktm deleted the rm-confirm-delete branch March 1, 2022 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants