From 1d470b758666648ede1c2d4cffdbeb1252ab3d18 Mon Sep 17 00:00:00 2001
From: Tim Otten <totten@civicrm.org>
Date: Thu, 22 Feb 2018 08:38:41 -0800
Subject: [PATCH] Extension.get API - Fix regression (4.7.13) in result
 filtering

Overview
--------

When one calls the `Extension.get` API, it should support filtering.  For
example, these commands would return only installed extensions or only
uninstalled extensions (respectively):

```
cv api Extension.get status=installed
cv api Extension.get status=uninstalled
```

Before
------

The implementation of `Extension.get` passes a list of all extensions down to
`_civicrm_api3_basic_array_get()` which is supposed to interpret any APIv3
options (such as filter-values/offsets/limits).  However, it confuses the
`return` list (i.e.  the names of any columns *that the user wants to see*) with the
*filterable* list (i.e.  the names of any columns *for which filtering is
permitted*).

After
-----

The implementation of `Extension.get` passes a list of all extensions down to
`_civicrm_api3_basic_array_get()`.  It also passes a fixed list of columns
which one might reasonably want to filter.

It might be nice to support filtering on all fields, but (a) several fields
wouldn't be sensible (because they're nested arrays) and (b) this provides a
smaller surface-area to maintain.

Comments
--------

The doc-block for `_civicrm_api3_basic_array_get()` describes the purpose of this field:

https://github.com/civicrm/civicrm-core/blob/4.7.0/api/v3/utils.php#L2390-L2391

Since ~v4.7.13, this line of code appears to have gone through several revisions, e.g.

 * The regression was originally introduced by 517dfba8b
 * Subsequent partial-fixes were d4c44c700, 9776f417, 34239e81, 525ccb68

CC @lucianov88  @twomice @tschuettler @seamuslee001 for review
---
 api/v3/Extension.php                   |  8 ++------
 tests/phpunit/api/v3/ExtensionTest.php | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/api/v3/Extension.php b/api/v3/Extension.php
index 3b9378e6c8..441a7f5a88 100644
--- a/api/v3/Extension.php
+++ b/api/v3/Extension.php
@@ -368,12 +368,8 @@ function civicrm_api3_extension_get($params) {
       $result[] = $info;
     }
   }
-  $options = _civicrm_api3_get_options_from_params($params);
-  $returnFields = !empty($options['return']) ? $options['return'] : array();
-  if (!in_array('id', $returnFields)) {
-    $returnFields = array_merge($returnFields, array('id'));
-  }
-  return _civicrm_api3_basic_array_get('Extension', $params, $result, 'id', $returnFields);
+  $filterableFields = array('id', 'key', 'type', 'status', 'path');
+  return _civicrm_api3_basic_array_get('Extension', $params, $result, 'id', $filterableFields);
 }
 
 /**
diff --git a/tests/phpunit/api/v3/ExtensionTest.php b/tests/phpunit/api/v3/ExtensionTest.php
index bb04b2a717..f9cdd0c992 100644
--- a/tests/phpunit/api/v3/ExtensionTest.php
+++ b/tests/phpunit/api/v3/ExtensionTest.php
@@ -92,6 +92,26 @@ class api_v3_ExtensionTest extends CiviUnitTestCase {
     $this->assertEquals(6, $result['count']);
   }
 
+  /**
+   * Filtering by status=installed or status=uninstalled should produce different results.
+   */
+  public function testExtensionGetByStatus() {
+    $installed = $this->callAPISuccess('extension', 'get', array('status' => 'installed'));
+    $uninstalled = $this->callAPISuccess('extension', 'get', array('status' => 'uninstalled'));
+
+    // If the filter works, then results should be strictly independent.
+    $this->assertEquals(
+      array(),
+      array_intersect(
+        CRM_Utils_Array::collect('key', $installed['values']),
+        CRM_Utils_Array::collect('key', $uninstalled['values'])
+      )
+    );
+
+    $all = $this->callAPISuccess('extension', 'get', array());
+    $this->assertEquals($all['count'], $installed['count'] + $uninstalled['count']);
+  }
+
   public function testGetMultipleExtensions() {
     $result = $this->callAPISuccess('extension', 'get', array('key' => array('test.extension.manager.paymenttest', 'test.extension.manager.moduletest')));
     $this->assertEquals(2, $result['count']);
-- 
2.25.1