search

wp-e-commerce / WP-e-Commerce

WP eCommerce - The most popular independent eCommerce platform for WordPress
https://wpecommerce.org
GNU General Public License v2.0
215 stars 216 forks source link

shipping region not updating when "same as billing" is selected #625

Closed lunarman9 closed 10 years ago

lunarman9 commented 11 years ago

I resolved this bug in a prior release, but it has resurfaced. To duplicate:

This would be fine if, when the checkout is submitted with the "same as billing address" option checked, the billing fields were used for the shipping fields, but they are not - the hidden shipping fields are used. This is fine, but when these fields are hidden it is imperative they be updated to the billing fields.

instinct commented 11 years ago

How did you resolve it last time? Do you have a patch you could share with us here?

Sent from my iPhone

On 30/08/2013, at 3:50 AM, lunarman9 notifications@github.com wrote:

I resolved this bug in a prior release, but it has resurfaced. To duplicate:

install 3.8.12.1 add item to cart go to checkout check "same as billing address" under shipping options update a state/province in the billing address uncheck "same as billing address" and you'll find that the region has not been updated This would be fine if, when the checkout is submitted with the "same as billing address" option checked, the billing fields were used for the shipping fields, but they are not - the hidden shipping fields are used. This is fine, but when these fields are hidden it is imperative they be updated to the billing fields.

— Reply to this email directly or view it on GitHub.

lunarman9 commented 11 years ago

Hi, The resolution last time was limited to a JS change, but this time it's not so simple. I'm working on it now - will hopefully have a fix later today.

Cheers, Sam

lunarman9 commented 11 years ago

No luck. Between the JS and AJAX PHP files, the code is so convoluted and lacking in comments that it's almost impossible to find the cause, let alone the fix. I'll keep trying.

instinct commented 11 years ago

I'm not sure whether this is helpful of not but the new Theme Engine V2 Plugin does shipping same as billing differently. It might be worth having a look since this code might be getting replaced soon anyway.

Over to @garyc40 though as he's the ultimate authority on that :)

leewillis77 commented 11 years ago

I've got two client's having problems with shipping same as billing which sound like they could be the same as this. @lunarman9 can you explain roughly what needed fixing, maybe I can help work through a fix?

JeffPyeBrook commented 11 years ago

@leewillis77 I am having what seems to be related problems as well. Currently trying to figure this one out. It seems that the logic starting at with submit_change_country() isn't doing what it needs to do. I am having shipping same as billing issues, and tax calculation based on the the wrong state.

When selecting to changing a country/state/zip that information isn't copies to the shipping address. I'm walking through in the debugger trying to understand the intended behavior.

lunarman9 commented 11 years ago 
Hi Lee,
I solved this in 3.8.9.3, but back then it was only a jquery
selector issue that broke the form in FF.
3.8.9.12.1 appears to have moved a lot of what was being done in an
"eval" statement in 3.8.9.3. This move has reduced the number of
AJAX calls to 1 (a good thing), but has broken the "copy from
billing to shipping if 'shipping same as billing' is selected". I'm
amazed no one else has encountered this.
I originally approached this bug by trying to do a direct copy - if
no data available from shipping, and 'shipping same as
billing'=true, then rewrite customer_meta with billing, and output a
flag to the JS to copy over. No dice - this got very complicated
very quickly.
I then thought that triggering one of the onchange functions would
be the answer, but I stopped there. This is only one of at least 10
bugs I need to fix every time I upgrade WPEC; I'm a huge proponent
of WPEC, but bugs and old (unreadable) code are slowly killing this
project.
lunarOn 9/1/2013 2:06 PM, Lee Willis wrote:

  I've got two client's having problems with shipping same as
    billing which sound like they could be the same as this. @lunarman9 can you explain roughly
    what needed fixing, maybe I can help work through a fix?
  —
    Reply to this email directly or view
      it on GitHub.
instinct commented 11 years ago

What's your email address lunarman? I've got an idea.

Also what are the 10 bugs you always need to fix when you upgrade.

Sent from my iPhone

On 4/09/2013, at 6:40 AM, lunarman9 notifications@github.com wrote:

