From a411046650216799f201720e41df5ab512733af5 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 2 Jan 2024 14:02:10 +0000 Subject: [PATCH 01/11] feat: add blank homepage, update routing and tests --- app/controllers/start_controller.rb | 2 +- app/helpers/navigation_items_helper.rb | 6 +++++ app/views/home/index.html.erb | 0 spec/features/start_page_spec.rb | 2 +- spec/features/user_spec.rb | 2 +- spec/helpers/navigation_items_helper_spec.rb | 23 +++++++++++++++++++ .../auth/passwords_controller_spec.rb | 1 - spec/requests/maintenance_controller_spec.rb | 1 - spec/requests/users_controller_spec.rb | 10 ++++---- 9 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 app/views/home/index.html.erb diff --git a/app/controllers/start_controller.rb b/app/controllers/start_controller.rb index f2b96f557..318e7cace 100644 --- a/app/controllers/start_controller.rb +++ b/app/controllers/start_controller.rb @@ -1,7 +1,7 @@ class StartController < ApplicationController def index if current_user - redirect_to(lettings_logs_path) + render "home/index" end end diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index 56954361d..18132a8a1 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -4,6 +4,7 @@ module NavigationItemsHelper def primary_items(path, current_user) if current_user.support? [ + NavigationItem.new("Home", root_path, home_current?(path)), NavigationItem.new("Organisations", organisations_path, organisations_current?(path)), NavigationItem.new("Users", users_path, users_current?(path)), NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)), @@ -12,6 +13,7 @@ module NavigationItemsHelper ].compact else [ + NavigationItem.new("Home", root_path, home_current?(path)), NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)), NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)), (NavigationItem.new("Schemes", schemes_path, non_support_supported_housing_schemes_current?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), @@ -44,6 +46,10 @@ module NavigationItemsHelper private + def home_current?(path) + path == root_path + end + def lettings_logs_current?(path) path.starts_with?(lettings_logs_path) end diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb new file mode 100644 index 000000000..e69de29bb diff --git a/spec/features/start_page_spec.rb b/spec/features/start_page_spec.rb index 569ea4cfa..f5416a549 100644 --- a/spec/features/start_page_spec.rb +++ b/spec/features/start_page_spec.rb @@ -12,7 +12,7 @@ RSpec.describe "Start Page Features" do it "takes you to logs" do visit("/") - expect(page).to have_current_path("/lettings-logs") + expect(page).to have_current_path("/") end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 2da53970b..04d5bd323 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -132,7 +132,7 @@ RSpec.describe "User Features" do fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") - expect(page).to have_current_path("/lettings-logs") + expect(page).to have_current_path("/") end it "tries to access account page, redirects to log in page" do diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index fcfbefc5b..0da64c9ba 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -12,6 +12,7 @@ RSpec.describe NavigationItemsHelper do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), @@ -34,6 +35,7 @@ RSpec.describe NavigationItemsHelper do let(:stock_owner) { create(:organisation) } let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -53,6 +55,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the lettings logs page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -71,6 +74,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the sales logs page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", true), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -89,6 +93,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the users page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -107,6 +112,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on their organisation details page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -125,6 +131,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the account page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -143,6 +150,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the individual user's page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -161,6 +169,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the individual scheme's page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), @@ -191,6 +200,7 @@ RSpec.describe NavigationItemsHelper do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), @@ -213,6 +223,7 @@ RSpec.describe NavigationItemsHelper do let(:stock_owner) { create(:organisation) } let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), @@ -236,6 +247,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the lettings logs page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), @@ -252,6 +264,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the sales logs page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -268,6 +281,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the users page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", true), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -284,6 +298,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the account page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -300,6 +315,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the Schemes page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -316,6 +332,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the individual user's page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", true), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -332,6 +349,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the individual scheme's page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -356,6 +374,7 @@ RSpec.describe NavigationItemsHelper do context "when the user is on the scheme locations page" do let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -382,6 +401,7 @@ RSpec.describe NavigationItemsHelper do let(:required_sub_path) { "lettings-logs" } let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -412,6 +432,7 @@ RSpec.describe NavigationItemsHelper do let(:required_sub_path) { "users" } let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -442,6 +463,7 @@ RSpec.describe NavigationItemsHelper do let(:required_sub_path) { "schemes" } let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), @@ -472,6 +494,7 @@ RSpec.describe NavigationItemsHelper do let(:required_sub_path) { "details" } let(:expected_navigation_items) do [ + NavigationItemsHelper::NavigationItem.new("Home", "/", false), NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index 92c36608c..333985d9e 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -67,7 +67,6 @@ RSpec.describe Auth::PasswordsController, type: :request do put "/account/password", params: update_password_params # Devise redirects once after re-sign in with new password and then root redirects as well. follow_redirect! - follow_redirect! expect(page).to have_css("div", class: "govuk-notification-banner__heading", text: message) end end diff --git a/spec/requests/maintenance_controller_spec.rb b/spec/requests/maintenance_controller_spec.rb index 4d7f8ab8c..39e587302 100644 --- a/spec/requests/maintenance_controller_spec.rb +++ b/spec/requests/maintenance_controller_spec.rb @@ -153,7 +153,6 @@ RSpec.describe MaintenanceController, type: :request do end it "the cookie banner is visible" do - follow_redirect! follow_redirect! expect(page).to have_content("We’d like to use analytics cookies so we can understand how you use the service and make improvements.") end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 7f1b3ffef..f335b61e6 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -73,11 +73,10 @@ RSpec.describe UsersController, type: :request do end describe "title link" do - it "routes user to the /logs page" do + it "routes user to the home page" do sign_in user get "/", headers:, params: {} - follow_redirect! - expect(path).to include("/lettings-logs") + expect(path).to include("/") expected_link = "" expect(CGI.unescape_html(response.body)).to include(expected_link) end @@ -2025,10 +2024,9 @@ RSpec.describe UsersController, type: :request do sign_in user end - it "routes user to the /logs page" do + it "routes user to the home page" do get "/", headers:, params: {} - follow_redirect! - expect(path).to include("/lettings-logs") + expect(path).to include("/") expected_link = "" expect(CGI.unescape_html(response.body)).to include(expected_link) end From f2fa9de46331fa91bbf630d9749dedf8c44750d1 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 2 Jan 2024 14:34:47 +0000 Subject: [PATCH 02/11] feat: add welcome message and thoroughly test routing --- app/controllers/auth/sessions_controller.rb | 2 +- app/views/home/index.html.erb | 1 + spec/features/start_page_spec.rb | 8 ++- spec/features/user_spec.rb | 1 + spec/helpers/navigation_items_helper_spec.rb | 73 +++++++++++++++----- spec/requests/users_controller_spec.rb | 2 + 6 files changed, 65 insertions(+), 22 deletions(-) diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 307e79f15..d2d6a6dd1 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -24,7 +24,7 @@ private if resource.need_two_factor_authentication?(request) user_two_factor_authentication_path else - params.dig("user", "start").present? ? lettings_logs_path : super + params.dig("user", "start").present? ? root_path : super end end end diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb index e69de29bb..e691b3e5b 100644 --- a/app/views/home/index.html.erb +++ b/app/views/home/index.html.erb @@ -0,0 +1 @@ +

<%= "Welcome back, #{@current_user.name}" %>

diff --git a/spec/features/start_page_spec.rb b/spec/features/start_page_spec.rb index f5416a549..f7c008553 100644 --- a/spec/features/start_page_spec.rb +++ b/spec/features/start_page_spec.rb @@ -10,21 +10,23 @@ RSpec.describe "Start Page Features" do sign_in user end - it "takes you to logs" do + it "takes you to the home page" do visit("/") expect(page).to have_current_path("/") + expect(page).to have_content("Welcome back") end end context "when the user is not signed in" do - it "takes you to sign in and then to logs" do + it "takes you to sign in and then to the home page" do visit("/") click_link("Start now") expect(page).to have_current_path("/account/sign-in?start=true") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") - expect(page).to have_current_path("/lettings-logs") + expect(page).to have_current_path("/") + expect(page).to have_content("Welcome back") end end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 04d5bd323..9e1aae35d 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -133,6 +133,7 @@ RSpec.describe "User Features" do fill_in("user[password]", with: "pAssword1") click_button("Sign in") expect(page).to have_current_path("/") + expect(page).to have_content("Welcome back") end it "tries to access account page, redirects to log in page" do diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index 0da64c9ba..1dc930884 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -22,7 +22,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) end @@ -46,11 +46,29 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) end end end + context "when the user is on the home page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Home", "/", true), + NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}/details", false), + NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), + NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), + ] + end + + it "returns navigation items with the home item set as current" do + expect(primary_items("/", current_user)).to eq(expected_navigation_items) + end + end context "when the user is on the lettings logs page" do let(:expected_navigation_items) do @@ -66,7 +84,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) end end @@ -85,7 +103,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the sales logs item set as current" do expect(primary_items("/sales-logs", current_user)).to eq(expected_navigation_items) end end @@ -123,7 +141,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the organisation item set as current" do expect(primary_items("/organisations/#{current_user.organisation.id}/details", current_user)).to eq(expected_navigation_items) end end @@ -142,7 +160,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with no items set as current" do expect(primary_items("/account", current_user)).to eq(expected_navigation_items) end end @@ -180,7 +198,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with Schemes item set as current" do + it "returns navigation items with schemes item set as current" do expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) end end @@ -210,7 +228,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) end @@ -234,7 +252,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) end end @@ -244,6 +262,25 @@ RSpec.describe NavigationItemsHelper do context "when the user is a support user" do let(:current_user) { create(:user, :support) } + context "when the user is on the home page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Home", "/", true), + NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}/details", false), + NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), + NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), + ] + end + + it "returns navigation items with the home item set as current" do + expect(primary_items("/", current_user)).to eq(expected_navigation_items) + end + end + context "when the user is on the lettings logs page" do let(:expected_navigation_items) do [ @@ -256,7 +293,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) end end @@ -273,7 +310,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the sales logs item set as current" do expect(primary_items("/sales-logs", current_user)).to eq(expected_navigation_items) end end @@ -307,7 +344,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the no items set as current" do expect(primary_items("/account", current_user)).to eq(expected_navigation_items) end end @@ -324,7 +361,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the users item set as current" do + it "returns navigation items with the schemes item set as current" do expect(primary_items("/schemes", current_user)).to eq(expected_navigation_items) end end @@ -365,7 +402,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with Schemes item set as current" do + it "returns navigation items with schemes item set as current" do expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) expect(scheme_items("/schemes/1", 1)).to eq(expected_scheme_items) end @@ -390,7 +427,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with Schemes item set as current" do + it "returns navigation items with schemes item set as current" do expect(primary_items("/schemes/1/locations", current_user)).to eq(expected_navigation_items) expect(scheme_items("/schemes/1/locations", 1)).to eq(expected_scheme_items) end @@ -422,7 +459,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the logs item set as current" do + it "returns navigation items with the lettings logs item set as current" do expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) end @@ -453,7 +490,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the logs item set as current" do + it "returns navigation items with the users item set as current" do expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) end @@ -515,7 +552,7 @@ RSpec.describe NavigationItemsHelper do ] end - it "returns navigation items with the logs item set as current" do + it "returns navigation items with the organisation item set as current" do expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index f335b61e6..59d4e27c9 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -77,6 +77,7 @@ RSpec.describe UsersController, type: :request do sign_in user get "/", headers:, params: {} expect(path).to include("/") + expect(page).to have_content("Welcome back") expected_link = "
" expect(CGI.unescape_html(response.body)).to include(expected_link) end @@ -2027,6 +2028,7 @@ RSpec.describe UsersController, type: :request do it "routes user to the home page" do get "/", headers:, params: {} expect(path).to include("/") + expect(page).to have_content("Welcome back") expected_link = "" expect(CGI.unescape_html(response.body)).to include(expected_link) end From d7bf4d22221e010ef62b2e9947ca2a41db79f34b Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 2 Jan 2024 14:43:08 +0000 Subject: [PATCH 03/11] refactor: lint --- spec/helpers/navigation_items_helper_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index 1dc930884..367a43258 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -51,6 +51,7 @@ RSpec.describe NavigationItemsHelper do end end end + context "when the user is on the home page" do let(:expected_navigation_items) do [ From de3d486bd0c669efe600bf20b3029bf51908dbdf Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 2 Jan 2024 15:01:36 +0000 Subject: [PATCH 04/11] feat: update tests --- spec/helpers/navigation_items_helper_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index 367a43258..2d789c028 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -267,13 +267,11 @@ RSpec.describe NavigationItemsHelper do let(:expected_navigation_items) do [ NavigationItemsHelper::NavigationItem.new("Home", "/", true), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", false), NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), - NavigationItemsHelper::NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}/details", false), - NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), - NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", false), ] end From 70cefd7d8280b195fa9d55976e8a5a66befbf936 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Tue, 2 Jan 2024 16:36:04 +0000 Subject: [PATCH 05/11] CLDC-3076: Make example dates consistent (#2107) * CLDC-3076: Make example dates consistent * Use example date even when some hint text provided already * Temp remove some date restrictions * Update to 2 line breaks * Revert "Temp remove some date restrictions" This reverts commit cd7f18f9f10957be0cbabee7665c0abe4239acb6. --- app/helpers/collection_time_helper.rb | 3 ++- app/views/form/_date_question.html.erb | 2 +- spec/helpers/collection_time_helper_spec.rb | 14 +++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/helpers/collection_time_helper.rb b/app/helpers/collection_time_helper.rb index 34426bab2..ea7601c70 100644 --- a/app/helpers/collection_time_helper.rb +++ b/app/helpers/collection_time_helper.rb @@ -13,7 +13,8 @@ module CollectionTimeHelper end def date_mid_collection_year_formatted(date) - example_date = date.nil? ? Time.zone.today : collection_start_date(date).to_date + 5.months + relevant_year = date.nil? ? current_collection_start_year : collection_start_year_for_date(date) + example_date = Date.new(relevant_year, 9, 13) example_date.to_formatted_s(:govuk_date_number_month) end diff --git a/app/views/form/_date_question.html.erb b/app/views/form/_date_question.html.erb index c84d60d0e..9de5b8f2d 100644 --- a/app/views/form/_date_question.html.erb +++ b/app/views/form/_date_question.html.erb @@ -3,7 +3,7 @@ <%= f.govuk_date_field question.id.to_sym, caption: caption(caption_text, page_header, conditional), legend: legend(question, page_header, conditional), - hint: { text: question.hint_text&.html_safe || "For example, #{date_mid_collection_year_formatted(lettings_log.startdate)}" }, + hint: { text: (question.hint_text.nil? ? "" : (question.hint_text.html_safe + "

".html_safe)) + "For example, #{date_mid_collection_year_formatted(lettings_log.startdate)}" }, width: 20, **stimulus_html_attributes(question) do %> <%= govuk_inset_text(text: question.unresolved_hint_text) if question.unresolved_hint_text.present? && @log.unresolved %> diff --git a/spec/helpers/collection_time_helper_spec.rb b/spec/helpers/collection_time_helper_spec.rb index e9ec52292..3eef01b5e 100644 --- a/spec/helpers/collection_time_helper_spec.rb +++ b/spec/helpers/collection_time_helper_spec.rb @@ -85,9 +85,9 @@ RSpec.describe CollectionTimeHelper do context "when called with nil" do let(:input) { nil } - it "returns the current date" do - today = Time.zone.today - expect(result).to eq("#{today.day} #{today.month} #{today.year}") + it "returns the 13th of September in the current collection year" do + year = current_collection_start_year + expect(result).to eq("13 9 #{year}") end end @@ -95,8 +95,8 @@ RSpec.describe CollectionTimeHelper do calendar_year = 2030 let(:input) { Date.new(calendar_year, 7, 7) } - it "returns the first of September from that year" do - expect(result).to eq("1 9 #{calendar_year}") + it "returns the 13th of September from that year" do + expect(result).to eq("13 9 #{calendar_year}") end end @@ -104,8 +104,8 @@ RSpec.describe CollectionTimeHelper do calendar_year = 2040 let(:input) { Date.new(calendar_year, 2, 7) } - it "returns the first of September from the previous year" do - expect(result).to eq("1 9 #{calendar_year - 1}") + it "returns the 13th of September from the previous year" do + expect(result).to eq("13 9 #{calendar_year - 1}") end end end From eb99ac32ce8306c7f55343c8aeb8554fda6884ba Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 3 Jan 2024 12:51:01 +0000 Subject: [PATCH 06/11] Fix lettings log accessor in date question (#2117) * Fix lettings log accessor in date question * Remove hardcoded date example from mrcdate question (#2118) --------- Co-authored-by: Rachael Booth --- app/models/form/lettings/questions/mrcdate.rb | 1 - app/views/form/_date_question.html.erb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/form/lettings/questions/mrcdate.rb b/app/models/form/lettings/questions/mrcdate.rb index cadcd3a7a..09176dca6 100644 --- a/app/models/form/lettings/questions/mrcdate.rb +++ b/app/models/form/lettings/questions/mrcdate.rb @@ -6,7 +6,6 @@ class Form::Lettings::Questions::Mrcdate < ::Form::Question @header = "When were the repairs completed?" @type = "date" @check_answers_card_number = 0 - @hint_text = "For example, 27 3 2021." @question_number = 24 end end diff --git a/app/views/form/_date_question.html.erb b/app/views/form/_date_question.html.erb index 9de5b8f2d..1be21d212 100644 --- a/app/views/form/_date_question.html.erb +++ b/app/views/form/_date_question.html.erb @@ -3,7 +3,7 @@ <%= f.govuk_date_field question.id.to_sym, caption: caption(caption_text, page_header, conditional), legend: legend(question, page_header, conditional), - hint: { text: (question.hint_text.nil? ? "" : (question.hint_text.html_safe + "

".html_safe)) + "For example, #{date_mid_collection_year_formatted(lettings_log.startdate)}" }, + hint: { text: (question.hint_text.nil? ? "" : (question.hint_text.html_safe + "

".html_safe)) + "For example, #{date_mid_collection_year_formatted(@log.startdate)}" }, width: 20, **stimulus_html_attributes(question) do %> <%= govuk_inset_text(text: question.unresolved_hint_text) if question.unresolved_hint_text.present? && @log.unresolved %> From d3a893522d60c7a7a77ae00a71bd3e9c2c04f936 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 5 Jan 2024 11:47:12 +0000 Subject: [PATCH 07/11] CLDC-3061 Add guidance page (#2121) * Add guidance page * Link to guidance from start page --- app/views/start/guidance.html.erb | 68 ++++++++++++++++++++++++++ app/views/start/index.html.erb | 1 + config/routes.rb | 1 + spec/requests/start_controller_spec.rb | 56 +++++++++++++++++++++ 4 files changed, 126 insertions(+) create mode 100644 app/views/start/guidance.html.erb create mode 100644 spec/requests/start_controller_spec.rb diff --git a/app/views/start/guidance.html.erb b/app/views/start/guidance.html.erb new file mode 100644 index 000000000..127b56b3b --- /dev/null +++ b/app/views/start/guidance.html.erb @@ -0,0 +1,68 @@ +

+ Guidance for submitting social housing lettings and sales data +

+ +
+
+

This page includes details of when a CORE log is and is not required, what to do if a tenant or buyer is reluctant to answer questions in a log, and other information about submitting logs using CORE.

+ <%= govuk_accordion do |accordion| %> + <%= accordion.with_section(heading_text: "How to create logs", expanded: true) do %> +

There are 2 ways to create logs on CORE.

+

You can create logs one at a time by answering questions using the online form. Click the “Create a new log” button on the logs page to create logs this way.

+

You can also create many logs at once by uploading a CSV file. This might be faster than creating logs individually if your organisation has its own database and a way to export the data. Click the “Upload logs in bulk” button on the logs page to create logs this way. For more information, <%= govuk_link_to "read the full guidance on bulk upload", bulk_upload_lettings_log_path(id: "guidance", form: { year: current_collection_start_year }) %>.

+

Once you have created and completed a log, there is nothing more you need to do to submit the data.

+ <% end %> + + <%= accordion.with_section(heading_text: "What scenarios require a new log?") do %> +

For general needs, you should complete a log for each new tenancy intended to last 2 years or more if it is social rent or affordable rent, or of any length if it is intermediate rent.

+

For supported housing, you should complete a log for each new letting of any length.

+

If a new tenancy agreement is signed, create a new log.

+ <% end %> + + <%= accordion.with_section(heading_text: "Types of lettings you should create logs for") do %> +

You’ll need to create a log for:

+
    +
  • Tenants in general needs housing allocated a new letting. This includes tenants moving into the social rented sector from outside, existing social tenants moving between properties or landlords, and existing social tenants renewing lettings in the same property. If fixed-term and social or affordable rent, only include tenancies of 2 years or more.
  • +
  • Tenants in supported housing (social housing, sheltered accommodation and care homes) allocated a new letting. This includes tenants moving into the social rented sector from outside, existing social tenants moving between properties or landlords, and existing social tenants renewing lettings in the same property. All supported housing tenancies should be reported regardless of length.
  • +
  • Starter tenancies provided by local authorities (LAs) and lettings with an introductory period provided by private registered providers (PRPs) should be completed in CORE at the beginning of the starter or introductory period. The tenancy type and length entered should be based on the tenancy the tenant will roll onto once the starter or introductory period has been completed. You do not need to submit another CORE log once the period has been completed.
  • +
  • Room moves within a shared housing unit that result in a different property type or support needs – this is classed as an internal transfer of an existing social tenant to another property.
  • +
  • Existing tenants who are issued with a new tenancy agreement when stock is acquired, transferred or permanently decanted.
  • +
  • Tenants under the Rough Sleepers Initiative or Rough Sleeping Accommodation Programme, where accommodation is permanent.
  • +
  • Households previously provided with temporary accommodation to meet a duty under the homelessness legislation who are allocated a tenancy as a settled home ending the duty (this may be the same property).
  • +
  • Refugees and asylum seekers who have been granted indefinite leave to remain, humanitarian protection or exceptional leave to remain.
  • +
  • Affordable Rent lettings – where up to 80% of market rent can be charged and a new supply agreement is signed.
  • +
  • London Affordable Rent lettings – a type of Affordable Rent available in London through the Greater London Authority (GLA).
  • +
  • Intermediate Rent lettings – where the rent must not exceed 80% of the current market rate (including any service charges).
  • +
  • Rent to Buy lettings – where a discount of up to 20% market rent is charged for a single rental period for a minimum of 5 years. After that period, the tenant is offered the chance to purchase the property (either shared ownership or outright) at full market value.
  • +
  • London Living Rent lettings – a type of Intermediate Rent available in London through the Greater London Authority (GLA).
  • +
+ <% end %> + + <%= accordion.with_section(heading_text: "Types of lettings you should not create logs for") do %> +

You don’t need to create a log for:

+
    +
  • Temporary general needs housing with a fixed period of less than 2 years if they are social or affordable rent. (Temporary lettings for intermediate rent and supported housing should be recorded).
  • +
  • Starter tenancies or lettings with an introductory period that roll onto or convert into the main tenancy. The CORE log should be completed at the beginning of this period.
  • +
  • Changes from sole to joint or joint to sole tenancies, where the number of people in the household has not changed.
  • +
  • Moves within a shared housing unit resulting in the same support needs or property type, even if a new tenancy or licence agreement is issued.
  • +
  • Lettings where no new tenancy agreement is signed.
  • +
  • Where stock is acquired, transferred or permanently decanted and the existing tenants are not issued with a new tenancy agreement.
  • +
  • Mutual exchanges including lettings where registered provider tenants have exchanged homes, for example through the national HOMESWAP system.
  • +
  • Successions and assignments.
  • +
  • Demotion of a secure or assured tenancy, and any subsequent conversion of the demoted tenancy to a secure or assured tenancy.
  • +
  • Lettings made to asylum seekers who are awaiting a decision on their applications for asylum under the Immigration and Asylum Act 1999.
  • +
  • Non-social lettings, including market-rented properties, employer-provided housing where the employer provides financial support, homes for staff of social landlords linked to employment, homes social landlords manage for organisations who are not social landlords, homes social landlords own but lease in entirety to organisations who are not social landlords, and freehold housing with variable charges for services and communal facilities.
  • +
+ <% end %> + + <%= accordion.with_section(heading_text: "What if someone is reluctant to answer any questions?") do %> +

If a tenant or buyer is reluctant to answer questions as part of a log, you should explain that:

+
    +
  • all information they provide is anonymous and will not affect their housing, benefits or other services they receive.
  • +
  • the data they provide is vital in helping to build a complete picture of social housing in England and is used to inform social housing policy.
  • +
+

If a tenant or buyer is still unwilling or unable to answer questions, select the ‘Don’t know’ or ‘Tenant/person prefers not to say’ options.

+ <% end %> + <% end %> +
+
diff --git a/app/views/start/index.html.erb b/app/views/start/index.html.erb index 3c10232d2..d6ec6d392 100644 --- a/app/views/start/index.html.erb +++ b/app/views/start/index.html.erb @@ -20,6 +20,7 @@

Use your account details to sign in.

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) %>.

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.

