Skip to content

Document registering a custom Menu in Config/services.php (Mautic 8) - #627

Open
promptless-for-oss wants to merge 5 commits into
mautic:8.0from
Promptless:promptless/pr-17116-menus-services-php
Open

Document registering a custom Menu in Config/services.php (Mautic 8)#627
promptless-for-oss wants to merge 5 commits into
mautic:8.0from
Promptless:promptless/pr-17116-menus-services-php

Conversation

@promptless-for-oss

Copy link
Copy Markdown
Contributor

Open in Promptless

Mautic 8 (mautic/mautic PR #17116) removed the ServicePass compiler pass and the services > menus array in a bundle's Config/config.php. A Plugin that registered its own top-level Menu must now declare two services explicitly in Config/services.php: a Knp\Menu\MenuItem tagged knp_menu.menu, and a Mautic\CoreBundle\Menu\MenuRenderer tagged knp_menu.renderer, paired by a matching alias tag argument.

This adds a "Registering a custom Menu" section to docs/plugins/config.rst documenting this previously-undocumented extension point and its migration path — a before (config.php) / after (services.php) example, the multi-menu loop form, and a note that the menu builder must be referenced by class (Mautic\CoreBundle\Menu\MenuBuilder::class) because the mautic.menu.builder string alias was removed. A forward cross-reference from the "Available menus" list points readers to the new section.

Trigger Events

Mautic 8 (mautic/mautic PR #17116) removed the ServicePass compiler pass
and the 'services > menus' array in bundle Config/config.php. Add a
'Registering a custom Menu' section to plugins/config.rst documenting the
knp_menu.menu MenuItem and knp_menu.renderer MenuRenderer services a Plugin
now declares in Config/services.php, with a before/after migration example
and a forward cross-reference from 'Available menus'.
Comment thread docs/plugins/config.rst
Comment thread docs/plugins/config.rst
Comment thread docs/plugins/config.rst
Comment thread docs/plugins/config.rst
Comment thread docs/plugins/config.rst
@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

I noticed that some CI checks failed for this PR. I'm investigating whether the failures are caused by this suggestion. If they're unrelated or pre-existing, I'll leave this suggestion unchanged and create a separate suggestion if a standalone docs fix is needed.

@adiati98 adiati98 added this to the 8.0 milestone Sep 1, 2026

@TomasVotruba TomasVotruba left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct way

@adiati98 adiati98 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@promptless-for-oss please address the Vale errors and warnings. If the capitalizations are correct, wrap them in vale off/on statements.

@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

Thanks @adiati98 — on it. I'll address the Vale errors and warnings on this PR, wrapping any intentionally-correct capitalizations in vale off/on statements, and follow up here once the changes are pushed.

Wrap intentionally-correct code/doc identifiers (reference-path
segments and the lowercase plugins/ filesystem path) in vale off/on
comments per maintainer review, and rephrase one passive sentence to
active voice. No capitalization changed.
@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

Thanks @adiati98 — I've addressed the Vale review on this PR. The three flagged capitalizations that are intentionally correct — the :ref: cross-reference path segments (plugins/config, plugins/autowiring:Autowiring) — are now wrapped in .. vale off/.. vale on as you asked, without changing any capitalization, and I rephrased one passive sentence to active voice. Those three errors/warnings now clear.

One finding can't be cleared this way: the plugins/ path in the // plugins/HelloWorldBundle/Config/config.php code comment. That token is a genuine lowercase filesystem path (matching the convention used across the developer docs), but .. vale off/.. vale on doesn't suppress alerts inside code blocks — even wrapping the whole file in a vale-off region leaves it flagged, because it's triggered by a Vale/RST parsing quirk involving unrelated content further down the page. Clearing it would need a repo-level change (scoping the Vale.Terms/Mautic rules to skip code blocks, or a vocabulary adjustment) rather than an in-page edit. Happy to open a separate change for that if you'd like — just let me know.

One note: our publishing account couldn't re-request your review through GitHub (it's a fork contributor without push access to the base repo), so please re-review at your convenience.

@adiati98

Copy link
Copy Markdown
Contributor

@promptless-for-oss please rebase to 8.0 branch.

1 similar comment
@adiati98

Copy link
Copy Markdown
Contributor

@promptless-for-oss please rebase to 8.0 branch.

@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

In reply to this discussion

Thanks @adiati98 — on it. We're rebasing this branch onto 8.0 and will confirm here once it's done.

@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

In reply to this discussion

Thanks @adiati98 — on it. We're rebasing this branch onto 8.0 and will confirm here once it's done.

@promptless-for-oss
promptless-for-oss changed the base branch from 7.2 to 8.0 September 10, 2026 11:11
@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

In reply to this discussion

Done, @adiati98 — this PR now targets the 8.0 branch as requested. No prose changed: the reviewed content is byte-identical (one file, docs/plugins/config.rst, +95/−0) and the PR is mergeable pending review.

One note on method: our fork's publishing account can't force-push a rewritten branch history, so we performed the rebase by moving the PR's base branch from 7.2 to 8.0. Since 8.0 and 7.2 currently point at the same commit, the resulting diff is identical to what you reviewed. Please merge or re-review at your convenience — our fork-based account can't trigger GitHub's re-request-review button on your repo.

@promptless-for-oss

Copy link
Copy Markdown
Contributor Author

In reply to this discussion

Done, @adiati98 — this PR now targets the 8.0 branch as requested. No prose changed: the reviewed content is byte-identical (one file, docs/plugins/config.rst, +95/−0) and the PR is mergeable pending review.

One note on method: our fork's publishing account can't force-push a rewritten branch history, so we performed the rebase by moving the PR's base branch from 7.2 to 8.0. Since 8.0 and 7.2 currently point at the same commit, the resulting diff is identical to what you reviewed. Please merge or re-review at your convenience — our fork-based account can't trigger GitHub's re-request-review button on your repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants