CLDC-2490 Tidy up DSA model (#1766)

* Remove new_data_protection_confirmation? flag

* Update schema

- drop index
- add org and user data to dpc table

* Persist org and user data at import

* Do not show invalid emails

* Persist user and org data while signing agreement

* Add migration to persist org and user data on DPC

* Rebase fix

* fix typo

app/components/create_log_actions_component.rb

@@ -13,7 +13,6 @@ class CreateLogActionsComponent < ViewComponent::Base
 
   def display_actions?
     return false if bulk_upload.present?
-    return true unless FeatureToggle.new_data_protection_confirmation?
     return true if user.support?
 
     user.organisation.data_protection_confirmed?

app/components/data_protection_confirmation_banner_component.rb

@@ -13,7 +13,6 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base
   end
 
   def display_banner?
-    return false unless FeatureToggle.new_data_protection_confirmation?
     return false if user.support? && organisation.blank?
     return true if org_without_dpo?
 

app/controllers/bulk_upload_lettings_logs_controller.rb

@@ -25,7 +25,6 @@ class BulkUploadLettingsLogsController < ApplicationController
 private
 
   def validate_data_protection_agrement_signed!
-    return unless FeatureToggle.new_data_protection_confirmation?
     return if @current_user.organisation.data_protection_confirmed?
 
     redirect_to lettings_logs_path

app/controllers/bulk_upload_sales_logs_controller.rb

@@ -25,7 +25,6 @@ class BulkUploadSalesLogsController < ApplicationController
 private
 
   def validate_data_protection_agrement_signed!
-    return unless FeatureToggle.new_data_protection_confirmation?
     return if @current_user.organisation.data_protection_confirmed?
 
     redirect_to sales_logs_path

app/controllers/organisations_controller.rb

@@ -159,13 +159,10 @@ class OrganisationsController < ApplicationController
   end
 
   def data_sharing_agreement
-    return render_not_found unless FeatureToggle.new_data_protection_confirmation?
-
     @data_protection_confirmation = current_user.organisation.data_protection_confirmation
   end
 
   def confirm_data_sharing_agreement
-    return render_not_found unless FeatureToggle.new_data_protection_confirmation?
     return render_not_found unless current_user.is_dpo?
     return render_not_found if @organisation.data_protection_confirmed?
 
@@ -173,14 +170,25 @@ class OrganisationsController < ApplicationController
       @organisation.data_protection_confirmation.update!(
         confirmed: true,
         data_protection_officer: current_user,
-        # When it was signed
         created_at: Time.zone.now,
+        signed_at: Time.zone.now,
+        organisation_name: @organisation.name,
+        organisation_address: @organisation.address_row,
+        organisation_phone_number: @organisation.phone,
+        data_protection_officer_email: current_user.email,
+        data_protection_officer_name: current_user.name,
       )
     else
       DataProtectionConfirmation.create!(
-        organisation: current_user.organisation,
+        organisation: @organisation,
         confirmed: true,
         data_protection_officer: current_user,
+        signed_at: Time.zone.now,
+        organisation_name: @organisation.name,
+        organisation_address: @organisation.address_row,
+        organisation_phone_number: @organisation.phone,
+        data_protection_officer_email: current_user.email,
+        data_protection_officer_name: current_user.name,
       )
     end
 

app/helpers/data_sharing_agreement_helper.rb

@@ -61,8 +61,12 @@ module DataSharingAgreementHelper
       end
     end
 
+    if data_protection_confirmation&.confirmed? && @dpo_email.exclude?("@") # Do not show invalid email addresses
+      "12.2. For #{@org_name}: Name: #{@dpo_name}, Postal Address: #{@org_address}, Telephone number: #{@org_phone}"
+    else
       "12.2. For #{@org_name}: Name: #{@dpo_name}, Postal Address: #{@org_address}, E-mail address: #{@dpo_email}, Telephone number: #{@org_phone}"
     end
+  end
 # rubocop:enable Rails/HelperInstanceVariable
 
 private

app/models/organisation.rb

@@ -103,7 +103,7 @@ class Organisation < ApplicationRecord
   end
 
   def display_organisation_attributes
-    attrs = [
+    [
       { name: "Name", value: name, editable: true },
       { name: "Organisation ID", value: "ORG#{id}", editable: false },
       { name: "Address", value: address_string, editable: true },
@@ -112,13 +112,7 @@ class Organisation < ApplicationRecord
       { name: "Registration number", value: housing_registration_no || "", editable: false },
       { name: "Rent periods", value: rent_period_labels, editable: false, format: :bullet },
       { name: "Owns housing stock", value: holds_own_stock ? "Yes" : "No", editable: false },
-    ].compact
-
-    unless FeatureToggle.new_data_protection_confirmation?
-      attrs << { name: "Data protection agreement", value: data_protection_agreement_string, editable: false }
-    end
-
-    attrs
+    ]
   end
 
   def has_managing_agents?

app/models/user.rb

@@ -186,7 +186,6 @@ protected
 private
 
   def send_data_protection_confirmation_reminder
-    return unless FeatureToggle.new_data_protection_confirmation?
     return unless persisted?
     return unless is_dpo?
 

app/models/validations/setup_validations.rb

@@ -49,8 +49,6 @@ module Validations::SetupValidations
   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

app/models/validations/shared_validations.rb

@@ -118,8 +118,6 @@ module Validations::SharedValidations
   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

app/services/feature_toggle.rb

@@ -30,10 +30,6 @@ class FeatureToggle
     !Rails.env.production?
   end
 
-  def self.new_data_protection_confirmation?
-    true
-  end
-
   def self.deduplication_flow_enabled?
     !Rails.env.production? && !Rails.env.staging?
   end

app/services/imports/data_protection_confirmation_import_service.rb

@@ -38,6 +38,12 @@ module Imports
         old_id: record_field_value(xml_document, "id"),
         old_org_id: record_field_value(xml_document, "institution"),
         created_at: record_field_value(xml_document, "change-date").to_time(:utc),
+        signed_at: record_field_value(xml_document, "change-date").to_time(:utc),
+        organisation_name: org.name,
+        organisation_address: org.address_row,
+        organisation_phone_number: org.phone,
+        data_protection_officer_email: dp_officer.email,
+        data_protection_officer_name: dp_officer.name,
       )
     end
 

app/views/logs/_create_for_org_actions.html.erb

@@ -1,5 +1,5 @@
 <div class="govuk-button-group app-filter-toggle">
-  <% if !FeatureToggle.new_data_protection_confirmation? || @organisation.data_protection_confirmed? %>
+  <% if @organisation.data_protection_confirmed? %>
     <% if current_page?(controller: 'organisations', action: 'lettings_logs') %>
       <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post) %>
     <% end %>

app/views/organisations/show.html.erb

@@ -33,10 +33,8 @@
           <% end %>
         <% end %>
       <% end %>
-      <% if FeatureToggle.new_data_protection_confirmation? %>
       <%= data_sharing_agreement_row(organisation: @organisation, user: current_user, summary_list:) %>
     <% end %>
-    <% end %>
     <% if FeatureToggle.merge_organisations_enabled? %>
       <p>Is your organisation merging with another? <%= govuk_link_to "Let us know using this form", merge_request_organisation_path(@organisation) %></p>
     <% end %>

db/migrate/20230710100120_persist_user_and_organisation_info_on_data_protection_confirmation.rb

@@ -0,0 +1,12 @@
+class PersistUserAndOrganisationInfoOnDataProtectionConfirmation < ActiveRecord::Migration[7.0]
+  def change
+    change_table :data_protection_confirmations, bulk: true do |t|
+      t.column :signed_at, :datetime
+      t.column :organisation_name, :string
+      t.column :organisation_address, :string
+      t.column :organisation_phone_number, :string
+      t.column :data_protection_officer_email, :string
+      t.column :data_protection_officer_name, :string
+    end
+  end
+end

db/migrate/20230710101532_remove_data_protection_confirmation_unique_index.rb

@@ -0,0 +1,14 @@
+class RemoveDataProtectionConfirmationUniqueIndex < ActiveRecord::Migration[7.0]
+  def up
+    remove_index :data_protection_confirmations,
+                 %i[organisation_id data_protection_officer_id confirmed],
+                 unique: true
+  end
+
+  def down
+    add_index :data_protection_confirmations,
+              %i[organisation_id data_protection_officer_id confirmed],
+              unique: true,
+              name: "data_protection_confirmations_unique"
+  end
+end

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_06_29_125541) do
+ActiveRecord::Schema[7.0].define(version: 2023_07_10_101532) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -52,8 +52,13 @@ ActiveRecord::Schema[7.0].define(version: 2023_06_29_125541) do
     t.string "old_org_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.datetime "signed_at"
+    t.string "organisation_name"
+    t.string "organisation_address"
+    t.string "organisation_phone_number"
+    t.string "data_protection_officer_email"
+    t.string "data_protection_officer_name"
     t.index ["data_protection_officer_id"], name: "dpo_user_id"
-    t.index ["organisation_id", "data_protection_officer_id", "confirmed"], name: "data_protection_confirmations_unique", unique: true
     t.index ["organisation_id"], name: "index_data_protection_confirmations_on_organisation_id"
   end
 

lib/tasks/data_import.rake

@@ -29,26 +29,20 @@ namespace :core do
     end
   end
 
-  desc "Import missing data sharing agreement XMLs from legacy CORE"
-  task :data_protection_confirmation_import, %i[type path] => :environment do |_task|
-    orgs_without_dpc = Organisation.includes(:data_protection_confirmation)
-      .where.not(id: DataProtectionConfirmation.all.select(:organisation_id))
-      .where.not(old_org_id: nil)
-
-    puts "Processing #{orgs_without_dpc.count} orgs without data protection confirmation"
-    s3_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"])
-
-    orgs_without_dpc.each do |org|
-      archive_path = "#{org.old_org_id}.zip"
-      archive_io = s3_service.get_file_io(archive_path)
-      archive_service = Storage::ArchiveService.new(archive_io)
-      if archive_service.folder_present?("dataprotect")
-        Rails.logger.info("Start importing folder dataprotect")
-        Imports::DataProtectionConfirmationImportService.new(archive_service).create_data_protection_confirmations("dataprotect")
-        Rails.logger.info("Imported")
-      else
-        Rails.logger.info("dataprotect does not exist, skipping import")
-      end
+  desc "Persist user and org data on data sharing confirmations"
+  task persist_user_and_org_data_on_data_sharing_confirmations: :environment do |_task|
+    DataProtectionConfirmation.all.includes(:data_protection_officer, :organisation).each do |dpc|
+      dpc.update!(
+        organisation_name: dpc.organisation.name,
+        organisation_address: dpc.organisation.address_row,
+        signed_at: dpc.created_at,
+        organisation_phone_number: dpc.organisation.phone,
+        data_protection_officer_email: dpc.data_protection_officer.email,
+        data_protection_officer_name: dpc.data_protection_officer.name,
+      )
+      print "."
     end
+
+    puts "done"
   end
 end

spec/components/create_log_actions_component_spec.rb

@@ -20,56 +20,6 @@ RSpec.describe CreateLogActionsComponent, type: :component do
   context "when bulk upload nil" do
     let(:bulk_upload) { nil }
 
-    context "when flag disabled" do
-      before do
-        allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-      end
-
-      it "renders actions" do
-        expect(component.display_actions?).to eq(true)
-      end
-
-      it "returns create button copy" do
-        expect(component.create_button_copy).to eq("Create a new lettings log")
-      end
-
-      it "returns create button href" do
-        render
-        expect(component.create_button_href).to eq("/lettings-logs")
-      end
-
-      it "returns upload button copy" do
-        expect(component.upload_button_copy).to eq("Upload lettings logs in bulk")
-      end
-
-      it "returns upload button href" do
-        render
-        expect(component.upload_button_href).to eq("/lettings-logs/bulk-upload-logs/start")
-      end
-
-      context "when sales log type" do
-        let(:log_type) { "sales" }
-
-        it "renders actions" do
-          expect(component.display_actions?).to eq(true)
-        end
-
-        it "returns create button copy" do
-          expect(component.create_button_copy).to eq("Create a new sales log")
-        end
-
-        it "returns create button href" do
-          render
-          expect(component.create_button_href).to eq("/sales-logs")
-        end
-      end
-    end
-
-    context "when flag enabled" do
-      before do
-        allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
-      end
-
     context "when support user" do
       let(:user) { create(:user, :support) }
 
@@ -166,5 +116,4 @@ RSpec.describe CreateLogActionsComponent, type: :component do
       end
     end
   end
-  end
 end

spec/components/data_protection_confirmation_banner_component_spec.rb

@@ -8,10 +8,9 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
   let(:user) { create(:user) }
   let(:organisation) { user.organisation }
 
-  context "when flag disabled" do
-    before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-    end
+  context "when user is support and organisation is blank" do
+    let(:user) { create(:user, :support) }
+    let(:organisation) { nil }
 
     it "does not display banner" do
       expect(component.display_banner?).to eq(false)
@@ -19,15 +18,52 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
     end
   end
 
-  context "when flag enabled", :aggregate_failures do
+  context "when org does not have a DPO" do
     before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
+      organisation.users.where(is_dpo: true).destroy_all
     end
 
-    context "when user is support and organisation is blank" do
-      let(:user) { create(:user, :support) }
-      let(:organisation) { nil }
+    it "displays the banner" do
+      expect(component.display_banner?).to eq(true)
+      expect(render).to have_link(
+        "Contact helpdesk to assign a data protection officer",
+        href: "https://dluhcdigital.atlassian.net/servicedesk/customer/portal/6/group/11",
+      )
+      expect(render).to have_selector("p", text: "To create logs your organisation must state a data protection officer. They must sign the Data Sharing Agreement.")
+    end
+  end
+
+  context "when org has a DPO" do
+    context "when org does not have a signed data sharing agreement" do
+      context "when user is not a DPO" do
+        let(:organisation) { create(:organisation, :without_dpc) }
+        let(:user) { create(:user, organisation:) }
+        let!(:dpo) { create(:user, :data_protection_officer, organisation:) }
+
+        it "displays the banner and shows DPOs" do
+          expect(component.display_banner?).to eq(true)
+          expect(render.css("a")).to be_empty
+          expect(render).to have_selector("p", text: "Your data protection officer must accept the Data Sharing Agreement on CORE before you can create any logs.")
+          expect(render).to have_selector("p", text: "You can ask: #{dpo.name}")
+        end
+      end
+
+      context "when user is a DPO" do
+        let(:organisation) { create(:organisation, :without_dpc) }
+        let(:user) { create(:user, :data_protection_officer, organisation:) }
 
+        it "displays the banner and asks to sign" do
+          expect(component.display_banner?).to eq(true)
+          expect(render).to have_link(
+            "Read the Data Sharing Agreement",
+            href: "/organisations/#{organisation.id}/data-sharing-agreement",
+          )
+          expect(render).to have_selector("p", text: "Your organisation must accept the Data Sharing Agreement before you can create any logs.")
+        end
+      end
+    end
+
+    context "when org has a signed data sharing agremeent" do
       it "does not display banner" do
         expect(component.display_banner?).to eq(false)
         expect(render.content).to be_empty

spec/lib/tasks/data_import_spec.rb

@@ -176,42 +176,4 @@ describe "data import", type: :task do
       expect { task.invoke("unknown_type", "my_path") }.to raise_error(/Type unknown_type is not supported/)
     end
   end
-
-  describe "core:data_protection_confirmation_import" do
-    subject(:task) { Rake::Task["core:data_protection_confirmation_import"] }
-
-    let(:import_service) { instance_double(Imports::DataProtectionConfirmationImportService) }
-    # let(:fixture_path) { "spec/fixtures/imports/data_protection_confirmations" }
-    let(:storage_service) { instance_double(Storage::S3Service) }
-    let(:archive_service) { instance_double(Storage::ArchiveService) }
-
-    let!(:organisation_without_dpc_with_old_org_id) { create(:organisation, :without_dpc, old_org_id: 1) }
-
-    before do
-      Rake.application.rake_require("tasks/data_import")
-      Rake::Task.define_task(:environment)
-      task.reenable
-
-      allow(Storage::S3Service).to receive(:new).and_return(storage_service)
-      allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service)
-      allow(ENV).to receive(:[])
-      allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name)
-      allow(Imports::DataProtectionConfirmationImportService).to receive(:new).and_return(import_service)
-
-      allow(Storage::ArchiveService).to receive(:new).and_return(archive_service)
-      allow(archive_service).to receive(:folder_present?).with("dataprotect").and_return(true)
-
-      _org_without_old_org_id = create(:organisation, old_org_id: nil)
-      _organisation_without_dp = create(:organisation, :without_dpc, old_org_id: nil)
-    end
-
-    it "creates an organisation from the given XML file" do
-      expect(Storage::S3Service).to receive(:new).with(paas_config_service, instance_name)
-      expect(storage_service).to receive(:get_file_io).with("#{organisation_without_dpc_with_old_org_id.old_org_id}.zip")
-      expect(Imports::DataProtectionConfirmationImportService).to receive(:new).with(archive_service)
-      expect(import_service).to receive(:create_data_protection_confirmations).with("dataprotect")
-
-      task.invoke
-    end
-  end
 end

