Noella94 has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/405222 )
Change subject: [WIP]Replacing target Arrays with Objects. ...................................................................... [WIP]Replacing target Arrays with Objects. Instead of passing targets as Arrays passing now but target Objects Bug: T178215 Bug: T178431 Change-Id: I2c7fb5d30fd20bf06393cced73027089f2533b6e --- M extension.json M includes/ApiEditMassMessageList.php M includes/MassMessage.php M includes/MassMessageTargets.php A includes/Target.php M includes/content/MassMessageListContent.php M includes/content/MassMessageListContentHandler.php M tests/phpunit/content/MassMessageContentTest.php 8 files changed, 78 insertions(+), 62 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MassMessage refs/changes/22/405222/1 diff --git a/extension.json b/extension.json index 6a1b354..5d0ec41 100644 --- a/extension.json +++ b/extension.json @@ -225,6 +225,7 @@ "MediaWiki\\MassMessage\\MassMessageListContent": "includes/content/MassMessageListContent.php", "MediaWiki\\MassMessage\\MassMessageListContentHandler": "includes/content/MassMessageListContentHandler.php", "MediaWiki\\MassMessage\\MassMessageListDiffEngine": "includes/content/MassMessageListDiffEngine.php", + "MediaWiki\\MassMessage\\Target": "includes/Target.php", "MediaWiki\\MassMessage\\MassMessageTestCase": "tests/phpunit/MassMessageTestCase.php", "MediaWiki\\MassMessage\\MassMessageApiTestCase": "tests/phpunit/MassMessageApiTestCase.php" }, diff --git a/includes/ApiEditMassMessageList.php b/includes/ApiEditMassMessageList.php index 9ffd8e7..f53068e 100644 --- a/includes/ApiEditMassMessageList.php +++ b/includes/ApiEditMassMessageList.php @@ -44,14 +44,14 @@ foreach ( $data['add'] as $page ) { $target = MassMessageListContentHandler::extractTarget( $page ); - if ( isset( $target['errors'] ) ) { + if ( isset( $target->error ) ) { $item = [ '*' => $page ]; - foreach ( $target['errors'] as $error ) { + foreach ( $target->error as $error ) { $item[$error] = ''; } $invalidAdd[] = $item; } else { - $newTargets[] = $target; + $newTargets = $target; } } @@ -66,10 +66,10 @@ foreach ( $data['remove'] as $page ) { $target = MassMessageListContentHandler::extractTarget( $page ); - if ( isset( $target['errors'] ) || !in_array( $target, $newTargets ) ) { + if ( isset( $target->error ) || !in_array( $target, $newTargets ) ) { $invalidRemove[] = $page; } else { - $toRemove[] = $target; + $toRemove = $target; } } diff --git a/includes/MassMessage.php b/includes/MassMessage.php index 8a79902..86c890a 100644 --- a/includes/MassMessage.php +++ b/includes/MassMessage.php @@ -58,20 +58,20 @@ return self::parserError( 'massmessage-parse-badpage', $page ); } - $data = [ 'title' => $page, 'site' => trim( $site ) ]; - if ( $data['site'] === '' ) { - $data['site'] = UrlHelper::getBaseUrl( $wgCanonicalServer ); - $data['wiki'] = wfWikiID(); + $data = new Target(); + if ( $data->site === '' ) { + $data->site = UrlHelper::getBaseUrl( $wgCanonicalServer ); + $data->wiki = wfWikiID(); } else { - $data['wiki'] = DatabaseLookup::getDBName( $data['site'] ); - if ( $data['wiki'] === null ) { + $data->wiki = DatabaseLookup::getDBName( $data->site ); + if ( $data->wiki === null ) { return self::parserError( 'massmessage-parse-badurl', $site ); } - if ( !$wgAllowGlobalMessaging && $data['wiki'] !== wfWikiID() ) { + if ( !$wgAllowGlobalMessaging && $data->wiki !== wfWikiID() ) { return self::parserError( 'massmessage-global-disallowed' ); } } - if ( $data['wiki'] === wfWikiID() && $titleObj->isExternal() ) { + if ( $data->wiki === wfWikiID() && $titleObj->isExternal() ) { // interwiki links don't work if ( $wgAllowGlobalMessaging ) { // tell them they need to use the |site= parameter diff --git a/includes/MassMessageTargets.php b/includes/MassMessageTargets.php index 9052682..230b6e2 100644 --- a/includes/MassMessageTargets.php +++ b/includes/MassMessageTargets.php @@ -22,8 +22,7 @@ * wiki: The ID of the wiki (wfWikiID() for the local wiki) * site: The hostname and port (if exists) of the wiki * - * Normalized targets are briefly cached because it can be expensive to parse PF targets on both - * preview and save in SpecialMassMessage. + * Normalized targets are briefly cached because it can be expensive to parse PF targets on both preview and save in SpecialMassMessage. * * @param Title $spamlist * @param bool $normalize Whether to normalize and deduplicate the targets @@ -115,10 +114,12 @@ /** @var Title $member */ foreach ( $members as $member ) { - $targets[] = [ - 'title' => $member->getPrefixedText(), - 'wiki' => wfWikiID(), - 'site' => UrlHelper::getBaseUrl( $wgCanonicalServer ), + $target = new Target(); + $target->title = $member->getPrefixedText(); + $target->wiki = wfWikiID(); + $target->site = UrlHelper::getBaseUrl( $wgCanonicalServer ); + $targets = [ + 'target' => $target, ]; } return $targets; @@ -134,11 +135,11 @@ $targets = Revision::newFromTitle( $spamlist )->getContent()->getValidTargets(); foreach ( $targets as &$target ) { - if ( array_key_exists( 'site', $target ) ) { - $target['wiki'] = DatabaseLookup::getDBName( $target['site'] ); + if ( /*array_key_exists( 'site', $target )*/ ) {//TODO: check $target->site exist + $target->wiki = DatabaseLookup::getDBName( $target->site ); } else { - $target['site'] = UrlHelper::getBaseUrl( $wgCanonicalServer ); - $target['wiki'] = wfWikiId(); + $target->site = UrlHelper::getBaseUrl( $wgCanonicalServer ); + $target->wiki = wfWikiId(); } } return $targets; diff --git a/includes/Target.php b/includes/Target.php new file mode 100644 index 0000000..28407a7 --- /dev/null +++ b/includes/Target.php @@ -0,0 +1,15 @@ +<?php + +namespace MediaWiki\MassMessage; + +class Target { + + public $title; + public $site; + public $wiki; + public $error = []; + + function __constructor(){ + + } +} diff --git a/includes/content/MassMessageListContent.php b/includes/content/MassMessageListContent.php index 592d66c..9d4ac81 100644 --- a/includes/content/MassMessageListContent.php +++ b/includes/content/MassMessageListContent.php @@ -20,16 +20,16 @@ protected $description; /** - * Array of target pages - * @var array|null - */ - protected $targets; - - /** * Whether $description and $targets have been populated * @var bool */ protected $decoded = false; + + /** + * object of target pages + * @var object|null + */ + protected $targets; /** * @param string $text @@ -44,13 +44,13 @@ */ public function isValid() { $this->decode(); // Populate $this->description and $this->targets. - if ( !is_string( $this->description ) || !is_array( $this->targets ) ) { + if ( !is_string( $this->description ) || !is_object( $this->targets ) ) { return false; } foreach ( $this->getTargets() as $target ) { - if ( !is_array( $target ) - || !array_key_exists( 'title', $target ) - || Title::newFromText( $target['title'] ) === null + if ( !is_object( $target ) + || !$target->title + || $target->title === null ) { return false; } @@ -75,14 +75,14 @@ } /** - * @return array + * @return object */ public function getTargets() { $this->decode(); - if ( is_array( $this->targets ) ) { + if ( is_object( $this->targets ) ) { return $this->targets; } - return []; + return $target = new Target(); } /** @@ -95,9 +95,9 @@ $targets = $this->getTargets(); $validTargets = []; foreach ( $targets as $target ) { - if ( !array_key_exists( 'site', $target ) + if ( !$target->site || $wgAllowGlobalMessaging - && DatabaseLookup::getDBName( $target['site'] ) !== null + && DatabaseLookup::getDBName( $target->site ) !== null ) { $validTargets[] = $target; } @@ -115,14 +115,14 @@ $targets = $this->getTargets(); $targetStrings = []; foreach ( $targets as $target ) { - if ( array_key_exists( 'site', $target ) ) { - $targetStrings[] = $target['title'] . '@' . $target['site']; - } elseif ( strpos( $target['title'], '@' ) !== false ) { + if ( $target->site ) { + $targetStrings[] = $target->title . '@' . $target->site; + } elseif ( strpos( $target->title, '@' ) !== false ) { // List the site if it'd otherwise be ambiguous - $targetStrings[] = $target['title'] . '@' + $targetStrings[] = $target->title . '@' . UrlHelper::getBaseUrl( $wgCanonicalServer ); } else { - $targetStrings[] = $target['title']; + $targetStrings[] = $target->title; } } return $targetStrings; @@ -140,16 +140,15 @@ if ( $data ) { $this->description = isset( $data->description ) ? $data->description : null; if ( isset( $data->targets ) && is_array( $data->targets ) ) { - $this->targets = []; + $this->targets = new Target(); foreach ( $data->targets as $target ) { if ( !is_object( $target ) ) { // There is a malformed target. - $this->targets = null; break; } - $this->targets[] = wfObjectToArray( $target ); // Convert to associative array. + $this->targets = wfObjectToArray( $target ); } } else { - $this->targets = null; + $this->targets; } } $this->decoded = true; diff --git a/includes/content/MassMessageListContentHandler.php b/includes/content/MassMessageListContentHandler.php index a04f78c..bcfcac7 100644 --- a/includes/content/MassMessageListContentHandler.php +++ b/includes/content/MassMessageListContentHandler.php @@ -145,28 +145,28 @@ $site = null; } - $result = []; + $result = new target(); $title = Title::newFromText( $titleText ); if ( !$title || $title->getText() === '' || !$title->canExist() ) { - $result['errors'][] = 'invalidtitle'; + $result->error = 'invalidtitle'; } else { - $result['title'] = $title->getPrefixedText(); // Use the canonical form. + $result->title = $title->getPrefixedText(); // Use the canonical form. } if ( $site !== null && $site !== UrlHelper::getBaseUrl( $wgCanonicalServer ) ) { - if ( !$wgAllowGlobalMessaging || DatabaseLookup::getDBName( $site ) === null ) { - $result['errors'][] = 'invalidsite'; + if ( !$wgAllowGlobalMessaging || DatabaseLookup::getDBName( $result->site ) === null ) { + $result->error = 'invalidsite'; } else { - $result['site'] = $site; + $result->site = $site; } } elseif ( $title && $title->isExternal() ) { // Target has site set to current wiki, but external title // TODO: Provide better error message? - $result['errors'][] = 'invalidtitle'; + $result->error = 'invalidtitle'; } return $result; diff --git a/tests/phpunit/content/MassMessageContentTest.php b/tests/phpunit/content/MassMessageContentTest.php index 9c77d49..12b0916 100644 --- a/tests/phpunit/content/MassMessageContentTest.php +++ b/tests/phpunit/content/MassMessageContentTest.php @@ -58,15 +58,15 @@ * @covers \Mediawiki\MassMessage\MassMessageListContent::getTargets */ public function testGetTargets() { - $text = '{"description":"","targets":[' - . '{"title":"A"},' - . '{"title":"B","site":"en.wikipedia.org"}' - . ']}'; + $text = '{"description":"","targets->title":"A"},' + . '{"targets->title":"B"},' + . '{"target->site":"en.wikipedia.org"}' + . '}'; $content = new MassMessageListContent( $text ); - $expected = [ - [ 'title' => 'A' ], - [ 'title' => 'B', 'site' => 'en.wikipedia.org' ] - ]; + $expected = new Target(); + $expected->title = 'A'; + $expected->title = 'B'; $expected->site = 'en.wikipedia.org'; + $this->assertEquals( $expected, $content->getTargets() ); } -- To view, visit https://gerrit.wikimedia.org/r/405222 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2c7fb5d30fd20bf06393cced73027089f2533b6e Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/MassMessage Gerrit-Branch: master Gerrit-Owner: Noella94 <tekenoell...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits