Problem/Motivation

We support a Drupal distribution with installation from existing config. This generally works well, except when contrib modules do odd things that break our tests. We're running into just such an issue right now where a sequence of events happens that isn't expected.

  1. webform_install()
  2. _webform_update_html_editor()
  3. manually insert filter.format.webform_default (missing UUID)

StorageComparer::addChangelistUpdate compares uuid values and blows up a non existent uuid array key in the $target_data. Adding a null uuid in the optional config solves this. Leaving things with no uuid at all in the optional config, leads to PHP 8.1 complaining about passing a NULL uuid value to mb_strtolower in ConfigEntityBase and Entity\Query\Condition::compile()

Proposed resolution

Add a null uuid to the optional format configuration.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 3369127-11.patch882 byteshchonov
#4 3369127-4.patch476 bytesjrockowitz
#2 3369127.patch332 bytesheddn

Comments

heddn created an issue. See original summary.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 3369127.patch332 bytes
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Related issues: +#3302838: Querying with NULL values results in warning mb_strtolower(): Passing null to parameter is deprecated

See #3302838: Querying with NULL values results in warning mb_strtolower(): Passing null to parameter is deprecated for what this causes.

jrockowitz’s picture

jrockowitz commented
StatusFileSize
new 3369127-4.patch476 bytes

I did not have time to reproduce the issue but the attached patch might address the root cause which is don't alter config when config is being sync'd

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work

I get the following with the patch in #4:

1) Drupal\Tests\install_profile\Functional\InstallProfileTest::testInstall
editor.editor.webform_default: Drupal\Component\Diff\Engine\DiffOpChange::__set_state(array(
   'type' => 'change',
   'orig' => 
  array (
    0 => 'uuid: null',
  ),
   'closing' => 
  array (
    0 => 'uuid: 94ff9c7f-4743-4b76-a020-5d83cf93b3ba',
  ),
))

I got no error with the patch in #2.

jrockowitz’s picture

jrockowitz commented

I am open to patch #2, I am assuming the filter.format.webform_default does get assigned a UUID.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

If you assign a UUID in your exported config, it will override the null value installed by this module when the exported config is imported. Ideally #2 would assign a unique UUID to the config as it is inserting into the DB. Otherwise, it is gets inserted as a null value and site admins have to ignore the warnings from #3.

jrockowitz’s picture

jrockowitz commented
Priority: Normal » Critical
Status: Needs work » Reviewed & tested by the community
Parent issue: » #3358973: Plan for Webform 6.2.0 Release

Moving this to critical because it should be a release blocker.

@hedden I am fine with your recommendation from #4.

I am changing this to RTBC so that https://www.drupal.org/files/issues/2023-06-23/3369127.patch can be merged.

  • Liam Morland committed d4b760bb on 6.2.x authored by heddn 
    Issue #3369127 by heddn, jrockowitz: Add missing UUID to 'filter.format...
liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed
hchonov’s picture

hchonov
Primary language Bulgarian
Location Burgas
commented
Status: Fixed » Needs review
StatusFileSize
new 3369127-11.patch882 bytes

The real issue here is that _webform_update_html_editor() is using the config storage directly to write the config and thus leaving the config without an UUID. We identified this since we use the config_plus module that has a protection against saving configs without an UUID. The UUID is being added to an entity only when using the entity storage to save the entity. Therefore we should not be using the config storage here to create config entities but rather the config entity storage. I am attaching a patch that properly installs the config entities.

jrockowitz’s picture

jrockowitz commented
Status: Needs review » Reviewed & tested by the community

@hchonov Thank you for swopping in and saving the day. Your patch looks fine to me.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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