diff --git a/Gemfile b/Gemfile index 332359f98..4dc93aaf7 100644 --- a/Gemfile +++ b/Gemfile @@ -23,6 +23,7 @@ gem "govuk-components", "~> 5.1" gem "govuk_design_system_formbuilder", "~> 5.0" # Convert Markdown into GOV.UK frontend-styled HTML gem "govuk_markdown" +gem "redcarpet", "~> 3.6" # GOV UK Notify gem "notifications-ruby-client" # A modest javascript framework for the html you already have diff --git a/Gemfile.lock b/Gemfile.lock index 086491321..7cc2addda 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -559,6 +559,7 @@ DEPENDENCIES rack-mini-profiler (~> 2.0) rails (~> 7.0.8.3) rails_admin (~> 3.1) + redcarpet (~> 3.6) redis (~> 4.8) roo rspec-rails diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index e3aff82e6..22f284992 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,19 +1,77 @@ class NotificationsController < ApplicationController + before_action :authenticate_user!, except: %i[show] + before_action :authenticate_scope!, except: %i[show dismiss] + def dismiss - if current_user.blank? - redirect_to root_path - else - current_user.newest_active_unread_notification.mark_as_read! for: current_user if current_user.newest_active_unread_notification.present? - redirect_back(fallback_location: root_path) - end + @notification = Notification.find_by(id: params[:notification_id]) + @notification.mark_as_read! for: current_user + redirect_back(fallback_location: root_path) end def show - @notification = current_user&.newest_active_unread_notification || Notification.newest_active_unauthenticated_notification - if @notification&.page_content + @notification = Notification.find_by(id: params[:id]) + if @notification&.page_content && (@notification&.show_on_unauthenticated_pages || current_user) render "show" else redirect_back(fallback_location: root_path) end end + + def new + @notification = Notification.new + end + + def create + @notification = Notification.new(notification_params) + + if @notification.errors.empty? && @notification.save + redirect_to notification_check_answers_path(@notification) + else + render :new, status: :unprocessable_entity + end + end + + def check_answers + @notification = Notification.find_by(id: params[:notification_id]) + render_not_found and return unless @notification + + render "notifications/check_answers" + end + + def edit + @notification = Notification.find_by(id: params[:id]) + end + + def update + @notification = Notification.find_by(id: params[:id]) + render_not_found and return unless @notification + + start_now = params[:notification][:start_now] + + update_params = notification_params.except(:start_now) + update_params[:start_date] = Time.zone.now if start_now + + if @notification.errors.empty? && @notification.update(update_params) + if start_now + flash[:notice] = "The notification has been created" + redirect_to root_path + else + redirect_to notification_check_answers_path(@notification) + end + elsif start_now + render :check_answers, status: :unprocessable_entity + else + render :edit, status: :unprocessable_entity + end + end + +private + + def notification_params + params.require(:notification).permit(:title, :show_on_unauthenticated_pages, :start_now) + end + + def authenticate_scope! + render_not_found unless current_user.support? + end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 86978ebea..9049dcd3b 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,7 +1,9 @@ class Notification < ApplicationRecord acts_as_readable - scope :active, -> { where("start_date <= ? AND end_date >= ?", Time.zone.now, Time.zone.now) } + validates :title, presence: { message: I18n.t("activerecord.errors.models.notification.attributes.title.blank") } + + scope :active, -> { where("start_date <= ? AND (end_date >= ? OR end_date is null)", Time.zone.now, Time.zone.now) } scope :unauthenticated, -> { where(show_on_unauthenticated_pages: true) } def self.active_unauthenticated_notifications @@ -11,4 +13,25 @@ class Notification < ApplicationRecord def self.newest_active_unauthenticated_notification active_unauthenticated_notifications.last end + + def rendered_title(options = {}) + renderer = NotificationTitleRenderer.new(options) + Redcarpet::Markdown.new(renderer, no_intra_emphasis: true).render(title) + end +end + +class NotificationTitleRenderer < Redcarpet::Render::HTML + def initialize(options = {}) + link_class = "govuk-link" + link_class += " govuk-link--inverse" if options[:invert_link_colour] + @bold = options[:bold_all_text] + base_options = { escape_html: true, safe_links_only: true, link_attributes: { class: link_class } } + super base_options + end + + def paragraph(text) + return %(

#{text}

) if @bold + + %(

#{text}

) + end end diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb index 6995c28e5..bb4792e68 100644 --- a/app/views/home/index.html.erb +++ b/app/views/home/index.html.erb @@ -6,6 +6,12 @@

<%= @homepage_presenter.title_text_for_user %>

