D6 branch does not obey a number of the code style guidelines. Improve.

I probably won't bother with it in D5.

CommentFileSizeAuthor
#11 uc_payflowpro-7.x.tar_.gz127.33 KBles lim

Comments

kwinters’s picture

kwinters commented
Title: D6 Branch Code Style » Convert to D7 and Improve Code Style and Organization
Category: task » feature
Priority: Minor » Normal
Status: Active » Postponed

Just gonna postpone this out until D7 comes around, when it will need a major rewrite anyway.

mgifford’s picture

mgifford
he/him
Primary language English
commented

any thoughts on the upgrade now that D7 is out and stable?

kwinters’s picture

kwinters commented

Well... my wife is pregnant so I honestly haven't been giving it much thought. :)

There are lots of things that would be nice to do, but I don't have any idea when I could get to them personally.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Hey congrats! Having a kid is wonderful. Always a challenge, but certainly one worth the effort. Best of luck to your wife with the delivery.

So, if we need this we can create a patch & submit it for review and hopefully it won't be too close to the due date. You might also (possibly) be open to having a co-maintainer if someone's up for it.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Quick followup for whoever takes on the task of upgrading this module. The home page for this module states:

"The Drupal 7 branch conversion will likely begin after uc_recurring has a D7 release with a stable API."

I got a response from @univate of uc_recurring http://drupal.org/node/1247770

"I have no intention to change the API for D7. The only thing missing from the D7 version is the CA to rules port."

So the only thing missing to make the upgrade is someone with the itch to make the upgrade happen.

rickmanelius’s picture

rickmanelius commented

Subscribe. I might have that itch :)

rickmanelius’s picture

rickmanelius commented

Are the changes needed from ca to rules similar to the ones found in the patch for ubercart?

http://drupal.org/node/736770
http://drupal.org/files/issues/rules_merge_2.txt

Some parts look almost search/replaceable. Others look like they could take some time...

kwinters’s picture

kwinters commented

I would think you only have to change the CA hooks and such. If you look specifically at the Authorize.net and PayPal portions of the Ubercart changes, you'll have a better idea of what's going on.

rickmanelius’s picture

rickmanelius commented

Thanks kwinters. So in this issue, 2 things are listed: conditional actions to rules port and some code cleanup. For a D7 upgrade, I know the port to rules is mandatory or a release. What about the cleanup? Would it be sufficient to release a dev before or after that considering your available time, etc.

kwinters’s picture

kwinters commented

Some of the really old code is messy and I wanted to clean it up. An issue where I'd have to test everything anyway seemed like a natural time to do it. However, it is inherently optional.

les lim’s picture

les lim
he/they
Primary language English
commented
Status: Postponed » Needs work
StatusFileSize
new uc_payflowpro-7.x.tar_.gz127.33 KB

Here's a start to D7 conversion, based on the latest 6.x-1.x-dev version. Hook implementations have been updated for D7 and some PHP 5.3 compatibility issues were fixed, but none of the Conditional Actions -> Rules conversion has been attempted. Transactions in test mode appear to be working correctly, though.

marameodesign’s picture

marameodesign
she/her
Location Sydney
commented

Hi all,

Is there any news about having a stable D7 version. I'd be happy to sponsor it. Please PM if interested.

Anonymous’s picture

Anonymous (not verified) commented

Any progress on a Drupal 7 version?

Anonymous’s picture

Anonymous (not verified) commented

Quick update - I just tried the dev module in #11 and was pleasantly surprised. The card processes just fine and sets the order to 'Payment Received'. I'm unsure what the 'certificate path' setting is all about, but I just left that default and it all works fine. Will report any problems as they arise.

kwinters’s picture

kwinters commented

@afestein The cert is Paypal's and it should be in the module already. It's used to help prevent a man in the middle or similar attack. There should be instructions in one of the readme files.

If you confirm that it works well enough, I can set up a branch in git. Beyond that I really can't be doing any unpaid work right now, super busy :(

Anonymous’s picture

Anonymous (not verified) commented

Couple of issues cropped up after a week or two of using the dev version above. Firstly the below error displayed on the site:

Warning: Cannot modify header information - headers already sent by (output started at /var/www/vhosts/mysite.com.au/httpdocs/includes/common.inc:2613) in /var/www/vhosts/mysite.com.au/httpdocs/includes/bootstrap.inc on line 1239

Secondly, the following in the error logs:

MESSAGE Recoverable fatal error: Argument 2 passed to db_query() must be an array, integer given, called in /var/www/vhosts/mysite.com.au/httpdocs/sites/all/modules/uc_payflowpro/uc_payflowpro.module on line 1287 and defined in db_query() (line 2313 of /var/www/vhosts/mysite.com.au/httpdocs/includes/database/database.inc).

I'm not sure how serious these are, but if anyone has any input please let me know.

kwinters’s picture

kwinters commented

Do you know what actions triggered the errors? During the order process, etc.

Anonymous’s picture

Anonymous (not verified) commented

This occurs simply when viewing the site. Looking at line 1287 of uc_payflowpro.module I can see that this relates to cron and recurring profiles (the uc_payflowpro_cron function). So I suspect this error occurs on cron run, which is why it didn't crop up until now.

