Problem/Motivation

As previously discussed in #2867707: WI: Phase H: Conflict management and local workspace merging support, and recently at DrupalCon Vienna in a meeting related to Canvas and Workspaces, we need a conflict management API to unblock concurrent editing in core.

Proposed resolution

Review and agree upon the conflict management API proposed in the issue mentioned above.

Remaining tasks

Revive the patch from #2867707-33: WI: Phase H: Conflict management and local workspace merging support, turn it into a MR, and go from there :)

User interface changes

None yet, we'll be adding only the API at first.

Introduced terminology

TBD.

API changes

API additions likely.

Data model changes

Nope.

Release notes snippet

TBD.

Issue fork drupal-3553775

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

amateescu created an issue. See original summary.

geek-merlin’s picture

Looked at the patch and i looks like great and solid work! Preliminary notes:
- The API expectation where an entity is a ContentEntity and where maybe not and why did not appear clear to me
- The assumption that the different entities are instances of the same entity should be made explicit and maybe assert statements added
- That the language of the local entity is applied to all the others (except result entity) should be made explicit in the docs
- The conflicts array deserves a value object (immutable, withers, withouters)

larowlan’s picture

Assigned: Unassigned » larowlan

Working on the rebase as an MR

larowlan’s picture

Assigned: larowlan » Unassigned
Status: Active » Needs review

Patch from the other issue up as a MR and updated to current core standards etc

mstrelan’s picture

Status: Needs review » Needs work

Added some annoying nits

larowlan’s picture

Status: Needs work » Needs review

This is green now

I have some thoughts on the naming.

We're calling the two revisions 'local' and 'remote' but I'm wondering if we should stick with the naming that git uses, namely 'ours' and 'theirs' as I think that is clearer which is which.

Thoughts?

catch’s picture

'ours' and 'theirs' sounds sensible to me.

larowlan’s picture

Also, is the next step here to put this into a validation constraint and port EntityChangedConstraint into an event listener that extends the base discovery one?

smustgrave’s picture

Recommended way for testing this one?

wim leers’s picture

Assigned: Unassigned » amateescu
Issue tags: +Experience Builder

#8++

Do we need @amateescu to respond to #9? Or could that be @catch? 🤔

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

smustgrave’s picture

Status: Needs review » Needs work

Believe there is feedback on the MR

hchonov’s picture

From what I see here the MR was initially based on the Conflict 2.x module. Why don't we work towards a stable release of the contrib module first and then consider adding it to core? Would be happy to help transition the changes over there.

peximo made their first commit to this issue’s fork.

peximo’s picture

We’re also interested (cc @francescoq) in collaborating on/using this API. We’ve talked it over with @
amateescu, and in the meantime I’ve taken care of the feedback.