r105916 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r105915‎ | r105916 | r105917 >
Date:19:09, 12 December 2011
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
updating FundraisingStatistics to use summary table
Modified paths:
  • /trunk/extensions/ContributionReporting/FundraiserStatistics_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ContributionReporting/FundraiserStatistics_body.php
@@ -307,7 +307,7 @@
308308 * Retrieve the donation data from the database
309309 *
310310 * @param boolean $mostRecent Is this query for the most recent fundraiser?
311 - * @param string $start The start date for a fundraiser
 311+ * @param string $start The start date for a fundraiser, e.g. 'Oct 22 2007'
312312 * @param string $end The end date for a fundraiser
313313 * @return an array of results or null
314314 */
@@ -325,29 +325,61 @@
326326
327327 // Use database
328328 $dbr = efContributionReportingConnection();
329 - $conditions = array(
330 - 'received >= ' . $dbr->addQuotes( wfTimestamp( TS_UNIX, strtotime( $start ) ) ),
331 - 'received <= ' . $dbr->addQuotes( wfTimestamp( TS_UNIX, strtotime( $end ) + 24 * 60 * 60 ) ),
332 - 'converted_amount >= ' . $egFundraiserStatisticsMinimum,
333 - 'converted_amount <= ' . $egFundraiserStatisticsMaximum
334 - );
 329+ // Set timezone to UTC just in case
 330+ date_default_timezone_set( 'UTC' );
335331
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 - );
 332+ // If the summary table exists, use that, otherwise compute from scratch
 333+ if ( mysql_num_rows( mysql_query( "SHOW TABLES LIKE 'public_reporting_days'" ) ) ) {
 334+
 335+ $conditions = array(
 336+ 'prd_date >= ' . $dbr->addQuotes( wfTimestamp( TS_DB, strtotime( $start ) ) ),
 337+ 'prd_date <= ' . $dbr->addQuotes( wfTimestamp( TS_DB, strtotime( $end ) ) ),
 338+ );
 339+
 340+ // Get the data for a fundraiser
 341+ $select = $dbr->select( 'public_reporting_days',
 342+ array(
 343+ 'prd_date',
 344+ 'prd_total',
 345+ 'prd_number',
 346+ 'prd_average',
 347+ 'prd_maximum',
 348+ ),
 349+ $conditions,
 350+ __METHOD__,
 351+ array(
 352+ 'ORDER BY' => 'prd_date',
 353+ )
 354+ );
 355+
 356+ } else {
 357+
 358+ $conditions = array(
 359+ 'received >= ' . $dbr->addQuotes( wfTimestamp( TS_UNIX, strtotime( $start ) ) ),
 360+ 'received <= ' . $dbr->addQuotes( wfTimestamp( TS_UNIX, strtotime( $end ) + 24 * 60 * 60 ) ),
 361+ 'converted_amount >= ' . $egFundraiserStatisticsMinimum,
 362+ 'converted_amount <= ' . $egFundraiserStatisticsMaximum,
 363+ );
 364+
 365+ // Get the data for a fundraiser
 366+ $select = $dbr->select( 'public_reporting',
 367+ array(
 368+ "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')",
 369+ 'sum(converted_amount)',
 370+ 'count(*)',
 371+ 'avg(converted_amount)',
 372+ 'max(converted_amount)',
 373+ ),
 374+ $conditions,
 375+ __METHOD__,
 376+ array(
 377+ 'ORDER BY' => 'received',
 378+ 'GROUP BY' => "DATE_FORMAT(FROM_UNIXTIME(received),'%Y-%m-%d')"
 379+ )
 380+ );
 381+
 382+ }
 383+
352384 $result = array();
353385 $ytd = 0;
354386 while ( $row = $dbr->fetchRow( $select ) ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r106696MFT r105145, r105454, r105641, r105657, r105659, r105757, r105916, r105965, r...awjrichards20:59, 19 December 2011

Comments

#Comment by Catrope (talk | contribs)   17:27, 14 December 2011
+		date_default_timezone_set( 'UTC' );

Again, this is a hack you shouldn't need. See r105641 CR.

+		if ( mysql_num_rows( mysql_query( "SHOW TABLES LIKE 'public_reporting_days'" ) ) ) {

Never ever ever use mysql_*() functions directly. You could have done this with $res = $dbr->query( "SHOW TABLES ... " ); if ( $res->numRows() ) if you really wanted to, but we've got $dbr->tableExists() so use that.

OK otherwise.

#Comment by Kaldari (talk | contribs)   22:19, 15 December 2011

I've left in the hack for now (as it works and doesn't cause any problems). I have a good idea of how to implement that differently though. The only trick is that in some contexts we want end time minus 24 hours (the date of the end), and in some contexts we want the exact end time (which is actually the beginning of the day after the end day). So what to set in the config isn't obvious if we're asking for a timestamp. I have a good idea for how to address this though.

The mysql function problem is fixed though. Changing status to new since we're hoping to get this out the door today.

#Comment by Awjrichards (talk | contribs)   20:39, 19 December 2011

Yeah, I don't consider the time-handling issue as a deployment blocker. However, it would be trivial to change the date stamps for fundraisers to a format the wfTimestamp likes (http://www.mediawiki.org/wiki/Manual:WfTimestamp) so we don't have to rely on strtotime - we should do this in a future iteration. Marking as resolved.

#Comment by Catrope (talk | contribs)   17:27, 14 December 2011

Hmm, the table existence check was removed in r105965.

#Comment by Kaldari (talk | contribs)   21:42, 14 December 2011

Yeah, I decided that falling back to the old-style queries is a bad idea, so the table check was taken out completely.

Status & tagging log