From 652ff3815f4958f0c5ec1650f095df1b7be0bb53 Mon Sep 17 00:00:00 2001 From: Vic-L Date: Sat, 26 Oct 2019 17:57:30 +0800 Subject: [PATCH 1/3] Allow storing drafts under user of other class names --- lib/drafting/base_class_methods.rb | 12 ++++++++++-- lib/drafting/draft.rb | 2 +- lib/drafting/instance_methods.rb | 3 ++- .../drafting/migration/templates/migration.rb | 4 ++-- spec/drafting/class_methods_spec.rb | 10 ++++++++++ spec/drafting/instance_methods_spec.rb | 15 +++++++++++++++ spec/factories/admin_user.rb | 5 +++++ spec/models/admin_user.rb | 3 +++ spec/spec_helper.rb | 1 + spec/support/spec_migration.rb | 5 +++++ 10 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 spec/factories/admin_user.rb create mode 100644 spec/models/admin_user.rb diff --git a/lib/drafting/base_class_methods.rb b/lib/drafting/base_class_methods.rb index 000aab5..b0dc26e 100644 --- a/lib/drafting/base_class_methods.rb +++ b/lib/drafting/base_class_methods.rb @@ -15,11 +15,19 @@ def has_drafts(options={}) unless parent_class.method_defined? :drafts parent_class.class_eval do def drafts(user) - Draft.where(user: user, parent: self) + Draft.where( + user: user, + user_type: user.try(:class).try(:name), + parent: self + ) end def self.child_drafts(user) - Draft.where(user: user, parent_type: self.base_class.name) + Draft.where( + user: user, + user_type: user.try(:class).try(:name), + parent_type: self.base_class.name + ) end end end diff --git a/lib/drafting/draft.rb b/lib/drafting/draft.rb index ad0c9ab..47dbbce 100644 --- a/lib/drafting/draft.rb +++ b/lib/drafting/draft.rb @@ -1,5 +1,5 @@ class Draft < ActiveRecord::Base - belongs_to :user + belongs_to :user, polymorphic: true belongs_to :parent, polymorphic: true validates_presence_of :data, :target_type diff --git a/lib/drafting/instance_methods.rb b/lib/drafting/instance_methods.rb index a7819d1..0e511ec 100644 --- a/lib/drafting/instance_methods.rb +++ b/lib/drafting/instance_methods.rb @@ -7,7 +7,8 @@ def save_draft(user=nil) draft.data = dump_to_draft draft.target_type = self.class.name - draft.user = user + 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 result = draft.save diff --git a/lib/generators/drafting/migration/templates/migration.rb b/lib/generators/drafting/migration/templates/migration.rb index 54e3934..e664cb2 100644 --- a/lib/generators/drafting/migration/templates/migration.rb +++ b/lib/generators/drafting/migration/templates/migration.rb @@ -2,13 +2,13 @@ class DraftingMigration < Drafting::MIGRATION_BASE_CLASS def self.up create_table :drafts do |t| t.string :target_type, null: false - t.references :user + t.references :user, polymorphic: true, index: true t.references :parent, polymorphic: true, index: true t.binary :data, limit: 16777215, null: false t.datetime :updated_at, null: false end - add_index :drafts, [:user_id, :target_type] + add_index :drafts, [:target_type] end def self.down diff --git a/spec/drafting/class_methods_spec.rb b/spec/drafting/class_methods_spec.rb index d4ee315..1fa6650 100644 --- a/spec/drafting/class_methods_spec.rb +++ b/spec/drafting/class_methods_spec.rb @@ -2,6 +2,7 @@ describe Drafting::ClassMethods do let(:user) { FactoryBot.create(:user) } + let(:admin_user) { FactoryBot.create(:admin_user) } let(:topic) { FactoryBot.create(:topic) } let(:message) { topic.messages.build user: user, content: 'foo' } let(:page) { Page.new title: 'First post' } @@ -20,6 +21,15 @@ expect(Page.drafts(nil).count).to eq(1) expect(Page.drafts(nil).first).to be_a(Draft) end + + it 'should find Draft objects with non user' do + page.save_draft(admin_user) + + expect(Page.drafts(admin_user).count).to eq(1) + expect(Page.drafts(admin_user).first).to be_a(Draft) + + expect(Page.drafts(user).count).to eq(0) + end end describe 'from_draft' do diff --git a/spec/drafting/instance_methods_spec.rb b/spec/drafting/instance_methods_spec.rb index 1165d61..a7b65da 100644 --- a/spec/drafting/instance_methods_spec.rb +++ b/spec/drafting/instance_methods_spec.rb @@ -3,6 +3,7 @@ describe Drafting::InstanceMethods do let(:user) { FactoryBot.create(:user) } let(:other_user) { FactoryBot.create(:user) } + let(:admin_user) { FactoryBot.create(:admin_user) } let(:topic) { FactoryBot.create(:topic) } let(:message) { topic.messages.build user: user, content: 'foo' } let(:page) { Page.new title: 'First post' } @@ -62,6 +63,20 @@ expect(draft.user_id).to eq(nil) end + it 'should store Draft object for non user' do + expect { + result = page.save_draft(admin_user) + + expect(result).to eq(true) + }.to change(Draft, :count).by(1).and \ + change(Message, :count).by(0) + + draft = Draft.find(page.draft_id) + expect(draft.user).to eq(admin_user) + expect(draft.user_type).to eq(admin_user.class.name) + expect(draft.user_id).to eq(admin_user.id) + end + it 'should store extra attributes to Draft' do message.priority = 5 message.save_draft(user) diff --git a/spec/factories/admin_user.rb b/spec/factories/admin_user.rb new file mode 100644 index 0000000..a02f985 --- /dev/null +++ b/spec/factories/admin_user.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :admin_user do + name { 'Doe' } + end +end diff --git a/spec/models/admin_user.rb b/spec/models/admin_user.rb new file mode 100644 index 0000000..3efce7a --- /dev/null +++ b/spec/models/admin_user.rb @@ -0,0 +1,3 @@ +class AdminUser < ActiveRecord::Base + validates_presence_of :name +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ff4a407..1c690f3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,7 @@ # Require some example models require 'models/user' +require 'models/admin_user' require 'models/topic' require 'models/message' require 'models/page' diff --git a/spec/support/spec_migration.rb b/spec/support/spec_migration.rb index 09351fb..352a2ef 100644 --- a/spec/support/spec_migration.rb +++ b/spec/support/spec_migration.rb @@ -4,6 +4,10 @@ def self.up t.string :name, null: false end + create_table :admin_users, force: true do |t| + t.string :name, null: false + end + create_table :authors, force: true do |t| t.string :name, null: false end @@ -44,5 +48,6 @@ def self.down drop_table :topics drop_table :authors drop_table :users + drop_table :admin_users end end From 476d31dc2200b2cace98853840f36ac567002804 Mon Sep 17 00:00:00 2001 From: Vic-L Date: Sat, 26 Oct 2019 20:03:06 +0800 Subject: [PATCH 2/3] Generator spec on generating second migration file for allowing polymorphic user references without breaking previous versions --- drafting.gemspec | 1 + .../drafting/migration/migration_generator.rb | 1 + .../drafting/migration/templates/migration.rb | 4 +- .../migration/templates/non_user_migration.rb | 13 +++++ .../migration/migration_generator_spec.rb | 54 +++++++++++++++++++ spec/spec_helper.rb | 3 ++ 6 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 lib/generators/drafting/migration/templates/non_user_migration.rb create mode 100644 spec/lib/generators/drafting/migration/migration_generator_spec.rb diff --git a/drafting.gemspec b/drafting.gemspec index c163c93..33a838e 100644 --- a/drafting.gemspec +++ b/drafting.gemspec @@ -30,4 +30,5 @@ Gem::Specification.new do |spec| spec.add_development_dependency "coveralls" spec.add_development_dependency "simplecov" spec.add_development_dependency "appraisal" + spec.add_development_dependency "generator_spec" end diff --git a/lib/generators/drafting/migration/migration_generator.rb b/lib/generators/drafting/migration/migration_generator.rb index e5dbe4d..a2268fe 100644 --- a/lib/generators/drafting/migration/migration_generator.rb +++ b/lib/generators/drafting/migration/migration_generator.rb @@ -10,6 +10,7 @@ class MigrationGenerator < Rails::Generators::Base 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' end def self.next_migration_number(dirname) diff --git a/lib/generators/drafting/migration/templates/migration.rb b/lib/generators/drafting/migration/templates/migration.rb index e664cb2..54e3934 100644 --- a/lib/generators/drafting/migration/templates/migration.rb +++ b/lib/generators/drafting/migration/templates/migration.rb @@ -2,13 +2,13 @@ class DraftingMigration < Drafting::MIGRATION_BASE_CLASS def self.up create_table :drafts do |t| t.string :target_type, null: false - t.references :user, polymorphic: true, index: true + t.references :user t.references :parent, polymorphic: true, index: true t.binary :data, limit: 16777215, null: false t.datetime :updated_at, null: false end - add_index :drafts, [:target_type] + add_index :drafts, [:user_id, :target_type] end def self.down diff --git a/lib/generators/drafting/migration/templates/non_user_migration.rb b/lib/generators/drafting/migration/templates/non_user_migration.rb new file mode 100644 index 0000000..815f21a --- /dev/null +++ b/lib/generators/drafting/migration/templates/non_user_migration.rb @@ -0,0 +1,13 @@ +class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS + def self.up + add_column :drafts, :user_type, :string, index: true + + # add in user_type for existing drafts table + # for migration from old version + Draft.update_all(user_type: 'User') + end + + def self.down + remove_column :drafts, :user_type + end +end diff --git a/spec/lib/generators/drafting/migration/migration_generator_spec.rb b/spec/lib/generators/drafting/migration/migration_generator_spec.rb new file mode 100644 index 0000000..4562a5c --- /dev/null +++ b/spec/lib/generators/drafting/migration/migration_generator_spec.rb @@ -0,0 +1,54 @@ +require 'generator_spec' +require 'spec_helper' +require 'generators/drafting/migration/migration_generator' + +module Drafting + describe MigrationGenerator, type: :generator do + root_dir = File.expand_path("../../../../../../tmp", __FILE__) + destination root_dir + + describe 'new app' do + before :each do + prepare_destination + run_generator + end + + it "creates two installation db migration" do + migration_files = + Dir.glob("#{root_dir}/db/migrate/*drafting*.rb").sort + + assert_file migration_files[0], + /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + assert_file migration_files[1], + /class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + end + end + + describe 'existing app' do + before :each do + prepare_destination + run_generator + FileUtils.rm Dir.glob("#{root_dir}/db/migrate/*non_user_drafting_migration.rb") + + 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 + + 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 + + assert_file migration_files[0], + /class DraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + assert_file migration_files[1], + /class NonUserDraftingMigration < Drafting::MIGRATION_BASE_CLASS/ + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1c690f3..7878cb8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,6 +12,7 @@ $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 'factory_bot' FactoryBot.find_definitions @@ -33,6 +34,7 @@ RSpec.configure do |config| config.after(:suite) do SpecMigration.down + NonUserDraftingMigration.down DraftingMigration.down end end @@ -44,6 +46,7 @@ def setup_db ActiveRecord::Migration.verbose = false DraftingMigration.up + NonUserDraftingMigration.up SpecMigration.up end From 59636a2408595ac9f0ef00493b3ae43956f88b58 Mon Sep 17 00:00:00 2001 From: Vic-L Date: Sat, 2 Nov 2019 16:42:56 +0800 Subject: [PATCH 3/3] Add documentation on non user namespace feature for 0.5.0 --- README.md | 12 +++++++++++- lib/drafting/version.rb | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5f45f1d..d6d0344 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,17 @@ drafts = Message.drafts(current_user) messages = drafts.restore_all ``` +### Migration + +#### 0.5.x + +Starting from 0.5.x, you will be able to save drafts under a non `User` model as such: +``` +message.save_draft(author) +``` + +If you are upgrading from previous versions, simply run `rails g drafting:migration` again to generate the mising migration files. + ### Linking to parent instance ```ruby @@ -100,7 +111,6 @@ message.save! * The `user` argument can be nil if you don't want to distinguish between multiple users * Saving draft stores the data via `Marshal.dump` and `Marshal.load`. If you don't like this or need some customization, you can override the instance methods `dump_to_draft` and `load_from_draft` (see source) - ## Development After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. diff --git a/lib/drafting/version.rb b/lib/drafting/version.rb index bddcd3e..7cf61ef 100644 --- a/lib/drafting/version.rb +++ b/lib/drafting/version.rb @@ -1,3 +1,3 @@ module Drafting - VERSION = "0.4.2" + VERSION = "0.5.0" end