Merge pull request #2221 from alphagov/fix-travel-advice-email-alert

Fix travel advice email alert

Author: minhngocd

app/notifiers/publishing_api_notifier.rb

@@ -38,7 +38,7 @@ def publish_index
   end
 
   def send_alert(edition)
-    return unless send_alert?(edition)
+    return if edition.is_minor_update?
 
     presenter = EmailAlertPresenter.new(edition)
     payload = presenter.present.as_json
@@ -58,10 +58,6 @@ def worker
     PublishingApiWorker
   end
 
-  def send_alert?(edition)
-    edition.state == "published" && !edition.is_minor_update?
-  end
-
   def validate_tasks_order
     endpoints = tasks.map(&:first)
     send_alert_count = endpoints.count { |e| e == "send_alert" }

app/workers/scheduled_publishing_worker.rb

@@ -25,19 +25,19 @@ def perform(edition_id)
       return
     end
 
-    notifier.put_content(edition)
-    notifier.patch_links(edition)
-    notifier.email_signup(edition) if edition.previous_version.nil?
-    notifier.publish(edition)
-    notifier.send_alert(edition)
-    notifier.enqueue
-
     unless publishing_robot
       Sidekiq.logger.error("You must set up a Scheduled Publishing Robot")
       return
     end
 
     edition.publish_as(publishing_robot)
+
+    notifier.put_content(edition)
+    notifier.patch_links(edition)
+    notifier.email_signup(edition) if edition.previous_version.nil?
+    notifier.publish(edition)
+    notifier.send_alert(edition)
+    notifier.enqueue
   end
 
 private

spec/controllers/admin/editions_controller_spec.rb

@@ -232,6 +232,7 @@
       it "should publish the edition" do
         allow(TravelAdviceEdition).to receive(:find).with(draft.to_param).and_return(draft)
         allow(draft).to receive(:publish).and_return(true)
+        draft.published_at = Time.zone.now
 
         post :update, params: { id: draft.to_param, edition: {}, commit: "Save & Publish" }
 

spec/notifiers/publishing_api_notifier_spec.rb

@@ -105,15 +105,6 @@
       end
     end
 
-    context "for a draft edition" do
-      it "doesn't enqueue anything" do
-        subject.send_alert(edition)
-        subject.enqueue
-
-        expect(PublishingApiWorker.jobs).to be_empty
-      end
-    end
-
     context "for a minor update" do
       it "doesn't enqueue anything" do
         edition.update!(update_type: "minor", version_number: 2)

spec/workers/scheduled_publishing_worker_spec.rb

@@ -1,4 +1,7 @@
 RSpec.describe ScheduledPublishingWorker, type: :worker do
+  include GdsApi::TestHelpers::EmailAlertApi
+  include GdsApi::TestHelpers::PublishingApi
+
   describe "#perform" do
     let(:country) { Country.find_by_slug("afghanistan") }
     let(:edition) { create(:scheduled_travel_advice_edition, country_slug: country.slug) }
@@ -19,6 +22,26 @@
       expect(edition.reload.actions.first.requester.name).to eq robot.name
     end
 
+    it "#perform sends email alerts for an edition" do
+      stub_any_publishing_api_call
+      stub_email_alert_api_accepts_content_change
+
+      # Queue up and execute the ScheduledPublishingWorker, move to after scheduled time
+      ScheduledPublishingWorker.enqueue(edition)
+      travel_to(1.hour.from_now)
+      ScheduledPublishingWorker.drain
+
+      # Check that the above created a PublishingApiWorker, then execute it
+      expect(PublishingApiWorker.jobs.size).to eq(1)
+      PublishingApiWorker.drain
+
+      # Check that the above created a EmailAlertApiWorker, then execute it
+      expect(EmailAlertApiWorker.jobs.size).to eq(1)
+      EmailAlertApiWorker.drain
+
+      assert_email_alert_api_content_change_created
+    end
+
     it "does not publish edition that isn't yet due for publication" do
       expect(Sidekiq.logger).to receive(:info).with("Edition of ID '#{edition.id}' is not yet due for publication.").once