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 - Exclude disabled custom fields #23583

Merged
merged 1 commit into from
May 26, 2022

Conversation

colemanw
Copy link
Member

Overview

Ensures that disabled custom fields do not show up in API results or in Search Kit.

Before

Disabled custom fields available in SearchKit.

After

Disabled fields hidden.

@civibot
Copy link

civibot bot commented May 25, 2022

(Standard links)

@civibot civibot bot added the master label May 25, 2022
@@ -122,6 +122,7 @@ private function addCustomFields($entity, RequestSpec $spec) {

$query = CustomField::get(FALSE)
->setSelect(['custom_group_id.name', 'custom_group_id.title', '*'])
->addWhere('is_active', '=', TRUE)
->addWhere('custom_group_id.is_multiple', '=', '0');
Copy link
Contributor

Choose a reason for hiding this comment

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

@colemanw do we need to do something about the multi record custom fields as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. I've updated the PR and added a test for that as well.

@colemanw colemanw force-pushed the apiDisabledFields branch from cf6146b to 032c091 Compare May 26, 2022 01:22
@guyiac
Copy link
Contributor

guyiac commented May 26, 2022

I tested the first demo just now and it worked fine. Waiting for the second demo to finish completing checks to test that.

@guyiac
Copy link
Contributor

guyiac commented May 26, 2022

Tested the second demo and it worked fine.

  1. Activated Search Kit extension.
  2. Created search with Most Important Issue custom field.
  3. Disabled Most Important Issue custom field.
  4. Verified that field was gone from first search and didn't show up in a new one.
  5. Created multiple value custom field set.
  6. Created two new custom fields in set.
  7. Verified both fields were available for search views.
  8. Disabled one of the fields.
  9. Verified that the field was no longer available for search views.

@demeritcowboy
Copy link
Contributor

For reference there's a longstanding discussion about whether they should or shouldn't show: #14240 (comment) and also https://lab.civicrm.org/dev/core/-/issues/19. I suppose this PR matches where it is most places.

@colemanw
Copy link
Member Author

Ok I went back and re-read that issue, and then double-checked the APIv3 behavior, and this PR matches it (disabled custom fields are not shown in APIv3 getFields). Based on that, I think this PR is ok to merge, but we could consider further refinements in the future.

@demeritcowboy demeritcowboy merged commit 12f6fca into civicrm:master May 26, 2022
@colemanw colemanw deleted the apiDisabledFields branch May 26, 2022 18:14
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.

4 participants