🇺🇸United States @mtalt

@ok4p1 I have merged this in and created a release.

mtalt → created an issue.

I was also encountering the issue with Drupal 10.3.1 and Layout Builder Browser 1.7. I've opened a MR that attaches the libraries to the layout_builder element through hook_element_info_alter(). This results in the libraries only being loaded once. I saw this method of attaching a library to layout builder in this issue 💬 Generic way to attach library to layout_builder? Closed: works as designed .

mtalt → made their first commit to this issue’s fork.

Looks great @simonbaese. Thank you!

Sounds good @cosmicdreams. I'm going to close this issue, but feel free to re-open if you think of a viable alternative.

@cosmicdreams

At one point, we were using hook_entity_insert, but the documentation says "Note that hook implementations may not alter the stored entity data.". Saving the entity again was causing other issues, such as webform handlers to be called more than once 🐛 Estimated Read Time module make webform send emails twice Fixed .

Does your site also break on previewing a node? I've run into issues in the past where a NID is not always available in the template and we had to wrap code in a condition that checked if the NID was available. The NID will not be present when previewing a new entity. The code you see where we set that we are in a preview is because of similar functionality in core for nodes. This occurs in the links field where those links will not be rendered while previewing.

Is Cohesion using that node ID to render content that would have a meaningful difference on the read time? If not, could you only render that content if a node ID is present? Alternatively, could you create a view mode dedicated for read time calculations that does not require a node ID in the template?

Opened https://github.com/mtownsend5512/read-time/issues/20 to start the conversation of making a change to the package.

@sidharth_soman This does appear to be a bug in Mtownsend\ReadTime\ReadTime in the calculateMinutes method. Ideally, the issue would be fixed in that repository.

I do worry about this being an unexpected change for users of this module. There are likely many people that only use minutes in their field formatter and this fix would cause no read time to be displayed for short content. Perhaps the simple fix for this would be to check if the tokenized string only contains @minutes and if the minutes value is 0, then we change it to 1 so that "1 min read" is still shown? Or we could introduce a setting on the field formatter for a minimum read time of 1 minute.

https://www.drupal.org/project/estimated_read_time/issues/3377947 ✨ Introduce switch to manually set read time Needs work could serve as a work-a-round by being able to manually set read times.

Thanks @simonbaese and @kevinquillen! With the fix merged from https://www.drupal.org/project/estimated_read_time/issues/3395919 🐛 Creating multi-language entities programmatically does not estimate read time for all translations Active , the tests passed.

Thank you @simonbaese. I tested programmatically creating a translation and replicated the issue. Your changes resulted in the estimated read time being calculated on the translation.

mtalt → made their first commit to this issue’s fork.

Thanks @FiNeX. I could replicate the issue on 10.2.x and your patch fixes the issue. I've merged this into 1.0.x.

mtalt → made their first commit to this issue’s fork.

@simonbaese Thanks again for your contributions.

The main case that caused issues was on creation of new nodes where the core links field was attempted to be rendered, but neither a node ID nor revision ID was available yet and the node couldn't be loaded. The issues involving translations was due to the same error.

Potential tests:

1. Creation of new node
2. Update the node
3. Create a new translation and test that both the translated estimated read time and source estimated read time are correct
4. Update a translation

Thank you for your contributions @simonbaese! I have tested the changes and everything is working as expected. I created a new release today and will merge this into dev.

Marking this as fixed since the lingering link field issue should be resolved with the patch in 3361590 🐛 Estimated Read Time module make webform send emails twice Fixed being committed.

This has been committed to dev.

@weseze Thank you for testing. Agreed that the code should never have been saving the entity in the presave or insert hooks. I'll get this into the dev branch.

@shasha821110 My apologies for the delayed response, I wasn't notified of your comment to me a few months ago. I opened https://www.drupal.org/project/geolocation/issues/3374505 🐛 Cannot set properties of undefined error using Google Places API Geocoder Fixed because I'm not sure if we are running into the same issue. If it is the same issue you are running into, please review if you have a chance.

mtalt → made their first commit to this issue’s fork.

mtalt → created an issue.

+1 RTBC. I have not experienced any issues with Domain 2.0.0-beta1.

A potential fix for the links field issue has been added in issue 3361590 🐛 Estimated Read Time module make webform send emails twice Fixed

@FiNeX - Thank you for the issue and patch.

The switch to using a combination of hook_entity_presave() and hook_entity_insert() instead of just hook_entity_presave was done in an attempt to solve for the link field issue you had reported in issue 3322934 🐛 WSOD saving node if "Links" field is enabled on view mode Fixed . The patch in #11 would revert that fix and cause the error when creating new nodes that display the links field.

But it does look like it was a mistake to attempt to workaround the issue by using hook_entity_insert() because you run into issues like yours with Webform when the entity is saved again. In order to attempt to solve for both the original links field issue and the improper use of hook_entity_insert(), I have updated the patch to include code to set the in_preview property for nodes to TRUE, which will prevent the links field links from being built.

Please review!

I have ran into this issue today. I was able to workaround it by deleting the facets_summary config that contained the facet_source_id that matched the plugin ID in the error message. The facets summary that I needed to delete was not displayed on the /admin/config/search/facets page and was unused.

