AaronLasseigne / active_interaction

:briefcase: Manage application specific business logic.
MIT License
2.07k stars 137 forks source link

Easier fetching of given inputs #498

Open jgrau opened 3 years ago

jgrau commented 3 years ago

Hi!

I recently started using AI and I've run into what seems like a topic that has already been discussed a few times: partial updates by using default: nil and given?(:my_attribute).

When reading previous issues I see that there's some pushback around using something like optional: true with the result that if an argument is optional and not passed to the interaction it is not present in the inputs hash. Fair enough.

I find myself doing something like

    listing.assign_attributes(
      inputs
        .slice(
          :a,
          :bunch,
          :of,
          :attrs,
        )
        .select { |k, _v| given?(k) },
    )

in order to support partial updates which works but is not super friendly.

How do you feel about adding a given method in ActiveInteraction::Inputs that returns a new ActiveInteraction::Inputs instance but only containing the keys that was given? It would make the above use case look like

    listing.assign_attributes(
      inputs
        .given
        .slice(
          :a,
          :bunch,
          :of,
          :attrs,
        )
    )

which imo is pretty slick 😎

I'm not familiar with this codebase yet but if you like the idea and would accept a pull request I could take a shot at it.

For the record: I do feel having something like optional: true would be great :)

AaronLasseigne commented 3 years ago

This seems like a good idea. Do you still want to take a shot at making a PR?

jgrau commented 3 years ago

Cool! Yes I’ll give it a shot. :)

AaronLasseigne commented 3 years ago

I've been looking at this at it might actually be tricky to do. There are some changes I'm wanting to make under the hood that will make this easier. Let's put a pause on it and I'll let you know when I've done the internal stuff.

jgrau commented 2 years ago

Hi @AaronLasseigne. It was really cool to see the 5.0 upgrade. I have this piece of code in a superclass that all my interactions inherit from:

  # We want to support "partial updates" in our "update" commands.
  # The way ActiveInteraction works we need to use `default: nil` for
  # all attributes but that would not allow for partial updates as all
  # the attributes that was not given to the command, would be set
  # to nil. To combat that, we override the `inputs` method so that
  # only the inputs that was actually given to the command is
  # present in the `inputs` hash. In order to support default values
  # in the command we also select attributes if they are not
  # nil even though the attribute is not provided (given) to the
  # command.
  def inputs
    ActiveInteraction::Inputs.process(
      super.select { |input| given?(input) || !public_send(input).nil? },
    )
  end

I'm trying to achieve the same functionality with the 5.0 upgrade but I can't seem to find a good way to do that.

With the 5.0 upgrade and move of given? into Inputs this issue seems a lot more doable, do you agree? If so, I can try to make a PR.

AaronLasseigne commented 2 years ago

I'm wondering about how this should work for hashes. Should it only be the first level of inputs or should it dive deep into hashes and check those too?

jgrau commented 2 years ago

I'm wondering about how this should work for hashes. Should it only be the first level of inputs or should it dive deep into hashes and check those too?

IMO it should just be the first level of inputs.

AaronLasseigne commented 2 years ago

Sure, we'll start with just the top level of inputs and see what people think.