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

nested params are not found in route hash #280

Closed
marcusg opened this issue Nov 22, 2012 · 8 comments
Closed

nested params are not found in route hash #280

marcusg opened this issue Nov 22, 2012 · 8 comments

Comments

@marcusg
Copy link
Contributor

marcusg commented Nov 22, 2012

it seems like the nested parameter names overwrite same parameter-names in different namespaces.
when i declare my parameter validation like this:

params do
  requires :name, :type => String, :desc => "Wish item id."
  group :target_location_attributes do
    requires :lat, :type => Float, :desc => "Wish item target location latitude."
    requires :lng, :type => Float, :desc => "Wish item target location longitude."
    requires :country_code, :type => String, :desc => "Wish item target location county code."
    requires :query, :type => String, :desc => "Wish item target query string."
  end
  group :source_location_attributes do
    optional :lat, :type => Float, :desc => "Wish item source location latitude."
    optional :lng, :type => Float, :desc => "Wish item source location longitude."
    optional :country_code, :type => String, :desc => "Wish item source location county code."
    optional :query, :type => String, :desc => "Wish item source query string."
  end
end

the resulting route parameters look like:

API::V1::Api.routes[11].route_params
{
            "name" => {
         :required => true,
             :type => "String",
             :desc => "Wish item id.",
        :full_name => "name"
    },
     "description" => {
         :required => true,
             :type => "String",
             :desc => "Wish item description.",
        :full_name => "description"
    },
           "price" => {
         :required => true,
             :type => "Float",
             :desc => "Wish item price.",
        :full_name => "price"
    },
       "allowance" => {
         :required => true,
             :type => "Float",
             :desc => "Wish item allowance.",
        :full_name => "allowance"
    },
             "lat" => {
         :required => false,
             :type => "Float",
             :desc => "Wish item source location latitude.",
        :full_name => "source_location_attributes[lat]"
    },
             "lng" => {
         :required => false,
             :type => "Float",
             :desc => "Wish item source location longitude.",
        :full_name => "source_location_attributes[lng]"
    },
    "country_code" => {
         :required => false,
             :type => "String",
             :desc => "Wish item source location county code.",
        :full_name => "source_location_attributes[country_code]"
    },
           "query" => {
         :required => false,
             :type => "String",
             :desc => "Wish item source query string.",
        :full_name => "source_location_attributes[query]"
    }
}

so no "target"-stuff can be found here and i think it's an issue. any ideas?

@dblock
Copy link
Member

dblock commented Nov 25, 2012

Looks like it's a real problem. Could you please build a quick repro / failing Grape test please?

@marcusg
Copy link
Contributor Author

marcusg commented Dec 14, 2012

Yes, I'll try to build a failing test soon...

@marcusg
Copy link
Contributor Author

marcusg commented Dec 14, 2012

I've added a failing test here marcusg@ecaa3f8. Is there a chance to implement this behaviour? Maybe i'm missing something important here, but there needs to be a way to access all required/optional nested parameters.

@dblock
Copy link
Member

dblock commented Dec 28, 2012

Now that you have a failing test, try to fix this and make a PR - definitely the quickest way to get this fixed!

@dblock dblock closed this as completed in f283686 Jan 5, 2013
@dblock
Copy link
Member

dblock commented Jan 5, 2013

Fixed. It would be non-trivial to preserve the nested group structure, so I chose to simply correct the problem and keep the flat list where the keys are the unique parameter names. By that I mean that your example produces this:

[{
 "group1[param1]"=>{:required=>false, :desc=>"group1 param1 desc"},
 "group1[param2]"=>{:required=>true, :desc=>"group1 param2 desc"},
 "group2[param1]"=>{:required=>false, :desc=>"group2 param1 desc"},
 "group2[param2]"=>{:required=>true, :desc=>"group2 param2 desc"}
}]

@marcusg
Copy link
Contributor Author

marcusg commented Jan 8, 2013

@dblock that looks really nice, thank you!

@moofish32
Copy link

Is this issue fixed? I recently attempted this with the following definition:

module API
module V1
class BlockedUsers < Grape::API
BLOCKED_USERS_ATTRS = [:first_name, :last_name, :date_of_birth]

    resources :blocked_users do
      namespace :match do
        desc 'Checks to see if the supplied user matches a blocked user'
        params do
          group :user do
            requires :first_name, type: String, desc: "The persons first name"
            requires :last_name, type: String, desc: "The persons last name"
            requires :date_of_birth, type: Time, desc: 'The persons birth date'
          end
        end
        before do
          @user = User.new(strong_params(BLOCKED_USERS_ATTRS, params[:user]))
        end

        get do
          BlockedUser.blocked_users_for(@user)
        end
      end
    end
  end
end

end

I am unable to get my rspec test to pass because I find empty params:

    describe 'GET match' do
      context 'with a user that is not blocked' do
        it 'returns an empty list' do
          get 'api/v1/blocked_users/match', user: user_to_block
          expect(JSON.parse(response.body)).to eq []
        end
      end

      context 'with user information that is blocked' do
        before {Oddz::User::BlockedUser.create(user_to_block)}
        it 'returns a list of blocked users matching' do
          get 'api/v1/blocked_users/match', user: user_to_block
          expect(JSON.parse(response.body).first['last_name']).to eq user_to_block[:last_name]
        end
      end
    end

TIA,

Mike

@dblock
Copy link
Member

dblock commented Jul 21, 2014

The issue at hand here should be fixed. Can you reopen something new with a repro inside Grape if you don't see the expected behavior?

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

No branches or pull requests

3 participants