AP-633 Update following review - improve way state change is handled

ministryofjustice · Jun 21, 2019 · ae130bb · ae130bb
1 parent e5ea501
commit ae130bb
Show file tree

Hide file tree

Showing 5 changed files with 7 additions and 19 deletions.
diff --git a/app/controllers/providers/substantive_applications_controller.rb b/app/controllers/providers/substantive_applications_controller.rb
@@ -8,7 +8,11 @@ def show
     def update
       authorize legal_aid_application
       @form = LegalAidApplications::SubstantiveApplicationForm.new(form_params)
-      render :show unless save_continue_or_draft(@form)
+      if save_continue_or_draft(@form)
+        legal_aid_application.provider_used_delegated_functions!
+      else
+        render :show
+      end
     end
 
     private

diff --git a/app/forms/legal_aid_applications/substantive_application_form.rb b/app/forms/legal_aid_applications/substantive_application_form.rb
@@ -15,10 +15,6 @@ def substantive_application_deadline_on
       attributes['substantive_application_deadline_on'] ||= substantive_application_deadline
     end
 
-    def save
-      super && model.provider_used_delegated_functions!
-    end
-
     private
 
     def substantive_application_deadline

diff --git a/app/models/bank_holiday.rb b/app/models/bank_holiday.rb
@@ -8,7 +8,7 @@ class BankHoliday < ApplicationRecord
 
   def self.dates
     BankHolidayUpdateWorker.perform_in 10.seconds
-    instance = by_updated_at.last || create
+    instance = by_updated_at.last || create!
     instance.dates
   end
 

diff --git a/app/services/bank_holiday_retriever.rb b/app/services/bank_holiday_retriever.rb
@@ -18,7 +18,7 @@ def data
   def dates(group)
     return if data.empty?
 
-    data.dig(group, 'events')&.map { |details| details['date'] }
+    data.dig(group, 'events')&.pluck('date')
   end
 
   private

diff --git a/spec/forms/legal_aid_applications/substantive_application_form_spec.rb b/spec/forms/legal_aid_applications/substantive_application_form_spec.rb
@@ -33,10 +33,6 @@
       expect(application).not_to be_substantive_application
     end
 
-    it 'changes the state' do
-      expect { subject.save }.to change { application.reload.state }.to('delegated_functions_used')
-    end
-
     context 'when a deadline is already set' do
       let(:deadline) { 3.days.from_now.to_date }
       let(:application) do
@@ -61,10 +57,6 @@
         expect(subject.save).to be true
         expect(application).to be_substantive_application
       end
-
-      it 'changes the state' do
-        expect { subject.save }.to change { application.reload.state }.to('delegated_functions_used')
-      end
     end
 
     context 'with no entry' do
@@ -74,10 +66,6 @@
         expect(subject.save).to be false
         expect(application.substantive_application).to be_nil
       end
-
-      it 'changes the state' do
-        expect { subject.save }.not_to change { application.reload.state }
-      end
     end
   end
 end