Browse Source

CLDC-3609 Add existing absorbing organisation merge request question (#2600)

* Add existing absorbing organisation page and field

* Update status calculation and add new question to check answers

* Call correct merge service flow

* Update test

* Update test
pull/2620/head
kosiakkatrina 2 years ago
parent
commit
2d2989cc67
  1. 8
      app/controllers/merge_requests_controller.rb
  2. 5
      app/helpers/merge_requests_helper.rb
  3. 5
      app/jobs/process_merge_request_job.rb
  4. 7
      app/models/merge_request.rb
  5. 27
      app/views/merge_requests/existing_absorbing_organisation.html.erb
  6. 2
      config/locales/en.yml
  7. 1
      config/routes.rb
  8. 5
      db/migrate/20240822080228_add_existing_absorbing_organisation_field.rb
  9. 3
      db/schema.rb
  10. 22
      spec/jobs/process_merge_request_job_spec.rb
  11. 10
      spec/models/merge_request_spec.rb
  12. 50
      spec/requests/merge_requests_controller_spec.rb

8
app/controllers/merge_requests_controller.rb

@ -6,6 +6,7 @@ class MergeRequestsController < ApplicationController
def absorbing_organisation; end def absorbing_organisation; end
def merge_date; end def merge_date; end
def existing_absorbing_organisation; end
def helpdesk_ticket; end def helpdesk_ticket; end
def merge_start_confirmation; end def merge_start_confirmation; end
def user_outcomes; end def user_outcomes; end
@ -94,6 +95,8 @@ private
when "merging_organisations" when "merging_organisations"
merge_date_merge_request_path(@merge_request) merge_date_merge_request_path(@merge_request)
when "merge_date" when "merge_date"
existing_absorbing_organisation_merge_request_path(@merge_request)
when "existing_absorbing_organisation"
helpdesk_ticket_merge_request_path(@merge_request) helpdesk_ticket_merge_request_path(@merge_request)
when "helpdesk_ticket" when "helpdesk_ticket"
merge_request_path(@merge_request) merge_request_path(@merge_request)
@ -123,6 +126,7 @@ private
:status, :status,
:absorbing_organisation_id, :absorbing_organisation_id,
:merge_date, :merge_date,
:existing_absorbing_organisation,
) )
merge_params[:requesting_organisation_id] = current_user.organisation.id merge_params[:requesting_organisation_id] = current_user.organisation.id
@ -148,6 +152,10 @@ private
else else
@merge_request.errors.add(:merge_date, :invalid) @merge_request.errors.add(:merge_date, :invalid)
end end
when "existing_absorbing_organisation"
if merge_request_params[:existing_absorbing_organisation].nil?
@merge_request.errors.add(:existing_absorbing_organisation, :blank)
end
end end
end end

5
app/helpers/merge_requests_helper.rb

@ -19,6 +19,7 @@ module MergeRequestsHelper
{ label: "Absorbing organisation", value: display_value_or_placeholder(merge_request.absorbing_organisation_name), action: merge_request_action(merge_request, "absorbing_organisation") }, { label: "Absorbing organisation", value: display_value_or_placeholder(merge_request.absorbing_organisation_name), action: merge_request_action(merge_request, "absorbing_organisation") },
{ label: "Merging organisations", value: merge_request.merge_request_organisations.any? ? merge_request.merge_request_organisations.map(&:merging_organisation_name).join("<br>").html_safe : display_value_or_placeholder(nil), action: merge_request_action(merge_request, "merging_organisations") }, { label: "Merging organisations", value: merge_request.merge_request_organisations.any? ? merge_request.merge_request_organisations.map(&:merging_organisation_name).join("<br>").html_safe : display_value_or_placeholder(nil), action: merge_request_action(merge_request, "merging_organisations") },
{ label: "Merge date", value: display_value_or_placeholder(merge_request.merge_date), action: merge_request_action(merge_request, "merge_date") }, { label: "Merge date", value: display_value_or_placeholder(merge_request.merge_date), action: merge_request_action(merge_request, "merge_date") },
{ label: "Absorbing organisation already active?", value: display_value_or_placeholder(merge_request.existing_absorbing_organisation_label), action: merge_request_action(merge_request, "existing_absorbing_organisation") },
] ]
end end
@ -67,8 +68,10 @@ module MergeRequestsHelper
absorbing_organisation_merge_request_path(merge_request) absorbing_organisation_merge_request_path(merge_request)
when "merge_date" when "merge_date"
merging_organisations_merge_request_path(merge_request) merging_organisations_merge_request_path(merge_request)
when "helpdesk_ticket" when "existing_absorbing_organisation"
merge_date_merge_request_path(merge_request) merge_date_merge_request_path(merge_request)
when "helpdesk_ticket"
existing_absorbing_organisation_merge_request_path(merge_request)
end end
end end

