Skip to content

[access] refactor(people): split PeopleWidgetService directory paging, candidates and account fields into named steps - #1080

Open
rubenvdlinde wants to merge 2 commits into
developmentfrom
quality/i40-people-visibility
Open

rubenvdlinde wants to merge 2 commits into
developmentfrom
quality/i40-people-visibility

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Five PeopleWidgetService units that decide which people and fields the People widget shows read as named steps. This PR touches different methods than the open birthdate / listUsers split of the same file and merges cleanly with it.

  • collectDisplayNamePage streams the directory in chunks (displayNameBatch) into a new DisplayNamePageCollector, a small final run object built for one page: disabled users skipped when asked, the window's offset skipped, users taken until the page is full, then stopping on an exhausted backend, as before.
  • searchCandidates → directoryMatches → withFieldMatches (profile-field matches the viewer may see, merged after the name matches; UIDs that no longer resolve left out), then withinGroups for a group filter.
  • resolveCandidates → groupFilterMembers; collectGroupMembers → groupUsers, keeping each user's first place.
  • buildAccountFields → shownProperties (no birthdate when birthdays are off, so it is not even read) → accountField / birthdateField.

Which people are listed and which fields a viewer sees are unchanged, with the same backend calls in the same order.

Characterisation tests

tests/Unit/Service/PeopleWidgetVisibilityGoldenTest.php was committed first, against the old code, and passes before and after: directory pages with and without disabled users, offsets inside and past the directory, an empty directory, and chunked streaming that stops when the page is full or the backend is exhausted (with the exact backend calls); group members de-duplicated in order with malformed ids and missing groups dropped; every candidate filter shape; the search merge order with its exact calls and the group intersection; and the account fields for visible, private, birthday-off and unparsable values, a failing account lookup and the unread birthdate. Mutation check: no longer skipping disabled users on the old code turned four goldens red; restored before committing.

What ran

  • vendor/bin/phpunit -c phpunit.xml --no-coverage tests/Unit: 2764 tests OK (13 skipped, as on development).
  • vendor/bin/phpcs and vendor/bin/phpstan analyse on the touched lib files: clean.
  • vendor/bin/phpmd on PeopleWidgetService: one new finding, TooManyMethods (27 methods against a limit of 25), the cost of turning long methods into named private steps; composer phpmd is already red on development for other classes.

Figures (touched files lib/Service/PeopleWidgetService.php, lib/Service/DisplayNamePageCollector.php)

before after
units over complexity 5 9 units, 251 lines 4 units, 123 lines (the birthdate helpers and listUsers, split in the other open PR)
units over 15 lines of code 13 units, 354 lines 9 units, 239 lines
units over 2 parameters 3 units, 55 lines 3 units, 55 lines

App-wide: unit complexity 4.02 → 4.08, unit size 3.40 → 3.42.

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

…d account fields before the split

Assisted-by: ClaudeCode:claude-opus-5-5
…tes and account fields 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