From 752b85f83bf8e1dfb7ba8febfd18abbe9acb8aa3 Mon Sep 17 00:00:00 2001 From: meyric Date: Tue, 3 Dec 2024 15:43:59 +0000 Subject: [PATCH] Fix planned disbursement providing organisation The name of the providing organisation does not match the reference number in a lot of cases, which we assume happened when BEIS became DSIT. This migration fixes that. Let me explain why this looks 'unusual': The Forecast model is protected against direct access, for background see our docs: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/blob/develop/doc/forecasts-and-versioning.md In this instance the data is simply incorrect, the reference 'GB-GOV-26' should be named 'DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY' and nothing else. We are not creating new or deleting old Forecast and so should not upset the versioning. As we are correcting data we have to touch a lot of Forecasts and the protection makes this interesting! For the actual data migration, things are pretty straight forward, we can use `update_column` to set the name and nothing else. Testing is where things get more difficult. Due to the protections, creating a set of Forecasts for testing is extremely convoluted (by design) we need to put Forecasts into a specific incorrect state. Instead we've opted to test closer to the database. We've provided a method to create Forecast in the right state, they would be considered invalid as models, but that is not under test here, we simply want to verify that: - only records in the appropriate state are processed - only the column we want to change is altered As it is harder to get the data into the right state to test, we've included some assertions for the initial state to try and show what is happening. I think it is worth remembering that this is a one off fix and is not really 'production' code that will run day in, day out! --- .../20241203084616_fix_dsit_beis_forecasts.rb | 56 +++++++++++++ ...1203084616_fix_dsit_beis_forecasts_spec.rb | 78 +++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 db/data/20241203084616_fix_dsit_beis_forecasts.rb create mode 100644 spec/db/data/20241203084616_fix_dsit_beis_forecasts_spec.rb diff --git a/db/data/20241203084616_fix_dsit_beis_forecasts.rb b/db/data/20241203084616_fix_dsit_beis_forecasts.rb new file mode 100644 index 000000000..0f87d3657 --- /dev/null +++ b/db/data/20241203084616_fix_dsit_beis_forecasts.rb @@ -0,0 +1,56 @@ +# Require me in the console with `require Rails.root + "db/data/20241203084616_fix_dsit_beis_forecasts.rb"` +# then run me with `FixDsitBeisForecasts.new.migrate!` +# +# Description: +# +# For every Forecast where `providing_organisation_reference` is "GB-GOV-26" +# AND the `providing_organisation_name` is NOT "DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY" +# Then set `providing_organisation_name` to "DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY" +# +# Forecasts store history and should generally not be accessed directly, however we are not creating +# new ones or destroying old ones, see +# +# https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/blob/develop/doc/forecasts-and-versioning.md +# +# We use `unscoped` to allow direct access and `update_column` so that the modification date is not +# updated, see: +# +# https://api.rubyonrails.org/classes/ActiveRecord/Persistence.html#method-i-update_column +# +class FixDsitBeisForecasts + DSIT_NAME = "DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY" + DSIT_REF = "GB-GOV-26" + + attr_reader :target, :fixed + + def initialize + @target = 0 + @fixed = 0 + end + + def migrate! + target_forecasts.each do |forecast| + puts "Fixing: #{forecast.id} #{forecast.providing_organisation_reference} #{forecast.providing_organisation_name}" + fix_forecast(forecast) + puts "Fixed!" + @fixed += 1 + end + + puts "Target: #{@target}" + puts "Fixed: #{@fixed}" + true + end + + def target_forecasts + targets = Forecast.unscoped + .where(providing_organisation_reference: DSIT_REF) + .where.not(providing_organisation_name: DSIT_NAME) + + @target = targets.count + targets + end + + def fix_forecast(forecast) + forecast.update_column(:providing_organisation_name, DSIT_NAME) + end +end diff --git a/spec/db/data/20241203084616_fix_dsit_beis_forecasts_spec.rb b/spec/db/data/20241203084616_fix_dsit_beis_forecasts_spec.rb new file mode 100644 index 000000000..45b678006 --- /dev/null +++ b/spec/db/data/20241203084616_fix_dsit_beis_forecasts_spec.rb @@ -0,0 +1,78 @@ +require Rails.root + "db/data/20241203084616_fix_dsit_beis_forecasts.rb" + +RSpec.describe FixDsitBeisForecasts do + describe "#migrate!" do + it "fixes only the appropriate forecasts and includes a count" do + activity = create(:programme_activity) + create_forecast_with(activity, "GB-GOV-13", "DEPARTMENT FOR BUSINESS, ENERGY & INDUSTRIAL STRATEGY") + create_forecast_with(activity, "GB-GOV-26", "DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY") + + create_forecast_with(activity, "GB-GOV-26", "NOT CORRECT") + + subject = Forecast.unscoped.order(:providing_organisation_name).last + + expect(Forecast.unscoped.count).to be 3 + expect(subject.providing_organisation_name).to eql "NOT CORRECT" + + migration = described_class.new + migration.migrate! + + expect(Forecast.unscoped.count).to be 3 + expect(subject.reload.providing_organisation_name).to eql "DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY" + + expect(migration.target).to be 1 + expect(migration.fixed).to be 1 + end + end + + describe "#fix_forecast" do + it "updates the providing_organisation_name" do + activity = create(:programme_activity) + create_forecast_with(activity, "GB-GOV-26", "DEPARTMENT FOR BUSINESS, ENERGY & INDUSTRIAL STRATEGY") + subject = Forecast.unscoped.last + + expect(subject.providing_organisation_name).to eql("DEPARTMENT FOR BUSINESS, ENERGY & INDUSTRIAL STRATEGY") + + described_class.new.fix_forecast(subject) + + expect(subject.reload.providing_organisation_name).to eql("DEPARTMENT FOR SCIENCE, INNOVATION AND TECHNOLOGY") + end + + it "does not update the update_at value" do + activity = create(:programme_activity) + create_forecast_with(activity, "GB-GOV-26", "DEPARTMENT FOR BUSINESS, ENERGY & INDUSTRIAL STRATEGY") + subject = Forecast.unscoped.last + previous_updated_at = subject.updated_at + + travel_to(Date.today + 1.day) do + described_class.new.fix_forecast(subject) + + expect(subject.reload.updated_at).to eql previous_updated_at + end + end + end + + def create_forecast_with(activity, reference, name) + sql = "INSERT INTO forecasts \ + ( + providing_organisation_reference, + providing_organisation_name, + parent_activity_id, + created_at, + updated_at, + financial_quarter, + financial_year) \ + VALUES \ + ( + '#{reference}', + '#{name}', + '#{activity.id}', + CURRENT_TIMESTAMP, + CURRENT_TIMESTAMP, + '4', + '2024' + );" + + ActiveRecord::Base.connection.execute(sql) + end +end