CLDC-4114: Silence DPO warning if DSA is signed (#3143)

* CLDC-4114: Silence DPO warning if DSA is signed

also restructure redundant logic in the file

* CLDC-4114: Update tests

* fixup! CLDC-4114: Silence DPO warning if DSA is signed

Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com>

* CLDC-4114: Restructure data protection confirmation banner tests

* fixup! CLDC-4114: Restructure data protection confirmation banner tests

* fixup! CLDC-4114: Restructure data protection confirmation banner tests

fix typo

Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com>

* CLDC-4114: Lint after merge

since rubocop updated

---------

Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com>

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 them 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

spec/components/data_protection_confirmation_banner_component_spec.rb

@@ -23,13 +23,25 @@ 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 be(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 be(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
+
+    context "when org does have a signed data sharing agreement" do
+      it "does not display banner" do
+        expect(component.display_banner?).to be(false)
+        expect(render.content).to be_empty
+      end
     end
   end
 
@@ -81,7 +93,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 agreement" do
       it "does not display banner" do
         expect(component.display_banner?).to be(false)
         expect(render.content).to be_empty
@@ -121,88 +133,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 "displays the banner" do
-        expect(component.display_banner?).to be(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
-
-    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 be(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 be(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 be(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 be(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 be(false)
-          expect(render.content).to be_empty
-        end
-      end
-    end
   end
 end