Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update organisation fetcher with dry run version #1650

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions lib/organisations_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(',')}"
Expand All @@ -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)
Expand All @@ -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
14 changes: 5 additions & 9 deletions lib/tasks/organisations.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
31 changes: 31 additions & 0 deletions spec/lib/organisations_fetcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading