CLDC-3726 Display Helpdesk ticket question conditionally (#2808)

* Add has_helpdesk_ticket question

* lint

* typo

app/controllers/merge_requests_controller.rb

@@ -116,6 +116,7 @@ private
   def merge_request_params
     merge_params = params.fetch(:merge_request, {}).permit(
       :requesting_organisation_id,
+      :has_helpdesk_ticket,
       :helpdesk_ticket,
       :status,
       :absorbing_organisation_id,
@@ -124,6 +125,7 @@ private
     )
 
     merge_params[:requesting_organisation_id] = current_user.organisation.id
+    merge_params[:helpdesk_ticket] = nil if merge_params[:has_helpdesk_ticket] == "false"
 
     merge_params
   end
@@ -151,6 +153,14 @@ private
       if merge_request_params[:existing_absorbing_organisation].nil?
         @merge_request.errors.add(:existing_absorbing_organisation, :blank)
       end
+    when "helpdesk_ticket"
+      @merge_request.has_helpdesk_ticket = merge_request_params[:has_helpdesk_ticket]
+      @merge_request.helpdesk_ticket = merge_request_params[:helpdesk_ticket]
+      if merge_request_params[:has_helpdesk_ticket].blank?
+        @merge_request.errors.add(:has_helpdesk_ticket, :blank)
+      elsif merge_request_params[:has_helpdesk_ticket] == "true" && merge_request_params[:helpdesk_ticket].blank?
+        @merge_request.errors.add(:helpdesk_ticket, :blank)
+      end
     end
   end
 

app/helpers/merge_requests_helper.rb

@@ -9,7 +9,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://mhclgdigital.atlassian.net/browse/#{merge_request.helpdesk_ticket}", target: "_blank", rel: "noopener noreferrer") : display_value_or_placeholder(nil), action: merge_request_action(merge_request, "helpdesk_ticket") },
+      { label: "Helpdesk ticket", value: helpdesk_ticket_value(merge_request), action: merge_request_action(merge_request, "helpdesk_ticket") },
       { label: "Status", value: status_tag(merge_request.status) },
     ]
   end
@@ -280,4 +280,14 @@ module MergeRequestsHelper
   def begin_merge_disabled?(merge_request)
     merge_request.status != "ready_to_merge" || merge_request.merge_date.future?
   end
+
+  def helpdesk_ticket_value(merge_request)
+    if merge_request.helpdesk_ticket.present?
+      link_to("#{merge_request.helpdesk_ticket} (opens in a new tab)", "https://mhclgdigital.atlassian.net/browse/#{merge_request.helpdesk_ticket}", target: "_blank", rel: "noopener noreferrer")
+    elsif merge_request.has_helpdesk_ticket == false
+      "Not reported by a helpdesk ticket"
+    else
+      display_value_or_placeholder(nil)
+    end
+  end
 end

app/views/merge_requests/helpdesk_ticket.html.erb

@@ -7,14 +7,27 @@
 <%= form_with model: @merge_request, url: submit_merge_request_url(request.query_parameters["referrer"]), method: :patch do |f| %>
   <%= f.govuk_error_summary %>
 
-  <h1 class="govuk-heading-l">Which helpdesk ticket reported this merge?</h1>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds-from-desktop">
-      <p class="govuk-hint">If this merge was reported via a helpdesk ticket, provide the ticket number.<br>The ticket will be linked to the merge request for reference.</p>
-      <br>
       <div class="govuk-grid-row">
         <div class="govuk-grid-column-two-thirds-from-desktop">
-          <%= f.govuk_text_field :helpdesk_ticket, caption: { text: "Ticket number", class: "govuk-label govuk-label--s" }, label: { text: "For example, MSD-12345", class: "app-!-colour-muted" } %>
+          <%= f.govuk_radio_buttons_fieldset :has_helpdesk_ticket,
+            legend: { text: "Was this merge reported by a helpdesk ticket?", size: "l" } do %>
+
+            <%= f.govuk_radio_button "has_helpdesk_ticket",
+              true,
+              label: { text: "Yes" },
+              **basic_conditional_html_attributes({ "helpdesk_ticket" => [true] }, "merge_request") do %>
+              <%= f.govuk_text_field :helpdesk_ticket,
+                caption: { text: "Ticket number", class: "govuk-label govuk-label--s" },
+                label: { text: "For example, MSD-12345", class: "app-!-colour-muted" } %>
+            <% end %>
+
+            <%= f.govuk_radio_button "has_helpdesk_ticket",
+              false,
+              label: { text: "No" } %>
+          <% end %>
+
           <%= f.hidden_field :page, value: "helpdesk_ticket" %>
           <div class="govuk-button-group">
             <%= f.govuk_submit submit_merge_request_button_text(request.query_parameters["referrer"]) %>

