diff --git a/app/components/lettings_log_summary_component.html.erb b/app/components/lettings_log_summary_component.html.erb index 89fddad20..7686e3559 100644 --- a/app/components/lettings_log_summary_component.html.erb +++ b/app/components/lettings_log_summary_component.html.erb @@ -33,13 +33,13 @@ <% if log.owning_organisation %>
Owned by
-
<%= log.owning_organisation&.name %>
+
<%= log.owning_organisation&.label %>
<% end %> <% if log.managing_organisation %>
Managed by
-
<%= log.managing_organisation&.name %>
+
<%= log.managing_organisation&.label %>
<% end %> diff --git a/app/components/sales_log_summary_component.html.erb b/app/components/sales_log_summary_component.html.erb index 60a2a7eb5..1d376aa4a 100644 --- a/app/components/sales_log_summary_component.html.erb +++ b/app/components/sales_log_summary_component.html.erb @@ -26,13 +26,13 @@ <% if log.owning_organisation %>
Owned by
-
<%= log.owning_organisation&.name %>
+
<%= log.owning_organisation&.label %>
<% end %> <% if log.managing_organisation %>
Reported by
-
<%= log.managing_organisation&.name %>
+
<%= log.managing_organisation&.label %>
<% end %> diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 357a5f272..138baa78c 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -15,9 +15,9 @@ class OrganisationsController < ApplicationController redirect_to organisation_path(current_user.organisation) unless current_user.support? all_organisations = Organisation.order(:name) - @pagy, @organisations = pagy(filtered_collection(all_organisations, search_term)) + @pagy, @organisations = pagy(filtered_collection(all_organisations.visible, search_term)) @searched = search_term.presence - @total_count = all_organisations.size + @total_count = all_organisations.visible.size end def schemes @@ -117,7 +117,6 @@ class OrganisationsController < ApplicationController end def update - selected_rent_periods = rent_period_params[:rent_periods].compact_blank if (current_user.data_coordinator? && org_params[:active].nil?) || current_user.support? if @organisation.update(org_params) case org_params[:active] @@ -136,11 +135,14 @@ class OrganisationsController < ApplicationController else flash[:notice] = I18n.t("organisation.updated") end - used_rent_periods = @organisation.lettings_logs.pluck(:period).uniq.compact.map(&:to_s) - rent_periods_to_delete = rent_period_params[:all_rent_periods] - selected_rent_periods - used_rent_periods - OrganisationRentPeriod.transaction do - selected_rent_periods.each { |period| OrganisationRentPeriod.create(organisation: @organisation, rent_period: period) } - OrganisationRentPeriod.where(organisation: @organisation, rent_period: rent_periods_to_delete).destroy_all + if rent_period_params[:rent_periods].present? + selected_rent_periods = rent_period_params[:rent_periods].compact_blank + used_rent_periods = @organisation.lettings_logs.pluck(:period).uniq.compact.map(&:to_s) + rent_periods_to_delete = rent_period_params[:all_rent_periods] - selected_rent_periods - used_rent_periods + OrganisationRentPeriod.transaction do + selected_rent_periods.each { |period| OrganisationRentPeriod.create(organisation: @organisation, rent_period: period) } + OrganisationRentPeriod.where(organisation: @organisation, rent_period: rent_periods_to_delete).destroy_all + end end redirect_to details_organisation_path(@organisation) else @@ -152,6 +154,17 @@ class OrganisationsController < ApplicationController end end + def delete + authorize @organisation + + @organisation.discard! + redirect_to organisations_path, notice: I18n.t("notification.organisation_deleted", name: @organisation.name) + end + + def delete_confirmation + authorize @organisation + end + def lettings_logs organisation_logs = LettingsLog.visible.filter_by_organisation(@organisation).filter_by_years_or_nil(FormHandler.instance.years_of_available_lettings_forms) unpaginated_filtered_logs = filter_manager.filtered_logs(organisation_logs, search_term, session_filters) @@ -306,7 +319,7 @@ private end def authenticate_scope! - if %w[create new lettings_logs sales_logs download_lettings_csv email_lettings_csv email_sales_csv download_sales_csv].include? action_name + if %w[create new lettings_logs sales_logs download_lettings_csv email_lettings_csv email_sales_csv download_sales_csv delete_confirmation delete].include? action_name head :unauthorized and return unless current_user.support? elsif current_user.organisation != @organisation && !current_user.support? render_not_found diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 7d37b9289..75d5869fa 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -19,7 +19,7 @@ module OrganisationsHelper { name: "Owns housing stock", value: organisation.holds_own_stock ? "Yes" : "No", editable: false }, { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet }, { name: "Data Sharing Agreement" }, - { name: "Status", value: status_tag(organisation.status), editable: false }, + { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false }, ] end @@ -39,9 +39,19 @@ module OrganisationsHelper end end + def delete_organisation_text(organisation) + if organisation.active == false && current_user.support? && !OrganisationPolicy.new(current_user, organisation).delete? + "
This organisation was active in an open or editable collection year, and cannot be deleted.
".html_safe + end + end + def rent_periods_with_checked_attr(checked_periods: nil) RentPeriod.rent_period_mappings.each_with_object({}) do |(period_code, period_value), result| result[period_code] = period_value.merge(checked: checked_periods.nil? || checked_periods.include?(period_code)) end end + + def delete_organisation_link(organisation) + govuk_button_link_to "Delete this organisation", delete_confirmation_organisation_path(organisation), warning: true + end end diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index 1cc89d3d5..f96f9e4c8 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -20,10 +20,10 @@ module SchemesHelper end def owning_organisation_options(current_user) - all_orgs = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } + 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.map { |org| OpenStruct.new(id: org.id, name: org.name) } - merged_organisations = current_user.organisation.absorbed_organisations.merged_during_open_collection_period.map { |org| OpenStruct.new(id: org.id, name: org.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 end diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index ea85ff207..81847876e 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -17,7 +17,8 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question return opts unless log if log.managing_organisation.present? - opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name }) + org_value = log.managing_organisation.label + opts = opts.merge({ log.managing_organisation.id => org_value }) end if user.support? @@ -31,14 +32,14 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question end orgs = if user.support? - log.owning_organisation.managing_agents.filter_by_active + log.owning_organisation.managing_agents.visible.filter_by_active elsif user.organisation.absorbed_organisations.include?(log.owning_organisation) - user.organisation.managing_agents.filter_by_active + log.owning_organisation.managing_agents.filter_by_active + user.organisation.managing_agents.visible.filter_by_active + log.owning_organisation.managing_agents.visible.filter_by_active # here else - user.organisation.managing_agents.filter_by_active + user.organisation.managing_agents.visible.filter_by_active end - user.organisation.absorbed_organisations.each do |absorbed_org| + user.organisation.absorbed_organisations.visible.each do |absorbed_org| opts[absorbed_org.id] = "#{absorbed_org.name} (inactive as of #{absorbed_org.merge_date.to_fs(:govuk_date)})" end @@ -72,7 +73,10 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question end def answer_label(log, _current_user = nil) - Organisation.find_by(id: log.managing_organisation_id)&.name + organisation = Organisation.find_by(id: log.managing_organisation_id) + return unless organisation + + organisation.label end private diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index 6ce0e5496..daa042004 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/app/models/form/lettings/questions/stock_owner.rb @@ -17,7 +17,8 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question return answer_opts unless log if log.owning_organisation_id.present? - answer_opts[log.owning_organisation.id] = log.owning_organisation.name + org_value = log.owning_organisation.label + answer_opts[log.owning_organisation.id] = org_value end recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period @@ -30,7 +31,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question end if user.support? - Organisation.filter_by_active.where(holds_own_stock: true).find_each do |org| + Organisation.visible.filter_by_active.where(holds_own_stock: true).find_each do |org| if org.merge_date.present? answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present? @@ -40,10 +41,10 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question end end else - user.organisation.stock_owners.filter_by_active.each do |stock_owner| + user.organisation.stock_owners.visible.filter_by_active.each do |stock_owner| answer_opts[stock_owner.id] = stock_owner.name end - recently_absorbed_organisations.each do |absorbed_org| + recently_absorbed_organisations.visible.each do |absorbed_org| answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock? end end @@ -64,7 +65,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question def hidden_in_check_answers?(_log, user = nil) return false if user.support? - stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true) + stock_owners = user.organisation.stock_owners.visible + user.organisation.absorbed_organisations.visible.where(holds_own_stock: true) if user.organisation.holds_own_stock? stock_owners.count.zero? diff --git a/app/models/form/sales/questions/managing_organisation.rb b/app/models/form/sales/questions/managing_organisation.rb index 54d7ab6e0..03609233e 100644 --- a/app/models/form/sales/questions/managing_organisation.rb +++ b/app/models/form/sales/questions/managing_organisation.rb @@ -17,7 +17,8 @@ class Form::Sales::Questions::ManagingOrganisation < ::Form::Question return opts unless log if log.managing_organisation.present? - opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name }) + org_value = log.managing_organisation.label + opts = opts.merge({ log.managing_organisation.id => org_value }) end if user.support? @@ -31,14 +32,14 @@ class Form::Sales::Questions::ManagingOrganisation < ::Form::Question end orgs = if user.support? - log.owning_organisation.managing_agents.filter_by_active + log.owning_organisation.managing_agents.visible.filter_by_active elsif user.organisation.absorbed_organisations.include?(log.owning_organisation) - user.organisation.managing_agents.filter_by_active + log.owning_organisation.managing_agents.filter_by_active + user.organisation.managing_agents.visible.filter_by_active + log.owning_organisation.managing_agents.visible.filter_by_active else - user.organisation.managing_agents.filter_by_active + user.organisation.managing_agents.visible.filter_by_active end.pluck(:id, :name).to_h - user.organisation.absorbed_organisations.each do |absorbed_org| + user.organisation.absorbed_organisations.visible.each do |absorbed_org| opts[absorbed_org.id] = "#{absorbed_org.name} (inactive as of #{absorbed_org.merge_date.to_fs(:govuk_date)})" end diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index dc3913882..669c43a0e 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -16,25 +16,22 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question return answer_opts unless user return answer_opts unless log - unless user.support? - if log.owning_organisation_id.present? - answer_opts[log.owning_organisation.id] = log.owning_organisation.name - end + if log.owning_organisation_id.present? + org_value = log.owning_organisation.label + answer_opts[log.owning_organisation.id] = org_value + end + unless user.support? if user.organisation.holds_own_stock? answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)" end - user.organisation.stock_owners.filter_by_active.where(holds_own_stock: true).find_each do |org| + user.organisation.stock_owners.visible.filter_by_active.where(holds_own_stock: true).find_each do |org| answer_opts[org.id] = org.name end end - if log.owning_organisation_id.present? - answer_opts[log.owning_organisation.id] = log.owning_organisation.name - end - - recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period + recently_absorbed_organisations = user.organisation.absorbed_organisations.visible.merged_during_open_collection_period if !user.support? && user.organisation.holds_own_stock? answer_opts[user.organisation.id] = if recently_absorbed_organisations.exists? && user.organisation.available_from.present? "#{user.organisation.name} (Your organisation, active as of #{user.organisation.available_from.to_fs(:govuk_date)})" @@ -44,7 +41,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question end if user.support? - Organisation.filter_by_active.where(holds_own_stock: true).find_each do |org| + Organisation.visible.filter_by_active.where(holds_own_stock: true).find_each do |org| if org.merge_date.present? answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present? @@ -54,7 +51,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question end end else - recently_absorbed_organisations.each do |absorbed_org| + recently_absorbed_organisations.visible.each do |absorbed_org| answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock? end end @@ -75,7 +72,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question def hidden_in_check_answers?(_log, user = nil) return false if user.support? - stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true) + stock_owners = user.organisation.stock_owners.visible + user.organisation.absorbed_organisations.visible.where(holds_own_stock: true) if user.organisation.holds_own_stock? stock_owners.count.zero? diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 25a3fdd0d..8f77df166 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -17,6 +17,7 @@ class Organisation < ApplicationRecord belongs_to :absorbing_organisation, class_name: "Organisation", optional: true has_many :absorbed_organisations, class_name: "Organisation", foreign_key: "absorbing_organisation_id" + scope :visible, -> { where(discarded_at: nil) } def affiliated_stock_owners ids = [] @@ -143,6 +144,7 @@ class Organisation < ApplicationRecord end def status_at(date) + return :deleted if discarded_at.present? return :merged if merge_date.present? && merge_date < date return :deactivated unless active @@ -178,4 +180,14 @@ class Organisation < ApplicationRecord false end + + def discard! + owned_schemes.each(&:discard!) + users.each(&:discard!) + update!(discarded_at: Time.zone.now) + end + + def label + status == :deleted ? "#{name} (deleted)" : name + end end diff --git a/app/policies/organisation_policy.rb b/app/policies/organisation_policy.rb index 3e0c9a946..9c27d6e91 100644 --- a/app/policies/organisation_policy.rb +++ b/app/policies/organisation_policy.rb @@ -13,4 +13,25 @@ class OrganisationPolicy def reactivate? user.support? && organisation.status == :deactivated end + + def delete_confirmation? + delete? + end + + def delete? + return false unless user.support? + return false unless organisation.status == :deactivated || organisation.status == :merged + + !has_any_logs_in_editable_collection_period + end + + def has_any_logs_in_editable_collection_period + editable_from_date = FormHandler.instance.earliest_open_for_editing_collection_start_date + editable_lettings_logs = organisation.lettings_logs.visible.after_date(editable_from_date) + + return true if organisation.lettings_logs.visible.where(startdate: nil).any? || editable_lettings_logs.any? + + editable_sales_logs = organisation.sales_logs.visible.after_date(editable_from_date) + organisation.sales_logs.visible.where(saledate: nil).any? || editable_sales_logs.any? + end end diff --git a/app/views/organisations/delete_confirmation.html.erb b/app/views/organisations/delete_confirmation.html.erb new file mode 100644 index 000000000..96dba012a --- /dev/null +++ b/app/views/organisations/delete_confirmation.html.erb @@ -0,0 +1,24 @@ +<% content_for :before_content do %> + <% content_for :title, "Are you sure you want to delete this organisation?" %> + <%= govuk_back_link(href: :back) %> +<% end %> + +
+
+ Delete <%= @organisation.name %> +

