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 • 2 comments

Description

fixes #99 As I explained in https://github.com/nicolasblanco/rails_param/issues/99, it would be really useful to automatically permit the validated params. I'm already sharing this PR to collect feedbacks

The idea is to create a hierarchy structure, similar to params that is passed from the upper element to its child and so on. Each child update the hierarchy element it received and since it is passed by reference the hierarchy gets updated step by step, at the end we have the full structure.

Todos

List any remaining work that needs to be done, i.e:

  • [ ] Add configuration to enable/disable params permitting
  • [ ] Investigate edge cases
  • [ ] Documentation

Additional Notes

Let me know what's your idea to make this configurable, I was thinking of an initializer to enable it globally, something like

RailsParam.configure do |config|
    config.permit_params = true
end

a-chris avatar Sep 07 '23 19:09 a-chris

Thank for taking a stab at this @a-chris. I had a quick look to provide some early feedback, I'd love to hear from @ifellinaholeonce as well.

Let me know what's your idea to make this configurable, I was thinking of an initializer to enable it globally

This looks great and it's how I've done it in other gems, but we don't currently have configuration support in this gem and it would be out of scope from this PR. So I'd prefer to see that introduced in a separate PR first, and then used here.

The idea is to create a hierarchy structure, similar to params that is passed from the upper element to its child and so on

This is an interesting idea and I can see the implementation was not easy 👏 ! Overall it seems like you're trying to build this structure on the way down, bubble it up to the original caller and then call permit on it.

A possibly easier alternative would be to only pass this information downwards (like breadcrumbs of where you're at in params) and, upon reaching a "leaf", calling params.permit. This would increase the number of times we call params.permit, but it would not require changing the return type of the methods as the call would happen on the leaves as opposed to the root.

We already do something similar with context for printing error messages; although I'm not sure you could use it as-is given it's a string, I do wonder if it could be combined with the new hierarchy/breadcrumbs information into a single field?

iMacTia avatar Sep 11 '23 09:09 iMacTia

I like the look of this!

I haven't thought deeply about this yet but could a potential solution to the configurability be a keyword such as permit: true similar to required: true? It might perhaps be a simpler way of adding a config for this behaviour. Excuse me if I am missing some obvious complexity here.

A possibly easier alternative would be to only pass this information downwards (like breadcrumbs of where you're at in params) and, upon reaching a "leaf", calling params.permit.

This seems intriguing to me. And, as you suggested, seems like it would pair well with a small refactor of context.

ifellinaholeonce avatar Sep 12 '23 13:09 ifellinaholeonce