jenkins-bot has submitted this change and it was merged.
Change subject: Link directly to the edited section from edit-user-talk events
......................................................................
Link directly to the edited section from edit-user-talk events
Adjusted the edit-user-talk event creation to detect and record which section
of the talk page was edited. Flyout, special page, and email messages have
been adjusted to use this section title as a URL fragment when available.
Bug: 46937
Change-Id: I161e2ffda2f2540f64de90cc621fb3b69479d0db
---
M Echo.i18n.php
M Echo.php
M formatters/BasicFormatter.php
M formatters/CommentFormatter.php
M formatters/EditFormatter.php
M formatters/NotificationFormatter.php
M formatters/UserRightsFormatter.php
M includes/DiscussionParser.php
A tests/NotificationFormatterTest.php
A tests/TalkPageFunctionalTest.php
10 files changed, 286 insertions(+), 25 deletions(-)
Approvals:
Bsitu: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Echo.i18n.php b/Echo.i18n.php
index 0aa22e7..cc48b8a 100644
--- a/Echo.i18n.php
+++ b/Echo.i18n.php
@@ -63,8 +63,8 @@
// Notification
'echo-quotation-marks' => '"$1"',
- 'notification-edit-talk-page2' => '[[User:$1|$1]] {{GENDER:$1|posted}}
on your [[User talk:$2|talk page]].',
- 'notification-edit-talk-page-flyout2' => '<b>$1</b>
{{GENDER:$1|posted}} on your [[User talk:$2|talk page]].',
+ 'notification-edit-talk-page2' => '[[User:$1|$1]] {{GENDER:$1|posted}}
on your [[User talk:$2#$3|talk page]].',
+ 'notification-edit-talk-page-flyout2' => '<b>$1</b>
{{GENDER:$1|posted}} on your [[User talk:$2#$3|talk page]].',
'notification-page-linked' => '[[:$2]] was {{GENDER:$1|linked}} from
[[:$3]]: [[Special:WhatLinksHere/$2|See all links to this page]]',
'notification-page-linked-flyout' => '<b>$2</b> was
{{GENDER:$1|linked}} from <b>$3</b>: [[Special:WhatLinksHere/$2|See all links
to this page]]',
'notification-add-comment2' => '[[User:$1|$1]] {{GENDER:$1|commented}}
on "[[$3|$2]]" on the "$4" talk page',
@@ -280,12 +280,14 @@
'notification-edit-talk-page2' => "Format for displaying notifications
of a user talk page being edited
* $1 is the username of the person who edited, plain text. Can be used for
GENDER.
* $2 is the current user's name, used in the link to their talk page.
+* $3 is the section title of the discussion, if any, used in the link to their
talk page
See also:
* {{msg-mw|Notification-edit-talk-page-flyout2}}
* {{msg-mw|Notification-add-talkpage-topic2}}",
'notification-edit-talk-page-flyout2' => "Flyout-specific format for
displaying notifications of a user talk page being edited
* $1 is the username of the person who edited, plain text. Can be used for
GENDER.
* $2 is the current user's name, used in the link to their talk page.
+* $3 is the section title of the discussion, if any, used in the link to their
talk page
See also:
* {{msg-mw|Notification-edit-talk-page2}}
* {{msg-mw|Notification-add-talkpage-topic2}}",
diff --git a/Echo.php b/Echo.php
index a2baef3..44a01f3 100644
--- a/Echo.php
+++ b/Echo.php
@@ -377,12 +377,12 @@
'bundle' => array( 'web' => true, 'email' => false ),
'formatter-class' => 'EchoEditFormatter',
'title-message' => 'notification-edit-talk-page2',
- 'title-params' => array( 'agent', 'user' ),
+ 'title-params' => array( 'agent', 'user', 'subject-anchor' ),
'bundle-message' => 'notification-edit-talk-page-bundle',
'bundle-params' => array( 'agent', 'user',
'agent-other-display', 'agent-other-count' ),
'payload' => array( 'summary' ),
'flyout-message' => 'notification-edit-talk-page-flyout2',
- 'flyout-params' => array( 'agent', 'user' ),
+ 'flyout-params' => array( 'agent', 'user', 'subject-anchor' ),
'email-subject-message' =>
'notification-edit-talk-page-email-subject2',
'email-body-message' =>
'notification-edit-talk-page-email-body2',
'email-body-params' => array( 'email-intro', 'titlelink',
'summary', 'email-footer' ),
diff --git a/formatters/BasicFormatter.php b/formatters/BasicFormatter.php
index b294f55..eb7a57b 100644
--- a/formatters/BasicFormatter.php
+++ b/formatters/BasicFormatter.php
@@ -334,6 +334,25 @@
}
/**
+ * Extract the subject anchor (linkable portion of the edited page) from
+ * the event.
+ *
+ * @param $event EchoEvent The event to format the subject anchor of
+ * @return string The anchor on page, or an empty string
+ */
+ protected function formatSubjectAnchor( EchoEvent $event ) {
+ global $wgParser;
+
+ $extra = $event->getExtra();
+ if ( empty( $extra['section-title'] ) ) {
+ return '';
+ }
+
+ // Strip out #, keeping # in the i18n message makes it look
more clear
+ return substr( $wgParser->guessLegacySectionNameFromWikiText(
$extra['section-title'] ), 1 );
+ }
+
+ /**
* Generate links based on output format and passed properties
* $event EchoEvent
* $message Message
@@ -353,8 +372,11 @@
}
if ( isset( $props['fragment'] ) ) {
- $title->setFragment( '#' . $props['fragment'] );
+ $fragment = $props['fragment'];
+ } else {
+ $fragment = $this->formatSubjectAnchor( $event );
}
+ $title->setFragment( "#$fragment" );
if ( $this->outputFormat === 'html' || $this->outputFormat ===
'flyout' ) {
$class = array();
diff --git a/formatters/CommentFormatter.php b/formatters/CommentFormatter.php
index 2fd7c28..8e7f333 100644
--- a/formatters/CommentFormatter.php
+++ b/formatters/CommentFormatter.php
@@ -48,28 +48,14 @@
*/
protected function processParam( $event, $param, $message, $user ) {
$extra = $event->getExtra();
- if ( $param === 'subject-anchor' ) {
- global $wgParser;
- if ( !empty( $extra['section-title'] ) ) {
- $message->params(
- // Strip out #, keeping # in the i18n
message makes it look more clear
- substr(
$wgParser->guessLegacySectionNameFromWikiText( $extra['section-title'] ), 1 )
- );
- } else {
- $message->params( '' );
- }
- } elseif ( $param === 'content-page' ) {
+ if ( $param === 'content-page' ) {
if ( $event->getTitle() ) {
$message->params(
$event->getTitle()->getSubjectPage()->getPrefixedText() );
} else {
$message->params( '' );
}
} elseif ( $param === 'subject-link' ) {
- $prop = array();
- if ( isset( $extra['section-title'] ) &&
$extra['section-title'] ) {
- $prop['fragment'] = $extra['section-title'];
- }
- $this->setTitleLink( $event, $message, $prop );
+ $this->setTitleLink( $event, $message );
} else {
parent::processParam( $event, $param, $message, $user );
}
diff --git a/formatters/EditFormatter.php b/formatters/EditFormatter.php
index 3b9cc06..41ac126 100644
--- a/formatters/EditFormatter.php
+++ b/formatters/EditFormatter.php
@@ -9,7 +9,9 @@
* @param $user User
*/
protected function processParam( $event, $param, $message, $user ) {
- if ( $param === 'difflink' ) {
+ if ( $param === 'subject-anchor' ) {
+ $message->params( $this->formatSubjectAnchor( $event )
);
+ } elseif ( $param === 'difflink' ) {
$eventData = $event->getExtra();
if ( !isset( $eventData['revid'] ) ) {
$message->params( '' );
diff --git a/formatters/NotificationFormatter.php
b/formatters/NotificationFormatter.php
index 8dc8c3e..cb47205 100644
--- a/formatters/NotificationFormatter.php
+++ b/formatters/NotificationFormatter.php
@@ -62,7 +62,7 @@
/**
* Set the output format that the notification will be displayed in.
- * @param $format string A valid output format (by default, 'text',
'html', and 'email' are allowed)
+ * @param $format string A valid output format (by default, 'text',
'html', 'flyout', and 'email' are allowed)
* @throws MWException
*/
public function setOutputFormat( $format ) {
diff --git a/formatters/UserRightsFormatter.php
b/formatters/UserRightsFormatter.php
index ca3307e..ddb2de0 100644
--- a/formatters/UserRightsFormatter.php
+++ b/formatters/UserRightsFormatter.php
@@ -22,7 +22,7 @@
$list = array();
foreach ( array( 'add', 'remove' ) as $action )
{
- if ( $extra[$action] ) {
+ if ( isset( $extra[$action] ) &&
$extra[$action] ) {
// Messages that can be used
here:
// *
notification-user-rights-add
// *
notification-user-rights-remove
diff --git a/includes/DiscussionParser.php b/includes/DiscussionParser.php
index 60cf30c..6cdebef 100644
--- a/includes/DiscussionParser.php
+++ b/includes/DiscussionParser.php
@@ -84,7 +84,11 @@
EchoEvent::create( array(
'type' => 'edit-user-talk',
'title' => $title,
- 'extra' => array( 'revid' =>
$revision->getID(), 'minoredit' => $revision->isMinor() ),
+ 'extra' => array(
+ 'revid' =>
$revision->getID(),
+ 'minoredit' =>
$revision->isMinor(),
+ 'section-title' =>
self::detectSectionTitle( $interpretation ),
+ ),
'agent' => $user,
) );
}
@@ -93,6 +97,31 @@
}
/**
+ * Attempts to determine what section title the edit was performed
under (if any)
+ *
+ * @param $interpretation array Results of
self::getChangeInterpretationForRevision
+ * @return string The section title if found otherwise a blank string
+ */
+ protected static function detectSectionTitle( array $interpretation ) {
+ $header = '';
+ foreach ( $interpretation as $action ) {
+ switch( $action['type'] ) {
+ case 'add-comment':
+ $header = self::extractHeader(
$action['full-section'] );
+ break;
+
+ case 'new-section-with-comment':
+ $header = self::extractHeader(
$action['content'] );
+ break;
+ }
+ if ( $header ) {
+ return $header;
+ }
+ }
+ return '';
+ }
+
+ /**
* For an action taken on a talk page, notify users whose user pages
* are linked.
* @param $header string The subject line for the discussion.
diff --git a/tests/NotificationFormatterTest.php
b/tests/NotificationFormatterTest.php
new file mode 100644
index 0000000..251777e
--- /dev/null
+++ b/tests/NotificationFormatterTest.php
@@ -0,0 +1,132 @@
+
+<?php
+
+class EchoNotificationFormatterTest extends MediaWikiTestCase {
+
+ public static function provider_editUserTalkEmail() {
+ return array(
+ array( '/Main_Page#Section_8/', 'Section 8' ),
+ array( '/Main_Page[^#]/', null ),
+ );
+ }
+
+ /**
+ * @dataProvider provider_editUserTalkEmail
+ */
+ public function testEditUserTalkEmailNotificationLink( $pattern,
$sectionTitle ) {
+ $event = $this->mockEvent( 'edit-user-talk', array(
+ 'section-title' => $sectionTitle,
+ ) );
+ $event->expects( $this->any() )
+ ->method( 'getTitle' )
+ ->will( $this->returnValue( Title::newMainPage() ) );
+
+ $formatted = $this->format( $event, 'email' );
+ $this->assertRegExp( $pattern, $formatted['body'] );
+ }
+
+ public static function provider_editUserTalk() {
+ return array(
+ array( '/[[User talk:[^#]+#moar_cowbell|talk page]]/',
'moar_cowbell', 'text' ),
+ array( '/#moar_cowbell/', 'moar_cowbell', 'html' ),
+ array( '/#moar_cowbell/', 'moar_cowbell', 'flyout' ),
+ );
+ }
+
+ /**
+ * @dataProvider provider_editUserTalk
+ */
+ public function testEditUserTalkFlyoutSectionLinkFragment( $pattern,
$sectionTitle, $format ) {
+ // Required hack so parser doesnt turn the links into redlinks
which contain no fragment
+ LinkCache::singleton()->addGoodLinkObj( 42, Title::newFromText(
'127.0.0.1', NS_USER_TALK ) );
+
+ $event = $this->mockEvent( 'edit-user-talk', array(
+ 'section-title' => $sectionTitle,
+ ) );
+
+ $this->assertRegExp( $pattern, $this->format( $event, $format )
);
+ }
+
+ public function provider_formatterDoesntFail() {
+ // Remove events from this array once they have specific tests
for their formatting
+ $untested = array(
+ 'welcome' => array(),
+ 'reverted' => array(
+ 'revid' => 42,
+ 'reverted-user-id' => 77,
+ 'reverted-revision-id' => 13,
+ 'method' => 'undo',
+ ),
+ 'page-linked' => array(
+ 'link-from-namespace' => 0,
+ 'link-from-title' => 'Karl Sims',
+ ),
+ 'mention' => array(
+ 'content' => 'lorem ipsum dolar sit amet',
+ 'section-title' => 'Zombies',
+ 'revid' => 42,
+ 'mentionedusers' => array( 101 => 101 ),
+ ),
+ 'user-rights' => array(
+ 'user' => 187,
+ 'add' => array( 'aaa', 'bbb' ),
+ 'remove' => array( 'other' ),
+ ),
+ );
+ $formats = array( 'html', 'flyout', 'email', 'text' );
+ $tests = array();
+ foreach ( $untested as $type => $extra ) {
+ foreach ( $formats as $format ) {
+ // Run tests with blank extra data and with the
provided extra data
+ $tests[] = array( $format, $type, $extra );
+ $tests[] = array( $format, $type, array() );
+ }
+ }
+
+ return $tests;
+ }
+
+ /**
+ * @dataProvider provider_formatterDoesntFail
+ */
+ public function testFormatterDoesntFail( $format, $type, array $extra )
{
+ $result = $this->format( $this->mockEvent( $type, $extra ),
$format );
+
+ // generic assertion, could do better
+ if ( $format === 'email' ) {
+ $this->assertInternalType( 'array', $result );
+ $this->assertCount( 3, $result );
+ } else {
+ $this->assertInternalType( 'string', $result );
+ $this->assertGreaterThan( 0, strlen( $result ) );
+ }
+ }
+
+ protected function format( EchoEvent $event, $format, $type = 'web',
array $params = array() ) {
+ global $wgEchoNotifications;
+
+ $params += $wgEchoNotifications[ $event->getType() ];
+ $formatter = EchoNotificationFormatter::factory( $params );
+ $formatter->setOutputFormat( $format );
+
+ return $formatter->format( $event, new User, $type );
+ }
+
+ protected function mockEvent( $type, array $extra = array(), Revision
$rev = null ) {
+ $event = $this->getMockBuilder( 'EchoEvent' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $event->expects( $this->any() )
+ ->method( 'getType' )
+ ->will( $this->returnValue( $type ) );
+ $event->expects( $this->any() )
+ ->method( 'getExtra' )
+ ->will( $this->returnValue( $extra ) );
+ if ( $rev !== null ) {
+ $event->expects( $this->any() )
+ ->method( 'getRevision' )
+ ->will( $this->returnValue( $rev ) );
+ }
+ return $event;
+ }
+}
diff --git a/tests/TalkPageFunctionalTest.php b/tests/TalkPageFunctionalTest.php
new file mode 100644
index 0000000..e5af785
--- /dev/null
+++ b/tests/TalkPageFunctionalTest.php
@@ -0,0 +1,88 @@
+<?php
+/**
+ * @group DataBase
+ * @group medium
+ */
+class EchoTalkPageFunctionalTest extends ApiTestCase {
+
+ protected $dbr;
+
+ public function setUp() {
+ parent::setUp();
+ $this->dbr = MWEchoDbFactory::getDB( DB_SLAVE );
+ }
+
+ /**
+ * Creates and updates a user talk page a few times to ensure proper
events are
+ * created. The user performing the edits is self::$users['sysop'].
+ */
+ public function testAddCommentsToTalkPage() {
+ $editor = self::$users['sysop']->user->getName();
+ $talkPage = self::$users['uploader']->user->getName();
+ // A set of messages which will be inserted
+ $messages = array(
+ 'Moar Cowbell',
+ "I can haz test\n\nplz?", // checks that the parser
allows multi-line comments
+ 'blah blah',
+ );
+
+ $messageCount = 0;
+ $this->assertCount($messageCount, $this->fetchAllEvents() );
+
+ // Start a talkpage
+ $content = "== Section 8 ==\n\n" . $this->signedMessage(
$editor, $messages[$messageCount] );
+ $this->editPage( $talkPage, $content, '', NS_USER_TALK );
+
+ // Ensure the proper event was created
+ $events = $this->fetchAllEvents();
+ $this->assertCount(1 + $messageCount, $events, 'After initial
edit a single event must exist.'); // +1 is due to 0 index
+ $row = end( $events );
+ $this->assertEquals( 'edit-user-talk', $row->event_type );
+ $this->assertEventSectionTitle( 'Section 8', $row );
+
+ // Add another message to the talk page
+ $messageCount++;
+ $content .= $this->signedMessage( $editor,
$messages[$messageCount] );
+ $this->editPage( $talkPage, $content, '', NS_USER_TALK );
+
+ // Ensure another event was created
+ $events = $this->fetchAllEvents();
+ $this->assertCount(1 + $messageCount, $events);
+ $row = end( $events );
+ $this->assertEquals( 'edit-user-talk', $row->event_type );
+ $this->assertEventSectionTitle( 'Section 8', $row );
+
+ // Add a new section and a message within it
+ $messageCount++;
+ $content .= "\n\n== EE ==\n\n" . $this->signedMessage( $editor,
$messages[$messageCount] );
+ $this->editPage( $talkPage, $content, '', NS_USER_TALK );
+
+ // Ensure this event has the new section title
+ $events = $this->fetchAllEvents();
+ $this->assertCount(1 + $messageCount, $events);
+ $row = end( $events );
+ $this->assertEquals( 'edit-user-talk', $row->event_type );
+ $this->assertEventSectionTitle( 'EE', $row );
+ }
+
+ protected function assertEventSectionTitle( $sectionTitle, $row ) {
+ $this->assertNotNull( $row->event_extra, 'Event must contain
extra data.' );
+ $extra = unserialize( $row->event_extra );
+ $this->assertArrayHasKey( 'section-title', $extra, 'Extra data
must include a section-title key.' );
+ $this->assertEquals( $sectionTitle, $extra['section-title'],
'Detected section title must match' );
+ }
+
+ /**
+ * @return array All events in db sorted from oldest to newest
+ */
+ protected function fetchAllEvents() {
+ $res = $this->dbr->select( 'echo_event', array( '*' ), array(),
__METHOD__, array( 'ORDER BY' => 'event_id ASC' ) );
+
+ return iterator_to_array( $res );
+ }
+
+ protected function signedMessage( $name, $content = 'Moar cowbell',
$depth = 1 ) {
+ return str_repeat(':', $depth)." $content [[User:$name|$name]]
([[User talk:$name|$name]]) 00:17, 7 May 2013 (UTC)\n";
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/63789
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I161e2ffda2f2540f64de90cc621fb3b69479d0db
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: EBernhardson (WMF) <[email protected]>
Gerrit-Reviewer: Bsitu <[email protected]>
Gerrit-Reviewer: EBernhardson (WMF) <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Lwelling <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