config/locales/en.yml

@@ -188,6 +188,10 @@ en:
               blank: "You must answer absorbing organisation already active?"
             merging_organisation_id:
               part_of_another_merge: "Another merge request records %{organisation} as merging into %{absorbing_organisation} on %{merge_date}. Select another organisation or remove this organisation from the other merge request."
+            has_helpdesk_ticket:
+              blank: "You must answer was this merge reported by a helpdesk ticket?"
+            helpdesk_ticket:
+              blank: "You must answer the ticket number"
         notification:
           attributes:
             title:

db/migrate/20241122154743_add_has_helpdest_ticket.rb

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

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: 2024_11_18_104046) do
+ActiveRecord::Schema[7.0].define(version: 2024_11_22_154743) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -478,6 +478,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_11_18_104046) do
     t.boolean "request_merged"
     t.boolean "processing"
     t.boolean "existing_absorbing_organisation"
+    t.boolean "has_helpdesk_ticket"
   end
 
   create_table "notifications", force: :cascade do |t|

spec/factories/merge_request.rb

@@ -2,6 +2,7 @@ FactoryBot.define do
   factory :merge_request do
     status { "incomplete" }
     merge_date { nil }
+    has_helpdesk_ticket { true }
     helpdesk_ticket { "MSD-99999" }
     association :requesting_organisation, factory: :organisation
   end

spec/requests/merge_requests_controller_spec.rb

@@ -260,7 +260,7 @@ RSpec.describe MergeRequestsController, type: :request do
         end
 
         it "shows the correct content" do
-          expect(page).to have_content("Which helpdesk ticket reported this merge?")
+          expect(page).to have_content("Was this merge reported by a helpdesk ticket?")
           expect(page).to have_link("Back", href: existing_absorbing_organisation_merge_request_path(merge_request))
           expect(page).to have_button("Save and continue")
         end
@@ -476,6 +476,82 @@ RSpec.describe MergeRequestsController, type: :request do
           end
         end
       end
+
+      describe "from helpdesk_ticket 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: "helpdesk_ticket" } }
+          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 was this merge reported by a helpdesk ticket?")
+          end
+
+          it "does not update the request" do
+            expect { request }.not_to(change { merge_request.reload.attributes })
+          end
+        end
+
+        context "when has_helpdesk_ticket is true but no ticket is given" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+          let(:params) do
+            { merge_request: { page: "helpdesk_ticket", has_helpdesk_ticket: true } }
+          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 the ticket number")
+          end
+
+          it "does not update the request" do
+            expect { request }.not_to(change { merge_request.reload.attributes })
+          end
+        end
+
+        context "when has_helpdesk_ticket is false" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation, helpdesk_ticket: "123") }
+          let(:params) do
+            { merge_request: { page: "helpdesk_ticket", has_helpdesk_ticket: false } }
+          end
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "updates has_helpdesk_ticket and clears helpdesk_ticket" do
+            request
+            expect(merge_request.reload.has_helpdesk_ticket).to eq(false)
+            expect(merge_request.helpdesk_ticket).to eq(nil)
+          end
+        end
+
+        context "when has_helpdesk_ticket is true and ticket is given" do
+          let(:merge_request) { MergeRequest.create!(requesting_organisation: organisation, absorbing_organisation: other_organisation) }
+          let(:params) do
+            { merge_request: { page: "helpdesk_ticket", has_helpdesk_ticket: true, helpdesk_ticket: "321" } }
+          end
+          let(:request) do
+            patch "/merge-request/#{merge_request.id}", headers:, params:
+          end
+
+          it "updates has_helpdesk_ticket and clears helpdesk_ticket" do
+            request
+            expect(merge_request.reload.has_helpdesk_ticket).to eq(true)
+            expect(merge_request.helpdesk_ticket).to eq("321")
+          end
+        end
+      end
     end
 
     describe "#merge_start_confirmation" do