From 234dd006b7540feeb6993704a9d5da837e1f74e0 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 25 Aug 2022 13:59:11 +0100 Subject: [PATCH 01/53] feat: display label columns for la and prevloc, and show codes for previous la and prevloc fields --- app/models/case_log.rb | 8 ++++++++ app/services/csv/case_log_csv_service.rb | 19 +++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 3f61ed4e4..f5d9c94d1 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -442,6 +442,14 @@ class CaseLog < ApplicationRecord created_by&.is_dpo end + def la_label + la + end + + def prevloc_label + prevloc + end + def self.to_csv(user = nil) Csv::CaseLogCsvService.new(user).to_csv end diff --git a/app/services/csv/case_log_csv_service.rb b/app/services/csv/case_log_csv_service.rb index ff6474b80..c763e269f 100644 --- a/app/services/csv/case_log_csv_service.rb +++ b/app/services/csv/case_log_csv_service.rb @@ -1,6 +1,6 @@ module Csv class CaseLogCsvService - CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check sale_or_letting 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 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].freeze + CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check sale_or_letting 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 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].freeze def initialize(user) @user = user @@ -13,7 +13,13 @@ module Csv CaseLog.all.find_each do |record| csv << @attributes.map do |att| - record.form.get_question(att, record)&.label_from_value(record.send(att)) || label_from_value(record.send(att)) + if %w[la prevloc].include? att + label_from_value(record.send(att)) + elsif %w[la_label prevloc_label].include? att + record.form.get_question(att.remove("_label"), record)&.label_from_value(record.send(att)) || label_from_value(record.send(att)) + else + record.form.get_question(att, record)&.label_from_value(record.send(att)) || label_from_value(record.send(att)) + end end end end @@ -34,9 +40,10 @@ module Csv scheme_attributes = %w[scheme_id location_id] intersecting_attributes = ordered_form_questions & CaseLog.attribute_names - scheme_attributes remaining_attributes = CaseLog.attribute_names - intersecting_attributes - scheme_attributes + la_fields = %w[la_label prevloc_label] - @attributes = (metadata_fields + intersecting_attributes + remaining_attributes - metadata_id_fields + %w[unittype_sh] + scheme_attributes).uniq - move_is_inferred_fields + @attributes = (metadata_fields + intersecting_attributes + remaining_attributes - metadata_id_fields + %w[unittype_sh] + scheme_attributes + la_fields).uniq + move_la_fields @attributes -= CSV_FIELDS_TO_OMIT if @user.present? && !@user.support? end @@ -59,8 +66,8 @@ module Csv attributes end - def move_is_inferred_fields - { la: "is_la_inferred", prevloc: "is_previous_la_inferred" }.each do |inferred_field, field| + def move_la_fields + { la: "is_la_inferred", la: "la_label", prevloc: "is_previous_la_inferred", prevloc: "prevloc_label" }.each do |inferred_field, field| @attributes.delete(field) @attributes.insert(@attributes.find_index(inferred_field.to_s), field) end From 8959533a34afb21454441d1cbdac6f9ac0fd618c Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 25 Aug 2022 14:06:40 +0100 Subject: [PATCH 02/53] refactor: combine duplicate has keys --- app/services/csv/case_log_csv_service.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/csv/case_log_csv_service.rb b/app/services/csv/case_log_csv_service.rb index c763e269f..aa5ebb106 100644 --- a/app/services/csv/case_log_csv_service.rb +++ b/app/services/csv/case_log_csv_service.rb @@ -67,9 +67,11 @@ module Csv end def move_la_fields - { la: "is_la_inferred", la: "la_label", prevloc: "is_previous_la_inferred", prevloc: "prevloc_label" }.each do |inferred_field, field| - @attributes.delete(field) - @attributes.insert(@attributes.find_index(inferred_field.to_s), field) + { la: %w[is_la_inferred la_label], prevloc: %w[is_previous_la_inferred prevloc_label] }.each do |inferred_field, fields| + fields.each do |field| + @attributes.delete(field) + @attributes.insert(@attributes.find_index(inferred_field.to_s), field) + end end end end From 5894e822fb3afd50605fda5b9a283278b3af2a92 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 25 Aug 2022 14:36:28 +0100 Subject: [PATCH 03/53] test: add new columns --- spec/fixtures/files/case_logs_download.csv | 4 ++-- spec/fixtures/files/case_logs_download_non_support.csv | 2 +- spec/services/csv/case_log_csv_service_spec.rb | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/spec/fixtures/files/case_logs_download.csv b/spec/fixtures/files/case_logs_download.csv index 62f6e4b64..5bf6618f1 100644 --- a/spec/fixtures/files/case_logs_download.csv +++ b/spec/fixtures/files/case_logs_download.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,needstype,renewal,startdate,rent_type,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,illness_type_1,illness_type_2,is_la_inferred,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,sale_or_letting,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,sale_completion_date,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,ethnic_other,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,rent_value_check,old_form_id,lar,irproduct,old_id,joint,illness_type_0,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_id,location_id -{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,Supported housing,,,,,,,,,,,,,,,,,,,,,,,No,,,,No,Westminster,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,,9,,,,,,,6,{scheme_id},SE1 1TE +id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,needstype,renewal,startdate,rent_type,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,sale_or_letting,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,sale_completion_date,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,ethnic_other,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,rent_value_check,old_form_id,lar,irproduct,old_id,joint,illness_type_0,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_id,location_id +{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,Supported housing,,,,,,,,,,,,,,,,,,,,,,,No,,,,,No,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,,9,,,,,,,6,{scheme_id},SE1 1TE diff --git a/spec/fixtures/files/case_logs_download_non_support.csv b/spec/fixtures/files/case_logs_download_non_support.csv index a2b12c89b..837742a52 100644 --- a/spec/fixtures/files/case_logs_download_non_support.csv +++ b/spec/fixtures/files/case_logs_download_non_support.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,renewal,startdate,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,relat2,age2,sex2,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,prevloc,illness_type_1,illness_type_2,la,postcode_full,wchair,preg_occ,cbl,earnings,incfreq,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,sale_completion_date,unitletas,builtype,voiddate,lettype,nocharge,household_charge,referral,tshortfall,chcharge,ppcodenk,ethnic_group,ethnic_other,has_benefits,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,lar,irproduct,joint,illness_type_0,sheltered,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_id,location_id +id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,renewal,startdate,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,relat2,age2,sex2,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,prevloc_label,illness_type_1,illness_type_2,la_label,postcode_full,wchair,preg_occ,cbl,earnings,incfreq,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,sale_completion_date,unitletas,builtype,voiddate,lettype,nocharge,household_charge,referral,tshortfall,chcharge,ppcodenk,ethnic_group,ethnic_other,has_benefits,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,lar,irproduct,joint,illness_type_0,sheltered,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_id,location_id {id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,,,,,,,,,,,,,,,,,,,,,,,Westminster,SE1 1TE,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,,0,0,,,,,,,,,,,,,,,,,,,6,{scheme_id},SE1 1TE diff --git a/spec/services/csv/case_log_csv_service_spec.rb b/spec/services/csv/case_log_csv_service_spec.rb index 1815b5ea2..5d7c71a1e 100644 --- a/spec/services/csv/case_log_csv_service_spec.rb +++ b/spec/services/csv/case_log_csv_service_spec.rb @@ -29,6 +29,7 @@ RSpec.describe Csv::CaseLogCsvService do postcode_known postcode_full is_la_inferred + la_label la first_time_property_let_as_social_housing unitletas @@ -134,6 +135,7 @@ RSpec.describe Csv::CaseLogCsvService do ppostcode_full previous_la_known is_previous_la_inferred + prevloc_label prevloc reasonpref rp_homeless From 6dd5f671c0c9a1bcf7baa30b2d57e84593efaf01 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Thu, 25 Aug 2022 15:24:13 +0100 Subject: [PATCH 04/53] test: add new columns post merge --- spec/fixtures/files/case_logs_download.csv | 4 ++-- spec/fixtures/files/case_logs_download_non_support.csv | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/fixtures/files/case_logs_download.csv b/spec/fixtures/files/case_logs_download.csv index 483641117..a09e5a523 100644 --- a/spec/fixtures/files/case_logs_download.csv +++ b/spec/fixtures/files/case_logs_download.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,needstype,renewal,startdate,rent_type,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,illness_type_1,illness_type_2,is_la_inferred,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,sale_or_letting,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,sale_completion_date,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,ethnic_other,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,rent_value_check,old_form_id,lar,irproduct,old_id,joint,illness_type_0,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_id,location_id,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_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,Supported housing,,,,,,,,,,,,,,,,,,,,,,,No,,,,No,Westminster,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,,9,,,,,,,6,{scheme_id},SE1 1TE,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2022-06-05 01: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,needstype,renewal,startdate,rent_type,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,sale_or_letting,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,sale_completion_date,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,ethnic_other,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,rent_value_check,old_form_id,lar,irproduct,old_id,joint,illness_type_0,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_id,location_id,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_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,Supported housing,,,,,,,,,,,,,,,,,,,,,,,No,,,,,No,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,,9,,,,,,,6,{scheme_id},SE1 1TE,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2022-06-05 01: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/case_logs_download_non_support.csv b/spec/fixtures/files/case_logs_download_non_support.csv index 12aca6dcf..a4815fb53 100644 --- a/spec/fixtures/files/case_logs_download_non_support.csv +++ b/spec/fixtures/files/case_logs_download_non_support.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,renewal,startdate,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,relat2,age2,sex2,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,prevloc,illness_type_1,illness_type_2,la,postcode_full,wchair,preg_occ,cbl,earnings,incfreq,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,sale_completion_date,unitletas,builtype,voiddate,lettype,nocharge,household_charge,referral,tshortfall,chcharge,ppcodenk,ethnic_group,ethnic_other,has_benefits,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,lar,irproduct,joint,illness_type_0,sheltered,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_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,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,renewal,startdate,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,relat2,age2,sex2,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,prevloc_label,illness_type_1,illness_type_2,la_label,postcode_full,wchair,preg_occ,cbl,earnings,incfreq,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,sale_completion_date,unitletas,builtype,voiddate,lettype,nocharge,household_charge,referral,tshortfall,chcharge,ppcodenk,ethnic_group,ethnic_other,has_benefits,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,lar,irproduct,joint,illness_type_0,sheltered,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_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,,,,,,,,,,,,,,,,,,,,,,,Westminster,SE1 1TE,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,,0,0,,,,,,,,,,,,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2022-06-05 01:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,Bungalow,Fitted with equipment and adaptations,Westminster,{location_startdate} From 013b78ee74e2427e636a9139e72c4e512fde7ce3 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Fri, 2 Sep 2022 09:43:23 +0100 Subject: [PATCH 05/53] tests: update --- app/services/csv/lettings_log_csv_service.rb | 2 +- spec/fixtures/files/lettings_logs_download_non_support.csv | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/csv/lettings_log_csv_service.rb b/app/services/csv/lettings_log_csv_service.rb index c0a660c76..35d8fb53e 100644 --- a/app/services/csv/lettings_log_csv_service.rb +++ b/app/services/csv/lettings_log_csv_service.rb @@ -44,7 +44,7 @@ module Csv remaining_attributes = LettingsLog.attribute_names - intersecting_attributes - scheme_and_location_ids @attributes = (metadata_fields + intersecting_attributes + remaining_attributes - metadata_id_fields + %w[unittype_sh] + scheme_attributes + location_attributes).uniq - move_is_inferred_fields + move_la_fields @attributes -= CSV_FIELDS_TO_OMIT if @user.present? && !@user.support? end diff --git a/spec/fixtures/files/lettings_logs_download_non_support.csv b/spec/fixtures/files/lettings_logs_download_non_support.csv index a4815fb53..3aeb62131 100644 --- a/spec/fixtures/files/lettings_logs_download_non_support.csv +++ b/spec/fixtures/files/lettings_logs_download_non_support.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,renewal,startdate,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,relat2,age2,sex2,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,prevloc_label,illness_type_1,illness_type_2,la_label,postcode_full,wchair,preg_occ,cbl,earnings,incfreq,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,sale_completion_date,unitletas,builtype,voiddate,lettype,nocharge,household_charge,referral,tshortfall,chcharge,ppcodenk,ethnic_group,ethnic_other,has_benefits,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,lar,irproduct,joint,illness_type_0,sheltered,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_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,,,,,,,,,,,,,,,,,,,,,,,Westminster,SE1 1TE,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,,0,0,,,,,,,,,,,,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2022-06-05 01: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,renewal,startdate,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,relat2,age2,sex2,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,prevloc_label,prevloc,illness_type_1,illness_type_2,la_label,la,postcode_full,wchair,preg_occ,cbl,earnings,incfreq,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,sale_completion_date,unitletas,builtype,voiddate,lettype,nocharge,household_charge,referral,tshortfall,chcharge,ppcodenk,ethnic_group,ethnic_other,has_benefits,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,lar,irproduct,joint,illness_type_0,sheltered,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_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,,,,,,,,,,,,,,,,,,,,,,,,Westminster,E09000033,SE1 1TE,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,,0,0,,,,,,,,,,,,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2022-06-05 01:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,Bungalow,Fitted with equipment and adaptations,Westminster,{location_startdate} From bc8371632a346933a4559b4d453e46bf07f5befd Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Fri, 9 Sep 2022 10:12:28 +0100 Subject: [PATCH 06/53] feat: relocate redundant methods --- app/models/lettings_log.rb | 8 -------- app/services/csv/lettings_log_csv_service.rb | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index aa854f420..142c48e52 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -465,14 +465,6 @@ class LettingsLog < ApplicationRecord location&.id end - def la_label - la - end - - def prevloc_label - prevloc - end - def self.to_csv(user = nil) Csv::LettingsLogCsvService.new(user).to_csv end diff --git a/app/services/csv/lettings_log_csv_service.rb b/app/services/csv/lettings_log_csv_service.rb index 35d8fb53e..6070e91c2 100644 --- a/app/services/csv/lettings_log_csv_service.rb +++ b/app/services/csv/lettings_log_csv_service.rb @@ -16,7 +16,7 @@ module Csv if %w[la prevloc].include? att label_from_value(record.send(att)) elsif %w[la_label prevloc_label].include? att - record.form.get_question(att.remove("_label"), record)&.label_from_value(record.send(att)) || label_from_value(record.send(att)) + record.form.get_question(att.remove("_label"), record)&.label_from_value(record.send(att.remove("_label"))) || label_from_value(record.send(att.remove("_label"))) else record.form.get_question(att, record)&.label_from_value(record.send(att)) || label_from_value(record.send(att)) end From f03949abd524549d856a33b9105d00248f9ba3d7 Mon Sep 17 00:00:00 2001 From: Mo Seedat <47113046+mseedat-moj@users.noreply.github.com> Date: Mon, 12 Sep 2022 14:19:07 +0100 Subject: [PATCH 07/53] CLDC-1551 Remove text 'as you told us' (#865) Co-authored-by: Mo Seedat --- config/locales/en.yml | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index c6403408c..388dff133 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -136,8 +136,8 @@ en: first_let_not_social: "Enter a reason for vacancy that is not 'first let' if unit has been previously let as social housing" first_let_social: "Reason for vacancy must be first let if unit has been previously let as social housing" 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 you already told us this accommodation is not temporary" - referral_invalid: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as you already told us a different source of referral for this letting" + 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" 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" @@ -207,7 +207,7 @@ en: not_provided: "Enter how much rent and other charges the household pays %{period}" household: reasonpref: - not_homeless: "Answer cannot be ‘homeless or about to lose their home’ as you already told us the tenant was not homeless immediately prior to this letting" + not_homeless: "Answer cannot be ‘homeless or about to lose their home’ as the tenant was not homeless immediately prior to this letting" reasonable_preference_reason: reason_required: "Enter a reason if you've answered 'yes' to reasonable preference" reason_not_required: "Do not enter a reason if you've answered 'no' to reasonable preference" @@ -245,31 +245,31 @@ en: housingneeds_a: one_or_two_choices: "You can only select one option or ‘other disabled access needs’ plus ‘wheelchair-accessible housing’, ‘wheelchair access to essential rooms’ or ‘level access housing’" prevten: - non_temp_accommodation: "Answer cannot be non-temporary accommodation as you already told us this is a re-let to a tenant who occupied the same property as temporary accommodation" + non_temp_accommodation: "Answer cannot be non-temporary accommodation as this is a re-let to a tenant who occupied the same property as temporary accommodation" over_20_foster_care: "Answer cannot be a children’s home or foster care as the lead tenant is 20 or older" male_refuge: "Answer cannot be a refuge as the lead tenant identifies as male" - internal_transfer: "Answer cannot be %{prevten} as you already told us this tenancy is an internal transfer" + internal_transfer: "Answer cannot be %{prevten} as this tenancy is an internal transfer" la_general_needs: - internal_transfer: "Answer cannot be a fixed-term or lifetime local authority general needs tenancy as you already told us it’s an internal transfer and a private registered provider is on the tenancy agreement" + internal_transfer: "Answer cannot be a fixed-term or lifetime local authority general needs tenancy as it’s an internal transfer and a private registered provider is on the tenancy agreement" referral: - secure_tenancy: "Answer must be internal transfer as you already told us this is a secure tenancy" - rsnvac_non_temp: "Answer cannot be this source of referral as you already told us this is a re-let to tenant who occupied the same property as temporary accommodation" - cannot_be_secure_tenancy: "Answer cannot be secure tenancy as you already told us this is not an internal transfer" - assessed_homeless: "Answer cannot be internal transfer as you already told us the tenant was assessed as homeless" - other_homeless: "Answer cannot be internal transfer as you already told us the tenant was considered homeless by their landlord" - prevten_invalid: "Answer cannot be internal transfer as you already told us the household situation immediately before this letting was %{prevten}" - reason_permanently_decanted: "Answer must be internal transfer as you already told us the tenant was permanently decanted from another property owned by this landlord" + secure_tenancy: "Answer must be internal transfer as this is a secure tenancy" + rsnvac_non_temp: "Answer cannot be this source of referral as this is a re-let to tenant who occupied the same property as temporary accommodation" + cannot_be_secure_tenancy: "Answer cannot be secure tenancy as this is not an internal transfer" + assessed_homeless: "Answer cannot be internal transfer as the tenant was assessed as homeless" + other_homeless: "Answer cannot be internal transfer as the tenant was considered homeless by their landlord" + prevten_invalid: "Answer cannot be internal transfer as the household situation immediately before this letting was %{prevten}" + reason_permanently_decanted: "Answer must be internal transfer as the tenant was permanently decanted from another property owned by this landlord" la_general_needs: - internal_transfer: "Answer cannot be internal transfer as you already told us it’s the same landlord on the tenancy agreement and the household had either a fixed-term or lifetime local authority general needs tenancy immediately before this letting" + internal_transfer: "Answer cannot be internal transfer as it’s the same landlord on the tenancy agreement and the household had either a fixed-term or lifetime local authority general needs tenancy immediately before this letting" prp: - local_housing_referral: "Answer cannot be ‘nominated by a local housing authority’ as you already told us a local authority is on the tenancy agreement" + local_housing_referral: "Answer cannot be ‘nominated by a local housing authority’ as a local authority is on the tenancy agreement" homeless: assessed: - internal_transfer: "Answer cannot be 'assessed as homeless' as you already told us this tenancy is an internal transfer" + internal_transfer: "Answer cannot be 'assessed as homeless' as this tenancy is an internal transfer" other: - internal_transfer: "Answer cannot be 'other homelessness' as you already told us this tenancy was an internal transfer" + internal_transfer: "Answer cannot be 'other homelessness' as this tenancy was an internal transfer" reasonpref: - not_homeless: "Answer cannot be ‘no’ as you already told us the tenant was homeless or about to lose their home" + not_homeless: "Answer cannot be ‘no’ as the tenant was homeless or about to lose their home" previous_la_known: "Enter name of local authority" gender: retired_male: "Answer cannot be ‘male’ as tenant is under 65 and retired" @@ -285,8 +285,8 @@ en: 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" - internal_transfer: "Answer must be secure tenancy as you already told us this tenancy is an internal transfer" - cannot_be_internal_transfer: "Answer cannot be internal transfer as you already told us this is not a secure tenancy" + 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" joint_more_than_one_member: "There must be more than one person in the household as you've told us this is a joint tenancy" From a92fb1030c2dd449ca8e729fb8f86fccd9a77f6d Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 13 Sep 2022 13:49:16 +0100 Subject: [PATCH 08/53] CLDC-1362 Improve CSV download performance (#851) * Replaced log CSV direct download with email * Tidy up authorization of organisations controller - We already have a method to authenticate the scope of the user, so we can reuse that. * Use Rails routes instead of absolute paths for CSV download links * Introduce base NotifyMailer to to abstract away shared Notify mail functionality * Fix mailer spec name * Add worker instance to PaaS manifest * Add CSV download bucket instance name into environment * Update tests for improved search term handling * Fix download mailer tests * Clarifying comments Co-authored-by: natdeanlewissoftwire Co-authored-by: James Rose --- .github/workflows/production_pipeline.yml | 2 + .github/workflows/staging_pipeline.yml | 2 + Gemfile | 2 + Gemfile.lock | 5 + app/controllers/lettings_logs_controller.rb | 29 +++- .../modules/lettings_logs_filter.rb | 30 ++-- app/controllers/modules/search_filter.rb | 8 +- app/controllers/organisations_controller.rb | 43 ++--- app/jobs/email_csv_job.rb | 21 +++ app/mailers/csv_download_mailer.rb | 11 ++ app/mailers/notify_mailer.rb | 37 ++++ app/services/filter_service.rb | 22 +++ app/services/storage/s3_service.rb | 6 + app/views/lettings_logs/_log_list.html.erb | 2 +- .../lettings_logs/csv_confirmation.html.erb | 15 ++ app/views/lettings_logs/download_csv.html.erb | 16 ++ app/views/lettings_logs/index.html.erb | 2 +- app/views/organisations/logs.html.erb | 2 +- config/application.rb | 2 + config/routes.rb | 6 + manifest.yml | 4 + spec/jobs/email_csv_job_spec.rb | 159 ++++++++++++++++++ spec/mailers/csv_download_mailer_spec.rb | 30 ++++ spec/rails_helper.rb | 1 + .../requests/lettings_logs_controller_spec.rb | 150 +++++++++-------- .../requests/organisations_controller_spec.rb | 63 ++++--- spec/services/filter_service_spec.rb | 28 +++ 27 files changed, 552 insertions(+), 146 deletions(-) create mode 100644 app/jobs/email_csv_job.rb create mode 100644 app/mailers/csv_download_mailer.rb create mode 100644 app/mailers/notify_mailer.rb create mode 100644 app/services/filter_service.rb create mode 100644 app/views/lettings_logs/csv_confirmation.html.erb create mode 100644 app/views/lettings_logs/download_csv.html.erb create mode 100644 spec/jobs/email_csv_job_spec.rb create mode 100644 spec/mailers/csv_download_mailer_spec.rb create mode 100644 spec/services/filter_service_spec.rb diff --git a/.github/workflows/production_pipeline.yml b/.github/workflows/production_pipeline.yml index 333e1f87b..4400d9239 100644 --- a/.github/workflows/production_pipeline.yml +++ b/.github/workflows/production_pipeline.yml @@ -238,6 +238,7 @@ jobs: RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} IMPORT_PAAS_INSTANCE: ${{ secrets.IMPORT_PAAS_INSTANCE }} EXPORT_PAAS_INSTANCE: ${{ secrets.EXPORT_PAAS_INSTANCE }} + CSV_DOWNLOAD_PAAS_INSTANCE: ${{ secrets.CSV_DOWNLOAD_PAAS_INSTANCE }} SENTRY_DSN: ${{ secrets.SENTRY_DSN }} run: | cf api $CF_API_ENDPOINT @@ -248,5 +249,6 @@ jobs: cf set-env $APP_NAME RAILS_MASTER_KEY $RAILS_MASTER_KEY cf set-env $APP_NAME IMPORT_PAAS_INSTANCE $IMPORT_PAAS_INSTANCE cf set-env $APP_NAME EXPORT_PAAS_INSTANCE $EXPORT_PAAS_INSTANCE + cf set-env $APP_NAME CSV_DOWNLOAD_PAAS_INSTANCE $CSV_DOWNLOAD_PAAS_INSTANCE cf set-env $APP_NAME SENTRY_DSN $SENTRY_DSN cf push $APP_NAME --strategy rolling diff --git a/.github/workflows/staging_pipeline.yml b/.github/workflows/staging_pipeline.yml index 4c588e919..72d561652 100644 --- a/.github/workflows/staging_pipeline.yml +++ b/.github/workflows/staging_pipeline.yml @@ -214,6 +214,7 @@ jobs: RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} IMPORT_PAAS_INSTANCE: ${{ secrets.IMPORT_PAAS_INSTANCE }} EXPORT_PAAS_INSTANCE: ${{ secrets.EXPORT_PAAS_INSTANCE }} + CSV_DOWNLOAD_PAAS_INSTANCE: ${{ secrets.CSV_DOWNLOAD_PAAS_INSTANCE }} SENTRY_DSN: ${{ secrets.SENTRY_DSN }} run: | cf api $CF_API_ENDPOINT @@ -226,5 +227,6 @@ jobs: cf set-env $APP_NAME RAILS_MASTER_KEY $RAILS_MASTER_KEY cf set-env $APP_NAME IMPORT_PAAS_INSTANCE $IMPORT_PAAS_INSTANCE cf set-env $APP_NAME EXPORT_PAAS_INSTANCE $EXPORT_PAAS_INSTANCE + cf set-env $APP_NAME CSV_DOWNLOAD_PAAS_INSTANCE $CSV_DOWNLOAD_PAAS_INSTANCE cf set-env $APP_NAME SENTRY_DSN $SENTRY_DSN cf push $APP_NAME --strategy rolling diff --git a/Gemfile b/Gemfile index ad3e1893d..e6dac1995 100644 --- a/Gemfile +++ b/Gemfile @@ -58,6 +58,8 @@ gem "sentry-ruby" gem "possessive" # Strip whitespace from active record attributes gem "auto_strip_attributes" +# Use sidekiq for background processing +gem "sidekiq" group :development, :test do # Check gems for known vulnerabilities diff --git a/Gemfile.lock b/Gemfile.lock index 4f4b352bb..bd7111af6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -381,6 +381,10 @@ GEM sentry-ruby (~> 5.4.2) sentry-ruby (5.4.2) concurrent-ruby (~> 1.0, >= 1.0.2) + sidekiq (6.5.4) + connection_pool (>= 2.2.2) + rack (~> 2.0) + redis (>= 4.5.0) simplecov (0.21.2) docile (~> 1.1) simplecov-html (~> 0.11) @@ -470,6 +474,7 @@ DEPENDENCIES selenium-webdriver sentry-rails sentry-ruby + sidekiq simplecov stimulus-rails timecop (~> 0.9.4) diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index a19fdbe6d..651dd7225 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -7,23 +7,20 @@ class LettingsLogsController < ApplicationController before_action :authenticate, if: :json_api_request? before_action :authenticate_user!, unless: :json_api_request? before_action :find_resource, except: %i[create index edit] + before_action :session_filters, if: :current_user + before_action :set_session_filters, if: :current_user def index - set_session_filters - - all_logs = current_user.lettings_logs - unpaginated_filtered_logs = filtered_lettings_logs(filtered_collection(all_logs, search_term)) - respond_to do |format| format.html do + all_logs = current_user.lettings_logs + unpaginated_filtered_logs = filtered_lettings_logs(all_logs, search_term, @session_filters) + + @search_term = search_term @pagy, @lettings_logs = pagy(unpaginated_filtered_logs) @searched = search_term.presence @total_count = all_logs.size end - - format.csv do - send_data byte_order_mark + unpaginated_filtered_logs.to_csv(current_user), filename: "logs-#{Time.zone.now}.csv" - end end end @@ -91,6 +88,20 @@ class LettingsLogsController < ApplicationController end end + def download_csv + unpaginated_filtered_logs = filtered_lettings_logs(current_user.lettings_logs, search_term, @session_filters) + + render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path } + end + + def email_csv + all_orgs = params["organisation_select"] == "all" + EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs) + redirect_to csv_confirmation_lettings_logs_path + end + + def csv_confirmation; end + private API_ACTIONS = %w[create show update destroy].freeze diff --git a/app/controllers/modules/lettings_logs_filter.rb b/app/controllers/modules/lettings_logs_filter.rb index c074e48bb..200a0db41 100644 --- a/app/controllers/modules/lettings_logs_filter.rb +++ b/app/controllers/modules/lettings_logs_filter.rb @@ -1,23 +1,21 @@ module Modules::LettingsLogsFilter - def filtered_lettings_logs(logs) - if session[:lettings_logs_filters].present? - filters = JSON.parse(session[:lettings_logs_filters]) - filters.each do |category, values| - next if Array(values).reject(&:empty?).blank? - next if category == "organisation" && params["organisation_select"] == "all" - - logs = logs.public_send("filter_by_#{category}", values, current_user) - end - end - logs = logs.order(created_at: :desc) - current_user.support? ? logs.all.includes(:owning_organisation, :managing_organisation) : logs + def filtered_lettings_logs(logs, search_term, filters) + all_orgs = params["organisation_select"] == "all" + FilterService.filter_lettings_logs(logs, search_term, filters, all_orgs, current_user) end - def set_session_filters(specific_org: false) - new_filters = session[:lettings_logs_filters].present? ? JSON.parse(session[:lettings_logs_filters]) : {} + def load_session_filters(specific_org: false) + current_filters = session[:lettings_logs_filters] + new_filters = current_filters.present? ? JSON.parse(current_filters) : {} current_user.lettings_logs_filters(specific_org:).each { |filter| new_filters[filter] = params[filter] if params[filter].present? } - new_filters = new_filters.except("organisation") if params["organisation_select"] == "all" + params["organisation_select"] == "all" ? new_filters.except("organisation") : new_filters + end + + def session_filters(specific_org: false) + @session_filters ||= load_session_filters(specific_org:) + end - session[:lettings_logs_filters] = new_filters.to_json + def set_session_filters + session[:lettings_logs_filters] = @session_filters.to_json end end diff --git a/app/controllers/modules/search_filter.rb b/app/controllers/modules/search_filter.rb index c32298987..82bf0d6c0 100644 --- a/app/controllers/modules/search_filter.rb +++ b/app/controllers/modules/search_filter.rb @@ -1,13 +1,9 @@ module Modules::SearchFilter def filtered_collection(base_collection, search_term = nil) - if search_term.present? - base_collection.search_by(search_term) - else - base_collection - end + FilterService.filter_by_search(base_collection, search_term) end def filtered_users(base_collection, search_term = nil) - filtered_collection(base_collection, search_term).includes(:organisation) + FilterService.filter_by_search(base_collection, search_term).includes(:organisation) end end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 1cf181788..3a979b78a 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -6,6 +6,8 @@ class OrganisationsController < ApplicationController before_action :authenticate_user! before_action :find_resource, except: %i[index new create] before_action :authenticate_scope!, except: [:index] + before_action -> { session_filters(specific_org: true) }, if: -> { current_user.support? } + before_action :set_session_filters, if: -> { current_user.support? } def index redirect_to organisation_path(current_user.organisation) unless current_user.support? @@ -88,29 +90,32 @@ class OrganisationsController < ApplicationController end def logs - if current_user.support? - set_session_filters(specific_org: true) - - organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) - unpaginated_filtered_logs = filtered_lettings_logs(filtered_collection(organisation_logs, search_term)) - - respond_to do |format| - format.html do - @pagy, @lettings_logs = pagy(unpaginated_filtered_logs) - @searched = search_term.presence - @total_count = organisation_logs.size - render "logs", layout: "application" - end + organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) + unpaginated_filtered_logs = filtered_lettings_logs(organisation_logs, search_term, @session_filters) - format.csv do - send_data byte_order_mark + unpaginated_filtered_logs.to_csv, filename: "logs-#{@organisation.name}-#{Time.zone.now}.csv" - end + respond_to do |format| + format.html do + @search_term = search_term + @pagy, @lettings_logs = pagy(unpaginated_filtered_logs) + @searched = search_term.presence + @total_count = organisation_logs.size + render "logs", layout: "application" end - else - redirect_to(lettings_logs_path) end end + def download_csv + organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) + unpaginated_filtered_logs = filtered_lettings_logs(organisation_logs, search_term, @session_filters) + + render "lettings_logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path } + end + + def email_csv + EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation) + redirect_to logs_csv_confirmation_organisation_path + end + private def org_params @@ -122,7 +127,7 @@ private end def authenticate_scope! - if %w[create new].include? action_name + if %w[create new logs download_csv email_csv].include? action_name head :unauthorized and return unless current_user.support? elsif current_user.organisation != @organisation && !current_user.support? render_not_found diff --git a/app/jobs/email_csv_job.rb b/app/jobs/email_csv_job.rb new file mode 100644 index 000000000..033ead7ae --- /dev/null +++ b/app/jobs/email_csv_job.rb @@ -0,0 +1,21 @@ +class EmailCsvJob < ApplicationJob + queue_as :default + + BYTE_ORDER_MARK = "\uFEFF".freeze # Required to ensure Excel always reads CSV as UTF-8 + + EXPIRATION_TIME = 3.hours.to_i + + def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params + unfiltered_logs = organisation.present? && user.support? ? LettingsLog.where(owning_organisation_id: organisation.id) : user.lettings_logs + filtered_logs = FilterService.filter_lettings_logs(unfiltered_logs, search_term, filters, all_orgs, user) + + filename = organisation.present? ? "logs-#{organisation.name}-#{Time.zone.now}.csv" : "logs-#{Time.zone.now}.csv" + + storage_service = Storage::S3Service.new(Configuration::PaasConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"]) + storage_service.write_file(filename, BYTE_ORDER_MARK + filtered_logs.to_csv(user)) + + url = storage_service.get_presigned_url(filename, EXPIRATION_TIME) + + CsvDownloadMailer.new.send_email(user, url, EXPIRATION_TIME) + end +end diff --git a/app/mailers/csv_download_mailer.rb b/app/mailers/csv_download_mailer.rb new file mode 100644 index 000000000..619d5e922 --- /dev/null +++ b/app/mailers/csv_download_mailer.rb @@ -0,0 +1,11 @@ +class CsvDownloadMailer < NotifyMailer + CSV_DOWNLOAD_TEMPLATE_ID = "7890e3b9-8c0d-4d08-bafe-427fd7cd95bf".freeze + + def send_csv_download_mail(user, link, duration) + send_email( + user.email, + CSV_DOWNLOAD_TEMPLATE_ID, + { name: user.name, link:, duration: ActiveSupport::Duration.build(duration).inspect }, + ) + end +end diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb new file mode 100644 index 000000000..506df5818 --- /dev/null +++ b/app/mailers/notify_mailer.rb @@ -0,0 +1,37 @@ +class NotifyMailer + require "notifications/client" + + def notify_client + @notify_client ||= ::Notifications::Client.new(ENV["GOVUK_NOTIFY_API_KEY"]) + end + + def send_email(email, template_id, personalisation) + return true if intercept_send?(email) + + notify_client.send_email( + email_address: email, + template_id:, + personalisation:, + ) + end + + def personalisation(record, token, url, username: false) + { + name: record.name || record.email, + email: username || record.email, + organisation: record.respond_to?(:organisation) ? record.organisation.name : "", + link: "#{url}#{token}", + } + end + + def intercept_send?(email) + return false unless email_allowlist + + email_domain = email.split("@").last.downcase + !(Rails.env.production? || Rails.env.test?) && email_allowlist.exclude?(email_domain) + end + + def email_allowlist + Rails.application.credentials[:email_allowlist] + end +end diff --git a/app/services/filter_service.rb b/app/services/filter_service.rb new file mode 100644 index 000000000..0986e7aca --- /dev/null +++ b/app/services/filter_service.rb @@ -0,0 +1,22 @@ +class FilterService + def self.filter_by_search(base_collection, search_term = nil) + if search_term.present? + base_collection.search_by(search_term) + else + base_collection + end + end + + def self.filter_lettings_logs(logs, search_term, filters, all_orgs, user) + logs = filter_by_search(logs, search_term) + + filters.each do |category, values| + next if Array(values).reject(&:empty?).blank? + next if category == "organisation" && all_orgs + + logs = logs.public_send("filter_by_#{category}", values, user) + end + logs = logs.order(created_at: :desc) + user.support? ? logs.all.includes(:owning_organisation, :managing_organisation) : logs + end +end diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb index aee76398d..c972225a1 100644 --- a/app/services/storage/s3_service.rb +++ b/app/services/storage/s3_service.rb @@ -20,6 +20,12 @@ module Storage response.key_count == 1 end + def get_presigned_url(file_name, duration) + Aws::S3::Presigner + .new({ client: @client }) + .presigned_url(:get_object, bucket: @configuration.bucket_name, key: file_name, expires_in: duration) + end + def get_file_io(file_name) @client.get_object(bucket: @configuration.bucket_name, key: file_name) .body diff --git a/app/views/lettings_logs/_log_list.html.erb b/app/views/lettings_logs/_log_list.html.erb index 93d365c40..c6ea5e9c3 100644 --- a/app/views/lettings_logs/_log_list.html.erb +++ b/app/views/lettings_logs/_log_list.html.erb @@ -1,6 +1,6 @@

<%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "logs", path: request.path)) %> - <%= govuk_link_to "Download (CSV)", "#{request.path}.csv", type: "text/csv" %> + <%= govuk_link_to "Download (CSV)", csv_download_url, type: "text/csv" %>

<% lettings_logs.map do |log| %> diff --git a/app/views/lettings_logs/csv_confirmation.html.erb b/app/views/lettings_logs/csv_confirmation.html.erb new file mode 100644 index 000000000..a5717cae9 --- /dev/null +++ b/app/views/lettings_logs/csv_confirmation.html.erb @@ -0,0 +1,15 @@ +<% content_for :title, "We've sending you an email" %> +
+
+ <%= govuk_panel(title_text: "We're sending you an email") %> + +

It should arrive in a few minutes, but it could take longer.

+ +

What happens next

+

Open your email inbox and click the link to download your CSV file.

+ +

+ <%= govuk_link_to "Return to logs", lettings_logs_path %> +

+
+
diff --git a/app/views/lettings_logs/download_csv.html.erb b/app/views/lettings_logs/download_csv.html.erb new file mode 100644 index 000000000..4e0da2704 --- /dev/null +++ b/app/views/lettings_logs/download_csv.html.erb @@ -0,0 +1,16 @@ +<% content_for :title, "Download CSV" %> + +<% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> +<% end %> + +
+
+

Download CSV

+ +

We'll send a secure download link to your email address <%= @current_user.email %>.

+

You've selected <%= count %> logs.

+ + <%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term } %> +
+
diff --git a/app/views/lettings_logs/index.html.erb b/app/views/lettings_logs/index.html.erb index 9378aee09..fe1004e69 100644 --- a/app/views/lettings_logs/index.html.erb +++ b/app/views/lettings_logs/index.html.erb @@ -15,7 +15,7 @@
<%= render SearchComponent.new(current_user:, search_label: "Search by log ID, tenant code, property reference or postcode", value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "log_list", locals: { lettings_logs: @lettings_logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %> + <%= render partial: "log_list", locals: { lettings_logs: @lettings_logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: csv_download_lettings_logs_path(search: @search_term) } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb index 916ac65ae..42c1697bc 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -21,7 +21,7 @@
<%= render SearchComponent.new(current_user:, search_label: "Search by log ID, tenant code, property reference or postcode", value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "lettings_logs/log_list", locals: { lettings_logs: @lettings_logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %> + <%= render partial: "lettings_logs/log_list", locals: { lettings_logs: @lettings_logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term) } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/config/application.rb b/config/application.rb index 7bdbfb93e..153782f54 100644 --- a/config/application.rb +++ b/config/application.rb @@ -33,5 +33,7 @@ module DataCollector # config.eager_load_paths << Rails.root.join("extras") config.exceptions_app = routes + + config.active_job.queue_adapter = :sidekiq end end diff --git a/config/routes.rb b/config/routes.rb index 5a2fda475..82fec6493 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -69,6 +69,9 @@ Rails.application.routes.draw do get "users", to: "organisations#users" get "users/invite", to: "users/account#new" get "logs", to: "organisations#logs" + get "logs/csv-download", to: "organisations#download_csv" + post "logs/email-csv", to: "organisations#email_csv" + get "logs/csv-confirmation", to: "lettings_logs#csv_confirmation" get "schemes", to: "organisations#schemes" end end @@ -77,6 +80,9 @@ Rails.application.routes.draw do collection do post "bulk-upload", to: "bulk_upload#bulk_upload" get "bulk-upload", to: "bulk_upload#show" + get "csv-download", to: "lettings_logs#download_csv" + post "email-csv", to: "lettings_logs#email_csv" + get "csv-confirmation", to: "lettings_logs#csv_confirmation" end member do diff --git a/manifest.yml b/manifest.yml index be4095eff..f84feb810 100644 --- a/manifest.yml +++ b/manifest.yml @@ -7,6 +7,10 @@ defaults: &defaults command: bundle exec rake cf:on_first_instance db:migrate && bin/rails server instances: 2 memory: 1G + - type: worker + command: bundle exec sidekiq + health-check-type: process + instances: 2 health-check-type: http health-check-http-endpoint: /health diff --git a/spec/jobs/email_csv_job_spec.rb b/spec/jobs/email_csv_job_spec.rb new file mode 100644 index 000000000..ae3e14b90 --- /dev/null +++ b/spec/jobs/email_csv_job_spec.rb @@ -0,0 +1,159 @@ +require "rails_helper" + +describe EmailCsvJob do + include Helpers + + test_url = :test_url + + let(:job) { described_class.new } + let(:user) { FactoryBot.create(:user) } + let(:organisation) { user.organisation } + let(:other_organisation) { FactoryBot.create(:organisation) } + + context "when a log exists" do + let!(:lettings_log) do + FactoryBot.create( + :lettings_log, + owning_organisation: organisation, + managing_organisation: organisation, + ecstat1: 1, + ) + end + + let(:storage_service) { instance_double(Storage::S3Service) } + let(:mailer) { instance_double(CsvDownloadMailer) } + + before do + FactoryBot.create(:lettings_log, + :completed, + owning_organisation: organisation, + managing_organisation: organisation, + created_by: user) + + allow(Storage::S3Service).to receive(:new).and_return(storage_service) + allow(storage_service).to receive(:write_file) + allow(storage_service).to receive(:get_presigned_url).and_return(test_url) + + allow(CsvDownloadMailer).to receive(:new).and_return(mailer) + allow(mailer).to receive(:send_email) + end + + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/logs-.*\.csv/, anything) + job.perform(user) + end + + it "includes the organisation name in the filename when one is provided" do + expect(storage_service).to receive(:write_file).with(/logs-#{organisation.name}-.*\.csv/, anything) + job.perform(user, nil, {}, nil, organisation) + end + + it "sends an E-mail with the presigned URL and duration" do + expect(mailer).to receive(:send_email).with(user, test_url, instance_of(Integer)) + job.perform(user) + end + + context "when writing to S3" do + before do + FactoryBot.create_list(:lettings_log, 4, owning_organisation: other_organisation) + end + + def expect_csv + expect(storage_service).to receive(:write_file) do |_filename, data| + # Ignore byte order marker + csv = CSV.parse(data[1..]) + yield(csv) + end + end + + it "writes CSV data with headers" do + expect_csv do |csv| + expect(csv.first.first).to eq("id") + expect(csv.second.first).to eq(lettings_log.id.to_s) + end + + job.perform(user) + end + + context "when there is no organisation provided" do + it "only writes logs from the user's organisation" do + expect_csv do |csv| + # Headings + 2 rows + expect(csv.count).to eq(3) + end + + job.perform(user) + end + end + + context "when the user is support and an organisation is provided" do + let(:user) { FactoryBot.create(:user, :support) } + + it "only writes logs from that organisation" do + expect_csv do |csv| + # other organisation => Headings + 4 rows + expect(csv.count).to eq(5) + end + + job.perform(user, nil, {}, nil, other_organisation) + end + end + + it "writes answer labels rather than values" do + expect_csv do |csv| + expect(csv.second[15]).to eq("Full-time – 30 hours or more") + end + + job.perform(user) + end + + it "writes filtered logs" do + expect_csv do |csv| + expect(csv.count).to eq(2) + end + + job.perform(user, nil, { status: "completed" }) + end + + it "writes searched logs" do + expect_csv do |csv| + expect(csv.count).to eq(LettingsLog.search_by(lettings_log.id.to_s).count + 1) + end + + job.perform(user, lettings_log.id.to_s) + end + + context "when both filter and search applied" do + let(:postcode) { "XX1 1TG" } + + before do + FactoryBot.create(:lettings_log, :in_progress, postcode_full: postcode, owning_organisation: organisation, created_by: user) + FactoryBot.create(:lettings_log, :completed, postcode_full: postcode, owning_organisation: organisation, created_by: user) + end + + it "downloads logs matching both csv and filter logs" do + expect_csv do |csv| + expect(csv.count).to eq(2) + end + + job.perform(user, postcode, { status: "completed" }) + end + end + + context "when there are more than 20 logs" do + before do + FactoryBot.create_list(:lettings_log, 26, owning_organisation: organisation) + end + + it "does not paginate, it downloads all the user's logs" do + expect_csv do |csv| + # Heading + 2 + 26 + expect(csv.count).to eq(29) + end + + job.perform(user) + end + end + end + end +end diff --git a/spec/mailers/csv_download_mailer_spec.rb b/spec/mailers/csv_download_mailer_spec.rb new file mode 100644 index 000000000..6c145c508 --- /dev/null +++ b/spec/mailers/csv_download_mailer_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +RSpec.describe CsvDownloadMailer do + describe "#send_csv_download_mail" do + let(:notify_client) { instance_double(Notifications::Client) } + let(:user) { FactoryBot.create(:user, email: "user@example.com") } + + before do + allow(Notifications::Client).to receive(:new).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) + end + + it "sends a CSV download E-mail via notify" do + link = :link + duration = 20.minutes.to_i + + expect(notify_client).to receive(:send_email).with( + email_address: user.email, + template_id: described_class::CSV_DOWNLOAD_TEMPLATE_ID, + personalisation: { + name: user.name, + link:, + duration: "20 minutes", + }, + ) + + described_class.new.send_csv_download_mail(user, link, duration) + end + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 78867bad9..cdeb71092 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -84,4 +84,5 @@ RSpec.configure do |config| config.include Devise::Test::IntegrationHelpers, type: :request config.include ViewComponent::TestHelpers, type: :component config.include Capybara::RSpecMatchers, type: :component + config.include ActiveJob::TestHelper end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index e70cace51..85b0c0f36 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -383,6 +383,12 @@ RSpec.describe LettingsLogsController, type: :request do end end + it "includes the search on the CSV link" do + search_term = "foo" + get "/logs?search=#{search_term}", headers: headers, params: {} + expect(page).to have_link("Download (CSV)", href: "/logs/csv-download?search=#{search_term}") + end + context "when more than one results with matching postcode" do let!(:matching_postcode_log) { FactoryBot.create(:lettings_log, :completed, owning_organisation: user.organisation, postcode_full: log_to_search.postcode_full) } @@ -491,8 +497,8 @@ RSpec.describe LettingsLogsController, type: :request do expect(page).to have_title("Logs - Submit social housing lettings and sales data (CORE) - GOV.UK") end - it "shows the download csv link" do - expect(page).to have_link("Download (CSV)", href: "/logs.csv") + it "shows the CSV download link" do + expect(page).to have_link("Download (CSV)", href: "/logs/csv-download") end it "does not show the organisation filter" do @@ -729,91 +735,43 @@ RSpec.describe LettingsLogsController, type: :request do expect(CGI.unescape_html(response.body)).to include("You didn’t answer this question") end end - end - describe "CSV download" do - let(:headers) { { "Accept" => "text/csv" } } - let(:user) { FactoryBot.create(:user) } - let(:organisation) { user.organisation } - let(:other_organisation) { FactoryBot.create(:organisation) } - - context "when a log exists" do - let!(:lettings_log) do - FactoryBot.create( - :lettings_log, - owning_organisation: organisation, - managing_organisation: organisation, - ecstat1: 1, - ) - end + context "when requesting CSV download" do + let(:headers) { { "Accept" => "text/html" } } + let(:search_term) { "foo" } before do sign_in user - FactoryBot.create(:lettings_log) - FactoryBot.create(:lettings_log, - :completed, - owning_organisation: organisation, - managing_organisation: organisation, - created_by: user) - get "/logs", headers:, params: {} + get "/logs/csv-download?search=#{search_term}", headers: end - it "downloads a CSV file with headers" do - csv = CSV.parse(response.body) - expect(csv.first.first).to eq("\uFEFFid") - expect(csv.second.first).to eq(lettings_log.id.to_s) - end - - it "does not download other orgs logs" do - csv = CSV.parse(response.body) - expect(csv.count).to eq(3) - end - - it "downloads answer labels rather than values" do - csv = CSV.parse(response.body) - expect(csv.second[15]).to eq("Full-time – 30 hours or more") + it "returns http success" do + expect(response).to have_http_status(:success) end - it "downloads filtered logs" do - get "/logs?status[]=completed", headers:, params: {} - csv = CSV.parse(response.body) - expect(csv.count).to eq(2) + it "shows a confirmation button" do + expect(page).to have_button("Send email") end - it "dowloads searched logs" do - get "/logs?search=#{lettings_log.id}", headers:, params: {} - csv = CSV.parse(response.body) - expect(csv.count).to eq(LettingsLog.search_by(lettings_log.id.to_s).count + 1) + it "includes the search term" do + expect(page).to have_field("search", type: "hidden", with: search_term) end + end - context "when both filter and search applied" do - let(:postcode) { "XX1 1TG" } + context "when confirming the CSV email" do + let(:headers) { { "Accept" => "text/html" } } + context "when a log exists" do before do - FactoryBot.create(:lettings_log, :in_progress, postcode_full: postcode, owning_organisation: organisation, created_by: user) - FactoryBot.create(:lettings_log, :completed, postcode_full: postcode, owning_organisation: organisation, created_by: user) + sign_in user end - it "downloads logs matching both csv and filter logs" do - get "/logs?status[]=completed&search=#{postcode}", headers:, params: {} - csv = CSV.parse(response.body) - expect(csv.count).to eq(2) + it "confirms that the user will receive an email with the requested CSV" do + get "/logs/csv-confirmation" + expect(CGI.unescape_html(response.body)).to include("We're sending you an email") end end end - - context "when there are more than 20 logs" do - before do - sign_in user - FactoryBot.create_list(:lettings_log, 26, owning_organisation: organisation) - get "/logs", headers:, params: {} - end - - it "does not paginate, it downloads all the user's logs" do - csv = CSV.parse(response.body) - expect(csv.count).to eq(27) - end - end end describe "PATCH" do @@ -966,4 +924,60 @@ RSpec.describe LettingsLogsController, type: :request do end end end + + describe "POST #email-csv" do + let(:other_organisation) { FactoryBot.create(:organisation) } + + context "when a log exists" do + let!(:lettings_log) do + FactoryBot.create( + :lettings_log, + owning_organisation:, + managing_organisation: owning_organisation, + ecstat1: 1, + ) + end + + before do + sign_in user + FactoryBot.create(:lettings_log) + FactoryBot.create(:lettings_log, + :completed, + owning_organisation:, + managing_organisation: owning_organisation, + created_by: user) + end + + it "creates an E-mail job" do + expect { + post "/logs/email-csv", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false) + end + + it "redirects to the confirmation page" do + post "/logs/email-csv", headers:, params: {} + expect(response).to redirect_to(csv_confirmation_lettings_logs_path) + end + + it "passes the search term" do + expect { + post "/logs/email-csv?search=#{lettings_log.id}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, lettings_log.id.to_s, {}, false) + end + + it "passes filter parameters" do + expect { + post "/logs/email-csv?status[]=completed", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false) + end + + it "passes a combination of search term and filter parameters" do + postcode = "XX1 1TG" + + expect { + post "/logs/email-csv?status[]=completed&search=#{postcode}", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, postcode, { "status" => %w[completed] }, false) + end + end + end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 20197f468..6e635da40 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -352,26 +352,30 @@ RSpec.describe OrganisationsController, type: :request do end context "when viewing logs for other organisation" do - before do + it "does not display the logs" do get "/organisations/#{unauthorised_organisation.id}/logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) end - it "returns not found 404 from org details route" do - expect(response).to have_http_status(:not_found) - end - - it "shows the 404 view" do - expect(page).to have_content("Page not found") + it "prevents CSV download" do + expect { + post "/organisations/#{unauthorised_organisation.id}/logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) end end context "when viewing logs for your organisation" do - before do + it "does not display the logs" do get "/organisations/#{organisation.id}/logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) end - it "redirects to /logs page" do - expect(response).to redirect_to("/logs") + it "prevents CSV download" do + expect { + post "/organisations/#{organisation.id}/logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) end end @@ -495,26 +499,30 @@ RSpec.describe OrganisationsController, type: :request do end context "when viewing logs for other organisation" do - before do + it "does not display the logs" do get "/organisations/#{unauthorised_organisation.id}/logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) end - it "returns not found 404 from org details route" do - expect(response).to have_http_status(:not_found) - end - - it "shows the 404 view" do - expect(page).to have_content("Page not found") + it "prevents CSV download" do + expect { + post "/organisations/#{unauthorised_organisation.id}/logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) end end context "when viewing logs for your organisation" do - before do + it "does not display the logs" do get "/organisations/#{organisation.id}/logs", headers:, params: {} + expect(response).to have_http_status(:unauthorized) end - it "redirects to /logs page" do - expect(response).to redirect_to("/logs") + it "prevents CSV download" do + expect { + post "/organisations/#{organisation.id}/logs/email-csv", headers:, params: {} + }.not_to enqueue_job(EmailCsvJob) + expect(response).to have_http_status(:unauthorized) end end end @@ -1035,11 +1043,10 @@ RSpec.describe OrganisationsController, type: :request do end it "has a CSV download button with the correct path" do - expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/logs.csv") + expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/logs/csv-download") end context "when you download the CSV" do - let(:headers) { { "Accept" => "text/csv" } } let(:other_organisation) { FactoryBot.create(:organisation) } before do @@ -1048,9 +1055,15 @@ RSpec.describe OrganisationsController, type: :request do end it "only includes logs from that organisation" do - get "/organisations/#{organisation.id}/logs", headers:, params: {} - csv = CSV.parse(response.body) - expect(csv.count).to eq(4) + get "/organisations/#{organisation.id}/logs/csv-download" + + expect(page).to have_text("You've selected 3 logs.") + end + + it "provides the organisation to the mail job" do + expect { + post "/organisations/#{organisation.id}/logs/email-csv?status[]=completed", headers:, params: {} + }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation) end end end diff --git a/spec/services/filter_service_spec.rb b/spec/services/filter_service_spec.rb new file mode 100644 index 000000000..1e892fd56 --- /dev/null +++ b/spec/services/filter_service_spec.rb @@ -0,0 +1,28 @@ +require "rails_helper" + +describe FilterService do + describe "filter_by_search" do + before do + FactoryBot.create_list(:organisation, 5) + FactoryBot.create(:organisation, name: "Acme LTD") + end + + let(:organisation_list) { Organisation.all } + + context "when given a search term" do + let(:search_term) { "Acme" } + + it "filters the collection on search term" do + expect(described_class.filter_by_search(organisation_list, search_term).count).to eq(1) + end + end + + context "when not given a search term" do + let(:search_term) { nil } + + it "does not filter the given collection" do + expect(described_class.filter_by_search(organisation_list, search_term).count).to eq(6) + end + end + end +end From 67ce147fc969218f1b094aebd4cbdbd4c6e1b460 Mon Sep 17 00:00:00 2001 From: Dushan <47317567+dushan-madetech@users.noreply.github.com> Date: Tue, 13 Sep 2022 16:41:20 +0100 Subject: [PATCH 09/53] Extract base log and create sales log (#858) * Add abstract log class and sales log class Created a parent log class for sales log and lettings log. Any bits common to both sales and lettings can live in the parent class. As the sales log functionality is built up any commonalities with lettings log can be extracted into the parent log class. The sales log model is set up without a json form and instead the form is defined in code - like the setup section of the lettings log. * update sales logs controller * update lettings controller specs * update filter method name * update organisations controller * use lettings method * Add deleted tests back * lint Co-authored-by: Kat Co-authored-by: Kat --- ...swers_summary_list_card_component.html.erb | 8 +- ...eck_answers_summary_list_card_component.rb | 10 +- app/components/log_summary_component.html.erb | 6 +- app/controllers/bulk_upload_controller.rb | 4 +- app/controllers/form_controller.rb | 99 ++- app/controllers/lettings_logs_controller.rb | 98 +-- app/controllers/logs_controller.rb | 73 ++ .../modules/lettings_logs_filter.rb | 21 - app/controllers/modules/logs_filter.rb | 21 + app/controllers/organisations_controller.rb | 35 +- app/controllers/sales_logs_controller.rb | 46 + app/helpers/check_answers_helper.rb | 4 + app/helpers/filters_helper.rb | 8 +- app/helpers/navigation_items_helper.rb | 35 +- app/helpers/tasklist_helper.rb | 32 +- app/jobs/email_csv_job.rb | 2 +- app/models/form.rb | 133 +-- app/models/form/question.rb | 4 +- .../form/sales/setup/pages/sale_date.rb | 15 + .../form/sales/setup/questions/sale_date.rb | 10 + app/models/form/sales/setup/sections/setup.rb | 10 + .../form/sales/setup/subsections/setup.rb | 14 + app/models/form_handler.rb | 13 +- app/models/lettings_log.rb | 57 +- app/models/log.rb | 61 ++ app/models/organisation.rb | 6 + app/models/sales_log.rb | 44 + app/models/user.rb | 14 +- app/services/filter_service.rb | 2 +- .../form/_check_answers_summary_list.html.erb | 12 +- app/views/form/_checkbox_question.html.erb | 4 +- app/views/form/_numeric_question.html.erb | 2 +- app/views/form/_radio_question.html.erb | 2 +- app/views/form/_select_question.html.erb | 6 +- app/views/form/check_answers.html.erb | 24 +- app/views/form/page.html.erb | 12 +- app/views/form/review.html.erb | 8 +- app/views/layouts/application.html.erb | 4 +- .../{lettings_logs => logs}/_log_filters.erb | 2 +- .../_log_list.html.erb | 3 +- .../_tasklist.html.erb | 8 +- .../bulk_upload.html.erb | 0 .../csv_confirmation.html.erb | 0 .../download_csv.html.erb | 0 .../{lettings_logs => logs}/edit.html.erb | 18 +- .../{lettings_logs => logs}/index.html.erb | 9 +- .../organisations/_organisation_list.html.erb | 2 +- app/views/organisations/logs.html.erb | 4 +- config/initializers/feature_toggle.rb | 6 + config/routes.rb | 22 +- db/migrate/20220826093411_add_sales_log.rb | 12 + db/schema.rb | 14 + ...nswers_summary_list_card_component_spec.rb | 14 +- spec/factories/sales_log.rb | 12 + .../form/accessible_autocomplete_spec.rb | 6 +- spec/features/form/check_answers_page_spec.rb | 60 +- spec/features/form/checkboxes_spec.rb | 8 +- .../form/conditional_questions_spec.rb | 6 +- spec/features/form/form_navigation_spec.rb | 48 +- spec/features/form/helpers.rb | 6 +- spec/features/form/page_routing_spec.rb | 46 +- .../form/progressive_total_field_spec.rb | 6 +- spec/features/form/saving_data_spec.rb | 10 +- spec/features/form/tasklist_page_spec.rb | 10 +- spec/features/form/validations_spec.rb | 36 +- .../{log_spec.rb => lettings_log_spec.rb} | 71 +- spec/features/organisation_spec.rb | 66 +- spec/features/sales_log_spec.rb | 37 + spec/features/schemes_helpers.rb | 6 +- spec/features/schemes_spec.rb | 18 +- spec/features/start_page_spec.rb | 4 +- spec/features/user_spec.rb | 54 +- spec/helpers/filters_helper_spec.rb | 12 +- spec/helpers/navigation_items_helper_spec.rb | 812 +++++++++++++----- spec/models/form/question_spec.rb | 2 +- .../form/sales/setup/pages/sale_date_spec.rb | 29 + .../sales/setup/questions/sale_date_spec.rb | 33 + .../form/sales/setup/sections/setup_spec.rb | 29 + .../sales/setup/subsections/setup_spec.rb | 27 + spec/models/form_handler_spec.rb | 5 + spec/models/form_spec.rb | 21 + spec/models/lettings_log_spec.rb | 10 + spec/models/log_spec.rb | 8 + spec/models/organisation_spec.rb | 29 + spec/models/sales_log_spec.rb | 90 ++ spec/models/user_spec.rb | 38 +- .../auth/passwords_controller_spec.rb | 2 +- spec/requests/bulk_upload_controller_spec.rb | 2 +- spec/requests/form_controller_spec.rb | 86 +- .../requests/lettings_logs_controller_spec.rb | 129 +-- .../requests/organisations_controller_spec.rb | 137 ++- spec/requests/sales_logs_controller_spec.rb | 433 ++++++++++ spec/requests/users_controller_spec.rb | 6 +- spec/views/form/page_view_spec.rb | 2 +- 94 files changed, 2589 insertions(+), 936 deletions(-) create mode 100644 app/controllers/logs_controller.rb delete mode 100644 app/controllers/modules/lettings_logs_filter.rb create mode 100644 app/controllers/modules/logs_filter.rb create mode 100644 app/controllers/sales_logs_controller.rb create mode 100644 app/models/form/sales/setup/pages/sale_date.rb create mode 100644 app/models/form/sales/setup/questions/sale_date.rb create mode 100644 app/models/form/sales/setup/sections/setup.rb create mode 100644 app/models/form/sales/setup/subsections/setup.rb create mode 100644 app/models/log.rb create mode 100644 app/models/sales_log.rb rename app/views/{lettings_logs => logs}/_log_filters.erb (95%) rename app/views/{lettings_logs => logs}/_log_list.html.erb (90%) rename app/views/{lettings_logs => logs}/_tasklist.html.erb (68%) rename app/views/{lettings_logs => logs}/bulk_upload.html.erb (100%) rename app/views/{lettings_logs => logs}/csv_confirmation.html.erb (100%) rename app/views/{lettings_logs => logs}/download_csv.html.erb (100%) rename app/views/{lettings_logs => logs}/edit.html.erb (63%) rename app/views/{lettings_logs => logs}/index.html.erb (61%) create mode 100644 db/migrate/20220826093411_add_sales_log.rb create mode 100644 spec/factories/sales_log.rb rename spec/features/{log_spec.rb => lettings_log_spec.rb} (68%) create mode 100644 spec/features/sales_log_spec.rb create mode 100644 spec/models/form/sales/setup/pages/sale_date_spec.rb create mode 100644 spec/models/form/sales/setup/questions/sale_date_spec.rb create mode 100644 spec/models/form/sales/setup/sections/setup_spec.rb create mode 100644 spec/models/form/sales/setup/subsections/setup_spec.rb create mode 100644 spec/models/log_spec.rb create mode 100644 spec/models/sales_log_spec.rb create mode 100644 spec/requests/sales_logs_controller_spec.rb diff --git a/app/components/check_answers_summary_list_card_component.html.erb b/app/components/check_answers_summary_list_card_component.html.erb index 73e9335c8..f299674b2 100644 --- a/app/components/check_answers_summary_list_card_component.html.erb +++ b/app/components/check_answers_summary_list_card_component.html.erb @@ -16,18 +16,18 @@ <% row.key { question.check_answer_label.to_s.presence || question.header.to_s } %> <% row.value do %> <%= get_answer_label(question) %> - <% extra_value = question.get_extra_check_answer_value(lettings_log) %> + <% extra_value = question.get_extra_check_answer_value(log) %> <% if extra_value %> <%= extra_value %> <% end %>
- <% question.get_inferred_answers(lettings_log).each do |inferred_answer| %> + <% question.get_inferred_answers(log).each do |inferred_answer| %> <%= inferred_answer %> <% end %> <% end %> <% row.action( - text: question.action_text(lettings_log), - href: question.action_href(lettings_log, question.page.id), + text: question.action_text(log), + href: question.action_href(log, question.page.id), visually_hidden_text: question.check_answer_label.to_s.downcase, ) %> <% end %> diff --git a/app/components/check_answers_summary_list_card_component.rb b/app/components/check_answers_summary_list_card_component.rb index bc7d8cdb9..de7fe9685 100644 --- a/app/components/check_answers_summary_list_card_component.rb +++ b/app/components/check_answers_summary_list_card_component.rb @@ -1,18 +1,18 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base - attr_reader :questions, :lettings_log, :user + attr_reader :questions, :log, :user - def initialize(questions:, lettings_log:, user:) + def initialize(questions:, log:, user:) @questions = questions - @lettings_log = lettings_log + @log = log @user = user super end def applicable_questions - questions.reject { |q| q.hidden_in_check_answers?(lettings_log, user) } + questions.reject { |q| q.hidden_in_check_answers?(log, user) } end def get_answer_label(question) - question.answer_label(lettings_log).presence || "You didn’t answer this question".html_safe + question.answer_label(log).presence || "You didn’t answer this question".html_safe end end diff --git a/app/components/log_summary_component.html.erb b/app/components/log_summary_component.html.erb index 07203b26b..65d2fc045 100644 --- a/app/components/log_summary_component.html.erb +++ b/app/components/log_summary_component.html.erb @@ -3,11 +3,11 @@

- <%= govuk_link_to lettings_log_path(log) do %> + <%= govuk_link_to log.lettings? ? lettings_log_path(log) : sales_log_path(log) do %> Log <%= log.id %> <% end %>

- <% if log.tenancycode? or log.propcode? %> + <% if log.lettings? && (log.tenancycode? or log.propcode?) %>
<% if log.tenancycode? %>
@@ -25,7 +25,7 @@ <% end %>
- <% if log.needstype? or log.startdate? %> + <% if log.is_a?(LettingsLog) && (log.needstype? or log.startdate?) %>

<% if log.needstype? %> <%= log.is_general_needs? ? "General needs" : "Supported housing" %>
diff --git a/app/controllers/bulk_upload_controller.rb b/app/controllers/bulk_upload_controller.rb index ffc22d028..ed2ac2627 100644 --- a/app/controllers/bulk_upload_controller.rb +++ b/app/controllers/bulk_upload_controller.rb @@ -3,7 +3,7 @@ class BulkUploadController < ApplicationController def show @bulk_upload = BulkUpload.new(nil, nil) - render "lettings_logs/bulk_upload" + render "logs/bulk_upload" end def bulk_upload @@ -12,7 +12,7 @@ class BulkUploadController < ApplicationController @bulk_upload = BulkUpload.new(file, content_type) @bulk_upload.process(current_user) if @bulk_upload.errors.present? - render "lettings_logs/bulk_upload", status: :unprocessable_entity + render "logs/bulk_upload", status: :unprocessable_entity else redirect_to(lettings_logs_path) end diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index a7ee469b8..6e71de6c6 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -4,22 +4,22 @@ class FormController < ApplicationController before_action :find_resource_by_named_id, except: %i[submit_form review] def submit_form - if @lettings_log - @page = @lettings_log.form.get_page(params[:lettings_log][:page]) + if @log + @page = @log.form.get_page(params[@log.model_name.param_key][:page]) responses_for_page = responses_for_page(@page) mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page) - if mandatory_questions_with_no_response.empty? && @lettings_log.update(responses_for_page) + if mandatory_questions_with_no_response.empty? && @log.update(responses_for_page) session[:errors] = session[:fields] = nil redirect_to(successful_redirect_path) else - redirect_path = "lettings_log_#{@page.id}_path" + redirect_path = "#{@log.model_name.param_key}_#{@page.id}_path" mandatory_questions_with_no_response.map do |question| - @lettings_log.errors.add question.id.to_sym, question.unanswered_error_message + @log.errors.add question.id.to_sym, question.unanswered_error_message end - session[:errors] = @lettings_log.errors.to_json - Rails.logger.info "User triggered validation(s) on: #{@lettings_log.errors.map(&:attribute).join(', ')}" - redirect_to(send(redirect_path, @lettings_log)) + session[:errors] = @log.errors.to_json + Rails.logger.info "User triggered validation(s) on: #{@log.errors.map(&:attribute).join(', ')}" + redirect_to(send(redirect_path, @log)) end else render_not_found @@ -27,9 +27,9 @@ class FormController < ApplicationController end def check_answers - if @lettings_log + if @log current_url = request.env["PATH_INFO"] - subsection = @lettings_log.form.get_subsection(current_url.split("/")[-2]) + subsection = @log.form.get_subsection(current_url.split("/")[-2]) render "form/check_answers", locals: { subsection:, current_user: } else render_not_found @@ -37,29 +37,26 @@ class FormController < ApplicationController end def review - if @lettings_log + if @log render "form/review" else render_not_found end end - FormHandler.instance.forms.each do |_key, form| - form.pages.map do |page| - define_method(page.id) do |_errors = {}| - if @lettings_log - restore_error_field_values - @subsection = @lettings_log.form.subsection_for_page(page) - @page = @lettings_log.form.get_page(page.id) - if @page.routed_to?(@lettings_log, current_user) - render "form/page" - else - redirect_to lettings_log_path(@lettings_log) - end - else - render_not_found - end + def show_page + if @log + restore_error_field_values + page_id = request.path.split("/")[-1].underscore + @page = @log.form.get_page(page_id) + @subsection = @log.form.subsection_for_page(@page) + if @page.routed_to?(@log, current_user) + render "form/page" + else + redirect_to lettings_log_path(@log) end + else + render_not_found end end @@ -68,13 +65,13 @@ private def restore_error_field_values if session["errors"] JSON(session["errors"]).each do |field, messages| - messages.each { |message| @lettings_log.errors.add field.to_sym, message } + messages.each { |message| @log.errors.add field.to_sym, message } end end if session["fields"] session["fields"].each do |field, value| - unless @lettings_log.form.get_question(field, @lettings_log)&.type == "date" - @lettings_log[field] = value + if @log.form.get_question(field, @log)&.type != "date" && @log.respond_to?(field) + @log[field] = value end end end @@ -82,11 +79,11 @@ private def responses_for_page(page) page.questions.each_with_object({}) do |question, result| - question_params = params["lettings_log"][question.id] + question_params = params[@log.model_name.param_key][question.id] if question.type == "date" - day = params["lettings_log"]["#{question.id}(3i)"] - month = params["lettings_log"]["#{question.id}(2i)"] - year = params["lettings_log"]["#{question.id}(1i)"] + day = params[@log.model_name.param_key]["#{question.id}(3i)"] + month = params[@log.model_name.param_key]["#{question.id}(2i)"] + year = params[@log.model_name.param_key]["#{question.id}(1i)"] next unless [day, month, year].any?(&:present?) result[question.id] = if Date.valid_date?(year.to_i, month.to_i, day.to_i) && year.to_i.between?(2000, 2200) @@ -109,11 +106,19 @@ private end def find_resource - @lettings_log = current_user.lettings_logs.find_by(id: params[:id]) + @log = if params.key?("sales_log") + current_user.sales_logs.find_by(id: params[:id]) + else + current_user.lettings_logs.find_by(id: params[:id]) + end end def find_resource_by_named_id - @lettings_log = current_user.lettings_logs.find_by(id: params[:lettings_log_id]) + @log = if params[:sales_log_id].present? + current_user.sales_logs.find_by(id: params[:sales_log_id]) + else + current_user.lettings_logs.find_by(id: params[:lettings_log_id]) + end end def is_referrer_check_answers? @@ -123,18 +128,18 @@ private def successful_redirect_path if is_referrer_check_answers? - page_ids = @lettings_log.form.subsection_for_page(@page).pages.map(&:id) + page_ids = @log.form.subsection_for_page(@page).pages.map(&:id) page_index = page_ids.index(@page.id) - next_page = @lettings_log.form.next_page(@page, @lettings_log, current_user) - previous_page = @lettings_log.form.previous_page(page_ids, page_index, @lettings_log, current_user) + next_page = @log.form.next_page(@page, @log, current_user) + previous_page = @log.form.previous_page(page_ids, page_index, @log, current_user) if next_page.to_s.include?("value_check") || next_page == previous_page - return "/logs/#{@lettings_log.id}/#{next_page.dasherize}?referrer=check_answers" + return send("#{@log.class.name.underscore}_#{next_page}_path", @log, { referrer: "check_answers" }) else - return send("lettings_log_#{@lettings_log.form.subsection_for_page(@page).id}_check_answers_path", @lettings_log) + return send("#{@log.model_name.param_key}_#{@log.form.subsection_for_page(@page).id}_check_answers_path", @log) end end - redirect_path = @lettings_log.form.next_page_redirect_path(@page, @lettings_log, current_user) - send(redirect_path, @lettings_log) + redirect_path = @log.form.next_page_redirect_path(@page, @log, current_user) + send(redirect_path, @log) end def mandatory_questions_with_no_response(responses_for_page) @@ -148,12 +153,12 @@ private end def question_is_required?(question) - LettingsLog::OPTIONAL_FIELDS.exclude?(question.id) && required_questions.include?(question.id) + @log.class::OPTIONAL_FIELDS.exclude?(question.id) && required_questions.include?(question.id) end def required_questions @required_questions ||= begin - log = @lettings_log + log = @log log.assign_attributes(responses_for_page(@page)) @page.subsection.applicable_questions(log).select { |q| q.enabled?(log) }.map(&:id) end @@ -162,12 +167,12 @@ private def question_missing_response?(responses_for_page, question) if %w[checkbox validation_override].include?(question.type) answered = question.answer_options.keys.reject { |x| x.match(/divider/) }.map do |option| - session["fields"][option] = @lettings_log[option] = params["lettings_log"][question.id].include?(option) ? 1 : 0 - params["lettings_log"][question.id].exclude?(option) + session["fields"][option] = @log[option] = params[@log.model_name.param_key][question.id].include?(option) ? 1 : 0 + params[@log.model_name.param_key][question.id].exclude?(option) end answered.all? else - session["fields"][question.id] = @lettings_log[question.id] = responses_for_page[question.id] + session["fields"][question.id] = @log[question.id] = responses_for_page[question.id] responses_for_page[question.id].nil? || responses_for_page[question.id].blank? end end diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index 651dd7225..eaec5a604 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -1,11 +1,4 @@ -class LettingsLogsController < ApplicationController - include Pagy::Backend - include Modules::LettingsLogsFilter - include Modules::SearchFilter - - skip_before_action :verify_authenticity_token, if: :json_api_request? - before_action :authenticate, if: :json_api_request? - before_action :authenticate_user!, unless: :json_api_request? +class LettingsLogsController < LogsController before_action :find_resource, except: %i[create index edit] before_action :session_filters, if: :current_user before_action :set_session_filters, if: :current_user @@ -14,39 +7,27 @@ class LettingsLogsController < ApplicationController respond_to do |format| format.html do all_logs = current_user.lettings_logs - unpaginated_filtered_logs = filtered_lettings_logs(all_logs, search_term, @session_filters) + unpaginated_filtered_logs = filtered_logs(all_logs, search_term, @session_filters) @search_term = search_term - @pagy, @lettings_logs = pagy(unpaginated_filtered_logs) + @pagy, @logs = pagy(unpaginated_filtered_logs) @searched = search_term.presence @total_count = all_logs.size + render "logs/index" end end end def create - lettings_log = LettingsLog.new(lettings_log_params) - respond_to do |format| - format.html do - lettings_log.save! - redirect_to lettings_log_url(lettings_log) - end - format.json do - if lettings_log.save - render json: lettings_log, status: :created - else - render json: { errors: lettings_log.errors.messages }, status: :unprocessable_entity - end - end - end + super { LettingsLog.new(log_params) } end def update - if @lettings_log - if @lettings_log.update(api_lettings_log_params) - render json: @lettings_log, status: :ok + if @log + if @log.update(api_log_params) + render json: @log, status: :ok else - render json: { errors: @lettings_log.errors.messages }, status: :unprocessable_entity + render json: { errors: @log.errors.messages }, status: :unprocessable_entity end else render_not_found_json("Log", params[:id]) @@ -58,8 +39,8 @@ class LettingsLogsController < ApplicationController # We don't have a dedicated non-editable show view format.html { edit } format.json do - if @lettings_log - render json: @lettings_log, status: :ok + if @log + render json: @log, status: :ok else render_not_found_json("Log", params[:id]) end @@ -68,20 +49,20 @@ class LettingsLogsController < ApplicationController end def edit - @lettings_log = current_user.lettings_logs.find_by(id: params[:id]) - if @lettings_log - render :edit, locals: { current_user: } + @log = current_user.lettings_logs.find_by(id: params[:id]) + if @log + render "logs/edit", locals: { current_user: } else render_not_found end end def destroy - if @lettings_log - if @lettings_log.delete + if @log + if @log.delete head :no_content else - render json: { errors: @lettings_log.errors.messages }, status: :unprocessable_entity + render json: { errors: @log.errors.messages }, status: :unprocessable_entity end else render_not_found_json("Log", params[:id]) @@ -89,7 +70,7 @@ class LettingsLogsController < ApplicationController end def download_csv - unpaginated_filtered_logs = filtered_lettings_logs(current_user.lettings_logs, search_term, @session_filters) + unpaginated_filtered_logs = filtered_logs(current_user.lettings_logs, search_term, @session_filters) render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path } end @@ -104,46 +85,15 @@ class LettingsLogsController < ApplicationController private - API_ACTIONS = %w[create show update destroy].freeze - - def search_term - params["search"] - end - - def json_api_request? - API_ACTIONS.include?(request["action"]) && request.format.json? + def permitted_log_params + params.require(:lettings_log).permit(LettingsLog.editable_fields) end - def authenticate - http_basic_authenticate_or_request_with name: ENV["API_USER"], password: ENV["API_KEY"] - end - - def lettings_log_params - if current_user && !current_user.support? - org_params.merge(api_lettings_log_params) - else - api_lettings_log_params - end - end - - def org_params - { - "owning_organisation_id" => current_user.organisation.id, - "managing_organisation_id" => current_user.organisation.id, - "created_by_id" => current_user.id, - } - end - - def api_lettings_log_params - return {} unless params[:lettings_log] - - permitted = params.require(:lettings_log).permit(LettingsLog.editable_fields) - owning_id = permitted["owning_organisation_id"] - permitted["owning_organisation"] = Organisation.find(owning_id) if owning_id - permitted + def find_resource + @log = LettingsLog.find_by(id: params[:id]) end - def find_resource - @lettings_log = LettingsLog.find_by(id: params[:id]) + def post_create_redirect_url(log) + lettings_log_url(log) end end diff --git a/app/controllers/logs_controller.rb b/app/controllers/logs_controller.rb new file mode 100644 index 000000000..3982d4a54 --- /dev/null +++ b/app/controllers/logs_controller.rb @@ -0,0 +1,73 @@ +class LogsController < ApplicationController + include Pagy::Backend + include Modules::LogsFilter + include Modules::SearchFilter + + skip_before_action :verify_authenticity_token, if: :json_api_request? + before_action :authenticate, if: :json_api_request? + before_action :authenticate_user!, unless: :json_api_request? + +private + + def create + log = yield + raise "Caller must pass a block that implements model creation" if log.blank? + + respond_to do |format| + format.html do + log.save! + redirect_to post_create_redirect_url(log) + end + format.json do + if log.save + render json: log, status: :created + else + render json: { errors: log.errors.messages }, status: :unprocessable_entity + end + end + end + end + + def post_create_redirect_url + raise "implement in sub class" + end + + API_ACTIONS = %w[create show update destroy].freeze + + def json_api_request? + API_ACTIONS.include?(request["action"]) && request.format.json? + end + + def authenticate + http_basic_authenticate_or_request_with name: ENV["API_USER"], password: ENV["API_KEY"] + end + + def log_params + if current_user && !current_user.support? + org_params.merge(api_log_params) + else + api_log_params + end + end + + def api_log_params + return {} unless params[:lettings_log] || params[:sales_log] + + permitted = permitted_log_params + owning_id = permitted["owning_organisation_id"] + permitted["owning_organisation"] = Organisation.find(owning_id) if owning_id + permitted + end + + def org_params + { + "owning_organisation_id" => current_user.organisation.id, + "managing_organisation_id" => current_user.organisation.id, + "created_by_id" => current_user.id, + } + end + + def search_term + params["search"] + end +end diff --git a/app/controllers/modules/lettings_logs_filter.rb b/app/controllers/modules/lettings_logs_filter.rb deleted file mode 100644 index 200a0db41..000000000 --- a/app/controllers/modules/lettings_logs_filter.rb +++ /dev/null @@ -1,21 +0,0 @@ -module Modules::LettingsLogsFilter - def filtered_lettings_logs(logs, search_term, filters) - all_orgs = params["organisation_select"] == "all" - FilterService.filter_lettings_logs(logs, search_term, filters, all_orgs, current_user) - end - - def load_session_filters(specific_org: false) - current_filters = session[:lettings_logs_filters] - new_filters = current_filters.present? ? JSON.parse(current_filters) : {} - current_user.lettings_logs_filters(specific_org:).each { |filter| new_filters[filter] = params[filter] if params[filter].present? } - params["organisation_select"] == "all" ? new_filters.except("organisation") : new_filters - end - - def session_filters(specific_org: false) - @session_filters ||= load_session_filters(specific_org:) - end - - def set_session_filters - session[:lettings_logs_filters] = @session_filters.to_json - end -end diff --git a/app/controllers/modules/logs_filter.rb b/app/controllers/modules/logs_filter.rb new file mode 100644 index 000000000..7c60bb027 --- /dev/null +++ b/app/controllers/modules/logs_filter.rb @@ -0,0 +1,21 @@ +module Modules::LogsFilter + def filtered_logs(logs, search_term, filters) + all_orgs = params["organisation_select"] == "all" + FilterService.filter_logs(logs, search_term, filters, all_orgs, current_user) + end + + def load_session_filters(specific_org: false) + current_filters = session[:logs_filters] + new_filters = current_filters.present? ? JSON.parse(current_filters) : {} + current_user.logs_filters(specific_org:).each { |filter| new_filters[filter] = params[filter] if params[filter].present? } + params["organisation_select"] == "all" ? new_filters.except("organisation") : new_filters + end + + def session_filters(specific_org: false) + @session_filters ||= load_session_filters(specific_org:) + end + + def set_session_filters + session[:logs_filters] = @session_filters.to_json + end +end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 3a979b78a..7f2094926 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -1,6 +1,6 @@ class OrganisationsController < ApplicationController include Pagy::Backend - include Modules::LettingsLogsFilter + include Modules::LogsFilter include Modules::SearchFilter before_action :authenticate_user! @@ -89,14 +89,14 @@ class OrganisationsController < ApplicationController end end - def logs - organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) - unpaginated_filtered_logs = filtered_lettings_logs(organisation_logs, search_term, @session_filters) + def lettings_logs + organisation_logs = LettingsLog.where(owning_organisation_id: @organisation.id) + unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) respond_to do |format| format.html do @search_term = search_term - @pagy, @lettings_logs = pagy(unpaginated_filtered_logs) + @pagy, @logs = pagy(unpaginated_filtered_logs) @searched = search_term.presence @total_count = organisation_logs.size render "logs", layout: "application" @@ -106,9 +106,9 @@ class OrganisationsController < ApplicationController def download_csv organisation_logs = LettingsLog.all.where(owning_organisation_id: @organisation.id) - unpaginated_filtered_logs = filtered_lettings_logs(organisation_logs, search_term, @session_filters) + unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) - render "lettings_logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path } + render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path } end def email_csv @@ -116,6 +116,25 @@ class OrganisationsController < ApplicationController redirect_to logs_csv_confirmation_organisation_path end + def sales_logs + organisation_logs = SalesLog.where(owning_organisation_id: @organisation.id) + unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters) + + respond_to do |format| + format.html do + @search_term = search_term + @pagy, @logs = pagy(unpaginated_filtered_logs) + @searched = search_term.presence + @total_count = organisation_logs.size + render "logs", layout: "application" + end + + format.csv do + send_data byte_order_mark + unpaginated_filtered_logs.to_csv, filename: "sales-logs-#{@organisation.name}-#{Time.zone.now}.csv" + end + end + end + private def org_params @@ -127,7 +146,7 @@ private end def authenticate_scope! - if %w[create new logs download_csv email_csv].include? action_name + if %w[create new lettings_logs download_csv email_csv].include? action_name head :unauthorized and return unless current_user.support? elsif current_user.organisation != @organisation && !current_user.support? render_not_found diff --git a/app/controllers/sales_logs_controller.rb b/app/controllers/sales_logs_controller.rb new file mode 100644 index 000000000..8a6c9937f --- /dev/null +++ b/app/controllers/sales_logs_controller.rb @@ -0,0 +1,46 @@ +class SalesLogsController < LogsController + before_action :session_filters, if: :current_user + before_action :set_session_filters, if: :current_user + + def create + super { SalesLog.new(log_params) } + end + + def index + respond_to do |format| + format.html do + all_logs = current_user.sales_logs + unpaginated_filtered_logs = filtered_logs(all_logs, search_term, @session_filters) + + @search_term = search_term + @pagy, @logs = pagy(unpaginated_filtered_logs) + @searched = search_term.presence + @total_count = all_logs.size + render "logs/index" + end + end + end + + def show + respond_to do |format| + format.html { edit } + end + end + + def edit + @log = current_user.sales_logs.find_by(id: params[:id]) + if @log + render "logs/edit", locals: { current_user: } + else + render_not_found + end + end + + def post_create_redirect_url(log) + sales_log_url(log) + end + + def permitted_log_params + params.require(:sales_log).permit(SalesLog.editable_fields) + end +end diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index ebb4bb034..67f582c1c 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -28,6 +28,10 @@ module CheckAnswersHelper subsection.applicable_questions(lettings_log).map(&:check_answers_card_number).compact.length.positive? end + def next_incomplete_section_path(log, redirect_path) + "#{log.class.name.underscore}_#{redirect_path.underscore.tr('/', '_')}_path" + end + private def answered_questions_count(subsection, lettings_log, current_user) diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index a8f5582a7..507274be0 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -1,8 +1,8 @@ module FiltersHelper def filter_selected?(filter, value) - return false unless session[:lettings_logs_filters] + return false unless session[:logs_filters] - selected_filters = JSON.parse(session[:lettings_logs_filters]) + selected_filters = JSON.parse(session[:logs_filters]) return true if selected_filters.blank? && filter == "user" && value == :all return true if !selected_filters.key?("organisation") && filter == "organisation_select" && value == :all return true if selected_filters["organisation"].present? && filter == "organisation_select" && value == :specific_org @@ -18,8 +18,8 @@ module FiltersHelper end def selected_option(filter) - return false unless session[:lettings_logs_filters] + return false unless session[:logs_filters] - JSON.parse(session[:lettings_logs_filters])[filter] || "" + JSON.parse(session[:logs_filters])[filter] || "" end end diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index 244aaf1ab..261637484 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -6,39 +6,44 @@ module NavigationItemsHelper [ NavigationItem.new("Organisations", organisations_path, organisations_current?(path)), NavigationItem.new("Users", "/users", users_current?(path)), - NavigationItem.new("Logs", lettings_logs_path, logs_current?(path)), + NavigationItem.new("Lettings Logs", lettings_logs_path, lettings_logs_current?(path)), + FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil, NavigationItem.new("Schemes", "/schemes", supported_housing_schemes_current?(path)), - ] + ].compact elsif current_user.data_coordinator? && current_user.organisation.holds_own_stock? [ - NavigationItem.new("Logs", lettings_logs_path, logs_current?(path)), + NavigationItem.new("Lettings Logs", lettings_logs_path, lettings_logs_current?(path)), + FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil, NavigationItem.new("Schemes", "/schemes", subnav_supported_housing_schemes_path?(path)), NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)), NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)), - ] + ].compact else [ - NavigationItem.new("Logs", lettings_logs_path, logs_current?(path)), + NavigationItem.new("Lettings Logs", lettings_logs_path, lettings_logs_current?(path)), + FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)) : nil, NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)), NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)), - ] + ].compact end end def secondary_items(path, current_organisation_id) if current_user.organisation.holds_own_stock? [ - NavigationItem.new("Logs", "/organisations/#{current_organisation_id}/logs", subnav_logs_path?(path)), + NavigationItem.new("Lettings Logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_logs_path?(path)), + FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(path)) : nil, NavigationItem.new("Schemes", "/organisations/#{current_organisation_id}/schemes", subnav_supported_housing_schemes_path?(path)), NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)), NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)), - ] + ].compact else [ - NavigationItem.new("Logs", "/organisations/#{current_organisation_id}/logs", subnav_logs_path?(path)), + NavigationItem.new("Lettings Logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_logs_path?(path)), + FeatureToggle.sales_log_enabled? ? NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(path)) : nil, NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)), NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)), - ] + ].compact end end @@ -51,8 +56,12 @@ module NavigationItemsHelper private - def logs_current?(path) - path == "/logs" + def lettings_logs_current?(path) + path == "/lettings-logs" + end + + def sales_logs_current?(path) + path == "/sales-logs" end def users_current?(path) @@ -76,7 +85,7 @@ private end def subnav_logs_path?(path) - path.include?("/organisations") && path.include?("/logs") + path.include?("/organisations") && path.include?("/lettings-logs") end def subnav_details_path?(path) diff --git a/app/helpers/tasklist_helper.rb b/app/helpers/tasklist_helper.rb index e4308d97e..7ab955a2e 100644 --- a/app/helpers/tasklist_helper.rb +++ b/app/helpers/tasklist_helper.rb @@ -1,36 +1,36 @@ module TasklistHelper include GovukLinkHelper - def get_next_incomplete_section(lettings_log) - lettings_log.form.subsections.find { |subsection| subsection.is_incomplete?(lettings_log) } + def get_next_incomplete_section(log) + log.form.subsections.find { |subsection| subsection.is_incomplete?(log) } end - def get_subsections_count(lettings_log, status = :all) - return lettings_log.form.subsections.count { |subsection| subsection.applicable_questions(lettings_log).count.positive? } if status == :all + def get_subsections_count(log, status = :all) + return log.form.subsections.count { |subsection| subsection.applicable_questions(log).count.positive? } if status == :all - lettings_log.form.subsections.count { |subsection| subsection.status(lettings_log) == status && subsection.applicable_questions(lettings_log).count.positive? } + log.form.subsections.count { |subsection| subsection.status(log) == status && subsection.applicable_questions(log).count.positive? } end - def next_page_or_check_answers(subsection, lettings_log, current_user) - path = if subsection.is_started?(lettings_log) - "lettings_log_#{subsection.id}_check_answers_path" + def next_page_or_check_answers(subsection, log, current_user) + path = if subsection.is_started?(log) + "#{log.class.name.underscore}_#{subsection.id}_check_answers_path" else - "lettings_log_#{next_question_page(subsection, lettings_log, current_user)}_path" + "#{log.class.name.underscore}_#{next_question_page(subsection, log, current_user)}_path" end - send(path, lettings_log) + send(path, log) end - def next_question_page(subsection, lettings_log, current_user) - if subsection.pages.first.routed_to?(lettings_log, current_user) + def next_question_page(subsection, log, current_user) + if subsection.pages.first.routed_to?(log, current_user) subsection.pages.first.id else - lettings_log.form.next_page(subsection.pages.first, lettings_log, current_user) + log.form.next_page(subsection.pages.first, log, current_user) end end - def subsection_link(subsection, lettings_log, current_user) - if subsection.status(lettings_log) != :cannot_start_yet - next_page_path = next_page_or_check_answers(subsection, lettings_log, current_user).to_s + def subsection_link(subsection, log, current_user) + if subsection.status(log) != :cannot_start_yet + next_page_path = next_page_or_check_answers(subsection, log, current_user).to_s govuk_link_to(subsection.label, next_page_path.dasherize, aria: { describedby: subsection.id.dasherize }) else subsection.label diff --git a/app/jobs/email_csv_job.rb b/app/jobs/email_csv_job.rb index 033ead7ae..4bc7cc3c3 100644 --- a/app/jobs/email_csv_job.rb +++ b/app/jobs/email_csv_job.rb @@ -7,7 +7,7 @@ class EmailCsvJob < ApplicationJob def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params unfiltered_logs = organisation.present? && user.support? ? LettingsLog.where(owning_organisation_id: organisation.id) : user.lettings_logs - filtered_logs = FilterService.filter_lettings_logs(unfiltered_logs, search_term, filters, all_orgs, user) + filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user) filename = organisation.present? ? "logs-#{organisation.name}-#{Time.zone.now}.csv" : "logs-#{Time.zone.now}.csv" diff --git a/app/models/form.rb b/app/models/form.rb index 6fa92bba0..fd8e66317 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -5,20 +5,39 @@ class Form include Form::Setup - def initialize(form_path, name) - raise "No form definition file exists for given year".freeze unless File.exist?(form_path) - - @name = name - @setup_sections = [Form::Setup::Sections::Setup.new(nil, nil, self)] - @form_definition = JSON.parse(File.open(form_path).read) - @form_sections = form_definition["sections"].map { |id, s| Form::Section.new(id, s, self) } - @type = form_definition["form_type"] - @sections = setup_sections + form_sections - @subsections = sections.flat_map(&:subsections) - @pages = subsections.flat_map(&:pages) - @questions = pages.flat_map(&:questions) - @start_date = Time.iso8601(form_definition["start_date"]) - @end_date = Time.iso8601(form_definition["end_date"]) + def initialize(form_path, name, sections_in_form = [], type = "lettings") + if type == "sales" + @name = name + @setup_sections = [Form::Sales::Setup::Sections::Setup.new(nil, nil, self)] + @form_sections = sections_in_form.map { |sec| sec.new(nil, nil, self) } + @type = "sales" + @sections = setup_sections + form_sections + @subsections = sections.flat_map(&:subsections) + @pages = subsections.flat_map(&:pages) + @questions = pages.flat_map(&:questions) + @start_date = Time.zone.local(name[0..3], 4, 1) + @end_date = Time.zone.local(start_date.year + 1, 7, 1) + @form_definition = { + "form_type" => type, + "start_date" => start_date, + "end_date" => end_date, + "sections" => sections, + } + else + raise "No form definition file exists for given year".freeze unless File.exist?(form_path) + + @name = name + @setup_sections = [Form::Setup::Sections::Setup.new(nil, nil, self)] + @form_definition = JSON.parse(File.open(form_path).read) + @form_sections = form_definition["sections"].map { |id, s| Form::Section.new(id, s, self) } + @type = form_definition["form_type"] + @sections = setup_sections + form_sections + @subsections = sections.flat_map(&:subsections) + @pages = subsections.flat_map(&:pages) + @questions = pages.flat_map(&:questions) + @start_date = Time.iso8601(form_definition["start_date"]) + @end_date = Time.iso8601(form_definition["end_date"]) + end end def get_subsection(id) @@ -29,9 +48,9 @@ class Form pages.find { |p| p.id == id.to_s.underscore } end - def get_question(id, lettings_log, current_user = nil) + def get_question(id, log, current_user = nil) all_questions = questions.select { |q| q.id == id.to_s.underscore } - routed_question = all_questions.find { |q| q.page.routed_to?(lettings_log, current_user) } if lettings_log + routed_question = all_questions.find { |q| q.page.routed_to?(log, current_user) } if log routed_question || all_questions[0] end @@ -39,47 +58,51 @@ class Form subsections.find { |s| s.pages.find { |p| p.id == page.id } } end - def next_page(page, lettings_log, current_user) + def next_page(page, log, current_user) page_ids = subsection_for_page(page).pages.map(&:id) page_index = page_ids.index(page.id) - page_id = if page.id.include?("value_check") && lettings_log[page.questions[0].id] == 1 && page.routed_to?(lettings_log, current_user) - previous_page(page_ids, page_index, lettings_log, current_user) + page_id = if page.id.include?("value_check") && log[page.questions[0].id] == 1 && page.routed_to?(log, current_user) + previous_page(page_ids, page_index, log, current_user) else page_ids[page_index + 1] end nxt_page = get_page(page_id) return :check_answers if nxt_page.nil? - return nxt_page.id if nxt_page.routed_to?(lettings_log, current_user) + return nxt_page.id if nxt_page.routed_to?(log, current_user) - next_page(nxt_page, lettings_log, current_user) + next_page(nxt_page, log, current_user) end - def next_page_redirect_path(page, lettings_log, current_user) - nxt_page = next_page(page, lettings_log, current_user) + def next_page_redirect_path(page, log, current_user) + nxt_page = next_page(page, log, current_user) if nxt_page == :check_answers - "lettings_log_#{subsection_for_page(page).id}_check_answers_path" + "#{type}_log_#{subsection_for_page(page).id}_check_answers_path" else - "lettings_log_#{nxt_page}_path" + "#{type}_log_#{nxt_page}_path" end end - def next_incomplete_section_redirect_path(subsection, lettings_log) + def cancel_path(page, log) + "#{log.class.name.underscore}_#{page.subsection.id}_check_answers_path" + end + + def next_incomplete_section_redirect_path(subsection, log) subsection_ids = subsections.map(&:id) - if lettings_log.status == "completed" + if log.status == "completed" return first_question_in_last_subsection(subsection_ids) end - next_subsection = next_subsection(subsection, lettings_log, subsection_ids) + next_subsection = next_subsection(subsection, log, subsection_ids) - case next_subsection.status(lettings_log) + case next_subsection.status(log) when :completed - next_incomplete_section_redirect_path(next_subsection, lettings_log) + next_incomplete_section_redirect_path(next_subsection, log) when :in_progress "#{next_subsection.id}/check_answers".dasherize when :not_started - first_question_in_subsection = next_subsection.pages.find { |page| page.routed_to?(lettings_log, nil) }.id + first_question_in_subsection = next_subsection.pages.find { |page| page.routed_to?(log, nil) }.id first_question_in_subsection.to_s.dasherize else "error" @@ -92,21 +115,21 @@ class Form first_question_in_subsection.to_s.dasherize end - def next_subsection(subsection, lettings_log, subsection_ids) + def next_subsection(subsection, log, subsection_ids) next_subsection_id_index = subsection_ids.index(subsection.id) + 1 next_subsection = get_subsection(subsection_ids[next_subsection_id_index]) - if subsection_ids[subsection_ids.length - 1] == subsection.id && lettings_log.status != "completed" + if subsection_ids[subsection_ids.length - 1] == subsection.id && log.status != "completed" next_subsection = get_subsection(subsection_ids[0]) end next_subsection end - def all_subsections_except_declaration_completed?(lettings_log) + def all_subsections_except_declaration_completed?(log) subsection_ids = subsections.map(&:id) subsection_ids.delete_at(subsection_ids.length - 1) - return true if subsection_ids.all? { |subsection_id| get_subsection(subsection_id).status(lettings_log) == :completed } + return true if subsection_ids.all? { |subsection_id| get_subsection(subsection_id).status(log) == :completed } false end @@ -118,26 +141,26 @@ class Form }.flatten end - def invalidated_pages(lettings_log, current_user = nil) - pages.reject { |p| p.routed_to?(lettings_log, current_user) } + def invalidated_pages(log, current_user = nil) + pages.reject { |p| p.routed_to?(log, current_user) } end - def invalidated_questions(lettings_log) - invalidated_page_questions(lettings_log) + invalidated_conditional_questions(lettings_log) + def invalidated_questions(log) + invalidated_page_questions(log) + invalidated_conditional_questions(log) end - def invalidated_page_questions(lettings_log, current_user = nil) - # we're already treating these fields as a special case and reset their values upon saving a lettings_log + def invalidated_page_questions(log, current_user = nil) + # we're already treating these fields as a special case and reset their values upon saving a log callback_questions = %w[postcode_known la ppcodenk previous_la_known prevloc postcode_full ppostcode_full location_id] - questions.reject { |q| q.page.routed_to?(lettings_log, current_user) || q.derived? || callback_questions.include?(q.id) } || [] + questions.reject { |q| q.page.routed_to?(log, current_user) || q.derived? || callback_questions.include?(q.id) } || [] end - def enabled_page_questions(lettings_log) - questions - invalidated_page_questions(lettings_log) + def enabled_page_questions(log) + questions - invalidated_page_questions(log) end - def invalidated_conditional_questions(lettings_log) - questions.reject { |q| q.enabled?(lettings_log) } || [] + def invalidated_conditional_questions(log) + questions.reject { |q| q.enabled?(log) } || [] end def readonly_questions @@ -148,18 +171,18 @@ class Form questions.select { |q| q.type == "numeric" } end - def previous_page(page_ids, page_index, lettings_log, current_user) + def previous_page(page_ids, page_index, log, current_user) prev_page = get_page(page_ids[page_index - 1]) - return prev_page.id if prev_page.routed_to?(lettings_log, current_user) + return prev_page.id if prev_page.routed_to?(log, current_user) - previous_page(page_ids, page_index - 1, lettings_log, current_user) + previous_page(page_ids, page_index - 1, log, current_user) end - def send_chain(arr, lettings_log) - Array(arr).inject(lettings_log) { |o, a| o.public_send(*a) } + def send_chain(arr, log) + Array(arr).inject(log) { |o, a| o.public_send(*a) } end - def depends_on_met(depends_on, lettings_log) + def depends_on_met(depends_on, log) return true unless depends_on depends_on.any? do |conditions_set| @@ -169,12 +192,12 @@ class Form if value.is_a?(Hash) && value.key?("operator") operator = value["operator"] operand = value["operand"] - lettings_log[question]&.send(operator, operand) + log[question]&.send(operator, operand) else parts = question.split(".") - lettings_log_value = send_chain(parts, lettings_log) + log_value = send_chain(parts, log) - value.nil? ? lettings_log_value == value : !lettings_log_value.nil? && lettings_log_value == value + value.nil? ? log_value == value : !log_value.nil? && log_value == value end end end diff --git a/app/models/form/question.rb b/app/models/form/question.rb index fc05efe03..8e5778000 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -121,8 +121,8 @@ class Form::Question end end - def action_href(lettings_log, page_id) - "/logs/#{lettings_log.id}/#{page_id.to_s.dasherize}?referrer=check_answers" + def action_href(log, page_id) + "/#{log.model_name.param_key.dasherize}s/#{log.id}/#{page_id.to_s.dasherize}?referrer=check_answers" end def completed?(lettings_log) diff --git a/app/models/form/sales/setup/pages/sale_date.rb b/app/models/form/sales/setup/pages/sale_date.rb new file mode 100644 index 000000000..4cdfe5a71 --- /dev/null +++ b/app/models/form/sales/setup/pages/sale_date.rb @@ -0,0 +1,15 @@ +class Form::Sales::Setup::Pages::SaleDate < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "sale_date" + @header = "" + @description = "" + @subsection = subsection + end + + def questions + @questions ||= [ + Form::Sales::Setup::Questions::SaleDate.new(nil, nil, self), + ] + end +end diff --git a/app/models/form/sales/setup/questions/sale_date.rb b/app/models/form/sales/setup/questions/sale_date.rb new file mode 100644 index 000000000..4f9088aaf --- /dev/null +++ b/app/models/form/sales/setup/questions/sale_date.rb @@ -0,0 +1,10 @@ +class Form::Sales::Setup::Questions::SaleDate < ::Form::Question + def initialize(id, hsh, page) + super + @id = "saledate" + @check_answer_label = "Sale completion date" + @header = "What is the sale completion date?" + @type = "date" + @page = page + end +end diff --git a/app/models/form/sales/setup/sections/setup.rb b/app/models/form/sales/setup/sections/setup.rb new file mode 100644 index 000000000..42835ec39 --- /dev/null +++ b/app/models/form/sales/setup/sections/setup.rb @@ -0,0 +1,10 @@ +class Form::Sales::Setup::Sections::Setup < ::Form::Section + def initialize(id, hsh, form) + super + @id = "setup" + @label = "Before you start" + @description = "" + @form = form + @subsections = [Form::Sales::Setup::Subsections::Setup.new(nil, nil, self)] || [] + end +end diff --git a/app/models/form/sales/setup/subsections/setup.rb b/app/models/form/sales/setup/subsections/setup.rb new file mode 100644 index 000000000..ee0af6350 --- /dev/null +++ b/app/models/form/sales/setup/subsections/setup.rb @@ -0,0 +1,14 @@ +class Form::Sales::Setup::Subsections::Setup < ::Form::Subsection + def initialize(id, hsh, section) + super + @id = "setup" + @label = "Set up this sales log" + @section = section + end + + def pages + @pages ||= [ + Form::Sales::Setup::Pages::SaleDate.new(nil, nil, self), + ] + end +end diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index 13efcb9ca..bfee3169f 100644 --- a/app/models/form_handler.rb +++ b/app/models/form_handler.rb @@ -14,9 +14,12 @@ class FormHandler forms[forms.keys.max_by(&:to_i)] end -private + def sales_forms + sales_sections = [] # Add section classes here e.g. Form::Sales::Property::Sections::PropertyInformation + { "2022_2023_sales" => Form.new(nil, "2022_2023_sales", sales_sections, "sales") } + end - def get_all_forms + def lettings_forms forms = {} directories.each do |directory| Dir.glob("#{directory}/*.json").each do |form_path| @@ -27,6 +30,12 @@ private forms end +private + + def get_all_forms + lettings_forms.merge(sales_forms) + end + def directories Rails.env.test? ? ["spec/fixtures/forms"] : ["config/forms"] end diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 142c48e52..2db5e2101 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -14,7 +14,7 @@ class LettingsLogValidator < ActiveModel::Validator end end -class LettingsLog < ApplicationRecord +class LettingsLog < Log include Validations::SoftValidations include DerivedVariables::LettingsLogVariables @@ -29,31 +29,11 @@ class LettingsLog < ApplicationRecord before_validation :reset_location_fields!, unless: :postcode_known? before_validation :reset_previous_location_fields!, unless: :previous_postcode_known? before_validation :set_derived_fields! - before_save :update_status! - belongs_to :owning_organisation, class_name: "Organisation", optional: true - belongs_to :managing_organisation, class_name: "Organisation", optional: true - belongs_to :created_by, class_name: "User", optional: true belongs_to :scheme, optional: true belongs_to :location, optional: true - scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } - scope :filter_by_status, ->(status, _user = nil) { where status: } - scope :filter_by_years, lambda { |years, _user = nil| - first_year = years.shift - query = filter_by_year(first_year) - years.each { |year| query = query.or(filter_by_year(year)) } - query.all - } scope :filter_by_year, ->(year) { where(startdate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } - - scope :filter_by_user, lambda { |selected_user, user| - if !selected_user.include?("all") && user.present? - where(created_by: user) - end - } - - scope :filter_by_id, ->(id) { where(id:) } scope :filter_by_tenant_code, ->(tenant_code) { where("tenancycode ILIKE ?", "%#{tenant_code}%") } scope :filter_by_propcode, ->(propcode) { where("propcode ILIKE ?", "%#{propcode}%") } scope :filter_by_postcode, ->(postcode_full) { where("REPLACE(postcode_full, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") } @@ -70,24 +50,14 @@ class LettingsLog < ApplicationRecord OPTIONAL_FIELDS = %w[first_time_property_let_as_social_housing tenancycode propcode].freeze RENT_TYPE_MAPPING_LABELS = { 1 => "Social Rent", 2 => "Affordable Rent", 3 => "Intermediate Rent" }.freeze HAS_BENEFITS_OPTIONS = [1, 6, 8, 7].freeze - STATUS = { "not_started" => 0, "in_progress" => 1, "completed" => 2 }.freeze NUM_OF_WEEKS_FROM_PERIOD = { 2 => 26, 3 => 13, 4 => 12, 5 => 50, 6 => 49, 7 => 48, 8 => 47, 9 => 46, 1 => 52 }.freeze SUFFIX_FROM_PERIOD = { 2 => "every 2 weeks", 3 => "every 4 weeks", 4 => "every month" }.freeze RETIREMENT_AGES = { "M" => 67, "F" => 60, "X" => 67 }.freeze - enum status: STATUS def form FormHandler.instance.get_form(form_name) || FormHandler.instance.forms.first.second end - def collection_start_year - return @start_year if @start_year - return unless startdate - - window_end_date = Time.zone.local(startdate.year, 4, 1) - @start_year = startdate < window_end_date ? startdate.year - 1 : startdate.year - end - def recalculate_start_year! @start_year = nil collection_start_year @@ -527,20 +497,14 @@ class LettingsLog < ApplicationRecord location.type_of_unit_before_type_cast if location end + def lettings? + true + end + private PIO = PostcodeService.new - def update_status! - self.status = if all_fields_completed? && errors.empty? - "completed" - elsif all_fields_nil? - "not_started" - else - "in_progress" - end - end - def reset_not_routed_questions enabled_questions = form.enabled_page_questions(self) enabled_question_ids = enabled_questions.map(&:id) @@ -692,17 +656,6 @@ private end end - def all_fields_completed? - subsection_statuses = form.subsections.map { |subsection| subsection.status(self) }.uniq - subsection_statuses == [:completed] - end - - def all_fields_nil? - not_started_statuses = %i[not_started cannot_start_yet] - subsection_statuses = form.subsections.map { |subsection| subsection.status(self) }.uniq - subsection_statuses.all? { |status| not_started_statuses.include?(status) } - end - def age_refused? [age1_known, age2_known, age3_known, age4_known, age5_known, age6_known, age7_known, age8_known].any?(1) end diff --git a/app/models/log.rb b/app/models/log.rb new file mode 100644 index 000000000..d0a866ae9 --- /dev/null +++ b/app/models/log.rb @@ -0,0 +1,61 @@ +class Log < ApplicationRecord + self.abstract_class = true + + belongs_to :owning_organisation, class_name: "Organisation", optional: true + belongs_to :managing_organisation, class_name: "Organisation", optional: true + belongs_to :created_by, class_name: "User", optional: true + before_save :update_status! + + STATUS = { "not_started" => 0, "in_progress" => 1, "completed" => 2 }.freeze + enum status: STATUS + + scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } + scope :filter_by_status, ->(status, _user = nil) { where status: } + scope :filter_by_years, lambda { |years, _user = nil| + first_year = years.shift + query = filter_by_year(first_year) + years.each { |year| query = query.or(filter_by_year(year)) } + query.all + } + scope :filter_by_id, ->(id) { where(id:) } + scope :filter_by_user, lambda { |selected_user, user| + if !selected_user.include?("all") && user.present? + where(created_by: user) + end + } + + def collection_start_year + return @start_year if @start_year + return unless startdate + + window_end_date = Time.zone.local(startdate.year, 4, 1) + @start_year = startdate < window_end_date ? startdate.year - 1 : startdate.year + end + + def lettings? + false + end + +private + + def update_status! + self.status = if all_fields_completed? && errors.empty? + "completed" + elsif all_fields_nil? + "not_started" + else + "in_progress" + end + end + + def all_fields_completed? + subsection_statuses = form.subsections.map { |subsection| subsection.status(self) }.uniq + subsection_statuses == [:completed] + end + + def all_fields_nil? + not_started_statuses = %i[not_started cannot_start_yet] + subsection_statuses = form.subsections.map { |subsection| subsection.status(self) }.uniq + subsection_statuses.all? { |status| not_started_statuses.include?(status) } + end +end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index afaf6de35..9eb039ab1 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -2,6 +2,8 @@ class Organisation < ApplicationRecord has_many :users, dependent: :delete_all has_many :owned_lettings_logs, class_name: "LettingsLog", foreign_key: "owning_organisation_id", dependent: :delete_all has_many :managed_lettings_logs, class_name: "LettingsLog", foreign_key: "managing_organisation_id" + has_many :owned_sales_logs, class_name: "SalesLog", foreign_key: "owning_organisation_id", dependent: :delete_all + has_many :managed_sales_logs, class_name: "SalesLog", foreign_key: "managing_organisation_id" has_many :data_protection_confirmations has_many :organisation_rent_periods has_many :owned_schemes, class_name: "Scheme", foreign_key: "owning_organisation_id", dependent: :delete_all @@ -32,6 +34,10 @@ class Organisation < ApplicationRecord LettingsLog.filter_by_organisation(self) end + def sales_logs + SalesLog.filter_by_organisation(self) + end + def completed_lettings_logs lettings_logs.completed end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb new file mode 100644 index 000000000..f73b518e5 --- /dev/null +++ b/app/models/sales_log.rb @@ -0,0 +1,44 @@ +class SalesLogValidator < ActiveModel::Validator + def validate(record); end +end + +class SalesLog < Log + has_paper_trail + + validates_with SalesLogValidator + + scope :filter_by_year, ->(year) { where(saledate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } + scope :search_by, ->(param) { filter_by_id(param) } + + OPTIONAL_FIELDS = [].freeze + + def startdate + saledate + end + + def self.editable_fields + attribute_names + end + + def form_name + return unless saledate + + "#{collection_start_year}_#{collection_start_year + 1}_sales" + end + + def form + FormHandler.instance.get_form(form_name) || FormHandler.instance.get_form("2022_2023_sales") + end + + def optional_fields + [] + end + + def not_started? + status == "not_started" + end + + def completed? + status == "completed" + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 22cc14b77..56dd2821f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,8 +7,10 @@ class User < ApplicationRecord # Marked as optional because we validate organisation_id below instead so that # the error message is linked to the right field on the form belongs_to :organisation, optional: true - has_many :owned_lettings_logs, through: :organisation, dependent: :delete_all + has_many :owned_lettings_logs, through: :organisation has_many :managed_lettings_logs, through: :organisation + has_many :owned_sales_logs, through: :organisation + has_many :managed_sales_logs, through: :organisation validates :name, presence: true validates :email, presence: true @@ -58,6 +60,14 @@ class User < ApplicationRecord end end + def sales_logs + if support? + SalesLog.all + else + SalesLog.filter_by_organisation(organisation) + end + end + def completed_lettings_logs lettings_logs.completed end @@ -131,7 +141,7 @@ class User < ApplicationRecord ROLES.except(:support) end - def lettings_logs_filters(specific_org: false) + def logs_filters(specific_org: false) if support? && !specific_org %w[status years user organisation] else diff --git a/app/services/filter_service.rb b/app/services/filter_service.rb index 0986e7aca..8fe1030de 100644 --- a/app/services/filter_service.rb +++ b/app/services/filter_service.rb @@ -7,7 +7,7 @@ class FilterService end end - def self.filter_lettings_logs(logs, search_term, filters, all_orgs, user) + def self.filter_logs(logs, search_term, filters, all_orgs, user) logs = filter_by_search(logs, search_term) filters.each do |category, values| diff --git a/app/views/form/_check_answers_summary_list.html.erb b/app/views/form/_check_answers_summary_list.html.erb index 85c29beca..68a9cf00a 100644 --- a/app/views/form/_check_answers_summary_list.html.erb +++ b/app/views/form/_check_answers_summary_list.html.erb @@ -1,21 +1,21 @@ <%= govuk_summary_list do |summary_list| %> - <% total_applicable_questions(subsection, @lettings_log, current_user).each do |question| %> + <% total_applicable_questions(subsection, @log, current_user).each do |question| %> <% summary_list.row do |row| %> <% row.key { question.check_answer_label.to_s.presence || question.header.to_s } %> <% row.value do %> - <%= get_answer_label(question, @lettings_log) %> - <% extra_value = question.get_extra_check_answer_value(@lettings_log) %> + <%= get_answer_label(question, @log) %> + <% extra_value = question.get_extra_check_answer_value(@log) %> <% if extra_value %> <%= extra_value %> <% end %>
- <% question.get_inferred_answers(@lettings_log).each do |inferred_answer| %> + <% question.get_inferred_answers(@log).each do |inferred_answer| %> <%= inferred_answer %> <% end %> <% end %> <% row.action( - text: question.action_text(@lettings_log), - href: question.action_href(@lettings_log, question.page.id), + text: question.action_text(@log), + href: question.action_href(@log, question.page.id), visually_hidden_text: question.check_answer_label.to_s.downcase, ) %> <% end %> diff --git a/app/views/form/_checkbox_question.html.erb b/app/views/form/_checkbox_question.html.erb index 245056c39..c855e2d88 100644 --- a/app/views/form/_checkbox_question.html.erb +++ b/app/views/form/_checkbox_question.html.erb @@ -6,7 +6,7 @@ hint: { text: question.hint_text&.html_safe } do %> <% after_divider = false %> - <% question.displayed_answer_options(@lettings_log).map do |key, options| %> + <% question.displayed_answer_options(@log).map do |key, options| %> <% if key.starts_with?("divider") %> <% after_divider = true %> <%= f.govuk_check_box_divider %> @@ -14,7 +14,7 @@ <%= f.govuk_check_box question.id, key, label: { text: options["value"] }, hint: { text: options["hint"] }, - checked: @lettings_log[key] == 1, + checked: @log[key] == 1, exclusive: after_divider, **stimulus_html_attributes(question) %> <% end %> diff --git a/app/views/form/_numeric_question.html.erb b/app/views/form/_numeric_question.html.erb index fc6aa274a..0aeb63801 100644 --- a/app/views/form/_numeric_question.html.erb +++ b/app/views/form/_numeric_question.html.erb @@ -8,7 +8,7 @@ width: question.width, readonly: question.read_only?, prefix_text: question.prefix.to_s, - suffix_text: question.suffix_label(@lettings_log), + suffix_text: question.suffix_label(@log), **stimulus_html_attributes(question) %> <%= render partial: "form/guidance/#{question.guidance_partial}" if question.bottom_guidance? %> diff --git a/app/views/form/_radio_question.html.erb b/app/views/form/_radio_question.html.erb index f153a838b..ed5e71a0b 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(@lettings_log).map do |key, options| %> + <% question.displayed_answer_options(@log).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 fbdb20e9a..63f50fb94 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 = @lettings_log.public_send(question.id) || "" %> -<% answers = question.displayed_answer_options(@lettings_log).map { |key, value| OpenStruct.new(id: key, name: value.respond_to?(:service_name) ? value.service_name : nil, resource: value) } %> +<% 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) } %> <%= f.govuk_select(question.id.to_sym, label: legend(question, page_header, conditional), "data-controller": "accessible-autocomplete", @@ -12,7 +12,7 @@ data-synonyms="<%= question.answer_option_synonyms(answer.resource) %>" data-append="<%= question.answer_option_append(answer.resource) %>" data-hint="<%= question.answer_option_hint(answer.resource) %>" - <%= question.answer_selected?(@lettings_log, answer) ? "selected" : "" %> + <%= question.answer_selected?(@log, answer) ? "selected" : "" %> <%= answer.id == "" ? "disabled" : "" %>><%= answer.name || answer.resource %> <% end %> <% end %> diff --git a/app/views/form/check_answers.html.erb b/app/views/form/check_answers.html.erb index a2bcb09bc..46a945017 100644 --- a/app/views/form/check_answers.html.erb +++ b/app/views/form/check_answers.html.erb @@ -1,9 +1,9 @@ <% content_for :title, "#{subsection.id.humanize} - Check your answers" %> <% content_for :breadcrumbs, govuk_breadcrumbs(breadcrumbs: { "Logs" => "/logs", - "Log #{@lettings_log.id}" => "/logs/#{@lettings_log.id}", + "Log #{@log.id}" => send("#{@log.class.name.underscore}_path", @log), subsection.label => "", -}) %> + }) %>

