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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MegaChriz’s picture

Hm, 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.

MegaChriz’s picture

Found 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.

Michael-IDA’s picture

Anoying Ubercart just overwrites these values.

rofl. 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

MegaChriz’s picture

Priority: Normal » Major

Thanks 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.

MegaChriz’s picture

Issue tags: +7.x-1.0-alpha1

Tagging (this issue should be fixed before the release of 7.x-1.0-alpha1).

Michael-IDA’s picture

Hi 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):

- 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.

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.

Michael-IDA’s picture

$60 Bonus for fixing uc_addresses, Delivery info

http://groups.drupal.org/node/207913
http://drupal.org/node/1430202

Hello Drupallers,

We're offering a bonus to the first person that can submit a successful quick fix patch(s) for this module:

Module: Ubercart Addresses
http://drupal.org/project/uc_addresses

Issue: Delivery information does not display in checkout
http://drupal.org/node/1421720

Bonus: $60
Deadline: Feb. 13, 2012

Please submit your patch to the issue queue and I will do the testing/QA. Once accepted by MegaChriz (module owner), we'll PayPal you the bonus. After the deadline, we'll pay half, but I'll be working on it then as well...

Please add any module specific questions to the issue queue.

Best Regards,

Sam

PS: There are two others we are currently bonus-ing for uc_addresses, and if you can do all three, the total bonus will be bumped to $300.

Ubercart Addresses user tokens
http://drupal.org/node/1424032
Bonus: $25
Deadline: Feb. 13, 2012

Order administration feature
http://drupal.org/node/1424038
Bonus: $150
Deadline: Feb. 13, 2012

neokrish’s picture

Assigned: Unassigned » neokrish
Status: Active » Needs review
FileSize
1.5 KB

Please 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.

Michael-IDA’s picture

will test in about 2 hrs. sam

why 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.

MegaChriz’s picture

@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.

Michael-IDA’s picture

Hi 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

MegaChriz’s picture

Weird, 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.

Michael-IDA’s picture

Assigned: Unassigned » neokrish
Status: Needs work » Needs review

@neokrish,

Yeah, didn't work on our existing build.

On /cart/checkout :

- Delivery information shows a drop down Address book, which you can select a "Name" for, but no address fields are shown for Delivery information.

- Billing information shows a drop down Address book, which you can select a "Name" for, and address fields are shown, but no information is populated.

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.
* An illegal choice has been detected. Please contact the site administrator.
{RTFM Sam.}

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.

MegaChriz’s picture

I haven't tested the patch yet, but I think just unsetting the #process property is not right.

neokrish’s picture

Assigned: neokrish » Unassigned

thanks 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.

Status: Needs review » Needs work

The last submitted patch, 1421720.patch, failed testing.

Michael-IDA’s picture

Assigned: neokrish » Unassigned
Status: Needs review » Needs work

@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

function uc_addresses_install() {
  $t = get_t();
  // Setup default permissions. Authenticated users will automatically
  // be granted with permissions to manage their own addresses.
  user_role_grant_permissions(DRUPAL_AUTHENTICATED_RID, array(
      'view own addresses',
      'add/edit own addresses',
      'delete own addresses',
    )
  );
  // Increase the weight of uc_addresses relative to uc_quote by an
  // addition of 1 so that uc_addresses module can unset the property
  // set by uc_quote's form_alter.
  $weight = db_select('system', 's')
              ->fields('s', array('weight'))
              ->condition('name', 'uc_quote', '=')
              ->execute()
              ->fetchField();
  db_update('system')
    ->fields(array('weight' => $weight +1))
    ->condition('name', 'uc_addresses', '=')
    ->execute();
  drupal_set_message($t("Ubercart Addresses is installed. The authenticated user automatically was granted the permissions %view_own, %edit_own and %delete_own.", array('%view_own' => $t('view own addresses'), '%edit_own' => $t('add/edit own addresses'), '%delete_own' => $t('delete own addresses'))), 'status');
}

Add to bottom of file

/**
 * 
 * Increase the weight of uc_addresses relative
 * to uc_quote by an addition of 1 so that
 * uc_addresses module can unset the property
 * set by uc_quote's form_alter.
 */
 
function uc_addresses_update_7101() {
  $weight = db_select('system', 's')
              ->fields('s', array('weight'))
              ->condition('name', 'uc_quote', '=')
              ->execute()
              ->fetchField();
  db_update('system')
    ->fields(array('weight' => $weight +1))
    ->condition('name', 'uc_addresses', '=')
    ->execute();
}

File: uc_addresses.module

Add after function uc_addresses_uc_order_pane_alter

/** 
 * Implements hook_form_FORM_ID_alter() for uc_cart_checkout_form().
 *
 * unsets '#process' key set by uc_quote module which overwrites
 * the functionality of uc_addresses.
 */
function uc_addresses_form_uc_cart_checkout_form_alter(&$form, &$form_state) {
  if (isset($form['panes']['delivery']['address'])) {
    unset($form['panes']['delivery']['address']['#process']);
  }
}
Michael-IDA’s picture

Status: Needs work » Needs review

helps if I update the status.

MegaChriz, can you send me an email after you review/commit this?

Thanks,
Sam

MegaChriz’s picture

Status: Needs review » Needs work

Trying to avoid patching a moving dev target

The 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 to

array('uc_addresses_process_address_field', 'uc_quote_process_checkout_address');

but 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):

return array(
  $fieldName => array(
    '#type' => 'select',
    '#title' => $this->getFieldTitle(),
    '#required' => $this->isFieldRequired(),
    '#options' => $options,
    '#default_value' => $default,
    '#prefix' => '<div id="uc-address' . $this->getAddress()->getId() . '-zone-wrapper">',
    '#suffix' => '</div>',
    '#empty_value' => 0,
  ),
);

And this is what Ubercart uses (uc_store/uc_store.module, ± line 358):

case 'zone':
  $subelement = array(
    '#prefix' => '<div id="uc-store-address-' . str_replace('_', '-', $prefix) . 'zone-wrapper">',
    '#suffix' => '</div>',
  );

  $zones = db_query("SELECT zone_id, zone_name FROM {uc_zones} WHERE zone_country_id = :country", array(':country' => $value[$prefix . 'country']))->fetchAllKeyed();
  if (!empty($zones)) {
    natcasesort($zones);
    $subelement += array(
      '#type' => 'select',
      '#options' => $zones,
      '#empty_value' => 0,
    );
  }
  else {
    $subelement += array(
      '#type' => 'hidden',
      '#value' => 0,
      '#required' => FALSE,
    );
  }
  break;

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).

Michael-IDA’s picture

"filling in delivery doesn't update shipping costs".

And 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....

Michael-IDA’s picture

Title: Delivery information does not display in checkout » [$150 Bounty] Delivery information does not display in checkout
Assigned: Unassigned » Michael-IDA
Issue tags: +Novice, +bounty

http://drupal.org/node/1442760
http://groups.drupal.org/node/210518

[Open] $150 Bonus for fixing uc_addresses, Delivery info
Posted by Sam-Inet on February 16, 2012 at 3:56pm

Hello Drupallers,

We're offering a bonus to the first person that can submit a successful quick fix patch(s) for this module:

Module: Ubercart Addresses
http://drupal.org/project/uc_addresses

Issue: Delivery information does not display in checkout
http://drupal.org/node/1421720

Specifics: "filling in delivery doesn't update shipping costs"
http://drupal.org/node/1421720#comment-5605734

Bonus: $150
Deadline: Feb. 24, 2012

Please submit your patch to the issue queue and I will do the testing/QA. Once accepted by MegaChriz (module maintainer), we'll PayPal you the bonus. After the deadline, we'll pay half, but I'll be working on it then as well...

Please add any module specific questions to the issue queue.

Best Regards,

Sam

PS: There are others we are currently bonus-ing for uc_addresses, and if you do multiples, your bonus will be bumped an additional $20 per extra successful patch.

Order administration feature
http://drupal.org/node/1424038
Bonus: $150

MegaChriz’s picture

Status: Needs work » Needs review
FileSize
4.81 KB

I 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.

Michael-IDA’s picture

will test this in the morning

Michael-IDA’s picture

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.

That'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...

$ patch < uc-quote-incompatibility-fix-attempt-1421720-22.patch
can't find file to patch at input line 5
Perhaps you should have used...

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

MegaChriz’s picture

Hm, 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.

Michael-IDA’s picture

Did you test in a fresh installation?

Just 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.]

MegaChriz’s picture

As 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?

philpro’s picture

Patch in #27 fails lots of hunks for me. Did not work.

MegaChriz’s picture

Strange, 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?

Michael-IDA’s picture

With all three patches applied (manually fixing hunk failures), /cart/checkout works without Ubercart Shipping quotes (uc_quote) being enabled.

Seems good,
Sam

MegaChriz’s picture

Assigned: Michael-IDA » MegaChriz
Status: Needs review » Reviewed & tested by the community

Ok, 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.

MegaChriz’s picture

Status: Reviewed & tested by the community » Fixed

Patch in #27 committed with one small modification: I changed the comments a bit (no code changes).

Michael-IDA’s picture

hard to test all the patches together

Only 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 ;)

Automatically closed -- issue fixed for 2 weeks with no activity.