Problem with nested attributes

[alexey]

Given:

class Widget < ActiveRecord::Base
  accept_nested_attributes_for :cars, allow_destroy: true
  has_drafts ignore: [:slug]
end

class WidgetsController < InheritedResources::Base
  def update
    @widget.attributes = widget_params # Update nested attributes already here
    if @widget.draft_update
     # ok
    else
      update! # Inherited resources update without draft, raising exception if destroyed or create duplicates because they were created in above code
    end
  end
end

the problem - nested object are created or destroyed twice(raising not found on second destroy).

Does anyone faced with similar problem ?

[chrisdpeters]

@alexey I really hate to say this, but I would avoid nested attributes on models drafted with Draftsman.

If you're interested, I could share a form model that I created to handle this sort of thing. (I think I created a form model back in the day that mocked the nested attributes functionality while tricking the form in the view template into thinking accepts_nested_attributes_for was in use.)

[alexey]

Hi @chrisdpeters, thanks for response, i thought about this solution but my currect form has many nested forms in it, it may be problem.
I have idea how to make it on controller level, will try and post result on that

[chrisdpeters]

@alexey I would like to see support for nested attributes added, but I must admit that I do not have the need for it personally. The app that I'm working on uses Draftsman, but it is an API.

If you would like to work out the solution and submit a pull request, I'd be glad to integrate it as a new feature!

[jorgepinon]

Hey @chrisdpeters, thanks for this gem. It's perfect for my needs so far.

My case also uses nested attributes so I'd like your suggestion. I'm still trying to understand how Draftman works with 2-level associations, i.e.
Store has_many categories
Category has_many products

I did notice that "The largest risk at this time is functionality that assists with publishing or reverting dependencies through associations" so I realize that my usecase may not be fully supported.

[chrisdpeters]

I really don't see it being viable to add nested attribute support to Draftsman.

These would be the 2 possible solutions that I can think of. (Maybe someone can propose something better):

1. Override/monkey-patch ActiveRecord's nested attributes implementation to call save_draft instead of save under the hood.
2. Implement a separate Draftsman-specific implementation of accepts_nested_attributes_for.

Both solutions are super complex.

The first solution would cause all sorts of problems. For example, which version of ActiveRecord's nested attributes implementation should we start with when doing the monkey-patching? What if we implement AR 5, but someone wants to use Draftsman with AR 4? Did anything significant change from 4 to 5?

I did promise @alexey to share an example form model where I mock accepts_nested_attributes_for-style functionality using a class that extends ActiveModel. I'll try my best to draft up a blog post sometime this coming week. (Sorry for letting this slip through the cracks!)

Seriously, there could be a better solution out there than these 2 scenarios that I've proposed, so I am all ears if anyone has any better ideas.

[jorgepinon]

I think I can make the ux work without nested attributes as long as child and grandchildren can also be saved with the draft and are publishable

[NullVoxPopuli]

I think I have a solution to the nested attributes problem:

I have a module:

# frozen_string_literal: true
# To use this, define on any model that already has `has_drafts`
# ```
# class Blog < ApplicationRecord
#   include DeeplyPublishable
#
#   has_drafts
#   associations_to_publish :posts, :pages
#   ...
# end
# ```
#
# With the above definition, there are two publicly available
# instance methods provided:
# - deep_publish!
# - deep_discard!
#
# Both of these will traverse the associations as defined by
# associations_to_publish, either publishing or discarding drafts
# for any draft object encountered.
#
# NOTE: The draft becomes the real version when published.
#       Until publish, the data for the actual model in the model's table
#       is the same as it was before the draft was created.
module DeeplyPublishable
  extend ActiveSupport::Concern

  included do
    # Array of Symbols, representing association names
    cattr_accessor :publishable_associations

    class << self
      # can be called multiple times.
      # each call appends to publishable_associations
      def associations_to_publish(*association_list)
        self.publishable_associations ||= []
        self.publishable_associations.concat(Array[*association_list])
        self.publishable_associations.uniq!
      end
    end
  end

  def deep_publish!
    ActiveRecord::Base.transaction { _dangerous_deep_publish }
  end

  def deep_discard!
    ActiveRecord::Base.transaction { _dangerous_deep_discard }
  end

  def deep_save_draft!
    ActiveRecord::Base.transaction { _dangerous_deep_save }
  end

  def deep_trash!
    ActiveRecord::Base.transaction { _dangerous_deep_trash }
  end

  # Use instead of destroy
  def _dangerous_deep_trash
    draft_destruction
    _invoke_on_publishable_associations(:_dangerous_deep_trash)
  end

  # Use instead of save/update
  def _dangerous_deep_save
    save_draft
    _invoke_on_publishable_associations(:_dangerous_deep_save)
  end

  def _dangerous_deep_publish
    draft&.publish!
    _invoke_on_publishable_associations(:_dangerous_deep_publish)
  end

  def _dangerous_deep_discard
    draft&.revert!
    _invoke_on_publishable_associations(:_dangerous_deep_discard)
  end

  def _invoke_on_publishable_associations(method)
    return unless publishable_associations.present?

    publishable_associations.map do |association|
      # superclasses may not respond_to, but subclasses might
      next unless respond_to?(association)

      relation = send(association)
      _invoke_on_relation(relation, method)
    end.flatten
  end

  # A relation may be the result of a has_many
  # or a belongs_to relationship.
  #
  # @param [Symbol] method
  def _invoke_on_relation(relation, method)
    # has_many / collection of records
    return relation.each(&method) if relation.respond_to?(:each)

    # belongs_to / singular record
    relation.send(method)
  end
