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

Transforming inputs? #483

Open
vlad-pisanov opened this issue Jun 11, 2020 · 8 comments
Open

Transforming inputs? #483

vlad-pisanov opened this issue Jun 11, 2020 · 8 comments

Comments

@vlad-pisanov
Copy link

First, thanks for maintaining this gem, I love it! 🎉

I like that we can do things like string :name, strip: true which transforms the input by stripping it.

I think it would be useful to be able to define an arbitrary transformation lambda when the input is defined.
For example, this would snap all time inputs to the beginning of the day, and force variable names into the proper format:

time :start, transform: -> (t) { t.beginning_of_day }
string :var, transform: -> (s) { s.camelcase }

This would certainly DRY up a lot of our code.

@AaronLasseigne
Copy link
Owner

Stripping away whitespace from a string is a convenience because you'll almost always want to do it. Especially with user input. That's why we do it by default.

Why not transform the values in the execute method? What's the benefit to doing it before?

@vlad-pisanov
Copy link
Author

The payoff would be:

  • If the transformed param is used in helper methods outside of execute
  • The transformed param would be memoized and doesn't need to be re-computed on every call
  • Yes, strip is common, but only allowing this transformation is somewhat artificial

e.g. compare:

class Task < ActiveInteraction::Base
  time :start

  def execute
    p adjusted_start # "start" is never used; we only ever need "adjusted_start"
    foo
  end

  private

  def adjusted_start # have to introduce a new transformation helper
    start.beginning_of_day # this computation may be expensive
  end

  def foo
    p adjusted_start
  end
end

vs.

class Task < ActiveInteraction::Base
  time :start, transform: :beginning_of_day

  def execute
    p start # now, it's clear which param we refer to
    foo
  end

  private
  
  def foo
    p start # the transformed value is memoized and we get it instantly
  end
end

@h0jeZvgoxFepBQ2C
Copy link

yeees this would be also way more readable instead of putting this into execute ❤️

@tfausak
Copy link
Collaborator

tfausak commented Jun 6, 2023

Can you do this with callbacks? https://github.com/AaronLasseigne/active_interaction/tree/a7dc3769bb6b771856615db42f96c5d4812bb8ac#callbacks

set_callback :execute, :before, -> { start = start.beginning_of_day }

I haven't written Ruby in a while so the details may not be quite right, but I think the overall idea should work.

@antulik
Copy link
Contributor

antulik commented Jul 6, 2023

I experienced that limitation myself as well. Callbacks could be an alternative, however there are no callbacks when form object is used in new and edit controller actions to render a form.

class SomeController
  def new
    form = Form.new(x: params[:x])
  end
end

Personally I would prefer to add new callback e.g.initialize and clean up inputs in after initialize callback. That would be consistent with ActiveRecord#after_initialize

Alternatively if filters are processed outside of .run, then filter callbacks could be used. This approach is good because I can choose if I want to transform all or only valid data, by using before or after filter callbacks.

@antulik
Copy link
Contributor

antulik commented Jul 6, 2023

Another option is to have a generic option for all filters to manipulate data. That overlaps a little bit with ObjectFilter converter option.

class Task < ActiveInteraction::Base
  time :start, transform: -> { _1.beginning_of_day }
end

Part that requires some thinking is do we want to deal with invalid inputs as part of transformation and when transformation should be performed before, during, or after filter.

@vlad-pisanov
Copy link
Author

Related: Rails 7.1 now has the normalize feature in ActiveRecord models which is conceptually identical.

class User < ActiveRecord::Base
  normalizes :email, with: -> email { email&.downcase&.strip }
end

When the lambda is just a single method call, we could use symbols for brevity:

string :name, transform: :downcase

@nirname
Copy link

nirname commented Aug 14, 2023

Is there way to write a custom filter meet the needs?

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

6 participants