diff --git a/app/components/data_sharing_agreement_banner_component.html.erb b/app/components/data_sharing_agreement_banner_component.html.erb index 849b04843..3b0a82450 100644 --- a/app/components/data_sharing_agreement_banner_component.html.erb +++ b/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 %> diff --git a/app/helpers/data_sharing_agreement_helper.rb b/app/helpers/data_sharing_agreement_helper.rb index d8949de4f..365bb20cc 100644 --- a/app/helpers/data_sharing_agreement_helper.rb +++ b/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 diff --git a/app/models/data_sharing_agreement.rb b/app/models/data_sharing_agreement.rb index 7b366c9fe..b791afc98 100644 --- a/app/models/data_sharing_agreement.rb +++ b/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 diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb index 78d59b33b..2b73ac874 100644 --- a/app/views/logs/_log_filters.html.erb +++ b/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": { diff --git a/db/seeds.rb b/db/seeds.rb index 400308d1c..ca712fa22 100644 --- a/db/seeds.rb +++ b/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", diff --git a/spec/db/seeds_spec.rb b/spec/db/seeds_spec.rb index fb41ccf36..316f04ba6 100644 --- a/spec/db/seeds_spec.rb +++ b/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 diff --git a/spec/factories/data_sharing_agreement.rb b/spec/factories/data_sharing_agreement.rb index 055c4b14b..d3aadcd8d 100644 --- a/spec/factories/data_sharing_agreement.rb +++ b/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 diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb index 5fc28055e..71693a3a6 100644 --- a/spec/factories/organisation.rb +++ b/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 diff --git a/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb b/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb index 430d6f459..4f5eaa125 100644 --- a/spec/views/organisations/data_sharing_agreement.html.erb_spec.rb +++ b/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 diff --git a/spec/views/organisations/show.html.erb_spec.rb b/spec/views/organisations/show.html.erb_spec.rb index ea392d76d..31f52bcf8 100644 --- a/spec/views/organisations/show.html.erb_spec.rb +++ b/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