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

Dirty state is still cleared if post is edited while prolonged save request is made #6112

Closed
westonruter opened this issue Apr 10, 2018 · 4 comments · Fixed by #6209
Closed
Assignees
Labels
[Type] Bug An existing feature does not function as intended

Comments

@westonruter
Copy link
Member

Issue Overview

If you make a change to a post, then press update and the update request is slow to respond (either due to connectivity or server issues), and make another change while isPostSaving then when the response finishes the dirty state is erroneously cleared and I am unable to save the new changes I made the save request was pending.

Steps to Reproduce

Tested using master at ab7a429.

  1. To aid with testing, add sleep(5) to the top of \WP_REST_Posts_Controller::update_item().
  2. Open a post draft.
  3. Make a change to a post.
  4. Hit Save Draft
  5. While the save request is happening make another change to the post.
  6. Once the save request finishes, try to save the change you just made.

Expected Behavior

The Save Draft button should be re-shown to the user after the save request completes when a change was made during the request. Additionally, there should be the “Are you sure?” dialog when leaving the page at this point so that the user does not lose their changes.

Possible Solution

Introduce a new state flag for isEditedWhileSaving and if when the response completes, check this before resetting isDirty to false.

Screenshots / Video

Screencast of issue: https://youtu.be/HLF7VTd8gjY

@westonruter westonruter added the [Type] Bug An existing feature does not function as intended label Apr 10, 2018
@aduth
Copy link
Member

aduth commented Apr 10, 2018

@aduth
Copy link
Member

aduth commented Apr 10, 2018

Should note that this was explicitly accounted for in the original implementation, and if problematic, is a sign of regression.

Previously: #1093, #1092

@aduth
Copy link
Member

aduth commented Apr 10, 2018

Actually, I think it's more to do with iterations that have occurred over time in dirty detection: #1996, #3298

The approach in #1996 worked because state.editor.edits unsets values now persisted from its RESET_POST handling, so the selector was, in simplest terms, checking whether any keys existed here (slightly more complex, but general idea).

The withChangeDetection higher-order reducer introduced in #3298 is not so capable however, since it merely resets dirtiness upon a RESET_POST.

@aduth
Copy link
Member

aduth commented Apr 10, 2018

Aside: This is a good candidate for E2E testing, since it's very technically challenging and easy to overlook in general usage, and also easy to reproduce in an automated environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants