From ff788afcb5f91328b44d6b1d7e781e586c984b5c Mon Sep 17 00:00:00 2001 From: David May-Miller Date: Tue, 8 Nov 2022 16:33:48 +0000 Subject: [PATCH 01/18] CLDC-1698 Update tenancy length validations (#975) --- app/models/validations/tenancy_validations.rb | 16 +++++-- config/locales/en.yml | 4 +- .../validations/tenancy_validations_spec.rb | 48 +++++++++++++------ 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/app/models/validations/tenancy_validations.rb b/app/models/validations/tenancy_validations.rb index 25e09f205..e2ea108cf 100644 --- a/app/models/validations/tenancy_validations.rb +++ b/app/models/validations/tenancy_validations.rb @@ -5,7 +5,7 @@ module Validations::TenancyValidations def validate_fixed_term_tenancy(record) is_present = record.tenancylength.present? - is_in_range = record.tenancylength.to_i.between?(2, 99) + is_in_range = record.tenancylength.to_i.between?(min_tenancy_length(record), 99) conditions = [ { condition: !(record.is_secure_tenancy? || record.is_assured_shorthold_tenancy?) && is_present, @@ -13,11 +13,17 @@ module Validations::TenancyValidations }, { condition: (record.is_assured_shorthold_tenancy? && !is_in_range) && is_present, - error: I18n.t("validations.tenancy.length.shorthold"), + error: I18n.t( + "validations.tenancy.length.shorthold", + min_tenancy_length: min_tenancy_length(record), + ), }, { condition: record.is_secure_tenancy? && (!is_in_range && is_present), - error: I18n.t("validations.tenancy.length.secure"), + error: I18n.t( + "validations.tenancy.length.secure", + min_tenancy_length: min_tenancy_length(record), + ), }, ] @@ -41,4 +47,8 @@ module Validations::TenancyValidations record.errors.add :hhmemb, I18n.t("validations.tenancy.joint_more_than_one_member") end end + + def min_tenancy_length(record) + record.is_supported_housing? || record.renttype == 3 ? 1 : 2 + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 003f573dc..55ea3a0f7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -301,8 +301,8 @@ en: tenancy: length: fixed_term_not_required: "You must only answer the length of the tenancy if it's fixed-term" - shorthold: "Enter a tenancy length between 2 and 99 years for a Fixed Term – Assured Shorthold Tenancy (AST)" - secure: "Enter a tenancy length between 2 and 99 years (or don't specify the length) for a Secure (including flexible) tenancy" + shorthold: "Enter a tenancy length between %{min_tenancy_length} and 99 years for a tenancy of this type" + secure: "Enter a tenancy length between %{min_tenancy_length} and 99 years (or don't specify the length) for a tenancy of this type" internal_transfer: "Answer must be secure tenancy as this tenancy is an internal transfer" cannot_be_internal_transfer: "Answer cannot be internal transfer as this is not a secure tenancy" not_joint: "This cannot be a joint tenancy as you've told us there's only one person in the household" diff --git a/spec/models/validations/tenancy_validations_spec.rb b/spec/models/validations/tenancy_validations_spec.rb index ccb5503d0..2d008e6e3 100644 --- a/spec/models/validations/tenancy_validations_spec.rb +++ b/spec/models/validations/tenancy_validations_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Validations::TenancyValidations do subject(:tenancy_validator) { validator_class.new } let(:validator_class) { Class.new { include Validations::TenancyValidations } } - let(:record) { FactoryBot.create(:lettings_log, startdate: Time.zone.local(2021, 5, 1)) } + let(:record) { FactoryBot.create(:lettings_log, startdate: Time.zone.local(2021, 5, 1), needstype: 1, rent_type: 1) } describe "fixed term tenancy validations" do context "when fixed term tenancy" do @@ -21,11 +21,16 @@ RSpec.describe Validations::TenancyValidations do end context "when type of tenancy is assured shorthold" do - let(:expected_error) { I18n.t("validations.tenancy.length.shorthold") } + let(:expected_error) do + I18n.t( + "validations.tenancy.length.shorthold", + min_tenancy_length: 2, + ) + end before { record.tenancy = 4 } - context "when tenancy length is greater than 1" do + context "when tenancy length is less than 2" do it "adds an error" do record.tenancylength = 1 tenancy_validator.validate_fixed_term_tenancy(record) @@ -34,7 +39,7 @@ RSpec.describe Validations::TenancyValidations do end end - context "when tenancy length is less than 100" do + context "when tenancy length is greater than 99" do it "adds an error" do record.tenancylength = 100 tenancy_validator.validate_fixed_term_tenancy(record) @@ -64,11 +69,16 @@ RSpec.describe Validations::TenancyValidations do context "when the collection start year is before 2022" do context "when type of tenancy is secure" do - let(:expected_error) { I18n.t("validations.tenancy.length.secure") } + let(:expected_error) do + I18n.t( + "validations.tenancy.length.secure", + min_tenancy_length: 2, + ) + end before { record.tenancy = 1 } - context "when tenancy length is greater than 1" do + context "when tenancy length is less than 2" do it "adds an error" do record.tenancylength = 1 tenancy_validator.validate_fixed_term_tenancy(record) @@ -77,7 +87,7 @@ RSpec.describe Validations::TenancyValidations do end end - context "when tenancy length is less than 100" do + context "when tenancy length is greater than 99" do it "adds an error" do record.tenancylength = 100 tenancy_validator.validate_fixed_term_tenancy(record) @@ -107,14 +117,19 @@ RSpec.describe Validations::TenancyValidations do end context "when the collection start year is 2022 or later" do - let(:record) { FactoryBot.create(:lettings_log, startdate: Time.zone.local(2022, 5, 1)) } + let(:record) { FactoryBot.create(:lettings_log, startdate: Time.zone.local(2022, 5, 1), needstype: 1, rent_type: 1) } context "when type of tenancy is Secure - fixed term" do - let(:expected_error) { I18n.t("validations.tenancy.length.secure") } + let(:expected_error) do + I18n.t( + "validations.tenancy.length.secure", + min_tenancy_length: 2, + ) + end before { record.tenancy = 6 } - context "when tenancy length is greater than 1" do + context "when tenancy length is less than 2" do it "adds an error" do record.tenancylength = 1 tenancy_validator.validate_fixed_term_tenancy(record) @@ -123,7 +138,7 @@ RSpec.describe Validations::TenancyValidations do end end - context "when tenancy length is less than 100" do + context "when tenancy length is greater than 99" do it "adds an error" do record.tenancylength = 100 tenancy_validator.validate_fixed_term_tenancy(record) @@ -152,11 +167,16 @@ RSpec.describe Validations::TenancyValidations do end context "when type of tenancy is Secure - lifetime" do - let(:expected_error) { I18n.t("validations.tenancy.length.secure") } + let(:expected_error) do + I18n.t( + "validations.tenancy.length.secure", + min_tenancy_length: 2, + ) + end before { record.tenancy = 7 } - context "when tenancy length is greater than 1" do + context "when tenancy length is less than 2" do it "adds an error" do record.tenancylength = 1 tenancy_validator.validate_fixed_term_tenancy(record) @@ -165,7 +185,7 @@ RSpec.describe Validations::TenancyValidations do end end - context "when tenancy length is less than 100" do + context "when tenancy length is greater than 99" do it "adds an error" do record.tenancylength = 100 tenancy_validator.validate_fixed_term_tenancy(record) From ef354fada695d7f7ceaca928b07f7d5344760005 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 9 Nov 2022 15:42:52 +0000 Subject: [PATCH 02/18] Update routes (#978) * Update routes * Remove member from routes --- app/controllers/locations_controller.rb | 12 ++++++------ app/controllers/schemes_controller.rb | 2 +- app/helpers/check_answers_helper.rb | 6 +++--- app/views/locations/edit.html.erb | 2 +- app/views/locations/edit_local_authority.html.erb | 2 +- app/views/locations/edit_name.html.erb | 2 +- app/views/locations/index.html.erb | 2 +- app/views/locations/new.html.erb | 2 +- app/views/schemes/check_answers.html.erb | 2 +- config/routes.rb | 9 +++------ 10 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index 532ab915a..0a857bb13 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -23,9 +23,9 @@ class LocationsController < ApplicationController @location = Location.new(location_params) if @location.save if @location.location_admin_district.nil? - redirect_to(location_edit_local_authority_path(id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location])) + redirect_to(scheme_location_edit_local_authority_path(scheme_id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location])) elsif location_params[:add_another_location] == "Yes" - redirect_to new_location_path(@scheme) + redirect_to new_scheme_location_path(@scheme) else check_answers_path = @scheme.confirmed? ? scheme_check_answers_path(@scheme, anchor: "locations") : scheme_check_answers_path(@scheme) redirect_to check_answers_path @@ -69,9 +69,9 @@ class LocationsController < ApplicationController case page when "edit" if @location.location_admin_district.nil? - redirect_to(location_edit_local_authority_path(id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location])) + redirect_to(scheme_location_edit_local_authority_path(scheme_id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location])) elsif location_params[:add_another_location] == "Yes" - redirect_to(new_location_path(@location.scheme)) + redirect_to(new_scheme_location_path(@location.scheme)) else redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) end @@ -79,7 +79,7 @@ class LocationsController < ApplicationController redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) when "edit-local-authority" if params[:add_another_location] == "Yes" - redirect_to(new_location_path(@location.scheme)) + redirect_to(new_scheme_location_path(@location.scheme)) else redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) end @@ -107,7 +107,7 @@ private def find_scheme @scheme = if %w[new create index edit_name].include?(action_name) - Scheme.find(params[:id]) + Scheme.find(params[:scheme_id]) else @location&.scheme end diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index d4168e355..9a259e3a1 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -176,7 +176,7 @@ private when "secondary-client-group" scheme_support_path(@scheme) when "support" - new_location_path + new_scheme_location_path(@scheme) when "details" if @scheme.arrangement_type_before_type_cast == "D" scheme_primary_client_group_path(@scheme) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index b334ab073..1583b3791 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -18,14 +18,14 @@ module CheckAnswersHelper def get_location_change_link_href_postcode(scheme, location) if location.confirmed? - location_edit_name_path(id: scheme.id, location_id: location.id) + scheme_location_edit_name_path(scheme_id: scheme.id, location_id: location.id) else - location_edit_path(id: scheme.id, location_id: location.id) + edit_scheme_location_path(scheme_id: scheme.id, id: location.id) end end def get_location_change_link_href_location_admin_district(scheme, location) - location_edit_local_authority_path(id: scheme.id, location_id: location.id) + scheme_location_edit_local_authority_path(scheme_id: scheme.id, location_id: location.id) end def any_questions_have_summary_card_number?(subsection, lettings_log) diff --git a/app/views/locations/edit.html.erb b/app/views/locations/edit.html.erb index 6e14503ed..dca0d04fb 100644 --- a/app/views/locations/edit.html.erb +++ b/app/views/locations/edit.html.erb @@ -7,7 +7,7 @@ ) %> <% end %> -<%= form_for(@location, method: :patch, url: location_path(@location)) do |f| %> +<%= form_for(@location, method: :patch, url: scheme_location_path(scheme_id: @scheme.id, id: @location.id)) do |f| %>
<%= f.govuk_error_summary %> diff --git a/app/views/locations/edit_local_authority.html.erb b/app/views/locations/edit_local_authority.html.erb index 43c351d28..1e5ab9fbb 100644 --- a/app/views/locations/edit_local_authority.html.erb +++ b/app/views/locations/edit_local_authority.html.erb @@ -5,7 +5,7 @@ ) %> <% end %> -<%= form_for(@location, method: :patch, url: location_path(location_id: @location.id, add_another_location: params[:add_another_location])) do |f| %> +<%= form_for(@location, method: :patch, url: scheme_location_path(scheme_id: @scheme.id, id: @location.id, add_another_location: params[:add_another_location])) do |f| %>
<%= f.govuk_error_summary %> diff --git a/app/views/locations/edit_name.html.erb b/app/views/locations/edit_name.html.erb index 087ca0812..ece4df866 100644 --- a/app/views/locations/edit_name.html.erb +++ b/app/views/locations/edit_name.html.erb @@ -7,7 +7,7 @@ ) %> <% end %> -<%= form_for(@location, method: :patch, url: location_path(location_id: @location.id)) do |f| %> +<%= form_for(@location, method: :patch, url: scheme_location_path(scheme_id: @scheme.id, id: @location.id)) do |f| %>
<%= f.govuk_error_summary %> diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 6672fc661..703fd9fa5 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -62,6 +62,6 @@ <% end %> <% end %> <% end %> -<%= govuk_button_link_to "Add a location", new_location_path(id: @scheme.id), secondary: true %> +<%= govuk_button_link_to "Add a location", new_scheme_location_path(scheme_id: @scheme.id), secondary: true %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "locations" } %> diff --git a/app/views/locations/new.html.erb b/app/views/locations/new.html.erb index 71690f8e6..69f56744b 100644 --- a/app/views/locations/new.html.erb +++ b/app/views/locations/new.html.erb @@ -7,7 +7,7 @@ ) %> <% end %> -<%= form_for(@location, method: :post, url: locations_path) do |f| %> +<%= form_for(@location, method: :post, url: scheme_locations_path) do |f| %>
<%= f.govuk_error_summary %> diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb index 2cb154e1f..a41b202ad 100644 --- a/app/views/schemes/check_answers.html.erb +++ b/app/views/schemes/check_answers.html.erb @@ -78,7 +78,7 @@ <% end %> <% end %> <% end %> - <%= govuk_button_link_to "Add a location", new_location_path(id: @scheme.id), secondary: true %> + <%= govuk_button_link_to "Add a location", new_scheme_location_path(scheme_id: @scheme.id), secondary: true %> <% end %> <% end %> <%= f.hidden_field :page, value: "check-answers" %> diff --git a/config/routes.rb b/config/routes.rb index 9d5083e1f..eab3d3222 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -50,12 +50,9 @@ Rails.application.routes.draw do get "edit-name", to: "schemes#edit_name" get "support-services-provider", to: "schemes#support_services_provider" - member do - resources :locations do - get "edit-name", to: "locations#edit_name" - get "edit", to: "locations#edit" - get "edit-local-authority", to: "locations#edit_local_authority" - end + resources :locations do + get "edit-name", to: "locations#edit_name" + get "edit-local-authority", to: "locations#edit_local_authority" end end From a4fc795e31fba1914eab6eba68ded9131d73f1e7 Mon Sep 17 00:00:00 2001 From: James Rose Date: Thu, 10 Nov 2022 09:50:44 +0000 Subject: [PATCH 03/18] Parse meta fields where namespace declarations aren't included (#983) We import XML files that define logs from the previous CORE service. For an unknown reason, some of these files don't include the required namespace declarations to parse `meta` values. Instead of changing the source, this change introduces a new method that forces the namespace declaration for meta fields. --- app/services/imports/import_service.rb | 8 ++++++-- .../imports/lettings_logs_field_import_service.rb | 10 +++++----- app/services/imports/lettings_logs_import_service.rb | 12 ++++++------ 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/app/services/imports/import_service.rb b/app/services/imports/import_service.rb index e45498004..1db1f4d6a 100644 --- a/app/services/imports/import_service.rb +++ b/app/services/imports/import_service.rb @@ -19,8 +19,12 @@ module Imports end end - def field_value(xml_document, namespace, field) - xml_document.at_xpath("//#{namespace}:#{field}")&.text + def field_value(xml_document, namespace, field, *args) + xml_document.at_xpath("//#{namespace}:#{field}", *args)&.text + end + + def meta_field_value(xml_document, field) + field_value(xml_document, "meta", field, { "meta" => "http://data.gov.uk/core/metadata" }) end def overridden?(xml_document, namespace, field) diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 218decb7a..e59e178f2 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -16,8 +16,8 @@ module Imports private def update_lettings_allocation(xml_doc) - old_id = field_value(xml_doc, "meta", "document-id") - previous_status = field_value(xml_doc, "meta", "status") + old_id = meta_field_value(xml_doc, "document-id") + previous_status = meta_field_value(xml_doc, "status") record = LettingsLog.find_by(old_id:) if record.present? && previous_status.include?("submitted") @@ -46,11 +46,11 @@ module Imports end def update_major_repairs(xml_doc) - old_id = field_value(xml_doc, "meta", "document-id") + old_id = meta_field_value(xml_doc, "document-id") record = LettingsLog.find_by(old_id:) if record.present? - previous_status = field_value(xml_doc, "meta", "status") + previous_status = meta_field_value(xml_doc, "status") major_repairs_date = compose_date(xml_doc, "MRCDAY", "MRCMONTH", "MRCYEAR") major_repairs = if major_repairs_date.present? && previous_status.include?("submitted") 1 @@ -69,7 +69,7 @@ module Imports end def update_tenant_code(xml_doc) - old_id = field_value(xml_doc, "meta", "document-id") + old_id = meta_field_value(xml_doc, "document-id") record = LettingsLog.find_by(old_id:) if record.present? diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index c7721b666..1aecc4f81 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -57,7 +57,7 @@ module Imports def create_log(xml_doc) attributes = {} - previous_status = field_value(xml_doc, "meta", "status") + previous_status = meta_field_value(xml_doc, "status") # Required fields for status complete or logic to work # Note: order matters when we derive from previous values (attributes parameter) @@ -191,9 +191,9 @@ module Imports # Not specific to our form but required for consistency (present in import) attributes["old_form_id"] = safe_string_as_integer(xml_doc, "FORM") - attributes["created_at"] = Time.zone.parse(field_value(xml_doc, "meta", "created-date")) - attributes["updated_at"] = Time.zone.parse(field_value(xml_doc, "meta", "modified-date")) - attributes["old_id"] = field_value(xml_doc, "meta", "document-id") + attributes["created_at"] = Time.zone.parse(meta_field_value(xml_doc, "created-date")) + attributes["updated_at"] = Time.zone.parse(meta_field_value(xml_doc, "modified-date")) + attributes["old_id"] = meta_field_value(xml_doc, "document-id") # Required for our form invalidated questions (not present in import) attributes["previous_la_known"] = attributes["prevloc"].nil? ? 0 : 1 @@ -236,7 +236,7 @@ module Imports attributes["net_income_value_check"] = 0 # Sets the log creator - owner_id = field_value(xml_doc, "meta", "owner-user-id").strip + owner_id = meta_field_value(xml_doc, "owner-user-id").strip if owner_id.present? user = LegacyUser.find_by(old_user_id: owner_id)&.user @logger.warn "Missing user! We expected to find a legacy user with old_user_id #{owner_id}" unless user @@ -355,7 +355,7 @@ module Imports end def get_form_name_component(xml_doc, index) - form_name = field_value(xml_doc, "meta", "form-name") + form_name = meta_field_value(xml_doc, "form-name") form_type_components = form_name.split("-") form_type_components[index] end From ed956b23d32f5ac94a41bee7e3622e3b16897d94 Mon Sep 17 00:00:00 2001 From: James Rose Date: Thu, 10 Nov 2022 14:41:17 +0000 Subject: [PATCH 04/18] Add validation to dependent ccharge field (#979) Although both fields are on the same page, we need to add a validation here so that we can clear both values on import. --- app/models/validations/financial_validations.rb | 1 + spec/models/validations/financial_validations_spec.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb index 4dec7bb78..b1bddd9cb 100644 --- a/app/models/validations/financial_validations.rb +++ b/app/models/validations/financial_validations.rb @@ -110,6 +110,7 @@ module Validations::FinancialValidations if record.is_carehome? period = record.form.get_question("period", record).label_from_value(record.period).downcase if record.chcharge.blank? + record.errors.add :is_carehome, I18n.t("validations.financial.carehome.not_provided", period:) record.errors.add :chcharge, I18n.t("validations.financial.carehome.not_provided", period:) elsif !weekly_value_in_range(record, "chcharge", 10, 1000) max_chcharge = record.weekly_to_value_per_period(1000) diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb index c4c4958db..0676e900b 100644 --- a/spec/models/validations/financial_validations_spec.rb +++ b/spec/models/validations/financial_validations_spec.rb @@ -1010,6 +1010,8 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_care_home_charges(record) expect(record.errors["chcharge"]) .to include(match I18n.t("validations.financial.carehome.not_provided", period: "every 4 weeks")) + expect(record.errors["is_carehome"]) + .to include(match I18n.t("validations.financial.carehome.not_provided", period: "every 4 weeks")) end end From 219bf937ccdbb39d28c3b66b262c0bfa6492ef9a Mon Sep 17 00:00:00 2001 From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com> Date: Thu, 10 Nov 2022 14:44:29 +0000 Subject: [PATCH 05/18] CLDC-1694 Fix issue with non-renewal lettings allowing 'renewal' for vacancy reason (#982) * feat: add renewal validation on rsnvac * test: check sensible error seen when rsnvac is renewal and but letting is not a renewal * test: change renewal validation check to fetch error message dynamically * feat: lint code * feat: write record.renewal.present?, not record.renewal Co-authored-by: James Rose Co-authored-by: James Rose --- app/models/validations/property_validations.rb | 4 ++++ config/locales/en.yml | 1 + spec/models/validations/property_validations_spec.rb | 11 +++++++++++ 3 files changed, 16 insertions(+) diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb index d5ab8e9e0..c655d8c85 100644 --- a/app/models/validations/property_validations.rb +++ b/app/models/validations/property_validations.rb @@ -38,6 +38,10 @@ module Validations::PropertyValidations record.errors.add :rsnvac, I18n.t("validations.property.rsnvac.referral_invalid") record.errors.add :referral, I18n.t("validations.household.referral.rsnvac_non_temp") end + + if record.renewal.present? && record.renewal.zero? && record.rsnvac == 14 + record.errors.add :rsnvac, I18n.t("validations.property.rsnvac.not_a_renewal") + end end def validate_unitletas(record) diff --git a/config/locales/en.yml b/config/locales/en.yml index 55ea3a0f7..9a45d96dd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -141,6 +141,7 @@ en: previous_let_social: "Property cannot have a previous let type if being let as social housing for the first time" non_temp_accommodation: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as this accommodation is not temporary" referral_invalid: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as a different source of referral for this letting" + not_a_renewal: "Reason for vacancy cannot be 'Renewal of fixed-term tenancy' if letting is not a renewal" unittype_gn: one_bedroom_bedsit: "A bedsit can only have one bedroom" one_seven_bedroom_shared: "A shared house must have 1 to 7 bedrooms" diff --git a/spec/models/validations/property_validations_spec.rb b/spec/models/validations/property_validations_spec.rb index 153934ebc..b3965b3d6 100644 --- a/spec/models/validations/property_validations_spec.rb +++ b/spec/models/validations/property_validations_spec.rb @@ -233,6 +233,17 @@ RSpec.describe Validations::PropertyValidations do property_validator.validate_rsnvac(record) expect(record.errors["rsnvac"]).to be_empty end + + context "when the letting is not a renewal" do + it "validates that the reason for vacancy is not renewal" do + record.first_time_property_let_as_social_housing = 0 + record.renewal = 0 + record.rsnvac = 14 + property_validator.validate_rsnvac(record) + expect(record.errors["rsnvac"]) + .to include(match I18n.t("validations.property.rsnvac.not_a_renewal")) + end + end end context "when the property has been let before" do From 978d72f7e04b6ce0b7952833a9156e44a74e617b Mon Sep 17 00:00:00 2001 From: James Rose Date: Thu, 10 Nov 2022 14:52:39 +0000 Subject: [PATCH 06/18] Import locations that belong to now inactive schemes (#984) We were previously blocking the import of locations that belonged to schemes where the end date was before the current date. This meant that we were unable to import logs that are associated with schemes that _were_ valid, but have since expired. --- .../imports/scheme_location_import_service.rb | 32 ++++++++----------- .../scheme_location_import_service_spec.rb | 15 --------- 2 files changed, 13 insertions(+), 34 deletions(-) diff --git a/app/services/imports/scheme_location_import_service.rb b/app/services/imports/scheme_location_import_service.rb index cdcd9448d..4214555cc 100644 --- a/app/services/imports/scheme_location_import_service.rb +++ b/app/services/imports/scheme_location_import_service.rb @@ -97,25 +97,19 @@ module Imports end def add_location(scheme, attributes) - if attributes["end_date"].nil? || attributes["end_date"] >= Time.zone.now - begin - Location.create!( - name: attributes["location_name"], - postcode: attributes["postcode"], - mobility_type: attributes["mobility_type"], - units: attributes["units"], - type_of_unit: attributes["type_of_unit"], - old_visible_id: attributes["location_old_visible_id"], - old_id: attributes["location_old_id"], - startdate: attributes["start_date"], - scheme:, - ) - rescue ActiveRecord::RecordNotUnique - @logger.warn("Location is already present with legacy ID #{attributes['location_old_id']}, skipping") - end - else - @logger.warn("Location with legacy ID #{attributes['location_old_id']} is expired (#{attributes['end_date']}), skipping") - end + Location.create!( + name: attributes["location_name"], + postcode: attributes["postcode"], + mobility_type: attributes["mobility_type"], + units: attributes["units"], + type_of_unit: attributes["type_of_unit"], + old_visible_id: attributes["location_old_visible_id"], + old_id: attributes["location_old_id"], + startdate: attributes["start_date"], + scheme:, + ) + rescue ActiveRecord::RecordNotUnique + @logger.warn("Location is already present with legacy ID #{attributes['location_old_id']}, skipping") end def find_scheme_to_merge(attributes) diff --git a/spec/services/imports/scheme_location_import_service_spec.rb b/spec/services/imports/scheme_location_import_service_spec.rb index 70ff039cd..8b93d4969 100644 --- a/spec/services/imports/scheme_location_import_service_spec.rb +++ b/spec/services/imports/scheme_location_import_service_spec.rb @@ -160,21 +160,6 @@ RSpec.describe Imports::SchemeLocationImportService do expect(location.scheme.confirmed).to be_truthy end - context "and the end date is before the current date" do - before do - Timecop.freeze(2022, 6, 1) - location_xml.at_xpath("//scheme:end-date").content = "2022-05-01" - end - - after { Timecop.unfreeze } - - it "does not create the location" do - expect(logger).to receive(:warn).with("Location with legacy ID 0ae7ad6dc0f1cf7ef33c18cc8c108bebc1b4923e is expired (2022-05-01 00:00:00 +0100), skipping") - expect { location_service.create_scheme_location(location_xml) } - .not_to change(Location, :count) - end - end - context "and we import the same location twice" do before { location_service.create_scheme_location(location_xml) } From 4a3172e82c496f78a86a7673131859b82329d662 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 10 Nov 2022 14:53:22 +0000 Subject: [PATCH 07/18] Cldc 1667 location details page (#976) * Add show location page * Add availability to the locations table * Add active status * Add an empty deactivate page and button * styling * lint * Extract feature toggle * Update route * Move display_attributes * update path * update paths --- app/controllers/locations_controller.rb | 6 ++++ app/helpers/locations_helper.rb | 14 ++++++++ app/helpers/tag_helper.rb | 2 ++ app/models/location.rb | 16 ++++----- app/views/locations/edit_name.html.erb | 2 +- app/views/locations/index.html.erb | 2 +- app/views/locations/show.html.erb | 28 +++++++++++++++ app/views/locations/toggle_active.html.erb | 0 config/initializers/feature_toggle.rb | 6 ++++ config/routes.rb | 1 + spec/features/schemes_spec.rb | 42 +++++++++++++++++++--- spec/helpers/locations_helper_spec.rb | 27 ++++++++++++++ 12 files changed, 131 insertions(+), 15 deletions(-) create mode 100644 app/views/locations/show.html.erb create mode 100644 app/views/locations/toggle_active.html.erb diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index 0a857bb13..797e79544 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -18,6 +18,12 @@ class LocationsController < ApplicationController @location = Location.new end + def show; end + + def deactivate + render "toggle_active", locals: { action: "deactivate" } + end + def create if date_params_missing?(location_params) || valid_date_params?(location_params) @location = Location.new(location_params) diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb index e0fe9d18c..e21a4724e 100644 --- a/app/helpers/locations_helper.rb +++ b/app/helpers/locations_helper.rb @@ -22,4 +22,18 @@ module LocationsHelper resource.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } end + + def display_attributes(location) + [ + { name: "Postcode", value: location.postcode }, + { name: "Local authority", value: location.location_admin_district }, + { name: "Location name", value: location.name, edit: true }, + { name: "Total number of units at this location", value: location.units }, + { name: "Common type of unit", value: location.type_of_unit }, + { name: "Mobility type", value: location.mobility_type }, + { name: "Code", value: location.location_code }, + { name: "Availability", value: "Available from #{location.available_from.to_formatted_s(:govuk_date)}" }, + { name: "Status", value: location.status }, + ] + end end diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 1937394b0..08b6180b3 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -6,6 +6,7 @@ module TagHelper cannot_start_yet: "Cannot start yet", in_progress: "In progress", completed: "Completed", + active: "Active", }.freeze COLOUR = { @@ -13,6 +14,7 @@ module TagHelper cannot_start_yet: "grey", in_progress: "blue", completed: "green", + active: "green", }.freeze def status_tag(status, classes = []) diff --git a/app/models/location.rb b/app/models/location.rb index 40a115d43..0fdc24a8e 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -359,14 +359,6 @@ class Location < ApplicationRecord enum type_of_unit: TYPE_OF_UNIT - def display_attributes - [ - { name: "Location code ", value: location_code, suffix: false }, - { name: "Postcode", value: postcode, suffix: county }, - { name: "Type of unit", value: type_of_unit, suffix: false }, - ] - end - def postcode=(postcode) if postcode super UKPostcode.parse(postcode).to_s @@ -375,6 +367,14 @@ class Location < ApplicationRecord end end + def available_from + startdate || created_at + end + + def status + "active" + end + private PIO = PostcodeService.new diff --git a/app/views/locations/edit_name.html.erb b/app/views/locations/edit_name.html.erb index ece4df866..7e75edf9d 100644 --- a/app/views/locations/edit_name.html.erb +++ b/app/views/locations/edit_name.html.erb @@ -3,7 +3,7 @@ <% content_for :before_content do %> <%= govuk_back_link( text: "Back", - href: "/schemes/#{@scheme.id}/locations", + href: scheme_location_path(@scheme, @location), ) %> <% end %> diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 703fd9fa5..5b2cbae47 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -52,7 +52,7 @@ <%= table.body do |body| %> <%= body.row do |row| %> <% row.cell(text: location.id) %> - <% row.cell(text: simple_format(location_cell_postcode(location, "/schemes/#{@scheme.id}/locations/#{location.id}/edit-name"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> + <% row.cell(text: simple_format(location_cell_postcode(location, "/schemes/#{@scheme.id}/locations/#{location.id}"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> <% row.cell(text: location.units) %> <% row.cell(text: simple_format("#{location.type_of_unit}")) %> <% row.cell(text: location.mobility_type) %> diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb new file mode 100644 index 000000000..fdceb77ad --- /dev/null +++ b/app/views/locations/show.html.erb @@ -0,0 +1,28 @@ +<% title = @location.name %> +<% content_for :title, title %> + +<% content_for :before_content do %> + <%= govuk_back_link( + text: "Back", + href: scheme_locations_path(@scheme), + ) %> +<% end %> + +<%= render partial: "organisations/headings", locals: { main: @location.postcode, sub: @location.name } %> + +
+
+ <%= govuk_summary_list do |summary_list| %> + <% display_attributes(@location).each do |attr| %> + <%= summary_list.row do |row| %> + <% row.key { attr[:name] } %> + <% row.value { attr[:name].eql?("Status") ? status_tag(attr[:value]) : details_html(attr) } %> + <% row.action(text: "Change", href: scheme_location_edit_name_path(scheme_id: @scheme.id, location_id: @location.id)) if attr[:edit] %> + <% end %> + <% end %> + <% end %> +
+
+<% if FeatureToggle.location_toggle_enabled? %> + <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %> +<% end %> diff --git a/app/views/locations/toggle_active.html.erb b/app/views/locations/toggle_active.html.erb new file mode 100644 index 000000000..e69de29bb diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index ca4315e9e..d3ab1db57 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -14,4 +14,10 @@ class FeatureToggle false end + + def self.location_toggle_enabled? + return true unless Rails.env.production? + + false + end end diff --git a/config/routes.rb b/config/routes.rb index eab3d3222..ad088fe11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -53,6 +53,7 @@ Rails.application.routes.draw do resources :locations do get "edit-name", to: "locations#edit_name" get "edit-local-authority", to: "locations#edit_local_authority" + get "deactivate", to: "locations#deactivate" end end diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 35a380fb5..48f386fa7 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -692,7 +692,7 @@ RSpec.describe "Schemes scheme Features" do context "when I click to see individual scheme" do let(:scheme) { schemes.first } - let!(:location) { FactoryBot.create(:location, scheme:) } + let!(:location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 4), scheme:) } before do click_link(scheme.service_name) @@ -757,11 +757,31 @@ RSpec.describe "Schemes scheme Features" do click_link(location.postcode) end - it "shows available fields to edit" do - expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/edit-name") + it "displays details about the selected location" do + expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}") + expect(page).to have_content(location.postcode) + expect(page).to have_content(location.location_admin_district) + expect(page).to have_content(location.name) + expect(page).to have_content(location.units) + expect(page).to have_content(location.type_of_unit) + expect(page).to have_content(location.mobility_type) + expect(page).to have_content(location.location_code) + expect(page).to have_content("Available from 4 April 2022") + expect(page).to have_content("Active") + end + + it "only allows to edit the location name" do + assert_selector "a", text: "Change", count: 1 + + click_link("Change") expect(page).to have_content "Location name for #{location.postcode}" end + it "allows to deactivate a location" do + click_link("Deactivate this location") + expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/deactivate") + end + context "when I press the back button" do before do click_link "Back" @@ -775,15 +795,27 @@ RSpec.describe "Schemes scheme Features" do context "and I change the location name" do before do - fill_in "location-name-field", with: "NewName" - click_button "Save and continue" + click_link("Change") end it "returns to locations check your answers page and shows the new name" do + fill_in "location-name-field", with: "NewName" + click_button "Save and continue" expect(page).to have_content location.id expect(page).to have_content "NewName" expect(page.current_url.split("/").last).to eq("check-answers#locations") end + + context "when I press the back button" do + before do + click_link "Back" + end + + it "I see location details" do + expect(page).to have_content location.name + expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}") + end + end end end diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb index 402772dec..9a4550912 100644 --- a/spec/helpers/locations_helper_spec.rb +++ b/spec/helpers/locations_helper_spec.rb @@ -46,4 +46,31 @@ RSpec.describe LocationsHelper do expect(selection_options(%w[example])).to eq([OpenStruct.new(id: "example", name: "Example")]) end end + + describe "display_attributes" do + let(:location) { FactoryBot.build(:location, startdate: Time.zone.local(2022, 8, 8)) } + + it "returns correct display attributes" do + attributes = [ + { name: "Postcode", value: location.postcode }, + { name: "Local authority", value: location.location_admin_district }, + { name: "Location name", value: location.name, edit: true }, + { name: "Total number of units at this location", value: location.units }, + { name: "Common type of unit", value: location.type_of_unit }, + { name: "Mobility type", value: location.mobility_type }, + { name: "Code", value: location.location_code }, + { name: "Availability", value: "Available from 8 August 2022" }, + { name: "Status", value: "active" }, + ] + + expect(display_attributes(location)).to eq(attributes) + end + + it "displays created_at as availability date if startdate is not present" do + location.update!(startdate: nil) + availability_attribute = display_attributes(location).find { |x| x[:name] == "Availability" }[:value] + + expect(availability_attribute).to eq("Available from #{location.created_at.to_formatted_s(:govuk_date)}") + end + end end From 492ebb2d4bbdb66b98272d40c8cda3a60716c25e Mon Sep 17 00:00:00 2001 From: James Rose Date: Fri, 11 Nov 2022 09:00:51 +0000 Subject: [PATCH 08/18] Add validation to dependent earnings and incfreq fields (#985) We need to add a validation here so that we can clear both values on import. If we don't do this then the validation will continue to trigger and the importer will get stuck in an endless loop. --- app/models/validations/financial_validations.rb | 2 ++ spec/models/validations/financial_validations_spec.rb | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb index b1bddd9cb..87c506b4d 100644 --- a/app/models/validations/financial_validations.rb +++ b/app/models/validations/financial_validations.rb @@ -34,10 +34,12 @@ module Validations::FinancialValidations if record.earnings.present? && record.incfreq.blank? record.errors.add :incfreq, I18n.t("validations.financial.earnings.freq_missing") + record.errors.add :earnings, I18n.t("validations.financial.earnings.freq_missing") end if record.incfreq.present? && record.earnings.blank? record.errors.add :earnings, I18n.t("validations.financial.earnings.earnings_missing") + record.errors.add :incfreq, I18n.t("validations.financial.earnings.earnings_missing") end end diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb index 0676e900b..0b88fdd76 100644 --- a/spec/models/validations/financial_validations_spec.rb +++ b/spec/models/validations/financial_validations_spec.rb @@ -18,6 +18,8 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_net_income(record) expect(record.errors["incfreq"]) .to include(match I18n.t("validations.financial.earnings.freq_missing")) + expect(record.errors["earnings"]) + .to include(match I18n.t("validations.financial.earnings.freq_missing")) end it "when income frequency is provided it validates that earnings must be provided" do @@ -26,6 +28,8 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_net_income(record) expect(record.errors["earnings"]) .to include(match I18n.t("validations.financial.earnings.earnings_missing")) + expect(record.errors["incfreq"]) + .to include(match I18n.t("validations.financial.earnings.earnings_missing")) end end From 1630c386196ad15a811d4edd76da62588f199165 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 11 Nov 2022 10:31:08 +0000 Subject: [PATCH 09/18] Cldc 1668 deactivate location (#981) * add deactivation_date to locations * Change conditional question controller to accommodate all models * UI spike * Update toggle-active views and render them correctly * Display 2 errors * Update errors * Extract text to translation file * Add collection start date * Add out of range validation * Update affected logs label * lint * Add status method * update the displayed status tags * Keep deactivation_date_type selected if an error occurs * refactor deactivation_date_type to use default and other as options instead of 1 and 2 * refactor * refactor * update lettings logs * Add reactivate ocation button and path * Fix controller and update deactivate confirm page * Don't actually update the logs data when deactivating a location * lint and typos * update a path * update current_collection_start_date * Remove unused scope --- app/controllers/locations_controller.rb | 65 +++++- .../conditional_question_controller.js | 6 +- app/helpers/question_attribute_helper.rb | 10 +- app/helpers/tag_helper.rb | 4 + app/models/form_handler.rb | 4 + app/models/location.rb | 7 +- app/views/locations/show.html.erb | 6 +- app/views/locations/toggle_active.html.erb | 39 ++++ .../locations/toggle_active_confirm.html.erb | 16 ++ config/locales/en.yml | 16 ++ config/routes.rb | 2 + ...2033_add_deactivation_date_to_locations.rb | 5 + db/schema.rb | 3 +- spec/helpers/locations_helper_spec.rb | 2 +- .../helpers/question_attribute_helper_spec.rb | 2 +- spec/models/form_handler_spec.rb | 4 + spec/models/location_spec.rb | 32 +++ spec/requests/locations_controller_spec.rb | 185 ++++++++++++++++++ 18 files changed, 396 insertions(+), 12 deletions(-) create mode 100644 app/views/locations/toggle_active_confirm.html.erb create mode 100644 db/migrate/20221109122033_add_deactivation_date_to_locations.rb diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index 797e79544..6cd40161f 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -21,7 +21,23 @@ class LocationsController < ApplicationController def show; end def deactivate - render "toggle_active", locals: { action: "deactivate" } + if params[:location].blank? + render "toggle_active", locals: { action: "deactivate" } + elsif params[:location][:confirm].present? && params[:location][:deactivation_date].present? + confirm_deactivation + else + deactivation_date_errors + if @location.errors.present? + @location.deactivation_date_type = params[:location][:deactivation_date_type] + render "toggle_active", locals: { action: "deactivate" }, status: :unprocessable_entity + else + render "toggle_active_confirm", locals: { action: "deactivate", deactivation_date: } + end + end + end + + def reactivate + render "toggle_active", locals: { action: "reactivate" } end def create @@ -128,7 +144,7 @@ private end def authenticate_action! - if %w[new edit update create index edit_name edit_local_authority].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) + if %w[new edit update create index edit_name edit_local_authority deactivate].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) render_not_found and return end end @@ -146,4 +162,49 @@ private def valid_location_admin_district?(location_params) location_params["location_admin_district"] != "Select an option" end + + def confirm_deactivation + if @location.update(deactivation_date: params[:location][:deactivation_date]) + flash[:notice] = "#{@location.name || @location.postcode} has been deactivated" + end + redirect_to scheme_location_path(@scheme, @location) + end + + def deactivation_date_errors + if params[:location][:deactivation_date].blank? && params[:location][:deactivation_date_type].blank? + @location.errors.add(:deactivation_date_type, message: I18n.t("validations.location.deactivation_date.not_selected")) + end + + if params[:location][:deactivation_date_type] == "other" + day = params[:location]["deactivation_date(3i)"] + month = params[:location]["deactivation_date(2i)"] + year = params[:location]["deactivation_date(1i)"] + + collection_start_date = FormHandler.instance.current_collection_start_date + + if [day, month, year].any?(&:blank?) + { day:, month:, year: }.each do |period, value| + @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.not_entered", period: period.to_s)) if value.blank? + end + elsif !Date.valid_date?(year.to_i, month.to_i, day.to_i) + @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.invalid")) + elsif !Date.new(year.to_i, month.to_i, day.to_i).between?(collection_start_date, Date.new(2200, 1, 1)) + @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.out_of_range", date: collection_start_date.to_formatted_s(:govuk_date))) + end + end + end + + def deactivation_date + return if params[:location].blank? + + collection_start_date = FormHandler.instance.current_collection_start_date + return collection_start_date if params[:location][:deactivation_date_type] == "default" + return params[:location][:deactivation_date] if params[:location][:deactivation_date_type].blank? + + day = params[:location]["deactivation_date(3i)"] + month = params[:location]["deactivation_date(2i)"] + year = params[:location]["deactivation_date(1i)"] + + Date.new(year.to_i, month.to_i, day.to_i) + end end diff --git a/app/frontend/controllers/conditional_question_controller.js b/app/frontend/controllers/conditional_question_controller.js index fb52d07c1..d24d5c6c3 100644 --- a/app/frontend/controllers/conditional_question_controller.js +++ b/app/frontend/controllers/conditional_question_controller.js @@ -10,14 +10,14 @@ export default class extends Controller { const selectedValue = this.element.value const dataInfo = JSON.parse(this.element.dataset.info) const conditionalFor = dataInfo.conditional_questions - const logType = dataInfo.log_type + const type = dataInfo.type Object.entries(conditionalFor).forEach(([targetQuestion, conditions]) => { if (!conditions.map(String).includes(String(selectedValue))) { - const textNumericInput = document.getElementById(`${logType}-log-${targetQuestion.replaceAll('_', '-')}-field`) + const textNumericInput = document.getElementById(`${type}-${targetQuestion.replaceAll('_', '-')}-field`) if (textNumericInput == null) { const dateInputs = [1, 2, 3].map((idx) => { - return document.getElementById(`${logType}_log_${targetQuestion}_${idx}i`) + return document.getElementById(`${type.replaceAll('-', '_')}_${targetQuestion}_${idx}i`) }) this.clearDateInputs(dateInputs) } else { diff --git a/app/helpers/question_attribute_helper.rb b/app/helpers/question_attribute_helper.rb index 857ce5eb1..020ea1909 100644 --- a/app/helpers/question_attribute_helper.rb +++ b/app/helpers/question_attribute_helper.rb @@ -7,6 +7,14 @@ module QuestionAttributeHelper merge_controller_attributes(*attribs) end + def basic_conditional_html_attributes(conditional_for, type) + { + "data-controller": "conditional-question", + "data-action": "click->conditional-question#displayConditional", + "data-info": { conditional_questions: conditional_for, type: type }.to_json, + } + end + private def numeric_question_html_attributes(question) @@ -27,7 +35,7 @@ private { "data-controller": "conditional-question", "data-action": "click->conditional-question#displayConditional", - "data-info": { conditional_questions: question.conditional_for, log_type: question.form.type }.to_json, + "data-info": { conditional_questions: question.conditional_for, type: "#{question.form.type}-log" }.to_json, } end end diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 08b6180b3..2ea23a86a 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -7,6 +7,8 @@ module TagHelper in_progress: "In progress", completed: "Completed", active: "Active", + deactivating_soon: "Deactivating soon", + deactivated: "Deactivated", }.freeze COLOUR = { @@ -15,6 +17,8 @@ module TagHelper in_progress: "blue", completed: "green", active: "green", + deactivating_soon: "yellow", + deactivated: "grey", }.freeze def status_tag(status, classes = []) diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index c6dde13ab..c080e6e9b 100644 --- a/app/models/form_handler.rb +++ b/app/models/form_handler.rb @@ -49,6 +49,10 @@ class FormHandler today < window_end_date ? today.year - 1 : today.year end + def current_collection_start_date + Time.utc(current_collection_start_year, 4, 1) + end + def form_name_from_start_year(year, type) form_mappings = { 0 => "current_#{type}", 1 => "previous_#{type}", -1 => "next_#{type}" } form_mappings[current_collection_start_year - year] diff --git a/app/models/location.rb b/app/models/location.rb index 0fdc24a8e..ab1c3620a 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -2,6 +2,7 @@ class Location < ApplicationRecord validate :validate_postcode validates :units, :type_of_unit, :mobility_type, presence: true belongs_to :scheme + has_many :lettings_logs, class_name: "LettingsLog" has_paper_trail @@ -9,7 +10,7 @@ class Location < ApplicationRecord auto_strip_attributes :name - attr_accessor :add_another_location + attr_accessor :add_another_location, :deactivation_date_type scope :search_by_postcode, ->(postcode) { where("REPLACE(postcode, ' ', '') ILIKE ?", "%#{postcode.delete(' ')}%") } scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } @@ -372,7 +373,9 @@ class Location < ApplicationRecord end def status - "active" + return :active if deactivation_date.blank? + return :deactivating_soon if Time.zone.now < deactivation_date + return :deactivated if Time.zone.now >= deactivation_date end private diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index fdceb77ad..ccc0163c2 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -24,5 +24,9 @@
<% if FeatureToggle.location_toggle_enabled? %> - <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %> + <% if @location.status == :active %> + <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %> + <% else %> + <%= govuk_button_link_to "Reactivate this location", scheme_location_reactivate_path(scheme_id: @scheme.id, location_id: @location.id) %> + <% end %> <% end %> diff --git a/app/views/locations/toggle_active.html.erb b/app/views/locations/toggle_active.html.erb index e69de29bb..96cda6daa 100644 --- a/app/views/locations/toggle_active.html.erb +++ b/app/views/locations/toggle_active.html.erb @@ -0,0 +1,39 @@ +<% title = "#{action.humanize} #{@location.postcode}" %> +<% content_for :title, title %> + +<% content_for :before_content do %> + <%= govuk_back_link( + text: "Back", + href: scheme_location_path(scheme_id: @location.scheme.id, id: @location.id), + ) %> +<% end %> + +<%= form_with model: @location, url: scheme_location_deactivate_path(scheme_id: @location.scheme.id, location_id: @location.id), method: "patch", local: true do |f| %> +
+
+ <% collection_start_date = FormHandler.instance.current_collection_start_date %> + <%= f.govuk_error_summary %> + <%= f.govuk_radio_buttons_fieldset :deactivation_date_type, + legend: { text: I18n.t("questions.location.deactivation.apply_from") }, + caption: { text: "Deactivate #{@location.postcode}" }, + hint: { text: I18n.t("hints.location.deactivation", date: collection_start_date.to_formatted_s(:govuk_date)) } do %> + <%= govuk_warning_text text: I18n.t("warnings.location.deactivation.existing_logs") %> + <%= f.govuk_radio_button :deactivation_date_type, + "default", + label: { text: "From the start of the current collection period (#{collection_start_date.to_formatted_s(:govuk_date)})" } %> + + <%= f.govuk_radio_button :deactivation_date_type, + "other", + label: { text: "For tenancies starting after a certain date" }, + **basic_conditional_html_attributes({ "deactivation_date" => %w[other] }, "location") do %> + <%= f.govuk_date_field :deactivation_date, + legend: { text: "Date", size: "m" }, + hint: { text: "For example, 27 3 2008" }, + width: 20 %> + <% end %> + <% end %> + + <%= f.govuk_submit "Continue" %> +
+
+<% end %> diff --git a/app/views/locations/toggle_active_confirm.html.erb b/app/views/locations/toggle_active_confirm.html.erb new file mode 100644 index 000000000..452d66e48 --- /dev/null +++ b/app/views/locations/toggle_active_confirm.html.erb @@ -0,0 +1,16 @@ +<%= form_with model: @location, url: scheme_location_deactivate_path(@location), method: "patch", local: true do |f| %> + <% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> + <% end %> +

+ <%= @location.postcode %> + <%= "This change will affect #{@location.lettings_logs.count} logs" %> +

+ <%= govuk_warning_text text: I18n.t("warnings.location.deactivation.review_logs") %> + <%= f.hidden_field :confirm, value: true %> + <%= f.hidden_field :deactivation_date, value: deactivation_date %> +
+ <%= f.govuk_submit "Deactivate this location" %> + <%= govuk_button_link_to "Cancel", scheme_location_path(scheme_id: @scheme, id: @location.id), html: { method: :get }, secondary: true %> +
+ <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 9a45d96dd..3c4808696 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -312,6 +312,13 @@ en: declaration: missing: "You must show the DLUHC privacy notice to the tenant before you can submit this log." + location: + deactivation_date: + not_selected: "Select one of the options" + not_entered: "Enter a %{period}" + invalid: "Enter a valid date" + out_of_range: "The date must be on or after the %{date}" + soft_validations: net_income: title_text: "Net income is outside the expected range based on the lead tenant’s working situation" @@ -362,6 +369,8 @@ en: startdate: "When did the first property in this location become available under this scheme? (optional)" add_another_location: "Do you want to add another location?" mobility_type: "What are the mobility standards for the majority of units in this location?" + deactivation: + apply_from: "When should this change apply?" descriptions: location: mobility_type: @@ -374,6 +383,13 @@ en: postcode: "For example, SW1P 4DF." name: "This is how you refer to this location within your organisation" units: "A unit can be a bedroom in a shared house or flat, or a house with 4 bedrooms. Do not include bedrooms used for wardens, managers, volunteers or sleep-in staff." + deactivation: "If the date is before %{date}, select ‘From the start of the current collection period’ because the previous period has now closed." + + warnings: + location: + deactivation: + existing_logs: "It will not be possible to add logs with this location if their tenancy start date is on or after the date you enter. Any existing logs may be affected." + review_logs: "Your data providers will need to review these logs and answer a few questions again. We’ll email each log creator with a list of logs that need updating." test: one_argument: "This is based on the tenant’s work situation: %{ecstat1}" diff --git a/config/routes.rb b/config/routes.rb index ad088fe11..8bd78d6e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,6 +54,8 @@ Rails.application.routes.draw do get "edit-name", to: "locations#edit_name" get "edit-local-authority", to: "locations#edit_local_authority" get "deactivate", to: "locations#deactivate" + get "reactivate", to: "locations#reactivate" + patch "deactivate", to: "locations#deactivate" end end diff --git a/db/migrate/20221109122033_add_deactivation_date_to_locations.rb b/db/migrate/20221109122033_add_deactivation_date_to_locations.rb new file mode 100644 index 000000000..3360bfc7c --- /dev/null +++ b/db/migrate/20221109122033_add_deactivation_date_to_locations.rb @@ -0,0 +1,5 @@ +class AddDeactivationDateToLocations < ActiveRecord::Migration[7.0] + def change + add_column :locations, :deactivation_date, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 43fc94775..ea4b59670 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: 2022_10_19_082625) do +ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -260,6 +260,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_10_19_082625) do t.datetime "startdate", precision: nil t.string "location_admin_district" t.boolean "confirmed" + t.datetime "deactivation_date" t.index ["old_id"], name: "index_locations_on_old_id", unique: true t.index ["scheme_id"], name: "index_locations_on_scheme_id" end diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb index 9a4550912..f2a5a67a5 100644 --- a/spec/helpers/locations_helper_spec.rb +++ b/spec/helpers/locations_helper_spec.rb @@ -60,7 +60,7 @@ RSpec.describe LocationsHelper do { name: "Mobility type", value: location.mobility_type }, { name: "Code", value: location.location_code }, { name: "Availability", value: "Available from 8 August 2022" }, - { name: "Status", value: "active" }, + { name: "Status", value: :active }, ] expect(display_attributes(location)).to eq(attributes) diff --git a/spec/helpers/question_attribute_helper_spec.rb b/spec/helpers/question_attribute_helper_spec.rb index 2be903535..9d769cdf6 100644 --- a/spec/helpers/question_attribute_helper_spec.rb +++ b/spec/helpers/question_attribute_helper_spec.rb @@ -48,7 +48,7 @@ RSpec.describe QuestionAttributeHelper do "data-action": "input->numeric-question#calculateFields click->conditional-question#displayConditional", "data-target": "lettings-log-#{question.result_field.to_s.dasherize}-field", "data-calculated": question.fields_to_add.to_json, - "data-info": { conditional_questions: question.conditional_for, log_type: "lettings" }.to_json, + "data-info": { conditional_questions: question.conditional_for, type: "lettings-log" }.to_json, } end diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index 81a9b1c45..c6a47c107 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -118,6 +118,10 @@ RSpec.describe FormHandler do it "returns the correct next sales form name" do expect(form_handler.form_name_from_start_year(2023, "sales")).to eq("next_sales") end + + it "returns the correct current start date" do + expect(form_handler.current_collection_start_date).to eq(Time.utc(2022, 4, 1)) + end end context "with the date before 1st of April" do diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb index 856575932..8f9f452bc 100644 --- a/spec/models/location_spec.rb +++ b/spec/models/location_spec.rb @@ -111,4 +111,36 @@ RSpec.describe Location, type: :model do end end end + + describe "status" do + let(:location) { FactoryBot.build(:location) } + + before do + Timecop.freeze(2022, 6, 7) + end + + it "returns active if the location is not deactivated" do + location.deactivation_date = nil + location.save! + expect(location.status).to eq(:active) + end + + it "returns deactivating soon if deactivation_date is in the future" do + location.deactivation_date = Time.zone.local(2022, 8, 8) + location.save! + expect(location.status).to eq(:deactivating_soon) + end + + it "returns deactivated if deactivation_date is in the past" do + location.deactivation_date = Time.zone.local(2022, 4, 8) + location.save! + expect(location.status).to eq(:deactivated) + end + + it "returns deactivated if deactivation_date is today" do + location.deactivation_date = Time.zone.local(2022, 6, 7) + location.save! + expect(location.status).to eq(:deactivated) + end + end end diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index 96b3d1b21..feeae2b99 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -1212,4 +1212,189 @@ RSpec.describe LocationsController, type: :request do end end end + + describe "#deactivate" do + context "when not signed in" do + it "redirects to the sign in page" do + patch "/schemes/1/locations/1/deactivate" + expect(response).to redirect_to("/account/sign-in") + end + end + + context "when signed in as a data provider" do + let(:user) { FactoryBot.create(:user) } + + before do + sign_in user + patch "/schemes/1/locations/1/deactivate" + end + + it "returns 401 unauthorized" do + request + expect(response).to have_http_status(:unauthorized) + end + end + + context "when signed in as a data coordinator" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:location) { FactoryBot.create(:location, scheme:) } + let(:startdate) { Time.utc(2021, 1, 2) } + let(:deactivation_date) { Time.utc(2022, 10, 10) } + + before do + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params: + end + + context "with default date" do + let(:params) { { location: { deactivation_date_type: "default" } } } + + it "renders the confirmation page" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("This change will affect #{location.lettings_logs.count} logs") + end + end + + context "with other date" do + let(:params) { { location: { deactivation_date: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "10", "deactivation_date(1i)": "2022" } } } + + it "renders the confirmation page" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("This change will affect #{location.lettings_logs.count} logs") + end + end + + context "when confirming deactivation" do + let(:params) { { location: { deactivation_date:, confirm: true } } } + + it "updates existing location with valid deactivation date and renders location page" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + location.reload + expect(location.deactivation_date).to eq(deactivation_date) + end + end + + context "when the date is not selected" do + let(:params) { { location: { "deactivation_date": "" } } } + + it "displays the new page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_selected")) + end + end + + context "when invalid date is entered" do + let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "44", "deactivation_date(1i)": "2022" } } } + + it "displays the new page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.invalid")) + end + end + + context "when the date is entered is before the beginning of current collection window" do + let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "4", "deactivation_date(1i)": "2020" } } } + + it "displays the new page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.out_of_range", date: "1 April 2022")) + end + end + + context "when the day is not entered" do + let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "", "deactivation_date(2i)": "2", "deactivation_date(1i)": "2022" } } } + + it "displays page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "day")) + end + end + + context "when the month is not entered" do + let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "2", "deactivation_date(2i)": "", "deactivation_date(1i)": "2022" } } } + + it "displays page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "month")) + end + end + + context "when the year is not entered" do + let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "2", "deactivation_date(2i)": "2", "deactivation_date(1i)": "" } } } + + it "displays page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "year")) + end + end + end + end + + describe "#show" do + context "when not signed in" do + it "redirects to the sign in page" do + get "/schemes/1/locations/1" + expect(response).to redirect_to("/account/sign-in") + end + end + + context "when signed in as a data provider" do + let(:user) { FactoryBot.create(:user) } + + before do + sign_in user + get "/schemes/1/locations/1" + end + + it "returns 401 unauthorized" do + request + expect(response).to have_http_status(:unauthorized) + end + end + + context "when signed in as a data coordinator" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:location) { FactoryBot.create(:location, scheme:) } + + before do + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + location.deactivation_date = deactivation_date + location.save! + get "/schemes/#{scheme.id}/locations/#{location.id}" + end + + context "with active location" do + let(:deactivation_date) { nil } + + it "renders deactivate this location" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Deactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/deactivate") + end + end + + context "with deactivated location" do + let(:deactivation_date) { Time.utc(2022, 10, 9) } + + it "renders reactivate this location" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Reactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/reactivate") + end + end + + context "with location that's deactivating soon" do + let(:deactivation_date) { Time.utc(2022, 10, 12) } + + it "renders reactivate this location" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Reactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/reactivate") + end + end + end + end end From 8f9886cbc8538a99ed334cbe22244c96d6cb7cf4 Mon Sep 17 00:00:00 2001 From: James Rose Date: Fri, 11 Nov 2022 11:30:15 +0000 Subject: [PATCH 10/18] Change type of legacy old_visible_id fields to strings (#986) These fields are set using IDs from the previous CORE service. There was an incorrect assumption that these fields were integers so we were casting them as such. We have discovered that these fields are strings (e.g., `027`) so this change adjusts our types. --- .../imports/lettings_logs_import_service.rb | 6 +++--- .../imports/scheme_location_import_service.rb | 2 +- .../20221111102656_change_old_visible_id_type.rb | 13 +++++++++++++ db/schema.rb | 8 ++++---- spec/factories/location.rb | 2 +- .../logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml | 2 +- .../6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml | 2 +- .../imports/lettings_logs_import_service_spec.rb | 10 +++++----- .../imports/organisation_import_service_spec.rb | 10 +++++----- spec/services/imports/scheme_import_service_spec.rb | 4 ++-- .../imports/scheme_location_import_service_spec.rb | 2 +- 11 files changed, 37 insertions(+), 24 deletions(-) create mode 100644 db/migrate/20221111102656_change_old_visible_id_type.rb diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index 1aecc4f81..ab15e7f3d 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -205,8 +205,8 @@ module Imports # Supported Housing fields if attributes["needstype"] == GN_SH[:supported_housing] - location_old_visible_id = safe_string_as_integer(xml_doc, "_1cschemecode") - scheme_old_visible_id = safe_string_as_integer(xml_doc, "_1cmangroupcode") + location_old_visible_id = string_or_nil(xml_doc, "_1cschemecode") + scheme_old_visible_id = string_or_nil(xml_doc, "_1cmangroupcode") schemes = Scheme.where(old_visible_id: scheme_old_visible_id, owning_organisation_id: attributes["owning_organisation_id"]) location = Location.find_by(old_visible_id: location_old_visible_id, scheme: schemes) @@ -399,7 +399,7 @@ module Imports end def find_organisation_id(xml_doc, id_field) - old_visible_id = unsafe_string_as_integer(xml_doc, id_field) + old_visible_id = string_or_nil(xml_doc, id_field) organisation = Organisation.find_by(old_visible_id:) raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil? diff --git a/app/services/imports/scheme_location_import_service.rb b/app/services/imports/scheme_location_import_service.rb index 4214555cc..837be0b49 100644 --- a/app/services/imports/scheme_location_import_service.rb +++ b/app/services/imports/scheme_location_import_service.rb @@ -91,7 +91,7 @@ module Imports attributes["units"] = safe_string_as_integer(xml_doc, "total-units") attributes["type_of_unit"] = safe_string_as_integer(xml_doc, "unit-type") attributes["location_old_id"] = string_or_nil(xml_doc, "id") - attributes["location_old_visible_id"] = safe_string_as_integer(xml_doc, "visible-id") + attributes["location_old_visible_id"] = string_or_nil(xml_doc, "visible-id") attributes["scheme_old_id"] = string_or_nil(xml_doc, "mgmtgroup") attributes end diff --git a/db/migrate/20221111102656_change_old_visible_id_type.rb b/db/migrate/20221111102656_change_old_visible_id_type.rb new file mode 100644 index 000000000..d4c568bb7 --- /dev/null +++ b/db/migrate/20221111102656_change_old_visible_id_type.rb @@ -0,0 +1,13 @@ +class ChangeOldVisibleIdType < ActiveRecord::Migration[7.0] + def up + change_column :organisations, :old_visible_id, :string + change_column :schemes, :old_visible_id, :string + change_column :locations, :old_visible_id, :string + end + + def down + change_column :organisations, :old_visible_id, :integer + change_column :schemes, :old_visible_id, :integer + change_column :locations, :old_visible_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index ea4b59670..5faff28ba 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: 2022_11_09_122033) do +ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -255,7 +255,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do t.integer "units" t.integer "type_of_unit" t.string "old_id" - t.integer "old_visible_id" + t.string "old_visible_id" t.string "mobility_type" t.datetime "startdate", precision: nil t.string "location_admin_district" @@ -316,7 +316,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do t.integer "supported_housing_units" t.integer "unspecified_units" t.string "old_org_id" - t.integer "old_visible_id" + t.string "old_visible_id" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end @@ -395,7 +395,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do t.bigint "managing_organisation_id" t.string "arrangement_type" t.string "old_id" - t.integer "old_visible_id" + t.string "old_visible_id" t.integer "total_units" t.boolean "confirmed" t.index ["managing_organisation_id"], name: "index_schemes_on_managing_organisation_id" diff --git a/spec/factories/location.rb b/spec/factories/location.rb index a8418f8ba..3359f64cd 100644 --- a/spec/factories/location.rb +++ b/spec/factories/location.rb @@ -17,7 +17,7 @@ FactoryBot.define do units { 20 } mobility_type { "A" } scheme { FactoryBot.create(:scheme, :export) } - old_visible_id { 111 } + old_visible_id { "111" } end end end diff --git a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml index 59c1eada7..3faacf28b 100644 --- a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml +++ b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml @@ -19,7 +19,7 @@
123456
2 Local Authority - <_1cmangroupcode>123 + <_1cmangroupcode>0123 <_1cschemecode>10 3 No diff --git a/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml b/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml index 5a7b23b5b..712d3f6e5 100644 --- a/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml +++ b/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml @@ -5,5 +5,5 @@ 456 7c5bd5fb549c09z2c55d9cb90d7ba84927e64618 Approved - 123 + 0123 diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index 576249971..453ee7e30 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -11,8 +11,8 @@ RSpec.describe Imports::LettingsLogsImportService do let(:fixture_directory) { "spec/fixtures/imports/logs" } let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") } - let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) } - let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: 456, owning_organisation: organisation) } + let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: "0123", owning_organisation: organisation) } + let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: "456", owning_organisation: organisation) } def open_file(directory, filename) File.open("#{directory}/#{filename}.xml") @@ -23,16 +23,16 @@ RSpec.describe Imports::LettingsLogsImportService do .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {}) allow(Organisation).to receive(:find_by).and_return(nil) - allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id.to_i).and_return(organisation) + allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id).and_return(organisation) # Created by users FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:) FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f", organisation:) # Location setup - FactoryBot.create(:location, old_visible_id: 10, postcode: "LS166FT", scheme_id: scheme1.id, mobility_type: "W") + FactoryBot.create(:location, old_visible_id: "10", postcode: "LS166FT", scheme_id: scheme1.id, mobility_type: "W") FactoryBot.create(:location, scheme_id: scheme1.id) - FactoryBot.create(:location, old_visible_id: 10, postcode: "LS166FT", scheme_id: scheme2.id, mobility_type: "W") + FactoryBot.create(:location, old_visible_id: "10", postcode: "LS166FT", scheme_id: scheme2.id, mobility_type: "W") # Stub the form handler to use the real form allow(FormHandler.instance).to receive(:get_form).with("previous_lettings").and_return(real_2021_2022_form) diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb index 48a0d72bb..c8fcd4829 100644 --- a/spec/services/imports/organisation_import_service_spec.rb +++ b/spec/services/imports/organisation_import_service_spec.rb @@ -32,7 +32,7 @@ RSpec.describe Imports::OrganisationImportService do it "successfully create an organisation with the expected data" do import_service.create_organisations(folder_name) - organisation = Organisation.find_by(old_visible_id: 1) + organisation = Organisation.find_by(old_visible_id: "1") expect(organisation.name).to eq("HA Ltd") expect(organisation.provider_type).to eq("PRP") expect(organisation.phone).to eq("xxxxxxxx") @@ -53,7 +53,7 @@ RSpec.describe Imports::OrganisationImportService do expect(organisation.unspecified_units).to eq(0) expect(organisation.unspecified_units).to eq(0) expect(organisation.old_org_id).to eq("7c5bd5fb549c09z2c55d9cb90d7ba84927e64618") - expect(organisation.old_visible_id).to eq(1) + expect(organisation.old_visible_id).to eq("1") end it "successfully create multiple organisations" do @@ -62,8 +62,8 @@ RSpec.describe Imports::OrganisationImportService do expect(storage_service).to receive(:get_file_io).with(filenames[1]).ordered expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(2) - expect(Organisation).to exist(old_visible_id: 1) - expect(Organisation).to exist(old_visible_id: 2) + expect(Organisation).to exist(old_visible_id: "1") + expect(Organisation).to exist(old_visible_id: "2") end end @@ -86,7 +86,7 @@ RSpec.describe Imports::OrganisationImportService do expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(1) expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(0) - expect(Organisation).to exist(old_visible_id: 1, name: "HA Ltd") + expect(Organisation).to exist(old_visible_id: "1", name: "HA Ltd") end end end diff --git a/spec/services/imports/scheme_import_service_spec.rb b/spec/services/imports/scheme_import_service_spec.rb index efba81a1c..97e9359b5 100644 --- a/spec/services/imports/scheme_import_service_spec.rb +++ b/spec/services/imports/scheme_import_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Imports::SchemeImportService do let(:scheme_id) { "6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d" } let!(:owning_org) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618") } - let!(:managing_org) { FactoryBot.create(:organisation, old_visible_id: 456) } + let!(:managing_org) { FactoryBot.create(:organisation, old_visible_id: "456") } def open_file(directory, filename) File.open("#{directory}/#{filename}.xml") @@ -46,7 +46,7 @@ RSpec.describe Imports::SchemeImportService do expect(scheme.owning_organisation).to eq(owning_org) expect(scheme.managing_organisation).to eq(managing_org) expect(scheme.old_id).to eq("6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d") - expect(scheme.old_visible_id).to eq(123) + expect(scheme.old_visible_id).to eq("0123") expect(scheme.service_name).to eq("Management Group") expect(scheme.arrangement_type).to eq("Another organisation") end diff --git a/spec/services/imports/scheme_location_import_service_spec.rb b/spec/services/imports/scheme_location_import_service_spec.rb index 8b93d4969..c11c2ca2d 100644 --- a/spec/services/imports/scheme_location_import_service_spec.rb +++ b/spec/services/imports/scheme_location_import_service_spec.rb @@ -142,7 +142,7 @@ RSpec.describe Imports::SchemeLocationImportService do expect(location.mobility_type).to eq("Fitted with equipment and adaptations") expect(location.type_of_unit).to eq("Bungalow") expect(location.old_id).to eq(first_location_id) - expect(location.old_visible_id).to eq(10) + expect(location.old_visible_id).to eq("10") expect(location.startdate).to eq("1900-01-01") expect(location.scheme).to eq(scheme) end From 0e38ebd4b651c925968de1e3eed132f5bf87649f Mon Sep 17 00:00:00 2001 From: James Rose Date: Mon, 14 Nov 2022 11:39:27 +0000 Subject: [PATCH 11/18] Parse user fields where namespace declarations aren't included (#989) We import XML files that define logs from the previous CORE service. For an unknown reason, some of these files don't include the required namespace declarations to parse `user` values. Instead of changing the source, this change introduces a new method that forces the namespace declaration for user fields. --- app/services/imports/user_import_service.rb | 2 +- ...d9b73aa1c88b6e5c36ee49be9050b923b4a1bb.xml | 19 +++++++++++++++++++ .../imports/user_import_service_spec.rb | 13 +++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 spec/fixtures/imports/user/80d9b73aa1c88b6e5c36ee49be9050b923b4a1bb.xml diff --git a/app/services/imports/user_import_service.rb b/app/services/imports/user_import_service.rb index f5a730479..9ab8f6886 100644 --- a/app/services/imports/user_import_service.rb +++ b/app/services/imports/user_import_service.rb @@ -45,7 +45,7 @@ module Imports end def user_field_value(xml_document, field) - field_value(xml_document, "user", field) + field_value(xml_document, "user", field, { "user" => "dclg:user" }) end def role(field_value) diff --git a/spec/fixtures/imports/user/80d9b73aa1c88b6e5c36ee49be9050b923b4a1bb.xml b/spec/fixtures/imports/user/80d9b73aa1c88b6e5c36ee49be9050b923b4a1bb.xml new file mode 100644 index 000000000..8fd86a5b1 --- /dev/null +++ b/spec/fixtures/imports/user/80d9b73aa1c88b6e5c36ee49be9050b923b4a1bb.xml @@ -0,0 +1,19 @@ + + 80d9b73aa1c88b6e5c36ee49be9050b923b4a1bb + Jane Doe + 7c5bd5fb549c09a2c55d7cb90d7ba84927e64618 + 1158 1158 + true + Data Provider + + false + false + false + None + jane.doe@gov.uk + REDACTED + Jane Doe + 01111 000000 + false + 2021-09-28Z + \ No newline at end of file diff --git a/spec/services/imports/user_import_service_spec.rb b/spec/services/imports/user_import_service_spec.rb index b8a831dbc..ea3388169 100644 --- a/spec/services/imports/user_import_service_spec.rb +++ b/spec/services/imports/user_import_service_spec.rb @@ -107,6 +107,19 @@ RSpec.describe Imports::UserImportService do end end + context "when the user does not have namespace bindings" do + let(:old_user_id) { "80d9b73aa1c88b6e5c36ee49be9050b923b4a1bb" } + + it "imports them succesfully" do + FactoryBot.create(:organisation, old_org_id:) + import_service.create_users("user_directory") + + user = LegacyUser.find_by(old_user_id:)&.user + expect(user.name).to eq("Jane Doe") + expect(user.email).to eq("jane.doe@gov.uk") + end + end + context "when a user has already been imported with that email" do let!(:org) { FactoryBot.create(:organisation, old_org_id:) } let!(:user) { FactoryBot.create(:user, :data_provider, organisation: org, email: "john.doe@gov.uk") } From aa041784ddda68208dc76c63f84dc78dc8c16ac7 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 14 Nov 2022 11:44:07 +0000 Subject: [PATCH 12/18] Update routing to go back to location page (#988) --- app/controllers/locations_controller.rb | 6 +++++- app/helpers/check_answers_helper.rb | 2 +- spec/features/schemes_spec.rb | 4 ++-- spec/requests/locations_controller_spec.rb | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index 6cd40161f..7955ec944 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -98,7 +98,11 @@ class LocationsController < ApplicationController redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) end when "edit-name" - redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) + if @scheme.locations.count == @scheme.locations.active.count + redirect_to(scheme_location_path(@scheme, @location)) + else + redirect_to(scheme_check_answers_path(@scheme, anchor: "locations")) + end when "edit-local-authority" if params[:add_another_location] == "Yes" redirect_to(new_scheme_location_path(@location.scheme)) diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 1583b3791..1d4a1a9aa 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -18,7 +18,7 @@ module CheckAnswersHelper def get_location_change_link_href_postcode(scheme, location) if location.confirmed? - scheme_location_edit_name_path(scheme_id: scheme.id, location_id: location.id) + scheme_location_path(scheme_id: scheme.id, id: location.id) else edit_scheme_location_path(scheme_id: scheme.id, id: location.id) end diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 48f386fa7..48ab31acc 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -801,9 +801,9 @@ RSpec.describe "Schemes scheme Features" do it "returns to locations check your answers page and shows the new name" do fill_in "location-name-field", with: "NewName" click_button "Save and continue" - expect(page).to have_content location.id + expect(page).to have_content location.postcode expect(page).to have_content "NewName" - expect(page.current_url.split("/").last).to eq("check-answers#locations") + expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}") end context "when I press the back button" do diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index feeae2b99..a666b6495 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -597,7 +597,7 @@ RSpec.describe LocationsController, type: :request do it "updates existing location for scheme with valid params and redirects to correct page" do follow_redirect! expect(response).to have_http_status(:ok) - expect(page).to have_content("Locations") + expect(page).to have_content("Test") end it "updates existing location for scheme with valid params" do @@ -736,7 +736,7 @@ RSpec.describe LocationsController, type: :request do it "updates existing location for scheme with valid params and redirects to correct page" do follow_redirect! expect(response).to have_http_status(:ok) - expect(page).to have_content("Locations") + expect(page).to have_content("Test") end it "updates existing location for scheme with valid params" do From ecf1fa1a2822ba4aba2dc3b549683d8344175618 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 15 Nov 2022 14:04:05 +0000 Subject: [PATCH 13/18] Update enabled method (#990) * Change enabled? subsection method to check depends on using log questions and methods instead of a subsection status * Update sales subsections * Update rent validation tests to check all the affected fields * Update setup_completed? to check all the setup sections --- .../subsections/household_characteristics.rb | 2 +- .../form/sales/subsections/household_needs.rb | 2 +- .../income_benefits_and_outgoings.rb | 2 +- .../sales/subsections/property_information.rb | 2 +- app/models/form/subsection.rb | 6 +- app/models/lettings_log.rb | 4 ++ app/models/log.rb | 4 ++ app/models/sales_log.rb | 4 ++ config/forms/2021_2022.json | 12 ++-- config/forms/2022_2023.json | 20 +++---- spec/fixtures/forms/2021_2022.json | 7 ++- spec/fixtures/forms/2022_2023.json | 2 +- .../household_characteristics_spec.rb | 2 +- .../income_benefits_and_outgoings_spec.rb | 2 +- .../validations/financial_validations_spec.rb | 60 +++++++------------ 15 files changed, 60 insertions(+), 71 deletions(-) diff --git a/app/models/form/sales/subsections/household_characteristics.rb b/app/models/form/sales/subsections/household_characteristics.rb index fb2b7dc6e..5a8eeef83 100644 --- a/app/models/form/sales/subsections/household_characteristics.rb +++ b/app/models/form/sales/subsections/household_characteristics.rb @@ -4,7 +4,7 @@ class Form::Sales::Subsections::HouseholdCharacteristics < ::Form::Subsection @id = "household_characteristics" @label = "Household characteristics" @section = section - @depends_on = [{ "setup" => "completed" }] + @depends_on = [{ "setup_completed?" => true }] end def pages diff --git a/app/models/form/sales/subsections/household_needs.rb b/app/models/form/sales/subsections/household_needs.rb index bf7a5df3a..a85cadcf3 100644 --- a/app/models/form/sales/subsections/household_needs.rb +++ b/app/models/form/sales/subsections/household_needs.rb @@ -4,7 +4,7 @@ class Form::Sales::Subsections::HouseholdNeeds < ::Form::Subsection @id = "household_needs" @label = "Household needs" @section = section - @depends_on = [{ "setup" => "completed" }] + @depends_on = [{ "setup_completed?" => true }] end def pages diff --git a/app/models/form/sales/subsections/income_benefits_and_outgoings.rb b/app/models/form/sales/subsections/income_benefits_and_outgoings.rb index 2c9d779b4..6ec5c6488 100644 --- a/app/models/form/sales/subsections/income_benefits_and_outgoings.rb +++ b/app/models/form/sales/subsections/income_benefits_and_outgoings.rb @@ -4,7 +4,7 @@ class Form::Sales::Subsections::IncomeBenefitsAndOutgoings < ::Form::Subsection @id = "income_benefits_and_outgoings" @label = "Income, benefits and outgoings" @section = section - @depends_on = [{ "setup" => "completed" }] + @depends_on = [{ "setup_completed?" => true }] end def pages diff --git a/app/models/form/sales/subsections/property_information.rb b/app/models/form/sales/subsections/property_information.rb index 5ebe7b581..2a29e27c2 100644 --- a/app/models/form/sales/subsections/property_information.rb +++ b/app/models/form/sales/subsections/property_information.rb @@ -4,7 +4,7 @@ class Form::Sales::Subsections::PropertyInformation < ::Form::Subsection @id = "property_information" @label = "Property information" @section = section - @depends_on = [{ "setup" => "completed" }] + @depends_on = [{ "setup_completed?" => true }] end def pages diff --git a/app/models/form/subsection.rb b/app/models/form/subsection.rb index d3ab0ddd2..07e7fe65c 100644 --- a/app/models/form/subsection.rb +++ b/app/models/form/subsection.rb @@ -20,11 +20,7 @@ class Form::Subsection def enabled?(log) return true unless depends_on - depends_on.any? do |conditions_set| - conditions_set.all? do |subsection_id, dependent_status| - form.get_subsection(subsection_id).status(log) == dependent_status.to_sym - end - end + form.depends_on_met(depends_on, log) end def status(log) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index f09fd1ebd..bc011fdfd 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -507,6 +507,10 @@ class LettingsLog < Log form.get_question("rent_type", self)&.label_from_value(rent_type) end + def non_location_setup_questions_completed? + [needstype, renewal, rent_type, startdate, owning_organisation_id, created_by_id].all?(&:present?) + end + private PIO = PostcodeService.new diff --git a/app/models/log.rb b/app/models/log.rb index 54770515f..abde757e5 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -40,6 +40,10 @@ class Log < ApplicationRecord ethnic_group == 17 end + def managing_organisation_provider_type + managing_organisation&.provider_type + end + private def update_status! diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index e1458caa9..7c352aa78 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -47,4 +47,8 @@ class SalesLog < Log def completed? status == "completed" end + + def setup_completed? + form.setup_sections.all? { |sections| sections.subsections.all? { |subsection| subsection.status(self) == :completed } } + end end diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index a180bcc67..19d8c291d 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -10,7 +10,7 @@ "label": "Property information", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -952,7 +952,7 @@ "label": "Tenancy information", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -1133,7 +1133,7 @@ "label": "Household characteristics", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -5671,7 +5671,7 @@ "label": "Household needs", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -6060,7 +6060,7 @@ "label": "Household situation", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -7302,7 +7302,7 @@ "label": "Income, benefits and outgoings", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json index 76595bdc5..2c6d0763f 100644 --- a/config/forms/2022_2023.json +++ b/config/forms/2022_2023.json @@ -10,7 +10,7 @@ "label": "Property information", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -952,7 +952,7 @@ "label": "Tenancy information", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -1168,7 +1168,7 @@ "label": "Household characteristics", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -5670,7 +5670,7 @@ "label": "Household needs", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -6062,7 +6062,7 @@ "label": "Household situation", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { @@ -7070,7 +7070,7 @@ }, "depends_on": [ { - "managing_organisation.provider_type": "LA", + "managing_organisation_provider_type": "LA", "needstype": 1, "renewal": 0 } @@ -7127,7 +7127,7 @@ }, "depends_on": [ { - "managing_organisation.provider_type": "PRP", + "managing_organisation_provider_type": "PRP", "needstype": 1, "renewal": 0 } @@ -7184,7 +7184,7 @@ }, "depends_on": [ { - "managing_organisation.provider_type": "LA", + "managing_organisation_provider_type": "LA", "needstype": 2, "renewal": 0 } @@ -7244,7 +7244,7 @@ }, "depends_on": [ { - "managing_organisation.provider_type": "PRP", + "managing_organisation_provider_type": "PRP", "needstype": 2, "renewal": 0 } @@ -7261,7 +7261,7 @@ "label": "Income, benefits and outgoings", "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ], "pages": { diff --git a/spec/fixtures/forms/2021_2022.json b/spec/fixtures/forms/2021_2022.json index ac02a62a2..8e73f645b 100644 --- a/spec/fixtures/forms/2021_2022.json +++ b/spec/fixtures/forms/2021_2022.json @@ -1099,9 +1099,10 @@ "label": "Declaration", "depends_on": [ { - "household_characteristics": "completed", - "household_needs": "completed", - "property_information": "completed" + "armedforces": 1 + }, + { + "armedforces": 3 } ], "pages": { diff --git a/spec/fixtures/forms/2022_2023.json b/spec/fixtures/forms/2022_2023.json index a704a9229..5f5671b6f 100644 --- a/spec/fixtures/forms/2022_2023.json +++ b/spec/fixtures/forms/2022_2023.json @@ -47,7 +47,7 @@ }, "depends_on": [ { - "setup": "completed" + "non_location_setup_questions_completed?": true } ] } diff --git a/spec/models/form/sales/subsections/household_characteristics_spec.rb b/spec/models/form/sales/subsections/household_characteristics_spec.rb index dbe8a24a3..7aec7d56a 100644 --- a/spec/models/form/sales/subsections/household_characteristics_spec.rb +++ b/spec/models/form/sales/subsections/household_characteristics_spec.rb @@ -53,6 +53,6 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model end it "has correct depends on" do - expect(household_characteristics.depends_on).to eq([{ "setup" => "completed" }]) + expect(household_characteristics.depends_on).to eq([{ "setup_completed?" => true }]) end end diff --git a/spec/models/form/sales/subsections/income_benefits_and_outgoings_spec.rb b/spec/models/form/sales/subsections/income_benefits_and_outgoings_spec.rb index add0a6952..01028b0a8 100644 --- a/spec/models/form/sales/subsections/income_benefits_and_outgoings_spec.rb +++ b/spec/models/form/sales/subsections/income_benefits_and_outgoings_spec.rb @@ -28,6 +28,6 @@ RSpec.describe Form::Sales::Subsections::IncomeBenefitsAndOutgoings, type: :mode end it "has correct depends on" do - expect(subsection.depends_on).to eq([{ "setup" => "completed" }]) + expect(subsection.depends_on).to eq([{ "setup_completed?" => true }]) end end diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb index 0b88fdd76..409ed86c2 100644 --- a/spec/models/validations/financial_validations_spec.rb +++ b/spec/models/validations/financial_validations_spec.rb @@ -832,6 +832,11 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_rent_amount(record) expect(record.errors["brent"]) .to include(match I18n.t("validations.financial.brent.below_hard_min")) + + %w[beds la postcode_known scheme_id location_id rent_type needstype period].each do |field| + expect(record.errors[field]) + .to include(match I18n.t("validations.financial.brent.#{field}.below_hard_min")) + end end it "validates hard max for general needs" do @@ -846,22 +851,11 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_rent_amount(record) expect(record.errors["brent"]) .to include(match I18n.t("validations.financial.brent.above_hard_max")) - expect(record.errors["beds"]) - .to include(match I18n.t("validations.financial.brent.beds.above_hard_max")) - expect(record.errors["la"]) - .to include(match I18n.t("validations.financial.brent.la.above_hard_max")) - expect(record.errors["postcode_known"]) - .to include(match I18n.t("validations.financial.brent.postcode_known.above_hard_max")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.financial.brent.scheme_id.above_hard_max")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.financial.brent.location_id.above_hard_max")) - expect(record.errors["rent_type"]) - .to include(match I18n.t("validations.financial.brent.rent_type.above_hard_max")) - expect(record.errors["needstype"]) - .to include(match I18n.t("validations.financial.brent.needstype.above_hard_max")) - expect(record.errors["period"]) - .to include(match I18n.t("validations.financial.brent.period.above_hard_max")) + + %w[beds la postcode_known scheme_id location_id rent_type needstype period].each do |field| + expect(record.errors[field]) + .to include(match I18n.t("validations.financial.brent.#{field}.above_hard_max")) + end end it "validates hard max for supported housing" do @@ -875,22 +869,11 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_rent_amount(record) expect(record.errors["brent"]) .to include(match I18n.t("validations.financial.brent.above_hard_max")) - expect(record.errors["beds"]) - .to include(match I18n.t("validations.financial.brent.beds.above_hard_max")) - expect(record.errors["la"]) - .to include(match I18n.t("validations.financial.brent.la.above_hard_max")) - expect(record.errors["postcode_known"]) - .to include(match I18n.t("validations.financial.brent.postcode_known.above_hard_max")) - expect(record.errors["scheme_id"]) - .to include(match I18n.t("validations.financial.brent.scheme_id.above_hard_max")) - expect(record.errors["location_id"]) - .to include(match I18n.t("validations.financial.brent.location_id.above_hard_max")) - expect(record.errors["rent_type"]) - .to include(match I18n.t("validations.financial.brent.rent_type.above_hard_max")) - expect(record.errors["needstype"]) - .to include(match I18n.t("validations.financial.brent.needstype.above_hard_max")) - expect(record.errors["period"]) - .to include(match I18n.t("validations.financial.brent.period.above_hard_max")) + + %w[beds la postcode_known scheme_id location_id rent_type needstype period].each do |field| + expect(record.errors[field]) + .to include(match I18n.t("validations.financial.brent.#{field}.above_hard_max")) + end end it "validates hard max for correct collection year" do @@ -904,14 +887,11 @@ RSpec.describe Validations::FinancialValidations do financial_validator.validate_rent_amount(record) expect(record.errors["brent"]) .to include(match I18n.t("validations.financial.brent.above_hard_max")) - expect(record.errors["beds"]) - .to include(match I18n.t("validations.financial.brent.beds.above_hard_max")) - expect(record.errors["la"]) - .to include(match I18n.t("validations.financial.brent.la.above_hard_max")) - expect(record.errors["rent_type"]) - .to include(match I18n.t("validations.financial.brent.rent_type.above_hard_max")) - expect(record.errors["needstype"]) - .to include(match I18n.t("validations.financial.brent.needstype.above_hard_max")) + + %w[beds la postcode_known scheme_id location_id rent_type needstype period].each do |field| + expect(record.errors[field]) + .to include(match I18n.t("validations.financial.brent.#{field}.above_hard_max")) + end end it "does not error if some of the fields are missing" do From 719f9bd8cc66d5b8c7c7957ff4a6db04700ae053 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 16 Nov 2022 09:23:25 +0000 Subject: [PATCH 14/18] fix broken link in docs (#991) --- docs/form/builder.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/form/builder.md b/docs/form/builder.md index cd703b5f5..97d1e67a9 100644 --- a/docs/form/builder.md +++ b/docs/form/builder.md @@ -9,7 +9,7 @@ nav_order: 1 The setup this log section is treated slightly differently from the rest of the form. It is more accurately viewed as providing metadata about the form than as being part of the form itself. It also needs to know far more about the application specific context than other parts of the form such as who the current user is, what organisation they’re part of and what role they have etc. -As a result it’s not modelled as part of the config but rather as code. It still uses the same [Form Runner](runner) components though. +As a result it’s not modelled as part of the config but rather as code. It still uses the same [Form Runner](/form/runner) components though. ## Features the Form Config supports From ad0e3febcca15fcdf9452dded1884fba6ca80c23 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 16 Nov 2022 09:24:48 +0000 Subject: [PATCH 15/18] Add Parallel tests (#992) * able to run tests in parallel * update docs on parallel testing --- .rspec_parallel | 2 ++ Gemfile | 2 ++ Gemfile.lock | 4 ++++ config/database.yml | 2 +- config/environments/test.rb | 1 + docs/testing.md | 16 ++++++++++++++++ 6 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .rspec_parallel diff --git a/.rspec_parallel b/.rspec_parallel new file mode 100644 index 000000000..aaff198a3 --- /dev/null +++ b/.rspec_parallel @@ -0,0 +1,2 @@ +--format progress +--format ParallelTests::RSpec::RuntimeLogger --out tmp/parallel_runtime_rspec.log diff --git a/Gemfile b/Gemfile index e6dac1995..02bcdf3b7 100644 --- a/Gemfile +++ b/Gemfile @@ -68,6 +68,8 @@ group :development, :test do gem "byebug", platforms: %i[mri mingw x64_mingw] gem "dotenv-rails" gem "pry-byebug" + + gem "parallel_tests" end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 2a9fb454f..b509fb950 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -247,6 +247,8 @@ GEM globalid paper_trail (>= 3.0.0) parallel (1.22.1) + parallel_tests (4.0.0) + parallel parser (3.1.2.1) ast (~> 2.4.1) pg (1.4.3) @@ -430,6 +432,7 @@ PLATFORMS x86_64-darwin-19 x86_64-darwin-20 x86_64-darwin-21 + x86_64-darwin-22 x86_64-linux DEPENDENCIES @@ -456,6 +459,7 @@ DEPENDENCIES overcommit (>= 0.37.0) paper_trail paper_trail-globalid + parallel_tests pg (~> 1.1) possessive postcodes_io diff --git a/config/database.yml b/config/database.yml index 74e6632ef..c09e3600a 100644 --- a/config/database.yml +++ b/config/database.yml @@ -66,7 +66,7 @@ staging: # Do not set this db to the same as development or production. test: <<: *default - database: data_collector_test + database: data_collector_test<%= ENV['TEST_ENV_NUMBER'] %> # As with config/credentials.yml, you never want to store sensitive information, # like your database password, in your source code. If your source code is diff --git a/config/environments/test.rb b/config/environments/test.rb index a3a798981..2af4affe7 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -1,4 +1,5 @@ require "active_support/core_ext/integer/time" +require "faker" # The test environment is used exclusively to run your application's # test suite. You never need to work with it otherwise. Remember that diff --git a/docs/testing.md b/docs/testing.md index a453c69ca..57e40f747 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -15,3 +15,19 @@ nav_order: 4 - Feature specs are generally written sparingly as they’re also the slowest, where possible a request spec is preferred as this still tests a large surface area (route, controller, model, view) without the performance impact. They are not suitable for tests that need to run JavaScript or test that a specific set of interaction events that trigger a specific set of requests (with high confidence). - Test data is created with [FactoryBot](https://github.com/thoughtbot/factory_bot) where ever possible + +## Parallel testing + +- The RSpec test suite can be ran in parallel in local development for quicker turnaround times + +- Setup with the following: + +```sh +bundle exec rake parallel:setup +``` + +- Run with: + +```sh +RAILS_ENV=test bundle exec rake parallel:spec +``` From fbb06bcb9f0807d770ed83857fd72656517a0963 Mon Sep 17 00:00:00 2001 From: Jack S <113976590+bibblobcode@users.noreply.github.com> Date: Wed, 16 Nov 2022 13:46:48 +0000 Subject: [PATCH 16/18] [CLDC-20] Submit for other orgs (#968) * Add scopes to OrganisationRelationship * Update seeds to have more than one org relationships * Pass current_user to questions * Add new questions * Use feature flag * Update specs * Address comments --- app/helpers/question_attribute_helper.rb | 2 +- .../form/common/questions/created_by_id.rb | 2 +- .../questions/owning_organisation_id.rb | 2 +- app/models/form/lettings/pages/created_by.rb | 19 +++ .../form/lettings/pages/housing_provider.rb | 30 ++++ .../lettings/pages/managing_organisation.rb | 30 ++++ .../form/lettings/questions/created_by_id.rb | 48 ++++++ .../lettings/questions/housing_provider.rb | 68 ++++++++ .../questions/managing_organisation.rb | 74 +++++++++ .../form/lettings/questions/scheme_id.rb | 2 +- app/models/form/lettings/subsections/setup.rb | 38 ++++- app/models/form/question.rb | 2 +- app/models/organisation_relationship.rb | 3 + app/views/form/_radio_question.html.erb | 2 +- app/views/form/_select_question.html.erb | 2 +- config/initializers/feature_toggle.rb | 4 + db/schema.rb | 8 +- db/seeds.rb | 42 ++++- spec/features/lettings_log_spec.rb | 23 +-- .../form/lettings/pages/created_by_spec.rb | 55 +++++++ .../lettings/pages/housing_provider_spec.rb | 128 +++++++++++++++ .../pages/managing_organisation_spec.rb | 130 +++++++++++++++ .../lettings/questions/created_by_id_spec.rb | 82 +++++++++ .../questions/housing_provider_spec.rb | 117 +++++++++++++ .../questions/managing_organisation_spec.rb | 155 ++++++++++++++++++ .../form/lettings/subsections/setup_spec.rb | 66 ++++++-- spec/models/form_handler_spec.rb | 8 +- spec/requests/form_controller_spec.rb | 40 ++--- 28 files changed, 1112 insertions(+), 70 deletions(-) create mode 100644 app/models/form/lettings/pages/created_by.rb create mode 100644 app/models/form/lettings/pages/housing_provider.rb create mode 100644 app/models/form/lettings/pages/managing_organisation.rb create mode 100644 app/models/form/lettings/questions/created_by_id.rb create mode 100644 app/models/form/lettings/questions/housing_provider.rb create mode 100644 app/models/form/lettings/questions/managing_organisation.rb create mode 100644 spec/models/form/lettings/pages/created_by_spec.rb create mode 100644 spec/models/form/lettings/pages/housing_provider_spec.rb create mode 100644 spec/models/form/lettings/pages/managing_organisation_spec.rb create mode 100644 spec/models/form/lettings/questions/created_by_id_spec.rb create mode 100644 spec/models/form/lettings/questions/housing_provider_spec.rb create mode 100644 spec/models/form/lettings/questions/managing_organisation_spec.rb diff --git a/app/helpers/question_attribute_helper.rb b/app/helpers/question_attribute_helper.rb index 020ea1909..601368c22 100644 --- a/app/helpers/question_attribute_helper.rb +++ b/app/helpers/question_attribute_helper.rb @@ -11,7 +11,7 @@ module QuestionAttributeHelper { "data-controller": "conditional-question", "data-action": "click->conditional-question#displayConditional", - "data-info": { conditional_questions: conditional_for, type: type }.to_json, + "data-info": { conditional_questions: conditional_for, type: }.to_json, } end diff --git a/app/models/form/common/questions/created_by_id.rb b/app/models/form/common/questions/created_by_id.rb index b0558e5e2..b8409d99c 100644 --- a/app/models/form/common/questions/created_by_id.rb +++ b/app/models/form/common/questions/created_by_id.rb @@ -19,7 +19,7 @@ class Form::Common::Questions::CreatedById < ::Form::Question end end - def displayed_answer_options(log) + def displayed_answer_options(log, _user = nil) return answer_options unless log.owning_organisation user_ids = log.owning_organisation.users.pluck(:id) + [""] diff --git a/app/models/form/common/questions/owning_organisation_id.rb b/app/models/form/common/questions/owning_organisation_id.rb index 84eefbf21..9adb0bf64 100644 --- a/app/models/form/common/questions/owning_organisation_id.rb +++ b/app/models/form/common/questions/owning_organisation_id.rb @@ -19,7 +19,7 @@ class Form::Common::Questions::OwningOrganisationId < ::Form::Question end end - def displayed_answer_options(_log) + def displayed_answer_options(_log, _user = nil) answer_options end diff --git a/app/models/form/lettings/pages/created_by.rb b/app/models/form/lettings/pages/created_by.rb new file mode 100644 index 000000000..42289befa --- /dev/null +++ b/app/models/form/lettings/pages/created_by.rb @@ -0,0 +1,19 @@ +class Form::Lettings::Pages::CreatedBy < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "created_by" + @header = "" + @description = "" + @subsection = subsection + end + + def questions + @questions ||= [ + Form::Lettings::Questions::CreatedById.new(nil, nil, self), + ] + end + + def routed_to?(_log, current_user) + !!current_user&.support? + end +end diff --git a/app/models/form/lettings/pages/housing_provider.rb b/app/models/form/lettings/pages/housing_provider.rb new file mode 100644 index 000000000..a1c3a6687 --- /dev/null +++ b/app/models/form/lettings/pages/housing_provider.rb @@ -0,0 +1,30 @@ +class Form::Lettings::Pages::HousingProvider < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "housing_provider" + @header = "" + @description = "" + @subsection = subsection + end + + def questions + @questions ||= [ + Form::Lettings::Questions::HousingProvider.new(nil, nil, self), + ] + end + + def routed_to?(log, current_user) + return false unless current_user + return true if current_user.support? + return true unless current_user.organisation.holds_own_stock? + + housing_providers = current_user.organisation.housing_providers + + return false if housing_providers.count.zero? + return true if housing_providers.count > 1 + + log.update!(owning_organisation: housing_providers.first) + + false + end +end diff --git a/app/models/form/lettings/pages/managing_organisation.rb b/app/models/form/lettings/pages/managing_organisation.rb new file mode 100644 index 000000000..67e8ae586 --- /dev/null +++ b/app/models/form/lettings/pages/managing_organisation.rb @@ -0,0 +1,30 @@ +class Form::Lettings::Pages::ManagingOrganisation < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "managing_organisation" + @header = "" + @description = "" + @subsection = subsection + end + + def questions + @questions ||= [ + Form::Lettings::Questions::ManagingOrganisation.new(nil, nil, self), + ] + end + + def routed_to?(log, current_user) + return false unless current_user + return true if current_user.support? + return true unless current_user.organisation.holds_own_stock? + + managing_agents = current_user.organisation.managing_agents + + return false if managing_agents.count.zero? + return true if managing_agents.count > 1 + + log.update!(managing_organisation: managing_agents.first) + + false + end +end diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb new file mode 100644 index 000000000..cf7323940 --- /dev/null +++ b/app/models/form/lettings/questions/created_by_id.rb @@ -0,0 +1,48 @@ +class Form::Lettings::Questions::CreatedById < ::Form::Question + def initialize(id, hsh, page) + super + @id = "created_by_id" + @check_answer_label = "Log owner" + @header = "Which user are you creating this log for?" + @hint_text = "" + @type = "select" + @page = page + end + + def answer_options + answer_opts = { "" => "Select an option" } + return answer_opts unless ActiveRecord::Base.connected? + + User.select(:id, :name, :email).each_with_object(answer_opts) do |user, hsh| + hsh[user.id] = "#{user.name} (#{user.email})" + hsh + end + end + + def displayed_answer_options(log, _user = nil) + return answer_options unless log.owning_organisation + + user_ids = log.owning_organisation.users.pluck(:id) + [""] + answer_options.select { |k, _v| user_ids.include?(k) } + end + + def label_from_value(value) + return unless value + + answer_options[value] + end + + def hidden_in_check_answers?(_log, current_user) + !current_user.support? + end + + def derived? + true + end + +private + + def selected_answer_option_is_derived?(_log) + true + end +end diff --git a/app/models/form/lettings/questions/housing_provider.rb b/app/models/form/lettings/questions/housing_provider.rb new file mode 100644 index 000000000..f0b69f9f7 --- /dev/null +++ b/app/models/form/lettings/questions/housing_provider.rb @@ -0,0 +1,68 @@ +class Form::Lettings::Questions::HousingProvider < ::Form::Question + attr_accessor :current_user, :log + + def initialize(id, hsh, page) + super + @id = "owning_organisation_id" + @check_answer_label = "Housing provider" + @header = "Which organisation owns this property?" + @type = "select" + @page = page + end + + def answer_options + answer_opts = { "" => "Select an option" } + return answer_opts unless ActiveRecord::Base.connected? + return answer_opts unless current_user + + if !current_user.support? && current_user.organisation.holds_own_stock? + answer_opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)" + end + + answer_opts.merge(housing_providers_answer_options) + end + + def displayed_answer_options(log, user = nil) + @current_user = user + @log = log + + answer_options + end + + def label_from_value(value) + return unless value + + answer_options[value] + end + + def derived? + true + end + + def hidden_in_check_answers?(_log, user = nil) + @current_user = user + + return false unless @current_user + return false if @current_user.support? + + housing_providers_answer_options.count < 2 + end + + def enabled + true + end + +private + + def selected_answer_option_is_derived?(_log) + true + end + + def housing_providers_answer_options + @housing_providers_answer_options ||= if current_user.support? + Organisation + else + current_user.organisation.housing_providers + end.pluck(:id, :name).to_h + end +end diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb new file mode 100644 index 000000000..ff6da010a --- /dev/null +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -0,0 +1,74 @@ +class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question + attr_accessor :current_user, :log + + def initialize(id, hsh, page) + super + @id = "managing_organisation_id" + @check_answer_label = "Managing agent" + @header = "Which organisation manages this letting?" + @type = "select" + @answer_options = answer_options + @page = page + end + + def answer_options + opts = { "" => "Select an option" } + return opts unless ActiveRecord::Base.connected? + return opts unless current_user + return opts unless log + + if current_user.support? + if log.owning_organisation.holds_own_stock? + opts[log.owning_organisation.id] = "#{log.owning_organisation.name} (Owning organisation)" + end + elsif current_user.organisation.holds_own_stock? + opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)" + end + + opts.merge(managing_organisations_answer_options) + end + + def displayed_answer_options(log, user) + @current_user = user + @log = log + + answer_options + end + + def label_from_value(value) + return unless value + + answer_options[value] + end + + def derived? + true + end + + def hidden_in_check_answers?(_log, user = nil) + @current_user = user + + return false unless @current_user + return false if @current_user.support? + + managing_organisations_answer_options.count < 2 + end + + def enabled + true + end + +private + + def selected_answer_option_is_derived?(_log) + true + end + + def managing_organisations_answer_options + @managing_organisations_answer_options ||= if current_user.support? + log.owning_organisation + else + current_user.organisation + end.managing_agents.pluck(:id, :name).to_h + end +end diff --git a/app/models/form/lettings/questions/scheme_id.rb b/app/models/form/lettings/questions/scheme_id.rb index 7d7966c00..d576a3f72 100644 --- a/app/models/form/lettings/questions/scheme_id.rb +++ b/app/models/form/lettings/questions/scheme_id.rb @@ -20,7 +20,7 @@ class Form::Lettings::Questions::SchemeId < ::Form::Question end end - def displayed_answer_options(lettings_log) + def displayed_answer_options(lettings_log, _user = nil) organisation = lettings_log.owning_organisation || lettings_log.created_by&.organisation schemes = organisation ? Scheme.select(:id).where(owning_organisation_id: organisation.id, confirmed: true) : Scheme.select(:id).where(confirmed: true) filtered_scheme_ids = schemes.joins(:locations).merge(Location.where("startdate <= ? or startdate IS NULL", Time.zone.today)).map(&:id) diff --git a/app/models/form/lettings/subsections/setup.rb b/app/models/form/lettings/subsections/setup.rb index 79d346599..e871406be 100644 --- a/app/models/form/lettings/subsections/setup.rb +++ b/app/models/form/lettings/subsections/setup.rb @@ -8,8 +8,10 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection def pages @pages ||= [ - Form::Common::Pages::Organisation.new(nil, nil, self), - Form::Common::Pages::CreatedBy.new(nil, nil, self), + organisation_page, + housing_provider_page, + managing_organisation_page, + created_by_page, Form::Lettings::Pages::NeedsType.new(nil, nil, self), Form::Lettings::Pages::Scheme.new(nil, nil, self), Form::Lettings::Pages::Location.new(nil, nil, self), @@ -18,11 +20,7 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection Form::Lettings::Pages::RentType.new(nil, nil, self), Form::Lettings::Pages::TenantCode.new(nil, nil, self), Form::Lettings::Pages::PropertyReference.new(nil, nil, self), - ] - end - - def applicable_questions(lettings_log) - questions.select { |q| support_only_questions.include?(q.id) } + super + ].compact end def enabled?(_lettings_log) @@ -31,7 +29,29 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection private - def support_only_questions - %w[owning_organisation_id created_by_id].freeze + def organisation_page + return if FeatureToggle.managing_for_other_user_enabled? + + Form::Common::Pages::Organisation.new(nil, nil, self) + end + + def housing_provider_page + return unless FeatureToggle.managing_for_other_user_enabled? + + Form::Lettings::Pages::HousingProvider.new(nil, nil, self) + end + + def managing_organisation_page + return unless FeatureToggle.managing_for_other_user_enabled? + + Form::Lettings::Pages::ManagingOrganisation.new(nil, nil, self) + end + + def created_by_page + if FeatureToggle.managing_for_other_user_enabled? + Form::Lettings::Pages::CreatedBy.new(nil, nil, self) + else + Form::Common::Pages::CreatedBy.new(nil, nil, self) + end end end diff --git a/app/models/form/question.rb b/app/models/form/question.rb index 7f3717d4a..13b1c62c9 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -107,7 +107,7 @@ class Form::Question false end - def displayed_answer_options(log) + def displayed_answer_options(log, _current_user = nil) answer_options.select do |_key, val| !val.is_a?(Hash) || !val["depends_on"] || form.depends_on_met(val["depends_on"], log) end diff --git a/app/models/organisation_relationship.rb b/app/models/organisation_relationship.rb index acda8d2c4..d1f073737 100644 --- a/app/models/organisation_relationship.rb +++ b/app/models/organisation_relationship.rb @@ -2,6 +2,9 @@ class OrganisationRelationship < ApplicationRecord belongs_to :child_organisation, class_name: "Organisation" belongs_to :parent_organisation, class_name: "Organisation" + scope :owning, -> { where(relationship_type: OWNING) } + scope :managing, -> { where(relationship_type: MANAGING) } + OWNING = "owning".freeze MANAGING = "managing".freeze RELATIONSHIP_TYPE = { diff --git a/app/views/form/_radio_question.html.erb b/app/views/form/_radio_question.html.erb index ed5e71a0b..bac6fec8f 100644 --- a/app/views/form/_radio_question.html.erb +++ b/app/views/form/_radio_question.html.erb @@ -5,7 +5,7 @@ legend: legend(question, page_header, conditional), hint: { text: question.hint_text&.html_safe } do %> - <% question.displayed_answer_options(@log).map do |key, options| %> + <% question.displayed_answer_options(@log, current_user).map do |key, options| %> <% if key.starts_with?("divider") %> <%= f.govuk_radio_divider %> <% else %> diff --git a/app/views/form/_select_question.html.erb b/app/views/form/_select_question.html.erb index 63f50fb94..c1ee8c3e0 100644 --- a/app/views/form/_select_question.html.erb +++ b/app/views/form/_select_question.html.erb @@ -1,7 +1,7 @@ <%= render partial: "form/guidance/#{question.guidance_partial}" if question.top_guidance? %> <% selected = @log.public_send(question.id) || "" %> -<% answers = question.displayed_answer_options(@log).map { |key, value| OpenStruct.new(id: key, name: value.respond_to?(:service_name) ? value.service_name : nil, resource: value) } %> +<% answers = question.displayed_answer_options(@log, current_user).map { |key, value| OpenStruct.new(id: key, name: value.respond_to?(:service_name) ? value.service_name : nil, resource: value) } %> <%= f.govuk_select(question.id.to_sym, label: legend(question, page_header, conditional), "data-controller": "accessible-autocomplete", diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index d3ab1db57..1a4b8a862 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -20,4 +20,8 @@ class FeatureToggle false end + + def self.managing_for_other_user_enabled? + !Rails.env.production? + end end diff --git a/db/schema.rb b/db/schema.rb index 5faff28ba..0f2ad0025 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -360,16 +360,16 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do t.integer "hholdcount" t.integer "age3" t.integer "age3_known" + t.string "la" + t.integer "la_known" + t.integer "income1" + t.integer "income1nk" t.integer "age4" t.integer "age4_known" t.integer "age5" t.integer "age5_known" t.integer "age6" t.integer "age6_known" - t.string "la" - t.integer "la_known" - t.integer "income1" - t.integer "income1nk" t.integer "details_known_2" t.integer "details_known_3" t.integer "details_known_4" diff --git a/db/seeds.rb b/db/seeds.rb index e357e6efe..9d78fd8cb 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -8,8 +8,8 @@ # rubocop:disable Rails/Output unless Rails.env.test? - housing_provider = Organisation.find_or_create_by!( - name: "Housing Provider", + housing_provider1 = Organisation.find_or_create_by!( + name: "Housing Provider 1", address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", @@ -18,8 +18,28 @@ unless Rails.env.test? managing_agents_label: "None", provider_type: "LA", ) - managing_agent = Organisation.find_or_create_by!( - name: "Managing Agent", + housing_provider2 = Organisation.find_or_create_by!( + name: "Housing Provider 2", + address_line1: "2 Marsham Street", + address_line2: "London", + postcode: "SW1P 4DF", + holds_own_stock: true, + other_stock_owners: "None", + managing_agents_label: "None", + provider_type: "LA", + ) + managing_agent1 = Organisation.find_or_create_by!( + name: "Managing Agent 1", + address_line1: "2 Marsham Street", + address_line2: "London", + postcode: "SW1P 4DF", + holds_own_stock: true, + other_stock_owners: "None", + managing_agents_label: "None", + provider_type: "LA", + ) + managing_agent2 = Organisation.find_or_create_by!( + name: "Managing Agent 2", address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", @@ -49,11 +69,21 @@ unless Rails.env.test? OrganisationRelationship.create!( child_organisation: org, - parent_organisation: housing_provider, + parent_organisation: housing_provider1, + relationship_type: OrganisationRelationship::OWNING, + ) + OrganisationRelationship.create!( + child_organisation: org, + parent_organisation: housing_provider2, relationship_type: OrganisationRelationship::OWNING, ) OrganisationRelationship.create!( - child_organisation: managing_agent, + child_organisation: managing_agent1, + parent_organisation: org, + relationship_type: OrganisationRelationship::MANAGING, + ) + OrganisationRelationship.create!( + child_organisation: managing_agent2, parent_organisation: org, relationship_type: OrganisationRelationship::MANAGING, ) diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index 1bd72b5cf..294ef51cd 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -3,10 +3,10 @@ require "rails_helper" RSpec.describe "Lettings Log Features" do context "when searching for specific logs" do context "when I am signed in and there are logs in the database" do - let(:user) { FactoryBot.create(:user, last_sign_in_at: Time.zone.now) } - let!(:log_to_search) { FactoryBot.create(:lettings_log, owning_organisation: user.organisation) } - let!(:same_organisation_log) { FactoryBot.create(:lettings_log, owning_organisation: user.organisation) } - let!(:another_organisation_log) { FactoryBot.create(:lettings_log) } + let(:user) { create(:user, last_sign_in_at: Time.zone.now) } + let!(:log_to_search) { create(:lettings_log, owning_organisation: user.organisation) } + let!(:same_organisation_log) { create(:lettings_log, owning_organisation: user.organisation) } + let!(:another_organisation_log) { create(:lettings_log) } before do visit("/lettings-logs") @@ -58,7 +58,7 @@ RSpec.describe "Lettings Log Features" do end context "when the signed is user is a Support user" do - let(:support_user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now) } + let(:support_user) { create(:user, :support, last_sign_in_at: Time.zone.now) } let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } let(:mfa_template_id) { User::MFA_TEMPLATE_ID } @@ -77,26 +77,29 @@ RSpec.describe "Lettings Log Features" do click_button("Submit") end - context "when completing the setup lettings log section" do + context "when completing the setup lettings log section", :aggregate_failure do it "includes the organisation and created by questions" do visit("/lettings-logs") click_button("Create a new lettings log") click_link("Set up this lettings log") select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field") click_button("Save and continue") + select("#{support_user.organisation.name} (Owning organisation)", from: "lettings-log-managing-organisation-id-field") + click_button("Save and continue") select(support_user.name, from: "lettings-log-created-by-id-field") click_button("Save and continue") log_id = page.current_path.scan(/\d/).join visit("lettings-logs/#{log_id}/setup/check-answers") - expect(page).to have_content("Owning organisation #{support_user.organisation.name}") - expect(page).to have_content("User #{support_user.name}") - expect(page).to have_content("You have answered 2 of 8 questions") + expect(page).to have_content("Housing provider #{support_user.organisation.name}") + expect(page).to have_content("Managing agent #{support_user.organisation.name}") + expect(page).to have_content("Log owner #{support_user.name}") + expect(page).to have_content("You have answered 3 of 9 questions") end end end context "when the signed is user is not a Support user" do - let(:support_user) { FactoryBot.create(:user) } + let(:support_user) { create(:user) } let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } diff --git a/spec/models/form/lettings/pages/created_by_spec.rb b/spec/models/form/lettings/pages/created_by_spec.rb new file mode 100644 index 000000000..c4fd460cb --- /dev/null +++ b/spec/models/form/lettings/pages/created_by_spec.rb @@ -0,0 +1,55 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::CreatedBy, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + let(:lettings_log) { instance_double(LettingsLog) } + + describe "#routed_to?" do + context "when nil" do + it "is not shown" do + expect(page.routed_to?(nil, nil)).to eq(false) + end + end + + context "when support" do + it "is shown" do + expect(page.routed_to?(nil, create(:user, :support))).to eq(true) + end + end + + context "when not support" do + it "is not shown" do + expect(page.routed_to?(nil, create(:user, :data_coordinator))).to eq(false) + end + end + end + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[created_by_id]) + end + + it "has the correct id" do + expect(page.id).to eq("created_by") + end + + it "has the correct header" do + expect(page.header).to eq("") + end + + it "has the correct description" do + expect(page.description).to eq("") + end + + it "has the correct depends_on" do + expect(page.depends_on).to be nil + end +end diff --git a/spec/models/form/lettings/pages/housing_provider_spec.rb b/spec/models/form/lettings/pages/housing_provider_spec.rb new file mode 100644 index 000000000..97420506e --- /dev/null +++ b/spec/models/form/lettings/pages/housing_provider_spec.rb @@ -0,0 +1,128 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::HousingProvider, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[owning_organisation_id]) + end + + it "has the correct id" do + expect(page.id).to eq("housing_provider") + end + + it "has the correct header" do + expect(page.header).to eq("") + end + + it "has the correct description" do + expect(page.description).to eq("") + end + + it "has the correct depends_on" do + expect(page.depends_on).to be nil + end + + describe "#routed_to?" do + let(:log) { create(:lettings_log, owning_organisation_id: nil) } + + context "when user nil" do + it "is not shown" do + expect(page.routed_to?(log, nil)).to eq(false) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, nil) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + + context "when support" do + let(:user) { create(:user, :support) } + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + + context "when not support" do + context "when does not hold own stock" do + let(:user) do + create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + + context "when holds own stock" do + let(:user) do + create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) + end + + context "with 0 housing_providers" do + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + + context "with >1 housing_providers" do + before do + create(:organisation_relationship, :owning, child_organisation: user.organisation) + create(:organisation_relationship, :owning, child_organisation: user.organisation) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + + context "with 1 housing_providers" do + let(:housing_provider) { create(:organisation) } + + before do + create( + :organisation_relationship, + :owning, + child_organisation: user.organisation, + parent_organisation: housing_provider, + ) + end + + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "updates owning_organisation_id" do + expect { page.routed_to?(log, user) }.to change(log.reload, :owning_organisation).from(nil).to(housing_provider) + end + end + end + end + end +end diff --git a/spec/models/form/lettings/pages/managing_organisation_spec.rb b/spec/models/form/lettings/pages/managing_organisation_spec.rb new file mode 100644 index 000000000..fcb93d565 --- /dev/null +++ b/spec/models/form/lettings/pages/managing_organisation_spec.rb @@ -0,0 +1,130 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[managing_organisation_id]) + end + + it "has the correct id" do + expect(page.id).to eq("managing_organisation") + end + + it "has the correct header" do + expect(page.header).to eq("") + end + + it "has the correct description" do + expect(page.description).to eq("") + end + + it "has the correct depends_on" do + expect(page.depends_on).to be nil + end + + describe "#routed_to?" do + let(:log) { create(:lettings_log) } + let(:organisation) { create(:organisation) } + + context "when user nil" do + it "is not shown" do + expect(page.routed_to?(log, nil)).to eq(false) + end + + it "does not update managing_organisation_id" do + expect { page.routed_to?(log, nil) }.not_to change(log.reload, :managing_organisation) + end + end + + context "when support" do + let(:organisation) { create(:organisation) } + let(:user) { create(:user, :support, organisation:) } + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update managing_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) + end + end + + context "when not support" do + context "when does not hold own stock" do + let(:user) do + create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update managing_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) + end + end + + context "when holds own stock" do + let(:user) do + create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) + end + + context "with 0 managing_agents" do + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "does not update managing_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) + end + end + + context "with >1 managing_agents" do + before do + create(:organisation_relationship, :managing, parent_organisation: user.organisation) + create(:organisation_relationship, :managing, parent_organisation: user.organisation) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update managing_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) + end + end + + context "with 1 managing_agents" do + let(:managing_agent) { create(:organisation) } + + before do + create( + :organisation_relationship, + :managing, + child_organisation: managing_agent, + parent_organisation: user.organisation, + ) + end + + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "updates managing_organisation_id" do + expect { page.routed_to?(log, user) }.to change(log.reload, :managing_organisation).to(managing_agent) + end + end + end + end + end +end diff --git a/spec/models/form/lettings/questions/created_by_id_spec.rb b/spec/models/form/lettings/questions/created_by_id_spec.rb new file mode 100644 index 000000000..152a24265 --- /dev/null +++ b/spec/models/form/lettings/questions/created_by_id_spec.rb @@ -0,0 +1,82 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + let(:user_1) { FactoryBot.create(:user, name: "first user") } + let(:user_2) { FactoryBot.create(:user, name: "second user") } + let!(:expected_answer_options) do + { + "" => "Select an option", + user_1.id => "#{user_1.name} (#{user_1.email})", + user_2.id => "#{user_2.name} (#{user_2.email})", + } + end + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("created_by_id") + end + + it "has the correct header" do + expect(question.header).to eq("Which user are you creating this log for?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Log owner") + end + + it "has the correct type" do + expect(question.type).to eq("select") + end + + it "has the correct hint_text" do + expect(question.hint_text).to eq("") + end + + it "has the correct answer options" do + expect(question.answer_options).to eq(expected_answer_options) + end + + it "is marked as derived" do + expect(question.derived?).to be true + end + + context "when the current user is support" do + let(:support_user) { FactoryBot.build(:user, :support) } + + it "is shown in check answers" do + expect(question.hidden_in_check_answers?(nil, support_user)).to be false + end + end + + context "when the current user is not support" do + let(:user) { FactoryBot.build(:user) } + + it "is not shown in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be true + end + end + + context "when the owning organisation is already set" do + let(:lettings_log) { FactoryBot.create(:lettings_log, owning_organisation: user_2.organisation) } + let(:expected_answer_options) do + { + "" => "Select an option", + user_2.id => "#{user_2.name} (#{user_2.email})", + } + end + + it "only displays users that belong to that organisation" do + expect(question.displayed_answer_options(lettings_log)).to eq(expected_answer_options) + end + end +end diff --git a/spec/models/form/lettings/questions/housing_provider_spec.rb b/spec/models/form/lettings/questions/housing_provider_spec.rb new file mode 100644 index 000000000..073c59e37 --- /dev/null +++ b/spec/models/form/lettings/questions/housing_provider_spec.rb @@ -0,0 +1,117 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Questions::HousingProvider, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("owning_organisation_id") + end + + it "has the correct header" do + expect(question.header).to eq("Which organisation owns this property?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Housing provider") + end + + it "has the correct type" do + expect(question.type).to eq("select") + end + + it "has the correct hint_text" do + expect(question.hint_text).to be_nil + end + + describe "answer options" do + let(:options) { { "" => "Select an option" } } + + context "when current_user nil" do + it "shows default options" do + expect(question.answer_options).to eq(options) + end + end + + context "when user not support and owns own stock" do + let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) } + let(:options) do + { + "" => "Select an option", + user.organisation.id => "#{user.organisation.name} (Your organisation)", + } + end + + before do + question.current_user = user + end + + it "shows housing providers with own org at the top" do + expect(question.answer_options).to eq(options) + end + end + + context "when user support" do + before do + question.current_user = create(:user, :support) + end + + let(:expected_opts) do + Organisation.all.each_with_object(options) do |organisation, hsh| + hsh[organisation.id] = organisation.name + hsh + end + end + + it "shows all orgs" do + expect(question.answer_options).to eq(expected_opts) + end + end + end + + it "is marked as derived" do + expect(question.derived?).to be true + end + + describe "#hidden_in_check_answers?" do + context "when housing providers < 2" do + context "when not support user" do + let(:user) { create(:user) } + + it "is hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be true + end + end + + context "when support" do + let(:user) { create(:user, :support) } + + it "is not hiddes in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false + end + end + end + + context "when housing providers >= 2" do + let(:user) { create(:user) } + + before do + create(:organisation_relationship, :owning, child_organisation: user.organisation) + create(:organisation_relationship, :owning, child_organisation: user.organisation) + end + + it "is not hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false + end + end + end +end diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb new file mode 100644 index 000000000..f80099d67 --- /dev/null +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -0,0 +1,155 @@ +require "rails_helper" + +RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("managing_organisation_id") + end + + it "has the correct header" do + expect(question.header).to eq("Which organisation manages this letting?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Managing agent") + end + + it "has the correct type" do + expect(question.type).to eq("select") + end + + it "has the correct hint_text" do + expect(question.hint_text).to be_nil + end + + describe "#displayed_answer_options" do + let(:options) { { "" => "Select an option" } } + + context "when current_user nil" do + let(:log) { create(:lettings_log) } + + it "shows default options" do + expect(question.displayed_answer_options(log, nil)).to eq(options) + end + end + + context "when log nil" do + let(:user) { create(:user) } + + it "shows default options" do + expect(question.displayed_answer_options(nil, user)).to eq(options) + end + end + + context "when user not support and owns own stock" do + let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) } + + let(:log) { create(:lettings_log) } + let!(:org_rel1) { create(:organisation_relationship, :managing, parent_organisation: user.organisation) } + let!(:org_rel2) { create(:organisation_relationship, :managing, parent_organisation: user.organisation) } + + let(:options) do + { + "" => "Select an option", + user.organisation.id => "#{user.organisation.name} (Your organisation)", + org_rel1.child_organisation.id => org_rel1.child_organisation.name, + org_rel2.child_organisation.id => org_rel2.child_organisation.name, + } + end + + it "shows managing agents with own org at the top" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when support user and org does not own own stock" do + let(:user) { create(:user, :support) } + let(:log_owning_org) { create(:organisation, holds_own_stock: false) } + let(:log) { create(:lettings_log, owning_organisation: log_owning_org) } + let!(:org_rel1) { create(:organisation_relationship, :managing, parent_organisation: log_owning_org) } + let!(:org_rel2) { create(:organisation_relationship, :managing, parent_organisation: log_owning_org) } + + let(:options) do + { + "" => "Select an option", + org_rel1.child_organisation.id => org_rel1.child_organisation.name, + org_rel2.child_organisation.id => org_rel2.child_organisation.name, + } + end + + it "shows owning org managing agents with hint" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when support user and org does own stock" do + let(:user) { create(:user, :support) } + let(:log_owning_org) { create(:organisation, holds_own_stock: true) } + let(:log) { create(:lettings_log, owning_organisation: log_owning_org) } + let!(:org_rel1) { create(:organisation_relationship, :managing, parent_organisation: log_owning_org) } + let!(:org_rel2) { create(:organisation_relationship, :managing, parent_organisation: log_owning_org) } + + let(:options) do + { + "" => "Select an option", + log_owning_org.id => "#{log_owning_org.name} (Owning organisation)", + org_rel1.child_organisation.id => org_rel1.child_organisation.name, + org_rel2.child_organisation.id => org_rel2.child_organisation.name, + } + end + + it "shows owning org managing agents + " do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + end + + it "is marked as derived" do + expect(question.derived?).to be true + end + + describe "#hidden_in_check_answers?" do + context "when housing providers < 2" do + context "when not support user" do + let(:user) { create(:user) } + + it "is hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be true + end + end + + context "when support" do + let(:user) { create(:user, :support) } + + it "is not hiddes in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false + end + end + end + + context "when managing agents >= 2" do + let(:user) { create(:user) } + + before do + create(:organisation_relationship, :managing, parent_organisation: user.organisation) + create(:organisation_relationship, :managing, parent_organisation: user.organisation) + end + + it "is not hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false + end + end + end +end diff --git a/spec/models/form/lettings/subsections/setup_spec.rb b/spec/models/form/lettings/subsections/setup_spec.rb index 3e2fba32f..6188cc1e0 100644 --- a/spec/models/form/lettings/subsections/setup_spec.rb +++ b/spec/models/form/lettings/subsections/setup_spec.rb @@ -13,16 +13,19 @@ RSpec.describe Form::Lettings::Subsections::Setup, type: :model do it "has correct pages" do expect(setup.pages.map(&:id)).to eq( - %w[organisation - created_by - needs_type - scheme - location - renewal - tenancy_start_date - rent_type - tenant_code - property_reference], + %w[ + housing_provider + managing_organisation + created_by + needs_type + scheme + location + renewal + tenancy_start_date + rent_type + tenant_code + property_reference + ], ) end @@ -33,4 +36,47 @@ RSpec.describe Form::Lettings::Subsections::Setup, type: :model do it "has the correct label" do expect(setup.label).to eq("Set up this lettings log") end + + context "when not production" do + it "has correct pages" do + expect(setup.pages.map(&:id)).to eq( + %w[ + housing_provider + managing_organisation + created_by + needs_type + scheme + location + renewal + tenancy_start_date + rent_type + tenant_code + property_reference + ], + ) + end + end + + context "when production" do + before do + allow(Rails.env).to receive(:production?).and_return(true) + end + + it "has the correct pages" do + expect(setup.pages.map(&:id)).to eq( + %w[ + organisation + created_by + needs_type + scheme + location + renewal + tenancy_start_date + rent_type + tenant_code + property_reference + ], + ) + end + end end diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index c6a47c107..e4056c6c1 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -27,13 +27,13 @@ RSpec.describe FormHandler do it "is able to load a current lettings form" do form = form_handler.get_form("current_lettings") expect(form).to be_a(Form) - expect(form.pages.count).to eq(45) + expect(form.pages.count).to eq(46) end it "is able to load a next lettings form" do form = form_handler.get_form("next_lettings") expect(form).to be_a(Form) - expect(form.pages.count).to eq(12) + expect(form.pages.count).to eq(13) end end @@ -49,13 +49,13 @@ RSpec.describe FormHandler do it "is able to load a current lettings form" do form = form_handler.get_form("current_lettings") expect(form).to be_a(Form) - expect(form.pages.count).to eq(12) + expect(form.pages.count).to eq(13) end it "is able to load a previous lettings form" do form = form_handler.get_form("previous_lettings") expect(form).to be_a(Form) - expect(form.pages.count).to eq(45) + expect(form.pages.count).to eq(46) end it "is able to load a current sales form" do diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index ade87cfde..1bb73f399 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -2,18 +2,18 @@ require "rails_helper" RSpec.describe FormController, type: :request do let(:page) { Capybara::Node::Simple.new(response.body) } - let(:user) { FactoryBot.create(:user) } + let(:user) { create(:user) } let(:organisation) { user.organisation } - let(:other_organisation) { FactoryBot.create(:organisation) } + let(:other_organisation) { create(:organisation) } let!(:unauthorized_lettings_log) do - FactoryBot.create( + create( :lettings_log, owning_organisation: other_organisation, managing_organisation: other_organisation, ) end let(:setup_complete_lettings_log) do - FactoryBot.create( + create( :lettings_log, :about_completed, status: 1, @@ -23,7 +23,7 @@ RSpec.describe FormController, type: :request do ) end let(:completed_lettings_log) do - FactoryBot.create( + create( :lettings_log, :completed, owning_organisation: organisation, @@ -39,7 +39,7 @@ RSpec.describe FormController, type: :request do context "when a user is not signed in" do let!(:lettings_log) do - FactoryBot.create( + create( :lettings_log, owning_organisation: organisation, managing_organisation: organisation, @@ -68,7 +68,7 @@ RSpec.describe FormController, type: :request do context "when a user is signed in" do let!(:lettings_log) do - FactoryBot.create( + create( :lettings_log, owning_organisation: organisation, managing_organisation: organisation, @@ -83,8 +83,8 @@ RSpec.describe FormController, type: :request do describe "GET" do context "with form pages" do context "when forms exist for multiple years" do - let(:lettings_log_year_1) { FactoryBot.create(:lettings_log, startdate: Time.zone.local(2021, 5, 1), owning_organisation: organisation, created_by: user) } - let(:lettings_log_year_2) { FactoryBot.create(:lettings_log, :about_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) } + let(:lettings_log_year_1) { create(:lettings_log, startdate: Time.zone.local(2021, 5, 1), owning_organisation: organisation, created_by: user) } + let(:lettings_log_year_2) { create(:lettings_log, :about_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) } it "displays the correct question details for each lettings log based on form year" do get "/lettings-logs/#{lettings_log_year_1.id}/tenant-code-test", headers: headers, params: {} @@ -110,11 +110,11 @@ RSpec.describe FormController, type: :request do context "when viewing the setup section schemes page" do context "when the user is support" do - let(:user) { FactoryBot.create(:user, :support) } + let(:user) { create(:user, :support) } context "when organisation and user have not been selected yet" do let(:lettings_log) do - FactoryBot.create( + create( :lettings_log, owning_organisation: nil, managing_organisation: nil, @@ -124,7 +124,7 @@ RSpec.describe FormController, type: :request do end before do - locations = FactoryBot.create_list(:location, 5) + locations = create_list(:location, 5) locations.each { |location| location.scheme.update!(arrangement_type: "The same organisation that owns the housing stock", managing_organisation_id: location.scheme.owning_organisation_id) } end @@ -147,7 +147,7 @@ RSpec.describe FormController, type: :request do context "when no other sections are enabled" do let(:lettings_log_2022) do - FactoryBot.create( + create( :lettings_log, startdate: Time.zone.local(2022, 12, 1), owning_organisation: organisation, @@ -194,17 +194,17 @@ RSpec.describe FormController, type: :request do context "when viewing a user dependent page" do context "when the dependency is met" do - let(:user) { FactoryBot.create(:user, :support) } + let(:user) { create(:user, :support) } it "routes to the page" do - get "/lettings-logs/#{lettings_log.id}/organisation" + get "/lettings-logs/#{lettings_log.id}/created-by" expect(response).to have_http_status(:ok) end end context "when the dependency is not met" do it "redirects to the tasklist page" do - get "/lettings-logs/#{lettings_log.id}/organisation" + get "/lettings-logs/#{lettings_log.id}/created-by" expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}") end end @@ -213,10 +213,10 @@ RSpec.describe FormController, type: :request do describe "Submit Form" do context "with a form page" do - let(:user) { FactoryBot.create(:user) } + let(:user) { create(:user) } let(:organisation) { user.organisation } let(:lettings_log) do - FactoryBot.create( + create( :lettings_log, owning_organisation: organisation, managing_organisation: organisation, @@ -527,9 +527,9 @@ RSpec.describe FormController, type: :request do context "with lettings logs that are not owned or managed by your organisation" do let(:answer) { 25 } - let(:other_organisation) { FactoryBot.create(:organisation) } + let(:other_organisation) { create(:organisation) } let(:unauthorized_lettings_log) do - FactoryBot.create( + create( :lettings_log, owning_organisation: other_organisation, managing_organisation: other_organisation, From 200a38213833131dd8c3b4aa53d7fe162b851c33 Mon Sep 17 00:00:00 2001 From: Jack S <113976590+bibblobcode@users.noreply.github.com> Date: Wed, 16 Nov 2022 15:43:51 +0000 Subject: [PATCH 17/18] Remove memoization (#998) These variables seemed to be memoized across users --- app/models/form/lettings/questions/housing_provider.rb | 10 +++++----- .../form/lettings/questions/managing_organisation.rb | 10 +++++----- spec/features/lettings_log_spec.rb | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/models/form/lettings/questions/housing_provider.rb b/app/models/form/lettings/questions/housing_provider.rb index f0b69f9f7..8d8db24b6 100644 --- a/app/models/form/lettings/questions/housing_provider.rb +++ b/app/models/form/lettings/questions/housing_provider.rb @@ -59,10 +59,10 @@ private end def housing_providers_answer_options - @housing_providers_answer_options ||= if current_user.support? - Organisation - else - current_user.organisation.housing_providers - end.pluck(:id, :name).to_h + if current_user.support? + Organisation + else + current_user.organisation.housing_providers + end.pluck(:id, :name).to_h end end diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index ff6da010a..42662202c 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -65,10 +65,10 @@ private end def managing_organisations_answer_options - @managing_organisations_answer_options ||= if current_user.support? - log.owning_organisation - else - current_user.organisation - end.managing_agents.pluck(:id, :name).to_h + if current_user.support? + log.owning_organisation + else + current_user.organisation + end.managing_agents.pluck(:id, :name).to_h end end diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index 294ef51cd..91901659a 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -99,7 +99,7 @@ RSpec.describe "Lettings Log Features" do end context "when the signed is user is not a Support user" do - let(:support_user) { create(:user) } + let(:user) { create(:user) } let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } @@ -108,8 +108,8 @@ RSpec.describe "Lettings Log Features" do allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) allow(notify_client).to receive(:send_email).and_return(true) visit("/account/sign-in") - fill_in("user[email]", with: support_user.email) - fill_in("user[password]", with: support_user.password) + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: user.password) click_button("Sign in") end @@ -121,8 +121,8 @@ RSpec.describe "Lettings Log Features" do log_id = page.current_path.scan(/\d/).join expect(page).to have_current_path("/lettings-logs/#{log_id}/needs-type") visit("lettings-logs/#{log_id}/setup/check-answers") - expect(page).not_to have_content("Owning organisation #{support_user.organisation.name}") - expect(page).not_to have_content("User #{support_user.name}") + expect(page).not_to have_content("Owning organisation #{user.organisation.name}") + expect(page).not_to have_content("User #{user.name}") expect(page).to have_content("You have answered 0 of 6 questions") end end From 87793f24bda1a7aa9d14a6355a30041640d2ec38 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Wed, 16 Nov 2022 16:50:26 +0000 Subject: [PATCH 18/18] Cldc 1671 deactivate scheme (#980) * feat: wip update scheme summary page * feat: wip deactivate scheme schemes page * feat: wip toggle active page * feat: wip set deactivation_date to a datetime (to be more specific times later_ * Change conditional question controller to accommodate all models * feat: add specific datetimes for deactivation * feat: correct date and add notice * feat: wip error behaviour * feat: wip errors * feat: wip errors refactoring * feat: wip errors more refactoring * refactor: linting * feat: add second error in correct position and wip date range error * feat: remove unneccessary db field * feat: change type values to strings * refactor: tidy up controller logic * refactor: use same structure as locations deactivation * feat: add general partially missing date input error * feat: add tests ("updates existing scheme with valid deactivation date and renders scheme page" still wip) and add new partially nil date behaviour to locations controller * feat: fix tests, add status tag behaviour and remove unnecessary nils * refactor: linting * refactor: erblinting * refactor: remove redundant line * refactor: respond to PR comments 1 * refactor: respond to PR comments 2 * refactor: respond to PR comments 3 * refactor: respond to PR comments 3 (locations side) * fix: remove @locations in location model * fix: remove @locations in location model * fix: update status names * feat: wip validation update * feat: add validation to model layer * feat: further separate scheme deactivation behaviour * test: update tests to new flow * feat: respond to pr comments and add dynamic success text * feat: duplicate behaviour schemes -> locations (+ tests) * refactor: linting * refactor: typo and remove unnecessary line for this PR * refactor: feature toggle simplification * Refactor locations and schemes controller actions - Rename confirmation partials to `deactivate_confirm.html.erb` so that they match the actions in which they belong to - Make all deactivation date comparision UTC time * feat: update deactivation_date validation and add tests * refactor: linting Co-authored-by: Kat Co-authored-by: James Rose --- app/controllers/locations_controller.rb | 82 ++++----- app/controllers/schemes_controller.rb | 64 ++++++- app/helpers/locations_helper.rb | 2 +- app/helpers/schemes_helper.rb | 26 +++ app/helpers/tag_helper.rb | 4 + app/models/form_handler.rb | 2 +- app/models/location.rb | 35 +++- app/models/organisation.rb | 2 +- app/models/scheme.rb | 68 ++++--- ...m.html.erb => deactivate_confirm.html.erb} | 9 +- app/views/locations/show.html.erb | 6 +- app/views/locations/toggle_active.html.erb | 6 +- app/views/organisations/show.html.erb | 2 +- app/views/schemes/deactivate_confirm.html.erb | 19 ++ app/views/schemes/show.html.erb | 12 +- app/views/schemes/toggle_active.html.erb | 35 ++++ config/initializers/feature_toggle.rb | 14 +- config/locales/en.yml | 22 ++- config/routes.rb | 9 +- ...163351_add_deactivation_date_to_schemes.rb | 5 + db/schema.rb | 7 +- spec/features/schemes_spec.rb | 2 +- spec/helpers/locations_helper_spec.rb | 6 +- spec/helpers/schemes_helper_spec.rb | 27 +++ spec/models/form_handler_spec.rb | 2 +- spec/models/location_spec.rb | 12 ++ spec/models/scheme_spec.rb | 44 +++++ spec/requests/locations_controller_spec.rb | 32 +++- .../requests/organisations_controller_spec.rb | 2 +- spec/requests/schemes_controller_spec.rb | 173 ++++++++++++++++++ 30 files changed, 611 insertions(+), 120 deletions(-) create mode 100644 app/helpers/schemes_helper.rb rename app/views/locations/{toggle_active_confirm.html.erb => deactivate_confirm.html.erb} (58%) create mode 100644 app/views/schemes/deactivate_confirm.html.erb create mode 100644 app/views/schemes/toggle_active.html.erb create mode 100644 db/migrate/20221110163351_add_deactivation_date_to_schemes.rb create mode 100644 spec/helpers/schemes_helper_spec.rb diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index 7955ec944..b25f951ea 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -20,22 +20,35 @@ class LocationsController < ApplicationController def show; end - def deactivate + def new_deactivation if params[:location].blank? render "toggle_active", locals: { action: "deactivate" } - elsif params[:location][:confirm].present? && params[:location][:deactivation_date].present? - confirm_deactivation else - deactivation_date_errors - if @location.errors.present? - @location.deactivation_date_type = params[:location][:deactivation_date_type] - render "toggle_active", locals: { action: "deactivate" }, status: :unprocessable_entity + @location.run_deactivation_validations! + @location.deactivation_date = deactivation_date + @location.deactivation_date_type = params[:location][:deactivation_date_type] + if @location.valid? + redirect_to scheme_location_deactivate_confirm_path(@location, deactivation_date: @location.deactivation_date, deactivation_date_type: @location.deactivation_date_type) else - render "toggle_active_confirm", locals: { action: "deactivate", deactivation_date: } + render "toggle_active", locals: { action: "deactivate" }, status: :unprocessable_entity end end end + def deactivate_confirm + @deactivation_date = params[:deactivation_date] + @deactivation_date_type = params[:deactivation_date_type] + end + + def deactivate + @location.run_deactivation_validations! + + if @location.update!(deactivation_date:) + flash[:notice] = deactivate_success_notice + end + redirect_to scheme_location_path(@scheme, @location) + end + def reactivate render "toggle_active", locals: { action: "reactivate" } end @@ -148,13 +161,13 @@ private end def authenticate_action! - if %w[new edit update create index edit_name edit_local_authority deactivate].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) + if %w[new edit update create index edit_name edit_local_authority new_deactivation deactivate_confirm deactivate].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) render_not_found and return end end def location_params - required_params = params.require(:location).permit(:postcode, :name, :units, :type_of_unit, :add_another_location, :startdate, :mobility_type, :location_admin_district, :location_code).merge(scheme_id: @scheme.id) + required_params = params.require(:location).permit(:postcode, :name, :units, :type_of_unit, :add_another_location, :startdate, :mobility_type, :location_admin_district, :location_code, :deactivation_date).merge(scheme_id: @scheme.id) required_params[:postcode] = PostcodeService.clean(required_params[:postcode]) if required_params[:postcode] required_params end @@ -167,48 +180,29 @@ private location_params["location_admin_district"] != "Select an option" end - def confirm_deactivation - if @location.update(deactivation_date: params[:location][:deactivation_date]) - flash[:notice] = "#{@location.name || @location.postcode} has been deactivated" - end - redirect_to scheme_location_path(@scheme, @location) - end - - def deactivation_date_errors - if params[:location][:deactivation_date].blank? && params[:location][:deactivation_date_type].blank? - @location.errors.add(:deactivation_date_type, message: I18n.t("validations.location.deactivation_date.not_selected")) - end - - if params[:location][:deactivation_date_type] == "other" - day = params[:location]["deactivation_date(3i)"] - month = params[:location]["deactivation_date(2i)"] - year = params[:location]["deactivation_date(1i)"] - - collection_start_date = FormHandler.instance.current_collection_start_date - - if [day, month, year].any?(&:blank?) - { day:, month:, year: }.each do |period, value| - @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.not_entered", period: period.to_s)) if value.blank? - end - elsif !Date.valid_date?(year.to_i, month.to_i, day.to_i) - @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.invalid")) - elsif !Date.new(year.to_i, month.to_i, day.to_i).between?(collection_start_date, Date.new(2200, 1, 1)) - @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.out_of_range", date: collection_start_date.to_formatted_s(:govuk_date))) - end + def deactivate_success_notice + case @location.status + when :deactivated + "#{@location.name} has been deactivated" + when :deactivating_soon + "#{@location.name} will deactivate on #{@location.deactivation_date.to_formatted_s(:govuk_date)}" end end def deactivation_date - return if params[:location].blank? - - collection_start_date = FormHandler.instance.current_collection_start_date - return collection_start_date if params[:location][:deactivation_date_type] == "default" - return params[:location][:deactivation_date] if params[:location][:deactivation_date_type].blank? + if params[:location].blank? + return + elsif params[:location][:deactivation_date_type] == "default" + return FormHandler.instance.current_collection_start_date + elsif params[:location][:deactivation_date].present? + return params[:location][:deactivation_date] + end day = params[:location]["deactivation_date(3i)"] month = params[:location]["deactivation_date(2i)"] year = params[:location]["deactivation_date(1i)"] + return nil if [day, month, year].any?(&:blank?) - Date.new(year.to_i, month.to_i, day.to_i) + Time.zone.local(year.to_i, month.to_i, day.to_i) if Date.valid_date?(year.to_i, month.to_i, day.to_i) end end diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 9a259e3a1..6766ffe85 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -21,6 +21,39 @@ class SchemesController < ApplicationController render_not_found and return unless @scheme end + def new_deactivation + if params[:scheme].blank? + render "toggle_active", locals: { action: "deactivate" } + else + @scheme.run_deactivation_validations = true + @scheme.deactivation_date = deactivation_date + @scheme.deactivation_date_type = params[:scheme][:deactivation_date_type] + if @scheme.valid? + redirect_to scheme_deactivate_confirm_path(@scheme, deactivation_date: @scheme.deactivation_date, deactivation_date_type: @scheme.deactivation_date_type) + else + render "toggle_active", locals: { action: "deactivate" }, status: :unprocessable_entity + end + end + end + + def deactivate_confirm + @deactivation_date = params[:deactivation_date] + @deactivation_date_type = params[:deactivation_date_type] + end + + def deactivate + @scheme.run_deactivation_validations! + + if @scheme.update!(deactivation_date:) + flash[:notice] = deactivate_success_notice + end + redirect_to scheme_details_path(@scheme) + end + + def reactivate + render "toggle_active", locals: { action: "reactivate" } + end + def new @scheme = Scheme.new end @@ -206,7 +239,8 @@ private :support_type, :arrangement_type, :intended_stay, - :confirmed) + :confirmed, + :deactivation_date) if arrangement_type_changed_to_different_org?(required_params) required_params[:managing_organisation_id] = nil @@ -251,7 +285,7 @@ private def authenticate_scope! head :unauthorized and return unless current_user.data_coordinator? || current_user.support? - if %w[show locations primary_client_group confirm_secondary_client_group secondary_client_group support details check_answers edit_name].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) + if %w[show locations primary_client_group confirm_secondary_client_group secondary_client_group support details check_answers edit_name deactivate].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) render_not_found and return end end @@ -259,4 +293,30 @@ private def redirect_if_scheme_confirmed redirect_to @scheme if @scheme.confirmed? end + + def deactivate_success_notice + case @scheme.status + when :deactivated + "#{@scheme.service_name} has been deactivated" + when :deactivating_soon + "#{@scheme.service_name} will deactivate on #{@scheme.deactivation_date.to_formatted_s(:govuk_date)}" + end + end + + def deactivation_date + if params[:scheme].blank? + return + elsif params[:scheme][:deactivation_date_type] == "default" + return FormHandler.instance.current_collection_start_date + elsif params[:scheme][:deactivation_date].present? + return params[:scheme][:deactivation_date] + end + + day = params[:scheme]["deactivation_date(3i)"] + month = params[:scheme]["deactivation_date(2i)"] + year = params[:scheme]["deactivation_date(1i)"] + return nil if [day, month, year].any?(&:blank?) + + Time.zone.local(year.to_i, month.to_i, day.to_i) if Date.valid_date?(year.to_i, month.to_i, day.to_i) + end end diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb index e21a4724e..2a02ebc94 100644 --- a/app/helpers/locations_helper.rb +++ b/app/helpers/locations_helper.rb @@ -23,7 +23,7 @@ module LocationsHelper resource.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } end - def display_attributes(location) + def display_location_attributes(location) [ { name: "Postcode", value: location.postcode }, { name: "Local authority", value: location.location_admin_district }, diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb new file mode 100644 index 000000000..0a042528b --- /dev/null +++ b/app/helpers/schemes_helper.rb @@ -0,0 +1,26 @@ +module SchemesHelper + def display_scheme_attributes(scheme) + base_attributes = [ + { name: "Scheme code", value: scheme.id_to_display }, + { name: "Name", value: scheme.service_name, edit: true }, + { name: "Confidential information", value: scheme.sensitive, edit: true }, + { name: "Type of scheme", value: scheme.scheme_type }, + { name: "Registered under Care Standards Act 2000", value: scheme.registered_under_care_act }, + { name: "Housing stock owned by", value: scheme.owning_organisation.name, edit: true }, + { name: "Support services provided by", value: scheme.arrangement_type }, + { name: "Organisation providing support", value: scheme.managing_organisation&.name }, + { name: "Primary client group", value: scheme.primary_client_group }, + { name: "Has another client group", value: scheme.has_other_client_group }, + { name: "Secondary client group", value: scheme.secondary_client_group }, + { name: "Level of support given", value: scheme.support_type }, + { name: "Intended length of stay", value: scheme.intended_stay }, + { name: "Availability", value: "Available from #{scheme.available_from.to_formatted_s(:govuk_date)}" }, + { name: "Status", value: scheme.status }, + ] + + if scheme.arrangement_type_same? + base_attributes.delete({ name: "Organisation providing support", value: scheme.managing_organisation&.name }) + end + base_attributes + end +end diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 2ea23a86a..6682f97fd 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -7,7 +7,9 @@ module TagHelper in_progress: "In progress", completed: "Completed", active: "Active", + incomplete: "Incomplete", deactivating_soon: "Deactivating soon", + reactivating_soon: "Reactivating soon", deactivated: "Deactivated", }.freeze @@ -17,7 +19,9 @@ module TagHelper in_progress: "blue", completed: "green", active: "green", + incomplete: "red", deactivating_soon: "yellow", + reactivating_soon: "blue", deactivated: "grey", }.freeze diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index c080e6e9b..61b981436 100644 --- a/app/models/form_handler.rb +++ b/app/models/form_handler.rb @@ -50,7 +50,7 @@ class FormHandler end def current_collection_start_date - Time.utc(current_collection_start_year, 4, 1) + Time.zone.local(current_collection_start_year, 4, 1) end def form_name_from_start_year(year, type) diff --git a/app/models/location.rb b/app/models/location.rb index ab1c3620a..2df620292 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -1,5 +1,6 @@ class Location < ApplicationRecord validate :validate_postcode + validate :deactivation_date_errors validates :units, :type_of_unit, :mobility_type, presence: true belongs_to :scheme has_many :lettings_logs, class_name: "LettingsLog" @@ -10,7 +11,7 @@ class Location < ApplicationRecord auto_strip_attributes :name - attr_accessor :add_another_location, :deactivation_date_type + attr_accessor :add_another_location, :deactivation_date_type, :run_deactivation_validations scope :search_by_postcode, ->(postcode) { where("REPLACE(postcode, ' ', '') ILIKE ?", "%#{postcode.delete(' ')}%") } scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } @@ -375,7 +376,37 @@ class Location < ApplicationRecord def status return :active if deactivation_date.blank? return :deactivating_soon if Time.zone.now < deactivation_date - return :deactivated if Time.zone.now >= deactivation_date + + :deactivated + end + + def active? + status == :active + end + + def run_deactivation_validations! + @run_deactivation_validations = true + end + + def implicit_run_deactivation_validations + deactivation_date.present? || @run_deactivation_validations + end + + def deactivation_date_errors + return unless implicit_run_deactivation_validations + + if deactivation_date.blank? + if deactivation_date_type.blank? + errors.add(:deactivation_date_type, message: I18n.t("validations.location.deactivation_date.not_selected")) + elsif deactivation_date_type == "other" + errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.invalid")) + end + else + collection_start_date = FormHandler.instance.current_collection_start_date + unless deactivation_date.between?(collection_start_date, Date.new(2200, 1, 1)) + errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.out_of_range", date: collection_start_date.to_formatted_s(:govuk_date))) + end + end end private diff --git a/app/models/organisation.rb b/app/models/organisation.rb index e420ca9d8..cc0df4ef6 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -77,7 +77,7 @@ class Organisation < ApplicationRecord DISPLAY_PROVIDER_TYPE[provider_type.to_sym] end - def display_attributes + def display_organisation_attributes [ { name: "Name", value: name, editable: true }, { name: "Address", value: address_string, editable: true }, diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 83899f494..5de804c58 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -18,9 +18,12 @@ class Scheme < ApplicationRecord } validate :validate_confirmed + validate :deactivation_date_errors auto_strip_attributes :service_name + attr_accessor :deactivation_date_type, :run_deactivation_validations + SENSITIVE = { No: 0, Yes: 1, @@ -153,29 +156,6 @@ class Scheme < ApplicationRecord ] end - def display_attributes - base_attributes = [ - { name: "Scheme code", value: id_to_display }, - { name: "Name", value: service_name, edit: true }, - { name: "Confidential information", value: sensitive, edit: true }, - { name: "Type of scheme", value: scheme_type }, - { name: "Registered under Care Standards Act 2000", value: registered_under_care_act }, - { name: "Housing stock owned by", value: owning_organisation.name, edit: true }, - { name: "Support services provided by", value: arrangement_type }, - { name: "Organisation providing support", value: managing_organisation&.name }, - { name: "Primary client group", value: primary_client_group }, - { name: "Has another client group", value: has_other_client_group }, - { name: "Secondary client group", value: secondary_client_group }, - { name: "Level of support given", value: support_type }, - { name: "Intended length of stay", value: intended_stay }, - ] - - if arrangement_type_same? - base_attributes.delete({ name: "Organisation providing support", value: managing_organisation&.name }) - end - base_attributes - end - def synonyms locations.map(&:postcode).join(",") end @@ -219,7 +199,7 @@ class Scheme < ApplicationRecord end def validate_confirmed - required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units has_other_client_group] + required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units has_other_client_group deactivation_date deactivation_date_type] if confirmed == true required_attributes.any? do |attribute| @@ -230,4 +210,44 @@ class Scheme < ApplicationRecord end end end + + def available_from + created_at + end + + def status + return :active if deactivation_date.blank? + return :deactivating_soon if Time.zone.now < deactivation_date + + :deactivated + end + + def active? + status == :active + end + + def run_deactivation_validations! + @run_deactivation_validations = true + end + + def implicit_run_deactivation_validations + deactivation_date.present? || @run_deactivation_validations + end + + def deactivation_date_errors + return unless implicit_run_deactivation_validations + + if deactivation_date.blank? + if deactivation_date_type.blank? + errors.add(:deactivation_date_type, message: I18n.t("validations.scheme.deactivation_date.not_selected")) + elsif deactivation_date_type == "other" + errors.add(:deactivation_date, message: I18n.t("validations.scheme.deactivation_date.invalid")) + end + else + collection_start_date = FormHandler.instance.current_collection_start_date + unless deactivation_date.between?(collection_start_date, Date.new(2200, 1, 1)) + errors.add(:deactivation_date, message: I18n.t("validations.scheme.deactivation_date.out_of_range", date: collection_start_date.to_formatted_s(:govuk_date))) + end + end + end end diff --git a/app/views/locations/toggle_active_confirm.html.erb b/app/views/locations/deactivate_confirm.html.erb similarity index 58% rename from app/views/locations/toggle_active_confirm.html.erb rename to app/views/locations/deactivate_confirm.html.erb index 452d66e48..e3f1ae175 100644 --- a/app/views/locations/toggle_active_confirm.html.erb +++ b/app/views/locations/deactivate_confirm.html.erb @@ -4,13 +4,14 @@ <% end %>

<%= @location.postcode %> - <%= "This change will affect #{@location.lettings_logs.count} logs" %> + This change will affect <%= @location.lettings_logs.count %> logs

<%= govuk_warning_text text: I18n.t("warnings.location.deactivation.review_logs") %> <%= f.hidden_field :confirm, value: true %> - <%= f.hidden_field :deactivation_date, value: deactivation_date %> -
+ <%= f.hidden_field :deactivation_date, value: @deactivation_date %> + <%= f.hidden_field :deactivation_date_type, value: @deactivation_date_type %> +
<%= f.govuk_submit "Deactivate this location" %> - <%= govuk_button_link_to "Cancel", scheme_location_path(scheme_id: @scheme, id: @location.id), html: { method: :get }, secondary: true %> + <%= govuk_button_link_to "Cancel", scheme_location_path(@scheme, @location), html: { method: :get }, secondary: true %>
<% end %> diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index ccc0163c2..52607d582 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -13,7 +13,7 @@
<%= govuk_summary_list do |summary_list| %> - <% display_attributes(@location).each do |attr| %> + <% display_location_attributes(@location).each do |attr| %> <%= summary_list.row do |row| %> <% row.key { attr[:name] } %> <% row.value { attr[:name].eql?("Status") ? status_tag(attr[:value]) : details_html(attr) } %> @@ -24,8 +24,8 @@
<% if FeatureToggle.location_toggle_enabled? %> - <% if @location.status == :active %> - <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %> + <% if @location.active? %> + <%= govuk_button_link_to "Deactivate this location", scheme_location_new_deactivation_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %> <% else %> <%= govuk_button_link_to "Reactivate this location", scheme_location_reactivate_path(scheme_id: @scheme.id, location_id: @location.id) %> <% end %> diff --git a/app/views/locations/toggle_active.html.erb b/app/views/locations/toggle_active.html.erb index 96cda6daa..6d2f4a9ba 100644 --- a/app/views/locations/toggle_active.html.erb +++ b/app/views/locations/toggle_active.html.erb @@ -4,11 +4,11 @@ <% content_for :before_content do %> <%= govuk_back_link( text: "Back", - href: scheme_location_path(scheme_id: @location.scheme.id, id: @location.id), + href: scheme_location_path(@location.scheme, @location), ) %> <% end %> -<%= form_with model: @location, url: scheme_location_deactivate_path(scheme_id: @location.scheme.id, location_id: @location.id), method: "patch", local: true do |f| %> +<%= form_with model: @location, url: scheme_location_new_deactivation_path(scheme_id: @location.scheme.id, location_id: @location.id), method: "patch", local: true do |f| %>
<% collection_start_date = FormHandler.instance.current_collection_start_date %> @@ -28,7 +28,7 @@ **basic_conditional_html_attributes({ "deactivation_date" => %w[other] }, "location") do %> <%= f.govuk_date_field :deactivation_date, legend: { text: "Date", size: "m" }, - hint: { text: "For example, 27 3 2008" }, + hint: { text: "For example, 27 3 2022" }, width: 20 %> <% end %> <% end %> diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 5cd8b64e0..bbe9ae67a 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -14,7 +14,7 @@
<%= govuk_summary_list do |summary_list| %> - <% @organisation.display_attributes.each do |attr| %> + <% @organisation.display_organisation_attributes.each do |attr| %> <% if can_edit_org?(current_user) && attr[:editable] %> <%= summary_list.row do |row| %> <% row.key { attr[:name].to_s.humanize } %> diff --git a/app/views/schemes/deactivate_confirm.html.erb b/app/views/schemes/deactivate_confirm.html.erb new file mode 100644 index 000000000..45f7d4341 --- /dev/null +++ b/app/views/schemes/deactivate_confirm.html.erb @@ -0,0 +1,19 @@ +<% title = "Deactivate #{@scheme.service_name}" %> +<% content_for :title, title %> +<%= form_with model: @scheme, url: scheme_deactivate_path(@scheme), method: "patch", local: true do |f| %> + <% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> + <% end %> +

