Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
On the checkout page, "Enter your delivery address and information here" is blank.
The Address book drop down lists the label for the "Default billing address" and "Default shipping address" that were created off of the Address book tab on the user account page.
Billing information shows up.
Comment | File | Size | Author |
---|---|---|---|
#27 | uc-quote-incompatibility-fix-1421720-27.patch | 5.52 KB | MegaChriz |
#22 | uc-quote-incompatibility-fix-attempt-1421720-22.patch | 4.81 KB | MegaChriz |
#8 | 1421720.patch | 1.5 KB | neokrish |
Comments
Comment #1
MegaChriz CreditAttribution: MegaChriz commentedHm, I thought I had fixed that one. Delivery was disappearing at first with me too, but then after some puzzling I finally made it appear.
I currently have set up two test sites with Ubercart Addresses 7.x-1.x installed. With the first test site (which I had set up a few months ago) I encounter the same problem as you. On the other test site (fresh installed today) I do not have the same problem; delivery appears as expected.
I shall investigate the differences in the two sites. On the first test site I have some orders placed and a lot more addresses in the address book than in the second test site.
If you want to tackle this issue, let me know.
Comment #2
MegaChriz CreditAttribution: MegaChriz commentedFound one thing out: the problem only occurs when the Shipping quotes module (uc_quote) is turned on (this module is one of the Ubercart core modules). The function uc_quote_form_uc_cart_checkout_form_alter() is removing the Ubercart Addresses address field element callback and thus Ubercart Addresses doesn't get a chance to do it's logics on the checkout page. Anoying Ubercart just overwrites these values.
Possible fixes:
- Implement hook_form_FORM_ID_alter() for uc_cart_checkout_form and undo the actions of the uc_quote module there. The module weight of Ubercart Addresses should be adjusted to become heavier than uc_quote.
- Rename the form elements (but in that case shipping costs will not be calculated automatically anymore when the customer changes the address information).
- Hack uc_quote and submit this as a patch in the Ubercart issue queue.
@Sam-Inet
Maybe you can think of an other solution? I'll leave this issue open for you at the moment and work on the registration issue first.
Comment #3
Michael-IDA CreditAttribution: Michael-IDA commentedrofl. yeah.
# # #
We need shipping quotes, so I'll probably tackle this first. My first thought is to fix uc_quote's bad behavior, but I'll finish a full QA walk though today first and see if there is anything that we need fixed that gives more value.
Our original install was ~Nov w/ D7.9, and has been upgraded several times since. Currently we don't have any orders placed ( a few in shopping cart status), and I only had the single Default address set.
Back later,
Sam
Comment #4
MegaChriz CreditAttribution: MegaChriz commentedThanks for picking this up. I think it has quite a high priority (comparing to the other issues). I'd like to assign you to this issue (so it's clear you're working on it), but I can only assign other maintainers.
Comment #5
MegaChriz CreditAttribution: MegaChriz commentedTagging (this issue should be fixed before the release of 7.x-1.0-alpha1).
Comment #6
Michael-IDA CreditAttribution: Michael-IDA commentedHi MegaChriz,
Yeah, we need shipping costs to be calculated automatically and in the interest of timeliness, fixing uc_quote is problematic, as I need uc_addresses working to finish the user conversion.
So, we'll be offering a $ bonus to whoever can do your a):
I have a meeting with the boss Monday morning, so I'll post the details here and Drupal Jobs afterwards.
Best,
Sam
PS: We're welcoming developers to bid on this, so if you don't want to wait till Monday, send me an email off my contact form with a flat dollar amount bid and expected completion date.
Comment #7
Michael-IDA CreditAttribution: Michael-IDA commented$60 Bonus for fixing uc_addresses, Delivery info
http://groups.drupal.org/node/207913
http://drupal.org/node/1430202
Comment #8
neokrish CreditAttribution: neokrish commentedPlease can you test the patch attached? It works in a fresh ubercart installation that I did for the purpose of this issue. The patch works based on the suggestion in #6 above.
Comment #9
Michael-IDA CreditAttribution: Michael-IDA commentedwhy do i ever say things like that? Ukrainians bit us in the butt, had to track down what they broke all day, working on this now.
Comment #10
MegaChriz CreditAttribution: MegaChriz commented@neokrish
I may be wrong, but I think the module weight is not increased on a fresh install, because update functions are only called when updating the module. On a fresh install all currently present update functions will never be called.
Comment #11
Michael-IDA CreditAttribution: Michael-IDA commentedHi MegaChriz,
Well, I was all set to test out the patches tonight, but both Development releases (6 and 7, tar and zip) 404...
also (guessing 404s as well)
$ drush dl uc_addresses
File uc_addresses-7.x-1.x-dev.tar.gz is corrupt (wrong md5 checksum).
I have the 7 dev tar ball on a work machine, guess I'll check the patch in the morning then.
Sorry about the delays neokrish,
Sam
Comment #12
MegaChriz CreditAttribution: MegaChriz commentedWeird, I did do a commit yesterday (very small change), but I can't see what went wrong. As a temporary solution, you can also get the module by downloading a snapshot in the repository viewer.
Comment #13
Michael-IDA CreditAttribution: Michael-IDA commented@neokrish,
Yeah, didn't work on our existing build.
On /cart/checkout :
Also get these messages below "Calculate shipping cost" and above "Payment method," possibly not relevant.
* Checkout cannot be completed without any payment methods enabled. Please contact an administrator to resolve the issue.{RTFM Sam.}* An illegal choice has been detected. Please contact the site administrator.
Sam
Edit 01:
- Using the latest dev release the Billing information does pre-populate.
Edit 02:
- Patch as written does not increase the weight in the system table.
Edit 03:
- Manually changing the weight allows the Delivery information form to be shown and pre-populated with the default address.
- But, selecting a different address from the drop down does not populate the fields with the new selection. In either Delivery or billing (MegaChriz, new bug?).
Edit 04:
I should RTFM.
Comment #14
MegaChriz CreditAttribution: MegaChriz commentedI haven't tested the patch yet, but I think just unsetting the #process property is not right.
Comment #15
neokrish CreditAttribution: neokrish commentedthanks MegaChriz and Sam-Inet for your reviews. I am currently engaged on other tasks and may not be able to handle this patch further. Hope someone else resolves this.
Comment #17
Michael-IDA CreditAttribution: Michael-IDA commented@neokrish, send me your PayPal info. Your code was good (a bit mangled by patch, but okay).
# # #
Seriously Is Bizarre.
Finally got a chance to dig through this, more than just patching and re-loading.
Manually applying neokrish's patch on the latest dev works, if you already had uc_addresses installed. If it was a brand new install, the update didn't fire, as MegaChriz indicated in #10. The patch originally not working might have been related to "\ No newline at end of file" fubar-ing something?
- Adding the update code to function uc_addresses_install, allowed a good install.
- Also tested $weight if uc_quote was set to anything but zero. Works.
- Addresses populate checkout form correctly.
Thanks neokrish and MegaChriz,
Best,
Sam
Trying to avoid patching a moving dev target, the changes are inline below (or I can email you the files if you want).
File: uc_addresses.install
Replace function
Add to bottom of file
File: uc_addresses.module
Add after function uc_addresses_uc_order_pane_alter
Comment #18
Michael-IDA CreditAttribution: Michael-IDA commentedhelps if I update the status.
MegaChriz, can you send me an email after you review/commit this?
Thanks,
Sam
Comment #19
MegaChriz CreditAttribution: MegaChriz commentedThe benefit of posting a patch is that it will go through the automated tests and this could reveal if you accidentally broke something.
I've tested your code and while it brought back the delivery address form at checkout, it undoes a bit too much of the uc_quote module. When a customer changes it's delivery information, the shipping costs won't be updated automatically anymore. Normally, when a customer changes the country or postal code or chooses an other address, the shipping quotes are updated automatically. Now Ubercart Addresses overrides too much so uc_quote doesn't get a chance to do it's logics.
While this situation is certainly better than the current situation (where no delivery address form at all is displayed) I can imagine someone will post an issue "filling in delivery doesn't update shipping costs".
I've tried to just set
#process
tobut that had given me some "Illegal choices" errors, probably because of the differences in the Ubercart Addresses address form and the original delivery address form.
I was thinking what to do about it. One option is to copy more logics over from uc_quote, the other one is to make some fundamental changes in Ubercart Addresses to match the original delivery checkout pane some more. One of the differences is that the ID the wrapper div of the zone field gets on the checkout page. Ubercart Addresses uses the address ID in the ID, while Ubercart uses the pane type (e.g. "delivery"). I've chosen to include the address ID in the zone wrapper to make it possible to have multiple address forms on one page without having to worry about a pane type to set.
This is the code Ubercart Addresses uses to generate the zone field (handlers/ubercart.handlers.inc, ± line 119):
And this is what Ubercart uses (uc_store/uc_store.module, ± line 358):
I guess the first thing to investigate is what exactly should be matched some more and then we can think about how to implement that nicely. I've not tested if it's because of the zone wrapper ID that adding the uc_quote_process_checkout_address() callback result into errors (so that's why we need to investigate first).
Comment #20
Michael-IDA CreditAttribution: Michael-IDA commentedAnd I thought that was normal behavior and was just going to custom module fix it. Okay, suppose to work here, will add bonus/bounty then...
Best,
Sam
PS: okay, I'll submit patches, even if they are against three commits ago dev versions....
Comment #21
Michael-IDA CreditAttribution: Michael-IDA commentedhttp://drupal.org/node/1442760
http://groups.drupal.org/node/210518
Comment #22
MegaChriz CreditAttribution: MegaChriz commentedI took about half an hour today to check if matching the zone wrapper ID would solve the issue. At first glance, it looks likes it's did! The next step was to figure out how to implement it nicely. While I thought that would be very hard and would take hours, I saw I could just make use of the
#key_prefix
property of the uc_addresses_address form element (I should document this one).So here's a patch that may solve the issue, but I didn't thoroughly tested it. Else we are one step closer in the right direction.
Comment #23
Michael-IDA CreditAttribution: Michael-IDA commentedwill test this in the morning
Comment #24
Michael-IDA CreditAttribution: Michael-IDA commentedThat's not the current behavior I'm seeing without uc_addresses installed. Or the behavior I'm seeing, plain Ubercart is:
No matter if the Country or State/Province fields are changed programmatically or manually, the "Calculate shipping cost" section and the shipping line item in "Payment method" does not change.
I'm only getting changes when the button "Click to calculate shipping" in the "Calculate shipping cost" section is clicked. Clicking the button fires all Rules properly and the shipping options for that State/Province display correctly (and update the shipping line item in "Payment method" per radio button selection).
So, is this a bug with uc_quote?
# # #
Downloading and testing your patch now.
Sam
Edit:
I hate patching -dev...
yeah, I patched it, testing now
Edit 02:
Nope didn't work:
- initial load doesn't show proper shipping options for default delivery
- selecting different delivery address from drop down doesn't change shipping options
- manually changing Country, then State/Province fields, doesn't change shipping options
- Manually clicking "Click to calculate" shows correct shipping options
Comment #25
MegaChriz CreditAttribution: MegaChriz commentedHm, it looks like not all automated tests are executed. The patch in #22 breaks checkout when uc_quote is disabled, so it should have failed the automated tests. I will fix that later this week; it doesn't affect testing when uc_quote is enabled. It's still worth to test the patch.
@Sam-Inet
Strange you didn't encounter the behaviour that changes in the delivery address form (only country and postal code field) updates the shipping costs when uc_quote is enabled and uc_addresses is disabled.
Did you test in a fresh installation? Did you flushed all caches? If you don't experience this behaviour, I guess you will see no difference with the patch applied.
Comment #26
Michael-IDA CreditAttribution: Michael-IDA commentedJust did a clean install with only Ubercart, "Calculate shipping cost" on checkout does not change when Delivery State/Province is changed.
So.... Guess this isn't an addresses problem....
Edit:
#1373236: Allow multiple modules to react on checkout Ajax events
#1317692: Fetch shipping quote based on State/Province selection rather than postal code
So, shipping quotes updates when the Postal code field is exited (doesn't have to be changed)*. But not when the State/Province is changed.
* Not that I can get it to fire a Rule correctly based upon the zip code entered, but, hey, that's tomorrow's problem. [Fixed, but am I really going to have to do a OR condition on 99,999 zip codes? UHG.]
Comment #27
MegaChriz CreditAttribution: MegaChriz commentedAs promised in #25, a new patch. This should fix the problem of breaking checkout when uc_quote is disabled. I've done some more testing and I've not found any other issues with it. No "Illegal choice" errors or something like that.
Can you test again, Sam?
Comment #28
philpro CreditAttribution: philpro commentedPatch in #27 fails lots of hunks for me. Did not work.
Comment #29
MegaChriz CreditAttribution: MegaChriz commentedStrange, I've no errors when I apply this patch against the latest dev. Did you apply patches from other issues too? They may not work well together. Which hunks did fail?
Comment #30
Michael-IDA CreditAttribution: Michael-IDA commentedWith all three patches applied (manually fixing hunk failures), /cart/checkout works without Ubercart Shipping quotes (uc_quote) being enabled.
Seems good,
Sam
Comment #31
MegaChriz CreditAttribution: MegaChriz commentedOk, I'm going to commit this one in a few days and adapt the patches in the other issues. I realize it's getting hard to test all the patches together if they don't apply well together. Not sure what else would be the best practice.
Comment #32
MegaChriz CreditAttribution: MegaChriz commentedPatch in #27 committed with one small modification: I changed the comments a bit (no code changes).
Comment #33
Michael-IDA CreditAttribution: Michael-IDA commentedOnly a problem if the same code base (file) is changed in multiple patches. But, it is much easier to patch/test/commit one issue at a time ;)