- <% 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 %> diff --git a/app/components/data_protection_confirmation_banner_component.rb b/app/components/data_protection_confirmation_banner_component.rb index b11c01f02..cbb8a501e 100644 --- a/app/components/data_protection_confirmation_banner_component.rb +++ b/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,26 +15,62 @@ 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? "You can ask: #{org_or_user_org.data_protection_officers.map(&:name).sort_by(&:downcase).join(', ')}" 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 diff --git a/app/mailers/data_protection_confirmation_mailer.rb b/app/mailers/data_protection_confirmation_mailer.rb new file mode 100644 index 000000000..eaa8fde9a --- /dev/null +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 7c86d8796..c9ef7cd02 100644 --- a/app/models/user.rb +++ b/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 diff --git a/spec/components/data_protection_confirmation_banner_component_spec.rb b/spec/components/data_protection_confirmation_banner_component_spec.rb index 792e3a5a3..decc44b30 100644 --- a/spec/components/data_protection_confirmation_banner_component_spec.rb +++ b/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 + context "when user is support and organisation is blank" do + let(:user) { create(:user, :support) } + let(:organisation) { nil } - 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 + it "does not display banner" do + expect(component.display_banner?).to eq(false) + expect(render.content).to be_empty end end - context "when user is not support and not dpo" do - let(:user) { create(:user) } - - context "when org blank" do - let(:organisation) { nil } + context "when org does not have a DPO" do + before do + organisation.users.where(is_dpo: true).destroy_all + end - before do - allow(DataProtectionConfirmation).to receive(:exists?).and_call_original - 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 + 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: user.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 - let(:user) { create(:user, organisation: create(:organisation, :without_dpc)) } + 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) - 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) + 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 - end - - context "when user is support" do - let(:user) { create(:user, :support) } - - context "when org blank" do - let(:organisation) { nil } + context "when org has a signed data sharing agremeent" do 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 - - it "verifies DSA exists for organisation" do - render - expect(DataProtectionConfirmation).to have_received(:exists?).with(organisation:, confirmed: true) - end - end - - context "when data sharing agreement not present" do - let(:organisation) { create(:organisation, :without_dpc) } - - it "displays the banner" do - expect(component.display_banner?).to eq(true) - 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 + expect(render.content).to be_empty end end end diff --git a/spec/mailers/data_protection_confirmation_mailer_spec.rb b/spec/mailers/data_protection_confirmation_mailer_spec.rb new file mode 100644 index 000000000..57e62f361 --- /dev/null +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index edc065a4f..69c5f8a7e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4,36 +4,60 @@ RSpec.describe User, type: :model do describe "#new" do let(:user) { create(:user, old_user_id: "3") } let(:other_organisation) { create(:organisation) } - 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 "belongs to an organisation" do expect(user.organisation).to be_a(Organisation) end - it "has owned lettings logs through their organisation" do - expect(user.owned_lettings_logs.first).to eq(owned_lettings_log) + describe "#owned_lettings_logs" do + let!(:owned_lettings_log) do + create( + :lettings_log, + :completed, + managing_organisation: other_organisation, + 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 - it "has managed lettings logs through their organisation" do - expect(user.managed_lettings_logs.first).to eq(managed_lettings_log) + describe "#managed_lettings_logs" do + let!(:managed_lettings_log) do + create( + :lettings_log, + created_by: user, + owning_organisation: other_organisation, + ) + end + + it "has managed lettings logs through their organisation" do + expect(user.managed_lettings_logs.first).to eq(managed_lettings_log) + end end - it "has lettings logs through their organisation" do - expect(user.lettings_logs.to_a).to match_array([owned_lettings_log, 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 @@ -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