Fix factory

app/components/data_sharing_agreement_banner_component.html.erb

@@ -6,7 +6,7 @@
     <%= govuk_link_to(
       "Read the Data Sharing Agreement",
       data_sharing_agreement_href,
-      class: "govuk-notification-banner__link govuk-!-font-weight-bold"
+      class: "govuk-notification-banner__link govuk-!-font-weight-bold",
     ) %>
   <% end %>
 <% end %>

app/helpers/data_sharing_agreement_helper.rb

@@ -23,7 +23,7 @@ module DataSharingAgreementHelper
 
   def name_for_data_sharing_agreement(data_sharing_agreement, user)
     if data_sharing_agreement.present?
-      data_sharing_agreement.data_protection_officer.name
+      data_sharing_agreement.dpo_name
     elsif user.is_dpo?
       user.name
     else
@@ -79,7 +79,7 @@ private
 
   def data_sharing_agreement_second_line(organisation:, user:)
     if organisation.data_sharing_agreement.present?
-      organisation.data_sharing_agreement.data_protection_officer.name if user.support?
+      organisation.data_sharing_agreement.dpo_name if user.support?
     else
       "Data protection officer must sign" unless user.is_dpo?
     end

app/models/data_sharing_agreement.rb

@@ -1,4 +1,4 @@
 class DataSharingAgreement < ApplicationRecord
   belongs_to :organisation
-  belongs_to :data_protection_officer, class_name: "User"
+  belongs_to :data_protection_officer, class_name: "User", optional: true
 end

app/views/logs/_log_filters.html.erb

@@ -11,7 +11,7 @@
         <% if bulk_upload_options(@bulk_upload).present? %>
           <%= render partial: "filters/checkbox_filter",
                      locals: {
-                       f: f,
+                       f:,
                        options: bulk_upload_options(@bulk_upload),
                        label: "Bulk upload",
                        category: "bulk_upload_id",
@@ -21,7 +21,7 @@
         <% if bulk_upload_options(@bulk_upload).blank? %>
           <%= render partial: "filters/checkbox_filter",
                      locals: {
-                       f: f,
+                       f:,
                        options: collection_year_options,
                        label: "Collection year",
                        category: "years",
@@ -29,7 +29,7 @@
 
           <%= render partial: "filters/checkbox_filter",
                      locals: {
-                       f: f,
+                       f:,
                        options: status_filters,
                        label: "Status",
                        category: "status",
@@ -38,7 +38,7 @@
 
         <%= render partial: "filters/radio_filter",
                    locals: {
-                     f: f,
+                     f:,
                      options: all_or_yours,
                      label: "Logs",
                      category: "user",
@@ -46,7 +46,7 @@
 
         <% if (@current_user.support? || @current_user.organisation.has_managing_agents?) && request.path == "/lettings-logs" %>
           <%= render partial: "filters/radio_filter", locals: {
-            f: f,
+            f:,
             options: {
               "all": { label: "All" },
               "specific_org": {

db/seeds.rb

@@ -7,6 +7,19 @@
 #   Character.create(name: 'Luke', movie: movies.first)
 
 # rubocop:disable Rails/Output
+
+def create_dsa(org)
+  DataSharingAgreement.find_or_create_by!(
+    dpo_name: "DPO Name",
+    dpo_email: "dpo@example.com",
+    organisation: org,
+    organisation_address: org.address_row,
+    organisation_phone_number: org.phone,
+    organisation_name: org.name,
+    signed_at: Time.zone.now,
+  )
+end
+
 unless Rails.env.test?
   stock_owner1 = Organisation.find_or_create_by!(
     name: "Stock Owner 1",
@@ -18,6 +31,7 @@ unless Rails.env.test?
     managing_agents_label: "None",
     provider_type: "LA",
   )
+  create_dsa(stock_owner1)
   stock_owner2 = Organisation.find_or_create_by!(
     name: "Stock Owner 2",
     address_line1: "2 Marsham Street",
@@ -28,6 +42,7 @@ unless Rails.env.test?
     managing_agents_label: "None",
     provider_type: "LA",
   )
+  create_dsa(stock_owner2)
   managing_agent1 = Organisation.find_or_create_by!(
     name: "Managing Agent 1",
     address_line1: "2 Marsham Street",
@@ -38,6 +53,7 @@ unless Rails.env.test?
     managing_agents_label: "None",
     provider_type: "LA",
   )
+  create_dsa(managing_agent1)
   managing_agent2 = Organisation.find_or_create_by!(
     name: "Managing Agent 2",
     address_line1: "2 Marsham Street",
@@ -48,6 +64,7 @@ unless Rails.env.test?
     managing_agents_label: "None",
     provider_type: "LA",
   )
+  create_dsa(managing_agent2)
 
   org = Organisation.find_or_create_by!(
     name: "DLUHC",
@@ -66,6 +83,7 @@ unless Rails.env.test?
       Rails.logger.info info
     end
   end
+  create_dsa(org)
 
   standalone_owns_stock = Organisation.find_or_create_by!(
     name: "Standalone Owns Stock 1 Ltd",

spec/db/seeds_spec.rb

@@ -24,12 +24,12 @@ RSpec.describe "seeding process", type: task do
   it "sets up correct data" do
     expect {
       Rails.application.load_seed
-    }.to change(User, :count).by(7)
-     .and change(Organisation, :count).by(8)
-     .and change(OrganisationRelationship, :count).by(4)
-     .and change(Scheme, :count).by(3)
-     .and change(Location, :count).by(3)
-     .and change(LaRentRange, :count).by(22_850)
+    }.to change(User, :count)
+     .and change(Organisation, :count)
+     .and change(OrganisationRelationship, :count)
+     .and change(Scheme, :count)
+     .and change(Location, :count)
+     .and change(LaRentRange, :count)
   end
 
   it "is idempotent" do

spec/factories/data_sharing_agreement.rb

@@ -1,16 +1,15 @@
 FactoryBot.define do
   factory :data_sharing_agreement do
     organisation { association :organisation, data_sharing_agreement: instance }
-    data_protection_officer { association :user, :data_protection_officer, organisation: (instance.organisation || organisation) }
 
     signed_at { Time.zone.now }
     created_at { Time.zone.now }
     updated_at { Time.zone.now }
 
-    dpo_name { data_protection_officer.name }
-    dpo_email { data_protection_officer.email }
-    organisation_address { organisation.address_string }
-    organisation_phone_number { organisation.phone }
-    organisation_name { organisation.name }
+    dpo_name { data_protection_officer&.name || "DPO Name" }
+    dpo_email { data_protection_officer&.email || "test@example.com" }
+    organisation_address { organisation }
+    organisation_phone_number { organisation }
+    organisation_name { organisation }
   end
 end

spec/factories/organisation.rb

@@ -9,7 +9,24 @@ FactoryBot.define do
     created_at { Time.zone.now }
     updated_at { Time.zone.now }
     holds_own_stock { true }
-    association :data_sharing_agreement
+
+    transient do
+      with_dsa { true }
+    end
+
+    after(:create) do |org, evaluator|
+      if evaluator.with_dsa
+        create(
+          :data_sharing_agreement,
+          organisation: org,
+          dpo_name: "DPO Name",
+          dpo_email: "test@email.com",
+          organisation_address: "address 123",
+          organisation_phone_number: "123456789",
+          organisation_name: "Organisation Name",
+        )
+      end
+    end
 
     trait :with_old_visible_id do
       old_visible_id { rand(9_999_999).to_s }
@@ -24,6 +41,10 @@ FactoryBot.define do
     end
 
     trait :without_dsa do
+      transient do
+        with_dsa { false }
+      end
+
       data_sharing_agreement { nil }
     end
   end

spec/views/organisations/data_sharing_agreement.html.erb_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe "organisations/data_sharing_agreement.html.erb", :aggregate_failu
   let(:data_sharing_agreement) { nil }
 
   context "when dpo" do
-    let(:user) { create(:user, is_dpo: true) }
+    let(:user) { create(:user, is_dpo: true, organisation: create(:organisation, :without_dsa)) }
 
     it "renders dynamic content" do
       render

spec/views/organisations/show.html.erb_spec.rb

@@ -4,8 +4,7 @@ RSpec.describe "organisations/show.html.erb" do
   before do
     Timecop.freeze(Time.zone.local(2023, 1, 10))
     allow(view).to receive(:current_user).and_return(user)
-    assign(:organisation, organisation)
-    organisation.update!(data_sharing_agreement:)
+    assign(:organisation, user.organisation)
   end
 
   after do
@@ -13,11 +12,11 @@ RSpec.describe "organisations/show.html.erb" do
   end
 
   let(:fragment) { Capybara::Node::Simple.new(rendered) }
-  let(:organisation) { user.organisation }
-  let(:data_sharing_agreement) { nil }
+  let(:organisation_without_dsa) { create(:organisation, :without_dsa) }
+  let(:organisation_with_dsa) { create(:organisation) }
 
   context "when flag disabled" do
-    let(:user) { create(:user) }
+    let(:user) { create(:user, organisation: organisation_without_dsa) }
 
     before do
       allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(false)
@@ -31,7 +30,7 @@ RSpec.describe "organisations/show.html.erb" do
   end
 
   context "when dpo" do
-    let(:user) { create(:user, is_dpo: true) }
+    let(:user) { create(:user, is_dpo: true, organisation: organisation_without_dsa) }
 
     it "includes data sharing agreement row" do
       render
@@ -48,11 +47,11 @@ RSpec.describe "organisations/show.html.erb" do
     it "shows link to view data sharing agreement" do
       render
 
-      expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation.id}/data-sharing-agreement")
+      expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation_without_dsa.id}/data-sharing-agreement")
     end
 
     context "when accepted" do
-      let(:data_sharing_agreement) { create(:data_sharing_agreement, organisation:, signed_at: Time.zone.now - 1.day) }
+      let(:user) { create(:user, organisation: organisation_with_dsa) }
 
       it "includes data sharing agreement row" do
         render
@@ -69,13 +68,13 @@ RSpec.describe "organisations/show.html.erb" do
       it "shows link to view data sharing agreement" do
         render
 
-        expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation.id}/data-sharing-agreement")
+        expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation_with_dsa.id}/data-sharing-agreement")
       end
     end
   end
 
   context "when support user" do
-    let(:user) { create(:user, :support) }
+    let(:user) { create(:user, :support, organisation: organisation_without_dsa) }
 
     it "includes data sharing agreement row" do
       render
@@ -98,11 +97,11 @@ RSpec.describe "organisations/show.html.erb" do
     it "shows link to view data sharing agreement" do
       render
 
-      expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation.id}/data-sharing-agreement")
+      expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation_without_dsa.id}/data-sharing-agreement")
     end
 
     context "when accepted" do
-      let(:data_sharing_agreement) { create(:data_sharing_agreement, organisation:, signed_at: Time.zone.now - 1.day) }
+      let(:user) { create(:user, :support, organisation: organisation_with_dsa) }
 
       it "includes data sharing agreement row" do
         render
@@ -113,25 +112,25 @@ RSpec.describe "organisations/show.html.erb" do
       it "shows data sharing agreement accepted with date" do
         render
 
-        expect(fragment).to have_content("Accepted 09/01/2023")
+        expect(fragment).to have_content("Accepted 10/01/2023")
       end
 
       it "shows show name of who signed the agreement" do
         render
 
-        expect(fragment).to have_content(data_sharing_agreement.dpo_name)
+        expect(fragment).to have_content(user.organisation.data_sharing_agreement.dpo_name)
       end
 
       it "shows link to view data sharing agreement" do
         render
 
-        expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation.id}/data-sharing-agreement")
+        expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation_with_dsa.id}/data-sharing-agreement")
       end
     end
   end
 
   context "when not dpo" do
-    let(:user) { create(:user) }
+    let(:user) { create(:user, organisation: organisation_without_dsa) }
 
     it "includes data sharing agreement row" do
       render
@@ -151,13 +150,11 @@ RSpec.describe "organisations/show.html.erb" do
 
     it "shows link to view data sharing agreement" do
       render
-      expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation.id}/data-sharing-agreement")
+      expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation_without_dsa.id}/data-sharing-agreement")
     end
 
     context "when accepted" do
-      let(:data_sharing_agreement) do
-        create(:data_sharing_agreement, organisation:, signed_at: Time.zone.now - 1.day)
-      end
+      let(:user) { create(:user, organisation: organisation_with_dsa) }
 
       it "includes data sharing agreement row" do
         render
@@ -172,7 +169,7 @@ RSpec.describe "organisations/show.html.erb" do
 
       it "shows link to view data sharing agreement" do
         render
-        expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation.id}/data-sharing-agreement")
+        expect(fragment).to have_link(text: "View agreement", href: "/organisations/#{organisation_with_dsa.id}/data-sharing-agreement")
       end
     end
   end