Problem/Motivation

If you apply a recipe which uses the addToAllBundles action but leave either the label or description, unset, there will be a warning message like Undefined array key "description" AddToAllBundles.php.

Proposed resolution

Set these by default to null.

Remaining tasks

Implement.

User interface changes

None except the warnings are not displayed.

Introduced terminology

None.

API changes

None.

Data model changes

None.

Release notes snippet

Issue fork drupal-3572679

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

liam morland created an issue. See original summary.

liam morland opened merge request !14722

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Assigned: liam morland » Unassigned
Status: Active » Needs review
sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented
Status: Needs review » Reviewed & tested by the community

Straightforward fix.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Neither value can be NULL.

The label is required, per config schema: https://git.drupalcode.org/project/drupal/-/blob/main/core/config/schema...

And the description is of the text type, which is not nullable.

Committing this as-is will generate broken configuration, and recipes that use this action will immediately break because the recipe system validates against config schema.

So what this really should look like is (pseudocode):

'label' => $value['label'] ?? throw new ConfigActionException()
'description' => $value['description'] ?? '',

Also, we definitely need a test of this.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I have implemented the changes in #5.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Looks good, but still needs a test. :)

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Yes, that is why I left it at "needs work".

neptune-dc made their first commit to this issue’s fork.

neptune-dc’s picture

neptune-dc commented

Added a test!

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
Issue tags: -Needs tests

Thanks, @neptune-dc! One question about the test, otherwise that's an easy RTBC from me.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I have addressed the question by adding the ID.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Ship it!

godotislate closed merge request !14722

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Why was the merge request closed?

  • godotislate committed b7fce18f on main 
    fix: #3572679 Prevent undefined array key warnings in AddToAllBundles...

  • godotislate committed 5ef61a66 on 11.x 
    fix: #3572679 Prevent undefined array key warnings in AddToAllBundles...

  • godotislate committed 7d7980a8 on 11.3.x 
    fix: #3572679 Prevent undefined array key warnings in AddToAllBundles...
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed b7fce18 and pushed to main, committed 5ef61a6 and pushed to 11.x, and committed 7d7980a and pushed to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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