r94677 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r94676‎ \| r94677 \| r94678 >
Date:	19:17, 16 August 2011
Author:	kaldari
Status:	resolved (Comments)
Tags:
Comment:	more work on campaign log filtering
Modified paths:	/trunk/extensions/CentralNotice/CentralNotice.i18n.php (modified) (history) /trunk/extensions/CentralNotice/CentralNoticeLogPager.php (modified) (history) /trunk/extensions/CentralNotice/centralnotice.css (modified) (history) /trunk/extensions/CentralNotice/centralnotice.js (modified) (history) /trunk/extensions/CentralNotice/special/SpecialCentralNoticeLogs.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/special/SpecialCentralNoticeLogs.php
—	—	@@ -47,7 +47,9 @@
48	48	// Begin log selection fieldset
49	49	$htmlOut .= Xml::openElement( 'fieldset', array( 'class' => 'prefsection' ) );
50	50
51		~~- $htmlOut .= Xml::openElement( 'form', array( 'method' => 'post' ) );~~
	51	+ $title = SpecialPage::getTitleFor( 'CentralNoticeLogs' );
	52	+ $actionUrl = $title->getLocalURL();
	53	+ $htmlOut .= Xml::openElement( 'form', array( 'method' => 'get', 'action' => $actionUrl ) );
