diff --git a/app/controllers/organisation_relationships_controller.rb b/app/controllers/organisation_relationships_controller.rb index f731209a8..590e41a79 100644 --- a/app/controllers/organisation_relationships_controller.rb +++ b/app/controllers/organisation_relationships_controller.rb @@ -9,108 +9,106 @@ class OrganisationRelationshipsController < ApplicationController housing_providers = organisation.housing_providers unpaginated_filtered_housing_providers = filtered_collection(housing_providers, search_term) organisations = Organisation.where.not(id: @organisation.id).pluck(:id, :name) - respond_to do |format| - format.html do - @pagy, @housing_providers = pagy(unpaginated_filtered_housing_providers) - @organisations = organisations - @searched = search_term.presence - @total_count = housing_providers.size - render "organisation_relationships/housing_providers", layout: "application" - end - end + respond_to :html + @pagy, @housing_providers = pagy(unpaginated_filtered_housing_providers) + @organisations = organisations + @searched = search_term.presence + @total_count = housing_providers.size end def managing_agents managing_agents = organisation.managing_agents unpaginated_filtered_managing_agents = filtered_collection(managing_agents, search_term) organisations = Organisation.where.not(id: @organisation.id).pluck(:id, :name) - respond_to do |format| - format.html do - @pagy, @managing_agents = pagy(unpaginated_filtered_managing_agents) - @organisations = organisations - @searched = search_term.presence - @total_count = managing_agents.size - render "organisation_relationships/managing_agents", layout: "application" - end - end + respond_to :html + @pagy, @managing_agents = pagy(unpaginated_filtered_managing_agents) + @organisations = organisations + @searched = search_term.presence + @total_count = managing_agents.size end def add_housing_provider @organisations = Organisation.where.not(id: @organisation.id).pluck(:id, :name) - respond_to do |format| - format.html do - render "organisation_relationships/add_housing_provider", layout: "application" - end - end + respond_to :html end def add_managing_agent @organisations = Organisation.where.not(id: @organisation.id).pluck(:id, :name) - respond_to do |format| - format.html do - render "organisation_relationships/add_managing_agent", layout: "application" - end - end + respond_to :html end def create_housing_provider - child_organisation_id = @organisation.id - parent_organisation_id = related_organisation_id + child_organisation = @organisation relationship_type = OrganisationRelationship::OWNING - if related_organisation_id.empty? + if params[:organisation][:related_organisation_id].empty? @organisation.errors.add :related_organisation_id, "You must choose a housing provider" - @organisations = Organisation.where.not(id: child_organisation_id).pluck(:id, :name) - render "organisation_relationships/add_housing_provider" - return - elsif OrganisationRelationship.exists?(child_organisation_id:, parent_organisation_id:, relationship_type:) - @organisation.errors.add :related_organisation_id, "You have already added this housing provider" - @organisations = Organisation.where.not(id: child_organisation_id).pluck(:id, :name) + @organisations = Organisation.where.not(id: child_organisation.id).pluck(:id, :name) render "organisation_relationships/add_housing_provider" return + else + parent_organisation = related_organisation + if OrganisationRelationship.exists?(child_organisation:, parent_organisation:, relationship_type:) + @organisation.errors.add :related_organisation_id, "You have already added this housing provider" + @organisations = Organisation.where.not(id: child_organisation.id).pluck(:id, :name) + render "organisation_relationships/add_housing_provider" + return + end end - create!(child_organisation_id:, parent_organisation_id:, relationship_type:) - redirect_to housing_providers_organisation_path(related_organisation_id:) + create!(child_organisation:, parent_organisation:, relationship_type:) + flash[:notice] = "#{related_organisation.name} is now one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} housing providers" + redirect_to housing_providers_organisation_path end def create_managing_agent - parent_organisation_id = @organisation.id - child_organisation_id = related_organisation_id + parent_organisation = @organisation relationship_type = OrganisationRelationship::MANAGING - - if related_organisation_id.empty? + if params[:organisation][:related_organisation_id].empty? @organisation.errors.add :related_organisation_id, "You must choose a managing agent" - @organisations = Organisation.where.not(id: parent_organisation_id).pluck(:id, :name) - render "organisation_relationships/add_managing_agent" - return - elsif OrganisationRelationship.exists?(child_organisation_id:, parent_organisation_id:, relationship_type:) - @organisation.errors.add :related_organisation_id, "You have already added this managing agent" - @organisations = Organisation.where.not(id: parent_organisation_id).pluck(:id, :name) + @organisations = Organisation.where.not(id: parent_organisation.id).pluck(:id, :name) render "organisation_relationships/add_managing_agent" return + else + child_organisation = related_organisation + if OrganisationRelationship.exists?(child_organisation:, parent_organisation:, relationship_type:) + @organisation.errors.add :related_organisation_id, "You have already added this managing agent" + @organisations = Organisation.where.not(id: parent_organisation.id).pluck(:id, :name) + render "organisation_relationships/add_managing_agent" + return + end end + create!(child_organisation:, parent_organisation:, relationship_type:) + flash[:notice] = "#{related_organisation.name} is now one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} managing agents" + redirect_to managing_agents_organisation_path + end + + def remove_housing_provider + @target_organisation_id = target_organisation.id + end - create!(child_organisation_id:, parent_organisation_id:, relationship_type:) - redirect_to managing_agents_organisation_path(related_organisation_id:) + def delete_housing_provider + organisation_relationship_to_remove = OrganisationRelationship.find_by!(child_organisation: @organisation, parent_organisation: target_organisation, relationship_type: OrganisationRelationship::OWNING) + organisation_relationship_to_remove.destroy! + flash[:notice] = "#{target_organisation.name} is no longer one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} housing providers" + redirect_to housing_providers_organisation_path end private - def create!(child_organisation_id:, parent_organisation_id:, relationship_type:) - @resource = OrganisationRelationship.new(child_organisation_id:, parent_organisation_id:, relationship_type:) + def create!(child_organisation:, parent_organisation:, relationship_type:) + @resource = OrganisationRelationship.new(child_organisation:, parent_organisation:, relationship_type:) @resource.save! end - def create(child_organisation_id, parent_organisation_id, relationship_type) - @resource = OrganisationRelationship.new(child_organisation_id:, parent_organisation_id:, relationship_type:) - @resource.save! + def organisation + @organisation ||= Organisation.find(params[:id]) end - def related_organisation_id - params["organisation"]["related_organisation_id"] + def related_organisation + @related_organisation ||= Organisation.find(params[:organisation][:related_organisation_id]) end - def organisation - @organisation ||= Organisation.find(params[:id]) + def target_organisation + @target_organisation ||= Organisation.find(params[:target_organisation_id]) end def search_term diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index e99ee4a26..07c125a8d 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -40,8 +40,8 @@ module NavigationItemsHelper NavigationItem.new("Schemes", "/organisations/#{current_organisation_id}/schemes", subnav_supported_housing_schemes_path?(path)), NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)), NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)), - (NavigationItem.new("Housing providers", housing_providers_organisation_path(current_user.organisation), housing_providers_path?(path)) if FeatureToggle.managing_owning_enabled?), - (NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?), + (NavigationItem.new("Housing providers", housing_providers_organisation_path(current_organisation_id), housing_providers_path?(path)) if FeatureToggle.managing_owning_enabled?), + (NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?), ].compact else [ @@ -49,8 +49,8 @@ module NavigationItemsHelper FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(path)) : nil, NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)), NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)), - (NavigationItem.new("Housing providers", housing_providers_organisation_path(current_user.organisation), housing_providers_path?(path)) if FeatureToggle.managing_owning_enabled?), - (NavigationItem.new("Managing agents", managing_agents_organisation_path(current_user.organisation), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?), + (NavigationItem.new("Housing providers", housing_providers_organisation_path(current_organisation_id), housing_providers_path?(path)) if FeatureToggle.managing_owning_enabled?), + (NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)) if FeatureToggle.managing_owning_enabled?), ].compact end end diff --git a/app/views/organisation_relationships/_housing_provider_list.erb b/app/views/organisation_relationships/_housing_provider_list.erb index e43766c5b..0aa2244f6 100644 --- a/app/views/organisation_relationships/_housing_provider_list.erb +++ b/app/views/organisation_relationships/_housing_provider_list.erb @@ -12,7 +12,7 @@ scope: "row", class: "govuk-!-text-align-right", }) do %> - <%= govuk_link_to("Remove", "housing-providers/#{housing_provider.id}") %> + <%= govuk_link_to("Remove", housing_providers_remove_organisation_path(target_organisation_id: housing_provider.id)) %> <% end %> <% end %> <% end %> diff --git a/app/views/organisation_relationships/housing_providers.html.erb b/app/views/organisation_relationships/housing_providers.html.erb index 9ff96e68d..3598cb280 100644 --- a/app/views/organisation_relationships/housing_providers.html.erb +++ b/app/views/organisation_relationships/housing_providers.html.erb @@ -1,29 +1,8 @@ <% item_label = format_label(@pagy.count, "housing providers") %> - -<% if current_user.data_coordinator? %> - <% if params["related_organisation_id"] %> - <%= govuk_notification_banner( - title_text: "Success", - success: true, title_heading_level: 3, - title_id: "swanky-notifications" - ) do |notification_banner| - notification_banner.heading(text: "#{Organisation.find(params['related_organisation_id']).name} is now one of your housing providers") - end %> - <% end %> -<% end %> <% if current_user.support? %> <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> <%= render SubNavigationComponent.new(items: secondary_items(request.path, @organisation.id)) %>

