diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 11241576f..65cbbd8ba 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -29,13 +29,17 @@ class MergeRequestsController < ApplicationController def update_organisations if @merge_request.merging_organisation_ids @merge_request.merging_organisation_ids << params[:merge_request][:merging_organisation].to_i - @merge_request.save! else - @merge_request.update!(merging_organisation_ids: [params[:merge_request][:merging_organisation]]) + @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 - render "organisations" + if @merge_request.save + @merging_organisations_list = [@merge_request.requesting_organisation] + @merge_request.merging_organisations + render "organisations" + else + render :organisations, status: :unprocessable_entity + end end private diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1e0f888f6..44c588155 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -2,8 +2,21 @@ class MergeRequest < ApplicationRecord belongs_to :requesting_organisation, class_name: "Organisation" # has_many :merging_organisations, class_name: "Organisation", primary_key: "merging_organisation_ids", foreign_key: "id" # default_scope -> { select(column_names + ["merging_organisation_ids"]) } + validate :validate_merging_organisations def merging_organisations Organisation.where(id: merging_organisation_ids) end + +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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 074a00147..8c5858076 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -503,6 +503,8 @@ en: discounted_ownership_value: "Mortgage, deposit, and grant total must equal %{value_with_discount}" monthly_rent: higher_than_expected: "Basic monthly rent must be between £0.00 and £9,999.00" + merge_request: + organisation_part_of_another_merge: "This organisation is part of another merge - select a different one" soft_validations: net_income: diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 66a2cbb75..b9f963278 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe MergeRequestsController, type: :request do let(:organisation) { user.organisation } - let!(:other_organisation) { FactoryBot.create(:organisation) } + let!(:other_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } let(:headers) { { "Accept" => "text/html" } } let(:page) { Capybara::Node::Simple.new(response.body) } let(:user) { FactoryBot.create(:user, :data_coordinator) } @@ -30,17 +30,51 @@ RSpec.describe MergeRequestsController, type: :request do describe "#update_organisations" do let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } - before do - other_organisation.update!(name: "Other Test Org") - patch "/merge-request/#{merge_request.id}/organisations", headers:, params: + context "when updating a merge request with a new organisation" do + before do + patch "/merge-request/#{merge_request.id}/organisations", headers:, params: + end + + it "updates the merge request" do + merge_request.reload + expect(merge_request.merging_organisations.count).to eq(1) + expect(page).to have_content("Test Org") + expect(page).to have_content("Other Test Org") + expect(page).to have_link("Remove") + end end - it "updates the merge request" do - merge_request.reload - expect(merge_request.merging_organisations.count).to eq(1) - expect(page).to have_content("Test Org") - expect(page).to have_content("Other Test Org") - expect(page).to have_link("Remove") + context "when the user selects an organisation that requested another merge" do + let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } + + before do + MergeRequest.create!(requesting_organisation_id: other_organisation.id) + 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(0) + expect(response).to have_http_status(:unprocessable_entity) + 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 another merge" do + let(:another_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } + 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]) + 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(0) + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge")) + end end end end