Currently, the Autopromote class uses multiple globals:
$wgAutopromote $wgAutopromoteOnce $wgEmailAuthentication
as well as the PermissionManager service, and soon the HookContainer/HookRunner and UserGroupManager services
I propose that this be moved into a service with the globals and dependencies injected.
Within deployed code, autopromote is used in two places: the SpecialUserrights and User classes, both in core, so the old class can be immediately deprecated
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Move User::addAutopromoteOnceGroups to UserGroupManager | mediawiki/core | master | +163 -43 | |
| Move autopromote groups logic into UserGroupManager | mediawiki/core | master | +712 -204 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • Pchelolo | T234921 Factor group membership management out of User class | |||
| Resolved | • Pchelolo | T252621 Move Autopromotion logic to UserGroupManager service | |||
| Resolved | • Pchelolo | T254537 UserGroupManager/PermissionManager cyclic dependency | |||
| Resolved | Peter.ovchyn | T271593 Replace specific PermissionManager calls with GroupPermissionsLookup calls in all deployed extensions | |||
| Resolved | Peter.ovchyn | T271596 Migrate relevant User method usages to GroupPermissionsLookup and hard-deprecate User methods. |
Hm... Given that the two public methods are getAutopromoteGroups and getAutopromoteOnceGroups do you think it would be ok to move the methods into UserGroupManager created in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/545690 and deprecate/remove the class entirely?
They seem like an OK fit there. With introduction of the UseGroupManager, $user->getGroups() and $user->getFormerGroups() will belong in the manager, so the new service will have to depend on the UserGroupManager anyway. And I do not really see a lot of value in having these functions in a separate place.
One more pro for moving these into UserGroupManager is that it depends on Autopromote. Thus if we split the two services, we'd inevitably have a cyclic dependency.
Sure - will do once the UserGroupManager is merged and deployed (make sure it won't be reverted)
Change 602426 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Move autopromote groups logic into UserGroupManager
Change 602737 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Move User::addAutopromoteOnceGroups to UserGroupManager
Change 602426 merged by jenkins-bot:
[mediawiki/core@master] Move autopromote groups logic into UserGroupManager
Change 602737 merged by jenkins-bot:
[mediawiki/core@master] Move User::addAutopromoteOnceGroups to UserGroupManager