From 72a52a58c957641db6b2fa40c8f7dc4170125d78 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Wed, 18 Sep 2024 15:47:21 +0100 Subject: [PATCH 1/9] CLDC-3387: Allow support users to create notifications (#2613) * CLDC-3387: Allow support users to create notifications * Refactor rendering * Adjust notifications helper * Add rubocop ignore * Fix path checks around notification viewing * Notification path regex should work on review apps * Tidy * Remove unused function * Reference external url in link to markdown syntax guide * CLDC-3614: Allow markdown page content when creating notifications (#2622) * CLDC-3614: Allow markdown page content when creating notifications * Try using beginning of day for time in failing test * Remove ignored breaks * Add paper trail to notifications * CLDC-3607: Allow support users to delete notifications (by setting their end date) (#2630) * CLDC-3607: Allow support users to delete notifications (by setting their end date) * Refactor render_for_page helper * Move db queries to homepage presenter * Fix lint * Use a before_action to find notification in controller * Make delete notification links red * Tweak link to markdown guide * Open markdown guide in new tab --- Gemfile | 1 + Gemfile.lock | 1 + app/controllers/notifications_controller.rb | 90 +++++++++- app/helpers/application_helper.rb | 4 +- app/helpers/navigation_items_helper.rb | 2 +- app/helpers/notifications_helper.rb | 82 +++++++++ app/models/notification.rb | 21 ++- app/presenters/homepage_presenter.rb | 3 +- app/views/home/index.html.erb | 4 + app/views/notifications/_form.html.erb | 33 ++++ .../_notification_banner.html.erb | 8 +- .../_notification_home_section.html.erb | 17 ++ .../notifications/check_answers.html.erb | 61 +++++++ .../delete_confirmation.html.erb | 27 +++ app/views/notifications/edit.html.erb | 3 + app/views/notifications/new.html.erb | 3 + app/views/notifications/show.html.erb | 5 +- config/locales/en.yml | 15 +- config/routes.rb | 5 +- ...additional_page_column_to_notifications.rb | 5 + db/schema.rb | 15 +- spec/factories/notification.rb | 1 + ...ons_page_spec.rb => notifications_spec.rb} | 22 ++- spec/features/start_page_spec.rb | 7 +- .../lettings_log_derived_fields_spec.rb | 4 +- spec/models/notification_spec.rb | 37 ++++ .../requests/notifications_controller_spec.rb | 163 ++++++++++++++++++ 27 files changed, 592 insertions(+), 47 deletions(-) create mode 100644 app/views/notifications/_form.html.erb create mode 100644 app/views/notifications/_notification_home_section.html.erb create mode 100644 app/views/notifications/check_answers.html.erb create mode 100644 app/views/notifications/delete_confirmation.html.erb create mode 100644 app/views/notifications/edit.html.erb create mode 100644 app/views/notifications/new.html.erb create mode 100644 db/migrate/20240904135306_add_show_additional_page_column_to_notifications.rb rename spec/features/{notifications_page_spec.rb => notifications_spec.rb} (51%) create mode 100644 spec/models/notification_spec.rb create mode 100644 spec/requests/notifications_controller_spec.rb 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..d0181ead6 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -1,19 +1,93 @@ class NotificationsController < ApplicationController + before_action :authenticate_user!, except: %i[show] + before_action :authenticate_scope!, except: %i[show dismiss] + before_action :find_notification, except: %i[new create] + + rescue_from ActiveRecord::RecordNotFound, with: :render_not_found + 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.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 + if !@notification.show_on_unauthenticated_pages && !current_user + render_not_found and return + end + + if @notification.show_additional_page render "show" else redirect_back(fallback_location: root_path) end end + + def new + @notification = Notification.new + end + + def create + @notification = Notification.new(notification_model_params) + + if @notification.errors.empty? && @notification.save + redirect_to notification_check_answers_path(@notification) + else + render :new, status: :unprocessable_entity + end + end + + def update + start_now = params[:notification][:start_now] + + if @notification.errors.empty? && @notification.update(notification_model_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 + + def delete + @notification.update!(end_date: Time.zone.now) + flash[:notice] = "The notification has been deleted" + redirect_to root_path + end + +private + + def notification_params + params.require(:notification).permit(:title, :show_on_unauthenticated_pages, :show_additional_page, :link_text, :page_content, :start_now) + end + + def authenticate_scope! + render_not_found unless current_user.support? + end + + def notification_model_params + model_params = notification_params.except(:start_now) + + if notification_params[:show_additional_page] == "0" + model_params[:link_text] = nil + model_params[:page_content] = nil + end + + model_params[:start_date] = Time.zone.now if notification_params[:start_now] + + model_params + end + + def find_notification + id = params[:id] || params[:notification_id] + @notification = current_user&.support? ? Notification.find_by(id:) : Notification.active.find_by(id:) + + raise ActiveRecord::RecordNotFound unless @notification + + @notification + end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 01f7734c2..bb119b29e 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -12,7 +12,7 @@ module ApplicationHelper def govuk_header_classes(current_user) if current_user&.support? "app-header app-header--orange" - elsif ((current_user.blank? && Notification.active_unauthenticated_notifications.present?) || current_user&.active_unread_notifications.present?) && !current_page?(notifications_path) + elsif notifications_to_display? "app-header app-header__no-border-bottom" else "app-header" @@ -28,7 +28,7 @@ module ApplicationHelper end def notifications_to_display? - !current_page?(notifications_path) && (authenticated_user_has_notifications? || unauthenticated_user_has_notifications?) + !request.path.match?(/\/notifications\/\d+$/) && (authenticated_user_has_notifications? || unauthenticated_user_has_notifications?) end private diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index 1cecb97ec..b1a0c6a4c 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 == notifications_path + path == root_path || path.match?(/\/notifications\/\d+$/) end def lettings_logs_current?(path) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index ab4c8ec21..318918134 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -14,4 +14,86 @@ module NotificationsHelper Notification.newest_active_unauthenticated_notification end end + + def render_for_banner(title) + # rubocop:disable Rails/HelperInstanceVariable + @banner_renderer ||= NotificationRenderer.new({ invert_link_colour: true, bold_all_text: true }) + @banner_markdown ||= Redcarpet::Markdown.new(@banner_renderer, no_intra_emphasis: true) + @banner_markdown.render(title) + # rubocop:enable Rails/HelperInstanceVariable + end + + def render_for_summary(title) + render_normal_markdown(title) + end + + def render_for_page(notification) + content_includes_own_title = /\A\s*#[^#]/.match?(notification.page_content) + return render_normal_markdown(notification.page_content) if content_includes_own_title + + content = "# #{notification.title}\n#{notification.page_content}" + render_normal_markdown(content) + end + + def render_for_home(notification) + return render_normal_markdown(notification.title) unless notification.show_additional_page + + content = "#{notification.title} \n[#{notification.link_text}](#{notification_path(notification)})" + render_normal_markdown(content) + end + +private + + def render_normal_markdown(content) + # rubocop:disable Rails/HelperInstanceVariable + @on_page_renderer ||= NotificationRenderer.new({ invert_link_colour: false, bold_all_text: false }) + @on_page_markdown ||= Redcarpet::Markdown.new(@on_page_renderer, no_intra_emphasis: true) + @on_page_markdown.render(content) + # rubocop:enable Rails/HelperInstanceVariable + end +end + +class NotificationRenderer < 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] # rubocop:disable Rails/HelperInstanceVariable + base_options = { escape_html: true, safe_links_only: true, link_attributes: { class: link_class } } + super base_options + end + + def header(text, header_level) + header_size = case header_level + when 1 + "xl" + when 2 + "l" + when 3 + "m" + else + "s" + end + + %(#{text}) + end + + def paragraph(text) + return %(

#{text}

) if @bold # rubocop:disable Rails/HelperInstanceVariable + + %(

