Skip to content

fix(usecase): carry the pack's recommended profile into the governance link - #811

Merged
CybotTM merged 1 commit into
mainfrom
fix/pack-governance-profile-link
Aug 18, 2026
Merged

fix(usecase): carry the pack's recommended profile into the governance link#811
CybotTM merged 1 commit into
mainfrom
fix/pack-governance-profile-link

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 18, 2026

Copy link
Copy Markdown
Member

The pack detail page prints the governance profile a pack recommends, then says "Compare this posture with what is in force" and links the readout. The link carried no profile, so the operator landed on an unselected readout and had to find the recommendation again by hand — one screen after reading its name.

One thing the issue expected turned out to be already done

#778 asked for the route parameter "plus whatever the readout needs to honour a pre-selection". The readout needs nothing: LlmModuleController::governanceAction() already reads profile off the query string, and its own profile buttons already link that way. Only the sender was missing.

So this is routeUrl() gaining an optional extra-parameter argument and one call site using it. GovernanceProfile is a backed enum, so the value is the same string the buttons use.

The test dispatches the controller, which is new here

UseCasePackRenderTest hands governanceUrl to a view as a fixed string. That is right for what those tests check — the template's markup — and it means none of them could ever have caught this, because the value under test was supplied by the test.

So this one runs the real action and asserts on the rendered body. Nothing in the suite dispatched a backend module action before, and three things were needed to make it work; each is commented where it sits rather than left as incantation:

  • the request must exist before the controller is resolved — Extbase's ConfigurationManager captures the ambient one, which the neighbouring DI test already works around;
  • BackendViewFactory resolves template paths from the route's packageName, so the request needs a route attribute;
  • ModuleTemplate translates its own chrome from $GLOBALS['LANG'].

Seen to fail first. With the argument removed the assertion fails on the rendered body; restored, the fifteen tests in the class pass.

Gates: phpstan level 10 clean, unit 7137 pass, functional -d sqlite on UseCasePack* 32 pass, cgl and rector -n stable at PHP 8.2 after applying.

Closes #778

…e link

The pack page names its recommended governance profile and then invites
the operator to compare it against what is in force. The link opened the
readout with nothing selected, so the profile had to be found by hand one
screen after reading its name.

The readout has always honoured a `profile` query parameter — the issue
expected work there and there was none. Only the link never sent one.
routeUrl() gains an optional extra-parameter argument; nothing else moves.

The test dispatches the real action. The existing render tests hand
`governanceUrl` to a view as a fixed string, which can only prove the
template prints what it is given, so none of them could have caught this.
Dispatching a backend module action was not something this suite did
before: it needs the request to exist before the controller is resolved
(Extbase's ConfigurationManager captures the ambient one), a route
carrying packageName for BackendViewFactory, and $GLOBALS['LANG'] for
ModuleTemplate's own chrome. The helper says why for each.

Seen to fail first: with the argument removed the assertion fails on the
rendered body, and the fifteen tests in the class pass with it back.

Closes #778

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
Copilot AI lite review requested due to automatic review settings August 18, 2026 06:38
@CybotTM
CybotTM requested a review from a team as a code owner August 18, 2026 06:38
@sonarqubecloud

Copy link
Copy Markdown

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests Test-related changes labels Aug 18, 2026

@github-actions github-actions Bot 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.

Automated approval for maintainer PR

All automated quality gates passed. See SECURITY_CONTROLS.md for compensating controls.

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.43%. Comparing base (105f0c1) to head (242ec1e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...asses/Controller/Backend/UseCasePackController.php 0.00% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #811      +/-   ##
============================================
- Coverage     87.43%   87.43%   -0.01%     
  Complexity     9710     9710              
============================================
  Files           566      566              
  Lines         31418    31420       +2     
============================================
  Hits          27471    27471              
- Misses         3947     3949       +2     
Flag Coverage Δ
unit 65.37% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...asses/Controller/Backend/UseCasePackController.php 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@CybotTM

CybotTM commented Aug 18, 2026

Copy link
Copy Markdown
Member Author

Review record — no Copilot review exists for this diff (monthly, account-wide quota exhausted), so the green gate is not a read of the change. What was checked:

One premise of the issue was wrong and checking it removed most of the work. #778 asked for the route parameter "plus whatever the readout needs to honour a pre-selection". The readout needs nothing — governanceAction() already reads profile off the query string and its own buttons already link that way. Only the sender was missing, so this is one argument and an optional parameter on a private helper.

The existing tests could not have caught this, and that is why a new kind of test is here. UseCasePackRenderTest hands governanceUrl to a view as a fixed string; it can only prove the template prints what it is given. Asserting a URL the controller builds needs the controller to build it, so this dispatches the real action. Nothing in the suite did that before — it needs the request to exist before the controller is resolved, a route carrying packageName, and $GLOBALS[LANG]. Each is commented where it sits rather than left as incantation, because the next person to dispatch an action will hit all three.

Seen to fail first: with the argument removed the assertion fails on the rendered body; restored, the fifteen tests in the class pass.

Merge safety checked rather than assumed: this branch and #801 both add to [Unreleased]. Merged together locally on top of main, the result has three headings, no bullet twice, and 14 bullets where 12 + 1 + 1 is expected — a conflict-free auto-merge of this file is exactly the case this repository has been burned by, so the content was counted rather than the exit code trusted. #810 does conflict and is deliberately held back for a rebase.

phpstan level 10 clean, unit 7137 pass, functional -d sqlite on UseCasePack* 32 pass, cgl and rector -n stable at PHP 8.2.

Merging on that basis.

@CybotTM
CybotTM added this pull request to the merge queue Aug 18, 2026
Merged via the queue into main with commit 2f5edd5 Aug 18, 2026
93 of 94 checks passed
@CybotTM
CybotTM deleted the fix/pack-governance-profile-link branch August 18, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation tests Test-related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The pack page links to the governance readout without pre-selecting the recommended profile

2 participants