diff --git a/README.md b/README.md index 53ea2b6..a80bce0 100644 --- a/README.md +++ b/README.md @@ -66,16 +66,45 @@ drafts = Message.drafts(current_user) messages = drafts.restore_all ``` -### Migration +### Migrations & Their Features + +If you are upgrading from previous versions, simply run `rails g drafting:migration` again to generate the mising migration files. #### 0.5.x Starting from 0.5.x, you will be able to save drafts under a non `User` model as such: -``` + +```ruby message.save_draft(author) ``` -If you are upgrading from previous versions, simply run `rails g drafting:migration` again to generate the mising migration files. +#### 0.6.x + +Starting from 0.6.x, you will be able to save metadata to your `draft` (eg. to label your drafts) as such: + +```ruby +message.save_draft(author, { title: 'Final Draft 2022-08-06' }) +draft = Draft.find(message.draft_id) +draft.title # => 'Final Draft 2022-08-06' + +message.update_draft( + author, + { content: 'New content for message' }, + { + title: 'Final Final Draft 2022-08-06', + version: '1.123' + } +) +message.content # => 'New content for message' +draft = Draft.find(message.draft_id) +draft.title # => 'Final Final Draft 2022-08-06' +draft.version # => '1.123' +draft.version = '1.5' +draft.save +draft.reload.version # => '1.5' +``` + +Note that the `metadata` on the draft is essentially a [Rails store](https://api.rubyonrails.org/classes/ActiveRecord/Store.html) whose keys are dynamically generated according to your application needs. They are generated on the fly via `method_missing`. ### Linking to parent instance diff --git a/bin/console b/bin/console index b29aea3..8f2d48a 100755 --- a/bin/console +++ b/bin/console @@ -4,7 +4,9 @@ require "bundler/setup" require "drafting" $LOAD_PATH.unshift(File.expand_path('..', File.dirname(__FILE__))) -require 'lib/generators/drafting/migration/templates/migration.rb' +Dir["#{Drafting.root}/lib/generators/drafting/migration/templates/*.rb"].each do |filename| + require filename +end require 'spec/support/spec_migration.rb' require 'spec/models/user' @@ -24,7 +26,9 @@ ActiveRecord::Base.configurations = YAML.load_file('spec/database.yml') ActiveRecord::Base.establish_connection(:sqlite) ActiveRecord::Migration.verbose = false -DraftingMigration.up +Drafting::MigrationGenerator.loop_through_migration_files do |_, filename| + Object.const_get(filename.gsub('.rb', '').camelize).up +end SpecMigration.up require "irb" diff --git a/lib/drafting/draft.rb b/lib/drafting/draft.rb index 47dbbce..f3bad0d 100644 --- a/lib/drafting/draft.rb +++ b/lib/drafting/draft.rb @@ -4,6 +4,8 @@ class Draft < ActiveRecord::Base validates_presence_of :data, :target_type + store :metadata, accessors: [], coder: JSON + def restore target_type.constantize.from_draft(self) end @@ -11,4 +13,21 @@ def restore def self.restore_all find_each.map(&:restore) end + + private + + def method_missing(name, *args) + method_name = name.to_s + if method_name !~ /[a-z0-9_]+=?$/ + super + else + # TODO: executing on instance's class, not on it's eigenclass, so that store_accessors are permanent in application lifecycle, good idea? 🤔 + Draft.store_accessor :metadata, method_name.gsub('=', "").to_sym + public_send(name, *args) + end + end + + def respond_to_missing?(sym, include_private) + true + end end diff --git a/lib/drafting/instance_methods.rb b/lib/drafting/instance_methods.rb index 0e511ec..672bc31 100644 --- a/lib/drafting/instance_methods.rb +++ b/lib/drafting/instance_methods.rb @@ -1,6 +1,6 @@ module Drafting module InstanceMethods - def save_draft(user=nil) + def save_draft(user=nil, metadata={}) return false unless self.new_record? draft = Draft.find_by_id(self.draft_id) || Draft.new @@ -10,16 +10,20 @@ def save_draft(user=nil) draft.user_id = user.try(:id) draft.user_type = user.try(:class).try(:name) draft.parent = self.send(self.class.draft_parent) if self.class.draft_parent + metadata = metadata.with_indifferent_access + metadata.each_key do |key| + draft.public_send("#{key}=", metadata[key]) + end result = draft.save self.draft_id = draft.id if result result end - def update_draft(user, attributes) + def update_draft(user, attributes, metadata={}) with_transaction_returning_status do assign_attributes(attributes) - save_draft(user) + save_draft(user, metadata) end end diff --git a/lib/drafting/version.rb b/lib/drafting/version.rb index d211b4e..98fd687 100644 --- a/lib/drafting/version.rb +++ b/lib/drafting/version.rb @@ -1,3 +1,7 @@ module Drafting - VERSION = "0.5.1" + VERSION = "0.6.0" + + def self.root + File.expand_path '../../..', __FILE__ + end end diff --git a/lib/generators/drafting/migration/migration_generator.rb b/lib/generators/drafting/migration/migration_generator.rb index 1262460..8c4d43b 100644 --- a/lib/generators/drafting/migration/migration_generator.rb +++ b/lib/generators/drafting/migration/migration_generator.rb @@ -8,19 +8,54 @@ class MigrationGenerator < Rails::Generators::Base desc 'Generates migration for Drafting' source_root File.expand_path('../templates', __FILE__) - def create_migration_file - migration_template 'migration.rb', 'db/migrate/drafting_migration.rb' - migration_template 'non_user_migration.rb', 'db/migrate/non_user_drafting_migration.rb' + def self.loop_through_migration_files(reverse: false) + files = Dir.glob("#{MigrationGenerator.source_root}/*.rb") + + files = files.reverse if reverse + + files.each_with_index do |abs_path, index| + original_filename = File.basename(abs_path) + filename = original_filename.split('-').last + + yield original_filename, filename, index + end + end + + def validation + Drafting::MigrationGenerator.loop_through_migration_files do |original_filename| + # these numbers will keep the migration files generated in order + # for backwards compatibility, do NOT change the order of existing migration file templates🙏 + raise 'Migration files should start with a number followed by a dash to dictate the order of migration files to be generated' if original_filename !~ /^[\d]+\-.*drafting_migration\.rb/ + end + end + + ######### + # USAGE # + ######### + + # Instance methods in this Generator will run in sequence (starting from `validation` above👆) + # The methods generated dynamically below will create the migration file in order + # This order is dictated by the number prefix in the name of the migration template files + # naming format should follow: `-custom_name_drafting_migration.rb` + + loop_through_migration_files do |original_filename, filename, index| + define_method "create_migration_file#{index}" do + migration_template original_filename, "db/migrate/#{filename}" + end end def self.next_migration_number(dirname) - if ActiveRecord::Base.timestamped_migrations - # ensure timestamp of the multiple migration files generated - # will be different - timestamp = Time.now.utc.strftime("%Y%m%d%H%M%S").to_i - timestamp += 1 if current_migration_number(dirname) == timestamp + format = '%Y%m%d%H%M%S' + + # check if migration number already a timestamp for timestamped migrations + # strptime throws error, and rescue handles if not the case + DateTime.strptime(current_migration_number(dirname).to_s, format) - timestamp + (DateTime.parse(current_migration_number(dirname).to_s) + 1.second).strftime(format) + rescue ArgumentError + if ActiveRecord::Base.timestamped_migrations + # this will only run the first migration file is generated + Time.now.utc.strftime("%Y%m%d%H%M%S").to_i else "%.3d" % (current_migration_number(dirname) + 1) end diff --git a/lib/generators/drafting/migration/templates/migration.rb b/lib/generators/drafting/migration/templates/0-drafting_migration.rb similarity index 100% rename from lib/generators/drafting/migration/templates/migration.rb rename to lib/generators/drafting/migration/templates/0-drafting_migration.rb diff --git a/lib/generators/drafting/migration/templates/non_user_migration.rb b/lib/generators/drafting/migration/templates/1-non_user_drafting_migration.rb similarity index 100% rename from lib/generators/drafting/migration/templates/non_user_migration.rb rename to lib/generators/drafting/migration/templates/1-non_user_drafting_migration.rb diff --git a/lib/generators/drafting/migration/templates/2-metadata_drafting_migration.rb b/lib/generators/drafting/migration/templates/2-metadata_drafting_migration.rb new file mode 100644 index 0000000..f4ba06d --- /dev/null +++ b/lib/generators/drafting/migration/templates/2-metadata_drafting_migration.rb @@ -0,0 +1,10 @@ +class MetadataDraftingMigration < Drafting::MIGRATION_BASE_CLASS + def self.up + add_column :drafts, :metadata, :text + Draft.reset_column_information + end + + def self.down + remove_column :drafts, :metadata + end +end diff --git a/spec/drafting/draft_spec.rb b/spec/drafting/draft_spec.rb index d7b4bbd..20f3ddf 100644 --- a/spec/drafting/draft_spec.rb +++ b/spec/drafting/draft_spec.rb @@ -20,7 +20,7 @@ before do Draft.delete_all - message1.save_draft(user) + message1.save_draft(user, title: 'Title1') message2.save_draft(user) end diff --git a/spec/drafting/instance_methods_spec.rb b/spec/drafting/instance_methods_spec.rb index a7b65da..340b4ba 100644 --- a/spec/drafting/instance_methods_spec.rb +++ b/spec/drafting/instance_methods_spec.rb @@ -77,6 +77,23 @@ expect(draft.user_id).to eq(admin_user.id) end + it 'should save metadata on draft and allow changes' do + message.save_draft( + nil, + { + title: 'Message Title', + sub_title: 'Message SubTitle' + } + ) + draft = Draft.find(message.draft_id) + expect(draft.title).to eq 'Message Title' + expect(draft.sub_title).to eq 'Message SubTitle' + + draft.title = 'New Title' + draft.save + expect(draft.reload.title).to eq 'New Title' + end + it 'should store extra attributes to Draft' do message.priority = 5 message.save_draft(user) @@ -85,7 +102,7 @@ expect(draft.restore.priority).to eq(5) end - it 'should store assocations to Draft' do + it 'should store associations to Draft' do message = topic.messages.build user: user, content: 'foo' message.tags.build name: 'important' message.tags.build name: 'ruby' @@ -118,7 +135,7 @@ end describe 'update_draft' do - it 'should update existing Draft object' do + it 'should update existing Draft object (without metadata)' do message.save_draft(user) expect { @@ -129,6 +146,31 @@ draft = Draft.find(message.draft_id) expect(draft.restore.attributes).to eq(message.attributes) end + + it 'should update existing Draft object (with metadata)' do + message.save_draft(user) + + expect { + message.update_draft( + user, + { content: 'bar' }, + { + title: 'Message Title', + sub_title: 'Message SubTitle' + } + ) + }.to change(Draft, :count).by(0).and \ + change(Message, :count).by(0) + + draft = Draft.find(message.draft_id) + expect(draft.title).to eq 'Message Title' + expect(draft.sub_title).to eq 'Message SubTitle' + expect(draft.restore.attributes).to eq(message.attributes) + + draft.title = 'New Title' + draft.save + expect(draft.reload.title).to eq 'New Title' + end end describe 'clear_draft' do diff --git a/spec/lib/generators/drafting/migration/migration_generator_spec.rb b/spec/lib/generators/drafting/migration/migration_generator_spec.rb index c89a388..a0f596d 100644 --- a/spec/lib/generators/drafting/migration/migration_generator_spec.rb +++ b/spec/lib/generators/drafting/migration/migration_generator_spec.rb @@ -28,26 +28,18 @@ module Drafting run_generator end - it "creates two installation db migration" do - migration_files = - Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort - - assert_equal migration_files.count, 2 - - assert_file migration_files[0], - /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ - assert_file migration_files[1], - /class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ - end + include_examples 'eventual output', root_dir it "creates migration files of different timestamp" do migration_files = Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort - migration_no1 = File.basename(migration_files[0]).split("_").first - migration_no2 = File.basename(migration_files[1]).split("_").first + timestamps = migration_files.map do |migration_file| + File.basename(migration_file).split("_").first + end - assert_not_equal migration_no1, migration_no2 + assert_equal timestamps.size, 3 + assert_equal timestamps.uniq.size, 3 end end @@ -55,26 +47,79 @@ module Drafting before :each do prepare_destination run_generator - FileUtils.rm Dir.glob("#{root_dir}/db/migrate/*non_user_drafting_migration.rb") + end - migration_files = - Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort - expect(migration_files.count).to eq 1 - assert_file migration_files[0], - /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + describe 'before 0.5.x' do + before :each do + FileUtils.rm Dir.glob("#{root_dir}/db/migrate/*non_user_drafting_migration.rb") + FileUtils.rm Dir.glob("#{root_dir}/db/migrate/*metadata_drafting_migration.rb") - run_generator + migration_files = + Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort + expect(migration_files.count).to eq 1 + assert_file migration_files[0], + /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + + run_generator + end + + include_examples 'eventual output', root_dir end - it "creates only one more db migration" do - migration_files = - Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort - expect(migration_files.count).to eq 2 + describe 'after 0.5.x' do + before :each do + FileUtils.rm Dir.glob("#{root_dir}/db/migrate/*metadata_drafting_migration.rb") + + migration_files = + Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort + expect(migration_files.count).to eq 2 + assert_file migration_files[0], + /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + assert_file migration_files[1], + /class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + + run_generator + end + + include_examples 'eventual output', root_dir + end + end + + describe 'wrongly named migration files' do + before :each do + prepare_destination + end + + describe 'not starting with "-"' do + let!(:filename) { "#{Drafting.root}/lib/generators/drafting/migration/templates/something_drafting_migration.rb" } + + before :each do + FileUtils.touch(filename) + end + + after :each do + FileUtils.rm(filename) + end + + it 'should raise error' do + expect { run_generator }.to raise_error('Migration files should start with a number followed by a dash to dictate the order of migration files to be generated') + end + end + + describe 'not ending with "drafting_migration.rb"' do + let!(:filename) { "#{Drafting.root}/lib/generators/drafting/migration/templates/something_migration.rb" } + + before :each do + FileUtils.touch(filename) + end + + after :each do + FileUtils.rm(filename) + end - assert_file migration_files[0], - /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ - assert_file migration_files[1], - /class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + it 'should raise error' do + expect { run_generator }.to raise_error('Migration files should start with a number followed by a dash to dictate the order of migration files to be generated') + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7878cb8..93ef169 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,8 +11,11 @@ $LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) require 'drafting' -require 'generators/drafting/migration/templates/migration.rb' -require 'generators/drafting/migration/templates/non_user_migration.rb' + +require "#{Drafting.root}/lib/generators/drafting/migration/migration_generator" +Dir["#{Drafting.root}/lib/generators/drafting/migration/templates/*.rb"].each do |filename| + require filename +end require 'factory_bot' FactoryBot.find_definitions @@ -34,8 +37,9 @@ RSpec.configure do |config| config.after(:suite) do SpecMigration.down - NonUserDraftingMigration.down - DraftingMigration.down + Drafting::MigrationGenerator.loop_through_migration_files(reverse: true) do |_, filename| + Object.const_get(filename.gsub('.rb', '').camelize).down + end end end @@ -45,8 +49,9 @@ def setup_db ActiveRecord::Base.establish_connection(:sqlite) ActiveRecord::Migration.verbose = false - DraftingMigration.up - NonUserDraftingMigration.up + Drafting::MigrationGenerator.loop_through_migration_files do |_, filename| + Object.const_get(filename.gsub('.rb', '').camelize).up + end SpecMigration.up end diff --git a/spec/support/shared/examples/migrations.rb b/spec/support/shared/examples/migrations.rb new file mode 100644 index 0000000..018cff8 --- /dev/null +++ b/spec/support/shared/examples/migrations.rb @@ -0,0 +1,15 @@ +RSpec.shared_examples 'eventual output' do |root_dir| + it 'creates 3 installation db migration files in order eventually' do + migration_files = + Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort + + assert_equal migration_files.count, 3 + + assert_file migration_files[0], + /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + assert_file migration_files[1], + /class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + assert_file migration_files[2], + /class MetadataDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + end +end