#Needs change record

Here's another one. The patch in #86 was missing use statements. Corrected in a new patch.

@lincoln-batsirayi Try step debugging with Xdebug. Ideally configure it to break on exceptions. You can also try disable modules one by one to find a possible culprit. Sidenote: this kind of support question is somewhat out of scope of this issue, so you might take any follow-up questions elsewhere (eg #drupal-support in Slack). I'm still not really convinced that having an uncaught exception for a 403 ending up in logs is useful in this case either.

I think this is good background reading for the other issue, but let's explicitly postpone it on that one.

Yes, extending existing Render Elements classes (as proposed in 📌 Slowly, very slowly start OOPifying the render system Needs review ) looks better than adding a Builder class for each Render Element as proposed in the current MR.

Also, when we will replace most (but not all 😉) of Render & Form elements by SDC, all the logic to remove will be contained in a single file.

I spent quite some time working on making just 1 conversion, I agree with #153, this is not the way to go about it. 📌 Slowly, very slowly start OOPifying the render system Needs review is a much nicer solution.

I tried #142 but the amount of methods you'd need to add is staggering and at the end of the day since people use arbitrary properties in render arrays you would need to support some sort of set/get anyways so I leaned heavily on that and so 📌 Slowly, very slowly start OOPifying the render system Needs review is born. If that issue gets accepted this issue might be outdated.

Patch in #85 contained unrelated file. Here's the correct patch rerolled against drupal 11.1.7.

For the time being, I rerolled the patch against drupal 11.1.7.

godotislate → closed merge request !12285

catch → closed merge request !12284

Went ahead and committed/pushed this to 11.x and 11.2.x so it's in 11.2.0-rc1, thanks!

Hi all, a bit of an odd one here, but the issue being described here of the path and permission required to view a page being shown in the log message would actually be extremely useful to me right now... I’ve just installed the domain_access module and I'm also using the acb module, but unfortunately for some unknown reason to me every role on the site (accept administrator because they have the bypass permission) is unable to edit unpublished content and I’d like to know where and what exactly is causing this 403...

In my db log messages I'm just getting "Uncaught PHP Exception Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException: "" at AccessAwareRouter.php line 115" which is obviously not ideal for my situation, i can see in the code of AccessAwareRouter an empty string will be given if $access_result isn’t an instance of AccessResultReasonInterface, does anyone have any idea of why this might be the case?

I’ve got webprofiler set up to try and figure out why the node edit pages of unpublished nodes are triggering this access denied but i haven’t been able to work it out from there. So does anyone have any tips for getting to the source of such a permission issue?

I was on 10.3.14 but then i downgraded to 10.3.5 when i saw @astutonet was on that version and seeing the missing permission but no dice for me.

Made one more change here directly on the MR - deprecation was for 11.3, but this is breaking things in 11.2 with to the fallback class loader - I'm not entirely sure if the fallback classloader is making things worse, or if it's dealing with a regression elsewhere but failing to fully compensate for it.

Last couple of commits look good.

Looks like fixing docblocks in AccessGroupAnd is enough for PHPStan.

Sorry I think we should add the return types in their own issue, especially given the short amount of time before 11.2.0, back to needs work for that.

I think PHPStan was complaining about some missing types with the moved classes, so I added them. It is a break, but the classes are all @internal. I can remove the types and put in phpstan ignores as needed if preferred.

Aren't the added types a bc break considering that the old classes are automatically aliased to the new ones?

This is looking good, great work on the tests!

Pretty stupid test failure.
We check

[data-once="admin-toolbar-document-triggers-listener"]

even though we should

[data-once*="admin-toolbar-document-triggers-listener"]

because in this task data-once has two values

Please review!

For some ideas

I haven't reviewed this since my earlier comment.

Hook implementations are not covered strictly under BC. I know core sometimes still provides it.

I'm not sure either how we would do it in this case.

@smustgrave, I guess I'm not sure how to best go about implementing a deprecation since the current functionality is built into the user module (in a hook_ENTITY_TYPE_insert() implementation that gets triggered any time a user role entity is created) and not part of any API code that would be invoked directly by contrib extensions.

We could add something to trigger a deprecation warning inside the hook implementation but there wouldn't really be anything contrib extension maintainers could do to address the deprecation until functionality contained in the hook implementation is removed or moved elsewhere.

I'm also not sure whether this constitutes a major change or not. @alexpott mentioned in #18 🐛 user_user_role_insert should not exist Needs work that what user_user_role_insert() currently does isn't strictly needed:

user_user_role_insert() is doing something that does not have to happen at all. It's a UI niceness but from an API point-of-view just gets in the way.

godotislate → changed the visibility of the branch 3255804-test-only to hidden.

MR 12284 for #36 is up.
Since the test only job wasn't failing in the way expected, created test only MR 12285 that does: https://git.drupalcode.org/issue/drupal-3255804/-/jobs/5417607

CR for the class moves: https://www.drupal.org/node/3527501 →

godotislate → changed the visibility of the branch 3255804-subclass to hidden.

godotislate → changed the visibility of the branch 3255804-hidden-dependency-on to hidden.

FWIW, it looks like this Drupal CMS recipe installs layout_builder w/o block_content: https://git.drupalcode.org/project/drupal_cms/-/blob/1.x/recipes/drupal_...

+1 to #36, if we move this to core but keep it tagged internal then we solve the problem and I don't think we are adding any new debt.

Merged into 2.x and cherry-picked to 1.2.x.

thejimbirch → credited tim.plunkett → .

Adding credit for discussions in slack.

@jurgenhaas your last 2 patches barely have any code in them, are you sure they work?

thejimbirch → credited pameeela → .

One alternative would be to move these to the Core namespace. There doesn't seem to be anything particularly block_content specific in this code, either:
Drupal\block_content\Access\AccessGroupAnd
Drupal\block_content\Access\DependentAccessInterface
Drupal\block_content\Access\RefinableDependentAccessInterface
Drupal\block_content\Access\RefinableDependentAccessTrait

Should be pretty easy to do using https://www.drupal.org/node/3509577 →

There are been some reports about things failing with the fallback autoloader, such as this slack thread: https://drupal.slack.com/archives/C1BMUQ9U6/p1748549559293769.

It can apparently happen that the missing trait gets autoloaded as the fallback, but is then actually needed later and things fail when the trait doesn't provide methods that classes require for their interfaces.

We agreed to reopen this.

At this point with the upgrade path worries mostly out of the way, I'd say we should just make layout_builder depend on block_content and add an update function that enables it if that's not yet the case.

Think we will have to trigger some deprecation to warn people

Then think this needs to go back to NW for backwards compatibility can’t just break things for people immediately

• Addressed @smustgrave's MR comments
• Added the actions as exported config yml files to config/install in the standard and demo_umami install profiles
• Added the actions as exported config yml files in the administrator_role and content_editor_role recipes

@smustgrave, yes, I think if contrib module tests involve the creation of user roles, they could be affected by these changes. I think maintainers of profiles and recipes that include custom roles are even more likely to be affected.

The code in this merge request looks fine, but I think the ideal situation would be to also remove the recipes from the code base.

Like project browser can list and install projects from Drupal.org's JSON API, I feel like we should also have a browser that would list the approved Drupal CMS recipes from Drupal.org. When a user chooses to apply a recipe, the recipe is downloaded, applied, unpacked, and removed in one step.

That way, the recipe is truly ephemeral, and the user gets the latest version when they choose to browse and apply.

Added Windows 11 wording for setting.

How to manually test this:

composer create-project drupal/cms:dev-recipe-unpack -s dev --repository='{"type":"vcs","url":"https://git.drupalcode.org/issue/drupal_cms-3524255.git"}' DIRNAME

Everything should install as normal, and you should be able to install Drupal CMS, but you should also see many modules added to the top-level composer.json, and few (if any) recipes.

CR will need some work as believe we missed 11.2

Left some comments on the MR.

If the test changes were needed to make them pass then wouldn't this change potentially break contrib tests.