Fixes #37824 - Hide taxonomies from parts of api documentation #10322
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adamlazik1
force-pushed
the
fix-37824-hide-taxonomy-options
branch
2 times, most recently
from
dcb6f04
to
396604b
Compare
Some resources like user groups, external user groups, and architectures
are not scoped by taxonomies, yet the the api endpoints associated with
these resources accept the `organization-id` and `location-id` options.
I didn't observe any effect of these options on the api call, except for
when trying to create an external user group and providing either
organization-id or location-id, which causes the action to fail with an
error appearing in the logs:
`undefined method external_usergroups for #<{Organization/Location}: ...`
I have not, however, found any simple way of fixing this. All `Api::V2`
controllers inherit from `Api::V2::BaseController`, where the taxonomy
options are added through the `resource_description` method from Apipie.
While this method can be overridden in child classes, there appears to
be no way (at least I have not found such a way) of removing a parameter
once it is added.
The most correct solution would be of course to create a child class
inheriting from BaseController, provide the resource description with
taxonomy options there, and then have all taxonomy-scoped resource
controllers inherit from it. The problem is that there are many plugins
in which the controllers inherit from BaseController that would all need
to be updated as well. I see too much potential for breaking because of
a relatively harmless bug, so in my opinion the risk is not worth to fix
the issue this way.
Hence, I propose a partial solution. Hide the taxonomy options from the
API documentation of the relevant resources. Hammer can also be updated
to not display options with the `show => false` flag set.
This would not completely solve the issue but in my opinion has the best
effort/result/risk reduction ratio.
adamlazik1
force-pushed
the
fix-37824-hide-taxonomy-options
branch
from
396604b
to
a6ec7b9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some resources like user groups, external user groups, and architectures are not scoped by taxonomies, yet the the api endpoints associated with these resources accept the
organization-idandlocation-idoptions. I didn't observe any effect of these options on the api call, except for when trying to create an external user group and providing either organization-id or location-id, which causes the action to fail with an error appearing in the logs:undefined method external_usergroups for #<{Organization/Location}: ...I have not, however, found any simple way of fixing this. All
Api::V2controllers inherit fromApi::V2::BaseController, where the taxonomy options are added through theresource_descriptionmethod from Apipie. While this method can be overridden in child classes, there appears to be no way (at least I have not found such a way) of removing a parameter once it is added.The most correct solution would be of course to create a child class inheriting from BaseController, provide the resource description with taxonomy options there, and then have all taxonomy-scoped resource controllers inherit from it. The problem is that there are many plugins in which the controllers inherit from BaseController that would all need to be updated as well. I see too much potential for breaking because of a relatively harmless bug, so in my opinion the risk is not worth to fix the issue this way.
Hence, I propose a partial solution. Hide the taxonomy options from the API documentation of the relevant resources. Hammer can also be updated to not display options with the
show => falseflag set.This would not completely solve the issue but in my opinion has the best effort/result/risk reduction ratio.