CRM457-2094: Make critical job enqueueings atomic (#812)

## Description of change
This means if Redis fails we won't end up in an inconsistent state.

[Link to relevant
ticket](https://dsdmoj.atlassian.net/browse/CRM457-2094)

app/forms/nsm/make_decision_form.rb

@@ -21,20 +21,25 @@ class MakeDecisionForm
     def save
       return false unless valid?
 
-      previous_state = claim.state
-      Claim.transaction do
-        claim.data.merge!('status' => state, 'updated_at' => Time.current, 'assessment_comment' => comment)
-        claim.update!(state:)
-        ::Event::Decision.build(submission: claim,
-                                comment: comment,
-                                previous_state: previous_state,
-                                current_user: current_user)
+      claim.with_lock do
+        change_data_and_notify_app_store!
       end
-      NotifyAppStore.perform_later(submission: claim)
 
       true
     end
 
+    def change_data_and_notify_app_store!
+      previous_state = claim.state
+
+      claim.data.merge!('status' => state, 'updated_at' => Time.current, 'assessment_comment' => comment)
+      claim.update!(state:)
+      ::Event::Decision.build(submission: claim,
+                              comment: comment,
+                              previous_state: previous_state,
+                              current_user: current_user)
+      NotifyAppStore.perform_later(submission: claim)
+    end
+
     def comment
       case state
       when Claim::GRANTED

app/forms/nsm/send_back_form.rb

@@ -21,11 +21,11 @@ def stash
     def save
       return false unless valid?
 
-      Claim.transaction do
+      claim.with_lock do
         update_local_data
-      end
 
-      NotifyAppStore.perform_later(submission: claim)
+        NotifyAppStore.perform_later(submission: claim)
+      end
 
       true
     end

app/forms/prior_authority/decision_form.rb

@@ -29,23 +29,27 @@ def summary
     def save
       return false unless valid?
 
-      PriorAuthorityApplication.transaction do
-        stash(add_draft_decision_event: false)
-        previous_state = submission.state
-
-        submission.data.merge!('status' => pending_decision, 'updated_at' => Time.current)
-        submission.update!(state: pending_decision)
-        ::Event::Decision.build(submission: submission,
-                                comment: explanation,
-                                previous_state: previous_state,
-                                current_user: current_user)
+      submission.with_lock do
+        change_data_and_notify_app_store!
       end
 
-      NotifyAppStore.perform_later(submission:)
-
       true
     end
 
+    def change_data_and_notify_app_store!
+      stash(add_draft_decision_event: false)
+      previous_state = submission.state
+
+      submission.data.merge!('status' => pending_decision, 'updated_at' => Time.current)
+      submission.update!(state: pending_decision)
+      ::Event::Decision.build(submission: submission,
+                              comment: explanation,
+                              previous_state: previous_state,
+                              current_user: current_user)
+
+      NotifyAppStore.perform_later(submission:)
+    end
+
     def stash(add_draft_decision_event: true)
       submission.data.merge!(attributes.except('submission', 'current_user').merge('assessment_comment' => explanation))
       submission.save!

app/forms/prior_authority/send_back_form.rb

@@ -29,13 +29,13 @@ def summary
     def save
       return false unless valid?
 
-      PriorAuthorityApplication.transaction do
+      submission.with_lock do
         discard_all_adjustments
         stash(add_draft_send_back_event: false)
         update_local_records
-      end
 
-      NotifyAppStore.perform_later(submission:)
+        NotifyAppStore.perform_later(submission:)
+      end
 
       true
     end

app/models/event.rb

@@ -24,6 +24,7 @@ class Event < ApplicationRecord
 
   LOCAL_EVENTS = [
     'Event::DraftDecision',
+    'Event::NewVersion',
     'Event::Edit',
     'Event::Note',
     'Event::UndoEdit',

app/models/event/new_version.rb

@@ -2,7 +2,6 @@ class Event
   class NewVersion < Event
     def self.build(submission:)
       create(submission: submission, submission_version: submission.current_version)
-        .tap(&:notify)
     end
 
     def body

app/services/notify_app_store.rb

@@ -7,10 +7,16 @@ def self.perform_later(submission:, trigger_email: true)
   end
 
   def perform(submission:, trigger_email: true)
-    notify(MessageBuilder.new(submission:))
+    # This lock is important because it forces the job to wait until
+    # the locked transaction that enqueued this job is released,
+    # ensuring that when this job runs it is working with fresh
+    # data
+    submission.with_lock do
+      notify(MessageBuilder.new(submission:))
 
-    send_email(submission:, trigger_email:)
-    submission.update!(notify_app_store_completed: true)
+      send_email(submission:, trigger_email:)
+      submission.update!(notify_app_store_completed: true)
+    end
   end
 
   def notify(message_builder)

spec/models/event/new_version_spec.rb

@@ -13,14 +13,6 @@
     )
   end
 
-  it 'notifies the app store' do
-    event = Event.send(:new)
-    expect(described_class).to receive(:create).and_return(event)
-    expect(NotifyEventAppStore).to receive(:perform_later).with(event:)
-
-    subject
-  end
-
   it 'has a valid title' do
     expect(subject.title).to eq('Received')
   end

spec/services/notify_app_store_spec.rb

@@ -9,6 +9,8 @@
   before do
     allow(described_class::MessageBuilder).to receive(:new)
       .and_return(message_builder)
+
+    allow(submission).to receive(:with_lock).and_yield
   end
 
   describe '.perform_later' do