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

[Idea] Extract validation to external file #86

Open
mike927 opened this issue Dec 31, 2021 · 5 comments
Open

[Idea] Extract validation to external file #86

mike927 opened this issue Dec 31, 2021 · 5 comments

Comments

@mike927
Copy link

mike927 commented Dec 31, 2021

Basic Info

  • rails_param Version: 0.11.1
  • Ruby Version: 3.0.1
  • Rails Version: 6.1.1

Issue description

I've been leveraging rails_params in one of prior projects. It works great however I prefer to keep controller neat. I wrote a chuck of code to extract validation rules to external files under some path e.g /app/validators/.../*._validator.rb. Validation file itself could look like:

module Api
  module V1
    class SessionsValidator
      include RailsParam::Param

      attr_reader :params

      def initialize(params)
        @params = params
      end

      def create
        param! :username, String, required: true, blank: false
        param! :password, String, required: true, blank: false
      end

      def refresh
        param! :refresh_token, String, required: true, blank: false
      end
    end
  end
end

What do you think about such solution? If you find it as useful I could open a PR.

@iMacTia
Copy link
Collaborator

iMacTia commented Dec 31, 2021

This sounds interesting and it would work similarly to "Form" objects.
I'd like to understand what rails_param can do to help with this though.
You can already move validation rules into a separate module, include that in the controller and simply call them from there.

# app/validators/foo_controller_validator.rb
module FooControllerValidator
  def validate_create
     param! :username, String, required: true, blank: false
     param! :password, String, required: true, blank: false
  end
end

# app/controllers/foo_controller.rb
class FooController
  include FooControllerValidator
  
  def create
    validate_create
    # rest of the implementation
  end
end

What would be the usage on the controller side with this proposal and how would that be better than the one already possible?

@ifellinaholeonce your input on this would also be appreciated

@mike927
Copy link
Author

mike927 commented Dec 31, 2021

That could be either module or class tho. I came up with class and final controller usage looks like:

   before_action :validate_params, only: %i[create refresh]

The point is as I mentioned to keep it neat and automated. validate_params is decelerated somewhere else out of specific controller (e.g. included in BaseController). Then devs don't have to manually include validators to each controller and call it in methods.

@iMacTia
Copy link
Collaborator

iMacTia commented Dec 31, 2021

That looks cool and similar to other gems behaviour, though we'd need to be extra careful with performance as Ruby can be quite slow with reflections. Test and documentation coverage will also be important.

This looks like a big feature! Feel free to open a PR early on the development so we can see how it could work and potentially help with making decisions or tackling complex code together 👍

@mike927
Copy link
Author

mike927 commented Dec 31, 2021

Sounds great :)

@ifellinaholeonce
Copy link
Collaborator

This is an interesting idea for sure.

Personally, I don't think this is a feature that I would use much. I find the "documenting" that rails_params does in my controllers to be a really nice feature. This abstraction adds some misdirection to that. I'm not saying this to mean this isn't an interesting feature, just sharing some thoughts. And I guess to say let's make sure this feature is an extension of current behaviour and doesn't break how things currently work.

validate_params is decelerated somewhere else out of specific controller (e.g. included in BaseController). Then devs don't have to manually include validators to each controller and call it in methods.

Is this meant to help DRY up controller actions that have the exact same params and validations as other controller actions? Do you find a lot of your endpoints have the same params and validations? Defining unique parameters outside of the specific controller seems a little unnecessary. I think I am just missing something here.

As iMacTia mentioned, feel free to open a PR early on. Looking forward to seeing how this all could work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants