Page MenuHomePhabricator
Create Task
Maniphest T265769

Research to create service for BotPassword class
Open, Needs TriagePublic
Actions

Description

Many static function in class BotPassword using services

Details

SubjectRepoBranchLines +/-
Use BotPasswordStore in SpecialBotPasswordsmediawiki/coremaster+38 -9
AuthManager: inject more servicesmediawiki/coremaster+152 -31
BotPasswordStore: accept UserIdentity objectsmediawiki/coremaster+12 -11
Move BotPassword lookup methods to BotPasswordStoremediawiki/coremaster+131 -63
Deprecate unused BotPassword methodsmediawiki/coremaster+8 -2
Add BotPasswordStore servicemediawiki/coremaster+350 -73
Cleanup BotPassword class to prepare for refactormediawiki/coremaster+39 -26
Customize query in gerrit

Related Objects
Search...

Event Timeline

Umherirrender created this task.Oct 16 2020, 10:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 16 2020, 10:00 PM
Umherirrender added a parent task: T259960: Inject services into API modules and special pages.Oct 16 2020, 10:00 PM
Tgr subscribed.Oct 17 2020, 12:53 AM
Tgr added a comment.Oct 17 2020, 12:55 AM

More related to T141495: AuthManager should use dependency injection than T259960: Inject services into API modules and special pages.

Umherirrender added a comment.Oct 17 2020, 10:39 AM
In T265769#6557109, @Tgr wrote:

More related to T141495: AuthManager should use dependency injection than T259960: Inject services into API modules and special pages.

I have seen the usage on Special:BotPasswords and was creating this task, but AuthManager also using that class

Umherirrender added a parent task: T141495: AuthManager should use dependency injection.Oct 17 2020, 10:39 AM
Umherirrender added a subtask: T265767: Research to create service for CentralIdLookup::factory.
DannyS712 claimed this task.Oct 22 2020, 2:13 AM
Restricted Application added a project: User-DannyS712. · View Herald TranscriptOct 22 2020, 2:13 AM
gerritbot added a comment.Oct 22 2020, 2:16 AM

Change 635599 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Cleanup BotPassword class to prepare for refactor

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

gerritbot added a project: Patch-For-Review.Oct 22 2020, 2:16 AM
DannyS712 moved this task from Unsorted to In progress on the User-DannyS712 board.Oct 22 2020, 2:44 AM
DannyS712 added a comment.Oct 22 2020, 2:49 AM

Plan:
BotPasswordStore service:
Handle DB
BotPassword::save -> BotPasswordStore::insertBotPassword and ::updateBotPassword
BotPassword::delete -> BotPasswordStore::deleteBotPassword

BotPassword::invalidateAllPasswordsForUser - BotPasswordStore::invalidateAllPasswordsForUser
BotPassword::invalidateAllPasswordsForCentralId -> merged with above method, no other users

BotPassword::removeAllPasswordsForUser -> BotPasswordStore::removeAllPasswordsForUser
BotPassword::removeAllPasswordsForCentralId -> merged with above method, no other users

That handles part of the class, the rest TBD

Tgr added a comment.Oct 22 2020, 2:58 AM

Nit: there is no point in repeating the class name name in the verbs, IMO. It's a bot password store, of course it's going to insert bot passwords. Just call it insert / update / invalidateAllForUser etc.

DannyS712 added a comment.Oct 22 2020, 2:59 AM
In T265769#6570498, @Tgr wrote:

Nit: there is no point in repeating the class name name in the verbs, IMO. It's a bot password store, of course it's going to insert bot passwords. Just call it insert / update / invalidateAllForUser etc.

I find it helps with tracking uses if generic method names like 'insert' and 'update' are avoided - I'll use invalidateAllForUser, but would prefer insertPassword or insertBotPassword

Tgr added a comment.Oct 22 2020, 3:13 AM

I don't have a strong opinion on it but I'm not sure it's a good idea to build our naming conventions around plaintext search. A decent IDE can list usages of a specific method.

gerritbot added a comment.Oct 22 2020, 6:42 AM

