From 72cec310299263a0be504d9938ce042ce2d9f05a Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Thu, 15 Jan 2026 15:08:58 +0000 Subject: [PATCH 1/6] CLDC-4114: Silence DPO warning if DSA is signed also restructure redundant logic in the file --- ...rotection_confirmation_banner_component.rb | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/app/components/data_protection_confirmation_banner_component.rb b/app/components/data_protection_confirmation_banner_component.rb index 7ce852ae0..16921cf89 100644 --- a/app/components/data_protection_confirmation_banner_component.rb +++ b/app/components/data_protection_confirmation_banner_component.rb @@ -12,16 +12,16 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base def display_banner? return false if user.support? && organisation.blank? - return true if org_without_dpo? + return true if show_no_dpo_message? return false if !org_or_user_org.holds_own_stock? && org_or_user_org.stock_owners.empty? && org_or_user_org.absorbed_organisations.empty? - !org_or_user_org.data_protection_confirmed? || !org_or_user_org.organisation_or_stock_owner_signed_dsa_and_holds_own_stock? + !dsa_signed? || !org_or_user_org.organisation_or_stock_owner_signed_dsa_and_holds_own_stock? end def header_text - if org_without_dpo? + if show_no_dpo_message? "To create logs your organisation must state a data protection officer. They must sign the Data Sharing Agreement." - elsif !org_or_user_org.holds_own_stock? && org_or_user_org.data_protection_confirmed? + elsif show_no_stock_owner_message? "Your organisation does not own stock. To create logs your stock owner(s) must accept the Data Sharing Agreement on CORE." elsif user.is_dpo? "Your organisation must accept the Data Sharing Agreement before you can create any logs." @@ -31,7 +31,7 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base end def banner_text - if org_without_dpo? || user.is_dpo? || !org_or_user_org.holds_own_stock? + if show_no_dpo_message? || user.is_dpo? || !org_or_user_org.holds_own_stock? govuk_link_to( link_text, link_href, @@ -51,9 +51,9 @@ private end def link_text - if dpo_required? + if show_no_dpo_message? "Contact helpdesk to assign a data protection officer" - elsif !org_or_user_org.holds_own_stock? && org_or_user_org.data_protection_confirmed? + elsif show_no_stock_owner_message? "View or add stock owners" else "Read the Data Sharing Agreement" @@ -61,24 +61,32 @@ private end def link_href - if dpo_required? + if show_no_dpo_message? GlobalConstants::HELPDESK_URL - elsif !org_or_user_org.holds_own_stock? && org_or_user_org.data_protection_confirmed? + elsif show_no_stock_owner_message? stock_owners_organisation_path(org_or_user_org) else data_sharing_agreement_organisation_path(org_or_user_org) end end - def dpo_required? - org_or_user_org.data_protection_officers.empty? + def show_no_dpo_message? + # it is fine if an org has a DSA and the DPO has moved on + # CORE staff do this sometimes as a single DPO covers multiple 'orgs' that exist as branches of the same real world org + # so, they move the DPO to all the mini orgs and have the sign each DSA + # so the DSA being signed can silence this warning + org_or_user_org.data_protection_officers.empty? && !dsa_signed? end - def org_or_user_org - organisation.presence || user.organisation + def dsa_signed? + org_or_user_org.data_protection_confirmed? + end + + def show_no_stock_owner_message? + !org_or_user_org.holds_own_stock? && dsa_signed? end - def org_without_dpo? - org_or_user_org.data_protection_officers.empty? + def org_or_user_org + organisation.presence || user.organisation end end From 4185d9daf85fd804136427232bfddb225fcba051 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Thu, 15 Jan 2026 16:17:21 +0000 Subject: [PATCH 2/6] CLDC-4114: Update tests --- ...tion_confirmation_banner_component_spec.rb | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/spec/components/data_protection_confirmation_banner_component_spec.rb b/spec/components/data_protection_confirmation_banner_component_spec.rb index fb687f25c..587a5c2c0 100644 --- a/spec/components/data_protection_confirmation_banner_component_spec.rb +++ b/spec/components/data_protection_confirmation_banner_component_spec.rb @@ -23,13 +23,18 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component 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://mhclgdigital.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.") + context "when org does not have a signed data sharing agreement" do + let(:organisation) { create(:organisation, :without_dpc) } + let(:user) { create(:user, organisation:, with_dsa: false) } + + 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://mhclgdigital.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 end @@ -127,13 +132,9 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component 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://mhclgdigital.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.") + it "doesn't display the banner" do + expect(component.display_banner?).to eq(false) + expect(render.content).to be_empty end end From ecf2648002dda792aabf5fecc003c5ff47d5d339 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Fri, 30 Jan 2026 14:00:32 +0000 Subject: [PATCH 3/6] fixup! CLDC-4114: Silence DPO warning if DSA is signed Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com> --- app/components/data_protection_confirmation_banner_component.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/data_protection_confirmation_banner_component.rb b/app/components/data_protection_confirmation_banner_component.rb index 16921cf89..079b90b98 100644 --- a/app/components/data_protection_confirmation_banner_component.rb +++ b/app/components/data_protection_confirmation_banner_component.rb @@ -73,7 +73,7 @@ private def show_no_dpo_message? # it is fine if an org has a DSA and the DPO has moved on # CORE staff do this sometimes as a single DPO covers multiple 'orgs' that exist as branches of the same real world org - # so, they move the DPO to all the mini orgs and have the sign each DSA + # so, they move the DPO to all the mini orgs and have them sign each DSA # so the DSA being signed can silence this warning org_or_user_org.data_protection_officers.empty? && !dsa_signed? end From 15d56c0b6156435f52ec6c67d9ed2f44a19f634c Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Fri, 30 Jan 2026 14:14:50 +0000 Subject: [PATCH 4/6] CLDC-4114: Restructure data protection confirmation banner tests --- ...tion_confirmation_banner_component_spec.rb | 81 +------------------ 1 file changed, 1 insertion(+), 80 deletions(-) diff --git a/spec/components/data_protection_confirmation_banner_component_spec.rb b/spec/components/data_protection_confirmation_banner_component_spec.rb index 587a5c2c0..6dbb1e666 100644 --- a/spec/components/data_protection_confirmation_banner_component_spec.rb +++ b/spec/components/data_protection_confirmation_banner_component_spec.rb @@ -86,7 +86,7 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do end end - context "when org has a signed data sharing agremeent" do + context "when org has a signed data sharing agreemeent" do it "does not display banner" do expect(component.display_banner?).to eq(false) expect(render.content).to be_empty @@ -126,84 +126,5 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do end end end - - context "when org does not have a DPO" do - before do - organisation.users.where(is_dpo: true).destroy_all - end - - it "doesn't display the banner" do - expect(component.display_banner?).to eq(false) - expect(render.content).to be_empty - 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:, with_dsa: false) } - let!(:dpo) { create(:user, :data_protection_officer, organisation:, with_dsa: false) } - - 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 - - context "and has a parent organisation that owns stock and has signed DSA" do - before do - parent_organisation = create(:organisation, holds_own_stock: true) - create(:organisation_relationship, child_organisation: organisation, parent_organisation:) - end - - 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 - end - - context "when user is a DPO" do - let(:organisation) { create(:organisation, :without_dpc) } - let(:user) { create(:user, :data_protection_officer, organisation:, with_dsa: false) } - - 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 - - context "and has a parent organisation that owns stock and has signed DSA" do - before do - parent_organisation = create(:organisation, holds_own_stock: true) - create(:organisation_relationship, child_organisation: organisation, parent_organisation:) - end - - 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 - 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 end end From a336bd7c5d258a0bf561e7ba10bdb7999f3ebdf3 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Mon, 2 Feb 2026 15:15:58 +0000 Subject: [PATCH 5/6] fixup! CLDC-4114: Restructure data protection confirmation banner tests --- .../data_protection_confirmation_banner_component_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/components/data_protection_confirmation_banner_component_spec.rb b/spec/components/data_protection_confirmation_banner_component_spec.rb index 6dbb1e666..e5674c15d 100644 --- a/spec/components/data_protection_confirmation_banner_component_spec.rb +++ b/spec/components/data_protection_confirmation_banner_component_spec.rb @@ -36,6 +36,13 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do 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 does have a signed data sharing agreement" do + it "does not display banner" do + expect(component.display_banner?).to eq(false) + expect(render.content).to be_empty + end + end end context "when org has a DPO" do From c13318accb96bc2eec00a932d2d5dbf4d37b4681 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Wed, 11 Feb 2026 17:28:44 +0000 Subject: [PATCH 6/6] fixup! CLDC-4114: Restructure data protection confirmation banner tests fix typo Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com> --- .../data_protection_confirmation_banner_component_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/components/data_protection_confirmation_banner_component_spec.rb b/spec/components/data_protection_confirmation_banner_component_spec.rb index e5674c15d..36bd360d3 100644 --- a/spec/components/data_protection_confirmation_banner_component_spec.rb +++ b/spec/components/data_protection_confirmation_banner_component_spec.rb @@ -93,7 +93,7 @@ RSpec.describe DataProtectionConfirmationBannerComponent, type: :component do end end - context "when org has a signed data sharing agreemeent" do + context "when org has a signed data sharing agreement" do it "does not display banner" do expect(component.display_banner?).to eq(false) expect(render.content).to be_empty