Problem/Motivation

The build method of BlockContentBlock uses procedural functions as follows
url
entity_view
entity_load_by_uuid

Proposed resolution

Inject the dependencies and implement ContainerFactoryPluginInterface then remove the procedural cruft
Replace url with UrlGenerator
Replace entity_view with EntityViewBuilder->view
Replace entity_load_by_uuid with EntityManager->getStorage()->loadByUuid
Bonus points: write a phpunit test for the build method

Remaining tasks

Patch

User interface changes

None

API changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rpayanm’s picture

Assigned: Unassigned » rpayanm
rpayanm’s picture

Status: Active » Needs review
FileSize
1.19 KB

Status: Needs review » Needs work

The last submitted patch, 2: Cleanup-BlockContentBlock-2342949-2.patch, failed testing.

rpayanm’s picture

Assigned: rpayanm » Unassigned
larowlan’s picture

Issue summary: View changes

Thanks, but this isn't what I was chasing

+++ b/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php
@@ -149,14 +149,14 @@ public function blockSubmit($form, FormStateInterface $form_state) {
-    if ($block = entity_load_by_uuid('block_content', $uuid)) {
-      return entity_view($block, $this->configuration['view_mode']);
+    if ($block = \Drupal::entityManager()->loadEntityByUuid('block_content', $uuid)) {
+      return \Drupal::entityManager()->getViewBuilder($block)->view($block, $this->configuration['view_mode']);
...
-          '!url' => url('block/add')
+          '!url' => \Drupal::urlGenerator()->generate('block_content.add_page'),

using \Drupal is no better than using procedural functions. We need to implement ContainerFactoryPluginInterface and inject the dependencies - updated the OP to make it explicit.

larowlan’s picture

+++ b/core/modules/block_content/src/Plugin/Block/BlockContentBlock.php
@@ -149,14 +149,14 @@ public function blockSubmit($form, FormStateInterface $form_state) {
+      return \Drupal::entityManager()->getViewBuilder($block)->view($block, $this->configuration['view_mode']);

this expects 'block_content' not $block for getViewBuilder - thats why the tests fail

rpayanm’s picture

Assigned: Unassigned » rpayanm
Status: Needs work » Needs review
FileSize
3.04 KB

Sorry for that code :) I am a novice and thank you for the comments ;)

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @rpayanm - awesome work!
No need to apologize.

  • alexpott committed f99eb04 on 8.0.x
    Issue #2342949 by rpayanm | larowlan: Cleanup BlockContentBlock.
    

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: Cleanup-BlockContentBlock-2342949-7.patch, failed testing.

larowlan’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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

rpayanm’s picture

Assigned: rpayanm » Unassigned