- Issue created by @FeyP
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 5:07pm 20 April 2023 - 🇩🇪Germany FeyP
Found an issue with the bubbling of cache metadata, so new patch and interdiff attached. Also updated the tests to assert correct metadata.
Like the previous patch, this requires PHP 7.4 or later, but I think this is just due to some of the type hinting. Looks like the default testing is currently for PHP 7.3. If needed, I can probably provide an updated patch that is compatible with 7.3 as well. Let me know.
- last update
over 1 year ago 39 pass, 1 fail - last update
over 1 year ago 40 pass - 🇬🇧United Kingdom jaydenpearly
Thanks @FeyP ...
#3 - 3352640-03.patch is working.Tested versions
Drupal: 10.2.7
diff: 1.7
token: 1.14
php: 8.3+1 for RTBC
- Status changed to Needs work
6 months ago 3:39am 28 June 2024 - 🇦🇺Australia acbramley
Thank you for your contribution. This issue currently does not meet the Contribution guidelines which are required to get this change committed.
- Status changed to Needs review
6 months ago 1:43am 30 June 2024 - 🇩🇪Germany FeyP
Thanks @acbramley! I created an MR against the 2.x branch, that should be all that was missing to meet the contribution guidelines for NR.
One small note on this part of the code in TokenProvider:
$default_layout = $this->diffLayoutManager->getDefaultLayout(); if ($default_layout === '') { $replacements[$original] = $this->t('(No default diff layout configured.)'); continue; }
This was an empty() check in the original patch for 1.x, because I actually ran into this during development. empty() checks are no longer allowed by the phpstan configuration of this module. Thus I looked at this again and the method has now a string return type hint. So I changed this to check for an empty string instead. But:
getDefaultLayout()
still has this return statementreturn \reset($plugins);
and if$plugins
is an empty array, it will not return a string, but booleanfalse
. This means that we could probably remove this check in TokenProvider entirely, since the Layout Manager will then generate a white page (wrong return type). Or we could fix the wrong return type. For now I didn't change anything about this in the layout manager, because unlike the other change in the layout manager that is a part of this patch, this bug fix is not needed to pass tests here, so it could be done in a follow-up issue, if desired. - First commit to issue fork.
- 🇦🇺Australia acbramley
IMO this functionality could live in a separate module. Judging by the activity on this issue, it's not something that seems to be needed by a lot of users and is a non-trivial amount of code and tests to include.
- 🇩🇪Germany FeyP
Alright, separate module is not gonna happen, so let's close this won't fix.