Skip to content

refactor: clear the phpmd violations every open PR inherits from development - #1400

Merged
WilcoLouwerse merged 5 commits into
developmentfrom
fix/phpmd-session-portalshell
Oct 8, 2026
Merged

WilcoLouwerse merged 5 commits into
developmentfrom
fix/phpmd-session-portalshell

Conversation

@WilcoLouwerse

@WilcoLouwerse WilcoLouwerse commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Why

quality / PHP Quality (phpmd) has been red on every open PR into development since #1250 and #1253 landed (for example #1243, #1364, #1366, #1391 to #1397). phpmd scans all of lib/, not only the PR diff, so each PR inherits the failure, and the Quality Report that sums up the checks goes red with it. phpmd is not a required check, so the PRs could still merge while red, and that is how the debt got onto development.

The violations, all in code from those PRs:

File Rule Value Limit
SessionController ExcessiveClassLength 1087 lines 1000
SessionController TooManyMethods 26 25
SessionController::exampleResidentUser() CyclomaticComplexity / NPathComplexity 13 / 200 10 / 200
PortalShell ExcessiveClassComplexity 59 (53 before #1394) 50
PortalShell::residentMenu() CyclomaticComplexity / NPathComplexity 11 / 200 (split by #1394) 10 / 200

What

  • The one-click demo sign-in gets its own controller. GET /portal/api/session/example-resident keeps its URL and its attributes (#[PublicPage], #[NoCSRFRequired], #[AnonRateLimit(10/60)], #[BruteForceProtection]), so the frontend is unchanged. It now maps to ExampleResidentController::signIn (exampleResident#signIn).
    • The switch, declaration and install-record checks move into ExampleResidentSignIn (open(), userFor()). That service already reads the same switch for the site's demo card.
    • The controller keeps the portal check: the portal must be the named one and offer the nextcloud mode.
    • Every refusal is still a throttled 404.
  • The resident menu projection moves into PortalResidentMenu: card label, groups and items, and the leave-out list that feat(site): a portal may leave items out of the resident menu (resident-menu-leave-out) #1394 added. It works the same way PortalSignInText owns the sign-in text. The output of PortalShell::project() is unchanged, and feat(site): a portal may leave items out of the resident menu (resident-menu-leave-out) #1394's leave-out test passes through the new class.
  • The demo sign-in tests move to ExampleResidentControllerTest and are tightened.
    • The refusal tests used to give the resident no portal account, so a missing-account 404 hid every guard: removing the switch check or the portal check left them green.
    • The shared parts now include an active account.
    • New cases: a portal without the nextcloud mode, a lookup that answers with the resident's portal for another portal the caller named, and no portal lookup while the switch is off.
  • Coverage for the new branches, for the PHPUnit coverage guard: the 503 when no session can be issued, the refusal when the portal lookup throws, and new ExampleResidentSignInTest and PortalResidentMenuTest for the service's early refusals, the menu limits and non-text values.

No suppressions were added. The existing @SuppressWarnings on SessionController (coupling, class complexity, parameter list) are untouched.

Test

  • phpmd lib text phpmd.xml and the unused-params ruleset → exit 0 (before: 7 violations).
  • run-hydra-gates.sh --scope-to-diff --base origin/development → exit 0, 45 of 45 applicable gates ran.
  • phpstan on the changed files → no errors. phpcs → the new files are clean; the findings in appinfo/routes.php exist on development too.
  • Each of the six demo sign-in guards was removed in turn (switch, open(), nextcloud mode, portal slug, declaration, declared portal), and every removal fails a test in ExampleResidentControllerTest.
  • PHPUnit phpunit-unit.xml: 2969 tests, 62 errors, the same counts as development before the merge. Those errors need a running Nextcloud (Class "OC" not found). SessionControllerTest (56), ExampleResidentControllerTest (7), ExampleResidentSignInTest (2), PortalResidentMenuTest (2), PortalShellTest (13), PortalRuntimeConfigResolverTest (25) and PortaliqRegisterConfigTest (26) all pass.
  • psalm could not run locally: psalm 5.26 crashes on this PHP version before it analyses anything. CI runs it.

🤖 Generated with Claude Code

WilcoLouwerse and others added 2 commits October 8, 2026 13:37
…ller

phpmd failed on SessionController after #1250 added the example-resident
sign-in: 1087 lines (limit 1000), 26 methods (limit 25), and a guard with
cyclomatic complexity 13 (limit 10). phpmd scans all of lib/, so every open
PR inherited the failure.

The route keeps its URL (/portal/api/session/example-resident) and its
attributes; it now maps to ExampleResidentController::signIn. The switch,
declaration and install-record checks move into ExampleResidentSignIn,
which already reads the same switch for the site's demo card. The four
tests move with the route, unchanged apart from the method name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
phpmd failed on PortalShell after #1253 added the resident menu groups:
class complexity 53 (limit 50) and residentMenu() at cyclomatic
complexity 11 (limit 10). PortalResidentMenu now owns that projection,
split into groups and items, the way PortalSignInText owns the sign-in
text. The output is unchanged; PortalShellTest covers it through
PortalShell::project().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WilcoLouwerse and others added 2 commits October 8, 2026 14:46
#1394 split PortalShell::residentMenu() and added the leave-out list; the
resident menu now lives in PortalResidentMenu, so its leaveOut() moves
there. PortalShell on development is at class complexity 59 (limit 50);
with the projection moved out it passes phpmd.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The refusal tests gave the resident no portal account, so every refusal
was also a missing-account 404: removing the switch check or the portal
check left them green. The shared parts now include an active account,
so a 404 can only come from the guard under test.

New cases: a portal without the nextcloud mode, a lookup that answers
with the resident's portal for another portal the caller named, and no
portal lookup at all while the switch is off. Each guard in the controller
and in ExampleResidentSignIn was removed in turn; every removal now fails
a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/portaliq @ 163eab6

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-specs ✅
check-manifest ✅
format ✅
check-l10n-js ✅
check-schema-l10n ✅
check-reuse ✅
composer ✅ ✅ 108/108
npm ✅ ✅ 984/984
app:check-code ⏭️
info.xml ✅
REUSE ✅
lockfile sync ✅
PHPUnit ❌
Newman ✅
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ✅

Quality workflow — 2026-10-08 12:52 UTC

Download the full PDF report from the workflow artifacts.

The coverage guard failed the PR: the code it kept or added was covered
at 96.44% against 96.94% on the base. The uncovered statements were
the 503 when no session can be issued, the refusal when the portal
lookup throws, ExampleResidentSignIn's early refusals, and a non-text
value in PortalResidentMenu.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/portaliq @ e5d19c5

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-specs ✅
check-manifest ✅
format ✅
check-l10n-js ✅
check-schema-l10n ✅
check-reuse ✅
composer ✅ ✅ 108/108
npm ✅ ✅ 984/984
app:check-code ⏭️
info.xml ✅
REUSE ✅
lockfile sync ✅
PHPUnit ✅ 2983/2983 · 1 skipped
Newman ✅ 1/1
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it locally with npx playwright test, or from the Actions tab on a branch with no open pull request into development.
Hydra gates ✅

Quality workflow — 2026-10-08 14:24 UTC

Download the full PDF report from the workflow artifacts.

@WilcoLouwerse
WilcoLouwerse merged commit 1cc8957 into development Oct 8, 2026
47 checks passed
@WilcoLouwerse
WilcoLouwerse deleted the fix/phpmd-session-portalshell branch October 8, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant