r75397 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r75396‎ | r75397 | r75398 >
Date:23:39, 25 October 2010
Author:awjrichards
Status:deferred (Comments)
Tags:
Comment:
Fixed broken checking for OWA definition; Updated evaluation of when updatecontirbutiontracking runs so that it actually works like it's supposed to
Modified paths:
  • /branches/fundraising/OWA/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php (modified) (history)

Diff [purge]

Index: branches/fundraising/OWA/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php
——@@ -390,7 +390,7 @@
391391
392392 // update contribution tracking
393393 // only forced when OWA is defined
394 - $this->updateContributionTracking( $data, defined(OWA) );
 394+ $this->updateContributionTracking( $data, defined( 'OWA' ) );
395395
396396 // create payflow query string, include string lengths
397397 $queryArray = array(
——@@ -969,7 +969,7 @@
970970 if( $owa_ref != null && !is_numeric( $owa_ref )){
971971 $owa_ref = $this->get_owa_ref_id( $owa_ref );
972972 }
973 -
 973+ echo $ow
974974 // if we're in testing mode and an action hasn't yet be specified, prepopulate the form
975975 if ( !$wgRequest->getText( 'action', false ) && !$numAttempt && $wgPayflowGatewayTest ) {
976976 // define arrays of cc's and cc #s for random selection
——@@ -1156,8 +1156,10 @@
11571157 public function updateContributionTracking( &$data, $force=false ) {
11581158 // ony update contrib tracking if we're coming from a single-step landing page
11591159 // which we know with cc# in utm_source or if force=true
1160 - if ( !$force || !preg_match( "/cc[0-9]/", $data[ 'utm_source' ] )) {
1161 - return;
 1160+ if ( $force === false ) {
 1161+ if ( !preg_match( "/cc[0-9]/", $data[ 'utm_source' ] )) {
 1162+ return;
 1163+ }
11621164 }
11631165
11641166

Comments

#Comment by Nikerabbit (talk | contribs)   11:37, 26 October 2010

Echo removed in r75398.

Not actually related to this commit: 

if( $owa_ref != null  && !is_numeric( $owa_ref )){

Two spaces after null. Is is_numeric really what you want here? It accepts all kinds of stuff besides integers and floats. Missing spaces in )){ 

+if ( !preg_match( "/cc[0-9]/", $data[ 'utm_source' ] )) {

Array indexes are normally spaced like $data['utm_source'].

#Comment by Awjrichards (talk | contribs)   16:06, 26 October 2010

Woops, thanks for taking out that echo, that's what I get for too hastily committing.

Nimish actually wrote the if( ... is_numeric()) line, I'll dbl check with him about it. $owa_ref should either be a url or an integer - in the event that it's NOT an integer (which is the ID related to the URL pulled from the database), then we check the database to see if there's already an ID associated with the given URL. What do you think would be a better check to see if $owa_ref is a URL?

As for the spacing, that's my bad - I always have a hard time breaking convention from previous projects I've worked on. Personally, I really like the extra spaces around the array index, I find i tmuch more readable, same with the )) { rather than ) ) {, but I realize that's not Mediawiki convention. I'll work on that :)

#Comment by Nikerabbit (talk | contribs)   17:30, 26 October 2010

You took the echo out yourself, I was just documenting it for other readers :)

#Comment by Awjrichards (talk | contribs)   17:35, 26 October 2010

hahaha wow - the fundraiser's already got my mind whirling. thanks again :D

Status & tagging log