From 2b139d1bfb55457662d5eabcd9baa9e7b82195bb Mon Sep 17 00:00:00 2001
From: Coleman Watts <coleman@civicrm.org>
Date: Fri, 23 Apr 2021 10:55:45 -0400
Subject: [PATCH] APIv4 - Deprecate nonstandard syntax for implicit joins

The APIv4 prototype used a string-manipulation hack to make implicit joins appear more friendly,
e.g. it would transform `contact_id.*` to `contact.*`.
However this made the generation of such joins less predictable,
as not all FK fields end in `_id` (notably, custom ContactRef fields do not).

This preserves the nonstandard syntax for backward-compat while favoring the new syntx
in the API Explorer.
---
 Civi/Api4/Action/Entity/GetLinks.php          |  4 +++-
 .../Api4/Service/Schema/Joinable/Joinable.php | 22 +++++++++++++++++++
 Civi/Api4/Service/Schema/SchemaMapBuilder.php | 12 +++++++---
 .../phpunit/api/v4/Entity/ContactJoinTest.php | 20 +++++++++++++++--
 4 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/Civi/Api4/Action/Entity/GetLinks.php b/Civi/Api4/Action/Entity/GetLinks.php
index 2d224da335..59b152eab7 100644
--- a/Civi/Api4/Action/Entity/GetLinks.php
+++ b/Civi/Api4/Action/Entity/GetLinks.php
@@ -40,7 +40,9 @@ class GetLinks extends \Civi\Api4\Generic\BasicGetAction {
           'links' => [],
         ];
         foreach ($table->getTableLinks() as $link) {
-          $item['links'][] = $link->toArray();
+          if (!$link->isDeprecated()) {
+            $item['links'][] = $link->toArray();
+          }
         }
         $result[] = $item;
       }
diff --git a/Civi/Api4/Service/Schema/Joinable/Joinable.php b/Civi/Api4/Service/Schema/Joinable/Joinable.php
index 8e6d4e7116..b45b8c6856 100644
--- a/Civi/Api4/Service/Schema/Joinable/Joinable.php
+++ b/Civi/Api4/Service/Schema/Joinable/Joinable.php
@@ -78,6 +78,11 @@ class Joinable {
    */
   protected $entity;
 
+  /**
+   * @var bool
+   */
+  protected $deprecated = FALSE;
+
   /**
    * @param $targetTable
    * @param $targetColumn
@@ -252,6 +257,23 @@ class Joinable {
     return $this;
   }
 
+  /**
+   * @return bool
+   */
+  public function isDeprecated() {
+    return $this->deprecated;
+  }
+
+  /**
+   * @param bool $deprecated
+   *
+   * @return $this
+   */
+  public function setDeprecated(bool $deprecated = TRUE) {
+    $this->deprecated = $deprecated;
+    return $this;
+  }
+
   /**
    * @return array
    */
diff --git a/Civi/Api4/Service/Schema/SchemaMapBuilder.php b/Civi/Api4/Service/Schema/SchemaMapBuilder.php
index 77666cb4a7..752e729caa 100644
--- a/Civi/Api4/Service/Schema/SchemaMapBuilder.php
+++ b/Civi/Api4/Service/Schema/SchemaMapBuilder.php
@@ -91,9 +91,15 @@ class SchemaMapBuilder {
     if ($fkClass) {
       $tableName = AllCoreTables::getTableForClass($fkClass);
       $fkKey = $data['FKKeyColumn'] ?? 'id';
-      // Fixme: Clumsy string manipulation to transform e.g. "contact_id" to "contact" - we never should have done this
-      $alias = str_replace('_id', '', $field);
-      $joinable = new Joinable($tableName, $fkKey, $alias);
+      // Backward-compatibility for older api calls using e.g. "contact" instead of "contact_id"
+      if (strpos($field, '_id')) {
+        $alias = str_replace('_id', '', $field);
+        $joinable = new Joinable($tableName, $fkKey, $alias);
+        $joinable->setJoinType($joinable::JOIN_TYPE_MANY_TO_ONE);
+        $joinable->setDeprecated();
+        $table->addTableLink($field, $joinable);
+      }
+      $joinable = new Joinable($tableName, $fkKey, $field);
       $joinable->setJoinType($joinable::JOIN_TYPE_MANY_TO_ONE);
       $table->addTableLink($field, $joinable);
     }
diff --git a/tests/phpunit/api/v4/Entity/ContactJoinTest.php b/tests/phpunit/api/v4/Entity/ContactJoinTest.php
index a97e6266cc..0c92abac54 100644
--- a/tests/phpunit/api/v4/Entity/ContactJoinTest.php
+++ b/tests/phpunit/api/v4/Entity/ContactJoinTest.php
@@ -46,14 +46,14 @@ class ContactJoinTest extends UnitTestCase {
     return parent::setUpHeadless();
   }
 
-  public function testContactJoin() {
-
+  public function testContactJoinDeprecated() {
     $contact = $this->getReference('test_contact_1');
     $entitiesToTest = ['Address', 'OpenID', 'IM', 'Website', 'Email', 'Phone'];
 
     foreach ($entitiesToTest as $entity) {
       $results = civicrm_api4($entity, 'get', [
         'where' => [['contact_id', '=', $contact['id']]],
+        // Deprecated syntax (new syntax is `contact_id.*` not `contact.*`)
         'select' => ['contact.*_name', 'contact.id'],
       ]);
       foreach ($results as $result) {
@@ -63,6 +63,22 @@ class ContactJoinTest extends UnitTestCase {
     }
   }
 
+  public function testContactJoin() {
+    $contact = $this->getReference('test_contact_1');
+    $entitiesToTest = ['Address', 'OpenID', 'IM', 'Website', 'Email', 'Phone'];
+
+    foreach ($entitiesToTest as $entity) {
+      $results = civicrm_api4($entity, 'get', [
+        'where' => [['contact_id', '=', $contact['id']]],
+        'select' => ['contact_id.*_name', 'contact_id.id'],
+      ]);
+      foreach ($results as $result) {
+        $this->assertEquals($contact['id'], $result['contact_id.id']);
+        $this->assertEquals($contact['display_name'], $result['contact_id.display_name']);
+      }
+    }
+  }
+
   public function testJoinToPCMWillReturnArray() {
     $contact = Contact::create()->setValues([
       'preferred_communication_method' => [1, 2, 3],
-- 
2.25.1