westonganger / active_snapshot

Simplified snapshots and restoration for ActiveRecord models and associations with a transparent white-box implementation
MIT License
103 stars 16 forks source link

Is `has_snapshot_children` optional or mandatory? #4

Closed pjmartorell closed 2 years ago

pjmartorell commented 2 years ago

In the documentation is stated that has_snapshot_children is optional:

It defines an optional extension to your model has_snapshot_children

But when I try to create a snapshot without defining it, it raises an error Invalid `has_snapshot_children` requires block to be defined

westonganger commented 2 years ago

Should be optional. I've made a comment on the PR.

danderozier commented 2 years ago

I don't think this resolves the issue. Calling create_snapshot! on an instance of a model that doesn't define has_snapshot_children results in an ArgumentError. See snapshots_concern:53.

Here's an example failing test:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", "~> 6.x.x"
  gem "sqlite3"
  gem "active_snapshot"
end

require "active_record"
require "minitest/autorun"
require "logger"
require "active_snapshot"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.string :title
  end

  create_table :snapshots do |t|
    t.belongs_to :item, polymorphic: true, null: false, index: true
    t.string :identifier, null: false, unique: [:item_id, :item_type], index: true
    t.belongs_to :user, polymorphic: true
    t.text :metadata
    t.datetime :created_at, null: false
  end

  create_table :snapshot_items do |t|
    t.belongs_to :snapshot, null: false, index: true
    t.belongs_to :item, polymorphic: true, null: false, unique: [:snapshot_id], index: true
    t.text :object, null: false
    t.datetime :created_at, null: false
    t.string :child_group_name
  end
end

class Post < ActiveRecord::Base
  include ActiveSnapshot
end

class BugTest < ActiveSupport::TestCase
  def test_snapshot
    post = Post.create!(title: 'Test')
    snapshot = post.create_snapshot!('snapshot_1')

    assert_not_nil snapshot.id
  end
end
westonganger commented 2 years ago

v0.2.4 is now released which contains the latest fix for this.