#{text}

) + end + + def list(contents, list_type) + return %(
    #{contents}
) if list_type == :ordered + + %() + end + + def hrule + %(
) + end + + def block_quote(quote) + %(
#{quote}
) + end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 86978ebea..cd7c9972a 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,7 +1,12 @@ class Notification < ApplicationRecord acts_as_readable - scope :active, -> { where("start_date <= ? AND end_date >= ?", Time.zone.now, Time.zone.now) } + has_paper_trail + + validates :title, presence: { message: I18n.t("activerecord.errors.models.notification.attributes.title.blank") } + validate :validate_additional_page_information + + 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 +16,18 @@ class Notification < ApplicationRecord def self.newest_active_unauthenticated_notification active_unauthenticated_notifications.last end + +private + + def validate_additional_page_information + return unless show_additional_page + + if link_text.blank? + errors.add :link_text, I18n.t("activerecord.errors.models.notification.attributes.link_text.blank_when_additional_page_set") + end + + if page_content.blank? + errors.add :page_content, I18n.t("activerecord.errors.models.notification.attributes.page_content.blank_when_additional_page_set") + end + end end diff --git a/app/presenters/homepage_presenter.rb b/app/presenters/homepage_presenter.rb index 1cb784e3a..3b3314667 100644 --- a/app/presenters/homepage_presenter.rb +++ b/app/presenters/homepage_presenter.rb @@ -2,7 +2,7 @@ class HomepagePresenter include Rails.application.routes.url_helpers include CollectionTimeHelper - attr_reader :current_year_in_progress_lettings_data, :current_year_completed_lettings_data, :current_year_in_progress_sales_data, :current_year_completed_sales_data, :last_year_in_progress_lettings_data, :last_year_completed_lettings_data, :last_year_in_progress_sales_data, :last_year_completed_sales_data, :incomplete_schemes_data + attr_reader :current_year_in_progress_lettings_data, :current_year_completed_lettings_data, :current_year_in_progress_sales_data, :current_year_completed_sales_data, :last_year_in_progress_lettings_data, :last_year_completed_lettings_data, :last_year_in_progress_sales_data, :last_year_completed_sales_data, :incomplete_schemes_data, :active_notifications def initialize(user) @user = user @@ -25,6 +25,7 @@ class HomepagePresenter path: schemes_path(status: [:incomplete], owning_organisation_select: "all"), } end + @active_notifications = Notification.active if @user.support? end def title_text_for_user diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb index 6995c28e5..e55dc6c99 100644 --- a/app/views/home/index.html.erb +++ b/app/views/home/index.html.erb @@ -6,6 +6,10 @@

<%= @homepage_presenter.title_text_for_user %>

+ <% if @current_user.support? %> + <%= render partial: "notifications/notification_home_section", locals: { active_notifications: @homepage_presenter.active_notifications } %> + <% 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..32af1adc0 --- /dev/null +++ b/app/views/notifications/_form.html.erb @@ -0,0 +1,33 @@ +
+
+ <%= 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 :display_options, 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" } %> + <%= f.govuk_check_box :show_additional_page, 1, 0, multiple: false, label: { text: "Include a link to a separate page with additional information" } do %> + <%= f.govuk_text_field :link_text, label: { text: "Link text" }, hint: { text: "Use descriptive language and relevant terms. The link text should make sense out of context." } %> + <%= f.govuk_text_area :page_content, label: { text: "Page content" }, hint: { text: "Use markdown to format the page content. The page title will be the notification title by default. Use a heading level one if you want to override it." } %> + <% end %> + <% end %> + + <%= govuk_link_to "Find out more about using Markdown at Markdown Guide", "https://www.markdownguide.org/basic-syntax/", new_tab: true %> + + <%= f.govuk_submit "Continue" %> +
+
diff --git a/app/views/notifications/_notification_banner.html.erb b/app/views/notifications/_notification_banner.html.erb index cd7dfffac..a9cc8bdff 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 %>

- <% if notification.page_content.present? %> + <%== render_for_banner(notification.title) %> + <% if notification.show_additional_page %>
- <%= 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/_notification_home_section.html.erb b/app/views/notifications/_notification_home_section.html.erb new file mode 100644 index 000000000..522bc2840 --- /dev/null +++ b/app/views/notifications/_notification_home_section.html.erb @@ -0,0 +1,17 @@ +
+

<%== I18n.t("active_notifications", count: active_notifications.count) %>

