From 066cfdfa046b8b490720b85b8cc687615fe858af Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Mon, 12 Aug 2024 16:58:45 +0100
Subject: [PATCH] Move table spec to helper

---
 app/helpers/merge_requests_helper.rb   | 25 +++++++++++++++++++++++++
 app/views/merge_requests/show.html.erb | 25 +++----------------------
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index e687dec4a..863c00be2 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -2,4 +2,29 @@ module MergeRequestsHelper
   def display_value_or_placeholder(value, placeholder = "You didn't answer this question")
     value.presence || content_tag(:span, placeholder, class: "app-!-colour-muted")
   end
+
+  def request_details(merge_request)
+    [
+      { label: "Requester", value: display_value_or_placeholder(merge_request.requestor&.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: "Status", value: status_tag(merge_request.status) },
+    ]
+  end
+
+  def merge_details(merge_request)
+    [
+      { label: "Absorbing organisation", value: display_value_or_placeholder(merge_request.absorbing_organisation_name), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: absorbing_organisation_merge_request_path(merge_request), visually_hidden_text: "absorbing organisation" } },
+      { label: "Merging organisations", value: merge_request.other_merging_organisations.present? ? merge_request.other_merging_organisations.split(",").map(&:strip).join("<br>").html_safe : display_value_or_placeholder(nil), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: organisations_merge_request_path(merge_request), visually_hidden_text: "merging organisations" } },
+      { label: "Merge date", value: display_value_or_placeholder(merge_request.merge_date), action: merge_request.status == "request_merged" ? nil : { text: "Change", href: merge_date_merge_request_path(merge_request), visually_hidden_text: "merge date" } },
+    ]
+  end
+
+  def merge_outcomes(merge_request)
+    [
+      { label: "Total users after merge", value: display_value_or_placeholder(merge_request.total_users), action: { text: "View", href: "#", visually_hidden_text: "total users after merge" } },
+      { label: "Total schemes after merge", value: display_value_or_placeholder(merge_request.total_schemes), action: { text: "View", href: "#", visually_hidden_text: "total schemes after merge" } },
+      { label: "Total logs after merge", value: merge_request.total_lettings_logs.present? || merge_request.total_sales_logs.present? ? "#{merge_request.total_lettings_logs} lettings logs<br>#{merge_request.total_sales_logs} sales logs".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total logs after merge" } },
+      { label: "Total stock owners & managing agents after merge", value: merge_request.total_stock_owners.present? || merge_request.total_managing_agents.present? ? "#{merge_request.total_stock_owners} stock owners<br>#{merge_request.total_managing_agents} managing agents".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total stock owners & managing agents after merge" } },
+    ]
+  end
 end
diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb
index a6e2440d7..b28ee3c1e 100644
--- a/app/views/merge_requests/show.html.erb
+++ b/app/views/merge_requests/show.html.erb
@@ -36,29 +36,10 @@
 </div>
 <% end %>
 
-<% request_details = [
-  { label: "Requester", value: display_value_or_placeholder(@merge_request.requestor&.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: "Status", value: status_tag(@merge_request.status) },
-] %>
+<%= render partial: "merge_requests/summary_card", locals: { title: "Request details", details: request_details(@merge_request) } %>
 
-<%= render partial: "merge_requests/summary_card", locals: { title: "Request details", details: request_details } %>
-
-<% merge_details = [
-  { label: "Absorbing organisation", value: display_value_or_placeholder(@merge_request.absorbing_organisation_name), action: @merge_request.status == "request_merged" ? nil : { text: "Change", href: absorbing_organisation_merge_request_path(@merge_request), visually_hidden_text: "absorbing organisation" } },
-  { label: "Merging organisations", value: @merge_request.other_merging_organisations.present? ? @merge_request.other_merging_organisations.split(",").map(&:strip).join("<br>").html_safe : display_value_or_placeholder(nil), action: @merge_request.status == "request_merged" ? nil : { text: "Change", href: organisations_merge_request_path(@merge_request), visually_hidden_text: "merging organisations" } },
-  { label: "Merge date", value: display_value_or_placeholder(@merge_request.merge_date), action: @merge_request.status == "request_merged" ? nil : { text: "Change", href: merge_date_merge_request_path(@merge_request), visually_hidden_text: "merge date" } },
-] %>
-
-<%= render partial: "merge_requests/summary_card", locals: { title: "Merge details", details: merge_details } %>
+<%= render partial: "merge_requests/summary_card", locals: { title: "Merge details", details: merge_details(@merge_request) } %>
 
 <% unless @merge_request.status == "incomplete" %>
-  <% merge_outcomes = [
-    { label: "Total users after merge", value: display_value_or_placeholder(@merge_request.total_users), action: { text: "View", href: "#", visually_hidden_text: "total users after merge" } },
-    { label: "Total schemes after merge", value: display_value_or_placeholder(@merge_request.total_schemes), action: { text: "View", href: "#", visually_hidden_text: "total schemes after merge" } },
-    { label: "Total logs after merge", value: @merge_request.total_lettings_logs.present? || @merge_request.total_sales_logs.present? ? "#{@merge_request.total_lettings_logs} lettings logs<br>#{@merge_request.total_sales_logs} sales logs".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total logs after merge" } },
-    { label: "Total stock owners & managing agents after merge", value: @merge_request.total_stock_owners.present? || @merge_request.total_managing_agents.present? ? "#{@merge_request.total_stock_owners} stock owners<br>#{@merge_request.total_managing_agents} managing agents".html_safe : display_value_or_placeholder(nil), action: { text: "View", href: "#", visually_hidden_text: "total stock owners & managing agents after merge" } },
-  ] %>
-
-  <%= render partial: "merge_requests/summary_card", locals: { title: "Merge outcomes", details: merge_outcomes } %>
+  <%= render partial: "merge_requests/summary_card", locals: { title: "Merge outcomes", details: merge_outcomes(@merge_request) } %>
 <% end %>