Housing Providers

- <% if params["related_organisation_id"] %> - <%= govuk_notification_banner( - title_text: "Success", - success: true, title_heading_level: 3, - title_id: "swanky-notifications" - ) do |notification_banner| - notification_banner.heading(text: "#{Organisation.find(params['related_organisation_id']).name} is now one of this organisation's housing providers") - end %> - <% end %>

This organisation can submit logs for its housing providers.

<% if @total_count == 0 %>

This organisation does not currently have any housing providers.

@@ -35,10 +14,7 @@

You do not currently have any housing providers.

<% end %> <% end %> - -<% if current_user.support? %> - <%= govuk_button_link_to "Add a housing provider", housing_providers_add_organisation_path(organisation_id: @organisation.id), html: { method: :get } %> - <% elsif current_user.data_coordinator? %> +<% if current_user.support? || current_user.data_coordinator? %> <%= govuk_button_link_to "Add a housing provider", housing_providers_add_organisation_path, html: { method: :get } %> <% end %> <% if @total_count != 0 %> diff --git a/app/views/organisation_relationships/managing_agents.html.erb b/app/views/organisation_relationships/managing_agents.html.erb index f09646454..db423366c 100644 --- a/app/views/organisation_relationships/managing_agents.html.erb +++ b/app/views/organisation_relationships/managing_agents.html.erb @@ -38,9 +38,7 @@

