Problem/Motivation

I want to use Conflict along with the Scheduler module.

But, when there's a conflict, the dev version of Conflict marks optional fields as required, when they are not.

This problem does not occur with the alpha version of Conflict.

Steps to reproduce

# 1. Start with vanilla Drupal install (9.3.6 in my case)

# 2. Install dev version of Conflict
composer require 'drupal/conflict:2.x-dev@dev'

# 3. Install Scheduler
composer require 'drupal/scheduler:^1.4'

# 4. Enable
drush en conflict scheduler

5. At /admin/structure/types/manage/article enable Scheduler for publishing and unpublishing this content type

6. Create an article and save

7. Edit the article, open in two tabs (with same user is fine)

8. First tab: change title and save -> saved

9. Second tab: change title and save

Expected result:

Conflict module shows dialog, can resolve as usual

Actual result:

Shows "This value should not be null." and the publish/unpublish fields are shown as required, even though they're not really required

Note

Cannot reproduce with the last Conflict 8.x-2.0-alpha2 release: composer require 'drupal/conflict:^2.0@alpha'

https://git.drupalcode.org/project/conflict/-/commits/8.x-2.x

There's only been one commit between 8.x-2.0-alpha2 and dev:

CommentFileSizeAuthor
#6 conflict-remove-widget-3266501-5.patch847 bytesjocowood

Comments

hugovk created an issue. See original summary.

heikkiy’s picture

heikkiy commented

I can verify the problem with the latest Alpha 3 version of Conflict and both Scheduler 1.5 and 2.0 versions.

Only option is to uninstall Conflict which we were using to add support with Content lock for locking only content translations.

heikkiy’s picture

heikkiy commented
Priority: Normal » Critical

I bumped the priority to Critical because at current state the bug prevents users from saving content completely if the content type is using Scheduler.

sagar25’s picture

sagar25 commented
Assigned: Unassigned » sagar25
jocowood’s picture

jocowood
Primary language German
Location Kamp-Lintfort
commented

I have debugged this problem together with a colleague, as we also want to use conflict together with scheduler.

We are pretty sure that this added line of code causes the problem:
https://git.drupalcode.org/project/conflict/-/commit/d1958eca3852ef55c87...

The key/value pair conflictWidget will be added to the value array of all fields. Later on the validation of the entity is triggered. Because of the additional information in the value (here data) array, the array is not empty and is considered complex (see [1]). For this reason the array entries are validated individually. Because the datetime information is still null, the NotNullValidator reports a validation.

Another negative side effect is, that the comparison of the local node and the server node in the conflict module always leads to the result "has changes". FieldComparatorDefault is the only available comparator, which does a simple equals comparison. This comparison is always false, because locally there is this additional entry conflictWidget.

Proposed solution:

A) Place the conflictWidget information at a different location

or

B) Add a tidy up function, which is called, when conflict finished its job. We did a quick and dirty try at the end of \Drupal\conflict\Entity\ContentEntityConflictHandler::entityFormEntityBuilder, which seem to fix the bug.

foreach (array_keys($form_display->getComponents()) as $field_name) {
  if ($entity->getFieldDefinition($field_name)) {
    unset($entity->get($field_name)->conflictWidget);
  }
}

We are working on a patch, which we will provide soon.

[1] \Drupal\Core\TypedData\Validation\RecursiveContextualValidator::validateNode L.163

jocowood’s picture

jocowood
Primary language German
Location Kamp-Lintfort
commented
StatusFileSize
new conflict-remove-widget-3266501-5.patch847 bytes

It turned out that the quick and dirty solution is also a good and clean solution. Here is a patch for conflict:2.0-alpha3

msteurs’s picture

msteurs commented

I confirm the patch in comment #6 works for us.

olli’s picture

olli commented
Status: Active » Needs review
harkonn’s picture

harkonn commented
Assigned: sagar25 » Unassigned
Status: Needs review » Reviewed & tested by the community

The patch works. Thank you.

jocowood’s picture

jocowood
Primary language German
Location Kamp-Lintfort
commented

Hi! I would really appreciate a merge into the main branch and a new version. This issue is RTBC for 2 years.

  • joseph.olstad committed eb11a964 on 3.0.x 
    [#3266501] feat: dev: "This value should not be null" for optional...
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Reviewed & tested by the community » Fixed

beta2 will have this

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

https://www.drupal.org/project/conflict/releases/3.0.0-beta2

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

jocowood’s picture

jocowood
Primary language German
Location Kamp-Lintfort
commented

Thank you very much