Hoo man has uploaded a new change for review.
https://gerrit.wikimedia.org/r/246190
Change subject: Fix rc comments for pages directly affected by a sitelink change
......................................................................
Fix rc comments for pages directly affected by a sitelink change
Bug: T114664
Change-Id: I1ad53554af6ae6e38241bbb8fa0be6dd145495a2
---
M client/includes/Changes/ChangeHandler.php
M client/includes/Changes/WikiPageUpdater.php
M client/includes/SiteLinkCommentCreator.php
M client/tests/phpunit/includes/Changes/MockPageUpdater.php
M client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
5 files changed, 87 insertions(+), 33 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/90/246190/1
diff --git a/client/includes/Changes/ChangeHandler.php
b/client/includes/Changes/ChangeHandler.php
index 83d91fb..0e7d26d 100644
--- a/client/includes/Changes/ChangeHandler.php
+++ b/client/includes/Changes/ChangeHandler.php
@@ -276,14 +276,17 @@
break;
case self::RC_ENTRY_ACTION:
- $rcAttribs = $this->getRCAttributes( $change );
+ if ( !$this->injectRecentChanges ) {
+ break;
+ }
+ $rcAttribsByPage = $this->getRCAttributes(
$change, $titlesToUpdate );
- if ( $rcAttribs !== false &&
$this->injectRecentChanges ) {
+ if ( $rcAttribsByPage !== array() ) {
//FIXME: The same change may be
reported to several target pages;
// The comment we generate should
be adapted to the role that page
// plays in the change, e.g. when
a sitelink changes from one page to another,
// the link was effectively
removed from one and added to the other page.
- $this->updater->injectRCRecords(
$titlesToUpdate, $rcAttribs );
+ $this->updater->injectRCRecords(
$titlesToUpdate, $rcAttribsByPage );
}
break;
@@ -335,16 +338,33 @@
}
/**
+ * @param EntityChange $change The Change that caused the update
+ * @param Title[] $titles
+ *
+ * @return array[] Page id -> array of RC attributes
+ */
+ private function getRCAttributes( EntityChange $change, array $titles )
{
+ $res = array();
+
+ foreach ( $titles as $title ) {
+ $res[ $title->getArticleID() ] =
$this->getRCAttributesForTitle( $change, $title );
+ }
+
+ return $res;
+ }
+
+ /**
* Constructs RC attributes for the given change
*
* @see ExternalRecentChange::buildAttributes
*
* @param EntityChange $change The Change that caused the update
+ * @param Title $title The page we get attribs for
*
* @return array[]|bool an array of RC attributes,
* as understood by ExternalRecentChange::buildAttributes.
*/
- private function getRCAttributes( EntityChange $change ) {
+ private function getRCAttributesForTitle( EntityChange $change, Title
$title ) {
$rcinfo = $change->getMetadata();
//@todo: add getFields() to the interface, or provide getters!
@@ -364,7 +384,7 @@
$changeParams['site_id'] = $this->repoId;
}
- $comment = $this->getEditCommentMulti( $changesForComment );
+ $comment = $this->getEditCommentMulti( $changesForComment,
$title );
// Use keys known to ExternalRecentChange::buildAttributes.
// FIXME: Simplify the way this is passed around.
@@ -380,17 +400,18 @@
* Returns a human readable comment representing the given changes.
*
* @param EntityChange[] $changes
+ * @param Title $title The page we create an edit summary for
*
* @throws MWException
* @return string
*/
- private function getEditCommentMulti( array $changes ) {
+ private function getEditCommentMulti( array $changes, Title $title ) {
$comments = array();
$c = 0;
foreach ( $changes as $change ) {
$c++;
- $comments[] = $this->getEditComment( $change );
+ $comments[] = $this->getEditComment( $change , $title );
}
if ( $c === 0 ) {
@@ -409,11 +430,12 @@
* @since 0.4
*
* @param EntityChange $change the change to get a comment for
+ * @param Title $title The page we create an edit summary for
*
* @throws MWException
* @return string
*/
- public function getEditComment( EntityChange $change ) {
+ public function getEditComment( EntityChange $change, Title $title ) {
$siteLinkDiff = $change instanceof ItemChange
? $change->getSiteLinkDiff()
: null;
@@ -423,7 +445,7 @@
if ( $siteLinkDiff !== null && !$siteLinkDiff->isEmpty() ) {
$action = $change->getAction();
$commentCreator = new SiteLinkCommentCreator(
$this->language, $this->siteStore, $this->localSiteId );
- $siteLinkComment = $commentCreator->getEditComment(
$siteLinkDiff, $action );
+ $siteLinkComment = $commentCreator->getEditComment(
$siteLinkDiff, $action, $title );
$editComment = $siteLinkComment === null ? '' :
$siteLinkComment;
}
diff --git a/client/includes/Changes/WikiPageUpdater.php
b/client/includes/Changes/WikiPageUpdater.php
index f898d45..0b39bde 100644
--- a/client/includes/Changes/WikiPageUpdater.php
+++ b/client/includes/Changes/WikiPageUpdater.php
@@ -80,14 +80,19 @@
* Injects an RC entry into the recentchanges, using the the given
title and attribs
*
* @param Title[] $titles
- * @param array $attribs
+ * @param array $attribsByTitle Page id -> array of rc attribs
*/
- public function injectRCRecords( array $titles, array $attribs ) {
+ public function injectRCRecords( array $titles, array $attribsByTitle )
{
foreach ( $titles as $title ) {
if ( !$title->exists() ) {
continue;
}
+ $attribs = $attribsByTitle[ $title->getArticleID() ];
+ if ( $attribs === false ) {
+ continue;
+ }
+
$rc = ExternalRecentChange::newFromAttribs( $attribs,
$title );
wfDebugLog( __CLASS__, __FUNCTION__ . ": saving RC
entry for " . $title->getFullText() );
diff --git a/client/includes/SiteLinkCommentCreator.php
b/client/includes/SiteLinkCommentCreator.php
index 3e8ba59..a9b8213 100644
--- a/client/includes/SiteLinkCommentCreator.php
+++ b/client/includes/SiteLinkCommentCreator.php
@@ -11,6 +11,7 @@
use Message;
use MWException;
use SiteStore;
+use Title;
/**
* Creates an array structure with comment information for storing
@@ -59,13 +60,14 @@
*
* @param Diff|null $siteLinkDiff
* @param string $action e.g. 'remove', see the constants in
EntityChange
+ * @param Title $title The page we create an edit summary for
*
* @return string|null A human readable edit summary (limited wikitext),
* or null if no summary could be created for the sitelink
change.
*/
- public function getEditComment( Diff $siteLinkDiff = null, $action ) {
+ public function getEditComment( Diff $siteLinkDiff = null, $action,
Title $title ) {
if ( $siteLinkDiff !== null && !$siteLinkDiff->isEmpty() ) {
- $siteLinkMessage = $this->getSiteLinkMessage( $action,
$siteLinkDiff );
+ $siteLinkMessage = $this->getSiteLinkMessage( $action,
$siteLinkDiff, $title );
if ( !empty( $siteLinkMessage ) ) {
return $this->generateComment( $siteLinkMessage
);
@@ -81,10 +83,11 @@
*
* @param string $action e.g. 'remove', see the constants in
EntityChange
* @param Diff $siteLinkDiff The change's site link diff
+ * @param Title $title The page we create an edit summary for
*
* @return array|null
*/
- private function getSiteLinkMessage( $action, Diff $siteLinkDiff ) {
+ private function getSiteLinkMessage( $action, Diff $siteLinkDiff, Title
$title ) {
if ( $siteLinkDiff->isEmpty() ) {
return null;
}
@@ -110,7 +113,7 @@
}
}
- $params = $this->getSiteLinkAddRemoveParams( $diffOp,
$action, $siteId );
+ $params = $this->getSiteLinkAddRemoveParams( $diffOp,
$action, $siteId, $title );
} else {
$diffOpCount = count( $diffOps );
if ( $diffOpCount === 1 ) {
@@ -206,10 +209,11 @@
* @param DiffOp $diffOp
* @param string $action e.g. 'remove', see the constants in
EntityChange
* @param string $siteId
+ * @param Title $title The page we create an edit summary for
*
* @return array|null
*/
- private function getSiteLinkAddRemoveParams( DiffOp $diffOp, $action,
$siteId ) {
+ private function getSiteLinkAddRemoveParams( DiffOp $diffOp, $action,
$siteId, Title $title ) {
$params = array();
if ( in_array( $action, array( 'remove', 'restore' ) ) ) {
@@ -220,18 +224,24 @@
} elseif ( $diffOp instanceof DiffOpRemove ) {
$params['message'] = 'wikibase-comment-unlink';
} elseif ( $diffOp instanceof DiffOpChange ) {
- $params['message'] = 'wikibase-comment-sitelink-change';
+ if ( $title->getFullText() === $diffOp->getOldValue() )
{
+ $params['message'] = 'wikibase-comment-unlink';
+ } elseif ( $title->getFullText() ===
$diffOp->getNewValue() ) {
+ $params['message'] = 'wikibase-comment-linked';
+ } else {
+ $params['message'] =
'wikibase-comment-sitelink-change';
- $params['sitelink'] = array(
- 'oldlink' => array(
- 'site' => $siteId,
- 'page' => $diffOp->getOldValue()
- ),
- 'newlink' => array(
- 'site' => $siteId,
- 'page' => $diffOp->getNewValue()
- )
- );
+ $params['sitelink'] = array(
+ 'oldlink' => array(
+ 'site' => $siteId,
+ 'page' => $diffOp->getOldValue()
+ ),
+ 'newlink' => array(
+ 'site' => $siteId,
+ 'page' => $diffOp->getNewValue()
+ )
+ );
+ }
} else {
// whatever
$params = null;
diff --git a/client/tests/phpunit/includes/Changes/MockPageUpdater.php
b/client/tests/phpunit/includes/Changes/MockPageUpdater.php
index 8213d51..5836640 100644
--- a/client/tests/phpunit/includes/Changes/MockPageUpdater.php
+++ b/client/tests/phpunit/includes/Changes/MockPageUpdater.php
@@ -57,11 +57,17 @@
/**
* @param Title[] $titles
- * @param array $attribs
+ * @param array $attribsByTitle Page id -> array of rc attribs
*/
- public function injectRCRecords( array $titles, array $attribs ) {
+ public function injectRCRecords( array $titles, array $attribsByTitle )
{
foreach ( $titles as $title ) {
$key = $title->getPrefixedDBkey();
+
+ $attribs = $attribsByTitle[ $title->getArticleID() ];
+ if ( $attribs === false ) {
+ continue;
+ }
+
$this->updates['injectRCRecord'][ $key ] = $attribs;
}
}
diff --git a/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
b/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
index 3ffc0de..31555cd 100644
--- a/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
+++ b/client/tests/phpunit/includes/SiteLinkCommentCreatorTest.php
@@ -5,6 +5,7 @@
use Diff\DiffOp\Diff\Diff;
use Diff\DiffOp\DiffOpChange;
use Language;
+use Title;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\ItemChange;
@@ -29,7 +30,7 @@
*/
public function testGetEditComment( Diff $siteLinkDiff, $action,
$expected ) {
$commentCreator = new SiteLinkCommentCreator(
Language::factory( 'qqx' ), MockSiteStore::newFromTestSites(), 'enwiki' );
- $comment = $commentCreator->getEditComment( $siteLinkDiff,
$action );
+ $comment = $commentCreator->getEditComment( $siteLinkDiff,
$action, Title::newFromText( 'A fancy page' ) );
$this->assertEquals( $expected, $comment );
}
@@ -168,13 +169,13 @@
return $change->getSiteLinkDiff();
}
- protected function getChangeLinkDiff() {
+ protected function getChangeLinkDiff( $oldName = 'Japan', $newName =
'Japan' ) {
$item = $this->getNewItem();
- $item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
+ $item->getSiteLinkList()->addNewSiteLink( 'enwiki', $oldName );
$item->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Japan' );
$item2 = $this->getNewItem();
- $item2->getSiteLinkList()->addNewSiteLink( 'enwiki', 'Japan' );
+ $item2->getSiteLinkList()->addNewSiteLink( 'enwiki', $newName );
$item2->getSiteLinkList()->addNewSiteLink( 'dewiki', 'Tokyo' );
$changeFactory = TestChanges::getEntityChangeFactory();
@@ -251,6 +252,16 @@
'(wikibase-comment-sitelink-change: [[:de:Japan]],
[[:de:Tokyo]])',
);
+ $updates['Current page gets linked via link change'] = array(
+ $this->getChangeLinkDiff( 'Japan', 'A fancy page' ),
+ '(wikibase-comment-linked)',
+ );
+
+ $updates['Current page gets unlinked via link change'] = array(
+ $this->getChangeLinkDiff( 'A fancy page', 'Japan' ),
+ '(wikibase-comment-unlink)',
+ );
+
return $updates;
}
--
To view, visit https://gerrit.wikimedia.org/r/246190
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ad53554af6ae6e38241bbb8fa0be6dd145495a2
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
