do not attempt to import if broadcast is invalid

radiorabe · Jan 4, 2017 · a9c5507 · a9c5507
1 parent 01b9225
commit a9c5507
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 6 deletions.
diff --git a/app/services/import/importer.rb b/app/services/import/importer.rb
@@ -30,13 +30,20 @@ def run
     def ready_for_import?
       recordings? &&
         !mapping_imported? &&
-        mapping_complete?
+        mapping_complete? &&
+        broadcast_valid?
     end
 
     def recordings?
       mapping.recordings.present?
     end
 
+    def mapping_imported?
+      mapping.imported?.tap do |imported|
+        warn("Broadcast #{mapping} is already imported.") if imported
+      end
+    end
+
     def mapping_complete?
       mapping.complete?.tap do |complete|
         unless complete
@@ -47,9 +54,15 @@ def mapping_complete?
       end
     end
 
-    def mapping_imported?
-      mapping.imported?.tap do |imported|
-        warn("Broadcast #{mapping} is already imported.") if imported
+    def broadcast_valid?
+      broadcast = mapping.broadcast
+      broadcast.valid?.tap do |valid|
+        unless valid
+          error("Broadcast of #{broadcast.show} @ #{I18n.l(broadcast.started_at)} is invalid: " \
+                "#{broadcast.errors.full_messages.join(', ')}")
+          ExceptionNotifier.notify_exception(ActiveRecord::RecordInvalid.new(broadcast),
+                                             data: { mapping: mapping })
+        end
       end
     end
 

diff --git a/test/services/import/importer_test.rb b/test/services/import/importer_test.rb
@@ -11,14 +11,14 @@ class Import::ImporterTest < ActiveSupport::TestCase
   end
 
   test 'it does nothing if recordings are not complete' do
-    mapping.recordings << Import::Recording.new(file('2013-06-19T200000+0200_060.mp3'))
+    mapping.add_recording_if_overlapping(Import::Recording.new(file('2013-06-19T200000+0200_060.mp3')))
     Import::Archiver.expects(:new).never
     ExceptionNotifier.expects(:notify_exception).never
     importer.run
   end
 
   test 'it does nothing if broadcast is already imported' do
-    mapping.recordings << Import::Recording.new(file('2013-06-19T200000+0200_120.mp3'))
+    mapping.add_recording_if_overlapping(Import::Recording.new(file('2013-06-19T200000+0200_120.mp3')))
     mapping.broadcast.save!
     Import::Archiver.expects(:new).never
     ExceptionNotifier.expects(:notify_exception).never
@@ -63,6 +63,18 @@ class Import::ImporterTest < ActiveSupport::TestCase
     assert File.exists?(file('2013-06-19T200000+0200_120_imported.mp3'))
   end
 
+  test 'it notifies if broadcast is invalid' do
+    Broadcast.create!(show: shows(:g9s),
+                      started_at: Time.zone.local(2013, 6, 19, 20),
+                      finished_at: Time.zone.local(2013, 6, 19, 21))
+    mapping.add_recording_if_overlapping(Import::Recording.new(file('2013-06-19T200000+0200_120.mp3')))
+    Import::Archiver.expects(:new).never
+    ExceptionNotifier.expects(:notify_exception).with(
+      instance_of(ActiveRecord::RecordInvalid),
+      instance_of(Hash))
+    importer.run
+  end
+
   private
 
   def importer