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 - Code cleanup & improve links to @see annotations in Explorer #19798

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Mar 13, 2021

Overview

Minor code cleanup in a few APIv4 classes, and improve rendering of links to @see annoations in the APIv4 explorer help panel.

Before/After

Links for e.g. Permission::get in the APIv4 explorer were not correct (the ones at the bottom under See:). Now they are at least more correct.

image

@civibot civibot bot added the master label Mar 13, 2021
@civibot
Copy link

civibot bot commented Mar 13, 2021

(Standard links)

@@ -48,39 +49,33 @@ public static function getFields($checkPermissions = TRUE) {
[
'name' => 'group',
'title' => 'Group',
'required' => TRUE,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for these removals?

@eileenmcnaughton
Copy link
Contributor

This does what it says on the box - I just had the one question about the removed 'required' lines

@eileenmcnaughton
Copy link
Contributor

Hmm I hammered the UI a bit more on the 'before' & I can see that the required fields are which fields are 'required' don't actually make that much sense for the get action and don't seem to be enforced anywhere so I guess that's why - merging

@eileenmcnaughton eileenmcnaughton merged commit 86f8047 into civicrm:master Mar 16, 2021
@eileenmcnaughton eileenmcnaughton deleted the api4cleanup branch March 16, 2021 07:11
@colemanw
Copy link
Member Author

colemanw commented Mar 16, 2021

Thanks @eileenmcnaughton.
That's right, required is only enforced for Create. For other actions it's meaningless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants