🇺🇸United States @bvoynick

I've made a simple MR that removes all derivative-wiping from the uninstall hook, if that's an acceptable change.

bvoynick → changed the visibility of the branch 8.x-1.x to hidden.

bvoynick → created an issue.

Thanks, merged

I think it would be premature to commit asymmetric translation support in this module before Paragraphs commits to an approach to it in the issue linked in the description → . I'm changing the status to Postponed for that reason.

Appreciate the work, and this issue could continue to serve as a place to discuss and work on asymmetric support. But I think Paragraphs has to move first.

Resolved in https://www.drupal.org/project/paragraphs_browser/issues/3397513 🐛 Resolve access check error for D10 compatibility RTBC - credit for tsquared212 added there.

That ticket has more documentation and discussion, so decided to mark this one as the duplicate even though it's the original.

Thank you!

bvoynick → created an issue.

The access check is a false positive from the Upgrade Status module. More details here. 🐛 Resolve access check error for D10 compatibility RTBC

In some quick testing the module seems D11 compatible as-is without needing anything more than the .info.yml file adjustment. I'd suggest leaving the false positive to other existing tickets and simply tagging the module D11 compatible.

This issue is better documented though so might be better to continue here.

A little more information on this:

Omitting ->accessCheck() was only deprecated for content entities. paragraphs_browser_type is a config entity so this isn't actually a problem. But Upgrade Status thinks it is because it just sees a generic EntityStorageInterface object without a way to know if it's content or config.

This also works to make Upgrade Status happy:

$paragraphs_browser_type_storage = $this->entityTypeManager->getStorage('paragraphs_browser_type');
assert($paragraphs_browser_type_storage instanceof ConfigEntityStorageInterface);
$paragraph_browser_type_ids = $paragraphs_browser_type_storage->getQuery()->execute();

But it's arguably simpler to just include the accessCheck() call as in the already-reviewed MR.

This looks like a duplicate of https://www.drupal.org/project/paragraphs_browser/issues/3362240 📌 Drupal 10 compatibility fixes for Paragraphs Browser Active

Note that the access check issue was previously reported in https://www.drupal.org/project/paragraphs_browser/issues/3362240 📌 Drupal 10 compatibility fixes for Paragraphs Browser Active , and later again in https://www.drupal.org/project/paragraphs_browser/issues/3397513 🐛 Resolve access check error for D10 compatibility RTBC

bvoynick → created an issue.

I'm not able to apply a diff or patch for MR8127 to 10.3.x core, and #39 doesn't seem to apply on 10.3 anymore either. Here's a version of the current MR8127 made against 10.3.x specifically.

Merged in the latest 11.x branch. The same single test is still failing. These queries are not expected in core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php

+    34 => 'INSERT INTO "semaphore" ("name", "value", "expire") VALUES ("active-trail:route:view.frontpage.page_1:route_parameters:a:2:{s:10:"display_id";s:6:"page_1";s:7:"view_id";s:9:"frontpage";}:Drupal\Core\Cache\CacheCollector", "LOCK_ID", "EXPIRE")',
+    35 => 'DELETE FROM "semaphore"  WHERE ("name" = "active-trail:route:view.frontpage.page_1:route_parameters:a:2:{s:10:"display_id";s:6:"page_1";s:7:"view_id";s:9:"frontpage";}:Drupal\Core\Cache\CacheCollector") AND ("value" = "LOCK_ID")',

The menu.active_trail service is marked with needs_destruction, so it makes some sense / appears to have some relation, but not that much. What appears to have changed is that the active trail service comes into use at all during this test. And I don't understand why this change to destructable services would change whether the active trail service is used - one would think this would have been happening before, and therefore the test failing before.

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

bvoynick → created an issue.

bvoynick → created an issue.

bvoynick → created an issue.

bvoynick → changed the visibility of the branch 1.0.x to hidden.

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

bvoynick → changed the visibility of the branch 3431893-automated-drupal-11 to active.

bvoynick → changed the visibility of the branch 3431893-automated-drupal-11 to active.

bvoynick → changed the visibility of the branch 3431893-automated-drupal-11 to hidden.

While I haven't set up a full test scenario - the site I encountered this on hasn't updated to 10.2 yet - I see the same as nterbogt, looks like the change in this MR was already introduced in the 10.2.x and 11.x branches.

The use case is downstream rendering or other things that may need cache metadata.

For example, let's say I render something using text from a config. Using core's ImmutableConfig object I could write code like this:

