From 362e913bd822f1f5e33c8bf8081296f3d91845cb Mon Sep 17 00:00:00 2001 From: Jack S Date: Mon, 19 Jun 2023 11:47:23 +0100 Subject: [PATCH] Update banner to handle all possible permutations --- ...ion_confirmation_banner_component.html.erb | 12 +- ...rotection_confirmation_banner_component.rb | 52 ++++++- app/models/user.rb | 11 ++ ...tion_confirmation_banner_component_spec.rb | 128 ++++++------------ spec/models/user_spec.rb | 1 - 5 files changed, 103 insertions(+), 101 deletions(-) diff --git a/app/components/data_protection_confirmation_banner_component.html.erb b/app/components/data_protection_confirmation_banner_component.html.erb index 8f75081a1..dfbd3bdf0 100644 --- a/app/components/data_protection_confirmation_banner_component.html.erb +++ b/app/components/data_protection_confirmation_banner_component.html.erb @@ -1,16 +1,8 @@ <% if display_banner? %> <%= govuk_notification_banner(title_text: "Important") do %>

- <%= I18n.t("validations.organisation.data_sharing_agreement_not_signed") %> + <%= header_text %>

- <% 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 %> + <%= second_row %> <% end %> <% end %> diff --git a/app/components/data_protection_confirmation_banner_component.rb b/app/components/data_protection_confirmation_banner_component.rb index b11c01f02..07b264b37 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 second_row + 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/models/user.rb b/app/models/user.rb index 7c86d8796..eda023519 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_save :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,12 @@ protected def password_required? !persisted? || !password.nil? || !password_confirmation.nil? end + +private + + def send_data_protection_confirmation_reminder + return unless is_dpo? + + DataProtectionConfirmationMailer.send_confirmation_email(user) + 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/models/user_spec.rb b/spec/models/user_spec.rb index a78515e3e..b5580989a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -176,7 +176,6 @@ RSpec.describe User, type: :model do ) 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]) end