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

Reflections #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Reflections #9

wants to merge 4 commits into from

Conversation

leastbad
Copy link
Owner

@leastbad leastbad commented Feb 8, 2022

Closes #8

As I outlined in https://discordapp.com/channels/629472241427415060/911174663076446208/940471462911950881 this is a compromise between punting to documentation or attempting to boil the ocean by reopening the Active Entity classes and trying to teach them about id and CRUD. It could be a temporary fix.

There are a number of shortcomings to this approach; several important options are not available; specifically, there is no autosave and no dependent. autosave would give some flexibility and dependent would allow recursive destroy.

Also, if you run save on an embedded child model that has versioning enabled, the recursive nature of this approach causes the child model to be saved twice, meaning that the current version will be 2.

class Government < AllFutures::Base
  validates :name, presence: true
  attribute :name, :string, default: -> { Faker::Nation.nationality }
  embeds_one :spy
  enable_versioning!
end

class Spy < AllFutures::Base
  embedded_in :government
  attribute :government_id, :string
  attribute :name, :string, default: -> { Faker::Name.name }
  enable_versioning!
end

x = Government.new
x.build_spy
x.spy.save # saves both

y = Government.new
y.build_spy
y.save # saves both

One shortcoming of this approach is that child records won't automatically pick up the parent association id:

x = Government.new
x.save
x.build_spy
x.spy.save # does not have `government_id` set!

I don't have a current fix for this beyond documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActiveEntity embeds are not persisted by AllFutures v2
1 participant