trailblazer / reform

Form objects decoupled from models.
https://trailblazer.to/2.1/docs/reform.html
MIT License
2.49k stars 184 forks source link

Destroying nested form objects #535

Open Tab10id opened 2 years ago

Tab10id commented 2 years ago

With reform often use populators for creation of nested objects with one form. It works well but sometimes we need destroy nested objects too. For example:

class User < ActiveRecord::Base
  has_one :avatar, inverse_of: :user
end

class Avatar< ActiveRecord::Base
  belongs_to :user, inverse_of: :avatar
end

avatar have no sence without user but user can have no avatar.

We want to manage user and avatar by one form:

class UserForm < ReforForm
  property :avatar, populate_if_empty: Avatar, form: AvatarForm
end

But in this example we can handle only creation of avatar. Sometimes we want to destroy them too without separate destroy operation. We can do this with populator option. Example:

property :avatar,
         populator: :find_or_build_or_mark_destroy,
         form: AvatarForm

def find_or_build_or_mark_destroy(model:, fragment:, **)
  if model
    model.model.mark_for_destruction if fragment[:_destroy] == '1'
    model
  else
    self.avatar = Avatar.new
  end
end

full example - https://github.com/Tab10id/reform_destroy_populator_example

It work but this thing have some problems:

  1. it change original model state before actual validation
  2. it use mark_for_destruction feature implemented in ActiveRecord (https://apidock.com/rails/ActiveRecord/AutosaveAssociation/mark_for_destruction). So we have destroy feature in ActiveRecord and build feature in Reform. This can be a problem because mark_for_destruction feature works only if we have autosave: true option at association on parent model.
  3. If we mark object for destruction we can get ActiveRecord exception if we have association property for nested form. e.g. Avatar model also will have belongs_to :some_thing in this case we can mark avatar for destruction then we validate params and sync all properties to all model. Writing of some_thing property to avatar (if it is not nil) trigger autosave feature for some_thing on avatar save. But because avatar was marked for destruction and internally was frozen by ActiveRecord we have exception about frozen hash modification. I can prepare another code example of this case if needed (and hack to avoid it).
  4. Populator looks ugly and it much more uglier for collection properties.

In my mind if we still want to manage nested forms we need to have also destroy action for nested form (not only save). In this case we internally mark property for destruction on reform level and then call destroy action on each marked form model after all validations (in transaction). Also we need to skip all writing to marked for destruction models because it's no sence in this case.

denis-mueller commented 8 months ago

i had the same problem. By accident I noticed that if you remove the item to delete out of the collection in the populator and also have has_many :children, dependent: destroy in the parent model file, it automatically destroys the children without having to call mark_for_destruction. this might solve your problem