Simplify variables pass in & out of function

Rather than compiling an array, pass in the 3 values. Also
its now obvious that there is only a max of 1 contact_check in the function
so just return rather than 'continuing' (allowing us to de-loop in a more
whitespacey follow up)

diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php
index aac605be1970cd0c92888b030051915e22d791d3..6b2d129dc6fcfb9cd2a424593a1007b84dd4d70b 100644 (file)
--- a/CRM/Contact/BAO/Contact/Utils.php
+++ b/CRM/Contact/BAO/Contact/Utils.php
@@ -279,13 +279,8 @@ WHERE  id IN ( $idString )
       }
 
       // create employee of relationship
-      $relationshipParams = [
-        'is_active' => TRUE,
-        'relationship_type_id' => $relTypeId . '_a_b',
-        'contact_check' => [$organization => TRUE],
-      ];
       [$duplicate, $relationshipIds]
-        = self::legacyCreateMultiple($relationshipParams, $contactID);
+        = self::legacyCreateMultiple($relTypeId, $organization, $contactID);
 
       // In case we change employer, clean previous employer related records.
       if (!$previousEmployerID && !$newContact) {
@@ -300,7 +295,12 @@ WHERE  id IN ( $idString )
       // set current employer
       self::setCurrentEmployer([$contactID => $organization]);
 
-      $relationshipParams['relationship_ids'] = $relationshipIds;
+      $relationshipParams = [
+        'relationship_ids' => $relationshipIds,
+        'is_active' => 1,
+        'contact_check' => [$organization => TRUE],
+        'relationship_type_id' => $relTypeId . '_a_b',
+      ];
       // Handle related memberships. CRM-3792
       self::currentEmployerRelatedMembership($contactID, $organization, $relationshipParams, $duplicate, $previousEmployerID);
     }
@@ -315,16 +315,20 @@ WHERE  id IN ( $idString )
    * For multiple a new variant of this function needs to be written and migrated to as this is a bit
    * nasty
    *
-   * @param array $params
-   *   (reference ) an assoc array of name/value pairs.
+   * @param int $relationshipTypeID
+   * @param int $organization
    * @param int $contactID
    *
    * @return array
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  private static function legacyCreateMultiple(&$params, int $contactID) {
-    $invalid = $duplicate = 0;
+  private static function legacyCreateMultiple(int $relationshipTypeID, int $organization, int $contactID): array {
+    $params = [
+      'is_active' => TRUE,
+      'relationship_type_id' => $relationshipTypeID . '_a_b',
+      'contact_check' => [$organization => TRUE],
+    ];
     $ids = ['contact' => $contactID];
 
     $relationshipIds = [];
@@ -338,8 +342,7 @@ WHERE  id IN ( $idString )
       $contactFields = CRM_Contact_BAO_Relationship::setContactABFromIDs($params, $ids, $organizationID);
       $errors = CRM_Contact_BAO_Relationship::checkValidRelationship($contactFields, $ids, $organizationID);
       if ($errors) {
-        $invalid++;
-        continue;
+        return [0, []];
       }
 
       if (
@@ -350,8 +353,7 @@ WHERE  id IN ( $idString )
           $organizationID
         )
       ) {
-        $duplicate++;
-        continue;
+        return [1, []];
       }
 
       $singleInstanceParams = array_merge($params, $contactFields);
@@ -359,12 +361,9 @@ WHERE  id IN ( $idString )
       $relationshipIds[] = $relationship->id;
     }
 
-    // do not add to recent items for import, CRM-4399
-    if (!(!empty($params['skipRecentView']) || $invalid || $duplicate)) {
-      CRM_Contact_BAO_Relationship::addRecent($params, $relationship);
-    }
+    CRM_Contact_BAO_Relationship::addRecent($params, $relationship);
 
-    return [$duplicate, $relationshipIds];
+    return [0, $relationshipIds];
   }
 
   /**

diff --git a/tests/phpunit/api/v3/AddressTest.php b/tests/phpunit/api/v3/AddressTest.php
index b8acd353c02322f0f684dc0553e15a81d2d990ab..66e2f1c7c60b2e84e0729970623432ce4f0cf22c 100644 (file)
--- a/tests/phpunit/api/v3/AddressTest.php
+++ b/tests/phpunit/api/v3/AddressTest.php
@@ -609,20 +609,21 @@ class api_v3_AddressTest extends CiviUnitTestCase {
     $this->assertNotContains('Alabama', $result['values']);
   }
 
-  public function testUpdateSharedAddressWithCustomFields() {
+  /**
+   * Ensure an update to the second address doesn't cause error.
+   *
+   * Avoid "db error: already exists" when re-saving the custom fields.
+   */
+  public function testUpdateSharedAddressWithCustomFields(): void {
     $ids = $this->entityCustomGroupWithSingleFieldCreate(__FUNCTION__, __FILE__);
-
     $params = $this->_params;
-    $params['custom_' . $ids['custom_field_id']] = "custom string";
-
+    $params['custom_' . $ids['custom_field_id']] = 'custom string';
     $firstAddress = $this->callAPISuccess($this->_entity, 'create', $params);
-
     $contactIdB = $this->individualCreate();
 
     $secondAddressParams = array_merge(['contact_id' => $contactIdB, 'master_id' => $firstAddress['id']], $firstAddress);
     unset($secondAddressParams['id']);
     $secondAddress = $this->callAPISuccess('Address', 'create', $secondAddressParams);
-    // Ensure an update to the second address doesn't cause a "db error: already exists" when resaving the custom fields.
     $this->callAPISuccess('Address', 'create', ['id' => $secondAddress['id'], 'contact_id' => $contactIdB, 'master_id' => $firstAddress['id']]);
   }