$config = \Drupal::config('my_config');
$render_array = [
  '#plain_text' => $config->get('text_property'),
];
CacheableMetadata::createFromObject($config)
  ->applyTo($render_array);

But what if I want to use translated_config? Since TranslatedImmutableConfig doesn't implement the CacheableDependencyInterface, currently my code would have to look something like this:

$translated_config = \Drupal::service('translated_config.helper')->getTranslatedConfig('my_config');
$render_array = [
  '#plain_text' => $translated_config->get('text_property'),
  '#cache' => [
    'tags' => [
      'config:my_config',
    ],
    'contexts' => [
      'languages:language_interface',
    ],
  ],
];

Which requires me to know the right metadata to hardcode.

Or alternately, I could make my own separate call to \Drupal::config('my_config') as in the first example, using the TranslatedImmutableConfig object for values only and not metadata.

But anyhow, that's the purpose of this feature request. To have the same cache-related "developer experience," when it comes to handling cache metadata, as with the core ImmutableConfig object, where one can simply send it to the right addCacheableDependency() or similar method.

bvoynick → created an issue.

bvoynick → created an issue.

I would also like to see this reopened. Config Split cannot address the issue that uninstalling Fastly & related Purge modules, in an environment lacks the relevant credentials secrets, produces a ton of noisy errors related to inability to purge.

Pushed the simple fix to a fork. Still needs a test

bvoynick → created an issue.

Updated MR to use the method, and to fall back to the old < 4.3.0 constant if the method does not exist.

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

bvoynick → created an issue.

A use case where I'm seeing this come up is importing a snapshot of a production environment elsewhere. I just had a content restoration process on a pre-production environment killed during its drush deploy, because runtime went over 15 minutes, due to the large amount of POSTs it was attempting to chew through. (The API key for this site is provided by environment variable, in Production only, and not stored in the Drupal database. Hence, even though I am using a production database with the processors enabled, there is nevertheless no API key in the context of restoring that database in this other environment.)

The site is set up so that `drush cim` / `drush deploy` will indeed uninstall all processors, and in fact all Purge & Fastly suite modules, when in a non-Production environment. But module installation state is in the database. When importing a Prod database elsewhere, the modules will have to be actively uninstalled during configuration import. Simply uninstalling these modules is triggering these POSTs, and resulting 404s, due to the lack of service ID and API key.

Here's an alternative workaround that adds media-specific logic to LinkitHelper::getEntityFromUserInput rather than revert the change to the saved URL.

Like the reversion patches, this too is probably not the approach the module should take long term, but it will help work around this problem on sites with the canonical route turned off.

This was indeed caused by 🐛 IEF does not invoke hook_entity_prepare_form Fixed . The problem is that the *_prepare_form hooks send both an $entity object directly, and $form_state. Any hooks that make use of $form_state->getFormObject() (and $form_state->getFormObject()->getFormEntity()) will get the form object and associated entity for the main/parent entity being edited, not the inline entity that needs to be prepared. This is why it's causing multiple issues across the board.

For instance, in the case of content_moderation incompatibility, Drupal\content_moderation\EntityTypeInfo->entityPrepareForm() interacts with the form object but in doing so uses the inline $entity passed to create a new revision and then call $form_object->setEntity(), overwriting the main entity associated with the form object with a new revision of the inline entity sent to the hook.

I spent some time trying to figure out a reasonable way to work around this, but I am not sure that one exists. It would be possible to temporarily override the entity on the form object with ->setEntity() before calling hooks for an inline entity, then set it back to the original afterward. But this still doesn't address that the form object itself will still be for the main form/entity. Logic that uses the form object, such as in content_moderation, will still be incorrect.

I think there's a strong case for going ahead with reverting 🐛 IEF does not invoke hook_entity_prepare_form Fixed for now. It simply is not working as intended. A path forward for invoking that hook properly must include instantiation/use of a form object specific to the inline entity and inline entity form.

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

The ticket description says removing a tag will work to stop bot activity going forward:

If you want to use this issue as a starting point to remove deprecated API uses but then don't want new automated patches, remove the "ProjectUpdateBotD10" tag from the issue and use it like any other issue (the status does not matter then).

Since this was the only thing the bot found, removing the tag and marking this fixed seems fine? But I'll leave that choice to you @jonathan1055

@steinmb This is one of the issues noted in the table in 🌱 Dealing with unexpected file deletion due to incorrect file usage Active . And I don't know of a reason work on this cannot proceed in parallel to the others there?

