Testing with Drupal 4.7b5 & Store.module version $Id: store.module,v 1.80 2006/03/04 00:34:55 nedjo Exp $

User makes it through checkout process beautifully, goes through paypal payment screen, correctly returns to return url... BUT...

at return url (in this test case, store/history/3) the following error message presents:

user warning: You have an error in your SQL syntax. Check the manual that corresponds to your MySQL server version for the right syntax to use near '0, payment_method, uid, shipping_firstname, shipping_lastname, query: INSERT INTO ec_transaction (shipping_cost, gross, 0, payment_method, uid, shipping_firstname, shipping_lastname, shipping_street1, shipping_street2, shipping_zip, shipping_city, shipping_state, shipping_country, billing_firstname, billing_lastname, billing_street1, billing_street2, billing_zip, billing_city, billing_state, billing_country, changed, created, payment_status, workflow, txnid) VALUES('0', '1', 'Continue', 'paypal', '3', 'Joe', 'User', '123 Fake Street', '', '99999', 'Smalltownville', 'TX', 'us', 'Joe', 'User', '123 Fake Street', '', '99999', 'Smalltownville', 'TX', 'us', '1141456767 in /home/xaccountx/public_html/drupaldir/includes/database.mysql.inc on line 120.

the IPN hit from paypal follows, but the transaction has never been saved... no transaction history exists.

Note: the tables ec_transaction and ec_transaction_product are still not automatically created on install of the ecommerce module. I used the mysql dump data in ecommerce.mysql in the ecommerce/extra folder (ecommerce cvs-3-4-06). Is that the most correct and up-to-date table structure for those tables?

CommentFileSizeAuthor
#21 store_4-6_in_array.patch2.11 KBneclimdul
#19 store_in_array_0.patch2.82 KBneclimdul
#17 0items.png13.44 KBguckie
#14 store_in_array.patch2.76 KBneclimdul
#8 subcheckout.png54.88 KBguckie

Comments

neclimdul’s picture

neclimdul
He/Him
commented

I would venture to guess that the problem is there isn't 0 field in the paypal table. I'm not sure what's going on here though our how that queries being built though.

gordon’s picture

gordon commented

What it is, is that when the paypal_ec_transactionapi() is called for the load, it is not returning null (as there is no return) and for some reason this is being treated as true, and the in store_transaction_load() it is then doing a foreach (null as $key => $value) and inserting the 0 => '' into the transaction.

