Aude has uploaded a new change for review. https://gerrit.wikimedia.org/r/180481
Change subject: Fix EditFilter bug with creating items and properties ...................................................................... Fix EditFilter bug with creating items and properties Too often, other extensions make the assumption that context title is the target title (of the new content), and then run into bugs when the EditFilter hook is triggered. We set title / id later, after the hook is run. This patch moves id assignment earlier, before the filter hook. This means we might burn through more ids, if filter is triggered or other reason that the entity ends up not being created. Bug: T78645 Change-Id: Ie71ebdebd18cfd217daae6db1881951d356f88b0 --- M repo/includes/EditEntity.php M repo/tests/phpunit/includes/EditEntityTest.php 2 files changed, 31 insertions(+), 10 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/81/180481/1 diff --git a/repo/includes/EditEntity.php b/repo/includes/EditEntity.php index 4730e51..e726fa8 100644 --- a/repo/includes/EditEntity.php +++ b/repo/includes/EditEntity.php @@ -13,6 +13,7 @@ use Title; use User; use Wikibase\DataModel\Entity\Entity; +use Wikibase\DataModel\Entity\EntityDocument; use Wikibase\DataModel\Entity\EntityId; use Wikibase\Lib\Store\EntityRevisionLookup; use Wikibase\Lib\Store\EntityStore; @@ -761,21 +762,13 @@ return; } - if ( !$this->isNew() ) { - $context = clone $this->context; - - $title = $this->getTitle(); - $context->setTitle( $title ); - $context->setWikiPage( new WikiPage( $title ) ); - } else { - $context = $this->context; - } - // Run edit filter hooks $filterStatus = Status::newGood(); $entityContentFactory = WikibaseRepo::getDefaultInstance()->getEntityContentFactory(); $entityContent = $entityContentFactory->newFromEntity( $this->newEntity ); + + $context = $this->getContextForFilter( $this->newEntity ); if ( !wfRunHooks( 'EditFilterMergedContent', array( $context, $entityContent, &$filterStatus, $summary, $this->getUser(), false ) ) ) { @@ -791,6 +784,25 @@ $this->status->merge( $filterStatus ); } + /** + * EntityDocument $entity + */ + private function getContextForFilter( EntityDocument $entity ) { + if ( !$this->isNew() ) { + $context = clone $this->context; + $title = $this->getTitle(); + } else { + $context = $this->context; + $this->entityStore->assignFreshId( $entity ); + $title = $this->titleLookup->getTitleForId( $entity->getId() ); + } + + $context->setTitle( $title ); + $context->setWikiPage( new WikiPage( $title ) ); + + return $context; + } + protected function applyPreSaveChecks() { if ( $this->hasEditConflict() ) { if ( !$this->fixEditConflict() ) { diff --git a/repo/tests/phpunit/includes/EditEntityTest.php b/repo/tests/phpunit/includes/EditEntityTest.php index 0daedb9..898a635 100644 --- a/repo/tests/phpunit/includes/EditEntityTest.php +++ b/repo/tests/phpunit/includes/EditEntityTest.php @@ -315,6 +315,15 @@ } } + public function testAttemptSaveWithNewEntity() { + $item = Item::newEmpty(); + $item->setLabel( 'en', 'omg' ); + $editEntity = $this->makeEditEntity( $this->makeMockRepo(), $item ); + $editEntity->attemptSave( "Testing", EDIT_NEW, false ); + + $this->assertTrue( $item->getId() !== null, 'item id is set' ); + } + private function fingerprintToPartialArray( Fingerprint $fingerprint ) { return array( 'label' => $fingerprint->getLabels()->toTextArray(), -- To view, visit https://gerrit.wikimedia.org/r/180481 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie71ebdebd18cfd217daae6db1881951d356f88b0 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: wmf/1.25wmf12c Gerrit-Owner: Aude <aude.w...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits