Browse Source
* Create merge_request table and paths * Save merging organisations wip * Add update organisations * Add merging organisations validation * merge fixes * Update schema to have merge_request_organisations * Add relationships between merge request and organisations * Update validations and saving organisations * Add ability to remove merging orgs from the list * Allow support users to create merge request for any organisation * Update wording in organisations view * Allow adding other merging organisations * Add back button, update content * Add validation if the organisation is not selected * Fix path * Use generic update method * Update validations * fix remove organisation * remove reloads * Update routes * Authenticate scope * PR review changes * Add status, run validations unless the status is unsubmitted and save requesting organisation as a merging organisation as well * PR comments * Display continue button when there are at least 2 merging organisationspull/1562/head
kosiakkatrina
2 years ago
committed by
GitHub
15 changed files with 532 additions and 19 deletions
@ -0,0 +1,86 @@
|
||||
class MergeRequestsController < ApplicationController |
||||
before_action :find_resource, only: %i[update organisations update_organisations remove_merging_organisation] |
||||
before_action :authenticate_user! |
||||
before_action :authenticate_scope!, except: [:create] |
||||
|
||||
def create |
||||
ActiveRecord::Base.transaction do |
||||
@merge_request = MergeRequest.create!(merge_request_params.merge(status: :unsubmitted)) |
||||
MergeRequestOrganisation.create!({ merge_request: @merge_request, merging_organisation: @merge_request.requesting_organisation }) |
||||
end |
||||
redirect_to organisations_merge_request_path(@merge_request) |
||||
rescue ActiveRecord::RecordInvalid |
||||
render_not_found |
||||
end |
||||
|
||||
def organisations |
||||
@answer_options = organisations_answer_options |
||||
end |
||||
|
||||
def update |
||||
if @merge_request.update(merge_request_params) |
||||
redirect_to next_page_path |
||||
else |
||||
render previous_template, status: :unprocessable_entity |
||||
end |
||||
end |
||||
|
||||
def update_organisations |
||||
merge_request_organisation = MergeRequestOrganisation.new(merge_request_organisation_params) |
||||
@answer_options = organisations_answer_options |
||||
if merge_request_organisation.save |
||||
render :organisations |
||||
else |
||||
render :organisations, status: :unprocessable_entity |
||||
end |
||||
end |
||||
|
||||
def remove_merging_organisation |
||||
MergeRequestOrganisation.find_by(merge_request_organisation_params)&.destroy! |
||||
@answer_options = organisations_answer_options |
||||
render :organisations |
||||
end |
||||
|
||||
private |
||||
|
||||
def organisations_answer_options |
||||
answer_options = { "" => "Select an option" } |
||||
|
||||
Organisation.all.pluck(:id, :name).each do |organisation| |
||||
answer_options[organisation[0]] = organisation[1] |
||||
end |
||||
answer_options |
||||
end |
||||
|
||||
def merge_request_params |
||||
merge_params = params.fetch(:merge_request, {}).permit(:requesting_organisation_id, :other_merging_organisations, :status) |
||||
|
||||
if merge_params[:requesting_organisation_id].present? && (current_user.data_coordinator? || current_user.data_provider?) |
||||
merge_params[:requesting_organisation_id] = current_user.organisation.id |
||||
end |
||||
|
||||
merge_params |
||||
end |
||||
|
||||
def merge_request_organisation_params |
||||
{ merge_request: @merge_request, merging_organisation_id: params[:merge_request][:merging_organisation] } |
||||
end |
||||
|
||||
def find_resource |
||||
@merge_request = MergeRequest.find(params[:id]) |
||||
end |
||||
|
||||
def next_page_path |
||||
absorbing_organisation_merge_request_path(@merge_request) |
||||
end |
||||
|
||||
def previous_template |
||||
:organisations |
||||
end |
||||
|
||||
def authenticate_scope! |
||||
if current_user.organisation != @merge_request.requesting_organisation && !current_user.support? |
||||
render_not_found |
||||
end |
||||
end |
||||
end |
@ -0,0 +1,11 @@
|
||||
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 |
||||
|
||||
STATUS = { |
||||
"unsubmitted" => 0, |
||||
"submitted" => 1, |
||||
}.freeze |
||||
enum status: STATUS |
||||
end |
@ -0,0 +1,34 @@
|
||||
class MergeRequestOrganisation < ApplicationRecord |
||||
belongs_to :merge_request, class_name: "MergeRequest" |
||||
belongs_to :merging_organisation, class_name: "Organisation" |
||||
validates :merge_request, presence: { message: I18n.t("validations.merge_request.merge_request_id.blank") } |
||||
validates :merging_organisation, presence: { message: I18n.t("validations.merge_request.merging_organisation_id.blank") } |
||||
validate :validate_merging_organisations |
||||
|
||||
scope :not_unsubmitted, -> { joins(:merge_request).where.not(merge_requests: { status: "unsubmitted" }) } |
||||
scope :with_merging_organisation, ->(merging_organisation) { where(merging_organisation:) } |
||||
|
||||
has_paper_trail |
||||
|
||||
private |
||||
|
||||
def validate_merging_organisations |
||||
if MergeRequestOrganisation.where(merge_request:, merging_organisation:).count.positive? |
||||
errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge")) |
||||
end |
||||
|
||||
if MergeRequestOrganisation.not_unsubmitted.with_merging_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 |
||||
|
||||
if MergeRequest.not_unsubmitted.where.not(id: merge_request_id).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 |
||||
|
||||
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 |
@ -0,0 +1,49 @@
|
||||
<% content_for :before_content do %> |
||||
|
||||
<% title = "Tell us if your organisation is merging" %> |
||||
<% content_for :title, title %> |
||||
<%= govuk_back_link href: merge_request_organisation_path(id: @merge_request.requesting_organisation_id) %> |
||||
<% end %> |
||||
|
||||
<h2 class="govuk-heading-l">Which organisations are merging?</h2> |
||||
|
||||
<div class="govuk-grid-row"> |
||||
<div class="govuk-grid-column-two-thirds-from-desktop"> |
||||
<p class="govuk-body">Add all organisations to be merged - we have already added your own.</p> |
||||
|
||||
<%= form_with model: @merge_request, url: organisations_merge_request_path, method: :patch do |f| %> |
||||
<%= f.govuk_error_summary %> |
||||
<p class="govuk-body">Start typing to search</p> |
||||
<%= render partial: "organisation_relationships/related_organisation_select_question", locals: { |
||||
field: :merging_organisation, |
||||
question: Form::Question.new("", { "answer_options" => @answer_options }, nil), |
||||
f:, |
||||
} %> |
||||
<%= f.govuk_submit "Add organisation", classes: "govuk-button--secondary" %> |
||||
<%= govuk_table do |table| %> |
||||
<% @merge_request.merging_organisations.each do |merging_organisation| %> |
||||
<%= table.body do |body| %> |
||||
<%= body.row do |row| %> |
||||
<% row.cell(text: merging_organisation.name) %> |
||||
<% row.cell(html_attributes: { |
||||
scope: "row", |
||||
class: "govuk-!-text-align-right", |
||||
}) do %> |
||||
<% if @merge_request.requesting_organisation != merging_organisation %> |
||||
<%= govuk_link_to("Remove", organisations_remove_merge_request_path(merge_request: { merging_organisation: merging_organisation.id })) %> |
||||
<% end %> |
||||
<% end %> |
||||
<% end %> |
||||
<% end %> |
||||
<% end %> |
||||
<% end %> |
||||
<% end %> |
||||
<%= form_with model: @merge_request, url: merge_request_path(id: @merge_request.id), method: :patch do |f| %> |
||||
<%= govuk_details(summary_text: "I cannot find an organisation on the list") do %> |
||||
<%= f.govuk_text_area :other_merging_organisations, label: { text: "Other organisations" }, hint: { text: "List other organisations that are part of the merge but not registered on CORE." }, rows: 9 %> |
||||
<% end %> |
||||
<% if @merge_request.merging_organisations.count > 1 %> |
||||
<%= f.govuk_submit "Continue" %> |
||||
<% end %> |
||||
<% end %> |
||||
</div> |
@ -0,0 +1,9 @@
|
||||
class AddMergeRequestsTable < ActiveRecord::Migration[7.0] |
||||
def change |
||||
create_table :merge_requests do |t| |
||||
t.integer :requesting_organisation_id |
||||
t.text :other_merging_organisations |
||||
t.timestamps |
||||
end |
||||
end |
||||
end |
@ -0,0 +1,9 @@
|
||||
class AddMergeOrganisations < ActiveRecord::Migration[7.0] |
||||
def change |
||||
create_table :merge_request_organisations do |t| |
||||
t.integer :merge_request_id, foreign_key: true |
||||
t.integer :merging_organisation_id, foreign_key: true |
||||
t.timestamps |
||||
end |
||||
end |
||||
end |
@ -0,0 +1,5 @@
|
||||
class AddStatusToMergeRequest < ActiveRecord::Migration[7.0] |
||||
def change |
||||
add_column :merge_requests, :status, :integer |
||||
end |
||||
end |
@ -0,0 +1,278 @@
|
||||
require "rails_helper" |
||||
|
||||
RSpec.describe MergeRequestsController, type: :request do |
||||
let(:organisation) { user.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) } |
||||
let(:support_user) { FactoryBot.create(:user, :support, organisation:) } |
||||
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation) } |
||||
let(:other_merge_request) { MergeRequest.create!(requesting_organisation: other_organisation) } |
||||
|
||||
context "when user is signed in with a data coordinator user" do |
||||
before do |
||||
sign_in user |
||||
end |
||||
|
||||
describe "#organisations" do |
||||
let(:params) { { merge_request: { requesting_organisation_id: "9", status: "unsubmitted" } } } |
||||
|
||||
context "when creating a new merge request" do |
||||
before do |
||||
organisation.update!(name: "Test Org") |
||||
post "/merge-request", headers:, params: |
||||
end |
||||
|
||||
it "creates merge request with requesting organisation" do |
||||
follow_redirect! |
||||
expect(page).to have_content("Which organisations are merging?") |
||||
expect(page).to have_content("Test Org") |
||||
expect(page).not_to have_link("Remove") |
||||
end |
||||
|
||||
context "when passing a different requesting organisation id" do |
||||
let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } } |
||||
|
||||
it "creates merge request with current user organisation" do |
||||
follow_redirect! |
||||
expect(MergeRequest.count).to eq(1) |
||||
expect(MergeRequest.first.requesting_organisation_id).to eq(organisation.id) |
||||
expect(MergeRequest.first.merging_organisations.count).to eq(1) |
||||
expect(MergeRequest.first.merging_organisations.first.id).to eq(organisation.id) |
||||
end |
||||
end |
||||
end |
||||
|
||||
context "when viewing existing merge request" do |
||||
before do |
||||
organisation.update!(name: "Test Org") |
||||
get "/merge-request/#{merge_request.id}/organisations", headers:, params: |
||||
end |
||||
|
||||
it "shows merge request with requesting organisation" do |
||||
expect(page).to have_content("Which organisations are merging?") |
||||
expect(page).to have_content("Test Org") |
||||
end |
||||
end |
||||
|
||||
context "when viewing existing merge request of a different (unauthorised) organisation" do |
||||
before do |
||||
get "/merge-request/#{other_merge_request.id}/organisations", headers:, params: |
||||
end |
||||
|
||||
it "shows merge request with requesting organisation" do |
||||
expect(response).to have_http_status(:not_found) |
||||
end |
||||
end |
||||
end |
||||
|
||||
describe "#update_organisations" do |
||||
let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } |
||||
|
||||
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 |
||||
|
||||
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, status: "submitted") |
||||
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 has another non submitted merge" do |
||||
let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } |
||||
|
||||
before do |
||||
MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "unsubmitted") |
||||
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).not_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 |
||||
existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "submitted") |
||||
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 |
||||
|
||||
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 unsubmitted merge" do |
||||
let(:another_organisation) { FactoryBot.create(:organisation, name: "Other Test Org") } |
||||
let(:params) { { merge_request: { merging_organisation: another_organisation.id } } } |
||||
|
||||
before do |
||||
existing_merge_request = MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "unsubmitted") |
||||
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 |
||||
|
||||
it "does not update the merge request" do |
||||
merge_request.reload |
||||
expect(merge_request.merging_organisations.count).to eq(1) |
||||
expect(page).not_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 |
||||
|
||||
context "when the user selects an organisation that is requesting this merge" do |
||||
let(:params) { { merge_request: { merging_organisation: merge_request.requesting_organisation_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(page).not_to have_content(I18n.t("validations.merge_request.organisation_part_of_another_merge")) |
||||
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 |
||||
let(:params) { { merge_request: { merging_organisation: other_organisation.id } } } |
||||
|
||||
context "when removing an organisation from merge request" do |
||||
before do |
||||
MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id) |
||||
get "/merge-request/#{merge_request.id}/organisations/remove", headers:, params: |
||||
end |
||||
|
||||
it "updates the merge request" do |
||||
expect(merge_request.merging_organisations.count).to eq(0) |
||||
expect(page).not_to have_link("Remove") |
||||
end |
||||
end |
||||
|
||||
context "when removing an organisation that is not part of a merge from merge request" do |
||||
before do |
||||
get "/merge-request/#{merge_request.id}/organisations/remove", headers:, params: |
||||
end |
||||
|
||||
it "does not throw an error" do |
||||
expect(merge_request.merging_organisations.count).to eq(0) |
||||
expect(page).not_to have_link("Remove") |
||||
end |
||||
end |
||||
end |
||||
|
||||
describe "#other_merging_organisations" do |
||||
let(:params) { { merge_request: { other_merging_organisations: "A list of other merging organisations" } } } |
||||
|
||||
context "when adding other merging organisations" do |
||||
before do |
||||
MergeRequestOrganisation.create!(merge_request_id: merge_request.id, merging_organisation_id: other_organisation.id) |
||||
patch "/merge-request/#{merge_request.id}", headers:, params: |
||||
end |
||||
|
||||
it "updates the merge request" do |
||||
merge_request.reload |
||||
expect(merge_request.other_merging_organisations).to eq("A list of other merging organisations") |
||||
end |
||||
end |
||||
end |
||||
end |
||||
|
||||
context "when user is signed in as a support user" do |
||||
before do |
||||
allow(support_user).to receive(:need_two_factor_authentication?).and_return(false) |
||||
sign_in support_user |
||||
end |
||||
|
||||
describe "#organisations" do |
||||
let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } } |
||||
|
||||
before do |
||||
organisation.update!(name: "Test Org") |
||||
post "/merge-request", headers:, params: |
||||
end |
||||
|
||||
it "creates merge request with requesting organisation" do |
||||
follow_redirect! |
||||
expect(MergeRequest.count).to eq(1) |
||||
expect(MergeRequest.first.requesting_organisation_id).to eq(other_organisation.id) |
||||
end |
||||
end |
||||
end |
||||
end |
Loading…
Reference in new issue