From 3c086a2b34250b9311bca18264b0bf5df6cc3239 Mon Sep 17 00:00:00 2001
From: Samuel Culley <samuel.culley@digital.cabinet-office.gov.uk>
Date: Wed, 4 Dec 2024 12:38:08 +0000
Subject: [PATCH] Update organisation fetcher with dry run version

The OrganisationFetcher now takes a `dry_run` argument, which will toggle between the dry run and actual fetch process. The dry run will not make any changes to the database, and instead will list out all the proposed changes.

When iterating over the organisations from the API, any new additions will be presented, and any changes to existing orgs will have their changes presented.

The rake task that makes use of this has also been updated to incorporate this
---
 lib/organisations_fetcher.rb           | 43 ++++++++++++++++++++------
 lib/tasks/organisations.rake           | 14 +++------
 spec/lib/organisations_fetcher_spec.rb | 31 +++++++++++++++++++
 3 files changed, 69 insertions(+), 19 deletions(-)

diff --git a/lib/organisations_fetcher.rb b/lib/organisations_fetcher.rb
index 739372d4b..362e978ef 100644
--- a/lib/organisations_fetcher.rb
+++ b/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
diff --git a/lib/tasks/organisations.rake b/lib/tasks/organisations.rake
index ab8606a88..a2966ef2b 100644
--- a/lib/tasks/organisations.rake
+++ b/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
diff --git a/spec/lib/organisations_fetcher_spec.rb b/spec/lib/organisations_fetcher_spec.rb
index d80bdc229..db699ddd4 100644
--- a/spec/lib/organisations_fetcher_spec.rb
+++ b/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