From 9e1bac633597ecc90ba1cdccb52bab4dba6ca056 Mon Sep 17 00:00:00 2001 From: Jack S Date: Fri, 2 Jun 2023 16:16:40 +0100 Subject: [PATCH] Add banner --- .../create_log_actions_component.html.erb | 10 ++ .../create_log_actions_component.rb | 55 ++++++ ...haring_agreement_banner_component.html.erb | 12 ++ ...data_sharing_agreement_banner_component.rb | 28 +++ app/helpers/logs_helper.rb | 7 - app/views/layouts/application.html.erb | 2 +- .../logs/_create_for_org_actions.html.erb | 10 ++ ..._log_filters.erb => _log_filters.html.erb} | 0 app/views/logs/index.html.erb | 30 ++-- app/views/organisations/logs.html.erb | 16 +- .../create_log_actions_component_spec.rb | 170 ++++++++++++++++++ ...sharing_agreement_banner_component_spec.rb | 120 +++++++++++++ spec/factories/data_sharing_agreement.rb | 5 +- spec/factories/organisation.rb | 5 + .../_create_for_org_actions.html.erb_spec.rb | 50 ++++++ 15 files changed, 481 insertions(+), 39 deletions(-) create mode 100644 app/components/create_log_actions_component.html.erb create mode 100644 app/components/create_log_actions_component.rb create mode 100644 app/components/data_sharing_agreement_banner_component.html.erb create mode 100644 app/components/data_sharing_agreement_banner_component.rb create mode 100644 app/views/logs/_create_for_org_actions.html.erb rename app/views/logs/{_log_filters.erb => _log_filters.html.erb} (100%) create mode 100644 spec/components/create_log_actions_component_spec.rb create mode 100644 spec/components/data_sharing_agreement_banner_component_spec.rb create mode 100644 spec/views/logs/_create_for_org_actions.html.erb_spec.rb diff --git a/app/components/create_log_actions_component.html.erb b/app/components/create_log_actions_component.html.erb new file mode 100644 index 000000000..c92fc03b2 --- /dev/null +++ b/app/components/create_log_actions_component.html.erb @@ -0,0 +1,10 @@ +<% if display_actions? %> +
+ <% if create_button_href.present? %> + <%= govuk_button_to create_button_copy, create_button_href, class: "govuk-!-margin-right-6" %> + <% end %> + <% if upload_button_href.present? %> + <%= govuk_button_to upload_button_copy, upload_button_href, secondary: true %> + <% end %> +
+<% end %> diff --git a/app/components/create_log_actions_component.rb b/app/components/create_log_actions_component.rb new file mode 100644 index 000000000..9ae37d67a --- /dev/null +++ b/app/components/create_log_actions_component.rb @@ -0,0 +1,55 @@ +class CreateLogActionsComponent < ViewComponent::Base + include Rails.application.routes.url_helpers + + attr_reader :bulk_upload, :user, :log_type + + def initialize(user:, log_type:, bulk_upload: nil) + @bulk_upload = bulk_upload + @user = user + @log_type = log_type + + super + end + + def display_actions? + return false if bulk_upload.present? + return true unless FeatureToggle.new_data_sharing_agreement? + return true if user.support? + + user.organisation.data_sharing_agreement.present? + end + + def create_button_href + case log_type + when "lettings" + lettings_logs_path + when "sales" + sales_logs_path + end + end + + def create_button_copy + case log_type + when "lettings" + "Create a new lettings log" + when "sales" + "Create a new sales log" + end + end + + def upload_button_copy + if log_type == "lettings" + "Upload lettings logs in bulk" + elsif FeatureToggle.bulk_upload_sales_logs? && log_type == "sales" + "Upload sales logs in bulk" + end + end + + def upload_button_href + if log_type == "lettings" + bulk_upload_lettings_log_path(id: "start") + elsif FeatureToggle.bulk_upload_sales_logs? && log_type == "sales" + bulk_upload_sales_log_path(id: "start") + end + end +end diff --git a/app/components/data_sharing_agreement_banner_component.html.erb b/app/components/data_sharing_agreement_banner_component.html.erb new file mode 100644 index 000000000..40128d590 --- /dev/null +++ b/app/components/data_sharing_agreement_banner_component.html.erb @@ -0,0 +1,12 @@ +<% if display_banner? %> + <%= govuk_notification_banner(title_text: "Important") do %> +

+ Your organisation must accept the Data Sharing Agreement before you can create any logs. +

