From 77147e7974044090c968530e98fdc8df586e00a9 Mon Sep 17 00:00:00 2001 From: Adam Gwozdowski Date: Tue, 17 Sep 2019 18:57:14 +0200 Subject: [PATCH 1/6] Added migration for discarded_at in Litigation model --- app/models/litigation.rb | 1 + .../20190917165449_add_discarded_at_to_litigations.rb | 6 ++++++ db/schema.rb | 4 +++- spec/factories/litigations.rb | 1 + spec/models/litigation_spec.rb | 1 + 5 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20190917165449_add_discarded_at_to_litigations.rb diff --git a/app/models/litigation.rb b/app/models/litigation.rb index 6d9f208d9..200214dab 100644 --- a/app/models/litigation.rb +++ b/app/models/litigation.rb @@ -16,6 +16,7 @@ # visibility_status :string default("draft") # created_by_id :bigint # updated_by_id :bigint +# discarded_at :datetime # class Litigation < ApplicationRecord diff --git a/db/migrate/20190917165449_add_discarded_at_to_litigations.rb b/db/migrate/20190917165449_add_discarded_at_to_litigations.rb new file mode 100644 index 000000000..ccce0e3ae --- /dev/null +++ b/db/migrate/20190917165449_add_discarded_at_to_litigations.rb @@ -0,0 +1,6 @@ +class AddDiscardedAtToLitigations < ActiveRecord::Migration[5.2] + def change + add_column :litigations, :discarded_at, :datetime + add_index :litigations, :discarded_at + end +end diff --git a/db/schema.rb b/db/schema.rb index eb6932c0a..7912ea612 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_09_16_190258) do +ActiveRecord::Schema.define(version: 2019_09_17_165449) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -244,7 +244,9 @@ t.string "visibility_status", default: "draft" t.bigint "created_by_id" t.bigint "updated_by_id" + t.datetime "discarded_at" t.index ["created_by_id"], name: "index_litigations_on_created_by_id" + t.index ["discarded_at"], name: "index_litigations_on_discarded_at" t.index ["document_type"], name: "index_litigations_on_document_type" t.index ["geography_id"], name: "index_litigations_on_geography_id" t.index ["jurisdiction_id"], name: "index_litigations_on_jurisdiction_id" diff --git a/spec/factories/litigations.rb b/spec/factories/litigations.rb index 52aecd37a..be4e12b61 100644 --- a/spec/factories/litigations.rb +++ b/spec/factories/litigations.rb @@ -16,6 +16,7 @@ # visibility_status :string default("draft") # created_by_id :bigint # updated_by_id :bigint +# discarded_at :datetime # FactoryBot.define do diff --git a/spec/models/litigation_spec.rb b/spec/models/litigation_spec.rb index 9e0cc8450..517f6769a 100644 --- a/spec/models/litigation_spec.rb +++ b/spec/models/litigation_spec.rb @@ -16,6 +16,7 @@ # visibility_status :string default("draft") # created_by_id :bigint # updated_by_id :bigint +# discarded_at :datetime # require 'rails_helper' From 0ad535c347fef9043e42f928e62da2825bfe7866 Mon Sep 17 00:00:00 2001 From: Adam Gwozdowski Date: Tue, 17 Sep 2019 20:32:05 +0200 Subject: [PATCH 2/6] Added migration for discarded_at in LitigationSide model. Override default Destroy action. --- app/admin/litigations.rb | 12 +++++++++ app/commands/command/destroy/litigation.rb | 26 +++++++++++++++++++ app/models/litigation.rb | 1 + app/models/litigation_side.rb | 2 ++ ...07_add_discarded_at_to_litigation_sides.rb | 6 +++++ db/schema.rb | 4 ++- 6 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 app/commands/command/destroy/litigation.rb create mode 100644 db/migrate/20190917181907_add_discarded_at_to_litigation_sides.rb diff --git a/app/admin/litigations.rb b/app/admin/litigations.rb index da1a9b9b5..c47739d42 100644 --- a/app/admin/litigations.rb +++ b/app/admin/litigations.rb @@ -108,5 +108,17 @@ def scoped_collection super.includes(:geography, :created_by, :updated_by) end + + def destroy + destroy_command = ::Command::Destroy::Litigation.new(resource.object) + + results = if destroy_command.call + {notice: 'Successfully deleted selected Litigation'} + else + {alert: 'Could not delete selected Litigation'} + end + + redirect_to admin_litigations_path(scope: current_scope.name), results + end end end diff --git a/app/commands/command/destroy/litigation.rb b/app/commands/command/destroy/litigation.rb new file mode 100644 index 000000000..44525b8f4 --- /dev/null +++ b/app/commands/command/destroy/litigation.rb @@ -0,0 +1,26 @@ +module Command + module Destroy + class Litigation + def initialize(resource) + @resource = resource + end + + def call + ActiveRecord::Base.transaction do + @resource.tap do |r| + r.discard + + r.litigation_sides.discard_all + r.events.discard_all + r.documents.discard_all + + r.legislations = [] + r.external_legislations = [] + + r.save! + end + end + end + end + end +end diff --git a/app/models/litigation.rb b/app/models/litigation.rb index 200214dab..d4a447ce6 100644 --- a/app/models/litigation.rb +++ b/app/models/litigation.rb @@ -23,6 +23,7 @@ class Litigation < ApplicationRecord include UserTrackable include Taggable include VisibilityStatus + include Discardable extend FriendlyId friendly_id :title, use: :slugged, routes: :default diff --git a/app/models/litigation_side.rb b/app/models/litigation_side.rb index 66e80154c..2d5fa1517 100644 --- a/app/models/litigation_side.rb +++ b/app/models/litigation_side.rb @@ -14,6 +14,8 @@ # class LitigationSide < ApplicationRecord + include Discardable + SIDE_TYPES = %w[a b c].freeze PARTY_TYPES = %w[ government diff --git a/db/migrate/20190917181907_add_discarded_at_to_litigation_sides.rb b/db/migrate/20190917181907_add_discarded_at_to_litigation_sides.rb new file mode 100644 index 000000000..aac4a4830 --- /dev/null +++ b/db/migrate/20190917181907_add_discarded_at_to_litigation_sides.rb @@ -0,0 +1,6 @@ +class AddDiscardedAtToLitigationSides < ActiveRecord::Migration[5.2] + def change + add_column :litigation_sides, :discarded_at, :datetime + add_index :litigation_sides, :discarded_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 7912ea612..5d5f715aa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_09_17_165449) do +ActiveRecord::Schema.define(version: 2019_09_17_181907) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -226,7 +226,9 @@ t.datetime "updated_at", null: false t.string "connected_entity_type" t.bigint "connected_entity_id" + t.datetime "discarded_at" t.index ["connected_entity_type", "connected_entity_id"], name: "index_litigation_sides_connected_entity" + t.index ["discarded_at"], name: "index_litigation_sides_on_discarded_at" t.index ["litigation_id"], name: "index_litigation_sides_on_litigation_id" end From a2bfd5be0d8881437c290dab8b9e735b549815ed Mon Sep 17 00:00:00 2001 From: Adam Gwozdowski Date: Tue, 17 Sep 2019 21:41:09 +0200 Subject: [PATCH 3/6] Specs for litigation soft-delete --- .../admin/litigations_controller_spec.rb | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/spec/controllers/admin/litigations_controller_spec.rb b/spec/controllers/admin/litigations_controller_spec.rb index 7a51221ee..6695a906d 100644 --- a/spec/controllers/admin/litigations_controller_spec.rb +++ b/spec/controllers/admin/litigations_controller_spec.rb @@ -130,6 +130,79 @@ end end + describe 'DELETE destroy' do + let!(:litigation) { create(:litigation, discarded_at: nil) } + + context 'with valid params' do + let!(:litigation_side) { create(:litigation_side, litigation: litigation) } + let!(:event) { create(:litigation_event, eventable: litigation) } + let!(:document) { create(:document, documentable: litigation) } + + let!(:legislation) { create(:legislation, litigations: [litigation]) } + let!(:external_legislation) do + create(:external_legislation, litigations: [litigation]) + end + + subject { delete :destroy, params: {id: litigation.id} } + + before do + expect { subject }.to change { Litigation.count }.by(-1) + end + + it 'discards litigation object' do + expect(Litigation.find_by_id(litigation.id)).to be_nil + end + + it 'set discarded_at date to litigation object' do + expect(litigation.reload.discarded_at).to_not be_nil + end + + it 'shows discarded litigations in all_discarded scope' do + expect(Litigation.all_discarded.find(litigation.id)).to_not be_nil + end + + it 'discard all litigation sides' do + expect(LitigationSide.find_by_id(litigation_side.id)).to be_nil + end + + it 'discard all events' do + expect(Event.find_by_id(event.id)).to be_nil + end + + it 'discard all documents' do + expect(Document.find_by_id(document.id)).to be_nil + end + + it 'removes discarded litigation from legislation' do + expect(legislation.reload.litigations).to be_empty + end + + it 'removes discarded litigation from external_legislations' do + expect(external_legislation.reload.litigations).to be_empty + end + + it 'shows proper notice' do + expect(flash[:notice]).to match('Successfully deleted selected Litigation') + end + end + + context 'with invalid params' do + let(:command) { double } + + subject { delete :destroy, params: {id: litigation.id} } + + before do + expect(::Command::Destroy::Litigation).to receive(:new).and_return(command) + expect(command).to receive(:call).and_return(nil) + end + + it 'redirects to index & renders alert message' do + expect(subject).to redirect_to(admin_litigations_path(scope: 'All')) + expect(flash[:alert]).to match('Could not delete selected Litigation') + end + end + end + def last_litigation_created Litigation.order(:created_at).last end From fbae79f5535f49fd079d5033e596fea02c506d31 Mon Sep 17 00:00:00 2001 From: Adam Gwozdowski Date: Wed, 18 Sep 2019 11:02:48 +0200 Subject: [PATCH 4/6] Soft-delete ligitation and legislation even if geography is nil --- app/commands/command/destroy/legislation.rb | 5 ++++- app/commands/command/destroy/litigation.rb | 4 +++- .../admin/legislations_controller_spec.rb | 15 ++++++++++++++- .../admin/litigations_controller_spec.rb | 15 +++++++++++---- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/commands/command/destroy/legislation.rb b/app/commands/command/destroy/legislation.rb index 2ac527c27..59bc97966 100644 --- a/app/commands/command/destroy/legislation.rb +++ b/app/commands/command/destroy/legislation.rb @@ -15,7 +15,10 @@ def call r.litigations = [] r.targets = [] - r.save! + + # Validation is false because it's possible that some precedent + # record (like geography) was removed earlier + r.save(validate: false) end end end diff --git a/app/commands/command/destroy/litigation.rb b/app/commands/command/destroy/litigation.rb index 44525b8f4..1ede8934d 100644 --- a/app/commands/command/destroy/litigation.rb +++ b/app/commands/command/destroy/litigation.rb @@ -17,7 +17,9 @@ def call r.legislations = [] r.external_legislations = [] - r.save! + # Validation is false because it's possible that some precedent + # record (like geography) was removed earlier + r.save(validate: false) end end end diff --git a/spec/controllers/admin/legislations_controller_spec.rb b/spec/controllers/admin/legislations_controller_spec.rb index e4b0997c9..52b42adfe 100644 --- a/spec/controllers/admin/legislations_controller_spec.rb +++ b/spec/controllers/admin/legislations_controller_spec.rb @@ -125,8 +125,9 @@ end describe 'DELETE destroy' do + let!(:legislation_to_delete) { create(:legislation, discarded_at: nil) } + context 'with valid params' do - let!(:legislation_to_delete) { create(:legislation, discarded_at: nil) } let!(:document) { create(:document, documentable: legislation_to_delete) } let!(:event) { create(:legislation_event, eventable: legislation_to_delete) } @@ -159,6 +160,18 @@ expect(flash[:alert]).to match('Could not delete selected Legislations') end end + + context 'when geography does not exist' do + subject { delete :destroy, params: {id: legislation_to_delete.id} } + + before do + legislation_to_delete.geography.legislations = [] + end + + it 'soft-delete even if geography is nil' do + expect { subject }.to change { Legislation.count }.by(-1) + end + end end describe 'Batch Actions' do diff --git a/spec/controllers/admin/litigations_controller_spec.rb b/spec/controllers/admin/litigations_controller_spec.rb index 6695a906d..2e286fe4e 100644 --- a/spec/controllers/admin/litigations_controller_spec.rb +++ b/spec/controllers/admin/litigations_controller_spec.rb @@ -132,6 +132,7 @@ describe 'DELETE destroy' do let!(:litigation) { create(:litigation, discarded_at: nil) } + subject { delete :destroy, params: {id: litigation.id} } context 'with valid params' do let!(:litigation_side) { create(:litigation_side, litigation: litigation) } @@ -143,8 +144,6 @@ create(:external_legislation, litigations: [litigation]) end - subject { delete :destroy, params: {id: litigation.id} } - before do expect { subject }.to change { Litigation.count }.by(-1) end @@ -189,8 +188,6 @@ context 'with invalid params' do let(:command) { double } - subject { delete :destroy, params: {id: litigation.id} } - before do expect(::Command::Destroy::Litigation).to receive(:new).and_return(command) expect(command).to receive(:call).and_return(nil) @@ -201,6 +198,16 @@ expect(flash[:alert]).to match('Could not delete selected Litigation') end end + + context 'when geography does not exist' do + before do + litigation.geography.litigations = [] + end + + it 'soft-delete even if geography is nil' do + expect { subject }.to change { Litigation.count }.by(-1) + end + end end def last_litigation_created From 981bd8c5d4bf584557dce5cd60b021ffa1a434bf Mon Sep 17 00:00:00 2001 From: Adam Gwozdowski Date: Wed, 18 Sep 2019 14:34:08 +0200 Subject: [PATCH 5/6] Remove unnecessary save --- app/commands/command/destroy/geography.rb | 2 -- app/commands/command/destroy/legislation.rb | 4 ---- app/commands/command/destroy/litigation.rb | 4 ---- 3 files changed, 10 deletions(-) diff --git a/app/commands/command/destroy/geography.rb b/app/commands/command/destroy/geography.rb index da1999b68..2ce18bab8 100644 --- a/app/commands/command/destroy/geography.rb +++ b/app/commands/command/destroy/geography.rb @@ -13,8 +13,6 @@ def call r.litigations = [] r.legislations = [] - - r.save! end end end diff --git a/app/commands/command/destroy/legislation.rb b/app/commands/command/destroy/legislation.rb index 59bc97966..34b08b8b7 100644 --- a/app/commands/command/destroy/legislation.rb +++ b/app/commands/command/destroy/legislation.rb @@ -15,10 +15,6 @@ def call r.litigations = [] r.targets = [] - - # Validation is false because it's possible that some precedent - # record (like geography) was removed earlier - r.save(validate: false) end end end diff --git a/app/commands/command/destroy/litigation.rb b/app/commands/command/destroy/litigation.rb index 1ede8934d..f60d5e5ad 100644 --- a/app/commands/command/destroy/litigation.rb +++ b/app/commands/command/destroy/litigation.rb @@ -16,10 +16,6 @@ def call r.legislations = [] r.external_legislations = [] - - # Validation is false because it's possible that some precedent - # record (like geography) was removed earlier - r.save(validate: false) end end end From f0c7517f2f8b468be1d84b271626990007335df1 Mon Sep 17 00:00:00 2001 From: Adam Gwozdowski Date: Thu, 19 Sep 2019 12:05:10 +0200 Subject: [PATCH 6/6] Redirect to list without any scope --- app/admin/geographies.rb | 2 +- app/admin/legislations.rb | 2 +- app/admin/litigations.rb | 2 +- spec/controllers/admin/geographies_controller_spec.rb | 2 +- spec/controllers/admin/litigations_controller_spec.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/admin/geographies.rb b/app/admin/geographies.rb index 24bdaadbb..bd1c14f43 100644 --- a/app/admin/geographies.rb +++ b/app/admin/geographies.rb @@ -88,7 +88,7 @@ def destroy {alert: 'Could not delete selected Geography'} end - redirect_to admin_geographies_path(scope: current_scope.name), results + redirect_to admin_geographies_path, results end end end diff --git a/app/admin/legislations.rb b/app/admin/legislations.rb index 9907fd8c1..a89792d21 100644 --- a/app/admin/legislations.rb +++ b/app/admin/legislations.rb @@ -149,7 +149,7 @@ def destroy {alert: 'Could not delete selected Legislations'} end - redirect_to admin_legislations_path(scope: current_scope.name), results + redirect_to admin_legislations_path, results end end end diff --git a/app/admin/litigations.rb b/app/admin/litigations.rb index c47739d42..06129eaf3 100644 --- a/app/admin/litigations.rb +++ b/app/admin/litigations.rb @@ -118,7 +118,7 @@ def destroy {alert: 'Could not delete selected Litigation'} end - redirect_to admin_litigations_path(scope: current_scope.name), results + redirect_to admin_litigations_path, results end end end diff --git a/spec/controllers/admin/geographies_controller_spec.rb b/spec/controllers/admin/geographies_controller_spec.rb index 4faf8af13..23a25f318 100644 --- a/spec/controllers/admin/geographies_controller_spec.rb +++ b/spec/controllers/admin/geographies_controller_spec.rb @@ -145,7 +145,7 @@ end it 'redirects to index & renders alert message' do - expect(subject).to redirect_to(admin_geographies_path(scope: 'All')) + expect(subject).to redirect_to(admin_geographies_path) expect(flash[:alert]).to match('Could not delete selected Geography') end end diff --git a/spec/controllers/admin/litigations_controller_spec.rb b/spec/controllers/admin/litigations_controller_spec.rb index 2e286fe4e..a96ec11ee 100644 --- a/spec/controllers/admin/litigations_controller_spec.rb +++ b/spec/controllers/admin/litigations_controller_spec.rb @@ -194,7 +194,7 @@ end it 'redirects to index & renders alert message' do - expect(subject).to redirect_to(admin_litigations_path(scope: 'All')) + expect(subject).to redirect_to(admin_litigations_path) expect(flash[:alert]).to match('Could not delete selected Litigation') end end