Change 635599 merged by jenkins-bot:
[mediawiki/core@master] Cleanup BotPassword class to prepare for refactor

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.16; 2020-11-03).Oct 22 2020, 7:00 AM
Maintenance_bot removed a project: Patch-For-Review.Oct 22 2020, 7:10 AM
gerritbot added a comment.Oct 30 2020, 5:33 PM

Change 637663 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add BotPasswordStore service

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

gerritbot added a project: Patch-For-Review.Oct 30 2020, 5:33 PM
Umherirrender unsubscribed.Apr 4 2021, 7:51 PM
Umherirrender closed subtask T265767: Research to create service for CentralIdLookup::factory as Resolved.Jul 3 2021, 10:10 PM
gerritbot added a comment.Jul 15 2021, 3:59 AM

Change 637663 merged by jenkins-bot:

[mediawiki/core@master] Add BotPasswordStore service

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

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.15; 2021-07-19).Jul 15 2021, 4:00 AM
Maintenance_bot removed a project: Patch-For-Review.Jul 15 2021, 4:10 AM
gerritbot added a comment.Jul 15 2021, 4:17 AM

Change 704620 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Deprecate unused BotPassword methods

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

gerritbot added a project: Patch-For-Review.Jul 15 2021, 4:17 AM
gerritbot added a comment.Jul 15 2021, 4:38 PM

Change 704620 merged by jenkins-bot:

[mediawiki/core@master] Deprecate unused BotPassword methods

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

Maintenance_bot removed a project: Patch-For-Review.Jul 15 2021, 5:10 PM
gerritbot added a comment.Jul 15 2021, 8:27 PM

Change 704813 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Move BotPassword lookup methods to BotPasswordStore

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

gerritbot added a project: Patch-For-Review.Jul 15 2021, 8:27 PM
gerritbot added a comment.Jul 19 2021, 7:43 PM

Change 704813 merged by jenkins-bot:

[mediawiki/core@master] Move BotPassword lookup methods to BotPasswordStore

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

gerritbot added a comment.Jul 21 2021, 5:57 PM

Change 705987 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] BotPasswordStore: accept UserIdentity objects

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

gerritbot added a comment.Jul 22 2021, 12:57 AM

Change 705987 merged by jenkins-bot:

[mediawiki/core@master] BotPasswordStore: accept UserIdentity objects

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

ReleaseTaggerBot edited projects, added MW-1.37-notes (1.37.0-wmf.16; 2021-07-26); removed MW-1.37-notes (1.37.0-wmf.15; 2021-07-19).Jul 22 2021, 1:00 AM
gerritbot added a comment.Jul 28 2021, 10:04 PM

Change 708422 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Inject and use BotPasswordStore

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

gerritbot added a comment.Aug 5 2021, 9:09 PM

Change 710104 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] AuthManager: inject more services

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

gerritbot added a comment.Aug 10 2021, 12:40 PM

Change 710104 merged by jenkins-bot:

[mediawiki/core@master] AuthManager: inject more services

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

DannyS712 moved this task from In progress to Awaiting review and deployment on the User-DannyS712 board.Sep 21 2021, 11:55 PM
Aklapper edited projects, added Patch-Needs-Improvement; removed Patch-For-Review.Sep 17 2023, 4:11 PM
Restricted Application added a project: MediaWiki-Platform-Team. · View Herald TranscriptSep 17 2023, 4:11 PM
larissagaulia moved this task from Inbox, needs triage to Radar on the MediaWiki-Platform-Team board.Sep 18 2023, 12:25 PM
larissagaulia edited projects, added MediaWiki-Platform-Team (Radar); removed MediaWiki-Platform-Team.
DannyS712 removed DannyS712 as the assignee of this task.Oct 5 2023, 9:42 PM
DannyS712 subscribed.

I'm sorry that I haven't handled this task. I recently returned from a long bout of unexpected inactivity, and while I plan to resume my contributions here on Phabricator its unfair to claim tasks that I might not work on when others may be interested in handling them. I'm removing myself as the assignee in a batch-action, but if someone feels that I really should be the one to handle this task feel free to re-assign me and I'll take a look.

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