52	54	$htmlOut .= Xml::element( 'h2', null, wfMsg( 'centralnotice-view-logs' ) );
53	55	$htmlOut .= Xml::openElement( 'div', array( 'id' => 'cn-log-switcher' ) );
54	56	$title = SpecialPage::getTitleFor( 'CentralNoticeLogs' );
—	—	@@ -72,48 +74,97 @@
73	75	$htmlOut .= Xml::closeElement( 'div' );
74	76
75	77	if ( $this->logType == 'campaignsettings' ) {
76		-
	78	+
77	79	$reset = $wgRequest->getVal( 'centralnoticelogreset' );
	80	+ $campaign = $wgRequest->getVal( 'campaign' );
	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;
78	88
79		~~- $startArray = $wgRequest->getArray( 'start' );~~
80		~~- if ( $wgRequest->wasPosted() && $startArray && !$reset ) {~~
81		~~- $filterTimestamp = $startArray['year'] .~~
82		~~- $startArray['month'] .~~
83		~~- $startArray['day'] . '000000'~~
84		~~- ;~~
85		~~- } else { // Default~~
86		~~- $filterTimestamp = -1;~~
	89	+ $htmlOut .= Xml::openElement( 'div', array( 'id' => 'cn-log-filters-container' ) );
	90	+
	91	+ if ( $campaign \|\| $user \|\| $year \|\| $month \|\| $day ) { // filters on
	92	+ $htmlOut .= '<a href="javascript:toggleFilterDisplay()">'.
	93	+ '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/collapsed.png" id="cn-collapsed-filter-arrow" style="display:none;position:relative;top:-2px;"/>'.
	94	+ '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/uncollapsed.png" id="cn-uncollapsed-filter-arrow" style="display:inline-block;position:relative;top:-2px;"/>'.
	95	+ '</a>';
	96	+ $htmlOut .= Xml::tags( 'span', array( 'style' => 'margin-left: 0.3em;' ), wfMsg( 'centralnotice-filters' ) );
	97	+ $htmlOut .= Xml::openElement( 'div', array( 'id' => 'cn-log-filters' ) );
	98	+ } else { // filters off
	99	+ $htmlOut .= '<a href="javascript:toggleFilterDisplay()">'.
	100	+ '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/collapsed.png" id="cn-collapsed-filter-arrow" style="display:inline-block;position:relative;top:-2px;"/>'.
	101	+ '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/uncollapsed.png" id="cn-uncollapsed-filter-arrow" style="display:none;position:relative;top:-2px;"/>'.
	102	+ '</a>';
	103	+ $htmlOut .= Xml::tags( 'span', array( 'style' => 'margin-left: 0.3em;' ), 'Log filters' );
	104	+ $htmlOut .= Xml::openElement( 'div', array( 'id' => 'cn-log-filters', 'style' => 'display:none;' ) );
87	105	}
88	106
89		~~- $filters = Xml::openElement( 'span', array( 'class' => 'cn-log-filter' ) );~~
90		~~- $filters .= Xml::label( wfMsg( 'centralnotice-date' ), 'start[month]', array( 'class' => 'cn-log-filter-label' ) );~~
91		~~- $filters .= CentralNotice::dateSelector( 'start', true, $filterTimestamp );~~
92		~~- $filters .= Xml::closeElement( 'span' );~~
	107	+ $htmlOut .= Xml::openElement( 'table' );
	108	+ $htmlOut .= Xml::openElement( 'tr' );
93	109
94		~~- $filters .= Xml::openElement( 'span', array( 'class' => 'cn-log-filter' ) );~~
95		~~- $filters .= Xml::label( wfMsg( 'centralnotice-notice' ), 'campaign', array( 'class' => 'cn-log-filter-label' ) );~~
96		~~- $filters .= Xml::input( 'campaign', 25, ( $reset ? '' : $wgRequest->getVal( 'campaign' ) ) );~~
97		~~- $filters .= Xml::closeElement( 'span' );~~
	110	+ $htmlOut .= Xml::openElement( 'td' );
	111	+ $htmlOut .= Xml::label( wfMsg( 'centralnotice-date' ), 'month', array( 'class' => 'cn-log-filter-label' ) );
	112	+ $htmlOut .= Xml::closeElement( 'td' );
	113	+ $htmlOut .= Xml::openElement( 'td' );
	114	+ if ( $reset ) {
	115	+ $htmlOut .= $this->dateSelector();
	116	+ } else {
	117	+ $htmlOut .= $this->dateSelector( $year, $month, $day );
	118	+ }
	119	+ $htmlOut .= Xml::closeElement( 'td' );
98	120
99		~~- $filters .= Xml::submitButton( wfMsg( 'centralnotice-apply-filters' ),~~
	121	+ $htmlOut .= Xml::closeElement( 'tr' );
	122	+ $htmlOut .= Xml::openElement( 'tr' );
	123	+
	124	+ $htmlOut .= Xml::openElement( 'td' );
	125	+ $htmlOut .= Xml::label( wfMsg( 'centralnotice-notice' ), 'campaign', array( 'class' => 'cn-log-filter-label' ) );
	126	+ $htmlOut .= Xml::closeElement( 'td' );
	127	+ $htmlOut .= Xml::openElement( 'td' );
	128	+ $htmlOut .= Xml::input( 'campaign', 25, ( $reset ? '' : $campaign ) );
	129	+ $htmlOut .= Xml::closeElement( 'span' );
	130	+ $htmlOut .= Xml::closeElement( 'td' );
	131	+
	132	+ $htmlOut .= Xml::closeElement( 'tr' );
	133	+ $htmlOut .= Xml::openElement( 'tr' );
	134	+
	135	+ $htmlOut .= Xml::openElement( 'td' );
	136	+ $htmlOut .= Xml::label( wfMsg( 'centralnotice-user' ), 'user', array( 'class' => 'cn-log-filter-label' ) );
	137	+ $htmlOut .= Xml::closeElement( 'td' );
	138	+ $htmlOut .= Xml::openElement( 'td' );
	139	+ $htmlOut .= Xml::input( 'user', 25, ( $reset ? '' : $user ) );
	140	+ $htmlOut .= Xml::closeElement( 'span' );
	141	+ $htmlOut .= Xml::closeElement( 'td' );
	142	+
	143	+ $htmlOut .= Xml::closeElement( 'tr' );
	144	+ $htmlOut .= Xml::openElement( 'tr' );
	145	+
	146	+ $htmlOut .= Xml::openElement( 'td', array( 'colspan' => 2 ) );
	147	+ $htmlOut .= Xml::submitButton( wfMsg( 'centralnotice-apply-filters' ),
100	148	array(
101	149	'id' => 'centralnoticesubmit',
102	150	'name' => 'centralnoticesubmit',
103	151	'class' => 'cn-filter-buttons',
104	152	)
105	153	);
106		~~- $filters .= Xml::submitButton( wfMsg( 'centralnotice-clear-filters' ),~~
	154	+ $link = $title->getLinkUrl();
	155	+ $htmlOut .= Xml::submitButton( wfMsg( 'centralnotice-clear-filters' ),
107	156	array(
108	157	'id' => 'centralnoticelogreset',
109	158	'name' => 'centralnoticelogreset',
110	159	'class' => 'cn-filter-buttons',
	160	+ 'onclick' => "window.location = '$link'; return false;",
111	161	)
112	162	);
	163	+ $htmlOut .= Xml::closeElement( 'td' );
113	164
114		~~- $htmlOut .= Xml::fieldset( wfMsg( 'centralnotice-filters' ),~~
115		~~- $filters,~~
116		~~- array( 'class' => 'cn-bannerpreview')~~
117		~~- );~~
	165	+ $htmlOut .= Xml::closeElement( 'tr' );
	166	+ $htmlOut .= Xml::closeElement( 'table' );
	167	+ $htmlOut .= Xml::closeElement( 'div' );
	168	+ $htmlOut .= Xml::closeElement( 'div' );
118	169	}
119	170
120	171	$htmlOut .= Xml::closeElement( 'form' );
—	—	@@ -129,6 +180,28 @@
130	181	$wgOut->addHTML( Xml::closeElement( 'div' ) );
131	182	}
132	183
	184	+ private function dateSelector( $year = 0, $month = 0, $day = 0 ) {
	185	+ $years = range( 2010, 2016 );
	186	+ $months = CentralNotice::paddedRange( 1, 12 );
	187	+ $days = CentralNotice::paddedRange( 1, 31 );
	188	+
	189	+ $fields = array(
	190	+ array( "month", "centralnotice-month", $months, $month ),
	191	+ array( "day", "centralnotice-day", $days, $day ),
	192	+ array( "year", "centralnotice-year", $years, $year ),
	193	+ );
	194	+
	195	+ $out = '';
	196	+ foreach ( $fields as $data ) {
	197	+ list( $field, $label, $set, $current ) = $data;
	198	+ $out .= Xml::listDropDown( $field,
	199	+ CentralNotice::dropDownList( wfMsg( $label ), $set ),
	200	+ '',
	201	+ $current );
	202	+ }
	203	+ return $out;
	204	+ }
	205	+
133	206	/**
134	207	* Show a log of changes.
135	208	* @param $logType string: which type of log to show
Index: trunk/extensions/CentralNotice/centralnotice.css
—	—	@@ -57,9 +57,15 @@
58	58	margin-left:1.6em;
59	59	margin-right:1.6em;
60	60	}
61		~~-#preferences table#cn-campaign-logs {~~
	61	+#preferences div#cn-log-filters-container {
	62	+ margin: 1em 0;
	63	+}
	64	+#preferences table#cn-campaign-logs, #preferences div#cn-log-filters table {
62	65	width: auto;
63	66	}
	67	+#preferences div#cn-log-filters table td {
	68	+ padding: 0.5em;
	69	+}
64	70	#preferences table#cn-campaign-logs td.primary {
65	71	white-space: nowrap;
66	72	background-color: #F0F0F0;
—	—	@@ -73,12 +79,6 @@
74	80	#preferences .cn-log-label {
75	81	font-weight: bold;
76	82	}
77		~~-#preferences .cn-log-filter {~~
78		~~- margin-right: 1.5em;~~
79		-}
80		~~-#preferences .cn-log-filter-label {~~
81		~~- margin-right: 0.5em;~~
82		-}
83	83	#preferences .cn-filter-buttons {
84	84	margin-right: 0.5em;
85	85	}
Index: trunk/extensions/CentralNotice/CentralNotice.i18n.php
—	—	@@ -168,7 +168,7 @@
169	169	'centralnotice-landingpages' => 'Landing pages',
170	170	'centralnotice-banner-content' => 'Banner content',
171	171	'centralnotice-banner-content-changed' => 'Changed',
172		~~- 'centralnotice-filters' => 'Filters',~~
	172	+ 'centralnotice-filters' => 'Log filters',
173	173	'centralnotice-date' => 'Date',
174	174	'centralnotice-apply-filters' => 'Apply filters',
175	175	'centralnotice-clear-filters' => 'Clear filters',
Index: trunk/extensions/CentralNotice/CentralNoticeLogPager.php
—	—	@@ -28,9 +28,15 @@
29	29	function getQueryInfo() {
30	30	global $wgRequest;
31	31
32		~~- $dateArray = $wgRequest->getArray( 'start' );~~
33		~~- $filterDate = $dateArray['year'] . $dateArray['month'] . $dateArray['day'];~~
	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;
	38	+ }
34	39	$filterCampaign = $wgRequest->getVal( 'campaign' );
	40	+ $filterUser = $wgRequest->getVal( 'user' );
35	41	$reset = $wgRequest->getVal( 'centralnoticelogreset' );
36	42
37	43	$info = array(
—	—	@@ -40,14 +46,19 @@
41	47	);
42	48
43	49	if ( !$reset ) {
44		~~- $date1 = intval( $filterDate.'000000' );~~
45		~~- $date2 = intval( ( $filterDate + 1 ).'000000' );~~
46	50	if ( $filterDate > 0 ) {
	51	+ $date1 = intval( $filterDate.'000000' );
	52	+ $date2 = intval( ( $filterDate + 1 ).'000000' );
47	53	$info['conds'][] = "notlog_timestamp >= $date1 AND notlog_timestamp < $date2";
48	54	}
49	55	if ( $filterCampaign ) {
50	56	$info['conds'][] = "notlog_not_name LIKE '$filterCampaign'";
51	57	}
	58	+ if ( $filterUser ) {
	59	+ $user = User::newFromName( $filterUser );
	60	+ $userId = $user->getId();
	61	+ $info['conds'][] = "notlog_user_id = $userId";
	62	+ }
52	63	}
53	64
54	65	return $info;
—	—	@@ -77,9 +88,9 @@
78	89
79	90	$htmlOut .= Xml::openElement( 'td', array( 'valign' => 'top' ) );
80	91	if ( $row->notlog_action !== 'removed' ) {
81		~~- $htmlOut .= '<a href="javascript:toggleDisplay(\''.$row->notlog_id.'\')">'.~~
82		~~- '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/collapsed.png" id="cn-collapsed-'.$row->notlog_id.'" style="display:block;vertical-align:baseline;"/>'.~~
83		~~- '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/uncollapsed.png" id="cn-uncollapsed-'.$row->notlog_id.'" style="display:none;vertical-align:baseline;"/>'.~~
	92	+ $htmlOut .= '<a href="javascript:toggleLogDisplay(\''.$row->notlog_id.'\')">'.
	93	+ '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/collapsed.png" id="cn-collapsed-'.$row->notlog_id.'" style="display:block;"/>'.
	94	+ '<img src="'.$wgExtensionAssetsPath.'/CentralNotice/uncollapsed.png" id="cn-uncollapsed-'.$row->notlog_id.'" style="display:none;"/>'.
84	95	'</a>';
85	96	}
86	97	$htmlOut .= Xml::closeElement( 'td' );
Index: trunk/extensions/CentralNotice/centralnotice.js
—	—	@@ -1,4 +1,4 @@
2		~~-function toggleDisplay( logId ) {~~
	2	+function toggleLogDisplay( logId ) {
3	3	var thisCollapsed = document.getElementById( 'cn-collapsed-'+logId );
4	4	var thisUncollapsed = document.getElementById( 'cn-uncollapsed-'+logId );
5	5	var thisDetails = document.getElementById( 'cn-log-details-'+logId );
—	—	@@ -12,6 +12,20 @@
13	13	thisDetails.style.display = "table-row";
14	14	}
15	15	}
	16	+function toggleFilterDisplay() {
	17	+ var thisCollapsed = document.getElementById( 'cn-collapsed-filter-arrow' );
	18	+ var thisUncollapsed = document.getElementById( 'cn-uncollapsed-filter-arrow' );
	19	+ var thisFilters = document.getElementById( 'cn-log-filters' );
	20	+ if ( thisCollapsed.style.display == "none" ) {
	21	+ thisUncollapsed.style.display = "none";
	22	+ thisCollapsed.style.display = "inline-block";
	23	+ thisFilters.style.display = "none";
	24	+ } else {
	25	+ thisCollapsed.style.display = "none";
	26	+ thisUncollapsed.style.display = "inline-block";
	27	+ thisFilters.style.display = "block";
	28	+ }
	29	+}
16	30	function switchLogs( baseUrl, logType ) {
17	31	encodeURIComponent( logType );
18	32	window.location = baseUrl + '?log=' + logType;

Follow-up revisions

Revision	Commit summary	Author	Date
r95295	follow-up to r94677, functionalizing date range creation	kaldari	01:24, 23 August 2011

Comments

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

iirc,

+		$years = range( 2010, 2016 );
+		$months = CentralNotice::paddedRange( 1, 12 );
+		$days = CentralNotice::paddedRange( 1, 31 );

this happens in other places in CN code. I suggest functionalizing this so if it needs to change you don't need to hunt it down in every place you're doing it.

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

fixed in r95295.

Status & tagging log

01:54, 23 August 2011 Awjrichards (talk | contribs) changed the status of r94677 [removed: new added: resolved]
01:25, 23 August 2011 Kaldari (talk | contribs) changed the status of r94677 [removed: fixme added: new]
23:27, 19 August 2011 Awjrichards (talk | contribs) changed the status of r94677 [removed: new added: fixme]