Refactor and display a banner for failed requests

app/models/merge_request.rb

@@ -48,7 +48,7 @@ class MergeRequest < ApplicationRecord
     return "request_merged" if status == "request_merged"
     return "processing" if status == "processing"
     return "incomplete" unless required_questions_answered?
-    return "ready_to_merge" if absorbing_organisation.data_protection_confirmed?
+    return "ready_to_merge" if absorbing_organisation_signed_dsa?
 
     "merge_issues"
   end
@@ -59,4 +59,8 @@ class MergeRequest < ApplicationRecord
       merging_organisations.count.positive? &&
       errors.empty?
   end
+
+  def absorbing_organisation_signed_dsa?
+    absorbing_organisation&.data_protection_confirmed?
+  end
 end

app/views/merge_requests/_notification_banners.html.erb

@@ -0,0 +1,21 @@
+<% unless @merge_request.absorbing_organisation_signed_dsa? || @merge_request.absorbing_organisation_id.blank? %>
+  <%= govuk_notification_banner(title_text: "Important") do %>
+    <p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content">
+      The absorbing organisation must accept the Data Sharing Agreement before merging.
+    <p>
+    <% if @merge_request.dpo_user %>
+        Contact the Data Protection Officer: <%= link_to @merge_request.dpo_user.name, user_path(@merge_request.dpo_user.id) %>
+      <% else %>
+        <%= @merge_request.absorbing_organisation_name %> does not have a Data Protection Officer. You can assign one on the <%= link_to "users page", "#{organisation_path(@merge_request.absorbing_organisation_id)}/users" %>.
+      <% end %>
+  <% end %>
+<% end %>
+
+<% if @merge_request.last_failed_attempt.present? %>
+  <%= govuk_notification_banner(title_text: "Important") do %>
+    <p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content">
+      An error occurred while processing the merge.
+    <p>
+    No changes have been made. Try beginning the merge again.
+  <% end %>
+<% end %>

app/views/merge_requests/show.html.erb

@@ -3,24 +3,9 @@
   <% content_for :title, title %>
   <%= govuk_back_link href: organisations_path(anchor: "merge-requests") %>
 <% end %>
-<% unless @merge_request.signed_dsa || @merge_request.absorbing_organisation_id.blank? %>
-  <div class="govuk-notification-banner" role="region" aria-labelledby="govuk-notification-banner-title" data-module="govuk-notification-banner">
-    <div class="govuk-notification-banner__header">
-      <h2 class="govuk-notification-banner__title" id="govuk-notification-banner-title">
-        Important
-      </h2>
-    </div>
-    <div class="govuk-notification-banner__content">
-      <strong>The absorbing organisation must accept the Data Sharing Agreement before merging.</strong>
-      <br>
-      <% if @merge_request.dpo_user %>
-        Contact the Data Protection Officer: <%= link_to @merge_request.dpo_user.name, user_path(@merge_request.dpo_user.id) %>
-      <% else %>
-        <%= @merge_request.absorbing_organisation_name %> does not have a Data Protection Officer. You can assign one on the <%= link_to "users page", "#{organisation_path(@merge_request.absorbing_organisation_id)}/users" %>.
-      <% end %>
-    </div>
-  </div>
-<% end %>
+
+<%= render partial: "notification_banners" %>
+
 <h1 class="govuk-heading-l">
   <span class="govuk-caption-l">Merge details</span>
   <%= display_value_or_placeholder(@merge_request.absorbing_organisation_name) %>

spec/requests/merge_requests_controller_spec.rb

@@ -396,6 +396,30 @@ RSpec.describe MergeRequestsController, type: :request do
         end
       end
     end
+
+    describe "#show" do
+      before do
+        get "/merge-request/#{merge_request.id}", headers:
+      end
+
+      context "when request has previously failed" do
+        let(:merge_request) { create(:merge_request, last_failed_attempt: Time.zone.yesterday) }
+
+        it "shows a banner" do
+          expect(page).to have_content("An error occurred while processing the merge.")
+          expect(page).to have_content("No changes have been made. Try beginning the merge again.")
+        end
+      end
+
+      context "when request has not previously failed" do
+        let(:merge_request) { create(:merge_request, last_failed_attempt: nil) }
+
+        it "does not show a banner" do
+          expect(page).not_to have_content("An error occurred while processing the merge.")
+          expect(page).not_to have_content("No changes have been made. Try beginning the merge again.")
+        end
+      end
+    end
   end
 
   context "when user is signed in with a data coordinator user" do