Problem/Motivation

Follow-up from #3565802: Saving registration settings fails when the conflict module is installed
For additional context, this looks very similar to a long-standing Drupal 7 issue #3133091: Allow some fields to be skipped

When the conflict module is enabled alongside registration /
registration_waitlist, saving registration settings can fatal due to the host entity reference being cleared during conflict resolution.

The issue occurs when conflict auto-merges non-editable fields and sets the registration host entity field to an empty value. This wipes the computed host entity, leaving downstream consumers assuming an entity exists when it does not.

This surfaced while debugging a fatal error in registration_waitlist, but the root cause appears to be the interaction initiated by conflict.

Steps to reproduce

  1. Enable conflict, registration, and registration_waitlist.
  2. Create a host entity configured for registration (for example, an Event node).
  3. Set a non-zero capacity on its registration settings and save.
  4. Trigger a conflict resolution scenario where non-editable fields are auto-merged.
  5. Save the registration settings entity.
  6. Observe a fatal error downstream (for example, Call to a member function getSetting() on null) due to the host entity being cleared.

Proposed resolution

In Drupal\conflict\Entity\ContentEntityConflictHandler::autoMergeNonEditableFields(), avoid setting complex or computed entity reference field item lists to an empty value when doing so invalidates required computed state.

Specifically:

  • autoMergeNonEditableFields() currently calls HostEntityFieldItemList::setValue([]).
  • This propagates to HostEntityItem::setValue(), which nulls the computed host entity.
  • During preSave(), the item is filtered as empty, leaving the list empty with $valueComputed = TRUE.

I am proposing skipping auto-merge for computed fields that manage their own internal state.

Issue fork conflict-3566032

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:

About issue forks

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review

joelpittet opened merge request !18

john.oltman’s picture

john.oltman commented
Assigned: Unassigned » john.oltman
Status: Needs review » Postponed

Please hold on this until I determine if the registration module is at fault or not. Should have an answer within the next day or two.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Assigned: john.oltman » Unassigned
Status: Postponed » Needs review

@john.oltman regardless of the outcomes to your findings in #3565802: Saving registration settings fails when the conflict module is installed, this solution fixes the symptoms presenting in that registration issue and solves an issue in the D7 #3133091: Allow some fields to be skipped so I think it can stand on it's own and not be postponed. Feed 2 birds, one breadcrumb.

john.oltman’s picture

john.oltman commented

So the problem does indeed lie with the registration module. I posted the resolution over there and should have a fix committed within the next few days.

My concern with the proposed change to the conflict module as seen in the MR is there is an installed base of thousands of sites, hundreds of which probably have at least one computed field in their schema somewhere. Hard coding a change like this out of the blue, especially when the problem lies elsewhere to begin with, could create regressions for all those sites which rely on the existing behavior.

Regarding the issue in #3133091: Allow some fields to be skipped, that is a Feature Request, not a bug, and after 5+ years of things working a certain way, I recommend providing an opt-out mechanism for fields through config or an alter hook, as mentioned on that issue, instead of hard coding the type of change in this MR. That way people have a mechanism for opting *any* field out, not just computed fields, which I think is the intention of that issue. This would have the side benefit that it would not change existing behavior for current users of the conflict module - they would have to take steps to activate the opt-out. If you want to do that, you would post an MR to that issue and not to this one.

Given the above, I propose you close this issue as "works as designed". Since you opened the issue @joelpittet, I'll leave it up to you as to how you want to handle it, but that would be my advice. Thanks much for catching and posting this!