-
Notifications
You must be signed in to change notification settings - Fork 64
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
Problem with nested attributes #48
Comments
@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 |
Hi @chrisdpeters, thanks for response, i thought about this solution but my currect form has many nested forms in it, it may be problem. |
@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! |
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. 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. |
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):
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 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. |
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 |
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. |
actually, |
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 |
@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 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 |
Thanks! :-) well, a huge downside is this has to touch every node / leaf in your tree. :-( I strongly recommend people use https://github.com/salsify/goldiloader for implicit eager loading. |
@NullVoxPopuli I was unable to get this module to work as I expected. As far as I can tell, calling 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! |
Here is my module to make it work with nested attributes:
The idea here is that draft actions can be invoked implicitly using model attributes. Here is the usage:
|
Given:
the problem - nested object are created or destroyed twice(raising not found on second destroy).
Does anyone faced with similar problem ?
The text was updated successfully, but these errors were encountered: