Refs #5924 @1H; Add `before` actions

4 files changed, 116 insertions, 39 deletions

diff --git a/app/models/chouette/stop_area.rb b/app/models/chouette/stop_area.rb
index 5afe63747..c85a8b37d 100644
--- a/app/models/chouette/stop_area.rb
+++ b/app/models/chouette/stop_area.rb
@@ -370,6 +370,14 @@ module Chouette
       !activated?
     end
 
+    def activate
+      self.deleted_at = nil
+    end
+
+    def deactivate
+      self.deleted_at = Time.now
+    end
+
     def activate!
       update_attribute :deleted_at, nil
     end
diff --git a/app/models/simple_importer.rb b/app/models/simple_importer.rb
index 46c513732..760b98610 100644
--- a/app/models/simple_importer.rb
+++ b/app/models/simple_importer.rb
@@ -50,52 +50,62 @@ class SimpleImporter < ActiveRecord::Base
     status = :success
     statuses = ""
     log "#{"%#{padding}d" % 0}/#{number_of_lines}", clear: true
-    CSV.foreach(filepath, self.configuration.csv_options) do |row|
-      @current_record = self.configuration.find_record row
-      self.configuration.columns.each do |col|
-        @current_attribute = col[:attribute]
-        val = col[:value]
-        if val.nil? || val.is_a?(Proc)
-          if row.has_key? col.name
-            if val.is_a?(Proc)
-              val = instance_exec(row[col.name], &val)
+    ActiveRecord::Base.transaction do
+      self.configuration.before_actions(:all).each &:call
+
+      CSV.foreach(filepath, self.configuration.csv_options) do |row|
+        @current_record = self.configuration.find_record row
+        self.configuration.columns.each do |col|
+          @current_attribute = col[:attribute]
+          val = col[:value]
+          if val.nil? || val.is_a?(Proc)
+            if row.has_key? col.name
+              if val.is_a?(Proc)
+                val = instance_exec(row[col.name], &val)
+              else
+                val = row[col.name]
+              end
             else
-              val = row[col.name]
+              self.journal.push({event: :column_not_found, message: "Column not found: #{col.name}", kind: :warning})
+              status = :success_with_warnings
             end
-          else
-            self.journal.push({event: :column_not_found, message: "Column not found: #{col.name}", kind: :warning})
-            status = :success_with_warnings
           end
+          @current_record.send "#{@current_attribute}=", val if val
         end
-        @current_record.send "#{@current_attribute}=", val if val
-      end
 
-      fail_with_error ->(){ @current_record.errors.messages } do
-        new_record = @current_record.new_record?
-        @current_record.save!
-        self.journal.push({event: (new_record ? :creation : :update), kind: :log})
-        statuses += new_record ? colorize("✓", :green) : colorize("-", :orange)
-      end
-      self.configuration.columns.each do |col|
-        if col.name && @resolution_queue.any?
-          val = @current_record.send col[:attribute]
-          (@resolution_queue.delete([col.name, val]) || []).each do |res|
-            record = res[:record]
-            attribute = res[:attribute]
-            value = res[:block].call(val, record)
-            record.send "#{attribute}=", value
-            record.save!
+        fail_with_error ->(){ @current_record.errors.messages } do
+          new_record = @current_record.new_record?
+          self.configuration.before_actions(:each_save).each do |action|
+            action.call @current_record
+          end
+          ### This could fail if the record has a mandatory relation which is not yet resolved
+          ### TODO: do not attempt to save if the current record if waiting for resolution
+          ###       and fail at the end if there remains unresolved relations
+          @current_record.save!
+          self.journal.push({event: (new_record ? :creation : :update), kind: :log})
+          statuses += new_record ? colorize("✓", :green) : colorize("-", :orange)
+        end
+        self.configuration.columns.each do |col|
+          if col.name && @resolution_queue.any?
+            val = @current_record.send col[:attribute]
+            (@resolution_queue.delete([col.name, val]) || []).each do |res|
+              record = res[:record]
+              attribute = res[:attribute]
+              value = res[:block].call(val, record)
+              record.send "#{attribute}=", value
+              record.save!
+            end
           end
         end
+        current_line += 1
+        log "#{"%#{padding}d" % current_line}/#{number_of_lines}: #{statuses}", clear: true
       end
-      current_line += 1
-      log "#{"%#{padding}d" % current_line}/#{number_of_lines}: #{statuses}", clear: true
     end
     self.update_attribute :status, status
   rescue FailedImport
     self.update_attribute :status, :failed
   ensure
-    self.save
+    self.save!
   end
 
   protected
@@ -107,7 +117,6 @@ class SimpleImporter < ActiveRecord::Base
       msg = msg.call if msg.is_a?(Proc)
       log "\nFAILED: \n errors: #{msg}\n exception: #{e.message}\n#{e.backtrace.join("\n")}", color: :red
       self.journal.push({message: msg, error: e.message, event: :error, kind: :error})
-      self.save
       raise FailedImport
     end
   end
@@ -191,6 +200,16 @@ class SimpleImporter < ActiveRecord::Base
       @columns.push Column.new({attribute: attribute, value: value})
     end
 
+    def before group=:all, &block
+      @before ||= Hash.new{|h, k| h[k] = []}
+      @before[group].push block
+    end
+
+    def before_actions group=:all
+      @before ||= Hash.new{|h, k| h[k] = []}
+      @before[group]
+    end
+
     class Column
       attr_accessor :name
       def initialize opts={}
