r105145 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105144‎ | r105145 | r105146 >
Date:02:12, 5 December 2011
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
rewriting FundraisingStatistics to use 1 query instead of 6, and heavily cache results from previous years
Modified paths:
  • /trunk/extensions/ContributionReporting/ContributionReporting.php (modified) (history)
  • /trunk/extensions/ContributionReporting/FundraiserStatistics_body.php (modified) (history)
  • /trunk/extensions/ContributionReporting/modules/ext.fundraiserstatistics.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ContributionReporting/ContributionReporting.php
@@ -115,6 +115,10 @@
116116 ),
117117 );
118118
 119+// The first year of statistics to make visible by default.
 120+// We normally don't show all of them by default, since it makes the chart extremely wide.
 121+$egFundraiserStatisticsFirstYearDefault = 2009;
 122+
119123 // Thesholds for fundraiser statistics
120124 $egFundraiserStatisticsMinimum = 1;
121125 $egFundraiserStatisticsMaximum = 10000;
Index: trunk/extensions/ContributionReporting/FundraiserStatistics_body.php
@@ -1,6 +1,7 @@
22 <?php
33 /**
44 * Special Page for Contribution statistics extension
 5+ * This page displays charts and tables related to donation statistics.
56 *
67 * @file
78 * @ingroup Extensions
@@ -15,15 +16,22 @@
1617 }
1718
1819 public function execute( $sub ) {
19 - global $wgRequest, $wgOut, $wgLang, $wgScriptPath, $egFundraiserStatisticsFundraisers;
 20+ global $wgRequest, $wgOut, $wgLang, $wgScriptPath, $egFundraiserStatisticsFundraisers,
 21+ $egFundraiserStatisticsFirstYearDefault;
2022
 23+ // Figure out what years to display initially
2124 $showYear = array();
2225 foreach ( $egFundraiserStatisticsFundraisers as $fundraiser ) {
23 - if ( $wgRequest->wasPosted() ) {
24 - $showYear[$fundraiser['id']] = $wgRequest->getCheck( 'toogle'.$fundraiser['id'] );
 26+ // You can override the years to display by default via the query string.
 27+ // For example, Special:FundraiserStatistics?2007=show&2010=hide
 28+ $yearOverride = $wgRequest->getVal( $fundraiser['id'] );
 29+ if ( $yearOverride === "hide" ) {
 30+ $showYear[$fundraiser['id']] = false;
 31+ } else if ( $yearOverride === "show" ) {
 32+ $showYear[$fundraiser['id']] = true;
2533 } else {
26 - // By default, show only the fundraising years after 2008
27 - if ( intval( $fundraiser['id'] ) > 2008 ) {
 34+ // By default, show only the recent fundraising years
 35+ if ( intval( $fundraiser['id'] ) >= $egFundraiserStatisticsFirstYearDefault ) {
2836 $showYear[$fundraiser['id']] = true;
2937 } else {
3038 $showYear[$fundraiser['id']] = false;
@@ -37,7 +45,6 @@
3846 'totals' => array(
3947 'data' => array(),
4048 'index' => 1,
41 - 'query' => 'dailyTotalMax',
4249 'precision' => 2,
4350 'label' => 'fundraiserstats-total',
4451 'max' => 1,
@@ -45,7 +52,6 @@
4653 'contributions' => array(
4754 'data' => array(),
4855 'index' => 2,
49 - 'query' => 'contributionsMax',
5056 'precision' => 0,
5157 'label' => 'fundraiserstats-contributions',
5258 'max' => 1,
@@ -53,7 +59,6 @@
5460 'averages' => array(
5561 'data' => array(),
5662 'index' => 3,
57 - 'query' => 'averagesMax',
5863 'precision' => 2,
5964 'label' => 'fundraiserstats-avg',
6065 'max' => 1,
@@ -61,7 +66,6 @@
6267 'maximums' => array(
6368 'data' => array(),
6469 'index' => 4,
65 - 'query' => 'maximumsMax',
6670 'precision' => 2,
6771 'label' => 'fundraiserstats-max',
6872 'max' => 1,
@@ -69,7 +73,6 @@
7074 'ytd' => array(
7175 'data' => array(),
7276 'index' => 5,
73 - 'query' => 'yearlyTotalMax',
7477 'precision' => 2,
7578 'label' => 'fundraiserstats-ytd',
7679 'max' => 1,
@@ -85,10 +88,28 @@
8689
8790 $wgOut->addWikiMsg( 'contribstats-header' ); // Header (typically empty)
8891
 92+ // The $fundraisingData array contains all of the fundraising data in the following scheme:
 93+ // $fundraisingData[<fundraiserId>][<dayIndex>][<dataTypeIndex>]
 94+ $fundraisingData = array();
 95+
 96+ foreach ( $egFundraiserStatisticsFundraisers as $fundraiserIndex => $fundraiser ) {
 97+
 98+ // See if this is the most recent fundraiser or not
 99+ if ( $fundraiserIndex == count( $egFundraiserStatisticsFundraisers ) - 1 ) {
 100+ $mostRecent = true;
 101+ } else {
 102+ $mostRecent = false;
 103+ }
 104+
 105+ // Collect all the data
 106+ $fundraisingData[$fundraiser['id']] = $this->query( $mostRecent, $fundraiser['start'], $fundraiser['end'] );
 107+ }
 108+
89109 // Chart maximums
90 - foreach ( $egFundraiserStatisticsFundraisers as $fundraiser ) {
 110+ // We cycle through all the fundraisers with all the different chart types and collect the maximums.
 111+ foreach ( $fundraisingData as $fundraiser ) {
91112 foreach ( $charts as $name => $chart ) {
92 - $chartMax = $this->query( $charts[$name]['query'], $fundraiser['start'], $fundraiser['end'] );
 113+ $chartMax = $this->getMaximum( $fundraiser, $chart['index'] );
93114 if ( $chartMax > $charts[$name]['max'] ) {
94115 $charts[$name]['max'] = $chartMax;
95116 }
@@ -108,7 +129,7 @@
109130 foreach ( $egFundraiserStatisticsFundraisers as $fundraiserIndex => $fundraiser ) {
110131
111132 // Get all the daily data for a particular fundraiser
112 - $days = $this->query( 'dailyTotals', $fundraiser['start'], $fundraiser['end'] );
 133+ $days = $fundraisingData[$fundraiser['id']];
113134
114135 // See if this is the most recent fundraiser or not
115136 if ( $fundraiserIndex == count( $egFundraiserStatisticsFundraisers ) - 1 ) {
@@ -137,7 +158,7 @@
138159
139160 $height = $chart['factor'] * $day[$chart['index']];
140161 $style = "height:{$height}px;";
141 - if ( $showYear[$fundraiser['id']] !== true ) {
 162+ if ( !$showYear[$fundraiser['id']] ) {
142163 $style .= "display:none;";
143164 }
144165 $attributes = array(
@@ -212,8 +233,8 @@
213234
214235 $years = wfMsg( 'fundraiserstats-show-years' ).'<br/>';
215236 foreach ( $egFundraiserStatisticsFundraisers as $fundraiser ) {
216 - $years .= Xml::check( 'toogle'.$fundraiser['id'], $showYear[$fundraiser['id']], array( 'id' => 'bar-'.$fundraiser['id'], 'class' => 'yeartoggle' ) );
217 - $years .= Xml::label( $fundraiser['id'], 'toogle'.$fundraiser['id'] );
 237+ $years .= Xml::check( 'toggle'.$fundraiser['id'], $showYear[$fundraiser['id']], array( 'id' => 'bar-'.$fundraiser['id'], 'class' => 'yeartoggle' ) );
 238+ $years .= Xml::label( $fundraiser['id'], 'toggle'.$fundraiser['id'] );
218239 $years .= "<br/>";
219240 }
220241 $wgOut->addHTML( Xml::openElement( 'div', array( 'id' => 'configholder' ) ) );
@@ -285,16 +306,16 @@
286307 /**
287308 * Retrieve the donation data from the database
288309 *
289 - * @param string $type Which type of query to do
 310+ * @param boolean $mostRecent Is this query for the most recent fundraiser?
290311 * @param string $start The start date for a fundraiser
291312 * @param string $end The end date for a fundraiser
292313 * @return an array of results or null
293314 */
294 - private function query( $type, $start, $end ) {
 315+ private function query( $mostRecent, $start, $end ) {
295316 global $wgMemc, $egFundraiserStatisticsMinimum, $egFundraiserStatisticsMaximum, $egFundraiserStatisticsCacheTimeout;
296317
297318 // Conctruct the key for memcached
298 - $key = wfMemcKey( 'fundraiserstatistics', $type, $start, $end );
 319+ $key = wfMemcKey( 'fundraiserstatistics', $start, $end );
299320
300321 // If result exists in memcached, use that
301322 $cache = $wgMemc->get( $key );
@@ -310,88 +331,60 @@
311332 'converted_amount >= ' . $egFundraiserStatisticsMinimum,
312333 'converted_amount <= ' . $egFundraiserStatisticsMaximum
313334 );
314 - switch ( $type ) {
315 - case 'dailyTotals':
316 - $select = $dbr->select( 'public_reporting',
317 - array(
318 - "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')",
319 - 'sum(converted_amount)',
320 - 'count(*)',
321 - 'avg(converted_amount)',
322 - 'max(converted_amount)',
323 - ),
324 - $conditions,
325 - __METHOD__ . '-dailyTotals',
326 - array(
327 - 'ORDER BY' => 'received',
328 - 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
329 - )
330 - );
331 - $result = array();
332 - $ytd = 0;
333 - while ( $row = $dbr->fetchRow( $select ) ) {
334 - // Insert the year-to-date amount as a record in the row (existing $ytd + sum)
335 - $row[5] = $ytd += $row[1];
336 - $result[] = $row;
337 - }
338 - break;
339 - case 'dailyTotalMax':
340 - $result = $dbr->selectField( 'public_reporting',
341 - array( 'sum(converted_amount) as sum' ),
342 - $conditions,
343 - __METHOD__ . '-dailyTotalMax',
344 - array(
345 - 'ORDER BY' => 'sum DESC',
346 - 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
347 - )
348 - );
349 - break;
350 - case 'yearlyTotalMax':
351 - $result = $dbr->selectField( 'public_reporting',
352 - array( 'sum(converted_amount) as sum' ),
353 - $conditions,
354 - __METHOD__ . '-yearlyTotalMax'
355 - );
356 - break;
357 - case 'contributionsMax':
358 - $result = $dbr->selectField( 'public_reporting',
359 - array( 'count(converted_amount) as sum' ),
360 - $conditions,
361 - __METHOD__ . '-contributionsMax',
362 - array(
363 - 'ORDER BY' => 'sum DESC',
364 - 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
365 - )
366 - );
367 - break;
368 - case 'averagesMax':
369 - $result = $dbr->selectField( 'public_reporting',
370 - array( 'avg(converted_amount) as sum' ),
371 - $conditions,
372 - __METHOD__ . '-averagesMax',
373 - array(
374 - 'ORDER BY' => 'sum DESC',
375 - 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
376 - )
377 - );
378 - break;
379 - case 'maximumsMax':
380 - $result = $dbr->selectField( 'public_reporting',
381 - array( 'max(converted_amount) as sum' ),
382 - $conditions,
383 - __METHOD__ . '-maximumsMax',
384 - array(
385 - 'ORDER BY' => 'sum DESC',
386 - 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
387 - )
388 - );
389 - break;
 335+
 336+ // Get the data for a fundraiser
 337+ $select = $dbr->select( 'public_reporting',
 338+ array(
 339+ "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')",
 340+ 'sum(converted_amount)',
 341+ 'count(*)',
 342+ 'avg(converted_amount)',
 343+ 'max(converted_amount)',
 344+ ),
 345+ $conditions,
 346+ __METHOD__,
 347+ array(
 348+ 'ORDER BY' => 'received',
 349+ 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
 350+ )
 351+ );
 352+ $result = array();
 353+ $ytd = 0;
 354+ while ( $row = $dbr->fetchRow( $select ) ) {
 355+ // Insert the year-to-date amount as a record in the row (existing $ytd + sum)
 356+ $row[5] = $ytd += $row[1];
 357+ $result[] = $row;
390358 }
 359+
391360 if ( isset( $result ) ) {
392 - // Store the result in memcached
393 - $wgMemc->set( $key, $result, $egFundraiserStatisticsCacheTimeout );
 361+ // Store the result in memcached.
 362+ // If it's the most recent fundraiser, cache for a short period of time, otherwise
 363+ // cache for 24 hours (since the query is expensive).
 364+ if ( $mostRecent ) {
 365+ $wgMemc->set( $key, $result, $egFundraiserStatisticsCacheTimeout );
 366+ } else {
 367+ #$wgMemc->set( $key, $result, 86400 );
 368+ }
394369 return $result;
395370 }
396371 return null;
397372 }
 373+
 374+ /**
 375+ * Given a particular index, find the maximum for all values in a 2D array with that particular
 376+ * index as the 2nd key.
 377+ *
 378+ * @param array $fundraiserDays A 2D array of daily data for a particular fundraiser
 379+ * @param integer $comparisonIndex The index to find the maximum for
 380+ * @return an integer
 381+ */
 382+ private function getMaximum( $fundraiserDays, $comparisonIndex ) {
 383+ $max = 0;
 384+ foreach( $fundraiserDays as $day ) {
 385+ if ( $day[$comparisonIndex] > $max ) {
 386+ $max = $day[$comparisonIndex];
 387+ }
 388+ }
 389+ return $max;
 390+ }
398391 }
Index: trunk/extensions/ContributionReporting/modules/ext.fundraiserstatistics.js
@@ -33,9 +33,11 @@
3434 $j( '.fundraiserstats-current' ).each( function() {
3535 replaceView( $j(this).attr( 'rel' ) )
3636 } );
 37+ // When someone clicks on a year, hide or show that year in the chart