+ <% active_notifications.each do |notification| %> +
+
+ <%== render_for_home(notification) %> +
+
+ <%= govuk_link_to("Delete notification", notification_delete_confirmation_path(notification), class: "app-!-colour-red") %> +
+
+ <% end %> +
+ +
+ <%= govuk_button_link_to "Create a new notification", new_notification_path %> +
diff --git a/app/views/notifications/check_answers.html.erb b/app/views/notifications/check_answers.html.erb new file mode 100644 index 000000000..8ddbc0abe --- /dev/null +++ b/app/views/notifications/check_answers.html.erb @@ -0,0 +1,61 @@ +<% 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 %> + <%== render_for_summary(@notification.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 %> + <% summary_list.with_row do |row| %> + <% row.with_key { "Link to additional information?" } %> + <% row.with_value { @notification.show_additional_page ? "Yes" : "No" } %> + <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %> + <% end %> + <% if @notification.show_additional_page %> + <% summary_list.with_row do |row| %> + <% row.with_key { "Link text" } %> + <% row.with_value { @notification.link_text } %> + <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %> + <% end %> + <% summary_list.with_row do |row| %> + <% row.with_key { "Page content" } %> + <% row.with_value do %> + <%= govuk_link_to "View page preview", notification_path(@notification), new_tab: true %> + <% end %> + <% row.with_action(text: "Change", href: edit_notification_path(@notification)) %> + <% end %> + <% end %> + <% end %> +
+
+ + <%= f.hidden_field :start_now, value: true %> + <%= f.govuk_submit "Create notification" %> + <% end %> +
+
diff --git a/app/views/notifications/delete_confirmation.html.erb b/app/views/notifications/delete_confirmation.html.erb new file mode 100644 index 000000000..4107c3050 --- /dev/null +++ b/app/views/notifications/delete_confirmation.html.erb @@ -0,0 +1,27 @@ +<% content_for :before_content do %> + <% content_for :title, "Are you sure you want to delete this notification?" %> + <%= govuk_back_link(href: :back) %> +<% end %> + +
+
+

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

+ +

<%== render_for_summary("**Notification:** #{@notification.title}") %>

+ +

Users will no longer see this notification.

+ + <%= govuk_warning_text(text: "You will not be able to undo this action.") %> + +
+ <%= govuk_button_to( + "Delete notification", + notification_delete_path(@notification), + method: :delete, + ) %> + <%= govuk_button_link_to "Cancel", root_path, html: { method: :get }, secondary: true %> +
+
+
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/app/views/notifications/show.html.erb b/app/views/notifications/show.html.erb index abdc77044..a3933db55 100644 --- a/app/views/notifications/show.html.erb +++ b/app/views/notifications/show.html.erb @@ -5,10 +5,7 @@
-

<%= @notification.title %>

-

- <%= sanitize @notification.page_content %> -

+ <%== render_for_page(@notification) %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 42ef43be1..c8c5cffc4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -41,6 +41,11 @@ en: create_password: "Create a password to finish setting up your account" reset_password: "Reset your password" + active_notifications: + zero: "There are no active notifications" + one: "There is one active notification:" + other: "There are %{count} active notifications:" + activemodel: errors: models: @@ -180,11 +185,19 @@ en: attributes: absorbing_organisation_id: blank: "Select the absorbing organisation" - merge_date: + merge_date: blank: "Enter a merge date" invalid: "Enter a valid merge date" existing_absorbing_organisation: blank: "You must answer absorbing organisation already active?" + notification: + attributes: + title: + blank: "Enter a title" + link_text: + blank_when_additional_page_set: "Enter the link text" + page_content: + blank_when_additional_page_set: "Enter the page content" notification: logs_deleted: diff --git a/config/routes.rb b/config/routes.rb index f2c86e37c..77b862e5a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -142,8 +142,11 @@ 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" + get "delete-confirmation", to: "notifications#delete_confirmation" + delete "delete", to: "notifications#delete" end resources :organisations do diff --git a/db/migrate/20240904135306_add_show_additional_page_column_to_notifications.rb b/db/migrate/20240904135306_add_show_additional_page_column_to_notifications.rb new file mode 100644 index 000000000..c0992bb62 --- /dev/null +++ b/db/migrate/20240904135306_add_show_additional_page_column_to_notifications.rb @@ -0,0 +1,5 @@ +class AddShowAdditionalPageColumnToNotifications < ActiveRecord::Migration[7.0] + def change + add_column :notifications, :show_additional_page, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 2d6808e6f..80463eaad 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_09_11_152702) do +ActiveRecord::Schema[7.0].define(version: 2024_09_18_112702) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -55,7 +55,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_11_152702) do t.datetime "last_accessed" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying::text, 'sales'::character varying::text])", name: "log_type_check" + t.check_constraint "log_type::text = ANY (ARRAY['lettings'::character varying, 'sales'::character varying]::text[])", name: "log_type_check" t.check_constraint "year >= 2000 AND year <= 2099", name: "year_check" end @@ -206,14 +206,14 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_11_152702) do t.integer "hb" t.integer "hbrentshortfall" t.integer "property_relet" - t.datetime "mrcdate", precision: nil + t.datetime "mrcdate" t.integer "incref" - t.datetime "startdate", precision: nil + t.datetime "startdate" t.integer "armedforces" t.integer "first_time_property_let_as_social_housing" t.integer "unitletas" t.integer "builtype" - t.datetime "voiddate", precision: nil + t.datetime "voiddate" t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" t.integer "renttype" @@ -459,6 +459,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_11_152702) do t.boolean "show_on_unauthenticated_pages" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "show_additional_page" end create_table "organisation_relationships", force: :cascade do |t| @@ -779,8 +780,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_11_152702) do t.string "name" t.bigint "organisation_id" t.integer "sign_in_count", default: 0, null: false - t.datetime "current_sign_in_at", precision: nil - t.datetime "last_sign_in_at", precision: nil + t.datetime "current_sign_in_at" + t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" t.integer "role" diff --git a/spec/factories/notification.rb b/spec/factories/notification.rb index 05d4faa3f..92d4bf4c9 100644 --- a/spec/factories/notification.rb +++ b/spec/factories/notification.rb @@ -6,5 +6,6 @@ FactoryBot.define do start_date { Time.zone.yesterday } end_date { Time.zone.tomorrow } show_on_unauthenticated_pages { false } + show_additional_page { true } end end 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/models/lettings_log_derived_fields_spec.rb b/spec/models/lettings_log_derived_fields_spec.rb index 54dc438bc..aae1396d7 100644 --- a/spec/models/lettings_log_derived_fields_spec.rb +++ b/spec/models/lettings_log_derived_fields_spec.rb @@ -981,9 +981,9 @@ RSpec.describe LettingsLog, type: :model do end describe "deriving voiddate from startdate" do - let(:startdate) { Time.zone.now } + let(:startdate) { Time.zone.now.beginning_of_day } - it "correctly derives voiddate if the letting is a renewal and clears it if it is not" do + it "correctly derives voiddate if the letting is a renewal" do log.assign_attributes(renewal: 1, startdate:) expect { log.set_derived_fields! }.to change(log, :voiddate).to startdate diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb new file mode 100644 index 000000000..86c02476f --- /dev/null +++ b/spec/models/notification_spec.rb @@ -0,0 +1,37 @@ +require "rails_helper" + +RSpec.describe Notification, type: :model do + describe "#valid?" do + context "when show additional page is true" do + context "and page_content is blank" do + let(:notification) { build(:notification, show_additional_page: true, page_content: "") } + + it "adds an error to page_content" do + notification.valid? + + expect(notification.errors[:page_content]).to include("Enter the page content") + end + end + + context "and link_text is blank" do + let(:notification) { build(:notification, show_additional_page: true, link_text: nil) } + + it "adds an error to link_text" do + notification.valid? + + expect(notification.errors[:link_text]).to include("Enter the link text") + end + end + end + + context "when show additional page is false" do + context "and page_content and link_text are blank" do + let(:notification) { build(:notification, show_additional_page: false, link_text: nil, page_content: nil) } + + it "is valid" do + expect(notification).to be_valid + end + end + end + end +end diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb new file mode 100644 index 000000000..0982057ab --- /dev/null +++ b/spec/requests/notifications_controller_spec.rb @@ -0,0 +1,163 @@ +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: "1", show_additional_page: "1", link_text: "link", page_content: "page" } } } + + 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(true) + expect(notification.show_additional_page).to be(true) + expect(notification.link_text).to eq("link") + expect(notification.page_content).to eq("page") + 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: "1" } } } + + it "gives an error response" do + request + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context "when show additional page is false" do + let(:params) { { "notification": { title: "No Additional Page", show_on_unauthenticated_pages: "1", show_additional_page: "0", link_text: "text", page_content: "content" } } } + + it "ignores values for link_text and page_content" do + request + notification = Notification.find_by(title: "No Additional Page") + expect(notification.link_text).to be_nil + expect(notification.page_content).to be_nil + 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: "1" } } } + + 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 + + context "when show additional page is false" do + let(:notification) { create(:notification, show_additional_page: "0", link_text: "link", page_content: "page") } + let(:params) { { "notification": { show_additional_page: "0", link_text: "text", page_content: "content" } } } + + it "removes values for link_text and page_content" do + request + notification.reload + expect(notification.link_text).to be_nil + expect(notification.page_content).to be_nil + end + end + end + + describe "#delete" do + let(:notification) { create(:notification, end_date: nil) } + let(:request) { delete notification_delete_path(notification) } + + it "sets end_date on the notification" do + request + notification.reload + expect(notification.end_date).to be < Time.zone.now + 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" } } } + + 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 Update" } } } + + it "returns not found" do + request + expect(response).to have_http_status(:not_found) + end + end + + describe "#delete" do + let(:notification) { create(:notification, end_date: nil) } + let(:request) { delete notification_delete_path(notification) } + + it "returns not found" do + request + expect(response).to have_http_status(:not_found) + end + + it "does not set end_date on the notification" do + request + notification.reload + expect(notification.end_date).to be_nil + end + end + end +end From 380d63c3671f564a2953afa99c801e3146a0039e Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Wed, 18 Sep 2024 16:04:10 +0100 Subject: [PATCH 2/9] CLDC-3244: Replace postcodes_io gem to improve error handling (#2634) * CLDC-3244: Replace postcodes_io gem to improve error handling * Fix status code in default postcodes api mocking * Fix more mocking * Remove blank line * Handle non-Excon errors, in case of unexpected api behaviour * Remove outdated test * Remove another outdated test --- Gemfile | 3 +- Gemfile.lock | 6 +- app/services/postcode_service.rb | 24 +++--- ...ate_schemes_and_locations_from_csv_spec.rb | 2 +- spec/models/lettings_log_spec.rb | 11 --- spec/models/sales_log_spec.rb | 11 --- spec/request_helper.rb | 2 +- spec/services/postcode_service_spec.rb | 77 ++++++++++++++++++- 8 files changed, 91 insertions(+), 45 deletions(-) diff --git a/Gemfile b/Gemfile index 4dc93aaf7..a5c7dd01f 100644 --- a/Gemfile +++ b/Gemfile @@ -39,7 +39,6 @@ gem "devise_two_factor_authentication" # UK postcode parsing and validation gem "uk_postcode" # Get rich data from postcode lookups. Wraps postcodes.io -gem "postcodes_io" # Use Ruby objects to build reusable markup. A React inspired evolution of the presenter pattern gem "view_component", "~> 3.9" # Use the AWS S3 SDK as storage mechanism @@ -112,3 +111,5 @@ end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem "cssbundling-rails" gem "tzinfo-data", platforms: %i[mingw mswin x64_mingw jruby] + +gem "excon", "~> 0.111.0" diff --git a/Gemfile.lock b/Gemfile.lock index 7cc2addda..cdf923ae0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -183,7 +183,7 @@ GEM et-orbi (1.2.11) tzinfo event_stream_parser (1.0.0) - excon (0.109.0) + excon (0.111.0) factory_bot (6.4.6) activesupport (>= 5.0.0) factory_bot_rails (6.4.3) @@ -302,8 +302,6 @@ GEM racc pg (1.5.5) possessive (1.0.1) - postcodes_io (0.4.0) - excon (~> 0.39) propshaft (0.8.0) actionpack (>= 7.0.0) activesupport (>= 7.0.0) @@ -533,6 +531,7 @@ DEPENDENCIES devise_two_factor_authentication dotenv-rails erb_lint + excon (~> 0.111.0) factory_bot_rails faker govuk-components (~> 5.1) @@ -549,7 +548,6 @@ DEPENDENCIES parallel_tests pg (~> 1.1) possessive - postcodes_io propshaft pry-byebug puma (~> 5.6) diff --git a/app/services/postcode_service.rb b/app/services/postcode_service.rb index c3c6dcfdd..8cfe9c43c 100644 --- a/app/services/postcode_service.rb +++ b/app/services/postcode_service.rb @@ -1,26 +1,24 @@ class PostcodeService - def initialize - @pio = Postcodes::IO.new - end - def lookup(postcode) # Avoid network calls when postcode is invalid return unless postcode.match(POSTCODE_REGEXP) - postcode_lookup = nil + result = nil begin # URI encoding only supports ASCII characters ascii_postcode = self.class.clean(postcode) - Timeout.timeout(30) { postcode_lookup = @pio.lookup(ascii_postcode) } - rescue Timeout::Error - Rails.logger.warn("Postcodes.io lookup timed out") - end - if postcode_lookup && postcode_lookup.info.present? - { - location_code: postcode_lookup.codes["admin_district"], - location_admin_district: postcode_lookup&.admin_district, + response = Excon.get("https://api.postcodes.io/postcodes/#{ascii_postcode}", idempotent: true, timeout: 30, expects: [200]) + parsed_response = JSON.parse(response.body) + result = { + location_code: parsed_response["result"]["codes"]["admin_district"], + location_admin_district: parsed_response["result"]["admin_district"], } + rescue Excon::Error => e + Rails.logger.warn("Postcode lookup request was not successful: #{e} #{e.response.body}") + rescue StandardError => e + Rails.logger.error("Unexpected error with postcode lookup request: #{e}") end + result end def self.clean(postcode) diff --git a/spec/lib/tasks/update_schemes_and_locations_from_csv_spec.rb b/spec/lib/tasks/update_schemes_and_locations_from_csv_spec.rb index 0d61b08f1..818a2b589 100644 --- a/spec/lib/tasks/update_schemes_and_locations_from_csv_spec.rb +++ b/spec/lib/tasks/update_schemes_and_locations_from_csv_spec.rb @@ -25,7 +25,7 @@ RSpec.describe "bulk_update" do allow(ENV).to receive(:[]).with("BULK_UPLOAD_BUCKET").and_return(instance_name) WebMock.stub_request(:get, /api\.postcodes\.io/) - .to_return(status: 200, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) + .to_return(status: 404, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) WebMock.stub_request(:get, /api\.postcodes\.io\/postcodes\/B11BB/) .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E09000033"}}}', headers: {}) end diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 2001c0bfa..92452f197 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -329,17 +329,6 @@ RSpec.describe LettingsLog do .to raise_error(ActiveRecord::RecordInvalid, /#{I18n.t("validations.postcode")}/) end - context "when the local authority lookup times out" do - before do - allow(Timeout).to receive(:timeout).and_raise(Timeout::Error) - end - - it "logs a warning" do - expect(Rails.logger).to receive(:warn).with("Postcodes.io lookup timed out") - address_lettings_log.update!({ postcode_known: 1, postcode_full: "M1 1AD" }) - end - end - it "correctly resets all fields if property postcode not known" do address_lettings_log.update!({ postcode_known: 0 }) diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index ddc696be4..a568ca330 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -628,17 +628,6 @@ RSpec.describe SalesLog, type: :model do .to raise_error(ActiveRecord::RecordInvalid, /#{I18n.t("validations.postcode")}/) end - context "when the local authority lookup times out" do - before do - allow(Timeout).to receive(:timeout).and_raise(Timeout::Error) - end - - it "logs a warning" do - expect(Rails.logger).to receive(:warn).with("Postcodes.io lookup timed out") - address_sales_log.update!({ pcodenk: 1, postcode_full: "M1 1AD" }) - end - end - it "correctly resets all fields if property postcode not known" do address_sales_log.update!({ pcodenk: 1 }) diff --git a/spec/request_helper.rb b/spec/request_helper.rb index 15d178218..f1f208ec6 100644 --- a/spec/request_helper.rb +++ b/spec/request_helper.rb @@ -4,7 +4,7 @@ module RequestHelper def self.stub_http_requests WebMock.disable_net_connect!(allow_localhost: true) WebMock.stub_request(:get, /api\.postcodes\.io/) - .to_return(status: 200, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) + .to_return(status: 404, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/AA11AA") .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"AA1 1AA\",\"admin_district\":\"Westminster\",\"codes\":{\"admin_district\":\"E09000033\"}}}", headers: {}) diff --git a/spec/services/postcode_service_spec.rb b/spec/services/postcode_service_spec.rb index ecf20fdac..abc61129f 100644 --- a/spec/services/postcode_service_spec.rb +++ b/spec/services/postcode_service_spec.rb @@ -1,9 +1,80 @@ require "rails_helper" describe PostcodeService do - let(:postcode) { "s r81LS\u00A0" } + let(:service) { described_class.new } - it "returns clean postcode" do - expect(described_class.clean(postcode)).to eq "SR81LS" + describe "clean" do + let(:postcode) { "s r81LS\u00A0" } + + it "returns clean postcode" do + expect(described_class.clean(postcode)).to eq "SR81LS" + end + end + + describe "lookup" do + before do + Excon.defaults[:mock] = true + Excon.defaults[:stubs] = :local + end + + context "when the request returns a success response" do + before do + Excon.stub({}, { body: '{"result": { "admin_district": "District", "codes": { "admin_district": "123" } } }', status: 200 }) + end + + it "returns the admin district and admin district code" do + result = service.lookup("A00 0AA") + expect(result[:location_code]).to eq("123") + expect(result[:location_admin_district]).to eq("District") + end + end + + context "when the request returns a not found response" do + before do + Excon.stub({}, { status: 404 }) + end + + it "returns nil" do + result = service.lookup("A00 0AA") + expect(result).to be_nil + end + + it "logs the error at warning level" do + expect(Rails.logger).to receive(:warn).with(match "404 Not Found") + service.lookup("A00 0AA") + end + end + + context "when the request returns an error response" do + before do + Excon.stub({}, { body: "This is an error message that is not valid json", status: 500 }) + end + + it "returns nil" do + result = service.lookup("A00 0AA") + expect(result).to be_nil + end + + it "logs the error at warning level" do + expect(Rails.logger).to receive(:warn).with(match "This is an error message that is not valid json") + service.lookup("A00 0AA") + end + end + + context "when the request returns a success response that causes later errors" do + before do + Excon.stub({}, { body: '{"result": { "admin_district": "District" } }', status: 200 }) + end + + it "returns nil" do + result = service.lookup("A00 0AA") + expect(result).to be_nil + end + + it "logs the error at error level" do + expect(Rails.logger).to receive(:error).with(match "Unexpected error with postcode lookup request") + service.lookup("A00 0AA") + end + end end end From d5f242424796b771b50fef5b6cc5f6748a7d1ec6 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 18 Sep 2024 16:55:29 +0100 Subject: [PATCH 3/9] CLDC-3576 Update soft pregnancy validation (#2640) * Update soft pregnancy validation * Fix tests --- ...pregnant_household_lead_age_value_check.rb | 4 +-- ...gnant_household_lead_hhmemb_value_check.rb | 4 +-- ...les_pregnant_household_lead_value_check.rb | 4 +-- ...egnant_household_person_age_value_check.rb | 4 +-- ...s_pregnant_household_person_value_check.rb | 4 +-- ..._females_pregnant_household_value_check.rb | 4 +-- app/models/validations/soft_validations.rb | 12 +++++-- config/forms/2021_2022.json | 36 +++++++++---------- config/forms/2022_2023.json | 36 +++++++++---------- config/locales/en.yml | 2 +- ...t_household_person_age_value_check_spec.rb | 8 ++--- ...gnant_household_person_value_check_spec.rb | 8 ++--- .../validations/soft_validations_spec.rb | 12 +++---- 13 files changed, 73 insertions(+), 65 deletions(-) diff --git a/app/models/form/lettings/pages/no_females_pregnant_household_lead_age_value_check.rb b/app/models/form/lettings/pages/no_females_pregnant_household_lead_age_value_check.rb index 99d7e6bfa..68b1c7f09 100644 --- a/app/models/form/lettings/pages/no_females_pregnant_household_lead_age_value_check.rb +++ b/app/models/form/lettings/pages/no_females_pregnant_household_lead_age_value_check.rb @@ -2,13 +2,13 @@ class Form::Lettings::Pages::NoFemalesPregnantHouseholdLeadAgeValueCheck < ::For def initialize(id, hsh, subsection) super(id, hsh, subsection) @id = "no_females_pregnant_household_lead_age_value_check" - @depends_on = [{ "no_females_in_a_pregnant_household?" => true }] + @depends_on = [{ "all_male_tenants_in_a_pregnant_household?" => true }] @title_text = { "translation" => "soft_validations.pregnancy.title", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @informative_text = { - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } end diff --git a/app/models/form/lettings/pages/no_females_pregnant_household_lead_hhmemb_value_check.rb b/app/models/form/lettings/pages/no_females_pregnant_household_lead_hhmemb_value_check.rb index c39fecbc3..e1d1235ef 100644 --- a/app/models/form/lettings/pages/no_females_pregnant_household_lead_hhmemb_value_check.rb +++ b/app/models/form/lettings/pages/no_females_pregnant_household_lead_hhmemb_value_check.rb @@ -2,13 +2,13 @@ class Form::Lettings::Pages::NoFemalesPregnantHouseholdLeadHhmembValueCheck < :: def initialize(id, hsh, subsection) super @id = "no_females_pregnant_household_lead_hhmemb_value_check" - @depends_on = [{ "no_females_in_a_pregnant_household?" => true }] + @depends_on = [{ "all_male_tenants_in_a_pregnant_household?" => true }] @title_text = { "translation" => "soft_validations.pregnancy.title", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @informative_text = { - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } end diff --git a/app/models/form/lettings/pages/no_females_pregnant_household_lead_value_check.rb b/app/models/form/lettings/pages/no_females_pregnant_household_lead_value_check.rb index b90831b4b..416845735 100644 --- a/app/models/form/lettings/pages/no_females_pregnant_household_lead_value_check.rb +++ b/app/models/form/lettings/pages/no_females_pregnant_household_lead_value_check.rb @@ -2,13 +2,13 @@ class Form::Lettings::Pages::NoFemalesPregnantHouseholdLeadValueCheck < ::Form:: def initialize(id, hsh, subsection) super @id = "no_females_pregnant_household_lead_value_check" - @depends_on = [{ "no_females_in_a_pregnant_household?" => true }] + @depends_on = [{ "all_male_tenants_in_a_pregnant_household?" => true }] @title_text = { "translation" => "soft_validations.pregnancy.title", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @informative_text = { - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } end diff --git a/app/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check.rb b/app/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check.rb index 13c39ab11..12db3a08e 100644 --- a/app/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check.rb +++ b/app/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check.rb @@ -2,13 +2,13 @@ class Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonAgeValueCheck < ::F def initialize(id, hsh, subsection, person_index:) super(id, hsh, subsection) @id = "no_females_pregnant_household_person_#{person_index}_age_value_check" - @depends_on = [{ "no_females_in_a_pregnant_household?" => true, "age#{person_index}_known" => 0 }] + @depends_on = [{ "all_male_tenants_in_a_pregnant_household?" => true, "age#{person_index}_known" => 0 }] @title_text = { "translation" => "soft_validations.pregnancy.title", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @informative_text = { - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @person_index = person_index diff --git a/app/models/form/lettings/pages/no_females_pregnant_household_person_value_check.rb b/app/models/form/lettings/pages/no_females_pregnant_household_person_value_check.rb index 0f5e93021..f04333ec0 100644 --- a/app/models/form/lettings/pages/no_females_pregnant_household_person_value_check.rb +++ b/app/models/form/lettings/pages/no_females_pregnant_household_person_value_check.rb @@ -2,13 +2,13 @@ class Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonValueCheck < ::Form def initialize(id, hsh, subsection, person_index:) super(id, hsh, subsection) @id = "no_females_pregnant_household_person_#{person_index}_value_check" - @depends_on = [{ "no_females_in_a_pregnant_household?" => true, "details_known_#{person_index}" => 0 }] + @depends_on = [{ "all_male_tenants_in_a_pregnant_household?" => true, "details_known_#{person_index}" => 0 }] @title_text = { "translation" => "soft_validations.pregnancy.title", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @informative_text = { - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @person_index = person_index diff --git a/app/models/form/lettings/pages/no_females_pregnant_household_value_check.rb b/app/models/form/lettings/pages/no_females_pregnant_household_value_check.rb index bf4494135..9c84a953f 100644 --- a/app/models/form/lettings/pages/no_females_pregnant_household_value_check.rb +++ b/app/models/form/lettings/pages/no_females_pregnant_household_value_check.rb @@ -2,13 +2,13 @@ class Form::Lettings::Pages::NoFemalesPregnantHouseholdValueCheck < ::Form::Page def initialize(id, hsh, subsection) super @id = "no_females_pregnant_household_value_check" - @depends_on = [{ "no_females_in_a_pregnant_household?" => true }] + @depends_on = [{ "all_male_tenants_in_a_pregnant_household?" => true }] @title_text = { "translation" => "soft_validations.pregnancy.title", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } @informative_text = { - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [{ "key" => "sex1", "label" => true, "i18n_template" => "sex1" }], } end diff --git a/app/models/validations/soft_validations.rb b/app/models/validations/soft_validations.rb index 5f1ac9c7a..41309cef0 100644 --- a/app/models/validations/soft_validations.rb +++ b/app/models/validations/soft_validations.rb @@ -67,8 +67,8 @@ module Validations::SoftValidations end end - def no_females_in_a_pregnant_household? - !females_in_the_household? && all_tenants_gender_information_completed? && preg_occ == 1 + def all_male_tenants_in_a_pregnant_household? + all_male_tenants_in_the_household? && all_tenants_gender_information_completed? && preg_occ == 1 end def female_in_pregnant_household_in_soft_validation_range? @@ -226,6 +226,14 @@ private end end + def all_male_tenants_in_the_household? + person_count = hhmemb || 8 + + (1..person_count).all? do |n| + public_send("sex#{n}") == "M" + end + end + def tenant_is_retired?(economic_status) economic_status == 5 end diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index 4033c2534..6ebc090b6 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -1170,7 +1170,7 @@ "no_females_pregnant_household_lead_hhmemb_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { @@ -1312,7 +1312,7 @@ "no_females_pregnant_household_lead_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { @@ -1433,7 +1433,7 @@ "no_females_pregnant_household_lead_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { @@ -2091,7 +2091,7 @@ "no_females_pregnant_household_person_2_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age2_known": 0 } ], @@ -2214,7 +2214,7 @@ "no_females_pregnant_household_person_2_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_2": 0 } ], @@ -2626,7 +2626,7 @@ "no_females_pregnant_household_person_3_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age3_known": 0 } ], @@ -2749,7 +2749,7 @@ "no_females_pregnant_household_person_3_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_3": 0 } ], @@ -3158,7 +3158,7 @@ "no_females_pregnant_household_person_4_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age4_known": 0 } ], @@ -3281,7 +3281,7 @@ "no_females_pregnant_household_person_4_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_4": 0 } ], @@ -3687,7 +3687,7 @@ "no_females_pregnant_household_person_5_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age5_known": 0 } ], @@ -3810,7 +3810,7 @@ "no_females_pregnant_household_person_5_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_5": 0 } ], @@ -4213,7 +4213,7 @@ "no_females_pregnant_household_person_6_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age6_known": 0 } ], @@ -4336,7 +4336,7 @@ "no_females_pregnant_household_person_6_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_6": 0 } ], @@ -4736,7 +4736,7 @@ "no_females_pregnant_household_person_7_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age7_known": 0 } ], @@ -4859,7 +4859,7 @@ "no_females_pregnant_household_person_7_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_7": 0 } ], @@ -5256,7 +5256,7 @@ "no_females_pregnant_household_person_8_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age8_known": 0 } ], @@ -5379,7 +5379,7 @@ "no_females_pregnant_household_person_8_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_8": 0 } ], @@ -5794,7 +5794,7 @@ "no_females_pregnant_household_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json index 6065653fd..54cd67fd4 100644 --- a/config/forms/2022_2023.json +++ b/config/forms/2022_2023.json @@ -1200,7 +1200,7 @@ "no_females_pregnant_household_lead_hhmemb_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { @@ -1372,7 +1372,7 @@ "no_females_pregnant_household_lead_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { @@ -1523,7 +1523,7 @@ "no_females_pregnant_household_lead_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { @@ -2163,7 +2163,7 @@ "no_females_pregnant_household_person_2_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age2_known": 0 } ], @@ -2316,7 +2316,7 @@ "no_females_pregnant_household_person_2_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_2": 0 } ], @@ -2746,7 +2746,7 @@ "no_females_pregnant_household_person_3_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age3_known": 0 } ], @@ -2899,7 +2899,7 @@ "no_females_pregnant_household_person_3_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_3": 0 } ], @@ -3326,7 +3326,7 @@ "no_females_pregnant_household_person_4_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age4_known": 0 } ], @@ -3479,7 +3479,7 @@ "no_females_pregnant_household_person_4_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_4": 0 } ], @@ -3903,7 +3903,7 @@ "no_females_pregnant_household_person_5_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age5_known": 0 } ], @@ -4056,7 +4056,7 @@ "no_females_pregnant_household_person_5_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_5": 0 } ], @@ -4477,7 +4477,7 @@ "no_females_pregnant_household_person_6_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age6_known": 0 } ], @@ -4630,7 +4630,7 @@ "no_females_pregnant_household_person_6_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_6": 0 } ], @@ -5048,7 +5048,7 @@ "no_females_pregnant_household_person_7_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age7_known": 0 } ], @@ -5201,7 +5201,7 @@ "no_females_pregnant_household_person_7_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_7": 0 } ], @@ -5616,7 +5616,7 @@ "no_females_pregnant_household_person_8_age_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "age8_known": 0 } ], @@ -5769,7 +5769,7 @@ "no_females_pregnant_household_person_8_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true, + "all_male_tenants_in_a_pregnant_household?": true, "details_known_8": 0 } ], @@ -6205,7 +6205,7 @@ "no_females_pregnant_household_value_check": { "depends_on": [ { - "no_females_in_a_pregnant_household?": true + "all_male_tenants_in_a_pregnant_household?": true } ], "title_text": { diff --git a/config/locales/en.yml b/config/locales/en.yml index c8c5cffc4..c666746da 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -723,7 +723,7 @@ Make sure these answers are correct." hint_text: "This is higher than the purchase price minus the discount." pregnancy: title: "You told us somebody in the household is pregnant" - no_females: "You also told us there are no female tenants living at the property." + all_male_tenants: "You also told us that all the tenants living at the property are male." females_not_in_soft_age_range: "You also told us that any female tenants living at the property are in the following age ranges:" major_repairs_date: title_text: "You told us the property has been vacant for 2 years." diff --git a/spec/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check_spec.rb b/spec/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check_spec.rb index 06f17b190..d62fccc13 100644 --- a/spec/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check_spec.rb +++ b/spec/models/form/lettings/pages/no_females_pregnant_household_person_age_value_check_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonAgeValueCh [ { "age2_known" => 0, - "no_females_in_a_pregnant_household?" => true, + "all_male_tenants_in_a_pregnant_household?" => true, }, ], ) @@ -54,7 +54,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonAgeValueCh it "has the correct informative_text" do expect(page.informative_text).to eq({ - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [ { "key" => "sex1", @@ -78,7 +78,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonAgeValueCh [ { "age3_known" => 0, - "no_females_in_a_pregnant_household?" => true, + "all_male_tenants_in_a_pregnant_household?" => true, }, ], ) @@ -99,7 +99,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonAgeValueCh it "has the correct informative_text" do expect(page.informative_text).to eq({ - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [ { "key" => "sex1", diff --git a/spec/models/form/lettings/pages/no_females_pregnant_household_person_value_check_spec.rb b/spec/models/form/lettings/pages/no_females_pregnant_household_person_value_check_spec.rb index 4be9a5ef2..dca600e0f 100644 --- a/spec/models/form/lettings/pages/no_females_pregnant_household_person_value_check_spec.rb +++ b/spec/models/form/lettings/pages/no_females_pregnant_household_person_value_check_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonValueCheck [ { "details_known_2" => 0, - "no_females_in_a_pregnant_household?" => true, + "all_male_tenants_in_a_pregnant_household?" => true, }, ], ) @@ -54,7 +54,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonValueCheck it "has the correct informative_text" do expect(page.informative_text).to eq({ - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [ { "key" => "sex1", @@ -78,7 +78,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonValueCheck [ { "details_known_3" => 0, - "no_females_in_a_pregnant_household?" => true, + "all_male_tenants_in_a_pregnant_household?" => true, }, ], ) @@ -99,7 +99,7 @@ RSpec.describe Form::Lettings::Pages::NoFemalesPregnantHouseholdPersonValueCheck it "has the correct informative_text" do expect(page.informative_text).to eq({ - "translation" => "soft_validations.pregnancy.no_females", + "translation" => "soft_validations.pregnancy.all_male_tenants", "arguments" => [ { "key" => "sex1", diff --git a/spec/models/validations/soft_validations_spec.rb b/spec/models/validations/soft_validations_spec.rb index 6a3d9e051..8c7380fde 100644 --- a/spec/models/validations/soft_validations_spec.rb +++ b/spec/models/validations/soft_validations_spec.rb @@ -153,24 +153,24 @@ RSpec.describe Validations::SoftValidations do end describe "pregnancy soft validations" do - context "when there are no female tenants" do + context "when all tenants are male" do it "shows the interruption screen" do record.age1 = 43 record.sex1 = "M" record.preg_occ = 1 record.hhmemb = 1 record.age1_known = 0 - expect(record.no_females_in_a_pregnant_household?).to be true + expect(record.all_male_tenants_in_a_pregnant_household?).to be true end end - context "when there are no female tenants and age of other tenants is unknown" do + context "when there all tenants are male and age of tenants is unknown" do it "shows the interruption screen" do record.sex1 = "M" record.preg_occ = 1 record.hhmemb = 1 record.age1_known = 1 - expect(record.no_females_in_a_pregnant_household?).to be true + expect(record.all_male_tenants_in_a_pregnant_household?).to be true end end @@ -206,7 +206,7 @@ RSpec.describe Validations::SoftValidations do record.sex1 = "F" record.preg_occ = 1 record.hhmemb = 1 - expect(record.no_females_in_a_pregnant_household?).to be false + expect(record.all_male_tenants_in_a_pregnant_household?).to be false expect(record.female_in_pregnant_household_in_soft_validation_range?).to be false end end @@ -215,7 +215,7 @@ RSpec.describe Validations::SoftValidations do it "does not show the interruption screen" do record.preg_occ = 1 record.hhmemb = 2 - expect(record.no_females_in_a_pregnant_household?).to be false + expect(record.all_male_tenants_in_a_pregnant_household?).to be false expect(record.female_in_pregnant_household_in_soft_validation_range?).to be false end end From 8516aa1f231c218ff8b56f1b3a8498b5ff42cc48 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:12:50 +0100 Subject: [PATCH 4/9] Don't hide person known question in CYA (#2641) --- .../form/sales/questions/person_known.rb | 7 ----- .../form/sales/questions/person_known_spec.rb | 28 +++---------------- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/app/models/form/sales/questions/person_known.rb b/app/models/form/sales/questions/person_known.rb index 80dc6f36b..d0de89044 100644 --- a/app/models/form/sales/questions/person_known.rb +++ b/app/models/form/sales/questions/person_known.rb @@ -5,13 +5,6 @@ class Form::Sales::Questions::PersonKnown < ::Form::Question @header = "Do you know the details for person #{person_index}?" @type = "radio" @answer_options = ANSWER_OPTIONS - @hidden_in_check_answers = { - "depends_on" => [ - { - "details_known_#{person_index}" => 1, - }, - ], - } @check_answers_card_number = person_index end diff --git a/spec/models/form/sales/questions/person_known_spec.rb b/spec/models/form/sales/questions/person_known_spec.rb index 584187ec4..7668ca7b7 100644 --- a/spec/models/form/sales/questions/person_known_spec.rb +++ b/spec/models/form/sales/questions/person_known_spec.rb @@ -35,6 +35,10 @@ RSpec.describe Form::Sales::Questions::PersonKnown, type: :model do expect(question.hint_text).to be_nil end + it "has the correct hidden_in_check_answers" do + expect(question.hidden_in_check_answers).to be_nil + end + context "with person 2" do let(:question_id) { "details_known_2" } let(:person_index) { 2 } @@ -51,12 +55,6 @@ RSpec.describe Form::Sales::Questions::PersonKnown, type: :model do expect(question.check_answer_label).to eq("Details known for person 2?") end - it "has the correct hidden_in_check_answers" do - expect(question.hidden_in_check_answers).to eq( - "depends_on" => [{ "details_known_2" => 1 }], - ) - end - it "has the correct check_answers_card_number" do expect(question.check_answers_card_number).to eq(2) end @@ -78,12 +76,6 @@ RSpec.describe Form::Sales::Questions::PersonKnown, type: :model do expect(question.check_answer_label).to eq("Details known for person 3?") end - it "has the correct hidden_in_check_answers" do - expect(question.hidden_in_check_answers).to eq( - "depends_on" => [{ "details_known_3" => 1 }], - ) - end - it "has the correct check_answers_card_number" do expect(question.check_answers_card_number).to eq(3) end @@ -105,12 +97,6 @@ RSpec.describe Form::Sales::Questions::PersonKnown, type: :model do expect(question.check_answer_label).to eq("Details known for person 4?") end - it "has the correct hidden_in_check_answers" do - expect(question.hidden_in_check_answers).to eq( - "depends_on" => [{ "details_known_4" => 1 }], - ) - end - it "has the correct check_answers_card_number" do expect(question.check_answers_card_number).to eq(4) end @@ -132,12 +118,6 @@ RSpec.describe Form::Sales::Questions::PersonKnown, type: :model do expect(question.check_answer_label).to eq("Details known for person 5?") end - it "has the correct hidden_in_check_answers" do - expect(question.hidden_in_check_answers).to eq( - "depends_on" => [{ "details_known_5" => 1 }], - ) - end - it "has the correct check_answers_card_number" do expect(question.check_answers_card_number).to eq(5) end From 78096a7d984b68bb3c501337deeaf981beb3c7e2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:47:25 +0100 Subject: [PATCH 5/9] Bump express from 4.19.2 to 4.21.0 (#2647) Bumps [express](https://github.com/expressjs/express) from 4.19.2 to 4.21.0. - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/4.21.0/History.md) - [Commits](https://github.com/expressjs/express/compare/4.19.2...4.21.0) --- updated-dependencies: - dependency-name: express dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Rachael Booth --- yarn.lock | 95 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/yarn.lock b/yarn.lock index ea0395016..a855aa731 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1792,7 +1792,25 @@ bl@^4.1.0: inherits "^2.0.4" readable-stream "^3.4.0" -body-parser@1.20.2, body-parser@^1.20.2: +body-parser@1.20.3: + version "1.20.3" + resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.3.tgz#1953431221c6fb5cd63c4b36d53fab0928e548c6" + integrity sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g== + dependencies: + bytes "3.1.2" + content-type "~1.0.5" + debug "2.6.9" + depd "2.0.0" + destroy "1.2.0" + http-errors "2.0.0" + iconv-lite "0.4.24" + on-finished "2.4.1" + qs "6.13.0" + raw-body "2.5.2" + type-is "~1.6.18" + unpipe "1.0.0" + +body-parser@^1.20.2: version "1.20.2" resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.2.tgz#6feb0e21c4724d06de7ff38da36dad4f57a747fd" integrity sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA== @@ -2488,6 +2506,11 @@ encodeurl@~1.0.1, encodeurl@~1.0.2: resolved "https://registry.yarnpkg.com/encodeurl/-/encodeurl-1.0.2.tgz#ad3ff4c86ec2d029322f5a02c3a9a606c95b3f59" integrity sha512-TPJXq8JqFaVYm2CWmPvnP2Iyo4ZSM7/QKcSmuMLDObfpH5fi7RUGmd/rTDf+rut/saiDiQEeVTNgAmJEdAOx0w== +encodeurl@~2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/encodeurl/-/encodeurl-2.0.0.tgz#7b8ea898077d7e409d3ac45474ea38eaf0857a58" + integrity sha512-Q0n9HRi4m6JuGIV1eFlmvJB7ZEVxu93IrMyiMsGC0lrMJMWzRgx6WGquyfQgZVb31vhGgXnfmPNNXmxnOkRBrg== + engine.io-client@~6.5.2: version "6.5.4" resolved "https://registry.yarnpkg.com/engine.io-client/-/engine.io-client-6.5.4.tgz#b8bc71ed3f25d0d51d587729262486b4b33bd0d0" @@ -2953,36 +2976,36 @@ express-session@^1.18.0: uid-safe "~2.1.5" express@^4.18.2: - version "4.19.2" - resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" - integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== + version "4.21.0" + resolved "https://registry.yarnpkg.com/express/-/express-4.21.0.tgz#d57cb706d49623d4ac27833f1cbc466b668eb915" + integrity sha512-VqcNGcj/Id5ZT1LZ/cfihi3ttTn+NJmkli2eZADigjq29qTlWi/hAQ43t/VLPq8+UX06FCEx3ByOYet6ZFblng== dependencies: accepts "~1.3.8" array-flatten "1.1.1" - body-parser "1.20.2" + body-parser "1.20.3" content-disposition "0.5.4" content-type "~1.0.4" cookie "0.6.0" cookie-signature "1.0.6" debug "2.6.9" depd "2.0.0" - encodeurl "~1.0.2" + encodeurl "~2.0.0" escape-html "~1.0.3" etag "~1.8.1" - finalhandler "1.2.0" + finalhandler "1.3.1" fresh "0.5.2" http-errors "2.0.0" - merge-descriptors "1.0.1" + merge-descriptors "1.0.3" methods "~1.1.2" on-finished "2.4.1" parseurl "~1.3.3" - path-to-regexp "0.1.7" + path-to-regexp "0.1.10" proxy-addr "~2.0.7" - qs "6.11.0" + qs "6.13.0" range-parser "~1.2.1" safe-buffer "5.2.1" - send "0.18.0" - serve-static "1.15.0" + send "0.19.0" + serve-static "1.16.2" setprototypeof "1.2.0" statuses "2.0.1" type-is "~1.6.18" @@ -3106,13 +3129,13 @@ finalhandler@1.1.0: statuses "~1.3.1" unpipe "~1.0.0" -finalhandler@1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.2.0.tgz#7d23fe5731b207b4640e4fcd00aec1f9207a7b32" - integrity sha512-5uXcUVftlQMFnWC9qu/svkWv3GTd2PfUhK/3PLkYNAe7FbqJMt3515HaxE6eRL74GdsriiwujiawdaB1BpEISg== +finalhandler@1.3.1: + version "1.3.1" + resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.3.1.tgz#0c575f1d1d324ddd1da35ad7ece3df7d19088019" + integrity sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ== dependencies: debug "2.6.9" - encodeurl "~1.0.2" + encodeurl "~2.0.0" escape-html "~1.0.3" on-finished "2.4.1" parseurl "~1.3.3" @@ -4224,10 +4247,10 @@ meow@^13.2.0: resolved "https://registry.yarnpkg.com/meow/-/meow-13.2.0.tgz#6b7d63f913f984063b3cc261b6e8800c4cd3474f" integrity sha512-pxQJQzB6djGPXh08dacEloMFopsOqGVRKFPYvPOt9XDZ1HasbgDZA74CJGreSU4G3Ak7EFJGoiH2auq+yXISgA== -merge-descriptors@1.0.1: - version "1.0.1" - resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61" - integrity sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w== +merge-descriptors@1.0.3: + version "1.0.3" + resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.3.tgz#d80319a65f3c7935351e5cfdac8f9318504dbed5" + integrity sha512-gaNvAS7TZ897/rVaZ0nMtAyxNyi/pdbjbAwUpFQpN70GqnVfOiXpeUUMKRBmzXaSQ8DdTX4/0ms62r2K+hE6mQ== merge-stream@^2.0.0: version "2.0.0" @@ -4625,10 +4648,10 @@ path-parse@^1.0.7: resolved "https://registry.yarnpkg.com/path-parse/-/path-parse-1.0.7.tgz#fbc114b60ca42b30d9daf5858e4bd68bbedb6735" integrity sha512-LDJzPVEEEPR+y48z93A0Ed0yXb8pAByGWo/k5YYdYgpY2/2EsOsksJrq7lOHxryrVOn1ejG6oAp8ahvOIQD8sw== -path-to-regexp@0.1.7: - version "0.1.7" - resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.7.tgz#df604178005f522f15eb4490e7247a1bfaa67f8c" - integrity sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ== +path-to-regexp@0.1.10: + version "0.1.10" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.10.tgz#67e9108c5c0551b9e5326064387de4763c4d5f8b" + integrity sha512-7lf7qcQidTku0Gu3YDPc8DJ1q7OOucfa/BSsIwjuh56VU7katFvuM8hULfkwB3Fns/rsVF7PwPKVw1sl5KQS9w== path-type@^4.0.0: version "4.0.0" @@ -4809,7 +4832,7 @@ qs@6.11.0: dependencies: side-channel "^1.0.4" -qs@^6.4.0: +qs@6.13.0, qs@^6.4.0: version "6.13.0" resolved "https://registry.yarnpkg.com/qs/-/qs-6.13.0.tgz#6ca3bd58439f7e245655798997787b0d88a51906" integrity sha512-+38qI9SOr8tfZ4QmJNplMUxqjbe7LKvvZgWdExBOmd+egZTtjLB67Gu0HRX3u/XOq7UU2Nx6nsjvS16Z9uwfpg== @@ -5209,10 +5232,10 @@ send@0.16.2: range-parser "~1.2.0" statuses "~1.4.0" -send@0.18.0: - version "0.18.0" - resolved "https://registry.yarnpkg.com/send/-/send-0.18.0.tgz#670167cc654b05f5aa4a767f9113bb371bc706be" - integrity sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg== +send@0.19.0: + version "0.19.0" + resolved "https://registry.yarnpkg.com/send/-/send-0.19.0.tgz#bbc5a388c8ea6c048967049dbeac0e4a3f09d7f8" + integrity sha512-dW41u5VfLXu8SJh5bwRmyYUbAoSB3c9uQh6L8h/KtsFREPWpbX1lrljJo186Jc4nmci/sGUZ9a0a0J2zgfq2hw== dependencies: debug "2.6.9" depd "2.0.0" @@ -5258,15 +5281,15 @@ serve-static@1.13.2: parseurl "~1.3.2" send "0.16.2" -serve-static@1.15.0: - version "1.15.0" - resolved "https://registry.yarnpkg.com/serve-static/-/serve-static-1.15.0.tgz#faaef08cffe0a1a62f60cad0c4e513cff0ac9540" - integrity sha512-XGuRDNjXUijsUL0vl6nSD7cwURuzEgglbOaFuZM9g3kwDXOWVTck0jLzjPzGD+TazWbboZYu52/9/XPdUgne9g== +serve-static@1.16.2: + version "1.16.2" + resolved "https://registry.yarnpkg.com/serve-static/-/serve-static-1.16.2.tgz#b6a5343da47f6bdd2673848bf45754941e803296" + integrity sha512-VqpjJZKadQB/PEbEwvFdO43Ax5dFBZ2UECszz8bQ7pi7wt//PWe1P6MN7eCnjsatYtBT6EuiClbjSWP2WrIoTw== dependencies: - encodeurl "~1.0.2" + encodeurl "~2.0.0" escape-html "~1.0.3" parseurl "~1.3.3" - send "0.18.0" + send "0.19.0" server-destroy@1.0.1: version "1.0.1" From f96af46c6edd9f9019e886828f1614bc89090ac7 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 20 Sep 2024 09:17:28 +0100 Subject: [PATCH 6/9] Update postgres version (#2650) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index a26c80ee9..6eeee6511 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,7 +10,7 @@ RUN apk add --update --no-cache tzdata && \ # build-base: compilation tools for bundle # yarn: node package manager # postgresql-dev: postgres driver and libraries -RUN apk add --no-cache build-base=0.5-r3 busybox=1.36.1-r7 nodejs-current=20.8.1-r0 yarn=1.22.19-r0 postgresql13-dev=13.15-r0 git=2.40.3-r0 bash=5.2.15-r5 +RUN apk add --no-cache build-base=0.5-r3 busybox=1.36.1-r7 nodejs-current=20.8.1-r0 yarn=1.22.19-r0 postgresql13-dev=13.16-r0 git=2.40.3-r0 bash=5.2.15-r5 # Bundler version should be the same version as what the Gemfile.lock was bundled with RUN gem install bundler:2.3.14 --no-document From d56ec0986d5e876ef8a7544466a2d6ba18cf7765 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 20 Sep 2024 11:14:56 +0100 Subject: [PATCH 7/9] Bump body-parser from 1.20.2 to 1.20.3 (#2649) Bumps [body-parser](https://github.com/expressjs/body-parser) from 1.20.2 to 1.20.3. - [Release notes](https://github.com/expressjs/body-parser/releases) - [Changelog](https://github.com/expressjs/body-parser/blob/master/HISTORY.md) - [Commits](https://github.com/expressjs/body-parser/compare/1.20.2...1.20.3) --- updated-dependencies: - dependency-name: body-parser dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/yarn.lock b/yarn.lock index a855aa731..65de3d8d2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1792,7 +1792,7 @@ bl@^4.1.0: inherits "^2.0.4" readable-stream "^3.4.0" -body-parser@1.20.3: +body-parser@1.20.3, body-parser@^1.20.2: version "1.20.3" resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.3.tgz#1953431221c6fb5cd63c4b36d53fab0928e548c6" integrity sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g== @@ -1810,24 +1810,6 @@ body-parser@1.20.3: type-is "~1.6.18" unpipe "1.0.0" -body-parser@^1.20.2: - version "1.20.2" - resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.2.tgz#6feb0e21c4724d06de7ff38da36dad4f57a747fd" - integrity sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA== - dependencies: - bytes "3.1.2" - content-type "~1.0.5" - debug "2.6.9" - depd "2.0.0" - destroy "1.2.0" - http-errors "2.0.0" - iconv-lite "0.4.24" - on-finished "2.4.1" - qs "6.11.0" - raw-body "2.5.2" - type-is "~1.6.18" - unpipe "1.0.0" - bootstrap@^5.1.3: version "5.3.3" resolved "https://registry.yarnpkg.com/bootstrap/-/bootstrap-5.3.3.tgz#de35e1a765c897ac940021900fcbb831602bac38" @@ -4825,13 +4807,6 @@ punycode@^2.1.0: resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.3.1.tgz#027422e2faec0b25e1549c3e1bd8309b9133b6e5" integrity sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg== -qs@6.11.0: - version "6.11.0" - resolved "https://registry.yarnpkg.com/qs/-/qs-6.11.0.tgz#fd0d963446f7a65e1367e01abd85429453f0c37a" - integrity sha512-MvjoMCJwEarSbUYk5O+nmoSzSutSsTwF85zcHPQ9OrlFoZOYIjaqBAJIqIXjptyD5vThxGq52Xu/MaJzRkIk4Q== - dependencies: - side-channel "^1.0.4" - qs@6.13.0, qs@^6.4.0: version "6.13.0" resolved "https://registry.yarnpkg.com/qs/-/qs-6.13.0.tgz#6ca3bd58439f7e245655798997787b0d88a51906" From a8d389fc3b564cc02f26d39e55363f87deaf7a2c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:02:00 +0100 Subject: [PATCH 8/9] Bump puma from 5.6.8 to 5.6.9 (#2654) Bumps [puma](https://github.com/puma/puma) from 5.6.8 to 5.6.9. - [Release notes](https://github.com/puma/puma/releases) - [Changelog](https://github.com/puma/puma/blob/master/History.md) - [Commits](https://github.com/puma/puma/compare/v5.6.8...v5.6.9) --- updated-dependencies: - dependency-name: puma dependency-type: direct:production ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index cdf923ae0..43d9374d1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -314,7 +314,7 @@ GEM byebug (~> 11.0) pry (>= 0.13, < 0.15) public_suffix (5.0.4) - puma (5.6.8) + puma (5.6.9) nio4r (~> 2.0) pundit (2.3.1) activesupport (>= 3.0.0) From 204e022c8f8aac274f09369de89166ce8b1e840b Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:58:23 +0100 Subject: [PATCH 9/9] Refactor postcode tests to use webmock stubs (#2651) --- spec/services/postcode_service_spec.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/spec/services/postcode_service_spec.rb b/spec/services/postcode_service_spec.rb index abc61129f..4df809606 100644 --- a/spec/services/postcode_service_spec.rb +++ b/spec/services/postcode_service_spec.rb @@ -12,14 +12,10 @@ describe PostcodeService do end describe "lookup" do - before do - Excon.defaults[:mock] = true - Excon.defaults[:stubs] = :local - end - context "when the request returns a success response" do before do - Excon.stub({}, { body: '{"result": { "admin_district": "District", "codes": { "admin_district": "123" } } }', status: 200 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"A00 0AA\",\"admin_district\":\"District\",\"codes\":{\"admin_district\":\"123\"}}}", headers: {}) end it "returns the admin district and admin district code" do @@ -31,7 +27,8 @@ describe PostcodeService do context "when the request returns a not found response" do before do - Excon.stub({}, { status: 404 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 404, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) end it "returns nil" do @@ -47,7 +44,8 @@ describe PostcodeService do context "when the request returns an error response" do before do - Excon.stub({}, { body: "This is an error message that is not valid json", status: 500 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 500, body: "This is an error message that is not valid json", headers: {}) end it "returns nil" do @@ -63,7 +61,8 @@ describe PostcodeService do context "when the request returns a success response that causes later errors" do before do - Excon.stub({}, { body: '{"result": { "admin_district": "District" } }', status: 200 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"admin_district\":\"Westminster\"", headers: {}) end it "returns nil" do