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

BUGFIX: Improve conflict resolution edge-cases in sync and publish workflow #3910

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Jan 23, 2025

Resolves #3908

fixes edecases during the conflict resolution:

  • stopping publish if the attempted changes were dropped after a conflict: After resolved conflict, don't attempt to re-synchronise #3908
  • fixes 'Discard workspace "User workspace"' as resolution during publishing (previously froze)
  • fixes navigating back after selecting 'Discard workspace "User workspace"' in sync mode and selecting another strategy

…nflict resolution

Otherwise, the now up-to-date workspace is synced leading to a no-op exception:

> Skipped rebase workspace "admin-admington" because it is not outdated.
… conflict resolution

Attempting to publish 0 changes after the adjustments in the core neos/neos-development-collection#5337 will throw an error and is also needless:

> The command "PublishIndividualNodesFromWorkspace" for workspace admin-admington must contain nodes to publish

To avoid unnecessary interaction - which is also tested by this test:

> Publish + Syncing: Create a conflict state between two editors, then try to publish and choose "Drop conflicting changes" as a resolution strategy during automatic rebase

We dont continue publishing if the publish button is not orange basically ;)

In the future i think this should be improved that another phase and screen is added so this is done more transparently.
@github-actions github-actions bot added Bug Label to mark the change as bugfix 9.0 labels Jan 23, 2025
1. have conflicts
2. attempt to publish which fails due to conflict
3. choose 'Discard workspace "User workspace"'
4. click "Accept choice and continue"

current state:

After selecting the strategy no further modal is shown, no requests are send, no errors are logged, and the ui is partly unresponsive (the publish button)
The reason for this is that discard seems to be faulty implemented and the confimation dialog (SyncingPhase.RESOLVING) is NOT shown for this case resulting in this odd behaviour where the state mashine is not advanced because of missing interaction.
While the basic functionality is restored with 66c11d6, it now comes to and odd state that is caused by `actionTypes.CONFLICTS_RESOLVED` which moves the sync phase back to `ONGOING`.
Due to that there are now two modals during the finishing phase, once the sync rotating modal and on top the confirmation "1 change(s) in site "foo" were sucessfully published to workspace"

To fix the message the state mashine has to be properly replaced in nested calls for actionTypes.STARTED, as otherwise the modal would say that things were published, not discarded.

And we also stop the syncing workflow when discarding takes over to avoid two modals.
@mhsdesign
Copy link
Member Author

mhsdesign commented Jan 24, 2025

I also noticed what @pKallert found out, that during publishing the conflict resolution strategy "discard all" does not work. Thanks for the hint how to solve that :D I took the changes you prepared for #3893 into this pr in 66c11d6 - i hope thats okay:)

Further after staring a lot onto the code i was able to fix the overlaying modals and the result message in dc1dbd6 instead of using your approach to add a SyncingPhase.IDLE state cec189e to avoid the overlaying popups.

Bildschirmaufnahme.2025-01-24.um.15.51.40.mov

Further as my changes result in the use of the SyncWorkspaceDialog -> ResolutionStrategyConfirmationDialog -> DiscardAllConfirmationDialog instead of the PublishingDialog -> ConfirmationDialog the "No, cancel" button works correctly and leads back to the selection instead of stopping fully and the visual appearance and the text is slightly different:

Old New
image image

@mhsdesign mhsdesign changed the title BUGFIX: 3908 delicate operations after resolving conflict BUGFIX: Improve conflict resolution edge-cases in sync and publish workflow Jan 24, 2025
- test for selecting 'Discard workspace "User workspace"' as resolution during publishing (previously froze)

- test for navigating back after selecting 'Discard workspace "User workspace"' in sync mode and selecting another strategy
@mhsdesign mhsdesign force-pushed the bugfix/3908-delicate-operations-after-resolving-conflict branch from ee43458 to 6ee4e83 Compare January 25, 2025 14:20
@mhsdesign mhsdesign merged commit da19324 into neos:9.0 Jan 27, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.0 Bug Label to mark the change as bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After resolved conflict, don't attempt to re-synchronise
1 participant