From 313a2bf62595e8af5de00605ad70c5a9963df862 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 14 Aug 2024 16:31:16 +0100 Subject: [PATCH] CLDC-3588: Add helpdesk ticket question (#2572) --- app/controllers/merge_requests_controller.rb | 5 +++- app/controllers/organisations_controller.rb | 2 +- app/helpers/merge_requests_helper.rb | 12 +++++++++- .../absorbing_organisation.html.erb | 6 +++-- .../merge_requests/helpdesk_ticket.html.erb | 24 ++++++++++++++++++- app/views/merge_requests/merge_date.html.erb | 6 +++-- .../merging_organisations.html.erb | 7 ++++-- .../merge_requests_controller_spec.rb | 13 ++++++++++ 8 files changed, 65 insertions(+), 10 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 746de0a5a..93b730e20 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -11,7 +11,7 @@ class MergeRequestsController < ApplicationController def create ActiveRecord::Base.transaction do - @merge_request = MergeRequest.create!(merge_request_params.merge(status: :incomplete)) + @merge_request = MergeRequest.create!(merge_request_params.merge(status: :incomplete, requester: current_user)) end redirect_to absorbing_organisation_merge_request_path(@merge_request) rescue ActiveRecord::RecordInvalid @@ -63,6 +63,8 @@ private merge_date_merge_request_path(@merge_request) when "merge_date" helpdesk_ticket_merge_request_path(@merge_request) + when "helpdesk_ticket" + merge_request_path(@merge_request) end end @@ -85,6 +87,7 @@ private def merge_request_params merge_params = params.fetch(:merge_request, {}).permit( :requesting_organisation_id, + :helpdesk_ticket, :status, :absorbing_organisation_id, :merge_date, diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 572711aff..044bab74d 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -18,7 +18,7 @@ class OrganisationsController < ApplicationController @pagy, @organisations = pagy(filtered_collection(all_organisations.visible, search_term)) @merge_requests = MergeRequest.visible .joins("LEFT JOIN organisations ON organisations.id = merge_requests.absorbing_organisation_id") - .order("organisations.name") + .order("organisations.name, merge_requests.merge_date DESC NULLS LAST, merge_requests.id") @searched = search_term.presence @total_count = all_organisations.visible.size end diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 4975a238f..6546e39b2 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -6,7 +6,7 @@ module MergeRequestsHelper def request_details(merge_request) [ { label: "Requester", value: display_value_or_placeholder(merge_request.requester&.name) }, - { label: "Helpdesk ticket", value: merge_request.helpdesk_ticket.present? ? link_to("#{merge_request.helpdesk_ticket} (opens in a new tab)", "https://dluhcdigital.atlassian.net/browse/#{merge_request.helpdesk_ticket}", target: "_blank", rel: "noopener noreferrer") : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: "#", visually_hidden_text: "helpdesk ticket" } }, + { label: "Helpdesk ticket", value: merge_request.helpdesk_ticket.present? ? link_to("#{merge_request.helpdesk_ticket} (opens in a new tab)", "https://dluhcdigital.atlassian.net/browse/#{merge_request.helpdesk_ticket}", target: "_blank", rel: "noopener noreferrer") : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: helpdesk_ticket_merge_request_path(merge_request, referrer: "check_answers"), visually_hidden_text: "helpdesk ticket" } }, { label: "Status", value: status_tag(merge_request.status) }, ] end @@ -40,6 +40,16 @@ module MergeRequestsHelper end end + def secondary_merge_request_link_text(referrer, skip_for_now: false) + if accessed_from_check_answers?(referrer) + "Cancel" + elsif skip_for_now + "Skip for now" + else + "" + end + end + def accessed_from_check_answers?(referrer) %w[check_answers].include?(referrer) end diff --git a/app/views/merge_requests/absorbing_organisation.html.erb b/app/views/merge_requests/absorbing_organisation.html.erb index ea54de32a..33c246bb7 100644 --- a/app/views/merge_requests/absorbing_organisation.html.erb +++ b/app/views/merge_requests/absorbing_organisation.html.erb @@ -25,8 +25,10 @@ <% end %> <%= f.hidden_field :page, value: "absorbing_organisation" %> - - <%= f.govuk_submit %> +
+ <%= 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)) %> +
<% end %> diff --git a/app/views/merge_requests/helpdesk_ticket.html.erb b/app/views/merge_requests/helpdesk_ticket.html.erb index 3cd6b047f..accaf4aca 100644 --- a/app/views/merge_requests/helpdesk_ticket.html.erb +++ b/app/views/merge_requests/helpdesk_ticket.html.erb @@ -1,5 +1,27 @@ <% content_for :before_content do %> - <% title = "Helpdesk ticket" %> + <% title = "Which helpdesk ticket reported this merge?" %> <% content_for :title, title %> <%= govuk_back_link href: merge_date_merge_request_path(@merge_request) %> <% end %> + +<%= form_with model: @merge_request, url: merge_request_path, method: :patch do |f| %> + <%= f.govuk_error_summary %> + +

