CRM-21008 - Checkbox to explicitely change employer when doing shared address #12741
Conversation
|
(Standard links)
|
12739.diff
Outdated
| @@ -0,0 +1,260 @@ | |||
| diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php | |||
|
test fail relates |
|
I'm +1 on this change and happy to merge it when it's ready. |
|
@colemanw @eileenmcnaughton It seems that the change is breaking one test which is normal because i have assumed that we now need to be explicit about creating the relationship. Do you agree with that or do you prefer to keep the current behavior: i.e. by default, if no parameter is passed, a relationship is created ? |
CRM/Core/BAO/Address.php
Outdated
| @@ -1042,11 +1042,21 @@ public static function processSharedAddress($addressId, $params) { | |||
| $query = 'SELECT id, contact_id FROM civicrm_address WHERE master_id = %1'; | |||
| $dao = CRM_Core_DAO::executeQuery($query, array(1 => array($addressId, 'Integer'))); | |||
|
|
|||
| // TODO: VALIDATION REQUIRED = was implicit and is now explicit | |||
| // should we keep it backward compatible (i.e. no params means creating relationship) ? | |||
| // this should only impact API calls | |||
There was a problem hiding this comment.
Answer: yes, we need to keep it backwards compatible, so if the param is not set, treat it as if it were passed in TRUE.
|
@samuelsov since this breaks the API, yes. See code comment. |
|
@samuelsov -- plese remove the patch and diff files from this PR. |
|
oups (for the patch) and ok, i will make it backward compatible |
|
Backward compatibility code is not pretty but it works in the UI and in the API. The failing build doesn't seems to relate to the code. |
|
jenkins, test this please |
1 similar comment
|
jenkins, test this please |
|
jenkins, test this please (having issues with the test server) |
|
@colemanw anything blocking or can we merge ? |
|
@colemanw can we merge this? |
| @@ -164,6 +164,9 @@ public static function buildQuickForm(&$form, $addressBlockCount = NULL, $sharin | |||
| // shared address | |||
| $form->addElement('checkbox', "address[$blockId][use_shared_address]", NULL, ts('Use another contact\'s address')); | |||
|
|
|||
| // do we want to update employer for shared address | |||
| $form->addElement('checkbox', "address[$blockId][update_current_employer]", NULL, ts('Set this organization as current employer')); | |||
There was a problem hiding this comment.
Switch this to advcheckbox to always get a value back instead of undefined for unchecked.
There was a problem hiding this comment.
@samuelsov ^^ looks this is currently blocking
There was a problem hiding this comment.
@colemanw @eileenmcnaughton ok thanks, i was not aware of advcheckbox alternative.
|
test this please |
|
@samuelsov is this in your court to make a change now? |
|
ps @colemanw there seem to be a few bugs that suggest advCheckbox got broken in the 5.5 security release but I haven't investigated |
|
@eileenmcnaughton yes it is, thanks for the reminder. |
|
Hang on I think this can all be simplified without adding the extra ajax call to perform logic in php which could be done with js instead. |
|
@samuelsov see #13700 |
Overview
Following the work done #12574, adding a checkbox on shared addresses so that people can explicitly update the employer instead of assuming it's what the user wants.
Before
When doing a shared address on a individual with an organization address, CiviCRM always update the relationships to disable any current employee/employer relationship and set the shared address organization as the only current relationship.
After
When the checkbox is checked, it does exactly the same as before but if not checked, it won't mess with the current relationships.
Creating a new shared address:
Updating with or without checking the box:
Not an organization, no checkbox available:
Comments
The API has not been updated yet but the default behavior is to not create the relationship if the parameter is not set. I like the explicit behavior better but we might want to discuss this for backward compatibility.