Skip to content

[access] refactor(dashboard-api): split DashboardApiController create, update guard, group, fork and schedule actions into named steps - #1076

Open
rubenvdlinde wants to merge 2 commits into
developmentfrom
quality/i40-dashapi-steps
Open

rubenvdlinde wants to merge 2 commits into
developmentfrom
quality/i40-dashapi-steps

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What changed

Eight DashboardApiController units read as named steps. Every route action keeps its signature and route parameters, and every guarded endpoint still shows its guard call in its own body (the guard golden reads that).

  • create: denyCreateRequest (the denyCreate chain, then the create permissions, in the original order) → resolveCreateParams → createFromParams (createDashboardFrom, createdResponse) with the same quota / invalid-argument / generic mappings. resolveCreateParams → resolveCreateBody (stringOrNull, intOrZero).
  • denyUpdate → mayUpdate.
  • updateGroup / setGroupDefault share one runGroupMutation (404 for an unknown dashboard, the generic error otherwise) instead of two copies of the same catch blocks; the group and UUID travel as a GroupDashboardTarget. setGroupDefault still checks the login before the missing UUID (missingUuid).
  • fork checks the login, then forkFor → forkCreated / forkFailure (the same four outcomes in the same precedence, the last one logged by internalForkError).
  • schedule keeps the action check, then scheduleDated (no-time refusal) → scheduled (scheduleFor) with the same mappings; the dashboard and its times travel as a DashboardScheduleRequest.
  • visibleBreadcrumbs → visibleUuids, crumbFor.

Allowed and refused outcomes are unchanged; every refusal answers with the same status and body, and every check still runs before the call it guards.

Characterisation tests

tests/Unit/Controller/DashboardApiControllerActionsGoldenTest.php was committed first, against the old code, and passes before and after: every create outcome (no session, personal dashboards off, creation not allowed, second dashboard not allowed, success with the exact service arguments and envelope, quota / invalid / generic errors), the create-parameter resolution for flat and body input, the update guard for metadata-only and layout updates (allowed and refused) and without login, every group update and group default outcome, every fork outcome (with the logged failure), every schedule outcome, and the breadcrumb hiding. Mutation check: disabling the layout-permission check on the old code turned the update golden red; restored before committing. The existing DashboardApiControllerGuardGoldenTest passes unchanged.

What ran

  • vendor/bin/phpunit -c phpunit.xml --no-coverage tests/Unit: 2765 tests OK (13 skipped, as on development).
  • vendor/bin/phpcs and vendor/bin/phpstan analyse on the touched lib files: clean.

Figures (touched files lib/Controller/DashboardApiController.php, GroupDashboardTarget.php, DashboardScheduleRequest.php)

before after
units over complexity 5 9 units, 304 lines 2 units, 51 lines
units over complexity 10 create (11) none
units over 15 lines of code 26 units, 715 lines 19 units, 462 lines
units over 2 parameters 7 units, 199 lines 8 units, 136 lines

The two units left over complexity 5 are create (7) and update (9): each nullable route parameter counts as a branch, so their floor is set by the route signature (6 and 8), which stays as it is. The extra unit over 2 parameters is the DashboardScheduleRequest constructor. App-wide: unit complexity 4.02 → 4.14, unit size 3.40 → 3.44, unit interfacing 3.14 → 3.16, duplication 4.54 → 4.55.

Maintainability floor: compare against development holds.

AI-assisted: prepared with Claude Code (claude-opus-5-5); reviewed by a maintainer before merge.


Generated by Claude Code

…e the split

Assisted-by: ClaudeCode:claude-opus-5-5
…guard, group, fork and schedule actions into named steps

Assisted-by: ClaudeCode:claude-opus-5-5
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.

2 participants