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
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
Comment #2
geek-merlinLooked 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)
Comment #3
larowlanWorking on the rebase as an MR
Comment #5
larowlanPatch from the other issue up as a MR and updated to current core standards etc
Comment #6
mstrelan commentedAdded some annoying nits
Comment #7
larowlanThis 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?
Comment #8
catch'ours' and 'theirs' sounds sensible to me.
Comment #9
larowlanAlso, 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?
Comment #10
smustgrave commentedRecommended way for testing this one?
Comment #11
wim leers#8++
Do we need @amateescu to respond to #9? Or could that be @catch? 🤔
Comment #13
smustgrave commentedBelieve there is feedback on the MR
Comment #14
hchonovFrom 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.
Comment #16
peximo commentedWe’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.