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

Result of the block in the 'given' method are ignored #2191

Open
diko4096 opened this issue Sep 29, 2021 · 4 comments
Open

Result of the block in the 'given' method are ignored #2191

diko4096 opened this issue Sep 29, 2021 · 4 comments
Labels

Comments

@diko4096
Copy link

Hello!

In the Grape 1.0.2 the code below works fine:

params do
  requires :payment_type, type: String, values: ['ccard', 'paypal', 'applepay']
  given payment_type: ->(val) { val == 'applepay' } do
    optional :m_card_holder, type: Hash, as: :card_holder do
      optional :first_name, type: String, allow_blank: false, desc: 'Cardholder first name'
      optional :last_name, type: String, allow_blank: false, desc: 'Cardholder last name'
    end
  end
  given payment_type: ->(val) { !(val === 'applepay') } do
    optional :card_holder, type: String, allow_blank: false
  end
end
post '', root: 'payments' do
  # Payments processing
  # ...
end

When the request were from Apple Pay, the card_holder parameter became Hash and were parsed as Hash,
otherwise it been treated as String. It was fine for a time, but then the Grape were upgraded to 1.5.3,
this behavior became broken. In the version 1.5.3, result of the blocks in the 'given' method are ignored
and 'declared' method resturns all the parameters defined in 'params' without conditions. In the case above, the 'cardholder',
passed from request as string in accordance with payment_type='ccard', parsed by Grape as Hash instead of string,
and causes exception

undefined method `any?' for "NA ME":String

in the line

memo[memo_key] = handle_passed_param(params_nested_path_dup, passed_children_params.any?) do

It is unexpected behaviour and I didn't find any notes about changes in the 'given' methods, it's looks like a bug.

@dblock
Copy link
Member

dblock commented Sep 29, 2021

Try writing a spec for this that passed in 1.0.2?

@dblock dblock added the bug? label Sep 29, 2021
@diko4096
Copy link
Author

The controller:

module API
  module V3
    class Workflow < Grape::API
      include API::V3::Defaults

      resource "workflow" do
        namespace "grapetest" do
          params do
            requires :payment_type, type: String, values: ['ccard', 'applepay']
            given payment_type: ->(val) { val == 'applepay' } do
              optional :m_card_holder, type: Hash, as: :card_holder do
                optional :first_name, type: String, allow_blank: false, desc: 'Cardholder first name'
                optional :last_name, type: String, allow_blank: false, desc: 'Cardholder last name'
              end
            end
            given payment_type: ->(val) { !(val === 'applepay') } do
              optional :card_holder, type: String, allow_blank: false
            end
          end
          get '', root: 'grapetest' do
            declared(params, include_parent_namespaces: true, include_missing: false)
          end
        end
      end
    end
  end
end

The spec (with rspec-grape):

require 'rails_helper'

RSpec.describe ::API::V3::Workflow, type: :api do

  context "GET /v3/workflow/grapetest" do
    it "return card_holder as hash for applepay" do
      call_res = call_api({payment_type: :applepay, card_holder: {first_name: 'xxx', last_name: 'yyy'}})
      expect(call_res.status).to eq(200)
      expect(JSON.parse(call_res.body, symbolize_names: true).dig(:card_holder)).to be_an_instance_of(Hash)
    end
    it "return card_holder as string for ccard" do
     call_res = call_api({payment_type: :ccard, card_holder: 'cardholder name'})
      expect(call_res.status).to eq(200)
      expect(JSON.parse(call_res.body, symbolize_names: true).dig(:card_holder)).to be_an_instance_of(String)
    end
    it "422 if type is ccard and card_holder is a hash" do
      call_res = call_api({payment_type: :ccard, card_holder: {first_name: 'xxx', last_name: 'yyy'}})
      expect(call_res.status).to eq(422)
    end
  end
end

The result of rspec:

API::V3::Workflow
  GET /v3/workflow/grapetest
I, [2021-09-30T11:01:15.423048 #18937]  INFO -- : {:status=>200, :time=>{:total=>12.0, :db=>0.0, :view=>12.0}, :method=>"GET", :path=>"/v3/workflow/grapetest", :params=>{"payment_type"=>"applepay", "card
_holder"=>{"first_name"=>"xxx", "last_name"=>"yyy"}}, :host=>"example.org", :ip=>"127.0.0.1", :ua=>nil}

    return card_holder as hash for applepay

I, [2021-09-30T11:01:15.427819 #18937]  INFO -- : {:status=>200, :time=>{:total=>1.73, :db=>0.0, :view=>1.73}, :method=>"GET", :path=>"/v3/workflow/grapetest", :params=>{"payment_type"=>"ccard", "card_ho
lder"=>"cardholder name"}, :host=>"example.org", :ip=>"127.0.0.1", :ua=>nil}

    return card_holder as string for ccard

I, [2021-09-30T11:01:15.439926 #18937]  INFO -- : {:status=>400, :time=>{:total=>10.43, :db=>0.0, :view=>10.43}, :method=>"GET", :path=>"/v3/workflow/grapetest", :params=>{"payment_type"=>"ccard", "card_
holder"=>{"first_name"=>"xxx", "last_name"=>"yyy"}}, :host=>"example.org", :ip=>"127.0.0.1", :ua=>nil}
D, [2021-09-30T11:01:15.440771 #18937] DEBUG -- : {:code=>422, :user_message=>"Wrong parameters: card_holder is invalid", :internal_message=>"card_holder is invalid"}

    422 if type is ccard and card_holder is a hash

Finished in 0.06557 seconds (files took 3.13 seconds to load)
3 examples, 0 failures, 0 pending

@dblock
Copy link
Member

dblock commented Sep 30, 2021

PR the spec - and maybe try a fix?

@diko4096
Copy link
Author

Unfortunately not, I've looked in the params_scope.rb, it quite complicated. I only found the block calls, but its result doesn't stored or analyzed. But in the Grape 1.0.2 this piece of code looks the same, with different behaviour in results.

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

No branches or pull requests

2 participants