Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In D6 project_issue, most people "update" issues by posting new comments. Folks can do that in parallel and mostly it all works fine. The only problem is "cross posting" if one person tries to change an issue metadata field and the other doesn't, the 2nd user to post ends up reverting the change from the first (unless they're paying attention when they preview their comment). See #218066: Prevent cross posting from reverting metadata fields for more.
In D7 project_issue, there's a related but totally different problem. We'll no longer have the problem of a 2nd edit reverting the changes from the first, since core itself completely blocks the 2nd edit submission with a validation error. Occasionally we see this in D6 on D.o when 2 users are trying to edit an issue summary at the same time. But that's going to get vastly worse if *every* update to an issue in D7 is effectively a node edit.
It'd really be nice to avoid this. It'd be great to allow both edits to happen somehow, and only throw a validation error if both users edited the same field(s) or something. That's something it might be worth pouring some Special Sauce(tm) on so that the experience doesn't completely suck. Not exactly sure what ingredients we'd need in that sauce, yet, but we should ponder it. Perhaps there's a generic solution for this in contrib already, I'm not sure.
One thought would be to use node.js to automagically go out and update the edit forms of anyone else editing a given issue whenever a user successfully submits an update. There'd still have to be some conflict resolution logic on the client so that node.js doesn't just undo your edits out from under you, but somehow alerts you "someone else just updated this, click here to reload the current value" or something.
Frankly, this is a scary mess. People are going to *hate* us if we push forward and don't have a reasonably solid fix for this. Bumping to critical as a result.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | project_conflict.tgz | 1.46 KB | bdragon |
Comments
Comment #1
webchickI did some quick research on this, and here are the findings.
Going purely by project descriptions, http://drupal.org/project/revision_fu seems to be the best bet. "Allows simultaneous edits on a node without blocking saves. Instead separate revisions are stored with non-conflicting edits being displayed and conflicted edits simply stored for later review." D6 only.
http://drupal.org/project/content_lock goes the other direction and uses locks to prevent cross-posting. I think that'd be really frustrating, but it would get around the problem. D6 & D7.
http://drupal.org/project/collaborative_editor was a D4.7 GSoC project with no commits since then. :\
Comment #2
webchickIncidentally, https://trello.com/ has some really kick ass functionality around this space. A co-worker and I can have the same ticket open, and I can see them update fields while I'm updating them as well. Not sure what it does in case of conflict.
Comment #3
sunFYI: content_lock is the successor of checkout module. NOT recommended for large-scale community sites like drupal.org. Intended for small-scale/mid-scale company/enterprise sites only. The lock mechanism is fairly comparable to Views'. Also, I'm less confident on whether content_lock has been battle-tested on larger sites.
In other words, content_lock is even more embarrassing than Form/Node API's native concurrency validation. In both cases you're told to calm down, drink tea.
From my POV, this boils down to either:
1) in-Drupal solution, à la revision_fu, deeply integrating with Form API and whatnot. (Working on this benefits a wide range of Drupal sites)
2) Websockets solution, à la node.js, entirely outside and bypassing Drupal for most of the actual concurrent editing time, only stashing new issue revisions (via web service callbacks implemented in Drupal) when the content is supposed to be saved and resolved (no conflicts). (Working on this likely benefits http://drupal.org/project/nodejs and the small(er) crowd of advanced web developers who're capable of setting up node.js & co.)
Comment #4
MustangGB CreditAttribution: MustangGB commented#2
Trello looks super awesome, but IMO only really lends itself to edit-in-place solutions, which already got put on the backburner for Project*
---
This is the way I would see it working
1)
User 1 loads up edit form and changes:
- Version from 6.x to 7.x
- Priority from normal to major
- Status from NR to RTBC
2)
User 2 loads up edit form and changes:
- Version from 6.x to 7.x
- Category from bug report to feature request
- Status from NR to NW
3)
User 1 submits their form and all fields are successfully updated in the issue
4)
User 2 submits their form and Drupal realises there has been another submission whilst this user was editing the issue (i.e. original values != current values). It works out the following:
- Version, current issue value is 7.x, we were trying to change it to 7.x anyway so leave it as that, no conflicts
- Priority, current issue has changed from normal to major, but we weren't changing this value anyway so... no conflicts
- Category, we want to change this value and current issue still has the same value as when we started to edit so we can still change this, no conflicts
- Status, current issue value has changed from when we first started to edit, CONFLICT
If a conflict has arisen reload the edit form with latest values from the issue (Version and Priority) with our changes that weren't in conflict (Category) then highlight conflicting fields and set their value to the latest issue value (Status set to RTBC instead of desired value of NW) but display an error message at the top of the screen informing the user of their original value, something like, "Your status field update from NR to NW has already been updated by another user to RTBC. Confirm the values of the highlighted fields before submitting your issue update again."
If a conflict has not arisen (e.g. user 2 didn't update the status) then the form submits cleanly without error. Unchanged fields are not reverted (Priority), changed fields that have been updated to the same value as the current issue do not reaccredit the change to user 2 but leave it with user 1 as it would have been already (Version), and changed fields that are different to the current value but have not changed since we began editing are updated and accredited to user 2 (Category)
---
So no js autoupdating shizzle. The way to do this practically would be to store the original values in the form data then when submitted lock the record and pull the current values. You now have original values, submitted values and current values so do the working out for conflicts. No conflicts then update the issue and unlock the record. Conflicts then reload the edit form as described above and unlock the record.
Doing it this way rather than trying to keep track of who started editing when etc. makes it a lot simpler to work and and also accounts for users submitting not necessarily in the order they started editing. (i.e. u1 edit, u2 edit, u2 submit, u1 submit)
I also think the automagic thing would be annoying if it kept altering your changes as you made them.
Comment #5
dww@akamustang: Sure, we can do magic like that at form submission, but there are 2 problems:
1) Core makes it automatically a form validation error as soon as the 2nd user tries to submit the form at all. So we have to deal with that somehow.
2) All that you say is fine for the drop-down fields, but what about the body (summary) itself? The workflow you describe is going to potentially result in people losing 30 minutes of typing to update an issue summary when someone else fixes a minor typo.
Comment #6
dwwI just posted #1545922-66: [META] Issue page redesign that could help mitigate this problem. We'd still need to do something, but if we go that way, I think we could potentially downgrade this from critical to major. ;)
Comment #7
bdragon CreditAttribution: bdragon commentedHere is an experimental thingie that defangs core's conflict detector and compares things field by field.
In the event of non-conflicting changes, the new values are brought in and the form is rebuilt with warnings that the user needs to verify the fields. The user can hit save again to continue.
In the event of conflicting changes, the form is rebuilt with errors that there was a conflict. If the user decides to resave at this point, it will overwrite the other changes.
I wish that form_set_value worked when form errors were encountered. :(
* Is this something that is workable as a solution?
* Should it be project specific?
- Is this in-scope for diff or another module, or should I maybe make "conflict.module"?
Comment #8
bdragon CreditAttribution: bdragon commented* Made a sandbox @ https://drupal.org/sandbox/bdragon/1599370
Comment #9
dwwExcellent! Sounds great. I'll have to check it out.
Thanks!
-Derek
Comment #10
dwwAwesome! This seems extremely promising. And definitely could be a generally-useful stand-alone module. Thanks for starting it as a sandbox! I think you should just promote it. ;) I'd be happy to be a co-maintainer, and add it to projectinstall. I guess we don't even have to make it a dependency of project_issue, but it'd be a highly-recommended add-on.
I opened a few issues based on what I saw using it on a test site and looking through the code:
#1599480: Improve UI of the error messages during conflicts
#1599484: Allow admins to configure which content types should allow conflict resolution
#1599486: Ensure this works if fields have been moved around in $form
#1599488: Add a conflict.test file with automated tests
#1599490: Highlight modified and conflicting fields during conflict resolution
Let's just handle those separately instead of discussing them in here. I've tagged them all for
Drupal.org D7, project.Comment #11
sunThis made me resurrect my core patch in #1378126: Simplify tests needing multiple, concurrent user sessions / logins, which I've migrated into Conflict and completely refactored it there.
Comment #12
Senpai CreditAttribution: Senpai commentedComment #13
Senpai CreditAttribution: Senpai commentedAssigning to Bdragon.
Comment #14
Senpai CreditAttribution: Senpai commentedTagging for Sprint 5.
Comment #15
Senpai CreditAttribution: Senpai commentedThe conflict module has been promoted to a full project. Great!
@bdragon, the https://drupal.org/project/conflict page needs a description of how the module is supposed to work, and what it does for people. Fill that project description in with a saucy paragraph of your choosing, and we can close this ticket.
Comment #16
Senpai CreditAttribution: Senpai commentedComment #17
dwwBefore we call this fixed, we should probably add either an explicit dependency on conflict.module, or at least some documentation to recommend it. We probably also want to include in in the projectinstall distro.
Thanks,
-Derek
Comment #18
bdragon CreditAttribution: bdragon commentedAdded a dependency.
I had already included conflict in projectinstall as of the day I promoted it to a full project.
Comment #20
mitchell CreditAttribution: mitchell commentedCross-referencing the core issue: #106953: Better handling of editing conflicts.
Comment #21
dwwFYI: just so it doesn't (necessarily) get lost forever: I split off #1966010: Provide real-time notification of concurrent edit conflicts into a separate issue.