Page MenuHomePhabricator

Split up and servicify ChangeTags class
Open, MediumPublic

Description

Pending:

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/597913/
Move defineTag, undefineTag, deleteTagEverywhere, and purgeTagCacheAll to new ChangeTagStore service

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/598222/
Move getSoftwareTags, listSoftwareActivatedTags, listDefinedTags, listExplicitlyDefinedTags, and listSoftwareDefinedTags to new ChangeTagLookup service

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/598226/
Move modifyDisplayQuery and makeTagSummarySubquery to new ChangeTagQueryUtils service

Remaining work to split up the class into narrow services tracked at P11294

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Encapsulating a global state into a class is always a good idea.
At the moment I'm not quite familiar with that area, so can't give you more valuable suggestion.

But I'd appreciate it if you add me as Reviewer.

::tagShortDescriptionMessage, ::tagDescription, ::tagLongDescriptionMessage, ::truncateTagDescription, ::isTagNameValid
should be moved to a ChangeTag (singular) object that represents a specific tag, including providing its description methods

The patch for T27909 does this (sort of).

Change 597913 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add a ChangeTagStore service for defining and undefining tags

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

DannyS712 updated the task description. (Show Details)

Change 597913 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add a ChangeTagStore service for defining and undefining tags

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

Next up: ChangeTagLookup, which replaces:

  • getSoftwareTags
  • listSoftwareActivatedTags
  • listDefinedTags
  • listExplicitlyDefinedTags
  • listSoftwareDefinedTags

Change 598222 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add a ChangeTagLookup store for looking up different lists of tags

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

DannyS712 renamed this task from Factor some static methods out of the ChangeTags class to Split up and servicify ChangeTags class.May 24 2020, 6:29 AM
DannyS712 updated the task description. (Show Details)

Change 598226 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Add a new ChangeTagQueryUtils service for adding tag info to queries

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

Notes to help with review:

  • Each patch adds a narrow service for a clear group of methods in ChangeTags that go together, rather than trying to add one service for everything
  • Each patch only adds the new service and updates the ChangesTags method to use it, making no other replacements and making no official deprecations
  • Each patch includes 100% code coverage for the new service with pure unit testing