Closed (fixed)
Project:
Ubercart Addresses
Version:
7.x-1.x-dev
Component:
API
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
4 May 2013 at 16:15 UTC
Updated:
30 May 2013 at 09:51 UTC
Jump to comment: Most recent file
Problem/Motivation
The address book API has a flaw regarding setting default addresses. Currently, when you set an address as the default billing or default shipping through the API, you should take care for saving all addresses or else multiple addresses get marked as the same default type.
Though this behaviour is documented, the API should not allow this.
I've found out that this can lead to serious problems when working on addresses with third party modules, such as with Views Bulk Operations (this module has an option to change a value of an entity and "default billing" is one of the values that can be changed on address entities).
Proposed resolution
When saving an address, the API should check if the address is a default address. If that's the case, it should execute an additional query to ensure no other addresses of the user's address book are marked as the same default type.
Remaining tasks
- Implement the proposed solution.
- Extend the API test to ensure the problem is solved.
User interface changes
None.
API changes
For module developers it will be no longer required to save all addresses when they set an address as the default address.
Related Issues
- #1309546: Address import / Feeds integration
The patch in #9 would benefit from this change.
I'm working on it.
Comments
Comment #1
megachrizAs it's hard to extend the API test at the moment, I've opened a new issue for refactoring that test: #1986800: Refactor UcAddressesApiTestCase. That issue should first be fixed before a patch with a test can be provided for this issue.
Comment #2
megachrizHere are two patches. The first patch includes the fix and a test. The second patch contains the test only to ensure the test is failing without the fix.
Comment #4
megachrizThe second patch in #2 was supposed to be failing tests. Back to "needs review".
Comment #5
megachrizIn addition of the patch in #2, this patch makes setting a default addresses using the API and saving an addresses using the UI slightly more efficient.
Previously, all addresses were loaded when setting an other address is default to be absolutely sure no other address is marked as the same default type. Now only the default address(es) are loaded.
And when saving a single address in the UI, always all addresses (of the address owner) were loaded and saved to prevent having two addresses marked as the same default type.
If this patch doesn't break tests, I think it's ready for commit.
Comment #6
megachrizCommitted #5.
Moving to 6.x-2.x.
Comment #7
megachrizPatch for the 6.x-2.x version.
Comment #8
megachrizCommitted #7.
Setting issue back to the version it was originally reported for and mark it as fixed.