Merge pull request #2549 from alphagov/557-History-notes-tab_Update-i…

…mportant-note

557 history notes tab update important note
https://trello.com/c/8Mrwt4nG

The changes have added an Update Important Note function with button, to the side bar, visible from the History and Notes tab

app/controllers/editions_controller.rb

@@ -159,6 +159,10 @@ def add_edition_note
     render "secondary_nav_tabs/add_edition_note"
   end
 
+  def update_important_note
+    render "secondary_nav_tabs/update_important_note"
+  end
+
   def destroy
     @resource.destroy!
     flash[:success] = "Edition deleted"

app/controllers/notes_controller.rb

@@ -18,15 +18,10 @@ def create
       redirect_to history_edition_path(parent)
     else
       flash[:danger] = "Note failed to save"
-      redirect_to history_add_edition_note_edition_path(resource)
+      redirect_to type == Action::IMPORTANT_NOTE ? history_update_important_note_edition_path(resource) : history_add_edition_note_edition_path(resource)
     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

@@ -0,0 +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(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/secondary_nav_tabs/_history.html.erb

@@ -19,6 +19,12 @@
             margin_bottom: 3,
             href: history_add_edition_note_edition_path,
           }),
+          (render "govuk_publishing_components/components/button", {
+            text: "Update important note",
+            margin_bottom: 3,
+            secondary_solid: true,
+            href: history_update_important_note_edition_path,
+          }),
         ],
       } %>
     </div>

app/views/editions/secondary_nav_tabs/update_important_note.html.erb

@@ -0,0 +1,38 @@
+<% @edition = @resource %>
+<% content_for :title_context, @edition.title %>
+<% content_for :page_title, "Update important note" %>
+<% content_for :title, "Update important note" %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <p class="govuk-body">
+      Add important notes that anyone who works on this edition needs to see, eg “(Doesn’t) need fact check, don’t publish.”.
+      Each edition can have only one important note at a time.
+    </p>
+    <p class="govuk-body">
+      To delete the important note, clear any comments and select ‘Save’.
+    </p>
+
+    <%= form_for(:note, :url=> notes_path) do %>
+      <%= hidden_field_tag :edition_id, @edition.id %>
+      <%= hidden_field_tag "note[type]", Action::IMPORTANT_NOTE %>
+
+      <%= render "govuk_publishing_components/components/textarea", {
+        label: {
+          heading_size: "m",
+          text: "Important note",
+        },
+        name: "note[comment]",
+        rows: 14,
+        value: @edition.important_note&.comment,
+      } %>
+
+      <div class="govuk-button-group">
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Save",
+        } %>
+        <%= link_to("Cancel", history_edition_path, class: "govuk-link govuk-link--no-visited-state") %>
+      </div>
+    <% end %>
+  </div>
+</div>

app/views/editions/show.html.erb

@@ -23,6 +23,9 @@
     <%= render "govuk_publishing_components/components/summary_list", {
       items: document_summary_items(@resource),
     } %>
+    <% if @edition.important_note %>
+      <%= render partial: "important_note" %>
+    <% end %>
   </div>
 
   <% if @edition.in_review? && current_user.has_editor_permissions?(@edition) %>

config/routes.rb

@@ -28,6 +28,7 @@
         get "metadata"
         get "history"
         get "history/add_edition_note", to: "editions#add_edition_note", as: "history/add_edition_note"
+        get "history/update_important_note", to: "editions#update_important_note", as: "history/update_important_note"
         get "admin"
         post "duplicate"
         get "related_external_links"

test/functional/notes_controller_test.rb

@@ -49,6 +49,85 @@ class NotesControllerTest < ActionController::TestCase
       end
     end
 
