diff --git a/app/models/broadcast.rb b/app/models/broadcast.rb index 0bd1570e5..1d89ad5d4 100644 --- a/app/models/broadcast.rb +++ b/app/models/broadcast.rb @@ -91,7 +91,8 @@ def audio_file_blob_changed? event :start do transitions( from: [ :pending, :queued, :errored ], - to: :running + to: :running, + before_transaction: -> { self.error_message = nil } ) end @@ -140,6 +141,11 @@ def updatable? status == "pending" end + def mark_as_errored!(message) + self.error_message = message + self.error! + end + private def set_call_flow_logic diff --git a/app/workflows/populate_alerts.rb b/app/workflows/populate_alerts.rb index f9fd25a64..dc4fcdff1 100644 --- a/app/workflows/populate_alerts.rb +++ b/app/workflows/populate_alerts.rb @@ -1,6 +1,8 @@ class PopulateAlerts < ApplicationWorkflow attr_reader :broadcast + class BroadcastStartedError < StandardError; end + delegate :account, :beneficiary_filter, to: :broadcast, private: true def initialize(broadcast) @@ -9,32 +11,35 @@ def initialize(broadcast) def call ApplicationRecord.transaction do - create_alerts - return unless broadcast.audio_file.attached? + download_audio_file unless broadcast.audio_file.attached? - if broadcast.alerts.any? - create_delivery_attempts + create_alerts + create_delivery_attempts - broadcast.error_message = nil - broadcast.start! - else - mark_as_errored!("No beneficiaries match the filters") - end + broadcast.error_message = nil + broadcast.start! end + rescue BroadcastStartedError => e + broadcast.mark_as_errored!(e.message) end private def download_audio_file - audio_file = URI.open(broadcast.audio_url) - # FIXME: update file name - broadcast.audio_file.attach(avatar.attach(io: audio_file, filename: "audio.mp3")) - rescue OpenURI::HTTPError - mark_as_errored!("Unable to download audio file") + uri = URI.parse(broadcast.audio_url) + broadcast.audio_file.attach( + io: URI.open(uri), + filename: File.basename(uri) + ) + rescue OpenURI::HTTPError, URI::InvalidURIError + raise BroadcastStartedError, "Unable to download audio file" end def create_alerts - alerts = beneficiaries_scope.find_each.map do |beneficiary| + beneficiaries = beneficiaries_scope + raise BroadcastStartedError, "No beneficiaries match the filters" if beneficiaries.none? + + alerts = beneficiaries.find_each.map do |beneficiary| { broadcast_id: broadcast.id, beneficiary_id: beneficiary.id, @@ -70,9 +75,4 @@ def beneficiaries_scope BeneficiaryFilter.new(input_params: beneficiary_filter).output ).apply end - - def mark_as_errored!(message) - broadcast.error_message = message - broadcast.error! - end end diff --git a/spec/requests/open_ews_api/v1/broadcasts_spec.rb b/spec/requests/open_ews_api/v1/broadcasts_spec.rb index f9b07269b..5f88f9e47 100644 --- a/spec/requests/open_ews_api/v1/broadcasts_spec.rb +++ b/spec/requests/open_ews_api/v1/broadcasts_spec.rb @@ -101,6 +101,9 @@ } ) + stub_request(:get, "https://www.example.com/sample.mp3").to_return(status: 200) + allow(AudioFileProcessorJob).to receive(:perform_later).with(broadcast) + set_authorization_header_for(account) perform_enqueued_jobs do do_request( diff --git a/spec/workflows/populate_alerts_spec.rb b/spec/workflows/populate_alerts_spec.rb index c983217d3..fae7e0881 100644 --- a/spec/workflows/populate_alerts_spec.rb +++ b/spec/workflows/populate_alerts_spec.rb @@ -9,8 +9,11 @@ other_female_beneficiary = create(:beneficiary, account:, gender: "F") create(:beneficiary_address, beneficiary: other_female_beneficiary, iso_region_code: "KH-11") + stub_request(:get, "https://example.com/cowbell.mp3").to_return(status: 200) + broadcast = create( :broadcast, + audio_url: "https://example.com/cowbell.mp3", status: :pending, account:, error_message: "existing error message", @@ -38,12 +41,28 @@ ) end + it "marks errored when the audio file can't be downloaded" do + broadcast = create( + :broadcast, + status: :queued, + audio_url: "https://example.com/not-found.mp3", + ) + + stub_request(:get, "https://example.com/not-found.mp3").to_return(status: 404) + + PopulateAlerts.new(broadcast).call + + expect(broadcast.status).to eq("errored") + expect(broadcast.error_message).to eq("Unable to download audio file") + end + it "marks errored when there are no beneficiaries that match the filters" do account = create(:account) _male_beneficiary = create(:beneficiary, account:, gender: "M") broadcast = create( :broadcast, + audio_file: file_fixture("test.mp3"), status: :queued, beneficiary_filter: { gender: { eq: "F" } @@ -52,7 +71,7 @@ PopulateAlerts.new(broadcast).call - expect(broadcast.error_message).to eq("No beneficiaries match the filters") expect(broadcast.status).to eq("errored") + expect(broadcast.error_message).to eq("No beneficiaries match the filters") end end