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

Optional params inside require weird behavior #635

Closed
zetachang opened this issue Apr 22, 2014 · 2 comments · Fixed by #671
Closed

Optional params inside require weird behavior #635

zetachang opened this issue Apr 22, 2014 · 2 comments · Fixed by #671

Comments

@zetachang
Copy link

An optional param specify restricted values inside a require block not work.

Below is the failing spec.

require 'spec_helper'

describe Grape::Validations do

  subject { Class.new(Grape::API) }

  def app
    subject
  end

  describe 'params' do
    context 'optional' do
      it 'should fucking fail' do
        subject.params do
          optional :a_array do
            requires :a_string, type: String, :values => ["a", "b"], :default => "a"
          end
        end
        subject.get '/optional_with_required_values' do
          'optional works!'
        end

        get '/optional_with_required_values'
        expect(last_response.status).to eq(200)
      end
    end
  end
end
@dm1try
Copy link
Member

dm1try commented Apr 22, 2014

@zetachang , thx. seems like a bug. I think it's related to this commit 177a14c#diff-5952004de7a6d081b88629647a47d85aR11

@dblock dblock added the bug? label Apr 22, 2014
@dm1try dm1try added confirmed bug and removed bug? labels May 28, 2014
@dm1try
Copy link
Member

dm1try commented May 28, 2014

@dblock expected behaviour is described in README

optional :audio do
  requires :format, type: Symbol, values: [:mp3, :wav, :aac, :ogg], default: :mp3
end

... and params[:audio][:format] is required only if params[:audio] is present.

I confirm :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants