Merge pull request #8484 from alphagov/document_collections_orphan_do…

…cument_fix Fix Orphan Document scenario for Document Collection
alphagov · Nov 10, 2023 · b6a9bab · b6a9bab
2 parents edd126b + 5eca54c
commit b6a9bab
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 57 deletions.
diff --git a/app/helpers/admin/document_collection_group_memberships_helper.rb b/app/helpers/admin/document_collection_group_memberships_helper.rb
@@ -1,17 +1,23 @@
 module Admin::DocumentCollectionGroupMembershipsHelper
   def document_collection_group_member_title(membership)
-    if membership.non_whitehall_link
-      membership.non_whitehall_link.title
-    else
-      membership.document.latest_edition.title
-    end
+    return membership.non_whitehall_link.title if membership.non_whitehall_link
+
+    membership.document.latest_edition.title
   end
 
   def document_collection_group_member_url(membership)
-    if membership.non_whitehall_link
-      Plek.website_root + membership.non_whitehall_link.base_path
-    else
-      membership.document.latest_edition.public_url
+    return Plek.website_root + membership.non_whitehall_link.base_path if membership.non_whitehall_link
+
+    membership.document.latest_edition.public_url
+  end
+
+  def document_collection_group_member_unavailable?(membership)
+    !membership.non_whitehall_link && !membership.document&.latest_edition
+  end
+
+  def unavailable_document_count(memberships)
+    memberships.count do |membership|
+      document_collection_group_member_unavailable?(membership)
     end
   end
 end
diff --git a/app/views/admin/document_collection_group_memberships/confirm_destroy.html.erb b/app/views/admin/document_collection_group_memberships/confirm_destroy.html.erb
@@ -4,8 +4,9 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
-    <%= form_with url: admin_document_collection_group_document_collection_group_membership_path(@collection, @group, @membership), method: :delete do |form| %>
-      <p class="govuk-body govuk-!-margin-bottom-6">Are you sure you want to remove "<%= document_collection_group_member_title(@membership) %>" from this collection?</p>
+    <%= form_with url: admin_document_collection_group_document_collection_group_membership_path(@collection, @group, @membership), method: :delete do |_| %>
+      <% title = document_collection_group_member_unavailable?(@membership) ? tag.span("Unavailable Document", class: "govuk-!-font-weight-bold") : document_collection_group_member_title(@membership) %>
+      <p class="govuk-body govuk-!-margin-bottom-6">Are you sure you want to remove "<%= title %>" from this collection?</p>
 
        <div class="govuk-button-group">
         <%= render "govuk_publishing_components/components/button", {

diff --git a/app/views/admin/document_collection_group_memberships/index.html.erb b/app/views/admin/document_collection_group_memberships/index.html.erb
@@ -8,6 +8,19 @@
 <% content_for :context, @collection.title %>
 <% content_for :title_margin_bottom, 4 %>
 
+<% if unavailable_document_count(@group.memberships) > 0 %>
+  <% content_for :banner, render("govuk_publishing_components/components/notice", {
+    title: sanitize("<p>Remove #{unavailable_document_count(@group.memberships)} unavailable document(s) within the group.</p>"),
+    show_banner_title: true,
+    data_attributes: {
+      module: "gem-track-click",
+      "track-category": "important-banner",
+      "track-action": "unavailable-documents-exist-in-document-collection-group",
+      "track-label": "Important",
+    },
+  }) %>
+<% end %>
+
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <p class="govuk-body">
@@ -19,64 +32,50 @@
       items: secondary_navigation_tabs_items(@group, request.path),
     } %>
 
-    <% if @group.memberships.many? %>
-      <div class="govuk-grid-row">
-        <div class="govuk-grid-column-full app-view-document-collection-group-memberships-index__heading ">
-          <%= render "govuk_publishing_components/components/heading", {
-            text: "Documents",
-            margin_bottom: 6,
-          } %>
-          <ul class="govuk-list">
-            <li>
-              <%= link_to "Add document", admin_document_collection_group_search_options_path(@collection, @group), class: "govuk-link" %>
-            </li>
+    <div class="govuk-grid-row">
+      <div class="govuk-grid-column-full app-view-document-collection-group-memberships-index__heading ">
+        <%= render "govuk_publishing_components/components/heading", {
+          text: "Documents",
+          margin_bottom: 6,
+        } %>
+        <ul class="govuk-list">
+          <li>
+            <%= link_to "Add document", admin_document_collection_group_search_options_path(@collection, @group), class: "govuk-link" %>
+          </li>
 
+          <% if @group.memberships.many? %>
             <li>
               <%= link_to "Reorder document", reorder_admin_document_collection_group_document_collection_group_memberships_path(@collection, @group), class: "govuk-link" %>
             </li>
-          </ul>
-        </div>
+          <% end %>
+        </ul>
       </div>
-    <% else %>
-      <div class="govuk-grid-row">
-        <div class="govuk-grid-column-full app-view-document-collection-group-memberships-index__heading">
-          <%= render "govuk_publishing_components/components/heading", {
-            text: "Documents",
-            margin_bottom: 6,
-          } %>
-
-          <ul class="govuk-list">
-            <li>
-              <%= link_to "Add document", admin_document_collection_group_search_options_path(@collection, @group), class: "govuk-link" %>
-            </li>
-          </ul>
-        </div>
-      </div>
-    <% end %>
+    </div>
 
     <% if @group.memberships.present? %>
       <div class="govuk-table--with-actions app-view-document-collection-group-memberships-index__table">
         <%= render "govuk_publishing_components/components/table", {
           rows: @group.memberships.map do |membership|
-            title = document_collection_group_member_title(membership)
-            url = document_collection_group_member_url(membership)
-            [
-              {
-                text: title,
-              },
-              {
-                text: link_to(sanitize("View #{tag.span(title, class: "govuk-visually-hidden")}"), url, class: "govuk-link") +
-                  link_to(sanitize("Remove #{tag.span(title, class: "govuk-visually-hidden")}"), confirm_destroy_admin_document_collection_group_document_collection_group_membership_path(@collection, @group, membership), class: "govuk-link gem-link--destructive govuk-!-margin-left-3"),
-              },
-            ]
+            if document_collection_group_member_unavailable?(membership)
+              title = tag.span("Unavailable Document", class: "govuk-!-font-weight-bold")
+              remove_link = link_to(sanitize("Remove #{tag.span(title, class: "govuk-visually-hidden")}"), confirm_destroy_admin_document_collection_group_document_collection_group_membership_path(@collection, @group, membership), class: "govuk-link gem-link--destructive govuk-!-margin-left-3")
+              links = remove_link
+            else
+              title = document_collection_group_member_title(membership)
+              url = document_collection_group_member_url(membership)
+              view_link = link_to(sanitize("View #{tag.span(title, class: "govuk-visually-hidden")}"), url, class: "govuk-link")
+              remove_link = link_to(sanitize("Remove #{tag.span(title, class: "govuk-visually-hidden")}"), confirm_destroy_admin_document_collection_group_document_collection_group_membership_path(@collection, @group, membership), class: "govuk-link gem-link--destructive govuk-!-margin-left-3")
+              links = view_link + remove_link
+            end
+
+            [{ text: title }, { text: links } ]
           end,
         } %>
       </div>
     <% else %>
       <%= render "govuk_publishing_components/components/warning_text", {
         text: "There are no documents inside this group",
       } %>
-
       <hr class="govuk-section-break govuk-section-break--m govuk-section-break--visible">
     <% end %>
   </div>

diff --git a/app/views/admin/document_collection_group_memberships/reorder.html.erb b/app/views/admin/document_collection_group_memberships/reorder.html.erb
@@ -11,9 +11,10 @@
       } %>
       <%= render "govuk_publishing_components/components/reorderable_list", {
         items: @group.memberships.map do |membership|
+          title = document_collection_group_member_unavailable?(membership) ? tag.span("Unavailable Document", class: "govuk-!-font-weight-bold") : document_collection_group_member_title(membership)
           {
             id: membership.id,
-            title: document_collection_group_member_title(membership),
+            title: title,
           }
         end,
       } %>

diff --git a/test/functional/admin/document_collection_group_memberships_controller_test.rb b/test/functional/admin/document_collection_group_memberships_controller_test.rb
@@ -130,4 +130,42 @@ def id_params
     assert_redirected_to admin_document_collection_group_document_collection_group_memberships_path(@collection, @group),
                          notice: "'#{title}' added to '#{@group.heading}'"
   end
+
+  view_test "GET #index should render 'unavailable documents' and notification" do
+    document = create(:document, latest_edition: nil)
+    whitehall_membership = create(:document_collection_group_membership, document:)
+    whitehall_confirm_destroy_path = confirm_destroy_admin_document_collection_group_document_collection_group_membership_path(@collection, @group, whitehall_membership)
+
+    @group.memberships << whitehall_membership
+
+    get :index, params: { document_collection_id: @collection, group_id: @group }
+
+    assert_select ".govuk-table__row", { count: 0, text: "View Unavailable Document" }
+    assert_select ".govuk-table__row", text: /Remove Unavailable Document/
+    assert_select ".govuk-table__row", a: whitehall_confirm_destroy_path
+    assert_select ".govuk-notification-banner__heading", text: "Remove 1 unavailable document(s) within the group."
+  end
+
+  view_test "GET #reorder should render 'unavailable documents'" do
+    document = create(:document, latest_edition: nil)
+    whitehall_membership = create(:document_collection_group_membership, document:)
+    @group.memberships << whitehall_membership
+    get :reorder, params: { document_collection_id: @collection, group_id: @group }
+
+    assert_response :success
+    assert_select "h1", /Reorder documents/
+    assert_select ".gem-c-reorderable-list__title", text: "Unavailable Document"
+  end
+
+  view_test "GET #confirm_destroy should render 'unavailable documents'" do
+    document = create(:document, latest_edition: nil)
+    whitehall_membership = create(:document_collection_group_membership, document:)
+    @group.memberships << whitehall_membership
+
+    get :confirm_destroy, params: { document_collection_id: @collection, group_id: @group, id: whitehall_membership }
+
+    assert_response :success
+    assert_select "h1", /Remove document/
+    assert_select "p", /Are you sure you want to remove "Unavailable Document" from this collection?/
+  end
 end
diff --git a/test/unit/app/helpers/admin/document_collection_group_memberships_helper_test.rb b/test/unit/app/helpers/admin/document_collection_group_memberships_helper_test.rb
@@ -5,7 +5,7 @@ class Admin::DocumentCollectionGroupMembershipsHelperTest < ActionView::TestCase
     @document_collection_group = build(:document_collection, :with_group).groups.first
   end
 
-  test "document_collection_group_member_title should return title if membership is a non_whitehall_link" do
+  test "#document_collection_group_member_title should return title if membership is a non_whitehall_link" do
     non_whitehall_link = DocumentCollectionNonWhitehallLink.new(
       base_path: "GOVUK PATH",
       title: "GOVUK TITLE",
@@ -14,14 +14,14 @@ class Admin::DocumentCollectionGroupMembershipsHelperTest < ActionView::TestCase
     assert_equal "GOVUK TITLE", document_collection_group_member_title(membership)
   end
 
-  test "document_collection_group_member_title should return title if membership is a document" do
+  test "#document_collection_group_member_title should return title if membership is a document" do
     edition = build(:edition, title: "DOC TITLE")
     document = build(:document, slug: "DOC PATH", latest_edition: edition)
     membership = @document_collection_group.memberships.build(document:)
     assert_equal "DOC TITLE", document_collection_group_member_title(membership)
   end
 
-  test "document_collection_group_member_url should return full url if membership is a non_whitehall_link" do
+  test "#document_collection_group_member_url should return full url if membership is a non_whitehall_link" do
     non_whitehall_link = DocumentCollectionNonWhitehallLink.new(
       base_path: "/GOVUK-PATH",
       title: "GOVUK TITLE",
@@ -30,10 +30,55 @@ class Admin::DocumentCollectionGroupMembershipsHelperTest < ActionView::TestCase
     assert_equal "https://www.test.gov.uk/GOVUK-PATH", document_collection_group_member_url(membership)
   end
 
-  test "document_collection_group_member_url should return public url if membership is a document" do
+  test "#document_collection_group_member_url should return public url if membership is a document" do
     edition = build(:edition, title: "DOC TITLE")
     document = build(:document, slug: "DOC-PATH", latest_edition: edition)
     membership = @document_collection_group.memberships.build(document:)
     assert_equal "https://www.test.gov.uk/government/generic-editions/DOC-PATH", document_collection_group_member_url(membership)
   end
+
+  test "#document_collection_group_member_unavailable? should return true if membership is a document without a latest_edition" do
+    document = build(:document, slug: "DOC PATH", latest_edition: nil)
+    membership = @document_collection_group.memberships.build(document:)
+    assert_equal true, document_collection_group_member_unavailable?(membership)
+  end
+
+  test "#document_collection_group_member_unavailable? should return false if membership is a document with a latest_edition" do
+    edition = build(:edition, title: "DOC TITLE")
+    document = build(:document, slug: "DOC PATH", latest_edition: edition)
+    membership = @document_collection_group.memberships.build(document:)
+    assert_equal false, document_collection_group_member_unavailable?(membership)
+  end
+
+  test "#document_collection_group_member_unavailable? should return false if membership is a non_whitehall_link" do
+    non_whitehall_link = DocumentCollectionNonWhitehallLink.new(
+      base_path: "GOVUK PATH",
+      title: "GOVUK TITLE",
+    )
+    membership = @document_collection_group.memberships.build(non_whitehall_link:)
+    assert_equal false, document_collection_group_member_unavailable?(membership)
+  end
+
+  test "#unavailable_document_count should return the number of documents without editions" do
+    document = build(:document, slug: "DOC PATH", latest_edition: nil)
+    @document_collection_group.memberships.build(document:)
+    @document_collection_group.memberships.build(document:)
+    assert_equal 2, unavailable_document_count(@document_collection_group.memberships)
+  end
+
+  test "#unavailable_document_count should return 0 if there are documents with editions" do
+    edition = build(:edition, title: "DOC TITLE")
+    document = build(:document, slug: "DOC PATH", latest_edition: edition)
+    @document_collection_group.memberships.build(document:)
+    assert_equal 0, unavailable_document_count(@document_collection_group.memberships)
+  end
+
+  test "#unavailable_document_count should not count non_whitehall_links" do
+    non_whitehall_link = DocumentCollectionNonWhitehallLink.new(
+      base_path: "GOVUK PATH",
+      title: "GOVUK TITLE",
+    )
+    @document_collection_group.memberships.build(non_whitehall_link:)
+    assert_equal 0, unavailable_document_count(@document_collection_group.memberships)
+  end
 end