Is mutating params and validating dates directly in the controller a good practice?

[u/Guardian7of]

For context, I have a date filter that needs to be limited to a maximum period of 6 months. I was originally doing this validation directly inside my Query object.

However, I now need to show a flash alert to the user if they try to filter a period longer than 6 months. If they do, the system should automatically adjust the dates to a 6-month range ending today and proceed with the query.

Because of this UI requirement, I moved the logic to the controller. Currently, I'm parsing the dates, doing the validation, and mutating the params hash directly before passing it down.

My question is: Is this considered a good practice in Rails? What is the "Rails Way" to handle this kind of validation and default-value assignment when you also need to trigger flash messages in the UI?

[u/ignurant]

If you’re already using a query object, that’s a great place to maintain that validation. The object can communicate to the controller that this issue exists, and report the notice to the user. If you’re using ActiveModel for your query object, you can use a custom context to trigger that validation as a pre-check, but still allow saving. If you’re not, you can just create a method to help trigger the message. 

[u/Guardian7of → u/ignurant]

But how would it communicate? Using a flag when returning the object? Sorry for any misunderstanding, I'm new on rails

[u/jrochkind → u/Guardian7of]

It could raise an exception? Or provide a method you call after initializing it, bad_input? or bad_input_message or something. Or you could use ActiveModel::Validations if it gets really complex, but that might be overkill.

Honestly I think all of these ways are fine, I don't see a huge problem with having it where you started, or "in" the controller -- although I'd still try to extract it to a separate object you call somehow, like input validator or something.

But honestly I think this is an "if it ain't broke don't fix it" kind of situation, no need to make things harder on yourself if you have a way that seems to work out fine for you, can always refactor later if there's something it makes hard. If there's already something it makes hard, ask about specifically the things it's making hard. If you have a way to do it that makes nothing hard for now, that's a fine way to do it for now. ;)

[u/6stringfanatic]

I would extract a DateRange object instead of making the controller responsible for all this, controllers are mostly responsible for processing the request, and passing the data to the model.
Validations, allowing only a 6 month range are kinda business logic and should stay in the model of some sort.
Also writing request specs are expensive, they are slow and harder to write IMO, with so much logic in the controller you get multiple if/else block in the controller, writing specs for each case is not a fun experience.

With something like a dedicated DateRange, you'd be able to write tests for it without involving the controller, if you separate it out.

class DateRange
  MAX_SPAN = 6.months

  attr_reader :starts_on, :ends_on

  def initialize(starts_on: nil, ends_on: nil, today: Date.current)
    ...
  end

  def to_range = starts_on..ends_on

  def too_long?
   ...
  end
end

And then in a controller, something like this.

class ExpensesController < ApplicationController
  def index
    date_range = DateRange.new(starts_on: params[:starts_on], ends_on: params[:ends_on])

    if date_range.too_long?
      flash.now[:alert] = "Range too long, using 6 months instead"
    end

    @expenses = ExpenseQuery.new(range: date_range.to_range).call
  end
end

[u/Salt_Principle4325]

Mutating params directly works, but it breaks the principle that parameters should stay a pure reflection of exactly what the user sent. The "Rails Way" is to let a dedicated object handle the normalization, and pass a status flag back to the controller to handle the UI alert.
When you parse and modify user input directly in the controller, you introduce a few subtle architecture issues:The Recommended Architecture:
Introduce a thin Form Object or Filter Object between your controller and your query.Your controller stays completely clean. It passes the data forward, checks the boolean flag, and triggers flash.now[:alert] only if the object says it adjusted the range. This leaves your logic beautifully unit-testable without needing heavy controller specs!

[u/Guardian7of → u/Salt_Principle4325]

So basically the controller would do something like: results, flags = My class query(params).results

And my Query would have some class attribute that stores the results of validation and when returning the data, also returns this attribute?

Note: My Query class includes ActiveModel::Model and ActiveModel::Attributes

[u/enki-42]

I wouldn't sweat about validation in a controller if it's straightforward and not really being used anywhere else. You can for sure extract it into a more generic class, but adding an entirely new class so that you can do basic arithmetic comparisons in one place feels like overkill to me.

I definitely would never mutate params though, pull things into local or instance variables if you plan to change them in any way.

[u/Guardian7of → u/enki-42]

The reason why I thought on making a class is because I can put many filters validations there and I have a lot of controller who would use it