Integration and functionality testing

- Handle blank important note saving, whether an important note is present.
- Tests for New Edition and Edition History functionality are in place. These will have to be un-skipped in the future, when the functionality is ready.

app/controllers/notes_controller.rb

@@ -22,11 +22,6 @@ def create
     end
   end
 
-  def resolve
-    resolve_important_note
-    redirect_to history_edition_path(parent)
-  end
-
   def resource
     parent
   end
@@ -38,5 +33,6 @@ def resolve_important_note
       current_user.resolve_important_note(parent)
       flash[:success] = "Note resolved"
     end
+    redirect_to history_edition_path(parent)
   end
 end

app/views/editions/_important_note.html.erb

@@ -1,6 +1,9 @@
+<% note_author_name = User.find_by(id: @edition.important_note&.requester_id).name %>
+<% note_date = @edition.important_note&.created_at&.strftime("%d %B %Y") %>
+<% note_details = @edition.important_note&.comment %>
 <%= render "govuk_publishing_components/components/notice", {
-  description_govspeak: sanitize("<p class='govuk-body govuk-!-font-weight-bold govuk-!-text-break-word'>#{h(@edition.important_note&.comment)}</p>") +
-    sanitize("<p class='govuk-body-m'>#{h(User.find_by(id: @edition.important_note&.requester_id)&.name)} added an important note on
-                  #{h(@edition.important_note&.created_at&.strftime("%d %B %Y"))}</p>"),
-  show_banner_title: true
+  description_govspeak:
+    sanitize("<p class='govuk-body govuk-!-font-weight-bold govuk-!-text-break-word'>#{h(note_details)}</p>") +
+      sanitize("<p class='govuk-body-m'>#{h(note_author_name)} added an important note on #{h(note_date)}</p>"),
+  show_banner_title: true,
 } %>

app/views/editions/show.html.erb

@@ -23,14 +23,9 @@
       <%= render "govuk_publishing_components/components/summary_list", {
         items: document_summary_items(@resource),
       } %>
-  </div>
-
-  <div>
-    <%= render "govuk_publishing_components/components/notice", {
-      title: @edition.important_note.comment,
-      description_govspeak: sanitize("<p>#{User.find_by(id: @edition.important_note.requester_id).name} added an important note on #{@edition.important_note.created_at.strftime("%d %B %Y")}</p>"),
-      show_banner_title: true
-    } %>
+    <% if @edition.important_note %>
+      <%= render partial: "important_note" %>
+    <% end %>
   </div>
 
   <% if @edition.in_review? && current_user.has_editor_permissions?(@edition) %>

test/functional/notes_controller_test.rb

@@ -66,9 +66,49 @@ class NotesControllerTest < ActionController::TestCase
         assert_redirected_to history_edition_path(@edition)
         assert_includes flash[:success], "Note recorded"
       end
-    end
 
-    # TODO: Test that a blank note is saved and message is "Note resolved"
+      should "resolve important note if an empty note is saved, when an important note is already present" do
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: @note_text,
+               },
+             }
+
+        @edition.reload
+
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: "",
+               },
+             }
+
+        @edition.reload
+
+        assert_redirected_to history_edition_path(@edition)
+        assert_includes flash[:success], "Note resolved"
+      end
+
+      should "warn user if they try to save an empty important note, when there's no important note on the document already" do
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: "",
+               },
+             }
+
+        @edition.reload
+
+        assert_redirected_to history_edition_path(@edition)
+      end
+    end
 
     context "Welsh editors" do
       setup do

test/integration/edition_edit_test.rb

@@ -14,11 +14,9 @@ class EditionEditTest < IntegrationTest
   end
 
   context "edit page" do
-    setup do
+    should "show all the tabs when user has required permission and edition is published" do
       visit_published_edition
-    end
 
-    should "show all the tabs when user has required permission and edition is published" do
       assert page.has_text?("Edit")
       assert page.has_text?("Tagging")
       assert page.has_text?("Metadata")
@@ -29,6 +27,8 @@ class EditionEditTest < IntegrationTest
     end
 
     should "show document summary and title" do
+      visit_published_edition
+
       assert page.has_title?("Edit page title")
 
       row = find_all(".govuk-summary-list__row")
@@ -41,6 +41,8 @@ class EditionEditTest < IntegrationTest
     end
 
     should "indicate when an edition does not have an assignee" do
+      visit_published_edition
+
       within all(".govuk-summary-list__row")[0] do
         assert_selector(".govuk-summary-list__key", text: "Assigned to")
         assert_selector(".govuk-summary-list__value", text: "None")
