From c023a9f0bbe789e542fea6f2a2e466277ec754b8 Mon Sep 17 00:00:00 2001
From: Jack S <jacopo.scotti@softwire.com>
Date: Thu, 8 Jun 2023 16:33:20 +0100
Subject: [PATCH] Put validation behind feature flag

---
 .../create_log_actions_component.rb           |  4 +--
 ...data_sharing_agreement_banner_component.rb | 11 +++----
 app/controllers/organisations_controller.rb   | 32 ++++++++++++-------
 app/helpers/data_sharing_agreement_helper.rb  | 28 ++++++++--------
 app/models/log.rb                             |  3 +-
 app/models/organisation.rb                    | 12 ++++---
 app/services/feature_toggle.rb                |  2 +-
 .../logs/_create_for_org_actions.html.erb     |  2 +-
 .../data_sharing_agreement.html.erb           |  6 ++--
 ...01144_drop_data_sharing_adreement_table.rb |  5 +++
 db/schema.rb                                  | 18 +----------
 db/seeds.rb                                   | 23 +++++++++----
 .../factories/data_protection_confirmation.rb |  5 +--
 spec/factories/data_sharing_agreement.rb      | 15 ---------
 spec/factories/organisation.rb                |  6 +---
 spec/shared/shared_log_examples.rb            | 32 +++++++++++++++++--
 16 files changed, 112 insertions(+), 92 deletions(-)
 create mode 100644 db/migrate/20230609101144_drop_data_sharing_adreement_table.rb
 delete mode 100644 spec/factories/data_sharing_agreement.rb

diff --git a/app/components/create_log_actions_component.rb b/app/components/create_log_actions_component.rb
index 9ae37d67a..97b3e8ffa 100644
--- a/app/components/create_log_actions_component.rb
+++ b/app/components/create_log_actions_component.rb
@@ -13,10 +13,10 @@ class CreateLogActionsComponent < ViewComponent::Base
 
   def display_actions?
     return false if bulk_upload.present?
-    return true unless FeatureToggle.new_data_sharing_agreement?
+    return true unless FeatureToggle.new_data_protection_confirmation?
     return true if user.support?
 
-    user.organisation.data_sharing_agreement.present?
+    user.organisation.data_protection_confirmation&.confirmed?
   end
 
   def create_button_href
diff --git a/app/components/data_sharing_agreement_banner_component.rb b/app/components/data_sharing_agreement_banner_component.rb
index b47ae7fb3..a9d15bdc9 100644
--- a/app/components/data_sharing_agreement_banner_component.rb
+++ b/app/components/data_sharing_agreement_banner_component.rb
@@ -11,15 +11,14 @@ class DataSharingAgreementBannerComponent < ViewComponent::Base
   end
 
   def display_banner?
-    return false unless FeatureToggle.new_data_sharing_agreement?
+    return false unless FeatureToggle.new_data_protection_confirmation?
     return false if user.is_dpo?
     return false if user.support? && organisation.blank?
 
-    if organisation.present?
-      !DataSharingAgreement.exists?(organisation:)
-    else
-      !DataSharingAgreement.exists?(organisation: user.organisation)
-    end
+    !DataProtectionConfirmation.exists?(
+      organisation: (organisation.presence || user.organisation),
+      confirmed: true,
+    )
   end
 
   def data_sharing_agreement_href
diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb
index a1f8c595d..6fea6ec75 100644
--- a/app/controllers/organisations_controller.rb
+++ b/app/controllers/organisations_controller.rb
@@ -157,28 +157,36 @@ class OrganisationsController < ApplicationController
   end
 
   def data_sharing_agreement
-    return render_not_found unless FeatureToggle.new_data_sharing_agreement?
+    return render_not_found unless FeatureToggle.new_data_protection_confirmation?
 
-    @data_sharing_agreement = current_user.organisation.data_sharing_agreement
+    @data_protection_confirmation = current_user.organisation.data_protection_confirmation
   end
 
   def confirm_data_sharing_agreement
-    return render_not_found unless FeatureToggle.new_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_sharing_agreement.present?
+    return render_not_found if @organisation.latest_data_protection_confirmation&.confirmed?
 
-    data_sharing_agreement = DataSharingAgreement.new(
+
+    if @organisation.latest_data_protection_confirmation
+      @organisation.latest_data_protection_confirmation.update!(
+        confirmed: true,
+        data_protection_officer: current_user,
+        # When it was signed
+        created_at: Time.zone.now,
+
+      )
+      dpo.confirmed = true
+      dpo.data_protection_officer = current_user
+      dpo.save!
+
+    data_protection_confirmation = DataProtectionConfirmation.new(
       organisation: current_user.organisation,
-      signed_at: Time.zone.now,
+      confirmed: true,
       data_protection_officer: current_user,
-      organisation_name: @organisation.name,
-      organisation_address: @organisation.address_row,
-      organisation_phone_number: @organisation.phone,
-      dpo_email: current_user.email,
-      dpo_name: current_user.name,
     )
 
-    if data_sharing_agreement.save
+    if data_protection_confirmation.save
       flash[:notice] = "You have accepted the Data Sharing Agreement"
       flash[:notification_banner_body] = "Your organisation can now submit logs."
 
diff --git a/app/helpers/data_sharing_agreement_helper.rb b/app/helpers/data_sharing_agreement_helper.rb
index 365bb20cc..e3750a5fa 100644
--- a/app/helpers/data_sharing_agreement_helper.rb
+++ b/app/helpers/data_sharing_agreement_helper.rb
@@ -31,22 +31,22 @@ module DataSharingAgreementHelper
     end
   end
 
-  def org_name_for_data_sharing_agreement(data_sharing_agreement, user)
-    if data_sharing_agreement.present?
-      data_sharing_agreement.organisation_name
+  def org_name_for_data_sharing_agreement(data_protection_confirmation, user)
+    if data_protection_confirmation&.confirmed?
+      data_protection_confirmation.organisation.name
     else
       user.organisation.name
     end
   end
 
   # rubocop:disable Rails/HelperInstanceVariable
-  def section_12_2(data_sharing_agreement:, user:, organisation:)
-    if data_sharing_agreement
-      @org_address = data_sharing_agreement.organisation_address
-      @org_name = data_sharing_agreement.organisation_name
-      @org_phone = data_sharing_agreement.organisation_phone_number
-      @dpo_name = data_sharing_agreement.dpo_name
-      @dpo_email = data_sharing_agreement.dpo_email
+  def section_12_2(data_protection_confirmation:, user:, organisation:)
+    if data_protection_confirmation&.confirmed?
+      @org_address = data_protection_confirmation.organisation.address_row
+      @org_name = data_protection_confirmation.organisation.name
+      @org_phone = data_protection_confirmation.organisation.phone
+      @dpo_name = data_protection_confirmation.data_protection_officer.name
+      @dpo_email = data_protection_confirmation.data_protection_officer.email
     else
       @org_name = organisation.name
       @org_address = organisation.address_row
@@ -68,18 +68,18 @@ module DataSharingAgreementHelper
 private
 
   def data_sharing_agreement_first_line(organisation:, user:)
-    return "Not accepted" if organisation.data_sharing_agreement.blank?
+    return "Not accepted" if organisation.data_protection_confirmation&.confirmed?
 
     if user.support?
-      "Accepted #{organisation.data_sharing_agreement.signed_at.strftime('%d/%m/%Y')}"
+      "Accepted #{organisation.data_protection_confirmation.created_at.strftime('%d/%m/%Y')}"
     else
       "Accepted"
     end
   end
 
   def data_sharing_agreement_second_line(organisation:, user:)
-    if organisation.data_sharing_agreement.present?
-      organisation.data_sharing_agreement.dpo_name if user.support?
+    if organisation.data_protection_confirmation&.confirmed?
+      organisation.data_sharing_agreement.data_protection_officer.name if user.support?
     else
       "Data protection officer must sign" unless user.is_dpo?
     end
diff --git a/app/models/log.rb b/app/models/log.rb
index 3e37489b3..65f9c56e3 100644
--- a/app/models/log.rb
+++ b/app/models/log.rb
@@ -179,8 +179,9 @@ class Log < ApplicationRecord
 private
 
   def verify_dsa_signed
+    return unless FeatureToggle.new_data_protection_confirmation?
     return unless owning_organisation
-    return if owning_organisation.data_sharing_agreement.present?
+    return if owning_organisation.data_protection_confirmation&.confirmed?
 
     errors.add :owning_organisation, I18n.t("validations.organisation.data_sharing_agreement_not_signed")
   end
diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index cd2fd8c38..b11c07028 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -3,8 +3,7 @@ class Organisation < ApplicationRecord
   has_many :owned_lettings_logs, class_name: "LettingsLog", foreign_key: "owning_organisation_id", dependent: :delete_all
   has_many :managed_lettings_logs, class_name: "LettingsLog", foreign_key: "managing_organisation_id"
   has_many :owned_sales_logs, class_name: "SalesLog", foreign_key: "owning_organisation_id", dependent: :delete_all
-  has_many :data_protection_confirmations
-  has_one :data_sharing_agreement
+  has_one :data_protection_confirmation
   has_many :organisation_rent_periods
   has_many :owned_schemes, class_name: "Scheme", foreign_key: "owning_organisation_id", dependent: :delete_all
   has_many :parent_organisation_relationships, foreign_key: :child_organisation_id, class_name: "OrganisationRelationship"
@@ -103,7 +102,7 @@ class Organisation < ApplicationRecord
   end
 
   def display_organisation_attributes
-    [
+    attrs = [
       { name: "Name", value: name, editable: true },
       { name: "Address", value: address_string, editable: true },
       { name: "Telephone_number", value: phone, editable: true },
@@ -111,8 +110,13 @@ 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 },
-      { name: "Data protection agreement", value: data_protection_agreement_string, 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?
diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index f270f874b..edaa02fd7 100644
--- a/app/services/feature_toggle.rb
+++ b/app/services/feature_toggle.rb
@@ -42,7 +42,7 @@ class FeatureToggle
     !Rails.env.production?
   end
 
-  def self.new_data_sharing_agreement?
+  def self.new_data_protection_confirmation?
     !Rails.env.production?
   end
 end
diff --git a/app/views/logs/_create_for_org_actions.html.erb b/app/views/logs/_create_for_org_actions.html.erb
index 2320e7857..83cd1f27e 100644
--- a/app/views/logs/_create_for_org_actions.html.erb
+++ b/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_sharing_agreement? || @organisation.data_sharing_agreement.present? %>
+  <% if !FeatureToggle.new_data_protection_confirmation? || @organisation.data_protection_confirmation.&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 %>
diff --git a/app/views/organisations/data_sharing_agreement.html.erb b/app/views/organisations/data_sharing_agreement.html.erb
index 5109cbb2b..3c861f6c9 100644
--- a/app/views/organisations/data_sharing_agreement.html.erb
+++ b/app/views/organisations/data_sharing_agreement.html.erb
@@ -6,8 +6,8 @@
       <%= org_name_for_data_sharing_agreement(@data_sharing_agreement, current_user) %> and Department for Levelling Up, Housing and Communities
     </h2>
     <p class="govuk-body-m">
-      <% if @data_sharing_agreement %>
-        This agreement is made the <%= @data_sharing_agreement.signed_at.day.ordinalize %> day of <%= @data_sharing_agreement.signed_at.strftime("%B") %> <%= @data_sharing_agreement.signed_at.year %>
+      <% if @data_sharing_agreement&.confirmed? %>
+        This agreement is made the <%= @data_sharing_agreement.created_at.day.ordinalize %> day of <%= @data_sharing_agreement.created_at.strftime("%B") %> <%= @data_sharing_agreement.created_at.year %>
       <% elsif current_user.is_dpo? %>
         This agreement is made the <%= Time.zone.now.day.ordinalize %> day of <%= Time.zone.now.strftime("%B") %> <%= Time.zone.now.year %>
       <% else %>
@@ -134,7 +134,7 @@
       <li>Title: Deputy Director</li>
     </ul>
 
-    <% if current_user.is_dpo? && @organisation.data_sharing_agreement.nil? %>
+    <% if current_user.is_dpo? && !(@organisation.data_sharing_agreement&.confirmed?) %>
       <div class="govuk-button-group govuk-!-margin-top-9">
         <%= govuk_button_to("Accept this agreement", data_sharing_agreement_organisation_path(@organisation), method: :post) %>
         <%= govuk_button_link_to("Cancel", details_organisation_path(@organisation), secondary: true) %>
diff --git a/db/migrate/20230609101144_drop_data_sharing_adreement_table.rb b/db/migrate/20230609101144_drop_data_sharing_adreement_table.rb
new file mode 100644
index 000000000..25dfff66f
--- /dev/null
+++ b/db/migrate/20230609101144_drop_data_sharing_adreement_table.rb
@@ -0,0 +1,5 @@
+class DropDataSharingAdreementTable < ActiveRecord::Migration[7.0]
+  def change
+    drop_table :data_sharing_agreements # rubocop:disable Rails/ReversibleMigration
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index d01fa5935..8337c5305 100644
--- a/db/schema.rb
+++ b/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_05_30_094653) do
+ActiveRecord::Schema[7.0].define(version: 2023_06_09_101144) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -56,22 +56,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_30_094653) do
     t.index ["organisation_id"], name: "index_data_protection_confirmations_on_organisation_id"
   end
 
-  create_table "data_sharing_agreements", force: :cascade do |t|
-    t.bigint "organisation_id"
-    t.bigint "data_protection_officer_id"
-    t.datetime "signed_at", null: false
-    t.string "organisation_name", null: false
-    t.string "organisation_address", null: false
-    t.string "organisation_phone_number"
-    t.string "dpo_email", null: false
-    t.string "dpo_name", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-    t.index ["data_protection_officer_id"], name: "index_data_sharing_agreements_on_data_protection_officer_id"
-    t.index ["organisation_id", "data_protection_officer_id"], name: "data_sharing_agreements_unique", unique: true
-    t.index ["organisation_id"], name: "index_data_sharing_agreements_on_organisation_id"
-  end
-
   create_table "la_rent_ranges", force: :cascade do |t|
     t.integer "ranges_rent_id"
     t.integer "lettype"
diff --git a/db/seeds.rb b/db/seeds.rb
index ca712fa22..10352bcf2 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -8,15 +8,24 @@
 
 # rubocop:disable Rails/Output
 
+def create_dpo(org)
+  User.find_or_create_by!(
+    name: "#{org.name} User",
+    email: "#{org.name}@example.com",
+    organisation: standalone_owns_stock,
+    role: "data_provider",
+    is_dpo: true,
+  ) do |user|
+    user.password = "password"
+    user.confirmed_at = Time.zone.now
+  end
+end
+
 def create_dsa(org)
-  DataSharingAgreement.find_or_create_by!(
-    dpo_name: "DPO Name",
-    dpo_email: "dpo@example.com",
+  DataProtectionConfirmation.find_or_create_by!(
     organisation: org,
-    organisation_address: org.address_row,
-    organisation_phone_number: org.phone,
-    organisation_name: org.name,
-    signed_at: Time.zone.now,
+    confirmed: true,
+    data_protection_officer: create_dpo(org),
   )
 end
 
diff --git a/spec/factories/data_protection_confirmation.rb b/spec/factories/data_protection_confirmation.rb
index a6f42d350..704294d9e 100644
--- a/spec/factories/data_protection_confirmation.rb
+++ b/spec/factories/data_protection_confirmation.rb
@@ -1,7 +1,8 @@
 FactoryBot.define do
   factory :data_protection_confirmation do
-    organisation
-    data_protection_officer { FactoryBot.create(:user, :data_protection_officer) }
+    organisation { association :organisation, data_sharing_agreement: instance }
+    data_protection_officer { association :user, :data_protection_officer, organisation: (instance.organisation || organisation) }
+
     confirmed { true }
     old_org_id { "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618" }
     old_id { "7c5bd5fb549c09a2c55d7cb90d7ba84927e64618" }
diff --git a/spec/factories/data_sharing_agreement.rb b/spec/factories/data_sharing_agreement.rb
deleted file mode 100644
index d3aadcd8d..000000000
--- a/spec/factories/data_sharing_agreement.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-FactoryBot.define do
-  factory :data_sharing_agreement do
-    organisation { association :organisation, data_sharing_agreement: instance }
-
-    signed_at { Time.zone.now }
-    created_at { Time.zone.now }
-    updated_at { Time.zone.now }
-
-    dpo_name { data_protection_officer&.name || "DPO Name" }
-    dpo_email { data_protection_officer&.email || "test@example.com" }
-    organisation_address { organisation }
-    organisation_phone_number { organisation }
-    organisation_name { organisation }
-  end
-end
diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb
index 71693a3a6..20964c06c 100644
--- a/spec/factories/organisation.rb
+++ b/spec/factories/organisation.rb
@@ -19,11 +19,7 @@ FactoryBot.define do
         create(
           :data_sharing_agreement,
           organisation: org,
-          dpo_name: "DPO Name",
-          dpo_email: "test@email.com",
-          organisation_address: "address 123",
-          organisation_phone_number: "123456789",
-          organisation_name: "Organisation Name",
+          data_protection_officer: org.users.any? ? org.users.first : create(:user, :data_protection_officer, organisation: org),
         )
       end
     end
diff --git a/spec/shared/shared_log_examples.rb b/spec/shared/shared_log_examples.rb
index ed5810b34..96c5d8fc9 100644
--- a/spec/shared/shared_log_examples.rb
+++ b/spec/shared/shared_log_examples.rb
@@ -106,17 +106,38 @@ RSpec.shared_examples "shared log examples" do |log_type|
   end
 
   describe "#verify_dsa_signed" do
+    before do
+      allow(FeatureToggle).to receive(:new_data_sharing_agreement?).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_dsa))
 
-      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."])
+      expect(log).to be_valid
+    end
+  end
+
+  context "when flag enabled" do
+    before do
+      allow(FeatureToggle).to receive(:new_data_sharing_agreement?).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
@@ -124,6 +145,13 @@ RSpec.shared_examples "shared log examples" do |log_type|
 
       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_dsa))
+
+      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