r110807 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r110806‎ \| r110807 \| r110808 >
Date:	23:58, 6 February 2012
Author:	aaron
Status:	ok (Comments)
Tags:	api
Comment:	Follow-up r100165: * Fixed lack of $db->timestamp() formatting calls * Made a few w/s tweaks (to getAllowedParams, getParamDescription) * Broke some long lines
Modified paths:	/trunk/extensions/CheckUser/api/ApiQueryCheckUser.php (modified) (history) /trunk/extensions/CheckUser/api/ApiQueryCheckUserLog.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CheckUser/api/ApiQueryCheckUser.php
—	—	@@ -13,10 +13,11 @@
14	14
15	15	$db = $this->getDB( DB_SLAVE );
16	16	$params = $this->extractRequestParams();
17		-
18		~~- list( $request, $target, $reason, $timecond, $limit, $xff ) = array( $params['request'],~~
19		~~- $params['target'], $params['reason'], $params['timecond'], $params['limit'], $params['xff'] );~~
20	17
	18	+ list( $request, $target, $reason, $timecond, $limit, $xff ) = array(
	19	+ $params['request'], $params['target'], $params['reason'],
	20	+ $params['timecond'], $params['limit'], $params['xff'] );
	21	+
21	22	if ( !$wgUser->isAllowed( 'checkuser' ) ) {
22	23	$this->dieUsage( 'You need the checkuser right', 'permissionerror' );
23	24	}
—	—	@@ -26,17 +27,15 @@
27	28	}
28	29
29	30	$reason = wfMsgForContent( 'checkuser-reason-api', $reason );
30		~~- $time = wfTimestamp( TS_MW,~~
31		~~- strtotime( 'now' ) - ( strtotime( $timecond ? $timecond : '2 weeks' ) - strtotime( 'now' ) )~~
32		~~- );~~
33		~~- if ( !$time ) {~~
	31	+ $timeCutoff = strtotime( $timecond ); // absolute time
	32	+ if ( !$timeCutoff ) {
34	33	$this->dieUsage( 'You need use correct time limit (like "2 weeks")', 'invalidtime' );
35	34	}
36	35
37	36	$this->addTables( 'cu_changes' );
38	37	$this->addOption( 'LIMIT', $limit + 1 );
39	38	$this->addOption( 'ORDER BY', 'cuc_timestamp DESC' );
40		~~- $this->addWhere( "cuc_timestamp > " . $db->addQuotes( $time ) );~~
	39	+ $this->addWhere( "cuc_timestamp > " . $db->addQuotes( $db->timestamp( $timeCutoff ) ) );
41	40
42	41	switch ( $request ) {
43	42	case 'userips':
—	—	@@ -71,8 +70,10 @@
72	71	}
73	72
74	73	CheckUser::addLogEntry( 'userips', 'user', $target, $reason, $user_id );
75		~~- $result->addValue( array( 'query', $this->getModuleName() ), 'userips', $resultIPs );~~
76		~~- $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'userips' ), 'ip' );~~
	74	+ $result->addValue( array(
	75	+ 'query', $this->getModuleName() ), 'userips', $resultIPs );
	76	+ $result->setIndexedTagName_internal( array(
	77	+ 'query', $this->getModuleName(), 'userips' ), 'ip' );
77	78	break;
78	79
79	80	case 'edits':
—	—	@@ -92,15 +93,16 @@
93	94	} else {
94	95	$user_id = User::idFromName( $target );
95	96	if ( !$user_id ) {
96		~~- $this->dieUsage( 'Target user is not exists', 'nosuchuser' );~~
	97	+ $this->dieUsage( 'Target user does not exists', 'nosuchuser' );
97	98	}
98	99	$this->addWhereFld( 'cuc_user_text', $target );
99	100	$log_type = array( 'useredits', 'user' );
100	101	}
101	102
102		~~- $this->addFields( array( 'cuc_namespace', 'cuc_title', 'cuc_user_text', 'cuc_actiontext',~~
103		~~- 'cuc_comment', 'cuc_minor', 'cuc_timestamp', 'cuc_ip', 'cuc_xff', 'cuc_agent' )~~
104		~~- );~~
	103	+ $this->addFields( array(
	104	+ 'cuc_namespace', 'cuc_title', 'cuc_user_text', 'cuc_actiontext',
	105	+ 'cuc_comment', 'cuc_minor', 'cuc_timestamp', 'cuc_ip', 'cuc_xff', 'cuc_agent'
	106	+ ) );
