From c68e6e88ca5d3b3010e59af113ec42b69156c29a Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 11 Oct 2023 09:47:11 +0100 Subject: [PATCH 01/12] CLDC-2810 Send missing addresses template (#1952) * Add missing addresses csv job * Update missing addresses csv service methods * Add rake task * Update the job to send missing town or city templates * Update service to create mising town or city templates * Add send missing town or city csv rake task * Add IDs to the CSVs * Put all log in the same csv * Add issue type column * Write wrong uprn logs to csv * Add mailer methods * Skip uprn issue for specified orgs * Add sales csv rake task * set SKIP_UPRN_ISSUE_ORG_IDS on review apps * test * Update notify template IDs for testing * Initialize service with organisation instead of a hash * Add expiration time to url * Add optional tags and remove LA from csv * Extract log to csv methods * Update casing * Update old IDs in factories * Move constant * Extract some repeating scopes * Pass in organisations to skip instead of using an env var * update template id for sales * Update link expiry time and headers * Lower the threshold for testing * Add issue explanation to the email * Add how to fix * update emails * CLDC-2810 Create all addresses CSV (#1953) * Add rake tasks for creating all addresses CSV * Write headers if logs don't exist, update header names * Rename method * CLDC-2810 Correct addresses from csv (#1957) * Updating importing lettings addresses form csv * Add import_sales_addresses_from_csv rake * Allow correcting addresses from both templates * escape . * Reinfer LA if the postcode hasn't changed * Update labels and email content * Update missing addresses threshold * Remove unused env var --- app/jobs/create_addresses_csv_job.rb | 22 + app/jobs/email_missing_addresses_csv_job.rb | 33 ++ app/mailers/csv_download_mailer.rb | 75 +++ app/models/log.rb | 3 + .../csv/missing_addresses_csv_service.rb | 140 +++++ lib/tasks/import_address_from_csv.rake | 99 +++- lib/tasks/send_missing_addresses_csv.rake | 118 ++++ spec/factories/lettings_log.rb | 3 + spec/factories/sales_log.rb | 3 + spec/fixtures/files/addresses_reimport.csv | 13 +- .../files/addresses_reimport_all_logs.csv | 7 + .../files/missing_lettings_logs_addresses.csv | 2 + ...ing_lettings_logs_addresses_all_issues.csv | 4 + .../missing_lettings_logs_town_or_city.csv | 2 + .../missing_lettings_logs_wrong_uprn.csv | 2 + .../files/missing_sales_logs_addresses.csv | 2 + ...issing_sales_logs_addresses_all_issues.csv | 4 + .../files/missing_sales_logs_town_or_city.csv | 2 + .../files/missing_sales_logs_wrong_uprn.csv | 2 + .../files/sales_addresses_reimport.csv | 7 + .../sales_addresses_reimport_all_logs.csv | 7 + spec/jobs/create_addresses_csv_job_spec.rb | 49 ++ .../email_missing_addresses_csv_job_spec.rb | 66 +++ .../tasks/correct_address_from_csv_spec.rb | 537 ++++++++++++++--- .../tasks/send_missing_addresses_csv_spec.rb | 560 ++++++++++++++++++ spec/mailers/csv_download_mailer_spec.rb | 67 ++- .../csv/missing_addresses_csv_service_spec.rb | 512 ++++++++++++++++ 27 files changed, 2221 insertions(+), 120 deletions(-) create mode 100644 app/jobs/create_addresses_csv_job.rb create mode 100644 app/jobs/email_missing_addresses_csv_job.rb create mode 100644 app/services/csv/missing_addresses_csv_service.rb create mode 100644 lib/tasks/send_missing_addresses_csv.rake create mode 100644 spec/fixtures/files/addresses_reimport_all_logs.csv create mode 100644 spec/fixtures/files/missing_lettings_logs_addresses.csv create mode 100644 spec/fixtures/files/missing_lettings_logs_addresses_all_issues.csv create mode 100644 spec/fixtures/files/missing_lettings_logs_town_or_city.csv create mode 100644 spec/fixtures/files/missing_lettings_logs_wrong_uprn.csv create mode 100644 spec/fixtures/files/missing_sales_logs_addresses.csv create mode 100644 spec/fixtures/files/missing_sales_logs_addresses_all_issues.csv create mode 100644 spec/fixtures/files/missing_sales_logs_town_or_city.csv create mode 100644 spec/fixtures/files/missing_sales_logs_wrong_uprn.csv create mode 100644 spec/fixtures/files/sales_addresses_reimport.csv create mode 100644 spec/fixtures/files/sales_addresses_reimport_all_logs.csv create mode 100644 spec/jobs/create_addresses_csv_job_spec.rb create mode 100644 spec/jobs/email_missing_addresses_csv_job_spec.rb create mode 100644 spec/lib/tasks/send_missing_addresses_csv_spec.rb create mode 100644 spec/services/csv/missing_addresses_csv_service_spec.rb diff --git a/app/jobs/create_addresses_csv_job.rb b/app/jobs/create_addresses_csv_job.rb new file mode 100644 index 000000000..ef83ee746 --- /dev/null +++ b/app/jobs/create_addresses_csv_job.rb @@ -0,0 +1,22 @@ +class CreateAddressesCsvJob < ApplicationJob + queue_as :default + + BYTE_ORDER_MARK = "\uFEFF".freeze # Required to ensure Excel always reads CSV as UTF-8 + + def perform(organisation, log_type) + csv_service = Csv::MissingAddressesCsvService.new(organisation, []) + case log_type + when "lettings" + csv_string = csv_service.create_lettings_addresses_csv + filename = "#{['lettings-logs-addresses', organisation.name, Time.zone.now].compact.join('-')}.csv" + when "sales" + csv_string = csv_service.create_sales_addresses_csv + filename = "#{['sales-logs-addresses', organisation.name, Time.zone.now].compact.join('-')}.csv" + end + + storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"]) + storage_service.write_file(filename, BYTE_ORDER_MARK + csv_string) + + Rails.logger.info("Created addresses file: #{filename}") + end +end diff --git a/app/jobs/email_missing_addresses_csv_job.rb b/app/jobs/email_missing_addresses_csv_job.rb new file mode 100644 index 000000000..d8b756b20 --- /dev/null +++ b/app/jobs/email_missing_addresses_csv_job.rb @@ -0,0 +1,33 @@ +class EmailMissingAddressesCsvJob < ApplicationJob + queue_as :default + + BYTE_ORDER_MARK = "\uFEFF".freeze # Required to ensure Excel always reads CSV as UTF-8 + EXPIRATION_TIME = 1.week.to_i + MISSING_ADDRESSES_THRESHOLD = 50 + + def perform(user_ids, organisation, log_type, issue_types, skip_uprn_issue_organisations) + csv_service = Csv::MissingAddressesCsvService.new(organisation, skip_uprn_issue_organisations) + case log_type + when "lettings" + csv_string = csv_service.create_missing_lettings_addresses_csv + filename = "#{['missing-lettings-logs-addresses', organisation.name, Time.zone.now].compact.join('-')}.csv" + email_method = :send_missing_lettings_addresses_csv_download_mail + when "sales" + csv_string = csv_service.create_missing_sales_addresses_csv + filename = "#{['missing-sales-logs-addresses', organisation.name, Time.zone.now].compact.join('-')}.csv" + email_method = :send_missing_sales_addresses_csv_download_mail + end + + storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"]) + storage_service.write_file(filename, BYTE_ORDER_MARK + csv_string) + + url = storage_service.get_presigned_url(filename, EXPIRATION_TIME) + + user_ids.each do |id| + user = User.find(id) + next if user.blank? + + CsvDownloadMailer.new.send(email_method, user, url, EXPIRATION_TIME, issue_types) + end + end +end diff --git a/app/mailers/csv_download_mailer.rb b/app/mailers/csv_download_mailer.rb index 619d5e922..b590ebcfc 100644 --- a/app/mailers/csv_download_mailer.rb +++ b/app/mailers/csv_download_mailer.rb @@ -1,5 +1,7 @@ class CsvDownloadMailer < NotifyMailer CSV_DOWNLOAD_TEMPLATE_ID = "7890e3b9-8c0d-4d08-bafe-427fd7cd95bf".freeze + CSV_MISSING_LETTINGS_ADDRESSES_DOWNLOAD_TEMPLATE_ID = "7602b6c2-4f44-4da6-8a68-944e39cd8a05".freeze + CSV_MISSING_SALES_ADDRESSES_DOWNLOAD_TEMPLATE_ID = "1ee6da00-a65e-4a39-b5e5-1846debcb5f8".freeze def send_csv_download_mail(user, link, duration) send_email( @@ -8,4 +10,77 @@ class CsvDownloadMailer < NotifyMailer { name: user.name, link:, duration: ActiveSupport::Duration.build(duration).inspect }, ) end + + def send_missing_lettings_addresses_csv_download_mail(user, link, duration, issue_types) + send_email( + user.email, + CSV_MISSING_LETTINGS_ADDRESSES_DOWNLOAD_TEMPLATE_ID, + { name: user.name, issue_explanation: issue_explanation(issue_types, "lettings"), how_to_fix: how_to_fix(issue_types, link, "lettings"), duration: ActiveSupport::Duration.build(duration).inspect }, + ) + end + + def send_missing_sales_addresses_csv_download_mail(user, link, duration, issue_types) + send_email( + user.email, + CSV_MISSING_SALES_ADDRESSES_DOWNLOAD_TEMPLATE_ID, + { name: user.name, issue_explanation: issue_explanation(issue_types, "sales"), how_to_fix: how_to_fix(issue_types, link, "sales"), duration: ActiveSupport::Duration.build(duration).inspect }, + ) + end + +private + + HELPDESK_URL = "https://dluhcdigital.atlassian.net/servicedesk/customer/portal/6/group/11".freeze + + def issue_explanation(issue_types, log_type) + [ + "We have found #{multiple_issue_types?(issue_types) ? 'these issues' : 'this issue'} in your logs imported to the new version of CORE:\n", + issue_types.include?("missing_address") ? "## Full address required\nThe Unique Property Reference number (UPRN) in some logs is incorrect, so the address data was not imported. Provide the full address and leave the UPRN column blank.\n" : "", + issue_types.include?("missing_town") ? "## Missing town or city\nThe town or city in some logs is missing. This data is required in the new version of CORE.\n" : "", + has_uprn_issues(issue_types) ? uprn_issue_explanation(issue_types, log_type) : "", + ].join("") + end + + def how_to_fix(issue_types, link, log_type) + [ + "You need to:\n\n", + "- download [this spreadsheet for #{log_type} logs](#{link}). This link will expire in one week. To request another link, [contact the CORE helpdesk](#{HELPDESK_URL}).\n", + issue_types.include?("missing_address") || issue_types.include?("missing_town") ? "- fill in the missing address data\n" : "", + uprn_issues_only(issue_types) ? "- check that the address data is correct\n" : "- check that the existing address data is correct\n", + has_uprn_issues(issue_types) ? "- correct any address errors\n" : "", + ].join("") + end + + def uprn_issues_only(issue_types) + issue_types == %w[wrong_uprn] || issue_types == %w[bristol_uprn] || issue_types.count == 2 && issue_types.include?("wrong_uprn") && issue_types.include?("bristol_uprn") + end + + def has_uprn_issues(issue_types) + issue_types.include?("wrong_uprn") || issue_types.include?("bristol_uprn") + end + + def multiple_issue_types?(issue_types) + issue_types.count > 1 && !uprn_issues_only(issue_types) || issue_types.count > 2 + end + + def uprn_issue_explanation(issue_types, log_type) + if issue_types.include?("wrong_uprn") && !issue_types.include?("bristol_uprn") + "## Incorrect UPRN\nThe UPRN in some logs may be incorrect, so the wrong address data may have been imported. + +In some of your logs, the UPRN is the same as the #{log_type == 'lettings' ? 'tenant code or property reference' : 'purchaser code'}, but these are different things. #{log_type == 'lettings' ? 'Property references' : 'Purchaser codes'} are codes that your organisation uses to identify properties. UPRNs are unique numbers assigned by the Ordnance Survey. + +If a log has the correct UPRN, leave the UPRN unchanged. If the UPRN is incorrect, clear the value and provide the full address instead. Alternatively, you can change the UPRN on the CORE system.\n" + elsif issue_types.include?("bristol_uprn") && !issue_types.include?("wrong_uprn") + "## Incorrect UPRN\nThe UPRN in some logs may be incorrect, so the wrong address data may have been imported. In some of your logs, the UPRN links to an address in Bristol, but this is the first time your organisation has submitted logs for properties in Bristol. + +If a log has the correct UPRN, leave the UPRN unchanged. If the UPRN is incorrect, clear the value and provide the full address instead. Alternatively, you can change the UPRN on the CORE system.\n" + else + "## Incorrect UPRN\nThe UPRN in some logs may be incorrect, so the wrong address data may have been imported. We think this is an issue for two reasons. + +In some of your logs, the UPRN links to an address in Bristol, but this is the first time your organisation has submitted logs for properties in Bristol. + +In other logs, the UPRN is the same as the #{log_type == 'lettings' ? 'tenant code or property reference' : 'purchaser code'}, but these are different things. #{log_type == 'lettings' ? 'Property references' : 'Purchaser codes'} are codes that your organisation uses to identify properties. UPRNs are unique numbers assigned by the Ordnance Survey. + +If a log has the correct UPRN, leave the UPRN unchanged. If the UPRN is incorrect, clear the value and provide the full address instead. Alternatively, you can change the UPRN on the CORE system." + end + end end diff --git a/app/models/log.rb b/app/models/log.rb index c1778b299..5ba887c7a 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -46,6 +46,9 @@ class Log < ApplicationRecord scope :created_by, ->(user) { where(created_by: user) } scope :imported, -> { where.not(old_id: nil) } scope :not_imported, -> { where(old_id: nil) } + scope :has_old_form_id, -> { where.not(old_form_id: nil) } + scope :imported_2023_with_old_form_id, -> { imported.filter_by_year(2023).has_old_form_id } + scope :imported_2023, -> { imported.filter_by_year(2023) } attr_accessor :skip_update_status, :skip_update_uprn_confirmed diff --git a/app/services/csv/missing_addresses_csv_service.rb b/app/services/csv/missing_addresses_csv_service.rb new file mode 100644 index 000000000..a10beaa1d --- /dev/null +++ b/app/services/csv/missing_addresses_csv_service.rb @@ -0,0 +1,140 @@ +module Csv + class MissingAddressesCsvService + def initialize(organisation, skip_uprn_issue_organisations) + @organisation = organisation + @skip_uprn_issue_organisations = skip_uprn_issue_organisations + end + + def create_missing_lettings_addresses_csv + logs_with_missing_addresses = @organisation.managed_lettings_logs + .imported_2023_with_old_form_id + .where(needstype: 1, address_line1: nil, town_or_city: nil, uprn_known: [0, nil]) + + logs_with_missing_town_or_city = @organisation.managed_lettings_logs + .imported_2023_with_old_form_id + .where(needstype: 1, town_or_city: nil, uprn_known: [0, nil]) + .where.not(address_line1: nil) + + logs_with_wrong_uprn = if @skip_uprn_issue_organisations.include?(@organisation.id) + [] + else + @organisation.managed_lettings_logs + .imported_2023 + .where(needstype: 1) + .where.not(uprn: nil) + .where("uprn = propcode OR uprn = tenancycode OR town_or_city = 'Bristol'") + end + + return if logs_with_missing_addresses.empty? && logs_with_missing_town_or_city.empty? && logs_with_wrong_uprn.empty? + + CSV.generate(headers: true) do |csv| + csv << ["Issue type", "Log ID", "Tenancy start date", "Tenant code", "Property reference", "Log owner", "Owning organisation", "Managing organisation", "UPRN", "Address Line 1", "Address Line 2 (optional)", "Town or City", "County (optional)", "Property's postcode"] + + logs_with_missing_addresses.each do |log| + csv << ["Full address required"] + lettings_log_to_csv_row(log) + end + + logs_with_missing_town_or_city.each do |log| + csv << ["Missing town or city"] + lettings_log_to_csv_row(log) + end + + logs_with_wrong_uprn.each do |log| + csv << ["Incorrect UPRN"] + lettings_log_to_csv_row(log) + end + end + end + + def create_missing_sales_addresses_csv + logs_with_missing_addresses = @organisation.sales_logs + .imported_2023_with_old_form_id + .where(address_line1: nil, town_or_city: nil, uprn_known: [0, nil]) + + logs_with_missing_town_or_city = @organisation.sales_logs + .imported_2023_with_old_form_id + .where(town_or_city: nil, uprn_known: [0, nil]) + .where.not(address_line1: nil) + + logs_with_wrong_uprn = if @skip_uprn_issue_organisations.include?(@organisation.id) + [] + else + @organisation.sales_logs + .imported_2023 + .where.not(uprn: nil) + .where("uprn = purchid OR town_or_city = 'Bristol'") + end + return if logs_with_missing_addresses.empty? && logs_with_missing_town_or_city.empty? && logs_with_wrong_uprn.empty? + + CSV.generate(headers: true) do |csv| + csv << ["Issue type", "Log ID", "Sale completion date", "Purchaser code", "Log owner", "Owning organisation", "UPRN", "Address Line 1", "Address Line 2 (optional)", "Town or City", "County (optional)", "Property's postcode"] + + logs_with_missing_addresses.each do |log| + csv << ["Full address required"] + sales_log_to_csv_row(log) + end + + logs_with_missing_town_or_city.each do |log| + csv << ["Missing town or city"] + sales_log_to_csv_row(log) + end + + logs_with_wrong_uprn.each do |log| + csv << ["Incorrect UPRN"] + sales_log_to_csv_row(log) + end + end + end + + def create_lettings_addresses_csv + logs = @organisation.managed_lettings_logs.filter_by_year(2023) + + CSV.generate(headers: true) do |csv| + csv << ["Log ID", "Tenancy start date", "Tenant code", "Property reference", "Log owner", "Owning organisation", "Managing organisation", "UPRN", "Address Line 1", "Address Line 2 (optional)", "Town or City", "County (optional)", "Property's postcode"] + + logs.each do |log| + csv << lettings_log_to_csv_row(log) + end + end + end + + def create_sales_addresses_csv + logs = @organisation.sales_logs.filter_by_year(2023) + + CSV.generate(headers: true) do |csv| + csv << ["Log ID", "Sale completion date", "Purchaser code", "Log owner", "Owning organisation", "UPRN", "Address Line 1", "Address Line 2 (optional)", "Town or City", "County (optional)", "Property's postcode"] + + logs.each do |log| + csv << sales_log_to_csv_row(log) + end + end + end + + private + + def sales_log_to_csv_row(log) + [log.id, + log.saledate&.to_date, + log.purchid, + log.created_by&.email, + log.owning_organisation&.name, + log.uprn, + log.address_line1, + log.address_line2, + log.town_or_city, + log.county, + log.postcode_full] + end + + def lettings_log_to_csv_row(log) + [log.id, + log.startdate&.to_date, + log.tenancycode, + log.propcode, + log.created_by&.email, + log.owning_organisation&.name, + log.managing_organisation&.name, + log.uprn, + log.address_line1, + log.address_line2, + log.town_or_city, + log.county, + log.postcode_full] + end + end +end diff --git a/lib/tasks/import_address_from_csv.rake b/lib/tasks/import_address_from_csv.rake index e910f3a77..57496a56c 100644 --- a/lib/tasks/import_address_from_csv.rake +++ b/lib/tasks/import_address_from_csv.rake @@ -1,18 +1,35 @@ namespace :data_import do - desc "Import address data from a csv file" - task :import_address_from_csv, %i[file_name] => :environment do |_task, args| + desc "Import lettings address data from a csv file" + task :import_lettings_addresses_from_csv, %i[file_name] => :environment do |_task, args| file_name = args[:file_name] - raise "Usage: rake data_import:import_address_from_csv['csv_file_name']" if file_name.blank? + raise "Usage: rake data_import:import_lettings_addresses_from_csv['csv_file_name']" if file_name.blank? s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) addresses_csv = CSV.parse(s3_service.get_file_io(file_name), headers: true) + contains_issue_type = addresses_csv.headers.include?("Issue type") addresses_csv.each do |row| - lettings_log_id = row[0] + lettings_log_id = contains_issue_type ? row[1] : row[0] + uprn = contains_issue_type ? row[8] : row[7] + address_line1 = contains_issue_type ? row[9] : row[8] + address_line2 = contains_issue_type ? row[10] : row[9] + town_or_city = contains_issue_type ? row[11] : row[10] + county = contains_issue_type ? row[12] : row[11] + postcode_full = contains_issue_type ? row[13] : row[12] if lettings_log_id.blank? - Rails.logger.info("Lettings log ID not provided for address: #{[row[1], row[2], row[3], row[4]].join(', ')}") + Rails.logger.info("Lettings log ID not provided for address: #{[address_line1, address_line2, town_or_city, county, postcode_full].join(', ')}") + next + end + + if uprn.present? + Rails.logger.info("Lettings log with ID #{lettings_log_id} contains uprn, skipping log") + next + end + + if address_line1.blank? || town_or_city.blank? || postcode_full.blank? + Rails.logger.info("Lettings log with ID #{lettings_log_id} is missing required address data, skipping log") next end @@ -25,18 +42,78 @@ namespace :data_import do lettings_log.uprn_known = 0 lettings_log.uprn = nil lettings_log.uprn_confirmed = nil - lettings_log.address_line1 = row[1] - lettings_log.address_line2 = row[2] - lettings_log.town_or_city = row[3] - lettings_log.postcode_full = row[4] + lettings_log.address_line1 = address_line1 + lettings_log.address_line2 = address_line2 + lettings_log.town_or_city = town_or_city + lettings_log.county = county + lettings_log.postcode_full = postcode_full lettings_log.postcode_known = lettings_log.postcode_full.present? ? 1 : nil - lettings_log.county = nil lettings_log.is_la_inferred = nil lettings_log.la = nil + lettings_log.send("process_postcode_changes!") lettings_log.values_updated_at = Time.zone.now lettings_log.save! - Rails.logger.info("Updated lettings log #{lettings_log_id}, with address: #{[lettings_log.address_line1, lettings_log.address_line2, lettings_log.town_or_city, lettings_log.postcode_full].join(', ')}") + Rails.logger.info("Updated lettings log #{lettings_log_id}, with address: #{[lettings_log.address_line1, lettings_log.address_line2, lettings_log.town_or_city, lettings_log.county, lettings_log.postcode_full].join(', ')}") + end + end + + desc "Import sales address data from a csv file" + task :import_sales_addresses_from_csv, %i[file_name] => :environment do |_task, args| + file_name = args[:file_name] + + raise "Usage: rake data_import:import_sales_addresses_from_csv['csv_file_name']" if file_name.blank? + + s3_service = Storage::S3Service.new(PlatformHelper.is_paas? ? Configuration::PaasConfigurationService.new : Configuration::EnvConfigurationService.new, ENV["IMPORT_PAAS_INSTANCE"]) + addresses_csv = CSV.parse(s3_service.get_file_io(file_name), headers: true) + contains_issue_type = addresses_csv.headers.include?("Issue type") + + addresses_csv.each do |row| + sales_log_id = contains_issue_type ? row[1] : row[0] + uprn = contains_issue_type ? row[6] : row[5] + address_line1 = contains_issue_type ? row[7] : row[6] + address_line2 = contains_issue_type ? row[8] : row[7] + town_or_city = contains_issue_type ? row[9] : row[8] + county = contains_issue_type ? row[10] : row[9] + postcode_full = contains_issue_type ? row[11] : row[10] + + if sales_log_id.blank? + Rails.logger.info("Sales log ID not provided for address: #{[address_line1, address_line2, town_or_city, county, postcode_full].join(', ')}") + next + end + + if uprn.present? + Rails.logger.info("Sales log with ID #{sales_log_id} contains uprn, skipping log") + next + end + + if address_line1.blank? || town_or_city.blank? || postcode_full.blank? + Rails.logger.info("Sales log with ID #{sales_log_id} is missing required address data, skipping log") + next + end + + sales_log = SalesLog.find_by(id: sales_log_id) + if sales_log.blank? + Rails.logger.info("Could not find a sales log with id #{sales_log_id}") + next + end + + sales_log.uprn_known = 0 + sales_log.uprn = nil + sales_log.uprn_confirmed = nil + sales_log.address_line1 = address_line1 + sales_log.address_line2 = address_line2 + sales_log.town_or_city = town_or_city + sales_log.county = county + sales_log.postcode_full = postcode_full + sales_log.pcodenk = sales_log.postcode_full.present? ? 0 : nil + sales_log.is_la_inferred = nil + sales_log.la = nil + sales_log.send("process_postcode_changes!") + sales_log.values_updated_at = Time.zone.now + + sales_log.save! + Rails.logger.info("Updated sales log #{sales_log_id}, with address: #{[sales_log.address_line1, sales_log.address_line2, sales_log.town_or_city, sales_log.county, sales_log.postcode_full].join(', ')}") end end end diff --git a/lib/tasks/send_missing_addresses_csv.rake b/lib/tasks/send_missing_addresses_csv.rake new file mode 100644 index 000000000..9c8e60b59 --- /dev/null +++ b/lib/tasks/send_missing_addresses_csv.rake @@ -0,0 +1,118 @@ +namespace :correct_addresses do + desc "Send missing lettings addresses csv" + task :send_missing_addresses_lettings_csv, %i[skip_uprn_issue_organisations] => :environment do |_task, args| + skip_uprn_issue_organisations = args[:skip_uprn_issue_organisations]&.split(" ")&.map(&:to_i) || [] + + Organisation.all.each do |organisation| + logs_impacted_by_missing_address = organisation.managed_lettings_logs + .imported_2023_with_old_form_id + .where(needstype: 1, address_line1: nil, town_or_city: nil, uprn_known: [0, nil]).count + + logs_impacted_by_missing_town_or_city = organisation.managed_lettings_logs + .imported_2023_with_old_form_id + .where(needstype: 1, town_or_city: nil, uprn_known: [0, nil]) + .where.not(address_line1: nil).count + + logs_impacted_by_uprn_issue = if skip_uprn_issue_organisations.include?(organisation.id) + [] + else + organisation.managed_lettings_logs + .imported_2023 + .where(needstype: 1) + .where.not(uprn: nil) + .where("uprn = propcode OR uprn = tenancycode") + end + + logs_impacted_by_bristol_uprn_issue = if skip_uprn_issue_organisations.include?(organisation.id) + [] + else + organisation.managed_lettings_logs + .imported_2023 + .where(needstype: 1) + .where.not(uprn: nil) + .where("town_or_city = 'Bristol'") + end + + missing_addresses_threshold = EmailMissingAddressesCsvJob::MISSING_ADDRESSES_THRESHOLD + if logs_impacted_by_missing_address >= missing_addresses_threshold || logs_impacted_by_missing_town_or_city >= missing_addresses_threshold || logs_impacted_by_uprn_issue.any? || logs_impacted_by_bristol_uprn_issue.any? + issue_types = [] + issue_types << "missing_address" if logs_impacted_by_missing_address.positive? + issue_types << "missing_town" if logs_impacted_by_missing_town_or_city.positive? + issue_types << "wrong_uprn" if logs_impacted_by_uprn_issue.any? + issue_types << "bristol_uprn" if logs_impacted_by_bristol_uprn_issue.any? + data_coordinators = organisation.users.where(role: 2).filter_by_active + users_to_contact = data_coordinators.any? ? data_coordinators : organisation.users.filter_by_active + EmailMissingAddressesCsvJob.perform_later(users_to_contact.map(&:id), organisation, "lettings", issue_types, skip_uprn_issue_organisations) + Rails.logger.info("Sending missing lettings addresses CSV for #{organisation.name} to #{users_to_contact.map(&:email).join(', ')}") + else + Rails.logger.info("Missing addresses below threshold for #{organisation.name}") + end + end + end + + desc "Send missing sales addresses csv" + task :send_missing_addresses_sales_csv, %i[skip_uprn_issue_organisations] => :environment do |_task, args| + skip_uprn_issue_organisations = args[:skip_uprn_issue_organisations]&.split(" ")&.map(&:to_i) || [] + + Organisation.all.each do |organisation| + logs_impacted_by_missing_address = organisation.sales_logs + .imported_2023_with_old_form_id + .where(address_line1: nil, town_or_city: nil, uprn_known: [0, nil]).count + + logs_impacted_by_missing_town_or_city = organisation.sales_logs + .imported_2023_with_old_form_id + .where(town_or_city: nil, uprn_known: [0, nil]) + .where.not(address_line1: nil).count + + logs_impacted_by_uprn_issue = if skip_uprn_issue_organisations.include?(organisation.id) + [] + else + organisation.sales_logs + .imported_2023 + .where.not(uprn: nil) + .where("uprn = purchid OR town_or_city = 'Bristol'") + end + missing_addresses_threshold = EmailMissingAddressesCsvJob::MISSING_ADDRESSES_THRESHOLD + if logs_impacted_by_missing_address >= missing_addresses_threshold || logs_impacted_by_missing_town_or_city >= missing_addresses_threshold || logs_impacted_by_uprn_issue.any? + issue_types = [] + issue_types << "missing_address" if logs_impacted_by_missing_address.positive? + issue_types << "missing_town" if logs_impacted_by_missing_town_or_city.positive? + issue_types << "wrong_uprn" if logs_impacted_by_uprn_issue.any? + data_coordinators = organisation.users.where(role: 2).filter_by_active + users_to_contact = data_coordinators.any? ? data_coordinators : organisation.users.filter_by_active + EmailMissingAddressesCsvJob.perform_later(users_to_contact.map(&:id), organisation, "sales", issue_types, skip_uprn_issue_organisations) + Rails.logger.info("Sending missing sales addresses CSV for #{organisation.name} to #{users_to_contact.map(&:email).join(', ')}") + else + Rails.logger.info("Missing addresses below threshold for #{organisation.name}") + end + end + end + + desc "Send all 2023 lettings addresses csv" + task :create_lettings_addresses_csv, %i[organisation_id] => :environment do |_task, args| + organisation_id = args[:organisation_id] + raise "Usage: rake correct_addresses:create_lettings_addresses_csv['organisation_id']" if organisation_id.blank? + + organisation = Organisation.find_by(id: organisation_id) + if organisation.present? + CreateAddressesCsvJob.perform_later(organisation, "lettings") + Rails.logger.info("Creating lettings addresses CSV for #{organisation.name}") + else + Rails.logger.error("Organisation with ID #{organisation_id} not found") + end + end + + desc "Send all 2023 sales addresses csv" + task :create_sales_addresses_csv, %i[organisation_id] => :environment do |_task, args| + organisation_id = args[:organisation_id] + raise "Usage: rake correct_addresses:create_sales_addresses_csv['organisation_id']" if organisation_id.blank? + + organisation = Organisation.find_by(id: organisation_id) + if organisation.present? + CreateAddressesCsvJob.perform_later(organisation, "sales") + Rails.logger.info("Creating sales addresses CSV for #{organisation.name}") + else + Rails.logger.error("Organisation with ID #{organisation_id} not found") + end + end +end diff --git a/spec/factories/lettings_log.rb b/spec/factories/lettings_log.rb index 6cce1dfea..c85b4b00a 100644 --- a/spec/factories/lettings_log.rb +++ b/spec/factories/lettings_log.rb @@ -190,6 +190,9 @@ FactoryBot.define do status { 4 } discarded_at { Time.zone.now } end + trait :imported do + old_id { Random.hex } + end created_at { Time.zone.today } updated_at { Time.zone.today } end diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index 177ed600c..e930f3b2a 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -155,5 +155,8 @@ FactoryBot.define do status { 4 } discarded_at { Time.zone.now } end + trait :imported do + old_id { Random.hex } + end end end diff --git a/spec/fixtures/files/addresses_reimport.csv b/spec/fixtures/files/addresses_reimport.csv index 998d2fbfa..b797ac9b4 100644 --- a/spec/fixtures/files/addresses_reimport.csv +++ b/spec/fixtures/files/addresses_reimport.csv @@ -1,6 +1,7 @@ -lettings_log_id,address_line1,address_line2,town_or_city,postcode_full -{id},address 1,address 2,town,B1 1BB -{id2},address 3,address 4,city,B1 1BB -{id3},,,,C1 1CC -,address 5,address 6,city,D1 1DD -fake_id,address 7,address 8,city,D1 1DD +Issue type,Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Full address required,{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,address 1,address 2,town,county,B1 1BB +Missing town or city,{id2},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,address 3,,city,,B1 1BB +Incorrect UPRN,{id3},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,123,Some Place,,Bristol,,BS1 1AD +Incorrect UPRN,{id4},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,Some Place,,,,BS1 1AD +Incorrect UPRN,,2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,Some Place,,Bristol,,BS1 1AD +Incorrect UPRN,fake_id,2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,Some Place,,Bristol,,BS1 1AD diff --git a/spec/fixtures/files/addresses_reimport_all_logs.csv b/spec/fixtures/files/addresses_reimport_all_logs.csv new file mode 100644 index 000000000..0a1368d28 --- /dev/null +++ b/spec/fixtures/files/addresses_reimport_all_logs.csv @@ -0,0 +1,7 @@ +Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,address 1,address 2,town,county,B1 1BB +{id2},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,address 3,,city,,B1 1BB +{id3},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,123,Some Place,,Bristol,,BS1 1AD +{id4},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,Some Place,,,,BS1 1AD +,2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,Some Place,,Bristol,,BS1 1AD +fake_id,2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,Some Place,,Bristol,,BS1 1AD diff --git a/spec/fixtures/files/missing_lettings_logs_addresses.csv b/spec/fixtures/files/missing_lettings_logs_addresses.csv new file mode 100644 index 000000000..12475c026 --- /dev/null +++ b/spec/fixtures/files/missing_lettings_logs_addresses.csv @@ -0,0 +1,2 @@ +Issue type,Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Full address required,{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,,,,, diff --git a/spec/fixtures/files/missing_lettings_logs_addresses_all_issues.csv b/spec/fixtures/files/missing_lettings_logs_addresses_all_issues.csv new file mode 100644 index 000000000..ad6e1c865 --- /dev/null +++ b/spec/fixtures/files/missing_lettings_logs_addresses_all_issues.csv @@ -0,0 +1,4 @@ +Issue type,Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Full address required,{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,,,,, +Missing town or city,{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,existing address,,,, +Incorrect UPRN,{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,123,Some Place,,Bristol,,BS1 1AD diff --git a/spec/fixtures/files/missing_lettings_logs_town_or_city.csv b/spec/fixtures/files/missing_lettings_logs_town_or_city.csv new file mode 100644 index 000000000..4c8c65ea8 --- /dev/null +++ b/spec/fixtures/files/missing_lettings_logs_town_or_city.csv @@ -0,0 +1,2 @@ +Issue type,Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Missing town or city,{id},2023-04-05,tenancycode,propcode,testy@example.com,Address org,Address org,,existing address,,,, diff --git a/spec/fixtures/files/missing_lettings_logs_wrong_uprn.csv b/spec/fixtures/files/missing_lettings_logs_wrong_uprn.csv new file mode 100644 index 000000000..238cc5aef --- /dev/null +++ b/spec/fixtures/files/missing_lettings_logs_wrong_uprn.csv @@ -0,0 +1,2 @@ +Issue type,Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Incorrect UPRN,{id},2023-04-05,tenancycode,12,testy@example.com,Address org,Address org,12,Some Place,,Newcastle,,EC1N 2TD diff --git a/spec/fixtures/files/missing_sales_logs_addresses.csv b/spec/fixtures/files/missing_sales_logs_addresses.csv new file mode 100644 index 000000000..80892a2cf --- /dev/null +++ b/spec/fixtures/files/missing_sales_logs_addresses.csv @@ -0,0 +1,2 @@ +Issue type,Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Full address required,{id},2023-04-05,purchaser code,testy@example.com,Address org,,,,,, diff --git a/spec/fixtures/files/missing_sales_logs_addresses_all_issues.csv b/spec/fixtures/files/missing_sales_logs_addresses_all_issues.csv new file mode 100644 index 000000000..1ef3aecb6 --- /dev/null +++ b/spec/fixtures/files/missing_sales_logs_addresses_all_issues.csv @@ -0,0 +1,4 @@ +Issue type,Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Full address required,{id},2023-04-05,purchaser code,testy@example.com,Address org,,,,,, +Missing town or city,{id},2023-04-05,purchaser code,testy@example.com,Address org,,existing address line 1,,,, +Incorrect UPRN,{id},2023-04-05,purchaser code,testy@example.com,Address org,123,Some Place,,Bristol,,BS1 1AD diff --git a/spec/fixtures/files/missing_sales_logs_town_or_city.csv b/spec/fixtures/files/missing_sales_logs_town_or_city.csv new file mode 100644 index 000000000..b1f76611e --- /dev/null +++ b/spec/fixtures/files/missing_sales_logs_town_or_city.csv @@ -0,0 +1,2 @@ +Issue type,Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Missing town or city,{id},2023-04-05,purchaser code,testy@example.com,Address org,,existing address line 1,,,, diff --git a/spec/fixtures/files/missing_sales_logs_wrong_uprn.csv b/spec/fixtures/files/missing_sales_logs_wrong_uprn.csv new file mode 100644 index 000000000..1964dd673 --- /dev/null +++ b/spec/fixtures/files/missing_sales_logs_wrong_uprn.csv @@ -0,0 +1,2 @@ +Issue type,Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Incorrect UPRN,{id},2023-04-05,12,testy@example.com,Address org,12,Some Place,,Newcastle,,EC1N 2TD diff --git a/spec/fixtures/files/sales_addresses_reimport.csv b/spec/fixtures/files/sales_addresses_reimport.csv new file mode 100644 index 000000000..f9cd0bc13 --- /dev/null +++ b/spec/fixtures/files/sales_addresses_reimport.csv @@ -0,0 +1,7 @@ +Issue type,Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +Full address required,{id},2023-04-05,purchid,testy@example.com,Address org,,address 1,address 2,town,county,B1 1BB +Missing town or city,{id2},2023-04-05,purchid,testy@example.com,Address org,,address 3,,city,,B1 1BB +Incorrect UPRN,{id3},2023-04-05,purchid,testy@example.com,Address org,123,Some Place,,Bristol,,BS1 1AD +Incorrect UPRN,{id4},2023-04-05,purchid,testy@example.com,Address org,,Some Place,,,,BS1 1AD +Incorrect UPRN,,2023-04-05,purchid,testy@example.com,Address org,,Some Place,,Bristol,,BS1 1AD +Incorrect UPRN,fake_id,2023-04-05,purchid,testy@example.com,Address org,,Some Place,,Bristol,,BS1 1AD diff --git a/spec/fixtures/files/sales_addresses_reimport_all_logs.csv b/spec/fixtures/files/sales_addresses_reimport_all_logs.csv new file mode 100644 index 000000000..e9ebee3cb --- /dev/null +++ b/spec/fixtures/files/sales_addresses_reimport_all_logs.csv @@ -0,0 +1,7 @@ +Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode +{id},2023-04-05,purchid,testy@example.com,Address org,,address 1,address 2,town,county,B1 1BB +{id2},2023-04-05,purchid,testy@example.com,Address org,,address 3,,city,,B1 1BB +{id3},2023-04-05,purchid,testy@example.com,Address org,123,Some Place,,Bristol,,BS1 1AD +{id4},2023-04-05,purchid,testy@example.com,Address org,,Some Place,,,,BS1 1AD +,2023-04-05,purchid,testy@example.com,Address org,,Some Place,,Bristol,,BS1 1AD +fake_id,2023-04-05,purchid,testy@example.com,Address org,,Some Place,,Bristol,,BS1 1AD diff --git a/spec/jobs/create_addresses_csv_job_spec.rb b/spec/jobs/create_addresses_csv_job_spec.rb new file mode 100644 index 000000000..3e35feec4 --- /dev/null +++ b/spec/jobs/create_addresses_csv_job_spec.rb @@ -0,0 +1,49 @@ +require "rails_helper" + +describe CreateAddressesCsvJob do + include Helpers + + let(:job) { described_class.new } + let(:storage_service) { instance_double(Storage::S3Service) } + let(:mailer) { instance_double(CsvDownloadMailer) } + let(:missing_addresses_csv_service) { instance_double(Csv::MissingAddressesCsvService) } + let(:organisation) { build(:organisation) } + let(:users) { create_list(:user, 2) } + + before do + allow(Storage::S3Service).to receive(:new).and_return(storage_service) + allow(storage_service).to receive(:write_file) + + allow(Csv::MissingAddressesCsvService).to receive(:new).and_return(missing_addresses_csv_service) + allow(missing_addresses_csv_service).to receive(:create_lettings_addresses_csv).and_return("") + allow(missing_addresses_csv_service).to receive(:create_sales_addresses_csv).and_return("") + end + + context "when sending all lettings logs csv" do + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/lettings-logs-addresses-#{organisation.name}-.*\.csv/, anything) + expect(Rails.logger).to receive(:info).with(/Created addresses file: lettings-logs-addresses-#{organisation.name}-.*\.csv/) + job.perform(organisation, "lettings") + end + + it "creates a MissingAddressesCsvService with the correct organisation and calls create all lettings logs adresses csv" do + expect(Csv::MissingAddressesCsvService).to receive(:new).with(organisation, []) + expect(missing_addresses_csv_service).to receive(:create_lettings_addresses_csv) + job.perform(organisation, "lettings") + end + end + + context "when sending all sales logs csv" do + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/sales-logs-addresses-#{organisation.name}-.*\.csv/, anything) + expect(Rails.logger).to receive(:info).with(/Created addresses file: sales-logs-addresses-#{organisation.name}-.*\.csv/) + job.perform(organisation, "sales") + end + + it "creates a MissingAddressesCsvService with the correct organisation and calls create all sales logs adresses csv" do + expect(Csv::MissingAddressesCsvService).to receive(:new).with(organisation, []) + expect(missing_addresses_csv_service).to receive(:create_sales_addresses_csv) + job.perform(organisation, "sales") + end + end +end diff --git a/spec/jobs/email_missing_addresses_csv_job_spec.rb b/spec/jobs/email_missing_addresses_csv_job_spec.rb new file mode 100644 index 000000000..69d90cf51 --- /dev/null +++ b/spec/jobs/email_missing_addresses_csv_job_spec.rb @@ -0,0 +1,66 @@ +require "rails_helper" + +describe EmailMissingAddressesCsvJob do + include Helpers + + test_url = :test_url + + let(:job) { described_class.new } + let(:storage_service) { instance_double(Storage::S3Service) } + let(:mailer) { instance_double(CsvDownloadMailer) } + let(:missing_addresses_csv_service) { instance_double(Csv::MissingAddressesCsvService) } + let(:organisation) { build(:organisation) } + let(:users) { create_list(:user, 2) } + + before do + 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(Csv::MissingAddressesCsvService).to receive(:new).and_return(missing_addresses_csv_service) + allow(missing_addresses_csv_service).to receive(:create_missing_lettings_addresses_csv).and_return("") + allow(missing_addresses_csv_service).to receive(:create_missing_sales_addresses_csv).and_return("") + + allow(CsvDownloadMailer).to receive(:new).and_return(mailer) + allow(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail) + allow(mailer).to receive(:send_missing_sales_addresses_csv_download_mail) + end + + context "when sending missing lettings logs csv" do + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/missing-lettings-logs-addresses-#{organisation.name}-.*\.csv/, anything) + job.perform(users.map(&:id), organisation, "lettings", %w[missing_address wrong_uprn], [1, 2]) + end + + it "creates a MissingAddressesCsvService with the correct organisation and calls create missing lettings logs adresses csv" do + expect(Csv::MissingAddressesCsvService).to receive(:new).with(organisation, [1, 2]) + expect(missing_addresses_csv_service).to receive(:create_missing_lettings_addresses_csv) + job.perform(users.map(&:id), organisation, "lettings", %w[missing_address wrong_uprn], [1, 2]) + end + + it "sends emails to all the provided users" do + expect(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail).with(users[0], test_url, instance_of(Integer), %w[missing_address wrong_uprn]) + expect(mailer).to receive(:send_missing_lettings_addresses_csv_download_mail).with(users[1], test_url, instance_of(Integer), %w[missing_address wrong_uprn]) + job.perform(users.map(&:id), organisation, "lettings", %w[missing_address wrong_uprn], [1, 2]) + end + end + + context "when sending missing sales logs csv" do + it "uses an appropriate filename in S3" do + expect(storage_service).to receive(:write_file).with(/missing-sales-logs-addresses-#{organisation.name}-.*\.csv/, anything) + job.perform(users.map(&:id), organisation, "sales", %w[missing_town], [2, 3]) + end + + it "creates a MissingAddressesCsvService with the correct organisation and calls create missing sales logs adresses csv" do + expect(Csv::MissingAddressesCsvService).to receive(:new).with(organisation, [2, 3]) + expect(missing_addresses_csv_service).to receive(:create_missing_sales_addresses_csv) + job.perform(users.map(&:id), organisation, "sales", %w[missing_town], [2, 3]) + end + + it "sends emails to all the provided users" do + expect(mailer).to receive(:send_missing_sales_addresses_csv_download_mail).with(users[0], test_url, instance_of(Integer), %w[missing_town]) + expect(mailer).to receive(:send_missing_sales_addresses_csv_download_mail).with(users[1], test_url, instance_of(Integer), %w[missing_town]) + job.perform(users.map(&:id), organisation, "sales", %w[missing_town], [2, 3]) + end + end +end diff --git a/spec/lib/tasks/correct_address_from_csv_spec.rb b/spec/lib/tasks/correct_address_from_csv_spec.rb index 0a3d1c563..294ce535b 100644 --- a/spec/lib/tasks/correct_address_from_csv_spec.rb +++ b/spec/lib/tasks/correct_address_from_csv_spec.rb @@ -2,14 +2,45 @@ require "rails_helper" require "rake" RSpec.describe "data_import" do - def replace_entity_ids(lettings_log, second_lettings_log, third_lettings_log, export_template) - export_template.sub!(/\{id\}/, lettings_log.id.to_s) - export_template.sub!(/\{id2\}/, second_lettings_log.id.to_s) - export_template.sub!(/\{id3\}/, third_lettings_log.id.to_s) + def replace_entity_ids(log, second_log, third_log, fourth_log, export_template) + export_template.sub!(/\{id\}/, log.id.to_s) + export_template.sub!(/\{id2\}/, second_log.id.to_s) + export_template.sub!(/\{id3\}/, third_log.id.to_s) + export_template.sub!(/\{id4\}/, fourth_log.id.to_s) end - describe ":import_address_from_csv", type: :task do - subject(:task) { Rake::Task["data_import:import_address_from_csv"] } + before do + allow(Storage::S3Service).to receive(:new).and_return(storage_service) + allow(Configuration::EnvConfigurationService).to receive(:new).and_return(env_config_service) + allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service) + allow(ENV).to receive(:[]) + allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name) + allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy") + + WebMock.stub_request(:get, /api\.postcodes\.io/) + .to_return(status: 200, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) + WebMock.stub_request(:get, /api\.postcodes\.io\/postcodes\/B11BB/) + .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {}) + + body = { + results: [ + { + DPA: { + "POSTCODE": "LS16 6FT", + "POST_TOWN": "Westminster", + "PO_BOX_NUMBER": "Wrong Address Line1", + "DOUBLE_DEPENDENT_LOCALITY": "Double Dependent Locality", + }, + }, + ], + }.to_json + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key&uprn=1") + .to_return(status: 200, body:, headers: {}) + end + + describe ":import_lettings_addresses_from_csv", type: :task do + subject(:task) { Rake::Task["data_import:import_lettings_addresses_from_csv"] } let(:instance_name) { "paas_import_instance" } let(:storage_service) { instance_double(Storage::S3Service) } @@ -17,25 +48,14 @@ RSpec.describe "data_import" do let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) } before do - allow(Storage::S3Service).to receive(:new).and_return(storage_service) - allow(Configuration::EnvConfigurationService).to receive(:new).and_return(env_config_service) - allow(Configuration::PaasConfigurationService).to receive(:new).and_return(paas_config_service) - allow(ENV).to receive(:[]) - allow(ENV).to receive(:[]).with("IMPORT_PAAS_INSTANCE").and_return(instance_name) - allow(ENV).to receive(:[]).with("VCAP_SERVICES").and_return("dummy") - Rake.application.rake_require("tasks/import_address_from_csv") Rake::Task.define_task(:environment) task.reenable - - WebMock.stub_request(:get, /api.postcodes.io/) - .to_return(status: 200, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) - WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/B11BB/) - .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {}) end context "when the rake task is run" do let(:addresses_csv_path) { "addresses_reimport_123.csv" } + let(:all_addresses_csv_path) { "all_addresses_reimport_123.csv" } let(:wrong_file_path) { "/test/no_csv_here.csv" } let!(:lettings_log) do create(:lettings_log, @@ -52,102 +72,427 @@ RSpec.describe "data_import" do is_la_inferred: true) end - let!(:second_lettings_log) do - create(:lettings_log, - uprn_known: 1, - uprn: "1", - uprn_confirmed: nil, - address_line1: "wrong address line1", - address_line2: "wrong address 2", - town_or_city: "wrong town", - county: "wrong city", - postcode_known: 1, - postcode_full: "A1 1AA", - la: "E06000064", - is_la_inferred: true) + let!(:lettings_logs) do + create_list(:lettings_log, + 3, + uprn_known: 1, + uprn: "1", + uprn_confirmed: nil, + address_line1: "wrong address line1", + address_line2: "wrong address 2", + town_or_city: "wrong town", + county: "wrong city", + postcode_known: 1, + postcode_full: "A1 1AA", + la: "E06000064", + is_la_inferred: true) end - let!(:third_lettings_log) do - create(:lettings_log, - uprn_known: 1, - uprn: "1", + before do + allow(storage_service).to receive(:get_file_io) + .with("addresses_reimport_123.csv") + .and_return(replace_entity_ids(lettings_log, lettings_logs[0], lettings_logs[1], lettings_logs[2], File.open("./spec/fixtures/files/addresses_reimport.csv").read)) + + allow(storage_service).to receive(:get_file_io) + .with("all_addresses_reimport_123.csv") + .and_return(replace_entity_ids(lettings_log, lettings_logs[0], lettings_logs[1], lettings_logs[2], File.open("./spec/fixtures/files/addresses_reimport_all_logs.csv").read)) + end + + context "when the file contains issue type column" do + it "updates the log address when old address was not given" do + task.invoke(addresses_csv_path) + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.address_line1).to eq("address 1") + expect(lettings_log.address_line2).to eq("address 2") + expect(lettings_log.town_or_city).to eq("town") + expect(lettings_log.county).to eq("county") + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("B1 1BB") + expect(lettings_log.la).to eq("E08000035") + expect(lettings_log.is_la_inferred).to eq(true) + end + + it "updates the log address when old address was given" do + task.invoke(addresses_csv_path) + lettings_logs[0].reload + expect(lettings_logs[0].uprn_known).to eq(0) + expect(lettings_logs[0].uprn).to eq(nil) + expect(lettings_logs[0].uprn_confirmed).to eq(nil) + expect(lettings_logs[0].address_line1).to eq("address 3") + expect(lettings_logs[0].address_line2).to eq(nil) + expect(lettings_logs[0].town_or_city).to eq("city") + expect(lettings_logs[0].county).to eq(nil) + expect(lettings_logs[0].postcode_known).to eq(1) + expect(lettings_logs[0].postcode_full).to eq("B1 1BB") + expect(lettings_logs[0].la).to eq("E08000035") + expect(lettings_logs[0].is_la_inferred).to eq(true) + end + + it "does not update log address when uprn is given" do + task.invoke(addresses_csv_path) + lettings_logs[1].reload + expect(lettings_logs[1].uprn_known).to eq(1) + expect(lettings_logs[1].uprn).to eq("1") + expect(lettings_logs[1].uprn_confirmed).to eq(nil) + expect(lettings_logs[1].address_line1).to eq("wrong address line1") + expect(lettings_logs[1].address_line2).to eq("wrong address 2") + expect(lettings_logs[1].town_or_city).to eq("wrong town") + expect(lettings_logs[1].county).to eq("wrong city") + expect(lettings_logs[1].postcode_known).to eq(1) + expect(lettings_logs[1].postcode_full).to eq("A1 1AA") + expect(lettings_logs[1].la).to eq("E06000064") + end + + it "does not update log address when all required address fields are not present" do + task.invoke(addresses_csv_path) + lettings_logs[2].reload + expect(lettings_logs[2].uprn_known).to eq(1) + expect(lettings_logs[2].uprn).to eq("1") + expect(lettings_logs[2].uprn_confirmed).to eq(nil) + expect(lettings_logs[2].address_line1).to eq("wrong address line1") + expect(lettings_logs[2].address_line2).to eq("wrong address 2") + expect(lettings_logs[2].town_or_city).to eq("wrong town") + expect(lettings_logs[2].county).to eq("wrong city") + expect(lettings_logs[2].postcode_known).to eq(1) + expect(lettings_logs[2].postcode_full).to eq("A1 1AA") + expect(lettings_logs[2].la).to eq("E06000064") + end + + it "reinfers the LA if the postcode doesn't change" do + lettings_log.update!(postcode_full: "B1 1BB") + task.invoke(addresses_csv_path) + lettings_log.reload + expect(lettings_log.postcode_full).to eq("B1 1BB") + expect(lettings_log.la).to eq("E08000035") + expect(lettings_log.is_la_inferred).to eq(true) + end + + it "logs the progress of the update" do + expect(Rails.logger).to receive(:info).with("Updated lettings log #{lettings_log.id}, with address: address 1, address 2, town, county, B1 1BB") + expect(Rails.logger).to receive(:info).with("Updated lettings log #{lettings_logs[0].id}, with address: address 3, , city, , B1 1BB") + expect(Rails.logger).to receive(:info).with("Lettings log with ID #{lettings_logs[1].id} contains uprn, skipping log") + expect(Rails.logger).to receive(:info).with("Lettings log with ID #{lettings_logs[2].id} is missing required address data, skipping log") + expect(Rails.logger).to receive(:info).with("Lettings log ID not provided for address: Some Place, , Bristol, , BS1 1AD") + expect(Rails.logger).to receive(:info).with("Could not find a lettings log with id fake_id") + + task.invoke(addresses_csv_path) + end + + it "raises an error when no path is given" do + expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:import_lettings_addresses_from_csv['csv_file_name']") + end + end + + context "when the file does not contain issue type column" do + it "updates the log address when old address was not given" do + task.invoke(all_addresses_csv_path) + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.address_line1).to eq("address 1") + expect(lettings_log.address_line2).to eq("address 2") + expect(lettings_log.town_or_city).to eq("town") + expect(lettings_log.county).to eq("county") + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("B1 1BB") + expect(lettings_log.la).to eq("E08000035") + expect(lettings_log.is_la_inferred).to eq(true) + end + + it "updates the log address when old address was given" do + task.invoke(all_addresses_csv_path) + lettings_logs[0].reload + expect(lettings_logs[0].uprn_known).to eq(0) + expect(lettings_logs[0].uprn).to eq(nil) + expect(lettings_logs[0].uprn_confirmed).to eq(nil) + expect(lettings_logs[0].address_line1).to eq("address 3") + expect(lettings_logs[0].address_line2).to eq(nil) + expect(lettings_logs[0].town_or_city).to eq("city") + expect(lettings_logs[0].county).to eq(nil) + expect(lettings_logs[0].postcode_known).to eq(1) + expect(lettings_logs[0].postcode_full).to eq("B1 1BB") + expect(lettings_logs[0].la).to eq("E08000035") + expect(lettings_logs[0].is_la_inferred).to eq(true) + end + + it "does not update log address when uprn is given" do + task.invoke(all_addresses_csv_path) + lettings_logs[1].reload + expect(lettings_logs[1].uprn_known).to eq(1) + expect(lettings_logs[1].uprn).to eq("1") + expect(lettings_logs[1].uprn_confirmed).to eq(nil) + expect(lettings_logs[1].address_line1).to eq("wrong address line1") + expect(lettings_logs[1].address_line2).to eq("wrong address 2") + expect(lettings_logs[1].town_or_city).to eq("wrong town") + expect(lettings_logs[1].county).to eq("wrong city") + expect(lettings_logs[1].postcode_known).to eq(1) + expect(lettings_logs[1].postcode_full).to eq("A1 1AA") + expect(lettings_logs[1].la).to eq("E06000064") + end + + it "does not update log address when all required address fields are not present" do + task.invoke(all_addresses_csv_path) + lettings_logs[2].reload + expect(lettings_logs[2].uprn_known).to eq(1) + expect(lettings_logs[2].uprn).to eq("1") + expect(lettings_logs[2].uprn_confirmed).to eq(nil) + expect(lettings_logs[2].address_line1).to eq("wrong address line1") + expect(lettings_logs[2].address_line2).to eq("wrong address 2") + expect(lettings_logs[2].town_or_city).to eq("wrong town") + expect(lettings_logs[2].county).to eq("wrong city") + expect(lettings_logs[2].postcode_known).to eq(1) + expect(lettings_logs[2].postcode_full).to eq("A1 1AA") + expect(lettings_logs[2].la).to eq("E06000064") + end + + it "reinfers the LA if the postcode hasn't changed" do + lettings_log.update!(postcode_full: "B1 1BB") + task.invoke(all_addresses_csv_path) + lettings_log.reload + expect(lettings_log.postcode_full).to eq("B1 1BB") + expect(lettings_log.la).to eq("E08000035") + expect(lettings_log.is_la_inferred).to eq(true) + end + + it "logs the progress of the update" do + expect(Rails.logger).to receive(:info).with("Updated lettings log #{lettings_log.id}, with address: address 1, address 2, town, county, B1 1BB") + expect(Rails.logger).to receive(:info).with("Updated lettings log #{lettings_logs[0].id}, with address: address 3, , city, , B1 1BB") + expect(Rails.logger).to receive(:info).with("Lettings log with ID #{lettings_logs[1].id} contains uprn, skipping log") + expect(Rails.logger).to receive(:info).with("Lettings log with ID #{lettings_logs[2].id} is missing required address data, skipping log") + expect(Rails.logger).to receive(:info).with("Lettings log ID not provided for address: Some Place, , Bristol, , BS1 1AD") + expect(Rails.logger).to receive(:info).with("Could not find a lettings log with id fake_id") + + task.invoke(all_addresses_csv_path) + end + + it "raises an error when no path is given" do + expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:import_lettings_addresses_from_csv['csv_file_name']") + end + end + end + end + + describe ":import_sales_addresses_from_csv", type: :task do + subject(:task) { Rake::Task["data_import:import_sales_addresses_from_csv"] } + + let(:instance_name) { "paas_import_instance" } + let(:storage_service) { instance_double(Storage::S3Service) } + let(:env_config_service) { instance_double(Configuration::EnvConfigurationService) } + let(:paas_config_service) { instance_double(Configuration::PaasConfigurationService) } + + before do + Rake.application.rake_require("tasks/import_address_from_csv") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the rake task is run" do + let(:addresses_csv_path) { "addresses_reimport_123.csv" } + let(:all_addresses_csv_path) { "all_addresses_reimport_123.csv" } + let(:wrong_file_path) { "/test/no_csv_here.csv" } + let!(:sales_log) do + create(:sales_log, + :completed, + uprn_known: nil, + uprn: nil, uprn_confirmed: nil, - address_line1: "wrong address line1", - address_line2: "wrong address 2", - town_or_city: "wrong town", - county: "wrong city", - postcode_known: 1, - postcode_full: "A1 1AA", + address_line1: nil, + address_line2: nil, + town_or_city: nil, la: "E06000064", is_la_inferred: true) end + let!(:sales_logs) { create_list(:sales_log, 3, :completed, uprn_known: 1, uprn: "1", la: "E06000064", is_la_inferred: true) } + before do allow(storage_service).to receive(:get_file_io) .with("addresses_reimport_123.csv") - .and_return(replace_entity_ids(lettings_log, second_lettings_log, third_lettings_log, File.open("./spec/fixtures/files/addresses_reimport.csv").read)) - end + .and_return(replace_entity_ids(sales_log, sales_logs[0], sales_logs[1], sales_logs[2], File.open("./spec/fixtures/files/sales_addresses_reimport.csv").read)) - it "updates the log address when old address was not given" do - task.invoke(addresses_csv_path) - lettings_log.reload - expect(lettings_log.uprn_known).to eq(0) - expect(lettings_log.uprn).to eq(nil) - expect(lettings_log.uprn_confirmed).to eq(nil) - expect(lettings_log.address_line1).to eq("address 1") - expect(lettings_log.address_line2).to eq("address 2") - expect(lettings_log.town_or_city).to eq("town") - expect(lettings_log.county).to eq(nil) - expect(lettings_log.postcode_known).to eq(1) - expect(lettings_log.postcode_full).to eq("B1 1BB") - expect(lettings_log.la).to eq("E08000035") - expect(lettings_log.is_la_inferred).to eq(true) + allow(storage_service).to receive(:get_file_io) + .with("all_addresses_reimport_123.csv") + .and_return(replace_entity_ids(sales_log, sales_logs[0], sales_logs[1], sales_logs[2], File.open("./spec/fixtures/files/sales_addresses_reimport_all_logs.csv").read)) end - it "updates the log address when old address was given" do - task.invoke(addresses_csv_path) - second_lettings_log.reload - expect(second_lettings_log.uprn_known).to eq(0) - expect(second_lettings_log.uprn).to eq(nil) - expect(second_lettings_log.uprn_confirmed).to eq(nil) - expect(second_lettings_log.address_line1).to eq("address 3") - expect(second_lettings_log.address_line2).to eq("address 4") - expect(second_lettings_log.town_or_city).to eq("city") - expect(second_lettings_log.county).to eq(nil) - expect(second_lettings_log.postcode_known).to eq(1) - expect(second_lettings_log.postcode_full).to eq("B1 1BB") - expect(second_lettings_log.la).to eq("E08000035") - expect(second_lettings_log.is_la_inferred).to eq(true) - end + context "when the file contains issue type column" do + it "updates the log address when old address was not given" do + task.invoke(addresses_csv_path) + sales_log.reload + expect(sales_log.uprn_known).to eq(0) + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.address_line1).to eq("address 1") + expect(sales_log.address_line2).to eq("address 2") + expect(sales_log.town_or_city).to eq("town") + expect(sales_log.county).to eq("county") + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("B1 1BB") + expect(sales_log.la).to eq("E08000035") + expect(sales_log.is_la_inferred).to eq(true) + end - it "updates the log address when new address is not given" do - task.invoke(addresses_csv_path) - third_lettings_log.reload - expect(third_lettings_log.uprn_known).to eq(0) - expect(third_lettings_log.uprn).to eq(nil) - expect(third_lettings_log.uprn_confirmed).to eq(nil) - expect(third_lettings_log.address_line1).to eq(nil) - expect(third_lettings_log.address_line2).to eq(nil) - expect(third_lettings_log.town_or_city).to eq(nil) - expect(third_lettings_log.county).to eq(nil) - expect(third_lettings_log.postcode_known).to eq(1) - expect(third_lettings_log.postcode_full).to eq("C11CC") - expect(third_lettings_log.la).to eq(nil) - expect(third_lettings_log.is_la_inferred).to eq(false) - end + it "updates the log address when old address was given" do + task.invoke(addresses_csv_path) + sales_logs[0].reload + expect(sales_logs[0].uprn_known).to eq(0) + expect(sales_logs[0].uprn).to eq(nil) + expect(sales_logs[0].uprn_confirmed).to eq(nil) + expect(sales_logs[0].address_line1).to eq("address 3") + expect(sales_logs[0].address_line2).to eq(nil) + expect(sales_logs[0].town_or_city).to eq("city") + expect(sales_logs[0].county).to eq(nil) + expect(sales_logs[0].pcodenk).to eq(0) + expect(sales_logs[0].postcode_full).to eq("B1 1BB") + expect(sales_logs[0].la).to eq("E08000035") + expect(sales_logs[0].is_la_inferred).to eq(true) + end + + it "does not update log address when uprn is given" do + task.invoke(addresses_csv_path) + sales_logs[1].reload + expect(sales_logs[1].uprn_known).to eq(1) + expect(sales_logs[1].uprn).to eq("1") + expect(sales_logs[1].uprn_confirmed).to eq(nil) + expect(sales_logs[1].address_line1).to eq("Wrong Address Line1") + expect(sales_logs[1].address_line2).to eq("Double Dependent Locality") + expect(sales_logs[1].town_or_city).to eq("Westminster") + expect(sales_logs[1].county).to eq(nil) + expect(sales_logs[1].pcodenk).to eq(0) + expect(sales_logs[1].postcode_full).to eq("LS16 6FT") + expect(sales_logs[1].la).to eq("E06000064") + end + + it "does not update log address when all required address fields are not present" do + task.invoke(addresses_csv_path) + sales_logs[2].reload + expect(sales_logs[2].uprn_known).to eq(1) + expect(sales_logs[2].uprn).to eq("1") + expect(sales_logs[2].uprn_confirmed).to eq(nil) + expect(sales_logs[2].address_line1).to eq("Wrong Address Line1") + expect(sales_logs[2].address_line2).to eq("Double Dependent Locality") + expect(sales_logs[2].town_or_city).to eq("Westminster") + expect(sales_logs[2].county).to eq(nil) + expect(sales_logs[2].pcodenk).to eq(0) + expect(sales_logs[2].postcode_full).to eq("LS16 6FT") + expect(sales_logs[2].la).to eq("E06000064") + end - it "logs the progress of the update" do - expect(Rails.logger).to receive(:info).with("Updated lettings log #{lettings_log.id}, with address: address 1, address 2, town, B1 1BB") - expect(Rails.logger).to receive(:info).with("Updated lettings log #{second_lettings_log.id}, with address: address 3, address 4, city, B1 1BB") - expect(Rails.logger).to receive(:info).with("Updated lettings log #{third_lettings_log.id}, with address: , , , C11CC") - expect(Rails.logger).to receive(:info).with("Lettings log ID not provided for address: address 5, address 6, city, D1 1DD") - expect(Rails.logger).to receive(:info).with("Could not find a lettings log with id fake_id") + it "reinfers the LA if the postcode hasn't changed" do + sales_log.update!(postcode_full: "B1 1BB") + task.invoke(addresses_csv_path) + sales_log.reload + expect(sales_log.postcode_full).to eq("B1 1BB") + expect(sales_log.la).to eq("E08000035") + expect(sales_log.is_la_inferred).to eq(true) + end - task.invoke(addresses_csv_path) + it "logs the progress of the update" do + expect(Rails.logger).to receive(:info).with("Updated sales log #{sales_log.id}, with address: address 1, address 2, town, county, B1 1BB") + expect(Rails.logger).to receive(:info).with("Updated sales log #{sales_logs[0].id}, with address: address 3, , city, , B1 1BB") + expect(Rails.logger).to receive(:info).with("Sales log with ID #{sales_logs[1].id} contains uprn, skipping log") + expect(Rails.logger).to receive(:info).with("Sales log with ID #{sales_logs[2].id} is missing required address data, skipping log") + expect(Rails.logger).to receive(:info).with("Sales log ID not provided for address: Some Place, , Bristol, , BS1 1AD") + expect(Rails.logger).to receive(:info).with("Could not find a sales log with id fake_id") + + task.invoke(addresses_csv_path) + end + + it "raises an error when no path is given" do + expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:import_sales_addresses_from_csv['csv_file_name']") + end end - it "raises an error when no path is given" do - expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:import_address_from_csv['csv_file_name']") + context "when the file does not contain issue type column" do + it "updates the log address when old address was not given" do + task.invoke(all_addresses_csv_path) + sales_log.reload + expect(sales_log.uprn_known).to eq(0) + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.address_line1).to eq("address 1") + expect(sales_log.address_line2).to eq("address 2") + expect(sales_log.town_or_city).to eq("town") + expect(sales_log.county).to eq("county") + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("B1 1BB") + expect(sales_log.la).to eq("E08000035") + expect(sales_log.is_la_inferred).to eq(true) + end + + it "updates the log address when old address was given" do + task.invoke(all_addresses_csv_path) + sales_logs[0].reload + expect(sales_logs[0].uprn_known).to eq(0) + expect(sales_logs[0].uprn).to eq(nil) + expect(sales_logs[0].uprn_confirmed).to eq(nil) + expect(sales_logs[0].address_line1).to eq("address 3") + expect(sales_logs[0].address_line2).to eq(nil) + expect(sales_logs[0].town_or_city).to eq("city") + expect(sales_logs[0].county).to eq(nil) + expect(sales_logs[0].pcodenk).to eq(0) + expect(sales_logs[0].postcode_full).to eq("B1 1BB") + expect(sales_logs[0].la).to eq("E08000035") + expect(sales_logs[0].is_la_inferred).to eq(true) + end + + it "does not update log address when uprn is given" do + task.invoke(all_addresses_csv_path) + sales_logs[1].reload + expect(sales_logs[1].uprn_known).to eq(1) + expect(sales_logs[1].uprn).to eq("1") + expect(sales_logs[1].uprn_confirmed).to eq(nil) + expect(sales_logs[1].address_line1).to eq("Wrong Address Line1") + expect(sales_logs[1].address_line2).to eq("Double Dependent Locality") + expect(sales_logs[1].town_or_city).to eq("Westminster") + expect(sales_logs[1].county).to eq(nil) + expect(sales_logs[1].pcodenk).to eq(0) + expect(sales_logs[1].postcode_full).to eq("LS16 6FT") + expect(sales_logs[1].la).to eq("E06000064") + end + + it "does not update log address when all required address fields are not present" do + task.invoke(all_addresses_csv_path) + sales_logs[2].reload + expect(sales_logs[2].uprn_known).to eq(1) + expect(sales_logs[2].uprn).to eq("1") + expect(sales_logs[2].uprn_confirmed).to eq(nil) + expect(sales_logs[2].address_line1).to eq("Wrong Address Line1") + expect(sales_logs[2].address_line2).to eq("Double Dependent Locality") + expect(sales_logs[2].town_or_city).to eq("Westminster") + expect(sales_logs[2].county).to eq(nil) + expect(sales_logs[2].pcodenk).to eq(0) + expect(sales_logs[2].postcode_full).to eq("LS16 6FT") + expect(sales_logs[2].la).to eq("E06000064") + end + + it "reinfers the LA if the postcode hasn't changed" do + sales_log.update!(postcode_full: "B1 1BB") + task.invoke(all_addresses_csv_path) + sales_log.reload + expect(sales_log.postcode_full).to eq("B1 1BB") + expect(sales_log.la).to eq("E08000035") + expect(sales_log.is_la_inferred).to eq(true) + end + + it "logs the progress of the update" do + expect(Rails.logger).to receive(:info).with("Updated sales log #{sales_log.id}, with address: address 1, address 2, town, county, B1 1BB") + expect(Rails.logger).to receive(:info).with("Updated sales log #{sales_logs[0].id}, with address: address 3, , city, , B1 1BB") + expect(Rails.logger).to receive(:info).with("Sales log with ID #{sales_logs[1].id} contains uprn, skipping log") + expect(Rails.logger).to receive(:info).with("Sales log with ID #{sales_logs[2].id} is missing required address data, skipping log") + expect(Rails.logger).to receive(:info).with("Sales log ID not provided for address: Some Place, , Bristol, , BS1 1AD") + expect(Rails.logger).to receive(:info).with("Could not find a sales log with id fake_id") + + task.invoke(all_addresses_csv_path) + end + + it "raises an error when no path is given" do + expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:import_sales_addresses_from_csv['csv_file_name']") + end end end end diff --git a/spec/lib/tasks/send_missing_addresses_csv_spec.rb b/spec/lib/tasks/send_missing_addresses_csv_spec.rb new file mode 100644 index 000000000..dab36f1ad --- /dev/null +++ b/spec/lib/tasks/send_missing_addresses_csv_spec.rb @@ -0,0 +1,560 @@ +require "rails_helper" +require "rake" + +RSpec.describe "correct_addresses" do + describe ":send_missing_addresses_lettings_csv", type: :task do + subject(:task) { Rake::Task["correct_addresses:send_missing_addresses_lettings_csv"] } + + before do + organisation.users.destroy_all + Rake.application.rake_require("tasks/send_missing_addresses_csv") + Rake::Task.define_task(:environment) + task.reenable + + body_1 = { + results: [ + { + DPA: { + "POSTCODE": "BS1 1AD", + "POST_TOWN": "Bristol", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + body_2 = { + results: [ + { + DPA: { + "POSTCODE": "EC1N 2TD", + "POST_TOWN": "Newcastle", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=123") + .to_return(status: 200, body: body_1, headers: {}) + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=12") + .to_return(status: 200, body: body_2, headers: {}) + end + + context "when the rake task is run" do + let(:organisation) { create(:organisation, name: "test organisation") } + + before do + stub_const("EmailMissingAddressesCsvJob::MISSING_ADDRESSES_THRESHOLD", 5) + end + + context "when org has more than 5 missing addresses and data coordinators" do + let!(:data_coordinator) { create(:user, :data_coordinator, organisation:, email: "data_coordinator1@example.com") } + let!(:data_coordinator2) { create(:user, :data_coordinator, organisation:, email: "data_coordinator2@example.com") } + + before do + create(:user, :data_provider, organisation:, email: "data_provider1@example.com") + create_list(:lettings_log, 7, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, needstype: 1, old_form_id: "form_1", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", %w[missing_address], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_coordinator1@example.com, data_coordinator2@example.com") + task.invoke + end + end + + context "when org has 5 missing addresses and data providers only" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, needstype: 1, old_form_id: "form_2", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[missing_address], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org has less than 5 missing addresses" do + before do + create_list(:lettings_log, 3, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, needstype: 1, old_form_id: "form_2", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + create_list(:lettings_log, 2, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: nil, needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job with organisations that is missing less addresses than threshold amount" do + expect { task.invoke }.not_to enqueue_job(EmailMissingAddressesCsvJob) + end + end + + context "when org has more than 5 missing town_or_city and data coordinators" do + let!(:data_coordinator) { create(:user, :data_coordinator, organisation:, email: "data_coordinator1@example.com") } + let!(:data_coordinator2) { create(:user, :data_coordinator, organisation:, email: "data_coordinator2@example.com") } + + before do + create(:user, :data_provider, organisation:, email: "data_provider1@example.com") + create_list(:lettings_log, 7, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: "exists", town_or_city: nil, needstype: 1, old_form_id: "form_1", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", %w[missing_town], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_coordinator1@example.com, data_coordinator2@example.com") + task.invoke + end + end + + context "when org has 5 missing town or city and data providers only" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: "exists", town_or_city: nil, needstype: 1, old_form_id: "form_2", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[missing_town], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org has less than 5 missing town or city" do + before do + create_list(:lettings_log, 3, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: "address", town_or_city: nil, needstype: 1, old_form_id: "form_2", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + create_list(:lettings_log, 2, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: "address", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job with organisations that is missing less town or city data than threshold amount" do + expect { task.invoke }.not_to enqueue_job(EmailMissingAddressesCsvJob) + end + end + + context "when org has more than 5 wrong uprn and data coordinators" do + let!(:data_coordinator) { create(:user, :data_coordinator, organisation:, email: "data_coordinator1@example.com") } + let!(:data_coordinator2) { create(:user, :data_coordinator, organisation:, email: "data_coordinator2@example.com") } + + before do + create(:user, :data_provider, organisation:, email: "data_provider1@example.com") + create_list(:lettings_log, 7, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "123", town_or_city: "Bristol", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "lettings", %w[bristol_uprn], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_coordinator1@example.com, data_coordinator2@example.com") + task.invoke + end + end + + context "when org has 5 wrong uprn and data providers only" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "12", propcode: "12", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[wrong_uprn], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org has less than 5 wrong uprn" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:lettings_log, 3, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "123", town_or_city: "Bristol", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + create_list(:lettings_log, 2, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "12", tenancycode: "12", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[wrong_uprn bristol_uprn], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing lettings addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org is included in skip_uprn_issue_organisations list" do + before do + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "12", propcode: "12", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job" do + expect { task.invoke(organisation.id.to_s) }.not_to enqueue_job(EmailMissingAddressesCsvJob) + end + end + + context "when org is included in skip_uprn_issue_organisations list but faces a different issue" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, needstype: 1, old_form_id: "form_2", owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "12", propcode: "12", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job" do + expect { task.invoke(organisation.id.to_s) }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[missing_address], [organisation.id]) + end + end + + context "when skip_uprn_issue_organisations list is provided" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:lettings_log, 5, :imported, startdate: Time.zone.local(2023, 9, 9), uprn: "12", propcode: "12", needstype: 1, owning_organisation: organisation, managing_organisation: organisation, created_by: organisation.users.first) + end + + it "does enqueues the job with correct skip_uprn_issue_organisations" do + expect { task.invoke("100 400") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "lettings", %w[wrong_uprn], [100, 400]) + end + end + end + end + + describe ":send_missing_addresses_sales_csv", type: :task do + subject(:task) { Rake::Task["correct_addresses:send_missing_addresses_sales_csv"] } + + before do + organisation.users.destroy_all + Rake.application.rake_require("tasks/send_missing_addresses_csv") + Rake::Task.define_task(:environment) + task.reenable + + body_1 = { + results: [ + { + DPA: { + "POSTCODE": "BS1 1AD", + "POST_TOWN": "Bristol", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + body_2 = { + results: [ + { + DPA: { + "POSTCODE": "EC1N 2TD", + "POST_TOWN": "Newcastle", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=123") + .to_return(status: 200, body: body_1, headers: {}) + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=12") + .to_return(status: 200, body: body_2, headers: {}) + end + + context "when the rake task is run" do + let(:organisation) { create(:organisation, name: "test organisation") } + + before do + stub_const("EmailMissingAddressesCsvJob::MISSING_ADDRESSES_THRESHOLD", 5) + end + + context "when org has more than 5 missing addresses and data coordinators" do + let!(:data_coordinator) { create(:user, :data_coordinator, organisation:, email: "data_coordinator1@example.com") } + let!(:data_coordinator2) { create(:user, :data_coordinator, organisation:, email: "data_coordinator2@example.com") } + + before do + create(:user, :data_provider, organisation:, email: "data_provider1@example.com") + create_list(:sales_log, 7, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, old_form_id: "form_1", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke("70 90") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", %w[missing_address], [70, 90]) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_coordinator1@example.com, data_coordinator2@example.com") + task.invoke + end + end + + context "when org has 5 missing addresses and data providers only" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:sales_log, 5, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, old_form_id: "form_2", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[missing_address], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org has less than 5 missing addresses" do + before do + create_list(:sales_log, 3, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: nil, town_or_city: nil, old_form_id: "form_2", owning_organisation: organisation, created_by: organisation.users.first) + create_list(:sales_log, 2, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: nil, owning_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job with organisations that is missing less addresses than threshold amount" do + expect { task.invoke }.not_to enqueue_job(EmailMissingAddressesCsvJob) + end + end + + context "when org has more than 5 missing town_or_city and data coordinators" do + let!(:data_coordinator) { create(:user, :data_coordinator, organisation:, email: "data_coordinator1@example.com") } + let!(:data_coordinator2) { create(:user, :data_coordinator, organisation:, email: "data_coordinator2@example.com") } + + before do + create(:user, :data_provider, organisation:, email: "data_provider1@example.com") + create_list(:sales_log, 7, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: "exists", town_or_city: nil, old_form_id: "form_1", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", %w[missing_town], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_coordinator1@example.com, data_coordinator2@example.com") + task.invoke + end + end + + context "when org has 5 missing town or city and data providers only" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:sales_log, 5, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: "exists", town_or_city: nil, old_form_id: "form_2", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[missing_town], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org has less than 5 missing town or city" do + before do + create_list(:sales_log, 3, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: "address", town_or_city: nil, old_form_id: "form_2", owning_organisation: organisation, created_by: organisation.users.first) + create_list(:sales_log, 2, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), address_line1: "address", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job with organisations that is missing less town or city data than threshold amount" do + expect { task.invoke }.not_to enqueue_job(EmailMissingAddressesCsvJob) + end + end + + context "when org has more than 5 wrong uprn and data coordinators" do + let!(:data_coordinator) { create(:user, :data_coordinator, organisation:, email: "data_coordinator1@example.com") } + let!(:data_coordinator2) { create(:user, :data_coordinator, organisation:, email: "data_coordinator2@example.com") } + + before do + create(:user, :data_provider, organisation:, email: "data_provider1@example.com") + create_list(:sales_log, 7, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), uprn_known: 1, uprn: "123", town_or_city: "Bristol", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_coordinator.id, data_coordinator2.id), organisation, "sales", %w[wrong_uprn], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_coordinator1@example.com, data_coordinator2@example.com") + task.invoke + end + end + + context "when org has 5 wrong uprn and data providers only" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:sales_log, 5, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), uprn_known: 1, uprn: "12", purchid: "12", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[wrong_uprn], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org has less than 5 wrong uprn" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:sales_log, 3, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), uprn_known: 1, uprn: "123", town_or_city: "Bristol", owning_organisation: organisation, created_by: organisation.users.first) + create_list(:sales_log, 2, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), uprn_known: 1, uprn: "12", purchid: "12", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "enqueues the job with correct organisations" do + expect { task.invoke }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[wrong_uprn], []) + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Sending missing sales addresses CSV for test organisation to data_provider3@example.com, data_provider4@example.com") + task.invoke + end + end + + context "when org is included in skip_uprn_issue_organisations list" do + before do + create_list(:sales_log, 5, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), uprn_known: 1, uprn: "12", purchid: "12", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "does not enqueue the job" do + expect { task.invoke("#{organisation.id} 4") }.not_to enqueue_job(EmailMissingAddressesCsvJob) + end + end + + context "when skip_uprn_issue_organisations list is provided" do + let!(:data_provider) { create(:user, :data_provider, organisation:, email: "data_provider3@example.com") } + let!(:data_provider2) { create(:user, :data_provider, organisation:, email: "data_provider4@example.com") } + + before do + create_list(:sales_log, 5, :completed, :imported, saledate: Time.zone.local(2023, 9, 9), uprn_known: 1, uprn: "12", purchid: "12", owning_organisation: organisation, created_by: organisation.users.first) + end + + it "does enqueues the job with correct skip_uprn_issue_organisations" do + expect { task.invoke("100 400") }.to enqueue_job(EmailMissingAddressesCsvJob).with(include(data_provider.id, data_provider2.id), organisation, "sales", %w[wrong_uprn], [100, 400]) + end + end + end + end + + describe ":create_lettings_addresses_csv", type: :task do + subject(:task) { Rake::Task["correct_addresses:create_lettings_addresses_csv"] } + + before do + organisation.users.destroy_all + Rake.application.rake_require("tasks/send_missing_addresses_csv") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the rake task is run" do + let(:organisation) { create(:organisation, name: "test organisation") } + + context "and organisation ID is provided" do + it "enqueues the job with correct organisation" do + expect { task.invoke(organisation.id) }.to enqueue_job(CreateAddressesCsvJob).with(organisation, "lettings") + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Creating lettings addresses CSV for test organisation") + task.invoke(organisation.id) + end + end + + context "when organisation with given ID cannot be found" do + it "prints out error" do + expect(Rails.logger).to receive(:error).with("Organisation with ID fake not found") + task.invoke("fake") + end + end + + context "when organisation ID is not given" do + it "raises an error" do + expect { task.invoke }.to raise_error(RuntimeError, "Usage: rake correct_addresses:create_lettings_addresses_csv['organisation_id']") + end + end + end + end + + describe ":create_sales_addresses_csv", type: :task do + subject(:task) { Rake::Task["correct_addresses:create_sales_addresses_csv"] } + + before do + organisation.users.destroy_all + Rake.application.rake_require("tasks/send_missing_addresses_csv") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the rake task is run" do + let(:organisation) { create(:organisation, name: "test organisation") } + + context "and organisation ID is provided" do + it "enqueues the job with correct organisation" do + expect { task.invoke(organisation.id) }.to enqueue_job(CreateAddressesCsvJob).with(organisation, "sales") + end + + it "prints out the jobs enqueued" do + expect(Rails.logger).to receive(:info).with(nil) + expect(Rails.logger).to receive(:info).with("Creating sales addresses CSV for test organisation") + task.invoke(organisation.id) + end + end + + context "when organisation with given ID cannot be found" do + it "prints out error" do + expect(Rails.logger).to receive(:error).with("Organisation with ID fake not found") + task.invoke("fake") + end + end + + context "when organisation ID is not given" do + it "raises an error" do + expect { task.invoke }.to raise_error(RuntimeError, "Usage: rake correct_addresses:create_sales_addresses_csv['organisation_id']") + end + end + end + end +end diff --git a/spec/mailers/csv_download_mailer_spec.rb b/spec/mailers/csv_download_mailer_spec.rb index 6c145c508..704d71e1e 100644 --- a/spec/mailers/csv_download_mailer_spec.rb +++ b/spec/mailers/csv_download_mailer_spec.rb @@ -1,15 +1,15 @@ 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") } + 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 + before do + allow(Notifications::Client).to receive(:new).and_return(notify_client) + allow(notify_client).to receive(:send_email).and_return(true) + end + describe "#send_csv_download_mail" do it "sends a CSV download E-mail via notify" do link = :link duration = 20.minutes.to_i @@ -27,4 +27,57 @@ RSpec.describe CsvDownloadMailer do described_class.new.send_csv_download_mail(user, link, duration) end end + + describe "#send_missing_lettings_addresses_csv_download_mail" do + 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_MISSING_LETTINGS_ADDRESSES_DOWNLOAD_TEMPLATE_ID, + personalisation: { + name: user.name, + issue_explanation: "We have found this issue in your logs imported to the new version of CORE: +## Missing town or city +The town or city in some logs is missing. This data is required in the new version of CORE.\n", + how_to_fix: "You need to:\n +- download [this spreadsheet for lettings logs](#{link}). This link will expire in one week. To request another link, [contact the CORE helpdesk](https://dluhcdigital.atlassian.net/servicedesk/customer/portal/6/group/11). +- fill in the missing address data +- check that the existing address data is correct\n", + duration: "20 minutes", + }, + ) + + described_class.new.send_missing_lettings_addresses_csv_download_mail(user, link, duration, %w[missing_town]) + end + end + + describe "#send_missing_sales_addresses_csv_download_mail" do + 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_MISSING_SALES_ADDRESSES_DOWNLOAD_TEMPLATE_ID, + personalisation: { + name: user.name, + issue_explanation: "We have found this issue in your logs imported to the new version of CORE: +## Incorrect UPRN\nThe UPRN in some logs may be incorrect, so the wrong address data may have been imported. + +In some of your logs, the UPRN is the same as the purchaser code, but these are different things. Purchaser codes are codes that your organisation uses to identify properties. UPRNs are unique numbers assigned by the Ordnance Survey. + +If a log has the correct UPRN, leave the UPRN unchanged. If the UPRN is incorrect, clear the value and provide the full address instead. Alternatively, you can change the UPRN on the CORE system.\n", + how_to_fix: "You need to:\n +- download [this spreadsheet for sales logs](#{link}). This link will expire in one week. To request another link, [contact the CORE helpdesk](https://dluhcdigital.atlassian.net/servicedesk/customer/portal/6/group/11). +- check that the address data is correct +- correct any address errors\n", + duration: "20 minutes", + }, + ) + + described_class.new.send_missing_sales_addresses_csv_download_mail(user, link, duration, %w[wrong_uprn]) + end + end end diff --git a/spec/services/csv/missing_addresses_csv_service_spec.rb b/spec/services/csv/missing_addresses_csv_service_spec.rb new file mode 100644 index 000000000..e8482f4cb --- /dev/null +++ b/spec/services/csv/missing_addresses_csv_service_spec.rb @@ -0,0 +1,512 @@ +require "rails_helper" + +RSpec.describe Csv::MissingAddressesCsvService do + let(:organisation) { create(:organisation, name: "Address org") } + let(:user) { create(:user, organisation:, email: "testy@example.com") } + let(:service) { described_class.new(organisation, skip_uprn_issue_organisations) } + let(:skip_uprn_issue_organisations) { [100, 200] } + + before do + body_1 = { + results: [ + { + DPA: { + "POSTCODE": "BS1 1AD", + "POST_TOWN": "Bristol", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + body_2 = { + results: [ + { + DPA: { + "POSTCODE": "EC1N 2TD", + "POST_TOWN": "Newcastle", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=123") + .to_return(status: 200, body: body_1, headers: {}) + + stub_request(:get, "https://api.os.uk/search/places/v1/uprn?key=OS_DATA_KEY&uprn=12") + .to_return(status: 200, body: body_2, headers: {}) + end + + def replace_entity_ids(lettings_log, export_template) + export_template.sub!(/\{id\}/, lettings_log.id.to_s) + end + + describe "#create_missing_lettings_addresses_csv" do + let!(:lettings_log) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + address_line1: nil, + town_or_city: nil, + old_id: "old_id", + old_form_id: "old_form_id", + needstype: 1, + uprn_known: 0) + end + + let!(:lettings_log_missing_town) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + address_line1: "existing address", + town_or_city: nil, + old_id: "older_id", + old_form_id: "old_form_id", + needstype: 1, + uprn_known: 0) + end + + let!(:lettings_log_wrong_uprn) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + uprn: "123", + uprn_known: 1, + old_id: "oldest_id", + needstype: 1) + end + + context "when the organisation has logs with missing addresses, missing town or city and wrong uprn" do + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(lettings_log, File.open("spec/fixtures/files/missing_lettings_logs_addresses_all_issues.csv").read) + expected_content = replace_entity_ids(lettings_log_missing_town, expected_content) + expected_content = replace_entity_ids(lettings_log_wrong_uprn, expected_content) + csv = service.create_missing_lettings_addresses_csv + expect(csv).to eq(expected_content) + end + end + + context "when the organisation has logs with missing addresses only" do + before do + lettings_log_missing_town.update!(town_or_city: "towncity") + lettings_log_wrong_uprn.update!(uprn: "12") + end + + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(lettings_log, File.open("spec/fixtures/files/missing_lettings_logs_addresses.csv").read) + csv = service.create_missing_lettings_addresses_csv + expect(csv).to eq(expected_content) + end + end + + context "when the organisation has logs with missing town or city only" do + before do + lettings_log.update!(address_line1: "existing address", town_or_city: "towncity") + lettings_log_wrong_uprn.update!(uprn: "12") + end + + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(lettings_log_missing_town, File.open("spec/fixtures/files/missing_lettings_logs_town_or_city.csv").read) + csv = service.create_missing_lettings_addresses_csv + expect(csv).to eq(expected_content) + end + end + + context "when the organisation has logs with wrong uprn only" do + before do + lettings_log.update!(address_line1: "existing address", town_or_city: "towncity") + lettings_log_missing_town.update!(town_or_city: "towncity") + lettings_log_wrong_uprn.update!(uprn: "12", propcode: "12") + end + + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(lettings_log_wrong_uprn, File.open("spec/fixtures/files/missing_lettings_logs_wrong_uprn.csv").read) + csv = service.create_missing_lettings_addresses_csv + expect(csv).to eq(expected_content) + end + + context "and the organisation is marked as an organisation to skip" do + let(:skip_uprn_issue_organisations) { [organisation.id] } + + it "returns nil" do + expect(service.create_missing_lettings_addresses_csv).to be_nil + end + end + end + + context "when the organisation only has supported housing logs with missing addresses or town or city" do + before do + lettings_log.update!(needstype: 2) + lettings_log_missing_town.update!(needstype: 2) + lettings_log_wrong_uprn.update!(needstype: 2) + end + + it "returns nil" do + expect(service.create_missing_lettings_addresses_csv).to be_nil + end + end + + context "when the organisation only has logs with missing addresses or town or city from 2022" do + before do + lettings_log.update!(startdate: Time.zone.local(2022, 4, 5)) + lettings_log_missing_town.update!(startdate: Time.zone.local(2022, 4, 5)) + lettings_log_wrong_uprn.update!(startdate: Time.zone.local(2022, 4, 5)) + end + + it "returns nil" do + expect(service.create_missing_lettings_addresses_csv).to be_nil + end + end + + context "when the organisation has any address and town or city fields filled in or correct uprn" do + before do + lettings_log.update!(address_line1: "address_line1", town_or_city: "towncity") + lettings_log_missing_town.update!(address_line1: "address_line1", town_or_city: "towncity") + lettings_log_wrong_uprn.update!(uprn: "12") + end + + it "returns nil" do + expect(service.create_missing_lettings_addresses_csv).to be_nil + end + end + end + + describe "#create_missing_sales_addresses_csv" do + let!(:sales_log) do + create(:sales_log, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + address_line1: nil, + town_or_city: nil, + old_id: "old_id", + old_form_id: "old_form_id", + uprn_known: 0) + end + + let!(:sales_log_missing_town) do + create(:sales_log, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + address_line1: "existing address line 1", + town_or_city: nil, + old_id: "older_id", + old_form_id: "old_form_id", + uprn_known: 0) + end + + let!(:sales_log_wrong_uprn) do + create(:sales_log, + :completed, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + uprn: "123", + town_or_city: "Bristol", + old_id: "oldest_id", + uprn_known: 1, + uprn_confirmed: 1, + la: nil) + end + + context "when the organisation has logs with missing addresses, town or city and wrong uprn" do + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(sales_log, File.open("spec/fixtures/files/missing_sales_logs_addresses_all_issues.csv").read) + expected_content = replace_entity_ids(sales_log_missing_town, expected_content) + expected_content = replace_entity_ids(sales_log_wrong_uprn, expected_content) + csv = service.create_missing_sales_addresses_csv + expect(csv).to eq(expected_content) + end + end + + context "when the organisation has logs with missing addresses" do + before do + sales_log_missing_town.update!(town_or_city: "towncity") + sales_log_wrong_uprn.update!(uprn: "12") + end + + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(sales_log, File.open("spec/fixtures/files/missing_sales_logs_addresses.csv").read) + csv = service.create_missing_sales_addresses_csv + expect(csv).to eq(expected_content) + end + end + + context "when the organisation has logs with missing town_or_city only" do + before do + sales_log.update!(address_line1: "address", town_or_city: "towncity") + sales_log_wrong_uprn.update!(uprn: "12") + end + + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(sales_log_missing_town, File.open("spec/fixtures/files/missing_sales_logs_town_or_city.csv").read) + csv = service.create_missing_sales_addresses_csv + expect(csv).to eq(expected_content) + end + end + + context "when the organisation has logs with wrong uprn only" do + before do + sales_log.update!(address_line1: "address", town_or_city: "towncity") + sales_log_missing_town.update!(town_or_city: "towncity") + sales_log_wrong_uprn.update!(uprn: "12", purchid: "12") + end + + it "returns a csv with relevant logs" do + expected_content = replace_entity_ids(sales_log_wrong_uprn, File.open("spec/fixtures/files/missing_sales_logs_wrong_uprn.csv").read) + csv = service.create_missing_sales_addresses_csv + expect(csv).to eq(expected_content) + end + + context "and the organisation is marked as an organisation to skip" do + let(:skip_uprn_issue_organisations) { [organisation.id] } + + it "returns nil" do + expect(service.create_missing_sales_addresses_csv).to be_nil + end + end + end + + context "when the organisation only has logs with missing addresses from 2022" do + before do + sales_log.update!(saledate: Time.zone.local(2022, 4, 5)) + sales_log_missing_town.update!(saledate: Time.zone.local(2022, 4, 5)) + sales_log_wrong_uprn.update!(saledate: Time.zone.local(2022, 4, 5)) + end + + it "returns nil" do + expect(service.create_missing_sales_addresses_csv).to be_nil + end + end + + context "when the organisation has address fields filled in" do + before do + sales_log.update!(town_or_city: "town", address_line1: "line1") + sales_log_missing_town.update!(town_or_city: "town") + sales_log_wrong_uprn.update!(uprn: "12") + end + + it "returns nil" do + expect(service.create_missing_sales_addresses_csv).to be_nil + end + end + end + + describe "#create_lettings_addresses_csv" do + context "when the organisation has lettings logs" do + let!(:lettings_log) do + create(:lettings_log, + tenancycode: "tenancycode1", + propcode: "propcode1", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + address_line1: "address", + town_or_city: "town", + old_id: "old_id_1", + old_form_id: "old_form_id_1", + needstype: 1, + uprn_known: 0) + end + + let!(:lettings_log_missing_address) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + address_line1: nil, + town_or_city: nil, + old_id: "old_id", + old_form_id: "old_form_id", + needstype: 1, + uprn_known: 0) + end + + let!(:lettings_log_missing_town) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + address_line1: "existing address", + town_or_city: nil, + old_id: "older_id", + old_form_id: "old_form_id", + needstype: 1, + uprn_known: 0) + end + + let!(:lettings_log_wrong_uprn) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + uprn: "123", + uprn_known: 1, + old_id: "oldest_id", + needstype: 1) + end + + let!(:lettings_log_not_imported) do + create(:lettings_log, + tenancycode: "tenancycode", + propcode: "propcode", + startdate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + managing_organisation: organisation, + uprn: "123", + uprn_known: 1, + needstype: 1) + end + + before do + create(:lettings_log, managing_organisation: organisation, old_id: "exists", startdate: Time.zone.local(2022, 4, 5)) + end + + it "returns a csv with relevant logs" do + csv = CSV.parse(service.create_lettings_addresses_csv) + expect(csv.count).to eq(6) + expect(csv).to include([lettings_log.id.to_s, "2023-04-05", "tenancycode1", "propcode1", "testy@example.com", "Address org", "Address org", nil, "address", nil, "town", nil, nil]) + expect(csv).to include([lettings_log_missing_address.id.to_s, "2023-04-05", "tenancycode", "propcode", "testy@example.com", "Address org", "Address org", nil, nil, nil, nil, nil, nil]) + expect(csv).to include([lettings_log_missing_town.id.to_s, "2023-04-05", "tenancycode", "propcode", "testy@example.com", "Address org", "Address org", nil, "existing address", nil, nil, nil, nil]) + expect(csv).to include([lettings_log_wrong_uprn.id.to_s, "2023-04-05", "tenancycode", "propcode", "testy@example.com", "Address org", "Address org", "123", "Some Place", nil, "Bristol", nil, "BS1 1AD"]) + expect(csv).to include([lettings_log_not_imported.id.to_s, "2023-04-05", "tenancycode", "propcode", "testy@example.com", "Address org", "Address org", "123", "Some Place", nil, "Bristol", nil, "BS1 1AD"]) + end + end + + context "when the organisation does not have relevant lettings logs" do + before do + create(:lettings_log, managing_organisation: organisation, startdate: Time.zone.local(2022, 4, 5)) + end + + it "returns only headers" do + csv = service.create_lettings_addresses_csv + expect(csv).to eq "Log ID,Tenancy start date,Tenant code,Property reference,Log owner,Owning organisation,Managing organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode\n" + end + end + end + + describe "#create_sales_addresses_csv" do + context "when the organisation has sales" do + let!(:sales_log) do + create(:sales_log, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + address_line1: "address", + town_or_city: "city", + old_id: "old_id_1", + old_form_id: "old_form_id_1", + uprn_known: 0) + end + + let!(:sales_log_missing_address) do + create(:sales_log, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + address_line1: nil, + town_or_city: nil, + old_id: "old_id", + old_form_id: "old_form_id", + uprn_known: 0) + end + + let!(:sales_log_missing_town) do + create(:sales_log, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + address_line1: "existing address line 1", + town_or_city: nil, + old_id: "older_id", + old_form_id: "old_form_id", + uprn_known: 0) + end + + let!(:sales_log_wrong_uprn) do + create(:sales_log, + :completed, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + uprn: "123", + town_or_city: "Bristol", + old_id: "oldest_id", + uprn_known: 1, + uprn_confirmed: 1, + la: nil) + end + + let!(:sales_log_not_imported) do + create(:sales_log, + :completed, + purchid: "purchaser code", + saledate: Time.zone.local(2023, 4, 5), + created_by: user, + owning_organisation: organisation, + uprn: "123", + town_or_city: "Bristol", + uprn_known: 1, + uprn_confirmed: 1, + la: nil) + end + + before do + create(:sales_log, :completed, saledate: Time.zone.local(2022, 4, 5)) + end + + it "returns a csv with relevant logs" do + csv = CSV.parse(service.create_sales_addresses_csv) + expect(csv.count).to eq(6) + expect(csv).to include([sales_log.id.to_s, "2023-04-05", "purchaser code", "testy@example.com", "Address org", nil, "address", nil, "city", nil, nil]) + expect(csv).to include([sales_log_missing_address.id.to_s, "2023-04-05", "purchaser code", "testy@example.com", "Address org", nil, nil, nil, nil, nil, nil]) + expect(csv).to include([sales_log_missing_town.id.to_s, "2023-04-05", "purchaser code", "testy@example.com", "Address org", nil, "existing address line 1", nil, nil, nil, nil]) + expect(csv).to include([sales_log_wrong_uprn.id.to_s, "2023-04-05", "purchaser code", "testy@example.com", "Address org", "123", "Some Place", nil, "Bristol", nil, "BS1 1AD"]) + expect(csv).to include([sales_log_not_imported.id.to_s, "2023-04-05", "purchaser code", "testy@example.com", "Address org", "123", "Some Place", nil, "Bristol", nil, "BS1 1AD"]) + end + end + + context "when the organisation does not have relevant sales logs" do + before do + create(:sales_log, :completed, saledate: Time.zone.local(2022, 4, 5)) + end + + it "returns only headers" do + csv = service.create_sales_addresses_csv + expect(csv).to eq("Log ID,Sale completion date,Purchaser code,Log owner,Owning organisation,UPRN,Address Line 1,Address Line 2 (optional),Town or City,County (optional),Property's postcode\n") + end + end + end +end From 8adc8ed8483683f2b5c350080cea47af010f0d86 Mon Sep 17 00:00:00 2001 From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com> Date: Wed, 11 Oct 2023 10:09:56 +0100 Subject: [PATCH 02/12] CLDC-2563: Update prod deployment pipeline (#1960) * CLDC-2563: Update prod deployment pipeline * CLDC-2563: remove redundant REPO_URL assignment * CLDC-2563: use GITHUB_OUTPUT only where necessary * CLDC-2563: don't push image if tag already exists * CLDC-2563: actually use github.sha for production as well * CLDC-2563: remove obsolete env check in push docker image job * CLDC-2563: remove redundant main branch check --- .github/workflows/aws_deploy.yml | 15 ++++++++++++++- .github/workflows/production_pipeline.yml | 16 +++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/.github/workflows/aws_deploy.yml b/.github/workflows/aws_deploy.yml index 247efbfab..8fa267a69 100644 --- a/.github/workflows/aws_deploy.yml +++ b/.github/workflows/aws_deploy.yml @@ -12,6 +12,9 @@ on: environment: required: true type: string + release_tag: + required: false + type: string concurrency: group: deploy-${{ inputs.environment }} @@ -45,8 +48,13 @@ jobs: with: mask-password: 'true' + - name: Check if image with tag already exists + run: | + echo "image-exists=$(if aws ecr list-images --repository-name=$repository --query "imageIds[*].imageTag" | grep -q ${{ github.sha }}; then echo true; else echo false; fi)" >> $GITHUB_ENV + - name: Build, tag, and push docker image to ECR id: build-image + if: ${{ env.image-exists == 'false' }} env: registry: ${{ steps.ecr-login.outputs.registry }} commit_tag: ${{ github.sha }} @@ -77,11 +85,16 @@ jobs: id: timestamp run: echo "timestamp=$(date +%Y%m%d%H%M%S)" >> $GITHUB_ENV + - name: Get additional tag + run: | + echo "additional-tag=$(if [[ ${{ inputs.environment }} == 'production' ]]; then echo ${{ inputs.release_tag }}-${{ env.timestamp }}; else echo ${{ env.timestamp }}; fi)" >> $GITHUB_ENV + - name: Add environment tag to existing image + id: update-image-tags env: registry: ${{ steps.ecr-login.outputs.registry }} commit_tag: ${{ github.sha }} - readable_tag: ${{ inputs.environment }}-${{ env.timestamp }} + readable_tag: ${{ inputs.environment }}-${{ env.additional-tag }} run: | manifest=$(aws ecr batch-get-image --repository-name $repository --image-ids imageTag=$commit_tag --output text --query images[].imageManifest) aws ecr put-image --repository-name $repository --image-tag $readable_tag --image-manifest "$manifest" diff --git a/.github/workflows/production_pipeline.yml b/.github/workflows/production_pipeline.yml index fd0a73b9d..76a85eca8 100644 --- a/.github/workflows/production_pipeline.yml +++ b/.github/workflows/production_pipeline.yml @@ -16,6 +16,8 @@ jobs: test: name: Test runs-on: ubuntu-latest + outputs: + releasetag: ${{ steps.latestrelease.outputs.releasetag }} services: postgres: @@ -48,7 +50,7 @@ jobs: - name: Get latest release with tag id: latestrelease run: | - echo "::set-output name=releasetag::$(curl -s https://api.github.com/repos/${REPO_URL}/releases/latest | jq '.tag_name' | sed 's/\"//g')" + echo "releasetag=$(curl -s https://api.github.com/repos/${REPO_URL}/releases/latest | jq '.tag_name' | sed 's/\"//g')" >> $GITHUB_OUTPUT - name: Confirm release tag run: | @@ -257,3 +259,15 @@ jobs: 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 + + aws_deploy: + name: AWS Deploy + needs: [lint, test, feature_test, audit] + uses: ./.github/workflows/aws_deploy.yml + with: + aws_account_id: 977287343304 + aws_resource_prefix: core-prod + environment: production + release_tag: ${{ needs.test.outputs.releasetag }} + permissions: + id-token: write From 7c53bd853d4f8879b6afb1f3274c2027345d5583 Mon Sep 17 00:00:00 2001 From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com> Date: Wed, 11 Oct 2023 17:21:27 +0100 Subject: [PATCH 03/12] Delete obsolete data_export_csv task (#1974) --- lib/tasks/data_export.rake | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/tasks/data_export.rake b/lib/tasks/data_export.rake index e57c3779d..84908d635 100644 --- a/lib/tasks/data_export.rake +++ b/lib/tasks/data_export.rake @@ -1,9 +1,4 @@ namespace :core do - desc "Export data CSVs for import into Central Data System (CDS)" - task data_export_csv: :environment do |_task, _args| - DataExportCsvJob.perform_later - end - desc "Export data XMLs for import into Central Data System (CDS)" task :data_export_xml, %i[full_update] => :environment do |_task, args| full_update = args[:full_update].present? && args[:full_update] == "true" From 2bc8501ffedfaeb29fb27e10653c166245393f60 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 12 Oct 2023 09:19:23 +0100 Subject: [PATCH 04/12] CLDC-2773 Update lettings (and sales) csv date format (#1954) * feat: update datetime format in csv services * feat: update tests * feat: keep system date fields in iso8601 format * feat: merge fix * feat: update tests --- app/services/csv/lettings_log_csv_service.rb | 13 +++++++++---- app/services/csv/sales_log_csv_service.rb | 6 +++--- .../files/lettings_log_csv_export_codes.csv | 2 +- .../files/lettings_log_csv_export_labels.csv | 2 +- .../lettings_log_csv_export_non_support_codes.csv | 2 +- .../lettings_log_csv_export_non_support_labels.csv | 2 +- 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/app/services/csv/lettings_log_csv_service.rb b/app/services/csv/lettings_log_csv_service.rb index c7f63def1..499230464 100644 --- a/app/services/csv/lettings_log_csv_service.rb +++ b/app/services/csv/lettings_log_csv_service.rb @@ -137,12 +137,15 @@ module Csv "prevloc_label" => "prevloc", }.freeze - DATE_FIELDS = %w[ + SYSTEM_DATE_FIELDS = %w[ + created_at + updated_at + ].freeze + + USER_DATE_FIELDS = %w[ mrcdate startdate voiddate - created_at - updated_at ].freeze def value(attribute, log) @@ -156,8 +159,10 @@ module Csv attribute = FIELDS_ALWAYS_EXPORTED_AS_LABELS[attribute] value = log.public_send(attribute) get_label(value, attribute, log) - elsif DATE_FIELDS.include? attribute + elsif SYSTEM_DATE_FIELDS.include? attribute log.public_send(attribute)&.iso8601 + elsif USER_DATE_FIELDS.include? attribute + log.public_send(attribute)&.strftime("%F") elsif PERSON_DETAILS.any? { |key, _value| key == attribute } && (person_details_not_known?(log, attribute) || age_not_known?(log, attribute)) case @export_type when "codes" diff --git a/app/services/csv/sales_log_csv_service.rb b/app/services/csv/sales_log_csv_service.rb index 1e5bd1ed6..89bfdeb8c 100644 --- a/app/services/csv/sales_log_csv_service.rb +++ b/app/services/csv/sales_log_csv_service.rb @@ -74,7 +74,7 @@ module Csv "prevloc_label" => "prevloc", }.freeze - DATE_FIELDS = %w[ + SYSTEM_DATE_FIELDS = %w[ created_at updated_at ].freeze @@ -89,8 +89,8 @@ module Csv attribute = FIELDS_ALWAYS_EXPORTED_AS_LABELS[attribute] value = log.send(attribute) get_label(value, attribute, log) - elsif DATE_FIELDS.include? attribute - log.send(attribute)&.iso8601 + elsif SYSTEM_DATE_FIELDS.include? attribute + log.public_send(attribute)&.iso8601 elsif PERSON_DETAILS.any? { |key, _value| key == attribute } && (person_details_not_known?(log, attribute) || age_not_known?(log, attribute)) case @export_type when "codes" diff --git a/spec/fixtures/files/lettings_log_csv_export_codes.csv b/spec/fixtures/files/lettings_log_csv_export_codes.csv index 823bf4340..1a6292a3b 100644 --- a/spec/fixtures/files/lettings_log_csv_export_codes.csv +++ b/spec/fixtures/files/lettings_log_csv_export_codes.csv @@ -1,2 +1,2 @@ id,status,created_by,is_dpo,created_at,updated_by,updated_at,creation_method,old_id,old_form_id,collection_start_year,owning_organisation_name,managing_organisation_name,needstype,lettype,renewal,startdate,renttype,rent_type_detail,irproduct,irproduct_other,lar,tenancycode,propcode,uprn_known,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,postcode_full,is_la_inferred,la_label,la,first_time_property_let_as_social_housing,unitletas,rsnvac,newprop,offered,unittype_gn,builtype,wchair,beds,voiddate,vacdays,void_date_value_check,majorrepairs,mrcdate,major_repairs_date_value_check,joint,startertenancy,tenancy,tenancyother,tenancylength,sheltered,declaration,hhmemb,pregnancy_value_check,refused,hhtype,totchild,totelder,totadult,age1,retirement_value_check,sex1,ethnic_group,ethnic,national,ecstat1,details_known_2,relat2,age2,sex2,ecstat2,details_known_3,relat3,age3,sex3,ecstat3,details_known_4,relat4,age4,sex4,ecstat4,details_known_5,relat5,age5,sex5,ecstat5,details_known_6,relat6,age6,sex6,ecstat6,details_known_7,relat7,age7,sex7,ecstat7,details_known_8,relat8,age8,sex8,ecstat8,armedforces,leftreg,reservist,preg_occ,housingneeds,housingneeds_type,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_f,housingneeds_g,housingneeds_h,housingneeds_other,illness,illness_type_4,illness_type_5,illness_type_2,illness_type_6,illness_type_7,illness_type_3,illness_type_9,illness_type_8,illness_type_1,illness_type_10,layear,waityear,reason,reasonother,prevten,new_old,homeless,ppcodenk,ppostcode_full,previous_la_known,is_previous_la_inferred,prevloc_label,prevloc,reasonpref,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,cbl,cap,chr,letting_allocation_unknown,referral,referral_value_check,net_income_known,incref,earnings,incfreq,net_income_value_check,hb,has_benefits,benefits,household_charge,nocharge,period,is_carehome,chcharge,wchchrg,carehome_charges_value_check,brent,wrent,rent_value_check,scharge,wscharge,pscharge,wpschrge,supcharg,wsupchrg,tcharge,wtcharge,scharge_value_check,pscharge_value_check,supcharg_value_check,hbrentshortfall,tshortfall_known,tshortfall,wtshortfall,scheme_code,scheme_service_name,scheme_sensitive,SCHTYPE,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate -,completed,s.port@jeemayle.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,1,,,2023,DLUHC,DLUHC,1,7,0,2023-06-26T00:00:00+01:00,2,1,,,,HIJKLMN,ABCDEFG,0,,,fake address,,London,,NW9 5LL,false,Barnet,E09000003,0,2,6,2,2,7,1,1,3,2023-06-24T00:00:00+01:00,,,1,2023-06-25T00:00:00+01:00,,3,1,4,,2,,1,4,,1,4,0,0,2,35,,F,0,2,13,0,0,P,32,M,6,1,R,-9,R,10,0,R,-9,R,10,,,,,,,,,,,,,,,,,,,,,1,4,1,2,1,0,1,0,0,0,0,0,0,1,0,0,1,0,0,0,0,0,0,0,2,7,4,,6,2,1,0,TN23 6LZ,1,false,Ashford,E07000105,1,0,1,0,0,0,0,0,1,,2,,0,0,68,1,,6,1,1,,0,2,,,,,200.0,100.0,,50.0,25.0,40.0,20.0,35.0,17.5,325.0,162.5,,,,1,0,12.0,6.0,,,,,,,,,,,,,,,,,,,, +,completed,s.port@jeemayle.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,1,,,2023,DLUHC,DLUHC,1,7,0,2023-06-26,2,1,,,,HIJKLMN,ABCDEFG,0,,,fake address,,London,,NW9 5LL,false,Barnet,E09000003,0,2,6,2,2,7,1,1,3,2023-06-24,,,1,2023-06-25,,3,1,4,,2,,1,4,,1,4,0,0,2,35,,F,0,2,13,0,0,P,32,M,6,1,R,-9,R,10,0,R,-9,R,10,,,,,,,,,,,,,,,,,,,,,1,4,1,2,1,0,1,0,0,0,0,0,0,1,0,0,1,0,0,0,0,0,0,0,2,7,4,,6,2,1,0,TN23 6LZ,1,false,Ashford,E07000105,1,0,1,0,0,0,0,0,1,,2,,0,0,68,1,,6,1,1,,0,2,,,,,200.0,100.0,,50.0,25.0,40.0,20.0,35.0,17.5,325.0,162.5,,,,1,0,12.0,6.0,,,,,,,,,,,,,,,,,,,, diff --git a/spec/fixtures/files/lettings_log_csv_export_labels.csv b/spec/fixtures/files/lettings_log_csv_export_labels.csv index cfa2df5b4..fd93b4015 100644 --- a/spec/fixtures/files/lettings_log_csv_export_labels.csv +++ b/spec/fixtures/files/lettings_log_csv_export_labels.csv @@ -1,2 +1,2 @@ id,status,created_by,is_dpo,created_at,updated_by,updated_at,creation_method,old_id,old_form_id,collection_start_year,owning_organisation_name,managing_organisation_name,needstype,lettype,renewal,startdate,renttype,rent_type_detail,irproduct,irproduct_other,lar,tenancycode,propcode,uprn_known,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,postcode_full,is_la_inferred,la_label,la,first_time_property_let_as_social_housing,unitletas,rsnvac,newprop,offered,unittype_gn,builtype,wchair,beds,voiddate,vacdays,void_date_value_check,majorrepairs,mrcdate,major_repairs_date_value_check,joint,startertenancy,tenancy,tenancyother,tenancylength,sheltered,declaration,hhmemb,pregnancy_value_check,refused,hhtype,totchild,totelder,totadult,age1,retirement_value_check,sex1,ethnic_group,ethnic,national,ecstat1,details_known_2,relat2,age2,sex2,ecstat2,details_known_3,relat3,age3,sex3,ecstat3,details_known_4,relat4,age4,sex4,ecstat4,details_known_5,relat5,age5,sex5,ecstat5,details_known_6,relat6,age6,sex6,ecstat6,details_known_7,relat7,age7,sex7,ecstat7,details_known_8,relat8,age8,sex8,ecstat8,armedforces,leftreg,reservist,preg_occ,housingneeds,housingneeds_type,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_f,housingneeds_g,housingneeds_h,housingneeds_other,illness,illness_type_4,illness_type_5,illness_type_2,illness_type_6,illness_type_7,illness_type_3,illness_type_9,illness_type_8,illness_type_1,illness_type_10,layear,waityear,reason,reasonother,prevten,new_old,homeless,ppcodenk,ppostcode_full,previous_la_known,is_previous_la_inferred,prevloc_label,prevloc,reasonpref,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,cbl,cap,chr,letting_allocation_unknown,referral,referral_value_check,net_income_known,incref,earnings,incfreq,net_income_value_check,hb,has_benefits,benefits,household_charge,nocharge,period,is_carehome,chcharge,wchchrg,carehome_charges_value_check,brent,wrent,rent_value_check,scharge,wscharge,pscharge,wpschrge,supcharg,wsupchrg,tcharge,wtcharge,scharge_value_check,pscharge_value_check,supcharg_value_check,hbrentshortfall,tshortfall_known,tshortfall,wtshortfall,scheme_code,scheme_service_name,scheme_sensitive,SCHTYPE,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate -,completed,s.port@jeemayle.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,single log,,,2023,DLUHC,DLUHC,General needs,7,No,2023-06-26T00:00:00+01:00,2,Affordable Rent,,,,HIJKLMN,ABCDEFG,No,,,fake address,,London,,NW9 5LL,No,Barnet,E09000003,No,Affordable rent basis,Tenant abandoned property,2,2,House,Purpose built,Yes,3,2023-06-24T00:00:00+01:00,,,Yes,2023-06-25T00:00:00+01:00,,Don’t know,Yes,Assured Shorthold Tenancy (AST) – Fixed term,,2,,1,4,,1,4,0,0,2,35,,Female,White,Irish,Tenant prefers not to say,Other,Yes,Partner,32,Male,Not seeking work,No,Prefers not to say,Not known,Prefers not to say,Prefers not to say,Yes,Person prefers not to say,Not known,Person prefers not to say,Person prefers not to say,,,,,,,,,,,,,,,,,,,,,Yes – the person is a current or former regular,No – they left up to and including 5 years ago,Yes,No,Yes,Fully wheelchair accessible housing,1,0,0,0,0,0,No,Yes,0,0,1,0,0,0,0,0,0,0,Less than 1 year,1 year but under 2 years,Loss of tied accommodation,,Other supported housing,2,No,Yes,TN23 6LZ,Yes,No,Ashford,E07000105,Yes,0,1,0,0,0,0,0,1,,Tenant applied directly (no referral or nomination),,Yes,0,68,Weekly,,Universal Credit housing element,1,All,,0,Every 2 weeks,,,,,200.0,100.0,,50.0,25.0,40.0,20.0,35.0,17.5,325.0,162.5,,,,Yes,Yes,12.0,6.0,,,,,,,,,,,,,,,,,,,, +,completed,s.port@jeemayle.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,single log,,,2023,DLUHC,DLUHC,General needs,7,No,2023-06-26,2,Affordable Rent,,,,HIJKLMN,ABCDEFG,No,,,fake address,,London,,NW9 5LL,No,Barnet,E09000003,No,Affordable rent basis,Tenant abandoned property,2,2,House,Purpose built,Yes,3,2023-06-24,,,Yes,2023-06-25,,Don’t know,Yes,Assured Shorthold Tenancy (AST) – Fixed term,,2,,1,4,,1,4,0,0,2,35,,Female,White,Irish,Tenant prefers not to say,Other,Yes,Partner,32,Male,Not seeking work,No,Prefers not to say,Not known,Prefers not to say,Prefers not to say,Yes,Person prefers not to say,Not known,Person prefers not to say,Person prefers not to say,,,,,,,,,,,,,,,,,,,,,Yes – the person is a current or former regular,No – they left up to and including 5 years ago,Yes,No,Yes,Fully wheelchair accessible housing,1,0,0,0,0,0,No,Yes,0,0,1,0,0,0,0,0,0,0,Less than 1 year,1 year but under 2 years,Loss of tied accommodation,,Other supported housing,2,No,Yes,TN23 6LZ,Yes,No,Ashford,E07000105,Yes,0,1,0,0,0,0,0,1,,Tenant applied directly (no referral or nomination),,Yes,0,68,Weekly,,Universal Credit housing element,1,All,,0,Every 2 weeks,,,,,200.0,100.0,,50.0,25.0,40.0,20.0,35.0,17.5,325.0,162.5,,,,Yes,Yes,12.0,6.0,,,,,,,,,,,,,,,,,,,, diff --git a/spec/fixtures/files/lettings_log_csv_export_non_support_codes.csv b/spec/fixtures/files/lettings_log_csv_export_non_support_codes.csv index b25aa9aef..1d8c1d14e 100644 --- a/spec/fixtures/files/lettings_log_csv_export_non_support_codes.csv +++ b/spec/fixtures/files/lettings_log_csv_export_non_support_codes.csv @@ -1,2 +1,2 @@ id,status,created_by,is_dpo,created_at,updated_by,updated_at,creation_method,collection_start_year,owning_organisation_name,managing_organisation_name,lettype,renewal,startdate,irproduct,irproduct_other,lar,tenancycode,propcode,uprn_known,uprn,address_line1,address_line2,town_or_city,county,postcode_full,la_label,unitletas,rsnvac,newprop,offered,unittype_gn,builtype,wchair,beds,voiddate,void_date_value_check,majorrepairs,mrcdate,major_repairs_date_value_check,joint,startertenancy,tenancy,tenancyother,tenancylength,sheltered,declaration,refused,age1,sex1,ethnic_group,ethnic,national,ecstat1,relat2,age2,sex2,ecstat2,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,relat7,age7,sex7,ecstat7,relat8,age8,sex8,ecstat8,armedforces,leftreg,reservist,preg_occ,housingneeds,housingneeds_type,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_f,housingneeds_g,housingneeds_h,housingneeds_other,illness,illness_type_4,illness_type_5,illness_type_2,illness_type_6,illness_type_7,illness_type_3,illness_type_9,illness_type_8,illness_type_1,illness_type_10,layear,waityear,reason,reasonother,prevten,homeless,ppcodenk,ppostcode_full,prevloc_label,reasonpref,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,cbl,cap,chr,referral,referral_value_check,incref,earnings,incfreq,hb,has_benefits,benefits,household_charge,nocharge,period,chcharge,wchchrg,carehome_charges_value_check,brent,scharge,pscharge,supcharg,tcharge,scharge_value_check,pscharge_value_check,supcharg_value_check,hbrentshortfall,tshortfall,scheme_code,scheme_service_name,scheme_sensitive,SCHTYPE,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate -,completed,choreographer@owtluk.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,1,2023,DLUHC,DLUHC,7,0,2023-06-26T00:00:00+01:00,,,,HIJKLMN,ABCDEFG,0,,fake address,,London,,NW9 5LL,Barnet,2,6,2,2,7,1,1,3,2023-06-24T00:00:00+01:00,,1,2023-06-25T00:00:00+01:00,,3,1,4,,2,,1,1,35,F,0,2,13,0,P,32,M,6,R,-9,R,10,R,-9,R,10,,,,,,,,,,,,,,,,,1,4,1,2,1,0,1,0,0,0,0,0,0,1,0,0,1,0,0,0,0,0,0,0,2,7,4,,6,1,0,TN23 6LZ,Ashford,1,0,1,0,0,0,0,0,1,2,,0,68,1,6,1,1,,0,2,,,,200.0,50.0,40.0,35.0,325.0,,,,1,12.0,,,,,,,,,,,,,,,,,,,, +,completed,choreographer@owtluk.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,1,2023,DLUHC,DLUHC,7,0,2023-06-26,,,,HIJKLMN,ABCDEFG,0,,fake address,,London,,NW9 5LL,Barnet,2,6,2,2,7,1,1,3,2023-06-24,,1,2023-06-25,,3,1,4,,2,,1,1,35,F,0,2,13,0,P,32,M,6,R,-9,R,10,R,-9,R,10,,,,,,,,,,,,,,,,,1,4,1,2,1,0,1,0,0,0,0,0,0,1,0,0,1,0,0,0,0,0,0,0,2,7,4,,6,1,0,TN23 6LZ,Ashford,1,0,1,0,0,0,0,0,1,2,,0,68,1,6,1,1,,0,2,,,,200.0,50.0,40.0,35.0,325.0,,,,1,12.0,,,,,,,,,,,,,,,,,,,, diff --git a/spec/fixtures/files/lettings_log_csv_export_non_support_labels.csv b/spec/fixtures/files/lettings_log_csv_export_non_support_labels.csv index efc0ed0f7..d88c6622f 100644 --- a/spec/fixtures/files/lettings_log_csv_export_non_support_labels.csv +++ b/spec/fixtures/files/lettings_log_csv_export_non_support_labels.csv @@ -1,2 +1,2 @@ id,status,created_by,is_dpo,created_at,updated_by,updated_at,creation_method,collection_start_year,owning_organisation_name,managing_organisation_name,lettype,renewal,startdate,irproduct,irproduct_other,lar,tenancycode,propcode,uprn_known,uprn,address_line1,address_line2,town_or_city,county,postcode_full,la_label,unitletas,rsnvac,newprop,offered,unittype_gn,builtype,wchair,beds,voiddate,void_date_value_check,majorrepairs,mrcdate,major_repairs_date_value_check,joint,startertenancy,tenancy,tenancyother,tenancylength,sheltered,declaration,refused,age1,sex1,ethnic_group,ethnic,national,ecstat1,relat2,age2,sex2,ecstat2,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,relat7,age7,sex7,ecstat7,relat8,age8,sex8,ecstat8,armedforces,leftreg,reservist,preg_occ,housingneeds,housingneeds_type,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_f,housingneeds_g,housingneeds_h,housingneeds_other,illness,illness_type_4,illness_type_5,illness_type_2,illness_type_6,illness_type_7,illness_type_3,illness_type_9,illness_type_8,illness_type_1,illness_type_10,layear,waityear,reason,reasonother,prevten,homeless,ppcodenk,ppostcode_full,prevloc_label,reasonpref,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,cbl,cap,chr,referral,referral_value_check,incref,earnings,incfreq,hb,has_benefits,benefits,household_charge,nocharge,period,chcharge,wchchrg,carehome_charges_value_check,brent,scharge,pscharge,supcharg,tcharge,scharge_value_check,pscharge_value_check,supcharg_value_check,hbrentshortfall,tshortfall,scheme_code,scheme_service_name,scheme_sensitive,SCHTYPE,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate -,completed,choreographer@owtluk.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,single log,2023,DLUHC,DLUHC,7,No,2023-06-26T00:00:00+01:00,,,,HIJKLMN,ABCDEFG,No,,fake address,,London,,NW9 5LL,Barnet,Affordable rent basis,Tenant abandoned property,2,2,House,Purpose built,Yes,3,2023-06-24T00:00:00+01:00,,Yes,2023-06-25T00:00:00+01:00,,Don’t know,Yes,Assured Shorthold Tenancy (AST) – Fixed term,,2,,1,1,35,Female,White,Irish,Tenant prefers not to say,Other,Partner,32,Male,Not seeking work,Prefers not to say,Not known,Prefers not to say,Prefers not to say,Person prefers not to say,Not known,Person prefers not to say,Person prefers not to say,,,,,,,,,,,,,,,,,Yes – the person is a current or former regular,No – they left up to and including 5 years ago,Yes,No,Yes,Fully wheelchair accessible housing,1,0,0,0,0,0,No,Yes,0,0,1,0,0,0,0,0,0,0,Less than 1 year,1 year but under 2 years,Loss of tied accommodation,,Other supported housing,No,Yes,TN23 6LZ,Ashford,Yes,0,1,0,0,0,0,0,1,Tenant applied directly (no referral or nomination),,0,68,Weekly,Universal Credit housing element,1,All,,0,Every 2 weeks,,,,200.0,50.0,40.0,35.0,325.0,,,,Yes,12.0,,,,,,,,,,,,,,,,,,,, +,completed,choreographer@owtluk.com,false,2023-06-26T00:00:00+01:00,,2023-06-26T00:00:00+01:00,single log,2023,DLUHC,DLUHC,7,No,2023-06-26,,,,HIJKLMN,ABCDEFG,No,,fake address,,London,,NW9 5LL,Barnet,Affordable rent basis,Tenant abandoned property,2,2,House,Purpose built,Yes,3,2023-06-24,,Yes,2023-06-25,,Don’t know,Yes,Assured Shorthold Tenancy (AST) – Fixed term,,2,,1,1,35,Female,White,Irish,Tenant prefers not to say,Other,Partner,32,Male,Not seeking work,Prefers not to say,Not known,Prefers not to say,Prefers not to say,Person prefers not to say,Not known,Person prefers not to say,Person prefers not to say,,,,,,,,,,,,,,,,,Yes – the person is a current or former regular,No – they left up to and including 5 years ago,Yes,No,Yes,Fully wheelchair accessible housing,1,0,0,0,0,0,No,Yes,0,0,1,0,0,0,0,0,0,0,Less than 1 year,1 year but under 2 years,Loss of tied accommodation,,Other supported housing,No,Yes,TN23 6LZ,Ashford,Yes,0,1,0,0,0,0,0,1,Tenant applied directly (no referral or nomination),,0,68,Weekly,Universal Credit housing element,1,All,,0,Every 2 weeks,,,,200.0,50.0,40.0,35.0,325.0,,,,Yes,12.0,,,,,,,,,,,,,,,,,,,, From 0b4024bb5072574ea6237294abb9eaf7bf843fb5 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 12 Oct 2023 11:14:34 +0100 Subject: [PATCH 05/12] Bump postcss from 8.4.25 to 8.4.31 (#1969) Bumps [postcss](https://github.com/postcss/postcss) from 8.4.25 to 8.4.31. - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/postcss/postcss/compare/8.4.25...8.4.31) --- updated-dependencies: - dependency-name: postcss dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index decc34e47..906b56638 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4451,9 +4451,9 @@ postcss-value-parser@^4.1.0, postcss-value-parser@^4.2.0: integrity sha512-1NNCs6uurfkVbeXG4S8JFT9t19m45ICnif8zWLd5oPSZ50QnwMfK+H3jv408d4jw/7Bttv5axS5IiHoLaVNHeQ== postcss@^8.4.24, postcss@^8.4.7: - version "8.4.25" - resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.25.tgz#4a133f5e379eda7f61e906c3b1aaa9b81292726f" - integrity sha512-7taJ/8t2av0Z+sQEvNzCkpDynl0tX3uJMCODi6nT3PfASC7dYCWV9aQ+uiCf+KBD4SEFcu+GvJdGdwzQ6OSjCw== + version "8.4.31" + resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.31.tgz#92b451050a9f914da6755af352bdc0192508656d" + integrity sha512-PS08Iboia9mts/2ygV3eLpY5ghnUcfLV/EXTOW1E2qYxJKGGBUtNjN76FYHnMs36RmARn41bC0AZmn+rR0OVpQ== dependencies: nanoid "^3.3.6" picocolors "^1.0.0" From f4bb3e2610e10f3933ceea382030794253351a09 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 12 Oct 2023 12:02:39 +0100 Subject: [PATCH 06/12] Correctly map equity ranges (#1964) --- app/models/validations/sales/financial_validations.rb | 4 ++-- .../validations/sales/financial_validations_spec.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb index 82c57e75d..410f6d250 100644 --- a/app/models/validations/sales/financial_validations.rb +++ b/app/models/validations/sales/financial_validations.rb @@ -114,8 +114,8 @@ private }.freeze DEFAULT_EQUITY_RANGES = { - 2 => 10..75, - 30 => 25..75, + 2 => 25..75, + 30 => 10..75, 18 => 25..75, 16 => 10..75, 24 => 25..75, diff --git a/spec/models/validations/sales/financial_validations_spec.rb b/spec/models/validations/sales/financial_validations_spec.rb index ab41885ff..68cf19a14 100644 --- a/spec/models/validations/sales/financial_validations_spec.rb +++ b/spec/models/validations/sales/financial_validations_spec.rb @@ -311,16 +311,16 @@ RSpec.describe Validations::Sales::FinancialValidations do record.type = 2 record.equity = 1 financial_validator.validate_equity_in_range_for_year_and_type(record) - expect(record.errors["equity"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 10)) - expect(record.errors["type"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 10)) + expect(record.errors["equity"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 25)) + expect(record.errors["type"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 25)) end it "adds an error for type 30, equity below min with the correct percentage" do record.type = 30 record.equity = 1 financial_validator.validate_equity_in_range_for_year_and_type(record) - expect(record.errors["equity"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 25)) - expect(record.errors["type"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 25)) + expect(record.errors["equity"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 10)) + expect(record.errors["type"]).to include(match I18n.t("validations.financial.equity.under_min", min_equity: 10)) end it "does not add an error for equity in range with the correct percentage" do From 683dc1ad20d05be2ead7f3977f4c5b15e7c86cc6 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 12 Oct 2023 12:26:12 +0100 Subject: [PATCH 07/12] CLDC-2723 Allow data coordinators access to stock owners' schemes (#1958) * feat: allow data coordinators to create, edit and view stock owners' schemes * feat: only show data coordinators their org and stock owners in scheme owner select, update location policy * feat: update some tests * feat: update tests * feat: use zero? where appropriate and lint * feat: update tests * refactor: lint * feat: use helper for answer options --- app/controllers/schemes_controller.rb | 6 +- app/helpers/check_answers_helper.rb | 2 +- app/helpers/schemes_helper.rb | 25 +- app/models/scheme.rb | 2 +- app/policies/location_policy.rb | 12 +- app/policies/scheme_policy.rb | 14 +- app/views/schemes/check_answers.html.erb | 1 - app/views/schemes/details.html.erb | 12 +- app/views/schemes/edit_name.html.erb | 8 +- app/views/schemes/new.html.erb | 24 +- app/views/schemes/show.html.erb | 2 +- spec/features/schemes_spec.rb | 2 +- spec/helpers/schemes_helper_spec.rb | 62 +--- spec/requests/locations_controller_spec.rb | 6 +- spec/requests/schemes_controller_spec.rb | 323 ++++++++++++++++----- 15 files changed, 326 insertions(+), 175 deletions(-) diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 44341b48f..92669bcbe 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -273,7 +273,7 @@ private required_params[:sensitive] = required_params[:sensitive].to_i if required_params[:sensitive] - if current_user.data_coordinator? + if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? required_params[:owning_organisation_id] = current_user.organisation_id end required_params @@ -291,10 +291,6 @@ private @scheme end - def user_allowed_action? - current_user.support? || current_user.organisation == @scheme&.owning_organisation || current_user.organisation.parent_organisations.exists?(@scheme&.owning_organisation_id) - end - def redirect_if_scheme_confirmed redirect_to @scheme if @scheme.confirmed? end diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index ed07f3b56..80f4326f7 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -14,7 +14,7 @@ module CheckAnswersHelper def can_change_scheme_answer?(attribute_name, scheme) return false unless current_user.support? || current_user.data_coordinator? - editable_attributes = current_user.support? ? ["Name", "Confidential information", "Housing stock owned by"] : ["Name", "Confidential information"] + editable_attributes = ["Name", "Confidential information", "Housing stock owned by"] !scheme.confirmed? || editable_attributes.include?(attribute_name) end diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index f7da6d3f7..d111b402c 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -1,6 +1,6 @@ module SchemesHelper - def display_scheme_attributes(scheme, user) - base_attributes = [ + def display_scheme_attributes(scheme) + [ { name: "Scheme code", value: scheme.id_to_display }, { name: "Name", value: scheme.service_name, edit: true }, { name: "Status", value: status_tag_from_resource(scheme) }, @@ -16,16 +16,6 @@ module SchemesHelper { name: "Intended length of stay", value: scheme.intended_stay }, { name: "Availability", value: scheme_availability(scheme) }, ] - - if user.data_coordinator? - base_attributes.delete_if { |item| item[:name] == "Housing stock owned by" } - end - - if scheme.has_other_client_group == "Yes" - base_attributes.append - end - - base_attributes end def scheme_availability(scheme) @@ -44,6 +34,17 @@ module SchemesHelper return govuk_button_link_to "Reactivate this scheme", scheme_new_reactivation_path(scheme) if scheme.deactivated? end + def owning_organisation_options(current_user) + all_orgs = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } + user_org = [OpenStruct.new(id: current_user.organisation_id, name: current_user.organisation.name)] + stock_owners = current_user.organisation.stock_owners.map { |org| OpenStruct.new(id: org.id, name: org.name) } + current_user.support? ? all_orgs : user_org + stock_owners + end + + def null_option + [OpenStruct.new(id: "", name: "Select an option")] + end + private ActivePeriod = Struct.new(:from, :to) diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 55d15dfb2..19a6491fd 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -252,7 +252,7 @@ class Scheme < ApplicationRecord end def validate_owning_organisation - unless owning_organisation.holds_own_stock? + unless owning_organisation&.holds_own_stock? errors.add(:owning_organisation_id, :does_not_own_stock, message: I18n.t("validations.scheme.owning_organisation.does_not_own_stock")) end end diff --git a/app/policies/location_policy.rb b/app/policies/location_policy.rb index f10f96ef5..5d6d6d467 100644 --- a/app/policies/location_policy.rb +++ b/app/policies/location_policy.rb @@ -16,14 +16,14 @@ class LocationPolicy if location == Location user.data_coordinator? else - user.data_coordinator? && user.organisation == scheme&.owning_organisation + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner end end def update? return true if user.support? - user.data_coordinator? && scheme&.owning_organisation == user.organisation + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner end %w[ @@ -51,7 +51,7 @@ class LocationPolicy define_method method_name do return true if user.support? - user.data_coordinator? && scheme&.owning_organisation == user.organisation + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner end end @@ -62,7 +62,7 @@ class LocationPolicy define_method method_name do return true if user.support? - user.organisation.parent_organisations.exists?(scheme&.owning_organisation_id) || scheme&.owning_organisation == user.organisation + scheme_owned_by_user_org_or_stock_owner end end @@ -71,4 +71,8 @@ private def scheme location.scheme end + + def scheme_owned_by_user_org_or_stock_owner + scheme&.owning_organisation == user.organisation || user.organisation.stock_owners.exists?(scheme&.owning_organisation_id) + end end diff --git a/app/policies/scheme_policy.rb b/app/policies/scheme_policy.rb index 39842a160..30aa1543c 100644 --- a/app/policies/scheme_policy.rb +++ b/app/policies/scheme_policy.rb @@ -12,7 +12,7 @@ class SchemePolicy if scheme == Scheme true else - user.organisation.parent_organisations.exists?(scheme&.owning_organisation_id) || scheme&.owning_organisation == user.organisation + scheme_owned_by_user_org_or_stock_owner end end @@ -27,7 +27,7 @@ class SchemePolicy def update? return true if user.support? - user.data_coordinator? && (scheme&.owning_organisation == user.organisation) + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner end %w[ @@ -37,7 +37,7 @@ class SchemePolicy define_method method_name do return true if user.support? - user.organisation.parent_organisations.exists?(scheme&.owning_organisation_id) || scheme&.owning_organisation == user.organisation + scheme_owned_by_user_org_or_stock_owner end end @@ -57,7 +57,13 @@ class SchemePolicy define_method method_name do return true if user.support? - user.data_coordinator? && scheme&.owning_organisation == user.organisation + user.data_coordinator? && scheme_owned_by_user_org_or_stock_owner end end + +private + + def scheme_owned_by_user_org_or_stock_owner + scheme&.owning_organisation == user.organisation || user.organisation.stock_owners.exists?(scheme&.owning_organisation_id) + end end diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb index 7924768f0..dccefc565 100644 --- a/app/views/schemes/check_answers.html.erb +++ b/app/views/schemes/check_answers.html.erb @@ -9,7 +9,6 @@

Scheme

<% @scheme.check_details_attributes.each do |attr| %> - <% next if current_user.data_coordinator? && attr[:name] == ("owned by") %> <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: @scheme.confirmed? ? scheme_edit_name_path(@scheme) : scheme_details_path(@scheme, check_answers: true) } %> <% end %> diff --git a/app/views/schemes/details.html.erb b/app/views/schemes/details.html.erb index 0e4f3a7d9..3dc6208b1 100644 --- a/app/views/schemes/details.html.erb +++ b/app/views/schemes/details.html.erb @@ -25,10 +25,6 @@ label: { text: "This scheme contains confidential information" } %> <% end %> - <% if current_user.data_coordinator? %> - <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> - <% end %> - <% scheme_types_selection = Scheme.scheme_types.keys.excluding("Missing").map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %> <%= f.govuk_collection_radio_buttons :scheme_type, @@ -48,11 +44,11 @@ :description, legend: { text: "Is this scheme registered under the Care Standards Act 2000?", size: "m" } %> - <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> - - <% if current_user.support? %> + <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? %> + <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> + <% else %> <%= f.govuk_collection_select :owning_organisation_id, - organisations, + owning_organisation_options(current_user), :id, :name, label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, diff --git a/app/views/schemes/edit_name.html.erb b/app/views/schemes/edit_name.html.erb index 1dc8bc147..0c7c1091e 100644 --- a/app/views/schemes/edit_name.html.erb +++ b/app/views/schemes/edit_name.html.erb @@ -25,11 +25,11 @@ label: { text: "This scheme contains confidential information" } %> <% end %> - <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> - - <% if current_user.support? %> + <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? %> + <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> + <% else %> <%= f.govuk_collection_select :owning_organisation_id, - organisations, + owning_organisation_options(current_user), :id, :name, label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, diff --git a/app/views/schemes/new.html.erb b/app/views/schemes/new.html.erb index 81c5febbf..af5c2089a 100644 --- a/app/views/schemes/new.html.erb +++ b/app/views/schemes/new.html.erb @@ -26,14 +26,6 @@ label: { text: "This scheme contains confidential information" } %> <% end %> - <% null_option = [OpenStruct.new(id: "", name: "Select an option")] %> - <% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %> - <% answer_options = null_option + organisations %> - - <% if current_user.data_coordinator? %> - <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> - <% end %> - <% scheme_types_selection = Scheme.scheme_types.keys.excluding("Missing").map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %> <%= f.govuk_collection_radio_buttons :scheme_type, scheme_types_selection, @@ -52,13 +44,15 @@ :description, legend: { text: "Is this scheme registered under the Care Standards Act 2000?", size: "m" } %> - <% if current_user.support? %> - <%= f.govuk_collection_select :owning_organisation_id, - answer_options, - :id, - :name, - label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, - "data-controller": %w[accessible-autocomplete conditional-filter] %> + <% if current_user.data_coordinator? && current_user.organisation.stock_owners.count.zero? %> + <%= f.hidden_field :owning_organisation_id, value: current_user.organisation.id %> + <% else %> + <%= f.govuk_collection_select :owning_organisation_id, + null_option + owning_organisation_options(current_user), + :id, + :name, + label: { text: "Which organisation owns the housing stock for this scheme?", size: "m" }, + "data-controller": %w[accessible-autocomplete conditional-filter] %> <% end %> <% mantype_selection = Scheme.arrangement_types.keys.excluding("Missing").map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %> diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb index 66209fa28..7115dc07e 100644 --- a/app/views/schemes/show.html.erb +++ b/app/views/schemes/show.html.erb @@ -16,7 +16,7 @@

Scheme

<%= govuk_summary_list do |summary_list| %> - <% display_scheme_attributes(@scheme, current_user).each do |attr| %> + <% display_scheme_attributes(@scheme).each do |attr| %> <%= summary_list.row do |row| %> <% row.key { attr[:name] } %> <% row.value do %> diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index dd683312e..12bd76697 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -1069,7 +1069,7 @@ RSpec.describe "Schemes scheme Features" do it "lets me see amended details on the check answers page" do expect(page).to have_content "FooBar" expect(page).to have_current_path("/schemes/#{scheme.id}/check-answers") - expect(page).to have_link("Change", href: /schemes\/#{scheme.id}\/edit-name/, count: 2) + expect(page).to have_link("Change", href: /schemes\/#{scheme.id}\/edit-name/, count: 3) end end end diff --git a/spec/helpers/schemes_helper_spec.rb b/spec/helpers/schemes_helper_spec.rb index 95d3c78e3..ef4b5fcec 100644 --- a/spec/helpers/schemes_helper_spec.rb +++ b/spec/helpers/schemes_helper_spec.rb @@ -111,7 +111,7 @@ RSpec.describe SchemesHelper do let(:coordinator_user) { FactoryBot.create(:user, :data_coordinator) } context "when scheme has no locations" do - it "returns correct display attributes for a support user" do + it "returns correct display attributes" do attributes = [ { name: "Scheme code", value: "S#{scheme.id}" }, { name: "Name", value: "Test service_name", edit: true }, @@ -128,26 +128,7 @@ RSpec.describe SchemesHelper do { name: "Intended length of stay", value: "Permanent" }, { name: "Availability", value: "Active from 1 April 2021" }, ] - expect(display_scheme_attributes(scheme, support_user)).to eq(attributes) - end - - it "returns correct display attributes for a coordinator user" do - attributes = [ - { name: "Scheme code", value: "S#{scheme.id}" }, - { name: "Name", value: "Test service_name", edit: true }, - { name: "Status", value: status_tag(:incomplete) }, - { name: "Confidential information", value: "No", edit: true }, - { name: "Type of scheme", value: "Housing for older people" }, - { name: "Registered under Care Standards Act 2000", value: "Yes – registered care home providing personal care" }, - { name: "Support services provided by", value: "A registered charity or voluntary organisation" }, - { name: "Primary client group", value: "Rough sleepers" }, - { name: "Has another client group", value: "Yes" }, - { name: "Secondary client group", value: "Refugees (permanent)" }, - { name: "Level of support given", value: "High level" }, - { name: "Intended length of stay", value: "Permanent" }, - { name: "Availability", value: "Active from 1 April 2021" }, - ] - expect(display_scheme_attributes(scheme, coordinator_user)).to eq(attributes) + expect(display_scheme_attributes(scheme)).to eq(attributes) end end @@ -156,7 +137,7 @@ RSpec.describe SchemesHelper do FactoryBot.create(:location, scheme:) end - it "returns correct display attributes for a support user" do + it "returns correct display attributes" do attributes = [ { name: "Scheme code", value: "S#{scheme.id}" }, { name: "Name", value: "Test service_name", edit: true }, @@ -173,31 +154,12 @@ RSpec.describe SchemesHelper do { name: "Intended length of stay", value: "Permanent" }, { name: "Availability", value: "Active from 1 April 2021" }, ] - expect(display_scheme_attributes(scheme, support_user)).to eq(attributes) - end - - it "returns correct display attributes for a coordinator user" do - attributes = [ - { name: "Scheme code", value: "S#{scheme.id}" }, - { name: "Name", value: "Test service_name", edit: true }, - { name: "Status", value: status_tag(:active) }, - { name: "Confidential information", value: "No", edit: true }, - { name: "Type of scheme", value: "Housing for older people" }, - { name: "Registered under Care Standards Act 2000", value: "Yes – registered care home providing personal care" }, - { name: "Support services provided by", value: "A registered charity or voluntary organisation" }, - { name: "Primary client group", value: "Rough sleepers" }, - { name: "Has another client group", value: "Yes" }, - { name: "Secondary client group", value: "Refugees (permanent)" }, - { name: "Level of support given", value: "High level" }, - { name: "Intended length of stay", value: "Permanent" }, - { name: "Availability", value: "Active from 1 April 2021" }, - ] - expect(display_scheme_attributes(scheme, coordinator_user)).to eq(attributes) + expect(display_scheme_attributes(scheme)).to eq(attributes) end context "when the managing organisation is the owning organisation" do it "doesn't show the organisation providing support" do - attributes = display_scheme_attributes(scheme_where_managing_organisation_is_owning_organisation, support_user).find { |x| x[:name] == "Organisation providing support" } + attributes = display_scheme_attributes(scheme_where_managing_organisation_is_owning_organisation).find { |x| x[:name] == "Organisation providing support" } expect(attributes).to be_nil end end @@ -206,7 +168,7 @@ RSpec.describe SchemesHelper do context "with no deactivations" do it "displays current collection start date as availability date if created_at is later than collection start date" do scheme.update!(created_at: Time.zone.local(2022, 4, 16)) - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021") end @@ -221,7 +183,7 @@ RSpec.describe SchemesHelper do end it "displays the timeline of availability" do - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021 to 9 August 2022\nDeactivated on 10 August 2022\nActive from 1 September 2022 to 14 September 2022\nDeactivated on 15 September 2022\nActive from 28 September 2022") end @@ -235,7 +197,7 @@ RSpec.describe SchemesHelper do end it "displays the timeline of availability" do - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021 to 9 August 2022\nDeactivated on 10 August 2022\nActive from 1 September 2022 to 14 September 2022\nDeactivated on 15 September 2022") end @@ -251,7 +213,7 @@ RSpec.describe SchemesHelper do end it "displays the timeline of availability" do - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021 to 14 June 2022\nDeactivated on 15 June 2022\nActive from 18 June 2022 to 23 September 2022\nDeactivated on 24 September 2022\nActive from 28 September 2022") end @@ -265,7 +227,7 @@ RSpec.describe SchemesHelper do end it "displays the timeline of availability" do - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021 to 14 June 2022\nDeactivated on 15 June 2022\nActive from 28 September 2022") end @@ -282,7 +244,7 @@ RSpec.describe SchemesHelper do end it "displays the timeline of availability" do - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021 to 14 June 2022\nDeactivated on 15 June 2022\nActive from 28 September 2022 to 23 October 2022\nDeactivated on 24 October 2022\nActive from 28 October 2022") end @@ -297,7 +259,7 @@ RSpec.describe SchemesHelper do end it "displays the timeline of availability" do - availability_attribute = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Availability" }[:value] + availability_attribute = display_scheme_attributes(scheme).find { |x| x[:name] == "Availability" }[:value] expect(availability_attribute).to eq("Active from 1 April 2021 to 9 October 2022\nDeactivated on 10 October 2022\nActive from 11 December 2022") end diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index 7dccbec1b..fd3bd48e5 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -179,7 +179,7 @@ RSpec.describe LocationsController, type: :request do get "/schemes/#{scheme.id}/locations" end - context "when coordinator attempts to see scheme belonging to a different organisation" do + context "when coordinator attempts to see scheme belonging to a different (and not their parent) organisation" do let(:another_scheme) { create(:scheme) } before do @@ -302,8 +302,8 @@ RSpec.describe LocationsController, type: :request do end end - it "does not allow adding new locations" do - expect(page).not_to have_button("Add a location") + it "does allow adding new locations" do + expect(page).to have_button("Add a location") end end end diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 50d83a5b3..9d322ec87 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -457,7 +457,7 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content(specific_scheme.intended_stay) end - context "when coordinator attempts to see scheme belonging to a different organisation" do + context "when coordinator attempts to see scheme belonging to a different (and not their parent) organisation" do let!(:specific_scheme) { create(:scheme) } it "returns 401" do @@ -474,7 +474,6 @@ RSpec.describe SchemesController, type: :request do end context "when looking at scheme details" do - let(:user) { create(:user, :data_coordinator) } let!(:scheme) { create(:scheme, owning_organisation: user.organisation) } let(:add_deactivations) { scheme.scheme_deactivation_periods << scheme_deactivation_period } @@ -535,21 +534,68 @@ RSpec.describe SchemesController, type: :request do context "when coordinator attempts to see scheme belonging to a parent organisation" do let(:parent_organisation) { create(:organisation) } let!(:specific_scheme) { create(:scheme, owning_organisation: parent_organisation) } + let(:add_deactivations) { specific_scheme.scheme_deactivation_periods << scheme_deactivation_period } before do create(:location, scheme: specific_scheme) create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation) + Timecop.freeze(Time.utc(2022, 10, 10)) + sign_in user + add_deactivations + specific_scheme.save! get "/schemes/#{specific_scheme.id}" end - it "shows the scheme" do - expect(page).to have_content(specific_scheme.id_to_display) + after do + Timecop.unfreeze end - it "does not allow editing the scheme" do - expect(page).not_to have_link("Change") - expect(page).not_to have_content("Reactivate this scheme") - expect(page).not_to have_content("Deactivate this scheme") + context "with active scheme" do + let(:add_deactivations) {} + + it "shows the scheme" do + expect(page).to have_content(specific_scheme.id_to_display) + end + + it "allows editing" do + expect(page).to have_link("Change") + end + + it "renders deactivate this scheme" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Deactivate this scheme", href: "/schemes/#{specific_scheme.id}/new-deactivation") + end + end + + context "with deactivated scheme" do + let(:scheme_deactivation_period) { create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 10, 9), scheme: specific_scheme) } + + it "renders reactivate this scheme" do + expect(response).to have_http_status(:ok) + expect(page).to have_link("Reactivate this scheme", href: "/schemes/#{specific_scheme.id}/new-reactivation") + end + end + + context "with scheme that's deactivating soon" do + let(:scheme_deactivation_period) { create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2022, 10, 12), scheme: specific_scheme) } + + it "does not render toggle scheme link" do + expect(response).to have_http_status(:ok) + expect(page).not_to have_link("Reactivate this scheme") + expect(page).not_to have_link("Deactivate this scheme") + end + end + + context "with scheme that's deactivating in more than 6 months" do + let(:scheme_deactivation_period) { create(:scheme_deactivation_period, deactivation_date: Time.zone.local(2023, 5, 12), scheme: specific_scheme) } + + it "does not render toggle scheme link" do + expect(response).to have_http_status(:ok) + expect(page).not_to have_link("Reactivate this scheme") + expect(page).to have_link("Deactivate this scheme") + expect(response.body).not_to include("Deactivating soon") + expect(response.body).to include("Active") + end end end @@ -678,62 +724,159 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { create(:user, :data_coordinator) } - let(:params) do - { scheme: { service_name: " testy ", - sensitive: "1", - scheme_type: "Foyer", - registered_under_care_act: "No", - arrangement_type: "D" } } - end before do sign_in user end - it "creates a new scheme for user organisation with valid params and renders correct page" do - expect { post "/schemes", params: }.to change(Scheme, :count).by(1) - follow_redirect! - expect(response).to have_http_status(:ok) - expect(page).to have_content("What client group is this scheme intended for?") - end + context "when making a scheme in the user's organisation" do + let!(:params) do + { scheme: { service_name: " testy ", + sensitive: "1", + scheme_type: "Foyer", + registered_under_care_act: "No", + owning_organisation_id: user.organisation.id, + arrangement_type: "D" } } + end - it "creates a new scheme for user organisation with valid params" do - post "/schemes", params: params + it "creates a new scheme for user organisation with valid params and renders correct page" do + expect { post "/schemes", params: }.to change(Scheme, :count).by(1) + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content("What client group is this scheme intended for?") + end - expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) - expect(Scheme.last.service_name).to eq("testy") - expect(Scheme.last.scheme_type).to eq("Foyer") - expect(Scheme.last.sensitive).to eq("Yes") - expect(Scheme.last.registered_under_care_act).to eq("No") - expect(Scheme.last.id).not_to eq(nil) - expect(Scheme.last.has_other_client_group).to eq(nil) - expect(Scheme.last.primary_client_group).to eq(nil) - expect(Scheme.last.secondary_client_group).to eq(nil) - expect(Scheme.last.support_type).to eq(nil) - expect(Scheme.last.intended_stay).to eq(nil) - expect(Scheme.last.id_to_display).to match(/S*/) + it "creates a new scheme for user organisation with valid params" do + post "/schemes", params: params + + expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + expect(Scheme.last.service_name).to eq("testy") + expect(Scheme.last.scheme_type).to eq("Foyer") + expect(Scheme.last.sensitive).to eq("Yes") + expect(Scheme.last.registered_under_care_act).to eq("No") + expect(Scheme.last.id).not_to eq(nil) + expect(Scheme.last.has_other_client_group).to eq(nil) + expect(Scheme.last.primary_client_group).to eq(nil) + expect(Scheme.last.secondary_client_group).to eq(nil) + expect(Scheme.last.support_type).to eq(nil) + expect(Scheme.last.intended_stay).to eq(nil) + expect(Scheme.last.id_to_display).to match(/S*/) + end + + context "when support services provider is selected" do + let(:params) do + { scheme: { service_name: "testy", + sensitive: "1", + scheme_type: "Foyer", + registered_under_care_act: "No", + owning_organisation_id: user.organisation.id, + arrangement_type: "R" } } + end + + it "creates a new scheme for user organisation with valid params and renders correct page" do + expect { post "/schemes", params: }.to change(Scheme, :count).by(1) + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content(" What client group is this scheme intended for?") + end + + it "creates a new scheme for user organisation with valid params" do + post "/schemes", params: params + + expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + expect(Scheme.last.service_name).to eq("testy") + expect(Scheme.last.scheme_type).to eq("Foyer") + expect(Scheme.last.sensitive).to eq("Yes") + expect(Scheme.last.registered_under_care_act).to eq("No") + expect(Scheme.last.id).not_to eq(nil) + expect(Scheme.last.has_other_client_group).to eq(nil) + expect(Scheme.last.primary_client_group).to eq(nil) + expect(Scheme.last.secondary_client_group).to eq(nil) + expect(Scheme.last.support_type).to eq(nil) + expect(Scheme.last.intended_stay).to eq(nil) + expect(Scheme.last.id_to_display).to match(/S*/) + end + end + + context "when required params are missing" do + let(:params) do + { scheme: { service_name: "", + scheme_type: "", + registered_under_care_act: "", + arrangement_type: "" } } + end + + it "renders the same page with error message" do + post "/schemes", params: params + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Create a new supported housing scheme") + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.scheme_type.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.registered_under_care_act.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.arrangement_type.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.service_name.invalid")) + end + end + + context "when there are no stock owners" do + let(:params) do + { scheme: { service_name: " testy ", + sensitive: "1", + scheme_type: "Foyer", + registered_under_care_act: "No", + arrangement_type: "D" } } + end + + before do + user.organisation.stock_owners.destroy_all + end + + it "infers the user's organisation" do + post "/schemes", params: params + expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + end + end + + context "when the organisation id param is included" do + let(:organisation) { create(:organisation) } + let(:params) { { scheme: { owning_organisation: organisation } } } + + it "sets the owning organisation correctly" do + post "/schemes", params: params + expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + end + end end - context "when support services provider is selected" do + context "when making a scheme in a parent organisation of the user's organisation" do + let(:parent_organisation) { create(:organisation) } + let!(:parent_schemes) { create_list(:scheme, 5, owning_organisation: parent_organisation) } let(:params) do - { scheme: { service_name: "testy", + { scheme: { service_name: " testy ", sensitive: "1", scheme_type: "Foyer", registered_under_care_act: "No", - arrangement_type: "R" } } + owning_organisation_id: user.organisation.stock_owners.first.id, + arrangement_type: "D" } } + end + + before do + create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation) + parent_schemes.each do |scheme| + create(:location, scheme:) + end end it "creates a new scheme for user organisation with valid params and renders correct page" do expect { post "/schemes", params: }.to change(Scheme, :count).by(1) follow_redirect! expect(response).to have_http_status(:ok) - expect(page).to have_content(" What client group is this scheme intended for?") + expect(page).to have_content("What client group is this scheme intended for?") end it "creates a new scheme for user organisation with valid params" do post "/schemes", params: params - expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + expect(Scheme.last.owning_organisation_id).to eq(user.organisation.stock_owners.first.id) expect(Scheme.last.service_name).to eq("testy") expect(Scheme.last.scheme_type).to eq("Foyer") expect(Scheme.last.sensitive).to eq("Yes") @@ -746,34 +889,71 @@ RSpec.describe SchemesController, type: :request do expect(Scheme.last.intended_stay).to eq(nil) expect(Scheme.last.id_to_display).to match(/S*/) end - end - context "when required params are missing" do - let(:params) do - { scheme: { service_name: "", - scheme_type: "", - registered_under_care_act: "", - arrangement_type: "" } } + context "when support services provider is selected" do + let(:params) do + { scheme: { service_name: "testy", + sensitive: "1", + scheme_type: "Foyer", + registered_under_care_act: "No", + owning_organisation_id: user.organisation.stock_owners.first.id, + arrangement_type: "R" } } + end + + it "creates a new scheme for user organisation with valid params and renders correct page" do + expect { post "/schemes", params: }.to change(Scheme, :count).by(1) + follow_redirect! + expect(response).to have_http_status(:ok) + expect(page).to have_content(" What client group is this scheme intended for?") + end + + it "creates a new scheme for user organisation with valid params" do + post "/schemes", params: params + + expect(Scheme.last.owning_organisation_id).to eq(user.organisation.stock_owners.first.id) + expect(Scheme.last.service_name).to eq("testy") + expect(Scheme.last.scheme_type).to eq("Foyer") + expect(Scheme.last.sensitive).to eq("Yes") + expect(Scheme.last.registered_under_care_act).to eq("No") + expect(Scheme.last.id).not_to eq(nil) + expect(Scheme.last.has_other_client_group).to eq(nil) + expect(Scheme.last.primary_client_group).to eq(nil) + expect(Scheme.last.secondary_client_group).to eq(nil) + expect(Scheme.last.support_type).to eq(nil) + expect(Scheme.last.intended_stay).to eq(nil) + expect(Scheme.last.id_to_display).to match(/S*/) + end end - it "renders the same page with error message" do - post "/schemes", params: params - expect(response).to have_http_status(:unprocessable_entity) - expect(page).to have_content("Create a new supported housing scheme") - expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.scheme_type.invalid")) - expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.registered_under_care_act.invalid")) - expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.arrangement_type.invalid")) - expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.service_name.invalid")) + context "when required params are missing" do + let(:params) do + { scheme: { service_name: "", + scheme_type: "", + registered_under_care_act: "", + arrangement_type: "", + owning_organisation_id: "" } } + end + + it "renders the same page with error message" do + post "/schemes", params: params + expect(response).to have_http_status(:unprocessable_entity) + expect(page).to have_content("Create a new supported housing scheme") + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.scheme_type.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.registered_under_care_act.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.arrangement_type.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.owning_organisation_id.invalid")) + expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.service_name.invalid")) + end end - end - context "when the organisation id param is included" do - let(:organisation) { create(:organisation) } - let(:params) { { scheme: { owning_organisation: organisation } } } + context "when the organisation id param is included" do + let(:organisation) { create(:organisation) } + let(:params) { { scheme: { owning_organisation: organisation } } } - it "sets the owning organisation correctly" do - post "/schemes", params: params - expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + it "sets the owning organisation correctly" do + post "/schemes", params: params + expect(Scheme.last.owning_organisation_id).to eq(user.organisation.stock_owners.first.id) + end end end end @@ -1813,7 +1993,7 @@ RSpec.describe SchemesController, type: :request do expect(response).to have_http_status(:ok) expect(path).to match("/schemes/#{scheme.id}") expect(page).to have_content(scheme.service_name) - assert_select "a", text: /Change/, count: 2 + assert_select "a", text: /Change/, count: 3 end end end @@ -1958,7 +2138,7 @@ RSpec.describe SchemesController, type: :request do expect(response).to have_http_status(:ok) expect(path).to match("/schemes/#{scheme.id}") expect(page).to have_content(scheme.service_name) - assert_select "a", text: /Change/, count: 2 + assert_select "a", text: /Change/, count: 3 end end end @@ -2016,7 +2196,20 @@ RSpec.describe SchemesController, type: :request do expect(response).to have_http_status(:ok) expect(page).to have_content("Scheme details") expect(page).to have_content("This scheme contains confidential information") - expect(page).not_to have_content("Which organisation owns the housing stock for this scheme?") + end + + context "when there are stock owners" do + let(:parent_organisation) { create(:organisation) } + + before do + create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation) + get "/schemes/#{scheme.id}/edit-name" + end + + it "includes the owning organisation question" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("Which organisation owns the housing stock for this scheme?") + end end context "when attempting to access secondary-client-group scheme page for another organisation" do From 4068d7a2f6ad80558e3a3724efab06be10b0a7e9 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:04:59 +0100 Subject: [PATCH 08/12] CLDC-2871 Fix bulk upload scheme lookup bug (#1971) * feat: wip tests * feat: update tests and functionality * refactor: lint * feat: update tests and don't add errors to scheme/location fields unless determined * feat: update duplicate log behaviour and tests * refactor: lint * feat: use needstype helpers * feat: use needstype helpers and test tweak * feat: make tests explicit regarding needstype * feat: make tests explicit regarding needstype --- .../lettings/year2023/row_parser.rb | 112 +++- .../lettings/year2023/row_parser_spec.rb | 598 +++++++++++++----- 2 files changed, 532 insertions(+), 178 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2023/row_parser.rb b/app/services/bulk_upload/lettings/year2023/row_parser.rb index 9e7d8eecf..413e0b10f 100644 --- a/app/services/bulk_upload/lettings/year2023/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2023/row_parser.rb @@ -329,9 +329,26 @@ class BulkUpload::Lettings::Year2023::RowParser }, on: :after_log + validates :field_15, + presence: { + if: proc { supported_housing? && log_uses_old_scheme_id? }, + message: I18n.t("validations.not_answered", question: "management group code"), + category: :setup, + }, + on: :after_log + validates :field_16, presence: { - if: proc { [2, 4, 6, 8, 10, 12].include?(field_5) }, + if: proc { supported_housing? }, + message: I18n.t("validations.not_answered", question: "scheme code"), + category: :setup, + }, + on: :after_log + + validates :field_17, + presence: { + if: proc { supported_housing? && log_uses_new_scheme_id? }, + message: I18n.t("validations.not_answered", question: "location code"), category: :setup, }, on: :after_log @@ -464,9 +481,9 @@ class BulkUpload::Lettings::Year2023::RowParser "field_8", # startdate "field_9", # startdate "field_13", # tenancycode - field_4 != 1 ? "field_17" : nil, # location - field_4 != 2 ? "field_23" : nil, # postcode - field_4 != 2 ? "field_24" : nil, # postcode + !general_needs? ? location_field.to_s : nil, # location + !supported_housing? ? "field_23" : nil, # postcode + !supported_housing? ? "field_24" : nil, # postcode "field_46", # age1 "field_47", # sex1 "field_50", # ecstat1 @@ -560,8 +577,8 @@ private "ecstat1", "owning_organisation", "tcharge", - field_4 != 2 ? "postcode_full" : nil, - field_4 != 1 ? "location" : nil, + !supported_housing? ? "postcode_full" : nil, + !general_needs? ? "location" : nil, "tenancycode", log.chcharge.present? ? "chcharge" : nil, ].compact @@ -732,45 +749,45 @@ private def validate_location_related return if scheme.blank? || location.blank? - unless location.scheme == scheme + if location.scheme != scheme && location_field.present? block_log_creation! - errors.add(:field_17, "Scheme code must relate to a location that is owned by owning organisation or managing organisation", category: :setup) + errors.add(location_field, "#{scheme_or_management_group.capitalize} code must relate to a #{location_or_scheme} that is owned by owning organisation or managing organisation", category: :setup) end end def validate_location_exists - if scheme && field_17.present? && location.nil? - errors.add(:field_17, "Location could not be found with the provided scheme code", category: :setup) + if scheme && location_id.present? && location.nil? && location_field.present? + errors.add(location_field, "#{location_or_scheme.capitalize} could not be found with the provided #{scheme_or_management_group} code", category: :setup) end end def validate_location_data_given - if supported_housing? && field_17.blank? - errors.add(:field_17, I18n.t("validations.not_answered", question: "scheme code"), category: "setup") + if supported_housing? && location_id.blank? && location_field.present? + errors.add(location_field, I18n.t("validations.not_answered", question: "#{location_or_scheme} code"), category: "setup") end end def validate_scheme_related - return unless field_16.present? && scheme.present? + return unless scheme_id.present? && scheme.present? owned_by_owning_org = owning_organisation && scheme.owning_organisation == owning_organisation owned_by_managing_org = managing_organisation && scheme.owning_organisation == managing_organisation - unless owned_by_owning_org || owned_by_managing_org + if !(owned_by_owning_org || owned_by_managing_org) && scheme_field.present? block_log_creation! - errors.add(:field_16, "This management group code does not belong to your organisation, or any of your stock owners / managing agents", category: :setup) + errors.add(scheme_field, "This #{scheme_or_management_group} code does not belong to your organisation, or any of your stock owners / managing agents", category: :setup) end end def validate_scheme_exists - if field_16.present? && scheme.nil? - errors.add(:field_16, "The management group code is not correct", category: :setup) + if scheme_id.present? && scheme_field.present? && scheme.nil? + errors.add(scheme_field, "The #{scheme_or_management_group} code is not correct", category: :setup) end end def validate_scheme_data_given - if supported_housing? && field_16.blank? - errors.add(:field_16, I18n.t("validations.not_answered", question: "management group code"), category: "setup") + if supported_housing? && scheme_field.present? && scheme_id.blank? + errors.add(scheme_field, I18n.t("validations.not_answered", question: "#{scheme_or_management_group} code"), category: "setup") end end @@ -851,16 +868,17 @@ private errors.add(:field_8, error_message) # startdate errors.add(:field_9, error_message) # startdate errors.add(:field_13, error_message) # tenancycode - errors.add(:field_17, error_message) if field_4 != 1 # location - errors.add(:field_23, error_message) if field_4 != 2 # postcode_full - errors.add(:field_24, error_message) if field_4 != 2 # postcode_full - errors.add(:field_25, error_message) if field_4 != 2 # la + errors.add(location_field, error_message) if !general_needs? && location_field.present? # location + errors.add(:field_16, error_message) if !general_needs? && location_field.blank? # add to Scheme field as unclear whether log uses New or Old CORE ids + errors.add(:field_23, error_message) unless supported_housing? # postcode_full + errors.add(:field_24, error_message) unless supported_housing? # postcode_full + errors.add(:field_25, error_message) unless supported_housing? # la errors.add(:field_46, error_message) # age1 errors.add(:field_47, error_message) # sex1 errors.add(:field_50, error_message) # ecstat1 errors.add(:field_132, error_message) # tcharge errors.add(:field_127, error_message) if log.chcharge.present? # chcharge - errors.add(:field_125, error_message) if bulk_upload.needstype != 1 # household_charge + errors.add(:field_125, error_message) unless general_needs? # household_charge end end @@ -876,8 +894,8 @@ private owning_organisation_id: [:field_1], managing_organisation_id: [:field_2], renewal: [:field_6], - scheme: %i[field_16], - location: %i[field_17], + scheme: [scheme_field], + location: [location_field], created_by: [:field_3], needstype: [:field_4], rent_type: %i[field_5 field_10 field_11], @@ -1258,13 +1276,51 @@ private end def scheme - @scheme ||= Scheme.find_by_id_on_multiple_fields(field_16) + return if field_16.nil? + + @scheme ||= Scheme.find_by_id_on_multiple_fields(scheme_id) end def location return if scheme.nil? - @location ||= scheme.locations.find_by_id_on_multiple_fields(field_17) + @location ||= scheme.locations.find_by_id_on_multiple_fields(location_id) + end + + def log_uses_new_scheme_id? + field_16&.start_with?("S") + end + + def log_uses_old_scheme_id? + field_16.present? && !field_16.start_with?("S") + end + + def scheme_field + return :field_16 if log_uses_new_scheme_id? + return :field_15 if log_uses_old_scheme_id? + end + + def scheme_id + return field_16 if log_uses_new_scheme_id? + return field_15 if log_uses_old_scheme_id? + end + + def location_field + return :field_17 if log_uses_new_scheme_id? + return :field_16 if log_uses_old_scheme_id? + end + + def location_id + return field_17 if log_uses_new_scheme_id? + return field_16 if log_uses_old_scheme_id? + end + + def scheme_or_management_group + log_uses_new_scheme_id? ? "scheme" : "management group" + end + + def location_or_scheme + log_uses_new_scheme_id? ? "location" : "scheme" end def renttype diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index d532c8094..6959dff29 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -20,7 +20,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do field_1: owning_org.old_visible_id, field_2: managing_org.old_visible_id, field_4: "1", - field_5: "2", + field_5: "1", field_6: "2", field_7: now.day.to_s, field_8: now.month.to_s, @@ -327,7 +327,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do :field_8, # startdate :field_9, # startdate :field_13, # tenancycode - :field_17, # location + :field_16, # location :field_46, # age1 :field_47, # sex1 :field_50, # ecstat1 @@ -342,106 +342,290 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - context "when a supported housing log with chcharges already exists in the db" do - let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } - let(:attributes) do - valid_attributes.merge({ field_16: scheme.old_visible_id, - field_4: "2", - field_5: "2", - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - field_125: 0, - field_44: 4, - field_127: "88" }) - end + context "with old core scheme and location ids" do + context "when a supported housing log already exists in the db" do + let(:attributes) { { bulk_upload:, field_4: "2", field_16: "123" } } - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end + + it "is not a valid row" do + expect(parser).not_to be_valid + end + + it "adds an error to all the fields used to determine duplicates" do + parser.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_16, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].each do |field| + expect(parser.errors[field]).to include(error_message) + end + + expect(parser.errors[:field_23]).not_to include(error_message) + expect(parser.errors[:field_24]).not_to include(error_message) + expect(parser.errors[:field_25]).not_to include(error_message) + end end - it "is not a valid row" do - expect(parser).not_to be_valid + context "when a supported housing log with chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_15: scheme.old_visible_id, + field_4: "2", + field_5: "2", + field_16: location.old_visible_id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end + + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end + + it "is not a valid row" do + expect(parser).not_to be_valid + end + + it "adds an error to all the fields used to determine duplicates" do + parser.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_16, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_127, # chcharge + :field_125, # household_charge + ].each do |field| + expect(parser.errors[field]).to include(error_message) + end + + expect(parser.errors[:field_23]).not_to include(error_message) + expect(parser.errors[:field_24]).not_to include(error_message) + expect(parser.errors[:field_25]).not_to include(error_message) + end end - it "adds an error to all the fields used to determine duplicates" do - parser.valid? + context "when a supported housing log different chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_15: scheme.old_visible_id, + field_4: "2", + field_5: "2", + field_16: location.old_visible_id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end + let(:attributes_too) do + valid_attributes.merge({ field_15: scheme.old_visible_id, + field_4: "2", + field_5: "2", + field_16: location.old_visible_id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "98" }) + end + let(:parser_too) { described_class.new(attributes_too) } - error_message = "This is a duplicate log" + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end - [ - :field_1, # owning_organisation - :field_7, # startdate - :field_8, # startdate - :field_9, # startdate - :field_13, # tenancycode - :field_17, # location - :field_46, # age1 - :field_47, # sex1 - :field_50, # ecstat1 - :field_127, # chcharge - :field_125, # household_charge - ].each do |field| - expect(parser.errors[field]).to include(error_message) + it "is a valid row" do + expect(parser_too).to be_valid end - expect(parser.errors[:field_23]).not_to include(error_message) - expect(parser.errors[:field_24]).not_to include(error_message) - expect(parser.errors[:field_25]).not_to include(error_message) + it "does not add an error to all the fields used to determine duplicates" do + parser_too.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_16, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].each do |field| + expect(parser_too.errors[field]).not_to include(error_message) + end + end end end - context "when a supported housing log different chcharges already exists in the db" do - let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } - let(:attributes) do - valid_attributes.merge({ field_16: scheme.old_visible_id, - field_4: "2", - field_5: "2", - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - field_125: 0, - field_44: 4, - field_127: "88" }) - end - let(:attributes_too) do - valid_attributes.merge({ field_16: scheme.old_visible_id, - field_4: "2", - field_5: "2", - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - field_125: 0, - field_44: 4, - field_127: "98" }) - end - let(:parser_too) { described_class.new(attributes_too) } + context "with new core scheme and location ids" do + context "when a supported housing log already exists in the db" do + let(:attributes) { { bulk_upload:, field_4: "2", field_16: "S123" } } - before do - parser.log.save! - parser.instance_variable_set(:@valid, nil) + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end + + it "is not a valid row" do + expect(parser).not_to be_valid + end + + it "adds an error to all the fields used to determine duplicates" do + parser.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_17, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].each do |field| + expect(parser.errors[field]).to include(error_message) + end + + expect(parser.errors[:field_23]).not_to include(error_message) + expect(parser.errors[:field_24]).not_to include(error_message) + expect(parser.errors[:field_25]).not_to include(error_message) + end end - it "is a valid row" do - expect(parser_too).to be_valid + context "when a supported housing log with chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_16: "S#{scheme.id}", + field_4: "2", + field_5: "2", + field_17: location.id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end + + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end + + it "is not a valid row" do + expect(parser).not_to be_valid + end + + it "adds an error to all the fields used to determine duplicates" do + parser.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_17, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_127, # chcharge + :field_125, # household_charge + ].each do |field| + expect(parser.errors[field]).to include(error_message) + end + + expect(parser.errors[:field_23]).not_to include(error_message) + expect(parser.errors[:field_24]).not_to include(error_message) + expect(parser.errors[:field_25]).not_to include(error_message) + end end - it "does not add an error to all the fields used to determine duplicates" do - parser_too.valid? + context "when a supported housing log different chcharges already exists in the db" do + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, needstype: 2) } + let(:attributes) do + valid_attributes.merge({ field_16: "S#{scheme.id}", + field_4: "2", + field_5: "2", + field_17: location.id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "88" }) + end + let(:attributes_too) do + valid_attributes.merge({ field_16: "S#{scheme.id}", + field_4: "2", + field_5: "2", + field_17: location.id, + field_1: owning_org.old_visible_id, + field_125: 0, + field_44: 4, + field_127: "98" }) + end + let(:parser_too) { described_class.new(attributes_too) } - error_message = "This is a duplicate log" + before do + parser.log.save! + parser.instance_variable_set(:@valid, nil) + end - [ - :field_1, # owning_organisation - :field_7, # startdate - :field_8, # startdate - :field_9, # startdate - :field_13, # tenancycode - :field_17, # location - :field_46, # age1 - :field_47, # sex1 - :field_50, # ecstat1 - :field_132, # tcharge - ].each do |field| - expect(parser_too.errors[field]).not_to include(error_message) + it "is a valid row" do + expect(parser_too).to be_valid + end + + it "does not add an error to all the fields used to determine duplicates" do + parser_too.valid? + + error_message = "This is a duplicate log" + + [ + :field_1, # owning_organisation + :field_7, # startdate + :field_8, # startdate + :field_9, # startdate + :field_13, # tenancycode + :field_17, # location + :field_46, # age1 + :field_47, # sex1 + :field_50, # ecstat1 + :field_132, # tcharge + ].each do |field| + expect(parser_too.errors[field]).not_to include(error_message) + end end end end @@ -652,108 +836,206 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end end - describe "#field_16" do + describe "#field_15, field_16, field_17" do # scheme and location fields context "when nullable not permitted" do - let(:attributes) { { bulk_upload:, field_5: "2", field_16: nil } } + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: nil, field_16: nil, field_17: nil } } it "cannot be nulled" do - expect(parser.errors[:field_16]).to be_present + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to eq(["You must answer scheme code"]) + expect(parser.errors[:field_17]).to be_blank end end context "when nullable permitted" do - let(:attributes) { { bulk_upload:, field_5: "1", field_16: nil } } + let(:attributes) { { bulk_upload:, field_4: "1", field_5: "1", field_15: nil, field_16: nil, field_17: nil } } it "can be nulled" do expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank end end - context "when matching scheme cannot be found" do - let(:attributes) { { bulk_upload:, field_5: "1", field_16: "123" } } + context "when using New CORE ids" do + let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } + let(:location) { create(:location, :with_old_visible_id, scheme:) } + + context "when matching scheme cannot be found" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S123", field_17: location.id } } - it "returns a setup error" do - expect(parser.errors.where(:field_16, category: :setup)).to be_present + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["The scheme code is not correct"]) + expect(parser.errors[:field_17]).to be_blank + end end - end - context "when scheme belongs to someone else" do - let(:other_scheme) { create(:scheme, :with_old_visible_id) } - let(:attributes) { { bulk_upload:, field_5: "1", field_16: other_scheme.old_visible_id, field_1: owning_org.old_visible_id } } + context "when missing location" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: nil } } - it "returns a setup error" do - expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to include("This management group code does not belong to your organisation, or any of your stock owners / managing agents") + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["You must answer location code"]) + end end - end - context "when scheme belongs to owning org" do - let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } - let(:attributes) { { bulk_upload:, field_5: "1", field_16: scheme.old_visible_id, field_1: owning_org.old_visible_id } } + context "when matching location cannot be found" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: "123" } } - it "does not return an error" do - expect(parser.errors[:field_16]).to be_blank + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["Location could not be found with the provided scheme code"]) + end end - end - context "when scheme belongs to managing org" do - let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) } - let(:attributes) { { bulk_upload:, field_5: "1", field_16: scheme.old_visible_id, field_2: managing_org.old_visible_id } } + context "when matching location exists" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: location.id } } - it "does not return an error" do - expect(parser.errors[:field_16]).to be_blank + it "does not return an error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end end - end - end - describe "#field_17" do - context "when location does not exist" do - let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } - let(:attributes) do - { - bulk_upload:, - field_5: "1", - field_16: scheme.old_visible_id, - field_17: "dontexist", - field_1: owning_org.old_visible_id, - } + context "when location exists but not related" do + let(:other_scheme) { create(:scheme, :with_old_visible_id) } + let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) } + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: other_location.id } } + + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors.where(:field_17, category: :setup).map(&:message)).to eq(["Location could not be found with the provided scheme code"]) + end end - it "returns a setup error" do - expect(parser.errors.where(:field_17, category: :setup)).to be_present + context "when scheme belongs to someone else" do + let(:other_scheme) { create(:scheme, :with_old_visible_id) } + let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) } + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_16: "S#{other_scheme.id}", field_17: other_location.id, field_1: owning_org.old_visible_id } } + + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["This scheme code does not belong to your organisation, or any of your stock owners / managing agents"]) + expect(parser.errors[:field_17]).to be_blank + end + end + + context "when scheme belongs to owning org" do + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_16: "S#{scheme.id}", field_17: location.id, field_1: owning_org.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end + end + + context "when scheme belongs to managing org" do + let(:managing_org_scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) } + let(:managing_org_location) { create(:location, :with_old_visible_id, scheme: managing_org_scheme) } + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_16: "S#{managing_org_scheme.id}", field_17: managing_org_location.id, field_2: managing_org.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end end end - context "when location exists" do + context "when using Old CORE ids" do let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } - let(:attributes) do - { - bulk_upload:, - field_5: "1", - field_16: scheme.old_visible_id, - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - } + let(:location) { create(:location, :with_old_visible_id, scheme:) } + + context "when matching scheme cannot be found" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: "123", field_16: location.old_visible_id } } + + it "returns a setup error" do + expect(parser.errors.where(:field_15, category: :setup).map(&:message)).to eq(["The management group code is not correct"]) + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end end - it "does not return an error" do - expect(parser.errors[:field_17]).to be_blank + context "when missing location" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: nil } } + + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["You must answer scheme code"]) + expect(parser.errors[:field_17]).to be_blank + end end - end - context "when location exists but not related" do - let(:location) { create(:scheme, :with_old_visible_id) } - let(:attributes) do - { - bulk_upload:, - field_5: "1", - field_16: scheme.old_visible_id, - field_17: location.old_visible_id, - field_1: owning_org.old_visible_id, - } + context "when matching location cannot be found" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: "123" } } + + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme could not be found with the provided management group code"]) + expect(parser.errors[:field_17]).to be_blank + end end - it "returns a setup error" do - expect(parser.errors.where(:field_17, category: :setup)).to be_present + context "when matching location exists" do + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end + end + + context "when location exists but not related" do + let(:other_scheme) { create(:scheme, :with_old_visible_id) } + let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) } + let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: other_location.old_visible_id } } + + it "returns a setup error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors.where(:field_16, category: :setup).map(&:message)).to eq(["Scheme could not be found with the provided management group code"]) + expect(parser.errors[:field_17]).to be_blank + end + end + + context "when scheme belongs to someone else" do + let(:other_scheme) { create(:scheme, :with_old_visible_id) } + let(:other_location) { create(:location, :with_old_visible_id, scheme: other_scheme) } + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: other_scheme.old_visible_id, field_16: other_location.old_visible_id, field_1: owning_org.old_visible_id } } + + it "returns a setup error" do + expect(parser.errors.where(:field_15, category: :setup).map(&:message)).to eq(["This management group code does not belong to your organisation, or any of your stock owners / managing agents"]) + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end + end + + context "when scheme belongs to owning org" do + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end + end + + context "when scheme belongs to managing org" do + let(:managing_org_scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) } + let(:managing_org_location) { create(:location, :with_old_visible_id, scheme: managing_org_scheme) } + let(:attributes) { { bulk_upload:, field_4: "2", field_5: "2", field_15: managing_org_scheme.old_visible_id, field_16: managing_org_location.old_visible_id, field_2: managing_org.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_15]).to be_blank + expect(parser.errors[:field_16]).to be_blank + expect(parser.errors[:field_17]).to be_blank + end end end end @@ -1410,7 +1692,15 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do describe "#location" do context "when lookup is via new core id" do - let(:attributes) { { bulk_upload:, field_16: scheme.old_visible_id, field_17: location.id, field_1: owning_org } } + let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_17: location.id, field_1: owning_org } } + + it "assigns the correct location" do + expect(parser.log.location).to eql(location) + end + end + + context "when lookup is via old core id" do + let(:attributes) { { bulk_upload:, field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org } } it "assigns the correct location" do expect(parser.log.location).to eql(location) @@ -1419,13 +1709,21 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do end describe "#scheme" do - context "when lookup is via id prefixed with S" do + context "when lookup is via new core id" do let(:attributes) { { bulk_upload:, field_16: "S#{scheme.id}", field_1: owning_org } } it "assigns the correct scheme" do expect(parser.log.scheme).to eql(scheme) end end + + context "when lookup is via old core id" do + let(:attributes) { { bulk_upload:, field_15: scheme.old_visible_id, field_16: location.old_visible_id, field_1: owning_org } } + + it "assigns the correct scheme" do + expect(parser.log.scheme).to eql(scheme) + end + end end describe "#owning_organisation" do From 445883e796585aec4dcb379ebf7571ac818d62c9 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:38:52 +0100 Subject: [PATCH 09/12] Update schemes banner text (#1968) --- app/controllers/schemes_controller.rb | 2 +- spec/features/schemes_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 92669bcbe..3b1c5dc8a 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -141,7 +141,7 @@ class SchemesController < ApplicationController flash[:notice] = if scheme_previously_confirmed "#{@scheme.service_name} has been updated." else - "#{@scheme.service_name} has been created." + "#{@scheme.service_name} has been created. It does not require helpdesk approval." end redirect_to scheme_path(@scheme) elsif check_answers diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 12bd76697..569a7af03 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -657,7 +657,7 @@ RSpec.describe "Schemes scheme Features" do end it "adds scheme to the list of schemes" do - expect(page).to have_content "#{scheme.service_name} has been created." + expect(page).to have_content "#{scheme.service_name} has been created. It does not require helpdesk approval." click_link "Schemes" expect(page).to have_content "Supported housing schemes" expect(page).to have_content scheme.id_to_display From 15cd5e09ea0333aafbe4cabc0b0ac44f83a8122f Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 12 Oct 2023 15:50:00 +0100 Subject: [PATCH 10/12] CLDC-2796 Display managed logs for support (#1966) * Display managed logs * Update tests --- app/controllers/organisations_controller.rb | 2 +- .../requests/organisations_controller_spec.rb | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index cce0c60fd..a45b4a66f 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -94,7 +94,7 @@ class OrganisationsController < ApplicationController end def lettings_logs - organisation_logs = LettingsLog.visible.where(owning_organisation_id: @organisation.id) + organisation_logs = LettingsLog.visible.filter_by_organisation(@organisation) unpaginated_filtered_logs = filter_manager.filtered_logs(organisation_logs, search_term, session_filters) @search_term = search_term diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index bacb5ca33..91eb591e7 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -716,11 +716,20 @@ RSpec.describe OrganisationsController, type: :request do end context "when viewing a specific organisation's lettings logs" do - let(:number_of_org1_lettings_logs) { 2 } + let(:parent_organisation) { create(:organisation) } + let(:child_organisation) { create(:organisation) } + let(:number_of_owned_org1_lettings_logs) { 2 } + let(:number_of_managed_org1_lettings_logs) { 2 } + let(:number_of_owned_and_managed_org1_lettings_logs) { 2 } + let(:total_number_of_org1_logs) { number_of_owned_org1_lettings_logs + number_of_managed_org1_lettings_logs + number_of_owned_and_managed_org1_lettings_logs } let(:number_of_org2_lettings_logs) { 4 } before do - create_list(:lettings_log, number_of_org1_lettings_logs, created_by: user) + create(:organisation_relationship, child_organisation: organisation, parent_organisation:) + create(:organisation_relationship, child_organisation:, parent_organisation: organisation) + create_list(:lettings_log, number_of_owned_org1_lettings_logs, created_by: user, owning_organisation: organisation, managing_organisation: child_organisation) + create_list(:lettings_log, number_of_managed_org1_lettings_logs, created_by: user, owning_organisation: parent_organisation, managing_organisation: organisation) + create_list(:lettings_log, number_of_owned_and_managed_org1_lettings_logs, created_by: user, owning_organisation: organisation, managing_organisation: organisation) create(:lettings_log, created_by: user, status: "pending", skip_update_status: true) create_list(:lettings_log, number_of_org2_lettings_logs, created_by: nil, owning_organisation_id: unauthorised_organisation.id, managing_organisation_id: unauthorised_organisation.id) @@ -728,12 +737,16 @@ RSpec.describe OrganisationsController, type: :request do end it "only shows logs for that organisation" do - expect(page).to have_content("#{number_of_org1_lettings_logs} total logs") + expect(page).to have_content("#{total_number_of_org1_logs} total logs") organisation.lettings_logs.visible.map(&:id).each do |lettings_log_id| expect(page).to have_link lettings_log_id.to_s, href: "/lettings-logs/#{lettings_log_id}" end + organisation.managed_lettings_logs.visible.map(&:id).each do |lettings_log_id| + expect(page).to have_link lettings_log_id.to_s, href: "/lettings-logs/#{lettings_log_id}" + end + unauthorised_organisation.lettings_logs.map(&:id).each do |lettings_log_id| expect(page).not_to have_link lettings_log_id.to_s, href: "/lettings-logs/#{lettings_log_id}" end From aa65651828477946d1127c5be2d930dc40b71cee Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:20:34 +0100 Subject: [PATCH 11/12] feat: add schemes to navbar when org has stock owners and code refactoring (#1980) --- app/helpers/navigation_items_helper.rb | 31 +++---- spec/helpers/navigation_items_helper_spec.rb | 90 ++++++++++++++++++++ 2 files changed, 100 insertions(+), 21 deletions(-) diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb index 8f8c2d295..adf109f15 100644 --- a/app/helpers/navigation_items_helper.rb +++ b/app/helpers/navigation_items_helper.rb @@ -14,7 +14,7 @@ module NavigationItemsHelper [ NavigationItem.new("Lettings logs", lettings_logs_path, lettings_logs_current?(path)), NavigationItem.new("Sales logs", sales_logs_path, sales_logs_current?(path)), - (NavigationItem.new("Schemes", "/schemes", subnav_supported_housing_schemes_path?(path)) if current_user.organisation.holds_own_stock?), + (NavigationItem.new("Schemes", "/schemes", supported_housing_schemes_current?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), 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)), NavigationItem.new("Stock owners", stock_owners_organisation_path(current_user.organisation), stock_owners_path?(path)), @@ -24,26 +24,15 @@ module NavigationItemsHelper end def secondary_items(path, current_organisation_id) - if current_user.organisation.holds_own_stock? - [ - NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)), - NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", subnav_sales_logs_path?(path)), - 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)), - NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)), - NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)), - ].compact - else - [ - NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)), - NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", sales_logs_current?(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)), - NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)), - NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)), - ].compact - end + [ + NavigationItem.new("Lettings logs", "/organisations/#{current_organisation_id}/lettings-logs", subnav_lettings_logs_path?(path)), + NavigationItem.new("Sales logs", "/organisations/#{current_organisation_id}/sales-logs", subnav_sales_logs_path?(path)), + (NavigationItem.new("Schemes", "/organisations/#{current_organisation_id}/schemes", subnav_supported_housing_schemes_path?(path)) if current_user.organisation.holds_own_stock? || current_user.organisation.stock_owners.present?), + 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)), + NavigationItem.new("Stock owners", stock_owners_organisation_path(current_organisation_id), stock_owners_path?(path)), + NavigationItem.new("Managing agents", managing_agents_organisation_path(current_organisation_id), managing_agents_path?(path)), + ].compact end def scheme_items(path, current_scheme_id, title) diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb index 1d753c31e..5730d52ae 100644 --- a/spec/helpers/navigation_items_helper_spec.rb +++ b/spec/helpers/navigation_items_helper_spec.rb @@ -8,6 +8,51 @@ RSpec.describe NavigationItemsHelper do describe "#primary_items" do context "when the user is a data coordinator" do + context "when the user's org does not own stock" do + before do + current_user.organisation.update!(holds_own_stock: false) + end + + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), + NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", 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 + + context "when the user's org has a stock owner" do + before do + current_user.organisation.update!(holds_own_stock: false) + create(:organisation_relationship, child_organisation: current_user.organisation, parent_organisation: stock_owner) + end + + let(:stock_owner) { create(:organisation) } + 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), + NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), + NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", 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 + end + context "when the user is on the lettings logs page" do let(:expected_navigation_items) do [ @@ -141,6 +186,51 @@ RSpec.describe NavigationItemsHelper do it "includes schemes" do expect(primary_items("/", current_user)).to include(NavigationItemsHelper::NavigationItem.new("Schemes", "/schemes", false)) end + + context "when the user's org does not own stock" do + before do + current_user.organisation.update!(holds_own_stock: false) + end + + let(:expected_navigation_items) do + [ + NavigationItemsHelper::NavigationItem.new("Lettings logs", "/lettings-logs", true), + NavigationItemsHelper::NavigationItem.new("Sales logs", "/sales-logs", false), + NavigationItemsHelper::NavigationItem.new("Users", users_path, false), + NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false), + NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), + NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", 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 + + context "when the user's org has a stock owner" do + before do + current_user.organisation.update!(holds_own_stock: false) + create(:organisation_relationship, child_organisation: current_user.organisation, parent_organisation: stock_owner) + end + + let(:stock_owner) { create(:organisation) } + 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), + NavigationItemsHelper::NavigationItem.new("Stock owners", "/organisations/#{current_user.organisation.id}/stock-owners", false), + NavigationItemsHelper::NavigationItem.new("Managing agents", "/organisations/#{current_user.organisation.id}/managing-agents", 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 + end end context "when the user is a support user" do From 2f3f364a571aeed3291ade7afa5172a9bad33394 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:20:49 +0100 Subject: [PATCH 12/12] CLDC-2824 Update bulk upload header guidance (#1975) * feat: update copy in 23/24 lettings and sales prepare file pages * feat: add full stops in alignment with designs --- .../forms/prepare_your_file_2022.html.erb | 10 +++++----- .../forms/prepare_your_file_2023.html.erb | 11 ++++++----- .../forms/prepare_your_file_2022.html.erb | 10 +++++----- .../forms/prepare_your_file_2023.html.erb | 11 ++++++----- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2022.html.erb b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2022.html.erb index 632f320a2..60fc158a2 100644 --- a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2022.html.erb +++ b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2022.html.erb @@ -21,9 +21,9 @@

Create your file

<%= govuk_inset_text(text: "Upload separate files for general needs and supported housing logs for 2022/23 data.") %> @@ -31,8 +31,8 @@

Save your file

<%= f.govuk_submit class: "govuk-!-margin-top-7" %> diff --git a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb index f15972491..0dbd25c46 100644 --- a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb +++ b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2023.html.erb @@ -26,9 +26,10 @@

Create your file

<%= govuk_inset_text(text: "You can upload both general needs and supported housing logs in the same file for 2023/24 data.") %> @@ -36,8 +37,8 @@

Save your file

<%= f.govuk_submit class: "govuk-!-margin-top-7" %> diff --git a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2022.html.erb b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2022.html.erb index 624b3fe8b..fc1c1c31c 100644 --- a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2022.html.erb +++ b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2022.html.erb @@ -17,15 +17,15 @@

Create your file

Save your file

<%= f.govuk_submit %> diff --git a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb index 7fc522e88..0d2d8831c 100644 --- a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb +++ b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2023.html.erb @@ -18,16 +18,17 @@

Create your file

Save your file

<%= f.govuk_submit %>