- πΊπΈ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.
Reviewed the changes and removing the moduleExist check looks good.
Ran the tests locally to make sure they fail and got
Failed asserting that an array has the key 'system_menu_block:another_menu'.
Which is expected.
Looks good!
- Status changed to Needs work
over 1 year ago 4:06pm 6 February 2023 - π¬π§United Kingdom catch
If used a memory cache for the static cache, we could rely on cache tags here, cross-linking π± [policy] Standardize how we implement in-memory caches Needs work .
One nit on the patch itself:
+++ b/core/modules/system/src/Plugin/Derivative/SystemMenuBlock.php @@ -44,6 +44,8 @@ public static function create(ContainerInterface $container, $base_plugin_id) { * {@inheritdoc} */ public function getDerivativeDefinitions($base_plugin_definition) { + // Reset the discovered definitions. + $this->derivatives = []; foreach ($this->menuStorage->loadMultiple() as $menu => $entity) {
This needs a better comment - why do we need to reset the property every time it's called?