105	107
106	108	$res = $this->select( __METHOD__ );
107	109	$result = $this->getResult();
—	—	@@ -109,11 +111,11 @@
110	112	foreach ( $res as $row ) {
111	113	$edit = array(
112	114	'timestamp' => wfTimestamp( TS_ISO_8601, $row->cuc_timestamp ),
113		~~- 'ns' => intval( $row->cuc_namespace ),~~
114		~~- 'title' => $row->cuc_title,~~
115		~~- 'user' => $row->cuc_user_text,~~
116		~~- 'ip' => $row->cuc_ip,~~
117		~~- 'agent' => $row->cuc_agent,~~
	115	+ 'ns' => intval( $row->cuc_namespace ),
	116	+ 'title' => $row->cuc_title,
	117	+ 'user' => $row->cuc_user_text,
	118	+ 'ip' => $row->cuc_ip,
	119	+ 'agent' => $row->cuc_agent,
118	120	);
119	121	if ( $row->cuc_actiontext ) {
120	122	$edit['summary'] = $row->cuc_actiontext;
—	—	@@ -129,9 +131,12 @@
130	132	$edits[] = $edit;
131	133	}
132	134
133		~~- CheckUser::addLogEntry( $log_type[0], $log_type[1], $target, $reason, isset($user_id) ? $user_id : '0' );~~
134		~~- $result->addValue( array( 'query', $this->getModuleName() ), 'edits', $edits );~~
135		~~- $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'edits' ), 'action' );~~
	135	+ CheckUser::addLogEntry( $log_type[0], $log_type[1],
	136	+ $target, $reason, isset($user_id) ? $user_id : '0' );
	137	+ $result->addValue( array(
	138	+ 'query', $this->getModuleName() ), 'edits', $edits );
	139	+ $result->setIndexedTagName_internal( array(
	140	+ 'query', $this->getModuleName(), 'edits' ), 'action' );
136	141	break;
137	142
138	143	case 'ipusers':
—	—	@@ -146,7 +151,8 @@
147	152	$this->dieUsage( 'IP or range is invalid', 'invalidip' );
148	153	}
149	154
150		~~- $this->addFields( array( 'cuc_user_text', 'cuc_timestamp', 'cuc_ip', 'cuc_agent' ) );~~
	155	+ $this->addFields( array(
	156	+ 'cuc_user_text', 'cuc_timestamp', 'cuc_ip', 'cuc_agent' ) );
151	157
152	158	$res = $this->select( __METHOD__ );
153	159	$result = $this->getResult();
—	—	@@ -184,8 +190,10 @@
185	191	}
186	192
187	193	CheckUser::addLogEntry( $log_type, 'ip', $target, $reason );
188		~~- $result->addValue( array( 'query', $this->getModuleName() ), 'ipusers', $resultUsers );~~
189		~~- $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'ipusers' ), 'user' );~~
	194	+ $result->addValue( array(
	195	+ 'query', $this->getModuleName() ), 'ipusers', $resultUsers );
	196	+ $result->setIndexedTagName_internal( array(
	197	+ 'query', $this->getModuleName(), 'ipusers' ), 'user' );