spec/models/organisation_spec.rb

@@ -234,11 +234,6 @@ RSpec.describe Organisation, type: :model do
   describe "display_organisation_attributes" do
     let(:organisation) { create(:organisation) }
 
-    context "when new_data_protection_confirmation flag enabled" do
-      before do
-        allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
-      end
-
     it "does not include data protection agreement" do
       expect(organisation.display_organisation_attributes).to eq(
         [{ editable: true, name: "Name", value: "DLUHC" },
@@ -254,27 +249,4 @@ RSpec.describe Organisation, type: :model do
       )
     end
   end
-
-    context "when new_data_protection_confirmation flag disabled" do
-      before do
-        allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-      end
-
-      it "includes data protection agreement" do
-        expect(organisation.display_organisation_attributes).to eq(
-          [{ editable: true, name: "Name", value: "DLUHC" },
-           { editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" },
-           { editable: true,
-             name: "Address",
-             value: "2 Marsham Street\nLondon\nSW1P 4DF" },
-           { editable: true, name: "Telephone number", value: nil },
-           { editable: false, name: "Type of provider", value: "Local authority" },
-           { editable: false, name: "Registration number", value: "1234" },
-           { editable: false, format: :bullet, name: "Rent periods", value: %w[All] },
-           { editable: false, name: "Owns housing stock", value: "Yes" },
-           { editable: false, name: "Data protection agreement", value: "Accepted" }],
-        )
-      end
-    end
-  end
 end

spec/models/user_spec.rb

@@ -382,16 +382,6 @@ RSpec.describe User, type: :model do
           args: [user],
         )
       end
-
-      context "when feature flag disabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-        end
-
-        it "does not send the email" do
-          expect { user.update!(is_dpo: true) }.not_to enqueue_job(ActionMailer::MailDeliveryJob)
-        end
-      end
     end
 
     context "when updating to non dpo" do

spec/models/validations/setup_validations_spec.rb

@@ -444,34 +444,6 @@ RSpec.describe Validations::SetupValidations do
   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))
 

spec/models/validations/shared_validations_spec.rb

@@ -177,34 +177,6 @@ RSpec.describe Validations::SharedValidations do
 
     %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))
 

spec/requests/bulk_upload_lettings_logs_controller_spec.rb

@@ -18,18 +18,6 @@ RSpec.describe BulkUploadLettingsLogsController, type: :request do
 
         expect(response).to redirect_to("/lettings-logs")
       end
-
-      context "when feature flag disabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-        end
-
-        it "does not redirect to lettings index page" do
-          get "/lettings-logs/bulk-upload-logs/start", params: {}
-
-          expect(response).not_to redirect_to("/lettings-logs")
-        end
-      end
     end
 
     context "when not in crossover period" do

spec/requests/bulk_upload_sales_logs_controller_spec.rb

@@ -18,18 +18,6 @@ RSpec.describe BulkUploadSalesLogsController, type: :request do
 
         expect(response).to redirect_to("/sales-logs")
       end
-
-      context "when feature flag disabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-        end
-
-        it "does not redirect to lettings index page" do
-          get "/lettings-logs/bulk-upload-logs/start", params: {}
-
-          expect(response).not_to redirect_to("/sales-logs")
-        end
-      end
     end
 
     context "when not in crossover period" do

spec/requests/organisations_controller_spec.rb

@@ -1501,29 +1501,12 @@ RSpec.describe OrganisationsController, type: :request do
         sign_in user
       end
 
-      context "when flag not enabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-        end
-
-        it "returns not found" do
-          get "/organisations/#{organisation.id}/data-sharing-agreement", headers: headers
-          expect(response).to have_http_status(:not_found)
-        end
-      end
-
-      context "when flag enabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
-        end
-
       it "returns ok" do
         get "/organisations/#{organisation.id}/data-sharing-agreement", headers: headers
         expect(response).to have_http_status(:ok)
       end
     end
   end
-  end
 
   describe "POST #data_sharing_agreement" do
     let(:organisation) { create(:organisation, :without_dpc) }
@@ -1541,22 +1524,6 @@ RSpec.describe OrganisationsController, type: :request do
         sign_in user
       end
 
-      context "when flag not enabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-        end
-
-        it "returns not found" do
-          post "/organisations/#{organisation.id}/data-sharing-agreement", headers: headers
-          expect(response).to have_http_status(:not_found)
-        end
-      end
-
-      context "when flag enabled" do
-        before do
-          allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
-        end
-
       context "when user not dpo" do
         let(:user) { create(:user, is_dpo: false) }
 
@@ -1577,6 +1544,14 @@ RSpec.describe OrganisationsController, type: :request do
         end
 
         context "when the organisation does not have a confirmation" do
+          before do
+            Timecop.freeze(Time.zone.local(2022, 2, 1))
+          end
+
+          after do
+            Timecop.unfreeze
+          end
+
           let(:user) { create(:user, is_dpo: true, organisation:) }
 
           it "returns redirects to details page" do
@@ -1594,10 +1569,14 @@ RSpec.describe OrganisationsController, type: :request do
 
             data_protection_confirmation = organisation.reload.data_protection_confirmation
 
-              expect(data_protection_confirmation.organisation.address_row).to eq(organisation.address_row)
-              expect(data_protection_confirmation.organisation.name).to eq(organisation.name)
-              expect(data_protection_confirmation.organisation.phone).to eq(organisation.phone)
+            expect(data_protection_confirmation.organisation).to eq(organisation)
             expect(data_protection_confirmation.data_protection_officer).to eq(user)
+            expect(data_protection_confirmation.signed_at).to eq(Time.zone.local(2022, 2, 1))
+            expect(data_protection_confirmation.organisation_name).to eq(organisation.name)
+            expect(data_protection_confirmation.organisation_address).to eq(organisation.address_row)
+            expect(data_protection_confirmation.organisation_phone_number).to eq(organisation.phone)
+            expect(data_protection_confirmation.data_protection_officer_email).to eq(user.email)
+            expect(data_protection_confirmation.data_protection_officer_name).to eq(user.name)
           end
 
           context "when the user has already accepted the agreement" do
@@ -1614,5 +1593,4 @@ RSpec.describe OrganisationsController, type: :request do
       end
     end
   end
-  end
 end

spec/services/imports/data_protection_confirmation_import_service_spec.rb

@@ -29,7 +29,7 @@ RSpec.describe Imports::DataProtectionConfirmationImportService do
     end
 
     context "when the organisation does exist" do
-      let!(:organisation) { create(:organisation, :without_dpc, old_org_id:) }
+      let!(:organisation) { create(:organisation, :without_dpc, old_org_id:, phone: "123") }
 
       context "when a data protection officer with matching name does not exists for the organisation" do
         it "creates an inactive data protection officer" do
@@ -40,12 +40,18 @@ RSpec.describe Imports::DataProtectionConfirmationImportService do
           expect(data_protection_officer.active).to be false
         end
 
-        it "successfully create a data protection confirmation record with the expected data" do
+        it "successfully create a data protection confirmation record with the expected data", :aggregate_failures do
           import_service.create_data_protection_confirmations("data_protection_directory")
           confirmation = Organisation.find_by(old_org_id:).data_protection_confirmation
-          expect(confirmation.data_protection_officer.name).to eq("John Doe")
           expect(confirmation.confirmed).to be_truthy
+
+          expect(confirmation.data_protection_officer.name).to eq("John Doe")
+          expect(confirmation.data_protection_officer_name).to eq("John Doe")
+          expect(confirmation.organisation_address).to eq("2 Marsham Street, London, SW1P 4DF")
+          expect(confirmation.organisation_name).to eq("DLUHC")
+          expect(confirmation.organisation_phone_number).to eq("123")
           expect(Time.zone.local_to_utc(confirmation.created_at)).to eq(Time.utc(2018, 0o6, 0o5, 10, 36, 49))
+          expect(Time.zone.local_to_utc(confirmation.signed_at)).to eq(Time.utc(2018, 0o6, 0o5, 10, 36, 49))
         end
       end
 
@@ -60,7 +66,15 @@ RSpec.describe Imports::DataProtectionConfirmationImportService do
           confirmation = Organisation.find_by(old_org_id:).data_protection_confirmation
           expect(confirmation.data_protection_officer.id).to eq(data_protection_officer.id)
           expect(confirmation.confirmed).to be_truthy