5
app/jobs/process_merge_request_job.rb

@ -5,9 +5,10 @@ class ProcessMergeRequestJob < ApplicationJob
absorbing_organisation_id = merge_request.absorbing_organisation_id absorbing_organisation_id = merge_request.absorbing_organisation_id
merging_organisation_ids = merge_request.merging_organisations.pluck(:id) merging_organisation_ids = merge_request.merging_organisations.pluck(:id)
merge_date = merge_request.merge_date merge_date = merge_request.merge_date
absorbing_organisation_active_from_merge_date = !merge_request.existing_absorbing_organisation unless merge_request.existing_absorbing_organisation.nil?
Merge::MergeOrganisationsService.new(absorbing_organisation_id:, merging_organisation_ids:, merge_date:).call Merge::MergeOrganisationsService.new(absorbing_organisation_id:, merging_organisation_ids:, merge_date:, absorbing_organisation_active_from_merge_date:).call
merge_request.update!(request_merged: true) merge_request.update!(request_merged: true, last_failed_attempt: nil)
rescue StandardError rescue StandardError
merge_request.update!(last_failed_attempt: Time.zone.now, processing: false, total_users: nil, total_schemes: nil) merge_request.update!(last_failed_attempt: Time.zone.now, processing: false, total_users: nil, total_schemes: nil)
end end

7
app/models/merge_request.rb

@ -47,6 +47,7 @@ class MergeRequest < ApplicationRecord
def required_questions_answered? def required_questions_answered?
absorbing_organisation_id.present? && absorbing_organisation_id.present? &&
merge_date.present? && merge_date.present? &&
existing_absorbing_organisation.present? &&
merging_organisations.count.positive? && merging_organisations.count.positive? &&
errors.empty? errors.empty?
end end
@ -99,6 +100,12 @@ class MergeRequest < ApplicationRecord
([absorbing_organisation] + merging_organisations).reject(&:has_visible_schemes?) ([absorbing_organisation] + merging_organisations).reject(&:has_visible_schemes?)
end end
def existing_absorbing_organisation_label
return if existing_absorbing_organisation.nil?
existing_absorbing_organisation ? "Yes" : "No"
end
def filter_relationships(absorbing_relationships, merging_relationships, absorbing_organisation, merging_organisations) def filter_relationships(absorbing_relationships, merging_relationships, absorbing_organisation, merging_organisations)
filtered_absorbing_relationships = absorbing_relationships.reject do |relationship| filtered_absorbing_relationships = absorbing_relationships.reject do |relationship|
merging_relationships.include?(relationship) || merging_organisations.include?(relationship) merging_relationships.include?(relationship) || merging_organisations.include?(relationship)

27
app/views/merge_requests/existing_absorbing_organisation.html.erb

