-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rewrite item link with page sitelink #24
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,55 +1,13 @@ | ||
parameters: | ||
ignoreErrors: | ||
- | ||
message: '#^Call to method addHTML\(\) on an unknown class OutputPage\.$#' | ||
identifier: class.notFound | ||
message: '#^Method ProfessionalWiki\\WikibaseFacetedSearch\\EntryPoints\\WikibaseFacetedSearchHooks\:\:rewriteLinkForNonEntityResult\(\) never assigns null to &\$titleSnippet so it can be removed from the by\-ref type\.$#' | ||
identifier: parameterByRef.unusedType | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Call to method addModuleStyles\(\) on an unknown class OutputPage\.$#' | ||
identifier: class.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Call to method getNamespace\(\) on an unknown class Title\.$#' | ||
identifier: class.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Call to static method element\(\) on an unknown class Html\.$#' | ||
identifier: class.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Constant WB_NS_ITEM not found\.$#' | ||
identifier: constant.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Parameter \$output of method ProfessionalWiki\\WikibaseFacetedSearch\\EntryPoints\\WikibaseFacetedSearchHooks\:\:onSpecialSearchResultsAppend\(\) has invalid type OutputPage\.$#' | ||
identifier: class.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Parameter \$specialSearch of method ProfessionalWiki\\WikibaseFacetedSearch\\EntryPoints\\WikibaseFacetedSearchHooks\:\:onShowSearchHitTitle\(\) has invalid type SpecialSearch\.$#' | ||
identifier: class.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Parameter \$specialSearch of method ProfessionalWiki\\WikibaseFacetedSearch\\EntryPoints\\WikibaseFacetedSearchHooks\:\:onSpecialSearchResultsAppend\(\) has invalid type SpecialSearch\.$#' | ||
identifier: class.notFound | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php | ||
|
||
- | ||
message: '#^Parameter \$title of method ProfessionalWiki\\WikibaseFacetedSearch\\EntryPoints\\WikibaseFacetedSearchHooks\:\:onShowSearchHitTitle\(\) has invalid type Title\.$#' | ||
identifier: class.notFound | ||
message: '#^Method ProfessionalWiki\\WikibaseFacetedSearch\\EntryPoints\\WikibaseFacetedSearchHooks\:\:rewriteLinkForNonEntityResult\(\) never assigns string to &\$titleSnippet so it can be removed from the by\-ref type\.$#' | ||
identifier: parameterByRef.unusedType | ||
count: 1 | ||
path: src/EntryPoints/WikibaseFacetedSearchHooks.php |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,17 +5,26 @@ | |
namespace ProfessionalWiki\WikibaseFacetedSearch\EntryPoints; | ||
|
||
use HtmlArmor; | ||
use IContextSource; | ||
use Language; | ||
use OutputPage; | ||
use ProfessionalWiki\WikibaseFacetedSearch\WikibaseFacetedSearchExtension; | ||
use SearchResult; | ||
use SpecialSearch; | ||
use Title; | ||
use Wikibase\DataModel\Entity\ItemId; | ||
use Wikibase\DataModel\Term\TermFallback; | ||
use Wikibase\Lib\Store\LanguageFallbackLabelDescriptionLookup; | ||
use Wikibase\Repo\Hooks\Formatters\EntityLinkFormatter; | ||
use Wikibase\Repo\WikibaseRepo; | ||
use Wikibase\Search\Elastic\EntityResult; | ||
|
||
class WikibaseFacetedSearchHooks { | ||
|
||
/** | ||
* @param string[] $terms | ||
* @param array<string, mixed> $query | ||
* @param array<string, mixed> $attributes | ||
* @param string[] $query | ||
* @param string[] $attributes | ||
*/ | ||
public static function onShowSearchHitTitle( | ||
Title &$title, | ||
|
@@ -26,11 +35,94 @@ public static function onShowSearchHitTitle( | |
array &$query, | ||
array &$attributes | ||
): void { | ||
if ( $title->getNamespace() !== WB_NS_ITEM ) { | ||
$itemId = self::getItemId( $title ); | ||
|
||
if ( $itemId === null ) { | ||
return; | ||
} | ||
|
||
$pageTitle = self::getItemPage( $itemId ); | ||
|
||
if ( $pageTitle === null ) { | ||
return; | ||
} | ||
|
||
// TODO: get item site link and replace $title | ||
$title = $pageTitle; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Set the new target page. For EntityResult results we just need to point to the new page, since the correct $titleSnippet gets set in 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 | ||
); | ||
} | ||
} | ||
Comment on lines
+52
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
private static function getItemId( Title $title ): ?ItemId { | ||
$entityId = WikibaseRepo::getEntityIdLookup()->getEntityIdForTitle( $title ); | ||
|
||
if ( $entityId instanceof ItemId ) { | ||
return $entityId; | ||
} | ||
|
||
return null; | ||
} | ||
|
||
private static function getItemPage( ItemId $itemId ): ?Title { | ||
return WikibaseFacetedSearchExtension::getInstance()->getItemPageLookup()->getPageTitle( $itemId ); | ||
} | ||
|
||
private static function newLinkFormatter( Language $language ): EntityLinkFormatter { | ||
return WikibaseRepo::getEntityLinkFormatterFactory()->getDefaultLinkFormatter( $language ); | ||
} | ||
|
||
private static function newLabelDescriptionLookup( IContextSource $context ): LanguageFallbackLabelDescriptionLookup { | ||
return new LanguageFallbackLabelDescriptionLookup( | ||
WikibaseRepo::getTermLookup(), | ||
WikibaseRepo::getLanguageFallbackChainFactory()->newFromContext( $context ) | ||
); | ||
} | ||
|
||
/** | ||
* @param string[] $attributes | ||
*/ | ||
private static function rewriteLinkForNonEntityResult( | ||
LanguageFallbackLabelDescriptionLookup $labelDescriptionLookup, | ||
EntityLinkFormatter $linkFormatter, | ||
ItemId $itemId, | ||
string|HtmlArmor|null &$titleSnippet, | ||
array &$attributes | ||
): void { | ||
$labelData = self::termFallbackToTermData( | ||
$labelDescriptionLookup->getLabel( $itemId ) | ||
); | ||
$descriptionData = self::termFallbackToTermData( | ||
$labelDescriptionLookup->getDescription( $itemId ) | ||
); | ||
|
||
$titleSnippet = new HtmlArmor( $linkFormatter->getHtml( $itemId, $labelData ) ); | ||
|
||
$attributes['title'] = $linkFormatter->getTitleAttribute( | ||
$itemId, | ||
$labelData, | ||
$descriptionData | ||
); | ||
} | ||
|
||
/** | ||
* @return string[]|null | ||
*/ | ||
private static function termFallbackToTermData( ?TermFallback $term = null ): ?array { | ||
if ( $term ) { | ||
return [ | ||
'value' => $term->getText(), | ||
'language' => $term->getActualLanguageCode(), | ||
]; | ||
} | ||
|
||
return null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
public static function onSpecialSearchResultsAppend( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
<?php | ||
|
||
declare( strict_types = 1 ); | ||
|
||
namespace ProfessionalWiki\WikibaseFacetedSearch\Persistence; | ||
|
||
use Title; | ||
use Wikibase\DataModel\Entity\ItemId; | ||
|
||
interface ItemPageLookup { | ||
|
||
public function getPageTitle( ItemId $itemId ): ?Title; | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
<?php | ||
|
||
declare( strict_types = 1 ); | ||
|
||
namespace ProfessionalWiki\WikibaseFacetedSearch\Persistence; | ||
|
||
use Title; | ||
use Wikibase\DataModel\Entity\ItemId; | ||
use Wikibase\Lib\Store\SiteLinkLookup; | ||
|
||
class SiteLinkItemPageLookup implements ItemPageLookup { | ||
|
||
public function __construct( | ||
private readonly SiteLinkLookup $siteLinksStore, | ||
private readonly string $siteLinkSiteId | ||
) { | ||
} | ||
|
||
public function getPageTitle( ItemId $itemId ): ?Title { | ||
$siteLink = array_filter( | ||
$this->siteLinksStore->getSiteLinksForItem( $itemId ), | ||
fn( $siteLink ) => $siteLink->getSiteId() === $this->siteLinkSiteId | ||
)[0] ?? null; | ||
malberts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if ( $siteLink === null ) { | ||
return null; | ||
} | ||
|
||
return Title::newFromText( $siteLink->getPageName() ); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
<?php | ||
|
||
declare( strict_types = 1 ); | ||
|
||
namespace ProfessionalWiki\WikibaseFacetedSearch\Tests\Persistence; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use ProfessionalWiki\WikibaseFacetedSearch\Persistence\SiteLinkItemPageLookup; | ||
use Title; | ||
use Wikibase\DataModel\Entity\Item; | ||
use Wikibase\DataModel\Entity\ItemId; | ||
use Wikibase\Lib\Store\HashSiteLinkStore; | ||
|
||
/** | ||
* @covers \ProfessionalWiki\WikibaseFacetedSearch\Persistence\SiteLinkItemPageLookup | ||
*/ | ||
class SiteLinkItemPageLookupTest extends TestCase { | ||
|
||
private const SITE_ID = 'testSiteId'; | ||
|
||
private HashSiteLinkStore $siteLinkStore; | ||
private SiteLinkItemPageLookup $lookup; | ||
|
||
protected function setUp(): void { | ||
$this->siteLinkStore = new HashSiteLinkStore(); | ||
$this->lookup = new SiteLinkItemPageLookup( | ||
$this->siteLinkStore, | ||
self::SITE_ID | ||
); | ||
} | ||
|
||
public function testReturnsPageWhenSiteLinkExists(): void { | ||
$this->createItemWithSiteLink( 'Q42', self::SITE_ID, 'Page for Q42' ); | ||
|
||
$this->assertItemPageHasTitle( 'Q42', 'Page for Q42' ); | ||
} | ||
|
||
private function createItemWithSiteLink( string $itemId, string $siteId, string $pageName ): void { | ||
$item = new Item( new ItemId( $itemId ) ); | ||
$item->getSiteLinkList()->addNewSiteLink( $siteId, $pageName ); | ||
$this->siteLinkStore->saveLinksOfItem( $item ); | ||
} | ||
|
||
private function assertItemPageHasTitle( string $itemId, $pageTitle ): void { | ||
$this->assertEquals( | ||
Title::newFromText( $pageTitle ), | ||
$this->lookup->getPageTitle( new ItemId( $itemId ) ) | ||
); | ||
} | ||
|
||
public function testReturnsNullWhenNoSiteLinksExist(): void { | ||
$this->assertNull( $this->lookup->getPageTitle( new ItemId( 'Q404' ) ) ); | ||
} | ||
|
||
public function testReturnsNullWhenOnlyOtherSiteLinksExist(): void { | ||
$this->createItemWithSiteLink( 'Q100', 'otherSiteId', 'Other page' ); | ||
$this->createItemWithSiteLink( 'Q200', 'anotherSiteId', 'Another page' ); | ||
|
||
$this->assertNull( $this->lookup->getPageTitle( new ItemId( 'Q42' ) ) ); | ||
} | ||
|
||
public function testReturnsPageWhenManySiteLinksExist(): void { | ||
$this->createItemWithSiteLink( 'Q42', self::SITE_ID, 'Page for Q42' ); | ||
$this->createItemWithSiteLink( 'Q100', 'otherSiteId', 'Other page' ); | ||
$this->createItemWithSiteLink( 'Q200', 'anotherSiteId', 'Another page' ); | ||
malberts marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
$this->assertItemPageHasTitle( 'Q42', 'Page for Q42' ); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for PHPStan to understand
class_alias()
.