+ <% if @current_user.support? %> +
+ <%= govuk_button_link_to "Create a new notification", new_notification_path %> +
+ <% end %> +
<% if @homepage_presenter.in_crossover_period? %> diff --git a/app/views/notifications/_form.html.erb b/app/views/notifications/_form.html.erb new file mode 100644 index 000000000..6b9bd9708 --- /dev/null +++ b/app/views/notifications/_form.html.erb @@ -0,0 +1,29 @@ +
+
+ <%= f.govuk_error_summary %> + + <% content_for :page_title, "Create a new notification" %> + + <% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> + <% end %> + +

+ <%= content_for(:page_title) %> +

+ + This notification will be visible to all users until you delete it + + <%= f.govuk_text_field :title, + label: { text: "Title", size: "m" }, + hint: { text: "Use markdown for links to existing pages" } %> + + <%= f.govuk_check_boxes_fieldset :show_on_unauthenticated_pages, multiple: false, legend: { text: "Display Options" } do %> + <%= f.govuk_check_box :show_on_unauthenticated_pages, 1, 0, multiple: false, label: { text: "Show this notification on unauthenticated pages, for example the start page" } %> + <% end %> + + View <%= govuk_link_to "markdown syntax guide", "https://www.markdownguide.org/basic-syntax/" %> + + <%= f.govuk_submit "Continue" %> +
+
diff --git a/app/views/notifications/_notification_banner.html.erb b/app/views/notifications/_notification_banner.html.erb index cd7dfffac..67de16afa 100644 --- a/app/views/notifications/_notification_banner.html.erb +++ b/app/views/notifications/_notification_banner.html.erb @@ -6,16 +6,16 @@ <% if notification_count > 1 && current_user.present? %>

Notification 1 of <%= notification_count %>

<% end %> -

<%= notification.title.html_safe %>

+ <%== notification.rendered_title(invert_link_colour: true, bold_all_text: true) %> <% if notification.page_content.present? %>
- <%= govuk_link_to notification.link_text, notifications_path, class: "govuk-link--inverse govuk-!-font-weight-bold" %> + <%= govuk_link_to notification.link_text, notification_path(notification), class: "govuk-link--inverse govuk-!-font-weight-bold" %>
<% end %>
<% if current_user.present? %>

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

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

+ Create a notification + Check your answers +

+ + This notification will be visible to all users until you delete it + + <%= form_for(@notification, method: :patch) do |f| %> + <%= f.govuk_error_summary %> + +
+
+ <%= govuk_summary_list do |summary_list| %> + <% summary_list.with_row do |row| %> + <% row.with_key { "Title" } %> + <% row.with_value do %> + <%== @notification.rendered_title %> + <% end %> + <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %> + <% end %> + <% summary_list.with_row do |row| %> + <% row.with_key { "Show on unauthenticated pages?" } %> + <% row.with_value { @notification.show_on_unauthenticated_pages ? "Yes" : "No" } %> + <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %> + <% end %> + <% end %> +
+
+ + <%= f.hidden_field :start_now, value: true %> + <%= f.govuk_submit "Create notification" %> + <% end %> +
+
diff --git a/app/views/notifications/edit.html.erb b/app/views/notifications/edit.html.erb new file mode 100644 index 000000000..373cff20d --- /dev/null +++ b/app/views/notifications/edit.html.erb @@ -0,0 +1,3 @@ +<%= form_for(@notification, as: :notification, method: :patch) do |f| %> + <% render partial: "form", locals: { notification: @notification, f: } %> +<% end %> diff --git a/app/views/notifications/new.html.erb b/app/views/notifications/new.html.erb new file mode 100644 index 000000000..d996b9624 --- /dev/null +++ b/app/views/notifications/new.html.erb @@ -0,0 +1,3 @@ +<%= form_for(@notification, as: :notification, method: :post) do |f| %> + <% render partial: "form", locals: { notification: @notification, f: } %> +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index bae5e568d..6a37a81ad 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -186,6 +186,10 @@ en: invalid: "An organisation with this name already exists" new_organisation_telephone_number: blank: "Enter a valid telephone number" + notification: + attributes: + title: + blank: "Enter a title" notification: logs_deleted: diff --git a/config/routes.rb b/config/routes.rb index faea457fe..507a8da2a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -138,8 +138,9 @@ Rails.application.routes.draw do end end - resource :notifications do + resources :notifications do get "dismiss", to: "notifications#dismiss" + get "check-answers", to: "notifications#check_answers" end resources :organisations do diff --git a/spec/features/notifications_page_spec.rb b/spec/features/notifications_spec.rb similarity index 51% rename from spec/features/notifications_page_spec.rb rename to spec/features/notifications_spec.rb index 97bbeb7eb..e2bd4b151 100644 --- a/spec/features/notifications_page_spec.rb +++ b/spec/features/notifications_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" require_relative "form/helpers" -RSpec.describe "Notifications Page Features" do +RSpec.describe "Notifications Features" do include Helpers context "when there are notifications" do @@ -9,32 +9,30 @@ RSpec.describe "Notifications Page Features" do 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") + create(:notification, start_date: Time.zone.yesterday, title: "Notification title 1") + create(:notification, start_date: Time.zone.yesterday, end_date: Time.zone.tomorrow, title: "Notification title 2") sign_in user - visit(notifications_path) + visit(root_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") + it "shows the notification banner" do + expect(page).to have_content("Notification 1 of") + expect(page).to have_link("Dismiss") 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.yesterday, end_date: Time.zone.yesterday, title: "Notification title 1") create(:notification, start_date: Time.zone.tomorrow, title: "Notification title 2") + create(:notification, start_date: nil, title: "Notification title 3") sign_in user - visit(notifications_path) + visit(root_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 diff --git a/spec/features/start_page_spec.rb b/spec/features/start_page_spec.rb index d90a0c1f0..ab09fd446 100644 --- a/spec/features/start_page_spec.rb +++ b/spec/features/start_page_spec.rb @@ -30,14 +30,15 @@ RSpec.describe "Start Page Features" do end context "when the unauthenticated user clicks a notification link" do + let!(:notification) { create(:notification, title: "Notification title", link_text: "link", page_content: "Some html content", show_on_unauthenticated_pages: true) } + before do - create(:notification, show_on_unauthenticated_pages: true) visit(root_path) - click_link("Link text") + click_link("link") end it "takes them to the notification details page" do - expect(page).to have_current_path(notifications_path) + expect(page).to have_current_path(notification_path(notification)) expect(page).to have_content("Notification title") expect(page).to have_content("Some html content") expect(page).to have_link("Back to Start") diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb new file mode 100644 index 000000000..fbd6f55cd --- /dev/null +++ b/spec/requests/notifications_controller_spec.rb @@ -0,0 +1,110 @@ +require "rails_helper" + +RSpec.describe NotificationsController, type: :request do + context "when user is signed in as a support user" do + let(:support_user) { create(:user, :support) } + + before do + allow(support_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in support_user + end + + describe "#create" do + let(:request) { post notifications_path, params: params } + + context "with valid parameters" do + let(:params) { { "notification": { title: "Test Create", show_on_unauthenticated_pages: true } } } + + it "creates a new notification with no start date set" do + request + notification = Notification.find_by(title: "Test Create") + expect(notification.show_on_unauthenticated_pages).to be_truthy + expect(notification.start_date).to be_nil + end + + it "redirects to check answers page" do + request + notification = Notification.find_by(title: "Test Create") + expect(response).to redirect_to(notification_check_answers_path(notification)) + end + end + + context "with invalid parameters" do + let(:params) { { "notification": { title: "", show_on_unauthenticated_pages: true } } } + + it "gives an error response" do + request + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe "#update" do + let(:notification) { create(:notification, title: "Initial Title", start_date: nil, end_date: nil) } + let(:request) { patch notification_path(notification), params: params } + + context "when start_now is set to true" do + let(:params) { { "notification": { start_now: true } } } + + it "sets the start date on the notification" do + request + notification.reload + expect(notification.start_date).not_to be_nil + expect(notification.start_date).to be < Time.zone.now + end + + it "redirects to the home page" do + request + expect(response).to redirect_to(root_path) + end + end + + context "when start_now is not set" do + let(:params) { { "notification": { title: "Updated Title", show_on_unauthenticated_pages: true } } } + + it "sets the relevant values on the notification" do + request + notification.reload + expect(notification.title).to eql("Updated Title") + expect(notification.start_date).to be_nil + end + + it "redirects to check answers" do + request + expect(response).to redirect_to(notification_check_answers_path(notification)) + end + end + end + end + + context "when user is signed in as a non-support user" do + let(:user) { create(:user, :data_coordinator) } + + before do + sign_in user + end + + describe "#create" do + let(:request) { post notifications_path, params: { "notification": { title: "Test Create", show_on_unauthenticated_pages: true } } } + + it "returns not found" do + request + expect(response).to have_http_status(:not_found) + end + + it "does not create a notification" do + expect { request }.not_to change(Notification, :count) + end + end + + describe "#update" do + let(:notification) { create(:notification) } + let(:request) { patch notification_path(notification), params: { "notification": { title: "Test Create", show_on_unauthenticated_pages: true } } } + + it "returns not found" do + request + expect(response).to have_http_status(:not_found) + end + end + end +end