Problem/Motivation

Block module should react on hook_entity_bundle_delete() similar to block_configurable_language_delete and block_user_role_delete.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#45 interdiff-44-45.txt605 bytesmstrelan
#45 2549087-45.patch2.4 KBmstrelan
#44 2549087-44-test-only.patch1.44 KBmstrelan
#44 2549087-44.patch2.38 KBmstrelan
#43 2549087-43-test-only.patch1.96 KBmstrelan
#24 2549087-24.patch2.85 KBwebflo
#22 2549087-21.interdiff.txt1.91 KBwebflo
#22 2549087-21.patch2.86 KBwebflo
#19 interdiff-19.txt906 bytesjoshi.rohit100
#19 2549087-19.patch2.8 KBjoshi.rohit100
#17 interdiff-16.txt1.04 KBjoshi.rohit100
#17 2549087-16.patch3.24 KBjoshi.rohit100
#14 interdiff-14.txt641 bytesjoshi.rohit100
#14 2549087-14.patch2.77 KBjoshi.rohit100
#12 interdiff-12.txt535 bytesjoshi.rohit100
#12 2549087-12.patch2.72 KBjoshi.rohit100
#10 interdiff-10.txt738 bytesjoshi.rohit100
#10 2549087-10.patch2.35 KBjoshi.rohit100
#7 2549087-6.patch2.48 KBwebflo
#5 2549087-5.patch2.48 KBjoshi.rohit100

Comments

webflo created an issue. See original summary.

webflo’s picture

webflo commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +Needs tests

+1

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Assigned: Unassigned » joshi.rohit100
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Active » Needs review
StatusFileSize
new 2549087-5.patch2.48 KB

Status: Needs review » Needs work

The last submitted patch, 5: 2549087-5.patch, failed testing.

webflo’s picture

webflo commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2549087-6.patch2.48 KB 
+++ b/core/modules/block/block.module
@@ -281,3 +281,20 @@ function block_configurable_language_delete(ConfigurableLanguageInterface $langu
+    if (isset($visibility['node_type']['bundle'][$node_type->id()])) {

Should be 'bundles' according to Drupal\node\Plugin\Condition\NodeType

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented 
+++ b/core/modules/block/src/Tests/BlockTest.php
@@ -480,4 +481,41 @@ public function testBlockUserRoleDelete() {
+    $node_type1 = NodeType::create(['id' => 'test_nodetype1', 'name' => $this->randomString()]);
+    $node_type1->save();

I think, this should be $node_type = $this->drupalCreateContentType();

?

Status: Needs review » Needs work

The last submitted patch, 7: 2549087-6.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2549087-10.patch2.35 KB
new interdiff-10.txt738 bytes

Status: Needs review » Needs work

The last submitted patch, 10: 2549087-10.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2549087-12.patch2.72 KB
new interdiff-12.txt535 bytes

Status: Needs review » Needs work

The last submitted patch, 12: 2549087-12.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2549087-14.patch2.77 KB
new interdiff-14.txt641 bytes

Again ):

Status: Needs review » Needs work

The last submitted patch, 14: 2549087-14.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Try adding \Drupal::moduleHandler()->install(['node']); at the top of the test.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2549087-16.patch3.24 KB
new interdiff-16.txt1.04 KB
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented

@tim :- Sorry I saw your feedback after uploading the patch. :(

So should I enable the node module globably or only for this test ?

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
StatusFileSize
new 2549087-19.patch2.8 KB
new interdiff-19.txt906 bytes

Here is the 'moduleHandler' version with interdiff from #17.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work 
+++ b/core/modules/block/block.module
@@ -281,3 +282,20 @@ function block_configurable_language_delete(ConfigurableLanguageInterface $langu
+  foreach (Block::loadMultiple() as $block) {
...
+    if (isset($visibility['node_type']['bundles'][$node_type->id()])) {

Instead of loading all of the blocks, we can use EQ:

    $ids = \Drupal::entityQuery('block')
      ->condition('visibility.node_type.bundles.' . $node_type->id(), $node_type->id())
      ->execute();
    foreach (Block::loadMultiple($ids) as $block) {

+++ b/core/modules/block/src/Tests/BlockTestBase.php
@@ -19,7 +19,7 @@
-  public static $modules = array('block', 'filter', 'test_page_test', 'help', 'block_test');
+  public static $modules = array('block', 'filter', 'test_page_test', 'help', 'block_test', 'node');

I don't think installing the node module for *every* test is the right thing to do. We want it just for this one test.

The last submitted patch, 19: 2549087-19.patch, failed testing.

webflo’s picture

webflo commented
Status: Needs work » Needs review
StatusFileSize
new 2549087-21.patch2.86 KB
new 2549087-21.interdiff.txt1.91 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/modules/block/src/Tests/BlockTest.php
@@ -486,7 +486,7 @@ public function testBlockUserRoleDelete() {
-    \Drupal::moduleHandler()->install(['node']);
+    \Drupal::service('module_installer')->install(['node']);

Why change this? Not important to fix, just seems unnecessary.

+++ b/core/modules/block/block.module
@@ -6,12 +6,13 @@
+use Drupal\node\NodeTypeInterface;;

Two ;;

webflo’s picture

webflo commented
StatusFileSize
new 2549087-24.patch2.85 KB

Why change this? Not important to fix, just seems unnecessary.

Hmm. I don´t understand ModuleHandlerInterface has no install method since #2324055: Split up the module manager into runtime information and extension information. got in.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

You're absolutely right, my apologies.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

I think this problem is exposing a missing configuration dependency. Block has a dependency on the node type and it's not being added to the block configuration. This should then be fixed an onDependencyRemoval implementation for the block config entity.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

We also should create a new issue to handle the missing language dependency and remove block_configurable_language_delete

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

And actually same for block_user_role_delete

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Btw the massive advantage of doing this is when implemented it will tell you that a block will affected when one of these things is deleted.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +missing config dependencies, +Configurables

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mstrelan’s picture

mstrelan commented
Title: Block module should remove node_type visibility conditions » Block module should remove entity_bundle visibility conditions
Version: 9.5.x-dev » 10.1.x-dev
Issue tags: +Bug Smash Initiative, +Needs steps to reproduce
StatusFileSize
new 2549087-43-test-only.patch1.96 KB

The node_type visibility condition no longer exists, see Node type condition plugin deprecated by generic entity bundle condition.

I've re-rolled the test part of the patch and uploaded to confirm this is still an issue.

mstrelan’s picture

mstrelan commented
Issue summary: View changes
StatusFileSize
new 2549087-44.patch2.38 KB
new 2549087-44-test-only.patch1.44 KB

Updating the fix as well.

mstrelan’s picture

mstrelan commented
StatusFileSize
new 2549087-45.patch2.4 KB
new interdiff-44-45.txt605 bytes

Fix phpstan issues

mstrelan’s picture

mstrelan commented

@alexpott are you suggesting in #26-30 that this patch takes a different direction? Or that we roll one or more follow up issues?

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.