- Merge request !4334Issue #2990549: Overriding configuration variable to empty array has no effect → (Open) created by moshe weitzman
- Open on Drupal.org →Environment: PHP 8.2 & MySQL 8last update
over 1 year ago Not currently mergeable. - last update
over 1 year ago Build Successful - last update
over 1 year ago Build Successful - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago 29,822 pass - Status changed to Needs review
over 1 year ago 3:48pm 19 July 2023 - 🇺🇸United States moshe weitzman Boston, MA
In order to overcome errors with overrides that already return `[]`, I had to change the override indicator to \EmptyIterator(). This is slightly less elegant but good enough and breaks no existing code. Tests are now passing.
- Status changed to Needs work
over 1 year ago 5:20pm 19 July 2023 - 🇺🇸United States smustgrave
Can the issue summary be updated with the modern template, to include the proposed solution please
- Status changed to Needs review
over 1 year ago 5:43pm 19 July 2023 - Status changed to Needs work
over 1 year ago 11:40am 21 July 2023 - 🇵🇱Poland Graber
Tested this a bit, unfortunately EmptyIterator is not supported everywhere, got:
TypeError: in_array(): Argument #2 ($haystack) must be of type array, EmptyIterator given in in_array() (line 123 of core/modules/user/src/Entity/Role.php).
when saving permissions with overridden role config, it may occur on different occasions in different places as well if we map empty config this way, hard to tell really. - Status changed to Needs review
over 1 year ago 12:08pm 21 July 2023 - 🇵🇱Poland Graber
Ok, my mistake, haven't applied the patch properly, back to previous status.
- Status changed to Needs work
over 1 year ago 12:17pm 21 July 2023 - 🇵🇱Poland Graber
Just got back from holiday so forgive the struggle. My previous comment vas valid after all, suggestion added.
- Open on Drupal.org →Environment: PHP 8.2 & MySQL 8
22:11 29:10 Queued - Status changed to Needs review
over 1 year ago 12:38pm 21 July 2023 - 🇺🇸United States moshe weitzman Boston, MA
Yes, good suggestion, Thats what I had locally but missed it when submitting the change in WebIDE.
- Status changed to Needs work
over 1 year ago 1:08pm 21 July 2023 - 🇵🇱Poland Graber
Regarding the tests.. I think it just needs 2 more asserts that prove the config has actually been overridden to
['red', 'orange', 'yellow']
and[]
, however implemented. - last update
over 1 year ago 29,839 pass - Status changed to Needs review
over 1 year ago 1:29pm 21 July 2023 - Status changed to RTBC
over 1 year ago 2:54pm 21 July 2023 - 🇺🇸United States bkosborne New Jersey, USA
Can a change record be written for this? Or docs be updated? This is a good example of an issue that a lot of devs would benefit from seeing.
- 🇺🇸United States moshe weitzman Boston, MA
I don't think there is a meaningful change so a change record is not applicable. I think there are release highlights but IMO this does not qualify. I think this qualifies as a routine bug fix for most devs.
- last update
over 1 year ago 29,877 pass, 2 fail - last update
over 1 year ago 29,881 pass - last update
over 1 year ago 29,885 pass - last update
over 1 year ago 29,908 pass - last update
over 1 year ago 29,887 pass, 2 fail - last update
over 1 year ago 29,946 pass - last update
over 1 year ago 29,953 pass - last update
over 1 year ago 29,953 pass - last update
over 1 year ago 29,958 pass - last update
over 1 year ago 29,958 pass - last update
over 1 year ago 29,958 pass 4:07 0:17 Running- last update
over 1 year ago 29,967 pass - last update
over 1 year ago 30,049 pass - last update
over 1 year ago 30,056 pass - 🇳🇿New Zealand quietone
Doing triage on the core RTBC queue → .
The issue summary is clear and has an proposed resolution which matches the MR. I do not see any unanswered questions in the comments and there are comments about the code in the MR.
I did not review the change or test it.
Leaving at RTBC.
- last update
over 1 year ago 30,056 pass - last update
over 1 year ago 29,680 pass, 131 fail - last update
over 1 year ago 30,060 pass - last update
over 1 year ago 30,063 pass - last update
over 1 year ago 30,129 pass, 1 fail - last update
over 1 year ago 30,135 pass - last update
over 1 year ago 30,134 pass, 1 fail - last update
over 1 year ago 30,136 pass - last update
over 1 year ago 30,146 pass - last update
over 1 year ago 30,146 pass 4:07 2:55 Running- last update
over 1 year ago 30,154 pass - last update
over 1 year ago 30,161 pass - last update
about 1 year ago 30,167 pass - last update
about 1 year ago 30,168 pass - last update
about 1 year ago 30,062 pass, 43 fail - last update
about 1 year ago 30,062 pass, 43 fail 49:08 44:45 Running- last update
about 1 year ago 30,218 pass, 43 fail 4:08 0:36 Running- last update
about 1 year ago 30,228 pass, 44 fail - last update
about 1 year ago 30,236 pass, 44 fail - Status changed to Needs work
about 1 year ago 8:10am 6 October 2023 - 🇬🇧United Kingdom alexpott 🇪🇺🌍
We need to also add testing for this behaviour to
\Drupal\Tests\Component\Utility\NestedArrayTest
. The change seems like a good idea but I wonder if we need to change config overrider to find empty arrays and change them to \EmptyIterator objects because I think people would expect setting a config value in settings to an empty array to work.