@@ -57,13 +59,26 @@ class EditionEditTest < IntegrationTest
     end
 
     should "display the important note if an important note exists" do
-      setup do
-        @note_text = "This is really really urgent!"
-        create_important_note_for_edition(@published_edition, @note_text)
-      end
+      note_text = "This is really really urgent!"
+      create_draft_edition
+      create_important_note_for_edition(@draft_edition, note_text)
+      visit edition_path(@draft_edition)
 
       assert page.has_text?("Important")
-      assert page.has_text?(@note_text)
+      assert page.has_text?(note_text)
+    end
+
+    should "display only the most recent important note at the top" do
+      first_note = "This is really really urgent!"
+      second_note = "This should display only!"
+      create_draft_edition
+      create_important_note_for_edition(@draft_edition, first_note)
+      create_important_note_for_edition(@draft_edition, second_note)
+
+      visit edition_path(@draft_edition)
+
+      assert page.has_text?("Important")
+      assert page.has_text?(second_note)
     end
   end
 
@@ -182,13 +197,11 @@ class EditionEditTest < IntegrationTest
   end
 
   context "Update important note page" do
-    setup do
+    should "render the 'Update important note' page" do
       visit_draft_edition
       click_link("History and notes")
       click_link("Update important note")
-    end
 
-    should "render the 'Update important note' page" do
       within :css, ".gem-c-heading" do
         assert page.has_css?("h1", text: "Update important note")
         assert page.has_css?(".gem-c-heading__context", text: @draft_edition.title)
@@ -207,11 +220,49 @@ class EditionEditTest < IntegrationTest
     end
 
     should "pre-populate with the existing note" do
-      create_important_note_for_edition(@draft_edition, "An updated note")
+      note_text = "This is really really urgent!"
+      create_draft_edition
+      create_important_note_for_edition(@draft_edition, note_text)
+      visit edition_path(@draft_edition)
+
+      click_link("History and notes")
       click_link("Update important note")
 
-      assert page.has_field?("Important note", with: "An updated note")
+      assert page.has_field?("Important note", with: note_text)
     end
+
+    # TODO
+    # NOTICE Should *not* show important note in edition history.
+
+    # should "not show important notes in edition history" do
+    #   note_text = "This is really really urgent!"
+    #   note_text_2 = "Another note"
+    #   note_text_3 = "Yet another note"
+    #   create_draft_edition
+    #   create_important_note_for_edition(@draft_edition, note_text)
+    #   create_important_note_for_edition(@draft_edition, note_text_2)
+    #   create_important_note_for_edition(@draft_edition, note_text_3)
+    #   visit edition_path(@draft_edition)
+    #   click_link("History and notes")
+    #   TODO Expand 'All Notes' sections! Currently in progress.
+
+    #   New asserts go here
+    # end
+
+    # NOTICE: This is futureproofing, would come in handy when the "Create new edition" functionality
+    # has been implemented.
+
+    # should "not be carried forward to new editions" do
+    #   note_text = "This important note should not appear on a new edition."
+    #   create_published_edition
+    #   create_important_note_for_edition(@published_edition, note_text)
+    #   visit edition_path(@published_edition)
+
+    #   assert page.has_content? note_text
+
+    #   click_on "Create new edition"
+    #   assert page.has_no_text? ("Important note")
+    # end
   end
 
   context "unpublish tab" do
@@ -854,8 +905,12 @@ class EditionEditTest < IntegrationTest
 
 private
 
-  def visit_draft_edition
+  def create_draft_edition
     @draft_edition = FactoryBot.create(:edition, title: "Edit page title", state: "draft", overview: "metatags", in_beta: 1, body: "The body")
+  end
+
+  def visit_draft_edition
+    create_draft_edition
     visit edition_path(@draft_edition)
   end
 
@@ -928,7 +983,6 @@ def create_published_edition
       state: "published",
       slug: "can-i-get-a-driving-licence",
     )
-    visit edition_path(@published_edition)
   end
 
   def visit_retired_edition_in_published
@@ -962,6 +1016,7 @@ def visit_old_edition_of_published_edition
   def create_important_note_for_edition(edition, note_text)
     FactoryBot.create(
       :action,
+      requester: @govuk_editor,
       request_type: Action::IMPORTANT_NOTE,
       edition: edition,
       comment: note_text,

test/support/factories.rb

@@ -384,4 +384,10 @@
       end
     end
   end
+
+  factory :action do
+    request_type { Action::IMPORTANT_NOTE }
+    edition
+    comment { "Default comment" }
+  end
 end