Page MenuHomePhabricator
Create Task
Maniphest T88510

Create common interface for User, UserRightsProxy, and CentralAuthGroupMembershipProxy
Closed, DeclinedPublic
Actions

Description

The code of Special:UserRights misleadingly implies that we're performing actions upon a User object, when in reality it could be a UserRightsProxy or a CentralAuthGroupMembershipProxy, which just happen to have the same functions. We should create a common interface for those functions and explicitly specify through documentation and type hinting that that interface is being used rather than blindly assuming User.

The real goal of this refactoring and cleanup is to ensure that bugs like T88505 don't happen again.

Details

SubjectRepoBranchLines +/-
Make CentralAuthGroupMembershipProxy implement UserGroupMembermediawiki/extensions/CentralAuthmaster+1 -1
Add common interface for User and UserRightsProxymediawiki/coremaster+84 -20
Customize query in gerrit

Related Objects

Event Timeline

Legoktm created this task.Feb 4 2015, 6:37 AM
Legoktm claimed this task.
Legoktm raised the priority of this task from to Medium.
Legoktm updated the task description. (Show Details)
Legoktm added projects: MediaWiki-Core-Team, MediaWiki-Special-pages, Technical-Debt, MediaWiki-extensions-CentralAuth.
Legoktm subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 4 2015, 6:37 AM
gerritbot added a project: Patch-For-Review.Feb 4 2015, 6:38 AM
gerritbot subscribed.

Change 188516 had a related patch set uploaded (by Legoktm):
Add common interface for User and UserRightsProxy

https://gerrit.wikimedia.org/r/188516

Patch-For-Review

Legoktm moved this task from Backlog to Needs Review/Feedback on the MediaWiki-Core-Team board.Feb 5 2015, 7:47 PM
Legoktm moved this task from Needs Review/Feedback to In Dev/Progress on the MediaWiki-Core-Team board.Mar 13 2015, 7:03 AM
Legoktm edited projects, added Reading-Infrastructure-Team-Old (Don't use); removed MediaWiki-Core-Team.Mar 30 2015, 8:34 PM
• bd808 moved this task from Backlog to In Dev/Progress on the Reading-Infrastructure-Team-Old (Don't use) board.Apr 3 2015, 4:13 PM
• Reguyla moved this task from To triage to User/User rights on the MediaWiki-Special-pages board.May 26 2015, 11:53 AM
Danny_B moved this task from Unsorted to Add / Create on the Technical-Debt board.Jan 23 2016, 12:27 AM
gerritbot added a comment.Aug 10 2016, 3:13 AM

Change 188516 abandoned by Legoktm:
Add common interface for User and UserRightsProxy

https://gerrit.wikimedia.org/r/188516

Legoktm removed Legoktm as the assignee of this task.Aug 10 2016, 4:08 AM
gerritbot added a comment.Dec 21 2016, 2:53 AM

Change 188516 restored by TTO:
Add common interface for User and UserRightsProxy

Reason:
This seems like an important precursor to T12493

https://gerrit.wikimedia.org/r/188516

Krenair subscribed.Dec 21 2016, 3:01 AM
TTO claimed this task.Dec 21 2016, 5:03 AM
gerritbot added a comment.Dec 21 2016, 5:17 AM

Change 328462 had a related patch set uploaded (by TTO):
Make CentralAuthGroupMembershipProxy implement UserGroupMember

https://gerrit.wikimedia.org/r/328462

Tgr removed a project: Reading-Infrastructure-Team-Old (Don't use).May 2 2017, 2:23 PM
Krinkle edited projects, added MediaWiki-User-management; removed MediaWiki-Special-pages.May 8 2017, 12:00 AM
Krinkle moved this task from Backlog to User rights on the MediaWiki-User-management board.May 8 2017, 12:01 AM
gerritbot added a comment.May 26 2018, 12:32 AM

Change 188516 abandoned by Legoktm:
Add common interface for User and UserRightsProxy

Reason:
Likely not necessary now that UserGroupMembership class exists.

https://gerrit.wikimedia.org/r/188516

gerritbot added a comment.Jan 25 2020, 8:24 AM

Change 328462 abandoned by Thiemo Kreuz (WMDE):
Make CentralAuthGroupMembershipProxy implement UserGroupMember

Reason:
The core change got abandoned. I don't think this applies any more. If it does, it can easily be recreated or reopened.

https://gerrit.wikimedia.org/r/328462

Aklapper removed a project: Patch-For-Review.Mar 1 2020, 6:50 AM

@TTO: Hi, all related patches in Gerrit have been abandoned. Do you still plan to work on this? Asking as you are set as task assignee. Thanks in advance!

DannyS712 subscribed.Mar 1 2020, 6:57 AM
Aklapper removed TTO as the assignee of this task.Mar 31 2020, 2:39 PM
Aklapper added a subscriber: TTO.

@TTO: I am resetting the assignee of this task because there has not been progress lately (please correct me if I am wrong!). Please claim this task again when you plan to work on it (via Add Action... → Assign / Claim in the dropdown menu) - it would be welcome. Thanks for your understanding!

Zabe subscribed.Aug 24 2021, 4:48 PM
Daimona subscribed.Feb 13 2022, 7:03 PM

I suppose this should be declined in favour of T255309?

Legoktm closed this task as Declined.Feb 14 2022, 9:31 AM

Yeah, though the intent of this bug was satisfied, which was to get rid of the fake interface that wasn't an interface between the various User/UserRightsProxy/etc. objects.

taavi mentioned this in T302408: Argument 1 passed to HAWelcomeHooks::onUserGroupsChanged() must be an instance of User, instance of CentralAuthGroupMembershipProxy given, called in /srv/mediawiki/w/includes/HookContainer/HookContainer.php on line 338.Feb 23 2022, 3:14 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits