Simplify Location Types and is_primary / is_billing
Problem: feature bloat long ago on location type stuff leading to confusing experience for new users (and existing ones!).
Objective: improve the UX by simplifying the overlap between flags for is_primary and is_billing and Location Types that are Main and Billing.
Proposal:
- Retain: Work, Home, Other as Location Types in default install
- Retain: Primary and Billing Checkboxes on Address, IM, Phone, Email
- Retain: Primary Checkbox on Email
Remove:
- Main, Billing as Location Types in tarball, also all code uses and references to them.
Aim in most cases to use is_billing to determine which address is a billing address, and set is_billing=true on an address when it is being automatically created eg on an IPN callback. Similarly for any other automatic, non-UI uses in code of billing location for address, email, IM or phone. There is a possibility some code may need a different approach.
I don't expect that are any (or at least many) places where Main location is set automatically in code. If they exist, it should similarly be replaced by a use of is_primary. In this case, I expect that there would be less need to set is_primary, since it is generally used for the first created entity of a type (address, IM, email, phone), and then when the current is_primary is deleted, a different entity of same type gets sets to is_primary=1.
Note that there may be performance issues that need to be sorted out for retrieving in certain cases.
Upgrade
- Location Type of Main is not a significant source of confusion and isn't really used by default anywhere. I suggest that the upgrade check if there are any email, address, IM or phone entities with this Location Type; if there are, don't delete it else delete it.
- Location Type of Billing will have been set in most instances using contributions, and is_billing will also be set. There will be a change in how receipts determine which address to use. There may be workflows of organizations that rely on using Location Type of Billing. Leaving those location types unchanged is a partial approach that won't likely work, as the new code will set is_billing on addresses, etc that are not Location Type of Billing. I think the best approach in these kinds of situations is to create a LExIM extension that will sit alongside old code for a while, probably a long while in this case. This problem is likely why there has been no fix to this for 10+ years. Maybe we should just aim to improve it on new installs and those who want to adopt new approach, but provide support for old approach for a long time, perhaps through an extension that can be optionally enabled.
Discussion: There will likely need to be some work to decide what Location Type to use by default in various cases. For example, Billing Location Type is currently used on IPN callbacks from a payment processor at https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment/BaseIPN.php#L440 (note this is deprecated). I propose that contributions by individuals should default to using a Home address with is_billing=1 in these cases, and contributions by organizations would default to Work address with is_billing=1.
Interesting code references:
Change to use is_billing rather than location type at https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/BAO/LocationType.php#L90
Contribution and Event pages retrieve the billing address and billing name from here: https://github.com/civicrm/civicrm-core/blob/master/CRM//Core/Payment.php#L1006
Receipt current retrieves info based on location type rather than is_billing at https://github.com/civicrm/civicrm-core/blob/master/CRM//Contribute/BAO/ContributionPage.php#L289