Prevent empty block_content info fields from causing php deprecation notices when placing blocks with no label.

Created on 8 February 2023, over 1 year ago
Updated 12 April 2024, 2 months ago

Problem/Motivation

Error when adding a custom block entity to the block layout when no block description have been added (i.e., the field base was hidden from the form).

Error stack:

Deprecated function: str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in Drupal\Component\Transliteration\PhpTransliteration->transliterate() (line 135 of /app/web/core/lib/Drupal/Component/Transliteration/PhpTransliteration.php)

#0 /app/web/core/includes/bootstrap.inc(164): _drupal_error_handler_real(8192, 'str_contains():...', '/app/web/core/l...', 135)
#1 [internal function]: _drupal_error_handler(8192, 'str_contains():...', '/app/web/core/l...', 135)
#2 /app/web/core/lib/Drupal/Component/Transliteration/PhpTransliteration.php(135): str_contains(NULL, '?')
#3 /app/web/core/lib/Drupal/Core/Block/BlockPluginTrait.php(254): Drupal\Component\Transliteration\PhpTransliteration->transliterate(NULL, 'x-default', '_')
#4 /app/web/core/modules/block/src/BlockForm.php(388): Drupal\Core\Block\BlockBase->getMachineNameSuggestion()
#5 /app/web/core/modules/block/src/BlockForm.php(136): Drupal\block\BlockForm->getUniqueMachineName(Object(Drupal\block\Entity\Block))
#6 /app/web/core/lib/Drupal/Core/Entity/EntityForm.php(107): Drupal\block\BlockForm->form(Array, Object(Drupal\Core\Form\FormState))
#7 [internal function]: Drupal\Core\Entity\EntityForm->buildForm(Array, Object(Drupal\Core\Form\FormState))
#8 /app/web/core/lib/Drupal/Core/Form/FormBuilder.php(536): call_user_func_array(Array, Array)
#9 /app/web/core/lib/Drupal/Core/Form/FormBuilder.php(283): Drupal\Core\Form\FormBuilder->retrieveForm('block_form', Object(Drupal\Core\Form\FormState))
#10 /app/web/core/lib/Drupal/Core/Entity/EntityFormBuilder.php(48): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\block\BlockForm), Object(Drupal\Core\Form\FormState))
#11 /app/web/core/modules/block/src/Controller/BlockAddController.php(27): Drupal\Core\Entity\EntityFormBuilder->getForm(Object(Drupal\block\Entity\Block))

Steps to reproduce

  1. Create or edit a custom block type
  2. Go to Manage Form Display
  3. Hide the 'Block Description' field
  4. Go to `/block/add` and choose the custom block type you just edited/created
  5. Fill the required fields and then press `Save`
  6. Place the newly created block to layout (admin/structure/block/library/olivero)

You can also trigger it when adding an existing custom block entity through the Block Layout UI.

Proposed resolution

