Merge pull request #956 from alphagov/fix-fetch-constituencies-error-…

…handling

Fix error handling when fetching constituencies

app/jobs/fetch_constituencies_job.rb

@@ -5,9 +5,9 @@ class FetchConstituenciesJob < ApplicationJob
 
   def perform
     constituencies.each do |external_id, name, ons_code, start_date, end_date|
-      begin
-        retried = false
+      retried = false
 
+      begin
         Constituency.for(external_id) do |constituency|
           constituency.name = name
           constituency.ons_code = ons_code
@@ -31,8 +31,9 @@ def perform
           constituency.save!
         end
       rescue ActiveRecord::RecordNotUnique => e
-        retry unless retried
+        next if retried
         retried = true
+        retry
       end
     end
   end

app/views/admin/sites/_features.html.erb

@@ -1,5 +1,4 @@
 <%= hidden_field_tag :tab, "features" %>
-<%= hidden_field_tag :"site[features]", "" %>
 
 <div class="grid-row">
   <div class="column-half">

spec/jobs/fetch_constituencies_job_spec.rb

@@ -166,6 +166,21 @@ def odata_response(status, body = nil, &block)
         stub_constituency_api.to_return(odata_response(:ok, "constituencies"))
       end
 
+      context "when a record is duplicated" do
+        let(:constituency) { instance_spy(Constituency) }
+
+        before do
+          allow(Constituency).to receive(:find_or_initialize_by).and_call_original
+          allow(Constituency).to receive(:find_or_initialize_by).with(external_id: "3320").and_return(constituency)
+          allow(constituency).to receive(:save!).and_raise(ActiveRecord::RecordNotUnique)
+        end
+
+        it "retries twice before skipping" do
+          described_class.perform_now
+          expect(constituency).to have_received(:save!).twice
+        end
+      end
+
       context "when a record fails to save" do
         let!(:constituency) { FactoryBot.create(:constituency, :bethnal_green_and_bow) }
         let(:exception) { ActiveRecord::RecordInvalid.new(constituency) }