Then what is happening in the store_transaction_save() is that in_array(0, array('field a', 'field b', 'field c') is returning true so we get the set 0 = '' in the sql.

I can fix it quickly, but from the code this should not happen, so why is it?

Once I work this out then I will fix it.

guckie’s picture

guckie commented

Oh, good eye!

I think I stayed up looking at this way past my blurry-eyed stage..

Today I've been trying to deduce what changes have occurred since my last working cvs version and this one I pulled down on 3-4. Also looked at core includes/database.mysql.inc, since line 120 is referenced in the error message I got.

This is definitely beyond my capabilities to debug... but if I can state a few basic observations, maybe it can help someone locate the issue...

INSERT INTO ec_transaction (shipping_cost, gross, 0,
VALUES('0', '1', 'Continue',

The query (as built - somewhere?) is trying to slip a value 'Continue' into field '0' (which, as you pointed out, doesn't exist in the table).
Is it possible the 'Continue' value is somehow being derived from the 'Continue' button on either the *cart/checkout* or *cart/checkout/1* screen
during the checkout process in my tests?

I compared (just with my eyeballs, less blurry today) all the 'INSERT INTO ec_transaction' statements (and related query-build code) I could find in store.module. I didn't find anything that had changed since my test site last worked.

I compared database.mysql.inc (from my last working installation to this one) around line 120, shown below. (The only difference is that "TYPE=MyISAM" has been removed from line 119 in the drupal 4.7b5 version).

CREATE TABLE authmap (
  aid int(10) unsigned NOT NULL auto_increment,
  uid int(10) NOT NULL default '0',
  authname varchar(128) NOT NULL default '',
  module varchar(128) NOT NULL default '',
  PRIMARY KEY (aid),
  UNIQUE KEY authname (authname)
) TYPE=MyISAM
/*!40100 DEFAULT CHARACTER SET utf8 */ ;

I don't know if any of these observations will be helpful or not. I'm sure there's plenty I'm missing.

*shrug*

neclimdul’s picture

neclimdul
He/Him
commented

for reference 'TYPE=MyISAM' sets the way the database is stored by mysql on your hard drive. There are some limitations and advantages to this type but it won't have any affect on this.

It's almost certainly a code issue. And you are right, the continue submit button is added and it referenced by 0. It does make its way into the transaction variable as well since we merge all the fields in. But this shouldn't be changed from previous behavior so I'm not sure why the problem is poping up. I'm looking at it.

gordon’s picture

gordon commented

It is nothing to do with the database, it is the store_transaction_load() which is adding the field 0 into the array when it should not be.

guckie’s picture

guckie commented

Thank you, guys! It's great to know there are capable hands/minds on this! Maybe someday I'll have better debugging skills than my current "wonder-out-loud" method. =)

Until then...

I'm just adding a note to this thread that the Case Tracker module is spitting out a reference to the same line of database.mysql.inc. Not that it means anything yet, but if a pattern emerges across modules, then maybe a closer look at recent core changes may be in order.. I guess?

neclimdul’s picture

neclimdul
He/Him
commented

Yeah it would show up on the same line in database.mysql. its not that lines fault though. Its like if I told you to go fix the the paypal.module and gave you the store.module file. You'd throw and error but the problem would have been that I have you bad data. Same things happening here.

BTW, I spent a good amount of time trying to reproduce this and wasn't able to. Does anyone have any hints? could this be the effect of a module thats not up to date?

guckie’s picture

guckie commented
StatusFileSize
new subcheckout.png54.88 KB

I've tried everything I can think of (at this point) to *not* replicate this error! LOL

I freshly downloaded & installed drupal & e-commerce tarballs as of this afternoon.

Have been testing with a clean drupal installation with only poormanscron & the *required* e-commerce modules activated. 'Generic' is the only product type I'm working with. Paypal is the only payment option I'm working with.

My objective (at this point) is to sell a subscription (submanage) to a generic product, make it through the paypal payment process, and see the transaction saved to store history. Additionally, it would be nice if the associated role inheritance could take place at the appropriate time, but I realize that's a separate issue.

I'm attaching a screenshot of event messages relating to the above process. I've added a few notes to the screenshot in yellow text on black background.

I'm just as interested as many other folks are in seeing as many ecommerce components working with drupal 4.7 as possible... and am trying to be helpful (with my limited skills)... but if anyone feels my reports are interfering with progress, please don't hesitate to say so.

And IF interested in debugging on my test setup, gordon or neclimdul, I would gladly give you ftp access, (cpanel, if you want it) and my drupal admin password - just hit me up on my contact form. Whatever it is that's so special about my installation... it replicates this error all day long.

I know you guys are dreadfully busy. Please don't mistake this as a plea for personal debugging time - it's just an offer I hope you'll only take me up on if you believe it may help plenty of other folks with same or similar circumstances.

neclimdul’s picture

neclimdul
He/Him
commented

If its happening to you it could happen to someone else(or could be). Like gordon said in another issue, the e-commerce module aims to be compatible with any install that will run drupals. So with that in mind this is something that should be tracked down. I'll see if I've got time this week to get in touch with you and we'll see what we can do. :)

wpd’s picture

wpd commented

guckie-

The minor error mentioned at the bottom of you attached screenshot is fixed here:
http://drupal.org/node/47240
(The extra s's).

Not that helps you fix your big problem...

guckie’s picture

guckie commented

@neclimdul: Thank you very much!

@wpd: Thank you, too! Hey, every bug fixed is a step toward a(nother) drupal 4.7 production site using e-commerce! =)

neclimdul’s picture

neclimdul
He/Him
commented

I can finally confirm this behavior! it seems to be a php4 issue as it only turned up when I setup a php4 cgi to test it. So, its kinda important we sort it out since a lot of people are going to use php4.

gordon’s picture

gordon commented

Actually I also get this issue in php5.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Active » Needs review
StatusFileSize
new store_in_array.patch2.76 KB

Finally tracked this down. I'm not sure where the problem gets introduced but its a 'gotcha' of the in_array function as seen in a comment on the man page

I have no idea why this wasn't showing up in my sandbox but I can guess that for what ever reason my in_array function was doing strict testing.

So I'm going to rewrite the functions using this to only check for values known to be in the table. I could use is_array as strict but this would involve more comparisons at O(n) versus fewer comparisons at O(1)*. Attached is a patch rewritting all the foreach loops using in_array(even the ones not currently causing a problem) to work in this manner.

* For those without a lot of CS background the O notation is a effeciency measurment for an algorythm. O(n) means it will take an amount of time directly proportional to the dataset and O(1) means it will have a constant rate. In the this case our constant rate will always be cheaper so it is a good choice. Not much because our datasets small but it will be faster. Also, since it is in a foreach loop everything gets amplified in a sense so the foreach loops are actually O(n^2) and O(n). This means the old algorythm grew exponentially with the size of the data which is something you always want to avoided if possible. There are some benchmarks for this on the in_array man page.

guckie’s picture

guckie commented

Splendid! Thanks, neclimdul! Indeed this patch clears up the insertion error in my environment as well.

* For those without a lot of CS background...

*raises hand*

(and thanks for the terrific explanation - love it when I can get a handle on a bit of expert knowledge!)

gordon’s picture

gordon commented

There is also the flip side to this.

The array(0 => false) is getting adding by the store_transaction_load() when it calls the hook_ec_transactionapi('load').

<?php
  if ($extra = store_invoke_ec_transactionapi($t, 'load')) {
    foreach ($extra as $key => $value) {
      $t->$key = $value;
    }
  }
?>

Now in the paypay_ec_tranactionapi() it is not returning anything, eg NULL, and it is still going on to do the foreach().

This needs to be fixed as well.

Also we need to roll a patch for 4.6, but I can do this once we have bedded down this.

Gordon.

guckie’s picture

guckie commented
StatusFileSize
new 0items.png13.44 KB

Don't know if this error is symptomatic of this issue or something completely irrelevant, but I get this when I attempt to edit a transaction in 'administer > store > transactions'

Fatal error: Call to undefined function: payment_get_modulename() in /home/xaccountx/public_html/drupaldir/modules/ecommerce/store/store.inc on line 236

Also, the 'items ordered' column on the admin screen is reflecting '0 items' (screenshot attached) and the subscription (corresponding to a completed transaction) does not get created.

neclimdul’s picture

neclimdul
He/Him
commented

The payment_get_module problem sounds like you applied some of the patches dealing with http://drupal.org/node/51852 . It was fixed and that function removed on the 3rd. Makes sure you have a clean version of store.module from CVS and then apply this patch.

As for the item issue, your right. I seem to have this problem too. I'll look into it.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new store_in_array_0.patch2.82 KB

Item problem was a copy paste error in my patch. Here's a corrected patch against the latest cvs(1.82)

gordon’s picture

gordon commented
Version: master » 4.6.x-1.x-dev

Thanks, I have commited this to cvs, Would you be able to roll a patch for 4.6.

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new store_4-6_in_array.patch2.11 KB

Not to sure but I think this is right. I'm not good at working with branches. The checkout I did shows the function to work the same though so it was just a copy paste thing.

guckie’s picture

guckie commented

neclimdul, you were right about my payment_get_module problem. Silly me - I had managed to freshen up the store.module but left store.inc out of date. All is well with that now. Thanks!

And, the store_in_array_0.patch worked beautifully! Thanks for that!

This is starting to get exciting! I think once this half-an-issue can be cleared up, I'll be confident enough to test the waters with subscription sales on a live site.

Thanks to all who are working so hard to get this complex beast of a module package ported over for 4.7. Much appreciated!

gordon’s picture

gordon commented

Thanks, I have commited this to cvs.

gordon’s picture

gordon commented
Status: Needs review » Fixed
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)