-
Notifications
You must be signed in to change notification settings - Fork 472
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
use route_settings for hidden and operations extensions #596
Conversation
will give it a try and a deeper look on the weekend, ok? |
@LeFnord what do you think about this approach? |
grape-swagger.gemspec
Outdated
@@ -14,7 +14,7 @@ Gem::Specification.new do |s| | |||
s.license = 'MIT' | |||
|
|||
s.required_ruby_version = '>= 2.2.6' | |||
s.add_runtime_dependency 'grape', '>= 0.16.2' | |||
s.add_runtime_dependency 'grape', '>= 0.19.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert it back
@@ -337,7 +337,8 @@ def model_name(name) | |||
end | |||
|
|||
def hidden?(route, options) | |||
route_hidden = route.options[:hidden] | |||
route_hidden = route.settings.try(:[], :swagger).try(:[], :hidden) | |||
route_hidden = route.options[:hidden] if route.options.key?(:hidden) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am I right, the original behavior still available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the original behavior is default.
@@ -19,10 +19,11 @@ class ExtensionsApi < Grape::API | |||
{ 'declared_params' => declared(params) } | |||
end | |||
|
|||
route_setting :x_operation, some: 'stuff' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, makes the extensions more consistent, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please can you add more specs, so one can see the different possibilities to hide an endpoint, thanks
👍 @thogg4 please add the changelog entry, then I'll mörge it |
* use route_settings for hidden and operations extensions * remove puts statements * revert grape version * add some details to the readme * add route to demonstrate how to hide endpoint with route settings * correct hide with route setting in readme * add changelog entry * remove curly braces for rubocop
@LeFnord This pr may be a little heavy handed, but I wanted to show you what I meant by having grape-swagger use the preferred way to pass route settings.
Please let me know what you think and what I can improve and change.
Addresses #594