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

Dispatch an event for registering Afform entities #21899

Closed

Conversation

jensschuppe
Copy link
Contributor

Overview

Currently, Afform forms can only be created for hard-coded entities or entities provided by extensions through a PHP file for each entity. This adds an event which extensions can subscribe to for registering entities, e.g. "virtual" entities which no PHP files can be created for.

Before

No support for virtual entities.

After

Virtual entities supported by dispatching an event when generating the list of events.

Technical Details

Since CiviCRM Core supports virtual entities for API4 with #21771, the Afform builder should support those as well. Maybe this can be more generic in the future, i.e. allowing to build forms for all API4 entities.

Comments

With this, a refactoring of

// Explicitly load Contact and Custom entities because they do not have afformEntity files
$entities = Entity::get(TRUE)
->addClause('OR', ['name', '=', 'Contact'], ['type', 'CONTAINS', 'CustomValue'])
->execute()->indexBy('name');
foreach ($entities as $name => $entity) {
$data['entities'][$name] = self::entityToAfformMeta($entity);
}
might be useful.

@civibot
Copy link

civibot bot commented Oct 21, 2021

(Standard links)

@colemanw
Copy link
Member

colemanw commented Oct 21, 2021

After giving this some thought, I think there are basically 2 ways for the Afform project to approach entities:

  1. Specific: Entities are complex and require special handling and business logic.
    • By default, no entities are supported by Afform; each one must opt-in with a special metadata file
    • Special post-processing logic must be added for each entity to save correctly
    • Examples: Contacts, Cases, Relationships, Events, Memberships, Participants, Contributions
  2. Generic: Use metadata, treat all entities the same
    • By default, all API entities are supported (like SearchKit does)
    • CRUD operations handled in a generic way by the API
    • Examples: OptionValues, Notes, Grants, Custom values and other simple, CRUD-like entities.

As of this writing, Afform isn't fully invested in one approach or the other. The list of entities supported in the GUI is opt-in like #1 but the forms themselves can support any API entity like #2. Post-processing is a hybrid where saving is handed off to the API in a generic way, but first dispatches an event to allow special logic to be used per-entity.

All that is to say, maybe a quick-win to meet the user demand of "support more entities" would be to support #2 in the GUI. Currently when you pick from the entity list it only presents the opt-in (#1) entities. Maybe it could use a 2-tier list similar to the UI in SearchKit where "top level" entities are shown first but most other entities are available as "secondary". That would remove the need for this PR, as the list of secondary entities could be gleaned from a simple API.Entity.get call.

FYI In SearchKit the main entity selector looks like this, with top-level entities presented first and others available in a collapsible optgroup:
image

@jensschuppe
Copy link
Contributor Author

Post-processing is a hybrid where saving is handed off to the API in a generic way, but first dispatches an event to allow special logic to be used per-entity.

I don't quite get why entity-specific logic is to be implemented within Afform's post-processing in the first place. Isn't the API supposed to do all the heavy lifting for each entity? I see Afform validating contact entities having either a name or an e-mail address in

/**
* Validate contact(s) meet the minimum requirements to be created (name and/or email).
*
* This requires a function because simple required fields validation won't work
* across multiple entities (contact + n email addresses).
*
* @param \Civi\Afform\Event\AfformSubmitEvent $event
* @throws \API_Exception
* @see afform_civicrm_config
*/
public static function preprocessContact(AfformSubmitEvent $event): void {
if ($event->getEntityType() !== 'Contact') {
return;
}
// When creating a contact, verify they have a name or email address
foreach ($event->records as $index => $contact) {
if (!empty($contact['fields']['id'])) {
continue;
}
if (empty($contact['fields']) || \CRM_Contact_BAO_Contact::hasName($contact['fields'])) {
continue;
}
foreach ($contact['joins']['Email'] ?? [] as $email) {
if (!empty($email['email'])) {
continue 2;
}
}
// Contact has no id, name, or email. Stop creation.
$event->records[$index]['fields'] = NULL;
}
}
Shouldn't this be done in the API action instead and Afform not have to bother with entity-specific logic?

If at all, entity-specific logic would be necessary in constructing the form, e.g. fields being dependent on something, at least if Afform forms are considered a configurable UI for the API that exposes logic to the user (can't think of an example for this right now).

Anyway, Afform allowing all entities to be used to create forms for (i.e. #2) seems most reasonable to me, maybe with a filter for all CRUD actions Afform requires for an entity. But maybe the civi.afform.entities event can then be used for registering entities in the "top level" list of entities, leaving this PR valid as a starting point towards a fully-generic entity selector?

@colemanw
Copy link
Member

Shouldn't this be done in the API action instead and Afform not have to bother with entity-specific logic?

The Contact API cannot perform that check because it doesn't know whether or not you are planning to add an email address after creating a contact.

That's a specific example of a more general issue where Afform entity handling is different from general API processing because of the context of a form with user input. API CRUD operations lack this context and so are more permissive of input which either should not be permitted on a form, or should be subject to additional business logic.

@demeritcowboy
Copy link
Contributor

It sounds like this has stalled? If there are no short-term plans to work on it then I'd suggest closing and moving to gitlab.

@jensschuppe
Copy link
Contributor Author

Given that the following was suggested by @colemanw:

support #2 in the GUI. Currently when you pick from the entity list it only presents the opt-in (#1) entities. Maybe it could use a 2-tier list similar to the UI in SearchKit where "top level" entities are shown first but most other entities are available as "secondary". That would remove the need for this PR, as the list of secondary entities could be gleaned from a simple API.Entity.get call.

and the fact that we've moved away from using Afform forms for our "virtual entity" extension for now, moving this to GitLab for documenting the issue seems reasonable to me.

I'll let @colemanw decide however.

@colemanw colemanw closed this Feb 10, 2022
@colemanw
Copy link
Member

This is now replaced by #23303

@jensschuppe jensschuppe deleted the afformVirtualEntities branch July 18, 2022 10:39
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.

3 participants