r64852 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r64851‎ \| r64852 \| r64853 >
Date:	06:11, 10 April 2010
Author:	mah
Status:	ok
Tags:
Comment:	* Clean up some duplicated code in r64291 Would've like to refactor the $wgUser->Watch — but I'm not sure if the hooks that come along with $articleObj->Watch are ok.
Modified paths:	/trunk/phase3/includes/api/ApiBase.php (modified) (history) /trunk/phase3/includes/api/ApiDelete.php (modified) (history) /trunk/phase3/includes/api/ApiProtect.php (modified) (history) /trunk/phase3/includes/api/ApiRollback.php (modified) (history) /trunk/phase3/maintenance/tests/ApiWatchTest.php (added) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/ApiWatchTest.php
—	—	@@ -0,0 +1,205 @@
	2	+<?php
	3	+
	4	+global $IP;
	5	+require_once( "$IP/maintenance/tests/ApiSetup.php" );
	6	+
	7	+class ApiWatchTest extends ApiSetup {
	8	+
	9	+ function setUp() {
	10	+ ini_set( 'log_errors', 1 );
	11	+ ini_set( 'error_reporting', 1 );
	12	+ ini_set( 'display_errors', 1 );
	13	+ }
	14	+
	15	+ function doApiRequest( $params, $data = null ) {
	16	+ $_SESSION = isset( $data[2] ) ? $data[2] : array();
	17	+
	18	+ $req = new FauxRequest( $params, true, $session );
	19	+ $module = new ApiMain( $req, true );
	20	+ $module->execute();
	21	+
	22	+ $data[0] = $module->getResultData();
	23	+ $data[1] = $req;
	24	+ $data[2] = $_SESSION;
	25	+
	26	+ return $data;
	27	+ }
	28	+
	29	+ function testLogin() {
	30	+ $data = $this->doApiRequest( array(
	31	+ 'action' => 'login',
	32	+ 'lgname' => 'WikiSysop',
	33	+ 'lgpassword' => 'none' ), $data );
	34	+
	35	+ $this->assertArrayHasKey( "login", $data[0] );
	36	+ $this->assertArrayHasKey( "result", $data[0]['login'] );
	37	+ $this->assertEquals( "NeedToken", $data[0]['login']['result'] );
	38	+ $token = $data[0]['login']['token'];
	39	+
	40	+ $data = $this->doApiRequest( array(
	41	+ 'action' => 'login',
	42	+ "lgtoken" => $token,
	43	+ "lgname" => 'WikiSysop',
	44	+ "lgpassword" => 'none' ), $data );
	45	+
	46	+ $this->assertArrayHasKey( "login", $data[0] );
	47	+ $this->assertArrayHasKey( "result", $data[0]['login'] );
	48	+ $this->assertEquals( "Success", $data[0]['login']['result'] );
	49	+ $this->assertArrayHasKey( 'lgtoken', $data[0]['login'] );
	50	+
	51	+ return $data;
	52	+ }
	53	+
	54	+ function testGetToken() {
	55	+
	56	+ $data = $this->doApiRequest( array(
	57	+ 'action' => 'query',
	58	+ 'titles' => 'Main Page',
	59	+ 'intoken' => 'edit\|delete\|protect\|move\|block\|unblock',
	60	+ 'prop' => 'info'), $data);
	61	+
	62	+ $this->assertArrayHasKey( 'query', $data[0] );
	63	+ $this->assertArrayHasKey( 'pages', $data[0]['query'] );
	64	+ $key = array_pop( array_keys( $data[0]['query']['pages'] ) );
	65	+
	66	+ $this->assertArrayHasKey( $key, $data[0]['query']['pages'] );
	67	+ $this->assertArrayHasKey( 'edittoken', $data[0]['query']['pages'][$key]);
	68	+ $this->assertArrayHasKey( 'movetoken', $data[0]['query']['pages'][$key]);
	69	+ $this->assertArrayHasKey( 'deletetoken', $data[0]['query']['pages'][$key]);
	70	+ $this->assertArrayHasKey( 'blocktoken', $data[0]['query']['pages'][$key]);
	71	+ $this->assertArrayHasKey( 'unblocktoken', $data[0]['query']['pages'][$key]);
	72	+ $this->assertArrayHasKey( 'protecttoken', $data[0]['query']['pages'][$key]);
	73	+
	74	+ return $data;
	75	+ }
	76	+
	77	+ /**
	78	+ * @depends testGetToken
	79	+ */
	80	+ function testWatchEdit( $data ) {
	81	+ $key = array_pop( array_keys( $data[0]['query']['pages'] ) );
	82	+ $pageinfo = $data[0]['query']['pages'][$key];
	83	+
	84	+ $data = $this->doApiRequest( array(
	85	+ 'action' => 'edit',
	86	+ 'title' => 'Main Page',
	87	+ 'text' => 'new text',
	88	+ 'token' => $pageinfo['edittoken'],
	89	+ 'watchlist' => 'watch'), $data);
	90	+ $this->assertArrayHasKey( 'edit', $data[0] );
	91	+ $this->assertArrayHasKey( 'result', $data[0]['edit'] );
	92	+ $this->assertEquals( 'Success', $data[0]['edit']['result'] );
	93	+
	94	+ return $data;
	95	+ }
	96	+
	97	+
	98	+ /**
	99	+ * @depends testWatchEdit
	100	+ */
	101	+ function testWatchClear( $data ) {
	102	+ global $wgUser;
	103	+ $data = $this->doApiRequest( array(
	104	+ 'action' => 'query',
	105	+ 'list' => 'watchlist'), $data);
	106	+
	107	+ if(isset($data[0]['query']['watchlist'])) {
	108	+ $wl = $data[0]['query']['watchlist'];
	109	+
	110	+ foreach($wl as $page) {
	111	+ $data = $this->doApiRequest( array(
	112	+ 'action' => 'watch',
	113	+ 'title' => $page['title'],
	114	+ 'unwatch' => true), $data);
	115	+ }
	116	+ }
	117	+ $data = $this->doApiRequest( array(
	118	+ 'action' => 'query',
	119	+ 'list' => 'watchlist'), $data);
	120	+ $this->assertArrayHasKey( 'query', $data[0] );
	121	+ $this->assertArrayHasKey( 'watchlist', $data[0]['query'] );
	122	+ $this->assertEquals( 0, count($data[0]['query']['watchlist']) );
	123	+
	124	+ return $data;
	125	+ }
	126	+
	127	+ /**
	128	+ * @depends testGetToken
	129	+ */
	130	+ function testWatchProtect( $data ) {
	131	+ $key = array_pop( array_keys( $data[0]['query']['pages'] ) );
	132	+ $pageinfo = $data[0]['query']['pages'][$key];
	133	+
	134	+ $data = $this->doApiRequest( array(
	135	+ 'action' => 'protect',
	136	+ 'token' => $pageinfo['protecttoken'],
	137	+ 'title' => 'Main Page',
	138	+ 'protections' => 'edit=sysop',
	139	+ 'watchlist' => 'unwatch'), $data);
	140	+
	141	+ $this->assertArrayHasKey( 'protect', $data[0] );
	142	+ $this->assertArrayHasKey( 'protections', $data[0]['protect'] );
	143	+ $this->assertEquals( 1, count($data[0]['protect']['protections']) );
	144	+ $this->assertArrayHasKey( 'edit', $data[0]['protect']['protections'][0] );
	145	+ }
	146	+
	147	+ /**
	148	+ * @depends testGetToken
	149	+ */
	150	+ function testGetRollbackToken( $data ) {
	151	+ $data = $this->doApiRequest( array(
	152	+ 'action' => 'query',
	153	+ 'prop' => 'revisions',
	154	+ 'titles' => 'Main Page',
	155	+ 'rvtoken' => 'rollback'), $data);
	156	+
	157	+ $this->assertArrayHasKey( 'query', $data[0] );
	158	+ $this->assertArrayHasKey( 'pages', $data[0]['query'] );
	159	+ $key = array_pop( array_keys( $data[0]['query']['pages'] ) );
	160	+
	161	+ $this->assertArrayHasKey( 'pageid', $data[0]['query']['pages'][$key]);
	162	+ $this->assertArrayHasKey( 'revisions', $data[0]['query']['pages'][$key]);
	163	+ $this->assertArrayHasKey( 0, $data[0]['query']['pages'][$key]['revisions'] );
	164	+ $this->assertArrayHasKey( 'rollbacktoken', $data[0]['query']['pages'][$key]['revisions'][0] );
	165	+
	166	+ return $data;
	167	+ }
	168	+
	169	+ /**
	170	+ * @depends testGetRollbackToken
	171	+ */
	172	+ function testWatchRollback( $data ) {
	173	+ $key = array_pop( array_keys( $data[0]['query']['pages'] ) );
	174	+ $pageinfo = $data[0]['query']['pages'][$key]['revisions'][0];
	175	+
	176	+ $data = $this->doApiRequest( array(
	177	+ 'action' => 'rollback',
	178	+ 'title' => 'Main Page',
	179	+ 'user' => 'WikiSysop',
	180	+ 'token' => $pageinfo['rollbacktoken'],
	181	+ 'watchlist' => 'watch'), $data);
	182	+
	183	+ $this->assertArrayHasKey( 'rollback', $data[0] );
	184	+ $this->assertArrayHasKey( 'title', $data[0]['rollback'] );
	185	+ }
	186	+
	187	+ /**
	188	+ * @depends testGetToken
	189	+ */
	190	+ function testWatchDelete( $data ) {
	191	+ $key = array_pop( array_keys( $data[0]['query']['pages'] ) );
	192	+ $pageinfo = $data[0]['query']['pages'][$key];
	193	+
	194	+ $data = $this->doApiRequest( array(
	195	+ 'action' => 'delete',
	196	+ 'token' => $pageinfo['deletetoken'],
	197	+ 'title' => 'Main Page'), $data);
	198	+ $this->assertArrayHasKey( 'delete', $data[0] );
	199	+ $this->assertArrayHasKey( 'title', $data[0]['delete'] );
	200	+
	201	+ $data = $this->doApiRequest( array(
	202	+ 'action' => 'query',
	203	+ 'list' => 'watchlist'), $data);
	204	+ }
	205	+
	206	+}
