Browse Source

Add status, run validations unless the status is unsubmitted and save requesting organisation as a merging organisation as well

pull/1535/head
Kat 3 years ago
parent
commit
6d125c5dd8
  1. 4
      app/controllers/merge_requests_controller.rb
  2. 7
      app/models/merge_request.rb
  3. 7
      app/models/merge_request_organisation.rb
  4. 17
      app/views/merge_requests/organisations.html.erb
  5. 1
      app/views/organisations/merge_request.html.erb
  6. 5
      db/migrate/20230418095819_add_status_to_merge_request.rb
  7. 3
      db/schema.rb
  8. 58
      spec/requests/merge_requests_controller_spec.rb

4
app/controllers/merge_requests_controller.rb

@ -5,6 +5,8 @@ class MergeRequestsController < ApplicationController
def create
@merge_request = MergeRequest.create!(merge_request_params)
MergeRequestOrganisation.create!({ merge_request: @merge_request, merging_organisation_id: merge_request_params.fetch(:requesting_organisation_id) })
redirect_to organisations_merge_request_path(@merge_request)
end
@ -48,7 +50,7 @@ private
end
def merge_request_params
merge_params = params.fetch(:merge_request, {}).permit(:requesting_organisation_id, :other_merging_organisations)
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

7
app/models/merge_request.rb

@ -2,4 +2,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
scope :unsubmitted, -> { where.not(status: "unsubmitted") }
STATUS = {
"unsubmitted" => 0,
"submitted" => 1,
}.freeze
enum status: STATUS
end

7
app/models/merge_request_organisation.rb

@ -5,6 +5,9 @@ class MergeRequestOrganisation < ApplicationRecord
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
@ -14,12 +17,12 @@ private
errors.add(:merging_organisation, I18n.t("validations.merge_request.organisation_part_of_another_merge"))
end
if MergeRequestOrganisation.where.not(merge_request:).where(merging_organisation:).count.positive?
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.where(requesting_organisation: merging_organisation).count.positive?
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

17
app/views/merge_requests/organisations.html.erb

@ -21,15 +21,6 @@
} %>
<%= f.govuk_submit "Add organisation", classes: "govuk-button--secondary" %>
<%= govuk_table do |table| %>
<%= table.body do |body| %>
<%= body.row do |row| %>
<% row.cell(text: @merge_request.requesting_organisation.name) %>
<% row.cell(html_attributes: {
scope: "row",
class: "govuk-!-text-align-right",
}) %>
<% end %>
<% end %>
<% @merge_request.merging_organisations.each do |merging_organisation| %>
<%= table.body do |body| %>
<%= body.row do |row| %>
@ -38,7 +29,9 @@
scope: "row",
class: "govuk-!-text-align-right",
}) do %>
<%= govuk_link_to("Remove", organisations_remove_merge_request_path(merge_request: { merging_organisation: merging_organisation.id })) %>
<% 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 %>
@ -46,9 +39,11 @@
<% end %>
<% end %>
<%= form_with model: @merge_request, url: merge_request_path(id: @merge_request.id), method: :patch do |f| %>
<%= govuk_details(summary_text: "Can’t find the managing agent you’re looking for?") do %>
<%= 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.positive? %>
<%= f.govuk_submit "Continue" %>
<% end %>
<% end %>
</div>

1
app/views/organisations/merge_request.html.erb

@ -43,6 +43,7 @@
<%= form_for @merge_request, url: merge_requests_path do |f| %>
<%= f.hidden_field :requesting_organisation_id, value: @organisation.id %>
<%= f.hidden_field :status, value: "unsubmitted" %>
<%= f.submit "Start now", class: "govuk-button govuk-button--start" %>
<% end %>
</div>

5
db/migrate/20230418095819_add_status_to_merge_request.rb

@ -0,0 +1,5 @@
class AddStatusToMergeRequest < ActiveRecord::Migration[7.0]
def change
add_column :merge_requests, :status, :integer
end
end

3
db/schema.rb

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2023_04_13_135407) do
ActiveRecord::Schema[7.0].define(version: 2023_04_18_095819) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -366,6 +366,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_04_13_135407) do
t.text "other_merging_organisations"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.integer "status"
end
create_table "organisation_relationships", force: :cascade do |t|

58
spec/requests/merge_requests_controller_spec.rb

@ -16,7 +16,7 @@ RSpec.describe MergeRequestsController, type: :request do
end
describe "#organisations" do
let(:params) { { merge_request: { requesting_organisation_id: "9" } } }
let(:params) { { merge_request: { requesting_organisation_id: "9", status: "unsubmitted" } } }
context "when creating a new merge request" do
before do
@ -32,12 +32,14 @@ RSpec.describe MergeRequestsController, type: :request do
end
context "when passing a different requesting organisation id" do
let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id } } }
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
@ -86,7 +88,7 @@ RSpec.describe MergeRequestsController, type: :request do
let(:params) { { merge_request: { merging_organisation: other_organisation.id } } }
before do
MergeRequest.create!(requesting_organisation_id: other_organisation.id)
MergeRequest.create!(requesting_organisation_id: other_organisation.id, status: "submitted")
patch "/merge-request/#{merge_request.id}/organisations", headers:, params:
end
@ -98,12 +100,27 @@ RSpec.describe MergeRequestsController, type: :request do
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)
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
@ -116,6 +133,23 @@ RSpec.describe MergeRequestsController, type: :request do
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 } } }
@ -131,6 +165,20 @@ RSpec.describe MergeRequestsController, type: :request do
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 } } }
@ -213,7 +261,7 @@ RSpec.describe MergeRequestsController, type: :request do
end
describe "#organisations" do
let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id } } }
let(:params) { { merge_request: { requesting_organisation_id: other_organisation.id, status: "unsubmitted" } } }
before do
organisation.update!(name: "Test Org")

Loading…
Cancel
Save