I am not using recurring profiles, but this is something that needs to be addressed. I've just done a quick and dirty hack by changing that argument to an array as follows:

last_status_check IS NULL OR last_status_check < %d", array(time() - 86400));

This probably is not correct for those who want to use recurring profiles, but it works for my purposes.

Jedd Casella’s picture

Jedd Casella commented

Thanks afstein. I was also getting that error.

Here are a few more with some quick and dirty code hacks:

$cc_exp_month and $cc_exp_year were not defined at line 1108 for the // Generate XML for credit card sale

I amended the error with 

  $cc_exp_year = $order->payment_details['cc_exp_year'];
  $cc_exp_month = sprintf('%02d', $order->payment_details['cc_exp_month']

Also I was getting an array to string conversion error on the cart page pointing to line 158 for the "Implementation of hook_uc_cart_pane" function

Line 158 is:
'body' => '<div align="right">'. drupal_get_form('uc_payflowpro_ec_form') .'</div>',

Which appears to be payflowpro express checkout - something which I won't be using - I removed it and the error was gone.

Just thought I would post this here in case anyone else has the same issues for drupal 7 and the uc_payflowpro.module

Great work to all involved in the conversion.

morbus iff’s picture

morbus iff
they/them
commented
Issue summary: View changes

last_status_check IS NULL OR last_status_check < %d", array(time() - 86400));

The correct tweak to this would be:

last_status_check < :time", array(':time' => time() - 86400));

Also:

'body' => '<div align="right">'. drupal_get_form('uc_payflowpro_ec_form') .'</div>',

Which appears to be payflowpro express checkout - something which I won't be using - I removed it and the error was gone.

I don't use recurring either, but the correct tweak would likely be:

'body' => '<div align="right">'. render(drupal_get_form('uc_payflowpro_ec_form')) .'</div>',
Jedd Casella’s picture

Jedd Casella commented

Thanks mate, I'll try your suggestions.

js’s picture

js commented

Hi,

Is this module working and the best choice for PayFlow Pro in D7.

It looks pretty obvious that it is, so I hope you don't mind my asking to be sure.

Thanks, Jerry

kwinters’s picture

kwinters commented

Hi Jerry,

D7 is not officially supported, and I have no ETA on when it will be.

There are unofficial versions in this thread that might work for you.

That said, I'm not aware of any better modules at this point. I don't have a good place to test.

js’s picture

js commented

Thank you , Ken, I appreciate your help.

If we proceed, I will post back our results.

LeT’s picture

LeT commented

Just checking if anyone has got it to work?? I currently have it installed and when proceeding to submit my order. Looks like it is working but then the credit card is declined. I get this error in the UC store pages

Credit card declined for $x.xx with error code 12 (Declined).

The credit card details I put in are valid so don't know what is going on. Whether it is in the code or Paypal Manager settings

js’s picture

js commented

Our client opened an account with Authorize.net :)

Jedd Casella’s picture

Jedd Casella commented

@LeT - That seems to be a paypal issue. We have been using it fine for over a year.

LeT’s picture

LeT commented

Thanks Jedd, I am just waiting on Paypal support to get back to me.

Surf New Media’s picture

Surf New Media commented

Does anyone know what it would take to get this ported officially to D7? I have a client that would like to use this and is willing to pay, but I don't know what the process usually entails or costs. If someone could let me know, I would appreciate it. Thank you.

mgifford’s picture

mgifford
he/him
Primary language English
commented

@Surf New Media - There is rough D7 version that someone has bundled up. It's probably going to be thousands of dollars to look at the D6 code, that contributed D7 code and for someone to come up with a working D7 version that we can proceed with confidence.

I don't know if @kwinters is even still involved in Drupal, but it may be necessary to have a new maintainer selected if you want a release on d.o.

This is all pretty hard to estimate unfortunately. Would be a lot easier if @kwinters were still invovled mind you.

Maybe reach out to http://www.coalmarch.com where he worked...

kwinters’s picture

kwinters commented

Basic payment functionality is probably in a decent place already, using the version earlier in the thread as a base. Could be affordable to get that officially supported.

Recurring payment support is the complicated part. It's complex and hard to test. I also don't know how much of it is already ported.

My personal availability is near zero, but if someone needs a merge or git access then I can take care of that.

The Coalmarch guys would likely be able to help you. But any decent Drupal developer would be able to figure it out.

jenniferannwalsh’s picture

jenniferannwalsh commented

I was able to get this working, without recurring or express checkout, on a 7.44 site that I upgraded from 6. I did make the modifications in #20 and #19 with the exception that I got an error when adding this line:
$cc_exp_month = sprintf('%02d', $order->payment_details['cc_exp_month']

I changed it to:
$cc_exp_month = $order->payment_details['cc_exp_month'];

and php error went away as did log error.

Payment processing worked even with the error.

kwinters’s picture

kwinters commented

@jenniferannwalsh be sure to test that with an expiration month between 1 and 9, in case the leading zero was significant.

Do you have the error message? Comment #19 looks like it is missing the trailing ) and ;