+

<%= govuk_link_to guidance_path do %>Guidance for submitting social housing lettings and sales data (CORE)<% end %>

diff --git a/config/routes.rb b/config/routes.rb index af0b95d07..94626f34d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -30,6 +30,7 @@ Rails.application.routes.draw do resource :cookies, only: %i[show update] root to: "start#index" + get "/guidance", to: "start#guidance" get "/logs", to: redirect("lettings-logs") get "/accessibility-statement", to: "content#accessibility_statement" diff --git a/spec/requests/start_controller_spec.rb b/spec/requests/start_controller_spec.rb new file mode 100644 index 000000000..f24249749 --- /dev/null +++ b/spec/requests/start_controller_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +RSpec.describe StartController, type: :request do + let(:user) { create(:user) } + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:notify_client) { instance_double(Notifications::Client) } + let(:devise_notify_mailer) { DeviseNotifyMailer.new } + + before do + allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) + allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) + end + + describe "GET" do + context "when the user is not signed in" do + it "routes user to the start in page" do + get "/", headers: headers, params: {} + expect(path).to include("/") + expect(page).to have_content("Start now") + end + end + + context "when the user is signed in" do + before do + sign_in user + end + + it "routes user to the home page" do + get "/", headers:, params: {} + expect(page).to have_content("Welcome back") + end + end + end + + describe "guidance page" do + context "when the user is not signed in" do + it "routes user to the guidance page" do + get "/guidance", headers:, params: {} + expect(page).to have_content("Guidance for submitting social housing lettings and sales data") + end + end + + context "when the user is signed in" do + before do + sign_in user + end + + it "routes user to the guidance page" do + get "/guidance", headers:, params: {} + expect(page).to have_content("Guidance for submitting social housing lettings and sales data") + end + end + end +end From fc011f1d3f9fb992ecf497d72442b2e62578e37e Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 8 Jan 2024 10:26:59 +0000 Subject: [PATCH 08/11] feat: test home/start paths explicitly --- spec/requests/start_controller_spec.rb | 4 ++-- spec/requests/users_controller_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/requests/start_controller_spec.rb b/spec/requests/start_controller_spec.rb index f24249749..1bfd2b4b9 100644 --- a/spec/requests/start_controller_spec.rb +++ b/spec/requests/start_controller_spec.rb @@ -15,9 +15,9 @@ RSpec.describe StartController, type: :request do describe "GET" do context "when the user is not signed in" do - it "routes user to the start in page" do + it "routes user to the start page" do get "/", headers: headers, params: {} - expect(path).to include("/") + expect(path).to eq("/") expect(page).to have_content("Start now") end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 59d4e27c9..38458eb7f 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -76,7 +76,7 @@ RSpec.describe UsersController, type: :request do it "routes user to the home page" do sign_in user get "/", headers:, params: {} - expect(path).to include("/") + expect(path).to eq("/") expect(page).to have_content("Welcome back") expected_link = "" expect(CGI.unescape_html(response.body)).to include(expected_link) @@ -2027,7 +2027,7 @@ RSpec.describe UsersController, type: :request do it "routes user to the home page" do get "/", headers:, params: {} - expect(path).to include("/") + expect(path).to eq("/") expect(page).to have_content("Welcome back") expected_link = "" expect(CGI.unescape_html(response.body)).to include(expected_link) From 37dcf699be6f4b52036650ebd09a995f9580ffe6 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 8 Jan 2024 15:02:22 +0000 Subject: [PATCH 09/11] feat: add notification table --- db/migrate/20240108145545_create_notification.rb | 14 ++++++++++++++ db/schema.rb | 13 ++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20240108145545_create_notification.rb diff --git a/db/migrate/20240108145545_create_notification.rb b/db/migrate/20240108145545_create_notification.rb new file mode 100644 index 000000000..dc69a8efe --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 22d67bb9f..ffe8275cb 100644 --- a/db/schema.rb +++ b/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_145545) 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" From 7716c5c680b314882ee59e8e85b59c9c47186a94 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 8 Jan 2024 18:23:10 +0000 Subject: [PATCH 10/11] feat: add notification banner, use unread gem for notification management --- Gemfile | 1 + app/components/unread_notification.html.erb | 15 ++++++++++++ app/components/unread_notification.rb | 12 ++++++++++ app/controllers/notifications_controller.rb | 8 +++++++ app/frontend/styles/_unread-notification.scss | 7 ++++++ app/frontend/styles/application.scss | 5 ++-- app/models/notification.rb | 3 +++ app/models/user.rb | 2 ++ app/views/layouts/application.html.erb | 9 +++++-- config/routes.rb | 4 ++++ db/migrate/20240108152935_unread_migration.rb | 24 +++++++++++++++++++ db/schema.rb | 13 +++++++++- 12 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 app/components/unread_notification.html.erb create mode 100644 app/components/unread_notification.rb create mode 100644 app/controllers/notifications_controller.rb create mode 100644 app/frontend/styles/_unread-notification.scss create mode 100644 app/models/notification.rb create mode 100644 db/migrate/20240108152935_unread_migration.rb diff --git a/Gemfile b/Gemfile index 314849ff2..bafc88826 100644 --- a/Gemfile +++ b/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 diff --git a/app/components/unread_notification.html.erb b/app/components/unread_notification.html.erb new file mode 100644 index 000000000..acdf2d859 --- /dev/null +++ b/app/components/unread_notification.html.erb @@ -0,0 +1,15 @@ +
+
+
+
+
+

