Merge pull request #1650 from alphagov/add-dry-run-org-fetch-task

Update organisation fetcher with dry run version

lib/organisations_fetcher.rb

@@ -5,9 +5,13 @@
 # Based on similar code in Signon app:
 # https://github.com/alphagov/signon/blob/b7a53e282c55d8ef3ab6369a7cb358b6ae100d27/lib/organisations_fetcher.rb
 class OrganisationsFetcher
-  def call
+  def call(dry_run: false)
     organisations.each do |organisation_data|
-      update_or_create_organisation(organisation_data)
+      if dry_run
+        show_pending_organisation_update_or_creation(organisation_data)
+      else
+        update_or_create_organisation(organisation_data)
+      end
     end
   rescue ActiveRecord::RecordInvalid => e
     raise "Couldn't save organisation #{e.record.slug} because: #{e.record.errors.full_messages.join(',')}"
@@ -27,15 +31,24 @@ def update_or_create_organisation(organisation_data)
       Organisation.find_by(slug:) ||
       Organisation.new(govuk_content_id:)
 
-    update_data = {
-      govuk_content_id:,
-      slug:,
-      name: organisation_data[:title],
-      abbreviation: organisation_data[:details][:abbreviation],
-      closed: organisation_data[:details][:govuk_status] == "closed",
-    }
+    organisation.update!(allocate_update_data(organisation_data))
+  end
+
+  def show_pending_organisation_update_or_creation(organisation_data)
+    update_data = allocate_update_data(organisation_data)
+
+    unless (organisation = Organisation.find_by(govuk_content_id: update_data[:govuk_content_id]) || Organisation.find_by(slug: update_data[:slug]))
+      Rails.logger.info "Organisation Fetcher: Creating #{organisation_data[:title]} #{update_data}"
+      return
+    end
 
-    organisation.update!(update_data)
+    organisation_attributes = organisation.attributes.symbolize_keys.slice(*update_data.keys)
+
+    if organisation_attributes != update_data
+      organisation.assign_attributes(**update_data)
+
+      Rails.logger.info "Organisation Fetcher: Updating #{organisation_data[:title]} #{organisation.changes_to_save}"
+    end
   end
 
   def get_json_with_subsequent_pages(uri)
@@ -57,4 +70,14 @@ def get_json(uri)
       raise "error fetching organisations: #{response.code}: #{response.body}"
     end
   end
+
+  def allocate_update_data(organisation_data)
+    {
+      govuk_content_id: organisation_data[:details][:content_id],
+      slug: organisation_data[:details][:slug],
+      name: organisation_data[:title],
+      abbreviation: organisation_data[:details][:abbreviation],
+      closed: organisation_data[:details][:govuk_status] == "closed",
+    }
+  end
 end

lib/tasks/organisations.rake

@@ -5,7 +5,7 @@ namespace :organisations do
 
     Rails.logger.info("Starting organisation fetch")
 
-    run_organisation_fetch(rollback: false)
+    run_organisation_fetch(dry_run: false)
 
     Rails.logger.info("Organisation fetch complete")
   end
@@ -16,7 +16,7 @@ namespace :organisations do
 
     Rails.logger.info("Starting dry run of organisation fetch")
 
-    run_organisation_fetch(rollback: true)
+    run_organisation_fetch(dry_run: true)
 
     Rails.logger.info("Dry run of organisation fetch complete")
   end
@@ -38,12 +38,8 @@ namespace :organisations do
   end
 end
 
-def run_organisation_fetch(rollback:)
-  ActiveRecord::Base.transaction do
-    with_lock("forms-admin:organisations:fetch") do
-      OrganisationsFetcher.new.call
-    end
-
-    raise ActiveRecord::Rollback if rollback
+def run_organisation_fetch(dry_run:)
+  with_lock("forms-admin:organisations:fetch") do
+    OrganisationsFetcher.new.call(dry_run:)
   end
 end

spec/lib/organisations_fetcher_spec.rb

@@ -70,4 +70,35 @@ def organisation_details_for_slug(slug, content_id = Faker::Internet.uuid)
 
     expect(organisation.abbreviation).to eq "DfT"
   end
+
+  context "when doing a dry run" do
+    let(:dry_run) { true }
+
+    it "does not create an organisation" do
+      stub_organisation_api_has_organisations_with_bodies([
+        organisation_details_for_slug("department-for-tests"),
+      ])
+
+      expect(Rails.logger).to receive(:info).with(/Creating/)
+
+      expect {
+        organisations_fetcher.call(dry_run:)
+      }.not_to change(Organisation, :count)
+    end
+
+    it "does not update an existing organisation" do
+      organisation = create :organisation, slug: "department-for-testing", abbreviation: nil
+
+      stub_organisation_api_has_organisations_with_bodies([
+        organisation_details_for_slug("department-for-testing", organisation.govuk_content_id).deep_merge({ details: { abbreviation: "DfT" } }),
+      ])
+
+      expect(Rails.logger).to receive(:info).with(/Updating/)
+
+      organisations_fetcher.call(dry_run:)
+      organisation.reload
+
+      expect(organisation.abbreviation).to be_nil
+    end
+  end
 end