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.


jsacksick’s picture

Status: Active » Needs review
1.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

Same patch with an updated comment.

pjcdawkins’s picture

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

Status: Needs work » Needs review
2 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

Status: Needs review » Reviewed & tested by the community

Fine with me.

jsacksick’s picture

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

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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