diff --git a/Gemfile b/Gemfile index dde7f6d1f..f52a82458 100644 --- a/Gemfile +++ b/Gemfile @@ -54,7 +54,7 @@ end group :test do gem "database_cleaner", '1.6.2' - gem 'shoulda', ">= 3.5" + gem 'shoulda-matchers', '~> 3.1.2' gem 'fabrication' gem 'faker' gem 'capybara', '~> 2.7' diff --git a/Gemfile.lock b/Gemfile.lock index 6a7480ce9..57b021967 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -329,12 +329,8 @@ GEM selenium-webdriver (3.11.0) childprocess (~> 0.5) rubyzip (~> 1.2) - shoulda (3.5.0) - shoulda-context (~> 1.0, >= 1.0.1) - shoulda-matchers (>= 1.4.1, < 3.0) - shoulda-context (1.2.1) - shoulda-matchers (2.8.0) - activesupport (>= 3.0.0) + shoulda-matchers (3.1.2) + activesupport (>= 4.0.0) simple_form (3.1.0) actionpack (~> 4.0) activemodel (~> 4.0) @@ -427,7 +423,7 @@ DEPENDENCIES rubocop (~> 0.52.1) sass-rails (~> 5.0.7) select2-rails - shoulda (>= 3.5) + shoulda-matchers (~> 3.1.2) simple_form (>= 3.0.0) skylight uglifier (= 2.7.2) diff --git a/app/models/event.rb b/app/models/event.rb new file mode 100644 index 000000000..2058045b7 --- /dev/null +++ b/app/models/event.rb @@ -0,0 +1,20 @@ +class Event < ActiveRecord::Base + enum action: { created: 0, updated: 1 } + + belongs_to :post + belongs_to :member + belongs_to :transfer + + validates :action, presence: true + validate :resource_presence + + private + + # Validates that only one resource id is set + # + def resource_presence + return if post_id.present? ^ member_id.present? ^ transfer_id.present? + + errors.add(:base, 'Specify only one resource id: `post_id`, `member_id` or `transfer_id`') + end +end diff --git a/app/services/persister/member_persister.rb b/app/services/persister/member_persister.rb index 4bed82cf1..51594ffd6 100644 --- a/app/services/persister/member_persister.rb +++ b/app/services/persister/member_persister.rb @@ -7,7 +7,33 @@ def initialize(member) end def save - member.save + ::ActiveRecord::Base.transaction do + member.save! + create_save_event! + member + end + rescue ActiveRecord::RecordInvalid => _exception + false + end + + def update_attributes(params) + ::ActiveRecord::Base.transaction do + member.update_attributes!(params) + create_update_event! + member + end + rescue ActiveRecord::RecordInvalid => _exception + false + end + + private + + def create_save_event! + ::Event.create! action: :created, member: member + end + + def create_update_event! + ::Event.create! action: :updated, member: member end end end diff --git a/app/services/persister/post_persister.rb b/app/services/persister/post_persister.rb index eb4e94fe3..0c0bb8004 100644 --- a/app/services/persister/post_persister.rb +++ b/app/services/persister/post_persister.rb @@ -7,11 +7,33 @@ def initialize(post) end def save - post.save + ::ActiveRecord::Base.transaction do + post.save! + create_save_event! + post + end + rescue ActiveRecord::RecordInvalid => _exception + false end def update_attributes(params) - post.update_attributes(params) + ::ActiveRecord::Base.transaction do + post.update_attributes!(params) + create_update_event! + post + end + rescue ActiveRecord::RecordInvalid => _exception + false + end + + private + + def create_save_event! + ::Event.create! action: :created, post: post + end + + def create_update_event! + ::Event.create! action: :updated, post: post end end end diff --git a/app/services/persister/transfer_persister.rb b/app/services/persister/transfer_persister.rb index 5e72e46d7..a5fd86f12 100644 --- a/app/services/persister/transfer_persister.rb +++ b/app/services/persister/transfer_persister.rb @@ -7,7 +7,33 @@ def initialize(transfer) end def save - transfer.save + ::ActiveRecord::Base.transaction do + transfer.save! + create_save_event! + transfer + end + rescue ActiveRecord::RecordInvalid => _exception + false + end + + def update_attributes(params) + ::ActiveRecord::Base.transaction do + transfer.update_attributes!(params) + create_update_event! + transfer + end + rescue ActiveRecord::RecordInvalid => _exception + false + end + + private + + def create_save_event! + ::Event.create! action: :created, transfer: transfer + end + + def create_update_event! + ::Event.create! action: :updated, transfer: transfer end end end diff --git a/db/migrate/20180501093846_create_events.rb b/db/migrate/20180501093846_create_events.rb new file mode 100644 index 000000000..af3fffbee --- /dev/null +++ b/db/migrate/20180501093846_create_events.rb @@ -0,0 +1,43 @@ +# This migration does not use Rails ActiveRecord ORM DSL since +# it doesn't provide data integrity (foreign key) for polymorphic models. +# +# The technique applied is called Exclusive Belongs To (AKA Exclusive Arc) +# +# Please read the following article for more details: +# https://hashrocket.com/blog/posts/modeling-polymorphic-associations-in-a-relational-database +# +class CreateEvents < ActiveRecord::Migration + def up + create_table :events do |t| + t.integer :action, null:false + t.integer :post_id + t.integer :member_id + t.integer :transfer_id + t.timestamps + end + + add_foreign_key :events, :posts, name: 'events_post_id_fkey' + add_foreign_key :events, :members, name: 'events_member_id_fkey' + add_foreign_key :events, :transfers, name: 'events_transfer_id_fkey' + + add_index :events, :post_id, unique: true, where: 'post_id IS NOT NULL' + add_index :events, :member_id, unique: true, where: 'member_id IS NOT NULL' + add_index :events, :transfer_id, unique: true, where: 'transfer_id IS NOT NULL' + + execute <<-SQL + ALTER TABLE events + ADD CHECK(action IN (0, 1)), + ADD CHECK( + ( + (post_id IS NOT NULL)::integer + + (member_id IS NOT NULL)::integer + + (transfer_id IS NOT NULL)::integer + ) = 1 + ); + SQL + end + + def down + drop_table :events + end +end diff --git a/db/schema.rb b/db/schema.rb index e4e2e2e1d..f3751e936 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180221161343) do +ActiveRecord::Schema.define(version: 20180501093846) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -76,6 +76,19 @@ add_index "documents", ["documentable_id", "documentable_type"], name: "index_documents_on_documentable_id_and_documentable_type", using: :btree add_index "documents", ["label"], name: "index_documents_on_label", using: :btree + create_table "events", force: :cascade do |t| + t.integer "action", null: false + t.integer "post_id" + t.integer "member_id" + t.integer "transfer_id" + t.datetime "created_at" + t.datetime "updated_at" + end + + add_index "events", ["member_id"], name: "index_events_on_member_id", unique: true, where: "(member_id IS NOT NULL)", using: :btree + add_index "events", ["post_id"], name: "index_events_on_post_id", unique: true, where: "(post_id IS NOT NULL)", using: :btree + add_index "events", ["transfer_id"], name: "index_events_on_transfer_id", unique: true, where: "(transfer_id IS NOT NULL)", using: :btree + create_table "members", force: :cascade do |t| t.integer "user_id" t.integer "organization_id" @@ -198,4 +211,7 @@ add_index "users", ["email"], name: "index_users_on_email", using: :btree add_foreign_key "accounts", "organizations" + add_foreign_key "events", "members", name: "events_member_id_fkey" + add_foreign_key "events", "posts", name: "events_post_id_fkey" + add_foreign_key "events", "transfers", name: "events_transfer_id_fkey" end diff --git a/spec/fabricators/transfer_fabricator.rb b/spec/fabricators/transfer_fabricator.rb new file mode 100644 index 000000000..02b6c193a --- /dev/null +++ b/spec/fabricators/transfer_fabricator.rb @@ -0,0 +1,5 @@ +Fabricator(:transfer) do + source { Fabricate(:account) } + destination { Fabricate(:account) } + amount 10 +end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb new file mode 100644 index 000000000..2961fc2ee --- /dev/null +++ b/spec/models/event_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Event do + describe 'Validations' do + it { is_expected.to validate_presence_of(:action) } + it do + is_expected.to define_enum_for(:action) + .with([:created, :updated]) + end + + describe '#resource_presence validation' do + let(:post) { Fabricate(:post) } + let(:member) { Fabricate(:member) } + let(:event) { Event.new action: 'created' } + + context 'has no resources' do + it { expect(event).to_not be_valid } + end + + context 'has one resource' do + before { event.post = post } + + it { expect(event).to be_valid } + end + + context 'has two resources' do + before { event.post = post; event.member = member } + + it { expect(event).to_not be_valid } + end + end + end + + describe 'Relations' do + it { is_expected.to belong_to(:post) } + it { is_expected.to belong_to(:member) } + it { is_expected.to belong_to(:transfer) } + + it { is_expected.to have_db_column(:post_id) } + it { is_expected.to have_db_column(:member_id) } + it { is_expected.to have_db_column(:transfer_id) } + end + + describe 'Indexes' do + it { is_expected.to have_db_index(:post_id) } + it { is_expected.to have_db_index(:member_id) } + it { is_expected.to have_db_index(:transfer_id) } + end +end diff --git a/spec/services/persister/member_persister_spec.rb b/spec/services/persister/member_persister_spec.rb index 9ca7f61b6..7377ee375 100644 --- a/spec/services/persister/member_persister_spec.rb +++ b/spec/services/persister/member_persister_spec.rb @@ -3,15 +3,32 @@ describe Persister::MemberPersister do let(:organization) { Fabricate(:organization) } let(:user) { Fabricate(:user) } + let(:member) { Fabricate.build(:member, user: user, organization: organization) } + let(:persister) { ::Persister::MemberPersister.new(member) } describe '#save' do + before { persister.save } + it 'saves the member' do - member = Member.new(user: user, organization: organization) + expect(member).to be_persisted + end - persister = ::Persister::MemberPersister.new(member) - persister.save + # TODO: write better expectation + it 'creates an event' do + expect(Event.where(member_id: member.id).first.action).to eq('created') + end + end - expect(member).to be_persisted + describe '#update_attributes' do + before { persister.update_attributes(member_uid: 666) } + + it 'updates the resource attributes' do + expect(member.member_uid).to eq(666) + end + + # TODO: write better expectation + it 'creates an event' do + expect(Event.where(member_id: member.id).first.action).to eq('updated') end end end diff --git a/spec/services/persister/post_persister_spec.rb b/spec/services/persister/post_persister_spec.rb index 9592b1fd2..6ae8139b6 100644 --- a/spec/services/persister/post_persister_spec.rb +++ b/spec/services/persister/post_persister_spec.rb @@ -4,25 +4,40 @@ let(:organization) { Fabricate(:organization) } let(:user) { Fabricate(:user) } let(:category) { Fabricate(:category) } - let(:post) { Fabricate(:post, organization: organization) } + let(:post) do + Fabricate.build( + :offer, + organization: organization, + user: user, + category: category, + title: 'Title' + ) + end + let(:persister) { ::Persister::PostPersister.new(post) } describe '#save' do - it 'saves the post' do - post = Offer.new(organization: organization, user: user, category: category, title: 'Title') - persister = ::Persister::PostPersister.new(post) - - persister.save + before { persister.save } + it 'saves the post' do expect(post).to be_persisted end + + # TODO: write better expectation + it 'creates an event' do + expect(Event.where(post_id: post.id).first.action).to eq('created') + end end describe '#update_attributes' do - it 'updates the attributes' do - persister = ::Persister::PostPersister.new(post) - persister.update_attributes(title: 'New title') + before { persister.update_attributes(title: 'New title') } + it 'updates the resource attributes' do expect(post.title).to eq('New title') end + + # TODO: write better expectation + it 'creates an event' do + expect(Event.where(post_id: post.id).first.action).to eq('updated') + end end end diff --git a/spec/services/persister/transfer_persister_spec.rb b/spec/services/persister/transfer_persister_spec.rb index a7d77dab8..89acc338b 100644 --- a/spec/services/persister/transfer_persister_spec.rb +++ b/spec/services/persister/transfer_persister_spec.rb @@ -5,15 +5,39 @@ let(:destination_account) { Fabricate(:account) } let(:organization) { Fabricate(:organization) } let(:post) { Fabricate(:post, organization: organization) } + let(:transfer) do + Fabricate.build( + :transfer, + post: post, + source: source_account, + destination: destination_account + ) + end + let(:persister) { ::Persister::TransferPersister.new(transfer) } describe '#save' do + before { persister.save } + it 'saves the transfer' do - transfer = Transfer.new(post: post, source: source_account, destination: destination_account) + expect(transfer).to be_persisted + end + + # TODO: write better expectation + it 'creates an event' do + expect(Event.where(transfer_id: transfer.id).first.action).to eq('created') + end + end - persister = ::Persister::TransferPersister.new(transfer) - persister.save + describe '#update_attributes' do + before { persister.update_attributes(amount: 666) } - expect(transfer).to be_persisted + it 'updates the resource attributes' do + expect(transfer.amount).to eq(666) + end + + # TODO: write better expectation + it 'creates an event' do + expect(Event.where(transfer_id: transfer.id).first.action).to eq('updated') end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 43434d797..8e737cd26 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,6 +10,7 @@ require 'fabrication' require 'selenium/webdriver' require 'faker' +require 'shoulda/matchers' I18n.reload! Capybara.register_driver :chrome do |app| @@ -149,3 +150,10 @@ def set_browser_locale(locale) end RSpec.configure(&:infer_spec_type_from_file_location!) + +Shoulda::Matchers.configure do |config| + config.integrate do |with| + with.test_framework :rspec + with.library :rails + end +end