Rerolled #131 against 11.x, but this also applies to 10.3.x right now.
Les Lim β changed the visibility of the branch 2511548-135 to active.
Les Lim β changed the visibility of the branch 11.0.x to hidden.
Les Lim β changed the visibility of the branch 2511548-135 to hidden.
Les Lim β made their first commit to this issueβs fork.
Update message about availability in Drupal core
One problem with the previous MR is the getRouteCollectionForRequest()
call, which calls out to cache. The PreviewLinkEntityHooks
class that contains it is not a service; it is instantiated new for every entity access check, for which there could be dozens on any given page. Our call to get the current route collection thus gets called new every time, even though the result is the same each time.
Converting the class into a service saves memory and allows us to statically cache the getRouteCollectionForRequest()
result, cutting down substantially on queries.
Les Lim β created an issue.
Thanks for the report! This looks like a duplicate of π Deprecation error message Fixed , which was committed but not released. I'm putting out 1.0.0-alpha5 now.
Thanks! I've opted instead to prevent the function from being invoked for empty values.
Here's a patch that adds a "canonical_reroute" configuration in preview_link.settings. It defaults to TRUE, and is overrideable via normal means (i.e., settings.php).
Les Lim β created an issue.
Whoops, here's a fixed patch file.
Found out that entity access might be determined and cached before hook_entity_embed_context_alter() has a chance to run, so we need to reset the entity type access cache.
Here's a new patch for the 2.1.x branch. I've removed the section that larowlan highlighted in #3, this is just checking that route entity has a preview link, but this is already implicitly true when we check that the route is a preview link route.
@DieterHolvoet Thanks!
Shoot, MR #17 was intended to open against 2.1.x, but it's trying to merge into 2.0.x and I can't figure out how to change the MR or create a new one. What am I missing?
MR #16 doesn't apply cleanly anymore, since the entity hook logic has been moved to a service class in both 2.x branches. Opened MR #17 as a reroll with minor refactoring for dependency injection.
Les Lim β made their first commit to this issueβs fork.
The 2.x branch has the same issue regarding embedded entities as the 1.x branch. When on a Preview Link route and evaluating the access result of embedded entities, nothing in the result stack explicitly returns an AccessResultAllowed, so the embedded entity is not rendered.
Here's that patch.
Les Lim β created an issue.
Committed #2, new release coming shortly. Thanks for the review, @tyler36!
Yep! Got some time blocked out this weekend.
In our case, we're using Locale module, but the configuration in locale.settings.yml is not installed. Just looking through our git history, I'm not sure it ever has been.
This hadn't been causing us any problems that we knew of until the PHP 8.1 upgrade. During locale_requirements(), the function locale_translation_source_build() is called for each project and attempts to build a filepath to a local .po file for the project. To do this, it uses a default filename pattern that is defined in locale.settings.yml. If you don't have any locale.settings configuration, strtr() gets passed NULL instead of a filename pattern.
Not having any locale.settings is clearly a bug on our side, but it sounds like we're not the only ones this has happened to. I'm happy to resolve our bad config state, but should there be safeguards here to account for this edge case?
There are a couple things that need work on this issue:
1) Patch Drupal 8. If the problem does not exist in Drupal 8, update the issue summary to explain why this does not need to be fixed for D8.
2) The reason why the patches pass tests is because no tests exist to catch the original bug. Any patch that would be a candidate for committing to D7 will need to include a test that verifies that the bug exists and that the patch fixes it.