@ -0,0 +1,27 @@
<% content_for :before_content do %>
<% title = "Tell us if your organisation is merging" %>
<% content_for :title, title %>
<%= govuk_back_link href: merge_request_back_link(@merge_request, "existing_absorbing_organisation", request.query_parameters["referrer"]) %>
<% end %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<%= form_with model: @merge_request, url: submit_merge_request_url(request.query_parameters["referrer"]), method: :patch do |f| %>
<%= f.govuk_error_summary %>
<%= f.govuk_radio_buttons_fieldset :existing_absorbing_organisation,
legend: { text: "Was #{@merge_request.absorbing_organisation&.name} already active before the merge date?", size: "l" } do %>
<%= f.govuk_radio_button :existing_absorbing_organisation,
"true",
label: { text: "Yes, this organisation existed before the merge" } %>
<%= f.govuk_radio_button :existing_absorbing_organisation,
"false",
label: { text: "No, it is a new organisation created by this merge" } %>
<% end %>
<%= f.hidden_field :page, value: "existing_absorbing_organisation" %>
<div class="govuk-button-group">
<%= f.govuk_submit submit_merge_request_button_text(request.query_parameters["referrer"]) %>
<%= govuk_link_to(secondary_merge_request_link_text(request.query_parameters["referrer"]), merge_request_path(@merge_request)) %>
</div>
<% end %>
</div>
</div>

2
config/locales/en.yml

@ -179,6 +179,8 @@ en:
merge_date: merge_date:
blank: "Enter a merge date" blank: "Enter a merge date"
invalid: "Enter a valid merge date" invalid: "Enter a valid merge date"
existing_absorbing_organisation:
blank: "You must answer absorbing organisation already active?"
notification: notification:
logs_deleted: logs_deleted:

1
config/routes.rb

@ -208,6 +208,7 @@ Rails.application.routes.draw do
get "merging-organisations/remove", to: "merge_requests#remove_merging_organisation" get "merging-organisations/remove", to: "merge_requests#remove_merging_organisation"
get "absorbing-organisation" get "absorbing-organisation"
get "merge-date" get "merge-date"
get "existing-absorbing-organisation"
get "helpdesk-ticket" get "helpdesk-ticket"
get "merge-start-confirmation" get "merge-start-confirmation"
get "user-outcomes" get "user-outcomes"

5
db/migrate/20240822080228_add_existing_absorbing_organisation_field.rb

@ -0,0 +1,5 @@
class AddExistingAbsorbingOrganisationField < ActiveRecord::Migration[7.0]
def change
add_column :merge_requests, :existing_absorbing_organisation, :boolean
end
end

3
db/schema.rb

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do ActiveRecord::Schema[7.0].define(version: 2024_08_22_080228) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -446,6 +446,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
t.datetime "last_failed_attempt" t.datetime "last_failed_attempt"
t.boolean "request_merged" t.boolean "request_merged"
t.boolean "processing" t.boolean "processing"
t.boolean "existing_absorbing_organisation"
end end
create_table "notifications", force: :cascade do |t| create_table "notifications", force: :cascade do |t|

22
spec/jobs/process_merge_request_job_spec.rb

