From d6e3f634f0f14aa4773b06de06a1eec157073c17 Mon Sep 17 00:00:00 2001 From: Kat Date: Fri, 14 Apr 2023 15:57:26 +0100 Subject: [PATCH] Add validation if the organisation is not selected --- app/controllers/merge_requests_controller.rb | 2 +- app/models/merge_request_organisation.rb | 14 +++++---- config/locales/en.yml | 1 + .../merge_requests_controller_spec.rb | 30 +++++++++++++++++++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 9dd331b46..4b25b1eca 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -64,7 +64,7 @@ private end def merge_request_organisation_params - { merge_request: @merge_request, merging_organisation: Organisation.find(params[:merge_request][:merging_organisation]) } + { merge_request: @merge_request, merging_organisation_id: params[:merge_request][:merging_organisation] } end def other_merging_organisaitions_params diff --git a/app/models/merge_request_organisation.rb b/app/models/merge_request_organisation.rb index 4c9ccd186..abfdda6c1 100644 --- a/app/models/merge_request_organisation.rb +++ b/app/models/merge_request_organisation.rb @@ -1,8 +1,8 @@ class MergeRequestOrganisation < ApplicationRecord belongs_to :merge_request, class_name: "MergeRequest" 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") } + validates :merge_request_id, presence: { message: I18n.t("validations.merge_request.merge_request_id.blank") } + validates :merging_organisation_id, presence: { message: I18n.t("validations.merge_request.merging_organisation_id.blank") } validate :validate_merging_organisations has_paper_trail @@ -10,18 +10,22 @@ class MergeRequestOrganisation < ApplicationRecord private def validate_merging_organisations - if MergeRequestOrganisation.where(merge_request_id:, merging_organisation:).count.positive? + if MergeRequestOrganisation.where(merge_request_id:, merging_organisation_id:).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? + if MergeRequestOrganisation.where.not(merge_request_id:).where(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? + if MergeRequest.where(requesting_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 merging_organisation_id.blank? || !Organisation.where(id: merging_organisation_id).exists? + merge_request.errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_not_selected")) + end end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 8c5858076..b8c035a11 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -505,6 +505,7 @@ en: 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" + organisation_not_selected: "Select an organisation from the search list" soft_validations: net_income: diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 58bd26a61..8865ecaf5 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -105,6 +105,36 @@ RSpec.describe MergeRequestsController, type: :request do expect(merge_request.merging_organisations.count).to eq(1) end end + + context "when the user does not select an organisation" do + let(:params) { { merge_request: { merging_organisation: nil } } } + + before do + 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_not_selected")) + end + end + + context "when the user selects non existent id" do + let(:params) { { merge_request: { merging_organisation: "clearly_not_an_id" } } } + + before do + 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_not_selected")) + end + end end describe "#remove_organisation" do