Skip to content

fix bug for default value as proc when there is given options values#779

Closed
ShPakvel wants to merge 1 commit intoruby-grape:masterfrom
ShPakvel:bugfix_default_with_proc
Closed

fix bug for default value as proc when there is given options values#779
ShPakvel wants to merge 1 commit intoruby-grape:masterfrom
ShPakvel:bugfix_default_with_proc

Conversation

@ShPakvel
Copy link
Copy Markdown
Contributor

@ShPakvel ShPakvel commented Oct 8, 2014

When you have option default as a proc and also any kind of option values (as a proc or normal) the gem is crashed. This small change solves it.

@ShPakvel ShPakvel force-pushed the bugfix_default_with_proc branch from d507b2b to d2e7949 Compare October 8, 2014 22:09
@dblock
Copy link
Copy Markdown
Member

dblock commented Oct 9, 2014

Can you please update CHANGELOG? For future googling purposes, also, what's 'crashes'? What's the specific error?

@ShPakvel ShPakvel force-pushed the bugfix_default_with_proc branch from d2e7949 to 960923d Compare October 9, 2014 23:22
@ShPakvel
Copy link
Copy Markdown
Contributor Author

ShPakvel commented Oct 9, 2014

Using grape you can meet error like 'block (2 levels) in <class:API>': wrong number of arguments (1 for 0) (ArgumentError) with code like:

params do
  optional :type, values: ['valid-type1', 'valid-type2'], default: -> { ['valid-type1', 'valid-type2'].sample }
end
get '/default_lambda' do
  { type: params[:type] }
end

params do
  optional :type, values: -> { ValuesModel.values }, default: -> { ValuesModel.values.sample }
end
get '/default_and_values_lambda' do
  { type: params[:type] }
end

And this PR fixes this bug. The problem was about default option value is proc when used with values. There was not call in place which checks inclusion if default in values.

@ShPakvel
Copy link
Copy Markdown
Contributor Author

ShPakvel commented Oct 9, 2014

@dblock I've added info to changelog and put some description of the problem here.
Let me know if you need something else.

@dblock
Copy link
Copy Markdown
Member

dblock commented Oct 10, 2014

Merged via 00e745e, changed the language in the CHANGELOG a bit (and put back the your contribution here note).

@dblock dblock closed this Oct 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants