Account created on 5 February 2007, almost 18 years ago
#

Merge Requests

More

Recent comments

🇩🇪Germany hctom

Changed code to only target enum property plugins for now, simplified the implementation of EntityFieldSource::getPropValue() a little and used EnumTrait::convertValueToEnumType() for the final (rectified) return value.

Looking forward to the new review results and please don't forget to say something about the idea to log invalid values.

🇩🇪Germany hctom

Update issue title and summary to target enum property type plugins only with this ticket for now.

🇩🇪Germany hctom

So, finally got the time to create my first draft for this:

  • Added the typed_data values to the enum* PropType plugins
  • Added some logic to ensure field source value is a valid enum option - if not, it will fall back to a valid value in that order:
    • property default value (if defined)
    • first enum option value (if property is required)
    • NULL (if property is optional)

Looking forward to your review ;)

🇩🇪Germany hctom

Removed decimal from list of typed_data values to add, because there is not List (decimal) field type in Drupal.

🇩🇪Germany hctom

Update to issue summary to name the right typed_data values for number fields (because number is not correct there / not available as typed data type)

🇩🇪Germany hctom

So, finally got the time to write the test ;) Please review again!

🇩🇪Germany hctom

Please see the issue fork, which restores the old version constraints.

🇩🇪Germany hctom

Sorry, already wanted to do that the last days, but totally forgot about it ;) Please review the issue fork and MR for the necessary changes.

🇩🇪Germany hctom

Damn, I just realized, that this issue needs a core patch in order to work: Provide option to set ajax indicator with .use-ajax and .use-ajax-submit Needs work

So maybe this should be postponed until that other issues is resolved.

🇩🇪Germany hctom

Just checked the new changes by larowlan and I can confirm: comparing revisions of block content entities is now working as expected. Thank you!

🇩🇪Germany hctom

I used the latest patch on a Drupal 10.3.6 install with nothing fancy in there. It's just a block content entity bundle with a custom bundle class.
I also updated the patch with the latest changes now, but the errors persists - and as far as I see, the tests do not show any problems, because they only test the revision overview currently and not the actual revision comparison, where this error occurres.

🇩🇪Germany hctom

I just tested out the issue fork and it works like a charm - only found a problem with custom block content entities. The revision overview seems to work as expected (with the radio buttons to compare selected revisions), but when clicking Compare selected revisions, I get this error:

Drupal\Core\Entity\Exception\UndefinedLinkTemplateException: No link template 'revision' found for the 'block_content' entity type in Drupal\Core\Entity\EntityBase->toUrl() (line 211 of core/lib/Drupal/Core/Entity/EntityBase.php).

🇩🇪Germany hctom

Please see the issue fork and MR for a fix for this issue. This also fixes a warning, if there is no $nested_filters[$document_type] item at all.

🇩🇪Germany hctom

As discussed with @pdureau in Slack, increasing the core version requirement for Drupal 10 to ^10.3.4 is enough, because the latest 10.3.x version will be targeted.

Issue fork and MR are also update with the corresponding commit.

🇩🇪Germany hctom

Just checked drupal/core-recommended for the recommended Twig versions, and from 10.3.4 on, twig/twig is required with ~3.14.0 (https://github.com/drupal/core-recommended/blob/10.3.4/composer.json).

Ist it worth to support core versions below 10.3.4 or should the core_version_requirement of UI Patterns be changed to ^10.3.4 || ^11 instead?

🇩🇪Germany hctom

Thanks for the quick fix, this helped a lot, because now we know, that a cURL timeout is the underlying problem in our case.

But with the current solution from the MR from #6, the $response variable is just completely empty, which results in a log message like this:

Failed creating/updating documents: "cURL error 28: Operation timed out after 30001 milliseconds with 0 bytes received (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://api.swiftype.com/api/v1/engines/SWIFTYPE-ENGINE-ID/document_types/entity-node/documents/bulk_create_or_update_verbose". Response:

I guess the error handling should be able to deal with all of those cases:

  • No response at all (e.g. when a cURL error like ours occurred)
  • Array response (already covered)
  • String response (already covered)

Additionally, it would also be nice, if the external IDs of the items to index may be included in the log message - because those are mostly very important to track done any underlying issues.

🇩🇪Germany hctom

Done - all green again ;)

🇩🇪Germany hctom

I personally also did a basic check with Drupal 11.0.4 and everything worked fine - also with already available field data. So let's get this merged ;)

🇩🇪Germany hctom

Finally got the update hook working forDrupal >= 10.2. Please review again - at best with Drupal 10.2.x, 10.3.x and 11.x sites that already contain data.

🇩🇪Germany hctom

Unfortunately the column_changes_handled field storage setting (which is needed to alter a field's storage schema, if there is already field data available) is first introduced with Drupal 10.3 - see Provide a flag to allow updates to stored schema for fields Fixed

I am currently looking for another way to do this for Drupal >= 10.2. If there is none, this issue will increase the required core version to 10.3.

🇩🇪Germany hctom

@steffenr thanks for testing. I'll check later, why it is not working, because it worked on my side with data in a view mode switch field's table ;)

@rohit rana: The length of the field is INCREASED in the schema... so you do not have to backup/truncate/restore anything. If so, the update hook would be quite useless ;)

Back to needs work and assigning it to me.

🇩🇪Germany hctom

Good catch, thanks for the report. The field's value column length did not correspond to the actually allowed maxmimum length of view mode names. The issue fork provides a fix for that by increasing the column's length to 64 in the database.

Please remember to run database updates before testing this patch, because the schema is updated for all existing view mode switch fields with the new length.

🇩🇪Germany hctom

hctom made their first commit to this issue’s fork.

🇩🇪Germany hctom

Done - CI pipelines for previous minor are re-enabled.

🇩🇪Germany hctom

I just checked the patch from #29 with Drupal 10.3.2 and unfortunately it is for me also only working when creating a block content via Admin -> Content -> Blocks -> Add content block.

When using it with the "Automatically generate the label and hide the label field" setting in layout builder context with an inline block, the field is hidden as expected, but the label is always %AutoEntityLabel% (even when updating the inline block later).

I think the biggest problem here, is that for new/updated inline blocks, the auto_entitylabel_entity_presave() hook is not called directly when adding/updating the block, but when the surrounding entity's layout is saved as a whole. I debugged through the code and saw that AutoEntityLabelManager::setLabel() actually gets the correct label, but setting it on the inline block entity does not work somehow.

But in order to have the correct label immediately after adding/updating an inline block in layout builder context, maybe the label field value has to be set during validation or similar, so it is always available, even when the surrounding layout has not been saved as a whole?

🇩🇪Germany hctom

Only added the proposed fix as issue fork / MR, so we have a starting point and it may be used for patching the module.

🇩🇪Germany hctom

Thanks for your fast response and fix. This looks good to me. In my node forms etc. the Text format select element is gone again, when only a single text format is allowed and the Wysiwyg source still has the Text format select element available.

I'd also say moving the #pre_render callback assignement to the actual source plugin is definitely better, so that code does not run for all text_format elements.

Thanks a lot again and: RTBC from my side ;)

🇩🇪Germany hctom

You're welcome - I'm currently evaluating the state of ui_patterns and I am quite impressed so far! ;) Thanks a lot and keep up the good work.

🇩🇪Germany hctom

See the issue fork for the required changes to fix this issue. Only thing to check is, if the minimum required core version has to be bumped to 10.3 with this fix.

🇩🇪Germany hctom

Okay perfect, didn't see that one coming ;) I will follow along and remove D9 compatibility for my module as well now. Thanks!

🇩🇪Germany hctom

Updated issue summary with info about more findings of the PHP 7.4 code incompatibilities.

🇩🇪Germany hctom

That was the first thing I tried, when identifying the issue with the form element, but it does not fix the error, because the container itself still gets serialized due to $this usage in the getInfo() implementation (because all its information gets serialized when written to cache).

As I already mentioned: ALL core elements use the static approach instead of using $this, so this should be done with this element as well - and of course to avoid these errors ;)

🇩🇪Germany hctom

This is fixed by using the maximum supported PHP version for previous major tests now.

🇩🇪Germany hctom

I am closing this ticket now, because there was no feedback anymore. Feel free to open a new issue, if there is still an issue with this module.

🇩🇪Germany hctom

Any news on this? Because this prevents a site from being installed with both the cl_editorial and storybook module initially enabled.

🇩🇪Germany hctom

You are welcome... and this is actually really something I was already thinking about, to allow setting custom labels for the field options. I'll give this a try soon, when I have some spare time again.

In the meantime you may alter the displayed label of the field's empty value by implementing a corresponding field widget form alter hook.

For now, I will close this issue as "works as designed" and I will start on a follow up for the custom labels soon.

🇩🇪Germany hctom

Thanks for the details - I think, I know what the problem is: You are only switching the default view mode, but you are accessing the entity in full view mode.

So you have to check full view mode (because you want to switch away from that one) under "View modes to switch" in field storage config instead of enabling it under "View modes allowed to switch to".
Additionally your field should be optional - if no switch is selected later, the full view mode (with layout builder and potential overrides enabled) is displayed, if standard is selected, that view mode will be used instead. I have some of these setups running without problems.

Please let me know, if that fixes the issue for you.

Production build 0.71.5 2024