Undefined variable in the file block.api.php

CommentFileSizeAuthor
#14 block-api-php-undefined-2934527-14.patch577 bytesmsankhala
#9 evidence_01.png96.38 KBemartoni
#2 drupal_block-2934527-2.patch566 bytespifagor
Screenshot from 2018-01-05 01:51:11.png265.23 KBpifagor
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pifagor created an issue. See original summary.

pifagor’s picture

pifagor
Location 🇺🇦 Rivne
CreditAttribution: pifagor as a volunteer and at Drupal Ukraine Community for Drupal Ukraine Community commented
FileSize
drupal_block-2934527-2.patch566 bytes

I created patch

pifagor’s picture

pifagor
Location 🇺🇦 Rivne
CreditAttribution: pifagor as a volunteer and at Drupal Ukraine Community for Drupal Ukraine Community commented
Component: blog.module » block.module
pifagor’s picture

pifagor
Location 🇺🇦 Rivne
CreditAttribution: pifagor as a volunteer and at Drupal Ukraine Community for Drupal Ukraine Community commented
Status: Active » Needs review

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

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.

pifagor’s picture

pifagor
Location 🇺🇦 Rivne
CreditAttribution: pifagor as a volunteer and at Drupal Ukraine Community for Drupal Ukraine Community commented
Assigned: pifagor » Unassigned
pifagor’s picture

pifagor
Location 🇺🇦 Rivne
CreditAttribution: pifagor as a volunteer and at Drupal Ukraine Community, GOLEMS GABB for Drupal Ukraine Community, GOLEMS GABB commented

Please review this patch

cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented
Issue tags: +Novice

This will attract reviewers.

emartoni’s picture

emartoni CreditAttribution: emartoni at CI&T commented
Status: Needs review » Reviewed & tested by the community
FileSize
evidence_01.png96.38 KB

Worked for me!

cilefen’s picture

cilefen CreditAttribution: cilefen as a volunteer commented
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/block/block.api.php
@@ -149,6 +149,7 @@ function hook_block_view_BASE_BLOCK_ID_alter(array &$build, \Drupal\Core\Block\B
+  $some_condition = TRUE;
   // Add the 'user' cache context to some blocks.
   if ($some_condition) {

Hm, is this firing some warning in your IDE without the patch?

I mean it is not a better code example this way either :) If you copy this code, it will run fine but why have the condition in the first place in this setup?

Should we add a fake call to some fake service so it looks more logical? (Would that not fire a warning in your IDE?)

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Issue summary: View changes

#11: good suggestion! Something like this:

- if ($some_condition) {
+ if ($block->label() === 'some condition') {

Edit: Wow. I see 'View changes' tag was added to this post. And IS was updated (new image). But this is not my merit. I have 0 attached files. Looks like @emartoni added this image and updated IS.. via me! David Blaine?

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs review » Needs work

Moving this to NW because of suggestion in #12.

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Status: Needs work » Needs review
FileSize
block-api-php-undefined-2934527-14.patch577 bytes

Here is updated patch.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Reviewed & tested by the community

Thank you, @msankhala! #12 is relevant to #11. So, all feedbacks have been addressed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Title: Undefined variable » Improve example code in block.module

Improving the title.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented

Crediting @Gábor Hojtsy for the review that influenced the direction of the patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 13ef6d3dce to 8.6.x and 0eaf3c7982 to 8.5.x. Thanks!

Backported to 8.5.x as a docs only fix.

  • alexpott committed 13ef6d3 on 8.6.x 
    Issue #2934527 by pifagor, msankhala, Gábor Hojtsy: Improve example code...

  • alexpott committed 0eaf3c7 on 8.5.x 
    Issue #2934527 by pifagor, msankhala, Gábor Hojtsy: Improve example code...
lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented 
+++ b/core/modules/block/block.api.php
@@ -150,7 +150,7 @@ function hook_block_view_BASE_BLOCK_ID_alter(array &$build, \Drupal\Core\Block\B
+  if ($block->label() === 'some condition') {

would the label really be 'some condition'. Wouldn't the label just be 'some text' or some made up name?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented

@lokapujya does it matter?

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented

The best practice would have been to get a new animal name into Core. It is just a missed opportunity. - haha

msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented

I think this should not make much difference because this is just an example code. One must modify code before using it in actual module.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

#23: On the contrary, now it became opportunity to give the animal a unique name ;)

Status: Fixed » Closed (fixed)

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