Should we use strong params when we update only one attribute?
Asked Answered
A

4

6

I'm working on a Rails app and I have several actions( #delete_later, #ban_later and so on) where I only set one attribute from the request parameter( specifically, a reason field for doing that action).

I was wondering if it is ok to do it like this:

def ban_later
  @object.reason = params[:object][:reason]
  @object.save
end

Or is it a best practice to use strong params even in this situation?

def ban_later
  @object.reason = object_params[:reason]
  @object.save
end

private
  def object_params
    params.require(:object).permit(:permitted_1, :permitted_2, :reason)
  end

Which of these solutions is the best one? If none of them is, then what's the best solution to my problem?

Later Edit:

The #ban_later, #delete_later actions can indeed set a flag column status but that can be done without receiving it's value from the params hash. Since you will only set one status per method you can simply set the status "pending_delete" when you are in #delete_later and "pending_ban" when you are in #ban_later.

Later Later Edit

Why use #save and not update_attributes directly? Let's say you need to have a if @object.save statement. On the false branch( object not saved) you might still want to render a view where the contents of that @object are used.

Advocaat answered 13/10, 2015 at 6:4 Comment(0)
A
1

First one saves computation.

Second one checks for existence of :object sub-hash, which I think is good for fault-tolerance.

I initially would pick the 1st, but after some thought I liked the second one more.

Anglesey answered 13/10, 2015 at 6:16 Comment(0)
P
1

The simplest answer is that if you only use one parameter in params, and do not pass it to a multi attribute setter like model#create then you don't have to use strong_parameters to get a secure solution.

However, I expect that it is unlikely that this is the case for the whole controller. Where the ban_later method only needs one parameter, other controller methods will need more. In this case the question becomes: "do you want to handle params differently for ban_later to how you use it for the other controller methods?".

Also can you be sure that the functionality will not change, and that when you change the functionality, that you'll remember to change the way params is handled.

Therefore, I would use strong_parameters because it means:

  • parameters are handled consistently across all methods in the controller.
  • changes to methods are less likely to expose vulnerabilities as functionality changes.
Pyo answered 13/10, 2015 at 6:22 Comment(3)
Well the #ban_later, #delete_later can indeed set a flag column status but that can be done without receiving it's value from the params hash. Since you will only set one status per method you can simply set the status "pending_delete" when you are in #delete_later and "pending_ban" when you are in #ban_later.Advocaat
As you can see you don't need any params from the User other than a reason for doing that certain action.Advocaat
You are right. You don't have to use strong_parameters. But for the reasons I put forward, I would.Pyo
A
1

If you're updating a single attribute, why don't you use the update_attributes method? (update_attribute doesn't invoke validation)

def ban_later
  @object.update_attributes reason: params(:reason)
end

private

def params params
    params = %i(:permitted_1, :permitted_2, :permitted_3) unless params
    params.require(:object).permit params
end

In light of the comments by ReggieB, you could also use the update option:

def ban_later
    @object.update reason: params(:reason)
end 

As mentioned, Reggie and the other answers explain the schematics of how this works best (IE with mass-assignment etc). Above is actionable code which you're free to use.


The bottom line here is that if you want to keep your application versatile (IE having ultimate extensibility wherever you need), you'll need to adhere to the strong params setup.

The other answers outline how that setup works, and how its functionality is different dependent on what you need.

I have included a trick to make it so you only accept specific params in your params method. I've not tested it extensively, so we may have to refactor it to get the required result.

Asepsis answered 13/10, 2015 at 7:13 Comment(5)
How is this better than method= and save other than it does in a single line, what would otherwise take two.Pyo
Yes - I noticed your comment about update_attribute after I wrote my initial comment. I still think that using update_attributes for updating a single attribute is not a good pattern - especially when the developer is questioning whether to use strong parameters.Pyo
Probably - I put my suggestion forward. He could also use update which I'll add to the answerAsepsis
Key is this "The bottom line here is that if you want to keep your application versatile ....., you'll need to adhere to the strong params setup." Still not a fan of what you're doing, but I've removed my down vote.Pyo
Keep the downvote if you don't like it. I make a point of providing code wherever I can, if the code is wrong, it's wrong. I'd rather put up code and have it be wrong than leave the OP guessing. It's great that you pointed out such an intrinsic part of the system. I'm not super experienced with mass assignment etc, which is why I outlined that your answer and the other were much better in terms of the schematics of the system. I'm impartial to whether you agree or not, I put up answers to keep my own skills sharp ^_^Asepsis
H
1

After strong parameters check why not just update the object? Its just a standart workflow. (Please tell me if there are any reasons not to do that in your situation)

def ban_later
  @object.update(object_params)
  # dont forget validation check
end

private
  def object_params
    params.require(:object).permit(:permitted_1, :permitted_2, :reason)
  end

In this case it'd be much easier to add more updateble fields.

Hesterhesther answered 13/10, 2015 at 7:41 Comment(6)
You want to have a if @object.save statement. On the false branch( object not saved) you might still want to render a view where the contents of that @object are used.Advocaat
@Dmitri, whats the problem? What prevents you from rendering @object after update fails?Hesterhesther
If the update fails, AFAIK, the @object will contain the values that are in the database. I might want it to contain the new values from the params. Think of it like a form that needs where you have validations. If validations fail for a field you'd like the form to be re-rendered with the values you wrote, not the ones save to the database.Advocaat
On the other hand if you update the fields one by one the @object will contain the new values.Advocaat
@Dmitri, I think you are wrong. After validation fails the @object will be still in invalid state with invalid fieldsHesterhesther
Just check this in consoleHesterhesther

© 2022 - 2024 — McMap. All rights reserved.