+ <%= content_for(:title) %> +

+ + <%= govuk_warning_text(text: "You will not be able to undo this action.") %> + +
+ <%= govuk_button_to( + "Delete this organisation", + delete_organisation_path(@organisation), + method: :delete, + ) %> + <%= govuk_button_link_to "Cancel", organisation_path(@organisation), html: { method: :get }, secondary: true %> +
+
+
diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 26a20926e..214b988f0 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -52,3 +52,7 @@ <%= govuk_button_link_to "Reactivate this organisation", reactivate_organisation_path(@organisation) %> <% end %> + +<% if OrganisationPolicy.new(current_user, @organisation).delete? %> + <%= delete_organisation_link(@organisation) %> +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 537fe2114..9dcfed8c3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -202,6 +202,7 @@ en: location_deleted: "%{postcode} has been deleted." scheme_deleted: "%{service_name} has been deleted." user_deleted: "%{name} has been deleted." + organisation_deleted: "%{name} has been deleted." validations: organisation: diff --git a/config/routes.rb b/config/routes.rb index 5361f5733..e8932ea45 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -190,6 +190,8 @@ Rails.application.routes.draw do get "sales-logs/filters/#{filter}", to: "sales_logs_filters#organisation_#{filter.underscore}" get "sales-logs/filters/update-#{filter}", to: "sales_logs_filters#update_organisation_#{filter.underscore}" end + get "delete-confirmation", to: "organisations#delete_confirmation" + delete "delete", to: "organisations#delete" end end diff --git a/db/migrate/20240610142812_add_discarded_at_to_organisations.rb b/db/migrate/20240610142812_add_discarded_at_to_organisations.rb new file mode 100644 index 000000000..192e7095c --- /dev/null +++ b/db/migrate/20240610142812_add_discarded_at_to_organisations.rb @@ -0,0 +1,5 @@ +class AddDiscardedAtToOrganisations < ActiveRecord::Migration[7.0] + def change + add_column :organisations, :discarded_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 1430da65f..66693435f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_05_29_133005) do +ActiveRecord::Schema[7.0].define(version: 2024_06_10_142812) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -492,6 +492,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_05_29_133005) do t.datetime "merge_date" t.bigint "absorbing_organisation_id" t.datetime "available_from" + t.datetime "discarded_at" t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb index 70814b9d5..99319fc71 100644 --- a/spec/models/form/lettings/questions/managing_organisation_spec.rb +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -58,6 +58,7 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do let(:managing_org2) { create(:organisation, name: "Managing org 2") } let(:managing_org3) { create(:organisation, name: "Managing org 3") } let(:inactive_managing_org) { create(:organisation, name: "Inactive managing org", active: false) } + let(:deleted_managing_org) { create(:organisation, name: "Deleted managing org", discarded_at: Time.zone.yesterday) } let(:log) { create(:lettings_log, managing_organisation: managing_org1) } let!(:org_rel1) do @@ -79,6 +80,7 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do it "shows current managing agent at top, followed by user's org (with hint), followed by the active managing agents of the user's org" do create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: inactive_managing_org) + create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: deleted_managing_org) expect(question.displayed_answer_options(log, user)).to eq(options) end @@ -104,7 +106,10 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do end before do - create(:organisation, name: "Inactive managing org", active: false) + inactive_managing_org = create(:organisation, name: "Inactive managing org", active: false) + create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: inactive_managing_org) + deleted_managing_org = create(:organisation, name: "Deleted managing org", discarded_at: Time.zone.yesterday) + create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: deleted_managing_org) end context "when org owns stock" do @@ -192,10 +197,12 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do context "when organisation has merged" do let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) } let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) } + let!(:merged_deleted_org) { create(:organisation, name: "Merged org", holds_own_stock: false, discarded_at: Time.zone.yesterday) } let(:user) { create(:user, :data_coordinator, organisation: absorbing_org) } let(:log) do merged_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id) + merged_deleted_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id) create(:lettings_log, owning_organisation: absorbing_org, managing_organisation: nil) end diff --git a/spec/models/form/lettings/questions/stock_owner_spec.rb b/spec/models/form/lettings/questions/stock_owner_spec.rb index a4c7d8fd6..8443f3680 100644 --- a/spec/models/form/lettings/questions/stock_owner_spec.rb +++ b/spec/models/form/lettings/questions/stock_owner_spec.rb @@ -47,6 +47,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do let(:owning_org_1) { create(:organisation, name: "Owning org 1") } let(:owning_org_2) { create(:organisation, name: "Owning org 2") } let(:inactive_owning_org) { create(:organisation, name: "Inactive owning org", active: false) } + let(:deleted_owning_org) { create(:organisation, name: "Deleted owning org", discarded_at: Time.zone.yesterday) } let!(:org_rel) do create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) end @@ -64,6 +65,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do it "shows current stock owner at top, followed by user's org (with hint), followed by the active stock owners of the user's org" do create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: inactive_owning_org) + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: deleted_owning_org) user.organisation.update!(holds_own_stock: true) expect(question.displayed_answer_options(log, user)).to eq(options) end @@ -127,6 +129,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do context "when user's org has recently absorbed other orgs and does not have available from date" do let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:merged_deleted_organisation) { create(:organisation, name: "Merged deleted org", discarded_at: Time.zone.yesterday) } let(:options) do { "" => "Select an option", @@ -139,6 +142,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do before do merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation) + merged_deleted_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation) allow(Time).to receive(:now).and_return(Time.zone.local(2023, 11, 10)) end @@ -172,6 +176,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do context "when user's org has absorbed other orgs with parent organisations during closed collection periods" do let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:merged_deleted_organisation) { create(:organisation, name: "Merged deleted org", discarded_at: Time.zone.yesterday) } let(:options) do { "" => "Select an option", @@ -184,6 +189,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do allow(Time).to receive(:now).and_return(Time.zone.local(2023, 4, 2)) org_rel.update!(child_organisation: merged_organisation) merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation) + merged_deleted_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation) user.organisation.update!(available_from: Time.zone.local(2021, 2, 2)) end @@ -200,8 +206,9 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do it "shows active orgs where organisation holds own stock" do non_stock_organisation = create(:organisation, name: "Non-stockholding org", holds_own_stock: false) inactive_organisation = create(:organisation, name: "Inactive org", active: false) + deleted_organisation = create(:organisation, name: "Deleted org", discarded_at: Time.zone.yesterday) - expected_opts = Organisation.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh| + expected_opts = Organisation.visible.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh| hsh[organisation.id] = organisation.name hsh end @@ -209,10 +216,12 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do expect(question.displayed_answer_options(log, user)).to eq(expected_opts) expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id) expect(question.displayed_answer_options(log, user)).not_to include(inactive_organisation.id) + expect(question.displayed_answer_options(log, user)).not_to include(deleted_organisation.id) end context "and org has recently absorbed other orgs and does not have available from date" do let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:merged_deleted_organisation) { create(:organisation, name: "Merged deleted org", discarded_at: Time.zone.yesterday) } let(:org) { create(:organisation, name: "User org") } let(:options) do { @@ -226,6 +235,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do before do merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: org) + merged_deleted_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: org) allow(Time).to receive(:now).and_return(Time.zone.local(2023, 11, 10)) end @@ -273,6 +283,18 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do expect(question.hidden_in_check_answers?(nil, user)).to be false end end + + context "when stock owner is deleted" do + before do + organisation_relationship = create(:organisation_relationship, child_organisation: user.organisation) + organisation_relationship.parent_organisation.update!(discarded_at: Time.zone.yesterday) + end + + it "is hidden in check answers" do + expect(user.organisation.stock_owners.count).to eq(1) + expect(question.hidden_in_check_answers?(nil, user)).to be true + end + end end context "when org does not hold own stock", :aggregate_failures do diff --git a/spec/models/form/sales/questions/managing_organisation_spec.rb b/spec/models/form/sales/questions/managing_organisation_spec.rb index c9db71423..2ebbf5e01 100644 --- a/spec/models/form/sales/questions/managing_organisation_spec.rb +++ b/spec/models/form/sales/questions/managing_organisation_spec.rb @@ -58,6 +58,7 @@ RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do let(:managing_org2) { create(:organisation, name: "Managing org 2") } let(:managing_org3) { create(:organisation, name: "Managing org 3") } let(:inactive_org) { create(:organisation, name: "Inactive org", active: false) } + let(:deleted_org) { create(:organisation, name: "Deleted org", discarded_at: Time.zone.yesterday) } let(:log) do create(:lettings_log, owning_organisation: log_owning_org, managing_organisation: managing_org1, @@ -83,6 +84,7 @@ RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do it "shows current managing agent at top, followed by the current owning organisation (with hint), followed by the active managing agents of the current owning organisation" do create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: inactive_org) + create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: deleted_org) log_owning_org.update!(holds_own_stock: true) expect(question.displayed_answer_options(log, user)).to eq(options) end @@ -135,10 +137,12 @@ RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do context "when organisation has merged" do let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) } let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) } + let!(:merged_deleted_org) { create(:organisation, name: "Merged deleted org", holds_own_stock: false, discarded_at: Time.zone.yesterday) } let(:user) { create(:user, :support, organisation: absorbing_org) } let(:log) do merged_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id) + merged_deleted_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id) create(:lettings_log, owning_organisation: absorbing_org, managing_organisation: nil) end diff --git a/spec/models/form/sales/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb index ea904b400..bc6fad6e5 100644 --- a/spec/models/form/sales/questions/owning_organisation_id_spec.rb +++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb @@ -123,6 +123,11 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do it "shows merged organisation as an option" do expect(question.displayed_answer_options(log, user)).to eq(options) end + + it "does not show absorbed organisation if it has been deleted" do + merged_organisation.update!(discarded_at: Time.zone.yesterday) + expect(question.displayed_answer_options(log, user)).to eq(options.except(merged_organisation.id)) + end end context "when user's org has recently absorbed other orgs and it has available from date" do @@ -200,8 +205,9 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do it "shows active orgs where organisation holds own stock" do non_stock_organisation = create(:organisation, holds_own_stock: false) inactive_org = create(:organisation, active: false) + deleted_organisation = create(:organisation, discarded_at: Time.zone.yesterday) - expected_opts = Organisation.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh| + expected_opts = Organisation.visible.filter_by_active.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh| hsh[organisation.id] = organisation.name hsh end @@ -210,6 +216,7 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id) expect(question.displayed_answer_options(log, user)).not_to include(inactive_org.id) expect(question.displayed_answer_options(log, user)).to include(organisation_1.id) + expect(question.displayed_answer_options(log, user)).not_to include(deleted_organisation.id) end context "when an org has recently absorbed other orgs" do diff --git a/spec/policies/organisation_policy_spec.rb b/spec/policies/organisation_policy_spec.rb index 4fb4c6761..89cb62198 100644 --- a/spec/policies/organisation_policy_spec.rb +++ b/spec/policies/organisation_policy_spec.rb @@ -63,4 +63,112 @@ RSpec.describe OrganisationPolicy do expect(policy).not_to permit(support, organisation) end end + + permissions :delete? do + let(:organisation) { FactoryBot.create(:organisation) } + + context "with active organisation" do + it "does not allow deleting a organisation as a provider" do + expect(policy).not_to permit(data_provider, organisation) + end + + it "does not allow allows deleting a organisation as a coordinator" do + expect(policy).not_to permit(data_coordinator, organisation) + end + + it "does not allow deleting a organisation as a support user" do + expect(policy).not_to permit(support, organisation) + end + end + + context "with deactivated organisation" do + before do + organisation.update!(active: false) + end + + it "does not allow deleting a organisation as a provider" do + expect(policy).not_to permit(data_provider, organisation) + end + + it "does not allow allows deleting a organisation as a coordinator" do + expect(policy).not_to permit(data_coordinator, organisation) + end + + it "allows deleting a organisation as a support user" do + expect(policy).to permit(support, organisation) + end + + context "and associated lettings logs in editable collection period" do + before do + create(:lettings_log, owning_organisation: organisation) + end + + it "does not allow deleting a organisation as a provider" do + expect(policy).not_to permit(data_provider, organisation) + end + + it "does not allow allows deleting a organisation as a coordinator" do + expect(policy).not_to permit(data_coordinator, organisation) + end + + it "does not allow deleting a organisation as a support user" do + expect(policy).not_to permit(support, organisation) + end + end + + context "and deleted associated lettings logs in editable collection period" do + before do + create(:lettings_log, owning_organisation: organisation, discarded_at: Time.zone.yesterday) + end + + it "does not allow deleting a organisation as a provider" do + expect(policy).not_to permit(data_provider, organisation) + end + + it "does not allow allows deleting a organisation as a coordinator" do + expect(policy).not_to permit(data_coordinator, organisation) + end + + it "allows deleting a organisation as a support user" do + expect(policy).to permit(support, organisation) + end + end + + context "and associated sales logs in editable collection period" do + before do + create(:sales_log, owning_organisation: organisation) + end + + it "does not allow deleting a organisation as a provider" do + expect(policy).not_to permit(data_provider, organisation) + end + + it "does not allow allows deleting a organisation as a coordinator" do + expect(policy).not_to permit(data_coordinator, organisation) + end + + it "does not allow deleting a organisation as a support user" do + expect(policy).not_to permit(support, organisation) + end + end + + context "and deleted associated sales logs in editable collection period" do + before do + create(:sales_log, owning_organisation: organisation, discarded_at: Time.zone.yesterday) + end + + it "does not allow deleting a organisation as a provider" do + expect(policy).not_to permit(data_provider, organisation) + end + + it "does not allow allows deleting a organisation as a coordinator" do + expect(policy).not_to permit(data_coordinator, organisation) + end + + it "allows deleting a organisation as a support user" do + expect(policy).to permit(support, organisation) + end + end + end + end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index f30a1e4eb..68f69d9f2 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -47,6 +47,34 @@ RSpec.describe OrganisationsController, type: :request do expect(response).to redirect_to("/account/sign-in") end end + + describe "#delete-confirmation" do + let(:organisation) { create(:organisation) } + + before do + get "/organisations/#{organisation.id}/delete-confirmation" + end + + context "when not signed in" do + it "redirects to the sign in page" do + expect(response).to redirect_to("/account/sign-in") + end + end + end + + describe "#delete" do + let(:organisation) { create(:organisation) } + + before do + delete "/organisations/#{organisation.id}/delete" + end + + context "when not signed in" do + it "redirects to the sign in page" do + expect(response).to redirect_to("/account/sign-in") + end + end + end end context "when user is signed in" do @@ -747,6 +775,38 @@ RSpec.describe OrganisationsController, type: :request do end end end + + describe "#delete-confirmation" do + let(:organisation) { user.organisation } + + before do + get "/organisations/#{organisation.id}/delete-confirmation" + end + + context "with a data provider user" do + let(:user) { create(:user) } + + it "returns 401 unauthorized" do + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe "#delete" do + let(:organisation) { user.organisation } + + before do + delete "/organisations/#{organisation.id}/delete" + end + + context "with a data provider user" do + let(:user) { create(:user) } + + it "returns 401 unauthorized" do + expect(response).to have_http_status(:unauthorized) + end + end + end end context "with a data provider user" do @@ -876,6 +936,38 @@ RSpec.describe OrganisationsController, type: :request do expect(response).to have_http_status(:unauthorized) end end + + describe "#delete-confirmation" do + let(:organisation) { user.organisation } + + before do + get "/organisations/#{organisation.id}/delete-confirmation" + end + + context "with a data provider user" do + let(:user) { create(:user) } + + it "returns 401 unauthorized" do + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe "#delete" do + let(:organisation) { user.organisation } + + before do + delete "/organisations/#{organisation.id}/delete" + end + + context "with a data provider user" do + let(:user) { create(:user) } + + it "returns 401 unauthorized" do + expect(response).to have_http_status(:unauthorized) + end + end + end end context "with a support user" do @@ -1431,6 +1523,89 @@ RSpec.describe OrganisationsController, type: :request do end end + describe "#show" do + let(:organisation) { create(:organisation) } + + before do + get "/organisations/#{organisation.id}", headers:, params: {} + end + + context "with an active organisation" do + it "does not render delete this organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).not_to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation") + end + + it "does not render informative text about deleting the organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).not_to have_content("This organisation was active in an open or editable collection year, and cannot be deleted.") + end + end + + context "with an inactive organisation" do + let(:organisation) { create(:organisation, active: false) } + + it "renders delete this organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation") + end + + context "and associated lettings logs in editable collection period" do + before do + create(:lettings_log, owning_organisation: organisation) + get "/organisations/#{organisation.id}" + end + + it "does not render delete this organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).not_to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation") + end + + it "adds informative text about deleting the organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("This organisation was active in an open or editable collection year, and cannot be deleted.") + end + end + + context "and associated sales logs in editable collection period" do + before do + create(:sales_log, owning_organisation: organisation) + get "/organisations/#{organisation.id}" + end + + it "does not render delete this organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).not_to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation") + end + + it "adds informative text about deleting the organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("This organisation was active in an open or editable collection year, and cannot be deleted.") + end + end + end + + context "with merged organisation" do + before do + organisation.update!(merge_date: Time.zone.yesterday) + get "/organisations/#{organisation.id}", headers:, params: {} + end + + it "renders delete this organisation" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_link("Delete this organisation", href: "/organisations/#{organisation.id}/delete-confirmation") + end + end + end + describe "#update" do let(:params) { { id: organisation.id, organisation: { active:, rent_periods: [], all_rent_periods: [] } } } @@ -1581,6 +1756,79 @@ RSpec.describe OrganisationsController, type: :request do end end + describe "#delete-confirmation" do + let(:organisation) { create(:organisation, active: false) } + + before do + get "/organisations/#{organisation.id}/delete-confirmation" + end + + it "shows the correct title" do + expect(page.find("h1").text).to include "Are you sure you want to delete this organisation?" + end + + it "shows a warning to the user" do + expect(page).to have_selector(".govuk-warning-text", text: "You will not be able to undo this action") + end + + it "shows a button to delete the selected organisation" do + expect(page).to have_selector("form.button_to button", text: "Delete this organisation") + end + + it "the delete organisation button submits the correct data to the correct path" do + form_containing_button = page.find("form.button_to") + + expect(form_containing_button[:action]).to eq delete_organisation_path(organisation) + expect(form_containing_button).to have_field "_method", type: :hidden, with: "delete" + end + + it "shows a cancel link with the correct style" do + expect(page).to have_selector("a.govuk-button--secondary", text: "Cancel") + end + + it "shows cancel link that links back to the organisation page" do + expect(page).to have_link(text: "Cancel", href: organisation_path(organisation)) + end + end + + describe "#delete" do + let(:organisation) { create(:organisation, active: false) } + let!(:org_user) { create(:user, active: false, organisation:) } + let!(:scheme) { create(:scheme, owning_organisation: organisation) } + let!(:location) { create(:location, scheme:) } + + before do + scheme.scheme_deactivation_periods << SchemeDeactivationPeriod.new(deactivation_date: Time.zone.yesterday) + location.location_deactivation_periods << LocationDeactivationPeriod.new(deactivation_date: Time.zone.yesterday) + delete "/organisations/#{organisation.id}/delete" + end + + it "deletes the organisation and related resources" do + organisation.reload + expect(organisation.status).to eq(:deleted) + expect(organisation.discarded_at).not_to be nil + expect(location.reload.status).to eq(:deleted) + expect(location.discarded_at).not_to be nil + expect(scheme.reload.status).to eq(:deleted) + expect(scheme.discarded_at).not_to be nil + expect(org_user.reload.status).to eq(:deleted) + expect(org_user.discarded_at).not_to be nil + end + + it "redirects to the organisations list and displays a notice that the organisation has been deleted" do + expect(response).to redirect_to organisations_path + follow_redirect! + expect(page).to have_selector(".govuk-notification-banner--success") + expect(page).to have_selector(".govuk-notification-banner--success", text: "#{organisation.name} has been deleted.") + end + + it "does not display the deleted organisation" do + expect(response).to redirect_to organisations_path + follow_redirect! + expect(page).not_to have_content("Organisation to delete") + end + end + context "when they view the lettings logs tab" do let(:tenancycode) { "42" }