@@ -12,27 +12,27 @@ Check your answers - <% if subsection.id == "setup" && subsection.status(@lettings_log) == :completed %> + <% if subsection.id == "setup" && subsection.status(@log) == :completed %> <%= govuk_inset_text(text: "Changing these answers might remove answers you’ve already given in other sections.") %> <% end %> - <%= display_answered_questions_summary(subsection, @lettings_log, current_user) %> + <%= display_answered_questions_summary(subsection, @log, current_user) %> - <% if any_questions_have_summary_card_number?(subsection, @lettings_log) %> - <% subsection.applicable_questions(@lettings_log).group_by(&:check_answers_card_number).values.each do |question_group| %> - <%= render CheckAnswersSummaryListCardComponent.new(questions: question_group, lettings_log: @lettings_log, user: current_user) %> + <% if any_questions_have_summary_card_number?(subsection, @log) %> + <% subsection.applicable_questions(@log).group_by(&:check_answers_card_number).values.each do |question_group| %> + <%= render CheckAnswersSummaryListCardComponent.new(questions: question_group, log: @log, user: current_user) %> <% end %> <% else %> <%= render partial: "form/check_answers_summary_list", locals: { subsection:, - lettings_log: @lettings_log, + lettings_log: @log, } %> <% end %> - <%= form_with model: @lettings_log, method: "get" do |f| %> + <%= form_with model: @log, method: "get" do |f| %> <%= f.govuk_submit "Save and return to log" do %> - <% next_incomplete_section_redirect_path = @lettings_log.form.next_incomplete_section_redirect_path(subsection, @lettings_log) %> - <% if @lettings_log.status == "in_progress" && next_incomplete_section_redirect_path != "error" %> - <%= govuk_button_link_to "Save and go to next incomplete section", "/logs/#{@lettings_log.id}/#{next_incomplete_section_redirect_path}", secondary: true %> + <% next_incomplete_section_redirect_path = @log.form.next_incomplete_section_redirect_path(subsection, @log) %> + <% if @log.status == "in_progress" && next_incomplete_section_redirect_path != "error" %> + <%= govuk_button_link_to "Save and go to next incomplete section", send(next_incomplete_section_path(@log, next_incomplete_section_redirect_path), @log), secondary: true %> <% end %> <% end %> <% end %> diff --git a/app/views/form/page.html.erb b/app/views/form/page.html.erb index 0fe72f647..8d90f12b9 100644 --- a/app/views/form/page.html.erb +++ b/app/views/form/page.html.erb @@ -5,10 +5,10 @@ <% end %>
-<%= form_with model: @lettings_log, url: form_lettings_log_path(@lettings_log), method: "post", local: true do |f| %> +<%= form_with model: @log, url: form_lettings_log_path(@log), method: "post", local: true do |f| %>
"> - <% remove_other_page_errors(@lettings_log, @page) %> + <% remove_other_page_errors(@log, @page) %> <%= f.govuk_error_summary %> <% if @page.header.present? %> @@ -30,9 +30,9 @@ <%= govuk_section_break(visible: true, size: "m") %> <% end %> <% if question.type == "interruption_screen" %> - <%= render partial: "form/#{question.type}_question", locals: { question:, caption_text: @subsection.label, page_header: @page.header, lettings_log: @lettings_log, title_text: @page.title_text, informative_text: @page.informative_text, form: @form, f:, conditional: false } %> + <%= render partial: "form/#{question.type}_question", locals: { question:, caption_text: @subsection.label, page_header: @page.header, lettings_log: @log, title_text: @page.title_text, informative_text: @page.informative_text, form: @form, f:, conditional: false } %> <% else %> - <%= render partial: "form/#{question.type}_question", locals: { question:, caption_text: @subsection.label, page_header: @page.header, lettings_log: @lettings_log, f:, conditional: false } %> + <%= render partial: "form/#{question.type}_question", locals: { question:, caption_text: @subsection.label, page_header: @page.header, lettings_log: @log, f:, conditional: false } %> <% end %>
<% end %> @@ -42,10 +42,10 @@
<% if !@page.id.include?("value_check") && if request.query_parameters["referrer"] != "check_answers" %> <%= f.govuk_submit "Save and continue" %> - <%= govuk_link_to "Skip for now", send(@lettings_log.form.next_page_redirect_path(@page, @lettings_log, current_user), @lettings_log) %> + <%= govuk_link_to "Skip for now", send(@log.form.next_page_redirect_path(@page, @log, current_user), @log) %> <% else %> <%= f.govuk_submit "Save changes" %> - <%= govuk_link_to "Cancel", "/logs/#{@lettings_log.id}/setup/check-answers" %> + <%= govuk_link_to "Cancel", send(@log.form.cancel_path(@page, @log), @log) %> <% end %> <% end %>
diff --git a/app/views/form/review.html.erb b/app/views/form/review.html.erb index 55a693913..59e0bde40 100644 --- a/app/views/form/review.html.erb +++ b/app/views/form/review.html.erb @@ -1,7 +1,7 @@ <% content_for :title, "Review lettings log" %> <% content_for :breadcrumbs, govuk_breadcrumbs(breadcrumbs: { "Logs" => "/logs", - "Log #{@lettings_log.id}" => "/logs/#{@lettings_log.id}", + "Log #{@log.id}" => "/logs/#{@log.id}", "Review lettings log" => "", }) %> @@ -11,9 +11,9 @@ <%= content_for(:title) %>

