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

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

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

app/views/layouts/application.html.erb

@@ -99,12 +99,8 @@
       end
     end %>
 
-    <% unless current_page?(notifications_path) %>
+    <% if notifications_to_display? %>
-      <% if current_user&.active_unread_notifications.present? %>
+      <%= render "notifications/notification_banner", notification_count:, notification: %>
-        <%= 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 %>
     <% end %>
 
     <% feedback_link = govuk_link_to "giving us your feedback (opens in a new tab)", t("feedback_form"), rel: "noreferrer noopener", target: "_blank" %>

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