+    context "when an Important note is provided" do
+      should "show 'Note recorded' and save a completed note" do
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: @note_text,
+               },
+             }
+
+        @edition.reload
+
+        assert_equal(@note_text, @edition.important_note.comment)
+        assert_redirected_to history_edition_path(@edition)
+        assert_equal "Note recorded", flash[:success]
+      end
+
+      should "show 'Note resolved' if a parent note exists and then an empty note is saved" do
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: "Parent note",
+               },
+             }
+
+        @edition.reload
+
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: "",
+               },
+             }
+        @edition.reload
+
+        assert_nil(@edition.important_note)
+        assert_redirected_to history_edition_path(@edition)
+        assert_equal "Note resolved", flash[:success]
+      end
+
+      should "show no flash message if no parent note exists and then an empty note is saved" do
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: "",
+               },
+             }
+
+        assert_nil(@edition.important_note)
+        assert_redirected_to history_edition_path(@edition)
+        assert flash.empty?, "Expected no flash message, but found: #{flash.inspect}"
+      end
+
+      should "show 'Note failed to save' if an error occurs while saving the Important note" do
+        @user.stubs(:record_note).with(@edition, @note_text, "important_note").returns(false)
+
+        post :create,
+             params: {
+               edition_id: @edition.id,
+               note: {
+                 type: "important_note",
+                 comment: @note_text,
+               },
+             }
+
+        @edition.reload
+
+        assert_redirected_to history_update_important_note_edition_path(@edition)
+        assert_equal "Note failed to save", flash[:danger]
+      end
+    end
+
     context "Welsh editors" do
       setup do
         login_as_welsh_editor

test/integration/edition_edit_test.rb

@@ -15,11 +15,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")
@@ -30,6 +28,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")
@@ -42,6 +42,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")
@@ -56,6 +58,32 @@ class EditionEditTest < IntegrationTest
         assert_selector(".govuk-summary-list__value", text: @draft_edition.assignee)
       end
     end
+
+    should "display the important note if an important note exists" do
+      note_text = "This is really really urgent!"
+      create_draft_edition
+      create_important_note_for_edition(@draft_edition, note_text)
+      visit edition_path(@draft_edition)
+
+      within :css, ".govuk-notification-banner" do
+        assert page.has_text?("Important")
+        assert page.has_text?(note_text)
+      end
+    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)
+      assert page.has_no_text?(first_note)
+    end
   end
 
   context "edit assignee page" do
@@ -134,6 +162,16 @@ class EditionEditTest < IntegrationTest
 
       assert_current_path history_add_edition_note_edition_path(@draft_edition.id)
     end
+
+    should "show an 'Update important note' button" do
+      assert page.has_link?("Update important note")
+    end
+
+    should "navigate to the 'Update important note' page when the button is clicked" do
+      click_link("Update important note")
+
+      assert_current_path history_update_important_note_edition_path(@draft_edition.id)
+    end
   end
 
   context "Add edition note page" do
@@ -162,6 +200,68 @@ class EditionEditTest < IntegrationTest
     end
   end
 
+  context "Update important note page" do
+    should "render the 'Update important note' page" do
+      create_draft_edition
+      visit history_update_important_note_edition_path(@draft_edition)
+
+      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)
+      end
+
+      assert page.has_text?("Add important notes that anyone who works on this edition needs to see, eg “(Doesn’t) need fact check, don’t publish.”.")
+      assert page.has_text?("Each edition can have only one important note at a time.")
+      assert page.has_text?("To delete the important note, clear any comments and select ‘Save’.")
+
+      within :css, ".gem-c-textarea" do
+        assert page.has_css?("label", text: "Important note")
+        assert page.has_css?("textarea")
+      end
+
+      assert page.has_button?("Save")
+      assert page.has_link?("Cancel")
+    end
+
+    should "pre-populate with the existing note" do
+      note_text = "This is really really urgent!"
+      create_draft_edition
+      create_important_note_for_edition(@draft_edition, note_text)
+      visit history_update_important_note_edition_path(@draft_edition)
+
+      assert page.has_field?("Important note", with: note_text)
+    end
+
+    # TODO: Test to be switched on when the Edition notes history is implemented.
+    should "not show important notes in edition history" do
+      skip "Until this functionality is complete: #603 - History and notes tab (excluding sidebar) [Edit page]"
+      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
+
+    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
     context "user does not have required permissions" do
       setup do
@@ -974,8 +1074,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
 
@@ -1048,7 +1152,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
@@ -1078,4 +1181,14 @@ def visit_old_edition_of_published_edition
     )
     visit edition_path(published_edition)
   end
+
+  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,
+    )
+  end
 end

test/support/factories.rb

@@ -384,4 +384,11 @@
       end
     end
   end
+
+  factory :action do
+    request_type { Action::IMPORTANT_NOTE }
+    edition
+    requester { FactoryBot.create(:user) }
+    comment { "Default comment" }
+  end
 end