Merge pull request #20457 from colemanw/id_field

APIv4 - Return id_field as part of Entity.get
civicrm · Jun 1, 2021 · a076350 · a076350
2 parents 19b3d24 + 482a26e
commit a076350
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 45 deletions.
diff --git a/Civi/Api4/Entity.php b/Civi/Api4/Entity.php
@@ -85,6 +85,10 @@ public static function getFields($checkPermissions = TRUE) {
           'name' => 'dao',
           'description' => 'Class name for dao-based entities',
         ],
+        [
+          'name' => 'id_field',
+          'description' => 'Name of unique identifier field (e.g. "id")',
+        ],
         [
           'name' => 'label_field',
           'description' => 'Field to show when displaying a record',

diff --git a/Civi/Api4/Generic/AbstractEntity.php b/Civi/Api4/Generic/AbstractEntity.php
@@ -139,6 +139,7 @@ public static function getInfo() {
       'type' => [self::stripNamespace(get_parent_class(static::class))],
       'paths' => static::getEntityPaths(),
       'class' => static::class,
+      'id_field' => 'id',
     ];
     // Add info for entities with a corresponding DAO
     $dao = \CRM_Core_DAO_AllCoreTables::getFullName($info['name']);

diff --git a/Civi/Api4/Generic/BasicEntity.php b/Civi/Api4/Generic/BasicEntity.php
@@ -132,8 +132,17 @@ public static function delete($checkPermissions = TRUE) {
    * @return BasicReplaceAction
    */
   public static function replace($checkPermissions = TRUE) {
-    return (new BasicReplaceAction(static::getEntityName(), __FUNCTION__))
+    return (new BasicReplaceAction(static::getEntityName(), __FUNCTION__, static::$idField))
       ->setCheckPermissions($checkPermissions);
   }
 
+  /**
+   * @inheritDoc
+   */
+  public static function getInfo() {
+    $info = parent::getInfo();
+    $info['id_field'] = static::$idField;
+    return $info;
+  }
+
 }
diff --git a/ext/afform/core/Civi/Api4/Afform.php b/ext/afform/core/Civi/Api4/Afform.php
@@ -224,4 +224,13 @@ public static function permissions() {
     ];
   }
 
+  /**
+   * @inheritDoc
+   */
+  public static function getInfo() {
+    $info = parent::getInfo();
+    $info['id_field'] = 'name';
+    return $info;
+  }
+
 }
