State/Province select boxes need converting to Ajax

I am having the same problem, the drupal log says "Illegal choice 67 in State/Province element."

Yes, it looks like the AJAX callback is messed up.

OK has there been any progress on this yet? I am running the 7.x-3.x dev version of Ubercart from Mar 8 2011, and I am getting the same error.

Peter Fisera
Earth Angel Consulting

You, and anyone else, are welcome to develop a fix for this issue and post it here as a patch.

Patch attached that fixes the original issue with the checkout by converting the form to use D7 Ajax functionality.

State/province selection on country change also looks broken in the copy address box, the previous address selector, order editing and the shipping module, but I am out of time for fixing these now.

Extended version of the above.

This patch unifies all address forms in core to use a single Ajax handler for country changes and improves the uc_country_select() and uc_zone_select() API, but has a couple of problems.

The field title is hidden after changing the country in some settings forms, as a trade-off to showing the field title twice on the cart page due to the way the forms are themed. The shipping quote address on the node edit form also triggers a Drupal core bug with Ajax forms: #1049462: Usage of deprecated $form_state['clicked_button'] causes bugs during AJAX submissions by non-buttons

Adding tag.

Closed #1096386: UberCart shipping checkout error as duplicate

is this patch something a new user can apply now? are there step-by-step instructions available anywhere on how\where to apply such patches?

See http://drupal.org/patch/apply to learn how to apply patches. And if you do try it out, please report back with your results!

Patch applies but doesn't work quite right.

Test case:

Go to checkout, country defaults to US (for me). Select a zone. Change country to Canada. Select zone for new country. Click "billing is the same". Unclick "billing is the same" so you can see the billing address. Zone doesn't get copied and zone select is now only a few pixels wide with no options to select from. Go back to the delivery pane, change country back to US. Zone select still shows Canadian provinces, error message says "An illegal choice has been detected. Please contact the site administrator." From this point on you're stuck with the Canadian provinces, no matter what you change the country to be.

There's other weirdness, but it's probably all related and will go away when the above gets fixed.

This is still a work in progress, but further changes are now included:

- all address forms have been converted to a new and more flexible "uc_address" form API element
- address field tables have been replaced with CSS
- the "address is the same" box at checkout should work as expected

The dropdown address selectors still need converting over, and there's still some bugs relating to selecting a zone then changing country, and the uc_shipping module address fields don't quite work right for some reason I have yet to track down.

thanks longwave, i will be testing this patch for you soon!

if i knew more of how Drupal implements AJAX i might have tackled the suggestion to try fixing this myself... but it was something that i just felt that someone else would be more efficient with :-)

cheers
Fish

I might be doing something wrong, but I seem to get a different error everytime I try to run this patch in the following directory: C:\xampp\htdocs\drupal\sites\all\modules\ubercart\payment\uc_google_checkout

COMMAND: patch -p0 < 1071332-ajax-addresses.patch

Cygwin MESSAGE = can't find file to patch at input line 5 (then 43 , then 82, then 114, then 133)

@john_t: You should start from the main Ubercart directory, and use patch -p1 as this is a git format patch.

WOW THANKS A LOT longwave! For any other novice who needs to run this patch here is my step-by-step:

1. I moved '1071332-ajax-addresses.patch' to C:\xampp\htdocs\drupal\sites\all\modules\ubercart

2. I opened Cygwin and changed directories to this same location: (command: cd C:/xampp/htdocs/drupal/sites/all/modules/ubercart ... note forward slashes)

3. I then ran the update command as 'longwave' suggested in Cygwin (command: patch -p1 < 1071332-ajax-addresses.patch)

I guess my only other question is how does one know the difference between a 'git format patch' you copy to the UberCart main directory, and other patches you have to copy to their respective subdirectories?

Not sure what happened, but after doing what I said above (which did show the patch had run) I get the following error when trying to visit my drupal site (via. localhost/drupal)

Notice: Trying to get property of non-object in C:\xampp\htdocs\drupal\modules\dblog\dblog.module on line 144

