Rationalise relationship validation

The previous relationship validation was
1) throw a message is no valid employer relationship - this seems to be done
in a place that would make the site unusable I've centralised this check into a
cached function that gets the id & throws the exception & is used from
places previously fetching it (there is one that
didn't throw an exception previously but I think enough places did
that extending to that place would not break the site any more if it didn't exist

2) complicated look up to determine if the contact types match the relationship's
expectations. Since we already know the relationship's expectations we
can simplify this to just check contact a is an individual and b is an org which is
much more reasonable. It doesn;t seem like this would ever actually be false so
I added a deprecation to be a bit vocal if it is

diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php
index 27e5446b28d2d21ac93e063186fbd7fe724c9bfb..74578de9bb96dfcb1dad3d5c27fd3d4ba7af0779 100644 (file)
--- a/CRM/Contact/BAO/Contact/Utils.php
+++ b/CRM/Contact/BAO/Contact/Utils.php
@@ -270,12 +270,13 @@ WHERE  id IN ( $idString )
         ])->execute()->first()['id'];
     }
 
-    // get the relationship type id of "Employee of"
-    $relTypeId = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_RelationshipType', 'Employee of', 'id', 'name_a_b');
-    if (!$relTypeId) {
-      throw new CRM_Core_Exception(ts("You seem to have deleted the relationship type 'Employee of'"));
+    $relTypeId = CRM_Contact_BAO_RelationshipType::getEmployeeRelationshipTypeID();
+    if (!CRM_Contact_BAO_Contact::getContactType($contactID) || !CRM_Contact_BAO_Contact::getContactType($organization)) {
+      // There doesn't seem to be any reason to think this would ever be true but there
+      // was a previous more complicated check.
+      CRM_Core_Error::deprecatedWarning('attempting to create an employer with invalid contact types is deprecated');
+      return;
     }
-
     // create employee of relationship
     [$duplicate, $relationshipIds]
       = self::legacyCreateMultiple($relTypeId, $organization, $contactID);
@@ -340,10 +341,6 @@ WHERE  id IN ( $idString )
       'contact_id_b' => $organizationID,
       'relationship_type_id' => $relationshipTypeID,
     ];
-    if (!CRM_Contact_BAO_Relationship::checkRelationshipType($contactFields['contact_id_a'], $contactFields['contact_id_b'],
-      $contactFields['relationship_type_id'])) {
-      return [0, []];
-    }
 
     if (
       CRM_Contact_BAO_Relationship::checkDuplicateRelationship(
@@ -460,10 +457,7 @@ WHERE id={$contactId}; ";
       //2. delete related membership.
 
       //get the relationship type id of "Employee of"
-      $relTypeId = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_RelationshipType', 'Employee of', 'id', 'name_a_b');
-      if (!$relTypeId) {
-        throw new CRM_Core_Exception(ts("You seem to have deleted the relationship type 'Employee of'"));
-      }
+      $relTypeId = CRM_Contact_BAO_RelationshipType::getEmployeeRelationshipTypeID();
       $relMembershipParams['relationship_type_id'] = $relTypeId . '_a_b';
       $relMembershipParams['contact_check'][$employerId] = 1;
 

diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php
index 208e0198f8154f94841c479347c9e9fb753cc3ac..ace3d020aaaad6873cac269da2198b1acf09797d 100644 (file)
--- a/CRM/Contact/BAO/Relationship.php
+++ b/CRM/Contact/BAO/Relationship.php
@@ -603,12 +603,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship {
         $sharedContact->id = $relationship->contact_id_a;
         $sharedContact->find(TRUE);
 
-        // CRM-15881 UPDATES
-        // changed FROM "...relationship->relationship_type_id == 4..." TO "...relationship->relationship_type_id == 5..."
-        // As the system should be looking for type "employer of" (id 5) and not "sibling of" (id 4)
-        // As suggested by @davecivicrm, the employee relationship type id is fetched using the CRM_Core_DAO::getFieldValue() class and method, since these ids differ from system to system.
-        $employerRelTypeId = CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_RelationshipType', 'Employee of', 'id', 'name_a_b');
-
+        $employerRelTypeId = CRM_Contact_BAO_RelationshipType::getEmployeeRelationshipTypeID();
         if ($relationship->relationship_type_id == $employerRelTypeId && $relationship->contact_id_b == $sharedContact->employer_id) {
           CRM_Contact_BAO_Contact_Utils::clearCurrentEmployer($relationship->contact_id_a);
         }

diff --git a/CRM/Contact/BAO/RelationshipType.php b/CRM/Contact/BAO/RelationshipType.php
index b7c5c755801a55072d12d41811904aedd5e3dbfc..17e38025c008225d71ff2f4c617ec67658d360f8 100644 (file)
--- a/CRM/Contact/BAO/RelationshipType.php
+++ b/CRM/Contact/BAO/RelationshipType.php
@@ -9,7 +9,9 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Relationship;
 use Civi\Api4\RelationshipType;
+use Civi\Core\Event\PreEvent;
 
 /**
  *
@@ -110,19 +112,21 @@ class CRM_Contact_BAO_RelationshipType extends CRM_Contact_DAO_RelationshipType
 
   /**
    * Callback for hook_civicrm_pre().
+   *
    * @param \Civi\Core\Event\PreEvent $event
-   * @throws CRM_Core_Exception
+   *
+   * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
    */
-  public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) {
+  public static function self_hook_civicrm_pre(PreEvent $event): void {
     if ($event->action === 'delete') {
       // need to delete all option value field before deleting group
-      \Civi\Api4\Relationship::delete(FALSE)
+      Relationship::delete(FALSE)
         ->addWhere('relationship_type_id', '=', $event->id)
         ->execute();
     }
   }
 
-
   /**
    * Get the id of the employee relationship, checking it is valid.
    *

diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php
index 7f4e62aec049e800bcb5d79df53ca0782e27da6e..f1a1536dc91f9f9587acc2a8624cd6c936b146ed 100644 (file)
--- a/CRM/Contribute/Form/Contribution/Confirm.php
+++ b/CRM/Contribute/Form/Contribution/Confirm.php
@@ -1324,7 +1324,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr
         \Civi\Api4\Relationship::create(FALSE)
           ->addValue('contact_id_a', $contactID)
           ->addValue('contact_id_b', $orgID)
-          ->addValue('relationship_type_id', CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_RelationshipType', 'Employee of', 'id', 'name_a_b'))
+          ->addValue('relationship_type_id', $relTypeId = CRM_Contact_BAO_RelationshipType::getEmployeeRelationshipTypeID())
           ->addValue('is_permission_a_b:name', 'View and update')
           ->execute();
       }

diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php
index 040ef4ad3272b898e6175512fd4e70ebf6d83d59..3062f66b89fb4c503fb3a36e06cd7cf373f733b0 100644 (file)
--- a/tests/phpunit/api/v3/ContactTest.php
+++ b/tests/phpunit/api/v3/ContactTest.php
@@ -909,8 +909,6 @@ class api_v3_ContactTest extends CiviUnitTestCase {
    * Test creating a current employer through API.
    *
    * Check it will re-activate a de-activated employer
-   *
-   * @throws \CRM_Core_Exception
    */
   public function testContactCreateDuplicateCurrentEmployerEnables(): void {
     // Set up  - create employer relationship.