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

Reply via email to