Skip to content

Commit

Permalink
AtLeastOneOfValidator properly treats nested params in errors
Browse files Browse the repository at this point in the history
Unlike other validators `Grape::Valiations::AtLeastOneOfValidator` didn't wrap params
into a scope (namespace) for errors when params were nested.
  • Loading branch information
dnesteryuk committed Oct 1, 2019
1 parent 45b1288 commit ddc4805
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 18 deletions.
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).
* [#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

0 comments on commit ddc4805

Please sign in to comment.