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

initialize parent param in default validator only if it's not present #611

Merged
merged 1 commit into from
Mar 31, 2014
Merged

initialize parent param in default validator only if it's not present #611

merged 1 commit into from
Mar 31, 2014

Conversation

dm1try
Copy link
Member

@dm1try dm1try commented Mar 29, 2014

Maybe it's kind of tricky, but this one fixes #596

@dm1try
Copy link
Member Author

dm1try commented Mar 29, 2014

I add CHANGELOG if this solution will be acceptable :)

@dblock
Copy link
Member

dblock commented Mar 29, 2014

Can you explain?

@dm1try
Copy link
Member Author

dm1try commented Mar 31, 2014

There is only one case where parent element doesn't exist, see this spec and note.

params do
  optional :foo, type: Hash do
    optional :bar, default: 'foo-bar'
  end
end

We have optional param with default value in the group and no group params provided by client:

get '/group'

for example, in case

get '/group?foo'

we don't need initialize parent element

Anyway, changes in "default" validator was made by me for fixing #537 and before #545 PR( was added type for group/requires/optional params). Now it breaks default behavior for params with type Array. Why is tricky? Not sure, how should be implemented this behavior for params with type Array and whether it be responsibility of default validator :)

dblock added a commit that referenced this pull request Mar 31, 2014
initialize parent param in default validator only if it's not present
@dblock dblock merged commit e36e7d5 into ruby-grape:master Mar 31, 2014
@dblock
Copy link
Member

dblock commented Mar 31, 2014

Merged.

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