Show item label instead of Q-IDs if possible (#146)

* Show item label instead of Q-IDs if possible

* Fix various issues raised by CI

* Add missing argument to constructor function

* Add missing use statement

* Fix setLabel language code

* Fix language code not being string

* Use Wikibase MockRepo for test

* Set test to true for now

* Fix merge issues

* Add FacetLabelBuilder class and use existing class for label lookup

* Fix some of the lint issues

* Fix incorrect type for getFacetLabel

* Refactor getPropertyDataTypeId

* Drop public methods from FacetLabelBuilder

* Fix getFacetLabelBuilder scope

* Fix tests

* Fix tests

* Add test

* Refactor label handling with new FacetValueFormatter

* Fix lint issue

* Remove unused todo

---------

Co-authored-by: Morne Alberts <morne@malberts.net>

src/Presentation/FacetValueFormatter.php

@@ -0,0 +1,51 @@
+<?php
+
+declare( strict_types = 1 );
+
+namespace ProfessionalWiki\WikibaseFacetedSearch\Presentation;
+
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Services\Lookup\LabelLookup;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookupException;
+
+class FacetValueFormatter {
+
+	/** @var array<string, ?string> */
+	private array $propertyIdToType = [];
+
+	public function __construct(
+		private readonly PropertyDataTypeLookup $dataTypeLookup,
+		private readonly LabelLookup $labelLookup
+	) {
+	}
+
+	public function getLabel( string $value, PropertyId $propertyId ): string {
+		$dataTypeId = $this->getPropertyDataTypeId( $propertyId );
+
+		if ( $dataTypeId === null || $dataTypeId !== 'wikibase-item' ) {
+			return $value;
+		}
+
+		return $this->labelLookup->getLabel( new ItemId( $value ) )?->getText() ?? $value;
+	}
+
+	private function getPropertyDataTypeId( PropertyId $propertyId ): ?string {
+		$key = $propertyId->getSerialization();
+
+		if ( !array_key_exists( $key, $this->propertyIdToType ) ) {
+			$this->propertyIdToType[$key] = $this->getDataTypeIdForProperty( $propertyId );
+		}
+
+		return $this->propertyIdToType[$key];
+	}
+
+	private function getDataTypeIdForProperty( PropertyId $propertyId ): ?string {
+		try {
+			return $this->dataTypeLookup->getDataTypeIdForProperty( $propertyId );
+		} catch ( PropertyDataTypeLookupException ) {
+			return null;
+		}
+	}
+}

src/Presentation/ListFacetHtmlBuilder.php

@@ -24,7 +24,8 @@ class ListFacetHtmlBuilder implements FacetHtmlBuilder {
 
 	public function __construct(
 		private readonly TemplateParser $parser,
-		private readonly ValueCounter $valueCounter
+		private readonly ValueCounter $valueCounter,
+		private readonly FacetValueFormatter $valueFormatter
 	) {
 	}
 
@@ -104,7 +105,7 @@ private function buildCheckboxesViewModel( FacetConfig $config, PropertyConstrai
 
 		foreach ( $this->getValuesAndCounts( $config ) as $i => $valueCount ) {
 			$checkboxes[] = [
-				'label' => $valueCount->value,
+				'formattedValue' => $this->valueFormatter->getLabel( (string)$valueCount->value, $config->propertyId ),
 				'count' => $valueCount->count,
 				'checked' => in_array( $valueCount->value, $selectedValues ), // TODO: test with multiple types of values
 				'value' => $valueCount->value,

src/Presentation/UiBuilder.php

@@ -20,8 +20,8 @@ class UiBuilder {
 	public function __construct(
 		private readonly Config $config,
 		private readonly FacetHtmlBuilder $facetHtmlBuilder,
-		private readonly LabelLookup $labelLookup,
 		private readonly ItemTypeLabelLookup $itemTypeLabelLookup,
+		private readonly LabelLookup $labelLookup,
 		private readonly TemplateParser $templateParser,
 		private readonly QueryStringParser $queryStringParser,
 	) {
@@ -106,16 +106,16 @@ private function buildFacetsViewModel( ?ItemId $itemType, Query $query ): array
 
 	private function buildFacetViewModel( FacetConfig $facet, PropertyConstraints $state ): array {
 		return [
-			'label' => $this->getPropertyLabel( $facet->propertyId ),
+			'label' => $this->getFacetLabel( $facet->propertyId ),
 			'propertyId' => $facet->propertyId->getSerialization(),
 			'type' => $facet->type->value, // TODO: is this needed?
 			'expanded' => true, // TODO: get this from the URL somehow
 			'facetHtml' => $this->facetHtmlBuilder->buildHtml( $facet, $state )
 		];
 	}
 
-	private function getPropertyLabel( PropertyId $id ): string {
-		return $this->labelLookup->getLabel( $id )?->getText() ?? $id->getSerialization();
+	private function getFacetLabel( PropertyId $propertyId ): string {
+		return $this->labelLookup->getLabel( $propertyId )?->getText() ?? $propertyId->getSerialization();
 	}
 
 }

src/WikibaseFacetedSearchExtension.php

@@ -6,6 +6,7 @@
 
 use CirrusSearch\CirrusSearch;
 use CirrusSearch\SearchConfig;
+use MediaWiki\Html\TemplateParser;
 use MediaWiki\Language\Language;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Title\Title;
@@ -40,11 +41,11 @@
 use ProfessionalWiki\WikibaseFacetedSearch\Persistence\SitelinkBasedStatementsLookup;
 use ProfessionalWiki\WikibaseFacetedSearch\Presentation\DelegatingFacetHtmlBuilder;
 use ProfessionalWiki\WikibaseFacetedSearch\Presentation\FacetHtmlBuilder;
+use ProfessionalWiki\WikibaseFacetedSearch\Presentation\FacetValueFormatter;
 use ProfessionalWiki\WikibaseFacetedSearch\Presentation\ListFacetHtmlBuilder;
 use ProfessionalWiki\WikibaseFacetedSearch\Presentation\RangeFacetHtmlBuilder;
 use ProfessionalWiki\WikibaseFacetedSearch\Presentation\UiBuilder;
 use RuntimeException;
-use TemplateParser;
 use Wikibase\DataModel\Services\Lookup\LabelLookup;
 use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
 use Wikibase\Lib\Store\SiteLinkStore;
@@ -199,25 +200,33 @@ public function newConfigJsonValidator(): ConfigJsonValidator {
 	public function getUiBuilder( Language $language ): UiBuilder {
 		return new UiBuilder(
 			config: $this->getConfig(),
-			facetHtmlBuilder: $this->getFacetHtmlBuilder(),
-			labelLookup: $this->getLabelLookup( $language ),
+			facetHtmlBuilder: $this->getFacetHtmlBuilder( $language ),
 			itemTypeLabelLookup: $this->getItemTypeLabelLookup( $language ),
+			labelLookup: $this->getLabelLookup( $language ),
 			templateParser: $this->getTemplateParser(),
 			queryStringParser: $this->getQueryStringParser()
 		);
 	}
 
-	private function getFacetHtmlBuilder(): FacetHtmlBuilder {
+	private function getFacetHtmlBuilder( Language $language ): FacetHtmlBuilder {
 		$delegator = new DelegatingFacetHtmlBuilder();
-		$delegator->addBuilder( FacetType::LIST, $this->newListFacetHtmlBuilder() );
+		$delegator->addBuilder( FacetType::LIST, $this->newListFacetHtmlBuilder( $this->getFacetValueFormatter( $language ) ) );
 		$delegator->addBuilder( FacetType::RANGE, $this->newRangeFacetHtmlBuilder() );
 		return $delegator;
 	}
 
-	private function newListFacetHtmlBuilder(): FacetHtmlBuilder {
+	public function getFacetValueFormatter( Language $language ): FacetValueFormatter {
+		return new FacetValueFormatter(
+			dataTypeLookup: $this->getPropertyDataTypeLookup(),
+			labelLookup: $this->getLabelLookup( $language )
+		);
+	}
+
+	private function newListFacetHtmlBuilder( FacetValueFormatter $valueFormatter ): FacetHtmlBuilder {
 		return new ListFacetHtmlBuilder(
 			parser: $this->getTemplateParser(),
-			valueCounter: $this->getValueCounter()
+			valueCounter: $this->getValueCounter(),
+			valueFormatter: $valueFormatter
 		);
 	}
 

templates/FacetItemLabel.mustache

@@ -1 +1 @@
-<span class="wikibase-faceted-search__facet-label-text">{{label}}</span><span class="wikibase-faceted-search__facet-label-count">{{count}}</span>
+<span class="wikibase-faceted-search__facet-label-text">{{formattedValue}}</span><span class="wikibase-faceted-search__facet-label-count">{{count}}</span>

tests/phpunit/Presentation/FacetValueFormatterTest.php

@@ -0,0 +1,43 @@
+<?php
+
+declare( strict_types = 1 );
+
+namespace ProfessionalWiki\WikibaseFacetedSearch\Tests\Presentation;
+
+use PHPUnit\Framework\TestCase;
+use ProfessionalWiki\WikibaseFacetedSearch\Presentation\FacetValueFormatter;
+use ProfessionalWiki\WikibaseFacetedSearch\Tests\TestDoubles\StubLabelLookup;
+use ProfessionalWiki\WikibaseFacetedSearch\Tests\TestDoubles\StubPropertyDataTypeLookup;
+use Wikibase\DataModel\Entity\NumericPropertyId;
+
+/**
+ * @covers \ProfessionalWiki\WikibaseFacetedSearch\Presentation\FacetValueFormatter
+ */
+class FacetValueFormatterTest extends TestCase {
+
+	public function testGetLabelWithWikibaseItemDataType(): void {
+		$valueFormatter = $this->newFacetValueFormatter();
+
+		$this->assertSame( 'Q100', $valueFormatter->getLabel( 'Q100', new NumericPropertyId( 'P123' ) ) );
+	}
+
+	public function testGetLabelWithNonWikibaseItemDataType(): void {
+		$valueFormatter = $this->newFacetValueFormatter( new StubPropertyDataTypeLookup( 'not-wikibase-item' ) );
+
+		$this->assertSame( 'unimportant', $valueFormatter->getLabel( 'unimportant', new NumericPropertyId( 'P123' ) ) );
+	}
+
+	public function testGetLabelWithNullDataType(): void {
+		$valueFormatter = $this->newFacetValueFormatter( new StubPropertyDataTypeLookup( null ) );
+
+		$this->assertSame( 'unimportant', $valueFormatter->getLabel( 'unimportant', new NumericPropertyId( 'P123' ) ) );
+	}
+
+	private function newFacetValueFormatter( ?StubPropertyDataTypeLookup $dataTypeLookup = null, ?StubLabelLookup $labelLookup = null ): FacetValueFormatter {
+		return new FacetValueFormatter(
+			$dataTypeLookup ?? new StubPropertyDataTypeLookup( 'wikibase-item' ),
+			$labelLookup ?? new StubLabelLookup( null )
+		);
+	}
+
+}

tests/phpunit/Presentation/ListFacetHtmlBuilderTest.php

@@ -4,6 +4,7 @@
 
 namespace ProfessionalWiki\WikibaseFacetedSearch\Tests\Presentation;
 
+use MediaWiki\MediaWikiServices;
 use PHPUnit\Framework\TestCase;
 use ProfessionalWiki\WikibaseFacetedSearch\Application\FacetConfig;
 use ProfessionalWiki\WikibaseFacetedSearch\Application\FacetType;
@@ -44,7 +45,8 @@ private function newPropertyConstraints(): PropertyConstraints {
 	private function newListFacetHtmlBuilder(): ListFacetHtmlBuilder {
 		return new ListFacetHtmlBuilder(
 			parser: WikibaseFacetedSearchExtension::getInstance()->getTemplateParser(),
-			valueCounter: new StubValueCounter()
+			valueCounter: new StubValueCounter(),
+			valueFormatter: WikibaseFacetedSearchExtension::getInstance()->getFacetValueFormatter( MediaWikiServices::getInstance()->getContentLanguage() )
 		);
 	}
 
@@ -53,9 +55,9 @@ public function testCheckboxesViewModelContainsAllValues(): void {
 
 		$this->assertArrayHasKey( 'checkboxes', $viewModel );
 		$this->assertIsArray( $viewModel['checkboxes'] );
-		$this->assertSame( StubValueCounter::FIRST_VALUE, $viewModel['checkboxes'][0]['label'] );
-		$this->assertSame( StubValueCounter::SECOND_VALUE, $viewModel['checkboxes'][1]['label'] );
-		$this->assertSame( StubValueCounter::THIRD_VALUE, $viewModel['checkboxes'][2]['label'] );
+		$this->assertSame( StubValueCounter::FIRST_VALUE, $viewModel['checkboxes'][0]['formattedValue'] );
+		$this->assertSame( StubValueCounter::SECOND_VALUE, $viewModel['checkboxes'][1]['formattedValue'] );
+		$this->assertSame( StubValueCounter::THIRD_VALUE, $viewModel['checkboxes'][2]['formattedValue'] );
 		$this->assertCount( 3, $viewModel['checkboxes'] );
 	}
 

tests/phpunit/Presentation/UiBuilderUnitTest.php

@@ -45,8 +45,8 @@ private function newUiBuilder(
 		return new UiBuilder(
 			$config ?? new Config(),
 			new SpyFacetHtmlBuilder(),
-			new StubLabelLookup( null ),
 			new FakeItemTypeLabelLookup(),
+			new StubLabelLookup( null ),
 			$templateSpy ?? new SpyTemplateParser(),
 			$queryStringParser ?? new StubQueryStringParser()
 		);

tests/phpunit/TestDoubles/StubPropertyDataTypeLookup.php

@@ -0,0 +1,21 @@
+<?php
+
+declare( strict_types = 1 );
+
+namespace ProfessionalWiki\WikibaseFacetedSearch\Tests\TestDoubles;
+
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
+
+class StubPropertyDataTypeLookup implements PropertyDataTypeLookup {
+
+	public function __construct(
+		private readonly ?string $dataType
+	) {
+	}
+
+	public function getDataTypeIdForProperty( PropertyId $propertyId ): ?string {
+		return $this->dataType;
+	}
+
+}