Fix cleanup#destroy_time_tables should only destroy vehicle_journey of deleted time_tables

Refs #3917

2 files changed, 19 insertions, 5 deletions

diff --git a/app/models/clean_up.rb b/app/models/clean_up.rb
index e39928a17..cbcde72f5 100644
--- a/app/models/clean_up.rb
+++ b/app/models/clean_up.rb
@@ -16,10 +16,13 @@ class CleanUp < ActiveRecord::Base
 
   def clean
     {}.tap do |result|
-      result['time_table']        = send("destroy_time_tables_#{self.date_type}").try(:count)
+      processed = send("destroy_time_tables_#{self.date_type}")
+      if processed
+        result['time_table']      = processed[:time_tables].try(:count)
+        result['vehicle_journey'] = processed[:vehicle_journeys].try(:count)
+      end
       result['time_table_date']   = send("destroy_time_tables_dates_#{self.date_type}").try(:count)
       result['time_table_period'] = send("destroy_time_tables_periods_#{self.date_type}").try(:count)
-      result['vehicle_journey']   = destroy_vehicle_journey_without_time_table.try(:count)
       self.overlapping_periods.each do |period|
         exclude_dates_in_overlapping_period(period)
       end
@@ -106,13 +109,16 @@ class CleanUp < ActiveRecord::Base
   end
 
   def destroy_time_tables(time_tables)
+    results = { :time_tables => [], :vehicle_journeys => [] }
     # Delete vehicle_journey time_table association
     time_tables.each do |time_table|
       time_table.vehicle_journeys.each do |vj|
         vj.time_tables.delete(time_table)
+        results[:vehicle_journeys] << vj.destroy if vj.time_tables.empty?
       end
     end
-    time_tables.destroy_all
+    results[:time_tables] = time_tables.destroy_all
+    results
   end
 
   aasm column: :status do
diff --git a/spec/models/clean_up_spec.rb b/spec/models/clean_up_spec.rb
index ee88ca773..4dc692ab2 100644
--- a/spec/models/clean_up_spec.rb
+++ b/spec/models/clean_up_spec.rb
@@ -200,9 +200,17 @@ RSpec.describe CleanUp, :type => :model do
     end
 
     it 'should destroy time_table vehicle_journey association' do
+      vj = create(:vehicle_journey, time_tables: [time_table, create(:time_table)])
+      cleaner.destroy_time_tables(Chouette::TimeTable.where(id: time_table.id))
+
+      expect(vj.reload.time_tables.map(&:id)).to_not include(time_table.id)
+    end
+
+    it 'should also destroy associated vehicle_journey if it belongs to any other time_table' do
       vj = create(:vehicle_journey, time_tables: [time_table])
-      cleaner.destroy_time_tables(Chouette::TimeTable.all)
-      expect(vj.reload.time_tables).to be_empty
+      expect{cleaner.destroy_time_tables(Chouette::TimeTable.all)}.to change {
+        Chouette::VehicleJourney.count
+      }.by(-1)
     end
   end