As the notes column there says regarding this issue: "No data loss, but irrepairable, therefore still critical."

Rerolled #34 as a fork for Drupal 10. (Note that one can get patches from Gitlab MRs or commits, e.g. https://git.drupalcode.org/issue/drupal-2879023/-/commit/c68aa376a7fc262886cb56d9ac762e4b06e6b174.patch)

Still needs tests before it can be MR/review ready.

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

The 2.0.1 release is indeed broken under Drupal 10.x due to reinstating the core/jquery.ui* library dependencies. See follow up issue 🐛 core/jquery.ui library does not exist in Drupal 10 Fixed

Opened MR 8 with an implementation.

New jquery_ui_library_info_alter() logic replaces core library jQuery UI files with dependency declaration(s) where possible. Where that is not possible, due to the site not having the sub-module for a particular JQUI library installed, the file definition in the core library is instead replaced using the file & definition from the jquery_ui library data.

At first I tried just replacing the files that are actually present in declared libraries with dependencies - and leaving any other JQUI assets in core libraries alone. However, I found this caused ordering problems. The jquery_ui module uses a -11 weight across the board for its JS files, and the recommended approach of dependencies within a network of library definitions to establish ordering. Core's drupal.autocomplete and drupal.dialog libraries - as well as all the jquery_ui* libraries included in 9.x - take a different approach, declaring everything in one library, and using lesser weights like -11.9 and -11.8 to order files within the one library. On a test site without certain submodules like jquery_ui_autocomplete enabled, that was resulting in dependent files that were still declared on the core library - like autocomplete-min.js - being included before core library dependencies like widget-min.js that are always going to be provided by the jquery_ui/core library. Which results in javascript errors.

So I switched to re-working all assets found in core libraries, by one method or another, based on a comprehensive mapping of jquery_ui.libraries.data.json.

It does still happen with aggregation disabled. I think it's down to there being two distinct assets in the two distinct libraries.

Removing core's asset definitions and substituting dependencies on the jquery_ui module's libraries sounds like a good path forward, thanks.

Thanks, issues with MR 4170 should be addressed and ready for re-review.

Updated issue title and summary

Thank you for the report & suggestion to override hasLinkTemplate, it's coming in handy for me today.

Also running in to some of these pathauto issues @danflanagan8 is reporting.

We're using this type of logic for similar reasons: it's a very centralized way to "turn off" the canonical-having nature of specific entities, of types that need to optionally have it. Using a toUrl() override plus a #pre_render callback on the link element, most things that construct links will automatically not link to these non-existent detail pages.

It's true that an access check service, or some other approach such as overriding the access() entity method, is also necessary in case someone does make their way to the canonical route anyway. Doesn't make it unworkable though, and 2-3 places are far easier to track down than every last place an item might be output: things like the Label entity reference formatter, which otherwise needs to be overridden, and all other such programmatic toUrl() calls. (For example, even in the default Content overview, nodes that implement these kinds of overrides show up without the title being linked. Can't remember if that's just from the toUrl() override or if that relies on the #pre_render, but otherwise It's pretty fantastic.)

Going to bump Priority to Major, since this is still as severe as 🐛 Overlay blocking image upload modal in Drupal 10.1 Fixed , it just probably affects fewer people.

Opened MR 18 implementing proposed short term fix.

bvoynick → created an issue.

Opened MR 6 with proposed resolution.

bvoynick → created an issue.

Opened MR 33 adding proposed clone statement.

bvoynick → created an issue.

Thank you for the patches catch!

At my workplace, we typically dev & deploy sites similar to what sime posted in #43, with aggregated assets cacheable by exact URL for a long time. I would be very happy to see #64 go in to core - and perhaps that is the best option, since it should also fix any regressions that any folks who don't define a version number are experiencing with 10.1.

Personally I also like the idea of a setting to opt in to including the deployment_identifier in hashes, since we usually do have one set. But I imagine not everyone does.

Looks like you're using an outdated version of drupal/coder. Make sure you use the latest release, and the custom PHPCS configuration included with the module.

MR #1 simply pastes copy from the project page. I'm open to adding a hook_help() if it would be valuable, but I'm curious to see what kind of in-UI documentation y'all would like and an example of it. (And an explanation of why it is needed beyond existing project page, README, and in-settings-page text.)

MR #3 does not even seem to be talking about this module. nader.mouldi, please do not take up other's time with inaccurate work.