jenkins-bot has submitted this change and it was merged.
Change subject: Show an error to the user if the spamlist has no targets on it
......................................................................
Show an error to the user if the spamlist has no targets on it
MassMessage::getParserFunctionTargets is now cached so it can
be called multiple times.
Got rid of 'page-list' terminology, using 'list of pages' instead.
An API test also had to be updated, since it relied on sending
a message to a page with 0 targets on it.
Bug: T55970
Change-Id: If406411c76abbe5847fa554e93ea2557a73e682b
---
M i18n/en.json
M i18n/qqq.json
M includes/ApiMassMessage.php
M includes/MassMessage.php
M includes/MassMessageTargets.php
M includes/SpecialMassMessage.php
M tests/api/ApiMassMessageTest.php
7 files changed, 45 insertions(+), 16 deletions(-)
Approvals:
Siebrand: Looks good to me, approved
jenkins-bot: Verified
diff --git a/i18n/en.json b/i18n/en.json
index 902f97b..5494a77 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -19,7 +19,8 @@
"massmessage-preview-count": "Your message will be sent to
{{PLURAL:$1|$1 page|$1 pages}}.",
"massmessage-submitted": "Your message delivery to {{PLURAL:$1|$1
page|$1 pages}} has been queued.",
"massmessage-just-preview": "This is just a preview. Press
\"{{int:massmessage-form-submit}}\" to send the message.",
- "massmessage-spamlist-doesnotexist": "The specified delivery list page
or category does not exist.",
+ "massmessage-spamlist-doesnotexist": "The specified list of pages does
not exist.",
+ "massmessage-spamlist-notargets": "The specified list of pages does not
have any targets on it.",
"massmessage-spamlist-invalid": "The specified page does not contain a
valid delivery list.",
"massmessage-empty-subject": "The subject line is empty.",
"massmessage-empty-message": "The message body is empty.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 4c52951..d48b416 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -25,7 +25,8 @@
"massmessage-preview-count": "Text indicating how many pages the
message will be sent to. Parameters:\n* $1 - the number of pages",
"massmessage-submitted": "Confirmation message the user sees after the
form is submitted successfully and the request is queued in the job
queue.\n\nParameters:\n* $1 - the number of deliveries that have been queued",
"massmessage-just-preview": "Warning to user that what they are seeing
is just a preview, and they should hit the send button to actually submit
it.\n\nRefers to {{msg-mw|Massmessage-form-submit}}.",
- "massmessage-spamlist-doesnotexist": "Error message the user sees if
the delivery list page or category they entered does not exist.",
+ "massmessage-spamlist-doesnotexist": "Error message the user sees if an
invalid list of pages is provided.",
+ "massmessage-spamlist-notargets": "Error message that the user sees if
the list of pages provided by the user has no targets listed on it.",
"massmessage-spamlist-invalid": "Error message the user sees if the
delivery list page they entered is invalid.",
"massmessage-empty-subject": "Error message the user sees if the
\"subject\" field is empty.",
"massmessage-empty-message": "Error message the user sees if the
\"message\" field is empty.",
diff --git a/includes/ApiMassMessage.php b/includes/ApiMassMessage.php
index 25eb370..19e3dbf 100644
--- a/includes/ApiMassMessage.php
+++ b/includes/ApiMassMessage.php
@@ -16,7 +16,7 @@
$data = $this->extractRequestParams();
$status = new Status();
- MassMessage::verifyData( $data, $status );
+ MassMessage::verifyData( $data, $status, $this );
if ( !$status->isOK() ) {
$this->dieStatus( $status );
}
diff --git a/includes/MassMessage.php b/includes/MassMessage.php
index 615d8fc..c920766 100644
--- a/includes/MassMessage.php
+++ b/includes/MassMessage.php
@@ -195,8 +195,9 @@
* Verify and cleanup the main user submitted data
* @param array &$data should have subject, message, and spamlist keys
* @param Status &$status
+ * @param IContextSource $ctx
*/
- public static function verifyData( array &$data, Status &$status ) {
+ public static function verifyData( array &$data, Status &$status,
IContextSource $ctx ) {
// Trim all the things!
foreach ( $data as $k => $v ) {
$data[$k] = trim( $v );
@@ -219,10 +220,18 @@
);
}
$data['comment'] = [
- RequestContext::getMain()->getUser()->getName(),
+ $ctx->getUser()->getName(),
wfWikiID(),
$url
];
+
+ // Check that the spamlist has targets
+ $targets =
MassMessageTargets::getParserFunctionTargets( $spamlist, $ctx );
+ if ( !is_array( $targets ) ) {
+ $status->fatal( $targets );
+ } elseif ( !$targets ) {
+ $status->fatal(
'massmessage-spamlist-notargets' );
+ }
} else { // $spamlist contains a message key for an error
message
$status->fatal( $spamlist );
}
diff --git a/includes/MassMessageTargets.php b/includes/MassMessageTargets.php
index 8504ffd..ad2d402 100644
--- a/includes/MassMessageTargets.php
+++ b/includes/MassMessageTargets.php
@@ -140,11 +140,27 @@
* Get an array of targets via the #target parser function
* @param Title $spamlist
* @param IContextSource $context
- * @return array
+ * @return array|string if a string, it'll be the error message key
*/
- protected static function getParserFunctionTargets( Title $spamlist ) {
+ public static function getParserFunctionTargets( Title $spamlist ) {
+ global $wgMemc;
$page = WikiPage::factory( $spamlist );
- $text = $page->getContent( Revision::RAW )->getNativeData();
+
+ $content = $page->getContent( Revision::RAW );
+ $text = $content->getNativeData();
+
+ $key = wfMemcKey( 'massmessage', 'targets',
$page->getRevision()->getId() );
+ $data = $wgMemc->get( $key );
+ if ( $data !== false ) {
+ return $data;
+ }
+ if ( $content instanceof WikitextContent ) {
+ // The content type should have already been checked
+ // earlier, but we'll be safe.
+ $text = $content->getNativeData();
+ } else {
+ return 'massmessage-spamlist-doesnotexist';
+ }
// Prep the parser
$parserOptions = $page->makeParserOptions( 'canonical' );
@@ -161,9 +177,11 @@
$data = unserialize( $output->getProperty(
'massmessage-targets' ) );
if ( $data ) {
- return $data;
+ $data = self::normalizeTargets( $data );
} else {
- return []; // No parser functions on page
+ $data = []; // No parser functions on page
}
+ $wgMemc->set( $key, $data, 60 * 60 );
+ return $data;
}
}
diff --git a/includes/SpecialMassMessage.php b/includes/SpecialMassMessage.php
index 0a35e12..c1118b2 100644
--- a/includes/SpecialMassMessage.php
+++ b/includes/SpecialMassMessage.php
@@ -178,7 +178,7 @@
*/
public function callback( array $data ) {
- MassMessage::verifyData( $data, $this->status );
+ MassMessage::verifyData( $data, $this->status,
$this->getContext() );
// Die on errors.
if ( !$this->status->isOK() ) {
diff --git a/tests/api/ApiMassMessageTest.php b/tests/api/ApiMassMessageTest.php
index feb9b2f..c4ef23a 100644
--- a/tests/api/ApiMassMessageTest.php
+++ b/tests/api/ApiMassMessageTest.php
@@ -9,14 +9,14 @@
class ApiMassMessageTest extends MassMessageApiTestCase {
protected static $spamlist = 'Help:ApiMassMessageTest_spamlist';
- protected static $emptyspamlist = 'Help:ApiMassMessageTest_spamlist2';
+ protected static $spamlist2 = 'Help:ApiMassMessageTest_spamlist2';
protected function setUp() {
parent::setUp();
$spamlist = Title::newFromText( self::$spamlist );
self::updatePage( $spamlist, '{{#target:Project:ApiTest1}}' );
- $emptyspamlist = Title::newFromText( self::$emptyspamlist );
- self::updatePage( $emptyspamlist, 'rawr' );
+ $emptyspamlist = Title::newFromText( self::$spamlist2 );
+ self::updatePage( $emptyspamlist,
'{{#target:Project:ApiTest2}}{{#target:Project:ApiTest3}}' );
$this->doLogin();
}
@@ -57,7 +57,7 @@
*/
public function testInvalidSpamlist() {
$this->setExpectedException( 'UsageException',
- 'The specified delivery list page or category does not
exist.' );
+ 'The specified list of pages does not exist.' );
$this->doApiRequestWithToken( [
'action' => 'massmessage',
'spamlist' => '<InvalidPageTitle>',
@@ -69,7 +69,7 @@
public static function provideCount() {
return [
[ self::$spamlist, 1 ],
- [ self::$emptyspamlist, 0 ]
+ [ self::$spamlist2, 2 ]
];
}
--
To view, visit https://gerrit.wikimedia.org/r/94691
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If406411c76abbe5847fa554e93ea2557a73e682b
Gerrit-PatchSet: 15
Gerrit-Project: mediawiki/extensions/MassMessage
Gerrit-Branch: master
Gerrit-Owner: Legoktm <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: MZMcBride <[email protected]>
Gerrit-Reviewer: Paladox <[email protected]>
Gerrit-Reviewer: Reedy <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
