MenuRouterRebuildSubscriber has a hidden database dependency

Created on 20 December 2014, almost 10 years ago
Updated 13 May 2024, 5 months ago

Problem/Motivation

There is a db_transaction(); in MenuRouterRebuildSubscriber

The problem with that is that nothing ensures that the menu router is actually stored in the database.

Note that this does not break anything and the issue is just cleanup.

Proposed resolution

  1. Move the code block in MenuRouterRebuildSubscriber::menuLinksRebuild() starting with $transaction = db_transaction(); into MenuTreeStorage::rebuild. Keep the lock mechanism in MenuRouterRebuildSubscriber.
  2. Change the db_transaction you just moved into $this->connection->startTransaction.
  3. Optional and can be a followup: write a unit test for MenuLinkManager::rebuild. AFAIK there is no unit test currently that could be easily extended to include this method and so this part is not a novice task (but, feel free to try. You'd need to mock the plugin discovery, the module handler, the link override and the tree storage, I would borrow ideas/code from LocalTaskManagerTest ).

Remaining tasks

User interface changes

API changes

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Menu systemย  โ†’

Last updated about 14 hours ago

Created by

๐Ÿ‡จ๐Ÿ‡ฆCanada chx

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

Production build 0.71.5 2024