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

Undo / Redo Inconsistent Crud changes #6986

Closed
chuckn0rris opened this issue Jun 12, 2023 · 3 comments
Closed

Undo / Redo Inconsistent Crud changes #6986

chuckn0rris opened this issue Jun 12, 2023 · 3 comments
Assignees
Labels
bug Something isn't working forum Issues from forum large-account Reported by large customer OEM OEM customer premium resolved Fixed but not yet released (available in the nightly builds)
Milestone

Comments

@chuckn0rris
Copy link

chuckn0rris commented Jun 12, 2023

If you perform event drag, and check crudManager.crudStoreHasChanges(), it will have changes.
If you Undo this action and check crudManager.crudStoreHasChanges(), it will have changes.

If you schedule event (ex. drag from grid), and check crudManager.crudStoreHasChanges(), it will have changes
If you Undo this action and check crudManager.crudStoreHasChanges(), there are no changes will be available.

It should be working consistent.

Forum post

Hello Bryntum team!

We are experiencing some inconsistencies regarding the undo/redo and the changes available in the crudManager.

That happens when we don't perform a crudManager.acceptChanges();

Basically, if I move a scheduled event to a different timeline, the Undo / Redo will always trigger new changes in the crudManager.

But the same doesn't happen when scheduling / unscheduling events.

We just perform a crudManager.acceptChanges() after receiving a valid reply from the server.

Our server has a set of validation process that is triggered when it receives a new Change from the STM, so if the user performs multiple changes in a very short period, the server interrupts the running validation and starts new validation considering the latest changes received (which is an accumulated set of changes).

The fact that Undo is not triggering changes means that even though the Previous action was undone, it won't send that information to the server.

Example case:

Supposing the server take 5 seconds to validate a Scheduling action:

Drag Unscheduled event to scheduler => Trigger STM => there are crud changes => send to server
Server starts validation of the scheduled changes...
Undo is Clicked right after => Trigger STM => no changes in crud => nothing is send to the server

With that, after 5 seconds the server will finish the validation and apply the Scheduling action to the db. Which is wrong as the user clicked Undo for that action.

If we consider the same case but moving scheduled events to a new timeline:
Drag scheduled event from 5AM to 6AM => Trigger STM => there are crud changes => send to server 6AM
Server starts validation with the 6AM change...
Undo is Clicked right after => Trigger STM => there are changes => send to server 5AM
Server receives new change with 5AM, will interrupt the running validation and start a new validation with the most recent changes (5AM)

Follow attached a snippet and source code using drag-from-grid as a starting point.

You can see the logs are different when Undoing a Scheduling action compared to a Drag already scheduled event to a new timeline

@chuckn0rris chuckn0rris added bug Something isn't working premium forum Issues from forum large-account Reported by large customer OEM OEM customer labels Jun 12, 2023
@canonic-epicure canonic-epicure self-assigned this Jun 13, 2023
@canonic-epicure canonic-epicure added in progress invalid This doesn't seem right and removed in progress labels Jun 19, 2023
@canonic-epicure
Copy link

Screencast.2023-06-20.12.39.35.mp4

@canonic-epicure
Copy link

Screencast.2023-06-20.12.55.38.mp4

@canonic-epicure canonic-epicure added in progress and removed invalid This doesn't seem right labels Jun 20, 2023
@canonic-epicure
Copy link

Actually its not "invalid" as it should not include constraintDate : null in changes after the quick undo, going to fix that.

@canonic-epicure canonic-epicure added ready for review Issue is fixed, the pull request is being reviewed and removed in progress labels Jun 21, 2023
@isglass isglass added resolved Fixed but not yet released (available in the nightly builds) and removed ready for review Issue is fixed, the pull request is being reviewed labels Jun 21, 2023
@isglass isglass added this to the 5.3.8 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working forum Issues from forum large-account Reported by large customer OEM OEM customer premium resolved Fixed but not yet released (available in the nightly builds)
Projects
None yet
Development

No branches or pull requests

3 participants