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 tableName to Entity.get output #22829

Merged
merged 1 commit into from
Feb 26, 2022
Merged

Conversation

colemanw
Copy link
Member

Overview

Adds another useful bit of output to APIv4 Entity.get. Improves flexibility for virtual entities.

Before

Relationship between entities and tables pretty much hard-coded.

After

More flexible, each entity can decide on its table name using dynamic logic if needed.

Comments

There is massive test coverage on this.

@civibot
Copy link

civibot bot commented Feb 25, 2022

(Standard links)

@civibot civibot bot added the master label Feb 25, 2022
->addDefault('html_type', 'Text')
->addDefault('custom_group_id', '$id')
->addRecord(['label' => 'MyField1'])
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General patch makes sense.

What's the connection between Entity::getInfo()['table_name'] and the ability to join CustomGroup::create() to CustomField::save()? (I would have expected that the only magic in the chain is based on the FK between CG/CF.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@totten it turns out APIv4 only bothers to create a virtual entity from a custom field set if it contains fields. Which makes sense. So this test had been testing a scenario that wouldn't fully work.

@totten
Copy link
Member

totten commented Feb 26, 2022

Here's a basic r-run sanity-check:

cv api4 Entity.get -T +s name,table_name
+------------------------+----------------------------------+
| name                   | table_name                       |
+------------------------+----------------------------------+
| ACL                    | civicrm_acl                      |
| ACLEntityRole          | civicrm_acl_entity_role          |
| ActionSchedule         | civicrm_action_schedule          |
| Activity               | civicrm_activity                 |
| ActivityContact        | civicrm_activity_contact         |
| Address                | civicrm_address                  |
| Afform                 |                                  |
| AfformSubmission       | civicrm_afform_submission        |
...
| CustomField            | civicrm_custom_field             |
| CustomGroup            | civicrm_custom_group             |
| Custom_Education       | civicrm_value_education_4        |
...
| SubscriptionHistory    | civicrm_subscription_history     |
| Survey                 | civicrm_survey                   |
| System                 |                                  |
| Tag                    | civicrm_tag                      |
...

This was interesting because it had realistic examples of many kinds of entities (eg core-dao, core-special, extension-dao, extension-special, custom-group). The table_name presented on all of them seemed appropriate.

@totten totten merged commit 3ec640d into civicrm:master Feb 26, 2022
@totten totten deleted the tableName branch February 26, 2022 04:58
@colemanw
Copy link
Member Author

Thanks for the review @totten

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