feat: simplify controller behaviour further

app/controllers/organisation_relationships_controller.rb

@@ -5,13 +5,19 @@ class OrganisationRelationshipsController < ApplicationController
   before_action :authenticate_user!
   before_action :authenticate_scope!
 
+  before_action :organisations
+  before_action :target_organisation, only: %i[
+    remove_housing_provider
+    remove_managing_agent
+    delete_housing_provider
+    delete_managing_agent
+  ]
+
   def housing_providers
     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 :html
+
     @pagy, @housing_providers = pagy(unpaginated_filtered_housing_providers)
-    @organisations = organisations
     @searched = search_term.presence
     @total_count = housing_providers.size
   end
@@ -19,80 +25,60 @@ class OrganisationRelationshipsController < ApplicationController
   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 :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)
-    @organisation_relationship = OrganisationRelationship.new
-    respond_to :html
+    @organisation_relationship = organisation.parent_organisation_relationships.new
   end
 
   def add_managing_agent
-    @organisations = Organisation.where.not(id: @organisation.id).pluck(:id, :name)
-    @organisation_relationship = OrganisationRelationship.new
-    respond_to :html
+    @organisation_relationship = organisation.child_organisation_relationships.new
   end
 
   def create_housing_provider
-    child_organisation = @organisation
-    if params[:organisation_relationship].present? && params[:organisation_relationship][:parent_organisation_id].present?
-      parent_organisation = @parent_organisation ||= Organisation.find(params[:organisation_relationship][:parent_organisation_id])
-    end
-    @organisation_relationship = OrganisationRelationship.new(child_organisation:, parent_organisation:)
-    if @organisation_relationship.save(context: :housing_provider)
-      flash[:notice] = "#{parent_organisation.name} is now one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} housing providers"
+    @organisation_relationship = organisation.parent_organisation_relationships.new(organisation_relationship_params)
+    if @organisation_relationship.save
+      flash[:notice] = "#{@organisation_relationship.parent_organisation.name} is now one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} housing providers"
       redirect_to housing_providers_organisation_path
     else
-      @organisations = Organisation.where.not(id: child_organisation.id).pluck(:id, :name)
+      @organisations = Organisation.where.not(id: organisation.id).pluck(:id, :name)
       render "organisation_relationships/add_housing_provider", status: :unprocessable_entity
     end
   end
 
   def create_managing_agent
-    parent_organisation = @organisation
-    if params[:organisation_relationship].present? && params[:organisation_relationship][:child_organisation_id].present?
-      child_organisation = @child_organisation ||= Organisation.find(params[:organisation_relationship][:child_organisation_id])
-    end
-    @organisation_relationship = OrganisationRelationship.new(child_organisation:, parent_organisation:)
-    if @organisation_relationship.save(context: :managing_agent)
-      flash[:notice] = "#{child_organisation.name} is now one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} managing agents"
+    @organisation_relationship = organisation.child_organisation_relationships.new(organisation_relationship_params)
+    if @organisation_relationship.save
+      flash[:notice] = "#{@organisation_relationship.child_organisation.name} is now one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} managing agents"
       redirect_to managing_agents_organisation_path
     else
-      @organisations = Organisation.where.not(id: parent_organisation.id).pluck(:id, :name)
+      @organisations = Organisation.where.not(id: organisation.id).pluck(:id, :name)
       render "organisation_relationships/add_managing_agent", status: :unprocessable_entity
     end
   end
 
-  def remove_housing_provider
-    @target_organisation_id = target_organisation.id
-  end
+  def remove_housing_provider; end
 
   def delete_housing_provider
-    relationship = OrganisationRelationship.find_by!(
-      child_organisation: @organisation,
+    OrganisationRelationship.find_by!(
+      child_organisation: organisation,
       parent_organisation: target_organisation,
-    )
-    relationship.destroy!
+    ).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
 
-  def remove_managing_agent
-    @target_organisation_id = target_organisation.id
-  end
+  def remove_managing_agent; end
 
   def delete_managing_agent
-    relationship = OrganisationRelationship.find_by!(
-      parent_organisation: @organisation,
+    OrganisationRelationship.find_by!(
+      parent_organisation: organisation,
       child_organisation: target_organisation,
-    )
-    relationship.destroy!
+    ).destroy!
     flash[:notice] = "#{target_organisation.name} is no longer one of #{current_user.data_coordinator? ? 'your' : "this organisation's"} managing agents"
     redirect_to managing_agents_organisation_path
   end
@@ -100,7 +86,23 @@ class OrganisationRelationshipsController < ApplicationController
 private
 
   def organisation
-    @organisation ||= Organisation.find(params[:id])
+    @organisation ||= if current_user.support?
+                        Organisation.find(params[:id])
+                      else
+                        current_user.organisation
+                      end
+  end
+
+  def organisations
+    @organisations ||= Organisation.where.not(id: organisation.id).pluck(:id, :name)
+  end
+
+  def parent_organisation
+    @parent_organisation ||= Organisation.find(params[:organisation_relationship][:parent_organisation_id])
+  end
+
+  def child_organisation
+    @child_organisation ||= Organisation.find(params[:organisation_relationship][:child_organisation_id])
   end
 
   def target_organisation
@@ -111,6 +113,10 @@ private
     params["search"]
   end
 
+  def organisation_relationship_params
+    params.require(:organisation_relationship).permit(:parent_organisation_id, :child_organisation_id)
+  end
+
   def authenticate_scope!
     if current_user.organisation != organisation && !current_user.support?
       render_not_found

app/models/organisation_relationship.rb

@@ -1,11 +1,11 @@
 class OrganisationRelationship < ApplicationRecord
   belongs_to :child_organisation, class_name: "Organisation"
   belongs_to :parent_organisation, class_name: "Organisation"
-  validates :parent_organisation_id, presence: { message: "You must choose a housing provider" }, on: :housing_provider
-  validates :child_organisation_id, presence: { message: "You must choose a managing agent" }, on: :managing_agent
-  validates :parent_organisation_id, uniqueness: { scope: :child_organisation_id, message: "You have already added this housing provider" }, on: :housing_provider
-  validates :child_organisation_id, uniqueness: { scope: :parent_organisation_id, message: "You have already added this managing agent" }, on: :managing_agent
-  validate :validate_housing_provider_owns_stock, on: :housing_provider
+  validates :parent_organisation_id, presence: { message: "You must choose a housing provider" }
+  validates :child_organisation_id, presence: { message: "You must choose a managing agent" }
+  validates :parent_organisation_id, uniqueness: { scope: :child_organisation_id, message: "You have already added this housing provider" }
+  validates :child_organisation_id, uniqueness: { scope: :parent_organisation_id, message: "You have already added this managing agent" }
+  validate :validate_housing_provider_owns_stock
 
 private