When using `given` it does not actually check that it's required

[andrew13nguyen]

I have code similar to this:

params do
  optional :button_link, type: String
  given :button_link do
    requires :button_text, type: String
  end
end

post :something do
  puts params.inspect
end

When I post to "something" with no params, everything is valid, but I see that params has:

params.inspect
=> {"button_text"=>nil}

When I change the params block to this:

params do
  optional :button_link, type: String
  given :button_link do#, :authenticated_button_link do
    requires :button_text, type: String, allow_blank: false
  end
end

I get a validation error (button_text is empty). Am I using given incorrectly or is this a bug?

[dblock]

This doesn't seem right, you should write a spec next. Do try on HEAD first.

[andrew13nguyen]

Okay, so what I posted before was lacking and the tests passed. After tinkering around I got the below to fail and maybe I'm using grape incorrectly, but I feel like this is a valid use case. The use case is if I :c is a common variable for a label that :a and :b would use, and :b is a more complex type in which I wouldn't be able to use :a for.

(Borrowing from https://github.com/ruby-grape/grape/blob/v0.16.2/spec/grape/validations/params_scope_spec.rb#L319-L343):

it 'applies the validations of multiple parameters' do
  subject.params do
    optional :a
    given :a do
      requires :c, type: String
    end
    optional :b
    given :b do
      requires :c, type: String
    end
    mutually_exclusive :a, :b
  end
  subject.get('/multiple') { declared(params).to_json }

  get '/multiple'
  expect(last_response.status).to eq(200)

  get '/multiple', a: true, c: 'test'
  expect(last_response.status).to eq(200)

  get '/multiple', b: true, c: 'test'
  expect(last_response.status).to eq(200)

  # this passes
  get '/multiple', a: true
  expect(last_response.status).to eq(400)
  expect(last_response.body).to eq('c is missing')

  # this fails
  get '/multiple', b: true
  expect(last_response.status).to eq(400)
  expect(last_response.body).to eq('c is missing')

  get '/multiple', a: true, b: true, c: 'test'
  expect(last_response.status).to eq(400)
  expect(last_response.body).to eq('a, b are mutually exclusive')
end

If I change requires :c, type: String to requires :c then it passes, but I would think enforcing a check on type is desired.

[dblock]

Right, turn it into a failing PR?

[giedriusr]

I've discovered the same thing as @andrew13nguyen. If you remove type: String then everything works. Is this a bug?

[dblock]

It sounds like a bug to me.

[towanda]

@dblock also this can be closed.