Update validations and saving organisations

app/controllers/merge_requests_controller.rb

@@ -14,17 +14,14 @@ class MergeRequestsController < ApplicationController
   end
 
   def update_organisations
-    if @merge_request.merging_organisation_ids
+    merge_request_organisation = MergeRequestOrganisation.new(merge_request_organisation_params)
-      @merge_request.merging_organisation_ids << params[:merge_request][:merging_organisation].to_i
-    else
-      @merge_request.merging_organisation_ids = [params[:merge_request][:merging_organisation]]
-    end
     @answer_options = organisations_answer_options
-    @merging_organisations_list = [@merge_request.requesting_organisation] + @merge_request.merging_organisations
+    if merge_request_organisation.save
-    if @merge_request.save
+      @merge_request.reload
       @merging_organisations_list = [@merge_request.requesting_organisation] + @merge_request.merging_organisations
-      render "organisations"
+      render :organisations
     else
+      @merging_organisations_list = [@merge_request.requesting_organisation] + @merge_request.merging_organisations
       render :organisations, status: :unprocessable_entity
     end
   end
@@ -47,6 +44,10 @@ private
     merge_params
   end
 
+  def merge_request_organisation_params
+    { merge_request: @merge_request, merging_organisation: Organisation.find(params[:merge_request][:merging_organisation]) }
+  end
+
   def find_resource
     @merge_request = MergeRequest.find(params[:merge_request_id])
   end

app/models/merge_request.rb

@@ -2,18 +2,4 @@ class MergeRequest < ApplicationRecord
   belongs_to :requesting_organisation, class_name: "Organisation"
   has_many :merge_request_organisations
   has_many :merging_organisations, through: :merge_request_organisations, source: :merging_organisation
-
-  validate :validate_merging_organisations
-
-private
-
-  def validate_merging_organisations
-    if MergeRequest.where(requesting_organisation_id: merging_organisation_ids).count.positive?
-      errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
-    end
-
-    if merging_organisation_ids&.any? { |org_id| MergeRequest.where("merging_organisation_ids @> ARRAY[?]", org_id).exists? }
-      errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
-    end
-  end
 end

app/models/merge_request_organisation.rb

@@ -3,6 +3,25 @@ class MergeRequestOrganisation < ApplicationRecord
   belongs_to :merging_organisation, class_name: "Organisation"
   validates :merge_request_id, presence: { message: I18n.t("validations.organisation.stock_owner.blank") }
   validates :merging_organisation_id, presence: { message: I18n.t("validations.organisation.managing_agent.blank") }
+  validate :validate_merging_organisations
 
   has_paper_trail
+
+private
+
+  def validate_merging_organisations
+    if MergeRequestOrganisation.where(merge_request_id:, merging_organisation:).count.positive?
+      errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
+    end
+
+    if MergeRequestOrganisation.where.not(merge_request_id:).where(merging_organisation_id: merging_organisation.id).count.positive?
+      errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
+      merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
+    end
+
+    if MergeRequest.where(requesting_organisation: merging_organisation).count.positive?
+      errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
+      merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
+    end
+  end
 end

spec/requests/merge_requests_controller_spec.rb

@@ -65,7 +65,8 @@ RSpec.describe MergeRequestsController, type: :request do
         let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
 
         before do
-          MergeRequest.create!(requesting_organisation_id: other_organisation.id, merging_organisation_ids: [another_organisation.id])
+          existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id)
+          MergeRequestOrganisation.create!(merge_request_id: existing_merge_request.id, merging_organisation_id: another_organisation.id)
           patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
         end
 
@@ -76,6 +77,21 @@ RSpec.describe MergeRequestsController, type: :request do
           expect(page).to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge"))
         end
       end
+
+      context "when the user selects an organisation that is a part of current merge" do
+        let(:another_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") }
+        let(:params) { { merge_request: { merging_organisation: another_organisation.id } } }
+
+        before do
+          merge_request.merging_organisations << another_organisation
+          patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
+        end
+
+        it "does not update the merge request" do
+          merge_request.reload
+          expect(merge_request.merging_organisations.count).to eq(1)
+        end
+      end
     end
   end
 end