@ -13,7 +13,7 @@ describe ProcessMergeRequestJob do
let(:organisation) { create(:organisation) } let(:organisation) { create(:organisation) }
let(:merging_organisation) { create(:organisation) } let(:merging_organisation) { create(:organisation) }
let(:other_merging_organisation) { create(:organisation) } let(:other_merging_organisation) { create(:organisation) }
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3), total_users: 5, total_schemes: 5) } let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3), total_users: 5, total_schemes: 5, existing_absorbing_organisation: true) }
before do before do
create(:merge_request_organisation, merge_request:, merging_organisation:) create(:merge_request_organisation, merge_request:, merging_organisation:)
@ -21,12 +21,30 @@ describe ProcessMergeRequestJob do
end end
it "calls the merge organisations service with correct arguments" do it "calls the merge organisations service with correct arguments" do
expect(Merge::MergeOrganisationsService).to receive(:new).with(absorbing_organisation_id: organisation.id, merging_organisation_ids: [merging_organisation.id, other_merging_organisation.id], merge_date: Time.zone.local(2022, 3, 3)) expect(Merge::MergeOrganisationsService).to receive(:new).with(absorbing_organisation_id: organisation.id, merging_organisation_ids: [merging_organisation.id, other_merging_organisation.id], merge_date: Time.zone.local(2022, 3, 3), absorbing_organisation_active_from_merge_date: false)
job.perform(merge_request:) job.perform(merge_request:)
expect(merge_request.reload.status).to eq("request_merged") expect(merge_request.reload.status).to eq("request_merged")
end end
context "with new absorbing organisation" do
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3), existing_absorbing_organisation: false) }
it "calls the merge organisations service with correct arguments" do
expect(Merge::MergeOrganisationsService).to receive(:new).with(absorbing_organisation_id: organisation.id, merging_organisation_ids: [merging_organisation.id, other_merging_organisation.id], merge_date: Time.zone.local(2022, 3, 3), absorbing_organisation_active_from_merge_date: true)
job.perform(merge_request:)
expect(merge_request.reload.status).to eq("request_merged")
end
end
it "clears last_failed_attempt value" do
merge_request.update!(last_failed_attempt: Time.zone.now)
job.perform(merge_request:)
expect(merge_request.reload.last_failed_attempt).to be_nil
end
it "sets last_failed_attempt value, sets processing to false and clears total_schemes and total_users if there's an error" do it "sets last_failed_attempt value, sets processing to false and clears total_schemes and total_users if there's an error" do
allow(merge_organisations_service).to receive(:call).and_raise(ActiveRecord::Rollback) allow(merge_organisations_service).to receive(:call).and_raise(ActiveRecord::Rollback)

10
spec/models/merge_request_spec.rb

@ -55,13 +55,13 @@ RSpec.describe MergeRequest, type: :model do
end end
it "returns the correct status for a ready to merge request" do it "returns the correct status for a ready to merge request" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), merge_date: Time.zone.today) merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), merge_date: Time.zone.today, existing_absorbing_organisation: true)
create(:merge_request_organisation, merge_request:) create(:merge_request_organisation, merge_request:)
expect(merge_request.status).to eq MergeRequest::STATUS[:ready_to_merge] expect(merge_request.status).to eq MergeRequest::STATUS[:ready_to_merge]
end end
it "returns the merge issues if dsa is not signed for absorbing organisation" do it "returns the merge issues if dsa is not signed for absorbing organisation" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation, with_dsa: false), merge_date: Time.zone.today) merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation, with_dsa: false), merge_date: Time.zone.today, existing_absorbing_organisation: true)
create(:merge_request_organisation, merge_request:) create(:merge_request_organisation, merge_request:)
expect(merge_request.status).to eq MergeRequest::STATUS[:merge_issues] expect(merge_request.status).to eq MergeRequest::STATUS[:merge_issues]
end end
@ -83,6 +83,12 @@ RSpec.describe MergeRequest, type: :model do
expect(merge_request.status).to eq MergeRequest::STATUS[:incomplete] expect(merge_request.status).to eq MergeRequest::STATUS[:incomplete]
end end
it "returns the incomplete if existing absorbing organisation is missing" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation, with_dsa: false), merge_date: Time.zone.today)
create(:merge_request_organisation, merge_request:)
expect(merge_request.status).to eq MergeRequest::STATUS[:incomplete]
end
it "returns processing if merge is processing" do it "returns processing if merge is processing" do
merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), processing: true) merge_request = build(:merge_request, id: 1, absorbing_organisation: create(:organisation), processing: true)
create(:merge_request_organisation, merge_request:) create(:merge_request_organisation, merge_request:)

50
spec/requests/merge_requests_controller_spec.rb