+ <%= @scheme.service_name %> + This change will affect <%= @scheme.lettings_logs.count %> logs +

+ <%= govuk_warning_text text: I18n.t("warnings.scheme.deactivation.review_logs") %> + <%= f.hidden_field :confirm, value: true %> + <%= f.hidden_field :deactivation_date, value: @deactivation_date %> + <%= f.hidden_field :deactivation_date_type, value: @deactivation_date_type %> +
+ <%= f.govuk_submit "Deactivate this scheme" %> + <%= govuk_button_link_to "Cancel", scheme_details_path(@scheme), html: { method: :get }, secondary: true %> +
+<% end %> diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb index 75cb533f7..2c01c06f1 100644 --- a/app/views/schemes/show.html.erb +++ b/app/views/schemes/show.html.erb @@ -15,12 +15,20 @@

Scheme

<%= govuk_summary_list do |summary_list| %> - <% @scheme.display_attributes.each do |attr| %> + <% display_scheme_attributes(@scheme).each do |attr| %> <% next if current_user.data_coordinator? && attr[:name] == ("Housing stock owned by") %> <%= summary_list.row do |row| %> <% row.key { attr[:name].eql?("Registered under Care Standards Act 2000") ? "Registered under Care Standards Act 2000" : attr[:name].to_s.humanize } %> - <% row.value { details_html(attr) } %> + <% row.value { attr[:name].eql?("Status") ? status_tag(attr[:value]) : details_html(attr) } %> <% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] %> <% end %> <% end %> <% end %> + +<% if FeatureToggle.scheme_toggle_enabled? %> + <% if @scheme.active? %> + <%= govuk_button_link_to "Deactivate this scheme", scheme_new_deactivation_path(@scheme), warning: true %> + <% else %> + <%= govuk_button_link_to "Reactivate this scheme", scheme_reactivate_path(@scheme) %> + <% end %> +<% end %> diff --git a/app/views/schemes/toggle_active.html.erb b/app/views/schemes/toggle_active.html.erb new file mode 100644 index 000000000..11dd2f276 --- /dev/null +++ b/app/views/schemes/toggle_active.html.erb @@ -0,0 +1,35 @@ +<% title = "#{action.humanize} #{@scheme.service_name}" %> +<% content_for :title, title %> +<% content_for :before_content do %> + <%= govuk_back_link( + text: "Back", + href: scheme_details_path(@scheme), + ) %> +<% end %> +<%= form_with model: @scheme, url: scheme_new_deactivation_path(@scheme), method: "patch", local: true do |f| %> +
+
+ <% collection_start_date = FormHandler.instance.current_collection_start_date %> + <%= f.govuk_error_summary %> + <%= f.govuk_radio_buttons_fieldset :deactivation_date_type, + legend: { text: I18n.t("questions.scheme.deactivation.apply_from") }, + caption: { text: title }, + hint: { text: I18n.t("hints.scheme.deactivation", date: collection_start_date.to_formatted_s(:govuk_date)) } do %> + <%= govuk_warning_text text: I18n.t("warnings.scheme.deactivation.existing_logs") %> + <%= f.govuk_radio_button :deactivation_date_type, + "default", + label: { text: "From the start of the current collection period (#{collection_start_date.to_formatted_s(:govuk_date)})" } %> + <%= f.govuk_radio_button :deactivation_date_type, + "other", + label: { text: "For tenancies starting after a certain date" }, + **basic_conditional_html_attributes({ "deactivation_date" => %w[other] }, "scheme") do %> + <%= f.govuk_date_field :deactivation_date, + legend: { text: "Date", size: "m" }, + hint: { text: "For example, 27 3 2022" }, + width: 20 %> + <% end %> + <% end %> + <%= f.govuk_submit "Continue" %> +
+
+<% end %> diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index 1a4b8a862..7cd75ddd3 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -4,21 +4,19 @@ class FeatureToggle end def self.sales_log_enabled? - return true unless Rails.env.production? - - false + !Rails.env.production? end def self.managing_owning_enabled? - return true unless Rails.env.production? + !Rails.env.production? + end - false + def self.scheme_toggle_enabled? + !Rails.env.production? end def self.location_toggle_enabled? - return true unless Rails.env.production? - - false + !Rails.env.production? end def self.managing_for_other_user_enabled? diff --git a/config/locales/en.yml b/config/locales/en.yml index 3c4808696..a1c18330a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -312,11 +312,16 @@ en: declaration: missing: "You must show the DLUHC privacy notice to the tenant before you can submit this log." + scheme: + deactivation_date: + not_selected: "Select one of the options" + invalid: "Enter a valid day, month and year" + out_of_range: "The date must be on or after the %{date}" + location: deactivation_date: not_selected: "Select one of the options" - not_entered: "Enter a %{period}" - invalid: "Enter a valid date" + invalid: "Enter a valid day, month and year" out_of_range: "The date must be on or after the %{date}" soft_validations: @@ -371,6 +376,9 @@ en: mobility_type: "What are the mobility standards for the majority of units in this location?" deactivation: apply_from: "When should this change apply?" + scheme: + deactivation: + apply_from: "When should this change apply?" descriptions: location: mobility_type: @@ -384,12 +392,18 @@ en: name: "This is how you refer to this location within your organisation" units: "A unit can be a bedroom in a shared house or flat, or a house with 4 bedrooms. Do not include bedrooms used for wardens, managers, volunteers or sleep-in staff." deactivation: "If the date is before %{date}, select ‘From the start of the current collection period’ because the previous period has now closed." - + scheme: + deactivation: "If the date is before %{date}, select ‘From the start of the current collection period’ because the previous period has now closed." + warnings: location: - deactivation: + deactivation: existing_logs: "It will not be possible to add logs with this location if their tenancy start date is on or after the date you enter. Any existing logs may be affected." review_logs: "Your data providers will need to review these logs and answer a few questions again. We’ll email each log creator with a list of logs that need updating." + scheme: + deactivation: + existing_logs: "It will not be possible to add logs with this scheme if their tenancy start date is on or after the date you enter. Any existing logs may be affected." + review_logs: "Your data providers will need to review these logs and answer a few questions again. We’ll email each log creator with a list of logs that need updating." test: one_argument: "This is based on the tenant’s work situation: %{ecstat1}" diff --git a/config/routes.rb b/config/routes.rb index 8bd78d6e6..f90f70534 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,12 +49,19 @@ Rails.application.routes.draw do get "check-answers", to: "schemes#check_answers" get "edit-name", to: "schemes#edit_name" get "support-services-provider", to: "schemes#support_services_provider" + get "new-deactivation", to: "schemes#new_deactivation" + get "deactivate-confirm", to: "schemes#deactivate_confirm" + get "reactivate", to: "schemes#reactivate" + patch "new-deactivation", to: "schemes#new_deactivation" + patch "deactivate", to: "schemes#deactivate" resources :locations do get "edit-name", to: "locations#edit_name" get "edit-local-authority", to: "locations#edit_local_authority" - get "deactivate", to: "locations#deactivate" + get "new-deactivation", to: "locations#new_deactivation" + get "deactivate-confirm", to: "locations#deactivate_confirm" get "reactivate", to: "locations#reactivate" + patch "new-deactivation", to: "locations#new_deactivation" patch "deactivate", to: "locations#deactivate" end end diff --git a/db/migrate/20221110163351_add_deactivation_date_to_schemes.rb b/db/migrate/20221110163351_add_deactivation_date_to_schemes.rb new file mode 100644 index 000000000..6d717268f --- /dev/null +++ b/db/migrate/20221110163351_add_deactivation_date_to_schemes.rb @@ -0,0 +1,5 @@ +class AddDeactivationDateToSchemes < ActiveRecord::Migration[7.0] + def change + add_column :schemes, :deactivation_date, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 0f2ad0025..1ff8f157c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -364,15 +364,15 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do t.integer "la_known" t.integer "income1" t.integer "income1nk" + t.integer "details_known_2" + t.integer "details_known_3" + t.integer "details_known_4" t.integer "age4" t.integer "age4_known" t.integer "age5" t.integer "age5_known" t.integer "age6" t.integer "age6_known" - t.integer "details_known_2" - t.integer "details_known_3" - t.integer "details_known_4" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["managing_organisation_id"], name: "index_sales_logs_on_managing_organisation_id" t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id" @@ -398,6 +398,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do t.string "old_visible_id" t.integer "total_units" t.boolean "confirmed" + t.datetime "deactivation_date" t.index ["managing_organisation_id"], name: "index_schemes_on_managing_organisation_id" t.index ["owning_organisation_id"], name: "index_schemes_on_owning_organisation_id" end diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 48ab31acc..1500a728c 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -779,7 +779,7 @@ RSpec.describe "Schemes scheme Features" do it "allows to deactivate a location" do click_link("Deactivate this location") - expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/deactivate") + expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/new-deactivation") end context "when I press the back button" do diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb index f2a5a67a5..2adfab35f 100644 --- a/spec/helpers/locations_helper_spec.rb +++ b/spec/helpers/locations_helper_spec.rb @@ -47,7 +47,7 @@ RSpec.describe LocationsHelper do end end - describe "display_attributes" do + describe "display_location_attributes" do let(:location) { FactoryBot.build(:location, startdate: Time.zone.local(2022, 8, 8)) } it "returns correct display attributes" do @@ -63,12 +63,12 @@ RSpec.describe LocationsHelper do { name: "Status", value: :active }, ] - expect(display_attributes(location)).to eq(attributes) + expect(display_location_attributes(location)).to eq(attributes) end it "displays created_at as availability date if startdate is not present" do location.update!(startdate: nil) - availability_attribute = display_attributes(location).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Available from #{location.created_at.to_formatted_s(:govuk_date)}") end diff --git a/spec/helpers/schemes_helper_spec.rb b/spec/helpers/schemes_helper_spec.rb new file mode 100644 index 000000000..46dfbd7c2 --- /dev/null +++ b/spec/helpers/schemes_helper_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +RSpec.describe SchemesHelper do + describe "display_scheme_attributes" do + let!(:scheme) { FactoryBot.create(:scheme, created_at: Time.zone.local(2022, 8, 8)) } + + it "returns correct display attributes" do + attributes = [ + { name: "Scheme code", value: scheme.id_to_display }, + { name: "Name", value: scheme.service_name, edit: true }, + { name: "Confidential information", value: scheme.sensitive, edit: true }, + { name: "Type of scheme", value: scheme.scheme_type }, + { name: "Registered under Care Standards Act 2000", value: scheme.registered_under_care_act }, + { name: "Housing stock owned by", value: scheme.owning_organisation.name, edit: true }, + { name: "Support services provided by", value: scheme.arrangement_type }, + { name: "Primary client group", value: scheme.primary_client_group }, + { name: "Has another client group", value: scheme.has_other_client_group }, + { name: "Secondary client group", value: scheme.secondary_client_group }, + { name: "Level of support given", value: scheme.support_type }, + { name: "Intended length of stay", value: scheme.intended_stay }, + { name: "Availability", value: "Available from 8 August 2022" }, + { name: "Status", value: :active }, + ] + expect(display_scheme_attributes(scheme)).to eq(attributes) + end + end +end diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index e4056c6c1..e0ef853ae 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -120,7 +120,7 @@ RSpec.describe FormHandler do end it "returns the correct current start date" do - expect(form_handler.current_collection_start_date).to eq(Time.utc(2022, 4, 1)) + expect(form_handler.current_collection_start_date).to eq(Time.zone.local(2022, 4, 1)) end end diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb index 8f9f452bc..069bb21d7 100644 --- a/spec/models/location_spec.rb +++ b/spec/models/location_spec.rb @@ -121,26 +121,38 @@ RSpec.describe Location, type: :model do it "returns active if the location is not deactivated" do location.deactivation_date = nil + location.deactivation_date_type = nil location.save! expect(location.status).to eq(:active) end it "returns deactivating soon if deactivation_date is in the future" do location.deactivation_date = Time.zone.local(2022, 8, 8) + location.deactivation_date_type = "other" location.save! expect(location.status).to eq(:deactivating_soon) end it "returns deactivated if deactivation_date is in the past" do location.deactivation_date = Time.zone.local(2022, 4, 8) + location.deactivation_date_type = "other" location.save! expect(location.status).to eq(:deactivated) end it "returns deactivated if deactivation_date is today" do location.deactivation_date = Time.zone.local(2022, 6, 7) + location.deactivation_date_type = "other" location.save! expect(location.status).to eq(:deactivated) end end + + describe "with deactivation_date (but no deactivation_date_type)" do + let(:location) { FactoryBot.create(:location, deactivation_date: Date.new(2022, 4, 1)) } + + it "is valid" do + expect(location).to be_valid + end + end end diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb index 9d17d888c..b9e8fd886 100644 --- a/spec/models/scheme_spec.rb +++ b/spec/models/scheme_spec.rb @@ -91,4 +91,48 @@ RSpec.describe Scheme, type: :model do end end end + + describe "status" do + let(:scheme) { FactoryBot.build(:scheme) } + + before do + Timecop.freeze(2022, 6, 7) + end + + it "returns active if the scheme is not deactivated" do + scheme.deactivation_date = nil + scheme.deactivation_date_type = nil + scheme.save! + expect(scheme.status).to eq(:active) + end + + it "returns deactivating soon if deactivation_date is in the future" do + scheme.deactivation_date = Time.zone.local(2022, 8, 8) + scheme.deactivation_date_type = "other" + scheme.save! + expect(scheme.status).to eq(:deactivating_soon) + end + + it "returns deactivated if deactivation_date is in the past" do + scheme.deactivation_date = Time.zone.local(2022, 4, 8) + scheme.deactivation_date_type = "other" + scheme.save! + expect(scheme.status).to eq(:deactivated) + end + + it "returns deactivated if deactivation_date is today" do + scheme.deactivation_date = Time.zone.local(2022, 6, 7) + scheme.deactivation_date_type = "other" + scheme.save! + expect(scheme.status).to eq(:deactivated) + end + end + + describe "with deactivation_date (but no deactivation_date_type)" do + let(:scheme) { FactoryBot.create(:scheme, deactivation_date: Date.new(2022, 4, 1)) } + + it "is valid" do + expect(scheme).to be_valid + end + end end diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index a666b6495..2aca9bd93 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -1245,29 +1245,37 @@ RSpec.describe LocationsController, type: :request do before do Timecop.freeze(Time.utc(2022, 10, 10)) sign_in user - patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params: + patch "/schemes/#{scheme.id}/locations/#{location.id}/new-deactivation", params: end context "with default date" do - let(:params) { { location: { deactivation_date_type: "default" } } } + let(:params) { { location: { deactivation_date_type: "default", deactivation_date: } } } - it "renders the confirmation page" do + it "redirects to the confirmation page" do + follow_redirect! expect(response).to have_http_status(:ok) expect(page).to have_content("This change will affect #{location.lettings_logs.count} logs") end end context "with other date" do - let(:params) { { location: { deactivation_date: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "10", "deactivation_date(1i)": "2022" } } } + let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "10", "deactivation_date(1i)": "2022" } } } - it "renders the confirmation page" do + it "redirects to the confirmation page" do + follow_redirect! expect(response).to have_http_status(:ok) expect(page).to have_content("This change will affect #{location.lettings_logs.count} logs") end end context "when confirming deactivation" do - let(:params) { { location: { deactivation_date:, confirm: true } } } + let(:params) { { location: { deactivation_date:, confirm: true, deactivation_date_type: "other" } } } + + before do + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params: + end it "updates existing location with valid deactivation date and renders location page" do follow_redirect! @@ -1310,7 +1318,7 @@ RSpec.describe LocationsController, type: :request do it "displays page with an error message" do expect(response).to have_http_status(:unprocessable_entity) - expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "day")) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.invalid")) end end @@ -1319,7 +1327,7 @@ RSpec.describe LocationsController, type: :request do it "displays page with an error message" do expect(response).to have_http_status(:unprocessable_entity) - expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "month")) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.invalid")) end end @@ -1328,7 +1336,7 @@ RSpec.describe LocationsController, type: :request do it "displays page with an error message" do expect(response).to have_http_status(:unprocessable_entity) - expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "year")) + expect(page).to have_content(I18n.t("validations.location.deactivation_date.invalid")) end end end @@ -1365,21 +1373,24 @@ RSpec.describe LocationsController, type: :request do Timecop.freeze(Time.utc(2022, 10, 10)) sign_in user location.deactivation_date = deactivation_date + location.deactivation_date_type = deactivation_date_type location.save! get "/schemes/#{scheme.id}/locations/#{location.id}" end context "with active location" do let(:deactivation_date) { nil } + let(:deactivation_date_type) { nil } it "renders deactivate this location" do expect(response).to have_http_status(:ok) - expect(page).to have_link("Deactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/deactivate") + expect(page).to have_link("Deactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/new-deactivation") end end context "with deactivated location" do let(:deactivation_date) { Time.utc(2022, 10, 9) } + let(:deactivation_date_type) { "other" } it "renders reactivate this location" do expect(response).to have_http_status(:ok) @@ -1389,6 +1400,7 @@ RSpec.describe LocationsController, type: :request do context "with location that's deactivating soon" do let(:deactivation_date) { Time.utc(2022, 10, 12) } + let(:deactivation_date_type) { "other" } it "renders reactivate this location" do expect(response).to have_http_status(:ok) diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index d3d22513a..9b0e934f6 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -59,7 +59,7 @@ RSpec.describe OrganisationsController, type: :request do expect(page).to have_field("search", type: "search") end - it "has hidden accebility field with description" do + it "has hidden accessibility field with description" do expected_field = "