Fatal error: main() [function.require]: Failed opening required 'uc_cart_checkout_pane.inc' (include_path='.;C:\xampp\php\PEAR') in C:\xampp\htdocs\drupal\sites\all\modules\ubercart\uc_cart\uc_cart.module on line 13

What should I do now?

Not sure what has happened to uc_cart/uc_cart_checkout_pane.inc, the patch modifies this file but in your case it seems either deleted or no longer readable for some reason.

You can identify Git format patches by the "diff --git" in the first line and the ---/+++ lines starting with a/ and b/ respectively. As drupal.org has switched to Git now, all new patches are likely to be in this style.

In addition to the what longwave's already mentioned, we'll have to change the way shipping quotes are triggered as well. It used to be that filling out the delivery postal code field, or selecting a previous address, would recalculate shipping quotes. My test site also doesn't add the default shipping rate to the line items at first, but I think that's a separate issue.

I'm probably going to add a comment when #key_prefix is used when the #default_value is an array with custom keys. It took me a little bit to realize that you did that to keep system_settings_form() working. :)

Yeah, the element should be documented so other modules can use it too - it's quite complicated to handle the wide variety of methods used to store addresses without changing many more structures and lines of code. #key_prefix also lets us pass $order directly as #default_value for the checkout panes, and provides a useful unique CSS ID for the zone selector when there are multiple address forms on one page.

Not sure how to handle the shipping quotes issue but we should be able to either add more options to the element for this, or maybe use an additional #process or #after_build handler to do it.

I think I got the "illegal choice" bug fixed here. I set the zone field's value back to 0 ("Please select" or "Not applicable") if it's caused by an AJAX process, which should be a valid option for all countries. If that can be confirmed, we can get the rest of the problems pretty easily, I think.

Patch in #23 breaks the "Saved addresses" functionality - selecting an address in the saved addresses select does nothing, delivery/billing fields are not populated. It also breaks the appearance of the checkout page. Before and after images attached, both images are the same size so you can see the spacing difference as well as the positioning of the saved addresses select and the background color of the table cells.

subscribe

Wugh. I've tried to put theme_uc_address_pane() back and fix the address selector. So far, doing one or both of these seems to break something else.

These files both contain multiple commits to get you to where I'm at. Apply them to e68b8f5ddd827 with git am --3way as described at the end of this recent Lullabot article.

The checkout panes were deliberately redesigned in my patch to use floated form elements instead of tables as this is a more semantically correct way of doing things and helps with accessibility. I think we should stick with the floated elements approach and use CSS to reduce the vertical spacing between fields; otherwise, to me, this looks better than what we had before - I feel keeping "saved addresses" separate works better as it is not really an address component and hence shouldn't be lined up with the other fields.

Keeping the address fields out of a table certainly makes it easier for the zone options to be changed.

I've added a fix to keep the selected address from overriding the zone options if the country is changed afterwards. Still need to fix the quote triggers to go off the new location of the postal code fields. It might have to add another #process callback to add the #ajax it needs, after the whole address is split out.

How about making this a topic branch in the repository? That way we could checkout the branch, test it, and create patches against the branch until we ironed out all the bugs. Then we could merge the changes back into 7.x-3.x and delete the topic branch when we're done.

Sure thing. I've put it in 1071332/ajax-addresses

Hi just some feedback on the patch, for me it has fixed the problem with states/regions per country, however as mentioned above, the saved addresses while present in the pull down list box do not repopulate the address fields.

thanks for all the work, it is very much appreciated and everthing else in uc 3 is rocking!

That's strange. The patch works for me.

Guys i have problem with 'Phone number'. If i write the number and check 'My billing information is the same as my delivery information' the first time the form give me this error:

First name field is required.
Last name field is required.
Street address 1 field is required.
City field is required.
Postal code field is required.

I don't know is strange i think the ajax copy data don't work good

The second time update billing information and work, but in checkout/review i don't see the phone number instead i see delivery_phone, billing_phone

Thanks for Everything

I updated 1071332/ajax-addresses to fix the phone number issue. It wouldn't have worked even if you had entered both phone numbers in manually, though.

Hi, just reapplied unthemed patch and the saved addresses are available in the pull down box, however when you select one a blue timer icon comes on and then goes off and the address boxes are still not repopulated. Not sure why this is so, any help very much appreciated as am out of my depth here!

Hey Island, I'm running the new branch and noticed that if I select a country with no zones (therefore showing Not Applicable) in the zone dropdown, that then when I select a country with zones, while it makes the AJAX call, the zone list is still empty and says Not Applicable.

Just thought I'd report this before I forget about it.

Testing checkout of the 1071332/ajax-addresses topic branch. (Having the topic branch makes it very easy to test a complex set of changes like this!)

The vertical spacing of the form elements was still too large, so I fixed that in CSS and committed the change to the topic branch. Now that I know from #27 that the change to the "Saved addresses" select was deliberate, and the reasoning for it, it makes perfect sense. I do think it's better this way.

The problem described by the original poster is fixed by this patch. However, there are other problems that were introduced.

1) Select a saved address in the Delivery information fieldset. The delivery address details are properly filled in. Now check "My billing information is the same ...". Then uncheck again so we can see what's been filled in the billing information fields. You'll see they remain empty; the copying isn't working.

2) Formatting of store address at admin/store/settings/store form is messed up - see attached image.

3) Entry of default product pickup address at node/%edit doesn't work correctly anymore - changing country does not change options for State/Province. I fixed the CSS in the topic branch to reduce the vertical spacing between form elements here.

4) Entry of default product pickup address at admin/store/settings/quotes/edit doesn't work correctly anymore - changing country does not change options for State/Province.

subscribe

OK, fixed inolen's issue with countries that don't have zones. The process function for addresses wasn't dealing with parts that didn't have a value set.

Also fixed the zone swapping errors on the node-edit form and the shipping quote settings. The AJAX callback was still prepending a _ even if #key_prefix as empty.

Actually putting values into the address when "copy address" is selected might not be strictly necessary, but my first try to fix it didn't work. It's probably still worth fixing, but I don't think it qualifies as a release blocker by itself.

After reading the comment in seven/vertical-tabs.css, I can see why they made textfields 100% wide. I'm just not sure whether to override it like the node-edit form does with the pickup address, or to accommodate it and change the text alignment.

I think this is almost ready to be merged. Agreed that the "copy address" should be improved for visual effect, but this can be handled later if necessary.

The vertical tabs textfield issue is a late regression in D7, documented at #970426: Regression: Vertical Tabs styling on input elements overrides specifically set FAPI keys, but that we will probably have to work around here. It looks like overriding the CSS will be okay as the fields are fairly short and should not overrun the tabs in this case.

OK. Overriding sounds good to me.

I noticed that since 1071332/ajax-addresses doesn't have commit 95094982, that the default pickup address causes nothing but the body to be saved to the database. I never did figure out why that would be the case, since my debugger caused an Exception to be thrown instead. However, that particular change fixes the problem, so we should be good when we merge back in.

Anything else that we need to address here before merging?

Just a few more things ...

On the /cart page, the "Estimated shipping cost" pane now collects the entire address (first name, last name, etc.) instead of just country, zone, postal code like it used to.

On the /cart/checkout page, if you change country but don't select a zone for the new country, you can still submit the form despite the fact that the zone is a required field.

Selecting a new country in the "Default pickup address" fieldset on a product edit page does change the zone select properly now, but I get the following PHP warnings:

* Warning: end() [function.end]: Passed variable is not an array or object in file_managed_file_validate() (line 536 of /xxx/modules/file/file.module).
* Warning: end() [function.end]: Passed variable is not an array or object in file_managed_file_validate() (line 536 of /xxx/modules/file/file.module).

Actually putting values into the address when "copy address" is selected might not be strictly necessary, but my first try to fix it didn't work. It's probably still worth fixing, but I don't think it qualifies as a release blocker by itself.