Which helpdesk ticket reported this merge?

+
+
+

If this merge was reported via the helpdesk ticket, provide the ticket number.
The ticket will be linked to the merge request for reference.

+
+
+
+ <%= f.govuk_text_field :helpdesk_ticket, caption: { text: "Ticket number", class: "govuk-label--m" }, label: { text: "For example, MSD-12345" } %> + <%= f.hidden_field :page, value: "helpdesk_ticket" %> +
+ <%= f.govuk_submit submit_merge_request_button_text(request.query_parameters["referrer"]) %> + <%= govuk_link_to(secondary_merge_request_link_text(request.query_parameters["referrer"], skip_for_now: true), merge_request_path(@merge_request)) %> +
+
+
+ <% end %> +
+
diff --git a/app/views/merge_requests/merge_date.html.erb b/app/views/merge_requests/merge_date.html.erb index 6766cb552..90cb22159 100644 --- a/app/views/merge_requests/merge_date.html.erb +++ b/app/views/merge_requests/merge_date.html.erb @@ -17,8 +17,10 @@ width: 20 do %> <% end %> <%= f.hidden_field :page, value: "merge_date" %> - - <%= f.govuk_submit %> +
+ <%= 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)) %> +
<% end %> diff --git a/app/views/merge_requests/merging_organisations.html.erb b/app/views/merge_requests/merging_organisations.html.erb index f7dbeb378..fb4133889 100644 --- a/app/views/merge_requests/merging_organisations.html.erb +++ b/app/views/merge_requests/merging_organisations.html.erb @@ -4,7 +4,7 @@ <%= govuk_back_link href: merge_request_organisation_path(id: @merge_request.requesting_organisation_id) %> <% end %> -<%= form_with model: @merge_request, url: merging_organisations_merge_request_path, method: :patch do |f| %> +<%= form_with model: @merge_request, url: merging_organisations_merge_request_path(referrer: request.query_parameters["referrer"]), method: :patch do |f| %> <%= f.govuk_error_summary %>

Which organisations are merging into <%= @merge_request.absorbing_organisation&.name %>?

@@ -38,7 +38,10 @@ <%= form_with model: @merge_request, url: merge_request_path(id: @merge_request.id), method: :patch do |f| %> <% if @merge_request.merging_organisations.count.positive? %> <%= f.hidden_field :page, value: "merging_organisations" %> - <%= f.govuk_submit submit_merge_request_button_text(request.query_parameters["referrer"]) %> +
+ <%= 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)) %> +
<% end %> <% end %> diff --git a/spec/requests/merge_requests_controller_spec.rb b/spec/requests/merge_requests_controller_spec.rb index 8bbd419f7..2144b409b 100644 --- a/spec/requests/merge_requests_controller_spec.rb +++ b/spec/requests/merge_requests_controller_spec.rb @@ -222,6 +222,19 @@ RSpec.describe MergeRequestsController, type: :request do end end + describe "#helpdesk_ticket" 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}/helpdesk-ticket", headers: + end + + it "shows the correct content" do + expect(page).to have_content("Which helpdesk ticket reported this merge?") + end + end + end + describe "#update" do describe "from absorbing_organisation page" do context "when not answering the question" do