From 3f7deec2d1383a75849eee400f12615437e5c163 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 8 Mar 2021 17:37:54 +1300
Subject: [PATCH] Add api support for aggregate without group by

For example

SELECT SUM(amount) FROM civicrm_pledge

Should be doable with no group by if none is desired
---
 Civi/Api4/Query/Api4SelectQuery.php           | 17 +++++++--
 tests/phpunit/api/v4/Action/FkJoinTest.php    | 10 ++++--
 .../api/v4/Query/Api4SelectQueryTest.php      | 36 +++++++++++++++----
 3 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/Civi/Api4/Query/Api4SelectQuery.php b/Civi/Api4/Query/Api4SelectQuery.php
index e588e71085..411802b91a 100644
--- a/Civi/Api4/Query/Api4SelectQuery.php
+++ b/Civi/Api4/Query/Api4SelectQuery.php
@@ -88,7 +88,7 @@ class Api4SelectQuery {
     $this->api = $apiGet;
 
     // Always select ID of main table unless grouping by something else
-    $this->forceSelectId = !$this->getGroupBy() || $this->getGroupBy() === ['id'];
+    $this->forceSelectId = !$this->isAggregateQuery() || $this->getGroupBy() === ['id'];
 
     // Build field lists
     foreach ($this->api->entityFields() as $field) {
@@ -108,6 +108,19 @@ class Api4SelectQuery {
     $this->addExplicitJoins();
   }
 
+  protected function isAggregateQuery() {
+    if ($this->getGroupBy()) {
+      return TRUE;
+    }
+    foreach ($this->getSelect() as $sql) {
+      $classname = get_class(SqlExpression::convert($sql, TRUE));
+      if (method_exists($classname, 'getCategory') && $classname::getCategory() === SqlFunction::CATEGORY_AGGREGATE) {
+        return TRUE;
+      }
+    }
+    return FALSE;
+  }
+
   /**
    * Builds main final sql statement after initialization.
    *
@@ -381,7 +394,7 @@ class Api4SelectQuery {
    */
   protected function composeClause(array $clause, string $type) {
     // Pad array for unary operators
-    list($expr, $operator, $value) = array_pad($clause, 3, NULL);
+    [$expr, $operator, $value] = array_pad($clause, 3, NULL);
     if (!in_array($operator, CoreUtil::getOperators(), TRUE)) {
       throw new \API_Exception('Illegal operator');
     }
diff --git a/tests/phpunit/api/v4/Action/FkJoinTest.php b/tests/phpunit/api/v4/Action/FkJoinTest.php
index 7c52dfe9c6..faed114075 100644
--- a/tests/phpunit/api/v4/Action/FkJoinTest.php
+++ b/tests/phpunit/api/v4/Action/FkJoinTest.php
@@ -34,15 +34,19 @@ use Civi\Api4\Tag;
 class FkJoinTest extends UnitTestCase {
 
   public function setUpHeadless() {
+    $this->loadDataSet('DefaultDataSet');
+
+    return parent::setUpHeadless();
+  }
+
+  public function tearDown() {
     $relatedTables = [
       'civicrm_activity',
       'civicrm_phone',
       'civicrm_activity_contact',
     ];
     $this->cleanup(['tablesToTruncate' => $relatedTables]);
-    $this->loadDataSet('DefaultDataSet');
-
-    return parent::setUpHeadless();
+    parent::tearDown();
   }
 
   /**
diff --git a/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php b/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php
index f282733b72..5b84bc27f8 100644
--- a/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php
+++ b/tests/phpunit/api/v4/Query/Api4SelectQueryTest.php
@@ -19,6 +19,7 @@
 
 namespace api\v4\Query;
 
+use Civi\API\Request;
 use Civi\Api4\Query\Api4SelectQuery;
 use api\v4\UnitTestCase;
 
@@ -50,7 +51,7 @@ class Api4SelectQueryTest extends UnitTestCase {
     $phoneNum = $this->getReference('test_phone_1')['phone'];
     $contact = $this->getReference('test_contact_1');
 
-    $api = \Civi\API\Request::create('Phone', 'get', [
+    $api = Request::create('Phone', 'get', [
       'version' => 4,
       'checkPermissions' => FALSE,
       'select' => ['id', 'phone', 'contact.display_name', 'contact.first_name'],
@@ -64,8 +65,30 @@ class Api4SelectQueryTest extends UnitTestCase {
     $this->assertEquals($contact['display_name'], $firstResult['contact.display_name']);
   }
 
-  public function testInvaidSort() {
-    $api = \Civi\API\Request::create('Contact', 'get', [
+  /**
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   * @throws \Civi\API\Exception\NotImplementedException
+   */
+  public function testAggregateNoGroupBy(): void {
+    $api = Request::create('Pledge', 'get', [
+      'version' => 4,
+      'checkPermissions' => FALSE,
+      'select' => ['SUM(amount) AS SUM_amount'],
+    ]);
+    $query = new Api4SelectQuery($api);
+    $this->assertEquals(
+      'SELECT SUM(`a`.`amount`) AS `SUM_amount`
+FROM civicrm_pledge a',
+      trim($query->getSql())
+    );
+  }
+
+  /**
+   * @throws \Civi\API\Exception\NotImplementedException
+   */
+  public function testInvalidSort(): void {
+    $api = Request::create('Contact', 'get', [
       'version' => 4,
       'checkPermissions' => FALSE,
       'select' => ['id', 'display_name'],
@@ -74,12 +97,13 @@ class Api4SelectQueryTest extends UnitTestCase {
     ]);
     $query = new Api4SelectQuery($api);
     try {
-      $results = $query->run();
+      $query->run();
       $this->fail('An Exception Should have been raised');
     }
     catch (\API_Exception $e) {
     }
-    $api = \Civi\API\Request::create('Contact', 'get', [
+
+    $api = Request::create('Contact', 'get', [
       'version' => 4,
       'checkPermissions' => FALSE,
       'select' => ['id', 'display_name'],
@@ -88,7 +112,7 @@ class Api4SelectQueryTest extends UnitTestCase {
     ]);
     $query = new Api4SelectQuery($api);
     try {
-      $results = $query->run();
+      $query->run();
       $this->fail('An Exception Should have been raised');
     }
     catch (\API_Exception $e) {
-- 
2.25.1