I think it's necessary. Without it, there is no feeback about the result of the copy operation, and no way to verify that the correct billing information has been entered. Many users have billing information that is only slightly different than delivery information, e.g. credit card is in the name of parent/spouse/company but the billing address is the same as ship-to address except for that. If you can't use the copy checkbox and then alter or verify the copied information, you end up typing all the fields for both the delivery and billing address. This totally negates the utility of the checkbox, and acts as a barrier to placing orders.

Besides, the copy checkbox doesn't seem to work correctly in other ways. If I enter a delivery address and a different billing address, check the "My billing information is the same" checkbox, then uncheck the checkbox to see the billing address, I still see the old billing address - it doesn't appear to be doing any copying at all.

The copy address box should be fixed by b7de0ce.

The /cart pane worked correctly earlier, but was broken by 5f7a990, and I haven't yet tracked down why #required isn't working on the zone selector.

The PHP warnings are a core issue: #1049462: Usage of deprecated $form_state['clicked_button'] causes bugs during AJAX submissions by non-buttons

Validation was all kinds of screwy, especially for the zone field. See 946e34dd for details. But now all of the address fields will be required when they are supposed to be.

The /cart pane worked correctly earlier, but was broken by 5f7a990

Should we revert that commit? I've done so locally and it does clear up the /cart issue. What problem was that commit meant to fix?

The validation issue seems to be fixed.

The copy address now works right. But shipping quotes aren't being fired automatically anymore. I have all shipping methods set up on my test site. Flatrate and Weightquote are always shown, but the other methods like USPS and UPS appear only after clicking on the "Click to calculate shipping" button. They are not generated automatically when a saved delivery address is selected or when changes are made to the delivery address.

That commit was to fix #36. If you try to change from a country that doesn't have zones, then that change causes the zone field to be processed even though there's no value for it.

It occurs to me that this might be is why the quote form on the cart page now shows all of the address fields.

Selecting a delivery address or changing the delivery postal code should now recalculate shipping quotes. I'm still not sure what to do if billing and delivery are switched and the copy address box is checked.

Leaving as "needs work" for the cart pane problem.

Just some more info. If I disable USA and just leave Canada the shipping quotes work fine. I haven't tried any of the patches as I just installed UC3/D7

I reverted 5f7a990 and changed the empty, disabled zone selector to a hidden field, which (I think) neatly works around #36 and also improves the UI.

This looks ready to go now, so should we merge into 7.x-3.x, and maybe release another beta?

Almost there. I think I've tested everything mentioned above. The only remaining problem I see is this:
Fill in Delivery information
Check "My billing information is the same"
Change Delivery country and zone
Uncheck "My billing information is the same" and you will see that the new Country has been copied into the billing pane but the new zone has *not* been copied. If you try to submit the order at this time there's a form validation error because the billing zone isn't set.

Seems to work fine when using a country with no zones. I like how you've disappeared the zone when not needed.

I'm all in favor of a new beta. In fact, we can probably call it good enough for a non-beta at this point. What we need now is some serious use to shake out any remaining issues, and the only way we're going to get that is to put out a non-beta release.

The zone copying issue should be fixed by 4301d06.

That fixed it for me. As far as I'm concerned this is ready to go. Great work! Let the merge begin ...

Here's the whole changeset as a single patch against current 7.x-3.x. Scanning it briefly I think there are some old API functions and bits of JavaScript that can still be cleaned up, and the new address element could do with better documentation, but it can probably be committed as-is.

It would however be useful to finally test this against all the other recent changes from 7.x-3.x, so setting back to "needs review".

I think that's the wrong way to do it. The patch should be made with format-patch to preserve the intermediate commits and their commit messages. The 1071332/ajax-addresses branch could be rebased to 7.x-3.x head, or you could merge 7.x-3.x into 1071332/ajax-addresses before you make the patch. Or what I did on my local copy of 1071332/ajax-addresses was to cherry-pick the commits from 7.x-3.x and apply them to 1071332/ajax-addresses. That's what I've been testing against, so my comment in #52 about this being ready to go is still true with all the other recent changes in 7.x-3.x (I haven't cherry-picked today's commits yet, though). I can push my cherry-picks to the repository if you want.

you could merge 7.x-3.x into 1071332/ajax-addresses before you make the patch

This is what I did, but then I used git diff to make the patch; it was only meant so other people can test quickly if needed, and in this instance I found reading a single diff easier than a bunch of separate commits.

Anyway, I've pushed the merge (72ecdfb), so this can be marked fixed.

Awesome.

There's a bug introduced by this patch, which only occurs when all your enabled shipping methods require calculations. See #1172088: Checkout > You must select a shipping option before continuing. for details. You can avoid the problem by enabling at least one flatrate or one weightquote method.

The cause appears to be that, with the new Ajax stuff, changing the fields in the delivery or billing panes results in a submission of the entire checkout form. If the delivery address is not completely filled out yet, there will not be enough information available to get a calculated quote, so there will be no shipping options. When this implicit submission occurs, then, there will be no shipping option selected, leading to the dreaded "you must select a shipping option" error. This error does not show when there's a flatrate or weightquote because those options will always be present, regardless of the contents of the delivery pane, so there will always be a default shipping option available when the implicit submission is made.

My tests in this thread were made with all available shipping methods enabled, so I always had a flatrate and a weightquote in addition to UPS, USPS, FedEx, and Canada Post. That's why I never found this bug.

Bumping up priority to major because this patch is now distributed in 7.x-3.0-beta3.

Wondering if there is any documentation on this?
I used to be able to call uc_country_select() or uc_zone_select() from my custom module to get nice ajax dropdowns... is it still that simple?

@jackread: Not yet, but for now if you look at uc_google_checkout_shipping_settings() you should see how to create an Ajax-enabled address field:

  $form['address'] = array(
    '#type' => 'uc_address',
    '#default_value' => array(
      'uc_google_checkout_delivery_city' => variable_get('uc_google_checkout_delivery_city', ''),
      'uc_google_checkout_delivery_zone' => variable_get('uc_google_checkout_delivery_zone', ''),
      'uc_google_checkout_delivery_country' => isset($form_state['values']['uc_google_checkout_delivery_country']) ? $form_state['values']['uc_google_checkout_delivery_country'] : variable_get('uc_google_checkout_delivery_country', ''),
      'uc_google_checkout_delivery_postal_code' => variable_get('uc_google_checkout_delivery_postal_code', ''),
    ),
    '#required' => FALSE,
    '#key_prefix' => 'uc_google_checkout_delivery',
  );

This will give you a partial address form for city, state, country and postal code, with the values returned in $form_state['values']['uc_google_checkout_delivery_city'] etc.

@longwave - thanks a lot!

@longwave - you're input was great - thanks again. Having trouble with the zone dropdown. this is my code...

  $form['address']['fields'] = array(
      '#type' => 'uc_address',
      '#default_value' => array(
        'address_street1' => isset($form_state['values']['address_street1']) ? $form_state['values']['address_street1'] : $contact['address_street1'],
        'address_street2' => isset($form_state['values']['address_street2']) ? $form_state['values']['address_street2'] : $contact['address_street2'],
        'address_city' => isset($form_state['values']['address_city']) ? $form_state['values']['address_city'] : $contact['address_city'],
        'address_zone' => isset($form_state['values']['address_zone']) ? $form_state['values']['address_zone'] : $contact['address_zone'],
        'address_country' => isset($form_state['values']['address_country']) ? $form_state['values']['address_country'] : $contact['address_country'],
        'address_postal_code' => isset($form_state['values']['address_postal_code']) ? $form_state['values']['address_postal_code'] : $contact['address_postal_code'],
      ),
      '#required' => FALSE,
      '#key_prefix' => 'address',
    );

when I print_r($form_state['values']); address_zone comes up blank.

UPDATE:When I change country it seems to prevent my submit function from triggering. If I submit with no country change it works fine.

@TR: Please test the attached patch against the issue you describe in #58.

@jackread: I think there are still some bugs in the uc_address element, I need to do more testing.

