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

!!!FEATURE: Offer resolution strategies when conflicts arise during rebase #3762

Merged
merged 29 commits into from
May 13, 2024

Conversation

grebaldi
Copy link
Contributor

@grebaldi grebaldi commented Mar 28, 2024

image

solves: #3744
builds on: #3759

Description

When editors work in parallel, it may happen that one editor publishes changes that create conflicts with the changes of another editor. Since #3665 it has been possible for users update their workspace whenever it has fallen out-of-sync with its respective base workspace. Up until now, this mechanism doesn't handle conflicts. When a rebase operation fails due to conflicts, users are trapped in a workspace that cannot be synced, published nor discarded. The only way out is the CLI (see: neos/neos-development-collection#4939).

TLDR; This PR introduces a dialog workflow that allows users to select a resolution strategy for dealing with conflicts that arise during the synchronization of their user workspace.

How to provoke a conflict

Assuming you have two user accounts with Neos.Neos:Editor role (let's call them "editor-a" and "editor-b"):

  1. Log in as "editor-a"
  2. Create a document "Test"
  3. Add a text node to "Test"
  4. Publish those changes
  5. Edit the text node, without publishing that change
  6. Log in as "editor-b" in a private window
  • You should see the orange "Sync" button now
  1. Sync "editor-b"'s workspace, so you can see document "Test"
  • This should trigger the synchronization workflow without conflicts
  1. Remove document "Test"
  2. Publish that change
  3. Switch to "editor-a"'s window and reload
  • You should see the orange "Sync" button now
  1. Edit the text in the text node on document "Test"
  2. Try now to sync "editor-a"'s workspace
  • This should trigger the synchronization workflow with conflicts

Conflict-Free Synchronization Workflow

Show video
Peek.2024-04-26.16-36.Sync.Workflow.mp4

When the user clicks on the orange "Sync" button, a dialog opens that asks them to confirm their intended action. Once they click "Yes, synchronize now", a process indicator opens that cannot be closed. During the operation a beforeunload handler prevents the user from leaving the page. After the operation has finished, the user is presented with a dialog stating the success of the synchronization. Once they click "OK", they see the updated UI state.

If synchronization has removed the document that the user was currently looking at, they have been silently redirected to the closest ancestor of that document.

Synchronization with Conflicts (Strategy FORCE)

Show video
Peek.2024-04-26.16-49.Sync.Workflow.With.Conflicts.Drop.mp4

Again, when the user clicks on the orange "Sync" button, a dialog opens that asks them to confirm their intended action. Once they click "Yes, synchronize now", a process indicator opens that cannot be closed. If the server-side rebase operation fails, the process indicator is interrupted by a dialog that states the presence of conflicts and offers a choice between two strategies to resolve those conflicts:

  1. Drop conflicting changes
  2. Discard workspace "workspace-name"

If they choose "Drop conflicting changes" as a resolution strategy, the user is again presented with a dialog that asks them to confirm their intended action. Once they click "Yes, drop those changes", the synchronization starts once again, this time transmitting force: true to the server-side command handler.

The rest of the workflow is analogous to the Conflict-Free Synchronization Workflow.

Conflict List

Show screenshot

image

The conflict list is shown both in the "Resolution Strategy Selection"-dialog and the "Confirm: Drop conflicting changes"-Dialog. Conflicts are calculated from a list of CommandThatFailedDuringRebase-model objects, that carry information about the attempted CR-level command and the exception that prevented this command from being applied.

The conflict list is a vertical list of <detail>-elements with a <summary> consisting of an annotated node type icon and the label of the affected node. The little orange annotation to the node type icon represents the type of change, which can be one of 4 things:

  1. The node has been created
  2. The node has been changed
  3. The node has been moved
  4. The node has been deleted

The list of conflicts has been squashed on the server-side, so only one (the very first) change per node aggregate will be displayed.

Conflict Details

Show screenshot

image

Opening any item in the conflict list will show the user details about that particular conflict. They will once again see what happened to the affected node in their workspace, and then what is the reason that prevents that change from being applied. Additionally, they'll see what document and what site the affected node belongs to (because conflicts don't necessarily pertain to the site the user is currently looking at).

The "reason for conflict" info is a best-effort guess from the exception that had prevented the respective command from being applied. At the moment, the only reason that will be displayed is "[Node] or one of its ancestor nodes has been deleted". It is possible however for this field to remain empty. Over time we can add more scenarios to the server-side handling of conflicts to better describe each situation.

Synchronization with Conflicts (Strategy DISCARD_ALL)

Show video
Peek.2024-04-26.16-58.Sync.Workflow.With.Conflicts.Discard.mp4

Once again, when the user clicks on the orange "Sync" button, a dialog opens that asks them to confirm their intended action. After they click "Yes, synchronize now", a process indicator opens that cannot be closed. Upon failure of the server-side rebase operation, they are presented with a "Resolution Strategy Selection" dialog.

If they choose "Discard workspace ..." as a resolution strategy, the user is again presented with a dialog that asks them to confirm their intended action. When they click "Yes, discard", their entire workspace will be discarded.

The synchronization process is resumed afterwards.

Error Handling

Errors are handled just like in #3759.