Because BlockContent entities allow to hide the Block Description (the entity label) field from the form, add a fallback value to handle cases where the field is hidden, to avoid deprecated errors. The fallback value will be the block content bundle label and the newly created entity ID. (e.g Basic block: 123

Summary

The current issue happens when you hide the Block Description field from the form display in a Block Content type. The field can be hidden through the UI.

If you do so, and create a Block Content entity and then place it into the block layout, a deprecated notice will be thrown.

This is error is triggered due to the fact that in code, when creating a block, the Block Content Block definition tries to add into the Block Description the block content entity label (i.e., the Block Description field). That plugin definition will be later used to suggest the machine name for the Block Content Block, which will throw the deprecated notice due to the fact of that definition being empty, as the field was never filled because it's hidden.

The proposed resolution is to add a fallback value in the Block Content Block derivative discovery, adding into the admin_label definition said fallback value. This fallback value is composed by the Block Content type and the entity ID. This way we avoid the deprecation errors and also handling cases where the entity label is empty.

🐛 Bug report
Status

Fixed

Version

10.2

Component
Block content 

Last updated 38 minutes ago

Created by

🇺🇦Ukraine ooa33

Live updates comments and jobs are added and updated live.
  • PHP 8.1

    The issue particularly affects sites running on PHP version 8.1.0 or later.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @ooa33
  • @ooa33 opened merge request.
  • 🇺🇸United States cilefen

    What are the steps to reproduce? We have had a lot of these issues providing defensive patches but we are more interested in knowing why it occurs.

  • Status changed to Postponed: needs info over 1 year ago
  • 🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

    #4 mentions needing steps to reproduce.

  • 🇨🇷Costa Rica robertarias

    I'm experiencing the same issue, this is the error stack:

    Deprecated function: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in Drupal\Component\Transliteration\PhpTransliteration->transliterate() (line 135 of core/lib/Drupal/Component/Transliteration/PhpTransliteration.php).
    Drupal\Component\Transliteration\PhpTransliteration->transliterate(NULL, 'x-default', '_') (Line: 254)
    Drupal\Core\Block\BlockBase->getMachineNameSuggestion() (Line: 390)
    Drupal\block\BlockForm->getUniqueMachineName(Object) (Line: 137)
    Drupal\block\BlockForm->form(Array, Object) (Line: 106)
    Drupal\Core\Entity\EntityForm->buildForm(Array, Object)
    call_user_func_array(Array, Array) (Line: 534)
    Drupal\Core\Form\FormBuilder->retrieveForm('block_form', Object) (Line: 281)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 48)
    Drupal\Core\Entity\EntityFormBuilder->getForm(Object) (Line: 27)
    Drupal\block\Controller\BlockAddController->blockAddConfigureForm('block_content:9252e0db-83d7-421b-b410-60cfa0629557', 'olivero')
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 48)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 686)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
    

    When creating custom block types, you have the choice to hide the block description field that comes along with the 'Block Content' block, i.e., the 'Admin Label', this is being the case since D8, if I'm not mistaken. There are some use cases when the Drupal builder decides to hide description from the form (though it's not the best decision as that field definition is the one used to identify the entity through the UI; it acts as the entity title). When you choose to hide it, then create an entity of that block, and finally try to add it to the block layout, the error will be triggered.

    The reason why is due to the `BlockForm` trying to generate a machine name suggestion. On `BlockPluginTrait::getMachineNameSuggestion`, we make the assumption that the `admin_label` definition (meaning the 'Block Description') is set, although we can hide it from the form when creating the custom block type (but when you decide to keep it, it is indeed required).

    I have updated the instructions on how to replicate it.

  • Status changed to Needs review about 1 year ago
  • Status changed to Needs work about 1 year ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • 🇺🇸United States smustgrave

    Thanks for the steps to reproduce.

    As a bug will also need a test case showing the issue.

  • First commit to issue fork.
  • 🇨🇷Costa Rica robertarias

    Updated patch with test.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    28,516 pass, 1 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update about 1 year ago
    28,520 pass
  • Status changed to Needs review about 1 year ago
  • 🇮🇹Italy apaderno Brescia, 🇮🇹
  • Status changed to Postponed: needs info about 1 year ago
  • 🇺🇸United States smustgrave

    I tried replicating the issue on 10.1.x following the steps in the issue summary but was not able to trigger the error.

    What additional steps are there?

  • 🇨🇷Costa Rica robertarias

    I've tested it with 10.1.x and the issue still persists. I have updated the steps in the issue summary to be more clear.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    Custom Commands Failed
  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    See smustgrave's comment.

    I tried replicating the issue on 10.1.x following the steps in the issue summary but was not able to trigger the error.

    What additional steps are there?

  • Status changed to Needs work 11 months ago
  • 🇷🇺Russia Chi

    I managed to reproduce this on 10.1.x. To get the error you need to open "Place block" form.

  • 🇷🇺Russia Chi

    + * @return string|null
    + * The suggested machine name or NULL if not applicable.

    I would try to avoid nullable types wherever possible. They will bite as later with extra conditions. If block description is not available we can return empty string or block plugin ID.
    Something like this.

    $admin_label = $definition['admin_label'] ?: $definition['id'];
    

    I think the test is too indirect. The bug belongs to BlockPluginTrait. No need to instantiate transliteration service to get it.
    The test should just verify output of \Drupal\Core\Block\BlockPluginTrait::getMachineNameSuggestion(), including the case when admin label is not available.

  • 🇷🇺Russia Chi

    The bug belongs to BlockPluginTrait.

    Actually it does not. There are other places where the "admin label" is always expected.
    https://git.drupalcode.org/project/drupal/-/blob/10.1.1/core/lib/Drupal/...

    Note that description field in block content entity is mandatory. So hiding it produces invalid block entities.

    Given that I think the proper fix should ensure that blocks are never have empty admin label.

  • 🇨🇷Costa Rica robertarias

    I agree on your feedback and assessment! I had already noticed the info field (the value used to set the admin_label value in BlockContentBlock block plugin) in the BlockEntity entity is required, I just took another route because I didn't want to add a fallback value into an entity. But I agree on that admin_label field shouldn't be empty in BlockContentBlock plugins.

    I think the best route would be to add a fallback option into the Drupal\block_content\Plugin\Derivative\BlockContent::getDerivativeDefinitions() function, where the admin_label is set. If the BlockContent entity label is null, fallback to a generated admin label. The value could be built from the BlockEntity values such as the id and the entity bundle label. This way, we don't have to modify the \Drupal\Core\Block\BlockPluginTrait::getMachineNameSuggestion() and we don't mind whether the block label is hidden from the block entity.

  • 🇷🇺Russia Chi

    Re #26. Adding fallback to Drupal\block_content\Plugin\Derivative\BlockContent::getDerivativeDefinitions() looks like the right approach.

  • 🇨🇷Costa Rica robertarias

    Patches addressing feedback from #25 and #26.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    Custom Commands Failed
  • 🇨🇷Costa Rica robertarias

    I attached the wrong patches. My apologies. Re-submitting.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    29,448 pass
  • 🇺🇸United States mikelutz Michigan, USA

    So, I've spent a few hours digging into this, and it is kind of interesting. I changed the title because this doesn't use strpos anymore, it triggers the same notice on str_contains now. Ultimately this patch would fix the issue listed here, and while I have some nits over the patch and tests which I'll get to, I'm not fully convinced this is the right approach. While this gets closer to the root issue than just handling nulls in transliteration or in the machine name suggestions, the bigger issue is that we use the baseFieldDefinitions of content entities to set the label to be required in the entity form, yet allow it to be configured (i.e. hidden on the form) and then treat it in code like it's actually required. To compare and contrast block_content entities with nodes and terms, all three set the form field for the label as required (node:title, term:name, block_content:info) and yet allow the form display to be configurable such that the field can be removed from the form. The difference between blocks and terms and nodes is that terms and nodes define their own storage_schema handlers that set the title and name fields in the database to be not nullable, and if you go through this process with either (hide the node title or term name on the entity form), you actually end up with a sql exception along the lines of `Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'name' cannot be null: INSERT INTO "taxonomy_term_field_data"`. See Do not require a 'title' field Needs work

    Ultimately it feels like display configurable base fields that are required should be restricted from being hidden in the form display UI. But then we need to go entity by entity and decide how required the labels actually are, and I feel like for block content entities, maybe not so much, and it seems like a fairly common use case to want to hide the info field in the block_content entity form. If we want to make that field not required, and we don't want to compute a dummy label at the content entity level, then computing a default at the plugin level seems reasonable. I'll note here that this does change the administrative experience when placing blocks, as blocks that would previously have had an empty label in the selection form will now have <block type> <id> as a label.

    1. +++ b/core/modules/block_content/src/Plugin/Derivative/BlockContent.php
      @@ -49,7 +50,7 @@ public function getDerivativeDefinitions($base_plugin_definition) {
      -      $this->derivatives[$block_content->uuid()]['admin_label'] = $block_content->label();
      

      Went back and forth on this in my head. I'm not a huge fan of using $block_content->type directly here to get the bundle config entity, but we do it elsewhere in core, and more methodical ways of getting it (i.e. \Drupal::entityTypeManager()->getStorage('block_content_type')->load($block_content->bundle())->label() or $block_content->get($block_content->getEntityType()->getKey('bundle'))->referencedEntities[0]->label() seem overly cumbersome here. I also considered whether we actually needed the human readable bundle label, or if we could just use the bundle machine name, or even just falling back to the uuid, but this string is going to end up on the administrative interface in the list of options when you click 'Place Block', so this seems appropriate That said, I don't think we need a separate method here just to do a simple string concatenation that we only need to execute in one place. So, I think I would just go with

      $this->derivatives[$block_content->uuid()]['admin_label'] = $block_content->label()
        ?? $block_content->type->entity->label() . ' ' . $block_content->id();
      

      or

      $this->derivatives[$block_content->uuid()]['admin_label'] = $block_content->label()
        ?? sprintf('%s %s', $block_content->type->entity->label(), $block_content->id());
      
    2. +++ b/core/modules/block_content/tests/src/Kernel/BlockContentDeriverTest.php
      @@ -62,4 +92,72 @@ public function testReusableBlocksOnlyAreDerived() {
      +    $this->blockContentStorage->expects($this->any())
      

      So, because we are creating actual block content entities and saving them here, we really don't need to mock the block content storage, we can use the real one. i.e. $definitions = (new DerivativeBlockContent(\Drupal::entityTypeManager()->getStorage('block_content')))->getDerivativeDefinitions($this->baseDefinition);
      Also, inside the foreach($block_content_entities as $entity) loop, we can do $plugin = \Drupal::service('plugin.manager.block')->createInstance('block_content:' . $entity->uuid()); and get the actual plugin. We could just check the derived definition there with $plugin->getPluginDefinition() and we can also directly call $plugin->getMachineNameSuggestion() and do some assertions on that which ultimately would trigger the original deprecation notice that started this issue in the first place, and would make for a better 'proof of bug and fix' test.

    3. +++ b/core/modules/block_content/tests/src/Kernel/BlockContentDeriverTest.php
      @@ -62,4 +92,72 @@ public function testReusableBlocksOnlyAreDerived() {
      +  public function testAdminLabelFallbackFunction() {
      

      Really, this should probably be a unit test that passes in a mocked BlockContent object, or just get rid of it if we are going to put the concatenation inline instead of making a new method.

    4. +++ b/core/modules/block_content/tests/src/Kernel/BlockContentDeriverTest.php
      @@ -62,4 +92,72 @@ public function testReusableBlocksOnlyAreDerived() {
      +  protected function getMockedBlockContentEntities() {
      

      nit: We aren't creating mocked block content entities here, we are creating and saving actual entities, so something like getTestBlockContentEntities() would be a better name.

  • last update 11 months ago
    Custom Commands Failed
  • @robert-arias opened merge request.
  • Status changed to Needs review 11 months ago
  • 🇨🇷Costa Rica robertarias

    @mikelutz thanks for the thorough review and feedback!
    I agree with your assessment; that's one of the reasons I took the first approach - I think the bigger issue is that of being able to hide required fields, but because I noticed other entities allow that behavior, I decided to go on this approach.

    As per your review: 1) yeah, I noticed doing it the more methodical way is too cumbersome, that's why I decided to do it that way. I actually did think whether to create a function or make the fallback code inline, I even thought of making it static in case this could be used on other places, but being honest I don't see it being used in other places, so I agree on making it inline. 2)3)4): I agree on these ones, thank you!

    I've applied the recommended changes on the 3340159-block-content-deprecation-notices branch on the issue fork.

  • last update 11 months ago
    29,456 pass
  • Status changed to Needs work 11 months ago
  • 🇺🇸United States smustgrave

    Can the MR be updated for 11.x please

  • 3:43
    0:59
    Running
  • 57:34
    53:45
    Running
  • 56:28
    53:37
    Running
  • Status changed to Needs review 11 months ago
  • 🇨🇷Costa Rica robertarias

    Done! I created a new branch with the 11.x base following the recommended guidelines , but I can change the base of the original MR to 11.x if needed.

  • Status changed to RTBC 11 months ago
  • 🇺🇸United States smustgrave

    Thanks!

    Verified the bug following the steps from the issue summary

    Create or edit a custom block type
    Go to Manage Form Display
    Hide the 'Block Description' field
    Go to `/block/add` and choose the custom block type you just edited/created
    Fill the required fields and then press `Save`
    Place the newly created block to layout (admin/structure/block/library/olivero)
    You can also trigger it when adding an existing custom block entity through the Block Layout UI

    Applying MR fixed the problem.

    And seems a deep review was done in #30

  • last update 11 months ago
    29,954 pass
  • last update 10 months ago
    29,959 pass
  • last update 10 months ago
    29,959 pass
  • last update 10 months ago
    29,959 pass
  • last update 10 months ago
    29,960 pass
  • last update 10 months ago
    29,968 pass
  • last update 10 months ago
    30,050 pass
  • last update 10 months ago
    30,057 pass
  • last update 10 months ago
    30,057 pass
  • Status changed to Needs work 10 months ago
  • 🇳🇿New Zealand quietone New Zealand

    I'm triaging RTBC issues .

    I read the issue summary and the comments. I think the proposed resolution needs more detail to explain how this is being fixed. And after the review by @mikelutz in #30, which resulted in code changes, I don't see that anyone has down another code review. Did I miss it?

    I looked at the MR, and left some comments. That is not a thorough code review. There are some things that need to change so setting to NW.

    Also changing component.

  • 🇨🇦Canada Laura Johnson Toronto

    I am using the 10.1 patch from #29 and it's working well. Thanks @robert-arias!

  • Pipeline finished with Success
    4 months ago
    Total: 2290s
    #109336
  • Pipeline finished with Success
    4 months ago
    Total: 476s
    #109949
  • Status changed to Needs review 4 months ago
  • 🇨🇷Costa Rica robertarias

    Thanks for the feedback, @quietone. I addressed your points, and added a more detailed proposed solution we came after some reviews.

    Changing it to Needs Review.

  • 🇦🇺Australia acbramley

    acbramley changed the visibility of the branch 3340159-block-content-deprecation-notices to hidden.

  • 🇦🇺Australia acbramley

    acbramley changed the visibility of the branch 3340159-deprecated-function-strpos to hidden.

  • 🇦🇺Australia acbramley

    acbramley changed the visibility of the branch 10.1.x to hidden.

  • Status changed to Needs work 4 months ago
  • 🇦🇺Australia acbramley

    Hiding old branches and patches, review to follow. We also need an IS update describing the latest solution and why this is happening.

    In my opinion, this sounds like a bit of an edge case issue. Should we really handle this in core? There are plenty of contrib/custom solutions for automatically pre-filling an entity's label ( https://www.drupal.org/project/auto_entitylabel comes to mind).

    As @mikelutz states in #30 - this is more of a mismatch between DB schema and base field definitions (required flag) than handling an empty label in a plugin definition.

    The only time I've needed an empty admin label for a block content entity is in layout_builder context, which doesn't run into this issue since inline blocks don't create derivatives.

  • 🇨🇷Costa Rica robertarias

    Sure, this is the latest update:

    The current issue, as described in the in the description and throughout the discussion, happens when you hide the Block Description field from the form display in a Block Content type. The field can be hidden through the UI.

    If you do so, and create a Block Content entity and then place it into the block layout, a deprecated notice will be thrown. The error is this one:

    Deprecated function: str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in Drupal\Component\Transliteration\PhpTransliteration->transliterate() (line 135 of /app/web/core/lib/Drupal/Component/Transliteration/PhpTransliteration.php)

    This is error is triggered due to the fact that in code, when creating a block, the Block Content Block definition tries to add into the Block Description the block content entity label (i.e., the Block Description field). That plugin definition will be later used to suggest the machine name for the Block Content Block, which will throw the deprecated notice due to the fact of that definition being empty, as the field was never filled because it's hidden.

    The proposed resolution is to add a fallback value in the Block Content Block derivative discovery, adding into the admin_label definition said fallback value. This fallback value is composed by the Block Content type and the entity ID. This way we avoid the deprecation errors and also handling cases where the entity label is empty.

    I will also add this update into the summary.

    And for your assessment, I disagree. I don't think this is an edge case, mainly by the fact that cores allows to hide the field without complaining, plus the lack of storage schema handlers that would make the info field required in the database as well. I think the discussion on whether this mismatch should be addressed through making the info field non-nullable at the database level is a fair discussion, but it's out of scope for this ticket in my opinion; and a similar discussion has started in Do not require a 'title' field Needs work .

    Ultimately, I think cases where the block content info field is to be hidden, computing a default value at the plugin level seems reasonable.

  • Status changed to Needs review 4 months ago
  • Status changed to Needs work 3 months ago
  • 🇦🇺Australia acbramley

    Had a chat with @larowlan about this and we are happy to go ahead with the fallback in the form of block_content:123

    I agree with @quietone that the deprecation testing is odd, we should be able to simply call the function that triggers the error. If it produces a deprecation error the test would fail.

    I will work on these changes today.

    Added a related issue with Shortcuts.

  • Status changed to Needs review 3 months ago
  • 🇦🇺Australia acbramley

    Pushed simplifications to the test and rebased against 11.x

    Tossed up between the fallback, but I think it would be useful to keep the bundle's label (although what happens when that is empty?)

    Back to NR

  • Pipeline finished with Success
    3 months ago
    Total: 499s
    #125692
  • Status changed to RTBC 3 months ago
  • 🇺🇸United States smustgrave

    For the shortcut issue we ended up setting a default to the database and an update hook.

    Not sure the correct approach but this fix does address the problem at hand. I think the label of bundle: ID is perfect. I was a little torn because most people might not know the block ID but if you're in the block layout I think it's a fair assumption you know what you're doing and can find out the ID.

    LGTM

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Committed b0cef94 and pushed to 10.2.x. Thanks!
    Committed ed13e94 and pushed to 10.3.x. Thanks!
    Committed 7cf9fb4 and pushed to 11.x. Thanks!

    Backported to 10.2.x as a non-disruptive bug fix.

    • alexpott committed b0cef942 on 10.2.x
      Issue #3340159 by robert-arias, ooa33, acbramley, smustgrave, Chi,...
    • alexpott committed ed13e942 on 10.3.x
      Issue #3340159 by robert-arias, ooa33, acbramley, smustgrave, Chi,...
  • Status changed to Fixed 3 months ago
    • alexpott committed 7cf9fb43 on 11.x
      Issue #3340159 by robert-arias, ooa33, acbramley, smustgrave, Chi,...
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024