r59048 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r59047‎ | r59048 | r59049 >
Date:02:00, 14 November 2009
Author:nimishg
Status:resolved (Comments)
Tags:
Comment:
add trailing zeroes for MW_TIMESTAMP format
Modified paths:
  • /trunk/extensions/UsabilityInitiative/ClickTracking/SpecialClickTracking.php (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/ClickTracking/SpecialClickTracking.php
——@@ -503,7 +503,7 @@
504504 * @param maxTime max day (YYYYMMDD)
505505 * NOTE: once some of the constraints have been finalized, this will use more of the Database functions and not raw SQL
506506 */
507 - static function getTimeConstraintsStatement( $minTime, $maxTime ){
 507+ static function getTimeConstraintsStatement( $minTime, $maxTime ){
508508 $minTime = addslashes($minTime);
509509 $maxTime = addslashes($maxTime);
510510 if( $minTime == 0 || $maxTime == 0 ||
——@@ -512,7 +512,9 @@
513513 return '';
514514 }
515515 else {
516 -
 516+ //the dates are stored in the DB as MW_TIMESTAMP formats, add the zeroes to fix that
 517+ $minTime .= "000000";
 518+ $maxTime .= "000000";
517519 return "WHERE `action_time` >= '$minTime' AND `action_time` <= '$maxTime'";
518520
519521 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r59149usability: Merge r59048 (ClickTracking fix) and r59146 (partial; OptIn fix) f...catrope21:34, 16 November 2009
r59204wmf-deployment: Merging usability changes from trunk...catrope18:53, 18 November 2009
r96592Followup to comments on r59048: remove everything related to Special:ClickTra...demon18:31, 8 September 2011

Comments

#Comment by Tim Starling (talk | contribs)   02:31, 18 November 2009

This file is wrong in lots of different ways. I'll have to write a full review.

#Comment by Tim Starling (talk | contribs)   02:56, 18 November 2009

I'm disabling this extension on Wikimedia until the correct SQL escaping functions are used.

#Comment by Tim Starling (talk | contribs)   06:27, 1 December 2009

The SQL is fixed but it still has the hilariously awkward time/date handling noted in my review.

#Comment by Nimish Gautam (talk | contribs)   20:47, 1 December 2009

Everything else from the UI and user_daiy_contribs etc is designed around seeing things per-day. I figured it was easier to keep things YYYYMMDD everywhere for consistency except in tracking the actions at the very bottom of the pipeline.

#Comment by Tim Starling (talk | contribs)   23:26, 1 December 2009

If you want to pass things around in that format, that's fine, but you need to convert to a unix timestamp before you add and subtract intervals, instead of writing code like this: 

// PHP 5.3...hurry!
$plural = ( $increment == 1 ? "" : "s" );
...
$currBeginDate->modify( "+$increment day$plural" );
#Comment by MarkAHershberger (talk | contribs)   02:19, 2 February 2011

This code has moved around a lot but, over a year later, still contains the same bits Tim called out here. I'd like to get it fixed before 1.17, but it looks like it won't be.

#Comment by Nimish Gautam (talk | contribs)   17:31, 17 August 2011

The Special:Clicktracking page has never been used and probably won't be any time in the future. Special:Clicktracking and references to it can be removed from the code entirely.

#Comment by 😂 (talk | contribs)   15:17, 2 September 2011

Can you please delete it then? I was going to, but I wasn't sure if ApiSpecialClickTracking was still used or not.

#Comment by Nimish Gautam (talk | contribs)   20:56, 7 September 2011

ApiSpecialClickTracking is not used either. ApiClickTracking most definitely *is* being used tho

#Comment by 😂 (talk | contribs)   18:36, 8 September 2011

Everything related to this special page was removed in r96592. Marking resolved.

Status & tagging log