- You can review and make changes to this log until 2nd June <%= @lettings_log.collection_start_year.present? ? @lettings_log.collection_start_year + 1 : "" %>. + You can review and make changes to this log until 2nd June <%= @log.collection_start_year.present? ? @log.collection_start_year + 1 : "" %>.

- <% @lettings_log.form.sections.map do |section| %> + <% @log.form.sections.map do |section| %>

<%= section.label %>

<% section.subsections.map do |subsection| %>
@@ -23,7 +23,7 @@
<%= render partial: "form/check_answers_summary_list", locals: { subsection:, - lettings_log: @lettings_log, + lettings_log: @log, } %>
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 5dc643e4e..f6111fce5 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -1,7 +1,7 @@ - <%= browser_title(yield(:title), @pagy, @admin_user, @user, @organisation, @lettings_log, @resource) %> + <%= browser_title(yield(:title), @pagy, @admin_user, @user, @organisation, @log, @resource) %> <%= csrf_meta_tags %> <%= csp_meta_tag %> <%= tag.meta name: "viewport", content: "width=device-width, initial-scale=1" %> @@ -34,7 +34,7 @@ <% end %> - <% if Rails.env.production? %> + <% if Rails.env.production? && ENV["APP_HOST"].present? %> <% end %> diff --git a/app/views/lettings_logs/_log_filters.erb b/app/views/logs/_log_filters.erb similarity index 95% rename from app/views/lettings_logs/_log_filters.erb rename to app/views/logs/_log_filters.erb index b0f3e07be..9defa0c84 100644 --- a/app/views/lettings_logs/_log_filters.erb +++ b/app/views/logs/_log_filters.erb @@ -10,7 +10,7 @@ <%= render partial: "filters/checkbox_filter", locals: { f: f, options: years, label: "Collection year", category: "years" } %> <%= render partial: "filters/checkbox_filter", locals: { f: f, options: status_filters, label: "Status", category: "status" } %> <%= render partial: "filters/radio_filter", locals: { f: f, options: all_or_yours, label: "Logs", category: "user", } %> - <% if @current_user.support? && request.path == "/logs" %> + <% if @current_user.support? && request.path == "/lettings-logs" %> <%= render partial: "filters/radio_filter", locals: { f: f, options: { diff --git a/app/views/lettings_logs/_log_list.html.erb b/app/views/logs/_log_list.html.erb similarity index 90% rename from app/views/lettings_logs/_log_list.html.erb rename to app/views/logs/_log_list.html.erb index c6ea5e9c3..702cd7d1f 100644 --- a/app/views/lettings_logs/_log_list.html.erb +++ b/app/views/logs/_log_list.html.erb @@ -2,7 +2,6 @@ <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "logs", path: request.path)) %> <%= govuk_link_to "Download (CSV)", csv_download_url, type: "text/csv" %> - -<% lettings_logs.map do |log| %> +<% logs.map do |log| %> <%= render(LogSummaryComponent.new(current_user:, log:)) %> <% end %> diff --git a/app/views/lettings_logs/_tasklist.html.erb b/app/views/logs/_tasklist.html.erb similarity index 68% rename from app/views/lettings_logs/_tasklist.html.erb rename to app/views/logs/_tasklist.html.erb index 03e712889..6c82eb377 100644 --- a/app/views/lettings_logs/_tasklist.html.erb +++ b/app/views/logs/_tasklist.html.erb @@ -1,5 +1,5 @@
    - <% @lettings_log.form.sections.map do |section| %> + <% @log.form.sections.map do |section| %>

  1. <%= section.label %> @@ -9,11 +9,11 @@ <% end %>
      <% section.subsections.map do |subsection| %> - <% if subsection.applicable_questions(@lettings_log).count > 0 || !subsection.enabled?(@lettings_log) %> - <% subsection_status = subsection.status(@lettings_log) %> + <% if subsection.applicable_questions(@log).count > 0 || !subsection.enabled?(@log) %> + <% subsection_status = subsection.status(@log) %>
    • - <%= subsection_link(subsection, @lettings_log, current_user) %> + <%= subsection_link(subsection, @log, current_user) %> <%= status_tag(subsection_status, "app-task-list__tag") %>
      • diff --git a/app/views/lettings_logs/bulk_upload.html.erb b/app/views/logs/bulk_upload.html.erb similarity index 100% rename from app/views/lettings_logs/bulk_upload.html.erb rename to app/views/logs/bulk_upload.html.erb diff --git a/app/views/lettings_logs/csv_confirmation.html.erb b/app/views/logs/csv_confirmation.html.erb similarity index 100% rename from app/views/lettings_logs/csv_confirmation.html.erb rename to app/views/logs/csv_confirmation.html.erb diff --git a/app/views/lettings_logs/download_csv.html.erb b/app/views/logs/download_csv.html.erb similarity index 100% rename from app/views/lettings_logs/download_csv.html.erb rename to app/views/logs/download_csv.html.erb diff --git a/app/views/lettings_logs/edit.html.erb b/app/views/logs/edit.html.erb similarity index 63% rename from app/views/lettings_logs/edit.html.erb rename to app/views/logs/edit.html.erb index acfbba053..52943e539 100644 --- a/app/views/lettings_logs/edit.html.erb +++ b/app/views/logs/edit.html.erb @@ -1,6 +1,6 @@ -<% content_for :title, "Log #{@lettings_log.id}" %> +<% content_for :title, "Log #{@log.id}" %> <% content_for :breadcrumbs, govuk_breadcrumbs(breadcrumbs: { - "Logs" => "/logs", + "Logs" => @log.lettings? ? lettings_logs_path : sales_logs_path, content_for(:title) => "", }) %> @@ -10,10 +10,10 @@ <%= content_for(:title) %>

    - <% if @lettings_log.status == "in_progress" %> -

    <%= get_subsections_count(@lettings_log, :completed) %> of <%= get_subsections_count(@lettings_log, :all) %> sections completed.

    + <% if @log.status == "in_progress" %> +

    <%= get_subsections_count(@log, :completed) %> of <%= get_subsections_count(@log, :all) %> sections completed.

    - <% next_incomplete_section = get_next_incomplete_section(@lettings_log) %> + <% next_incomplete_section = get_next_incomplete_section(@log) %>

    <% if next_incomplete_section.present? %> @@ -22,14 +22,14 @@ <% end %>

    - <% elsif @lettings_log.status == "not_started" %> + <% elsif @log.status == "not_started" %>

    This log has not been started.

    - <% elsif @lettings_log.status == "completed" %> + <% elsif @log.status == "completed" %>

    - <%= status_tag(@lettings_log.status) %> + <%= status_tag(@log.status) %>

    - You can <%= govuk_link_to "review and make changes to this log", "/logs/#{@lettings_log.id}/review" %> until 2nd June <%= @lettings_log.collection_start_year.present? ? @lettings_log.collection_start_year + 1 : "" %>. + You can <%= govuk_link_to "review and make changes to this log", "/lettings-logs/#{@log.id}/review" %> until 2nd June <%= @log.collection_start_year.present? ? @log.collection_start_year + 1 : "" %>.

    <% end %> <%= render "tasklist" %> diff --git a/app/views/lettings_logs/index.html.erb b/app/views/logs/index.html.erb similarity index 61% rename from app/views/lettings_logs/index.html.erb rename to app/views/logs/index.html.erb index fe1004e69..7c75230d4 100644 --- a/app/views/lettings_logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -7,7 +7,12 @@
    - <%= govuk_button_to "Create a new lettings log", lettings_logs_path %> + <% if current_page?(:controller => 'lettings_logs', :action => 'index') %> + <%= govuk_button_to "Create a new lettings log", lettings_logs_path %> + <% end %> + <% if FeatureToggle.sales_log_enabled? && current_page?(:controller => 'sales_logs', :action => 'index') %> + <%= govuk_button_to "Create a new sales log", sales_logs_path %> + <% end %> <%#= govuk_link_to "Upload logs", bulk_upload_lettings_logs_path %>
    @@ -15,7 +20,7 @@
    <%= render SearchComponent.new(current_user:, search_label: "Search by log ID, tenant code, property reference or postcode", value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "log_list", locals: { lettings_logs: @lettings_logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: csv_download_lettings_logs_path(search: @search_term) } %> + <%= render partial: "log_list", locals: { logs: @logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: csv_download_lettings_logs_path(search: @search_term) } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
    diff --git a/app/views/organisations/_organisation_list.html.erb b/app/views/organisations/_organisation_list.html.erb index 68b8f3233..389dff6b7 100644 --- a/app/views/organisations/_organisation_list.html.erb +++ b/app/views/organisations/_organisation_list.html.erb @@ -22,7 +22,7 @@ <% row.cell(header: true, html_attributes: { scope: "row", }) do %> - <%= govuk_link_to(organisation.name, "organisations/#{organisation.id}/logs") %> + <%= govuk_link_to(organisation.name, "organisations/#{organisation.id}/lettings-logs") %> <% end %> <% row.cell(text: organisation.housing_registration_no) %> <% row.cell(text: organisation.display_provider_type) %> diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb index 42c1697bc..3b03e55e4 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -17,11 +17,11 @@ <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post) %>