+
+          expect(confirmation.data_protection_officer.name).to eq(data_protection_officer.name)
+          expect(confirmation.data_protection_officer_name).to eq(data_protection_officer.name)
+          expect(confirmation.data_protection_officer_email).to eq(data_protection_officer.email)
+          expect(confirmation.organisation_address).to eq("2 Marsham Street, London, SW1P 4DF")
+          expect(confirmation.organisation_name).to eq("DLUHC")
+          expect(confirmation.organisation_phone_number).to eq("123")
           expect(Time.zone.local_to_utc(confirmation.created_at)).to eq(Time.utc(2018, 0o6, 0o5, 10, 36, 49))
+          expect(Time.zone.local_to_utc(confirmation.signed_at)).to eq(Time.utc(2018, 0o6, 0o5, 10, 36, 49))
         end
 
         context "when the data protection record has already been imported previously" do

spec/views/logs/_create_for_org_actions.html.erb_spec.rb

@@ -11,24 +11,6 @@ RSpec.describe "logs/_create_for_org_actions.html.erb" do
 
   let(:user) { create(:user) }
 
-  context "when flag disabled" do
-    before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-    end
-
-    it "shows create buttons" do
-      render
-
-      expect(fragment).to have_button("Create a new lettings log for this organisation")
-      expect(fragment).to have_button("Create a new sales log for this organisation")
-    end
-  end
-
-  context "when flag enabled" do
-    before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
-    end
-
   context "with data sharing agreement" do
     it "does include create log buttons" do
       render
@@ -46,5 +28,4 @@ RSpec.describe "logs/_create_for_org_actions.html.erb" do
       expect(fragment).not_to have_button("Create a new sales log for this organisation")
     end
   end
-  end
 end

spec/views/organisations/data_sharing_agreement.html.erb_spec.rb

@@ -65,6 +65,50 @@ RSpec.describe "organisations/data_sharing_agreement.html.erb", :aggregate_failu
         # Shows DPO and org details in 12.2
         expect(fragment).to have_content("12.2. For #{organisation.name}: Name: #{dpo.name}, Postal Address: #{organisation.address_row}, E-mail address: #{dpo.email}, Telephone number: #{organisation.phone}")
       end
+
+      context "when user email not valid" do
+        let(:dpo) do
+          u = User.new(
+            name: "test",
+            organisation:,
+            is_dpo: true,
+            encrypted_password: SecureRandom.hex(10),
+            email: SecureRandom.uuid,
+            confirmed_at: Time.zone.now,
+            active: false,
+          )
+          u.save!(validate: false)
+          u
+        end
+
+        let(:data_protection_confirmation) do
+          create(
+            :data_protection_confirmation,
+            organisation:,
+            created_at: Time.zone.now - 1.day,
+            data_protection_officer: dpo,
+          )
+        end
+
+        it "renders dynamic content" do
+          render
+
+          # dpo name
+          expect(fragment).to have_content("Name: #{dpo.name}")
+
+          # org details
+          expect(fragment).to have_content("#{organisation.name} of #{organisation.address_row} (“CORE Data Provider”)")
+          # header
+          expect(fragment).to have_css("h2", text: "#{organisation.name} and Department for Levelling Up, Housing and Communities")
+          # does not show action buttons
+          expect(fragment).not_to have_button(text: "Accept this agreement")
+          expect(fragment).not_to have_link(text: "Cancel", href: "/organisations/#{organisation.id}/details")
+          # sees signed_at date
+          expect(fragment).to have_content("9th day of January 2023")
+          # Shows DPO and org details in 12.2
+          expect(fragment).to have_content("12.2. For #{organisation.name}: Name: #{dpo.name}, Postal Address: #{organisation.address_row}, Telephone number: #{organisation.phone}")
+        end
+      end
     end
   end
 

spec/views/organisations/show.html.erb_spec.rb

@@ -15,20 +15,6 @@ RSpec.describe "organisations/show.html.erb" do
   let(:organisation_without_dpc) { create(:organisation, :without_dpc) }
   let(:organisation_with_dsa) { create(:organisation) }
 
-  context "when flag disabled" do
-    let(:user) { create(:user, organisation: organisation_without_dpc) }
-
-    before do
-      allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(false)
-    end
-
-    it "does not include data sharing agreement row" do
-      render
-
-      expect(fragment).not_to have_content("Data Sharing Agreement")
-    end
-  end
-
   context "when dpo" do
     let(:user) { create(:user, is_dpo: true, organisation: organisation_without_dpc) }