From ce3b14c8112e9066f3848d8b9dfef3b2d7d90eed Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Tue, 15 Nov 2022 16:38:50 +0000 Subject: [PATCH] feat: duplicate behaviour schemes -> locations (+ tests) --- app/controllers/locations_controller.rb | 58 +++++++++++++------ app/models/location.rb | 32 +++++----- app/views/locations/show.html.erb | 2 +- app/views/locations/toggle_active.html.erb | 2 +- .../locations/toggle_active_confirm.html.erb | 3 +- config/routes.rb | 4 +- spec/features/schemes_spec.rb | 2 +- spec/models/location_spec.rb | 4 ++ spec/requests/locations_controller_spec.rb | 26 ++++++--- 9 files changed, 85 insertions(+), 48 deletions(-) diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index a6cfe475c..f4e48c665 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -20,22 +20,32 @@ 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 - @location.deactivation_date_errors(params) - 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 = true + @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 + render "toggle_active_confirm", locals: { action: "deactivate", deactivation_date: params[:deactivation_date], deactivation_date_type: params[:deactivation_date_type] } + end + + def deactivate + @location.run_deactivation_validations = true + @location.deactivation_date = deactivation_date + @location.deactivation_date_type = params[:location][:deactivation_date_type] + confirm_deactivation + end + def reactivate render "toggle_active", locals: { action: "reactivate" } end @@ -144,13 +154,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 @@ -164,23 +174,35 @@ private end def confirm_deactivation - if @location.update(deactivation_date: params[:location][:deactivation_date]) - flash[:notice] = "#{@location.name || @location.postcode} has been deactivated" + if @location.update!(deactivation_date: @location.deactivation_date) + flash[:notice] = success_text end redirect_to scheme_location_path(@scheme, @location) end - def deactivation_date - return if params[:location].blank? + def success_text + 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 - 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? + def deactivation_date + 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.utc(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/models/location.rb b/app/models/location.rb index a877ce549..1647f8c8e 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}%") } @@ -383,25 +384,20 @@ class Location < ApplicationRecord status == :active end - def deactivation_date_errors(params) - if params[:location][:deactivation_date].blank? && params[:location][:deactivation_date_type].blank? - 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)"] + def implicit_run_deactivation_validations + deactivation_date.present? || @run_deactivation_validations + end - collection_start_date = FormHandler.instance.current_collection_start_date + def deactivation_date_errors + return unless implicit_run_deactivation_validations - if [day, month, year].any?(&:blank?) - errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.invalid")) - elsif !Date.valid_date?(year.to_i, month.to_i, day.to_i) - 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)) - errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.out_of_range", date: collection_start_date.to_formatted_s(:govuk_date))) - end + collection_start_date = FormHandler.instance.current_collection_start_date + if deactivation_date_type.blank? + errors.add(:deactivation_date_type, message: I18n.t("validations.location.deactivation_date.not_selected")) + elsif deactivation_date.blank? + errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.invalid")) + elsif !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 diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index 8b7ae9e0c..52607d582 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -25,7 +25,7 @@ <% if FeatureToggle.location_toggle_enabled? %> <% if @location.active? %> - <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %> + <%= 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 131cbda43..f5f6e1144 100644 --- a/app/views/locations/toggle_active.html.erb +++ b/app/views/locations/toggle_active.html.erb @@ -8,7 +8,7 @@ ) %> <% 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 %> diff --git a/app/views/locations/toggle_active_confirm.html.erb b/app/views/locations/toggle_active_confirm.html.erb index c9c58f331..908edceb7 100644 --- a/app/views/locations/toggle_active_confirm.html.erb +++ b/app/views/locations/toggle_active_confirm.html.erb @@ -9,7 +9,8 @@ <%= 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_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 %>
diff --git a/config/routes.rb b/config/routes.rb index a5e13b4cd..f90f70534 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -58,8 +58,10 @@ 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" + 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/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 48f386fa7..9aab9baf2 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/models/location_spec.rb b/spec/models/location_spec.rb index 8f9f452bc..304339af7 100644 --- a/spec/models/location_spec.rb +++ b/spec/models/location_spec.rb @@ -121,24 +121,28 @@ 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 diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index 33a082018..43cd3e66c 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_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! @@ -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)