From efa436ed15c0bcb35e10affecc3f7f269e938179 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 9 Jan 2024 16:46:51 +0000 Subject: [PATCH] feat: add tests of notification behaviour and routing and refactor --- app/helpers/application_helper.rb | 2 +- app/models/notification.rb | 6 +- app/views/layouts/application.html.erb | 4 +- spec/factories/notification.rb | 10 ++ spec/features/home_page_spec.rb | 132 +++++++++++++++++- spec/features/start_page_spec.rb | 19 ++- spec/features/test_spec.rb | 2 +- spec/features/user_spec.rb | 2 +- spec/views/layouts/application_layout_spec.rb | 32 +++++ 9 files changed, 194 insertions(+), 15 deletions(-) create mode 100644 spec/factories/notification.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 0ec004c5c..4a2b0dfbf 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -12,7 +12,7 @@ module ApplicationHelper def govuk_header_classes(current_user) if current_user&.support? "app-header app-header--orange" - elsif (current_user.blank? || current_user.active_unread_notifications.present?) && !current_page?(notifications_path) + 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" diff --git a/app/models/notification.rb b/app/models/notification.rb index 0b8bb77d1..430b4d704 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -4,7 +4,11 @@ class Notification < ApplicationRecord scope :active, -> { where("start_date <= ?", Time.zone.now).where("end_date >= ?", 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.last + active_unauthenticated_notifications.last end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 342e47bba..77b72f5dc 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -104,8 +104,8 @@ <% 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.present? && Notification.unauthenticated.present? %> - <%= render "notifications/notification_banner", notification_count: Notification.unauthenticated.count, notification: Notification.newest_active_unauthenticated_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 %> diff --git a/spec/factories/notification.rb b/spec/factories/notification.rb new file mode 100644 index 000000000..05d4faa3f --- /dev/null +++ b/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 diff --git a/spec/features/home_page_spec.rb b/spec/features/home_page_spec.rb index 908961e33..0e8e99ff7 100644 --- a/spec/features/home_page_spec.rb +++ b/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 diff --git a/spec/features/start_page_spec.rb b/spec/features/start_page_spec.rb index f7c008553..d90a0c1f0 100644 --- a/spec/features/start_page_spec.rb +++ b/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 diff --git a/spec/features/test_spec.rb b/spec/features/test_spec.rb index ef54fa631..6dc977a9b 100644 --- a/spec/features/test_spec.rb +++ b/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 diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 9e1aae35d..f31b05296 100644 --- a/spec/features/user_spec.rb +++ b/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) diff --git a/spec/views/layouts/application_layout_spec.rb b/spec/views/layouts/application_layout_spec.rb index ac4a10a98..eeb989cca 100644 --- a/spec/views/layouts/application_layout_spec.rb +++ b/spec/views/layouts/application_layout_spec.rb @@ -54,4 +54,36 @@ 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