From dd106812c01fca1ff2f0c40b39b35e079545d71c Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire <nat.dean-lewis@softwire.com>
Date: Thu, 11 Jan 2024 10:02:33 +0000
Subject: [PATCH] feat: test notifications page doesn't show notifications and
 code simplification

---
 app/helpers/application_helper.rb        | 28 ++++++++++++++++
 app/models/notification.rb               |  2 +-
 app/views/layouts/application.html.erb   |  8 ++---
 spec/features/notifications_page_spec.rb | 41 ++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 spec/features/notifications_page_spec.rb

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 4a2b0dfbf..6fff94f5e 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -27,6 +27,26 @@ module ApplicationHelper
     end
   end
 
+  def notifications_to_display?
+    !current_page?(notifications_path) && (authenticated_user_has_notifications? || unauthenticated_user_has_notifications?)
+  end
+
+  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
+
 private
 
   def paginated_title(title, pagy)
@@ -35,4 +55,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
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 430b4d704..86978ebea 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -1,7 +1,7 @@
 class Notification < ApplicationRecord
   acts_as_readable
 
-  scope :active, -> { where("start_date <= ?", Time.zone.now).where("end_date >= ?", Time.zone.now) }
+  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
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index f1809bebf..3a6924ae2 100644
--- a/app/views/layouts/application.html.erb
+++ b/app/views/layouts/application.html.erb
@@ -99,12 +99,8 @@
       end
     end %>
 
-    <% unless current_page?(notifications_path) %>
-      <% if current_user&.active_unread_notifications.present? %>
-        <%= render "notifications/notification_banner", notification_count: current_user.active_unread_notifications.count, notification: current_user.newest_active_unread_notification %>
-      <% elsif current_user.blank? && Notification.active_unauthenticated_notifications.present? %>
-        <%= render "notifications/notification_banner", notification_count: Notification.active_unauthenticated_notifications.count, notification: Notification.newest_active_unauthenticated_notification %>
-      <% end %>
+    <% if notifications_to_display? %>
+      <%= render "notifications/notification_banner", notification_count:, notification: %>
     <% end %>
 
     <% feedback_link = govuk_link_to "giving us your feedback (opens in a new tab)", t("feedback_form"), rel: "noreferrer noopener", target: "_blank" %>
diff --git a/spec/features/notifications_page_spec.rb b/spec/features/notifications_page_spec.rb
new file mode 100644
index 000000000..97bbeb7eb
--- /dev/null
+++ b/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