Page MenuHomePhabricator

Properly display federated properties on a local item's Revision History page
Closed, ResolvedPublic8 Estimated Story Points

Description

The revision history page for an item should display the federated property used in a new or revised claim. In the first version of the MVP, we should display a property label that is hyperlinked to an external Wikidata property page.

BDD
Given a statement about an item using a federated property has been added, edited, or removed
When I load the revision history page
Then the property label is displayed in the revision description, including language fallbacks
And it is linked to the external property page on Wikidata

State before work done
A federated property on the revision history page is currently displayed as a Pxx which is linked to non-existent local property page.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 9 2020, 3:58 PM

We took an initial look at this during the task breakdown today and came to the following conclutions:

We looked at doHtmlPageLinkRendererBegin in HtmlPageLinkRendererBeginHookHandler and we probably need to implement or alter the logic here for the title -> entity Id bit, or implement one of these using the API? does that make sense? what is this title? abusing this mediawiki hook in this way for federated properties feels quite evil.

		$entityId = $foreignEntityId ?: $this->entityIdLookup->getEntityIdForTitle( $target );

We also spotted a title existence check:

		if ( $isLocal && !$target->exists() ) {

For which we might be able to use another one of our new services (if we have the entity id)

The other part of this is:

		$labelDescriptionLookup = $this->getLabelDescriptionLookup( $context );

which we should(tm) get for free once T249797 and T249799 are done.

This first area of wikitext -> title -> entityid needs to be thought about more.

We might need to rewrite most of doHtmlPageLinkRendererBegin to use our new services.
This would essentially move away from Title and towards LinkTarget and Entity based checks.
We might still need a LinkTarget -> EntityId service?

We will need to look at this more to perform an actual breakdown.

Addshore added a comment.EditedMay 11 2020, 1:12 PM

During the today's task breakdown we came up with the following rough steps:

  • Create a service that can go from a Title / LinkTarget object to an EntityId object.
    • This is essentially the reverse of TitleLookupBasedEntityTitleTextLookup with some more logic
    • The extra logic would be that from HtmlPageLinkRendererBeginHookHandler::doHtmlPageLinkRendererBegin
  • Alter ApiPrefetchingTermLookup to also allow fetching of descriptions
  • Rewrite HtmlPageLinkRendererBeginHookHandler::doHtmlPageLinkRendererBegin to use the new service
    • This hook handler would use an EntityId throughout rather than messing about with a Title object
    • This should allow the links to be rendered with labels and descriptions as expected
    • Alter DefaultEntityLinkFormatter to not use Title, instead use and EntityId and EntityTitleTextLookup
  • Use HtmlPageLinkRendererEnd in wikibase repo to override the href of property links for federated properties only (using a EntityUrlLookup service)
Samantha_Alipio_WMDE set the point value for this task to 8.May 18 2020, 8:19 AM
Jakob_WMDE removed Jakob_WMDE as the assignee of this task.May 20 2020, 10:10 AM
Jakob_WMDE added a subscriber: Jakob_WMDE.