diff --git a/CHANGELOG.md b/CHANGELOG.md index 52a86c4a8..07088a957 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). @@ -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). diff --git a/lib/grape/validations/validators/at_least_one_of.rb b/lib/grape/validations/validators/at_least_one_of.rb index 077393b63..cfe006c89 100644 --- a/lib/grape/validations/validators/at_least_one_of.rb +++ b/lib/grape/validations/validators/at_least_one_of.rb @@ -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 diff --git a/spec/grape/validations/validators/at_least_one_of_spec.rb b/spec/grape/validations/validators/at_least_one_of_spec.rb index 7a331fe9b..17eeb90a1 100644 --- a/spec/grape/validations/validators/at_least_one_of_spec.rb +++ b/spec/grape/validations/validators/at_least_one_of_spec.rb @@ -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) } @@ -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 diff --git a/spec/grape/validations_spec.rb b/spec/grape/validations_spec.rb index 90e04bafe..dd81fddaa 100644 --- a/spec/grape/validations_spec.rb +++ b/spec/grape/validations_spec.rb @@ -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 @@ -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