CLDC-2255 Add homepage notifications (#2131)

* feat: add notification table

* feat: add notification banner, use unread gem for notification management

* feat: add notifications page and remove unread_notification.rb

* feat: add blank homepage, update routing and tests

* feat: add welcome message and thoroughly test routing

* refactor: lint

* feat: update tests

* CLDC-3061 Add guidance page (#2121)

* Add guidance page

* Link to guidance from start page

* feat: test home/start paths explicitly

* feat: add notification table

* feat: add notification banner, use unread gem for notification management

* feat: add notifications page and remove unread_notification.rb

* feat: default p tag around sanitized page content

* feat: add active scope

* feat: use newest active unread/unauthenticated notification and update start page

* feat: add tests of notification behaviour and routing and refactor

* refactor: lint

* feat: update Gemfile.lock

* feat: add timestamps to readmark table

* feat: update gemfile.lock

* refactor: lint

* feat: test notifications page doesn't show notifications and code simplification

* feat: move notification helper methods to notifications_helper.rb

---------

Co-authored-by: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>

Gemfile

@@ -64,6 +64,7 @@ gem "auto_strip_attributes"
 # Use sidekiq for background processing
 gem "sidekiq"
 gem "sidekiq-cron"
+gem "unread"
 
 group :development, :test do
   # Check gems for known vulnerabilities

Gemfile.lock

@@ -408,6 +408,8 @@ GEM
       concurrent-ruby (~> 1.0)
     uk_postcode (2.1.8)
     unicode-display_width (2.4.2)
+    unread (0.13.0)
+      activerecord (>= 6.1)
     view_component (3.9.0)
       activesupport (>= 5.2.0, < 8.0)
       concurrent-ruby (~> 1.0)
@@ -493,6 +495,7 @@ DEPENDENCIES
   timecop (~> 0.9.4)
   tzinfo-data
   uk_postcode
+  unread
   view_component (~> 3.9)
   web-console (>= 4.1.0)
   webmock

app/controllers/notifications_controller.rb

@@ -0,0 +1,19 @@
+class NotificationsController < ApplicationController
+  def dismiss
+    if current_user.blank?
+      redirect_to root_path
+    else
+      current_user.newest_active_unread_notification.mark_as_read! for: current_user
+      redirect_back(fallback_location: root_path)
+    end
+  end
+
+  def show
+    @notification = current_user&.newest_active_unread_notification || Notification.newest_active_unauthenticated_notification
+    if @notification&.page_content
+      render "show"
+    else
+      redirect_back(fallback_location: root_path)
+    end
+  end
+end

app/frontend/styles/_header.scss

@@ -26,3 +26,7 @@
 .app-header--orange .govuk-header__container {
   border-bottom-color: govuk-colour("orange");
 }
+
+.app-header__no-border-bottom {
+  border-bottom: 0;
+}

app/frontend/styles/_unread-notification.scss

@@ -0,0 +1,7 @@
+.app-unread-notification {
+  background-color: govuk-colour("blue");
+}
+
+.app-unread-notification p {
+  color: govuk-colour("white");
+}

app/frontend/styles/application.scss

@@ -26,7 +26,9 @@ $govuk-breakpoints: (
 @import "button";
 @import "card";
 @import "data_box";
+@import "delete-logs-table";
 @import "document-list";
+@import "errors";
 @import "feedback";
 @import "filter";
 @import "filter-layout";
@@ -44,8 +46,7 @@ $govuk-breakpoints: (
 @import "primary-navigation";
 @import "search";
 @import "sub-navigation";
-@import "errors";
-@import "delete-logs-table";
+@import "unread-notification";
 
 // App utilities
 .app-\!-colour-muted {

app/helpers/application_helper.rb

@@ -10,21 +10,27 @@ module ApplicationHelper
   end
 
   def govuk_header_classes(current_user)
-    if current_user && current_user.support?
+    if current_user&.support?
       "app-header app-header--orange"
+    elsif ((current_user.blank? && Notification.active_unauthenticated_notifications.present?) || current_user&.active_unread_notifications.present?) && !current_page?(notifications_path)
+      "app-header app-header__no-border-bottom"
     else
       "app-header"
     end
   end
 
   def govuk_phase_banner_tag(current_user)
-    if current_user && current_user.support?
+    if current_user&.support?
       { colour: "orange", text: "Support beta" }
     else
       { text: "Beta" }
     end
   end
 
+  def notifications_to_display?
+    !current_page?(notifications_path) && (authenticated_user_has_notifications? || unauthenticated_user_has_notifications?)
+  end
+
 private
 
   def paginated_title(title, pagy)
@@ -33,4 +39,12 @@ private
 
     title + " (page #{pagy.page} of #{pagy.pages})"
   end
+
+  def authenticated_user_has_notifications?
+    current_user&.active_unread_notifications.present?
+  end
+
+  def unauthenticated_user_has_notifications?
+    current_user.blank? && Notification.active_unauthenticated_notifications.present?
+  end
 end

app/helpers/navigation_items_helper.rb

@@ -47,7 +47,7 @@ module NavigationItemsHelper
 private
 
   def home_current?(path)
-    path == root_path
+    path == root_path || path == notifications_path
   end
 
   def lettings_logs_current?(path)

app/helpers/notifications_helper.rb

@@ -0,0 +1,17 @@
+module NotificationsHelper
+  def notification_count
+    if current_user.present?
+      current_user.active_unread_notifications.count
+    else
+      Notification.active_unauthenticated_notifications.count
+    end
+  end
+
+  def notification
+    if current_user.present?
+      current_user.newest_active_unread_notification
+    else
+      Notification.newest_active_unauthenticated_notification
+    end
+  end
+end

app/models/notification.rb

@@ -0,0 +1,14 @@
+class Notification < ApplicationRecord
+  acts_as_readable
+
+  scope :active, -> { where("start_date <= ? AND end_date >= ?", Time.zone.now, Time.zone.now) }
+  scope :unauthenticated, -> { where(show_on_unauthenticated_pages: true) }
+
+  def self.active_unauthenticated_notifications
+    active.unauthenticated
+  end
+
+  def self.newest_active_unauthenticated_notification
+    active_unauthenticated_notifications.last
+  end
+end

app/models/user.rb

@@ -1,4 +1,6 @@
 class User < ApplicationRecord
+  acts_as_reader
+
   # Include default devise modules. Others available are:
   # :omniauthable
   devise :database_authenticatable, :recoverable, :rememberable,
@@ -227,6 +229,14 @@ class User < ApplicationRecord
     sales_logs.after_date(FormHandler.instance.sales_earliest_open_for_editing_collection_start_date).duplicate_sets(id).map { |array_str| array_str ? array_str.map(&:to_i) : [] }
   end
 
+  def active_unread_notifications
+    Notification.active.unread_by(self)
+  end
+
+  def newest_active_unread_notification
+    active_unread_notifications.last
+  end
+
 protected
 
   # Checks whether a password is needed or not. For validations only.

app/views/layouts/application.html.erb

@@ -99,6 +99,10 @@
       end
     end %>
 
+    <% if notifications_to_display? %>
+      <%= render "notifications/notification_banner" %>
+    <% end %>
+
     <% feedback_link = govuk_link_to "giving us your feedback (opens in a new tab)", t("feedback_form"), rel: "noreferrer noopener", target: "_blank" %>
 
     <%= govuk_phase_banner(
@@ -107,7 +111,7 @@
       text: "This is a new service – help us improve it by #{feedback_link}".html_safe,
     ) %>
 
-    <% if !current_user.nil? %>
+    <% if current_user.present? %>
       <%= render PrimaryNavigationComponent.new(
         items: primary_items(request.path, current_user),
       ) %>
@@ -122,7 +126,7 @@
           <%= govuk_notification_banner(
             title_text: "Success",
             success: true, title_heading_level: 3,
-            title_id: "swanky-notifications"
+            title_id: "flash-notice"
           ) do |notification_banner|
             notification_banner.with_heading(text: flash.notice.html_safe)
             if flash[:notification_banner_body]

app/views/notifications/_notification_banner.html.erb

@@ -0,0 +1,23 @@
+<div class="app-unread-notification">
+  <div class="govuk-width-container">
+    <br>
+    <div class="govuk-grid-row">
+      <div class="govuk-grid-column-three-quarters">
+        <% if notification_count > 1  && current_user.present? %>
+          <p>Notification 1 of <%= notification_count %></p>
+        <% end %>
+      <p class="govuk-!-font-weight-bold"><%= notification.title %></p>
+        <% if notification.page_content.present? %>
+          <div class="govuk-body">
+            <%= govuk_link_to notification.link_text, notifications_path, class: "govuk-link--inverse govuk-!-font-weight-bold" %>
+          </div>
+        <% end %>
+      </div>
+      <% if current_user.present? %>
+        <p class="govuk-grid-column-one-quarter govuk-!-text-align-right ">
+          <%= govuk_link_to "Dismiss", dismiss_notifications_path, class: "govuk-link--inverse" %>
+        </p>
+      <% end %>
+    </div>
+  </div>
+</div>

app/views/notifications/show.html.erb

@@ -0,0 +1,17 @@
+<% content_for :title, "Notification" %>
+<% content_for :before_content do %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <h1 class="govuk-heading-xl"><%= @notification.title %></h1>
+    <p>
+    <%= sanitize @notification.page_content %>
+    </p>
+  </div>
+</div>
+<br>
+<div>
+  <%= govuk_button_link_to "Back to #{current_user.present? ? 'Home' : 'Start'}", root_path %>
+</div>

app/views/start/index.html.erb

@@ -23,10 +23,10 @@
     <p class="govuk-body">If you need to set up a new account, speak to your organisation’s CORE data coordinator. If you don’t know who that is, <%= govuk_link_to("contact the helpdesk", GlobalConstants::HELPDESK_URL) %>.</p>
     <p class="govuk-body">You can <%= govuk_mail_to("dluhc.digital-services@levellingup.gov.uk", "request an account", subject: "CORE: Request a new account") %> if your organisation doesn’t have one.</p>
     <p class="govuk-body"><strong><%= govuk_link_to guidance_path do %>Guidance for submitting social housing lettings and sales data (CORE)<% end %></strong><p>
+    <hr class="govuk-section-break govuk-section-break--visible govuk-section-break--m">
   </div>
 </div>
 
-<hr class="govuk-section-break govuk-section-break--visible govuk-section-break--m">
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <%= render partial: "layouts/collection_resources" %>

config/routes.rb

@@ -128,6 +128,10 @@ Rails.application.routes.draw do
     end
   end
 
+  resource :notifications do
+    get "dismiss", to: "notifications#dismiss"
+  end
+
   resources :organisations do
     get "duplicates", to: "duplicate_logs#index"
 

db/migrate/20240108145545_create_notification.rb

@@ -0,0 +1,14 @@
+class CreateNotification < ActiveRecord::Migration[7.0]
+  def change
+    create_table :notifications do |t|
+      t.string :title
+      t.string :link_text
+      t.string :page_content
+      t.datetime :start_date
+      t.datetime :end_date
+      t.boolean :show_on_unauthenticated_pages
+
+      t.timestamps
+    end
+  end
+end

db/migrate/20240108152935_unread_migration.rb

@@ -0,0 +1,25 @@
+class UnreadMigration < ActiveRecord::Migration[6.0]
+  def self.up
+    create_table ReadMark, force: true, options: create_options do |t|
+      t.references :readable, polymorphic: { null: false }
+      t.references :reader,   polymorphic: { null: false }
+      t.datetime :timestamp, null: false
+      t.timestamps
+    end
+
+    add_index ReadMark, %i[reader_id reader_type readable_type readable_id], name: "read_marks_reader_readable_index", unique: true
+  end
+
+  def self.down
+    drop_table ReadMark
+  end
+
+  def self.create_options
+    options = ""
+    if defined?(ActiveRecord::ConnectionAdapters::Mysql2Adapter) \
+      && ActiveRecord::Base.connection.instance_of?(ActiveRecord::ConnectionAdapters::Mysql2Adapter)
+      options = "DEFAULT CHARSET=latin1"
+    end
+    options
+  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: 2023_12_18_105226) do
+ActiveRecord::Schema[7.0].define(version: 2024_01_08_152935) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -393,6 +393,17 @@ ActiveRecord::Schema[7.0].define(version: 2023_12_18_105226) do
     t.string "new_organisation_telephone_number"
   end
 
+  create_table "notifications", force: :cascade do |t|
+    t.string "title"
+    t.string "link_text"
+    t.string "page_content"
+    t.datetime "start_date"
+    t.datetime "end_date"
+    t.boolean "show_on_unauthenticated_pages"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
   create_table "organisation_relationships", force: :cascade do |t|
     t.integer "child_organisation_id"
     t.integer "parent_organisation_id"
@@ -446,6 +457,17 @@ ActiveRecord::Schema[7.0].define(version: 2023_12_18_105226) do
     t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true
   end
 
+  create_table "read_marks", force: :cascade do |t|
+    t.string "readable_type", null: false
+    t.bigint "readable_id"
+    t.string "reader_type", null: false
+    t.bigint "reader_id"
+    t.datetime "timestamp", precision: nil, null: false
+    t.index ["readable_type", "readable_id"], name: "index_read_marks_on_readable_type_and_readable_id"
+    t.index ["reader_id", "reader_type", "readable_type", "readable_id"], name: "read_marks_reader_readable_index", unique: true
+    t.index ["reader_type", "reader_id"], name: "index_read_marks_on_reader_type_and_reader_id"
+  end
+
   create_table "sales_logs", force: :cascade do |t|
     t.integer "status", default: 0
     t.datetime "saledate"

spec/factories/notification.rb

@@ -0,0 +1,10 @@
+FactoryBot.define do
+  factory :notification do
+    title { "Notification title" }
+    link_text { "Link text" }
+    page_content { "Some html content" }
+    start_date { Time.zone.yesterday }
+    end_date { Time.zone.tomorrow }
+    show_on_unauthenticated_pages { false }
+  end
+end

spec/features/home_page_spec.rb

@@ -4,6 +4,124 @@ require_relative "form/helpers"
 RSpec.describe "Home Page Features" do
   include Helpers
 
+  context "when there are notifications" do
+    let!(:user) { FactoryBot.create(:user) }
+
+    context "when the notifications are currently active" do
+      before do
+        create(:notification, title: "Notification title 1")
+        create(:notification, title: "Notification title 2")
+        create(:notification, title: "Notification title 3")
+        sign_in user
+        visit(root_path)
+      end
+
+      it "shows the latest notification with count and dismiss link" do
+        expect(page).to have_content("Notification 1 of 3")
+        expect(page).to have_content("Notification title 3")
+        expect(page).to have_link("Dismiss")
+        expect(page).to have_link("Link text")
+      end
+
+      context "when the user clicks a notification link" do
+        before do
+          click_link("Link text")
+        end
+
+        it "takes them to the notification details page" do
+          expect(page).to have_current_path(notifications_path)
+          expect(page).to have_content("Notification title 3")
+          expect(page).to have_content("Some html content")
+          expect(page).to have_link("Back to Home")
+        end
+
+        context "when they return" do
+          before do
+            click_link("Back to Home")
+          end
+
+          it "the notification has not been dismissed" do
+            expect(page).to have_current_path(root_path)
+            expect(page).to have_content("Notification 1 of 3")
+            expect(page).to have_content("Notification title 3")
+            expect(page).to have_link("Dismiss")
+            expect(page).to have_link("Link text")
+          end
+        end
+      end
+
+      context "when the user clicks a dismiss link" do
+        before do
+          click_link("Dismiss")
+        end
+
+        it "dismisses the notification and takes them back" do
+          expect(page).to have_current_path(root_path)
+          expect(page).to have_content("Notification 1 of 2")
+          expect(page).to have_content("Notification title 2")
+          expect(page).to have_link("Dismiss")
+          expect(page).to have_link("Link text")
+        end
+
+        context "when the user dismisses the penultimate notification" do
+          before do
+            click_link("Dismiss")
+          end
+
+          it "no longer displays the count" do
+            expect(page).to have_current_path(root_path)
+            expect(page).not_to have_content("Notification 1 of")
+            expect(page).to have_content("Notification title 1")
+          end
+
+          context "when the user dismisses the final notification" do
+            before do
+              click_link("Dismiss")
+            end
+
+            it "no longer displays any notification" do
+              expect(page).to have_current_path(root_path)
+              expect(page).not_to have_content("Notification")
+              expect(page).not_to have_link("Dismiss")
+              expect(page).not_to have_link("Link_text")
+            end
+          end
+        end
+      end
+
+      context "when another user has dismissed all their notifications" do
+        before do
+          other_user = create(:user)
+          Notification.mark_as_read! :all, for: other_user
+          visit(root_path)
+        end
+
+        it "the first user can still see the notifications" do
+          expect(page).to have_content("Notification 1 of 3")
+          expect(page).to have_content("Notification title 3")
+          expect(page).to have_link("Dismiss")
+          expect(page).to have_link("Link text")
+        end
+      end
+    end
+
+    context "when the notifications are not currently active" do
+      before do
+        create(:notification, end_date: Time.zone.yesterday, title: "Notification title 1")
+        create(:notification, start_date: Time.zone.tomorrow, title: "Notification title 2")
+        sign_in user
+        visit(root_path)
+      end
+
+      it "does not show any notifications" do
+        expect(page).not_to have_content("Notification title")
+        expect(page).not_to have_content("Notification 1 of")
+        expect(page).not_to have_link("Dismiss")
+        expect(page).not_to have_link("Link text")
+      end
+    end
+  end
+
   context "when the user is a data provider" do
     let(:user) { FactoryBot.create(:user, name: "Provider") }
 
@@ -13,7 +131,7 @@ RSpec.describe "Home Page Features" do
       create_list(:lettings_log, 4, :completed, owning_organisation: user.organisation, created_by: user)
       create_list(:lettings_log, 2, :completed)
       sign_in user
-      visit("/")
+      visit(root_path)
     end
 
     it "displays the correct welcome text" do
@@ -26,7 +144,7 @@ RSpec.describe "Home Page Features" do
       before do
         create_list(:sales_log, 5, :in_progress, owning_organisation: user.organisation, created_by: user)
         create_list(:sales_log, 3, :completed, owning_organisation: user.organisation, created_by: user)
-        visit("/")
+        visit(root_path)
       end
 
       it "displays correct data boxes, counts and links" do
@@ -41,7 +159,7 @@ RSpec.describe "Home Page Features" do
 
     context "when their organisation has never submitted sales logs" do
       before do
-        visit("/")
+        visit(root_path)
       end
 
       it "displays correct data boxes, counts and links" do
@@ -63,7 +181,7 @@ RSpec.describe "Home Page Features" do
       create_list(:lettings_log, 2, :completed)
       create_list(:scheme, 1, :incomplete, owning_organisation: user.organisation)
       sign_in user
-      visit("/")
+      visit(root_path)
     end
 
     let(:user) { FactoryBot.create(:user, :data_coordinator, name: "Coordinator") }
@@ -78,7 +196,7 @@ RSpec.describe "Home Page Features" do
       before do
         create_list(:sales_log, 5, :in_progress, owning_organisation: user.organisation)
         create_list(:sales_log, 3, :completed, owning_organisation: user.organisation)
-        visit("/")
+        visit(root_path)
       end
 
       it "displays correct data boxes, counts and links" do
@@ -95,7 +213,7 @@ RSpec.describe "Home Page Features" do
 
     context "when their organisation has never submitted sales logs" do
       before do
-        visit("/")
+        visit(root_path)
       end
 
       it "displays correct data boxes, counts and links" do
@@ -135,7 +253,7 @@ RSpec.describe "Home Page Features" do
       click_button("Sign in")
       fill_in("code", with: otp)
       click_button("Submit")
-      visit("/")
+      visit(root_path)
     end
 
     it "displays the correct welcome text" do

spec/features/notifications_page_spec.rb

@@ -0,0 +1,41 @@
+require "rails_helper"
+require_relative "form/helpers"
+
+RSpec.describe "Notifications Page Features" do
+  include Helpers
+
+  context "when there are notifications" do
+    let!(:user) { FactoryBot.create(:user) }
+
+    context "when the notifications are currently active" do
+      before do
+        create(:notification, title: "Notification title 1")
+        create(:notification, title: "Notification title 2")
+        create(:notification, title: "Notification title 3")
+        sign_in user
+        visit(notifications_path)
+      end
+
+      it "does not show the notification banner" do
+        expect(page).not_to have_content("Notification 1 of")
+        expect(page).not_to have_link("Dismiss")
+        expect(page).not_to have_link("Link text")
+      end
+    end
+
+    context "when the notifications are not currently active" do
+      before do
+        create(:notification, end_date: Time.zone.yesterday, title: "Notification title 1")
+        create(:notification, start_date: Time.zone.tomorrow, title: "Notification title 2")
+        sign_in user
+        visit(notifications_path)
+      end
+
+      it "does not show the notifications banner" do
+        expect(page).not_to have_content("Notification 1 of")
+        expect(page).not_to have_link("Dismiss")
+        expect(page).not_to have_link("Link text")
+      end
+    end
+  end
+end

spec/features/start_page_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe "Start Page Features" do
     end
 
     it "takes you to the home page" do
-      visit("/")
+      visit(root_path)
       expect(page).to have_current_path("/")
       expect(page).to have_content("Welcome back")
     end
@@ -19,7 +19,7 @@ RSpec.describe "Start Page Features" do
 
   context "when the user is not signed in" do
     it "takes you to sign in and then to the home page" do
-      visit("/")
+      visit(root_path)
       click_link("Start now")
       expect(page).to have_current_path("/account/sign-in?start=true")
       fill_in("user[email]", with: user.email)
@@ -28,5 +28,20 @@ RSpec.describe "Start Page Features" do
       expect(page).to have_current_path("/")
       expect(page).to have_content("Welcome back")
     end
+
+    context "when the unauthenticated user clicks a notification link" do
+      before do
+        create(:notification, show_on_unauthenticated_pages: true)
+        visit(root_path)
+        click_link("Link text")
+      end
+
+      it "takes them to the notification details page" do
+        expect(page).to have_current_path(notifications_path)
+        expect(page).to have_content("Notification title")
+        expect(page).to have_content("Some html content")
+        expect(page).to have_link("Back to Start")
+      end
+    end
   end
 end

spec/features/test_spec.rb

@@ -1,7 +1,7 @@
 require "rails_helper"
 RSpec.describe "Test Features" do
   it "Displays the name of the app" do
-    visit("/")
+    visit(root_path)
     expect(page).to have_content("Submit social housing lettings and sales data (CORE)")
   end
 

spec/features/user_spec.rb

@@ -126,7 +126,7 @@ RSpec.describe "User Features" do
     end
 
     it "Can navigate and sign in page with sign in button" do
-      visit("/")
+      visit(root_path)
       expect(page).to have_link("Sign in")
       click_link("Sign in")
       fill_in("user[email]", with: user.email)

spec/views/layouts/application_layout_spec.rb

@@ -54,4 +54,35 @@ RSpec.describe "layouts/application" do
 
     include_examples "analytics cookie elements", banner: false, scripts: false
   end
+
+  context "with a notification present" do
+    context "when notification is shown on unauthenticated pages" do
+      before do
+        create(:notification, title: "Old notification title", show_on_unauthenticated_pages: true)
+        create(:notification, title: "New notification title", show_on_unauthenticated_pages: true)
+        render
+      end
+
+      it "shows the most recent notification without dismiss link or count" do
+        expect(rendered).to have_content("New notification title")
+        expect(rendered).to have_link("Link text")
+        expect(rendered).not_to have_link("Dismiss")
+        expect(rendered).not_to have_content("Notification 1 of")
+      end
+    end
+
+    context "when notification is not shown on unauthenticated pages" do
+      before do
+        create(:notification)
+        render
+      end
+
+      it "does not show the notification banner" do
+        expect(rendered).not_to have_content("Notification title")
+        expect(rendered).not_to have_link("Link text")
+        expect(rendered).not_to have_link("Dismiss")
+        expect(rendered).not_to have_content("Notification 1 of")
+      end
+    end
+  end
 end