Test ApiMove

One bug caught: the watch and unwatch params were not being honored.
They have now been removed.

Depends-On: Ia21a974f2b463afc9324182137b95c80db86a6aa
Change-Id: I0e214339c9ae3f0fb5a40c88a84190bc32503151
(cherry picked from commit 80c7158f06b61acf5e8d8f46ee45c4c49829837b)

diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index de2abdd..4af03ee 100644 (file)
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -136,6 +136,9 @@ production.
 * (T185058) The 'name' value to tgprop for action=query&list=tags has been
   removed. It has never made a difference in the output, the name was always
   returned regardless.
+* The 'watch' and 'unwatch' parameters for action=move have been removed.  They
+  were deprecated and also accidentally nonfunctional since 1.17 in 2010.  Use
+  'watchlist' instead.
 
 === Action API internal changes in 1.31 ===
 * ApiBase::getProfileDBTime was removed (deprecated since 1.25)

diff --git a/includes/api/ApiMove.php b/includes/api/ApiMove.php
index 2814564..f6b6b35 100644 (file)
--- a/includes/api/ApiMove.php
+++ b/includes/api/ApiMove.php
@@ -99,7 +99,6 @@ class ApiMove extends ApiBase {
                // a redirect to the new title. This is not safe, but what we did before was
                // even worse: we just determined whether a redirect should have been created,
                // and reported that it was created if it should have, without any checks.
-               // Also note that isRedirect() is unreliable because of T39209.
                $r['redirectcreated'] = $fromTitle->exists();
 
                $r['moveoverredirect'] = $toTitleExists;
@@ -152,10 +151,6 @@ class ApiMove extends ApiBase {
                $watch = 'preferences';
                if ( isset( $params['watchlist'] ) ) {
                        $watch = $params['watchlist'];
-               } elseif ( $params['watch'] ) {
-                       $watch = 'watch';
-               } elseif ( $params['unwatch'] ) {
-                       $watch = 'unwatch';
                }
 
                // Watch pages
@@ -250,14 +245,6 @@ class ApiMove extends ApiBase {
                        'movetalk' => false,
                        'movesubpages' => false,
                        'noredirect' => false,
-                       'watch' => [
-                               ApiBase::PARAM_DFLT => false,
-                               ApiBase::PARAM_DEPRECATED => true,
-                       ],
-                       'unwatch' => [
-                               ApiBase::PARAM_DFLT => false,
-                               ApiBase::PARAM_DEPRECATED => true,
-                       ],
                        'watchlist' => [
                                ApiBase::PARAM_DFLT => 'preferences',
                                ApiBase::PARAM_TYPE => [

diff --git a/tests/phpunit/includes/api/ApiMoveTest.php b/tests/phpunit/includes/api/ApiMoveTest.php
new file mode 100644 (file)
index 0000000..fb697ff
--- /dev/null
+++ b/tests/phpunit/includes/api/ApiMoveTest.php
@@ -0,0 +1,393 @@
+<?php
+
+/**
+ * @group API
+ * @group Database
+ * @group medium
+ *
+ * @covers ApiMove
+ */
+class ApiMoveTest extends ApiTestCase {
+       /**
+        * @param string $from Prefixed name of source
+        * @param string $to Prefixed name of destination
+        * @param string $id Page id of the page to move
+        * @param array|string|null $opts Options: 'noredirect' to expect no redirect
+        */
+       protected function assertMoved( $from, $to, $id, $opts = null ) {
+               $opts = (array)$opts;
+
+               $fromTitle = Title::newFromText( $from );
+               $toTitle = Title::newFromText( $to );
+
+               $this->assertTrue( $toTitle->exists(),
+                       "Destination {$toTitle->getPrefixedText()} does not exist" );
+
+               if ( in_array( 'noredirect', $opts ) ) {
+                       $this->assertFalse( $fromTitle->exists(),
+                               "Source {$fromTitle->getPrefixedText()} exists" );
+               } else {
+                       $this->assertTrue( $fromTitle->exists(),
+                               "Source {$fromTitle->getPrefixedText()} does not exist" );
+                       $this->assertTrue( $fromTitle->isRedirect(),
+                               "Source {$fromTitle->getPrefixedText()} is not a redirect" );
+
+                       $target = Revision::newFromTitle( $fromTitle )->getContent()->getRedirectTarget();
+                       $this->assertSame( $toTitle->getPrefixedText(), $target->getPrefixedText() );
+               }
+
+               $this->assertSame( $id, $toTitle->getArticleId() );
+       }
+
+       /**
+        * Shortcut function to create a page and return its id.
+        *
+        * @param string $name Page to create
+        * @return int ID of created page
+        */
+       protected function createPage( $name ) {
+               return $this->editPage( $name, 'Content' )->value['revision']->getPage();
+       }
+
+       public function testFromWithFromid() {
+               $this->setExpectedException( ApiUsageException::class,
+                       'The parameters "from" and "fromid" can not be used together.' );
+
+               $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => 'Some page',
+                       'fromid' => 123,
+                       'to' => 'Some other page',
+               ] );
+       }
+
+       public function testMove() {
+               $name = ucfirst( __FUNCTION__ );
+
+               $id = $this->createPage( $name );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id );
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testMoveById() {
+               $name = ucfirst( __FUNCTION__ );
+
+               $id = $this->createPage( $name );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'fromid' => $id,
+                       'to' => "$name 2",
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id );
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testMoveNonexistent() {
+               $this->setExpectedException( ApiUsageException::class,
+                       "The page you specified doesn't exist." );
+
+               $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => 'Nonexistent page',
+                       'to' => 'Different page'
+               ] );
+       }
+
+       public function testMoveNonexistentId() {
+               $this->setExpectedException( ApiUsageException::class,
+                       'There is no page with ID 2147483647.' );
+
+               $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'fromid' => pow( 2, 31 ) - 1,
+                       'to' => 'Different page',
+               ] );
+       }
+
+       public function testMoveToInvalidPageName() {
+               $this->setExpectedException( ApiUsageException::class, 'Bad title "[".' );
+
+               $name = ucfirst( __FUNCTION__ );
+               $id = $this->createPage( $name );
+
+               try {
+                       $this->doApiRequestWithToken( [
+                               'action' => 'move',
+                               'from' => $name,
+                               'to' => '[',
+                       ] );
+               } finally {
+                       $this->assertSame( $id, Title::newFromText( $name )->getArticleId() );
+               }
+       }
+
+       // @todo File moving
+
+       public function testPingLimiter() {
+               global $wgRateLimits;
+
+               $this->setExpectedException( ApiUsageException::class,
+                       "You've exceeded your rate limit. Please wait some time and try again." );
+
+               $name = ucfirst( __FUNCTION__ );
+
+               $this->setMwGlobals( 'wgMainCacheType', 'hash' );
+
+               $this->stashMwGlobals( 'wgRateLimits' );
+               $wgRateLimits['move'] = [ '&can-bypass' => false, 'user' => [ 1, 60 ] ];
+
+               $id = $this->createPage( $name );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id );
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+
+               try {
+                       $this->doApiRequestWithToken( [
+                               'action' => 'move',
+                               'from' => "$name 2",
+                               'to' => "$name 3",
+                       ] );
+               } finally {
+                       $this->assertSame( $id, Title::newFromText( "$name 2" )->getArticleId() );
+                       $this->assertFalse( Title::newFromText( "$name 3" )->exists(),
+                               "\"$name 3\" should not exist" );
+               }
+       }
+
+       public function testTagsNoPermission() {
+               $this->setExpectedException( ApiUsageException::class,
+                       'You do not have permission to apply change tags along with your changes.' );
+
+               $name = ucfirst( __FUNCTION__ );
+
+               ChangeTags::defineTag( 'custom tag' );
+
+               $this->setGroupPermissions( 'user', 'applychangetags', false );
+
+               $id = $this->createPage( $name );
+
+               try {
+                       $this->doApiRequestWithToken( [
+                               'action' => 'move',
+                               'from' => $name,
+                               'to' => "$name 2",
+                               'tags' => 'custom tag',
+                       ] );
+               } finally {
+                       $this->assertSame( $id, Title::newFromText( $name )->getArticleId() );
+                       $this->assertFalse( Title::newFromText( "$name 2" )->exists(),
+                               "\"$name 2\" should not exist" );
+               }
+       }
+
+       public function testSelfMove() {
+               $this->setExpectedException( ApiUsageException::class,
+                       'The title is the same; cannot move a page over itself.' );
+
+               $name = ucfirst( __FUNCTION__ );
+               $this->createPage( $name );
+
+               $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => $name,
+               ] );
+       }
+
+       public function testMoveTalk() {
+               $name = ucfirst( __FUNCTION__ );
+
+               $id = $this->createPage( $name );
+               $talkId = $this->createPage( "Talk:$name" );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+                       'movetalk' => '',
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id );
+               $this->assertMoved( "Talk:$name", "Talk:$name 2", $talkId );
+
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testMoveTalkFailed() {
+               $name = ucfirst( __FUNCTION__ );
+
+               $id = $this->createPage( $name );
+               $talkId = $this->createPage( "Talk:$name" );
+               $talkDestinationId = $this->createPage( "Talk:$name 2" );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+                       'movetalk' => '',
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id );
+               $this->assertSame( $talkId, Title::newFromText( "Talk:$name" )->getArticleId() );
+               $this->assertSame( $talkDestinationId,
+                       Title::newFromText( "Talk:$name 2" )->getArticleId() );
+               $this->assertSame( [ [
+                       'message' => 'articleexists',
+                       'params' => [],
+                       'code' => 'articleexists',
+                       'type' => 'error',
+               ] ], $res[0]['move']['talkmove-errors'] );
+
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testMoveSubpages() {
+               global $wgNamespacesWithSubpages;
+
+               $name = ucfirst( __FUNCTION__ );
+
+               $this->stashMwGlobals( 'wgNamespacesWithSubpages' );
+               $wgNamespacesWithSubpages[NS_MAIN] = true;
+
+               $pages = [ $name, "$name/1", "$name/2", "Talk:$name", "Talk:$name/1", "Talk:$name/3" ];
+               $ids = [];
+               foreach ( array_merge( $pages, [ "$name/error", "$name 2/error" ] ) as $page ) {
+                       $ids[$page] = $this->createPage( $page );
+               }
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+                       'movetalk' => '',
+                       'movesubpages' => '',
+               ] );
+
+               foreach ( $pages as $page ) {
+                       $this->assertMoved( $page, str_replace( $name, "$name 2", $page ), $ids[$page] );
+               }
+
+               $this->assertSame( $ids["$name/error"],
+                       Title::newFromText( "$name/error" )->getArticleId() );
+               $this->assertSame( $ids["$name 2/error"],
+                       Title::newFromText( "$name 2/error" )->getArticleId() );
+
+               $results = array_merge( $res[0]['move']['subpages'], $res[0]['move']['subpages-talk'] );
+               foreach ( $results as $arr ) {
+                       if ( $arr['from'] === "$name/error" ) {
+                               $this->assertSame( [ [
+                                       'message' => 'articleexists',
+                                       'params' => [],
+                                       'code' => 'articleexists',
+                                       'type' => 'error'
+                               ] ], $arr['errors'] );
+                       } else {
+                               $this->assertSame( str_replace( $name, "$name 2", $arr['from'] ), $arr['to'] );
+                       }
+                       $this->assertCount( 2, $arr );
+               }
+
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testMoveNoPermission() {
+               $this->setExpectedException( ApiUsageException::class,
+                       'You must be a registered user and [[Special:UserLogin|logged in]] to move a page.' );
+
+               $name = ucfirst( __FUNCTION__ );
+
+               $id = $this->createPage( $name );
+
+               $user = new User();
+
+               try {
+                       $this->doApiRequestWithToken( [
+                               'action' => 'move',
+                               'from' => $name,
+                               'to' => "$name 2",
+                       ], null, $user );
+               } finally {
+                       $this->assertSame( $id, Title::newFromText( "$name" )->getArticleId() );
+                       $this->assertFalse( Title::newFromText( "$name 2" )->exists(),
+                               "\"$name 2\" should not exist" );
+               }
+       }
+
+       public function testSuppressRedirect() {
+               $name = ucfirst( __FUNCTION__ );
+
+               $id = $this->createPage( $name );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+                       'noredirect' => '',
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id, 'noredirect' );
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testSuppressRedirectNoPermission() {
+               $name = ucfirst( __FUNCTION__ );
+
+               $this->setGroupPermissions( 'sysop', 'suppressredirect', false );
+
+               $id = $this->createPage( $name );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => $name,
+                       'to' => "$name 2",
+                       'noredirect' => '',
+               ] );
+
+               $this->assertMoved( $name, "$name 2", $id );
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+
+       public function testMoveSubpagesError() {
+               $name = ucfirst( __FUNCTION__ );
+
+               // Subpages are allowed in talk but not main
+               $idBase = $this->createPage( "Talk:$name" );
+               $idSub = $this->createPage( "Talk:$name/1" );
+
+               $res = $this->doApiRequestWithToken( [
+                       'action' => 'move',
+                       'from' => "Talk:$name",
+                       'to' => $name,
+                       'movesubpages' => '',
+               ] );
+
+               $this->assertMoved( "Talk:$name", $name, $idBase );
+               $this->assertSame( $idSub, Title::newFromText( "Talk:$name/1" )->getArticleId() );
+               $this->assertFalse( Title::newFromText( "$name/1" )->exists(),
+                       "\"$name/1\" should not exist" );
+
+               $this->assertSame( [ 'errors' => [ [
+                       'message' => 'namespace-nosubpages',
+                       'params' => [ '' ],
+                       'code' => 'namespace-nosubpages',
+                       'type' => 'error',
+               ] ] ], $res[0]['move']['subpages'] );
+
+               $this->assertArrayNotHasKey( 'warnings', $res[0] );
+       }
+}