reform icon indicating copy to clipboard operation
reform copied to clipboard

Only validate/update collection forms that are being modified

Open nikhilmat opened this issue 6 years ago • 6 comments

Hi there!

Thanks for writing a great library! I'm using Reform to a manage a collection of nested forms from a parent form. My parent form looks like:

class GlobalVisitor::SaveForm < ApplicationForm
  property :token
  collection(
    :company_visitors,
    populator: :company_visitor!,
    form: Visitor::CreateForm,
  )

  validation do
    required(:token).filled
  end

  private

  def company_visitor!(fragment:, as:, collection:, **)
    form = collection.find { |model| model.company_id == fragment[:company_id] }
    form ? form : collection.append(model.company_visitors.build(company_id: fragment[:company_id]))
  end
end

I've noticed that if I have many company_visitors, all of them get validated and saved during the process of saving the GlobalVisitor::SaveForm, even if my company_visitors parameters do not reference any existing objects and my populator returns a new instance.

I am trying to find a way to configure my collection, such that it only contains the relevant items based on the incoming parameters. Or a way to exclude the items that are not being updated from being validated/saved. For a parent object that has many collection items, it results in quite a bit of unneeded work, when we know at the time of validation the subset of the collection we need to process. Any advice on how to go about this?

Thanks!

nikhilmat avatar Oct 12 '18 18:10 nikhilmat

Can you give some examples of the incoming params hash and how you'd like that to get transformed into a persistable object graph? :beer:

apotonick avatar Oct 13 '18 14:10 apotonick

BTW we're permanently working on a new approach to Reform called transform where this kind of mapping will be much easier to customize!

apotonick avatar Oct 13 '18 14:10 apotonick

hey @apotonick, for sure!

I'm backing this form by an ActiveRecord model: let's say in this case the model is GlobalVisitor id=1, and it has 100 CompanyVisitor children, each with company_id 1-100 (because I am using company_id as the attribute to lookup the correct form within the collection).

If I have a params hash like:

{
  token: '1234',
  company_visitors: [
    { company_id: 1, first_name: 'Shirley' },
    { company_id: 51, first_name: 'Bob' },
    { company_id: 501, first_name: 'Carol' },
  ]
}

When I validate and subsequently save I would expect this to:

  • Update the token on my GlobalVisitor instance to '1234'
  • Update the first_name on CompanyVisitor with company_id=1 to 'Shirley'
  • Update the first_name on CompanyVisitor with company_id=51 to Bob
  • Create a new CompanyVisitor that belongs to my GlobalVisitor with company_id=501 and first_name=Carol

Today, I've noticed that it will validate and save every item in the collection (in this example, all 100 CompanyVisitors), so I am looking for a way to only process the relevant collection items, as a result of what's passed in to the params hash.

Thanks for your help! Excited to hear more about the transform approach.

nikhilmat avatar Oct 13 '18 20:10 nikhilmat

I've tried to essentially make this a collection that is "write-only", by passing readable: false and then only having the items that are appended in the populator used in the collection. But collection always comes out nil, and I can't figure out how to instantiate it.

Is there a way to initialize it to an empty array, and only add items to the collection based on the work that is done in the populator? That should account for the case I mentioned above, where I only want objects that are referenced in the params to be part of the object graph.

nikhilmat avatar Oct 18 '18 20:10 nikhilmat

Any thoughts on how best to move forward with this @apotonick @fran-worley?

nikhilmat avatar Oct 24 '18 21:10 nikhilmat

Ok, I see the problem now. I am redesigning Reform/Disposable currently and making it easier to handle those "special" cases manually (because we can't cover all the needs without blowing up the API).

The main problem is that Disposable, for your collection will do model.company_visitors = [collection resulting from initializer/parsing], where you really only want to go over a selected amount of visitors and update those. That's basically why I decided to make the sync/save code customizable by the user.

apotonick avatar Oct 25 '18 05:10 apotonick