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

Easier fetching of given inputs #498

Open
jgrau opened this issue Mar 23, 2021 · 7 comments
Open

Easier fetching of given inputs #498

jgrau opened this issue Mar 23, 2021 · 7 comments

Comments

@jgrau
Copy link

jgrau commented Mar 23, 2021

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
Copy link
Owner

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

@jgrau
Copy link
Author

jgrau commented Jul 6, 2021

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

@AaronLasseigne
Copy link
Owner

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
Copy link
Author

jgrau commented Jul 12, 2022

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
Copy link
Owner

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
Copy link
Author

jgrau commented Jul 14, 2022

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
Copy link
Owner

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

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

2 participants