diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index ebaf9e64c..a3ad65170 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -1,5 +1,8 @@ class LettingsLogsController < LogsController - before_action :find_resource, except: %i[create index edit] + rescue_from ActiveRecord::RecordNotFound, with: :render_not_found + + before_action :find_resource, only: %i[update show] + before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv] before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv] before_action :authenticate_scope!, only: %i[download_csv email_csv] @@ -55,28 +58,31 @@ class LettingsLogsController < LogsController end def edit - @log = current_user.lettings_logs.find_by(id: params[:id]) - if @log - if @log.unresolved - redirect_to(send(@log.form.unresolved_log_path, @log)) - else - render("logs/edit", locals: { current_user: }) - end + @log = current_user.lettings_logs.find(params[:id]) + + if @log.unresolved + redirect_to(send(@log.form.unresolved_log_path, @log)) else - render_not_found + render("logs/edit", locals: { current_user: }) end end def destroy - if @log - if @log.delete - head :no_content - else - render json: { errors: @log.errors.messages }, status: :unprocessable_entity - end - else - render_not_found_json("Log", params[:id]) - end + @log = LettingsLog.visible.find(params[:id]) + + authorize @log + + @log.discard! + + redirect_to lettings_logs_path, notice: "Log #{@log.id} has been deleted" + end + + def delete_confirmation + @log = LettingsLog.visible.find(params[:lettings_log_id]) + + authorize @log, :destroy? + + render "logs/delete_confirmation" end def download_csv @@ -105,14 +111,14 @@ class LettingsLogsController < LogsController end end +private + def org_params super.merge( { "managing_organisation_id" => current_user.organisation.id }, ) end -private - def authenticate_scope! head :unauthorized and return if codes_only_export? && !current_user.support? end diff --git a/app/controllers/sales_logs_controller.rb b/app/controllers/sales_logs_controller.rb index 246d61b8c..7ed52be2d 100644 --- a/app/controllers/sales_logs_controller.rb +++ b/app/controllers/sales_logs_controller.rb @@ -1,4 +1,6 @@ class SalesLogsController < LogsController + rescue_from ActiveRecord::RecordNotFound, with: :render_not_found + before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv] before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv] before_action :authenticate_scope!, only: %i[download_csv email_csv] @@ -32,12 +34,26 @@ class SalesLogsController < LogsController end def edit - @log = current_user.sales_logs.visible.find_by(id: params[:id]) - if @log - render "logs/edit", locals: { current_user: } - else - render_not_found - end + @log = current_user.sales_logs.visible.find(params[:id]) + render "logs/edit", locals: { current_user: } + end + + def destroy + @log = SalesLog.visible.find(params[:id]) + + authorize @log + + @log.discard! + + redirect_to sales_logs_path, notice: "Log #{@log.id} has been deleted" + end + + def delete_confirmation + @log = SalesLog.visible.find(params[:sales_log_id]) + + authorize @log, :destroy? + + render "logs/delete_confirmation" end def download_csv diff --git a/app/helpers/log_actions_helper.rb b/app/helpers/log_actions_helper.rb new file mode 100644 index 000000000..760a5523b --- /dev/null +++ b/app/helpers/log_actions_helper.rb @@ -0,0 +1,46 @@ +module LogActionsHelper + include GovukLinkHelper + + def edit_actions_for_log(log) + back = back_button_for(log) + delete = delete_button_for_log(log) + + return if back.nil? && delete.nil? + + content_tag(:div, class: "govuk-button-group") do + safe_join([back, delete]) + end + end + +private + + def back_button_for(log) + if log.completed? + if log.bulk_uploaded? + if log.lettings? + govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(log.bulk_upload) + else + govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_sales_result_path(log.bulk_upload) + end + elsif log.lettings? + govuk_button_link_to "Back to lettings logs", lettings_logs_path + elsif log.sales? + govuk_button_link_to "Back to sales logs", sales_logs_path + end + end + end + + def policy_class_for(log) + log.lettings? ? LettingsLogPolicy : SalesLogPolicy + end + + def delete_button_for_log(log) + if policy_class_for(log).new(current_user, log).destroy? + govuk_button_link_to( + "Delete log", + lettings_log_delete_confirmation_path(log), + warning: true, + ) + end + end +end diff --git a/app/models/log.rb b/app/models/log.rb index 6b4af1009..512a22048 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -15,11 +15,13 @@ class Log < ApplicationRecord "in_progress" => 1, "completed" => 2, "pending" => 3, + "deleted" => 4, }.freeze enum status: STATUS enum status_cache: STATUS, _prefix: true scope :visible, -> { where(status: %w[not_started in_progress completed]) } + scope :exportable, -> { where(status: %w[not_started in_progress completed deleted]) } scope :filter_by_status, ->(status, _user = nil) { where status: } scope :filter_by_years, lambda { |years, _user = nil| @@ -129,7 +131,13 @@ class Log < ApplicationRecord end end + def discard! + update!(status: "deleted", discarded_at: Time.zone.now) + end + def calculate_status + return "deleted" if discarded_at.present? + if all_fields_completed? && errors.empty? "completed" elsif all_fields_nil? diff --git a/app/policies/lettings_log_policy.rb b/app/policies/lettings_log_policy.rb new file mode 100644 index 000000000..1d92cd72f --- /dev/null +++ b/app/policies/lettings_log_policy.rb @@ -0,0 +1,27 @@ +class LettingsLogPolicy + attr_reader :user, :log + + def initialize(user, log) + @user = user + @log = log + end + + def destroy? + return false unless log && user + + # Can only delete editable logs + return false unless log.collection_period_open? + + # Only delete logs with answered questions + return false unless log.in_progress? || log.completed? + + # Support users can delete any log + return true if user.support? + + # Data coordinators can delete any log visible to them + return true if user.data_coordinator? && user.lettings_logs.visible.include?(log) + + # Data providers can only delete the log if it is assigned to them + log.created_by == user + end +end diff --git a/app/policies/sales_log_policy.rb b/app/policies/sales_log_policy.rb new file mode 100644 index 000000000..671690831 --- /dev/null +++ b/app/policies/sales_log_policy.rb @@ -0,0 +1,27 @@ +class SalesLogPolicy + attr_reader :user, :log + + def initialize(user, log) + @user = user + @log = log + end + + def destroy? + return false unless log && user + + # Can only delete editable logs + return false unless log.collection_period_open? + + # Only delete logs with answered questions + return false unless log.in_progress? || log.completed? + + # Support users can delete any log + return true if user.support? + + # Data coordinators can delete any log visible to them + return true if user.data_coordinator? && user.sales_logs.visible.include?(log) + + # Data providers can only delete the log if it is assigned to them + log.created_by == user + end +end diff --git a/app/services/csv/lettings_log_csv_service.rb b/app/services/csv/lettings_log_csv_service.rb index e0815d164..e8c2815f5 100644 --- a/app/services/csv/lettings_log_csv_service.rb +++ b/app/services/csv/lettings_log_csv_service.rb @@ -1,6 +1,6 @@ module Csv class LettingsLogCsvService - CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check first_time_property_let_as_social_housing renttype needstype postcode_known is_la_inferred totchild totelder totadult net_income_known is_carehome previous_la_known is_previous_la_inferred age1_known age2_known age3_known age4_known age5_known age6_known age7_known age8_known letting_allocation_unknown details_known_2 details_known_3 details_known_4 details_known_5 details_known_6 details_known_7 details_known_8 rent_type_detail wrent wscharge wpschrge wsupchrg wtcharge wtshortfall rent_value_check old_form_id old_id retirement_value_check tshortfall_known pregnancy_value_check hhtype new_old vacdays la prevloc unresolved updated_by_id bulk_upload_id uprn_confirmed status_cache].freeze + CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check first_time_property_let_as_social_housing renttype needstype postcode_known is_la_inferred totchild totelder totadult net_income_known is_carehome previous_la_known is_previous_la_inferred age1_known age2_known age3_known age4_known age5_known age6_known age7_known age8_known letting_allocation_unknown details_known_2 details_known_3 details_known_4 details_known_5 details_known_6 details_known_7 details_known_8 rent_type_detail wrent wscharge wpschrge wsupchrg wtcharge wtshortfall rent_value_check old_form_id old_id retirement_value_check tshortfall_known pregnancy_value_check hhtype new_old vacdays la prevloc unresolved updated_by_id bulk_upload_id uprn_confirmed status_cache discarded_at].freeze def initialize(user, export_type:) @user = user diff --git a/app/services/exports/lettings_log_export_service.rb b/app/services/exports/lettings_log_export_service.rb index 429361913..8749d682f 100644 --- a/app/services/exports/lettings_log_export_service.rb +++ b/app/services/exports/lettings_log_export_service.rb @@ -119,10 +119,10 @@ module Exports if !full_update && recent_export params = { from: recent_export.started_at, to: start_time } - LettingsLog.visible.where("updated_at >= :from and updated_at <= :to", params) + LettingsLog.exportable.where("updated_at >= :from and updated_at <= :to", params) else params = { to: start_time } - LettingsLog.visible.where("updated_at <= :to", params) + LettingsLog.exportable.where("updated_at <= :to", params) end end diff --git a/app/views/logs/delete_confirmation.html.erb b/app/views/logs/delete_confirmation.html.erb new file mode 100644 index 000000000..247e6b1ba --- /dev/null +++ b/app/views/logs/delete_confirmation.html.erb @@ -0,0 +1,28 @@ +<% content_for :before_content do %> + <% content_for :title, "Are you sure you want to delete this log?" %> + <%= govuk_back_link href: @log.lettings? ? lettings_logs_path : sales_logs_path %> +<% end %> + +
+
+ Delete log <%= @log.id %> +

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

+ + <%= govuk_warning_text(text: "You will not be able to undo this action.") %> + +
+ <%= govuk_button_to( + "Delete this log", + @log.lettings? ? lettings_log_path(@log) : sales_log_path(@log), + method: :delete, + ) %> + <%= govuk_button_link_to( + "Cancel", + @log.lettings? ? lettings_log_path(@log) : sales_log_path(@log), + secondary: true, + ) %> +
+
+
diff --git a/app/views/logs/edit.html.erb b/app/views/logs/edit.html.erb index 81b87e861..170aefe4d 100644 --- a/app/views/logs/edit.html.erb +++ b/app/views/logs/edit.html.erb @@ -38,18 +38,6 @@ <%= render "tasklist" %> - <% if @log.completed? %> - <% if @log.bulk_uploaded? %> - <% if @log.lettings? %> - <%= govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(@log.bulk_upload) %> - <% else %> - <%= govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_sales_result_path(@log.bulk_upload) %> - <% end %> - <% elsif @log.lettings? %> - <%= govuk_button_link_to "Back to lettings logs", lettings_logs_path %> - <% elsif @log.sales? %> - <%= govuk_button_link_to "Back to sales logs", sales_logs_path %> - <% end %> - <% end %> + <%= edit_actions_for_log(@log) %> diff --git a/config/routes.rb b/config/routes.rb index 05b0f32fd..70cda158d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -150,6 +150,8 @@ Rails.application.routes.draw do end resources :lettings_logs, path: "/lettings-logs" do + get "delete-confirmation", to: "lettings_logs#delete_confirmation" + collection do post "bulk-upload", to: "bulk_upload#bulk_upload" get "bulk-upload", to: "bulk_upload#show" @@ -208,6 +210,8 @@ Rails.application.routes.draw do end resources :sales_logs, path: "/sales-logs" do + get "delete-confirmation", to: "sales_logs#delete_confirmation" + collection do get "csv-download", to: "sales_logs#download_csv" post "email-csv", to: "sales_logs#email_csv" diff --git a/db/migrate/20230515114101_add_discarded_at_to_logs.rb b/db/migrate/20230515114101_add_discarded_at_to_logs.rb new file mode 100644 index 000000000..10c1bb7e5 --- /dev/null +++ b/db/migrate/20230515114101_add_discarded_at_to_logs.rb @@ -0,0 +1,6 @@ +class AddDiscardedAtToLogs < ActiveRecord::Migration[7.0] + def change + add_column :sales_logs, :discarded_at, :datetime + add_column :lettings_logs, :discarded_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index a951c90a1..6c33dd973 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: 2023_05_05_105327) do +ActiveRecord::Schema[7.0].define(version: 2023_05_15_114101) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -288,6 +288,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_05_105327) do t.string "county" t.integer "carehome_charges_value_check" t.integer "status_cache", default: 0, null: false + t.datetime "discarded_at" t.index ["bulk_upload_id"], name: "index_lettings_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id" t.index ["location_id"], name: "index_lettings_logs_on_location_id" @@ -584,6 +585,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_05_105327) do t.integer "ethnicbuy2" t.integer "proplen_asked" t.string "old_id" + t.integer "buy2living" + t.integer "prevtenbuy2" t.integer "pregblank" t.string "uprn" t.integer "uprn_known" @@ -592,15 +595,14 @@ ActiveRecord::Schema[7.0].define(version: 2023_05_05_105327) do t.string "address_line2" t.string "town_or_city" t.string "county" - t.integer "buy2living" - t.integer "prevtenbuy2" t.integer "nationalbuy2" t.integer "discounted_sale_value_check" t.integer "student_not_child_value_check" t.integer "percentage_discount_value_check" t.integer "buyer_livein_value_check" - t.integer "combined_income_value_check" t.integer "status_cache", default: 0, null: false + t.integer "combined_income_value_check" + t.datetime "discarded_at" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["old_id"], name: "index_sales_logs_on_old_id", unique: true diff --git a/spec/factories/lettings_log.rb b/spec/factories/lettings_log.rb index 58c81e8f3..ec119fbdc 100644 --- a/spec/factories/lettings_log.rb +++ b/spec/factories/lettings_log.rb @@ -164,6 +164,10 @@ FactoryBot.define do trait :startdate_today do startdate { Time.zone.today } end + trait :deleted do + status { 4 } + discarded_at { Time.zone.now } + end created_at { Time.zone.today } updated_at { Time.zone.today } end diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index 0ea3787dc..29d95a1ec 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -129,5 +129,9 @@ FactoryBot.define do trait :with_uprn do uprn { rand(999_999_999_999).to_s } end + trait :deleted do + status { 4 } + discarded_at { Time.zone.now } + end end end diff --git a/spec/fixtures/files/lettings_logs_download.csv b/spec/fixtures/files/lettings_logs_download.csv index 075792363..5a68a29db 100644 --- a/spec/fixtures/files/lettings_logs_download.csv +++ b/spec/fixtures/files/lettings_logs_download.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,rent_value_check,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,old_form_id,lar,irproduct,old_id,joint,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,uprn,uprn_known,uprn_confirmed,address_line1,address_line2,town_or_city,county,carehome_charges_value_check,status_cache,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate -{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,2021,,Supported housing,,2 October 2021,London Affordable Rent,,,,,,,,,,,,,,,,,,,,No,,,,,No,Westminster,E09000033,,SE1 1TE,,No,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,9,1,,,,,,,,,,,,,,,,not_started,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,Bungalow,Fitted with equipment and adaptations,Westminster,{location_startdate} +id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,rent_value_check,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,old_form_id,lar,irproduct,old_id,joint,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,uprn,uprn_known,uprn_confirmed,address_line1,address_line2,town_or_city,county,carehome_charges_value_check,status_cache,discarded_at,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate +{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,2021,,Supported housing,,2 October 2021,London Affordable Rent,,,,,,,,,,,,,,,,,,,,No,,,,,No,Westminster,E09000033,,SE1 1TE,,No,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,9,1,,,,,,,,,,,,,,,,not_started,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,Bungalow,Fitted with equipment and adaptations,Westminster,{location_startdate} diff --git a/spec/fixtures/files/lettings_logs_download_codes_only.csv b/spec/fixtures/files/lettings_logs_download_codes_only.csv index 76dde6a7b..f7374afaa 100644 --- a/spec/fixtures/files/lettings_logs_download_codes_only.csv +++ b/spec/fixtures/files/lettings_logs_download_codes_only.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,rent_value_check,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,old_form_id,lar,irproduct,old_id,joint,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,uprn,uprn_known,uprn_confirmed,address_line1,address_line2,town_or_city,county,carehome_charges_value_check,status_cache,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate -{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,false,DLUHC,DLUHC,2021,,2,,2 October 2021,2,,,,,,,,,,,,,,,,,,,,false,,,,,false,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,9,1,,,,,,,,,,,,,,,,not_started,6,{scheme_code},{scheme_service_name},{scheme_sensitive},0,1,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,6,A,Westminster,{location_startdate} +id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,rent_value_check,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,old_form_id,lar,irproduct,old_id,joint,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,uprn,uprn_known,uprn_confirmed,address_line1,address_line2,town_or_city,county,carehome_charges_value_check,status_cache,discarded_at,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate +{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,false,DLUHC,DLUHC,2021,,2,,2 October 2021,2,,,,,,,,,,,,,,,,,,,,false,,,,,false,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,9,1,,,,,,,,,,,,,,,,not_started,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},0,1,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,6,A,Westminster,{location_startdate} diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 5e661fe16..6b946010e 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" require "shared/shared_examples_for_derived_fields" +require "shared/shared_log_examples" # rubocop:disable RSpec/MessageChain -# rubocop:disable RSpec/AnyInstance RSpec.describe LettingsLog do let(:different_managing_organisation) { create(:organisation) } let(:created_by_user) { create(:user) } @@ -23,6 +23,7 @@ RSpec.describe LettingsLog do end include_examples "shared examples for derived fields", :lettings_log + include_examples "shared log examples", :lettings_log it "inherits from log" do expect(described_class).to be < Log @@ -183,27 +184,7 @@ RSpec.describe LettingsLog do end describe "status" do - let!(:empty_lettings_log) { create(:lettings_log) } - let!(:in_progress_lettings_log) { create(:lettings_log, :in_progress) } - let!(:completed_lettings_log) { create(:lettings_log, :completed) } - - it "is set to not started for an empty lettings log" do - expect(empty_lettings_log.not_started?).to be(true) - expect(empty_lettings_log.in_progress?).to be(false) - expect(empty_lettings_log.completed?).to be(false) - end - - it "is set to in progress for a started lettings log" do - expect(in_progress_lettings_log.in_progress?).to be(true) - expect(in_progress_lettings_log.not_started?).to be(false) - expect(in_progress_lettings_log.completed?).to be(false) - end - - it "is set to completed for a completed lettings log" do - expect(completed_lettings_log.in_progress?).to be(false) - expect(completed_lettings_log.not_started?).to be(false) - expect(completed_lettings_log.completed?).to be(true) - end + let(:completed_lettings_log) { create(:lettings_log, :completed) } context "when only a subsection that is hidden in tasklist is not completed" do let(:household_characteristics_subsection) { completed_lettings_log.form.get_subsection("household_characteristics") } @@ -3009,62 +2990,6 @@ RSpec.describe LettingsLog do end end - describe "#process_uprn_change!" do - context "when UPRN set to a value" do - let(:lettings_log) do - create( - :lettings_log, - uprn: "123456789", - uprn_confirmed: 1, - county: "county", - ) - end - - it "updates sales log fields" do - lettings_log.uprn = "1111111" - - allow_any_instance_of(UprnClient).to receive(:call) - allow_any_instance_of(UprnClient).to receive(:result).and_return({ - "UPRN" => "UPRN", - "UDPRN" => "UDPRN", - "ADDRESS" => "full address", - "SUB_BUILDING_NAME" => "0", - "BUILDING_NAME" => "building name", - "THOROUGHFARE_NAME" => "thoroughfare", - "POST_TOWN" => "posttown", - "POSTCODE" => "postcode", - }) - - expect { lettings_log.process_uprn_change! }.to change(lettings_log, :address_line1).from(nil).to("0, Building Name, Thoroughfare") - .and change(lettings_log, :town_or_city).from(nil).to("Posttown") - .and change(lettings_log, :postcode_full).from(nil).to("POSTCODE") - .and change(lettings_log, :uprn_confirmed).from(1).to(nil) - .and change(lettings_log, :county).from("county").to(nil) - .and change(lettings_log, :uprn_known).from(nil).to(1) - end - end - - context "when UPRN nil" do - let(:lettings_log) { create(:lettings_log, uprn: nil) } - - it "does not update sales log" do - expect { lettings_log.process_uprn_change! }.not_to change(lettings_log, :attributes) - end - end - - context "when service errors" do - let(:lettings_log) { create(:lettings_log, uprn: "123456789", uprn_confirmed: 1) } - let(:error_message) { "error" } - - it "adds error to sales log" do - allow_any_instance_of(UprnClient).to receive(:call) - allow_any_instance_of(UprnClient).to receive(:error).and_return(error_message) - - expect { lettings_log.process_uprn_change! }.to change { lettings_log.errors[:uprn] }.from([]).to([error_message]) - end - end - end - describe "#beds_for_la_rent_range" do context "when beds nil" do let(:lettings_log) { build(:lettings_log, beds: nil) } @@ -3140,5 +3065,4 @@ RSpec.describe LettingsLog do end end end -# rubocop:enable RSpec/AnyInstance # rubocop:enable RSpec/MessageChain diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index bc0a96150..06d041b9d 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -1,13 +1,14 @@ require "rails_helper" require "shared/shared_examples_for_derived_fields" +require "shared/shared_log_examples" # rubocop:disable RSpec/MessageChain -# rubocop:disable RSpec/AnyInstance RSpec.describe SalesLog, type: :model do let(:owning_organisation) { create(:organisation) } let(:created_by_user) { create(:user) } include_examples "shared examples for derived fields", :sales_log + include_examples "shared log examples", :sales_log it "inherits from log" do expect(described_class).to be < Log @@ -112,27 +113,7 @@ RSpec.describe SalesLog, type: :model do end describe "status" do - let!(:empty_sales_log) { create(:sales_log) } - let!(:in_progress_sales_log) { create(:sales_log, :in_progress) } - let!(:completed_sales_log) { create(:sales_log, :completed) } - - it "is set to not started for an empty sales log" do - expect(empty_sales_log.not_started?).to be(true) - expect(empty_sales_log.in_progress?).to be(false) - expect(empty_sales_log.completed?).to be(false) - end - - it "is set to in progress for a started sales log" do - expect(in_progress_sales_log.in_progress?).to be(true) - expect(in_progress_sales_log.not_started?).to be(false) - expect(in_progress_sales_log.completed?).to be(false) - end - - it "is set to completed for a completed sales log" do - expect(completed_sales_log.in_progress?).to be(false) - expect(completed_sales_log.not_started?).to be(false) - expect(completed_sales_log.completed?).to be(true) - end + let(:completed_sales_log) { create(:sales_log, :completed) } context "when proplen is not given" do before do @@ -148,6 +129,7 @@ RSpec.describe SalesLog, type: :model do expect(completed_sales_log.in_progress?).to be(false) expect(completed_sales_log.not_started?).to be(false) expect(completed_sales_log.completed?).to be(true) + expect(completed_sales_log.deleted?).to be(false) end it "is set to in_progress for a log with a saledate after 23/24" do @@ -155,6 +137,7 @@ RSpec.describe SalesLog, type: :model do expect(completed_sales_log.in_progress?).to be(true) expect(completed_sales_log.not_started?).to be(false) expect(completed_sales_log.completed?).to be(false) + expect(completed_sales_log.deleted?).to be(false) end end end @@ -591,63 +574,6 @@ RSpec.describe SalesLog, type: :model do end end - describe "#process_uprn_change!" do - context "when UPRN set to a value" do - let(:sales_log) do - create( - :sales_log, - uprn: "123456789", - uprn_confirmed: 1, - county: "county", - ) - end - - it "updates sales log fields" do - sales_log.uprn = "1111111" - sales_log.uprn_confirmed = 1 - - allow_any_instance_of(UprnClient).to receive(:call) - allow_any_instance_of(UprnClient).to receive(:result).and_return({ - "UPRN" => "UPRN", - "UDPRN" => "UDPRN", - "ADDRESS" => "full address", - "SUB_BUILDING_NAME" => "0", - "BUILDING_NAME" => "building name", - "THOROUGHFARE_NAME" => "thoroughfare", - "POST_TOWN" => "posttown", - "POSTCODE" => "postcode", - }) - - expect { sales_log.process_uprn_change! }.to change(sales_log, :address_line1).from(nil).to("0, Building Name, Thoroughfare") - .and change(sales_log, :town_or_city).from(nil).to("Posttown") - .and change(sales_log, :postcode_full).from(nil).to("POSTCODE") - .and change(sales_log, :uprn_confirmed).from(1).to(nil) - .and change(sales_log, :county).from("county").to(nil) - .and change(sales_log, :uprn_known).from(nil).to(1) - end - end - - context "when UPRN nil" do - let(:sales_log) { create(:sales_log, uprn: nil) } - - it "does not update sales log" do - expect { sales_log.process_uprn_change! }.not_to change(sales_log, :attributes) - end - end - - context "when the API returns an error" do - let(:sales_log) { build(:sales_log, :outright_sale_setup_complete, uprn_known: 1, uprn: "123456789", uprn_confirmed: 1) } - let(:error_message) { "error" } - - it "adds error to sales log" do - allow_any_instance_of(UprnClient).to receive(:call) - allow_any_instance_of(UprnClient).to receive(:error).and_return(error_message) - - expect { sales_log.process_uprn_change! }.to change { sales_log.errors[:uprn] }.from([]).to([error_message]) - end - end - end - describe "#beds_for_la_sale_range" do context "when beds nil" do let(:sales_log) { build(:sales_log, beds: nil) } @@ -723,5 +649,4 @@ RSpec.describe SalesLog, type: :model do end end end -# rubocop:enable RSpec/AnyInstance # rubocop:enable RSpec/MessageChain diff --git a/spec/policies/lettings_log_policy_spec.rb b/spec/policies/lettings_log_policy_spec.rb new file mode 100644 index 000000000..a89280212 --- /dev/null +++ b/spec/policies/lettings_log_policy_spec.rb @@ -0,0 +1,114 @@ +require "rails_helper" + +RSpec.describe LettingsLogPolicy do + subject(:policy) { described_class } + + permissions :destroy? do + let(:log) { create(:lettings_log, :in_progress) } + + context "when log nil" do + before do + allow(log).to receive(:collection_period_open?).and_return(false) + end + + it "does not allow deletion of log" do + expect(policy).not_to permit(build(:user, :support), nil) + end + end + + context "when user nil" do + before do + allow(log).to receive(:collection_period_open?).and_return(false) + end + + it "does not allow deletion of log" do + expect(policy).not_to permit(nil, build(:lettings_log, :in_progress)) + end + end + + context "when collection period closed" do + before do + allow(log).to receive(:collection_period_open?).and_return(false) + end + + it "does not allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(build(:user, :support), log) + end + end + + context "when collection period open" do + before do + allow(log).to receive(:collection_period_open?).and_return(true) + end + + context "when not started" do + before do + allow(log).to receive(:in_progress?).and_return(false) + allow(log).to receive(:completed?).and_return(false) + end + + it "does not allow deletion of log" do + expect(log).to receive(:in_progress?) + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(build(:user, :support), log) + end + end + + [ + %i[lettings_log in_progress], + %i[lettings_log completed], + ].each do |type, status| + let(:log) { create(type, status) } + context "when #{type} status: #{status}" do + context "when user is data coordinator" do + let(:user) { create(:user, :data_coordinator) } + let(:user_of_managing_org) { create(:user, :data_coordinator, organisation: log.managing_organisation) } + + it "does not allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(user, log) + end + + it "allows deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).to permit(user_of_managing_org, log) + end + end + + context "when user is support" do + let(:user) { create(:user, :support) } + + it "does allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).to permit(user, log) + end + end + + context "when user is data provider" do + let(:user) { create(:user) } + + it "does not allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(user, log) + end + + context "when the log is assigned to the user" do + let(:log) { create(:lettings_log, :in_progress, created_by: user) } + + it "does allow deletion of log" do + expect(policy).to permit(user, log) + end + end + end + end + end + end + end +end diff --git a/spec/policies/sales_log_policy_spec.rb b/spec/policies/sales_log_policy_spec.rb new file mode 100644 index 000000000..38a8cc7b8 --- /dev/null +++ b/spec/policies/sales_log_policy_spec.rb @@ -0,0 +1,114 @@ +require "rails_helper" + +RSpec.describe SalesLogPolicy do + subject(:policy) { described_class } + + permissions :destroy? do + let(:log) { create(:sales_log, :in_progress) } + + context "when log nil" do + before do + allow(log).to receive(:collection_period_open?).and_return(false) + end + + it "does not allow deletion of log" do + expect(policy).not_to permit(build(:user, :support), nil) + end + end + + context "when user nil" do + before do + allow(log).to receive(:collection_period_open?).and_return(false) + end + + it "does not allow deletion of log" do + expect(policy).not_to permit(nil, build(:sales_log, :in_progress)) + end + end + + context "when collection period closed" do + before do + allow(log).to receive(:collection_period_open?).and_return(false) + end + + it "does not allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(build(:user, :support), log) + end + end + + context "when collection period open" do + before do + allow(log).to receive(:collection_period_open?).and_return(true) + end + + context "when not started" do + before do + allow(log).to receive(:in_progress?).and_return(false) + allow(log).to receive(:completed?).and_return(false) + end + + it "does not allow deletion of log" do + expect(log).to receive(:in_progress?) + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(build(:user, :support), log) + end + end + + [ + %i[sales_log in_progress], + %i[sales_log completed], + ].each do |type, status| + let(:log) { create(type, status) } + context "when #{type} status: #{status}" do + context "when user is data coordinator" do + let(:user) { create(:user, :data_coordinator) } + let(:user_of_owning_org) { create(:user, :data_coordinator, organisation: log.owning_organisation) } + + it "does not allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(user, log) + end + + it "allows deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).to permit(user_of_owning_org, log) + end + end + + context "when user is support" do + let(:user) { create(:user, :support) } + + it "does allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).to permit(user, log) + end + end + + context "when user is data provider" do + let(:user) { create(:user) } + + it "does not allow deletion of log" do + expect(log).to receive(:collection_period_open?) + + expect(policy).not_to permit(user, log) + end + + context "when the log is assigned to the user" do + let(:log) { create(:sales_log, :in_progress, created_by: user) } + + it "does allow deletion of log" do + expect(policy).to permit(user, log) + end + end + end + end + end + end + end +end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index e0c785b71..33108e465 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -1309,55 +1309,93 @@ RSpec.describe LettingsLogsController, type: :request do end describe "DELETE" do + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { create(:user, :support) } let!(:lettings_log) do - FactoryBot.create(:lettings_log, :in_progress) + create(:lettings_log, :completed) end let(:id) { lettings_log.id } + let(:delete_request) { delete "/lettings-logs/#{id}", headers: } - context "when deleting a lettings log" do - before do - delete "/lettings-logs/#{id}", headers: - end + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end - it "returns http success" do - expect(response).to have_http_status(:success) + context "when delete permitted" do + it "redirects to lettings logs and shows message" do + delete_request + expect(response).to redirect_to(lettings_logs_path) + follow_redirect! + expect(page).to have_content("Log #{id} has been deleted") end - it "deletes the lettings log" do - expect { LettingsLog.find(id) }.to raise_error(ActiveRecord::RecordNotFound) + it "marks the log as deleted" do + expect { delete_request }.to change { lettings_log.reload.status }.from("completed").to("deleted") end + end - context "with an invalid lettings log id" do - let(:id) { (LettingsLog.order(:id).last&.id || 0) + 1 } + context "when log does not exist" do + let(:id) { -1 } - it "returns 404" do - expect(response).to have_http_status(:not_found) - end + it "returns 404" do + delete_request + expect(response).to have_http_status(:not_found) end + end - context "with a request containing invalid credentials" do - let(:basic_credentials) do - ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops") - end + context "when user not authorised" do + let(:user) { create(:user) } - it "returns 401" do - expect(response).to have_http_status(:unauthorized) - end + it "returns 404" do + delete_request + expect(response).to have_http_status(:unauthorized) end end + end - context "when a lettings log deletion fails" do - let(:mock_scope) { instance_double("LettingsLog::ActiveRecord_Relation", find_by: lettings_log) } + describe "GET delete-confirmation" do + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { create(:user, :support) } + let!(:lettings_log) do + create(:lettings_log, :completed) + end + let(:id) { lettings_log.id } + let(:request) { get "/lettings-logs/#{id}/delete-confirmation", headers: } - before do - allow(LettingsLog).to receive(:visible).and_return(mock_scope) - allow(lettings_log).to receive(:delete).and_return(false) + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end - delete "/lettings-logs/#{id}", headers: + context "when delete permitted" do + it "renders page" do + request + expect(response).to have_http_status(:ok) + + expect(page).to have_content("Are you sure you want to delete this log?") + expect(page).to have_button(text: "Delete this log") + expect(page).to have_link(text: "Cancel", href: lettings_log_path(id)) end + end - it "returns an unprocessable entity 422" do - expect(response).to have_http_status(:unprocessable_entity) + context "when log does not exist" do + let(:id) { -1 } + + it "returns 404" do + request + expect(response).to have_http_status(:not_found) + end + end + + context "when user not authorised" do + let(:user) { create(:user) } + + it "returns 404" do + request + expect(response).to have_http_status(:unauthorized) end end end diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 6927d294a..a6f614a34 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -652,4 +652,96 @@ RSpec.describe SalesLogsController, type: :request do end end end + + describe "DELETE" do + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { create(:user, :support) } + let!(:sales_log) do + create(:sales_log, :completed) + end + let(:id) { sales_log.id } + let(:delete_request) { delete "/sales-logs/#{id}", headers: } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + context "when delete permitted" do + it "redirects to sales logs and shows message" do + delete_request + expect(response).to redirect_to(sales_logs_path) + follow_redirect! + expect(page).to have_content("Log #{id} has been deleted") + end + + it "marks the log as deleted" do + expect { delete_request }.to change { sales_log.reload.status }.from("completed").to("deleted") + end + end + + context "when log does not exist" do + let(:id) { -1 } + + it "returns 404" do + delete_request + expect(response).to have_http_status(:not_found) + end + end + + context "when user not authorised" do + let(:user) { create(:user) } + + it "returns 404" do + delete_request + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe "GET delete-confirmation" do + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { create(:user, :support) } + let!(:sales_log) do + create(:sales_log, :completed) + end + let(:id) { sales_log.id } + let(:request) { get "/sales-logs/#{id}/delete-confirmation", headers: } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + context "when delete permitted" do + it "renders page" do + request + expect(response).to have_http_status(:ok) + + expect(page).to have_content("Are you sure you want to delete this log?") + expect(page).to have_button(text: "Delete this log") + expect(page).to have_link(text: "Cancel", href: sales_log_path(id)) + end + end + + context "when log does not exist" do + let(:id) { -1 } + + it "returns 404" do + request + expect(response).to have_http_status(:not_found) + end + end + + context "when user not authorised" do + let(:user) { create(:user) } + + it "returns 404" do + request + expect(response).to have_http_status(:unauthorized) + end + end + end end diff --git a/spec/services/csv/lettings_log_csv_service_spec.rb b/spec/services/csv/lettings_log_csv_service_spec.rb index 1ee3b9df1..6cf5dce9f 100644 --- a/spec/services/csv/lettings_log_csv_service_spec.rb +++ b/spec/services/csv/lettings_log_csv_service_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe Csv::LettingsLogCsvService do context "when the user is support" do - let(:user) { FactoryBot.create(:user, :support) } + let(:user) { create(:user, :support) } let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json") } before do @@ -11,226 +11,229 @@ RSpec.describe Csv::LettingsLogCsvService do end it "sets csv attributes in correct order" do - expected_csv_attributes = %w[id - status - created_at - updated_at - created_by_name - is_dpo - owning_organisation_name - managing_organisation_name - collection_start_year - rent_value_check - needstype - renewal - startdate - rent_type_detail - irproduct_other - tenancycode - propcode - postcode_known - postcode_full - is_la_inferred - la_label - la - first_time_property_let_as_social_housing - unitletas - rsnvac - offered - unittype_gn - builtype - wchair - beds - voiddate - void_date_value_check - majorrepairs - mrcdate - major_repairs_date_value_check - startertenancy - tenancy - tenancyother - tenancylength - sheltered - declaration - hhmemb - pregnancy_value_check - age1_known - age1 - sex1 - ethnic_group - ethnic - national - ecstat1 - retirement_value_check - details_known_2 - relat2 - age2_known - age2 - sex2 - ecstat2 - details_known_3 - relat3 - age3_known - age3 - sex3 - ecstat3 - details_known_4 - relat4 - age4_known - age4 - sex4 - ecstat4 - details_known_5 - relat5 - age5_known - age5 - sex5 - ecstat5 - details_known_6 - relat6 - age6_known - age6 - sex6 - ecstat6 - details_known_7 - relat7 - age7_known - age7 - sex7 - ecstat7 - details_known_8 - relat8 - age8_known - age8 - sex8 - ecstat8 - armedforces - leftreg - reservist - preg_occ - housingneeds - housingneeds_type - housingneeds_other - illness - illness_type_4 - illness_type_5 - illness_type_2 - illness_type_6 - illness_type_7 - illness_type_3 - illness_type_9 - illness_type_8 - illness_type_1 - illness_type_10 - layear - waityear - reason - reasonother - prevten - underoccupation_benefitcap - homeless - ppcodenk - ppostcode_full - previous_la_known - is_previous_la_inferred - prevloc_label - prevloc - reasonpref - rp_homeless - rp_insan_unsat - rp_medwel - rp_hardship - rp_dontknow - cbl - cap - chr - letting_allocation_unknown - referral - net_income_known - earnings - incfreq - net_income_value_check - hb - benefits - household_charge - period - is_carehome - chcharge - carehome_charges_value_check - brent - scharge - pscharge - supcharg - tcharge - hbrentshortfall - tshortfall_known - tshortfall - housingneeds_a - housingneeds_b - housingneeds_c - housingneeds_f - housingneeds_g - housingneeds_h - property_owner_organisation - property_manager_organisation - purchaser_code - property_relet - incref - renttype - lettype - totchild - totelder - totadult - nocharge - has_benefits - wrent - wscharge - wpschrge - wsupchrg - wtcharge - wtshortfall - refused - wchchrg - newprop - old_form_id - lar - irproduct - old_id - joint - hhtype - new_old - vacdays - unresolved - updated_by_id - uprn - uprn_known - uprn_confirmed - address_line1 - address_line2 - town_or_city - county - status_cache - unittype_sh - scheme_code - scheme_service_name - scheme_sensitive - scheme_type - scheme_registered_under_care_act - scheme_owning_organisation_name - scheme_primary_client_group - scheme_has_other_client_group - scheme_secondary_client_group - scheme_support_type - scheme_intended_stay - scheme_created_at - location_code - location_postcode - location_name - location_units - location_type_of_unit - location_mobility_type - location_admin_district - location_startdate] + expected_csv_attributes = %w[ + id + status + created_at + updated_at + created_by_name + is_dpo + owning_organisation_name + managing_organisation_name + collection_start_year + rent_value_check + needstype + renewal + startdate + rent_type_detail + irproduct_other + tenancycode + propcode + postcode_known + postcode_full + is_la_inferred + la_label + la + first_time_property_let_as_social_housing + unitletas + rsnvac + offered + unittype_gn + builtype + wchair + beds + voiddate + void_date_value_check + majorrepairs + mrcdate + major_repairs_date_value_check + startertenancy + tenancy + tenancyother + tenancylength + sheltered + declaration + hhmemb + pregnancy_value_check + age1_known + age1 + sex1 + ethnic_group + ethnic + national + ecstat1 + retirement_value_check + details_known_2 + relat2 + age2_known + age2 + sex2 + ecstat2 + details_known_3 + relat3 + age3_known + age3 + sex3 + ecstat3 + details_known_4 + relat4 + age4_known + age4 + sex4 + ecstat4 + details_known_5 + relat5 + age5_known + age5 + sex5 + ecstat5 + details_known_6 + relat6 + age6_known + age6 + sex6 + ecstat6 + details_known_7 + relat7 + age7_known + age7 + sex7 + ecstat7 + details_known_8 + relat8 + age8_known + age8 + sex8 + ecstat8 + armedforces + leftreg + reservist + preg_occ + housingneeds + housingneeds_type + housingneeds_other + illness + illness_type_4 + illness_type_5 + illness_type_2 + illness_type_6 + illness_type_7 + illness_type_3 + illness_type_9 + illness_type_8 + illness_type_1 + illness_type_10 + layear + waityear + reason + reasonother + prevten + underoccupation_benefitcap + homeless + ppcodenk + ppostcode_full + previous_la_known + is_previous_la_inferred + prevloc_label + prevloc + reasonpref + rp_homeless + rp_insan_unsat + rp_medwel + rp_hardship + rp_dontknow + cbl + cap + chr + letting_allocation_unknown + referral + net_income_known + earnings + incfreq + net_income_value_check + hb + benefits + household_charge + period + is_carehome + chcharge + carehome_charges_value_check + brent + scharge + pscharge + supcharg + tcharge + hbrentshortfall + tshortfall_known + tshortfall + housingneeds_a + housingneeds_b + housingneeds_c + housingneeds_f + housingneeds_g + housingneeds_h + property_owner_organisation + property_manager_organisation + purchaser_code + property_relet + incref + renttype + lettype + totchild + totelder + totadult + nocharge + has_benefits + wrent + wscharge + wpschrge + wsupchrg + wtcharge + wtshortfall + refused + wchchrg + newprop + old_form_id + lar + irproduct + old_id + joint + hhtype + new_old + vacdays + unresolved + updated_by_id + uprn + uprn_known + uprn_confirmed + address_line1 + address_line2 + town_or_city + county + status_cache + discarded_at + unittype_sh + scheme_code + scheme_service_name + scheme_sensitive + scheme_type + scheme_registered_under_care_act + scheme_owning_organisation_name + scheme_primary_client_group + scheme_has_other_client_group + scheme_secondary_client_group + scheme_support_type + scheme_intended_stay + scheme_created_at + location_code + location_postcode + location_name + location_units + location_type_of_unit + location_mobility_type + location_admin_district + location_startdate + ] csv = CSV.parse(described_class.new(user, export_type: "labels").to_csv) @@ -248,174 +251,176 @@ RSpec.describe Csv::LettingsLogCsvService do end it "sets csv attributes in correct order and without omitted values" do - expected_csv_attributes = %w[id - status - created_at - updated_at - created_by_name - is_dpo - owning_organisation_name - managing_organisation_name - collection_start_year - renewal - startdate - irproduct_other - tenancycode - propcode - postcode_full - la_label - unitletas - rsnvac - offered - unittype_gn - builtype - wchair - beds - voiddate - void_date_value_check - majorrepairs - mrcdate - major_repairs_date_value_check - startertenancy - tenancy - tenancyother - tenancylength - sheltered - declaration - age1 - sex1 - ethnic_group - ethnic - national - ecstat1 - relat2 - age2 - sex2 - ecstat2 - relat3 - age3 - sex3 - ecstat3 - relat4 - age4 - sex4 - ecstat4 - relat5 - age5 - sex5 - ecstat5 - relat6 - age6 - sex6 - ecstat6 - relat7 - age7 - sex7 - ecstat7 - relat8 - age8 - sex8 - ecstat8 - armedforces - leftreg - reservist - preg_occ - housingneeds - housingneeds_type - housingneeds_other - illness - illness_type_4 - illness_type_5 - illness_type_2 - illness_type_6 - illness_type_7 - illness_type_3 - illness_type_9 - illness_type_8 - illness_type_1 - illness_type_10 - layear - waityear - reason - reasonother - prevten - underoccupation_benefitcap - homeless - ppcodenk - ppostcode_full - prevloc_label - reasonpref - rp_homeless - rp_insan_unsat - rp_medwel - rp_hardship - rp_dontknow - cbl - cap - chr - referral - earnings - incfreq - hb - benefits - household_charge - period - chcharge - carehome_charges_value_check - brent - scharge - pscharge - supcharg - tcharge - hbrentshortfall - tshortfall - housingneeds_a - housingneeds_b - housingneeds_c - housingneeds_f - housingneeds_g - housingneeds_h - property_owner_organisation - property_manager_organisation - purchaser_code - property_relet - incref - lettype - nocharge - has_benefits - refused - wchchrg - newprop - lar - irproduct - joint - uprn - uprn_known - address_line1 - address_line2 - town_or_city - county - unittype_sh - scheme_code - scheme_service_name - scheme_sensitive - scheme_type - scheme_registered_under_care_act - scheme_owning_organisation_name - scheme_primary_client_group - scheme_has_other_client_group - scheme_secondary_client_group - scheme_support_type - scheme_intended_stay - scheme_created_at - location_code - location_postcode - location_name - location_units - location_type_of_unit - location_mobility_type - location_admin_district - location_startdate] + expected_csv_attributes = %w[ + id + status + created_at + updated_at + created_by_name + is_dpo + owning_organisation_name + managing_organisation_name + collection_start_year + renewal + startdate + irproduct_other + tenancycode + propcode + postcode_full + la_label + unitletas + rsnvac + offered + unittype_gn + builtype + wchair + beds + voiddate + void_date_value_check + majorrepairs + mrcdate + major_repairs_date_value_check + startertenancy + tenancy + tenancyother + tenancylength + sheltered + declaration + age1 + sex1 + ethnic_group + ethnic + national + ecstat1 + relat2 + age2 + sex2 + ecstat2 + relat3 + age3 + sex3 + ecstat3 + relat4 + age4 + sex4 + ecstat4 + relat5 + age5 + sex5 + ecstat5 + relat6 + age6 + sex6 + ecstat6 + relat7 + age7 + sex7 + ecstat7 + relat8 + age8 + sex8 + ecstat8 + armedforces + leftreg + reservist + preg_occ + housingneeds + housingneeds_type + housingneeds_other + illness + illness_type_4 + illness_type_5 + illness_type_2 + illness_type_6 + illness_type_7 + illness_type_3 + illness_type_9 + illness_type_8 + illness_type_1 + illness_type_10 + layear + waityear + reason + reasonother + prevten + underoccupation_benefitcap + homeless + ppcodenk + ppostcode_full + prevloc_label + reasonpref + rp_homeless + rp_insan_unsat + rp_medwel + rp_hardship + rp_dontknow + cbl + cap + chr + referral + earnings + incfreq + hb + benefits + household_charge + period + chcharge + carehome_charges_value_check + brent + scharge + pscharge + supcharg + tcharge + hbrentshortfall + tshortfall + housingneeds_a + housingneeds_b + housingneeds_c + housingneeds_f + housingneeds_g + housingneeds_h + property_owner_organisation + property_manager_organisation + purchaser_code + property_relet + incref + lettype + nocharge + has_benefits + refused + wchchrg + newprop + lar + irproduct + joint + uprn + uprn_known + address_line1 + address_line2 + town_or_city + county + unittype_sh + scheme_code + scheme_service_name + scheme_sensitive + scheme_type + scheme_registered_under_care_act + scheme_owning_organisation_name + scheme_primary_client_group + scheme_has_other_client_group + scheme_secondary_client_group + scheme_support_type + scheme_intended_stay + scheme_created_at + location_code + location_postcode + location_name + location_units + location_type_of_unit + location_mobility_type + location_admin_district + location_startdate + ] csv = CSV.parse(described_class.new(user, export_type: "labels").to_csv) diff --git a/spec/shared/shared_log_examples.rb b/spec/shared/shared_log_examples.rb new file mode 100644 index 000000000..ef6e8c939 --- /dev/null +++ b/spec/shared/shared_log_examples.rb @@ -0,0 +1,107 @@ +require "rails_helper" + +# rubocop:disable RSpec/AnyInstance +RSpec.shared_examples "shared log examples" do |log_type| + describe "status" do + let(:empty_log) { create(log_type) } + let(:in_progress_log) { create(log_type, :in_progress) } + let(:completed_log) { create(log_type, :completed) } + + it "is set to not started for an empty #{log_type} log" do + expect(empty_log.not_started?).to be(true) + expect(empty_log.in_progress?).to be(false) + expect(empty_log.completed?).to be(false) + expect(empty_log.deleted?).to be(false) + end + + it "is set to in progress for a started #{log_type} log" do + expect(in_progress_log.in_progress?).to be(true) + expect(in_progress_log.not_started?).to be(false) + expect(in_progress_log.completed?).to be(false) + expect(in_progress_log.deleted?).to be(false) + end + + it "is set to completed for a completed #{log_type} log" do + expect(completed_log.in_progress?).to be(false) + expect(completed_log.not_started?).to be(false) + expect(completed_log.completed?).to be(true) + expect(completed_log.deleted?).to be(false) + end + end + + describe "discard!" do + around do |example| + Timecop.freeze(Time.zone.local(2022, 1, 1)) do + example.run + end + Timecop.return + end + + let(:log) { create(log_type) } + + it "updates discarded at with current time" do + expect { log.discard! }.to change { log.reload.discarded_at }.from(nil).to(Time.zone.now) + end + + it "updates status to deleted" do + expect { log.discard! }.to change { log.reload.status }.to("deleted") + end + end + + describe "#process_uprn_change!" do + context "when UPRN set to a value" do + let(:log) do + create( + log_type, + uprn: "123456789", + uprn_confirmed: 1, + county: "county", + ) + end + + it "updates log fields" do + log.uprn = "1111111" + + allow_any_instance_of(UprnClient).to receive(:call) + allow_any_instance_of(UprnClient).to receive(:result).and_return({ + "UPRN" => "UPRN", + "UDPRN" => "UDPRN", + "ADDRESS" => "full address", + "SUB_BUILDING_NAME" => "0", + "BUILDING_NAME" => "building name", + "THOROUGHFARE_NAME" => "thoroughfare", + "POST_TOWN" => "posttown", + "POSTCODE" => "postcode", + }) + + expect { log.process_uprn_change! }.to change(log, :address_line1).from(nil).to("0, Building Name, Thoroughfare") + .and change(log, :town_or_city).from(nil).to("Posttown") + .and change(log, :postcode_full).from(nil).to("POSTCODE") + .and change(log, :uprn_confirmed).from(1).to(nil) + .and change(log, :county).from("county").to(nil) + .and change(log, :uprn_known).from(nil).to(1) + end + end + + context "when UPRN nil" do + let(:log) { create(log_type, uprn: nil) } + + it "does not update log" do + expect { log.process_uprn_change! }.not_to change(log, :attributes) + end + end + + context "when service errors" do + let(:log) { create(log_type, uprn: "123456789", uprn_confirmed: 1) } + let(:error_message) { "error" } + + it "adds error to log" do + allow_any_instance_of(UprnClient).to receive(:call) + allow_any_instance_of(UprnClient).to receive(:error).and_return(error_message) + + expect { log.process_uprn_change! }.to change { log.errors[:uprn] }.from([]).to([error_message]) + end + end + end +end +# rubocop:enable RSpec/AnyInstance