Remaining TODOs

  • Convert WorkspaceSync container to typescript
  • Convert SyncWorkspaceDialog container to typescript
  • Create sub-dialogs for conflict resolution workflow
    • Split out ConfirmationDialog
    • Dialog for resolution strategy selection
    • Confirmation dialog for "Discard All"
    • Confirmation dialog for "Save what you can" a.k.a. "Force Rebase"
    • Result Dialog
    • Process Indicator
  • Move UI.SyncWorkspaceModal redux state to CR.Syncing
  • Wire up new dialogs with CR.Syncing state
  • Implement DISCARD_ALL strategy through integration with existing Publish/Discard workflow
  • Reload Document Tree and Content Canvas after successful rebase
  • Add beforeunload handler during rebase
  • Fix: Warning: Failed prop type: Invalid prop focusedValue of type number supplied to SelectBox_ListPreviewFlat
  • Fix: Warning: Each child in a list should have a unique "key" prop. Check the render method of ConflictList.
  • Fix: Could not find icon { prefix: "fas", iconName: "questionmark" }
  • Squash similar conflicts into one to reduce visual redundancy
  • Add E2E test scenario that creates a conflict state between two editors and asserts the results of choosing "Discard all" as a resolution strategy during rebase
  • Add E2E test scenario that creates a conflict state between two editors and asserts the results of choosing "Save what you can" as a resolution strategy during rebase
  • Remove legacy WorkspaceStatus.OUTDATED_CONFLICT from ts-interfaces
  • Remove legacy state.ui.remote.isSyncing
  • Remove legacy i18n labels
    • Neos.Neos.Ui:Main:workspaceSynchronizationApplied
    • Neos.Neos.Ui:Main:syncPersonalWorkSpaceConfirm
    • Neos.Neos.Ui:Main:syncPersonalWorkSpaceMessage
    • Neos.Neos.Ui:Main:syncPersonalWorkSpaceMessageOutdated
    • Neos.Neos.Ui:Main:syncPersonalWorkSpaceMessageOutdatedConflict
  • Fix: All 0 change(s) in workspace "user-admin" were sucessfully discarded.

@grebaldi grebaldi linked an issue Mar 28, 2024 that may be closed by this pull request
10 tasks
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from 7f50bb0 to 25e88e0 Compare March 29, 2024 12:29
@grebaldi grebaldi force-pushed the feature/conflict-resolution-02/rebase-conflicts branch 5 times, most recently from 6a0910d to 8d44c6d Compare April 4, 2024 14:23
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from 25e88e0 to f471b6b Compare April 8, 2024 15:53
@grebaldi grebaldi force-pushed the feature/conflict-resolution-02/rebase-conflicts branch from d79c71d to 5c51d02 Compare April 8, 2024 16:01
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch 2 times, most recently from 5a8a92c to 6645c4d Compare April 10, 2024 14:31
@grebaldi grebaldi force-pushed the feature/conflict-resolution-02/rebase-conflicts branch 4 times, most recently from bf53841 to b5b9068 Compare April 11, 2024 13:41
@grebaldi grebaldi force-pushed the feature/conflict-resolution-02/rebase-conflicts branch 2 times, most recently from 8567211 to 2b55b57 Compare April 18, 2024 10:07
@grebaldi grebaldi force-pushed the feature/conflict-resolution-01/publishing-modal branch from bec21c1 to 455ff67 Compare April 22, 2024 15:21
@grebaldi grebaldi force-pushed the feature/conflict-resolution-02/rebase-conflicts branch from 2b55b57 to 3069277 Compare April 22, 2024 15:25
@mhsdesign
Copy link
Member

mhsdesign commented Apr 25, 2024

Base automatically changed from feature/conflict-resolution-01/publishing-modal to 9.0 April 25, 2024 15:11
This includes a new data-point `totalNumberOfChanges` which gives us the
number of all changes in the workspace (as opposed to just the changes
per-site or per-document). This is needed to inform the `DISCARD_ALL`
strategy during rebase.
@grebaldi grebaldi force-pushed the feature/conflict-resolution-02/rebase-conflicts branch from 3069277 to f3ac501 Compare April 25, 2024 15:13
@grebaldi grebaldi marked this pull request as ready for review April 26, 2024 16:59
@grebaldi grebaldi changed the title WIP: Offer resolution strategies when conflicts arise during rebase !!!FEATURE: Offer resolution strategies when conflicts arise during rebase Apr 26, 2024
@bwaidelich
Copy link
Member

I lack the energy and brain to fully review this tonight. But I want to say that this is the best PR that I have ever seen.. Feature wise, but especially its description with screenshots and collapsed videos and all that swag <3

@dlubitz
Copy link
Contributor

dlubitz commented Apr 26, 2024

Ohh wow, this looks really great! Thank you @grebaldi!

Copy link
Member

@nezaniel nezaniel left a comment

Choose a reason for hiding this comment

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

Just one little thing, the rest looks nice 👍

Classes/Application/SyncWorkspace/ConflictsBuilder.php Outdated Show resolved Hide resolved
Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

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

Thank you so much ❤️

Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

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

Looks splendid <3 Thank you so much!

@mhsdesign mhsdesign merged commit f7424b3 into 9.0 May 13, 2024
10 of 11 checks passed
@mhsdesign mhsdesign deleted the feature/conflict-resolution-02/rebase-conflicts branch May 13, 2024 15:07
@mhsdesign
Copy link
Member

The new e2e tests are sadly a bit flaky in ci :O

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

Successfully merging this pull request may close these issues.

FEATURE: Offer resolution strategies when conflicts arise during rebase
6 participants