CLDC-3607: Allow support users to delete notifications (by setting their end date) (#2630)

* CLDC-3607: Allow support users to delete notifications (by setting their end date)

* Refactor render_for_page helper

* Move db queries to homepage presenter

* Fix lint

* Use a before_action to find notification in controller

* Make delete notification links red

* Tweak link to markdown guide

app/controllers/notifications_controller.rb

@@ -1,16 +1,21 @@
 class NotificationsController < ApplicationController
   before_action :authenticate_user!, except: %i[show]
   before_action :authenticate_scope!, except: %i[show dismiss]
+  before_action :find_notification, except: %i[new create]
+
+  rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
 
   def dismiss
-    @notification = Notification.find_by(id: params[:notification_id])
     @notification.mark_as_read! for: current_user
     redirect_back(fallback_location: root_path)
   end
 
   def show
-    @notification = current_user&.support? ? Notification.find_by(id: params[:id]) : Notification.active.find_by(id: params[:id])
+    if !@notification.show_on_unauthenticated_pages && !current_user
-    if @notification&.show_additional_page && (@notification&.show_on_unauthenticated_pages || current_user)
+      render_not_found and return
+    end
+
+    if @notification.show_additional_page
       render "show"
     else
       redirect_back(fallback_location: root_path)
@@ -31,21 +36,7 @@ class NotificationsController < ApplicationController
     end
   end
 
-  def check_answers
-    @notification = Notification.find_by(id: params[:notification_id])
-    render_not_found and return unless @notification
-
-    render "notifications/check_answers"
-  end
-
-  def edit
-    @notification = Notification.find_by(id: params[:id])
-  end
-
   def update
-    @notification = Notification.find_by(id: params[:id])
-    render_not_found and return unless @notification
-
     start_now = params[:notification][:start_now]
 
     if @notification.errors.empty? && @notification.update(notification_model_params)
@@ -62,6 +53,12 @@ class NotificationsController < ApplicationController
     end
   end
 
+  def delete
+    @notification.update!(end_date: Time.zone.now)
+    flash[:notice] = "The notification has been deleted"
+    redirect_to root_path
+  end
+
 private
 
   def notification_params
@@ -84,4 +81,13 @@ private
 
     model_params
   end
+
+  def find_notification
+    id = params[:id] || params[:notification_id]
+    @notification = current_user&.support? ? Notification.find_by(id:) : Notification.active.find_by(id:)
+
+    raise ActiveRecord::RecordNotFound unless @notification
+
+    @notification
+  end
 end

app/helpers/notifications_helper.rb

@@ -27,11 +27,18 @@ module NotificationsHelper
     render_normal_markdown(title)
   end
 
-  def render_for_page(title, page_content)
+  def render_for_page(notification)
-    content = page_content
+    content_includes_own_title = /\A\s*#[^#]/.match?(notification.page_content)
-    unless /\A\s*#[^#]/.match?(page_content)
+    return render_normal_markdown(notification.page_content) if content_includes_own_title
-      content = "# #{title}\n#{page_content}"
+
+    content = "# #{notification.title}\n#{notification.page_content}"
+    render_normal_markdown(content)
   end
+
+  def render_for_home(notification)
+    return render_normal_markdown(notification.title) unless notification.show_additional_page
+
+    content = "#{notification.title}  \n[#{notification.link_text}](#{notification_path(notification)})"
     render_normal_markdown(content)
   end
 

app/presenters/homepage_presenter.rb

@@ -2,7 +2,7 @@ class HomepagePresenter
   include Rails.application.routes.url_helpers
   include CollectionTimeHelper
 
-  attr_reader :current_year_in_progress_lettings_data, :current_year_completed_lettings_data, :current_year_in_progress_sales_data, :current_year_completed_sales_data, :last_year_in_progress_lettings_data, :last_year_completed_lettings_data, :last_year_in_progress_sales_data, :last_year_completed_sales_data, :incomplete_schemes_data
+  attr_reader :current_year_in_progress_lettings_data, :current_year_completed_lettings_data, :current_year_in_progress_sales_data, :current_year_completed_sales_data, :last_year_in_progress_lettings_data, :last_year_completed_lettings_data, :last_year_in_progress_sales_data, :last_year_completed_sales_data, :incomplete_schemes_data, :active_notifications
 
   def initialize(user)
     @user = user
@@ -25,6 +25,7 @@ class HomepagePresenter
         path: schemes_path(status: [:incomplete], owning_organisation_select: "all"),
       }
     end
+    @active_notifications = Notification.active if @user.support?
   end
 
   def title_text_for_user

app/views/home/index.html.erb

@@ -7,9 +7,7 @@
       </div>
 
       <% if @current_user.support? %>
