Remove usage of ItemId::newFromNumber and PropertyId::newFromNumber
Closed, ResolvedPublic

Description

ItemId::newFromNumber and PropertyId::newFromNumber should no longer be used. Production code calling these methods should be changed to use an EntityIdComposer. Test code that uses these methods should be changed to directly call the constructor of ItemId resp PropertyId.

daniel created this task.Nov 24 2016, 6:23 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 24 2016, 6:23 PM
daniel moved this task from Inbox to To Do on the User-Daniel board.Jan 5 2017, 6:57 PM
daniel moved this task from To Do to Push on the User-Daniel board.

Change 335973 had a related patch set uploaded (by Ladsgroup):
Use EntityIdComposer instead EntityId::newFromNumber in repo

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

Ladsgroup claimed this task.Feb 4 2017, 4:25 AM
Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptFeb 4 2017, 4:25 AM

Change 335973 merged by jenkins-bot:
Use EntityIdComposer instead EntityId::newFromNumber in repo

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

Client still needs clean up.

Ladsgroup moved this task from Review to Doing on the Wikidata-Former-Sprint-Board board.

Change 337045 had a related patch set uploaded (by Ladsgroup):
Use EntityIdComposer instead EntityId::newFromNumber in client

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

Change 337806 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Fix violations caused by to generic, not needed EntityIdComposers

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

Aleksey_WMDE triaged this task as Low priority.Feb 20 2017, 6:44 PM

Change 337045 abandoned by Thiemo Mättig (WMDE):
Use EntityIdComposer instead EntityId::newFromNumber in client

Reason:
Using the composer only makes sense if either the entity type is variable, or the repo prefix is (or if there is a chance we will make one of it variable in the near future). Neither is the case here. We know the repo prefix is empty. We know the type is "item". Nothing wrong with using the static constructor then. The "wb_items_per_site" table that is used here (via SiteLinkTable) does not support repo prefixes, and as far as I know it does not need to.

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

Ladsgroup removed Ladsgroup as the assignee of this task.Mar 15 2017, 9:05 PM
Ladsgroup removed a project: User-Ladsgroup.
Ladsgroup added a subscriber: Ladsgroup.
Lydia_Pintscher moved this task from incoming to ready to go on the Wikidata board.

Change 391187 had a related patch set uploaded (by Thiemo Mättig (WMDE); owner: Thiemo Mättig (WMDE)):
[mediawiki/extensions/ArticlePlaceholder@master] Refactor ItemNotabilityFilter to avoid ItemId::newFromNumber

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

thiemowmde closed this task as Resolved.
thiemowmde claimed this task.

I had a look at the remaining usages and they are all fine. All remaining calls of one of these newFromNumber methods are either done on a number as returned by Int32EntityId::getNumericId, which is well specified and encapsulated. Or they are because of T114904: Migrate wb_items_per_site to using prefixed entity IDs instead of numeric IDs, but that has it's own ticket already.

Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptNov 14 2017, 11:23 AM

Change 391187 merged by jenkins-bot:
[mediawiki/extensions/ArticlePlaceholder@master] Refactor ItemNotabilityFilter to avoid ItemId::newFromNumber

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

Change 337806 abandoned by Thiemo Kreuz (WMDE):
Fix violations caused by to generic, not needed EntityIdComposers

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