CLDC-2324 assign data protection officer (#1710)

* Add mailer

* Instantiate logs only where needed

* Update banner to handle all possible permutations

* wip

* Send confirmation email to user

* Address PO review

app/components/data_protection_confirmation_banner_component.html.erb

@@ -1,16 +1,8 @@
 <% if display_banner? %>
   <%= govuk_notification_banner(title_text: "Important") do %>
     <p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content">
-      <%= I18n.t("validations.organisation.data_sharing_agreement_not_signed") %>
+      <%= header_text %>
     <p>
-    <% if user.is_dpo? %>
-      <%= govuk_link_to(
-        "Read the Data Sharing Agreement",
-        data_sharing_agreement_href,
-        class: "govuk-notification-banner__link govuk-!-font-weight-bold",
-      ) %>
-    <% else %>
-      <%= data_protection_officers_text %>
-    <% end %>
+    <%= banner_text %>
   <% end %>
 <% end %>

app/components/data_protection_confirmation_banner_component.rb

@@ -3,6 +3,8 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base
 
   attr_reader :user, :organisation
 
+  HELPDESK_URL = "https://digital.dclg.gov.uk/jira/servicedesk/customer/portal/4/group/21".freeze
+
   def initialize(user:, organisation: nil)
     @user = user
     @organisation = organisation
@@ -13,12 +15,34 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base
   def display_banner?
     return false unless FeatureToggle.new_data_protection_confirmation?
     return false if user.support? && organisation.blank?
+    return true if org_without_dpo?
+
+    !org_or_user_org.data_protection_confirmed?
+  end
+
+  def header_text
+    if org_without_dpo?
+      "To create logs your organisation must state a data protection officer. They must sign the Data Sharing Agreement."
+    elsif user.is_dpo?
+      "Your organisation must accept the Data Sharing Agreement before you can create any logs."
+    else
+      "Your data protection officer must accept the Data Sharing Agreement on CORE before you can create any logs."
+    end
+  end
 
-    !DataProtectionConfirmation.exists?(
-      organisation: org_or_user_org,
-      confirmed: true,
+  def banner_text
+    if org_without_dpo? || user.is_dpo?
+      govuk_link_to(
+        link_text,
+        link_href,
+        class: "govuk-notification-banner__link govuk-!-font-weight-bold",
       )
+    else
+      tag.p data_protection_officers_text
     end
+  end
+
+private
 
   def data_protection_officers_text
     if org_or_user_org.data_protection_officers.any?
@@ -26,13 +50,27 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base
     end
   end
 
-  def data_sharing_agreement_href
-    data_sharing_agreement_organisation_path(org_or_user_org)
+  def link_text
+    if dpo_required?
+      "Contact helpdesk to assign a data protection officer"
+    else
+      "Read the Data Sharing Agreement"
+    end
   end
 
-private
+  def link_href
+    dpo_required? ? HELPDESK_URL : data_sharing_agreement_organisation_path(org_or_user_org)
+  end
+
+  def dpo_required?
+    org_or_user_org.data_protection_officers.empty?
+  end
 
   def org_or_user_org
     organisation.presence || user.organisation
   end
+
+  def org_without_dpo?
+    org_or_user_org.data_protection_officers.empty?
+  end
 end

app/mailers/data_protection_confirmation_mailer.rb

@@ -0,0 +1,15 @@
+class DataProtectionConfirmationMailer < NotifyMailer
+  include Rails.application.routes.url_helpers
+  EMAIL_TEMPLATE_ID = "3dbf78fe-a2c8-4d65-aa19-e4d62695d4a9".freeze
+
+  def send_confirmation_email(user)
+    send_email(
+      user.email,
+      EMAIL_TEMPLATE_ID,
+      {
+        organisation_name: user.organisation.name,
+        link: data_sharing_agreement_organisation_url(user.organisation),
+      },
+    )
+  end
+end

app/models/user.rb

@@ -20,6 +20,9 @@ class User < ApplicationRecord
   validates :password, presence: { if: :password_required? }
   validates :password, confirmation: { if: :password_required? }
   validates :password, length: { within: Devise.password_length, allow_blank: true }
+
+  after_validation :send_data_protection_confirmation_reminder, if: :is_dpo_changed?
+
   validates :organisation_id, presence: true
 
   has_paper_trail ignore: %w[last_sign_in_at
@@ -179,4 +182,14 @@ protected
   def password_required?
     !persisted? || !password.nil? || !password_confirmation.nil?
   end
+
+private
+
+  def send_data_protection_confirmation_reminder
+    return unless FeatureToggle.new_data_protection_confirmation?
+    return unless persisted?
+    return unless is_dpo?
+
+    DataProtectionConfirmationMailer.send_confirmation_email(self).deliver_later
+  end
 end

spec/components/data_protection_confirmation_banner_component_spec.rb

@@ -1,6 +1,8 @@
 require "rails_helper"
 
 RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
+  include GovukComponent
+
   let(:component) { described_class.new(user:, organisation:) }
   let(:render) { render_inline(component) }
   let(:user) { create(:user) }
@@ -13,114 +15,74 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do
 
     it "does not display banner" do
       expect(component.display_banner?).to eq(false)
+      expect(render.content).to be_empty
     end
   end
 
-  context "when flag enabled" do
+  context "when flag enabled", :aggregate_failures do
     before do
       allow(FeatureToggle).to receive(:new_data_protection_confirmation?).and_return(true)
     end
 
-    describe "#data_protection_officers_text" do
-      it "returns the correct text" do
-        expect(component.data_protection_officers_text).to eq("You can ask: Danny Rojas")
-      end
-
-      context "with two DPOs" do
-        before do
-          create(:user, organisation:, is_dpo: true, name: "Test McTest")
-        end
-
-        it "returns the correct list of names, in alphabetical order)" do
-          expect(component.data_protection_officers_text).to eq("You can ask: Danny Rojas, Test McTest")
-        end
-      end
-    end
-
-    context "when user is not support and not dpo" do
-      let(:user) { create(:user) }
-
-      context "when org blank" do
+    context "when user is support and organisation is blank" do
+      let(:user) { create(:user, :support) }
       let(:organisation) { nil }
 
-        before do
-          allow(DataProtectionConfirmation).to receive(:exists?).and_call_original
-        end
-
-        context "when data sharing agreement present" do
       it "does not display banner" do
         expect(component.display_banner?).to eq(false)
-          end
-
-          it "verifies DSA exists for organisation" do
-            render
-            expect(DataProtectionConfirmation).to have_received(:exists?).with(organisation: user.organisation, confirmed: true)
+        expect(render.content).to be_empty
       end
     end
 
-        context "when data sharing agreement not present" do
-          let(:user) { create(:user, organisation: create(:organisation, :without_dpc)) }
+    context "when org does not have a DPO" do
+      before do
+        organisation.users.where(is_dpo: true).destroy_all
+      end
 
       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://digital.dclg.gov.uk/jira/servicedesk/customer/portal/4/group/21",
+        )
+        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
-
-          it "produces the correct link" do
-            render
-            expect(component.data_sharing_agreement_href).to eq("/organisations/#{user.organisation.id}/data-sharing-agreement")
     end
 
-          it "verifies DSA exists for organisation" do
-            render
-            expect(DataProtectionConfirmation).to have_received(:exists?).with(organisation: user.organisation, confirmed: true)
-          end
-        end
-      end
-    end
-
-    context "when user is support" do
-      let(:user) { create(:user, :support) }
-
-      context "when org blank" do
-        let(:organisation) { nil }
-
-        it "does not display banner" do
-          expect(component.display_banner?).to eq(false)
-        end
-      end
-
-      context "when org present" do
-        before do
-          allow(DataProtectionConfirmation).to receive(:exists?).and_call_original
-        end
-
-        context "when data sharing agreement present" do
-          it "does not display banner" do
-            expect(component.display_banner?).to eq(false)
-          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 "verifies DSA exists for organisation" do
-            render
-            expect(DataProtectionConfirmation).to have_received(:exists?).with(organisation:, confirmed: true)
+          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 data sharing agreement not present" do
+        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" do
+          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
-
-          it "produces the correct link" do
-            render
-            expect(component.data_sharing_agreement_href).to eq("/organisations/#{organisation.id}/data-sharing-agreement")
         end
-
-          it "verifies DSA exists for organisation" do
-            render
-            expect(DataProtectionConfirmation).to have_received(:exists?).with(organisation:, confirmed: true)
       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
         end
       end
     end

spec/mailers/data_protection_confirmation_mailer_spec.rb

@@ -0,0 +1,27 @@
+require "rails_helper"
+
+RSpec.describe DataProtectionConfirmationMailer do
+  describe "#send_csv_download_mail" do
+    let(:notify_client) { instance_double(Notifications::Client) }
+    let(:user) { create(:user, email: "user@example.com") }
+    let(:organisation) { user.organisation }
+
+    before do
+      allow(Notifications::Client).to receive(:new).and_return(notify_client)
+      allow(notify_client).to receive(:send_email).and_return(true)
+    end
+
+    it "sends confirmation email to user" do
+      expect(notify_client).to receive(:send_email).with(
+        email_address: user.email,
+        template_id: "3dbf78fe-a2c8-4d65-aa19-e4d62695d4a9",
+        personalisation: {
+          organisation_name: organisation.name,
+          link: "#{ENV['APP_HOST']}/organisations/#{organisation.id}/data-sharing-agreement",
+        },
+      )
+
+      described_class.new.send_confirmation_email(user)
+    end
+  end
+end

spec/models/user_spec.rb

@@ -4,6 +4,12 @@ RSpec.describe User, type: :model do
   describe "#new" do
     let(:user) { create(:user, old_user_id: "3") }
     let(:other_organisation) { create(:organisation) }
+
+    it "belongs to an organisation" do
+      expect(user.organisation).to be_a(Organisation)
+    end
+
+    describe "#owned_lettings_logs" do
       let!(:owned_lettings_log) do
         create(
           :lettings_log,
@@ -12,6 +18,13 @@ RSpec.describe User, type: :model do
           created_by: user,
         )
       end
+
+      it "has owned lettings logs through their organisation" do
+        expect(user.owned_lettings_logs.first).to eq(owned_lettings_log)
+      end
+    end
+
+    describe "#managed_lettings_logs" do
       let!(:managed_lettings_log) do
         create(
           :lettings_log,
@@ -20,21 +33,32 @@ RSpec.describe User, type: :model do
         )
       end
 
-    it "belongs to an organisation" do
-      expect(user.organisation).to be_a(Organisation)
+      it "has managed lettings logs through their organisation" do
+        expect(user.managed_lettings_logs.first).to eq(managed_lettings_log)
       end
-
-    it "has owned lettings logs through their organisation" do
-      expect(user.owned_lettings_logs.first).to eq(owned_lettings_log)
     end
 
-    it "has managed lettings logs through their organisation" do
-      expect(user.managed_lettings_logs.first).to eq(managed_lettings_log)
+    describe "#lettings_logs" do
+      let!(:owned_lettings_log) do
+        create(
+          :lettings_log,
+          :completed,
+          managing_organisation: other_organisation,
+          created_by: user,
+        )
+      end
+      let!(:managed_lettings_log) do
+        create(
+          :lettings_log,
+          created_by: user,
+          owning_organisation: other_organisation,
+        )
       end
 
       it "has lettings logs through their organisation" do
         expect(user.lettings_logs.to_a).to match_array([owned_lettings_log, managed_lettings_log])
       end
+    end
 
     it "has a role" do
       expect(user.role).to eq("data_provider")
@@ -136,6 +160,21 @@ RSpec.describe User, type: :model do
     context "when the user is a Customer Support person" do
       let(:user) { create(:user, :support) }
       let!(:other_orgs_log) { create(:lettings_log) }
+      let!(:owned_lettings_log) do
+        create(
+          :lettings_log,
+          :completed,
+          managing_organisation: other_organisation,
+          created_by: user,
+        )
+      end
+      let!(:managed_lettings_log) do
+        create(
+          :lettings_log,
+          created_by: user,
+          owning_organisation: other_organisation,
+        )
+      end
 
       it "has access to logs from all organisations" do
         expect(user.lettings_logs.to_a).to match_array([owned_lettings_log, managed_lettings_log, other_orgs_log])
@@ -330,4 +369,45 @@ RSpec.describe User, type: :model do
       end
     end
   end
+
+  describe "#send_data_protection_confirmation_reminder" do
+    context "when updating to dpo" do
+      let!(:user) { create(:user, is_dpo: false) }
+
+      it "sends the email" do
+        expect { user.update!(is_dpo: true) }.to enqueue_job(ActionMailer::MailDeliveryJob).with(
+          "DataProtectionConfirmationMailer",
+          "send_confirmation_email",
+          "deliver_now",
+          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
+      let!(:user) { create(:user, is_dpo: true) }
+
+      it "does not send the email" do
+        expect { user.update!(is_dpo: false) }.not_to enqueue_job(ActionMailer::MailDeliveryJob)
+      end
+    end
+
+    context "when updating something else" do
+      let!(:user) { create(:user) }
+
+      it "does not send the email" do
+        expect { user.update!(name: "foobar") }.not_to enqueue_job(ActionMailer::MailDeliveryJob)
+      end
+    end
+  end
 end