Page MenuHomePhabricator

OutputPageBeforeHTMLHookHandler: Remove old fallback for missing OutputPage property
Closed, ResolvedPublic3 Estimated Story Points

Description

OutputPageBeforeHTMLHookHandler retrieves termbox entries for entity pages from the OutputPage (see OutputPageBeforeHTMLHookHandler::getEntityTermsListHtml). If this OutputPage property is empty we fall back to retrieving the full entity to extract the terms from that.
OutputPageBeforeHTMLHookHandler::getEntity claims that in this case // The parser cache content is too old to contain the terms list items, but given this was added years ago (in 4b52cd637a1 in 2016), this surely isn't the case anymore.

To do:

  1. Make sure the wikibase-terms-list-items property is actually set in all relevant cases
  2. Remove the fallback handling from OutputPageBeforeHTMLHookHandler and EntityViewPlaceholderExpander

Event Timeline

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

Yeah, I was also wondering if this was still relevant when I looked at it… can probably be removed indeed. (Maybe we can first log a warning if it’s missing and see if that shows up in logstash?)

Change 934322 had a related patch set uploaded (by Hoo man; author: Hoo man):

[mediawiki/extensions/Wikibase@master] OutputPageBeforeHTMLHookHandler: Log when loading full entities

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

Yeah, I was also wondering if this was still relevant when I looked at it… can probably be removed indeed. (Maybe we can first log a warning if it’s missing and see if that shows up in logstash?)

Sounds good to me, I've uploaded a patch. Once that has been deployed for a while, we should have enough clarity here to be able to remove this code (or adopt the comment/ fix the callers).

Change 934322 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] OutputPageBeforeHTMLHookHandler: Log when loading full entities

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

No T340642 warnings in logstash for the past 7 days as far as I can see (and Wikidata has also been on wmf.16 for that long, see SAL).

Change 938237 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/Wikibase@master] OutputPageBeforeHTMLHookHandler: Don’t load full entities

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

Change 938237 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] OutputPageBeforeHTMLHookHandler: Don’t load full entities

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

Lucas_Werkmeister_WMDE assigned this task to hoo.
Lucas_Werkmeister_WMDE set the point value for this task to 3.

I think this is done; I estimate at 3 points, feel free to change it.