WMDE-Fisch has uploaded a new change for review.
https://gerrit.wikimedia.org/r/304198
Change subject: Send mentions when editing multiple sections in between
sections.
......................................................................
Send mentions when editing multiple sections in between sections.
This patch fixes mentions not being send when multiple sections were added
in between sections.
Since we only want to send mentions when userlinks and signature are present
in the same section a new method was added extracting sections and the related
content from an addition. The results are checked whether a section content
contains a signature and might be relevant for mentions.
Bug: T141863
Change-Id: I434c664552bbadbeef6e897e20703e813f5a4c52
---
M includes/DiscussionParser.php
M tests/phpunit/DiscussionParserTest.php
A tests/phpunit/revision_txt/747798770.txt
A tests/phpunit/revision_txt/747798771.txt
A tests/phpunit/revision_txt/747798772.txt
5 files changed, 359 insertions(+), 25 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/98/304198/1
diff --git a/includes/DiscussionParser.php b/includes/DiscussionParser.php
index 0682d4f..335fbb0 100644
--- a/includes/DiscussionParser.php
+++ b/includes/DiscussionParser.php
@@ -413,9 +413,6 @@
* existing section.
* - new-section-with-comment: A new section is added, containing
* a single comment signed by the user in question.
- * - unknown-signed-addition: Some signed content is added, but it
- * includes section headers, is signed by another user or
- * otherwise confuses the interpretation engine.
* - unknown-multi-signed-addition: Some signed content is added,
* but it contains multiple signatures.
* - unknown-unsigned-addition: Some content is added, but it is
@@ -442,34 +439,30 @@
if ( $change['action'] == 'add' ) {
$content = trim( $change['content'] );
- // The \A means the regex must match at the
beginning of the string.
- // This is slightly different than ^ which
matches beginning of each
- // line in multiline mode.
- $startSection = preg_match( "/\A" .
self::HEADER_REGEX . '/um', $content );
- $sectionCount = self::getSectionCount( $content
);
$signedUsers = array_keys(
self::extractSignatures( $content, $title ) );
if (
count( $signedUsers ) == 1 &&
in_array( $username, $signedUsers )
) {
- if ( $sectionCount === 0 ) {
- $fullSection =
self::getFullSection( $changes['_info']['rhs'], $change['right-pos'] );
- $actions[] = array(
- 'type' => 'add-comment',
- 'content' => $content,
- 'full-section' =>
$fullSection,
- );
- } elseif ( $startSection &&
$sectionCount === 1 ) {
- $actions[] = array(
- 'type' =>
'new-section-with-comment',
- 'content' => $content,
- );
- } else {
- $actions[] = array(
- 'type' =>
'unknown-signed-addition',
- 'content' => $content,
- );
+ $sectionData = self::extractSections(
$content );
+ foreach ( $sectionData as $section ) {
+ $sectionSignedUsers =
array_keys( self::extractSignatures( $section['content'], $title ) );
+ if ( count( $sectionSignedUsers
) == 1 && in_array( $username, $sectionSignedUsers ) ) {
+ if (
!$section['header'] ) {
+ $fullSection =
self::getFullSection( $changes['_info']['rhs'], $change['right-pos'] );
+ $actions[] =
array(
+ 'type'
=> 'add-comment',
+
'content' => $section['content'],
+
'full-section' => $fullSection,
+ );
+ } else {
+ $actions[] =
array(
+ 'type'
=> 'new-section-with-comment',
+
'content' => $section['content'],
+ );
+ }
+ }
}
} elseif ( count( $signedUsers ) >= 1 ) {
$actions[] = array(
@@ -576,6 +569,46 @@
}
/**
+ * Extracts sections and there contents from text.
+ *
+ * @param string $text The text to parse.
+ * @return string|false The text below the first header or false if
nothing found.
+ */
+ static function extractSections( $text ) {
+ $matches = array();
+
+ if ( !preg_match_all( '/' . self::HEADER_REGEX . '/um', $text,
$matches, PREG_OFFSET_CAPTURE ) ) {
+ return array( array(
+ 'header' => false,
+ 'content' => $text
+ ) );
+ }
+
+ $sectionNum = count( $matches[0] );
+ $sections = array();
+
+ if ( $matches[0][0][1] > 1 ) { // is there text before the
first headline?
+ $sections[] = array(
+ 'header' => false,
+ 'content' => mb_substr( $text, 0,
$matches[0][0][1] - 1 )
+ );
+ }
+ for ( $i = 0; $i < $sectionNum; $i++ ) {
+ if ( $i + 1 < $sectionNum ) {
+ $content = substr( $text, $matches[0][$i][1],
$matches[0][$i + 1][1] - $matches[0][$i][1] );
+ } else {
+ $content = substr( $text, $matches[0][$i][1] );
+ }
+ $sections[] = array(
+ 'header' => $matches[0][$i][0],
+ 'content' => trim( $content )
+ );
+ }
+
+ return $sections;
+ }
+
+ /**
* Strips out a signature if possible.
*
* @param string $text The wikitext to strip
diff --git a/tests/phpunit/DiscussionParserTest.php
b/tests/phpunit/DiscussionParserTest.php
index e59f3a5..43f26ce 100644
--- a/tests/phpunit/DiscussionParserTest.php
+++ b/tests/phpunit/DiscussionParserTest.php
@@ -376,11 +376,13 @@
'type' => 'mention-failure',
'agent' => 'Admin',
'section-title' => 'Hello
Users',
+ 'subject-name' => 'Ping',
),
array(
'type' => 'mention-failure',
'agent' => 'Admin',
'section-title' => 'Hello
Users',
+ 'subject-name' => 'Po?ng',
),
),
),
@@ -396,11 +398,13 @@
'type' => 'mention',
'agent' => 'Admin',
'section-title' => 'Hello
Users',
+ 'subject-name' => null,
),
array(
'type' => 'mention-success',
'agent' => 'Admin',
'section-title' => 'Hello
Users',
+ 'subject-name' => 'Test11',
),
),
),
@@ -416,6 +420,7 @@
'type' => 'mention-failure',
'agent' => 'Admin',
'section-title' => 'Section 2',
+ 'subject-name' => 'NoUser',
),
),
),
@@ -431,6 +436,7 @@
'type' => 'mention-failure',
'agent' => 'Admin',
'section-title' => 'Section 2',
+ 'subject-name' => 'NoUser',
),
),
),
@@ -442,6 +448,78 @@
'pages' => array(),
'title' => 'UTPage',
'expected' => array(),
+ ),
+ array(
+ 'new' => 747798770,
+ 'old' => 747798765,
+ 'username' => 'Admin',
+ 'lang' => 'en',
+ 'pages' => array(),
+ 'title' => 'UTPage',
+ 'expected' => array(
+ array(
+ 'type' => 'mention',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section
1.5',
+ 'subject-name' => null,
+ ),
+ array(
+ 'type' => 'mention-success',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section
1.5',
+ 'subject-name' => 'Test11',
+ ),
+ ),
+ ),
+ array(
+ 'new' => 747798771,
+ 'old' => 747798765,
+ 'username' => 'Admin',
+ 'lang' => 'en',
+ 'pages' => array(),
+ 'title' => 'UTPage',
+ 'expected' => array(
+ array(
+ 'type' => 'mention-failure',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section
1.5',
+ 'subject-name' => 'NoUser1.5',
+ ),
+ array(
+ 'type' => 'mention-failure',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section 2',
+ 'subject-name' => 'NoUser2',
+ ),
+ ),
+ ),
+ array(
+ 'new' => 747798772,
+ 'old' => 747798765,
+ 'username' => 'Admin',
+ 'lang' => 'en',
+ 'pages' => array(),
+ 'title' => 'UTPage',
+ 'expected' => array(
+ array(
+ 'type' => 'mention-failure',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section 1',
+ 'subject-name' => 'NoUser1',
+ ),
+ array(
+ 'type' => 'mention-failure',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section
1.75',
+ 'subject-name' => 'NoUser1.75',
+ ),
+ array(
+ 'type' => 'mention-failure',
+ 'agent' => 'Admin',
+ 'section-title' => 'Section 2',
+ 'subject-name' => 'NoUser2',
+ ),
+ ),
),
);
}
@@ -462,6 +540,7 @@
'type' => $event->getType(),
'agent' =>
$event->getAgent()->getName(),
'section-title' =>
$event->getExtraParam( 'section-title' ),
+ 'subject-name' =>
$event->getExtraParam( 'subject-name' ),
);
return false;
}
@@ -475,6 +554,141 @@
$this->assertEquals( $expected, $events );
}
+ public function provider_extractSections() {
+ return array(
+ array(
+ 'content' => 'Just Text.',
+ 'result' => array(
+ array(
+ 'header' => false,
+ 'content' => 'Just Text.',
+ ),
+ ),
+ ),
+ array(
+ 'content' =>
+<<<TEXT
+Text and a
+== Headline ==
+with text
+TEXT
+ ,
+ 'result' => array(
+ array(
+ 'header' => false,
+ 'content' =>
+<<<TEXT
+Text and a
+TEXT
+ ,
+ ),
+ array(
+ 'header' => '== Headline ==',
+ 'content' =>
+<<<TEXT
+== Headline ==
+with text
+TEXT
+ ,
+ ),
+ ),
+ ),
+ array(
+ 'content' =>
+<<<TEXT
+== Headline ==
+Text and a [[User:Test]]
+TEXT
+ ,
+ 'result' => array(
+ array(
+ 'header' => '== Headline ==',
+ 'content' =>
+<<<TEXT
+== Headline ==
+Text and a [[User:Test]]
+TEXT
+ ,
+ ),
+ ),
+ ),
+ array(
+ 'content' =>
+<<<TEXT
+Content 0
+== Headline 1 ==
+Content 1
+=== Headline 2 ===
+Content 2
+TEXT
+ ,
+ 'result' => array(
+ array(
+ 'header' => false,
+ 'content' => 'Content 0',
+ ),
+ array(
+ 'header' => '== Headline 1 ==',
+ 'content' =>
+<<<TEXT
+== Headline 1 ==
+Content 1
+TEXT
+ ,
+ ),
+ array(
+ 'header' => '=== Headline 2
===',
+ 'content' =>
+<<<TEXT
+=== Headline 2 ===
+Content 2
+TEXT
+ ,
+ ),
+ ),
+ ),
+ array(
+ 'content' =>
+<<<TEXT
+== Headline 1 ==
+مرحبا كيف حالك
+=== Headline 2 ===
+انا بخير شكرا
+TEXT
+ ,
+ 'result' => array(
+ array(
+ 'header' => '== Headline 1 ==',
+ 'content' =>
+<<<TEXT
+== Headline 1 ==
+مرحبا كيف حالك
+TEXT
+ ,
+ ),
+ array(
+ 'header' => '=== Headline 2
===',
+ 'content' =>
+<<<TEXT
+=== Headline 2 ===
+انا بخير شكرا
+TEXT
+ ,
+ ),
+ ),
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provider_extractSections
+ */
+ public function testExtractSections( $content, $result ) {
+ $sections = EchoDiscussionParser::extractSections( $content );
+
+ $this->assertEquals( $result, $sections );
+ }
+
public function testGenerateEventsForRevision_tooManyMentionsFailure() {
$expected = array(
array(
diff --git a/tests/phpunit/revision_txt/747798770.txt
b/tests/phpunit/revision_txt/747798770.txt
new file mode 100644
index 0000000..55067bc
--- /dev/null
+++ b/tests/phpunit/revision_txt/747798770.txt
@@ -0,0 +1,25 @@
+Simultaneously edit multiple sections
+
+== Section 1 ==
+
+Content 1
+
+New Content with userlink ( dont try to mention this one )
+
+[[User:NoUser1]]
+
+== Section 1.5 ==
+
+New section in between with userlink & signature ( try to mention this one )
+
+[[User:Test11]]
+
+[[:User:Admin|Admin]] 13:22, 23 June 2016 (UTC)
+
+== Section 2 ==
+
+Content 2
+
+New Content with userlink ( dont try to mention this one )
+
+[[User:NoUser2]]
\ No newline at end of file
diff --git a/tests/phpunit/revision_txt/747798771.txt
b/tests/phpunit/revision_txt/747798771.txt
new file mode 100644
index 0000000..96c8ba5
--- /dev/null
+++ b/tests/phpunit/revision_txt/747798771.txt
@@ -0,0 +1,27 @@
+Simultaneously edit multiple sections
+
+== Section 1 ==
+
+Content 1
+
+New Content with userlink ( dont try to mention this one )
+
+[[User:NoUser1]]
+
+== Section 1.5 ==
+
+New section in between with userlink & signature ( try to mention this one )
+
+[[User:NoUser1.5]]
+
+[[:User:Admin|Admin]] 13:22, 23 June 2016 (UTC)
+
+== Section 2 ==
+
+Content 2
+
+New Content with userlink & signature ( try to mention this one )
+
+[[User:NoUser2]]
+
+[[:User:Admin|Admin]] 13:22, 23 June 2016 (UTC)
\ No newline at end of file
diff --git a/tests/phpunit/revision_txt/747798772.txt
b/tests/phpunit/revision_txt/747798772.txt
new file mode 100644
index 0000000..e44c454
--- /dev/null
+++ b/tests/phpunit/revision_txt/747798772.txt
@@ -0,0 +1,35 @@
+Simultaneously edit multiple sections
+
+== Section 1 ==
+
+Content 1
+
+New Content with userlink & signature ( try to mention this one )
+
+[[User:NoUser1]]
+
+[[:User:Admin|Admin]] 13:22, 23 June 2016 (UTC)
+
+== Section 1.5 ==
+
+New section in between with userlink ( don't try to mention this one )
+
+[[User:NoUser1.5]]
+
+== Section 1.75 ==
+
+New section in between with userlink & signature ( try to mention this one )
+
+[[User:NoUser1.75]]
+
+[[:User:Admin|Admin]] 13:22, 23 June 2016 (UTC
+
+== Section 2 ==
+
+Content 2
+
+New Content with userlink & signature ( try to mention this one )
+
+[[User:NoUser2]]
+
+[[:User:Admin|Admin]] 13:22, 23 June 2016 (UTC)
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/304198
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I434c664552bbadbeef6e897e20703e813f5a4c52
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: WMDE-Fisch <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
