"This block is broken or missing..." should only be shown to users that have access to do something about it

Another idea would be to use trigger_error instead, which can be suppressed on production.
But I definitely agree with the sentiment of the issue! Thanks.

In addition to a better broken blocks management in core, I just published the fixed block content module that solves the situation. The module provides a type of block based on configuration (the fixed block) that wraps a custom block (the content block). If the content block does not exist, it is created with a default content or empty if no default content was set. Fixed blocks are always present, so no "This block is broken or missing." messages to the users.

Yes, every test that installs block_content_test will get that block.

We probably should move the intentionally broken block to another module, and only use it in the tests that explicitly expect it.

I've tested this patch and it works well to hide the message from users who shouldn't see them.

Manual tests:
- admin: can see broken block
- user with block admin: can see broken block
- user without block admin: can't see broken block
- authenticated user: can't see broken block
- anonymous user: can't see broken block

Do we need to deal with the test issue to call this RTBC or can it move forward?

Simplified the https://www.drupal.org/project/drupal/issues/2918149#comment-12333965 patch As both error message should not trigger simultaneously for and also change message for anonymous user.

Simple patch to avoid showing error for non-admin user compatible with Drupal 8.6

Re-roll for 8.6.x & 8.7.x.

Please check this one.

I would say that the cache context is still needed. Un-hiding #15 for this.

I'd propose to fold this issue into #2906919: Allow users to create a block content entity with a specific UUID if it is a missing dependency of a config object.

We don’t want ordinary users to create block content. So the two issues are complementary.

The solutions that include 'trigger_error' still trigger a user error for anonymous users on my local installations (unless I completely hide all messages from anonymous users). For now #14 seems to be a valid solution where the message is hidden for anonymous users based on a permission.

#14 looks good to me. I agree to the fact that if trigger_error would be suppressed on production it need not be included in the patch. Rectifying line ending in the patch #14 so that Drupal ci test cases pass. This patch works with Drupal 9.1.x

Attaching the interdiff file (patch in #26 and #16) in this comment.

Patch #27 looks good to me.

Admin user:

Anonymus user:

Authenticated user:

1. Before it can be committed, it needs automated test coverage.

2. +++ b/core/lib/Drupal/Core/Block/Plugin/Block/Broken.php
   @@ -20,7 +20,12 @@ class Broken extends BlockBase {
   +    $build['#cache']['contexts'] = ['user.roles'];
   

   'user.permissions' is already a required cache context

Hi @timplunkett,
This patch contains the Functional Test for broken block visibility as well.

As per your comment:

user.permissions' is already a required cache context

the class Broken no longer extends BlockBase in version 9.1
Please let me know for any other required change.

I don't understand what the Broken/BlockBase change has to do with the fact that

1. The code doesn't check user roles, it checks user permissions
2. user.permissions is in required_cache_contexts: https://git.drupalcode.org/project/drupal/-/blob/9.0.x/core/core.service...

Also, please include an interdiff when making changes to a patch. Thanks!

Hi @timplunkett,

Here is the patch with changes as discussed.
Changes are :
1. Removal of user.role cache context
2. Added User permission check v.i.a dependency injection
3. Functional Test for Broken Block Visibility

Have generated an interdiff between 26 and 32 so that it shows all the changes.

Uploading a test patch to for Drupal CI test results that includes:
1. Removal of user.role cache context
2. Added User permission check v.i.a dependency injection
3. Functional Test for Broken Block Visibility
4. Changes in BlockManagerTest to use AccountProxyInterface object.

Not sure if it's the best way to fix the broken test, but would like to move this issue forward

I have applied the above patch on 9.2.x dev version ,the patch works fine
Before patch
After patch .RTBC

I went to go fix the usage of getMockBuilder and noticed that AccountProxyInterface was being used instead of AccountInterface.

No need for more manual testing as this doesn't affect the fix, but a review of my test changes is welcome.

The test changes look goods to me.
Thanks tim.plunkett

Moving to RTBC.

Committed 8e416f1 and pushed to 9.2.x. Thanks!

I'll discuss backporting once the 9.1.0 alpha is out with the release managers.

Cherry-picked to 9.1.x

In #3160629: BlockPluginTrait cannot call ::addContextAssignmentElement() itself which we want to backport to D8.9.x, we found out that the testing of this issue piggybacks on the test introduced in this issue (\Drupal\Tests\block\Functional\BlockUiTest::testBrokenBlockVisibility)

https://www.drupal.org/project/drupal/issues/3160629#comment-14048432
and
https://www.drupal.org/project/drupal/issues/3160629#comment-14048448

Therefore we like to backport this issue as well.

Since this issue is already Closed (fixed) I can't reopen it. Is there somebody around with the almighty powers to do so?

Reopening because I can, but I'm not sure above backporting this just because the other issue needs it. Not against that per se, I defer to the framework and release managers.

Thanks @tim.plunkett for re-opening.

I defer to the framework and release managers.

Tagged accordingly.

Rerolled #41 against 8.9.x. The layout builder parts of #41 don't exist in 8.9.x and aren't needed here, otherwise this applies cleanly.

Opened an issue about branch 8.9.x not passing the tests any more: #3206826: Branch 8.9.x doesn't pass tests anymore

Since Bugfix support for Drupal 8.9.x is restricted to selected low-disruption major and critical bug-fixes and this was committed to Drupal 9, changing status to Fixed.