r94856 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r94855‎ \| r94856 \| r94857 >
Date:	01:46, 18 August 2011
Author:	kaldari
Status:	ok (Comments)
Tags:
Comment:	date range for campaign log filter
Modified paths:	/trunk/extensions/CentralNotice/CentralNoticeLogPager.php (modified) (history) /trunk/extensions/CentralNotice/special/SpecialCentralNoticeLogs.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/special/SpecialCentralNoticeLogs.php
—	—	@@ -78,16 +78,22 @@
79	79	$reset = $wgRequest->getVal( 'centralnoticelogreset' );
80	80	$campaign = $wgRequest->getVal( 'campaign' );
81	81	$user = $wgRequest->getVal( 'user' );
82		~~- $year = $wgRequest->getVal( 'year' );~~
83		~~- if ( $year === 'other' ) $year = null;~~
84		~~- $month = $wgRequest->getVal( 'month' );~~
85		~~- if ( $month === 'other' ) $month = null;~~
86		~~- $day = $wgRequest->getVal( 'day' );~~
87		~~- if ( $day === 'other' ) $day = null;~~
	82	+ $startYear = $wgRequest->getVal( 'start_year' );
	83	+ if ( $startYear === 'other' ) $startYear = null;
	84	+ $startMonth = $wgRequest->getVal( 'start_month' );
	85	+ if ( $startMonth === 'other' ) $startMonth = null;
	86	+ $startDay = $wgRequest->getVal( 'start_day' );
	87	+ if ( $startDay === 'other' ) $startDay = null;
	88	+ $endYear = $wgRequest->getVal( 'end_year' );
	89	+ if ( $endYear === 'other' ) $endYear = null;
	90	+ $endMonth = $wgRequest->getVal( 'end_month' );
	91	+ if ( $endMonth === 'other' ) $endMonth = null;
	92	+ $endDay = $wgRequest->getVal( 'end_day' );
	93	+ if ( $endDay === 'other' ) $endDay = null;
88	94
89	95	$htmlOut .= Xml::openElement( 'div', array( 'id' => 'cn-log-filters-container' ) );
90	96
91		~~- if ( $campaign \|\| $user \|\| $year \|\| $month \|\| $day ) { // filters on~~
	97	+ if ( $campaign \|\| $user \|\| $startYear \|\| $startMonth \|\| $startDay \|\| $endYear \|\| $endMonth \|\| $endDay ) { // filters on
92	98	$htmlOut .= '<a href="javascript:toggleFilterDisplay()">'.
93	99	'<img src="'.$wgExtensionAssetsPath.'/CentralNotice/collapsed.png" id="cn-collapsed-filter-arrow" style="display:none;position:relative;top:-2px;"/>'.
94	100	'<img src="'.$wgExtensionAssetsPath.'/CentralNotice/uncollapsed.png" id="cn-uncollapsed-filter-arrow" style="display:inline-block;position:relative;top:-2px;"/>'.
—	—	@@ -107,13 +113,13 @@
108	114	$htmlOut .= Xml::openElement( 'tr' );
109	115
110	116	$htmlOut .= Xml::openElement( 'td' );
111		~~- $htmlOut .= Xml::label( wfMsg( 'centralnotice-date' ), 'month', array( 'class' => 'cn-log-filter-label' ) );~~
	117	+ $htmlOut .= Xml::label( wfMsg( 'centralnotice-start-date' ), 'month', array( 'class' => 'cn-log-filter-label' ) );
112	118	$htmlOut .= Xml::closeElement( 'td' );
113	119	$htmlOut .= Xml::openElement( 'td' );
114	120	if ( $reset ) {
115		~~- $htmlOut .= $this->dateSelector();~~
	121	+ $htmlOut .= $this->dateSelector( 'start' );
116	122	} else {
117		~~- $htmlOut .= $this->dateSelector( $year, $month, $day );~~
	123	+ $htmlOut .= $this->dateSelector( 'start', $startYear, $startMonth, $startDay );
118	124	}
119	125	$htmlOut .= Xml::closeElement( 'td' );
120	126
—	—	@@ -121,6 +127,20 @@
122	128	$htmlOut .= Xml::openElement( 'tr' );
123	129
124	130	$htmlOut .= Xml::openElement( 'td' );
	131	+ $htmlOut .= Xml::label( wfMsg( 'centralnotice-end-date' ), 'month', array( 'class' => 'cn-log-filter-label' ) );
	132	+ $htmlOut .= Xml::closeElement( 'td' );
	133	+ $htmlOut .= Xml::openElement( 'td' );
	134	+ if ( $reset ) {
	135	+ $htmlOut .= $this->dateSelector( 'end' );
	136	+ } else {
	137	+ $htmlOut .= $this->dateSelector( 'end', $endYear, $endMonth, $endDay );
	138	+ }
	139	+ $htmlOut .= Xml::closeElement( 'td' );
	140	+
	141	+ $htmlOut .= Xml::closeElement( 'tr' );
	142	+ $htmlOut .= Xml::openElement( 'tr' );
	143	+
	144	+ $htmlOut .= Xml::openElement( 'td' );
125	145	$htmlOut .= Xml::label( wfMsg( 'centralnotice-notice' ), 'campaign', array( 'class' => 'cn-log-filter-label' ) );
126	146	$htmlOut .= Xml::closeElement( 'td' );
127	147	$htmlOut .= Xml::openElement( 'td' );
—	—	@@ -180,15 +200,15 @@
181	201	$wgOut->addHTML( Xml::closeElement( 'div' ) );
182	202	}
183	203
184		~~- private function dateSelector( $year = 0, $month = 0, $day = 0 ) {~~
	204	+ private function dateSelector( $prefix, $year = 0, $month = 0, $day = 0 ) {
185	205	$years = range( 2010, 2016 );
186	206	$months = CentralNotice::paddedRange( 1, 12 );
187	207	$days = CentralNotice::paddedRange( 1, 31 );
188	208
189	209	$fields = array(
190		~~- array( "month", "centralnotice-month", $months, $month ),~~
191		~~- array( "day", "centralnotice-day", $days, $day ),~~
192		~~- array( "year", "centralnotice-year", $years, $year ),~~
	210	+ array( $prefix."_month", "centralnotice-month", $months, $month ),
	211	+ array( $prefix."_day", "centralnotice-day", $days, $day ),
	212	+ array( $prefix."_year", "centralnotice-year", $years, $year ),
193	213	);
194	214
195	215	$out = '';
Index: trunk/extensions/CentralNotice/CentralNoticeLogPager.php
—	—	@@ -28,13 +28,27 @@
29	29	function getQueryInfo() {
30	30	global $wgRequest;
31	31
32		~~- $filterDate = 0;~~
33		~~- $year = $wgRequest->getVal( 'year' );~~
34		~~- $month = $wgRequest->getVal( 'month' );~~
35		~~- $day = $wgRequest->getVal( 'day' );~~
36		~~- if ( $year !== 'other' && $month !== 'other' && $day !== 'other' ) {~~
37		~~- $filterDate = $year . $month . $day;~~
	32	+ $filterStartDate = 0;
	33	+ $filterEndDate = 0;
	34	+ $startYear = $wgRequest->getVal( 'start_year' );
	35	+ if ( $startYear === 'other' ) $startYear = null;
	36	+ $startMonth = $wgRequest->getVal( 'start_month' );
	37	+ if ( $startMonth === 'other' ) $startMonth = null;
	38	+ $startDay = $wgRequest->getVal( 'start_day' );
	39	+ if ( $startDay === 'other' ) $startDay = null;
	40	+ $endYear = $wgRequest->getVal( 'end_year' );
	41	+ if ( $endYear === 'other' ) $endYear = null;
	42	+ $endMonth = $wgRequest->getVal( 'end_month' );
	43	+ if ( $endMonth === 'other' ) $endMonth = null;
	44	+ $endDay = $wgRequest->getVal( 'end_day' );
	45	+ if ( $endDay === 'other' ) $endDay = null;
	46	+
	47	+ if ( $startYear && $startMonth && $startDay ) {
	48	+ $filterStartDate = $startYear . $startMonth . $startDay;
38	49	}
	50	+ if ( $endYear && $endMonth && $endDay ) {
	51	+ $filterEndDate = $endYear . $endMonth . $endDay;
	52	+ }
39	53	$filterCampaign = $wgRequest->getVal( 'campaign' );
40	54	$filterUser = $wgRequest->getVal( 'user' );
41	55	$reset = $wgRequest->getVal( 'centralnoticelogreset' );
—	—	@@ -46,11 +60,14 @@
47	61	);
48	62
49	63	if ( !$reset ) {
50		~~- if ( $filterDate > 0 ) {~~
51		~~- $date1 = intval( $filterDate.'000000' );~~
52		~~- $date2 = intval( ( $filterDate + 1 ).'000000' );~~
53		~~- $info['conds'][] = "notlog_timestamp >= $date1 AND notlog_timestamp < $date2";~~
	64	+ if ( $filterStartDate > 0 ) {
	65	+ $filterStartDate = intval( $filterStartDate.'000000' );
	66	+ $info['conds'][] = "notlog_timestamp >= $filterStartDate";
54	67	}
	68	+ if ( $filterEndDate > 0 ) {
	69	+ $filterEndDate = intval( $filterEndDate.'000000' );
	70	+ $info['conds'][] = "notlog_timestamp < $filterEndDate";
	71	+ }
55	72	if ( $filterCampaign ) {
56	73	$info['conds'][] = "notlog_not_name LIKE '$filterCampaign'";
57	74	}

Follow-up revisions

Revision	Commit summary	Author	Date
r95297	follow-up to r94856, functionalizing date retrieval	kaldari	01:42, 23 August 2011
r95522	copying from trunk r91487, r94856, r94187	awjrichards	21:05, 25 August 2011

Comments

#Comment by Awjrichards (talk | contribs) 19:00, 19 August 2011

Stylistic point - can things like:

+		$startYear = $wgRequest->getVal( 'start_year' );
+		if ( $startYear === 'other' ) $startYear = null;
+		$startMonth = $wgRequest->getVal( 'start_month' );
+		if ( $startMonth === 'other' ) $startMonth = null;
+		$startDay = $wgRequest->getVal( 'start_day' );
+		if ( $startDay === 'other' ) $startDay = null;
+		$endYear = $wgRequest->getVal( 'end_year' );
+		if ( $endYear === 'other' ) $endYear = null;
+		$endMonth = $wgRequest->getVal( 'end_month' );
+		if ( $endMonth === 'other' ) $endMonth = null;
+		$endDay = $wgRequest->getVal( 'end_day' );
+		if ( $endDay === 'other' ) $endDay = null;

be functionalized as this is happening in multiple places? Otherwise looks ok to me.

#Comment by Kaldari (talk | contribs) 01:42, 23 August 2011

Fixed in r95297.

Status & tagging log

20:32, 25 August 2011 Awjrichards (talk | contribs) changed the status of r94856 [removed: new added: ok]
01:42, 23 August 2011 Kaldari (talk | contribs) changed the status of r94856 [removed: fixme added: new]
19:00, 19 August 2011 Awjrichards (talk | contribs) changed the status of r94856 [removed: new added: fixme]