This organisation does not currently have any managing agents.

<% end %> <% end %> -<% if current_user.support? %> - <%= govuk_button_link_to "Add a managing agent", managing_agents_add_organisation_path(organisation_id: @organisation.id), html: { method: :get } %> -<% elsif current_user.data_coordinator? %> +<% if current_user.support? || current_user.data_coordinator? %> <%= govuk_button_link_to "Add a managing agent", managing_agents_add_organisation_path, html: { method: :get } %> <% end %> <% if @total_count != 0 %> diff --git a/app/views/organisation_relationships/remove_housing_provider.html.erb b/app/views/organisation_relationships/remove_housing_provider.html.erb new file mode 100644 index 000000000..301392753 --- /dev/null +++ b/app/views/organisation_relationships/remove_housing_provider.html.erb @@ -0,0 +1,21 @@ +<%= form_with url: housing_providers_organisation_path(target_organisation_id: @target_organisation.id), method: "delete", local: true do |f| %> + <% if current_user.support? %> + <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> + <%= render SubNavigationComponent.new(items: secondary_items(request.path, @organisation.id)) %> +

Remove Housing Provider

+ <% end %> + <% if current_user.support? %> + <%= govuk_back_link(href: :back) %> + <%= render partial: "organisations/headings", locals: { main: "You are removing ‘#{@target_organisation.name}’ from this organisation's housing providers", sub: nil } %> + <% else %> + <% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> + <% end %> + <%= render partial: "organisations/headings", locals: { main: "You are removing ‘#{@target_organisation.name}’ from your organisation's housing providers", sub: nil } %> + <% end %> + <%= govuk_warning_text text: "You will no longer be able to submit logs for #{@target_organisation.name}" %> +
+ <%= f.govuk_submit "Confirm" %> + <%= govuk_button_link_to "Cancel", housing_providers_organisation_path(current_user.organisation), html: { method: :get }, secondary: true %> +
+<% end %> diff --git a/config/routes.rb b/config/routes.rb index 838887f1b..9af179103 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -81,7 +81,9 @@ Rails.application.routes.draw do get "schemes", to: "organisations#schemes" get "housing-providers", to: "organisation_relationships#housing_providers" get "housing-providers/add", to: "organisation_relationships#add_housing_provider" + get "housing-providers/remove", to: "organisation_relationships#remove_housing_provider" post "housing-providers", to: "organisation_relationships#create_housing_provider" + delete "housing-providers", to: "organisation_relationships#delete_housing_provider" get "managing-agents", to: "organisation_relationships#managing_agents" get "managing-agents/add", to: "organisation_relationships#add_managing_agent" post "managing-agents", to: "organisation_relationships#create_managing_agent" diff --git a/db/schema.rb b/db/schema.rb index 02c7f3d90..ebe7598a5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -359,8 +359,6 @@ ActiveRecord::Schema[7.0].define(version: 2022_10_19_082625) do t.integer "hholdcount" t.integer "age3" t.integer "age3_known" - t.integer "income1" - t.integer "income1nk" t.integer "age4" t.integer "age4_known" t.integer "age5" @@ -369,6 +367,8 @@ ActiveRecord::Schema[7.0].define(version: 2022_10_19_082625) do t.integer "age6_known" t.string "la" t.integer "la_known" + t.integer "income1" + t.integer "income1nk" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["managing_organisation_id"], name: "index_sales_logs_on_managing_organisation_id" t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id" diff --git a/spec/requests/organisation_relationships_controller_spec.rb b/spec/requests/organisation_relationships_controller_spec.rb index 9fdb01ec9..c00c20fc8 100644 --- a/spec/requests/organisation_relationships_controller_spec.rb +++ b/spec/requests/organisation_relationships_controller_spec.rb @@ -158,7 +158,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do it "redirects to the organisation list" do request - expect(response).to redirect_to("/organisations/#{organisation.id}/housing-providers?related_organisation_id=#{housing_provider.id}") + expect(response).to redirect_to("/organisations/#{organisation.id}/housing-providers") end end @@ -186,7 +186,30 @@ RSpec.describe OrganisationRelationshipsController, type: :request do it "redirects to the organisation list" do request - expect(response).to redirect_to("/organisations/#{organisation.id}/managing-agents?related_organisation_id=#{managing_agent.id}") + expect(response).to redirect_to("/organisations/#{organisation.id}/managing-agents") + end + end + + describe "organisation_relationships#delete_housing_provider" do + let!(:housing_provider) { FactoryBot.create(:organisation) } + let(:params) do + { + "target_organisation_id": housing_provider.id, + } + end + let(:request) { delete "/organisations/#{organisation.id}/housing-providers", headers:, params: } + + before do + FactoryBot.create(:organisation_relationship, :owning, child_organisation: organisation, parent_organisation: housing_provider) + end + + it "deletes the new organisation relationship" do + expect { request }.to change(OrganisationRelationship, :count).by(-1) + end + + it "redirects to the organisation list" do + request + expect(response).to redirect_to("/organisations/#{organisation.id}/housing-providers") end end end @@ -337,7 +360,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do it "redirects to the organisation list" do request - expect(response).to redirect_to("/organisations/#{organisation.id}/housing-providers?related_organisation_id=#{housing_provider.id}") + expect(response).to redirect_to("/organisations/#{organisation.id}/housing-providers") end end @@ -365,7 +388,30 @@ RSpec.describe OrganisationRelationshipsController, type: :request do it "redirects to the organisation list" do request - expect(response).to redirect_to("/organisations/#{organisation.id}/managing-agents?related_organisation_id=#{managing_agent.id}") + expect(response).to redirect_to("/organisations/#{organisation.id}/managing-agents") + end + end + + describe "organisation_relationships#delete_housing_provider" do + let!(:housing_provider) { FactoryBot.create(:organisation) } + let(:params) do + { + "target_organisation_id": housing_provider.id, + } + end + let(:request) { delete "/organisations/#{organisation.id}/housing-providers", headers:, params: } + + before do + FactoryBot.create(:organisation_relationship, :owning, child_organisation: organisation, parent_organisation: housing_provider) + end + + it "deletes the new organisation relationship" do + expect { request }.to change(OrganisationRelationship, :count).by(-1) + end + + it "redirects to the organisation list" do + request + expect(response).to redirect_to("/organisations/#{organisation.id}/housing-providers") end end @@ -400,6 +446,10 @@ RSpec.describe OrganisationRelationshipsController, type: :request do expect(page).not_to have_content(other_org_housing_provider.name) end + it "shows remove link(s)" do + expect(response.body).to include("Remove") + end + it "shows the pagination count" do expect(page).to have_content("1 total housing providers") end