Skip to content

FIX: issuses with fast consecutive clicks on delete/restore buttons #32743

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

0-o0
Copy link

@0-o0 0-o0 commented May 15, 2025

FIX: issues with fast consecutive clicks on delete/restore buttons

issuses: when users rapidly click the delete/recover buttons on posts, asynchronous backend requests can get out of sync with the UI button state, leading to inconsistent behavior and error messages.
look this issue video:

2025-05-15.180327.mp4

what i do: adds state tracking functionality to prevent race conditions when users rapidly click delete/recover buttons:

Added an isDeleting property to track ongoing delete operations
Added an isRecovering property to track ongoing recover operations
Used these properties to disable buttons during API operations

@CLAassistant
Copy link

CLAassistant commented May 15, 2025

CLA assistant check
All committers have signed the CLA.

@0-o0 0-o0 changed the title FIX: Preventing Issues with Rapid Delete/Recover Button Clicks FIX: Issues with Rapid Delete/Recover Button Clicks May 15, 2025
@0-o0 0-o0 changed the title FIX: Issues with Rapid Delete/Recover Button Clicks FIX: issuses with fast consecutive clicks on delete/restore buttons May 15, 2025
@@ -568,10 +570,13 @@ export default class Post extends RestModel {
}

destroy(deletedBy, opts) {
this.set("isDeleting", true);
return this.setDeletedState(deletedBy).then(() => {
Copy link
Member

Choose a reason for hiding this comment

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

I think the finally might not be in the right spot? it only operates on the internal promise, we probably want outside.

To simplify I would probably just make this an async function.

Copy link
Author

@0-o0 0-o0 May 20, 2025

Choose a reason for hiding this comment

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

Thanks for your suggestion! I've updated the code to address both issues:

  1. Converted both destroy and recover methods to proper async functions
  2. Moved the finally block outside to wrap the entire operation, not just the internal promise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants