From d3cd29e3241bafe8065f926cb31226eb50a90324 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 25 Nov 2024 13:59:38 +0000 Subject: [PATCH 1/6] Remove wrong copy key (#2810) --- app/models/form/lettings/subsections/household_needs.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/form/lettings/subsections/household_needs.rb b/app/models/form/lettings/subsections/household_needs.rb index 3bfbbb336..2f6900f4f 100644 --- a/app/models/form/lettings/subsections/household_needs.rb +++ b/app/models/form/lettings/subsections/household_needs.rb @@ -2,7 +2,6 @@ class Form::Lettings::Subsections::HouseholdNeeds < ::Form::Subsection def initialize(id, hsh, section) super @id = "household_needs" - @copy_key = "lettings.household_needs.housingneeds_type" @label = "Household needs" @depends_on = [{ "non_location_setup_questions_completed?" => true }] end From b0500a297d6aa054a97f385237e9f42f3a381115 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:08:39 +0000 Subject: [PATCH 2/6] CLDC-3728 Add more merge request validations (#2814) * Add more_than_year_from_today merge date validation * Add a part of another merge validation * Disable merging if merge day is in the future --- app/controllers/merge_requests_controller.rb | 1 + app/helpers/merge_requests_helper.rb | 4 +++ app/models/merge_request_organisation.rb | 7 ++++ .../_notification_banners.html.erb | 8 +++++ app/views/merge_requests/show.html.erb | 2 +- config/locales/en.yml | 4 +++ .../merge_requests_controller_spec.rb | 34 +++++++++++++++++++ 7 files changed, 59 insertions(+), 1 deletion(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index a21d42bbb..a6e2c08e5 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -143,6 +143,7 @@ private if [day, month, year].none?(&:blank?) && Date.valid_date?(year.to_i, month.to_i, day.to_i) merge_request_params["merge_date"] = Time.zone.local(year.to_i, month.to_i, day.to_i) + @merge_request.errors.add(:merge_date, :more_than_year_from_today) if Time.zone.local(year.to_i, month.to_i, day.to_i) - 1.year > Time.zone.today else @merge_request.errors.add(:merge_date, :invalid) end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 6283ef42e..28c693935 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -276,4 +276,8 @@ module MergeRequestsHelper def any_organisations_share_logs?(organisations, type) organisations.any? { |organisation| organisation.send("#{type}_logs").filter_by_managing_organisation(organisations.where.not(id: organisation.id)).exists? } end + + def begin_merge_disabled?(merge_request) + merge_request.status != "ready_to_merge" || merge_request.merge_date.future? + end end diff --git a/app/models/merge_request_organisation.rb b/app/models/merge_request_organisation.rb index 6dda8b35e..5bfbe14d7 100644 --- a/app/models/merge_request_organisation.rb +++ b/app/models/merge_request_organisation.rb @@ -29,5 +29,12 @@ private if merging_organisation_id.blank? || !Organisation.where(id: merging_organisation_id).exists? merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_not_selected")) end + + existing_merges = MergeRequestOrganisation.with_merging_organisation(merging_organisation) + if existing_merges.count.positive? + existing_merge_request = existing_merges.first.merge_request + errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge")) + merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_incomplete_merge", organisation: merging_organisation.name, absorbing_organisation: existing_merge_request.absorbing_organisation&.name, merge_date: existing_merge_request.merge_date&.to_fs(:govuk_date))) + end end end diff --git a/app/views/merge_requests/_notification_banners.html.erb b/app/views/merge_requests/_notification_banners.html.erb index 38c05dbcd..9e6a085ca 100644 --- a/app/views/merge_requests/_notification_banners.html.erb +++ b/app/views/merge_requests/_notification_banners.html.erb @@ -19,3 +19,11 @@ No changes have been made. Try beginning the merge again. <% end %> <% end %> + +<% if @merge_request.merge_date&.future? %> + <%= govuk_notification_banner(title_text: "Important") do %> +

+ This merge is happening in the future. Wait until the merge date to begin this merge. +

+ <% end %> +<% end %> diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb index 0fbde7621..040cd7704 100644 --- a/app/views/merge_requests/show.html.erb +++ b/app/views/merge_requests/show.html.erb @@ -12,7 +12,7 @@ <% unless @merge_request.status == "request_merged" || @merge_request.status == "processing" %>
- <%= govuk_button_link_to "Begin merge", merge_start_confirmation_merge_request_path(@merge_request), disabled: @merge_request.status != "ready_to_merge" %> + <%= govuk_button_link_to "Begin merge", merge_start_confirmation_merge_request_path(@merge_request), disabled: begin_merge_disabled?(@merge_request) %> <%= govuk_button_link_to "Delete merge request", delete_confirmation_merge_request_path(@merge_request), warning: true %>
<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index f8bb8255b..698618717 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -183,8 +183,11 @@ en: merge_date: blank: "Enter a merge date." invalid: "Enter a valid merge date." + more_than_year_from_today: "The merge date must not be later than a year from today’s date." existing_absorbing_organisation: blank: "You must answer absorbing organisation already active?" + merging_organisation_id: + part_of_another_merge: "Another merge request records %{organisation} as merging into %{absorbing_organisation} on %{merge_date}. Select another organisation or remove this organisation from the other merge request." notification: attributes: title: @@ -370,6 +373,7 @@ en: during_deactivated_period: "The location is already deactivated during this date, please enter a different date." merge_request: organisation_part_of_another_merge: "This organisation is part of another merge - select a different one." + organisation_part_of_another_incomplete_merge: "Another merge request records %{organisation} as merging into %{absorbing_organisation} on %{merge_date}. Select another organisation or remove this organisation from the other merge request." organisation_not_selected: "Select an organisation from the search list." soft_validations: diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index dc1dd817d..a73db8067 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -84,6 +84,22 @@ RSpec.describe MergeRequestsController, type: :request do end end + context "when the user updates merge request with organisation that is already part of another merge" do + let(:another_organisation) { create(:organisation) } + let(:other_merge_request) { create(:merge_request, merge_date: Time.zone.local(2022, 5, 4)) } + let(:params) { { merge_request: { merging_organisation: another_organisation.id, new_merging_org_ids: [] } } } + + before do + MergeRequestOrganisation.create!(merge_request_id: other_merge_request.id, merging_organisation_id: another_organisation.id) + patch "/merge-request/#{merge_request.id}/merging-organisations", headers:, params: + end + + it "displays the page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Another merge request records #{another_organisation.name} as merging into #{other_merge_request.absorbing_organisation&.name} on 4 May 2022. Select another organisation or remove this organisation from the other merge request.") + end + end + context "when the user selects an organisation that is a part of another merge" do let(:another_organisation) { create(:organisation) } let(:params) { { merge_request: { merging_organisation: another_organisation.id, new_merging_org_ids: [] } } } @@ -396,6 +412,24 @@ RSpec.describe MergeRequestsController, type: :request do }.from(nil).to(Time.zone.local(2022, 4, 10)) end end + + context "when merge date set to a date more than 1 year in the future" do + let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } + let(:params) do + { merge_request: { page: "merge_date", "merge_date(3i)": (Time.zone.now.day + 1).to_s, "merge_date(2i)": Time.zone.now.month.to_s, "merge_date(1i)": (Time.zone.now.year + 1).to_s } } + end + + let(:request) do + patch "/merge-request/#{merge_request.id}", headers:, params: + end + + it "displays the page with an error message" do + request + + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("The merge date must not be later than a year from today’s date.") + end + end end describe "from merging_organisations page" do From eb995e75edf7eb2f857f7e7c62589ed34542eae5 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:08:57 +0000 Subject: [PATCH 3/6] Remove outdated copy (#2815) --- app/controllers/schemes_controller.rb | 2 +- spec/features/schemes_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 3df38a237..7f246b1e2 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -152,7 +152,7 @@ class SchemesController < ApplicationController flash[:notice] = if scheme_previously_confirmed "#{@scheme.service_name} has been updated." else - "#{@scheme.service_name} has been created. It does not require helpdesk approval." + "#{@scheme.service_name} has been created." end redirect_to scheme_path(@scheme) end diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 33ab00b34..7c7d9f3fb 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -677,7 +677,7 @@ RSpec.describe "Schemes scheme Features" do end it "adds scheme to the list of schemes" do - expect(page).to have_content "#{scheme.service_name} has been created. It does not require helpdesk approval." + expect(page).to have_content "#{scheme.service_name} has been created." click_link "Schemes" expect(page).to have_content "Supported housing schemes" expect(page).to have_content scheme.id_to_display From 4d14d4bbcf956ba086fdb864f195cf177058f1ef Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Tue, 26 Nov 2024 11:36:55 +0000 Subject: [PATCH 4/6] CLDC-3768: Call process_postcode_changes when setting postcode_full from postcode_full_input (#2803) --- app/models/log.rb | 1 + spec/models/lettings_log_spec.rb | 12 ++++++++++++ spec/models/sales_log_spec.rb | 12 ++++++++++++ 3 files changed, 25 insertions(+) diff --git a/app/models/log.rb b/app/models/log.rb index aed375117..dd4301550 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -111,6 +111,7 @@ class Log < ApplicationRecord self.town_or_city = nil self.county = nil self.postcode_full = postcode_full_input + process_postcode_changes! else self.uprn = uprn_selection self.uprn_confirmed = 1 diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 5702e65c8..11e663469 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -2021,5 +2021,17 @@ RSpec.describe LettingsLog do end end end + + describe "#process_address_change!" do + context "when uprn_selection is uprn_not_listed" do + let(:log) { build(:lettings_log, uprn_selection: "uprn_not_listed", address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") } + + it "sets log address fields, including postcode known" do + expect { log.process_address_change! }.to change(log, :address_line1).from(nil).to("Address line 1") + .and change(log, :postcode_full).from(nil).to("AA1 1AA") + .and change(log, :postcode_known).from(nil).to(1) + end + end + end end # rubocop:enable RSpec/MessageChain diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index f3dea90f9..9fe5a02a9 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -995,5 +995,17 @@ RSpec.describe SalesLog, type: :model do expect(sales_log.reload.is_la_inferred).to eq(false) end end + + describe "#process_address_change!" do + context "when uprn_selection is uprn_not_listed" do + let(:log) { build(:sales_log, uprn_selection: "uprn_not_listed", address_line1_input: "Address line 1", postcode_full_input: "AA1 1AA") } + + it "sets log address fields, including postcode known" do + expect { log.process_address_change! }.to change(log, :address_line1).from(nil).to("Address line 1") + .and change(log, :postcode_full).from(nil).to("AA1 1AA") + .and change(log, :pcodenk).from(nil).to(0) + end + end + end end # rubocop:enable RSpec/MessageChain From 0899ed43d6e7fff1599e412f9c7183b09c6ae701 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Tue, 26 Nov 2024 16:13:12 +0000 Subject: [PATCH 5/6] CLDC-3743: Allow coordinators to manage schemes for recently absorbed organisations (#2764) * CLDC-3743: Allow coordinators to manage schemes for recently absorbed organisations * Fix status calculation for locations * Avoid errors with merged stock owners * Adjust when to hide org field in scheme creation * Don't show activation buttons for schemes at merged orgs * Update hint message about created locations, and restrict location creation to relevant org statuses * Remove extra whitespace * Fix references to scheme organisation --- app/controllers/organisations_controller.rb | 2 +- app/controllers/schemes_controller.rb | 4 + app/helpers/filters_helper.rb | 8 +- app/helpers/schemes_helper.rb | 20 +++-- app/models/location.rb | 8 +- app/models/scheme.rb | 8 +- app/policies/location_policy.rb | 15 ++-- app/policies/scheme_policy.rb | 15 ++-- app/views/locations/index.html.erb | 15 ++-- app/views/locations/show.html.erb | 2 +- app/views/schemes/details.html.erb | 6 +- app/views/schemes/show.html.erb | 2 +- spec/models/location_spec.rb | 5 ++ spec/models/scheme_spec.rb | 5 ++ spec/requests/schemes_controller_spec.rb | 86 ++++++++++++++++++++- 15 files changed, 160 insertions(+), 41 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 61cd43674..8ffe426d7 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -24,7 +24,7 @@ class OrganisationsController < ApplicationController end def schemes - organisation_schemes = Scheme.visible.where(owning_organisation: [@organisation] + @organisation.parent_organisations) + organisation_schemes = Scheme.visible.where(owning_organisation: [@organisation] + @organisation.parent_organisations + @organisation.absorbed_organisations.visible.merged_during_open_collection_period) @pagy, @schemes = pagy(filter_manager.filtered_schemes(organisation_schemes, search_term, session_filters)) @searched = search_term.presence diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 7f246b1e2..3dc642345 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -118,6 +118,10 @@ class SchemesController < ApplicationController validation_errors scheme_params if @scheme.errors.empty? && @scheme.save + if @scheme.owning_organisation.merge_date.present? + deactivation = SchemeDeactivationPeriod.new(scheme: @scheme, deactivation_date: @scheme.owning_organisation.merge_date) + deactivation.save!(validate: false) + end redirect_to scheme_primary_client_group_path(@scheme) else if @scheme.errors.any? { |error| error.attribute == :owning_organisation } diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 3a4c337ea..99c69c636 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -192,9 +192,15 @@ module FiltersHelper end def show_scheme_managing_org_filter?(user) + return true if user.support? + org = user.organisation + stock_owners = org.stock_owners.count + recently_absorbed_with_stock = org.absorbed_organisations.visible.merged_during_open_collection_period.where(holds_own_stock: true).count + + relevant_orgs_count = stock_owners + recently_absorbed_with_stock + (org.holds_own_stock? ? 1 : 0) - user.support? || org.stock_owners.count > 1 || (org.holds_own_stock? && org.stock_owners.count.positive?) + relevant_orgs_count > 1 end def logs_for_both_needstypes_present?(organisation) diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index bcd40b082..12d86aba8 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -20,11 +20,14 @@ module SchemesHelper end def owning_organisation_options(current_user) - all_orgs = Organisation.visible.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.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } - merged_organisations = current_user.organisation.absorbed_organisations.visible.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 + if current_user.support? + Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: org.name) } + else + user_org = [current_user.organisation] + stock_owners = current_user.organisation.stock_owners.visible.filter { |org| org.status == :active || (org.status == :merged && org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period) } + merged_organisations = current_user.organisation.absorbed_organisations.visible.merged_during_open_collection_period + (user_org + stock_owners + merged_organisations).map { |org| OpenStruct.new(id: org.id, name: org.name) } + end end def null_option @@ -81,7 +84,12 @@ module SchemesHelper when :deactivating_soon "This scheme deactivates on #{scheme.last_deactivation_date.to_formatted_s(:govuk_date)}. Any locations you add will be deactivated on the same date. Reactivate the scheme to add locations active after this date." when :deactivated - "This scheme deactivated on #{scheme.last_deactivation_date.to_formatted_s(:govuk_date)}. Any locations you add will be deactivated on the same date. Reactivate the scheme to add locations active after this date." + case scheme.owning_organisation.status + when :active + "This scheme deactivated on #{scheme.last_deactivation_date.to_formatted_s(:govuk_date)}. Any locations you add will be deactivated on the same date. Reactivate the scheme to add locations active after this date." + when :merged + "This scheme has been deactivated due to #{scheme.owning_organisation.name} merging into #{scheme.owning_organisation.absorbing_organisation.name} on #{scheme.owning_organisation.merge_date.to_formatted_s(:govuk_date)}. Any locations you add will be deactivated on the same date. Use the after merge organisation for schemes and locations active after this date." + end end end diff --git a/app/models/location.rb b/app/models/location.rb index 03af24a94..12c6f2fad 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -54,13 +54,13 @@ class Location < ApplicationRecord } scope :deactivated, lambda { |date = Time.zone.now| - deactivated_by_organisation + deactivated_by_organisation(date) .or(deactivated_directly(date)) .or(deactivated_by_scheme(date)) } - scope :deactivated_by_organisation, lambda { - merge(Organisation.filter_by_inactive) + scope :deactivated_by_organisation, lambda { |date = Time.zone.now| + merge(Organisation.filter_by_inactive.or(Organisation.where("merge_date <= ?", date))) } scope :deactivated_by_scheme, lambda { |date = Time.zone.now| @@ -206,7 +206,7 @@ class Location < ApplicationRecord def status_at(date) return :deleted if discarded_at.present? return :incomplete unless confirmed - return :deactivated if scheme.owning_organisation.status_at(date) == :deactivated || + return :deactivated if scheme.owning_organisation.status_at(date) == :deactivated || scheme.owning_organisation.status_at(date) == :merged || open_deactivation&.deactivation_date.present? && date >= open_deactivation.deactivation_date || scheme.status_at(date) == :deactivated return :deactivating_soon if open_deactivation&.deactivation_date.present? && date < open_deactivation.deactivation_date || scheme.status_at(date) == :deactivating_soon return :activating_soon if startdate.present? && date < startdate diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 2c73acc06..33f236374 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -57,8 +57,8 @@ class Scheme < ApplicationRecord .or(deactivated_directly) } - scope :deactivated_by_organisation, lambda { - merge(Organisation.filter_by_inactive) + scope :deactivated_by_organisation, lambda { |date = Time.zone.now| + merge(Organisation.filter_by_inactive.or(Organisation.where("merge_date <= ?", date))) } scope :deactivated_directly, lambda { |date = Time.zone.now| @@ -96,7 +96,7 @@ class Scheme < ApplicationRecord scope :active, lambda { |date = Time.zone.now| where.not(id: joins(:scheme_deactivation_periods).reactivating_soon(date).pluck(:id)) .where.not(id: incomplete.pluck(:id)) - .where.not(id: joins(:owning_organisation).deactivated_by_organisation.pluck(:id)) + .where.not(id: joins(:owning_organisation).deactivated_by_organisation(date).pluck(:id)) .where.not(id: joins(:owning_organisation).joins(:scheme_deactivation_periods).deactivated_directly(date).pluck(:id)) .where.not(id: activating_soon(date).pluck(:id)) } @@ -314,7 +314,7 @@ class Scheme < ApplicationRecord def status_at(date) return :deleted if discarded_at.present? return :incomplete unless confirmed && locations.confirmed.any? - return :deactivated if owning_organisation.status_at(date) == :deactivated || + return :deactivated if owning_organisation.status_at(date) == :deactivated || owning_organisation.status_at(date) == :merged || (open_deactivation&.deactivation_date.present? && date >= open_deactivation.deactivation_date) return :deactivating_soon if open_deactivation&.deactivation_date.present? && date < open_deactivation.deactivation_date return :reactivating_soon if last_deactivation_before(date)&.reactivation_date.present? && date < last_deactivation_before(date).reactivation_date diff --git a/app/policies/location_policy.rb b/app/policies/location_policy.rb index 436b961c6..3b4a22131 100644 --- a/app/policies/location_policy.rb +++ b/app/policies/location_policy.rb @@ -16,14 +16,14 @@ class LocationPolicy if location == Location user.data_coordinator? else - user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end end def update? return true if user.support? - user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end def delete_confirmation? @@ -62,7 +62,7 @@ class LocationPolicy define_method method_name do return true if user.support? - user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end end @@ -73,7 +73,7 @@ class LocationPolicy define_method method_name do return true if user.support? - scheme_owned_by_user_org_or_stock_owner + scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end end @@ -83,8 +83,11 @@ private location.scheme end - def scheme_owned_by_user_org_or_stock_owner - scheme&.owning_organisation == user.organisation || user.organisation.stock_owners.exists?(scheme&.owning_organisation_id) + def scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org + scheme_owned_by_user_org = scheme&.owning_organisation == user.organisation + scheme_owned_by_stock_owner = user.organisation.stock_owners.exists?(scheme&.owning_organisation_id) + scheme_owned_by_recently_absorbed_org = user.organisation.absorbed_organisations.visible.merged_during_open_collection_period.exists?(scheme&.owning_organisation_id) + scheme_owned_by_user_org || scheme_owned_by_stock_owner || scheme_owned_by_recently_absorbed_org end def has_any_logs_in_editable_collection_period diff --git a/app/policies/scheme_policy.rb b/app/policies/scheme_policy.rb index 6b97a46de..54a2b9e89 100644 --- a/app/policies/scheme_policy.rb +++ b/app/policies/scheme_policy.rb @@ -12,7 +12,7 @@ class SchemePolicy if scheme == Scheme true else - scheme_owned_by_user_org_or_stock_owner + scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end end @@ -27,7 +27,7 @@ class SchemePolicy def update? return true if user.support? - user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end def changes? @@ -41,7 +41,7 @@ class SchemePolicy define_method method_name do return true if user.support? - scheme_owned_by_user_org_or_stock_owner + scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end end @@ -61,7 +61,7 @@ class SchemePolicy define_method method_name do return true if user.support? - user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org end end @@ -78,8 +78,11 @@ class SchemePolicy private - def scheme_owned_by_user_org_or_stock_owner - scheme&.owning_organisation == user.organisation || user.organisation.stock_owners.exists?(scheme&.owning_organisation_id) + def scheme_owned_by_user_org_or_stock_owner_or_recently_absorbed_org + scheme_owned_by_user_org = scheme&.owning_organisation == user.organisation + scheme_owned_by_stock_owner = user.organisation.stock_owners.exists?(scheme&.owning_organisation_id) + scheme_owned_by_recently_absorbed_org = user.organisation.absorbed_organisations.visible.merged_during_open_collection_period.exists?(scheme&.owning_organisation_id) + scheme_owned_by_user_org || scheme_owned_by_stock_owner || scheme_owned_by_recently_absorbed_org end def has_any_logs_in_editable_collection_period diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 64d9bf286..23550f894 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -56,14 +56,13 @@ <% end %> <% end %> - <% if status_hint_message = scheme_status_hint(@scheme) %> -
- <%= status_hint_message %> -
-
- <% end %> - - <% if LocationPolicy.new(current_user, @scheme.locations.new).create? %> + <% if LocationPolicy.new(current_user, @scheme.locations.new).create? && [:active, :merged].include?(@scheme.owning_organisation.status) %> + <% if status_hint_message = scheme_status_hint(@scheme) %> +
+ <%= status_hint_message %> +
+
+ <% end %> <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post" %> <% end %>
diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index 8ac8f6b23..f9ba6496c 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -47,7 +47,7 @@ -<% if @location.scheme.owning_organisation.active? && LocationPolicy.new(current_user, @location).deactivate? %> +<% if @location.scheme.owning_organisation.status == :active && LocationPolicy.new(current_user, @location).deactivate? %> <%= toggle_location_link(@location) %> <% end %> diff --git a/app/views/schemes/details.html.erb b/app/views/schemes/details.html.erb index cb29a56dc..4b23ab016 100644 --- a/app/views/schemes/details.html.erb +++ b/app/views/schemes/details.html.erb @@ -49,11 +49,13 @@ :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? && !current_user.organisation.has_recent_absorbed_organisations? %> + <% scheme_owning_organisation_options = owning_organisation_options(current_user) %> + + <% if scheme_owning_organisation_options.count == 1 %> <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> <% else %> <%= f.govuk_collection_select :owning_organisation_id, - owning_organisation_options(current_user), + scheme_owning_organisation_options, :id, :name, label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb index 6cefa5847..0aa25affc 100644 --- a/app/views/schemes/show.html.erb +++ b/app/views/schemes/show.html.erb @@ -52,7 +52,7 @@ -<% if @scheme.owning_organisation.active? && SchemePolicy.new(current_user, @scheme).deactivate? %> +<% if @scheme.owning_organisation.status == :active && SchemePolicy.new(current_user, @scheme).deactivate? %> <%= toggle_scheme_link(@scheme) %> <% end %> diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb index 79265d361..18581fb6e 100644 --- a/spec/models/location_spec.rb +++ b/spec/models/location_spec.rb @@ -929,6 +929,11 @@ RSpec.describe Location, type: :model do expect(location.status).to eq(:deactivated) end + it "returns deactivated if the owning organisation has been merged" do + location.scheme.owning_organisation.merge_date = 2.days.ago + expect(location.status).to eq(:deactivated) + end + it "returns deactivated if deactivation_date is in the past" do FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.yesterday, location:) location.save! diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb index 5ca529d3e..65174388d 100644 --- a/spec/models/scheme_spec.rb +++ b/spec/models/scheme_spec.rb @@ -363,6 +363,11 @@ RSpec.describe Scheme, type: :model do expect(scheme.status).to eq(:deactivated) end + it "returns deactivated if the owning organisation has been merged" do + scheme.owning_organisation.merge_date = 2.days.ago + expect(scheme.status).to eq(:deactivated) + end + it "returns deactivated if deactivation_date is in the past" do FactoryBot.create(:scheme_deactivation_period, deactivation_date: Time.zone.yesterday, scheme:) scheme.reload diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 19ede5cc4..83ba11fd9 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -89,9 +89,47 @@ RSpec.describe SchemesController, type: :request do end end + context "when a recently absorbed organisation has schemes" do + let(:absorbed_org) { create(:organisation) } + let!(:absorbed_org_schemes) { create_list(:scheme, 2, owning_organisation: absorbed_org) } + + before do + absorbed_org.merge_date = 2.days.ago + absorbed_org.absorbing_organisation = user.organisation + absorbed_org.save! + end + + it "shows absorbed organisation schemes" do + get "/schemes" + follow_redirect! + absorbed_org_schemes.each do |scheme| + expect(page).to have_content(scheme.id_to_display) + end + end + end + + context "when a non-recently absorbed organisation has schemes" do + let(:absorbed_org) { create(:organisation) } + let!(:absorbed_org_schemes) { create_list(:scheme, 2, owning_organisation: absorbed_org) } + + before do + absorbed_org.merge_date = 2.years.ago + absorbed_org.absorbing_organisation = user.organisation + absorbed_org.save! + end + + it "shows absorbed organisation schemes" do + get "/schemes" + follow_redirect! + absorbed_org_schemes.each do |scheme| + expect(page).not_to have_content(scheme.id_to_display) + end + end + end + context "when filtering" do context "with owning organisation filter" do - context "when user org does not have owning orgs" do + context "when user org does not have owning orgs or recently absorbed orgs" do it "does not show filter" do expect(page).not_to have_content("Owned by") end @@ -700,6 +738,27 @@ RSpec.describe SchemesController, type: :request do end end + context "when coordinator attempts to see scheme belonging to a recently absorbed organisation" do + let(:absorbed_organisation) { create(:organisation) } + let!(:specific_scheme) { create(:scheme, owning_organisation: absorbed_organisation) } + + before do + absorbed_organisation.merge_date = 2.days.ago + absorbed_organisation.absorbing_organisation = user.organisation + absorbed_organisation.save! + + get "/schemes/#{specific_scheme.id}" + end + + it "shows the scheme" do + expect(page).to have_content(specific_scheme.id_to_display) + end + + it "allows editing" do + expect(page).to have_link("Change") + end + end + context "when the scheme has all details but no confirmed locations" do it "shows the scheme as incomplete with text to explain" do get scheme_path(specific_scheme) @@ -1146,6 +1205,31 @@ RSpec.describe SchemesController, type: :request do end end end + + context "when making a scheme in an organisation recently absorbed by the users organisation" do + let(:absorbed_organisation) { create(:organisation) } + let(:params) do + { scheme: { service_name: " testy ", + sensitive: "1", + scheme_type: "Foyer", + registered_under_care_act: "No", + owning_organisation_id: absorbed_organisation.id, + arrangement_type: "D" } } + end + + before do + absorbed_organisation.merge_date = 2.days.ago + absorbed_organisation.absorbing_organisation = user.organisation + absorbed_organisation.save! + end + + it "creates a new scheme for this organisation and renders correct page" do + expect { post "/schemes", params: }.to change(Scheme, :count).by(1) + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("What client group is this scheme intended for?") + end + end end context "when signed in as a support user" do From 2a0a2a59a945325c7ddc4ef5265fc4fabd51767a Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 26 Nov 2024 17:32:55 +0000 Subject: [PATCH 6/6] Add success banner for new users (#2827) --- app/controllers/users_controller.rb | 1 + spec/requests/users_controller_spec.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f27bfc2b3..57036cabe 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -114,6 +114,7 @@ class UsersController < ApplicationController validate_attributes if @user.errors.empty? && @user.save + flash[:notice] = "Invitation sent to #{@user.email}" redirect_to created_user_redirect_path else unless @user.errors[:organisation].empty? diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 1b62196bb..2b7210402 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -1043,6 +1043,9 @@ RSpec.describe UsersController, type: :request do it "invites a new user" do expect { request }.to change(User, :count).by(1) + follow_redirect! + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + expect(page).to have_content("Invitation sent to new_user@example.com") end it "sends an invitation email" do