3738 $j( '#configholder .yeartoggle' ).click( function() {
3839 $j('.fundraiserstats-'+$j(this).attr( 'id' )).toggle();
3940 } );
 41+ // When someone clicks on Customize, display pop-up menu and change arrow icon.
4042 $j( '#configtoggle' ).click( function() {
4143 $j('#configholder').toggle();
4244 if ($j( '#configtoggle a' ).css( 'background-position' ) == '0px -18px') {
@@ -44,8 +46,5 @@
4547 $j( '#configtoggle a' ).css( 'background-position','0px -18px' );
4648 }
4749 } );
48 - $j( '#timezone' ).change( function() {
49 - $j('#configform').submit();
50 - } );
5150
5251 } );

Follow-up revisions

RevisionCommit summaryAuthorDate
r105214uncommenting cacheingkaldari20:11, 5 December 2011
r105485MFT r105051, r105064, r105145, r105214, r105353, r105454awjrichards23:27, 7 December 2011
r106696MFT r105145, r105454, r105641, r105657, r105659, r105757, r105916, r105965, r...awjrichards20:59, 19 December 2011

Comments

#Comment by Awjrichards (talk | contribs)   21:03, 6 December 2011
+			if ( $mostRecent ) {
+				$wgMemc->set( $key, $result, $egFundraiserStatisticsCacheTimeout );
+			} else {
+				#$wgMemc->set( $key, $result, 86400 );
+			}

It would be great to have both cache timeouts be configurable. I don't see why we can't potentially cache other results for even longer than 24 hours.

#Comment by Awjrichards (talk | contribs)   21:06, 6 December 2011

in fact... it would be slick to make the cache timeouts even more granular. For instance, years-old fundraisers will almost never have changing data. However, current fundraising data my change ex-post-facto. So perhaps having this year's non-most-recent data cache for 24 hours and everything else cache for even longer.

Status & tagging log