CLDC-2321 Add Data Protection Confirmation validation on log change (#1694)

* Add Data Protection Confirmation on log change

* Validate managing and owning org DPC presence

* Add missing translation

app/models/log.rb

@@ -9,7 +9,6 @@ class Log < ApplicationRecord
   belongs_to :bulk_upload, optional: true
 
   before_save :update_status!
-  before_validation :verify_data_protection_confirmation, on: :create
 
   STATUS = {
     "not_started" => 0,
@@ -178,14 +177,6 @@ class Log < ApplicationRecord
 
 private
 
-  def verify_data_protection_confirmation
-    return unless FeatureToggle.new_data_protection_confirmation?
-    return unless owning_organisation
-    return if owning_organisation.data_protection_confirmed?
-
-    errors.add :owning_organisation, I18n.t("validations.organisation.data_sharing_agreement_not_signed")
-  end
-
   # Handle logs that are older than previous collection start date
   def older_than_previous_collection_year?
     return false unless startdate

app/models/validations/setup_validations.rb

@@ -42,6 +42,14 @@ module Validations::SetupValidations
     end
   end
 
+  def validate_managing_organisation_data_sharing_agremeent_signed(record)
+    return unless FeatureToggle.new_data_protection_confirmation?
+
+    if record.managing_organisation_id_changed? && record.managing_organisation.present? && !record.managing_organisation.data_protection_confirmed?
+      record.errors.add :managing_organisation_id, I18n.t("validations.setup.managing_organisation.data_sharing_agreement_not_signed")
+    end
+  end
+
 private
 
   def active_collection_start_date

app/models/validations/shared_validations.rb

@@ -117,6 +117,14 @@ module Validations::SharedValidations
     end
   end
 
+  def validate_owning_organisation_data_sharing_agremeent_signed(record)
+    return unless FeatureToggle.new_data_protection_confirmation?
+
+    if record.owning_organisation_id_changed? && record.owning_organisation.present? && !record.owning_organisation.data_protection_confirmed?
+      record.errors.add :owning_organisation_id, I18n.t("validations.setup.owning_organisation.data_sharing_agreement_not_signed")
+    end
+  end
+
 private
 
   def person_is_partner?(relationship)

config/locales/en.yml

@@ -254,8 +254,10 @@ en:
           activating_soon: "%{name} is not available until %{date}. Enter a tenancy start date after %{date}"
       owning_organisation:
         invalid: "Please select the owning organisation or managing organisation that you belong to"
+        data_sharing_agreement_not_signed: "The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."
       managing_organisation:
         invalid: "Please select the owning organisation or managing organisation that you belong to"
+        data_sharing_agreement_not_signed: "The organisation must accept the Data Sharing Agreement before it can be selected as the managing organisation."
       created_by:
         invalid: "Please select the owning organisation or managing organisation that you belong to"
       lettype:

spec/models/validations/setup_validations_spec.rb

@@ -400,4 +400,67 @@ RSpec.describe Validations::SetupValidations do
       end
     end
   end
+
+  describe "#validate_managing_organisation_data_sharing_agremeent_signed" do
+    before do
+      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
+    end
+
+    it "is valid if the DSA is signed" do
+      log = build(:lettings_log, :in_progress, owning_organisation: create(:organisation))
+
+      expect(log).to be_valid
+    end
+
+    it "is valid when owning_organisation nil" do
+      log = build(:lettings_log, owning_organisation: nil)
+
+      expect(log).to be_valid
+    end
+
+    it "is not valid if the DSA is not signed" do
+      log = build(:lettings_log, owning_organisation: create(:organisation, :without_dpc))
+
+      expect(log).to be_valid
+    end
+  end
+
+  context "when flag enabled" do
+    before do
+      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
+    end
+
+    it "is valid if the Data Protection Confirmation is signed" do
+      log = build(:lettings_log, :in_progress, managing_organisation: create(:organisation))
+
+      expect(log).to be_valid
+    end
+
+    it "is valid when managing_organisation nil" do
+      log = build(:lettings_log, managing_organisation: nil)
+
+      expect(log).to be_valid
+    end
+
+    it "is not valid if the Data Protection Confirmation is not signed" do
+      log = build(:lettings_log, managing_organisation: create(:organisation, :without_dpc))
+
+      expect(log).not_to be_valid
+      expect(log.errors[:managing_organisation_id]).to eq(["The organisation must accept the Data Sharing Agreement before it can be selected as the managing organisation."])
+    end
+
+    context "when updating" do
+      let(:log) { create(:lettings_log, :in_progress) }
+      let(:org_with_dpc) { create(:organisation) }
+      let(:org_without_dpc) { create(:organisation, :without_dpc) }
+
+      it "is valid when changing to another org with a signed Data Protection Confirmation" do
+        expect { log.managing_organisation = org_with_dpc }.to not_change(log, :valid?)
+      end
+
+      it "invalid when changing to another org without a signed Data Protection Confirmation" do
+        expect { log.managing_organisation = org_without_dpc }.to change(log, :valid?).from(true).to(false).and(change { log.errors[:managing_organisation_id] }.to(["The organisation must accept the Data Sharing Agreement before it can be selected as the managing organisation."]))
+      end
+    end
+  end
 end

spec/models/validations/shared_validations_spec.rb

@@ -4,8 +4,8 @@ RSpec.describe Validations::SharedValidations do
   subject(:shared_validator) { validator_class.new }
 
   let(:validator_class) { Class.new { include Validations::SharedValidations } }
-  let(:lettings_log) { FactoryBot.create(:lettings_log) }
-  let(:sales_log) { FactoryBot.create(:sales_log, :completed) }
+  let(:lettings_log) { create(:lettings_log) }
+  let(:sales_log) { create(:sales_log, :completed) }
   let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") }
 
   describe "numeric min max validations" do
@@ -174,5 +174,70 @@ RSpec.describe Validations::SharedValidations do
         expect(sales_log.errors).to be_empty
       end
     end
+
+    %i[sales_log lettings_log].each do |log_type|
+      describe "validate_owning_organisation_data_sharing_agremeent_signed" do
+        before do
+          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
+        end
+
+        it "is valid if the DSA is signed" do
+          log = build(log_type, :in_progress, owning_organisation: create(:organisation))
+
+          expect(log).to be_valid
+        end
+
+        it "is valid when owning_organisation nil" do
+          log = build(log_type, owning_organisation: nil)
+
+          expect(log).to be_valid
+        end
+
+        it "is not valid if the DSA is not signed" do
+          log = build(log_type, owning_organisation: create(:organisation, :without_dpc))
+
+          expect(log).to be_valid
+        end
+      end
+
+      context "when flag enabled" do
+        before do
+          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
+        end
+
+        it "is valid if the Data Protection Confirmation is signed" do
+          log = build(log_type, :in_progress, owning_organisation: create(:organisation))
+
+          expect(log).to be_valid
+        end
+
+        it "is valid when owning_organisation nil" do
+          log = build(log_type, owning_organisation: nil)
+
+          expect(log).to be_valid
+        end
+
+        it "is not valid if the Data Protection Confirmation is not signed" do
+          log = build(log_type, owning_organisation: create(:organisation, :without_dpc))
+
+          expect(log).not_to be_valid
+          expect(log.errors[:owning_organisation_id]).to eq(["The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."])
+        end
+
+        context "when updating" do
+          let(:log) { create(log_type, :in_progress) }
+          let(:org_with_dpc) { create(:organisation) }
+          let(:org_without_dpc) { create(:organisation, :without_dpc) }
+
+          it "is valid when changing to another org with a signed Data Protection Confirmation" do
+            expect { log.owning_organisation = org_with_dpc }.not_to change(log, :valid?)
+          end
+
+          it "invalid when changing to another org without a signed Data Protection Confirmation" do
+            expect { log.owning_organisation = org_without_dpc }.to change(log, :valid?).from(true).to(false).and(change { log.errors[:owning_organisation_id] }.to(["The organisation must accept the Data Sharing Agreement before it can be selected as the owning organisation."]))
+          end
+        end
+      end
+    end
   end
 end

spec/shared/shared_log_examples.rb

@@ -104,54 +104,5 @@ RSpec.shared_examples "shared log examples" do |log_type|
       end
     end
   end
-
-  describe "#verify_data_protection_confirmation" do
-    before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-    end
-
-    it "is valid if the DSA is signed" do
-      log = build(log_type, :in_progress, owning_organisation: create(:organisation))
-
-      expect(log).to be_valid
-    end
-
-    it "is valid when owning_organisation nil" do
-      log = build(log_type, owning_organisation: nil)
-
-      expect(log).to be_valid
-    end
-
-    it "is not valid if the DSA is not signed" do
-      log = build(log_type, owning_organisation: create(:organisation, :without_dpc))
-
-      expect(log).to be_valid
-    end
-  end
-
-  context "when flag enabled" do
-    before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
-    end
-
-    it "is valid if the DSA is signed" do
-      log = build(log_type, :in_progress, owning_organisation: create(:organisation))
-
-      expect(log).to be_valid
-    end
-
-    it "is valid when owning_organisation nil" do
-      log = build(log_type, owning_organisation: nil)
-
-      expect(log).to be_valid
-    end
-
-    it "is not valid if the DSA is not signed" do
-      log = build(log_type, owning_organisation: create(:organisation, :without_dpc))
-
-      expect(log).not_to be_valid
-      expect(log.errors[:owning_organisation]).to eq(["Your organisation must accept the Data Sharing Agreement before you can create any logs."])
-    end
-  end
 end
 # rubocop:enable RSpec/AnyInstance