Property changes on: trunk/phase3/maintenance/tests/ApiWatchTest.php
___________________________________________________________________
Name: svn:eol-syle
1	207	+ native
Index: trunk/phase3/includes/api/ApiProtect.php
—	—	@@ -113,21 +113,10 @@
114	114
115	115	$cascade = $params['cascade'];
116	116	$articleObj = new Article( $titleObj );
117		-
118		~~- $watch = $this->getWatchlistValue( $params['watchlist'], $titleObj );~~
119		-
120		~~- if ( $params['watch'] ) {~~
121		~~- $watch = true;~~
122		~~- }~~
123		-
124		~~- if ( $watch !== null ) {~~
125		~~- if ( $watch ) {~~
126		~~- $articleObj->doWatch();~~
127		~~- } else {~~
128		~~- $articleObj->doUnwatch();~~
129		~~- }~~
130		~~- }~~
131	117
	118	+ $watch = $params['watch'] ? 'watch' : $params['watchlist'];
	119	+ $this->setWatch( $watch, $titleObj );
	120	+
132	121	if ( $titleObj->exists() ) {
133	122	$ok = $articleObj->updateRestrictions( $protections, $params['reason'], $cascade, $expiryarray );
134	123	} else {
Index: trunk/phase3/includes/api/ApiRollback.php
—	—	@@ -35,7 +35,7 @@
36	36	public function __construct( $main, $action ) {
37	37	parent::__construct( $main, $action );
38	38	}
39		-
	39	+
40	40	private $mTitleObj = null;
41	41
42	42	public function execute() {
—	—	@@ -52,17 +52,9 @@
53	53	// We don't care about multiple errors, just report one of them
54	54	$this->dieUsageMsg( reset( $retval ) );
55	55	}
56		-
57		~~- $watch = $this->getWatchlistValue( $params['watchlist'], $this->mTitleObj );~~
58		-
59		~~- if ( $watch !== null) {~~
60		~~- if ( $watch ) {~~
61		~~- $articleObj->doWatch();~~
62		~~- } else {~~
63		~~- $articleObj->doUnwatch();~~
64		~~- }~~
65		~~- }~~
66	56
	57	+ $this->setWatch( $params['watchlist'], $this->mTitleObj );
	58	+
67	59	$info = array(
68	60	'title' => $this->mTitleObj->getPrefixedText(),
69	61	'pageid' => intval( $details['current']->getPage() ),
Index: trunk/phase3/includes/api/ApiDelete.php
—	—	@@ -81,23 +81,19 @@
82	82	if ( count( $retval ) ) {
83	83	$this->dieUsageMsg( reset( $retval ) ); // We don't care about multiple errors, just report one of them
84	84	}
85		-
86		~~- $watch = $this->getWatchlistValue( $params['watchlist'], $titleObj ) \|\| $wgUser->getOption( 'watchdeletion' );~~
87	85
	86	+ $watch = 'nochange';
88	87	// Deprecated parameters
89	88	if ( $params['watch'] ) {
90		~~- $watch = true;~~
	89	+ $watch = 'watch';
91	90	} elseif ( $params['unwatch'] ) {
92		~~- $watch = false;~~
	91	+ $watch = 'unwatch';
	92	+ } elseif ( $wgUser->getOption( 'watchdeletion' ) ) {
	93	+ $watch = 'watch';
	94	+ } else {
	95	+ $watch = $params['watchlist'];
93	96	}
94		-
95		~~- if ( $watch !== null ) {~~
96		~~- if ( $watch ) {~~
97		~~- $articleObj->doWatch();~~
98		~~- } else {~~
99		~~- $articleObj->doUnwatch();~~
100		~~- }~~
101		~~- }~~
	97	+ $this->setWatch( $watch, $titleObj );
102	98	}
103	99
104	100	$r = array( 'title' => $titleObj->getPrefixedText(), 'reason' => $reason );
Index: trunk/phase3/includes/api/ApiBase.php
—	—	@@ -536,14 +536,19 @@
537	537	}
538	538
539	539	/**
540		~~- * Handle watchlist settings~~
541		~~- */~~
	540	+ * Return true if we're to watch the page, false if not, null if no change.
	541	+ * @param $watch String Valid values: 'watch', 'unwatch', 'preferences', 'nochange'
	542	+ * @param $titleObj Title (optional) the page under consideration
	543	+ * @returns mixed
	544	+ */
542	545	protected function getWatchlistValue ( $watchlist, $titleObj = null ) {
543	546	switch ( $watchlist ) {
544	547	case 'watch':
545	548	return true;
	549	+
546	550	case 'unwatch':
547	551	return false;
	552	+
548	553	case 'preferences':
549	554	global $wgUser;
550	555	if ( isset($titleObj)
—	—	@@ -553,13 +558,33 @@
554	559	return true;
555	560	}
556	561	return null;
	562	+
557	563	case 'nochange':
	564	+ return null;
	565	+
558	566	default:
559	567	return null;
560	568	}
561	569	}
562	570
563	571	/**
	572	+ * Set a watch (or unwatch) based the based on a watchlist parameter.
	573	+ * @param $watch String Valid values: 'watch', 'unwatch', 'preferences', 'nochange'
	574	+ * @param $titleObj Title the article's title to change
	575	+ */
	576	+ protected function setWatch ( $watch, $titleObj ) {
	577	+ $value = $this->getWatchlistValue( $watch, $titleObj );
	578	+ if( $value === null ) return;
	579	+
	580	+ $articleObj = new Article( $titleObj );
	581	+ if ( $value ) {
	582	+ $articleObj->doWatch();
	583	+ } else {
	584	+ $articleObj->doUnwatch();
	585	+ }
	586	+ }
	587	+
	588	+ /**
564	589	* Using the settings determine the value for the given parameter
565	590	*
566	591	* @param $paramName String: parameter name

Follow-up revisions

Revision	Commit summary	Author	Date
r64942	Fix param names mismatch in code/doc from r64852 and r64397	ialex	19:10, 11 April 2010
r64962	re r64291 and r64852 use ApiBase::setWatch instead of wgUser->*Watch to set w...	mah	15:44, 12 April 2010
r65010	* EOL ws clean on ApiBase.php...	mah	04:57, 14 April 2010

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r64291	Followup r64197...	reedy	15:08, 28 March 2010

Status & tagging log

12:17, 10 April 2010 Catrope (talk | contribs) changed the status of r64852 [removed: new added: ok]