@ -219,6 +219,23 @@ RSpec.describe MergeRequestsController, type: :request do
end end
end end
describe "#existing_absorbing_organisation" do
context "when viewing helpdesk ticket page" do
before do
merge_request.update!(absorbing_organisation_id: organisation.id, merge_date: Time.zone.today)
get "/merge-request/#{merge_request.id}/existing-absorbing-organisation", headers:
end
it "shows the correct content" do
expect(page).to have_content("Was #{merge_request.absorbing_organisation.name} already active before the merge date?")
expect(page).to have_content("Yes, this organisation existed before the merge")
expect(page).to have_content("No, it is a new organisation created by this merge")
expect(page).to have_link("Back", href: merge_date_merge_request_path(merge_request))
expect(page).to have_button("Save and continue")
end
end
end
describe "#helpdesk_ticket" do describe "#helpdesk_ticket" do
context "when viewing helpdesk ticket page" do context "when viewing helpdesk ticket page" do
before do before do
@ -228,6 +245,8 @@ RSpec.describe MergeRequestsController, type: :request do
it "shows the correct content" do it "shows the correct content" do
expect(page).to have_content("Which helpdesk ticket reported this merge?") expect(page).to have_content("Which helpdesk ticket reported this merge?")
expect(page).to have_link("Back", href: existing_absorbing_organisation_merge_request_path(merge_request))
expect(page).to have_button("Save and continue")
end end
end end
end end
@ -365,10 +384,10 @@ RSpec.describe MergeRequestsController, type: :request do
patch "/merge-request/#{merge_request.id}", headers:, params: patch "/merge-request/#{merge_request.id}", headers:, params:
end end
it "redirects to helpdesk ticket path" do it "redirects to existing absorbing organisation path" do
request request
expect(response).to redirect_to(helpdesk_ticket_merge_request_path(merge_request)) expect(response).to redirect_to(existing_absorbing_organisation_merge_request_path(merge_request))
end end
it "updates merge_date" do it "updates merge_date" do
@ -400,6 +419,29 @@ RSpec.describe MergeRequestsController, type: :request do
end end
end end
end end
describe "from existing_absorbing_organisation page" do
context "when not answering the question" do
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
let(:params) do
{ merge_request: { page: "existing_absorbing_organisation" } }
end
let(:request) do
patch "/merge-request/#{merge_request.id}", headers:, params:
end
it "renders the error" do
request
expect(response).to have_http_status(:unprocessable_entity)
expect(page).to have_content("You must answer absorbing organisation already active?")
end
it "does not update the request" do
expect { request }.not_to(change { merge_request.reload.attributes })
end
end
end
end end
describe "#merge_start_confirmation" do describe "#merge_start_confirmation" do
@ -416,7 +458,7 @@ RSpec.describe MergeRequestsController, type: :request do
end end
describe "#start_merge" do describe "#start_merge" do
let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3)) } let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: organisation, merge_date: Time.zone.local(2022, 3, 3), existing_absorbing_organisation: true) }
let(:merging_organisation) { create(:organisation, name: "Merging Test Org") } let(:merging_organisation) { create(:organisation, name: "Merging Test Org") }
before do before do
@ -481,7 +523,7 @@ RSpec.describe MergeRequestsController, type: :request do
end end
context "with unmerged request" do context "with unmerged request" do
let(:merge_request) { create(:merge_request, absorbing_organisation_id: organisation.id, merge_date: Time.zone.today) } let(:merge_request) { create(:merge_request, absorbing_organisation_id: organisation.id, merge_date: Time.zone.today, existing_absorbing_organisation: true) }
it "shows users and schemes count and has links to view merge outcomes" do it "shows users and schemes count and has links to view merge outcomes" do
expect(page).to have_link("View", href: scheme_outcomes_merge_request_path(merge_request)) expect(page).to have_link("View", href: scheme_outcomes_merge_request_path(merge_request))

Loading…
Cancel
Save