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

Ability to access unfiltered request parameters #79

Closed
weppos opened this issue Jan 25, 2015 · 4 comments · Fixed by #82
Closed

Ability to access unfiltered request parameters #79

weppos opened this issue Jan 25, 2015 · 4 comments · Fixed by #82
Assignees
Milestone

Comments

@weppos
Copy link
Member

weppos commented Jan 25, 2015

This is the result of a chat I had with @jodosha about a real-world scenario.

Scenario

Let's suppose I configure an action with some param whitelisting.

class Signup
  include Lotus::Action

  params do
    param :first_name
    param :last_name
    param :email
  end

  def call(params)
    # ...
  end
end

Once the action is triggered, the params object will return a filtered list of params. Therefore, if the action is called with

POST /signup?first_name=Simone&last_name=Carletti&email=whatever&foo=bar

the action will strip the unallowed foo parameter. In most cases, this is what we want. However, let's assume that I have a command (similar to the Interactor @jodosha's idea) that I use to process the signup, that takes as input a whitelisted Hash of signup attributes.

class Signup
  include Lotus::Action

  params do
    param :first_name
    param :last_name
    param :email
  end

  def call(params)
    SignupInteractor.execute(params)
  end
end

So far so good. But our use case is a little bit more complex. We want to allow people to signup, and being redirected to a specific return_to path passed in query string, or a default url if the return path is not present.

class Signup
  include Lotus::Action

  params do
    param :first_name
    param :last_name
    param :email
  end

  def call(params)
    SignupInteractor.execute(params)

    redirect_to params[:return_to] ? params[:return_to] : 'default'
  end
end

Here we have a problem. I don't have access anymore to the return_to. There are a number of possible workarounds, however both @jodosha and I agree that there should be a way to access the unfiltered list of params in a request.

This is just a possible use case, but there are a lot of them.

API

This is a call for feedback. Some options are:

  • params.raw[:whatever]
  • params.original[:whatever]

Proposals?

@AlfonsoUceda
Copy link
Contributor

Ok, you don't have to params[:return_to] because params only has first_name, last_nameand email, isn't it?

Yes, then we have an API separating the whitelisted params and all params or no-whitelisted params.

API

  • params.raw[:whatever]
  • params.whitelisted[:other]

What do you think? I prefer whitelisted than original

@weppos
Copy link
Member Author

weppos commented Jan 25, 2015

I don't think we should have two API. params should always represent the expected, (filtered if required) param list.

However, there should be a way to access the original, unfiltered list. If no param filtering is set, then params and params.original (or whatever name is) would be the same.

@AlfonsoUceda
Copy link
Contributor

Ok, I prefer too, this change doesn't break params api and it only adds the original or raw params. 👍

@jodosha
Copy link
Member

jodosha commented Jan 26, 2015

API

Quoting @weppos

I don't think we should have two API. params should always represent the expected, (filtered if required) param list.

👍 This will keep the existing whitelisting semantic.

I'm for naming it #raw.

Technical implementation

We can rename Params#_params with Params#_raw.

This method should memoize the result of that computation in @raw. This should be a Lotus::Utils::Attributes instance, in order to guarantee "indifferent access" until we drop support for 2.2-.

As last thing, we should expose an attr_reader :raw.

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