From 182913000d3eec35a76d957108dd9ab5931364b6 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:11:53 +0000 Subject: [PATCH 1/5] CLDC-3004 Do not reset created by for merged orgs (#2064) * Do not reset created by for merged orgs * Do not reset created by for sales --- app/models/lettings_log.rb | 1 + app/models/sales_log.rb | 1 + spec/requests/form_controller_spec.rb | 83 +++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 5bd06c972..a38f1eba1 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -559,6 +559,7 @@ class LettingsLog < Log return unless updated_by&.support? return if owning_organisation.blank? || managing_organisation.blank? || created_by.blank? return if created_by&.organisation == managing_organisation || created_by&.organisation == owning_organisation + return if created_by&.organisation == owning_organisation.absorbing_organisation || created_by&.organisation == managing_organisation.absorbing_organisation update!(created_by: nil) end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index bf059cc88..4345b9694 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -318,6 +318,7 @@ class SalesLog < Log return unless updated_by&.support? return if owning_organisation.blank? || created_by.blank? return if created_by&.organisation == owning_organisation + return if created_by&.organisation == owning_organisation.absorbing_organisation update!(created_by: nil) end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index fbe3dbb1d..94e8aeb30 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -189,6 +189,62 @@ RSpec.describe FormController, type: :request do end end + context "when submitting a sales log with valid owning organisation" do + let(:sales_log) { create(:sales_log) } + let(:created_by) { managing_organisation.users.first } + let(:params) do + { + id: sales_log.id, + sales_log: { + page: "organisation", + owning_organisation_id: managing_organisation.id, + }, + } + end + + before do + sales_log.update!(owning_organisation: managing_organisation, created_by:) + sales_log.reload + end + + it "does not reset created by" do + post "/sales-logs/#{sales_log.id}/organisation", params: params + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/created-by") + follow_redirect! + sales_log.reload + expect(sales_log.created_by).to eq(created_by) + end + end + + context "when submitting a sales log with valid merged owning organisation" do + let(:sales_log) { create(:sales_log) } + let(:created_by) { managing_organisation.users.first } + let(:merged_organisation) { create(:organisation) } + let(:params) do + { + id: sales_log.id, + sales_log: { + page: "organisation", + owning_organisation_id: merged_organisation.id, + }, + } + end + + before do + merged_organisation.update!(merge_date: Time.zone.today, absorbing_organisation: managing_organisation) + sales_log.update!(owning_organisation: managing_organisation, created_by:) + sales_log.reload + end + + it "does not reset created by" do + post "/sales-logs/#{sales_log.id}/organisation", params: params + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/created-by") + follow_redirect! + sales_log.reload + expect(sales_log.created_by).to eq(created_by) + end + end + context "with valid managing organisation" do let(:params) do { @@ -214,6 +270,33 @@ RSpec.describe FormController, type: :request do end end + context "with valid absorbed managing organisation" do + let(:params) do + { + id: lettings_log.id, + lettings_log: { + page: "stock_owner", + owning_organisation_id: stock_owner.id, + }, + } + end + let(:merged_org) { create(:organisation) } + + before do + merged_org.update!(merge_date: Time.zone.today, absorbing_organisation: organisation) + lettings_log.update!(owning_organisation: merged_org, created_by: user, managing_organisation: merged_org) + lettings_log.reload + end + + it "does not reset created by" do + post "/lettings-logs/#{lettings_log.id}/stock-owner", params: params + expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation") + follow_redirect! + lettings_log.reload + expect(lettings_log.created_by).to eq(user) + end + end + context "with only adding the stock owner" do let(:params) do { From 403ad05e422f7de5ccaa5ecbfe06a2104242d968 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:32:36 +0000 Subject: [PATCH 2/5] Display merge date for managing organisations question (#2063) --- .../questions/managing_organisation.rb | 12 ++++++++-- .../questions/managing_organisation_spec.rb | 22 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index 2e955fd22..55a38ee71 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -35,13 +35,21 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question user.organisation.managing_agents + log.owning_organisation.managing_agents else user.organisation.managing_agents - end.pluck(:id, :name).to_h + end user.organisation.absorbed_organisations.each do |absorbed_org| opts[absorbed_org.id] = "#{absorbed_org.name} (inactive as of #{absorbed_org.merge_date.to_fs(:govuk_date)})" end - opts.merge(orgs) + orgs.each do |org| + opts[org.id] = if org.merge_date.present? + "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" + else + org.name + end + end + + opts end def displayed_answer_options(log, user) diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb index dcd142151..62a46bc1e 100644 --- a/spec/models/form/lettings/questions/managing_organisation_spec.rb +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -119,6 +119,28 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do end end + context "when org owns stock and has merged managing agents" do + let(:options) do + { + "" => "Select an option", + log.managing_organisation.id => "Managing org 1", + log_owning_org.id => "Owning org (Owning organisation)", + org_rel1.child_organisation.id => "Managing org 2 (inactive as of 2 August 2023)", + org_rel2.child_organisation.id => "Managing org 3 (inactive as of 2 August 2023)", + } + end + + before do + org_rel1.child_organisation.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: log_owning_org.id) + org_rel2.child_organisation.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: log_owning_org.id) + end + + it "shows current managing agent at top, followed by the current owning organisation (with hint), followed by the managing agents of the current owning organisation" do + log_owning_org.update!(holds_own_stock: true) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + context "when org does not own stock" do let(:options) do { From 259252ae37d2e073e2f0c9a8e3173c52391bc162 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 4 Dec 2023 16:57:46 +0000 Subject: [PATCH 3/5] CLDC-3011 Display scheme owner question for absorbing orgs (#2066) * Display scheme owner question for absorbing orgs * Add test --- app/helpers/schemes_helper.rb | 3 +- app/models/organisation.rb | 4 +++ app/views/schemes/details.html.erb | 2 +- app/views/schemes/edit_name.html.erb | 2 +- app/views/schemes/new.html.erb | 2 +- spec/requests/schemes_controller_spec.rb | 41 ++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 4 deletions(-) diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index aea892a18..a1ec640eb 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -38,7 +38,8 @@ module SchemesHelper all_orgs = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } user_org = [OpenStruct.new(id: current_user.organisation_id, name: current_user.organisation.name)] stock_owners = current_user.organisation.stock_owners.map { |org| OpenStruct.new(id: org.id, name: org.name) } - current_user.support? ? all_orgs : user_org + stock_owners + merged_organisations = current_user.organisation.absorbed_organisations.merged_during_open_collection_period.map { |org| OpenStruct.new(id: org.id, name: org.name) } + current_user.support? ? all_orgs : user_org + stock_owners + merged_organisations end def null_option diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 1e05bddab..077f0413e 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -146,4 +146,8 @@ class Organisation < ApplicationRecord absorbed_organisations.merged_during_open_collection_period.group_by(&:merge_date) end + + def has_recent_absorbed_organisations? + absorbed_organisations&.merged_during_open_collection_period.present? + end end diff --git a/app/views/schemes/details.html.erb b/app/views/schemes/details.html.erb index 3dc6208b1..9c0c333e2 100644 --- a/app/views/schemes/details.html.erb +++ b/app/views/schemes/details.html.erb @@ -44,7 +44,7 @@ :description, legend: { text: "Is this scheme registered under the Care Standards Act 2000?", size: "m" } %> - <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? %> + <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? && !current_user.organisation.has_recent_absorbed_organisations? %> <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> <% else %> <%= f.govuk_collection_select :owning_organisation_id, diff --git a/app/views/schemes/edit_name.html.erb b/app/views/schemes/edit_name.html.erb index 0c7c1091e..e908fd1b5 100644 --- a/app/views/schemes/edit_name.html.erb +++ b/app/views/schemes/edit_name.html.erb @@ -25,7 +25,7 @@ label: { text: "This scheme contains confidential information" } %> <% end %> - <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? %> + <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? && !current_user.organisation.has_recent_absorbed_organisations? %> <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> <% else %> <%= f.govuk_collection_select :owning_organisation_id, diff --git a/app/views/schemes/new.html.erb b/app/views/schemes/new.html.erb index af5c2089a..1be0f7f74 100644 --- a/app/views/schemes/new.html.erb +++ b/app/views/schemes/new.html.erb @@ -44,7 +44,7 @@ :description, legend: { text: "Is this scheme registered under the Care Standards Act 2000?", size: "m" } %> - <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? %> + <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? && !current_user.organisation.has_recent_absorbed_organisations? %> <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> <% else %> <%= f.govuk_collection_select :owning_organisation_id, diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 997526edb..3016c3f4b 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -2224,6 +2224,47 @@ RSpec.describe SchemesController, type: :request do end end + context "when there are no stock owners" do + before do + get "/schemes/#{scheme.id}/edit-name" + end + + context "and there are no absorbed organisations" do + it "does not include the owning organisation question" do + expect(response).to have_http_status(:ok) + expect(page).not_to have_content("Which organisation owns the housing stock for this scheme?") + end + end + + context "and there are organisations absorbed during an open collection period" do + let(:merged_organisation) { create(:organisation) } + + before do + merged_organisation.update!(absorbing_organisation: user.organisation, merge_date: Time.zone.today) + get "/schemes/#{scheme.id}/edit-name" + end + + it "includes the owning organisation question" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("Which organisation owns the housing stock for this scheme?") + end + end + + context "and there are no recently absorbed organisations" do + let(:merged_organisation) { create(:organisation) } + + before do + merged_organisation.update!(absorbing_organisation: user.organisation, merge_date: Time.zone.today - 2.years) + get "/schemes/#{scheme.id}/edit-name" + end + + it "does not include the owning organisation question" do + expect(response).to have_http_status(:ok) + expect(page).not_to have_content("Which organisation owns the housing stock for this scheme?") + end + end + end + context "when attempting to access secondary-client-group scheme page for another organisation" do before do get "/schemes/#{another_scheme.id}/edit-name" From 671862b80bd83c9d569f7afc7b41fc5e13aaa019 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 5 Dec 2023 08:06:40 +0000 Subject: [PATCH 4/5] CLDC-3005 Display labels and filters for merged orgs (#2065) * Display labels and filters for merged orgs * update test names --- .../lettings_log_summary_component.html.erb | 2 +- .../sales_log_summary_component.html.erb | 2 +- app/helpers/filters_helper.rb | 4 ++-- .../requests/lettings_logs_controller_spec.rb | 14 +++++++++++++ spec/requests/sales_logs_controller_spec.rb | 20 +++++++++++++++++++ 5 files changed, 38 insertions(+), 4 deletions(-) diff --git a/app/components/lettings_log_summary_component.html.erb b/app/components/lettings_log_summary_component.html.erb index b0bc9fed8..1845a155f 100644 --- a/app/components/lettings_log_summary_component.html.erb +++ b/app/components/lettings_log_summary_component.html.erb @@ -34,7 +34,7 @@ <% end %>

<% end %> - <% if current_user.support? || current_user.organisation.has_managing_agents? %> + <% if current_user.support? || current_user.organisation.has_managing_agents? || current_user.organisation.has_recent_absorbed_organisations? %>
<% if log.owning_organisation %>
diff --git a/app/components/sales_log_summary_component.html.erb b/app/components/sales_log_summary_component.html.erb index 6b3be98cb..10b021fb7 100644 --- a/app/components/sales_log_summary_component.html.erb +++ b/app/components/sales_log_summary_component.html.erb @@ -24,7 +24,7 @@ Sale completed <% end %>

- <% if current_user.support? || current_user.organisation.has_managing_agents? %> + <% if current_user.support? || current_user.organisation.has_managing_agents? || current_user.organisation.has_recent_absorbed_organisations? %>
<% if log.owning_organisation %>
diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 8c5f45db5..087314863 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -124,11 +124,11 @@ module FiltersHelper end def non_support_with_multiple_owning_orgs? - current_user.organisation.stock_owners.count > 1 && user_lettings_path? + current_user.organisation.stock_owners.count > 1 && user_lettings_path? || current_user.organisation.has_recent_absorbed_organisations? end def non_support_with_multiple_managing_orgs? - current_user.organisation.managing_agents.count > 1 && user_lettings_path? + current_user.organisation.managing_agents.count > 1 && user_lettings_path? || current_user.organisation.has_recent_absorbed_organisations? end def user_lettings_path? diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 7e6da9ce7..354e1a5da 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -277,6 +277,20 @@ RSpec.describe LettingsLogsController, type: :request do expect(page).not_to have_link "Delete logs" end end + + context "and organisation has absorbed organisations" do + let(:merged_organisation) { FactoryBot.create(:organisation) } + + before do + merged_organisation.update!(absorbing_organisation: organisation, merge_date: Time.zone.yesterday) + end + + it "shows organisation labels" do + get "/lettings-logs", headers:, params: {} + expect(page).to have_content("Owned by") + expect(page).to have_content("Managed by") + end + end end context "when the user is a customer support user" do diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 2b7f6e172..b7e63661f 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -559,6 +559,26 @@ RSpec.describe SalesLogsController, type: :request do sign_in user end + it "does not show organisation labels" do + get "/sales-logs", headers: headers, params: {} + expect(page).not_to have_content("Owned by") + expect(page).not_to have_content("Managed by") + end + + context "and organisation has absorbed organisations" do + let(:merged_organisation) { FactoryBot.create(:organisation) } + + before do + merged_organisation.update!(absorbing_organisation: organisation, merge_date: Time.zone.yesterday) + end + + it "shows organisation labels" do + get "/sales-logs", headers: headers, params: {} + expect(page).to have_content("Owned by") + expect(page).not_to have_content("Managed by") + end + end + it "does not have organisation columns" do get "/sales-logs", headers: headers, params: {} expect(page).not_to have_content("Owning organisation") From eb7f301974e30bb646ddd3a480c7dc621ffebc42 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 5 Dec 2023 13:05:31 +0000 Subject: [PATCH 5/5] CLDC-2071 Add intermediate rent error to BU (#2053) * Add intermediate rent error * Make sure field 12 as an error --- .../bulk_upload/lettings/year2023/row_parser.rb | 8 ++++++++ .../lettings/year2023/row_parser_spec.rb | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 511f4336b..585ec7c93 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -329,6 +329,14 @@ class BulkUpload::Lettings::Year2023::RowParser }, on: :after_log + validates :field_11, + presence: { + if: proc { renttype == :intermediate }, + message: I18n.t("validations.not_answered", question: "intermediate rent type"), + category: :setup, + }, + on: :after_log + validates :field_15, presence: { if: proc { supported_housing? && log_uses_old_scheme_id? }, diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index dc7dbeaea..86e45ca2d 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -809,6 +809,23 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end + context "when intermediate rent and field_11 (Which type of Intermediate Rent) is not given" do + let(:attributes) { { bulk_upload:, field_5: "9", field_11: nil } } + + it "adds error on field_11" do + expect(parser.errors[:field_5]).to be_present + expect(parser.errors[:field_11]).to eq(["You must answer intermediate rent type"]) + end + end + + context "when intermediate rent other and field_12 is not given" do + let(:attributes) { { bulk_upload:, field_5: "9", field_11: "3", field_12: nil } } + + it "adds error on field_12" do + expect(parser.errors[:field_12]).to eq(["You must answer product name"]) + end + end + context "when bulk upload is for general needs" do context "when general needs option selected" do let(:attributes) { { bulk_upload:, field_5: "1", field_4: "1" } }