Browse Source
* Remove bulk_upload_lettings_logs flag * Add spacing above actions in data sharing agreement * Add banner * Add model validation * fix log component * Fix factory * Put validation behind feature flag * start adapting to data protection confirmation * Make tests more generic * Add tests for display_organisation_attributes * Use data_protection_confirmed? method * Address code review * rubocop * Prevent access to bulk upload pages when DPC not signed * Address PO reviewpull/1693/head
Jack
2 years ago
committed by
GitHub
45 changed files with 934 additions and 334 deletions
@ -0,0 +1,10 @@ |
|||||||
|
<% if display_actions? %> |
||||||
|
<div class="govuk-button-group app-filter-toggle govuk-!-margin-bottom-6"> |
||||||
|
<% 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_link_to upload_button_copy, upload_button_href, secondary: true %> |
||||||
|
<% end %> |
||||||
|
</div> |
||||||
|
<% end %> |
||||||
@ -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_protection_confirmation? |
||||||
|
return true if user.support? |
||||||
|
|
||||||
|
user.organisation.data_protection_confirmed? |
||||||
|
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 |
||||||
@ -0,0 +1,16 @@ |
|||||||
|
<% if display_banner? %> |
||||||
|
<%= govuk_notification_banner(title_text: "Important") do %> |
||||||
|
<p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content"> |
||||||
|
<%= I18n.t("validations.organisation.data_sharing_agreement_not_signed") %> |
||||||
|
<p> |
||||||
|
<% if user.is_dpo? %> |
||||||
|
<%= govuk_link_to( |
||||||
|
"Read the Data Sharing Agreement", |
||||||
|
data_sharing_agreement_href, |
||||||
|
class: "govuk-notification-banner__link govuk-!-font-weight-bold", |
||||||
|
) %> |
||||||
|
<% else %> |
||||||
|
<%= data_protection_officers_text %> |
||||||
|
<% end %> |
||||||
|
<% end %> |
||||||
|
<% end %> |
||||||
@ -0,0 +1,38 @@ |
|||||||
|
class DataProtectionConfirmationBannerComponent < 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_protection_confirmation? |
||||||
|
return false if user.support? && organisation.blank? |
||||||
|
|
||||||
|
!DataProtectionConfirmation.exists?( |
||||||
|
organisation: org_or_user_org, |
||||||
|
confirmed: true, |
||||||
|
) |
||||||
|
end |
||||||
|
|
||||||
|
def data_protection_officers_text |
||||||
|
if org_or_user_org.data_protection_officers.any? |
||||||
|
"You can ask: #{org_or_user_org.data_protection_officers.map(&:name).join(', ')}" |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
def data_sharing_agreement_href |
||||||
|
data_sharing_agreement_organisation_path(org_or_user_org) |
||||||
|
end |
||||||
|
|
||||||
|
private |
||||||
|
|
||||||
|
def org_or_user_org |
||||||
|
organisation.presence || user.organisation |
||||||
|
end |
||||||
|
end |
||||||
@ -1,4 +0,0 @@ |
|||||||
class DataSharingAgreement < ApplicationRecord |
|
||||||
belongs_to :organisation |
|
||||||
belongs_to :data_protection_officer, class_name: "User" |
|
||||||
end |
|
||||||
@ -0,0 +1,10 @@ |
|||||||
|
<div class="govuk-button-group app-filter-toggle"> |
||||||
|
<% if !FeatureToggle.new_data_protection_confirmation? || @organisation.data_protection_confirmed? %> |
||||||
|
<% 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 %> |
||||||
|
</div> |
||||||
@ -0,0 +1,5 @@ |
|||||||
|
class DropDataSharingAdreementTable < ActiveRecord::Migration[7.0] |
||||||
|
def change |
||||||
|
drop_table :data_sharing_agreements # rubocop:disable Rails/ReversibleMigration |
||||||
|
end |
||||||
|
end |
||||||
@ -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_protection_confirmation?).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_protection_confirmation?).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_dpc)) } |
||||||
|
|
||||||
|
it "does not render actions" do |
||||||
|
expect(component).not_to be_display_actions |
||||||
|
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 |
||||||
@ -0,0 +1,128 @@ |
|||||||
|
require "rails_helper" |
||||||
|
|
||||||
|
RSpec.describe DataProtectionConfirmationBannerComponent, 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_protection_confirmation?).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_protection_confirmation?).and_return(true) |
||||||
|
end |
||||||
|
|
||||||
|
describe "#data_protection_officers_text" do |
||||||
|
it "returns the correct text" do |
||||||
|
expect(component.data_protection_officers_text).to eq("You can ask: Danny Rojas") |
||||||
|
end |
||||||
|
|
||||||
|
context "with two DPOs" do |
||||||
|
before do |
||||||
|
create(:user, organisation:, is_dpo: true, name: "Test McTest") |
||||||
|
end |
||||||
|
|
||||||
|
it "returns the correct text" do |
||||||
|
expect(component.data_protection_officers_text).to eq("You can ask: Danny Rojas, Test McTest") |
||||||
|
end |
||||||
|
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(DataProtectionConfirmation).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(DataProtectionConfirmation).to have_received(:exists?).with(organisation: user.organisation, confirmed: true) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when data sharing agreement not present" do |
||||||
|
let(:user) { create(:user, organisation: create(:organisation, :without_dpc)) } |
||||||
|
|
||||||
|
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(DataProtectionConfirmation).to have_received(:exists?).with(organisation: user.organisation, confirmed: true) |
||||||
|
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(DataProtectionConfirmation).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(DataProtectionConfirmation).to have_received(:exists?).with(organisation:, confirmed: true) |
||||||
|
end |
||||||
|
end |
||||||
|
|
||||||
|
context "when data sharing agreement not present" do |
||||||
|
let(:organisation) { create(:organisation, :without_dpc) } |
||||||
|
|
||||||
|
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(DataProtectionConfirmation).to have_received(:exists?).with(organisation:, confirmed: true) |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
|
end |
||||||
@ -1,15 +0,0 @@ |
|||||||
FactoryBot.define do |
|
||||||
factory :data_sharing_agreement do |
|
||||||
organisation |
|
||||||
data_protection_officer { create(:user, is_dpo: true) } |
|
||||||
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 } |
|
||||||
end |
|
||||||
end |
|
||||||
@ -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_protection_confirmation?).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_protection_confirmation?).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_dpc)) } |
||||||
|
|
||||||
|
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 |
||||||
Loading…
Reference in new issue