mtalt → created an issue.

I have run into a similar issue where default domain values are not being assigned. I am on 8.x-1.0-rc1 of the module. The settings for the entity are:

1. Enable domain entity access
2. Behavior is set to User
3. Check any combination of default domain value(s) for the bundle

This portion of the patch in #4 fixes the issue:

function domain_entity_field_default_domains(FieldableEntityInterface $entity, FieldDefinitionInterface $definition) {
 -  return $definition->getThirdPartySetting('domain_entity', 'domains', []);
 +  return array_keys($definition->getThirdPartySetting('domain_entity', 'domains', []));
}

The current code results in only the first domain listed being set. This occurs because the array returned by the third party setting is keyed with strings instead of numerically. In Drupal Core's FieldInputValueNormalizerTrait::normalizeValue method, if the keys are not numeric, it will re-key all values under the 0 key. That results in only 1 of the selected default domains being set.

I have encountered two issues while testing. I am using:

Domain Entity - 1.0.0-rc1
Domain Simple Sitemap - 2.0.0-beta1
Country Path - latest dev-1.x

Issue 1:
Entities using the domain_entity field that have no domains checked were not showing in the sitemap. On the domain_entity field, if no domains are checked, then the content is considered affiliated to all domains.

The domain_entity module alters queries to add conditions to determine whether or not an entity is allowed for a domain. That logic is not accounted for in this issue's patches. The patch only checks if the entity was explicitly affiliated to the domain with:

$query->condition(\Drupal\domain_entity\DomainEntityMapper::FIELD_NAME, $sitemap_domain_id);

I made a change that lets the domain_entity module handle this logic with its hook_query_alter() code.

Issue 2:
Patch #19 changes the way URLs are replaced when using the Country Path module → . The change in #19 resulted in URLs with 2 country prefixes (e.g https://example.com/us/us/en/product/1 instead of https://example.com/us/en/product/1).
I have reverted that change.

It was only occurring for me when it saves a translation. I really need to get tests added 📌 Add kernel tests Needs review for all these scenarios.

Thanks everyone! I've merged the patch from #3 in and created a new release for the module. I'm going to move this issue back to "Needs Work" because many people are still having issues when the Links field is displayed in the view mode. The error described in the issue still occurs when that Link field is displayed. I know a work-a-round is to create and use a new view mode that doesn't use the Links field, but it is causing too many headaches.

One of the previous solutions for the Links field issue was to split up the hooks into hook_entity_presave and hook_entity_insert. That resulted in node ID being available when rendering the Links field. Unfortunately, when a node is translatable, the issue still occurs when it tries to render the Links field when saving a translation. Using a similar solution for translations by using hook_entity_translation_insert didn't work for me and the error still occurred.

One thought would be to set the node's $is_in_preview value to TRUE before rendering the node. That should fix the issue because this part of the renderLinks method will never run, which is where the error is occurring.

Thoughts?

@SamirMtl Your fix does appear to fix the error. However, it appeared that it does not calculate the proper read time for the translated content. Are you experiencing that same issue?

@shabana.navas

Btw, for some reason adding the estimated read time field via Layout Builder does NOT currently work on our site. The spinner just spins and stops without adding the field to a block section. There are no errors reported. I did a workaround by creating a custom block and rendering the field via the block.

This has been fixed in https://www.drupal.org/project/estimated_read_time/issues/3349101 🐛 Cannot add or modify read time field through layout builder Fixed

mtalt → created an issue.

@SamirMtl, I'll take a look at your issue with translations separately. I didn't want to combine the two issues into one.

Thanks all! I have tested and merged MR 10 from comment #8.

This appears to have been duplicated in https://www.drupal.org/project/country_path/issues/3258438 → , which has been resolved.

I might be running into the same problem after attempting to update to the latest version of the module and Drupal 9.5.x, but it is difficult to tell without additional information. Downgrading to Drupal 9.4.x also resulted in the map displaying again for me.

Are you using the Geolocation CommonMap view style with AJAX turned on in your view? Do you have any errors in your console?

mtalt → made their first commit to this issue’s fork.

This worked for me as well. I encountered this issue when upgrading to the 8.x-1.0-beta7 version of the Domain module. Should we add a composer dependency to this MR to prevent others from running into this issue?

@asirjacques Thanks for taking a look. The error is occurring for new nodes on this line of the renderLinks method of NodeViewBuilder. The issue is that since the node has not been saved, it does not have an ID, so $node_entity_id will be NULL.

Using estimated_read_time_entity_insert does seem to work, but estimated_read_time_entity_update is going to throw this error if revisions are on.

Perhaps we could check if the entity is new and use estimated_read_time_entity_insert, but continue to use estimated_read_time_entity_presave if it is an update.

We could get the value from the textfields directly and get rid of the call to the renderPlain funtion. The same way that it is done in the node_read_time module.

One of the reasons I made this module was because the other estimated read time solutions did not work well with layout builder. Only using fields on the entity will leave out any inline blocks that could meaningfully contribute to the read time. There are also some cases where content could be pulled in dynamically through a view or some other method. In those cases, the content dynamically pulled in would be left out of the read time. I'm wary of moving to that solution if it will not work with layout builder.