rails_param icon indicating copy to clipboard operation
rails_param copied to clipboard

automatically permit validated params

Open a-chris opened this issue 2 years ago • 3 comments

Basic Info

  • rails_param Version: 1.3.1
  • Ruby Version: 3.2.1
  • Rails Version: 7.0.x

Issue description

The param! methods does not call params.permit over validated parameters, this means that after the validation block we need to manually permit and extract only the required fields. e.g.

 param! :reaction, Hash, required: true do |r|
      r.param! :note, String, required: false, blank: true
      r.param! :like, :boolean, required: false
      r.param! :love, :boolean, required: false
      r.param! :reject, :boolean, required: false
end

# or with a separate method if you prefer
reaction_params = params.permit(reaction: [:like, :love, :reject, :note])

This is very repetitive and error-prone when deleting/adding new fields and could lead to dangerous errors.

The README is not very clear about this, from a first read I understood that I could use this gem and forget about the params.permit but now I figured out it's not like that.

I propose to automatically permit params (and nested params) based on the fields declared in param! by overriding the params variable or using a new instance variable such as @sanitized_params, @rails_params or whatever you prefer.

I'd like to open a PR if you like this idea, otherwise I will just keep the fork for me.

What do you think?

a-chris avatar Aug 18 '23 14:08 a-chris

This totally makes sense to me. @ifellinaholeonce what do you think? The only request from me would be to make this configurable, we can then debate before the next release if we want to this to default to being enabled or disabled (with backwards-compatibility in mind).

I'll be unavailable over the next couple of weeks, but I don't want to be a blocker for this proposal 🙂

iMacTia avatar Aug 21 '23 10:08 iMacTia

I think this is a great idea 👍 and I think the idea to make this configurable makes a lot of sense. I'm looking forward to it @a-chris. If you also think there is a part of the README that could be improved with this idea would you mind updating that as well?

Coincidentally, I am also offline for the rest of this week but happy to continue this conversation or review a PR next week.

ifellinaholeonce avatar Aug 21 '23 13:08 ifellinaholeonce

hi guys, I've been busy too (it's summer!) so I managed to start hacking with the code last week but it's being very hard to keep track of the fields to permit.

Since the structure is declared inside blocks, we have no way to know which keys the user wants to validate before actually executing the block. The blocks also acts in a recursive fashion but we are able to collect only the result from the last field of the block, e.g.:

controller.param! :array, Array do |a|
          a.param! :object, Hash do |h|
            h.param! :num, Integer, required: true
            h.param! :float, Float, required: true
          end
        end

if we make the block returns something, we will be able to collect :array, :object and :float results but we can't collect what :num returned.

So, yeah, it's way harder than I expected, I have a semi-working solution that attempt to build a hierachy structure of the validated fields in param! and it was working so good with Hashes until I ran the Array tests 💥 🤣

However I do not give up, I'll keep you updated, in the meantime feel free to share ideas and suggestions 😄

a-chris avatar Sep 06 '23 20:09 a-chris