CLDC-3614: Allow markdown page content when creating notifications

app/controllers/notifications_controller.rb

@@ -9,8 +9,8 @@ class NotificationsController < ApplicationController
   end
 
   def show
-    @notification = Notification.find_by(id: params[:id])
+    @notification = current_user&.support? ? Notification.find_by(id: params[:id]) : Notification.active.find_by(id: params[:id])
-    if @notification&.page_content && (@notification&.show_on_unauthenticated_pages || current_user)
+    if @notification&.show_additional_page && (@notification&.show_on_unauthenticated_pages || current_user)
       render "show"
     else
       redirect_back(fallback_location: root_path)
@@ -22,7 +22,7 @@ class NotificationsController < ApplicationController
   end
 
   def create
-    @notification = Notification.new(notification_params)
+    @notification = Notification.new(notification_model_params)
 
     if @notification.errors.empty? && @notification.save
       redirect_to notification_check_answers_path(@notification)
@@ -48,10 +48,7 @@ class NotificationsController < ApplicationController
 
     start_now = params[:notification][:start_now]
 
-    update_params = notification_params.except(:start_now)
+    if @notification.errors.empty? && @notification.update(notification_model_params)
-    update_params[:start_date] = Time.zone.now if start_now
-
-    if @notification.errors.empty? && @notification.update(update_params)
       if start_now
         flash[:notice] = "The notification has been created"
         redirect_to root_path
@@ -68,10 +65,23 @@ class NotificationsController < ApplicationController
 private
 
   def notification_params
-    params.require(:notification).permit(:title, :show_on_unauthenticated_pages, :start_now)
+    params.require(:notification).permit(:title, :show_on_unauthenticated_pages, :show_additional_page, :link_text, :page_content, :start_now)
   end
 
   def authenticate_scope!
     render_not_found unless current_user.support?
   end
+
+  def notification_model_params
+    model_params = notification_params.except(:start_now)
+
+    if notification_params[:show_additional_page] == "0"
+      model_params[:link_text] = nil
+      model_params[:page_content] = nil
+    end
+
+    model_params[:start_date] = Time.zone.now if notification_params[:start_now]
+
+    model_params
+  end
 end

app/helpers/notifications_helper.rb

@@ -16,17 +16,37 @@ module NotificationsHelper
   end
 
   def render_for_banner(title)
-    banner_renderer = NotificationTitleRenderer.new({ invert_link_colour: true, bold_all_text: true })
+    # rubocop:disable Rails/HelperInstanceVariable
-    Redcarpet::Markdown.new(banner_renderer, no_intra_emphasis: true).render(title)
+    @banner_renderer ||= NotificationRenderer.new({ invert_link_colour: true, bold_all_text: true })
+    @banner_markdown ||= Redcarpet::Markdown.new(@banner_renderer, no_intra_emphasis: true)
+    @banner_markdown.render(title)
+    # rubocop:enable Rails/HelperInstanceVariable
   end
 
   def render_for_summary(title)
-    plain_title_renderer = NotificationTitleRenderer.new({ invert_link_colour: false, bold_all_text: false })
+    render_normal_markdown(title)
-    Redcarpet::Markdown.new(plain_title_renderer, no_intra_emphasis: true).render(title)
+  end
+
+  def render_for_page(title, page_content)
+    content = page_content
+    unless /\A\s*#[^#]/.match?(page_content)
+      content = "# #{title}\n#{page_content}"
+    end
+    render_normal_markdown(content)
+  end
+
+private
+
+  def render_normal_markdown(content)
+    # rubocop:disable Rails/HelperInstanceVariable
+    @on_page_renderer ||= NotificationRenderer.new({ invert_link_colour: false, bold_all_text: false })
+    @on_page_markdown ||= Redcarpet::Markdown.new(@on_page_renderer, no_intra_emphasis: true)
+    @on_page_markdown.render(content)
+    # rubocop:enable Rails/HelperInstanceVariable
   end
 end
 
-class NotificationTitleRenderer < Redcarpet::Render::HTML
+class NotificationRenderer < Redcarpet::Render::HTML
   def initialize(options = {})
     link_class = "govuk-link"
     link_class += " govuk-link--inverse" if options[:invert_link_colour]