diff --git a/spec/fixtures/simple_importer/stop_area_incomplete.csv b/spec/fixtures/simple_importer/stop_area_incomplete.csv
index cd9447acd..9b11aa02c 100644
--- a/spec/fixtures/simple_importer/stop_area_incomplete.csv
+++ b/spec/fixtures/simple_importer/stop_area_incomplete.csv
@@ -1,2 +1,3 @@
 name;lat;long;type;street_name
+Foo;45.00;12;ZDEP
 ;45.00;12;ZDEP
diff --git a/spec/models/simple_importer_spec.rb b/spec/models/simple_importer_spec.rb
index 0184c7059..0f2682473 100644
--- a/spec/models/simple_importer_spec.rb
+++ b/spec/models/simple_importer_spec.rb
@@ -29,6 +29,8 @@ RSpec.describe SimpleImporter do
     let(:importer){ importer = SimpleImporter.new(configuration_name: :test, filepath: filepath) }
     let(:filepath){ Rails.root + "spec/fixtures/simple_importer/#{filename}" }
     let(:filename){ "stop_area.csv" }
+    let(:stop_area_referential){ create(:stop_area_referential, objectid_format: :stif_netex) }
+
     before(:each) do
       SimpleImporter.define :test do |config|
         config.model = Chouette::StopArea
@@ -39,7 +41,7 @@ RSpec.describe SimpleImporter do
         config.add_column :lat, attribute: :longitude, value: ->(raw){ raw.to_f + 1 }
         config.add_column :type, attribute: :area_type, value: ->(raw){ raw&.downcase }
         config.add_column :street_name
-        config.add_column :stop_area_referential, value: create(:stop_area_referential, objectid_format: :stif_netex)
+        config.add_column :stop_area_referential, value: stop_area_referential
         config.add_value  :kind, :commercial
       end
     end
@@ -101,10 +103,14 @@ RSpec.describe SimpleImporter do
 
     context "with a incomplete dataset" do
       let(:filename){ "stop_area_incomplete.csv" }
-      it "should create a StopArea" do
+      it "should fail" do
         expect{importer.import}.to_not raise_error
         expect(importer.status).to eq "failed"
-        expect(importer.reload.journal.first["message"]).to eq({"name" => ["doit être rempli(e)"]})
+        expect(importer.reload.journal.last["message"]).to eq({"name" => ["doit être rempli(e)"]})
+      end
+
+      it "should be transactional" do
+        expect{importer.import}.to_not change {Chouette::StopArea.count}
       end
     end
 
@@ -117,8 +123,41 @@ RSpec.describe SimpleImporter do
       end
     end
 
+    context "with a custom behaviour" do
+      let!(:present){ create :stop_area, name: "Nom du Stop", stop_area_referential: stop_area_referential }
+      let!(:missing){ create :stop_area, name: "Another", stop_area_referential: stop_area_referential }
+      before(:each){
+        importer.configure do |config|
+          config.before do
+            stop_area_referential.stop_areas.each &:deactivate!
+          end
+
+          config.before(:each_save) do |stop_area|
+            stop_area.activate!
+          end
+        end
+      }
+
+      it "should disable all missing areas" do
+        expect{importer.import}.to change{Chouette::StopArea.count}.by 0
+        expect(present.reload.activated?).to be_truthy
+        expect(missing.reload.activated?).to be_falsy
+      end
+
+      context "with an error" do
+        let(:filename){ "stop_area_incomplete.csv" }
+        it "should do nothing" do
+          expect{importer.import}.to_not change {Chouette::StopArea.count}
+          expect(present.reload.activated?).to be_truthy
+          expect(missing.reload.activated?).to be_truthy
+        end
+      end
+    end
+
     context "with a full file" do
       let(:filename){ "stop_area_full.csv" }
+      let!(:missing){ create :stop_area, name: "Another", stop_area_referential: stop_area_referential }
+
       before(:each) do
         SimpleImporter.define :test do |config|
           config.model = Chouette::StopArea
@@ -137,14 +176,21 @@ RSpec.describe SimpleImporter do
           config.add_column :address, attribute: :street_name
           config.add_column :postal_code, attribute: :zip_code
           config.add_column :city, attribute: :city_name
-          config.add_value  :stop_area_referential_id, create(:stop_area_referential, objectid_format: :stif_netex).id
+          config.add_value  :stop_area_referential_id, stop_area_referential.id
           config.add_value  :long_lat_type, "WGS84"
           config.add_value  :kind, :commercial
+          config.before do
+            stop_area_referential.stop_areas.each &:deactivate!
+          end
+
+          config.before(:each_save) do |stop_area|
+            stop_area.activate
+          end
         end
       end
 
       it "should import the given file" do
-        expect{importer.import}.to change{Chouette::StopArea.count}.by 2
+        expect{importer.import(verbose: false)}.to change{Chouette::StopArea.count}.by 2
         expect(importer.status).to eq "success"
         first = Chouette::StopArea.find_by registration_number: "PAR"
         last = Chouette::StopArea.find_by registration_number: "XED"
@@ -153,6 +199,9 @@ RSpec.describe SimpleImporter do
         expect(first.area_type).to eq "gdl"
         expect(last.area_type).to eq "zdep"
         expect(first.long_lat_type).to eq "WGS84"
+        expect(first.activated?).to be_truthy
+        expect(last.activated?).to be_truthy
+        expect(missing.reload.activated?).to be_falsy
       end
 
       context "with a relation in reverse order" do