serializeOrder() doesn't include shipping quote method (patch included) [#1157494]

Hi,

I have some taxes that depend upon the chosen shipping quote method. Currently serializeOrder() doesn't include that information in the order, which causes uc_quote_condition_order_shipping_method() to try to infer the quote method from the name of the line item. In my case I had methods with identical names and was getting bugs. I'm about to attach a patch that adds the method's machine name to the order. I haven't tested it with anything other than uc_taxes.

Thanks

Comment	File	Size	Author
#9	1157494-uc_payment-shipping-method.patch	1.1 KB	longwave
#9
#1	uc_payment_serializeorder_quote_method-1157494-1.patch	1.12 KB	AndyF
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

AndyF CreditAttribution: AndyF commented 14 May 2011 at 11:59

Status:

Active

» Needs review

File	Size
uc_payment_serializeorder_quote_method-1157494-1.patch	1.12 KB

Comment #2

14 May 2011 at 12:12

Status:

Needs review

» Needs work

The last submitted patch, uc_payment_serializeorder_quote_method-1157494-1.patch, failed testing.

Comment #3

AndyF CreditAttribution: AndyF commented 27 May 2011 at 12:46

Status:

Needs work

» Needs review

Think it failed due to #1096900: Releases should not be tested.

Comment #4

longwave

he/him

English

CreditAttribution: longwave commented 27 May 2011 at 12:56

Version:

6.x-2.4

» 6.x-2.x-dev

It is much easier for the maintainers to accept patches against -dev if possible.

Comment #5

AndyF CreditAttribution: AndyF commented 27 May 2011 at 14:57

Sorry, yep, I did check that there were no differences between the dev file and 2.4's, but must've been on autopilot when setting the version on this issue! I haven't tested on dev however.

Comment #6

AndyF CreditAttribution: AndyF commented 31 May 2011 at 08:18

#1: uc_payment_serializeorder_quote_method-1157494-1.patch queued for re-testing.

Comment #7

sin CreditAttribution: sin commented 14 July 2011 at 10:50

Exactly the same problem but with uc_conditional_payment + http://drupal.org/node/952532. The patch works with 2.4. Thank you!

Comment #8

sin CreditAttribution: sin commented 4 December 2012 at 10:14

Status:

Needs review

» Reviewed & tested by the community

It saved me the second time, the same issue with #952532: Dependency on shipping method. Please commit.

Comment #9

longwave

he/him

English

CreditAttribution: longwave commented 29 December 2012 at 22:44

Status:

Reviewed & tested by the community

» Needs review

File	Size
1157494-uc_payment-shipping-method.patch	1.1 KB

This patch is functionally the same, just reformatted to fit the coding standards a bit better. This seems to work for me, if someone else can confirm then it can be committed.

Comment #10

TR CreditAttribution: TR commented 30 December 2012 at 01:41

I think it's fundamentally wrong to have shipping-specific code in uc_payment.js. If it's a matter of serializing all the line items, then the code should look for line items in general, and not specifically named line items like shipping. Otherwise this same problem gets pushed to another type of line item, and won't work for custom line items.

Comment #11

longwave

he/him

English

CreditAttribution: longwave commented 30 December 2012 at 17:54

It is far too much work to refactor all the checkout JavaScript in 6.x to handle all possible cases, especially where other contrib may be relying on code that is already present. Committing this and #1132378: Checkout page does not contain order UID for AJAX-based conditional action calls will fix bugs in CA support for core checkout functionality, at this stage of the game I think this is the best we can aim for.