- <%= render partial: "lettings_logs/log_filters" %> + <%= render partial: "logs/log_filters" %>
<%= render SearchComponent.new(current_user:, search_label: "Search by log ID, tenant code, property reference or postcode", value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "lettings_logs/log_list", locals: { lettings_logs: @lettings_logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term) } %> + <%= render partial: "logs/log_list", locals: { logs: @logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term) } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index 50501fb65..0e01531ca 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -2,4 +2,10 @@ class FeatureToggle def self.startdate_two_week_validation_enabled? true end + + def self.sales_log_enabled? + return true unless Rails.env.production? + + false + end end diff --git a/config/routes.rb b/config/routes.rb index 82fec6493..0c4409713 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,6 +27,7 @@ Rails.application.routes.draw do root to: "start#index" + get "/logs", to: redirect("/lettings-logs") get "/accessibility-statement", to: "content#accessibility_statement" get "/privacy-notice", to: "content#privacy_notice" get "/data-sharing-agreement", to: "content#data_sharing_agreement" @@ -68,7 +69,8 @@ Rails.application.routes.draw do get "details", to: "organisations#details" get "users", to: "organisations#users" get "users/invite", to: "users/account#new" - get "logs", to: "organisations#logs" + get "lettings-logs", to: "organisations#lettings_logs" + get "sales-logs", to: "organisations#sales_logs" get "logs/csv-download", to: "organisations#download_csv" post "logs/email-csv", to: "organisations#email_csv" get "logs/csv-confirmation", to: "lettings_logs#csv_confirmation" @@ -76,7 +78,7 @@ Rails.application.routes.draw do end end - resources :lettings_logs, path: "/logs" do + resources :lettings_logs, path: "/lettings-logs" do collection do post "bulk-upload", to: "bulk_upload#bulk_upload" get "bulk-upload", to: "bulk_upload#show" @@ -90,9 +92,21 @@ Rails.application.routes.draw do get "review", to: "form#review" end - FormHandler.instance.forms.each do |_key, form| + FormHandler.instance.lettings_forms.each do |_key, form| form.pages.map do |page| - get page.id.to_s.dasherize, to: "form##{page.id}" + get page.id.to_s.dasherize, to: "form#show_page" + end + + form.subsections.map do |subsection| + get "#{subsection.id.to_s.dasherize}/check-answers", to: "form#check_answers" + end + end + end + + resources :sales_logs, path: "/sales-logs" do + FormHandler.instance.sales_forms.each do |_key, form| + form.pages.map do |page| + get page.id.to_s.dasherize, to: "form#show_page" end form.subsections.map do |subsection| diff --git a/db/migrate/20220826093411_add_sales_log.rb b/db/migrate/20220826093411_add_sales_log.rb new file mode 100644 index 000000000..8b14d7f5b --- /dev/null +++ b/db/migrate/20220826093411_add_sales_log.rb @@ -0,0 +1,12 @@ +class AddSalesLog < ActiveRecord::Migration[7.0] + def change + create_table :sales_logs do |t| + t.integer :status, default: 0 + t.datetime :saledate + t.timestamps + t.references :owning_organisation, class_name: "Organisation", foreign_key: { to_table: :organisations, on_delete: :cascade } + t.references :managing_organisation, class_name: "Organisation" + t.references :created_by, class_name: "User" + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 23db463fc..3d4db46c2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -313,6 +313,19 @@ ActiveRecord::Schema[7.0].define(version: 2022_09_02_082245) do t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end + create_table "sales_logs", force: :cascade do |t| + t.integer "status", default: 0 + t.datetime "saledate" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.bigint "owning_organisation_id" + t.bigint "managing_organisation_id" + t.bigint "created_by_id" + 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" + end + create_table "schemes", force: :cascade do |t| t.string "service_name" t.bigint "owning_organisation_id", null: false @@ -395,6 +408,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_09_02_082245) do add_foreign_key "lettings_logs", "organisations", column: "owning_organisation_id", on_delete: :cascade add_foreign_key "lettings_logs", "schemes" add_foreign_key "locations", "schemes" + add_foreign_key "sales_logs", "organisations", column: "owning_organisation_id", on_delete: :cascade add_foreign_key "schemes", "organisations", column: "managing_organisation_id" add_foreign_key "schemes", "organisations", column: "owning_organisation_id", on_delete: :cascade add_foreign_key "users", "organisations", on_delete: :cascade diff --git a/spec/components/check_answers_summary_list_card_component_spec.rb b/spec/components/check_answers_summary_list_card_component_spec.rb index 3c16a6254..6273da69d 100644 --- a/spec/components/check_answers_summary_list_card_component_spec.rb +++ b/spec/components/check_answers_summary_list_card_component_spec.rb @@ -3,23 +3,23 @@ require "rails_helper" RSpec.describe CheckAnswersSummaryListCardComponent, type: :component do context "when given a set of questions" do let(:user) { FactoryBot.build(:user) } - let(:lettings_log) { FactoryBot.build(:lettings_log, :completed, age2: 99) } + let(:log) { FactoryBot.build(:lettings_log, :completed, age2: 99) } let(:subsection_id) { "household_characteristics" } - let(:subsection) { lettings_log.form.get_subsection(subsection_id) } - let(:questions) { subsection.applicable_questions(lettings_log) } + let(:subsection) { log.form.get_subsection(subsection_id) } + let(:questions) { subsection.applicable_questions(log) } it "renders a summary list card for the answers to those questions" do - result = render_inline(described_class.new(questions:, lettings_log:, user:)) - expect(result).to have_content(questions.first.answer_label(lettings_log)) + result = render_inline(described_class.new(questions:, log:, user:)) + expect(result).to have_content(questions.first.answer_label(log)) end it "applicable questions doesn't return questions that are hidden in check answers" do - summary_list = described_class.new(questions:, lettings_log:, user:) + summary_list = described_class.new(questions:, log:, user:) expect(summary_list.applicable_questions.map(&:id).include?("retirement_value_check")).to eq(false) end it "has the correct answer label for a question" do - summary_list = described_class.new(questions:, lettings_log:, user:) + summary_list = described_class.new(questions:, log:, user:) sex1_question = questions[2] expect(summary_list.get_answer_label(sex1_question)).to eq("Female") end diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb new file mode 100644 index 000000000..bacb8e3e6 --- /dev/null +++ b/spec/factories/sales_log.rb @@ -0,0 +1,12 @@ +FactoryBot.define do + factory :sales_log do + created_by { FactoryBot.create(:user) } + owning_organisation { created_by.organisation } + managing_organisation { created_by.organisation } + created_at { Time.utc(2022, 2, 8, 16, 52, 15) } + updated_at { Time.utc(2022, 2, 8, 16, 52, 15) } + trait :completed do + saledate { Time.utc(2022, 2, 2, 10, 36, 49) } + end + end +end diff --git a/spec/features/form/accessible_autocomplete_spec.rb b/spec/features/form/accessible_autocomplete_spec.rb index 8e69b4bd1..52317107a 100644 --- a/spec/features/form/accessible_autocomplete_spec.rb +++ b/spec/features/form/accessible_autocomplete_spec.rb @@ -24,7 +24,7 @@ RSpec.describe "Accessible Automcomplete" do context "when using accessible autocomplete" do before do - visit("/logs/#{lettings_log.id}/accessible-select") + visit("/lettings-logs/#{lettings_log.id}/accessible-select") end it "allows type ahead filtering", js: true do @@ -66,7 +66,7 @@ RSpec.describe "Accessible Automcomplete" do FactoryBot.create(:location, scheme:, postcode: "W6 0ST") FactoryBot.create(:location, scheme:, postcode: "SE6 1LB") lettings_log.update!(needstype: 2) - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") end it "can match on synonyms", js: true do @@ -88,7 +88,7 @@ RSpec.describe "Accessible Automcomplete" do it "has the correct option selected if one has been saved" do lettings_log.update!(postcode_known: 0, previous_la_known: 1, prevloc: "E07000178") - visit("/logs/#{lettings_log.id}/accessible-select") + visit("/lettings-logs/#{lettings_log.id}/accessible-select") expect(page).to have_select("lettings-log-prevloc-field", selected: %w[Oxford]) end end diff --git a/spec/features/form/check_answers_page_spec.rb b/spec/features/form/check_answers_page_spec.rb index cff1b8d30..be5a8d46a 100644 --- a/spec/features/form/check_answers_page_spec.rb +++ b/spec/features/form/check_answers_page_spec.rb @@ -48,17 +48,17 @@ RSpec.describe "Form Check Answers Page" do let(:last_question_for_subsection) { "propcode" } it "can be visited by URL" do - visit("/logs/#{id}/#{subsection}/check-answers") + visit("/lettings-logs/#{id}/#{subsection}/check-answers") expect(page).to have_content("#{subsection.tr('-', ' ').humanize} Check your answers") end it "redirects to the check answers page when answering the last question and clicking save and continue" do fill_in_number_question(id, "propcode", 0, last_question_for_subsection) - expect(page).to have_current_path("/logs/#{id}/#{subsection}/check-answers") + expect(page).to have_current_path("/lettings-logs/#{id}/#{subsection}/check-answers") end it "has question headings based on the subsection" do - visit("/logs/#{id}/#{subsection}/check-answers") + visit("/lettings-logs/#{id}/#{subsection}/check-answers") question_labels = ["Tenant code", "Lead tenant’s age", "Lead tenant’s gender identity", "Number of Household Members"] question_labels.each do |label| expect(page).to have_content(label) @@ -69,7 +69,7 @@ RSpec.describe "Form Check Answers Page" do fill_in_number_question(empty_lettings_log.id, "age1", 28, "person-1-age") choose("lettings-log-sex1-x-field") click_button("Save and continue") - visit("/logs/#{empty_lettings_log.id}/#{subsection}/check-answers") + visit("/lettings-logs/#{empty_lettings_log.id}/#{subsection}/check-answers") expect(page).to have_content("28") expect(page).to have_content("Non-binary") end @@ -77,35 +77,35 @@ RSpec.describe "Form Check Answers Page" do # Regex explanation: match the string "Answer" but not if it's follow by "the missing questions" # This way only the links in the table will get picked up it "has an answer link for questions missing an answer" do - visit("/logs/#{empty_lettings_log.id}/#{subsection}/check-answers?referrer=check_answers") + visit("/lettings-logs/#{empty_lettings_log.id}/#{subsection}/check-answers?referrer=check_answers") assert_selector "a", text: /Answer (?!the missing questions)/, count: 5 assert_selector "a", text: "Change", count: 0 - expect(page).to have_link("Answer", href: "/logs/#{empty_lettings_log.id}/person-1-age?referrer=check_answers") + expect(page).to have_link("Answer", href: "/lettings-logs/#{empty_lettings_log.id}/person-1-age?referrer=check_answers") end it "has a change link for answered questions" do fill_in_number_question(empty_lettings_log.id, "age1", 28, "person-1-age") - visit("/logs/#{empty_lettings_log.id}/#{subsection}/check-answers") + visit("/lettings-logs/#{empty_lettings_log.id}/#{subsection}/check-answers") assert_selector "a", text: /Answer (?!the missing questions)/, count: 4 assert_selector "a", text: "Change", count: 1 - expect(page).to have_link("Change", href: "/logs/#{empty_lettings_log.id}/person-1-age?referrer=check_answers") + expect(page).to have_link("Change", href: "/lettings-logs/#{empty_lettings_log.id}/person-1-age?referrer=check_answers") end it "updates the change/answer link when answers get updated" do - visit("/logs/#{empty_lettings_log.id}/household-needs/check-answers") + visit("/lettings-logs/#{empty_lettings_log.id}/household-needs/check-answers") assert_selector "a", text: /Answer (?!the missing questions)/, count: 3 assert_selector "a", text: "Change", count: 1 - visit("/logs/#{empty_lettings_log.id}/accessibility-requirements") + visit("/lettings-logs/#{empty_lettings_log.id}/accessibility-requirements") check("lettings-log-accessibility-requirements-housingneeds-c-field") click_button("Save and continue") - visit("/logs/#{empty_lettings_log.id}/household-needs/check-answers") + visit("/lettings-logs/#{empty_lettings_log.id}/household-needs/check-answers") assert_selector "a", text: /Answer (?!the missing questions)/, count: 2 assert_selector "a", text: "Change", count: 2 - expect(page).to have_link("Change", href: "/logs/#{empty_lettings_log.id}/accessibility-requirements?referrer=check_answers") + expect(page).to have_link("Change", href: "/lettings-logs/#{empty_lettings_log.id}/accessibility-requirements?referrer=check_answers") end it "does not display conditional questions that were not visited" do - visit("/logs/#{id}/#{conditional_subsection}/check-answers") + visit("/lettings-logs/#{id}/#{conditional_subsection}/check-answers") question_labels = ["Has the condition been met?"] question_labels.each do |label| expect(page).to have_content(label) @@ -118,10 +118,10 @@ RSpec.describe "Form Check Answers Page" do end it "displays conditional question that were visited" do - visit("/logs/#{id}/conditional-question") + visit("/lettings-logs/#{id}/conditional-question") choose("lettings-log-preg-occ-2-field", allow_label_click: true) click_button("Save and continue") - visit("/logs/#{id}/#{conditional_subsection}/check-answers") + visit("/lettings-logs/#{id}/#{conditional_subsection}/check-answers") question_labels = ["Has the condition been met?", "Has the condition not been met?"] question_labels.each do |label| expect(page).to have_content(label) @@ -134,13 +134,13 @@ RSpec.describe "Form Check Answers Page" do end it "does not group questions into summary cards if the questions in the subsection don't have a check_answers_card_number attribute" do - visit("/logs/#{completed_lettings_log.id}/household-needs/check-answers") + visit("/lettings-logs/#{completed_lettings_log.id}/household-needs/check-answers") assert_selector ".x-govuk-summary-card__title", count: 0 end context "when the user is checking their answers for the household characteristics subsection" do it "they see a seperate summary card for each member of the household" do - visit("/logs/#{completed_lettings_log.id}/#{subsection}/check-answers") + visit("/lettings-logs/#{completed_lettings_log.id}/#{subsection}/check-answers") assert_selector ".x-govuk-summary-card__title", text: "Lead tenant", count: 1 assert_selector ".x-govuk-summary-card__title", text: "Person 2", count: 1 end @@ -153,14 +153,14 @@ RSpec.describe "Form Check Answers Page" do it "displays inferred postcode with the location id" do lettings_log.update!(location:) - visit("/logs/#{id}/setup/check-answers") + visit("/lettings-logs/#{id}/setup/check-answers") expect(page).to have_content("Location") expect(page).to have_content(location.name) end it "displays inferred postcode with the location_admin_district" do lettings_log.update!(location:) - visit("/logs/#{id}/setup/check-answers") + visit("/lettings-logs/#{id}/setup/check-answers") expect(page).to have_content("Location") expect(page).to have_content(location.location_admin_district) end @@ -168,15 +168,15 @@ RSpec.describe "Form Check Answers Page" do context "when the user changes their answer from check answer page" do it "routes back to check answers" do - visit("/logs/#{empty_lettings_log.id}/accessibility-requirements") + visit("/lettings-logs/#{empty_lettings_log.id}/accessibility-requirements") check("lettings-log-accessibility-requirements-housingneeds-c-field") click_button("Save and continue") - visit("/logs/#{empty_lettings_log.id}/household-needs/check-answers") + visit("/lettings-logs/#{empty_lettings_log.id}/household-needs/check-answers") first("a", text: /Change/).click uncheck("lettings-log-accessibility-requirements-housingneeds-c-field") check("lettings-log-accessibility-requirements-housingneeds-b-field") click_button("Save changes") - expect(page).to have_current_path("/logs/#{empty_lettings_log.id}/household-needs/check-answers") + expect(page).to have_current_path("/lettings-logs/#{empty_lettings_log.id}/household-needs/check-answers") end end @@ -247,27 +247,27 @@ RSpec.describe "Form Check Answers Page" do end it "they can click a button to move onto the first page of the next (not started) incomplete section" do - visit("/logs/#{section_completed_lettings_log.id}/household-characteristics/check-answers") + visit("/lettings-logs/#{section_completed_lettings_log.id}/household-characteristics/check-answers") click_link("Save and go to next incomplete section") - expect(page).to have_current_path("/logs/#{section_completed_lettings_log.id}/armed-forces") + expect(page).to have_current_path("/lettings-logs/#{section_completed_lettings_log.id}/armed-forces") end it "they can click a button to move onto the check answers page of the next (in progress) incomplete section" do - visit("/logs/#{next_section_in_progress_lettings_log.id}/household-characteristics/check-answers") + visit("/lettings-logs/#{next_section_in_progress_lettings_log.id}/household-characteristics/check-answers") click_link("Save and go to next incomplete section") - expect(page).to have_current_path("/logs/#{next_section_in_progress_lettings_log.id}/household-needs/check-answers") + expect(page).to have_current_path("/lettings-logs/#{next_section_in_progress_lettings_log.id}/household-needs/check-answers") end it "they can click a button to skip sections until the next incomplete section" do - visit("/logs/#{skip_section_lettings_log.id}/household-characteristics/check-answers") + visit("/lettings-logs/#{skip_section_lettings_log.id}/household-characteristics/check-answers") click_link("Save and go to next incomplete section") - expect(page).to have_current_path("/logs/#{skip_section_lettings_log.id}/property-information/check-answers") + expect(page).to have_current_path("/lettings-logs/#{skip_section_lettings_log.id}/property-information/check-answers") end it "they can click a button to cycle around to the next incomplete section" do - visit("/logs/#{cycle_sections_lettings_log.id}/declaration/check-answers") + visit("/lettings-logs/#{cycle_sections_lettings_log.id}/declaration/check-answers") click_link("Save and go to next incomplete section") - expect(page).to have_current_path("/logs/#{cycle_sections_lettings_log.id}/tenant-code-test") + expect(page).to have_current_path("/lettings-logs/#{cycle_sections_lettings_log.id}/tenant-code-test") end end end diff --git a/spec/features/form/checkboxes_spec.rb b/spec/features/form/checkboxes_spec.rb index e9386e458..f7a3df152 100644 --- a/spec/features/form/checkboxes_spec.rb +++ b/spec/features/form/checkboxes_spec.rb @@ -22,14 +22,14 @@ RSpec.describe "Checkboxes" do context "when exclusive checkbox is selected", js: true do it "deselects all other checkboxes" do - visit("/logs/#{id}/accessibility-requirements") + visit("/lettings-logs/#{id}/accessibility-requirements") page.check("lettings-log-accessibility-requirements-housingneeds-a-field", allow_label_click: true) click_button("Save and continue") lettings_log.reload expect(lettings_log["housingneeds_a"]).to eq(1) - visit("/logs/#{id}/accessibility-requirements") + visit("/lettings-logs/#{id}/accessibility-requirements") page.check("lettings-log-accessibility-requirements-housingneeds-h-field", allow_label_click: true) click_button("Save and continue") @@ -46,7 +46,7 @@ RSpec.describe "Checkboxes" do end it "shows an error summary" do - visit("/logs/#{id}/condition-effects") + visit("/lettings-logs/#{id}/condition-effects") page.check("lettings-log-condition-effects-illness-type-1-field") page.check("lettings-log-condition-effects-illness-type-2-field") click_button("Save and continue") @@ -54,7 +54,7 @@ RSpec.describe "Checkboxes" do end it "persists the original selections" do - visit("/logs/#{id}/condition-effects") + visit("/lettings-logs/#{id}/condition-effects") page.check("lettings-log-condition-effects-illness-type-1-field") page.check("lettings-log-condition-effects-illness-type-2-field") click_button("Save and continue") diff --git a/spec/features/form/conditional_questions_spec.rb b/spec/features/form/conditional_questions_spec.rb index 0643bf12c..051027bae 100644 --- a/spec/features/form/conditional_questions_spec.rb +++ b/spec/features/form/conditional_questions_spec.rb @@ -20,12 +20,12 @@ RSpec.describe "Form Conditional Questions" do context "with a page where some questions are only conditionally shown, depending on how you answer the first question" do it "initially hides conditional questions" do - visit("/logs/#{id}/armed-forces") + visit("/lettings-logs/#{id}/armed-forces") expect(page).not_to have_selector("#armed_forces_injured_div") end it "shows conditional questions if the required answer is selected and hides it again when a different answer option is selected", js: true do - visit("/logs/#{id}/armed-forces") + visit("/lettings-logs/#{id}/armed-forces") # Something about our styling makes the selenium webdriver think the actual radio buttons are not visible so we allow label click here choose("lettings-log-armedforces-1-field", allow_label_click: true) fill_in("lettings-log-leftreg-field", with: "text") @@ -39,7 +39,7 @@ RSpec.describe "Form Conditional Questions" do context "when a conditional question has a saved answer", js: true do it "is displayed correctly" do lettings_log.update!(postcode_known: 1, postcode_full: "NW1 6RT") - visit("/logs/#{id}/property-postcode") + visit("/lettings-logs/#{id}/property-postcode") expect(page).to have_field("lettings-log-postcode-full-field", with: "NW1 6RT") end end diff --git a/spec/features/form/form_navigation_spec.rb b/spec/features/form/form_navigation_spec.rb index abc4e4fdf..207ae19a3 100644 --- a/spec/features/form/form_navigation_spec.rb +++ b/spec/features/form/form_navigation_spec.rb @@ -39,7 +39,7 @@ RSpec.describe "Form Navigation" do describe "Create a new lettings log" do it "redirects to the task list for the new log" do - visit("/logs") + visit("/lettings-logs") click_button("Create a new lettings log") id = LettingsLog.order(created_at: :desc).first.id expect(page).to have_content("Log #{id}") @@ -48,64 +48,64 @@ RSpec.describe "Form Navigation" do describe "Viewing a log" do it "questions can be accessed by url" do - visit("/logs/#{id}/person-1-age") + visit("/lettings-logs/#{id}/person-1-age") expect(page).to have_field("lettings-log-age1-field") end it "a question page leads to the next question defined in the form definition" do pages = question_answers.map { |_key, val| val[:path] } pages[0..-2].each_with_index do |val, index| - visit("/logs/#{id}/#{val}") + visit("/lettings-logs/#{id}/#{val}") click_link("Skip for now") - expect(page).to have_current_path("/logs/#{id}/#{pages[index + 1]}") + expect(page).to have_current_path("/lettings-logs/#{id}/#{pages[index + 1]}") end end it "a question page has a Skip for now link that lets you move on to the next question without inputting anything" do - visit("logs/#{empty_lettings_log.id}/tenant-code-test") + visit("/lettings-logs/#{empty_lettings_log.id}/tenant-code-test") click_link(text: "Skip for now") - expect(page).to have_current_path("/logs/#{empty_lettings_log.id}/person-1-age") + expect(page).to have_current_path("/lettings-logs/#{empty_lettings_log.id}/person-1-age") end it "routes to check answers when skipping on the last page in the form" do - visit("logs/#{empty_lettings_log.id}/propcode") + visit("/lettings-logs/#{empty_lettings_log.id}/propcode") click_link(text: "Skip for now") - expect(page).to have_current_path("/logs/#{empty_lettings_log.id}/household-characteristics/check-answers") + expect(page).to have_current_path("/lettings-logs/#{empty_lettings_log.id}/household-characteristics/check-answers") end describe "Back link directs correctly", js: true do it "go back to tasklist page from tenant code" do - visit("/logs/#{id}") - visit("/logs/#{id}/tenant-code-test") + visit("/lettings-logs/#{id}") + visit("/lettings-logs/#{id}/tenant-code-test") click_link(text: "Back") expect(page).to have_content("Log #{id}") end it "go back to tenant code page from tenant age page", js: true do - visit("/logs/#{id}/tenant-code-test") + visit("/lettings-logs/#{id}/tenant-code-test") click_button("Save and continue") - visit("/logs/#{id}/person-1-age") + visit("/lettings-logs/#{id}/person-1-age") click_link(text: "Back") expect(page).to have_field("lettings-log-tenancycode-field") end it "doesn't get stuck in infinite loops", js: true do - visit("/logs") - visit("/logs/#{id}/net-income") + visit("/lettings-logs") + visit("/lettings-logs/#{id}/net-income") fill_in("lettings-log-earnings-field", with: 740) choose("lettings-log-incfreq-1-field", allow_label_click: true) click_button("Save and continue") click_link(text: "Back") click_link(text: "Back") - expect(page).to have_current_path("/logs") + expect(page).to have_current_path("/lettings-logs") end context "when changing an answer from the check answers page", js: true do it "the back button routes correctly" do - visit("/logs/#{id}/household-characteristics/check-answers") + visit("/lettings-logs/#{id}/household-characteristics/check-answers") first("a", text: /Answer/).click click_link("Back") - expect(page).to have_current_path("/logs/#{id}/household-characteristics/check-answers") + expect(page).to have_current_path("/lettings-logs/#{id}/household-characteristics/check-answers") end end end @@ -113,16 +113,16 @@ RSpec.describe "Form Navigation" do describe "Editing a log" do it "a question page has a link allowing you to cancel your input and return to the check answers page" do - visit("logs/#{id}/tenant-code-test?referrer=check_answers") + visit("lettings-logs/#{id}/tenant-code-test?referrer=check_answers") click_link(text: "Cancel") - expect(page).to have_current_path("/logs/#{id}/setup/check-answers") + expect(page).to have_current_path("/lettings-logs/#{id}/household-characteristics/check-answers") end context "when clicking save and continue on a mandatory question with no input" do let(:id) { empty_lettings_log.id } it "shows a validation error on radio questions" do - visit("/logs/#{id}/renewal") + visit("/lettings-logs/#{id}/renewal") click_button("Save and continue") expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#lettings-log-renewal-error") @@ -130,7 +130,7 @@ RSpec.describe "Form Navigation" do end it "shows a validation error on date questions" do - visit("/logs/#{id}/tenancy-start-date") + visit("/lettings-logs/#{id}/tenancy-start-date") click_button("Save and continue") expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#lettings-log-startdate-error") @@ -140,7 +140,7 @@ RSpec.describe "Form Navigation" do context "when the page has a main and conditional question" do context "when the conditional question is required but not answered" do it "shows a validation error for the conditional question" do - visit("/logs/#{id}/armed-forces") + visit("/lettings-logs/#{id}/armed-forces") choose("lettings-log-armedforces-1-field", allow_label_click: true) click_button("Save and continue") expect(page).to have_selector("#error-summary-title") @@ -155,11 +155,11 @@ RSpec.describe "Form Navigation" do let(:id) { empty_lettings_log.id } it "does not show a validation error" do - visit("/logs/#{id}/tenant-code") + visit("/lettings-logs/#{id}/tenant-code") click_button("Save and continue") expect(page).not_to have_selector("#error-summary-title") expect(page).not_to have_title("Error") - expect(page).to have_current_path("/logs/#{id}/property-reference") + expect(page).to have_current_path("/lettings-logs/#{id}/property-reference") end end end diff --git a/spec/features/form/helpers.rb b/spec/features/form/helpers.rb index 44d8c62ad..572a14d61 100644 --- a/spec/features/form/helpers.rb +++ b/spec/features/form/helpers.rb @@ -1,12 +1,12 @@ module Helpers def fill_in_number_question(lettings_log_id, question, value, path) - visit("/logs/#{lettings_log_id}/#{path}") + visit("/lettings-logs/#{lettings_log_id}/#{path}") fill_in("lettings-log-#{question.to_s.dasherize}-field", with: value) click_button("Save and continue") end def answer_all_questions_in_income_subsection(lettings_log) - visit("/logs/#{lettings_log.id}/net-income") + visit("/lettings-logs/#{lettings_log.id}/net-income") fill_in("lettings-log-earnings-field", with: 18_000) choose("lettings-log-incfreq-2-field") click_button("Save and continue") @@ -17,7 +17,7 @@ module Helpers end def sign_in(user) - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") diff --git a/spec/features/form/page_routing_spec.rb b/spec/features/form/page_routing_spec.rb index a568e3316..27f4feaf5 100644 --- a/spec/features/form/page_routing_spec.rb +++ b/spec/features/form/page_routing_spec.rb @@ -21,77 +21,77 @@ RSpec.describe "Form Page Routing" do end it "can route the user to a different page based on their answer on the current page", js: true do - visit("/logs/#{id}/conditional-question") + visit("/lettings-logs/#{id}/conditional-question") # using a question name that is already in the db to avoid # having to add a new column to the db for this test choose("lettings-log-preg-occ-1-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/conditional-question-yes-page") + expect(page).to have_current_path("/lettings-logs/#{id}/conditional-question-yes-page") click_link(text: "Back") - expect(page).to have_current_path("/logs/#{id}/conditional-question") + expect(page).to have_current_path("/lettings-logs/#{id}/conditional-question") choose("lettings-log-preg-occ-2-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/conditional-question-no-page") + expect(page).to have_current_path("/lettings-logs/#{id}/conditional-question-no-page") end it "can route based on multiple conditions", js: true do - visit("/logs/#{id}/person-1-gender") + visit("/lettings-logs/#{id}/person-1-gender") choose("lettings-log-sex1-f-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/person-1-working-situation") - visit("/logs/#{id}/conditional-question") + expect(page).to have_current_path("/lettings-logs/#{id}/person-1-working-situation") + visit("/lettings-logs/#{id}/conditional-question") choose("lettings-log-preg-occ-2-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/conditional-question-no-page") + expect(page).to have_current_path("/lettings-logs/#{id}/conditional-question-no-page") choose("lettings-log-cbl-0-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/conditional-question/check-answers") + expect(page).to have_current_path("/lettings-logs/#{id}/conditional-question/check-answers") end context "when the answers are inferred", js: true do it "shows question if the answer could not be inferred" do - visit("/logs/#{id}/property-postcode") + visit("/lettings-logs/#{id}/property-postcode") fill_in("lettings-log-postcode-full-field", with: "PO5 3TE") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/do-you-know-the-local-authority") + expect(page).to have_current_path("/lettings-logs/#{id}/do-you-know-the-local-authority") end it "shows question if the answer could not be inferred from an empty input" do - visit("/logs/#{id}/property-postcode") + visit("/lettings-logs/#{id}/property-postcode") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/do-you-know-the-local-authority") + expect(page).to have_current_path("/lettings-logs/#{id}/do-you-know-the-local-authority") end it "does not show question if the answer could be inferred" do stub_request(:get, /api.postcodes.io/) .to_return(status: 200, body: "{\"status\":200,\"result\":{\"admin_district\":\"Manchester\", \"codes\":{\"admin_district\": \"E08000003\"}}}", headers: {}) - visit("/logs/#{id}/property-postcode") + visit("/lettings-logs/#{id}/property-postcode") fill_in("lettings-log-postcode-full-field", with: "P0 5ST") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-wheelchair-accessible") + expect(page).to have_current_path("/lettings-logs/#{id}/property-wheelchair-accessible") end end context "when answer is invalid" do it "shows error with invalid value in the field" do - visit("/logs/#{id}/property-postcode") + visit("/lettings-logs/#{id}/property-postcode") fill_in("lettings-log-postcode-full-field", with: "fake_postcode") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-postcode") + expect(page).to have_current_path("/lettings-logs/#{id}/property-postcode") expect(find("#lettings-log-postcode-full-field-error").value).to eq("fake_postcode") end it "does not reset the displayed date" do lettings_log.update!(startdate: "2021/10/13") - visit("/logs/#{id}/tenancy-start-date") + visit("/lettings-logs/#{id}/tenancy-start-date") fill_in("lettings_log[startdate(1i)]", with: "202") fill_in("lettings_log[startdate(2i)]", with: "32") fill_in("lettings_log[startdate(3i)]", with: "0") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/tenancy-start-date") + expect(page).to have_current_path("/lettings-logs/#{id}/tenancy-start-date") expect(find_field("lettings_log[startdate(3i)]").value).to eq("13") expect(find_field("lettings_log[startdate(2i)]").value).to eq("10") expect(find_field("lettings_log[startdate(1i)]").value).to eq("2021") @@ -99,13 +99,13 @@ RSpec.describe "Form Page Routing" do it "does not reset the displayed date if it's empty" do lettings_log.update!(startdate: nil) - visit("/logs/#{id}/tenancy-start-date") + visit("/lettings-logs/#{id}/tenancy-start-date") fill_in("lettings_log[startdate(1i)]", with: "202") fill_in("lettings_log[startdate(2i)]", with: "32") fill_in("lettings_log[startdate(3i)]", with: "0") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/tenancy-start-date") + expect(page).to have_current_path("/lettings-logs/#{id}/tenancy-start-date") expect(find_field("lettings_log[startdate(3i)]").value).to eq(nil) expect(find_field("lettings_log[startdate(2i)]").value).to eq(nil) expect(find_field("lettings_log[startdate(1i)]").value).to eq(nil) @@ -129,13 +129,13 @@ RSpec.describe "Form Page Routing" do before do FactoryBot.create(:location, scheme:, startdate: Time.zone.today + 20.days) - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") select(scheme.service_name, from: "lettings_log[scheme_id]") click_button("Save and continue") end it "does not route to the scheme location question" do - expect(page).to have_current_path("/logs/#{lettings_log.id}/renewal") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/renewal") end it "infers the scheme location" do diff --git a/spec/features/form/progressive_total_field_spec.rb b/spec/features/form/progressive_total_field_spec.rb index 5be6ef1c2..5080de08c 100644 --- a/spec/features/form/progressive_total_field_spec.rb +++ b/spec/features/form/progressive_total_field_spec.rb @@ -18,12 +18,12 @@ RSpec.describe "Accessible Automcomplete" do end it "does not show when js is not enabled" do - visit("/logs/#{lettings_log.id}/rent") + visit("/lettings-logs/#{lettings_log.id}/rent") expect(page).to have_selector("#tcharge_div", visible: :all) end it "does show when js is enabled and calculates the total", js: true do - visit("/logs/#{lettings_log.id}/rent") + visit("/lettings-logs/#{lettings_log.id}/rent") expect(page).to have_selector("#tcharge_div") fill_in("lettings-log-brent-field", with: 5) expect(find("#lettings-log-tcharge-field").value).to eq("5.00") @@ -32,7 +32,7 @@ RSpec.describe "Accessible Automcomplete" do end it "total displays despite error message", js: true do - visit("/logs/#{lettings_log.id}/rent") + visit("/lettings-logs/#{lettings_log.id}/rent") choose("lettings-log-period-1-field", allow_label_click: true) fill_in("lettings-log-brent-field", with: 500) fill_in("lettings-log-scharge-field", with: 50) diff --git a/spec/features/form/saving_data_spec.rb b/spec/features/form/saving_data_spec.rb index ca08ee1fd..8312cfad9 100644 --- a/spec/features/form/saving_data_spec.rb +++ b/spec/features/form/saving_data_spec.rb @@ -39,7 +39,7 @@ RSpec.describe "Form Saving Data" do answer = hsh[:answer].respond_to?(:keys) ? hsh[:answer].keys.first : hsh[:answer] path = hsh[:path] original_value = lettings_log.send(question) - visit("/logs/#{id}/#{path.to_s.dasherize}") + visit("/lettings-logs/#{id}/#{path.to_s.dasherize}") case type when "text" fill_in("lettings-log-#{question.to_s.dasherize}-field", with: answer) @@ -55,7 +55,7 @@ RSpec.describe "Form Saving Data" do end it "updates total value of the rent", js: true do - visit("/logs/#{id}/rent") + visit("/lettings-logs/#{id}/rent") fill_in("lettings-log-brent-field", with: 3.02) expect(page.find("#lettings-log-tcharge-field")).to have_content("3.02") @@ -71,17 +71,17 @@ RSpec.describe "Form Saving Data" do end it "displays number answers in inputs if they are already saved" do - visit("/logs/#{id}/property-postcode") + visit("/lettings-logs/#{id}/property-postcode") expect(page).to have_field("lettings-log-postcode-full-field", with: lettings_log.postcode_full) end it "displays text answers in inputs if they are already saved" do - visit("/logs/#{id}/person-1-age") + visit("/lettings-logs/#{id}/person-1-age") expect(page).to have_field("lettings-log-age1-field", with: "17") end it "displays checkbox answers in inputs if they are already saved" do - visit("/logs/#{lettings_log_with_checkbox_questions_answered.id.to_s.dasherize}/accessibility-requirements") + visit("/lettings-logs/#{lettings_log_with_checkbox_questions_answered.id.to_s.dasherize}/accessibility-requirements") expect(page).to have_checked_field( "lettings-log-accessibility-requirements-housingneeds-a-field", visible: :all, diff --git a/spec/features/form/tasklist_page_spec.rb b/spec/features/form/tasklist_page_spec.rb index a82276c60..3f7ba6f1e 100644 --- a/spec/features/form/tasklist_page_spec.rb +++ b/spec/features/form/tasklist_page_spec.rb @@ -48,25 +48,25 @@ RSpec.describe "Task List" do end it "shows if the section has not been started" do - visit("/logs/#{empty_lettings_log.id}") + visit("/lettings-logs/#{empty_lettings_log.id}") expect(page).to have_content("This log has not been started.") end it "shows number of completed sections if one section is completed" do - visit("/logs/#{setup_completed_log.id}") + visit("/lettings-logs/#{setup_completed_log.id}") expect(page).to have_content("1 of 8 sections completed.") end it "show skip link for next incomplete section" do answer_all_questions_in_income_subsection(setup_completed_log) - visit("/logs/#{setup_completed_log.id}") + visit("/lettings-logs/#{setup_completed_log.id}") expect(page).to have_link("Skip to next incomplete section", href: /#household-characteristics/) end it "has a review section which has a button that allows the data inputter to review the lettings log" do - visit("/logs/#{completed_lettings_log.id}") + visit("/lettings-logs/#{completed_lettings_log.id}") expect(page).to have_content("review and make changes to this log") click_link(text: "review and make changes to this log") - expect(page).to have_current_path("/logs/#{completed_lettings_log.id}/review") + expect(page).to have_current_path("/lettings-logs/#{completed_lettings_log.id}/review") end end diff --git a/spec/features/form/validations_spec.rb b/spec/features/form/validations_spec.rb index d944f5161..6675150e0 100644 --- a/spec/features/form/validations_spec.rb +++ b/spec/features/form/validations_spec.rb @@ -38,7 +38,7 @@ RSpec.describe "validations" do describe "Question validation" do context "when the tenant age is invalid" do it "shows validation for under 0" do - visit("/logs/#{id}/person-1-age") + visit("/lettings-logs/#{id}/person-1-age") fill_in_number_question(empty_lettings_log.id, "age1", -5, "person-1-age") expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#lettings-log-age1-error") @@ -47,7 +47,7 @@ RSpec.describe "validations" do end it "shows validation for over 120" do - visit("/logs/#{id}/person-1-age") + visit("/lettings-logs/#{id}/person-1-age") fill_in_number_question(empty_lettings_log.id, "age1", 121, "person-1-age") expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#lettings-log-age1-error") @@ -59,7 +59,7 @@ RSpec.describe "validations" do describe "date validation", js: true do def fill_in_date(lettings_log_id, question, day, month, year, path) - visit("/logs/#{lettings_log_id}/#{path}") + visit("/lettings-logs/#{lettings_log_id}/#{path}") fill_in("lettings_log[#{question}(1i)]", with: year) fill_in("lettings_log[#{question}(2i)]", with: month) fill_in("lettings_log[#{question}(3i)]", with: day) @@ -68,45 +68,45 @@ RSpec.describe "validations" do it "does not allow out of range dates to be submitted" do fill_in_date(id, "mrcdate", 3100, 12, 2000, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") fill_in_date(id, "mrcdate", 12, 1, 20_000, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") fill_in_date(id, "mrcdate", 13, 100, 2020, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") fill_in_date(id, "mrcdate", 21, 11, 2020, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/local-authority/check-answers") + expect(page).to have_current_path("/lettings-logs/#{id}/local-authority/check-answers") end it "does not allow non numeric inputs to be submitted" do fill_in_date(id, "mrcdate", "abc", "de", "ff", "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") end it "does not allow partial inputs to be submitted" do fill_in_date(id, "mrcdate", 21, 12, nil, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") fill_in_date(id, "mrcdate", 12, nil, 2000, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") fill_in_date(id, "mrcdate", nil, 10, 2020, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/property-major-repairs") + expect(page).to have_current_path("/lettings-logs/#{id}/property-major-repairs") end it "allows valid inputs to be submitted" do fill_in_date(id, "mrcdate", 21, 11, 2020, "property-major-repairs") click_button("Save and continue") - expect(page).to have_current_path("/logs/#{id}/local-authority/check-answers") + expect(page).to have_current_path("/lettings-logs/#{id}/local-authority/check-answers") end end @@ -125,27 +125,27 @@ RSpec.describe "validations" do let(:income_under_soft_limit) { 700 } it "prompts the user to confirm the value is correct with an interruption screen" do - visit("/logs/#{lettings_log.id}/net-income") + visit("/lettings-logs/#{lettings_log.id}/net-income") fill_in("lettings-log-earnings-field", with: income_over_soft_limit) choose("lettings-log-incfreq-1-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{lettings_log.id}/net-income-value-check") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/net-income-value-check") expect(page).to have_content("Net income is outside the expected range based on the lead tenant’s working situation") expect(page).to have_content("You told us the lead tenant’s working situation is: full-time – 30 hours or more") expect(page).to have_content("The household income you have entered is £750.00 every week") choose("lettings-log-net-income-value-check-0-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{lettings_log.id}/net-income-uc-proportion") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/net-income-uc-proportion") end it "returns the user to the previous question if they do not confirm the value as correct on the interruption screen" do - visit("/logs/#{lettings_log.id}/net-income") + visit("/lettings-logs/#{lettings_log.id}/net-income") fill_in("lettings-log-earnings-field", with: income_over_soft_limit) choose("lettings-log-incfreq-1-field", allow_label_click: true) click_button("Save and continue") choose("lettings-log-net-income-value-check-1-field", allow_label_click: true) click_button("Save and continue") - expect(page).to have_current_path("/logs/#{lettings_log.id}/net-income") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/net-income") end end end @@ -153,7 +153,7 @@ RSpec.describe "validations" do describe "Submission validation" do context "when tenant has not seen the privacy notice" do it "shows a warning" do - visit("/logs/#{completed_without_declaration.id}/declaration") + visit("/lettings-logs/#{completed_without_declaration.id}/declaration") click_button("Save and continue") expect(page).to have_content("You must show the DLUHC privacy notice to the tenant") end diff --git a/spec/features/log_spec.rb b/spec/features/lettings_log_spec.rb similarity index 68% rename from spec/features/log_spec.rb rename to spec/features/lettings_log_spec.rb index cef344145..965f143e4 100644 --- a/spec/features/log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe "Log Features" do +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) } @@ -9,7 +9,7 @@ RSpec.describe "Log Features" do let!(:another_organisation_log) { FactoryBot.create(:lettings_log) } before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") @@ -77,9 +77,9 @@ RSpec.describe "Log Features" do click_button("Submit") end - context "when completing the setup log section" do + context "when completing the setup lettings log section" do it "includes the organisation and created by questions" do - visit("/logs") + 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") @@ -87,12 +87,28 @@ RSpec.describe "Log Features" do 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("logs/#{log_id}/setup/check-answers") + 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") end end + + context "when completing the setup sales log section" do + it "includes the sale completion date question" do + visit("/sales-logs") + click_button("Create a new sales log") + click_link("Set up this sales log") + fill_in("sales_log[saledate(1i)]", with: "2022") + fill_in("sales_log[saledate(2i)]", with: "08") + fill_in("sales_log[saledate(3i)]", with: "10") + click_button("Save and continue") + log_id = page.current_path.scan(/\d/).join + visit("sales-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Sale completion date") + expect(page).to have_content("2022") + end + end end context "when the signed is user is not a Support user" do @@ -112,16 +128,55 @@ RSpec.describe "Log Features" do context "when completing the setup log section" do it "does not include the organisation and created by questions" do - visit("/logs") + visit("/lettings-logs") click_button("Create a new lettings log") click_link("Set up this lettings log") log_id = page.current_path.scan(/\d/).join - expect(page).to have_current_path("/logs/#{log_id}/needs-type") - visit("logs/#{log_id}/setup/check-answers") + 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).to have_content("You have answered 0 of 6 questions") end end + + context "when completing the setup sales log section" do + it "includes the sale completion date question" do + visit("/sales-logs") + click_button("Create a new sales log") + click_link("Set up this sales log") + fill_in("sales_log[saledate(1i)]", with: "2022") + fill_in("sales_log[saledate(2i)]", with: "08") + fill_in("sales_log[saledate(3i)]", with: "10") + click_button("Save and continue") + log_id = page.current_path.scan(/\d/).join + visit("sales-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Sale completion date") + expect(page).to have_content("2022") + end + end + + context "when the sales log feature flag is toggled" do + before do + allow(Rails.env).to receive(:production?).and_return(true) + end + + it "hides the create sales log button in production" do + visit("/lettings-logs") + expect(page).not_to have_content("Create a new sales log") + end + end + + context "when returning to the list of logs via breadcrumbs link" do + before do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Logs") + end + + it "navigates you to the lettings logs page" do + expect(page).to have_current_path("/lettings-logs") + end + end end end diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index e14618109..7a14d286f 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -27,7 +27,7 @@ RSpec.describe "User Features" do context "when viewing organisation page" do it "defaults to organisation details" do - visit("/logs") + visit("/lettings-logs") click_link("About your organisation") expect(page).to have_content(user.organisation.name) end @@ -46,7 +46,7 @@ RSpec.describe "User Features" do end it "does not show schemes in the navigation bar" do - visit("/logs") + visit("/lettings-logs") expect(page).not_to have_link("Schemes", href: "/schemes") end end @@ -57,7 +57,7 @@ RSpec.describe "User Features" do end it "shows schemes in the navigation bar" do - visit("/logs") + visit("/lettings-logs") expect(page).to have_link("Schemes", href: "/schemes") end end @@ -109,7 +109,7 @@ RSpec.describe "User Features" do context "when viewing organisation page" do it "can see the details tab and users tab" do - visit("/logs") + visit("/lettings-logs") click_link("About your organisation") expect(page).to have_current_path("/organisations/#{org_id}/details") expect(page).to have_link("Logs") @@ -141,7 +141,7 @@ RSpec.describe "User Features" do let(:number_of_lettings_logs) { LettingsLog.count } before do - visit("/organisations/#{org_id}/logs") + visit("/organisations/#{org_id}/lettings-logs") end it "shows a create button for that organisation" do @@ -201,12 +201,12 @@ RSpec.describe "User Features" do it "can filter lettings logs" do expect(page).to have_content("#{number_of_lettings_logs} total logs") organisation.lettings_logs.map(&:id).each do |lettings_log_id| - expect(page).to have_link lettings_log_id.to_s, href: "/logs/#{lettings_log_id}" + expect(page).to have_link lettings_log_id.to_s, href: "/lettings-logs/#{lettings_log_id}" end check("years-2021-field") click_button("Apply filters") - expect(page).to have_current_path("/organisations/#{org_id}/logs?years[]=&years[]=2021&status[]=&user=all") - expect(page).not_to have_link first_log.id.to_s, href: "/logs/#{first_log.id}" + expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&user=all") + expect(page).not_to have_link first_log.id.to_s, href: "/lettings-logs/#{first_log.id}" end end @@ -252,36 +252,36 @@ RSpec.describe "User Features" do end end end + end + end + + describe "delete cascade" do + context "when the organisation is deleted" do + let!(:organisation) { FactoryBot.create(:organisation) } + let!(:user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } + let!(:scheme_to_delete) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:log_to_delete) { FactoryBot.create(:lettings_log, owning_organisation: user.organisation) } + + context "when organisation is deleted" do + it "child relationships ie logs, schemes and users are deleted too - application" do + organisation.destroy! + expect { organisation.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { LettingsLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + end - describe "delete cascade" do context "when the organisation is deleted" do let!(:organisation) { FactoryBot.create(:organisation) } let!(:user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } let!(:scheme_to_delete) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } - let!(:log_to_delete) { FactoryBot.create(:lettings_log, owning_organisation: user.organisation) } - - context "when organisation is deleted" do - it "child relationships ie logs, schemes and users are deleted too - application" do - organisation.destroy! - expect { organisation.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect { LettingsLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - - context "when the organisation is deleted" do - let!(:organisation) { FactoryBot.create(:organisation) } - let!(:user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } - let!(:scheme_to_delete) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } - let!(:log_to_delete) { FactoryBot.create(:lettings_log, :in_progress, needstype: 1, owning_organisation: user.organisation) } - - it "child relationships ie logs, schemes and users are deleted too - database" do - ActiveRecord::Base.connection.exec_query("DELETE FROM organisations WHERE id = #{organisation.id};") - expect { LettingsLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - end - end + let!(:log_to_delete) { FactoryBot.create(:lettings_log, :in_progress, needstype: 1, owning_organisation: user.organisation) } + + it "child relationships ie logs, schemes and users are deleted too - database" do + ActiveRecord::Base.connection.exec_query("DELETE FROM organisations WHERE id = #{organisation.id};") + expect { LettingsLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb new file mode 100644 index 000000000..c01997acd --- /dev/null +++ b/spec/features/sales_log_spec.rb @@ -0,0 +1,37 @@ +require "rails_helper" + +RSpec.describe "Sales Log Features" do + context "when searching for specific sales logs" do + context "when I am signed in and there are sales logs in the database" do + let(:user) { FactoryBot.create(:user, last_sign_in_at: Time.zone.now) } + let!(:log_to_search) { FactoryBot.create(:sales_log, owning_organisation: user.organisation) } + let!(:same_organisation_log) { FactoryBot.create(:sales_log, owning_organisation: user.organisation) } + let!(:another_organisation_log) { FactoryBot.create(:sales_log) } + + before do + visit("/sales-logs") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: user.password) + click_button("Sign in") + end + + it "displays the logs belonging to the same organisation" do + expect(page).to have_link(log_to_search.id.to_s) + expect(page).to have_link(same_organisation_log.id.to_s) + expect(page).not_to have_link(another_organisation_log.id.to_s) + end + + context "when returning to the list of logs via breadcrumbs link" do + before do + visit("/sales-logs") + click_button("Create a new sales log") + click_link("Logs") + end + + it "navigates you to the lettings logs page" do + expect(page).to have_current_path("/sales-logs") + end + end + end + end +end diff --git a/spec/features/schemes_helpers.rb b/spec/features/schemes_helpers.rb index 63d0d847b..0592a8ef2 100644 --- a/spec/features/schemes_helpers.rb +++ b/spec/features/schemes_helpers.rb @@ -1,12 +1,12 @@ module SchemesHelpers def fill_in_number_question(lettings_log_id, question, value, path) - visit("/logs/#{lettings_log_id}/#{path}") + visit("/lettings-logs/#{lettings_log_id}/#{path}") fill_in("lettings-log-#{question.to_s.dasherize}-field", with: value) click_button("Save and continue") end def answer_all_questions_in_income_subsection(lettings_log) - visit("/logs/#{lettings_log.id}/net-income") + visit("/lettings-logs/#{lettings_log.id}/net-income") fill_in("lettings-log-earnings-field", with: 18_000) choose("lettings-log-incfreq-2-field") click_button("Save and continue") @@ -17,7 +17,7 @@ module SchemesHelpers end def sign_in(user) - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index f189b2497..35a380fb5 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -10,7 +10,7 @@ RSpec.describe "Schemes scheme Features" do let!(:scheme_to_search) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") @@ -82,7 +82,7 @@ RSpec.describe "Schemes scheme Features" do allow(Devise).to receive(:friendly_token).and_return(confirmation_token) allow(notify_client).to receive(:send_email).and_return(true) allow(SecureRandom).to receive(:random_number).and_return(otp) - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") @@ -882,7 +882,7 @@ RSpec.describe "Schemes scheme Features" do let!(:schemes) { FactoryBot.create_list(:scheme, 5, owning_organisation_id: user.organisation_id) } before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") @@ -939,7 +939,7 @@ RSpec.describe "Schemes scheme Features" do FactoryBot.create(:location, scheme: schemes[1], startdate: nil) FactoryBot.create(:location, scheme: schemes[1], startdate: nil) FactoryBot.create(:location, scheme: schemes[1], startdate: Time.utc(2023, 6, 3)) - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") @@ -950,31 +950,31 @@ RSpec.describe "Schemes scheme Features" do end it "does not display the schemes without a location" do - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") expect(find("#lettings-log-scheme-id-field").all("option").count).to eq(4) end it "does not display the schemes with a location with a startdate in the future" do location.update!(startdate: Time.utc(2022, 7, 4)) - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") expect(find("#lettings-log-scheme-id-field").all("option").count).to eq(3) end it "does display the schemes with a location with a startdate in the past" do location.update!(startdate: Time.utc(2022, 5, 2)) - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") expect(find("#lettings-log-scheme-id-field").all("option").count).to eq(4) end it "does display the schemes with a location with a startdate being today" do location.update!(startdate: Time.utc(2022, 6, 3)) - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") expect(find("#lettings-log-scheme-id-field").all("option").count).to eq(4) end it "does display the schemes that are not completed" do schemes[2].update!(confirmed: false) - visit("/logs/#{lettings_log.id}/scheme") + visit("/lettings-logs/#{lettings_log.id}/scheme") expect(find("#lettings-log-scheme-id-field").all("option").count).to eq(3) end end diff --git a/spec/features/start_page_spec.rb b/spec/features/start_page_spec.rb index 625c4130b..569ea4cfa 100644 --- a/spec/features/start_page_spec.rb +++ b/spec/features/start_page_spec.rb @@ -12,7 +12,7 @@ RSpec.describe "Start Page Features" do it "takes you to logs" do visit("/") - expect(page).to have_current_path("/logs") + expect(page).to have_current_path("/lettings-logs") end end @@ -24,7 +24,7 @@ RSpec.describe "Start Page Features" do fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) click_button("Sign in") - expect(page).to have_current_path("/logs") + expect(page).to have_current_path("/lettings-logs") end end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 89447da65..530b038e6 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -16,26 +16,26 @@ RSpec.describe "User Features" do context "when the user navigates to lettings logs" do it "is required to log in" do - visit("/logs") + visit("/lettings-logs") expect(page).to have_current_path("/account/sign-in") expect(page).to have_content("Sign in to your account to submit CORE data") end it "does not see the default devise error message" do - visit("/logs") + visit("/lettings-logs") expect(page).to have_no_content("You need to sign in or sign up before continuing.") end it "is redirected to lettings logs after signing in" do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") - expect(page).to have_current_path("/logs") + expect(page).to have_current_path("/lettings-logs") end it "can log out again", js: true do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -45,7 +45,7 @@ RSpec.describe "User Features" do end it "can log out again with js disabled" do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -57,7 +57,7 @@ RSpec.describe "User Features" do context "when the user has forgotten their password" do it "is redirected to the reset password page when they click the reset password link" do - visit("/logs") + visit("/lettings-logs") click_link("reset your password") expect(page).to have_current_path("/account/password/new") end @@ -121,7 +121,7 @@ RSpec.describe "User Features" do context "when the user is not logged in" do it "'Your account' link does not display" do - visit("/logs") + visit("/lettings-logs") expect(page).to have_no_link("Your account") end @@ -132,7 +132,7 @@ RSpec.describe "User Features" do fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") - expect(page).to have_current_path("/logs") + expect(page).to have_current_path("/lettings-logs") end it "tries to access account page, redirects to log in page" do @@ -143,7 +143,7 @@ RSpec.describe "User Features" do context "when the user is trying to log in with incorrect credentials" do it "shows a gov uk error summary and no flash message" do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "nonsense") click_button("Sign in") @@ -153,7 +153,7 @@ RSpec.describe "User Features" do end it "show specific field error messages if a field was omitted" do - visit("/logs") + visit("/lettings-logs") click_button("Sign in") expect(page).to have_selector("#error-summary-title") expect(page).to have_selector("#user-email-field-error") @@ -162,7 +162,7 @@ RSpec.describe "User Features" do end it "show specific field error messages if an invalid email address is entered" do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: "thisisn'tanemail") click_button("Sign in") expect(page).to have_selector("#error-summary-title") @@ -178,7 +178,7 @@ RSpec.describe "User Features" do end it "shows a gov uk error summary and no flash message" do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -191,7 +191,7 @@ RSpec.describe "User Features" do context "when signed in as a data provider" do context "when viewing your account" do before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -216,7 +216,7 @@ RSpec.describe "User Features" do context "when viewing users" do before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -232,7 +232,7 @@ RSpec.describe "User Features" do context "when viewing your organisation details" do before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -248,14 +248,14 @@ RSpec.describe "User Features" do context "when viewing your account" do before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") end it "shows 'Your account' link in navigation if logged in and redirect to correct page" do - visit("/logs") + visit("/lettings-logs") expect(page).to have_link("Your account") click_link("Your account") expect(page).to have_current_path("/account") @@ -295,7 +295,7 @@ RSpec.describe "User Features" do context "when adding a new user" do before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -343,7 +343,7 @@ RSpec.describe "User Features" do let!(:other_user) { FactoryBot.create(:user, name: "Other name", is_dpo: false, organisation: user.organisation) } before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -392,7 +392,7 @@ RSpec.describe "User Features" do let!(:other_user) { FactoryBot.create(:user, name: "Other name", organisation: user.organisation) } before do - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -429,7 +429,7 @@ RSpec.describe "User Features" do before do other_user.update!(confirmation_token: "abc") - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -465,7 +465,7 @@ RSpec.describe "User Features" do allow(DeviseNotifyMailer).to receive(:new).and_return(devise_notify_mailer) allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client) allow(notify_client).to receive(:send_email).and_return(true) - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: support_user.email) fill_in("user[password]", with: "pAssword1") end @@ -568,7 +568,7 @@ RSpec.describe "User Features" do fill_in("code", with: otp) click_button("Submit") click_link("Sign out") - visit("/logs") + visit("/lettings-logs") fill_in("user[email]", with: support_user.email) fill_in("user[password]", with: "pAssword1") click_button("Sign in") @@ -638,15 +638,15 @@ RSpec.describe "User Features" do end it "clears the previously selected organisation value" do - visit("/logs") + visit("/lettings-logs") choose("organisation-select-specific-org-field", allow_label_click: true) expect(page).to have_field("organisation-field", with: "") find("#organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter) click_button("Apply filters") - expect(page).to have_current_path("/logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=specific_org&organisation=#{organisation.id}") + expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=specific_org&organisation=#{organisation.id}") choose("organisation-select-all-field", allow_label_click: true) click_button("Apply filters") - expect(page).to have_current_path("/logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=all") + expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=all") end end end diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb index 02ad348fb..b19f044be 100644 --- a/spec/helpers/filters_helper_spec.rb +++ b/spec/helpers/filters_helper_spec.rb @@ -11,7 +11,7 @@ RSpec.describe FiltersHelper do context "when the filter is the user filter but session filters is empty" do before do - session[:lettings_logs_filters] = {}.to_json + session[:logs_filters] = {}.to_json end context "when looking at the all value" do @@ -24,7 +24,7 @@ RSpec.describe FiltersHelper do context "when one filter is selected" do before do - session[:lettings_logs_filters] = { "status": "in_progress" }.to_json + session[:logs_filters] = { "status": "in_progress" }.to_json end it "returns false for non selected filters" do @@ -38,7 +38,7 @@ RSpec.describe FiltersHelper do context "when support user is using the organisation filter" do before do - session[:lettings_logs_filters] = { "organisation": "1" }.to_json + session[:logs_filters] = { "organisation": "1" }.to_json end it "returns true for the parent organisation_select filter" do @@ -49,7 +49,7 @@ RSpec.describe FiltersHelper do context "when support user has not set the organisation_select filter" do before do - session[:lettings_logs_filters] = {}.to_json + session[:logs_filters] = {}.to_json end it "defaults to all organisations" do @@ -60,7 +60,7 @@ RSpec.describe FiltersHelper do context "when the specific organisation filter is not set" do before do - session[:lettings_logs_filters] = { "status" => [""], "years" => [""], "user" => "all" }.to_json + session[:logs_filters] = { "status" => [""], "years" => [""], "user" => "all" }.to_json end it "marks the all options as checked" do @@ -72,7 +72,7 @@ RSpec.describe FiltersHelper do describe "#selected_option" do before do - session[:lettings_logs_filters] = {}.to_json + session[:logs_filters] = {}.to_json end context "when nothing has been selected" do diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index 60d0f3d0e..0da8f40fb 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -7,324 +7,708 @@ RSpec.describe NavigationItemsHelper do let(:organisation_path) { "/organisations/#{current_user.organisation.id}" } describe "#primary items" do - context "when the user is a data coordinator" do - context "when the user is on the logs page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", true), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), - ] - end - - it "returns navigation items with the users item set as current" do - expect(primary_items("/logs", current_user)).to eq(expected_navigation_items) - end + context "when the sales log feature flag is enabled" do + before do + allow(Rails.env).to receive(:production?).and_return(true) end - context "when the user is on the users page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, true), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), - ] - end + context "when the user is a data coordinator" do + context "when the user is on the logs page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end - it "returns navigation items with the users item set as current" do - expect(primary_items(users_path, current_user)).to eq(expected_navigation_items) + it "returns navigation items with the users item set as current" do + expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on their organisation details page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", users_path, false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, true), - ] - end + context "when the user is on the users page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, true), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end - it "returns navigation items with the users item set as current" do - expect(primary_items("#{organisation_path}/details", current_user)).to eq(expected_navigation_items) + it "returns navigation items with the users item set as current" do + expect(primary_items(users_path, current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on the account page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), - ] + context "when the user is on their organisation details page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, true), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("#{organisation_path}/details", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/account", current_user)).to eq(expected_navigation_items) + context "when the user is on the account page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/account", current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on the individual user's page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), - ] + context "when the user is on the individual user's page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items) + context "when the user is on the individual scheme's page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with Schemes item set as current" do + expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) + end end end - context "when the user is on the individual scheme's page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), - NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), - ] + context "when the user is a support user" do + let(:current_user) { FactoryBot.create(:user, :support) } + + context "when the user is on the logs page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with Schemes item set as current" do - expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) + context "when the user is on the users page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", true), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/users", current_user)).to eq(expected_navigation_items) + end end - end - end - context "when the user is a support user" do - let(:current_user) { FactoryBot.create(:user, :support) } - - context "when the user is on the logs page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", true), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - ] + context "when the user is on the account page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/account", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/logs", current_user)).to eq(expected_navigation_items) + context "when the user is on the Schemes page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/schemes", current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on the users page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", true), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - ] + context "when the user is on the individual user's page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", true), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/users", current_user)).to eq(expected_navigation_items) + context "when the user is on the individual scheme's page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), + ] + end + + let(:expected_scheme_items) do + [ + NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", true), + NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", false), + ] + end + + it "returns navigation items with Schemes item set as current" do + expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) + expect(scheme_items("/schemes/1", 1, "Locations")).to eq(expected_scheme_items) + end end - end - context "when the user is on the account page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - ] + context "when the user is on the scheme locations page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), + ] + end + + let(:expected_scheme_items) do + [ + NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", false), + NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", true), + ] + end + + it "returns navigation items with Schemes item set as current" do + expect(primary_items("/schemes/1/locations", current_user)).to eq(expected_navigation_items) + expect(scheme_items("/schemes/1/locations", 1, "Locations")).to eq(expected_scheme_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/account", current_user)).to eq(expected_navigation_items) + context "when the user is on the specific organisation's page" do + context "when the user is on organisation logs page" do + let(:required_sub_path) { "lettings-logs" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + ] + end + + it "returns navigation items with the logs item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end + + context "when the user is on organisation users page" do + let(:required_sub_path) { "users" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + ] + end + + it "returns navigation items with the logs item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end + + context "when the user is on organisation schemes page" do + let(:required_sub_path) { "schemes" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", true), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + ] + end + + it "returns navigation items with the schemes item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end + + context "when the user is on organisation details page" do + let(:required_sub_path) { "details" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", true), + ] + end + + it "returns navigation items with the logs item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end end end + end + + context "when the sales log feature flag is disabled" do + context "when the user is a data coordinator" do + context "when the user is on the lettings logs page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end - context "when the user is on the Schemes page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), - ] + it "returns navigation items with the users item set as current" do + expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/schemes", current_user)).to eq(expected_navigation_items) + context "when the user is on the sales logs page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", true), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/sales-logs", current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on the individual user's page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", true), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), - ] + context "when the user is on the users page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, true), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items(users_path, current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with the users item set as current" do - expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items) + context "when the user is on their organisation details page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, true), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("#{organisation_path}/details", current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on the individual scheme's page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), - ] + context "when the user is on the account page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/account", current_user)).to eq(expected_navigation_items) + end end - let(:expected_scheme_items) do - [ - NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", true), - NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", false), - ] + context "when the user is on the individual user's page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items) + end end - it "returns navigation items with Schemes item set as current" do - expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) - expect(scheme_items("/schemes/1", 1, "Locations")).to eq(expected_scheme_items) + context "when the user is on the individual scheme's page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + ] + end + + it "returns navigation items with Schemes item set as current" do + expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) + end end end - context "when the user is on the scheme locations page" do - let(:expected_navigation_items) do - [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), - NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), - ] - end + context "when the user is a support user" do + let(:current_user) { FactoryBot.create(:user, :support) } - let(:expected_scheme_items) do - [ - NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", false), - NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", true), - ] - end + context "when the user is on the lettings logs page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end - it "returns navigation items with Schemes item set as current" do - expect(primary_items("/schemes/1/locations", current_user)).to eq(expected_navigation_items) - expect(scheme_items("/schemes/1/locations", 1, "Locations")).to eq(expected_scheme_items) + it "returns navigation items with the users item set as current" do + expect(primary_items("/lettings-logs", current_user)).to eq(expected_navigation_items) + end end - end - context "when the user is on the specific organisation's page" do - context "when the user is on organisation logs page" do - let(:required_sub_path) { "logs" } + context "when the user is on the sales logs page" do let(:expected_navigation_items) do [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", true), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), ] end - let(:expected_secondary_navigation_items) do + it "returns navigation items with the users item set as current" do + expect(primary_items("/sales-logs", current_user)).to eq(expected_navigation_items) + end + end + + context "when the user is on the users page" do + let(:expected_navigation_items) do [ - NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", true), - NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), - NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", true), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), ] end - it "returns navigation items with the logs item set as current" do - expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) - expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + it "returns navigation items with the users item set as current" do + expect(primary_items("/users", current_user)).to eq(expected_navigation_items) end end - context "when the user is on organisation users page" do - let(:required_sub_path) { "users" } + context "when the user is on the account page" do let(:expected_navigation_items) do [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), ] end - let(:expected_secondary_navigation_items) do + it "returns navigation items with the users item set as current" do + expect(primary_items("/account", current_user)).to eq(expected_navigation_items) + end + end + + context "when the user is on the Schemes page" do + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), + ] + end + + it "returns navigation items with the users item set as current" do + expect(primary_items("/schemes", current_user)).to eq(expected_navigation_items) + end + end + + context "when the user is on the individual user's page" do + let(:expected_navigation_items) do [ - NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), - NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), + NavigationItemsHelper::NavigationItem.new("Users", "/users", true), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), ] end - it "returns navigation items with the logs item set as current" do - expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) - expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + it "returns navigation items with the users item set as current" do + expect(primary_items("/users/1", current_user)).to eq(expected_navigation_items) end end - context "when the user is on organisation schemes page" do - let(:required_sub_path) { "schemes" } + context "when the user is on the individual scheme's page" do let(:expected_navigation_items) do [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), ] end - let(:expected_secondary_navigation_items) do + let(:expected_scheme_items) do [ - NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", true), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), - NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", true), + NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", false), ] end - it "returns navigation items with the schemes item set as current" do - expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) - expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + it "returns navigation items with Schemes item set as current" do + expect(primary_items("/schemes/1", current_user)).to eq(expected_navigation_items) + expect(scheme_items("/schemes/1", 1, "Locations")).to eq(expected_scheme_items) end end - context "when the user is on organisation details page" do - let(:required_sub_path) { "details" } + context "when the user is on the scheme locations page" do let(:expected_navigation_items) do [ - NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false), NavigationItemsHelper::NavigationItem.new("Users", "/users", false), - NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", true), ] end - let(:expected_secondary_navigation_items) do + let(:expected_scheme_items) do [ - NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", false), - NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), - NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), - NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", true), + NavigationItemsHelper::NavigationItem.new("Scheme", "/schemes/1", false), + NavigationItemsHelper::NavigationItem.new("Locations", "/schemes/1/locations", true), ] end - it "returns navigation items with the logs item set as current" do - expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) - expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + it "returns navigation items with Schemes item set as current" do + expect(primary_items("/schemes/1/locations", current_user)).to eq(expected_navigation_items) + expect(scheme_items("/schemes/1/locations", 1, "Locations")).to eq(expected_scheme_items) + end + end + + context "when the user is on the specific organisation's page" do + context "when the user is on organisation logs page" do + let(:required_sub_path) { "lettings-logs" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/organisations/#{current_user.organisation.id}/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + ] + end + + it "returns navigation items with the logs item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end + + context "when the user is on organisation users page" do + let(:required_sub_path) { "users" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/organisations/#{current_user.organisation.id}/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + ] + end + + it "returns navigation items with the logs item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end + + context "when the user is on organisation schemes page" do + let(:required_sub_path) { "schemes" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/organisations/#{current_user.organisation.id}/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", true), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false), + ] + end + + it "returns navigation items with the schemes item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end + end + + context "when the user is on organisation details page" do + let(:required_sub_path) { "details" } + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true), + NavigationItemsHelper::NavigationItem.new("Users", "/users", false), + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false), + ] + end + + let(:expected_secondary_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings Logs", "/organisations/#{current_user.organisation.id}/lettings-logs", false), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/organisations/#{current_user.organisation.id}/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Schemes", "/organisations/#{current_user.organisation.id}/schemes", false), + NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false), + NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", true), + ] + end + + it "returns navigation items with the logs item set as current" do + expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items) + expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items) + end end end end diff --git a/spec/models/form/question_spec.rb b/spec/models/form/question_spec.rb index d1e2702fd..0c0b97a64 100644 --- a/spec/models/form/question_spec.rb +++ b/spec/models/form/question_spec.rb @@ -233,7 +233,7 @@ RSpec.describe Form::Question, type: :model do it "has an update answer link href helper" do lettings_log.id = 1 - expect(question.action_href(lettings_log, page.id)).to eq("/logs/1/net-income?referrer=check_answers") + expect(question.action_href(lettings_log, page.id)).to eq("/lettings-logs/1/net-income?referrer=check_answers") end context "when the question has an inferred answer" do diff --git a/spec/models/form/sales/setup/pages/sale_date_spec.rb b/spec/models/form/sales/setup/pages/sale_date_spec.rb new file mode 100644 index 000000000..56f6a6953 --- /dev/null +++ b/spec/models/form/sales/setup/pages/sale_date_spec.rb @@ -0,0 +1,29 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Setup::Pages::SaleDate, 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) } + + 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[saledate]) + end + + it "has the correct id" do + expect(page.id).to eq("sale_date") + 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 +end diff --git a/spec/models/form/sales/setup/questions/sale_date_spec.rb b/spec/models/form/sales/setup/questions/sale_date_spec.rb new file mode 100644 index 000000000..c340ea0c7 --- /dev/null +++ b/spec/models/form/sales/setup/questions/sale_date_spec.rb @@ -0,0 +1,33 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Setup::Questions::SaleDate, 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) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("saledate") + end + + it "has the correct header" do + expect(question.header).to eq("What is the sale completion date?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Sale completion date") + end + + it "has the correct type" do + expect(question.type).to eq("date") + end + + it "is not marked as derived" do + expect(question.derived?).to be false + end +end diff --git a/spec/models/form/sales/setup/sections/setup_spec.rb b/spec/models/form/sales/setup/sections/setup_spec.rb new file mode 100644 index 000000000..7bfab8cd1 --- /dev/null +++ b/spec/models/form/sales/setup/sections/setup_spec.rb @@ -0,0 +1,29 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Setup::Sections::Setup, type: :model do + subject(:setup) { described_class.new(section_id, section_definition, form) } + + let(:section_id) { nil } + let(:section_definition) { nil } + let(:form) { instance_double(Form) } + + it "has correct form" do + expect(setup.form).to eq(form) + end + + it "has correct subsections" do + expect(setup.subsections.map(&:id)).to eq(%w[setup]) + end + + it "has the correct id" do + expect(setup.id).to eq("setup") + end + + it "has the correct label" do + expect(setup.label).to eq("Before you start") + end + + it "has the correct description" do + expect(setup.description).to eq("") + end +end diff --git a/spec/models/form/sales/setup/subsections/setup_spec.rb b/spec/models/form/sales/setup/subsections/setup_spec.rb new file mode 100644 index 000000000..d2e1bedce --- /dev/null +++ b/spec/models/form/sales/setup/subsections/setup_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Setup::Subsections::Setup, type: :model do + subject(:setup) { described_class.new(subsection_id, subsection_definition, section) } + + let(:subsection_id) { nil } + let(:subsection_definition) { nil } + let(:section) { instance_double(Form::Sales::Setup::Sections::Setup) } + + it "has correct section" do + expect(setup.section).to eq(section) + end + + it "has correct pages" do + expect(setup.pages.map(&:id)).to eq( + %w[sale_date], + ) + end + + it "has the correct id" do + expect(setup.id).to eq("setup") + end + + it "has the correct label" do + expect(setup.label).to eq("Set up this sales log") + end +end diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index 94a3af2fe..4e6a61913 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -35,4 +35,9 @@ RSpec.describe FormHandler do expect(Form).not_to receive(:new).with(:any, test_form_name) expect(form_handler.get_form(test_form_name)).to be_a(Form) end + + it "can get a saleslog form" do + form_handler = described_class.instance + expect(form_handler.get_form("2022_2023_sales")).to be_a(Form) + end end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 7f1d4cb81..b779213c4 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -205,4 +205,25 @@ RSpec.describe Form, type: :model do end end end + + describe "when creating a sales log" do + it "creates a valid sales form" do + sections = [] + form = described_class.new(nil, "2022_23_sales", sections, "sales") + expect(form.type).to eq("sales") + expect(form.name).to eq("2022_23_sales") + expect(form.setup_sections.count).to eq(1) + expect(form.setup_sections[0].class).to eq(Form::Sales::Setup::Sections::Setup) + expect(form.sections.count).to eq(1) + expect(form.sections[0].class).to eq(Form::Sales::Setup::Sections::Setup) + expect(form.subsections.count).to eq(1) + expect(form.subsections.first.id).to eq("setup") + expect(form.pages.count).to eq(1) + expect(form.pages.first.id).to eq("sale_date") + expect(form.questions.count).to eq(1) + expect(form.questions.first.id).to eq("saledate") + expect(form.start_date).to eq(Time.zone.parse("2022-04-01")) + expect(form.end_date).to eq(Time.zone.parse("2023-07-01")) + end + end end diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index 772255b19..d53207226 100644 --- a/spec/models/lettings_log_spec.rb +++ b/spec/models/lettings_log_spec.rb @@ -5,6 +5,16 @@ RSpec.describe LettingsLog do let(:different_managing_organisation) { FactoryBot.create(:organisation) } let(:created_by_user) { FactoryBot.create(:user) } + it "inherits from log" do + expect(described_class).to be < Log + expect(described_class).to be < ApplicationRecord + end + + it "is a lettings log" do + lettings_log = FactoryBot.build(:lettings_log, created_by: created_by_user) + expect(lettings_log).to be_lettings + end + describe "#form" do let(:lettings_log) { FactoryBot.build(:lettings_log, created_by: created_by_user) } let(:lettings_log_2) { FactoryBot.build(:lettings_log, startdate: Time.zone.local(2022, 1, 1), created_by: created_by_user) } diff --git a/spec/models/log_spec.rb b/spec/models/log_spec.rb new file mode 100644 index 000000000..0cba24086 --- /dev/null +++ b/spec/models/log_spec.rb @@ -0,0 +1,8 @@ +require "rails_helper" + +RSpec.describe Log, type: :model do + it "has two child log classes" do + expect(SalesLog).to be < described_class + expect(LettingsLog).to be < described_class + end +end diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 9340e1b3a..7bc1d7cc1 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -131,6 +131,35 @@ RSpec.describe Organisation, type: :model do end end + describe "delete cascade" do + context "when the organisation is deleted" do + let!(:organisation) { FactoryBot.create(:organisation) } + let!(:user) { FactoryBot.create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } + let!(:scheme_to_delete) { FactoryBot.create(:scheme, owning_organisation: user.organisation) } + let!(:log_to_delete) { FactoryBot.create(:lettings_log, owning_organisation: user.organisation) } + let!(:sales_log_to_delete) { FactoryBot.create(:sales_log, owning_organisation: user.organisation, managing_organisation: user.organisation) } + + context "when organisation is deleted" do + it "child relationships ie logs, schemes and users are deleted too - application" do + organisation.destroy! + expect { organisation.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { LettingsLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { SalesLog.find(sales_log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it "child relationships ie logs, schemes and users are deleted too - database" do + ActiveRecord::Base.connection.exec_query("DELETE FROM organisations WHERE id = #{organisation.id};") + expect { LettingsLog.find(log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Scheme.find(scheme_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { SalesLog.find(sales_log_to_delete.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end + end + end + describe "scopes" do before do FactoryBot.create(:organisation, name: "Joe Bloggs") diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb new file mode 100644 index 000000000..8ccbf082a --- /dev/null +++ b/spec/models/sales_log_spec.rb @@ -0,0 +1,90 @@ +require "rails_helper" + +RSpec.describe SalesLog, type: :model do + let(:owning_organisation) { FactoryBot.create(:organisation) } + let(:created_by_user) { FactoryBot.create(:user) } + + it "inherits from log" do + expect(described_class).to be < Log + expect(described_class).to be < ApplicationRecord + end + + it "is a sales log" do + sales_log = FactoryBot.build(:sales_log, created_by: created_by_user) + expect(sales_log.lettings?).to be false + end + + describe "#new" do + context "when creating a record" do + let(:sales_log) do + described_class.create + end + + it "attaches the correct custom validator" do + expect(sales_log._validators.values.flatten.map(&:class)) + .to include(SalesLogValidator) + end + end + end + + describe "#form" do + let(:sales_log) { FactoryBot.build(:sales_log, created_by: created_by_user) } + let(:sales_log_2) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2022, 5, 1), created_by: created_by_user) } + + it "has returns the correct form based on the start date" do + expect(sales_log.form_name).to be_nil + expect(sales_log.form).to be_a(Form) + expect(sales_log_2.form_name).to eq("2022_2023_sales") + expect(sales_log_2.form).to be_a(Form) + end + end + + describe "status" do + let!(:empty_sales_log) { FactoryBot.create(:sales_log) } + # let!(:in_progress_sales_log) { FactoryBot.create(:sales_log, :in_progress) } + let!(:completed_sales_log) { FactoryBot.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 + end + + context "when filtering by organisation" do + let(:organisation_1) { FactoryBot.create(:organisation) } + let(:organisation_2) { FactoryBot.create(:organisation) } + let(:organisation_3) { FactoryBot.create(:organisation) } + + before do + FactoryBot.create(:sales_log, :in_progress, owning_organisation: organisation_1, managing_organisation: organisation_1) + FactoryBot.create(:sales_log, :completed, owning_organisation: organisation_1, managing_organisation: organisation_2) + FactoryBot.create(:sales_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_1) + FactoryBot.create(:sales_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_2) + end + + it "filters by given organisation id" do + expect(described_class.filter_by_organisation([organisation_1.id]).count).to eq(3) + expect(described_class.filter_by_organisation([organisation_1.id, organisation_2.id]).count).to eq(4) + expect(described_class.filter_by_organisation([organisation_3.id]).count).to eq(0) + end + + it "filters by given organisation" do + expect(described_class.filter_by_organisation([organisation_1]).count).to eq(3) + expect(described_class.filter_by_organisation([organisation_1, organisation_2]).count).to eq(4) + expect(described_class.filter_by_organisation([organisation_3]).count).to eq(0) + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3fa6354a8..ed77046fd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -115,7 +115,7 @@ RSpec.describe User, type: :model do end it "can filter lettings logs by user, year and status" do - expect(user.lettings_logs_filters).to eq(%w[status years user]) + expect(user.logs_filters).to eq(%w[status years user]) end end @@ -140,7 +140,7 @@ RSpec.describe User, type: :model do end it "can filter lettings logs by user, year, status and organisation" do - expect(user.lettings_logs_filters).to eq(%w[status years user organisation]) + expect(user.logs_filters).to eq(%w[status years user organisation]) end end @@ -269,4 +269,38 @@ RSpec.describe User, type: :model do end end end + + describe "delete" do + let(:user) { FactoryBot.create(:user) } + + before do + FactoryBot.create( + :lettings_log, + :completed, + owning_organisation: user.organisation, + managing_organisation: user.organisation, + created_by: user, + ) + + FactoryBot.create( + :sales_log, + owning_organisation: user.organisation, + created_by: user, + ) + end + + context "when the user is deleted" do + it "owned lettings logs are not deleted as a result" do + expect { user.destroy! } + .to change(described_class, :count).from(1).to(0) + .and change(LettingsLog, :count).by(0) + end + + it "owned sales logs are not deleted as a result" do + expect { user.destroy! } + .to change(described_class, :count).from(1).to(0) + .and change(SalesLog, :count).by(0) + end + end + end end diff --git a/spec/requests/auth/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb index 888bb8c70..830a06713 100644 --- a/spec/requests/auth/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -133,7 +133,7 @@ RSpec.describe Auth::PasswordsController, type: :request do it "sends you to the 2FA page and does not allow bypassing 2FA code" do put "/account/password", headers: headers, params: params expect(response).to redirect_to("/account/two-factor-authentication") - get "/logs", headers: headers + get "/lettings-logs", headers: headers expect(response).to redirect_to("/account/two-factor-authentication") end diff --git a/spec/requests/bulk_upload_controller_spec.rb b/spec/requests/bulk_upload_controller_spec.rb index 9869079af..bc75dbe84 100644 --- a/spec/requests/bulk_upload_controller_spec.rb +++ b/spec/requests/bulk_upload_controller_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" RSpec.describe BulkUploadController, type: :request do - let(:url) { "/logs/bulk-upload" } + let(:url) { "/lettings-logs/bulk-upload" } let(:user) { FactoryBot.create(:user) } let(:organisation) { user.organisation } let(:valid_file) { fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel") } diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 4d3a6dcc5..4b13b8895 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -42,19 +42,19 @@ RSpec.describe FormController, type: :request do context "when a user is not signed in" do describe "GET" do it "does not let you get lettings logs pages you don't have access to" do - get "/logs/#{lettings_log.id}/person-1-age", headers: headers, params: {} + get "/lettings-logs/#{lettings_log.id}/person-1-age", headers: headers, params: {} expect(response).to redirect_to("/account/sign-in") end it "does not let you get lettings log check answer pages you don't have access to" do - get "/logs/#{lettings_log.id}/household-characteristics/check-answers", headers: headers, params: {} + get "/lettings-logs/#{lettings_log.id}/household-characteristics/check-answers", headers: headers, params: {} expect(response).to redirect_to("/account/sign-in") end end describe "POST" do it "does not let you post form answers to lettings logs you don't have access to" do - post "/logs/#{lettings_log.id}/form", params: {} + post "/lettings-logs/#{lettings_log.id}/form", params: {} expect(response).to redirect_to("/account/sign-in") end end @@ -73,23 +73,23 @@ RSpec.describe FormController, type: :request do let(:lettings_log_year_2) { FactoryBot.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 "/logs/#{lettings_log_year_1.id}/tenant-code-test", headers: headers, params: {} + get "/lettings-logs/#{lettings_log_year_1.id}/tenant-code-test", headers: headers, params: {} expect(response.body).to include("What is the tenant code?") - get "/logs/#{lettings_log_year_2.id}/tenant-code-test", headers: headers, params: {} + get "/lettings-logs/#{lettings_log_year_2.id}/tenant-code-test", headers: headers, params: {} expect(response.body).to match("Different question header text for this year - 2023") end end context "when lettings logs are not owned or managed by your organisation" do it "does not show form pages for lettings logs you don't have access to" do - get "/logs/#{unauthorized_lettings_log.id}/person-1-age", headers: headers, params: {} + get "/lettings-logs/#{unauthorized_lettings_log.id}/person-1-age", headers: headers, params: {} expect(response).to have_http_status(:not_found) end end context "with a form page that has custom guidance" do it "displays the correct partial" do - get "/logs/#{lettings_log.id}/net-income", headers: headers, params: {} + get "/lettings-logs/#{lettings_log.id}/net-income", headers: headers, params: {} expect(response.body).to match("What counts as income?") end end @@ -115,7 +115,7 @@ RSpec.describe FormController, type: :request do end it "returns an unfiltered list of schemes" do - get "/logs/#{lettings_log.id}/scheme", headers: headers, params: {} + get "/lettings-logs/#{lettings_log.id}/scheme", headers: headers, params: {} expect(response.body.scan("