Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

Add event-driven method to alter field metadata #61

Closed
colemanw opened this issue May 31, 2017 · 2 comments
Closed

Add event-driven method to alter field metadata #61

colemanw opened this issue May 31, 2017 · 2 comments

Comments

@colemanw
Copy link
Member

colemanw commented May 31, 2017

The api3 has a convention of using magic-named callback functions e.g. _civicrm_api3_activity_create_spec() to modify the list of dao fields for the api consumer.

We need some similar functionality in api4, but something using events would be better. If there isn't already a suitable event being fired, let's add one.

Acceptance criteria:

  • An event which allows each individual entity api to alter field metadata.
  • Fields can be altered based on the action being performed (e.g. Contact::create can have different fields from Contact::get.
  • Setting the flag api.required on a field for a particular action will result in the api rejecting a request in which that field is empty.
  • Setting the flag api.default on a field for a particular action will result in that param automatically being supplied by the api wrapper (if not specified in the api request).
  • Unit tests to demonstrate the above.
@mickadoo
Copy link
Collaborator

mickadoo commented Jun 7, 2017

I'm first trying to figure out what's going on in api3. I seems request validation is done in the APIv3SchemaAdapter which uses getfields requests to check what to validate. The generic getfields request calls the spec helper function to modify the fields.

To keep the same functionality there will be a lot of code to port over and clean up:

  • civicrm_api3_generic_getfields (which seems to be used all over the place and in validation)
  • The helper functions to change the spec, called from inside this civicrm_api3_generic_getfields
  • _civicrm_api3_generic_get_metadata_options
  • _civicrm_api3_swap_out_aliases
  • civicrm_api3_verify_mandatory
  • _civicrm_api3_validate_fields

And after that we can decide how to allow changing the API spec. It shouldn't be hard to open up spec changes using an event, for example subscribing to the existing civi.api.prepare event, just checking if the request is for get_fields and dispatching a new 'civi.api.getspec' event.

We might consider making bigger changes though, like making the specification an object instead of an array.

@mickadoo
Copy link
Collaborator

mickadoo commented Jun 13, 2017

I've focused on making the spec gathering use a more object orientated approach. I have a single service that is responsible for gathering all the request specifications. It fetches fields from the DAO, adds custom fields and adds the options available for all fields. It returns this as a RequestSpec object, contianing an array of FieldSpec objects, each with properties like name, title, description, type, default value etc.

It allows extensions to tag services as a "spec_provider" which will at it to the list of providers. These services will receive the RequestSpec object and are free to modify it in any way.

Other than validation I guess this will be used for the API explorer. I would prefer to split validation based on this to another issue. I'm still not 100% convinced we should be doing validation at API level that's not repeated in the BAO. Other than validation and the API explorer it would be good for me to know why this spec gathering is so important. Maybe you could update the description @colemanw just so I can tell if what I've done will match the requirements.

I've added a JSON encoded sample of a RequestSpec object to show what's available.

This is a fairly big PR, adding a lot of classes and tests. I'll wait until the open PRs are merged, but it would be good to get any feedback on the changes.

mickadoo added a commit that referenced this issue Jul 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants