Steps to reproduce

  • update to commerce_addressbook 7.x-2.0-rc4
  • as a user with some addresses, none of which are default, visit user/UID/addressbook
  • click "set as default" on one of the addresses

The AJAX request completes, then the address disappears. Refreshing the page causes the default address to be displayed correctly.

I guess this is a regression after #1781548: Active Adress boxes, when the DIV #commerce-addressbook-billing-default was removed for users with no default addresses.

CommentFileSizeAuthor
#4 commerce_addressbook-fix-set-as-default-address-1951056-4.patch2 KBjsacksick
#2 commerce_addressbook-fix-set-as-default-address-1951056-2.patch1.98 KBjsacksick
#1 commerce_addressbook-fix-set-as-default-address-1951056-1.patch1.97 KBjsacksick

Comments

jsacksick’s picture

jsacksick commented
Status: Active » Needs review
StatusFileSize
new commerce_addressbook-fix-set-as-default-address-1951056-1.patch1.97 KB

You're right, this is a regression coming from #1781548: Active Adress boxes, the attached patch should fix the issue.

jsacksick’s picture

jsacksick commented
StatusFileSize
new commerce_addressbook-fix-set-as-default-address-1951056-2.patch1.98 KB

Same patch with an updated comment.

pjcdawkins’s picture

pjcdawkins commented
Status: Needs review » Needs work

+ // Check if the user already have defaults addresses before setting one.

The comment should have "has default addresses", and it's a bit confusing anyway (not sure how to fix).

+ $defaults_html = '<div id="commerce-addressbook-' . $customer_profile->type . '-default">'

Perhaps use drupal_html_id() for this sort of thing. It's not clear whether $customer_profile->type is sanitized.

Other than that the patch works for me - thanks!

jsacksick’s picture

jsacksick commented
Status: Needs work » Needs review
StatusFileSize
new commerce_addressbook-fix-set-as-default-address-1951056-4.patch2 KB

I updated the comment, I'm not sure we need to change anything for the $defaults_html, this doesn't need to be passed by drupal_html_id.

pjcdawkins’s picture

pjcdawkins commented
Status: Needs review » Reviewed & tested by the community

Fine with me.

jsacksick’s picture

jsacksick commented
Status: Reviewed & tested by the community » Needs work

This patch doesn't fix all the potential usecases.
Let me explain why :
You could break it by creating two addresses, then you'll end up with one default address and an other non default one.
Now if you remove the default address and set the other as "default" then you'll have an empty #commerce-addressbook-($customer_profile->type)-list div, we could potentially avoid that situation by forcing users to have a default address, so when you'd remove your default address we would set an other as default. I'm not 100% sure about that though.

jsacksick’s picture

jsacksick commented
Status: Needs work » Fixed

Committed, thanks for your help ! http://drupalcode.org/project/commerce_addressbook.git/commit/d11dc4c

Status: Fixed » Closed (fixed)

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