Supported housing schemes

" expect(CGI.unescape_html(response.body)).to include(expected_field) end diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index a5c0b6e4f..5ad18663e 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -242,6 +242,50 @@ RSpec.describe SchemesController, type: :request do expect(response).to have_http_status(:not_found) end end + + context "when looking at scheme details" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + + before do + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + scheme.deactivation_date = deactivation_date + scheme.deactivation_date_type = deactivation_date_type + scheme.save! + get "/schemes/#{scheme.id}" + end + + context "with active scheme" do + let(:deactivation_date) { nil } + let(:deactivation_date_type) { nil } + + it "renders deactivate this scheme" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Deactivate this scheme", href: "/schemes/#{scheme.id}/new-deactivation") + end + end + + context "with deactivated scheme" do + let(:deactivation_date) { Time.utc(2022, 10, 9) } + let(:deactivation_date_type) { "other" } + + it "renders reactivate this scheme" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Reactivate this scheme", href: "/schemes/#{scheme.id}/reactivate") + end + end + + context "with scheme that's deactivating soon" do + let(:deactivation_date) { Time.utc(2022, 10, 12) } + let(:deactivation_date_type) { "other" } + + it "renders reactivate this scheme" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Reactivate this scheme", href: "/schemes/#{scheme.id}/reactivate") + end + end + end end context "when signed in as a support user" do @@ -1698,4 +1742,133 @@ RSpec.describe SchemesController, type: :request do end end end + + describe "#deactivate" do + context "when not signed in" do + it "redirects to the sign in page" do + patch "/schemes/1/new-deactivation" + expect(response).to redirect_to("/account/sign-in") + end + end + + context "when signed in as a data provider" do + let(:user) { FactoryBot.create(:user) } + + before do + sign_in user + patch "/schemes/1/new-deactivation" + end + + it "returns 401 unauthorized" do + request + expect(response).to have_http_status(:unauthorized) + end + end + + context "when signed in as a data coordinator" do + let(:user) { FactoryBot.create(:user, :data_coordinator) } + let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let(:startdate) { Time.utc(2021, 1, 2) } + let(:deactivation_date) { Time.utc(2022, 10, 10) } + + before do + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + patch "/schemes/#{scheme.id}/new-deactivation", params: + end + + context "with default date" do + let(:params) { { scheme: { deactivation_date_type: "default", deactivation_date: } } } + + it "redirects to the confirmation page" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("This change will affect #{scheme.lettings_logs.count} logs") + end + end + + context "with other date" do + let(:params) { { scheme: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "10", "deactivation_date(1i)": "2022" } } } + + it "redirects to the confirmation page" do + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("This change will affect #{scheme.lettings_logs.count} logs") + end + end + + context "when confirming deactivation" do + let(:params) { { scheme: { deactivation_date:, confirm: true, deactivation_date_type: "other" } } } + + before do + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + patch "/schemes/#{scheme.id}/deactivate", params: + end + + it "updates existing scheme with valid deactivation date and renders scheme page" do + follow_redirect! + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + scheme.reload + expect(scheme.deactivation_date).to eq(deactivation_date) + end + end + + context "when the date is not selected" do + let(:params) { { scheme: { "deactivation_date": "" } } } + + it "displays the new page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.scheme.deactivation_date.not_selected")) + end + end + + context "when invalid date is entered" do + let(:params) { { scheme: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "44", "deactivation_date(1i)": "2022" } } } + + it "displays the new page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.scheme.deactivation_date.invalid")) + end + end + + context "when the date is entered is before the beginning of current collection window" do + let(:params) { { scheme: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "4", "deactivation_date(1i)": "2020" } } } + + it "displays the new page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.scheme.deactivation_date.out_of_range", date: "1 April 2022")) + end + end + + context "when the day is not entered" do + let(:params) { { scheme: { deactivation_date_type: "other", "deactivation_date(3i)": "", "deactivation_date(2i)": "2", "deactivation_date(1i)": "2022" } } } + + it "displays page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.scheme.deactivation_date.invalid")) + end + end + + context "when the month is not entered" do + let(:params) { { scheme: { deactivation_date_type: "other", "deactivation_date(3i)": "2", "deactivation_date(2i)": "", "deactivation_date(1i)": "2022" } } } + + it "displays page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.scheme.deactivation_date.invalid")) + end + end + + context "when the year is not entered" do + let(:params) { { scheme: { deactivation_date_type: "other", "deactivation_date(3i)": "2", "deactivation_date(2i)": "2", "deactivation_date(1i)": "" } } } + + it "displays page with an error message" do + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content(I18n.t("validations.scheme.deactivation_date.invalid")) + end + end + end + end end