-
-
Notifications
You must be signed in to change notification settings - Fork 827
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
Api4 - Remove unused key from getFields #27060
Conversation
Thank you for contributing to CiviCRM! ❤️ We will need to test and review the PR. 👷 Introduction for new contributors
Quick links for reviewers |
I'm not clear why any of these fields are being returned in getFields for every entity: help_pre, help_post, custom_field_id, custom_group_id. |
@larssandergreen I think the original implementation of APIv4 getFields added things a bit too hastily and now we're stuck with them. But at least this one is broken so we can safely remove it :) |
Ah, OK. I don't think I understand how custom_group_id is not implemented and custom_field_id is (they both seem to return null for every entity I try), so I'll leave this one for someone who understands better to review. |
@larssandergreen they are only not null for custom fields. Or at least, |
I see. help_pre and help_post seem to be always null though, maybe they could be eliminated too? |
Merging this as this part seems OK althought @colemanw still a question in the discussion about the other fields |
I was wrong about help_pre and help_post as I was testing for Custom Field Groups, rather than Custom Fields. They are populated for Custom Fields. |
Overview
Removes the unused
custom_group_id
key from APIv4 getFields, which was never populated.Before
APIv4 getFields includes
custom_group_id: null
for every field, even custom fields. Obviously it doesn't work so safe to remove.After
Removed.
Comments
It's not important enough for anyone to have noticed, we're better off without it.
Note that the name of the custom group is returned, which is more useful.