Aude has uploaded a new change for review.
https://gerrit.wikimedia.org/r/292987
Change subject: Fix display of outdated label and description in EntityView
header
......................................................................
Fix display of outdated label and description in EntityView header
Problem was use of EntityInfoTermLookup, which retrieves terms from
the terms table (via SqlEntityInfoBuilder).
When an edit is made, ParserOutput (including page html) is generated
before save and before the terms table is updated. So, whatever the
terms were in the previous revision of an entity is what was displayed.
This changes the code to use an EntityRetrievingTermLookup, with an
InMemoryEntityLookup (populated with the entity that is being saved).
This allows the language fallback to still be applied.
More ideal would be a solution with less layers of indirection,
and maybe not use LabelDescriptionLookup, though I think more refactoring
is needed to make this possible.
This is the smallest patch possible to fix the bug.
I am not sure how best to add tests for this, other than to have
a browser test that covers this and maybe we already have one?
DEPLOY: there is an off-chance that this may affect the operation of
SpamBlacklist and AbuseFilter. We did our best to check for this, but
we should send a heads-up to wiki admins when deploying this, and we
should double-check that the filters work after deployment.
Bug: T135714
Change-Id: I1062d4451eca0218b3481fe2c13743273552fb4d
(cherry picked from commit ed1ca9bbc886e1bf00a807fea9cc2c5272ce7c7f)
---
M repo/includes/ParserOutput/EntityParserOutputGenerator.php
M repo/includes/ParserOutput/PlaceholderEmittingEntityTermsView.php
M repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
M view/src/SimpleEntityTermsView.php
4 files changed, 78 insertions(+), 37 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/87/292987/1
diff --git a/repo/includes/ParserOutput/EntityParserOutputGenerator.php
b/repo/includes/ParserOutput/EntityParserOutputGenerator.php
index 7fcfcf1..aeda587 100644
--- a/repo/includes/ParserOutput/EntityParserOutputGenerator.php
+++ b/repo/includes/ParserOutput/EntityParserOutputGenerator.php
@@ -7,6 +7,8 @@
use SpecialPage;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Services\Lookup\EntityRetrievingTermLookup;
+use Wikibase\DataModel\Services\Lookup\InMemoryEntityLookup;
use Wikibase\DataModel\Term\AliasesProvider;
use Wikibase\DataModel\Term\LabelsProvider;
use Wikibase\LanguageFallbackChain;
@@ -157,13 +159,11 @@
$parserOutput->addJsConfigVars( $configVars );
$parserOutput->setExtensionData( 'wikibase-titletext',
$this->getTitleText( $entity ) );
- $entityId = $entity->getId();
-
if ( $generateHtml ) {
$this->addHtmlToParserOutput(
$parserOutput,
$entity,
- $this->getEntityInfo( $parserOutput, $entityId )
+ $this->getEntityInfo( $parserOutput )
);
} else {
// If we don't have HTML, the ParserOutput in question
@@ -184,6 +184,7 @@
// Sometimes extensions like SpamBlacklist might call
getParserOutput
// before the id is assigned, during the process of creating a
new entity.
// in that case, no alternate links are added, which probably
is no problem.
+ $entityId = $entity->getId();
if ( $entityId !== null ) {
$this->addAlternateLinks( $parserOutput, $entityId );
}
@@ -195,11 +196,10 @@
* Fetches some basic entity information from a set of entity IDs.
*
* @param ParserOutput $parserOutput
- * @param EntityId|null $entityId
*
* @return EntityInfo
*/
- private function getEntityInfo( ParserOutput $parserOutput, EntityId
$entityId = null ) {
+ private function getEntityInfo( ParserOutput $parserOutput ) {
/**
* Set in ReferencedEntitiesDataUpdater.
*
@@ -212,10 +212,6 @@
wfLogWarning( '$entityIds from ParserOutput
"referenced-entities" extension data'
. ' expected to be an array' );
$entityIds = [];
- }
-
- if ( $entityId !== null ) {
- $entityIds[] = $entityId;
}
$entityInfoBuilder =
$this->entityInfoBuilderFactory->newEntityInfoBuilder( $entityIds );
@@ -272,8 +268,13 @@
EntityDocument $entity,
EntityInfo $entityInfo
) {
- $labelDescriptionLookup = new
LanguageFallbackLabelDescriptionLookup(
- new EntityInfoTermLookup( $entityInfo ),
+ $entityLookup = new InMemoryEntityLookup();
+ if ( $entity->getId() !== null ) {
+ $entityLookup->addEntity( $entity );
+ }
+
+ $entityLabelDescriptionLookup = new
LanguageFallbackLabelDescriptionLookup(
+ new EntityRetrievingTermLookup( $entityLookup ),
$this->languageFallbackChain
);
@@ -298,7 +299,7 @@
$languageDirectionalityLookup,
$languageNameLookup
),
- $labelDescriptionLookup,
+ $entityLabelDescriptionLookup,
$this->templateFactory,
$editSectionGenerator,
$this->textProvider,
@@ -306,10 +307,15 @@
$textInjector
);
+ $referencedEntitiesLabelDescriptionLookup = new
LanguageFallbackLabelDescriptionLookup(
+ new EntityInfoTermLookup( $entityInfo ),
+ $this->languageFallbackChain
+ );
+
$entityView = $this->entityViewFactory->newEntityView(
$entity->getType(),
$this->languageCode,
- $labelDescriptionLookup,
+ $referencedEntitiesLabelDescriptionLookup,
$this->languageFallbackChain,
$editSectionGenerator,
$entityTermsView
diff --git a/repo/includes/ParserOutput/PlaceholderEmittingEntityTermsView.php
b/repo/includes/ParserOutput/PlaceholderEmittingEntityTermsView.php
index 06c1355..a472ca6 100644
--- a/repo/includes/ParserOutput/PlaceholderEmittingEntityTermsView.php
+++ b/repo/includes/ParserOutput/PlaceholderEmittingEntityTermsView.php
@@ -40,9 +40,13 @@
private $textInjector;
/**
+ * @param HtmlTermRenderer $htmlTermRenderer
+ * @param LabelDescriptionLookup $labelDescriptionLookup
* @param TemplateFactory $templateFactory
* @param EditSectionGenerator $sectionEditLinkGenerator
* @param LocalizedTextProvider $textProvider
+ * @param TermListView $termListView
+ * @param TextInjector $textInjector
*/
public function __construct(
HtmlTermRenderer $htmlTermRenderer,
diff --git
a/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
b/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
index 3c871ad..725342a 100644
---
a/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
+++
b/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
@@ -7,6 +7,7 @@
use SpecialPage;
use Title;
use Wikibase\DataModel\Entity\BasicEntityIdParser;
+use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
@@ -40,12 +41,43 @@
*/
class EntityParserOutputGeneratorTest extends MediaWikiTestCase {
- public function testGetParserOutput() {
+ public function provideTestGetParserOutput() {
+ return [
+ [
+ $this->newItem(),
+ 'kitten item' ,
+ [ 'http://an.url.com',
'https://another.url.org' ],
+ [ 'File:This_is_a_file.pdf', 'File:Selfie.jpg'
],
+ [
+ new ItemId( 'Q42' ),
+ new ItemId( 'Q35' ),
+ new PropertyId( 'P42' ),
+ new PropertyId( 'P10' )
+ ],
+ ],
+ [ new Item(), null, [], [], [] ]
+ ];
+ }
+
+ /**
+ * EntityDocument $entity
+ * string|null $titleText
+ * string[] $externalLinks
+ * string[] $images
+ * EntityId[] $referencedEntities
+ *
+ * @dataProvider provideTestGetParserOutput
+ */
+ public function testGetParserOutput(
+ EntityDocument $entity,
+ $titleText,
+ array $externalLinks,
+ array $images,
+ array $referencedEntities
+ ) {
$entityParserOutputGenerator =
$this->newEntityParserOutputGenerator();
- $item = $this->newItem();
-
- $parserOutput = $entityParserOutputGenerator->getParserOutput(
$item );
+ $parserOutput = $entityParserOutputGenerator->getParserOutput(
$entity );
$this->assertSame( '<TITLE>', $parserOutput->getTitleText(),
'title text' );
$this->assertSame( '<HTML>', $parserOutput->getText(), 'html
text' );
@@ -60,19 +92,19 @@
$this->assertSame( array( '<JS>' ),
$parserOutput->getJsConfigVars(), 'config vars' );
$this->assertEquals(
- 'kitten item',
+ $titleText,
$parserOutput->getExtensionData( 'wikibase-titletext' ),
'title text'
);
$this->assertEquals(
- array( 'http://an.url.com', 'https://another.url.org' ),
+ $externalLinks,
array_keys( $parserOutput->getExternalLinks() ),
'external links'
);
$this->assertEquals(
- array( 'File:This_is_a_file.pdf', 'File:Selfie.jpg' ),
+ $images,
array_keys( $parserOutput->getImages() ),
'images'
);
@@ -85,31 +117,30 @@
// );
$this->assertArrayEquals(
- array(
- new ItemId( 'Q42' ),
- new ItemId( 'Q35' ),
- new PropertyId( 'P42' ),
- new PropertyId( 'P10' )
- ),
+ $referencedEntities,
$parserOutput->getExtensionData( 'referenced-entities' )
);
- $jsonHref = SpecialPage::getTitleFor( 'EntityData',
$item->getId()->getSerialization() . '.json' )->getCanonicalURL();
- $ntHref = SpecialPage::getTitleFor( 'EntityData',
$item->getId()->getSerialization() . '.nt' )->getCanonicalURL();
-
- $this->assertEquals(
- array(
- array(
+ $alternateLinks = null;
+ if ( $entity->getId() ) {
+ $jsonHref = SpecialPage::getTitleFor( 'EntityData',
$entity->getId()->getSerialization() . '.json' )->getCanonicalURL();
+ $ntHref = SpecialPage::getTitleFor( 'EntityData',
$entity->getId()->getSerialization() . '.nt' )->getCanonicalURL();
+ $alternateLinks = [
+ [
'rel' => 'alternate',
'href' => $jsonHref,
'type' => 'application/json'
- ),
- array(
+ ],
+ [
'rel' => 'alternate',
'href' => $ntHref,
'type' => 'application/n-triples'
- )
- ),
+ ]
+ ];
+ }
+
+ $this->assertEquals(
+ $alternateLinks,
$parserOutput->getExtensionData(
'wikibase-alternate-links' ),
'alternate links (extension data)'
);
diff --git a/view/src/SimpleEntityTermsView.php
b/view/src/SimpleEntityTermsView.php
index eed0969..d80d42e 100644
--- a/view/src/SimpleEntityTermsView.php
+++ b/view/src/SimpleEntityTermsView.php
@@ -31,7 +31,7 @@
private $htmlTermRenderer;
/**
- * @var LabelDescriptionLookup
+ * @var LabelDescriptionLookup For getting label/description of entity
being rendered.
*/
private $labelDescriptionLookup;
--
To view, visit https://gerrit.wikimedia.org/r/292987
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1062d4451eca0218b3481fe2c13743273552fb4d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: wmf/1.28.0-wmf.3
Gerrit-Owner: Aude <aude.w...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
