Browse Source

CLDC-2480 Allow deleting organisations (#2459)

* Add delete confirmation page

* Allow deleting organisation

* Add delete button

* Do not display deleted organisations as an option

* Update policy

* Display informative text and delete org resources

* lint

* Update organisation labels to show when deleted

* Refactor organisation label

* Update more labels

* Remove deleted orgs from schemes and autorize delete_confirmation

* Fix tests
pull/2506/head
kosiakkatrina 6 months ago committed by GitHub
parent
commit
ab4915c2aa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 4
      app/components/lettings_log_summary_component.html.erb
  2. 4
      app/components/sales_log_summary_component.html.erb
  3. 31
      app/controllers/organisations_controller.rb
  4. 12
      app/helpers/organisations_helper.rb
  5. 6
      app/helpers/schemes_helper.rb
  6. 16
      app/models/form/lettings/questions/managing_organisation.rb
  7. 11
      app/models/form/lettings/questions/stock_owner.rb
  8. 11
      app/models/form/sales/questions/managing_organisation.rb
  9. 23
      app/models/form/sales/questions/owning_organisation_id.rb
  10. 12
      app/models/organisation.rb
  11. 21
      app/policies/organisation_policy.rb
  12. 24
      app/views/organisations/delete_confirmation.html.erb
  13. 4
      app/views/organisations/show.html.erb
  14. 1
      config/locales/en.yml
  15. 2
      config/routes.rb
  16. 5
      db/migrate/20240610142812_add_discarded_at_to_organisations.rb
  17. 3
      db/schema.rb
  18. 9
      spec/models/form/lettings/questions/managing_organisation_spec.rb
  19. 24
      spec/models/form/lettings/questions/stock_owner_spec.rb
  20. 4
      spec/models/form/sales/questions/managing_organisation_spec.rb
  21. 9
      spec/models/form/sales/questions/owning_organisation_id_spec.rb
  22. 108
      spec/policies/organisation_policy_spec.rb
  23. 248
      spec/requests/organisations_controller_spec.rb

4
app/components/lettings_log_summary_component.html.erb

@ -33,13 +33,13 @@
<% if log.owning_organisation %>
<div class="app-metadata__item">
<dt class="app-metadata__term">Owned by</dt>
<dd class="app-metadata__definition"><%= log.owning_organisation&.name %></dd>
<dd class="app-metadata__definition"><%= log.owning_organisation&.label %></dd>
</div>
<% end %>
<% if log.managing_organisation %>
<div class="app-metadata__item">
<dt class="app-metadata__term">Managed by</dt>
<dd class="app-metadata__definition"><%= log.managing_organisation&.name %></dd>
<dd class="app-metadata__definition"><%= log.managing_organisation&.label %></dd>
</div>
<% end %>
</dl>

4
app/components/sales_log_summary_component.html.erb

@ -26,13 +26,13 @@
<% if log.owning_organisation %>
<div class="app-metadata__item">
<dt class="app-metadata__term">Owned by</dt>
<dd class="app-metadata__definition"><%= log.owning_organisation&.name %></dd>
<dd class="app-metadata__definition"><%= log.owning_organisation&.label %></dd>
</div>
<% end %>
<% if log.managing_organisation %>
<div class="app-metadata__item">
<dt class="app-metadata__term">Reported by</dt>
<dd class="app-metadata__definition"><%= log.managing_organisation&.name %></dd>
<dd class="app-metadata__definition"><%= log.managing_organisation&.label %></dd>
</div>
<% end %>
</dl>

31
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

12
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?
"<div class=\"app-!-colour-muted\">This organisation was active in an open or editable collection year, and cannot be deleted.</div>".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

6
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

16
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

11
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?

11
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

23
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?

12
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

21
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

24
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 %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<span class="govuk-caption-xl">Delete <%= @organisation.name %></span>
<h1 class="govuk-heading-xl">
<%= content_for(:title) %>
</h1>
<%= govuk_warning_text(text: "You will not be able to undo this action.") %>
<div class="govuk-button-group">
<%= 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 %>
</div>
</div>
</div>

4
app/views/organisations/show.html.erb

@ -52,3 +52,7 @@
<%= govuk_button_link_to "Reactivate this organisation", reactivate_organisation_path(@organisation) %>
</span>
<% end %>
<% if OrganisationPolicy.new(current_user, @organisation).delete? %>
<%= delete_organisation_link(@organisation) %>
<% end %>

1
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:

2
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

5
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

3
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

9
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

24
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

4
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

9
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

108
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

248
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" }

Loading…
Cancel
Save