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.

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

APIv4 - Add metadata about class args #22831

Merged
merged 1 commit into from
Feb 27, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Feb 25, 2022

Overview

The CustomValue API is a virtual entity, where multiple api entities all get routed to the same class by virtue of all sharing the prefix "Custom_" and pass a class arg to the php factory functions e.g. CustomValue::get('MyCustomGroup').

Instead of hard-coding this idea into the API Explorer now it's part of the entity metadata so that other APIs, notaby ECK, can use a similar pattern.

Before

Hard-coded

After

Flexible, uses metadata.

@civibot
Copy link

civibot bot commented Feb 25, 2022

(Standard links)

@totten
Copy link
Member

totten commented Feb 26, 2022

OK, so I didn't realize that API Explorer had any special bits for Custom_ and played with it for a moment to understand. Just to verbalize/comprehend... here's a (redacted/simplified) example of the metadata from Entity.get:

$ cv api4 Entity.get +w name=Custom_Education
[
    {
        "name": "Custom_Education",
        "class": "Civi\\Api4\\CustomValue",
        "virtual_entity_prefix": "Custom_" // <== New
    }
]

With just name, you can generate a valid API call in almost every binding.

Procedural PHP: civicrm_api('Custom_Education', 'get', ...)
Angular:        crmApi4('Custom_Education', 'get', ...)
JS:             CRM.api4('Custom_Education', 'get', ...)
CLI:            cv api4 Custom_Education.get ...
REST:           https://example.com/civicrm/ajax/api4/Custom_Education/get

We need this functionality because the PHP class-binding looks quite different. (Change Custom to CustomValue and split apart the words.)

Civi\Api4\CustomValue::get('Education');

The addition of virtual_entity_prefix makes it possible to generate the above PHP code -- it tells you what should not pass into the PHP class. You have to synthesize things to figure what should pass in.

var extraArg = _.camelCase(entityInfo.name.substr(entityInfo.virtual_entity_prefix.length));
var phpCode = entityInfo.class + '::get(' + extraArg + ')';
$extraArg = CRM_Utils_String::convertStringToCamel(substr($entityInfo['name'], strlen($entityInfo['virtual_entity_prefix'])));
call_user_func([$entityInfo['class'], 'get'], $extraArg);

It might be cleaner to affirmatively specify what should pass into the PHP class (eg class_arg).

$ cv api4 Entity.get +w name=Custom_Education
[
    {
        "name": "Custom_Education",
        "class": "Civi\\Api4\\CustomValue",
        "class_arg": ["Education"] // <== New
    }
]
var phpCode = entityInfo.class + '::get(' + entityInfo.class_arg + ')';
call_user_func([$entityInfo['class'], 'get'], $entityInfo['class_arg']);

This contract allows the consumers to be simpler (less string-munging), and it makes the naming/mapping more flexible. (This is equally compatible with prefixes, suffixes, in-fixes, no-fixes, and completely-different-name-ixes. The underlying identities could be names or numbers or composite-keys.)

But this feedback is not a hard blocker. There could be other reasons/future-directions where virtual_entity_prefix seems better... Anyway, I've tested this, and it works. So if you still lean toward virtual_entity_prefix, then it's OK to merge.

@colemanw colemanw force-pushed the virtualEntityPrefix branch 2 times, most recently from 84db78c to 379016b Compare February 26, 2022 15:55
@colemanw
Copy link
Member Author

That's a really good idea @totten - that simplifies the API Explorer code and could be useful in other ways.

@colemanw colemanw changed the title APIv4 - Abstract the idea of a 'virtual_entity_prefix' APIv4 - Add metadata about class args Feb 26, 2022
@totten
Copy link
Member

totten commented Feb 26, 2022

Yeah, looks pretty good. I did a small tweak to the variable-name that appears in generated snippets ($custom_Educations / $customEducations).

Merge on pass.

@colemanw colemanw force-pushed the virtualEntityPrefix branch from ecff36f to febfaa1 Compare February 27, 2022 00:48
@colemanw
Copy link
Member Author

colemanw commented Feb 27, 2022

Thanks @totten - I just pushed a slightly different tweak that keeps the same behavior as before this PR. E.g. entity name Custom_My_Stuff becomes variable name myStuff (but now it works generically so other virtual entities like Eck get the same nice formatting).

The CustomValue API is a virtual API, where multiple entities all get routed to the
same class if they share the prefix "Custom_", and pass a class arg to the
php factory functions e.g. `CustomValue::get('MyCustomGroup')`.
Instead of hard-coding this idea into the  now it's part of the entity
metadata so that other APIs, notaby ECK, can use a similar pattern.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants