Conversation
8d22861 to
3e18688
Compare
d97cf44 to
7031c55
Compare
62516b9 to
bd341de
Compare
There was a problem hiding this comment.
Pull request overview
This pull request implements automated enforcement of the "qualified user" status based on PI group membership. The PR ensures that users are automatically qualified when they join a PI group and disqualified when they leave all PI groups.
Changes:
- Added a worker script to sync the qualified users group with PI group memberships
- Updated user qualification logic to automatically check group membership when users join/leave groups
- Improved email templates and documentation terminology to use "disqualified" consistently
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| workers/update-qualified-users-group.php | New worker script to synchronize qualified user group membership based on PI group memberships |
| test/functional/WorkerUpdateQualifiedUsersGroupTest.php | Tests for the new worker script verifying qualification and disqualification logic |
| test/functional/PiRemoveUserTest.php | Extended tests to verify qualified status changes when users are removed from PI groups by both PIs and admins |
| test/functional/PIBecomeApproveTest.php | Added assertion to verify users are disqualified when their PI group is deleted |
| test/functional/LeaveGroupTest.php | New tests verifying users are disqualified when leaving groups and on login after manual LDAP changes |
| test/Template.php | Template file for data provider pattern tests |
| resources/lib/UnityUser.php | Added updateIsQualified() method to centralize qualification status updates |
| resources/lib/UnityGroup.php | Updated group approval/removal methods to use new updateIsQualified() method |
| resources/lib/PosixGroup.php | Added overwriteMemberUIDs() method for bulk member updates |
| resources/init.php | Added automatic qualification check on user login to handle manual LDAP changes |
| resources/mail/*.php | Updated email templates to use "disqualified" terminology consistently |
| test/phpunit-bootstrap.php | Updated ensurePIGroupDoesNotExist() to properly disqualify removed members |
| tools/docker-dev/identity/bootstrap.ldif | Removed ghost users and other non-qualified users from the qualified users group |
| phpstan.neon | Added exclusions for Template.php and new test methods using dynamic invocation |
| LDAP.md | Clarified terminology definitions for qualified/unqualified and native/non-native users |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fe14506 to
6aacf0e
Compare
There was a problem hiding this comment.
Why remove seemingly random accounts from this list?
There was a problem hiding this comment.
Oh, are these supposed to be the "unqualified" or whatever other status?
There was a problem hiding this comment.
Yes, those accounts were removed by the update-qualified-users-group.php worker
03cc64e to
ef4065e
Compare
ef4065e to
fd80960
Compare
Any user who is not a PI or is not a member of at least one PI group should be disqualified and denied access to services.
Added tests:
groups.phppi.phppi-mgmt.php#520 allows PI groups to be disbanded and reinstated, and includes more tests that users are qualified and disqualified accordingly,
but those tests are currently commented out because they rely on functionality implemented in this PR.