end

Tests:

require 'rails_helper'

describe 'Versioning' do
  describe 'Drafting' do
    describe 'Nested Attributes' do
      context 'Updating' do
        it 'saves a nested model as a draft' do
          l = create(:question)
          s = create(:question_option, question: l)

          l.attributes = {
            name: 'updated parent',
            question_options_attributes: [
              {
                id: s.id,
                text: 'child updated'
              }
            ]
          }

          expect { l.deep_save_draft! }
            .to change(Versioning::DraftMetadata, :count).by(2)
        end

        it 'marks the nested model as trashed' do
          l = create(:question)
          s = create(:question_option, question: l)

          l.attributes = {
            name: 'updated parent',
            question_options_attributes: [
              {
                id: s.id,
                text: 'child updated'
              }
            ]
          }

          expect { l.deep_trash! }
            .to change(Versioning::DraftMetadata, :count).by(2)
            .and change(Question.trashed, :count).by(1)
            .and change(QuestionOption.trashed, :count).by(1)
        end
      end
    end
  end
end

Setup:

class Question < ApplicationRecord
  include DeeplyPublishable
  has_drafts
  associations_to_publish :question_options

  has_many :question_options
  accepts_nested_attributes_for :question_options, allow_destroy: true
end

class QuestionOption < ApplicationRecord
  include DeeplyPublishable
  has_drafts

  belongs_to :question


  # ... more stuff

There is all or nothing at the moment.
I currently can't delete a nested object while updating the parent.

[NullVoxPopuli]

actually, _destroy is stored on the models, so I just need to change _invoke_on_relation, and then I'll have all the use cases for nested_attributes

[NullVoxPopuli]

Ok, I have deleting children while updating the parent working.

Here is the final module:

# frozen_string_literal: true
# To use this, define on any model that already has `has_drafts`
# ```
# class Blog < ApplicationRecord
#   include DeeplyPublishable
#
#   has_drafts
#   associations_to_publish :posts, :pages
#   ...
# end
# ```
#
# With the above definition, there are two publicly available
# instance methods provided:
# - deep_publish!
# - deep_discard!
# - deep_save_draft!
# - deep_trash!
#
# Both of these will traverse the associations as defined by
# associations_to_publish, either publishing or discarding drafts
# for any draft object encountered.
#
# NOTE: The draft becomes the real version when published.
#       Until publish, the data for the actual model in the model's table
#       is the same as it was before the draft was created.
module DeeplyPublishable
  extend ActiveSupport::Concern

  included do
    # Array of Symbols, representing association names
    cattr_accessor :publishable_associations

    class << self
      # can be called multiple times.
      # each call appends to publishable_associations
      def associations_to_publish(*association_list)
        self.publishable_associations ||= []
        self.publishable_associations.concat(Array[*association_list])
        self.publishable_associations.uniq!
      end
    end
  end

  def deep_publish!
    ActiveRecord::Base.transaction { _dangerous_deep_publish }
  end

  def deep_discard!
    ActiveRecord::Base.transaction { _dangerous_deep_discard }
  end

  def deep_save_draft!
    ActiveRecord::Base.transaction { _dangerous_deep_save }
  end

  def deep_trash!
    ActiveRecord::Base.transaction { _dangerous_deep_trash }
  end

  # Use instead of destroy
  def _dangerous_deep_trash
    draft_destruction
    _invoke_on_publishable_associations(:_dangerous_deep_trash)
  end

  # Use instead of save/update
  def _dangerous_deep_save
    # _destroy will be true when using accepts_nested_attributes_for
    # and a nested model has been selected for deletion while
    # updating a parent model
    _destroy ? draft_destruction : save_draft

    _invoke_on_publishable_associations(:_dangerous_deep_save)
  end

  def _dangerous_deep_publish
    draft&.publish!
    _invoke_on_publishable_associations(:_dangerous_deep_publish)
  end

  def _dangerous_deep_discard
    draft&.revert!
    _invoke_on_publishable_associations(:_dangerous_deep_discard)
  end

  def _invoke_on_publishable_associations(method)
    return unless publishable_associations.present?

    publishable_associations.map do |association|
      # superclasses may not respond_to, but subclasses might
      next unless respond_to?(association)

      relation = send(association)
      _invoke_on_relation(relation, method)
    end.flatten
  end

  # A relation may be the result of a has_many
  # or a belongs_to relationship.
  #
  # @param [Symbol] method
  def _invoke_on_relation(relation, method)
    # has_many / collection of records
    return relation.each(&method) if relation.respond_to?(:each)

    # belongs_to / singular record
    relation.send(method)
  end
end

[chrisdpeters]

@NullVoxPopuli Looks great. My favorite part of running an open source project like this is being schooled by contributors. :)

I'll sit on this one little bit and think about its implications for the design of the overall API. Maybe there's a way to mix this logic into the base #save_draft, #draft_destruction and #publish! if, for example, .has_drafts can have an option similar to your proposed .associations_to_publish initializer?

This is a great workaround in the meantime. Also a reminder that I still need to convert this project into a more modular system based on the newer ActiveSupport::Concern style of doing things.

[NullVoxPopuli]

Thanks! :-)

well, a huge downside is this has to touch every node / leaf in your tree. :-(
No way to just magically know what all the nodes are and if they have drafts ahead of time.

I strongly recommend people use https://github.com/salsify/goldiloader for implicit eager loading.

[andymcintosh]

@NullVoxPopuli I was unable to get this module to work as I expected. As far as I can tell, calling save_draft on the parent resource (called indirectly by deep_save_draft!) will actually commit the changes specified in the nested_attributes to the child resource in addition to creating a draft.

So, with your Question/QuestionOption example in the test file, I'm seeing that a draft exists for QuestionOption as expected, but also that the name attribute for QuestionOption (in the question_options table) is set to "child updated" even without calling deep_publish!

Are you seeing this same behavior?

UPDATE: I see the expected behavior if I update an attribute of the parent model in addition to an attribute of the child model (via nested attributes)

In my case, this works:

@size.attributes = {dimensions_attributes: [{id: 596, value: "54"}], name: "XS (Updated)}
@size.deeply_save_draft!

But this does not (dimension has a draft, but also has a value of 54):

@size.attributes = {dimensions_attributes: [{id: 596, value: "54"}]}
@size.deeply_save_draft!

[Looooong]

Here is my module to make it work with nested attributes:

module Draftable
  extend ActiveSupport::Concern

  included do
    attr_accessor :_save_draft
    attr_accessor :_draft_destruction

    before_save if: :_save_draft do
      self._save_draft = false # the callback will be called recursively without this line
      save_draft
    end

    before_save if: :_draft_destruction do
      self._draft_destruction = false
      draft_destruction
    end
  end

end

The idea here is that draft actions can be invoked implicitly using model attributes.

Here is the usage:

class Course < ActiveRecord::Base
    include Draftable
    has_drafts

    has_many :units
    accepts_nested_attributes_for :units
end

class Unit < ActiveRecord::Base
    include Draftable
    has_drafts

    belongs_to :course
end

course = Course.create(title: 'Test Course', _save_draft: true, units_attributes: [{ title: 'Test Unit', _save_draft: true }])
course.draft? # true
course.units.first.draft? # true