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

AtLeastOneOfValidator properly treats nested params in errors #1911

Merged
merged 1 commit into from
Oct 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#### Fixes

* Your contribution here.
* [#1911](https://github.com/ruby-grape/grape/pull/1911): Make sure `Grape::Valiations::AtLeastOneOfValidator` properly treats nested params in errors - [@dnesteryuk](https://github.com/dnesteryuk).
* [#1893](https://github.com/ruby-grape/grape/pull/1893): Allows `Grape::API` to behave like a Rack::app in some instances where it was misbehaving - [@myxoh](https://github.com/myxoh).
* [#1898](https://github.com/ruby-grape/grape/pull/1898): Refactor `ValidatorFactory` to improve memory allocation - [@Bhacaz](https://github.com/Bhacaz).
* [#1900](https://github.com/ruby-grape/grape/pull/1900): Define boolean for `Grape::Api::Instance` - [@Bhacaz](https://github.com/Bhacaz).
Expand All @@ -17,7 +18,7 @@

#### Features

* [#1888](https://github.com/ruby-grape/grape/pull/1888): Makes the `configuration` hash widly available - [@myxoh](https://github.com/myxoh).
* [#1888](https://github.com/ruby-grape/grape/pull/1888): Makes the `configuration` hash widely available - [@myxoh](https://github.com/myxoh).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this.

* [#1864](https://github.com/ruby-grape/grape/pull/1864): Adds `finally` on the API - [@myxoh](https://github.com/myxoh).
* [#1869](https://github.com/ruby-grape/grape/pull/1869): Fix issue with empty headers after `error!` method call - [@anaumov](https://github.com/anaumov).

Expand Down
7 changes: 5 additions & 2 deletions lib/grape/validations/validators/at_least_one_of.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
require 'grape/validations/validators/multiple_params_base'

module Grape
module Validations
require 'grape/validations/validators/multiple_params_base'
class AtLeastOneOfValidator < MultipleParamsBase
def validate!(params)
super
if scope_requires_params && no_exclusive_params_are_present
raise Grape::Exceptions::Validation, params: all_keys, message: message(:at_least_one)
scoped_params = all_keys.map { |key| @scope.full_name(key) }
raise Grape::Exceptions::Validation, params: scoped_params,
message: message(:at_least_one)
end
params
end
Expand Down
18 changes: 14 additions & 4 deletions spec/grape/validations/validators/at_least_one_of_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,15 @@ def params(arg)
end

def required?; end

# mimics a method from Grape::Validations::ParamsScope which decides how a parameter must
# be named in errors
def full_name(name)
"food[#{name}]"
end
end
end

let(:at_least_one_of_params) { %i[beer wine grapefruit] }
let(:validator) { described_class.new(at_least_one_of_params, {}, false, scope.new) }

Expand Down Expand Up @@ -48,11 +55,14 @@ def required?; end

context 'when none of the restricted params is selected' do
let(:params) { { somethingelse: true } }

it 'raises a validation exception' do
expect do
validator.validate! params
end.to raise_error(Grape::Exceptions::Validation)
expected_params = at_least_one_of_params.map { |p| "food[#{p}]" }

expect { validator.validate! params }.to raise_error do |error|
expect(error).to be_a(Grape::Exceptions::Validation)
expect(error.params).to eq(expected_params)
expect(error.message).to eq(I18n.t('grape.errors.messages.at_least_one'))
end
end
end

Expand Down
22 changes: 11 additions & 11 deletions spec/grape/validations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1485,16 +1485,16 @@ def validate_param!(attr_name, params)
before :each do
subject.params do
requires :nested, type: Hash do
optional :beer_nested
optional :wine_nested
optional :juice_nested
at_least_one_of :beer_nested, :wine_nested, :juice_nested
optional :beer
optional :wine
optional :juice
at_least_one_of :beer, :wine, :juice
end
optional :nested2, type: Array do
optional :beer_nested2
optional :wine_nested2
optional :juice_nested2
at_least_one_of :beer_nested2, :wine_nested2, :juice_nested2
optional :beer
optional :wine
optional :juice
at_least_one_of :beer, :wine, :juice
end
end
subject.get '/at_least_one_of_nested' do
Expand All @@ -1505,17 +1505,17 @@ def validate_param!(attr_name, params)
it 'errors when none are present' do
get '/at_least_one_of_nested'
expect(last_response.status).to eq(400)
expect(last_response.body).to eq 'nested is missing, beer_nested, wine_nested, juice_nested are missing, at least one parameter must be provided'
expect(last_response.body).to eq 'nested is missing, nested[beer], nested[wine], nested[juice] are missing, at least one parameter must be provided'
end

it 'does not error when one is present' do
get '/at_least_one_of_nested', nested: { beer_nested: 'string' }, nested2: [{ beer_nested2: 'string' }]
get '/at_least_one_of_nested', nested: { beer: 'string' }, nested2: [{ beer: 'string' }]
expect(last_response.status).to eq(200)
expect(last_response.body).to eq 'at_least_one_of works!'
end

it 'does not error when two are present' do
get '/at_least_one_of_nested', nested: { beer_nested: 'string', wine_nested: 'string' }, nested2: [{ beer_nested2: 'string', wine_nested2: 'string' }]
get '/at_least_one_of_nested', nested: { beer: 'string', wine: 'string' }, nested2: [{ beer: 'string', wine: 'string' }]
expect(last_response.status).to eq(200)
expect(last_response.body).to eq 'at_least_one_of works!'
end
Expand Down