@longwave: Yes, the patch fixes the issue of the form_set_error() messages showing up within the Delivery information pane while the customer is in the process of entering address information. The shipping pane still shows "There were problems" during the entering process, until all the address information required by the shipping quotes is entered. That's potentially confusing. With the addition of the Ajax, I think "There were problems" should now only be displayed as a result of submitting the checkout form from the "Submit" button or from pressing the "Calculate shipping" button. If the quotes are triggered automatically by the customer changing address information the default "Shipping quotes are generated automatically" message should remain in place.

The quote pane in the cart doesn't work correctly, it always says state/province is required even if you select one. It is related to this code:

        // Avoid "illegal choice detected" errors when country field is changed.
        if ($form_state['process_input'] && !$form_state['submitted'] && isset($value[$prefix . 'zone']) && !isset($subelement['#options'][$value[$prefix . 'zone']])) {
          $subelement['#value'] = 0;
        }

Removing this makes the quote pane work in the cart, but then you get the "Illegal choice" error when changing the country field.

Committed #63. Back to "needs work" due to #65.

Also introduced the bug described in #1191168: Under Admin -> Store - Configuration -> Store Address, the field State/Province is not saved.

Tagging and escalating priority.

Marked #1193266: Shipping Quote Doesn't Recogize Drop Down Selection as duplicate, and corrected tag spelling.

This appears to be caused by a combination of problems with the Ajax address field implementation, in that we weren't storing updated values on Ajax submission for future form rebuilds, and #parents was wrong for the address subelements, so we weren't always seeing the correct values on submit anyway.

The patch worked for me, thanks.

Patch in #70 fixes the issues raised in #65, #67, and #69.

But there's a new problem. When choosing a saved address on the checkout page, if the saved address is a different country than the store address, the state/province select doesn't get set, in fact it doesn't even show the right selection. Specifically, my test store is in the US. My saved address is in Canada. Selecting the saved address shows Canada in the country select, but the state/province select still contains the US states.

The other remaining problem that I know about is #64, which is also described in #1197324: When first hitting Checkout, shipping always reports error.

Committed #70. Leaving open so #64 and #72 can be fixed.

#72 should be fixed by http://drupalcode.org/project/ubercart.git/commitdiff/a5b5b3d

I am tempted to reopen #1197324: When first hitting Checkout, shipping always reports error and deal with that separately, as it's technically not related to just the State/Province field, it's a separate issue in uc_quote.

Reopened #1197324: When first hitting Checkout, shipping always reports error with a patch, so marking this fixed again. Please reopen if there are further problems specifically related to the new address fields.

Patch in #70 worked for me.

Perhaps worth noting: it looks like everyone else had this problem on the Checkout page but for me the State/Province field wasn't saving on the Edit Orders page (admin/store/orders/#/edit) or on the Store settings page (admin/store/settings/store).

Commit a5b5b3d (see comment #74) broke the "Cart and Checkout" simpletests. I used git bisect to find this. It will be nice when the testbot starts working properly for contributed modules, so we can detect these test failures before we commit them.

I'm having a similar problem: USPS shipping quotes won't work on IE7 or IE8, but they work in all other browsers (including IE9). Looks like it's a problem with ajax not working. The kicker: I got it to work on my dev server, but not my production server! If you want to try it for yourself: http://calweasel.com

Any thoughts?

@koppie: Please don't post the same thing in multiple issues. The problem described by the original poster has been fixed in 7.x-3.x-dev. This current issue is still open only because it seems to have broken the test cases (which now need to be re-written to deal with the new Ajax checkout page).

Tests run green now. Commit 7813ce9ef3.

I am reopening this without changing the status.
Actually the address format per country is defined in the .cif files. So that the country selection should reshape the whole according to what is described in the .cif file.

For some areas like Hong Kong, Macau or Ireland, there is no zip code, whereas for some other places the zip is really mandatory. It makes the configuration of the shop broken for international sales.

Having a non existing field mandatory leaves the client locked.

That is not really related to this issue. See #344706-8: Change the order of address fields and later posts for discussion about changing the address fields based on the CIF file, and why it was decided that was not a good idea. If you want to continue discussing this please open a new issue.

Similar problem in 6.x-2.12