diff --git a/tests/phpunit/api/v4/Action/BasicActionsTest.php b/tests/phpunit/api/v4/Action/BasicActionsTest.php
@@ -28,41 +28,47 @@
 class BasicActionsTest extends UnitTestCase {
 
   private function replaceRecords(&$records) {
-    MockBasicEntity::delete()->addWhere('id', '>', 0)->execute();
+    MockBasicEntity::delete()->addWhere('identifier', '>', 0)->execute();
     foreach ($records as &$record) {
-      $record['id'] = MockBasicEntity::create()->setValues($record)->execute()->first()['id'];
+      $record['identifier'] = MockBasicEntity::create()->setValues($record)->execute()->first()['identifier'];
     }
   }
 
+  public function testGetInfo() {
+    $info = MockBasicEntity::getInfo();
+    $this->assertEquals('MockBasicEntity', $info['name']);
+    $this->assertEquals('identifier', $info['id_field']);
+  }
+
   public function testCrud() {
-    MockBasicEntity::delete()->addWhere('id', '>', 0)->execute();
+    MockBasicEntity::delete()->addWhere('identifier', '>', 0)->execute();
 
-    $id1 = MockBasicEntity::create()->addValue('foo', 'one')->execute()->first()['id'];
+    $id1 = MockBasicEntity::create()->addValue('foo', 'one')->execute()->first()['identifier'];
 
     $result = MockBasicEntity::get()->execute();
     $this->assertCount(1, $result);
 
-    $id2 = MockBasicEntity::create()->addValue('foo', 'two')->execute()->first()['id'];
+    $id2 = MockBasicEntity::create()->addValue('foo', 'two')->execute()->first()['identifier'];
 
     $result = MockBasicEntity::get()->selectRowCount()->execute();
     $this->assertEquals(2, $result->count());
 
-    MockBasicEntity::update()->addWhere('id', '=', $id2)->addValue('foo', 'new')->execute();
+    MockBasicEntity::update()->addWhere('identifier', '=', $id2)->addValue('foo', 'new')->execute();
 
-    $result = MockBasicEntity::get()->addOrderBy('id', 'DESC')->setLimit(1)->execute();
+    $result = MockBasicEntity::get()->addOrderBy('identifier', 'DESC')->setLimit(1)->execute();
     // The object's count() method will account for all results, ignoring limit, while the array results are limited
     $this->assertCount(2, $result);
     $this->assertCount(1, (array) $result);
     $this->assertEquals('new', $result->first()['foo']);
 
     $result = MockBasicEntity::save()
-      ->addRecord(['id' => $id1, 'foo' => 'one updated', 'weight' => '5'])
-      ->addRecord(['id' => $id2, 'group:label' => 'Second'])
+      ->addRecord(['identifier' => $id1, 'foo' => 'one updated', 'weight' => '5'])
+      ->addRecord(['identifier' => $id2, 'group:label' => 'Second'])
       ->addRecord(['foo' => 'three'])
       ->addDefault('color', 'pink')
       ->setReload(TRUE)
       ->execute()
-      ->indexBy('id');
+      ->indexBy('identifier');
 
     $this->assertTrue(5 === $result[$id1]['weight']);
     $this->assertEquals('new', $result[$id2]['foo']);
@@ -73,7 +79,7 @@ public function testCrud() {
       $this->assertEquals('pink', $item['color']);
     }
 
-    $ent1 = MockBasicEntity::get()->addWhere('id', '=', $id1)->execute()->first();
+    $ent1 = MockBasicEntity::get()->addWhere('identifier', '=', $id1)->execute()->first();
     $this->assertEquals('one updated', $ent1['foo']);
     $this->assertFalse(isset($ent1['group:label']));
 
@@ -90,7 +96,7 @@ public function testCrud() {
     // This one wasn't selected but did get used by the WHERE clause; ensure it isn't returned
     $this->assertFalse(isset($ent2['group:label']));
 
-    MockBasicEntity::delete()->addWhere('id', '=', $id2);
+    MockBasicEntity::delete()->addWhere('identifier', '=', $id2);
     $result = MockBasicEntity::get()->execute();
     $this->assertEquals('one updated', $result->first()['foo']);
   }
@@ -107,24 +113,24 @@ public function testReplace() {
 
     // Keep red, change blue, delete green, and add yellow
     $replacements = [
-      ['color' => 'red', 'id' => $objects[0]['id']],
-      ['color' => 'not blue', 'id' => $objects[1]['id']],
+      ['color' => 'red', 'identifier' => $objects[0]['identifier']],
+      ['color' => 'not blue', 'identifier' => $objects[1]['identifier']],
       ['color' => 'yellow'],
     ];
 
     MockBasicEntity::replace()->addWhere('group', '=', 'one')->setRecords($replacements)->execute();
 
-    $newObjects = MockBasicEntity::get()->addOrderBy('id', 'DESC')->execute()->indexBy('id');
+    $newObjects = MockBasicEntity::get()->addOrderBy('identifier', 'DESC')->execute()->indexBy('identifier');
 
     $this->assertCount(4, $newObjects);
 
     $this->assertEquals('yellow', $newObjects->first()['color']);
 
-    $this->assertEquals('not blue', $newObjects[$objects[1]['id']]['color']);
+    $this->assertEquals('not blue', $newObjects[$objects[1]['identifier']]['color']);
 
     // Ensure group two hasn't been altered
-    $this->assertEquals('orange', $newObjects[$objects[3]['id']]['color']);
-    $this->assertEquals('two', $newObjects[$objects[3]['id']]['group']);
+    $this->assertEquals('orange', $newObjects[$objects[3]['identifier']]['color']);
+    $this->assertEquals('two', $newObjects[$objects[3]['identifier']]['group']);
   }
 
   public function testBatchFrobnicate() {
@@ -145,14 +151,14 @@ public function testGetFields() {
     $getFields = MockBasicEntity::getFields()->execute()->indexBy('name');
 
     $this->assertCount(7, $getFields);
-    $this->assertEquals('Id', $getFields['id']['title']);
+    $this->assertEquals('Identifier', $getFields['identifier']['title']);
     // Ensure default data type is "String" when not specified
     $this->assertEquals('String', $getFields['color']['data_type']);
 
     // Getfields should default to loadOptions = false and reduce them to bool
     $this->assertTrue($getFields['group']['options']);
     $this->assertTrue($getFields['fruit']['options']);
-    $this->assertFalse($getFields['id']['options']);
+    $this->assertFalse($getFields['identifier']['options']);
 
     // Load simple options
     $getFields = MockBasicEntity::getFields()
@@ -215,15 +221,15 @@ public function testFieldsToGet() {
     $this->assertTrue($isFieldSelected->invoke($get, 'size', 'color', 'shape'));
 
     // With a non-empty "select" fieldsToSelect() will return fields needed to evaluate each clause.
-    $get->addSelect('id');
+    $get->addSelect('identifier');
     $this->assertTrue($isFieldSelected->invoke($get, 'color', 'shape', 'size'));
-    $this->assertTrue($isFieldSelected->invoke($get, 'id'));
+    $this->assertTrue($isFieldSelected->invoke($get, 'identifier'));
     $this->assertFalse($isFieldSelected->invoke($get, 'shape', 'size', 'weight'));
     $this->assertFalse($isFieldSelected->invoke($get, 'group'));
 
     $get->addClause('OR', ['shape', '=', 'round'], ['AND', [['size', '=', 'big'], ['weight', '!=', 'small']]]);
     $this->assertTrue($isFieldSelected->invoke($get, 'color'));
-    $this->assertTrue($isFieldSelected->invoke($get, 'id'));
+    $this->assertTrue($isFieldSelected->invoke($get, 'identifier'));
     $this->assertTrue($isFieldSelected->invoke($get, 'shape'));
     $this->assertTrue($isFieldSelected->invoke($get, 'size'));
     $this->assertTrue($isFieldSelected->invoke($get, 'group', 'weight'));
@@ -242,7 +248,7 @@ public function testWildcardSelect() {
 
     foreach (MockBasicEntity::get()->addSelect('*')->execute() as $result) {
       ksort($result);
-      $this->assertEquals(['color', 'group', 'id', 'shape', 'size', 'weight'], array_keys($result));
+      $this->assertEquals(['color', 'group', 'identifier', 'shape', 'size', 'weight'], array_keys($result));
     }
 
     $result = MockBasicEntity::get()
@@ -262,39 +268,39 @@ public function testEmptyAndNullOperators() {
 
     $result = MockBasicEntity::get()
       ->addWhere('color', 'IS NULL')
-      ->execute()->indexBy('id');
+      ->execute()->indexBy('identifier');
     $this->assertCount(1, $result);
-    $this->assertArrayHasKey($records[0]['id'], (array) $result);
+    $this->assertArrayHasKey($records[0]['identifier'], (array) $result);
 
     $result = MockBasicEntity::get()
       ->addWhere('color', 'IS EMPTY')
-      ->execute()->indexBy('id');
+      ->execute()->indexBy('identifier');
     $this->assertCount(2, $result);
-    $this->assertArrayNotHasKey($records[2]['id'], (array) $result);
+    $this->assertArrayNotHasKey($records[2]['identifier'], (array) $result);
 
     $result = MockBasicEntity::get()
       ->addWhere('color', 'IS NOT EMPTY')
-      ->execute()->indexBy('id');
+      ->execute()->indexBy('identifier');
     $this->assertCount(1, $result);
-    $this->assertArrayHasKey($records[2]['id'], (array) $result);
+    $this->assertArrayHasKey($records[2]['identifier'], (array) $result);
 
     $result = MockBasicEntity::get()
       ->addWhere('weight', 'IS NULL')
-      ->execute()->indexBy('id');
+      ->execute()->indexBy('identifier');
     $this->assertCount(1, $result);
-    $this->assertArrayHasKey($records[0]['id'], (array) $result);
+    $this->assertArrayHasKey($records[0]['identifier'], (array) $result);
 
     $result = MockBasicEntity::get()
       ->addWhere('weight', 'IS EMPTY')
-      ->execute()->indexBy('id');
+      ->execute()->indexBy('identifier');
     $this->assertCount(2, $result);
-    $this->assertArrayNotHasKey($records[2]['id'], (array) $result);
+    $this->assertArrayNotHasKey($records[2]['identifier'], (array) $result);
 
     $result = MockBasicEntity::get()
       ->addWhere('weight', 'IS NOT EMPTY')
-      ->execute()->indexBy('id');
+      ->execute()->indexBy('identifier');
     $this->assertCount(1, $result);
-    $this->assertArrayHasKey($records[2]['id'], (array) $result);
+    $this->assertArrayHasKey($records[2]['identifier'], (array) $result);
   }
 
   public function testContainsOperator() {

diff --git a/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php b/tests/phpunit/api/v4/Mock/Api4/MockBasicEntity.php
@@ -28,6 +28,8 @@
  */
 class MockBasicEntity extends Generic\BasicEntity {
 
+  protected static $idField = 'identifier';
+
   protected static $getter = [MockEntityDataStorage::CLASS, 'get'];
   protected static $setter = [MockEntityDataStorage::CLASS, 'write'];
   protected static $deleter = [MockEntityDataStorage::CLASS, 'delete'];
@@ -39,7 +41,7 @@ public static function getFields($checkPermissions = TRUE) {
     return (new Generic\BasicGetFieldsAction(__CLASS__, __FUNCTION__, function() {
       return [
         [
-          'name' => 'id',
+          'name' => 'identifier',
           'data_type' => 'Integer',
         ],
         [
@@ -94,9 +96,9 @@ public static function getFields($checkPermissions = TRUE) {
    * @return Generic\BasicBatchAction
    */
   public static function batchFrobnicate($checkPermissions = TRUE) {
-    return (new Generic\BasicBatchAction(__CLASS__, __FUNCTION__, ['id', 'number'], function($item) {
+    return (new Generic\BasicBatchAction(__CLASS__, __FUNCTION__, ['identifier', 'number'], function($item) {
       return [
-        'id' => $item['id'],
+        'identifier' => $item['identifier'],
         'frobnication' => $item['number'] * $item['number'],
       ];
     }))->setCheckPermissions($checkPermissions);

diff --git a/tests/phpunit/api/v4/Mock/MockEntityDataStorage.php b/tests/phpunit/api/v4/Mock/MockEntityDataStorage.php
@@ -33,18 +33,18 @@ public static function get() {
   }
 
   public static function write($record) {
-    if (empty($record['id'])) {
-      $record['id'] = self::$nextId++;
-      self::$data[$record['id']] = $record;
+    if (empty($record['identifier'])) {
+      $record['identifier'] = self::$nextId++;
+      self::$data[$record['identifier']] = $record;
     }
     else {
-      self::$data[$record['id']] = $record + self::$data[$record['id']];
+      self::$data[$record['identifier']] = $record + self::$data[$record['identifier']];
     }
     return $record;
   }
 
   public static function delete($record) {
-    unset(self::$data[$record['id']]);
+    unset(self::$data[$record['identifier']]);
     return $record;
   }