@@ -35,9 +55,38 @@ class NotificationTitleRenderer < Redcarpet::Render::HTML
     super base_options
   end
 
+  def header(text, header_level)
+    header_size = case header_level
+                  when 1
+                    "xl"
+                  when 2
+                    "l"
+                  when 3
+                    "m"
+                  else
+                    "s"
+                  end
+
+    %(<h#{header_level} class="govuk-heading-#{header_size}">#{text}</h#{header_level}>)
+  end
+
   def paragraph(text)
     return %(<p class="govuk-!-font-weight-bold">#{text}</p>) if @bold # rubocop:disable Rails/HelperInstanceVariable
 
-    %(<p>#{text}</p>)
+    %(<p class="govuk-body-m">#{text}</p>)
+  end
+
+  def list(contents, list_type)
+    return %(<ol class="govuk-list govuk-list--number">#{contents}</ol>) if list_type == :ordered
+
+    %(<ul class="govuk-list govuk-list--bullet">#{contents}</ul>)
+  end
+
+  def hrule
+    %(<hr class="govuk-section-break govuk-section-break--xl govuk-section-break--visible">)
+  end
+
+  def block_quote(quote)
+    %(<div class="govuk-inset-text">#{quote}</div>)
   end
 end

app/models/notification.rb

@@ -2,6 +2,7 @@ class Notification < ApplicationRecord
   acts_as_readable
 
   validates :title, presence: { message: I18n.t("activerecord.errors.models.notification.attributes.title.blank") }
+  validate :validate_additional_page_information
 
   scope :active, -> { where("start_date <= ? AND (end_date >= ? OR end_date is null)", Time.zone.now, Time.zone.now) }
   scope :unauthenticated, -> { where(show_on_unauthenticated_pages: true) }
@@ -13,4 +14,18 @@ class Notification < ApplicationRecord
   def self.newest_active_unauthenticated_notification
     active_unauthenticated_notifications.last
   end
+
+private
+
+  def validate_additional_page_information
+    return unless show_additional_page
+
+    if link_text.blank?
+      errors.add :link_text, I18n.t("activerecord.errors.models.notification.attributes.link_text.blank_when_additional_page_set")
+    end
+
+    if page_content.blank?
+      errors.add :page_content, I18n.t("activerecord.errors.models.notification.attributes.page_content.blank_when_additional_page_set")
+    end
+  end
 end

app/views/notifications/_form.html.erb

@@ -18,8 +18,12 @@
                            label: { text: "Title", size: "m" },
                            hint: { text: "Use markdown for links to existing pages" } %>
 
-    <%= f.govuk_check_boxes_fieldset :show_on_unauthenticated_pages, multiple: false, legend: { text: "Display Options" } do %>
+    <%= f.govuk_check_boxes_fieldset :display_options, multiple: false, legend: { text: "Display Options" } do %>
       <%= f.govuk_check_box :show_on_unauthenticated_pages, 1, 0, multiple: false, label: { text: "Show this notification on unauthenticated pages, for example the start page" } %>
+      <%= f.govuk_check_box :show_additional_page, 1, 0, multiple: false, label: { text: "Include a link to a separate page with additional information" } do %>
+        <%= f.govuk_text_field :link_text, label: { text: "Link text" }, hint: { text: "Use descriptive language and relevant terms. The link text should make sense out of context." } %>
+        <%= f.govuk_text_area :page_content, label: { text: "Page content" }, hint: { text: "Use markdown to format the page content. <br/><br/> The page title will be the notification title by default. Use a heading level one if you want to override it." } %>
+      <% 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>

app/views/notifications/_notification_banner.html.erb

@@ -7,7 +7,7 @@
           <p>Notification 1 of <%= notification_count %></p>
         <% end %>
       <%== render_for_banner(notification.title) %>
-        <% if notification.page_content.present? %>
+        <% if notification.show_additional_page %>
           <div class="govuk-body">
             <%= govuk_link_to notification.link_text, notification_path(notification), class: "govuk-link--inverse govuk-!-font-weight-bold" %>
           </div>

app/views/notifications/check_answers.html.erb

@@ -31,6 +31,25 @@
               <% row.with_value { @notification.show_on_unauthenticated_pages ? "Yes" : "No" } %>
               <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %>
             <% end %>
+            <% summary_list.with_row do |row| %>
+              <% row.with_key { "Link to additional information?" } %>
+              <% row.with_value { @notification.show_additional_page ? "Yes" : "No" } %>
+              <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %>
+            <% end %>
+            <% if @notification.show_additional_page %>
+              <% summary_list.with_row do |row| %>
+                <% row.with_key { "Link text" } %>
+                <% row.with_value { @notification.link_text } %>
+                <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %>
+              <% end %>
+              <% summary_list.with_row do |row| %>
+                <% row.with_key { "Page content" } %>
+                <% row.with_value do %>
+                  <%= govuk_link_to "View page preview", notification_path(@notification), new_tab: true %>
+                <% end %>
+                <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %>
+              <% end %>
+            <% end %>
           <% end %>
         </div>
       </div>

app/views/notifications/show.html.erb

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

config/locales/en.yml

@@ -190,6 +190,10 @@ en:
           attributes:
             title:
               blank: "Enter a title"
+            link_text:
+              blank_when_additional_page_set: "Enter the link text"
+            page_content:
+              blank_when_additional_page_set: "Enter the page content"
 
   notification:
     logs_deleted:

db/migrate/20240904135306_add_show_additional_page_column_to_notifications.rb

@@ -0,0 +1,5 @@
+class AddShowAdditionalPageColumnToNotifications < ActiveRecord::Migration[7.0]
+  def change
+    add_column :notifications, :show_additional_page, :boolean
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
+ActiveRecord::Schema[7.0].define(version: 2024_09_04_135306) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -54,7 +54,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
     t.datetime "last_accessed"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying::text, 'sales'::character varying::text])", name: "log_type_check"
+    t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying, 'sales'::character varying]::text[])", name: "log_type_check"
     t.check_constraint "year >= 2000 AND year <= 2099", name: "year_check"
   end
 
@@ -205,14 +205,14 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
     t.integer "hb"
     t.integer "hbrentshortfall"
     t.integer "property_relet"
-    t.datetime "mrcdate", precision: nil
+    t.datetime "mrcdate"
     t.integer "incref"
-    t.datetime "startdate", precision: nil
+    t.datetime "startdate"
     t.integer "armedforces"
     t.integer "first_time_property_let_as_social_housing"
     t.integer "unitletas"
     t.integer "builtype"
-    t.datetime "voiddate", precision: nil
+    t.datetime "voiddate"
     t.bigint "owning_organisation_id"
     t.bigint "managing_organisation_id"
     t.integer "renttype"
@@ -453,6 +453,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
     t.boolean "show_on_unauthenticated_pages"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.boolean "show_additional_page"
   end
 
   create_table "organisation_relationships", force: :cascade do |t|
@@ -773,8 +774,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_08_19_143150) do
     t.string "name"
     t.bigint "organisation_id"
     t.integer "sign_in_count", default: 0, null: false
-    t.datetime "current_sign_in_at", precision: nil
+    t.datetime "current_sign_in_at"
-    t.datetime "last_sign_in_at", precision: nil
+    t.datetime "last_sign_in_at"
     t.string "current_sign_in_ip"
     t.string "last_sign_in_ip"
     t.integer "role"

spec/factories/notification.rb

@@ -6,5 +6,6 @@ FactoryBot.define do
     start_date { Time.zone.yesterday }
     end_date { Time.zone.tomorrow }
     show_on_unauthenticated_pages { false }
+    show_additional_page { true }
   end
 end

spec/models/notification_spec.rb

@@ -0,0 +1,37 @@
+require "rails_helper"
+
+RSpec.describe Notification, type: :model do
+  describe "#valid?" do
+    context "when show additional page is true" do
+      context "and page_content is blank" do
+        let(:notification) { build(:notification, show_additional_page: true, page_content: "") }
+
+        it "adds an error to page_content" do
+          notification.valid?
+
+          expect(notification.errors[:page_content]).to include("Enter the page content")
+        end
+      end
+
+      context "and link_text is blank" do
+        let(:notification) { build(:notification, show_additional_page: true, link_text: nil) }
+
+        it "adds an error to link_text" do
+          notification.valid?
+
+          expect(notification.errors[:link_text]).to include("Enter the link text")
+        end
+      end
+    end
+
+    context "when show additional page is false" do
+      context "and page_content and link_text are blank" do
+        let(:notification) { build(:notification, show_additional_page: false, link_text: nil, page_content: nil) }
+
+        it "is valid" do
+          expect(notification).to be_valid
+        end
+      end
+    end
+  end
+end

spec/requests/notifications_controller_spec.rb

@@ -13,12 +13,15 @@ RSpec.describe NotificationsController, type: :request do
       let(:request) { post notifications_path, params: params }
 
       context "with valid parameters" do
-        let(:params) { { "notification": { title: "Test Create", show_on_unauthenticated_pages: true } } }
+        let(:params) { { "notification": { title: "Test Create", show_on_unauthenticated_pages: "1", show_additional_page: "1", link_text: "link", page_content: "page" } } }
 
         it "creates a new notification with no start date set" do
           request
           notification = Notification.find_by(title: "Test Create")
-          expect(notification.show_on_unauthenticated_pages).to be_truthy
+          expect(notification.show_on_unauthenticated_pages).to be(true)
+          expect(notification.show_additional_page).to be(true)
+          expect(notification.link_text).to eq("link")
+          expect(notification.page_content).to eq("page")
           expect(notification.start_date).to be_nil
         end
 
@@ -30,13 +33,24 @@ RSpec.describe NotificationsController, type: :request do
       end
 
       context "with invalid parameters" do
-        let(:params) { { "notification": { title: "", show_on_unauthenticated_pages: true } } }
+        let(:params) { { "notification": { title: "", show_on_unauthenticated_pages: "1" } } }
 
         it "gives an error response" do
           request
           expect(response).to have_http_status(:unprocessable_entity)
         end
       end
+
+      context "when show additional page is false" do
+        let(:params) { { "notification": { title: "No Additional Page", show_on_unauthenticated_pages: "1", show_additional_page: "0", link_text: "text", page_content: "content" } } }
+
+        it "ignores values for link_text and page_content" do
+          request
+          notification = Notification.find_by(title: "No Additional Page")
+          expect(notification.link_text).to be_nil
+          expect(notification.page_content).to be_nil
+        end
+      end
     end
 
     describe "#update" do
@@ -60,7 +74,7 @@ RSpec.describe NotificationsController, type: :request do
       end
 
       context "when start_now is not set" do
-        let(:params) { { "notification": { title: "Updated Title", show_on_unauthenticated_pages: true } } }
+        let(:params) { { "notification": { title: "Updated Title", show_on_unauthenticated_pages: "1" } } }
 
         it "sets the relevant values on the notification" do
           request
@@ -74,6 +88,18 @@ RSpec.describe NotificationsController, type: :request do
           expect(response).to redirect_to(notification_check_answers_path(notification))
         end
       end
+
+      context "when show additional page is false" do
+        let(:notification) { create(:notification, show_additional_page: "0", link_text: "link", page_content: "page") }
+        let(:params) { { "notification": { show_additional_page: "0", link_text: "text", page_content: "content" } } }
+
+        it "removes values for link_text and page_content" do
+          request
+          notification.reload
+          expect(notification.link_text).to be_nil
+          expect(notification.page_content).to be_nil
+        end
+      end
     end
   end
 
@@ -85,7 +111,7 @@ RSpec.describe NotificationsController, type: :request do
     end
 
     describe "#create" do
-      let(:request) { post notifications_path, params: { "notification": { title: "Test Create", show_on_unauthenticated_pages: true } } }
+      let(:request) { post notifications_path, params: { "notification": { title: "Test Create" } } }
 
       it "returns not found" do
         request
@@ -99,7 +125,7 @@ RSpec.describe NotificationsController, type: :request do
 
     describe "#update" do
       let(:notification) { create(:notification) }
-      let(:request) { patch notification_path(notification), params: { "notification": { title: "Test Create", show_on_unauthenticated_pages: true } } }
+      let(:request) { patch notification_path(notification), params: { "notification": { title: "Test Create" } } }
 
       it "returns not found" do
         request