From f55fb370f3aee969ddbc9b19be232c36ef6cf9d8 Mon Sep 17 00:00:00 2001 From: omar abu hussein Date: Mon, 26 Dec 2016 14:38:23 +0000 Subject: [PATCH] CRM-19801: Fixing changing a relationship type from A to B or vice versa If you tried to change (A) relationship type of an exiting relationship (Like parent of) to another (B) relationship type (Like Child of) or vice versa then it will show you that the relationship get updated but it will not be it actually get updated. The issue is related to how postProcess handle contact_id_a & contact_id_b paramterters . So in this PR I've refactored the postProcess method for the relationship form and with this refactoring I've have fixed this issue too by properly hanlding conact_id_a and contact_id_b parameters. --- CRM/Contact/Form/Relationship.php | 273 +++++++++++++++++------------- 1 file changed, 160 insertions(+), 113 deletions(-) diff --git a/CRM/Contact/Form/Relationship.php b/CRM/Contact/Form/Relationship.php index f764c3eac9..e698fcd944 100644 --- a/CRM/Contact/Form/Relationship.php +++ b/CRM/Contact/Form/Relationship.php @@ -363,87 +363,18 @@ class CRM_Contact_Form_Relationship extends CRM_Core_Form { // Store the submitted values in an array. $params = $this->controller->exportValues($this->_name); - // CRM-14612 - Don't use adv-checkbox as it interferes with the form js - $params['is_permission_a_b'] = CRM_Utils_Array::value('is_permission_a_b', $params, 0); - $params['is_permission_b_a'] = CRM_Utils_Array::value('is_permission_b_a', $params, 0); - - // action is taken depending upon the mode - if ($this->_action & CRM_Core_Action::DELETE) { - CRM_Contact_BAO_Relationship::del($this->_relationshipId); - - // CRM-15881 UPDATES - // Since the line above nullifies the organization_name and employer_id fiels in the contact record, we need to reload all blocks to reflect this chage on the user interface. - $this->ajaxResponse['reloadBlocks'] = array('#crm-contactinfo-content'); - - return; - } - - $relationshipTypeParts = explode('_', $params['relationship_type_id']); - $params['relationship_type_id'] = $relationshipTypeParts[0]; - if (!$this->_rtype) { - // Do we need to wrap this in an if - when is rtype used & is relationship_type_id always set then? - $this->_rtype = $params['relationship_type_id']; - } - $params['contact_id_' . $relationshipTypeParts[1]] = $this->_contactId; + switch ($this->getAction()) { + case CRM_Core_Action::DELETE: + $this->deleteAction($this->_relationshipId); + return; - // Update mode (always single) - if ($this->_action & CRM_Core_Action::UPDATE) { - $update = TRUE; - $params['id'] = $this->_relationshipId; - $ids['relationship'] = $this->_relationshipId; - $relation = CRM_Contact_BAO_Relationship::getRelationshipByID($this->_relationshipId); - if ($relation->contact_id_a == $this->_contactId) { - // I couldn't replicate this path in testing. See below. - $params['contact_id_a'] = $this->_contactId; - $params['contact_id_b'] = array($params['related_contact_id']); - $outcome = CRM_Contact_BAO_Relationship::createMultiple($params, $relationshipTypeParts[1]); - $relationshipIds = $outcome['relationship_ids']; - } - else { - // The only reason we have changed this to use the api & not the above is that this was broken. - // Recommend extracting all of update into a function that uses the api - // and ensuring api / bao take care of 'other stuff' in this form - // the contact_id_a & b can't be changed on this form so don't really need setting. - $params['contact_id_b'] = $this->_contactId; - $params['contact_id_a'] = $params['related_contact_id']; - $result = civicrm_api3('relationship', 'create', $params); - $relationshipIds = array($result['id']); - } - $ids['contactTarget'] = ($relation->contact_id_a == $this->_contactId) ? $relation->contact_id_b : $relation->contact_id_a; + case CRM_Core_Action::UPDATE: + list ($params, $relationshipIds) = $this->updateAction($params); + break; - // @todo this belongs in the BAO. - if ($this->_isCurrentEmployer) { - // if relationship type changes, relationship is disabled, or "current employer" is unchecked, - // clear the current employer. CRM-3235. - $relChanged = $params['relationship_type_id'] != $this->_values['relationship_type_id']; - if (!$params['is_active'] || !$params['is_current_employer'] || $relChanged) { - - // CRM-15881 UPDATES - // If not is_active then is_current_employer needs to be set false as well! Logically a contact cannot be a current employee of a disabled employer relationship. - // If this is not done, then the below process will go ahead and disable the organization_name and employer_id fields in the contact record (which is what is wanted) but then further down will be re-enabled becuase is_current_employer is not false, therefore undoing what was done correctly. - if (!$params['is_active']) { - $params['is_current_employer'] = FALSE; - } - - CRM_Contact_BAO_Contact_Utils::clearCurrentEmployer($this->_values['contact_id_a']); - // Refresh contact summary if in ajax mode - $this->ajaxResponse['reloadBlocks'] = array('#crm-contactinfo-content'); - } - } - if (empty($outcome['saved']) && !empty($update)) { - $outcome['saved'] = $update; - } - $this->setMessage($outcome); - } - // Create mode (could be 1 or more relationships) - else { - $params['contact_id_' . $relationshipTypeParts[2]] = explode(',', $params['related_contact_id']); - $outcome = CRM_Contact_BAO_Relationship::createMultiple($params, $relationshipTypeParts[1]); - $relationshipIds = $outcome['relationship_ids']; - if (empty($outcome['saved']) && !empty($update)) { - $outcome['saved'] = $update; - } - $this->setMessage($outcome); + default: + list ($params, $relationshipIds) = $this->createAction($params); + break; } // if this is called from case view, @@ -453,49 +384,18 @@ class CRM_Contact_Form_Relationship extends CRM_Core_Form { CRM_Case_BAO_Case::createCaseRoleActivity($this->_caseId, $relationshipIds, $params['contact_check'], $this->_contactId); } - // Save notes // @todo this belongs in the BAO. - if ($this->_action & CRM_Core_Action::UPDATE || $params['note']) { - foreach ($relationshipIds as $id) { - $noteParams = array( - 'entity_id' => $id, - 'entity_table' => 'civicrm_relationship', - ); - $existing = civicrm_api3('note', 'get', $noteParams); - if (!empty($existing['id'])) { - $noteParams['id'] = $existing['id']; - } - $noteParams['note'] = $params['note']; - $noteParams['contact_id'] = $this->_contactId; - if (!empty($existing['id']) || $params['note']) { - $action = $params['note'] ? 'create' : 'delete'; - civicrm_api3('note', $action, $noteParams); - } - } + if (isset($params['note'])) { + $this->saveRelationshipNotes($relationshipIds, $params['note']); } - $params['relationship_ids'] = $relationshipIds; + $this->setEmploymentRelationship($params, $relationshipIds); // Refresh contact tabs which might have been affected $this->ajaxResponse['updateTabs'] = array( '#tab_member' => CRM_Contact_BAO_Contact::getCountComponent('membership', $this->_contactId), '#tab_contribute' => CRM_Contact_BAO_Contact::getCountComponent('contribution', $this->_contactId), ); - - // Set current employee/employer relationship, CRM-3532 - if ($params['is_current_employer'] && $this->_allRelationshipNames[$params['relationship_type_id']]["name_a_b"] == - 'Employee of') { - $employerParams = array(); - foreach ($relationshipIds as $id) { - // Fixme this is dumb why do we have to look this up again? - $rel = CRM_Contact_BAO_Relationship::getRelationshipByID($id); - $employerParams[$rel->contact_id_a] = $rel->contact_id_b; - } - // @todo this belongs in the BAO. - CRM_Contact_BAO_Contact_Utils::setCurrentEmployer($employerParams); - // Refresh contact summary if in ajax mode - $this->ajaxResponse['reloadBlocks'] = array('#crm-contactinfo-content'); - } } /** @@ -590,4 +490,151 @@ class CRM_Contact_Form_Relationship extends CRM_Core_Form { return $jsData; } + /** + * Handling 'delete relationship' action + * + * @param int $id + * Relationship ID + */ + private function deleteAction($id) { + CRM_Contact_BAO_Relationship::del($id); + + // reload all blocks to reflect this change on the user interface. + $this->ajaxResponse['reloadBlocks'] = array('#crm-contactinfo-content'); + } + + /** + * Handling updating relationship action + * + * @param array $params + * + * @return array + */ + private function updateAction($params) { + $params = array_shift($this->preparePostProcessParameters($params)); + + CRM_Contact_BAO_Relationship::create($params); + + $this->clearCurrentEmployer($params); + + $this->setMessage(array('saved' => TRUE)); + + return array($params, array($this->_relationshipId)); + } + + /** + * Handling creating relationship action + * + * @param array $params + * + * @return array + */ + private function createAction($params) { + list($params, $primaryContactLetter) = $this->preparePostProcessParameters($params); + + $outcome = CRM_Contact_BAO_Relationship::createMultiple($params, $primaryContactLetter); + + $relationshipIds = $outcome['relationship_ids']; + + $this->setMessage($outcome); + + return array($params, $relationshipIds); + } + + + /** + * Prepares parameters to be used for create/update actions + * + * @param array $params + * + * @return array + */ + private function preparePostProcessParameters($params) { + $relationshipTypeParts = explode('_', $params['relationship_type_id']); + + $params['relationship_type_id'] = $relationshipTypeParts[0]; + $params['contact_id_' . $relationshipTypeParts[1]] = $this->_contactId; + + if (empty($this->_relationshipId)) { + $params['contact_id_' . $relationshipTypeParts[2]] = explode(',', $params['related_contact_id']); + } + else { + $params['id'] = $this->_relationshipId; + $params['contact_id_' . $relationshipTypeParts[2]] = $params['related_contact_id']; + } + + // CRM-14612 - Don't use adv-checkbox as it interferes with the form js + $params['is_permission_a_b'] = CRM_Utils_Array::value('is_permission_a_b', $params, 0); + $params['is_permission_b_a'] = CRM_Utils_Array::value('is_permission_b_a', $params, 0); + + return array($params, $relationshipTypeParts[1]); + } + + /** + * Updates/Creates relationship notes + * + * @param array $relationshipIds + * @param string $note + */ + private function saveRelationshipNotes($relationshipIds, $note) { + foreach ($relationshipIds as $id) { + $noteParams = array( + 'entity_id' => $id, + 'entity_table' => 'civicrm_relationship', + ); + $existing = civicrm_api3('note', 'get', $noteParams); + if (!empty($existing['id'])) { + $noteParams['id'] = $existing['id']; + } + $noteParams['note'] = $note; + $noteParams['contact_id'] = $this->_contactId; + if (!empty($existing['id']) || $note) { + $action = $note ? 'create' : 'delete'; + civicrm_api3('note', $action, $noteParams); + } + } + } + + /** + * Sets current employee/employer relationship + * + * @param $params + * @param array $relationshipIds + */ + private function setEmploymentRelationship($params, $relationshipIds) { + if ( + isset($params['is_current_employer']) && + $this->_allRelationshipNames[$params['relationship_type_id']]["name_a_b"] == 'Employee of') { + $employerParams = array(); + foreach ($relationshipIds as $id) { + // Fixme this is dumb why do we have to look this up again? + $rel = CRM_Contact_BAO_Relationship::getRelationshipByID($id); + $employerParams[$rel->contact_id_a] = $rel->contact_id_b; + } + // @todo this belongs in the BAO. + CRM_Contact_BAO_Contact_Utils::setCurrentEmployer($employerParams); + // Refresh contact summary if in ajax mode + $this->ajaxResponse['reloadBlocks'] = array('#crm-contactinfo-content'); + } + } + + /** + * Clears the current employer if the relationship type + * get changed, disabled or 'current employer' checkbox get unchecked. + * + * @param $params + */ + private function clearCurrentEmployer($params) { + // @todo this belongs in the BAO. + if ($this->_isCurrentEmployer) { + $relChanged = $params['relationship_type_id'] != $this->_values['relationship_type_id']; + if (!$params['is_active'] || !$params['is_current_employer'] || $relChanged) { + CRM_Contact_BAO_Contact_Utils::clearCurrentEmployer($this->_values['contact_id_a']); + + // Refresh contact summary if in ajax mode + $this->ajaxResponse['reloadBlocks'] = array('#crm-contactinfo-content'); + } + } + } + } -- 2.25.1