Rewrite item link with page sitelink #24
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24 +/- ##
=============================================
- Coverage 33.33% 18.91% -14.42%
- Complexity 4 18 +14
=============================================
Files 2 3 +1
Lines 12 74 +62
=============================================
+ Hits 4 14 +10
- Misses 8 60 +52 ☔ View full report in Codecov by Sentry. |
malberts
force-pushed
the
sitelink
branch
2 times, most recently
from
5780501 to
04cb34d
Compare
| - ../../extensions/Wikibase | ||
| - ../../extensions/WikibaseCirrusSearch | ||
| bootstrapFiles: | ||
| - ../../includes/AutoLoader.php |
There was a problem hiding this comment.
This is for PHPStan to understand class_alias().
|
All 4 scenarios work after rewriting this.
The default, additional language text also works: |
| return; | ||
| } | ||
|
|
||
| // TODO: get item site link and replace $title | ||
| $title = $pageTitle; |
There was a problem hiding this comment.
Set the new target page.
For EntityResult results we just need to point to the new page, since the correct $titleSnippet gets set in CirrusShowSearchHitHandler::onShowSearchHitTitle().
For non-EntityResult we also need to rewrite the text below.
| if ( !( $result instanceof EntityResult ) ) { | ||
| self::rewriteLinkForNonEntityResult( | ||
| self::newLabelDescriptionLookup( $specialSearch->getContext() ), | ||
| self::newLinkFormatter( $specialSearch->getLanguage() ), | ||
| $itemId, | ||
| $titleSnippet, | ||
| $attributes | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
For non-EntityResult there is no $titleSnippet by default, which means the link is going to use the text of the target page title, not the item title.
To get the item title again, this calls a minimal implementation based on HtmlPageLinkRendererEndHookHandler::internalDoHtmlPageLinkRendererEnd().
malberts
force-pushed
the
sitelink
branch
2 times, most recently
from
633f757 to
43d72f7
Compare
| ]; | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
There is a lot of static code here. Likely we can move much to one or more classes that do not bind to globals and can be tested easier
For #4
The search results behave differently depending on search criteria.
When searching with a mix of namespaces (Item and Main):
(Item does not get text highlight)
With just Item namespace:
(Item gets text highlight)
Search for Item ID with mix of namespaces:
Search for Item ID with just Item namespace:
I consider (3) broken, because without this PR it shows the same link title as the other scenarios. I will return to this in a follow-up, so that the initial code here can go in.