diff --git a/app/controllers/admin/notifications_controller.rb b/app/controllers/admin/notifications_controller.rb index 66aff8043..b81152eb1 100644 --- a/app/controllers/admin/notifications_controller.rb +++ b/app/controllers/admin/notifications_controller.rb @@ -3,7 +3,7 @@ class Admin::NotificationsController < AdminController def index markdown_parser = Redcarpet::Markdown.new(CustomMarkdownRenderer) - @published_notification = Notification.published.first + @published_notification = Notification.currently_active.first if @published_notification @published_notification_message = markdown_parser.render(@published_notification[:notification_message]) end @@ -22,9 +22,7 @@ def show end def create - @notification = Notification.new(summary: notification_params[:summary], - notification_message: notification_params[:notification_message], - user: current_user['email'], published: true, published_at: Time.zone.now) + @notification = create_notifications Notification.transaction do if @notification.save flash[:success] = 'Notification created successfully.' @@ -55,6 +53,23 @@ def unpublish private def notification_params - params.require(:notification).permit(:summary, :notification_message) + params.require(:notification).permit(:summary, :notification_message, :stop_datetime) + end + + def create_notifications + Notification.new(summary: notification_params[:summary], + notification_message: notification_params[:notification_message], + user: current_user['email'], + published: true, + published_at: Time.zone.now, + stop_datetime: parsed_stop_datetime) + end + + def parsed_stop_datetime + return nil if notification_params[:stop_datetime].blank? + + Time.use_zone('Europe/London') do + Time.zone.parse(notification_params[:stop_datetime]) + end end end diff --git a/app/controllers/v1/notifications_controller.rb b/app/controllers/v1/notifications_controller.rb index 07ff88a6e..2ea5d980e 100644 --- a/app/controllers/v1/notifications_controller.rb +++ b/app/controllers/v1/notifications_controller.rb @@ -2,6 +2,7 @@ class V1::NotificationsController < ApiController def index + Notification.expire_past_due! markdown_parser = Redcarpet::Markdown.new(CustomMarkdownRenderer) notifications = Notification.published.first notifications[:notification_message] = markdown_parser.render(notifications[:notification_message]) if notifications diff --git a/app/models/notification.rb b/app/models/notification.rb index b24979d60..80941e2db 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,9 +1,14 @@ class Notification < ApplicationRecord validates :summary, :notification_message, presence: true + validate :stop_datetime_in_future before_save :ensure_single_published_notification scope :published, -> { where(published: true) } + scope :currently_active, lambda { + where(published: true) + .where('stop_datetime IS NULL OR stop_datetime > ?', Time.current) + } def unpublish! self.published = false @@ -11,6 +16,15 @@ def unpublish! save! end + def self.expire_past_due! + expired = where('stop_datetime < ? AND published = ?', Time.current, true) + # We use update_all for performance to expire records in a single + # SQL query, bypassing validations for speed on every API request. + # rubocop:disable Rails/SkipsModelValidations + expired.update_all(published: false, unpublished_at: Time.zone.now) + # rubocop:enable Rails/SkipsModelValidations + end + private def ensure_single_published_notification @@ -18,4 +32,10 @@ def ensure_single_published_notification Notification.where.not(id: id).where(published: true).find_each(&:unpublish!) end + + def stop_datetime_in_future + return unless stop_datetime.present? && stop_datetime < Time.current + + errors.add(:stop_datetime, 'must be in the future') + end end diff --git a/app/views/admin/notifications/index.html.haml b/app/views/admin/notifications/index.html.haml index d369a9178..7215311a3 100644 --- a/app/views/admin/notifications/index.html.haml +++ b/app/views/admin/notifications/index.html.haml @@ -36,9 +36,11 @@ %th.govuk-table__header Header %th.govuk-table__header Published %th.govuk-table__header Unpublished + %th.govuk-table__header Date time to Unpublish %tbody.govuk-table__body - @notifications.each do |notification| %tr.govuk-table__row %td.govuk-table__cell= link_to notification.summary, admin_notification_path(notification.id) %td.govuk-table__cell= notification.published_at %td.govuk-table__cell= notification.unpublished_at + %td.govuk-table__cell= notification.stop_datetime diff --git a/app/views/admin/notifications/new.html.haml b/app/views/admin/notifications/new.html.haml index 21b0652a5..e771a2cae 100644 --- a/app/views/admin/notifications/new.html.haml +++ b/app/views/admin/notifications/new.html.haml @@ -1,15 +1,39 @@ .govuk-grid-row .govuk-grid-column-two-thirds = link_to 'Back', admin_notifications_path, { class: 'govuk-back-link govuk-!-margin-bottom-5', title: 'Back to notifications' } - = simple_form_for [:admin, @notification] do |form| %fieldset.govuk-fieldset %legend.govuk-fieldset__legend.govuk-fieldset__legend--xl %h1.govuk-fieldset__heading Create a new notification - = form.input :summary, hide_optional: true, input_html: { value: @published_notification&.summary } - = form.input :notification_message, label: 'Notification message', hide_optional: true, wrapper: :govuk_textarea_wrapper, input_html: { rows: '10', value: @published_notification&.notification_message } + = form.input :summary, + hide_optional: true, + input_html: { value: @notification.summary || @published_notification&.summary } + + = form.input :notification_message, + label: 'Notification message', + hide_optional: true, + wrapper: :govuk_textarea_wrapper, + input_html: { rows: '10', value: @notification.notification_message || @published_notification&.notification_message } + + .govuk-form-group{class: ("govuk-form-group--error" if @notification.errors[:stop_datetime].any?)} + %label.govuk-label{ for: "notification_stop_datetime" } + Unpublish on + + - if @notification.errors[:stop_datetime].any? + %span.govuk-error-message + %span.govuk-visually-hidden Error: + = @notification.errors[:stop_datetime].join(', ') + + - current_date = @notification.stop_datetime || @published_notification&.stop_datetime + - current_date_london = current_date&.in_time_zone('Europe/London') + = form.datetime_local_field :stop_datetime, + class: "govuk-input", + value: current_date_london&.strftime("%Y-%m-%dT%H:%M") + %button.govuk-button.govuk-button--secondary{ type: "button", onclick: "document.getElementById('notification_stop_datetime').value = ''", class: "govuk-!-margin-top-1" } + Clear date + %button#markdown-preview-btn{type: "button", class: 'govuk-button'} Preview = form.button :submit, value: 'Publish Notification', data: { disable_with: "Publish Notification" } diff --git a/db/migrate/20260302_add_stop_datetime_to_notifications.rb b/db/migrate/20260302_add_stop_datetime_to_notifications.rb new file mode 100644 index 000000000..09d4e3a2b --- /dev/null +++ b/db/migrate/20260302_add_stop_datetime_to_notifications.rb @@ -0,0 +1,6 @@ +class AddStopDatetimeToNotifications < ActiveRecord::Migration[6.0] + def change + add_column :notifications, :stop_datetime, :datetime, null: true + add_index :notifications, :stop_datetime + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index d4bca3270..b1e9143f9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -161,10 +161,12 @@ t.text "notification_message" t.boolean "published", default: false t.datetime "published_at" + t.datetime "stop_datetime", precision: nil t.text "summary", null: false t.datetime "unpublished_at" t.string "user" t.index ["published"], name: "index_notifications_on_published" + t.index ["stop_datetime"], name: "index_notifications_on_stop_datetime" end create_table "release_notes", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/spec/factories/notifications.rb b/spec/factories/notifications.rb index 465cddb96..d4d85a3b6 100644 --- a/spec/factories/notifications.rb +++ b/spec/factories/notifications.rb @@ -4,6 +4,7 @@ notification_message { 'Wear sunscreen' } published { false } published_at { Time.zone.now } + stop_datetime { nil } unpublished_at { nil } user { 'testy.mctestface@example.com' } end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index c22154658..b9ada794e 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -5,6 +5,56 @@ subject { create(:notification, published: true) } it { is_expected.to validate_presence_of(:summary) } it { is_expected.to validate_presence_of(:notification_message) } + + it 'is invalid if stop_datetime is in the past' do + notification = build(:notification, stop_datetime: 1.day.ago) + expect(notification).not_to be_valid + expect(notification.errors[:stop_datetime]).to include('must be in the future') + end + + it 'is valid if stop_datetime is in the future' do + notification = build(:notification, stop_datetime: 1.day.from_now) + expect(notification).to be_valid + end + + it 'is valid if stop_datetime is nil' do + notification = build(:notification, stop_datetime: nil) + expect(notification).to be_valid + end + end + + describe 'scopes' do + describe '.currently_active' do + it 'includes published notifications with future stop dates' do + active = create(:notification, published: true, stop_datetime: 1.hour.from_now) + expect(Notification.currently_active).to include(active) + end + + it 'includes published notifications with no stop date' do + permanent = create(:notification, published: true, stop_datetime: nil) + expect(Notification.currently_active).to include(permanent) + end + + it 'excludes notifications that have passed their stop date' do + # Use save(validate: false) to simulate an old record that was valid when created + expired = build(:notification, published: true, stop_datetime: 1.hour.ago) + expired.save(validate: false) + expect(Notification.currently_active).not_to include(expired) + end + end + end + + describe '.expire_past_due!' do + it 'updates all past-due notifications to be unpublished' do + expired = build(:notification, published: true, stop_datetime: 1.minute.ago) + expired.save(validate: false) + + Notification.expire_past_due! + + expired.reload + expect(expired.published).to be false + expect(expired.unpublished_at).to be_within(1.second).of(Time.zone.now) + end end describe '#unpublish!' do diff --git a/spec/requests/admin/notifications_spec.rb b/spec/requests/admin/notifications_spec.rb index 7ac896f66..a4de6dbd8 100644 --- a/spec/requests/admin/notifications_spec.rb +++ b/spec/requests/admin/notifications_spec.rb @@ -9,6 +9,33 @@ get '/auth/google_oauth2/callback' end + describe '#create' do + around do |example| + travel_to(Time.utc(2026, 4, 20, 12, 0, 0)) { example.run } + end + + it 'stores stop_datetime as Europ/London local time converted to UTC' do + london_input = '2026-04-20T14:15:00' # 2:15 PM London time on April 20, 2026 + + expect do + post admin_notifications_path, params: { + notification: { + summary: 'Test Notification', + notification_message: 'This is a test notification.', + stop_datetime: london_input + } + } + end.to change(Notification, :count).by(1) + + notification = Notification.order(published_at: :desc).first + + expected_time = ActiveSupport::TimeZone['Europe/London'].parse(london_input) + + expect(notification.stop_datetime).to eq(expected_time) + expect(notification.stop_datetime.utc).to eq(expected_time.utc) + end + end + describe '#preview' do it 'renders the Markdown content as HTML' do markdown_content = '**Bold Text**' diff --git a/spec/requests/v1/notifications_spec.rb b/spec/requests/v1/notifications_spec.rb index d7094e711..6873367eb 100644 --- a/spec/requests/v1/notifications_spec.rb +++ b/spec/requests/v1/notifications_spec.rb @@ -3,6 +3,13 @@ RSpec.describe '/v1' do let(:user) { FactoryBot.create(:user) } + let(:auth_headers) do + { + 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials('dxw', 'sdfhg'), + 'X-Auth-Id' => JWT.encode(user.auth_id, 'test') + } + end + describe 'GET /v1/notifications' do it 'returns 401 if authentication needed and not provided' do ClimateControl.modify API_PASSWORD: 'sdfhg' do @@ -17,25 +24,22 @@ it 'returns ok if authentication needed and provided' do ClimateControl.modify API_PASSWORD: 'sdfhg' do - get '/v1/notifications', params: {}, headers: { - HTTP_AUTHORIZATION: ActionController::HttpAuthentication::Basic.encode_credentials('dxw', 'sdfhg'), - 'X-Auth-Id' => JWT.encode(user.auth_id, 'test') - } + get '/v1/notifications', params: {}, headers: auth_headers expect(response).to be_successful end end it 'returns the details of the current published notification' do FactoryBot.create(:notification, published: true, summary: 'Testy McTestface', -notification_message: 'The answer is 42') + notification_message: 'The answer is 42') - get '/v1/notifications', headers: { 'X-Auth-Id' => JWT.encode(user.auth_id, 'test') } + ClimateControl.modify API_PASSWORD: 'sdfhg' do + get '/v1/notifications', headers: auth_headers - expect(response).to be_successful - expect(json['data']) - .to have_attribute(:summary) - expect(json['data']) - .to have_attribute(:notification_message) + expect(response).to be_successful + expect(json['data']).to have_attribute(:summary).with_value('Testy McTestface') + expect(json['data']).to have_attribute(:notification_message) + end end end end