<%= oldest_unread_notification.title %>

+ <%= govuk_link_to oldest_unread_notification.link_text, notification_path(oldest_unread_notification), class: "govuk-link--inverse govuk-!-font-weight-bold"%> +
+

+ <%= govuk_link_to "Dismiss", notification_dismiss_path(oldest_unread_notification), class: "govuk-link--inverse"%> +

+
+
+
+
diff --git a/app/components/unread_notification.rb b/app/components/unread_notification.rb new file mode 100644 index 000000000..d86280e31 --- /dev/null +++ b/app/components/unread_notification.rb @@ -0,0 +1,12 @@ +class UnreadNotification < ViewComponent::Base + attr_reader :current_user + + def initialize(current_user:) + @current_user = current_user + super + end + + def oldest_unread_notification + Notification.unread_by(@current_user).first + end +end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb new file mode 100644 index 000000000..73fe702d7 --- /dev/null +++ b/app/controllers/notifications_controller.rb @@ -0,0 +1,8 @@ +class NotificationsController < ApplicationController + + def dismiss + Notification.find(params[:notification_id]).mark_as_read! for: current_user + + redirect_to root_path + end +end diff --git a/app/frontend/styles/_unread-notification.scss b/app/frontend/styles/_unread-notification.scss new file mode 100644 index 000000000..d76b36fa2 --- /dev/null +++ b/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"); +} diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index ddf368807..272310b5b 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -25,7 +25,9 @@ $govuk-breakpoints: ( @import "accessible-autocomplete"; @import "button"; @import "card"; +@import "delete-logs-table"; @import "document-list"; +@import "errors"; @import "feedback"; @import "filter"; @import "filter-layout"; @@ -43,8 +45,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 { diff --git a/app/models/notification.rb b/app/models/notification.rb new file mode 100644 index 000000000..9e3a8482f --- /dev/null +++ b/app/models/notification.rb @@ -0,0 +1,3 @@ +class Notification < ApplicationRecord + acts_as_readable +end diff --git a/app/models/user.rb b/app/models/user.rb index bd04ba2b2..74ac34547 100644 --- a/app/models/user.rb +++ b/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, diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 30ec7d098..c540ad5cc 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -101,6 +101,11 @@ end end %> + <% if current_user.present? && current_page?("/") && Notification.unread_by(current_user).present? %> + <%= render UnreadNotification.new(current_user:) %> + <% 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( @@ -109,7 +114,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), ) %> @@ -124,7 +129,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.heading(text: flash.notice.html_safe) if flash[:notification_banner_body] diff --git a/config/routes.rb b/config/routes.rb index 94626f34d..4339892dd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -128,6 +128,10 @@ Rails.application.routes.draw do end end + resources :notifications do + get "dismiss", to: "notifications#dismiss" + end + resources :organisations do get "duplicates", to: "duplicate_logs#index" diff --git a/db/migrate/20240108152935_unread_migration.rb b/db/migrate/20240108152935_unread_migration.rb new file mode 100644 index 000000000..099e74b4a --- /dev/null +++ b/db/migrate/20240108152935_unread_migration.rb @@ -0,0 +1,24 @@ +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 + end + + add_index ReadMark, [: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 diff --git a/db/schema.rb b/db/schema.rb index ffe8275cb..e75f47e60 100644 --- a/db/schema.rb +++ b/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_01_08_145545) 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" @@ -457,6 +457,17 @@ ActiveRecord::Schema[7.0].define(version: 2024_01_08_145545) 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" From 0674f1e4bb11e169b1ac3535c757030ec1697775 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 9 Jan 2024 11:58:37 +0000 Subject: [PATCH 11/11] feat: add notifications page and remove unread_notification.rb --- app/components/unread_notification.html.erb | 15 ------------- app/components/unread_notification.rb | 12 ----------- app/controllers/notifications_controller.rb | 13 ++++++++++-- app/frontend/styles/_header.scss | 4 ++++ app/helpers/application_helper.rb | 2 ++ app/helpers/navigation_items_helper.rb | 2 +- app/models/user.rb | 8 +++++++ app/views/layouts/application.html.erb | 4 ++-- .../_notification_banner.html.erb | 21 +++++++++++++++++++ app/views/notifications/show.html.erb | 15 +++++++++++++ config/routes.rb | 2 +- 11 files changed, 65 insertions(+), 33 deletions(-) delete mode 100644 app/components/unread_notification.html.erb delete mode 100644 app/components/unread_notification.rb create mode 100644 app/views/notifications/_notification_banner.html.erb create mode 100644 app/views/notifications/show.html.erb diff --git a/app/components/unread_notification.html.erb b/app/components/unread_notification.html.erb deleted file mode 100644 index acdf2d859..000000000 --- a/app/components/unread_notification.html.erb +++ /dev/null @@ -1,15 +0,0 @@ -
-
-
-
-
-

<%= oldest_unread_notification.title %>

- <%= govuk_link_to oldest_unread_notification.link_text, notification_path(oldest_unread_notification), class: "govuk-link--inverse govuk-!-font-weight-bold"%> -
-

- <%= govuk_link_to "Dismiss", notification_dismiss_path(oldest_unread_notification), class: "govuk-link--inverse"%> -

-
-
-
-
diff --git a/app/components/unread_notification.rb b/app/components/unread_notification.rb deleted file mode 100644 index d86280e31..000000000 --- a/app/components/unread_notification.rb +++ /dev/null @@ -1,12 +0,0 @@ -class UnreadNotification < ViewComponent::Base - attr_reader :current_user - - def initialize(current_user:) - @current_user = current_user - super - end - - def oldest_unread_notification - Notification.unread_by(@current_user).first - end -end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 73fe702d7..d48d57b41 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,8 +1,17 @@ class NotificationsController < ApplicationController def dismiss - Notification.find(params[:notification_id]).mark_as_read! for: current_user + current_user.oldest_active_unread_notification.mark_as_read! for: current_user - redirect_to root_path + redirect_back(fallback_location: root_path) + end + + def show + @notification = current_user.oldest_active_unread_notification + if @notification&.page_content + render "show" + else + redirect_back(fallback_location: root_path) + end end end diff --git a/app/frontend/styles/_header.scss b/app/frontend/styles/_header.scss index 12cfd4e54..924276d5f 100644 --- a/app/frontend/styles/_header.scss +++ b/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; +} diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 971dd68d9..e91261466 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -12,6 +12,8 @@ module ApplicationHelper def govuk_header_classes(current_user) if current_user && current_user.support? "app-header app-header--orange" + elsif current_user && Notification.unread_by(current_user).present? && !current_page?(notifications_path) + "app-header app-header__no-border-bottom" else "app-header" end diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index 18132a8a1..95e8cc9cf 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/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) diff --git a/app/models/user.rb b/app/models/user.rb index 74ac34547..1aa7898e1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -229,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.unread_by(self) + end + + def oldest_active_unread_notification + active_unread_notifications.first + end + protected # Checks whether a password is needed or not. For validations only. diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index c540ad5cc..afe153016 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -101,8 +101,8 @@ end end %> - <% if current_user.present? && current_page?("/") && Notification.unread_by(current_user).present? %> - <%= render UnreadNotification.new(current_user:) %> + <% if current_user.present? && Notification.unread_by(current_user).present? && !current_page?(notifications_path)%> + <%= render "notifications/notification_banner" %> <% end %> diff --git a/app/views/notifications/_notification_banner.html.erb b/app/views/notifications/_notification_banner.html.erb new file mode 100644 index 000000000..18defb23f --- /dev/null +++ b/app/views/notifications/_notification_banner.html.erb @@ -0,0 +1,21 @@ +
+
+
+
+
+ <% if current_user.active_unread_notifications.count > 1 %> +