190	198	break;
191	199
192	200	default:
—	—	@@ -203,7 +211,7 @@
204	212
205	213	public function getAllowedParams() {
206	214	return array(
207		~~- 'request' => array(~~
	215	+ 'request' => array(
208	216	ApiBase::PARAM_REQUIRED => true,
209	217	ApiBase::PARAM_TYPE => array(
210	218	'userips',
—	—	@@ -211,35 +219,37 @@
212	220	'ipusers',
213	221	)
214	222	),
215		~~- 'target' => array(~~
	223	+ 'target' => array(
216	224	ApiBase::PARAM_REQUIRED => true,
217	225	),
218		~~- 'reason' => null,~~
219		~~- 'limit' => array(~~
	226	+ 'reason' => null,
	227	+ 'limit' => array(
220	228	ApiBase::PARAM_DFLT => 1000,
221	229	ApiBase::PARAM_TYPE => 'limit',
222		~~- ApiBase::PARAM_MIN => 1,~~
223		~~- ApiBase::PARAM_MAX => 500,~~
	230	+ ApiBase::PARAM_MIN => 1,
	231	+ ApiBase::PARAM_MAX => 500,
224	232	ApiBase::PARAM_MAX2 => 5000,
225	233	),
226		~~- 'timecond' => null,~~
227		~~- 'xff' => null,~~
	234	+ 'timecond' => array(
	235	+ ApiBase::PARAM_DFLT => '-2 weeks'
	236	+ ),
	237	+ 'xff' => null,
228	238	);
229	239	}
230	240
231	241	public function getParamDescription() {
232	242	return array(
233		~~- 'request' => array(~~
	243	+ 'request' => array(
234	244	'Type of CheckUser request',
235	245	' userips - get IP of target user',
236	246	' edits - get changes from target IP or range',
237	247	' ipusers - get users from target IP or range',
238	248	),
239		~~- 'target' => "Username or IP-address/range to perform check",~~
240		~~- 'reason' => 'Reason to check',~~
241		~~- 'limit' => 'Limit of rows',~~
	249	+ 'target' => "Username or IP-address/range to perform check",
	250	+ 'reason' => 'Reason to check',
	251	+ 'limit' => 'Limit of rows',
242	252	'timecond' => 'Time limit of user data (like "2 weeks")',
243		~~- 'xff' => 'Use xff data instead of IP',~~
	253	+ 'xff' => 'Use xff data instead of IP',
244	254	);
245	255	}
246	256
Index: trunk/extensions/CheckUser/api/ApiQueryCheckUserLog.php
—	—	@@ -16,14 +16,15 @@
17	17	if ( !$wgUser->isAllowed( 'checkuser-log' ) ) {
18	18	$this->dieUsage( 'You need the checkuser-log right', 'permissionerror' );
19	19	}
20		-
	20	+
21	21	list( $user, $limit, $target, $from, $to ) = array( $params['user'], $params['limit'],
22	22	$params['target'], $params['from'], $params['to'] );
23	23
24	24	$this->addTables( 'cu_log' );
25	25	$this->addOption( 'LIMIT', $limit + 1 );
26		~~- $this->addWhereRange( 'cul_timestamp', 'older', $from, $to );~~
27		~~- $this->addFields( array( 'cul_timestamp', 'cul_user_text', 'cul_reason', 'cul_type', 'cul_target_text' ) );~~
	26	+ $this->addTimestampWhereRange( 'cul_timestamp', 'older', $from, $to );
	27	+ $this->addFields( array(
	28	+ 'cul_timestamp', 'cul_user_text', 'cul_reason', 'cul_type', 'cul_target_text' ) );
28	29
29	30	if ( isset( $user ) ) {
30	31	$this->addWhereFld( 'cul_user_text', $user );
—	—	@@ -40,31 +41,32 @@
41	42	$log[] = array(
42	43	'timestamp' => wfTimestamp( TS_ISO_8601, $row->cul_timestamp ),
43	44	'checkuser' => $row->cul_user_text,
44		~~- 'type' => $row->cul_type,~~
45		~~- 'reason' => $row->cul_reason,~~
46		~~- 'target' => $row->cul_target_text,~~
	45	+ 'type' => $row->cul_type,
	46	+ 'reason' => $row->cul_reason,
	47	+ 'target' => $row->cul_target_text,
47	48	);
48	49	}
49	50
50	51	$result->addValue( array( 'query', $this->getModuleName() ), 'entries', $log );
51		~~- $result->setIndexedTagName_internal( array( 'query', $this->getModuleName(), 'entries' ), 'entry' );~~
	52	+ $result->setIndexedTagName_internal(
	53	+ array( 'query', $this->getModuleName(), 'entries' ), 'entry' );
52	54	}
53	55
54	56	public function getAllowedParams() {
55	57	return array(
56		~~- 'user' => null,~~
	58	+ 'user' => null,
57	59	'target' => null,
58		~~- 'limit' => array(~~
	60	+ 'limit' => array(
59	61	ApiBase::PARAM_DFLT => 10,
60	62	ApiBase::PARAM_TYPE => 'limit',
61		~~- ApiBase::PARAM_MIN => 1,~~
62		~~- ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1,~~
	63	+ ApiBase::PARAM_MIN => 1,
	64	+ ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1,
63	65	ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2,
64	66	),
65		~~- 'from' => array(~~
	67	+ 'from' => array(
66	68	ApiBase::PARAM_TYPE => 'timestamp',
67	69	),
68		~~- 'to' => array(~~
	70	+ 'to' => array(
69	71	ApiBase::PARAM_TYPE => 'timestamp',
70	72	),
71	73	);
—	—	@@ -72,11 +74,11 @@
73	75
74	76	public function getParamDescription() {
75	77	return array(
76		~~- 'user' => 'Username of CheckUser',~~
	78	+ 'user' => 'Username of CheckUser',
77	79	'target' => "Checked user or IP-address/range",
78		~~- 'limit' => 'Limit of rows',~~
79		~~- 'from' => 'The timestamp to start enumerating from',~~
80		~~- 'to' => 'The timestamp to end enumerating',~~
	80	+ 'limit' => 'Limit of rows',
	81	+ 'from' => 'The timestamp to start enumerating from',
	82	+ 'to' => 'The timestamp to end enumerating',
81	83	);
82	84	}
83	85

Follow-up revisions

Revision	Commit summary	Author	Date
r113532	Typo fix, followup r110807	awjrichards	22:16, 9 March 2012

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r100165	Adds API Module by cryptocoryne...	johnduhart	21:37, 18 October 2011

Comments

#Comment by Aaron Schulz (talk | contribs) 00:00, 7 February 2012

This also cleans up the 'timecond' logic.

#Comment by Jack Phoenix (talk | contribs) 14:56, 7 February 2012

-						$this->dieUsage( 'Target user is not exists', 'nosuchuser' );
+						$this->dieUsage( 'Target user does not exists', 'nosuchuser' );

Still not quite right yet ;-) How about "Target user does not exist"?

#Comment by Aaron Schulz (talk | contribs) 23:15, 7 February 2012

Do you have commit access?

#Comment by Jack Phoenix (talk | contribs) 20:09, 9 February 2012

Have had it since 2008, but I figured I'd comment on CR anyway.

Status & tagging log

22:19, 9 March 2012 Awjrichards (talk | contribs) changed the status of r110807 [removed: new added: ok]
00:48, 7 February 2012 RobLa-WMF (talk | contribs) changed the tags for r110807 [added: api]