Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CRM-21792 Extension.get API - Fix regression (4.7.13) in result filtering #11709

Merged
merged 3 commits into from
Feb 22, 2018
Merged

CRM-21792 Extension.get API - Fix regression (4.7.13) in result filtering #11709

merged 3 commits into from
Feb 22, 2018

Commits on Feb 22, 2018

  1. 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 517dfba
 * Subsequent partial-fixes were d4c44c7, 9776f41, 34239e8, 525ccb6

CC @lucianov88  @twomice @tschuettler @seamuslee001 for review
    @totten
    totten committed Feb 22, 2018
    Configuration menu
    Copy the full SHA
    1d470b7 View commit details
    Browse the repository at this point in the history

  2. Extension.get API - Only apply special filters once (key/keys/full_name) 

    Filtering extensions by key is a good idea -- so good we have three ways to do it! ;)

All of these fields accept a `string|array<string>` input, which is different from
the typical filtering supported by `_civicrm_api3_basic_array_get()` (which only
matches on values)...  If you happen to choose a basic filter that passes both
filters (i.e.  a simple string-match), then it works.  But if you choose a filter
where they behave differnetly (i.e.  an array), then you get weird results.
    @totten
    totten committed Feb 22, 2018
    Configuration menu
    Copy the full SHA
    c3733bf View commit details
    Browse the repository at this point in the history

  3. api_v3_ExtensionTest::testExtensionGet - Be a little less flaky 

    The test has a magic number which seems to be based on the number of
extensions in the `drupal-clean` build profile -- which is basically the
smallest number anyone might have.  However, it gives false-negatives if you
have any other extensions around.  Make it a little less flaky.
    @totten
    totten committed Feb 22, 2018
    Configuration menu
    Copy the full SHA
    0847651 View commit details
    Browse the repository at this point in the history