Updated: Comment #0

Problem/Motivation

\Drupal\custom_block\Plugin\Block\CustomBlockBlock depends on the module hanlder to check whether block.module is enabled. But custom_block.module depends on block.module so this is pointless.

This was changed a few times, most recently in #2003058: Replace drupal_container() with Drupal::service() in the block module, but was originally introduced in #1871772: Convert custom blocks to content entities.

Proposed resolution

Remove the dependency on the module handler.

Remaining tasks

Write patch.
Review patch.

#2003058: Replace drupal_container() with Drupal::service() in the block module
#1871772: Convert custom blocks to content entities

CommentFileSizeAuthor
#8 2069619-8.patch2.82 KBrpayanm
#8 2069619-interdiff.txt700 bytesrpayanm
#5 2069619-5.patch2.75 KBrpayanm
#1 2069619-1-custom-block-block-module-handler.patch2.21 KBtstoeckler

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 2069619-1-custom-block-block-module-handler.patch2.21 KB

Here we go. At least CustomBlockCreationTest passes on my machine, let's see what the bot says.

alansaviolobo queued 1: 2069619-1-custom-block-block-module-handler.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2069619-1-custom-block-block-module-handler.patch, failed testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Title: Remove the module handler dependency from CustomBlockBlock » Remove the module handler dependency from BlockContentBlock
Issue summary: View changes
Issue tags: +Needs reroll, +Novice

This is still a valid issue. The module was renamed in the meantime, though, so the class is now BlockContentBlock. Marking Novice, as the previous patch already outlines what needs to be done, the equivalent code just needs to be identified in BlockContentBlock now.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2069619-5.patch2.75 KB

Trying...

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Awesome, looks perfect already. Just one thing:

+++ b/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php
@@ -140,10 +129,7 @@ public function blockForm($form, FormStateInterface $form_state) {
-      $this->configuration['view_mode'] = $form_state->getValue('view_mode');

This should be kept as well.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Needs work
rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2069619-interdiff.txt700 bytes
new 2069619-8.patch2.82 KB

Ups...sorry :)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Reviewed & tested by the community

Perfect. I worked on the original version of the patch, but since this had to be recreated from scratch I think it's OK for me to RTBC this.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Task » Bug report
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs reroll

Unneeded dependencies is a bug not a task. This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?. Committed f0a7c35 and pushed to 8.0.x. Thanks!

  • alexpott committed f0a7c35 on 8.0.x 
    Issue #2069619 by rpayanm, tstoeckler: Remove the module handler...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.