Hi Lee, I solved this in 3.8.9.3, but back then it was only a jquery selector issue that broke the form in FF. 3.8.9.12.1 appears to have moved a lot of what was being done in an "eval" statement in 3.8.9.3. This move has reduced the number of AJAX calls to 1 (a good thing), but has broken the "copy from billing to shipping if 'shipping same as billing' is selected". I'm amazed no one else has encountered this. I originally approached this bug by trying to do a direct copy - if no data available from shipping, and 'shipping same as billing'=true, then rewrite customer_meta with billing, and output a flag to the JS to copy over. No dice - this got very complicated very quickly. I then thought that triggering one of the onchange functions would be the answer, but I stopped there. This is only one of at least 10 bugs I need to fix every time I upgrade WPEC; I'm a huge proponent of WPEC, but bugs and old (unreadable) code are slowly killing this project. lunarOn 9/1/2013 2:06 PM, Lee Willis wrote:

I've got two client's having problems with shipping same as billing which sound like they could be the same as this. @lunarman9 can you explain roughly what needed fixing, maybe I can help work through a fix? — Reply to this email directly or view it on GitHub. — Reply to this email directly or view it on GitHub.

JeffPyeBrook commented 11 years ago

@lunarman9 I have been having some issues as well and have made some progress. Any chance you could take a modified wp-e-commerce.js and do a little testing?

JeffPyeBrook commented 11 years ago

The modified wp-e-commerce.js file is in this branch JeffPyeBrook:shipping-same-as-billing-refactor. To date, it's only been tested against two sites, one with a highly customized cart, the other with a slightly customized cart.

lunarman9 commented 11 years ago

opps - didn't mean to close. Jeff - I appreciate your updates, but I won't be able to try them as I gave up and switched to a different plugin.

leewillis77 commented 11 years ago

Hi Jeff,

I've given this a quick run out this morning, and doesn't seem to make any tangible difference I'm afraid. I'm trying against current master WP e-Commerce with your modified wpsc-core/js/wp-e-commerce.js - is that what you'd expect?

JeffPyeBrook commented 11 years ago

@leewillis77 if you didn't have problems before this should be a drop in replacement, that's what I was hoping for

The problems I was seeing that it does seem to fix are these: 1) Checking shipping same as billing the unchecking shows the disabled shippingstate input field 2) Removing any of the standard shipping feild can sometimes cause a javascript error and stop the script prematurely 3) Multiple ajax requests when one would suffice 4) Recursive or cyclical (i couldn't figure out which) ajax requests that never end and stop the checkout process and bog down the server 5) shippingstate input field sometimes was never initialized, eventually it would pass through to the checkout and show up in the purchase log as the stores default region or the last region selected 6) code was a little hard to follow, tried to add some comments and clean it up 7) If you want to show the shipping fields even when shipping same as billing is checked you can do it by having a hidden control input field with the hide/show value(New feature) 8) wanted to add the ability to add new sipping / billing pairs without changing the shipping same as billing javascript pairs (New feature)

JeffPyeBrook commented 11 years ago

btw, it was working well enough in my test environment I pushed it to a live site, we'll see if any sales complete today :)

leewillis77 commented 11 years ago

Hi Jeff,

Here's my summary of what I'd expect.

Does that make sense?

JeffPyeBrook commented 11 years ago

It does make sense.

If you are running on master, using all of the default fields, and testing as a not logged in user, then I think I found and have a fix for problems 1 and 2. It seems that there are a couple of defects in the visitor meta wrapper functions that stop the updated fields from being saved and/or causes them to be lost after saving.

If you grab the helpers/ajax.php, wp-e-commerce.js, wpsc-functions.php from my master branch you can see if it changes anything for you.

I am going to do some more bench testing of the changes. If they seem ok i'll put them out to a live site and see what happens.

p.s. You might be interested in the function I added to the js file for getting customer meta. I'm using it to retrieve the error address validation error messages set in a couple of custom shipping modules I created.

JeffPyeBrook commented 11 years ago

@leewillis77 Still some issues when information is copied on the tick/untick. The complexity seems to be with the country/region/billing state fields and what has to happen when the values are changed. When I have a version I think has it correct i'll ping you again for another check.

JeffPyeBrook commented 11 years ago

I am experimenting with a different checkout flow (theme changes only) that is a little more structured, and would be a little easier to fit on a handheld screen. Still a little work to do on the theme, but if you are interested, you can see what I am playing with at www.sparkle-gear.com

JeffPyeBrook commented 11 years ago

I think I found one possible of issues. It looks like the HTML generated in response to the ajax calls can be a little different than the HTML generated when the checkout page is generated. If the javascript looks for a control using the id that was used when the page is generated, but the HTML was replaced by an ajax call the script stops because of an error. If the script stops before a control is updated, or before the data is passed back to the server odd things can be displayed. Because the fields are collected again at checkout I don't think the problem can result in a bad transaction. But a customer could see the odd things, not be able to correct them, decide there is something amiss and not make the purchase.\

I will fix.

leewillis77 commented 11 years ago

Hi jeff,

I grabbed the files you suggested, but I can't even get to checkout now with anything in my basket - I get to checkout and my basket is empty.

JeffPyeBrook commented 11 years ago

I'll pull all the changed files together in a branch so you can grab them all at once. The cart emptying inappropriately was one of the defects I was able to fix.

0161-Jon commented 11 years ago

I am having big problems with this too, seems to forget the shipping destination when selecting 'Shipping Same as billing address' and so refreshes the page as far as calculate shipping area.

0161-Jon commented 11 years ago

BTW I'm not a developer, just a user, just posted here to keep an eye on this issue as I'm desperately trying to fix it for my client, and I can't do that until an expert finds the fix ;)

JeffPyeBrook commented 11 years ago

@leewillis77 I put the filed changes into branch shipping-same-as-billing-refactor in my repo. I think they are all there. This is a branch off of wp-e-commerce/master rather than my local master.

THe changes have been pushed to www.psarkle-gear.com and seem to be working. This site has a highly customized cart page. The feature that hides the shipping same as billing fields when the box is ticked is disabled in this theme so you can observe the shipping fields change when the billing fields change.

leewillis77 commented 11 years ago

Hi Jeff,

I just ran up your branch on a fresh install, but I still get the empty basket problem I mentioned earlier - ie nothing stays in the basket.

JeffPyeBrook commented 11 years ago

I'll push push all my changes in a minute, it's what I'm running on sparkle-gear.com

JustinSainton commented 10 years ago

Reopening, as I'm an idiot, and #528 is totally different.

JustinSainton commented 10 years ago

Moving to the 3.8.14 milestone. While this is incredibly annoying, it's not a regression from the previous version.

mihaijoldis commented 10 years ago

Shipping calculator: USA/Texas.

Change billing country to non US, in my example i used Romania, and i left the State input field empty. After sending the form i get back to the checkout page, the error notice under the State field is telling me to not leave it empty but i also get this notice at the top of the page:

Notice: Undefined offset: 1 in D:\wamp\www\testwp\wp-content\plugins\WP-e-Commerce-master\wpsc-includes\checkout.class.php on line 494 Call Stack

rayardinanda commented 10 years ago

I made this video to demonstrate what I found.

There seems to be an extra jquery that is ran when checking shipping as billing that changed the region (in the shipping calculator).

Please see : http://dl.getdropbox.com/u/1184399/Jing/2014-01-15_0935.swf

Steps to reproduce: 1) Set the shipping calculator region to point to California 2) Set the billing details state to Hawaii 3) Click on same shipping as billing 4) Notice that in the shipping calculator, the state will says "hawaii", BUT in a few seconds it will change to previous state chosen.

Tested with WPEC 3.8.13.2

timstl commented 10 years ago

Did anyone ever resolve this issue? We are seeing the same scenario shown in the video on multiple sites. Running 3.8.13.3.

We're also seeing tax not being passed to PayPal (pro) in some cases. It seems like the two issues are probably related.

JustinSainton commented 10 years ago

Need to confirm with @JeffPyeBrook that #948 actually fixed this issue.

JeffPyeBrook commented 10 years ago

@JustinSainton This is fixed in #948

JustinSainton commented 10 years ago

@rayardinanda @timstl @misulicus @leewillis77 @0161-Jon Can you guys all confirm that this is fixed in the latest master branch?

whoiskenjackson commented 10 years ago

Just now discovered this was an issue. Is there a new update available with a fix or any kind of code snippet we can use in the mean time?