Merge pull request #710 from DEFRA/feature/RUBY-2729-pafs-pso-rfcc-re…

…factoring

Feature/ruby 2729 pafs pso rfcc refactoring

app/models/pafs_core/area.rb

@@ -18,7 +18,7 @@ class Area < ApplicationRecord
     validates :identifier, uniqueness: true, if: -> { rma? || authority? }, unless: :skip_identifier_validation
     validate :parentage
     validates :area_type, inclusion: { in: AREA_TYPES }
-    validates :sub_type, presence: true, if: :rma?
+    validates :sub_type, presence: true, if: -> { rma? || pso_area? }
 
     belongs_to :parent, class_name: "Area", optional: true
     has_many :children, class_name: "Area", foreign_key: "parent_id"

app/models/pafs_core/user.rb

@@ -34,7 +34,7 @@ def rfcc_code(area_name = nil)
       return if area.ea_area?
 
       area = area.parent if area.rma?
-      PafsCore::PSO_RFCC_MAP.fetch(area.name)
+      PafsCore::PsoRfccService.map.fetch(area.name)
     end
 
     def primary_area

app/services/pafs_core/pso_rfcc_service.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module PafsCore
+  class PsoRfccService
+    class << self
+      def map
+        PafsCore::Area.pso_areas.select(:name, :sub_type).to_h { |a| [a.name, a.sub_type] }
+      end
+    end
+  end
+end

db/migrate/20231002152826_add_rfcc_codes.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddRfccCodes < ActiveRecord::Migration[7.0]
+  def up
+    PafsCore::DataMigration::AddRfccCodes.up
+  end
+
+  def down
+    PafsCore::DataMigration::AddRfccCodes.down
+  end
+end

lib/pafs_core.rb

@@ -36,6 +36,7 @@
 require "pafs_core/data_migration/remove_duplicate_states"
 require "pafs_core/data_migration/update_areas"
 require "pafs_core/data_migration/update_authorities"
+require "pafs_core/data_migration/add_rfcc_codes"
 require "pafs_core/data_migration/update_projects"
 require "pafs_core/data_migration/export_to_pol"
 require "pafs_core/data_migration/generate_funding_contributor_fcerm"

lib/pafs_core/data_migration/add_rfcc_codes.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+module PafsCore
+  module DataMigration
+    class AddRfccCodes
+      class << self
+        def up
+          # map PSO areas to RFCC codes
+          pso_rfcc_map = {
+            "PSO Berkshire and Buckinghamshire" => "TH",
+            "PSO Cambridge and Bedfordshire" => "AC",
+            "PSO Cheshire and Merseyside" => "NW",
+            "PSO Coastal Essex Suffolk and Norfolk" => "AE",
+            "PSO Coastal Lincolnshire and Northamptonshire" => "AN",
+            "PSO Cumbria" => "NW",
+            "PSO Derbyshire and Leicestershire" => "TR",
+            "PSO Dorset and Wiltshire" => "WX",
+            "PSO Durham and Tees Valley" => "NO",
+            "PSO East Devon and Cornwall" => "SW",
+            "PSO East Kent" => "SO",
+            "PSO East Sussex" => "SO",
+            "PSO East Yorkshire" => "YO",
+            "PSO Essex" => "AE",
+            "PSO Greater Manchester" => "NW",
+            "PSO Hampshire and Isle of Wight" => "SO",
+            "PSO Herefordshire and Gloucestershire" => "SN",
+            "PSO Lancashire" => "NW",
+            "PSO Lincolnshire" => "AN",
+            "PSO London East" => "TH",
+            "PSO London West" => "TH",
+            "PSO Luton Hertfordshire and Essex" => "TH",
+            "PSO Norfolk and Suffolk" => "AE",
+            "PSO North Yorkshire" => "YO",
+            "PSO Nottinghamshire and Tidal Trent" => "TR",
+            "PSO Oxfordshire" => "TH",
+            "PSO Shropshire Worcestershire Telford and Wrekin" => "SN",
+            "PSO Somerset" => "WX",
+            "PSO South East London and North Kent" => "TH",
+            "PSO South West London and Mole" => "TH",
+            "PSO South Yorkshire" => "YO",
+            "PSO Staffordshire and the Black Country" => "TR",
+            "PSO Surrey" => "TH",
+            "PSO Test Area" => "TS",
+            "PSO Tyne and Wear and Northumberland" => "NO",
+            "PSO Warwickshire Birmingham Solihull and Coventry" => "SN",
+            "PSO Welland and Nene" => "AN",
+            "PSO West Devon and Cornwall" => "SW",
+            "PSO West Kent" => "SO",
+            "PSO West of England" => "WX",
+            "PSO West Sussex" => "SO",
+            "PSO West Yorkshire" => "YO"
+          }
+
+          pso_rfcc_map.each do |pso_name, code|
+            pso = PafsCore::Area.find_by(area_type: PafsCore::Area::PSO_AREA, name: pso_name)
+            next if pso.nil?
+
+            pso.update(sub_type: code)
+          end
+        end
+
+        def down
+          PafsCore::Area.where(area_type: PafsCore::Area::PSO_AREA).update(sub_type: nil)
+        end
+      end
+    end
+  end
+end

lib/pafs_core/rfcc_codes.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# require "pafs_core/models/area.rb"
+
 module PafsCore
   RFCC_CODES = %w[AC AE AN NO NW SN SO SW TH TR TS WX YO].freeze
 
@@ -19,50 +21,4 @@ module PafsCore
     "WX" => "Wessex",
     "YO" => "Yorkshire"
   }.freeze
-
-  # map PSO areas to RFCC codes
-  PSO_RFCC_MAP = {
-    "PSO Berkshire and Buckinghamshire" => "TH",
-    "PSO Cambridge and Bedfordshire" => "AC",
-    "PSO Cheshire and Merseyside" => "NW",
-    "PSO Coastal Essex Suffolk and Norfolk" => "AE",
-    "PSO Coastal Lincolnshire and Northamptonshire" => "AN",
-    "PSO Cumbria" => "NW",
-    "PSO Derbyshire and Leicestershire" => "TR",
-    "PSO Dorset and Wiltshire" => "WX",
-    "PSO Durham and Tees Valley" => "NO",
-    "PSO East Devon and Cornwall" => "SW",
-    "PSO East Kent" => "SO",
-    "PSO East Sussex" => "SO",
-    "PSO East Yorkshire" => "YO",
-    "PSO Essex" => "AE",
-    "PSO Greater Manchester" => "NW",
-    "PSO Hampshire and Isle of Wight" => "SO",
-    "PSO Herefordshire and Gloucestershire" => "SN",
-    "PSO Lancashire" => "NW",
-    "PSO Lincolnshire" => "AN",
-    "PSO London East" => "TH",
-    "PSO London West" => "TH",
-    "PSO Luton Hertfordshire and Essex" => "TH",
-    "PSO Norfolk and Suffolk" => "AE",
-    "PSO North Yorkshire" => "YO",
-    "PSO Nottinghamshire and Tidal Trent" => "TR",
-    "PSO Oxfordshire" => "TH",
-    "PSO Shropshire Worcestershire Telford and Wrekin" => "SN",
-    "PSO Somerset" => "WX",
-    "PSO South East London and North Kent" => "TH",
-    "PSO South West London and Mole" => "TH",
-    "PSO South Yorkshire" => "YO",
-    "PSO Staffordshire and the Black Country" => "TR",
-    "PSO Surrey" => "TH",
-    "PSO Test Area" => "TS",
-    "PSO Tyne and Wear and Northumberland" => "NO",
-    "PSO Warwickshire Birmingham Solihull and Coventry" => "SN",
-    "PSO Welland and Nene" => "AN",
-    "PSO West Devon and Cornwall" => "SW",
-    "PSO West Kent" => "SO",
-    "PSO West of England" => "WX",
-    "PSO West Sussex" => "SO",
-    "PSO West Yorkshire" => "YO"
-  }.freeze
 end

spec/dummy/db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_09_26_101350) do
+ActiveRecord::Schema[7.0].define(version: 2023_10_02_152826) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 

spec/factories/areas.rb

@@ -2,9 +2,7 @@
 
 FactoryBot.define do
   factory :area, class: "PafsCore::Area" do
-    sequence :name do |n|
-      PafsCore::PSO_RFCC_MAP.keys[n % PafsCore::PSO_RFCC_MAP.keys.size]
-    end
+    name { "Area #{Faker::Lorem.unique.sentence}" }
 
     factory :country do
       area_type { "Country" }
@@ -60,6 +58,7 @@
     factory :pso_area do
       area_type { "PSO Area" }
       parent { PafsCore::Area.country || create(:country) }
+      sub_type { PafsCore::RFCC_CODES.sample }
 
       trait :with_rma_areas do
         after(:create) do |pso_area|

spec/fixtures/areas/areas.csv

@@ -1,5 +1,5 @@
 name,"parent area","type","sub type"
 England,,Country,
 Wessex,England,EA Area,
-Bristol & Bath,Wessex,PSO Area,
+Bristol & Bath,Wessex,PSO Area,WX
 Bristol City Council,Bristol & Bath,RMA,Local Authority

spec/fixtures/areas/faulty_areas_data.csv

@@ -1,7 +1,7 @@
 name,"parent area","type","sub type"
 England,,Country,
 Wessex,England,EA Area,
-Bristol & Bath,Wessex,PSO Area,
+Bristol & Bath,Wessex,PSO Area,WX
 Bristol City Council,Bristil & Bath,RMA,Local Authority
 Bath & NES council,Bristol & Bath,RMA,,
 ,,Country,

spec/fixtures/test_areas.csv

@@ -1,5 +1,5 @@
 name,"parent area","type","sub type"
 England,,Country,
 EA Test Area,England,EA Area,
-PSO Test Area,EA Test Area,PSO Area,
+PSO Test Area,EA Test Area,PSO Area,TR
 RMA Test Area,PSO Test Area,RMA,Local Authority

spec/lib/pafs_core/data_migration/add_rfcc_codes_spec.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe PafsCore::DataMigration::AddRfccCodes do
+
+  describe "#up" do
+    it "updates RFCC codes for PSO records" do
+      pso = build(:pso_area, name: "PSO Berkshire and Buckinghamshire", sub_type: nil)
+      pso.save!(validate: false)
+
+      described_class.up
+      expect(pso.reload.sub_type).to eq("TH")
+    end
+  end
+
+  describe "#down" do
+    it "removes RFCC codes from PSOuthorities records" do
+      pso = create(:pso_area, name: "PSO Berkshire and Buckinghamshire", sub_type: "TH")
+      described_class.down
+      expect(pso.reload.sub_type).to eq("TH")
+    end
+  end
+end

spec/lib/pafs_core/data_migration/update_authorities_spec.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe PafsCore::DataMigration::UpdateAuthorities do
+
+  describe "#up" do
+    it "adds authorities records if not present" do
+      expect(PafsCore::Area.authorities.count).to eq(0)
+      described_class.up
+      expect(PafsCore::Area.authorities.count).to eq(7)
+    end
+
+    it "adds missing authorities records" do
+      create(:authority, name: "Environment Agency", identifier: "EA")
+      expect(PafsCore::Area.authorities.count).to eq(1)
+      described_class.up
+      expect(PafsCore::Area.authorities.count).to eq(7)
+    end
+  end
+
+  describe "#down" do
+    it "removes authorities records" do
+      create(:authority, name: "Environment Agency", identifier: "EA")
+      described_class.down
+      expect(PafsCore::Area.authorities.count).to eq(0)
+    end
+  end
+end

spec/models/pafs_core/area_spec.rb

@@ -47,15 +47,23 @@
       subject { create(:pso_area, parent_id: 1) }
 
       it { is_expected.to validate_presence_of :parent_id }
-      it { is_expected.not_to validate_presence_of :sub_type }
+      it { is_expected.to validate_presence_of :sub_type }
     end
 
     context "when RMA area" do
       subject { create(:rma_area, parent_id: 1) }
 
+      it { is_expected.to validate_presence_of :identifier }
       it { is_expected.to validate_presence_of :parent_id }
       it { is_expected.to validate_presence_of :sub_type }
     end
+
+    context "when Authority" do
+      subject { create(:authority) }
+
+      it { is_expected.to validate_presence_of :identifier }
+      it { is_expected.not_to validate_presence_of :end_date }
+    end
   end
 
   describe "#country?" do

spec/services/pafs_core/project_service_spec.rb

@@ -132,7 +132,7 @@
   end
 
   describe "#new_project" do
-    let(:reference_number) { "#{PafsCore::PSO_RFCC_MAP[pso_area_1.name]}C501E" }
+    let(:reference_number) { "#{PafsCore::PsoRfccService.map[pso_area_1.name]}C501E" }
     let(:project) { subject.new_project }
 
     it "builds a new project model without saving to the database" do