-        <div class="govuk-grid-row">
+        <%= render partial: "notifications/notification_home_section", locals: { active_notifications: @homepage_presenter.active_notifications } %>
-          <%= govuk_button_link_to "Create a new notification", new_notification_path %>
-        </div>
       <% end %>
 
       <div class="app-data-box-section govuk-grid-row">

app/views/notifications/_form.html.erb

@@ -26,7 +26,7 @@
       <% end %>
     <% end %>
 
-    <span class="govuk-caption-m govuk-!-margin-bottom-6">View <%= govuk_link_to "markdown syntax guide at markdownguide.org", "https://www.markdownguide.org/basic-syntax/" %></span>
+    <span class="govuk-caption-m govuk-!-margin-bottom-6"><%= govuk_link_to "Find out more about using Markdown at Markdown Guide", "https://www.markdownguide.org/basic-syntax/" %></span>
 
     <%= f.govuk_submit "Continue" %>
   </div>

app/views/notifications/_notification_home_section.html.erb

@@ -0,0 +1,17 @@
+<div class="govuk-grid-row">
+  <p class="govuk-!-font-weight-bold"><%== I18n.t("active_notifications", count: active_notifications.count) %></p>
+  <% active_notifications.each do |notification| %>
+    <div class="govuk-grid-row">
+      <div class="govuk-grid-column-three-quarters">
+        <%== render_for_home(notification) %>
+      </div>
+      <div class="govuk-grid-column-one-quarter">
+        <%= govuk_link_to("Delete notification", notification_delete_confirmation_path(notification), class: "app-!-colour-red") %>
+      </div>
+    </div>
+  <% end %>
+</div>
+
+<div class="govuk-grid-row">
+  <%= govuk_button_link_to "Create a new notification", new_notification_path %>
+</div>

app/views/notifications/delete_confirmation.html.erb

@@ -0,0 +1,27 @@
+<% content_for :before_content do %>
+  <% content_for :title, "Are you sure you want to delete this notification?" %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <h1 class="govuk-heading-xl">
+      <%= content_for(:title) %>
+    </h1>
+
+    <p class="govuk-body"><%== render_for_summary("**Notification:** #{@notification.title}") %></p>
+
+    <p class="govuk-body">Users will no longer see this notification.</p>
+
+    <%= govuk_warning_text(text: "You will not be able to undo this action.") %>
+
+    <div class="govuk-button-group">
+      <%= govuk_button_to(
+            "Delete notification",
+            notification_delete_path(@notification),
+            method: :delete,
+          ) %>
+      <%= govuk_button_link_to "Cancel", root_path, html: { method: :get }, secondary: true %>
+    </div>
+  </div>
+</div>

app/views/notifications/show.html.erb

@@ -5,7 +5,7 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
-    <%== render_for_page(@notification.title, @notification.page_content) %>
+    <%== render_for_page(@notification) %>
   </div>
 </div>
 <br>

config/locales/en.yml

@@ -41,6 +41,11 @@ en:
     create_password: "Create a password to finish setting up your account"
     reset_password: "Reset your password"
 
+  active_notifications:
+    zero: "There are no active notifications"
+    one: "There is one active notification:"
+    other: "There are %{count} active notifications:"
+
   activemodel:
     errors:
       models:

config/routes.rb

@@ -141,6 +141,8 @@ Rails.application.routes.draw do
   resources :notifications do
     get "dismiss", to: "notifications#dismiss"
     get "check-answers", to: "notifications#check_answers"
+    get "delete-confirmation", to: "notifications#delete_confirmation"
+    delete "delete", to: "notifications#delete"
   end
 
   resources :organisations do

spec/requests/notifications_controller_spec.rb

@@ -101,6 +101,17 @@ RSpec.describe NotificationsController, type: :request do
         end
       end
     end
+
+    describe "#delete" do
+      let(:notification) { create(:notification, end_date: nil) }
+      let(:request) { delete notification_delete_path(notification) }
+
+      it "sets end_date on the notification" do
+        request
+        notification.reload
+        expect(notification.end_date).to be < Time.zone.now
+      end
+    end
   end
 
   context "when user is signed in as a non-support user" do
@@ -125,12 +136,28 @@ RSpec.describe NotificationsController, type: :request do
 
     describe "#update" do
       let(:notification) { create(:notification) }
-      let(:request) { patch notification_path(notification), params: { "notification": { title: "Test Create" } } }
+      let(:request) { patch notification_path(notification), params: { "notification": { title: "Test Update" } } }
 
       it "returns not found" do
         request
         expect(response).to have_http_status(:not_found)
       end
     end
+
+    describe "#delete" do
+      let(:notification) { create(:notification, end_date: nil) }
+      let(:request) { delete notification_delete_path(notification) }
+
+      it "returns not found" do
+        request
+        expect(response).to have_http_status(:not_found)
+      end
+
+      it "does not set end_date on the notification" do
+        request
+        notification.reload
+        expect(notification.end_date).to be_nil
+      end
+    end
   end
 end