Notification 1 of <%= current_user.active_unread_notifications.count %>

+ <% end %> +

<%= current_user.oldest_active_unread_notification.title %>

+ <% if current_user.oldest_active_unread_notification.page_content.present? %> +
+ <%= govuk_link_to current_user.oldest_active_unread_notification.link_text, notifications_path, class: "govuk-link--inverse govuk-!-font-weight-bold"%> +
+ <% end %> +
+

+ <%= govuk_link_to "Dismiss", dismiss_notifications_path, class: "govuk-link--inverse"%> +

+
+
+
diff --git a/app/views/notifications/show.html.erb b/app/views/notifications/show.html.erb new file mode 100644 index 000000000..a9034531f --- /dev/null +++ b/app/views/notifications/show.html.erb @@ -0,0 +1,15 @@ +<% content_for :title, "Notification" %> +<% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> +<% end %> + +
+
+

<%= @notification.title %>

+ <%= @notification.page_content %> +
+
+
+
+ <%= govuk_button_link_to "Back to Home", root_path %> +
diff --git a/config/routes.rb b/config/routes.rb index 4339892dd..06cb4f8d3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -128,7 +128,7 @@ Rails.application.routes.draw do end end - resources :notifications do + resource :notifications do get "dismiss", to: "notifications#dismiss" end