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

Reply via email to