From 336dab9900aee831e73113e400609ec2ebd40c20 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Mon, 13 Nov 2017 17:16:24 +0100 Subject: Import: Destroy associated `Referential` on destroy if ready:false If the `Import`'s associated `Referential` has `ready: false`, destroy that `Referential` when the import is destroyed. Aaand looking back at the task it turns out that's not what I was supposed to do. Right, it does seem a little weird in retrospect. Instead we want to delete the `Referential`s of child imports. Refs #4962 --- app/models/import.rb | 7 +++++++ spec/models/import_spec.rb | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/models/import.rb b/app/models/import.rb index 20e7f2d8a..68cb62f86 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -22,6 +22,7 @@ class Import < ActiveRecord::Base validates_format_of :file, with: %r{\.zip\z}i, message: I18n.t('activerecord.errors.models.import.attributes.file.wrong_file_extension') before_create :initialize_fields + before_destroy :destroy_non_ready_referential def self.model_name ActiveModel::Name.new Import, Import, "Import" @@ -97,6 +98,12 @@ class Import < ActiveRecord::Base self.token_download = SecureRandom.urlsafe_base64 end + def destroy_non_ready_referential + if !referential.ready + referential.destroy + end + end + def self.symbols_with_indifferent_access(array) array.flat_map { |symbol| [symbol, symbol.to_s] } end diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb index 7be05908a..dfb947711 100644 --- a/spec/models/import_spec.rb +++ b/spec/models/import_spec.rb @@ -57,6 +57,24 @@ RSpec.describe Import, type: :model do expect(ImportMessage.count).to eq(0) end + + it "must destroy its associated Referential if ready: false" do + referential = create(:referential, ready: false) + import = create(:import, referential: referential) + + import.destroy + + expect(referential).to be_destroyed + end + + it "must not destroy its associated Referential if ready: true" do + referential = create(:referential, ready: true) + import = create(:import, referential: referential) + + import.destroy + + expect(referential).not_to be_destroyed + end end describe "#notify_parent" do -- cgit v1.2.3 From 51a0898506e9c6bcc379ed8db01d3f14a1dbe110 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Mon, 13 Nov 2017 18:27:48 +0100 Subject: NetexImport: Destroy referential on destroy if ready:false If the `NetexImport`'s associated `Referential` has `ready: false`, when the `NetexImport` is destroyed, also destroy the `Referential`. This prevents a case where users get an invalid `Referential` as a result of an import, but because of the `ready: false`, it doesn't display on the website, so they can't see what the problem is. Refs #4962 --- app/models/import.rb | 7 ------- app/models/netex_import.rb | 9 +++++++++ spec/models/import_spec.rb | 18 ------------------ spec/models/netex_import_spec.rb | 28 ++++++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 25 deletions(-) create mode 100644 spec/models/netex_import_spec.rb diff --git a/app/models/import.rb b/app/models/import.rb index 68cb62f86..20e7f2d8a 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -22,7 +22,6 @@ class Import < ActiveRecord::Base validates_format_of :file, with: %r{\.zip\z}i, message: I18n.t('activerecord.errors.models.import.attributes.file.wrong_file_extension') before_create :initialize_fields - before_destroy :destroy_non_ready_referential def self.model_name ActiveModel::Name.new Import, Import, "Import" @@ -98,12 +97,6 @@ class Import < ActiveRecord::Base self.token_download = SecureRandom.urlsafe_base64 end - def destroy_non_ready_referential - if !referential.ready - referential.destroy - end - end - def self.symbols_with_indifferent_access(array) array.flat_map { |symbol| [symbol, symbol.to_s] } end diff --git a/app/models/netex_import.rb b/app/models/netex_import.rb index 32939a741..3d8ab3809 100644 --- a/app/models/netex_import.rb +++ b/app/models/netex_import.rb @@ -1,6 +1,7 @@ require 'net/http' class NetexImport < Import after_commit :launch_java_import, on: :create + before_destroy :destroy_non_ready_referential validates_presence_of :parent @@ -16,4 +17,12 @@ class NetexImport < Import end end end + + private + + def destroy_non_ready_referential + if !referential.ready + referential.destroy + end + end end diff --git a/spec/models/import_spec.rb b/spec/models/import_spec.rb index dfb947711..7be05908a 100644 --- a/spec/models/import_spec.rb +++ b/spec/models/import_spec.rb @@ -57,24 +57,6 @@ RSpec.describe Import, type: :model do expect(ImportMessage.count).to eq(0) end - - it "must destroy its associated Referential if ready: false" do - referential = create(:referential, ready: false) - import = create(:import, referential: referential) - - import.destroy - - expect(referential).to be_destroyed - end - - it "must not destroy its associated Referential if ready: true" do - referential = create(:referential, ready: true) - import = create(:import, referential: referential) - - import.destroy - - expect(referential).not_to be_destroyed - end end describe "#notify_parent" do diff --git a/spec/models/netex_import_spec.rb b/spec/models/netex_import_spec.rb new file mode 100644 index 000000000..d52d475cc --- /dev/null +++ b/spec/models/netex_import_spec.rb @@ -0,0 +1,28 @@ +RSpec.describe NetexImport, type: :model do + describe "#destroy" do + it "must destroy its associated Referential if ready: false" do + workbench_import = create(:workbench_import) + referential_ready_false = create(:referential, ready: false) + referential_ready_true = create(:referential, ready: true) + create( + :netex_import, + parent: workbench_import, + referential: referential_ready_false + ) + create( + :netex_import, + parent: workbench_import, + referential: referential_ready_true + ) + + workbench_import.destroy + + expect( + Referential.where(id: referential_ready_false.id).exists? + ).to be false + expect( + Referential.where(id: referential_ready_true.id).exists? + ).to be true + end + end +end -- cgit v1.2.3 From a9c16ee132e242ed4304c2b5be0c49e31cde9d85 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Tue, 14 Nov 2017 14:22:41 +0100 Subject: NetexImport: Use `unless` instead of `if !` Suggestion from Robert. Refs #4962 --- app/models/netex_import.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/netex_import.rb b/app/models/netex_import.rb index 3d8ab3809..b8702a561 100644 --- a/app/models/netex_import.rb +++ b/app/models/netex_import.rb @@ -21,7 +21,7 @@ class NetexImport < Import private def destroy_non_ready_referential - if !referential.ready + unless referential.ready referential.destroy end end -- cgit v1.2.3