-
Notifications
You must be signed in to change notification settings - Fork 365
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
Custom parameter method #312
Merged
oestrich
merged 3 commits into
zipmark:master
from
sineed:sineed-custom-parameter-method
Dec 8, 2016
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
require 'rspec_api_documentation/dsl/endpoint/set_param' | ||
|
||
module RspecApiDocumentation | ||
module DSL | ||
module Endpoint | ||
class Params | ||
attr_reader :example_group, :example | ||
|
||
def initialize(example_group, example, extra_params) | ||
@example_group = example_group | ||
@example = example | ||
@extra_params = extra_params | ||
end | ||
|
||
def call | ||
parameters = example.metadata.fetch(:parameters, {}).inject({}) do |hash, param| | ||
SetParam.new(self, hash, param).call | ||
end | ||
parameters.deep_merge!(extra_params) | ||
parameters | ||
end | ||
|
||
private | ||
|
||
attr_reader :extra_params | ||
|
||
end | ||
end | ||
end | ||
end |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
module RspecApiDocumentation | ||
module DSL | ||
module Endpoint | ||
class SetParam | ||
def initialize(parent, hash, param) | ||
@parent = parent | ||
@hash = hash | ||
@param = param | ||
end | ||
|
||
def call | ||
return hash if path_params.include?(path_name) | ||
return hash unless method_name | ||
|
||
hash.deep_merge build_param_hash(key_scope || [key]) | ||
end | ||
|
||
private | ||
|
||
attr_reader :parent, :hash, :param | ||
delegate :example_group, :example, to: :parent | ||
|
||
def key | ||
@key ||= param[:name] | ||
end | ||
|
||
def key_scope | ||
@key_scope ||= param[:scope] && Array(param[:scope]).dup.push(key) | ||
end | ||
|
||
def scoped_key | ||
@scoped_key ||= key_scope && key_scope.join('_') | ||
end | ||
|
||
def custom_method_name | ||
param[:method] | ||
end | ||
|
||
def path_name | ||
scoped_key || key | ||
end | ||
|
||
def path_params | ||
example.metadata[:route].scan(/:(\w+)/).flatten | ||
end | ||
|
||
def method_name | ||
@method_name ||= begin | ||
[custom_method_name, scoped_key, key].find do |name| | ||
name && example_group.respond_to?(name) | ||
end | ||
end | ||
end | ||
|
||
def build_param_hash(keys) | ||
value = keys[1] ? build_param_hash(keys[1..-1]) : example_group.send(method_name) | ||
{ keys[0].to_s => value } | ||
end | ||
end | ||
end | ||
end | ||
end |
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
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.
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.
@oestrich @sineed This fallback won't work for some scenarios.
Example:
Even before entering the example block, rspec api documentation calls
status
to prepare the request, becausestatus_param
is not defined. When it callsstatus
, it fails withNo response yet. Request a page first
, because thestatus
method is only supposed to be called after a request is done.I don't see a reason to have this fallback mechanism. If the programmer specifies a different method name for a param, just use it if present, if not, do not use the param at all.
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.
Your example is working as intended. If instead the
example
is anexample_request
then we hit your failure, which does exist. Status is a weird method to override, and we've always had an issue with it.The problem line seems like it would be this one. I will look at a PR if you want to fix it, otherwise I don't see why you would change the method and not define it.
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.
@oestrich The status was just an example, but this issue happens to other reserved words on the context of a test example as well. I created a PR: #352
Thanks