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

param as is not work #2237

Open
ly123525 opened this issue Jan 28, 2022 · 8 comments
Open

param as is not work #2237

ly123525 opened this issue Jan 28, 2022 · 8 comments
Labels

Comments

@ly123525
Copy link

ly123525 commented Jan 28, 2022

params do
    optional :page_size, as: :per_page, type: Integer
end

params #=> {page_size: 10} not have per_page
declared(params) #=> {per_page: 10}

i upgrade from 1.31 to 1.6.0

@brynary @tmornini @ctennis @olleolleolle @bernd
How should I write? thanks

@ly123525 ly123525 reopened this Jan 28, 2022
@dblock
Copy link
Member

dblock commented Jan 28, 2022

This seems to work as expected, what output did you expect?

@dblock dblock added the bug? label Jan 28, 2022
@ly123525
Copy link
Author

ly123525 commented Jan 29, 2022

This seems to work as expected, what output did you expect?

@dblock
Thank you for your reply

params do
    optional :page_size, as: :per_page, type: Integer
end

params #=> {per_page: 10}
declared(params) #=> {per_page: 10}

@ly123525 ly123525 reopened this Jan 29, 2022
@dnesteryuk
Copy link
Member

@ly123525 it is recommended to use declared(params) to avoid mass assignment. I wouldn't fix anything in Grape.

@dm1try
Copy link
Member

dm1try commented Jan 29, 2022

@ly123525 it is recommended to use declared(params) to avoid mass assignment. I wouldn't fix anything in Grape.

@dnesteryuk while your suggestion is good, the current behavior still looks like a bug because we have a clear description in README for how it should work.

The value passed to as will be the key when calling params or declared(params).

And it'd worked it such way previously but not now. Looks like a breaking change.

@dblock
Copy link
Member

dblock commented Jan 30, 2022

So did we introduce a regression, and in which version? Or should we change nothing and update documentation?

@dnesteryuk does it change anything in your comment?

@dnesteryuk
Copy link
Member

ok, I figured it out when this behavior was changed, actually, it is described in the upgrading doc, so it isn't a bug, it is how Grape works after 1.6.0 and the motivation of the change makes sense.

@dm1try probably, we need to update the doc 😉

@dblock
Copy link
Member

dblock commented Jan 31, 2022

@ly123525 or someone else here, care to update? thanks

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

4 participants