+ <%= govuk_link_to( + "Read the Data Sharing Agreement", + data_sharing_agreement_href, + class: "govuk-notification-banner__link govuk-!-font-weight-bold" + ) %> + <% end %> +<% end %> diff --git a/app/components/data_sharing_agreement_banner_component.rb b/app/components/data_sharing_agreement_banner_component.rb new file mode 100644 index 000000000..b47ae7fb3 --- /dev/null +++ b/app/components/data_sharing_agreement_banner_component.rb @@ -0,0 +1,28 @@ +class DataSharingAgreementBannerComponent < ViewComponent::Base + include Rails.application.routes.url_helpers + + attr_reader :user, :organisation + + def initialize(user:, organisation: nil) + @user = user + @organisation = organisation + + super + end + + def display_banner? + return false unless FeatureToggle.new_data_sharing_agreement? + return false if user.is_dpo? + return false if user.support? && organisation.blank? + + if organisation.present? + !DataSharingAgreement.exists?(organisation:) + else + !DataSharingAgreement.exists?(organisation: user.organisation) + end + end + + def data_sharing_agreement_href + data_sharing_agreement_organisation_path(organisation.presence || user.organisation) + end +end diff --git a/app/helpers/logs_helper.rb b/app/helpers/logs_helper.rb index ca6fa838f..becc2d213 100644 --- a/app/helpers/logs_helper.rb +++ b/app/helpers/logs_helper.rb @@ -8,13 +8,6 @@ module LogsHelper end end - def bulk_upload_path_for_controller(controller, id:) - case log_type_for_controller(controller) - when "lettings" then bulk_upload_lettings_log_path(id:) - when "sales" then bulk_upload_sales_log_path(id:) - end - end - def bulk_upload_options(bulk_upload) array = bulk_upload ? [bulk_upload.id] : [] array.index_with { |_bulk_upload_id| "With logs from bulk upload" } diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index d86a622f6..ead90793a 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -126,7 +126,7 @@ ) do |notification_banner| notification_banner.heading(text: flash.notice.html_safe) if flash[:notification_banner_body] - tag.p flash[:notification_banner_body] + tag.p flash[:notification_banner_body]&.html_safe end end %> <% end %> diff --git a/app/views/logs/_create_for_org_actions.html.erb b/app/views/logs/_create_for_org_actions.html.erb new file mode 100644 index 000000000..2320e7857 --- /dev/null +++ b/app/views/logs/_create_for_org_actions.html.erb @@ -0,0 +1,10 @@ +

+ <% if !FeatureToggle.new_data_sharing_agreement? || @organisation.data_sharing_agreement.present? %> + <% if current_page?(controller: 'organisations', action: 'lettings_logs') %> + <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post) %> + <% end %> + <% if current_page?(controller: 'organisations', action: 'sales_logs') %> + <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post) %> + <% end %> + <% end %> +
diff --git a/app/views/logs/_log_filters.erb b/app/views/logs/_log_filters.html.erb similarity index 100% rename from app/views/logs/_log_filters.erb rename to app/views/logs/_log_filters.html.erb diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb index 5b854e382..9f29f7244 100644 --- a/app/views/logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -3,6 +3,11 @@ <% content_for :title, title %> +<%= render DataSharingAgreementBannerComponent.new( + user: current_user, + organisation: @organisation, +) %> + <% if current_page?(controller: 'lettings_logs', action: 'index') %> <% if @unresolved_count > 0 %> <%= govuk_notification_banner( @@ -45,26 +50,11 @@ <% end %>
- <% unless @bulk_upload %> -
- <% if current_page?(controller: 'lettings_logs', action: 'index') %> - <%= govuk_button_to "Create a new lettings log", lettings_logs_path, class: "govuk-!-margin-right-6" %> - <% end %> - - <% if current_page?(controller: 'sales_logs', action: 'index') %> - <%= govuk_button_to "Create a new sales log", sales_logs_path, class: "govuk-!-margin-right-6" %> - <% end %> - - <% if log_type_for_controller(controller) == "lettings" %> - <%= govuk_button_link_to "Upload lettings logs in bulk", bulk_upload_path_for_controller(controller, id: "start"), secondary: true %> - <% end %> - - <% if FeatureToggle.bulk_upload_sales_logs? && log_type_for_controller(controller) == "sales" %> - <%= govuk_button_link_to "Upload sales logs in bulk", bulk_upload_path_for_controller(controller, id: "start"), secondary: true %> - <% end %> -
- <% end %> - + <%= render CreateLogActionsComponent.new( + bulk_upload: @bulk_upload, + user: current_user, + log_type: log_type_for_controller(controller), + ) %> <%= render partial: "log_filters" %>
diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb index 55c130c29..100aeba39 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -5,6 +5,11 @@ <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> +<%= render DataSharingAgreementBannerComponent.new( + user: current_user, + organisation: @organisation, +) %> + <% if current_user.support? %> <%= render SubNavigationComponent.new( items: secondary_items(request.path, @organisation.id), @@ -13,14 +18,7 @@ <% end %>
-
- <% if current_page?(controller: 'organisations', action: 'lettings_logs') %> - <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post) %> - <% end %> - <% if current_page?(controller: 'organisations', action: 'sales_logs') %> - <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post) %> - <% end %> -
+ <%= render partial: "logs/create_for_org_actions" %> <%= render partial: "logs/log_filters" %>
@@ -37,6 +35,6 @@ csv_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: false), csv_codes_only_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: true), } %> - <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %> + <%= render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/spec/components/create_log_actions_component_spec.rb b/spec/components/create_log_actions_component_spec.rb new file mode 100644 index 000000000..03d7fe937 --- /dev/null +++ b/spec/components/create_log_actions_component_spec.rb @@ -0,0 +1,170 @@ +require "rails_helper" + +RSpec.describe CreateLogActionsComponent, type: :component do + include GovukComponentsHelper + include GovukLinkHelper + let(:component) { described_class.new(user:, log_type:, bulk_upload:) } + let(:render) { render_inline(component) } + + let(:log_type) { "lettings" } + let(:user) { create(:user) } + + context "when bulk upload present" do + let(:bulk_upload) { true } + + it "does not render actions" do + expect(component.display_actions?).to eq(false) + end + end + + context "when bulk upload nil" do + let(:bulk_upload) { nil } + + context "when flag disabled" do + before do + allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(false) + end + + it "renders actions" do + expect(component.display_actions?).to eq(true) + end + + it "returns create button copy" do + expect(component.create_button_copy).to eq("Create a new lettings log") + end + + it "returns create button href" do + render + expect(component.create_button_href).to eq("/lettings-logs") + end + + it "returns upload button copy" do + expect(component.upload_button_copy).to eq("Upload lettings logs in bulk") + end + + it "returns upload button href" do + render + expect(component.upload_button_href).to eq("/lettings-logs/bulk-upload-logs/start") + end + + context "when sales log type" do + let(:log_type) { "sales" } + + it "renders actions" do + expect(component.display_actions?).to eq(true) + end + + it "returns create button copy" do + expect(component.create_button_copy).to eq("Create a new sales log") + end + + it "returns create button href" do + render + expect(component.create_button_href).to eq("/sales-logs") + end + end + end + + context "when flag enabled" do + before do + allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(true) + end + + context "when support user" do + let(:user) { create(:user, :support) } + + it "renders actions" do + expect(component.display_actions?).to eq(true) + end + + it "returns create button copy" do + expect(component.create_button_copy).to eq("Create a new lettings log") + end + + it "returns create button href" do + render + expect(component.create_button_href).to eq("/lettings-logs") + end + + it "returns upload button copy" do + expect(component.upload_button_copy).to eq("Upload lettings logs in bulk") + end + + it "returns upload button href" do + render + expect(component.upload_button_href).to eq("/lettings-logs/bulk-upload-logs/start") + end + + context "when sales log type" do + let(:log_type) { "sales" } + + it "renders actions" do + expect(component.display_actions?).to eq(true) + end + + it "returns create button copy" do + expect(component.create_button_copy).to eq("Create a new sales log") + end + + it "returns create button href" do + render + expect(component.create_button_href).to eq("/sales-logs") + end + end + end + + context "when not support user" do + context "without data sharing agreement" do + let(:user) { create(:user, organisation: create(:organisation, :without_dsa)) } + + it "does not render actions" do + expect(component.display_actions?).to eq(false) + end + end + + context "when has data sharing agremeent" do + let(:user) { create(:user, :support) } + + it "renders actions" do + expect(component.display_actions?).to eq(true) + end + + it "returns create button copy" do + expect(component.create_button_copy).to eq("Create a new lettings log") + end + + it "returns create button href" do + render + expect(component.create_button_href).to eq("/lettings-logs") + end + + it "returns upload button copy" do + expect(component.upload_button_copy).to eq("Upload lettings logs in bulk") + end + + it "returns upload button href" do + render + expect(component.upload_button_href).to eq("/lettings-logs/bulk-upload-logs/start") + end + + context "when sales log type" do + let(:log_type) { "sales" } + + it "renders actions" do + expect(component.display_actions?).to eq(true) + end + + it "returns create button copy" do + expect(component.create_button_copy).to eq("Create a new sales log") + end + + it "returns create button href" do + render + expect(component.create_button_href).to eq("/sales-logs") + end + end + end + end + end + end +end diff --git a/spec/components/data_sharing_agreement_banner_component_spec.rb b/spec/components/data_sharing_agreement_banner_component_spec.rb new file mode 100644 index 000000000..0827f0855 --- /dev/null +++ b/spec/components/data_sharing_agreement_banner_component_spec.rb @@ -0,0 +1,120 @@ +require "rails_helper" + +RSpec.describe DataSharingAgreementBannerComponent, type: :component do + let(:component) { described_class.new(user:, organisation:) } + let(:render) { render_inline(component) } + let(:user) { create(:user) } + let(:organisation) { user.organisation } + + context "when flag disabled" do + before do + allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(false) + end + + it "does not display banner" do + expect(component.display_banner?).to eq(false) + end + end + + context "when flag enabled" do + before do + allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(true) + end + + context "when user is dpo" do + let(:user) { create(:user, is_dpo: true) } + + it "does not display banner" do + expect(component.display_banner?).to eq(false) + end + end + + context "when user is not support and not dpo" do + let(:user) { create(:user) } + + context "when org blank" do + let(:organisation) { nil } + + before do + allow(DataSharingAgreement).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(DataSharingAgreement).to have_received(:exists?).with(organisation: user.organisation) + end + end + + context "when data sharing agreement not present" do + let(:user) { create(:user, organisation: create(:organisation, :without_dsa)) } + + 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/#{user.organisation.id}/data-sharing-agreement") + end + + it "verifies DSA exists for organisation" do + render + expect(DataSharingAgreement).to have_received(:exists?).with(organisation: user.organisation) + end + end + end + end + + context "when user is support" do + let(:user) { create(:user, :support) } + + context "when org blank" do + let(:organisation) { nil } + + it "does not display banner" do + expect(component.display_banner?).to eq(false) + end + end + + context "when org present" do + before do + allow(DataSharingAgreement).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(DataSharingAgreement).to have_received(:exists?).with(organisation:) + end + end + + context "when data sharing agreement not present" do + let(:organisation) { create(:organisation, :without_dsa) } + + 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(DataSharingAgreement).to have_received(:exists?).with(organisation:) + end + end + end + end + end +end diff --git a/spec/factories/data_sharing_agreement.rb b/spec/factories/data_sharing_agreement.rb index 11e9eef02..055c4b14b 100644 --- a/spec/factories/data_sharing_agreement.rb +++ b/spec/factories/data_sharing_agreement.rb @@ -1,7 +1,8 @@ FactoryBot.define do factory :data_sharing_agreement do - organisation - data_protection_officer { create(:user, is_dpo: true) } + 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 } diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb index fa2650eb5..5fc28055e 100644 --- a/spec/factories/organisation.rb +++ b/spec/factories/organisation.rb @@ -9,6 +9,7 @@ FactoryBot.define do created_at { Time.zone.now } updated_at { Time.zone.now } holds_own_stock { true } + association :data_sharing_agreement trait :with_old_visible_id do old_visible_id { rand(9_999_999).to_s } @@ -21,6 +22,10 @@ FactoryBot.define do trait :does_not_own_stock do holds_own_stock { false } end + + trait :without_dsa do + data_sharing_agreement { nil } + end end factory :organisation_rent_period do diff --git a/spec/views/logs/_create_for_org_actions.html.erb_spec.rb b/spec/views/logs/_create_for_org_actions.html.erb_spec.rb new file mode 100644 index 000000000..651994dd9 --- /dev/null +++ b/spec/views/logs/_create_for_org_actions.html.erb_spec.rb @@ -0,0 +1,50 @@ +require "rails_helper" + +RSpec.describe "logs/_create_for_org_actions.html.erb" do + before do + allow(view).to receive(:current_user).and_return(user) + allow(view).to receive(:current_page?).and_return(true) + assign(:organisation, user.organisation) + end + + let(:fragment) { Capybara::Node::Simple.new(rendered) } + + let(:user) { create(:user) } + + context "when flag disabled" do + before do + allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(false) + end + + it "shows create buttons" do + render + + expect(fragment).to have_button("Create a new lettings log for this organisation") + expect(fragment).to have_button("Create a new sales log for this organisation") + end + end + + context "when flag enabled" do + before do + allow(FeatureToggle).to receive(:new_data_sharing_agreement?).and_return(true) + end + + context "with data sharing agreement" do + it "does include create log buttons" do + render + expect(fragment).to have_button("Create a new lettings log for this organisation") + expect(fragment).to have_button("Create a new sales log for this organisation") + end + end + + context "without data sharing agreement" do + let(:user) { create(:user, organisation: create(:organisation, :without_dsa)) } + + it "does not include create log buttons" do + render + expect(fragment).not_to have_button("Create a new lettings log for this organisation") + expect(fragment).not_to have_button("Create a new sales log for this organisation") + end + end + end +end