diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 8cb3a1bd9..e1be94eec 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -55,8 +55,10 @@ class BulkUpload::Lettings::Validator row_parsers.each do |row_parser| row_parser.log.blank_invalid_non_setup_fields! end - if any_logs_invalid? + + if invalid_row_numbers.any? Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") + Rails.logger.error("Lettings bulk upload #{bulk_upload.id} blocked due to invalid logs after blanking on rows #{invalid_row_numbers}") "logs_invalid" end end @@ -101,8 +103,8 @@ private end end - def any_logs_invalid? - row_parsers.any? { |row_parser| row_parser.log.invalid? } + def invalid_row_numbers + row_parsers.each_with_index.map { |row_parser, index| index + row_offset + 1 if row_parser.log.invalid? }.compact end def csv_parser diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index 0b2d68bc5..cef816ff0 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -56,8 +56,9 @@ class BulkUpload::Sales::Validator row_parser.log.blank_invalid_non_setup_fields! end - if any_logs_invalid? - Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") + if invalid_row_numbers.any? + Sentry.capture_message("Bulk upload #{bulk_upload.id} had log creation blocked due to invalid logs after blanking non setup fields. First invalid row number: #{invalid_row_numbers.first}.") + Rails.logger.error("Sales bulk upload #{bulk_upload.id} blocked due to invalid logs after blanking on rows #{invalid_row_numbers}") "logs_invalid" end end @@ -98,8 +99,8 @@ private end end - def any_logs_invalid? - row_parsers.any? { |row_parser| row_parser.log.invalid? } + def invalid_row_numbers + row_parsers.each_with_index.map { |row_parser, index| index + row_offset + 1 if row_parser.log.invalid? }.compact end def csv_parser diff --git a/lib/tasks/update_manual_address_entry_selected_prexisting_logs.rake b/lib/tasks/update_manual_address_entry_selected_prexisting_logs.rake deleted file mode 100644 index d11dc3219..000000000 --- a/lib/tasks/update_manual_address_entry_selected_prexisting_logs.rake +++ /dev/null @@ -1,207 +0,0 @@ -namespace :bulk_update do - desc "Update logs with specific criteria and set manual_address_entry_selected to true" - task update_manual_address_entry_selected: :environment do - updated_lettings_logs_count = 0 - lettings_postcode_fixed_count = 0 - lettings_postcode_fixed_status_changed_count = 0 - lettings_postcode_not_fixed_status_changed_count = 0 - lettings_postcode_fixed_status_changed_ids = [] - lettings_postcode_not_fixed_status_changed_ids = [] - lettings_updated_without_issue = 0 - - updated_sales_logs_count = 0 - sales_postcode_fixed_count = 0 - sales_postcode_fixed_status_changed_count = 0 - sales_postcode_not_fixed_status_changed_count = 0 - sales_postcode_fixed_status_changed_ids = [] - sales_postcode_not_fixed_status_changed_ids = [] - sales_updated_without_issue = 0 - - lettings_logs = LettingsLog.filter_by_year(2024) - .where(status: %w[in_progress completed]) - .where(needstype: 1, manual_address_entry_selected: false, uprn: nil) - .where("(address_line1 IS NOT NULL AND address_line1 != '') OR (address_line2 IS NOT NULL AND address_line2 != '') OR (town_or_city IS NOT NULL AND town_or_city != '') OR (county IS NOT NULL AND county != '') OR (postcode_full IS NOT NULL AND postcode_full != '')") - - lettings_logs.find_each do |log| - status_pre_change = log.status - log.manual_address_entry_selected = true - if log.save - updated_lettings_logs_count += 1 - Rails.logger.info "manual_address_entry_selected updated for lettings log #{log.id}" - else - Rails.logger.info "Could not save manual_address_entry_selected changes to lettings log #{log.id} : #{log.errors.full_messages.join(', ')}" - end - - postcode_fixed = false - if log.postcode_full.nil? && log.address_line1 == log.address_line1_input - log.postcode_full = log.postcode_full_input - if log.save - lettings_postcode_fixed_count += 1 - Rails.logger.info "postcode_full updated by address_line1_input for lettings log #{log.id}" - postcode_fixed = true - else - Rails.logger.info "Could not save postcode_full changes to lettings log #{log.id} : #{log.errors.full_messages.join(', ')}" - end - end - - if log.postcode_full.nil? && log.creation_method == "bulk upload" && log.address_line1 == log.address_line1_as_entered - log.postcode_full = log.postcode_full_as_entered - if log.save - lettings_postcode_fixed_count += 1 - Rails.logger.info "postcode_full updated by address_line1_as_entered for lettings log #{log.id}" - postcode_fixed = true - else - Rails.logger.info "Could not save postcode_full changes to lettings log #{log.id} : #{log.errors.full_messages.join(', ')}" - end - end - - status_post_change = log.status - if status_pre_change != status_post_change - if postcode_fixed - lettings_postcode_fixed_status_changed_count += 1 - lettings_postcode_fixed_status_changed_ids << log.id - else - lettings_postcode_not_fixed_status_changed_count += 1 - lettings_postcode_not_fixed_status_changed_ids << log.id - end - else - lettings_updated_without_issue += 1 - end - end - - sales_logs = SalesLog.filter_by_year(2024) - .where(status: %w[in_progress completed]) - .where(manual_address_entry_selected: false, uprn: nil) - .where("(address_line1 IS NOT NULL AND address_line1 != '') OR (address_line2 IS NOT NULL AND address_line2 != '') OR (town_or_city IS NOT NULL AND town_or_city != '') OR (county IS NOT NULL AND county != '') OR (postcode_full IS NOT NULL AND postcode_full != '')") - - sales_logs.find_each do |log| - status_pre_change = log.status - log.manual_address_entry_selected = true - if log.save - updated_sales_logs_count += 1 - Rails.logger.info "manual_address_entry_selected updated for sales log #{log.id}" - else - Rails.logger.info "Could not save manual_address_entry_selected changes to sales log #{log.id} : #{log.errors.full_messages.join(', ')}" - end - - postcode_fixed = false - if log.postcode_full.nil? && log.address_line1 == log.address_line1_input - log.postcode_full = log.postcode_full_input - if log.save - sales_postcode_fixed_count += 1 - Rails.logger.info "postcode_full updated by address_line1_input for sales log #{log.id}" - postcode_fixed = true - else - Rails.logger.info "Could not save postcode_full changes to sales log #{log.id} : #{log.errors.full_messages.join(', ')}" - end - end - - if log.postcode_full.nil? && log.creation_method == "bulk upload" && log.address_line1 == log.address_line1_as_entered - log.postcode_full = log.postcode_full_as_entered - if log.save - sales_postcode_fixed_count += 1 - Rails.logger.info "postcode_full updated by address_line1_as_entered for sales log #{log.id}" - postcode_fixed = true - else - Rails.logger.info "Could not save postcode_full changes to sales log #{log.id} : #{log.errors.full_messages.join(', ')}" - end - end - - status_post_change = log.status - if status_pre_change != status_post_change - if postcode_fixed - sales_postcode_fixed_status_changed_count += 1 - sales_postcode_fixed_status_changed_ids << log.id - else - sales_postcode_not_fixed_status_changed_count += 1 - sales_postcode_not_fixed_status_changed_ids << log.id - end - else - sales_updated_without_issue += 1 - end - end - - puts "#{updated_lettings_logs_count} lettings logs were updated." - puts "#{lettings_updated_without_issue} lettings logs were updated without issue." - puts "#{lettings_postcode_fixed_count} lettings logs where postcode fix was applied." - puts "#{lettings_postcode_fixed_status_changed_count} lettings logs with postcode fix and status changed." - puts "#{lettings_postcode_not_fixed_status_changed_count} lettings logs without postcode fix and status changed." - puts "IDs of lettings logs with postcode fix and status changed: [#{lettings_postcode_fixed_status_changed_ids.join(', ')}]" - puts "IDs of lettings logs without postcode fix and status changed: [#{lettings_postcode_not_fixed_status_changed_ids.join(', ')}]" - - lettings_postcode_fixed_org_counts = LettingsLog.where(id: lettings_postcode_fixed_status_changed_ids).group(:owning_organisation_id).count - lettings_postcode_fixed_org_counts.each do |org_id, count| - puts "Org #{org_id}: #{count} logs with postcode fix and status changed." - end - - lettings_postcode_not_fixed_org_counts = LettingsLog.where(id: lettings_postcode_not_fixed_status_changed_ids).group(:owning_organisation_id).count - lettings_postcode_not_fixed_org_counts.each do |org_id, count| - puts "Org #{org_id}: #{count} logs without postcode fix and status changed." - end - - puts "#{updated_sales_logs_count} sales logs were updated." - puts "#{sales_updated_without_issue} sales logs were updated without issue." - puts "#{sales_postcode_fixed_count} sales logs where postcode fix was applied." - puts "#{sales_postcode_fixed_status_changed_count} sales logs with postcode fix and status changed." - puts "#{sales_postcode_not_fixed_status_changed_count} sales logs without postcode fix and status changed." - puts "IDs of sales logs with postcode fix and status changed: [#{sales_postcode_fixed_status_changed_ids.join(', ')}]" - puts "IDs of sales logs without postcode fix and status changed: [#{sales_postcode_not_fixed_status_changed_ids.join(', ')}]" - - sales_postcode_fixed_org_counts = SalesLog.where(id: sales_postcode_fixed_status_changed_ids).group(:owning_organisation_id).count - sales_postcode_fixed_org_counts.each do |org_id, count| - puts "Org #{org_id}: #{count} logs with postcode fix and status changed." - end - - sales_postcode_not_fixed_org_counts = SalesLog.where(id: sales_postcode_not_fixed_status_changed_ids).group(:owning_organisation_id).count - sales_postcode_not_fixed_org_counts.each do |org_id, count| - puts "Org #{org_id}: #{count} logs without postcode fix and status changed." - end - end - - desc "Find logs to fix and update postcode_full if conditions are met" - task update_postcode_full_preexisting_manual_entry_logs: :environment do - updated_count = 0 - fixed_count = 0 - not_updated_count = 0 - not_updated_ids = [] - updated_but_not_fixed_ids = [] - - logs_to_fix = LettingsLog.filter_by_year(2024).where(manual_address_entry_selected: true, uprn: nil, status: "in_progress", postcode_full: nil, updated_at: Time.zone.parse("2025-03-19 16:00:00")..Time.zone.parse("2025-03-19 17:00:00")) - - logs_to_fix.find_each do |log| - previous_version = log.versions[-2] - previous_status = previous_version&.reify&.status - - if log.address_line1 == log.address_line1_input - log.postcode_full = log.postcode_full_input - elsif log.creation_method == "bulk upload" && log.address_line1 == log.address_line1_as_entered - log.postcode_full = log.postcode_full_as_entered - end - - if log.postcode_full.present? - if log.save - Rails.logger.info "Updated postcode_full for lettings log #{log.id}" - updated_count += 1 - if log.status == previous_status - fixed_count += 1 - else - updated_but_not_fixed_ids << log.id - end - else - Rails.logger.info "Could not save changes to lettings log #{log.id}: #{log.errors.full_messages.join(', ')}" - not_updated_count += 1 - not_updated_ids << log.id - end - else - not_updated_count += 1 - not_updated_ids << log.id - end - end - - puts "#{updated_count} logs updated." - puts "#{fixed_count} logs fixed." - puts "#{not_updated_count} logs not updated." - puts "IDs of logs not updated: [#{not_updated_ids.join(', ')}]" - puts "IDs of logs updated but not fixed: [#{updated_but_not_fixed_ids.join(', ')}]" - end -end diff --git a/spec/lib/tasks/update_manual_address_entry_selected_prexisting_logs_spec.rb b/spec/lib/tasks/update_manual_address_entry_selected_prexisting_logs_spec.rb deleted file mode 100644 index cbcf6e0e8..000000000 --- a/spec/lib/tasks/update_manual_address_entry_selected_prexisting_logs_spec.rb +++ /dev/null @@ -1,211 +0,0 @@ -require "rails_helper" -require "rake" - -RSpec.describe "update_manual_address_entry_selected_preexisting_logs_spec", type: :task do - before do - Rake.application.rake_require("tasks/update_manual_address_entry_selected_prexisting_logs") - Rake::Task.define_task(:environment) - task.reenable - end - - describe "bulk_update:update_manual_address_entry_selected" do - let(:task) { Rake::Task["bulk_update:update_manual_address_entry_selected"] } - - let(:lettings_log_uprn_entered) do - build(:lettings_log, :completed, startdate: Time.zone.local(2024, 6, 1), needstype: 1, manual_address_entry_selected: false) - end - - let(:lettings_log_uprn_found) do - build(:lettings_log, :completed, startdate: Time.zone.local(2024, 9, 1), needstype: 1, manual_address_entry_selected: false, address_line1_input: "1 Test Street", postcode_full_input: "SW1 1AA") - end - - let(:lettings_log_address_fields_not_entered) do - build(:lettings_log, :inprogress_without_address_fields, startdate: Time.zone.local(2024, 9, 1), needstype: 1) - end - - let(:lettings_log_address_manually_entered) do - build(:lettings_log, :completed_without_uprn, startdate: Time.zone.local(2024, 12, 1), needstype: 1) - end - - let(:sales_log_uprn_entered) do - build(:sales_log, :completed, saledate: Time.zone.local(2024, 12, 1), manual_address_entry_selected: false) - end - - let(:sales_log_uprn_found) do - build(:sales_log, :completed, saledate: Time.zone.local(2024, 7, 1), manual_address_entry_selected: false, address_line1_input: "1 Test Street", postcode_full_input: "SW1 1AA") - end - - let(:sales_log_address_fields_not_entered) do - build(:sales_log, :inprogress_without_address_fields, saledate: Time.zone.local(2024, 12, 30)) - end - - let(:sales_log_address_manually_entered) do - build(:sales_log, :completed_without_uprn, saledate: Time.zone.local(2024, 12, 30)) - end - - context "when running the task" do - context "when logs do not meet the criteria" do - before do - lettings_log_uprn_found.save!(validate: false) - lettings_log_uprn_entered.save!(validate: false) - lettings_log_address_fields_not_entered.save!(validate: false) - - sales_log_uprn_found.save!(validate: false) - sales_log_uprn_entered.save!(validate: false) - sales_log_address_fields_not_entered.save!(validate: false) - end - - it "does not update logs with a UPRN entered" do - task.invoke - - lettings_log_uprn_entered.reload - sales_log_uprn_entered.reload - - expect(lettings_log_uprn_entered.manual_address_entry_selected).to be false - expect(lettings_log_uprn_entered.uprn).to eq("10033558653") - expect(sales_log_uprn_entered.manual_address_entry_selected).to be false - expect(sales_log_uprn_entered.uprn).to eq("10033558653") - end - - it "does not update logs with a UPRN found" do - task.invoke - - lettings_log_uprn_found.reload - sales_log_uprn_found.reload - - expect(lettings_log_uprn_found.manual_address_entry_selected).to be false - expect(lettings_log_uprn_found.uprn).to eq("10033558653") - expect(sales_log_uprn_found.manual_address_entry_selected).to be false - expect(sales_log_uprn_found.uprn).to eq("10033558653") - end - - it "does not update logs with no UPRN or address fields entered" do - task.invoke - - lettings_log_address_fields_not_entered.reload - sales_log_address_fields_not_entered.reload - - expect(lettings_log_address_fields_not_entered.manual_address_entry_selected).to be false - expect(sales_log_address_fields_not_entered.manual_address_entry_selected).to be false - end - end - - context "when logs do meet the criteria" do - before do - lettings_log_address_manually_entered.manual_address_entry_selected = false - lettings_log_address_manually_entered.save!(validate: false) - - sales_log_address_manually_entered.manual_address_entry_selected = false - sales_log_address_manually_entered.save!(validate: false) - end - - it "updates logs with an address manually entered" do - expect(lettings_log_address_manually_entered.manual_address_entry_selected).to be false - expect(lettings_log_address_manually_entered.address_line1).to eq("1 Test Street") - expect(lettings_log_address_manually_entered.address_line2).to eq("Testville") - expect(lettings_log_address_manually_entered.town_or_city).to eq("Testford") - expect(lettings_log_address_manually_entered.postcode_full).to eq("SW1 1AA") - - expect(sales_log_address_manually_entered.manual_address_entry_selected).to be false - expect(sales_log_address_manually_entered.address_line1).to eq("1 Test Street") - expect(sales_log_address_manually_entered.address_line2).to eq("Testville") - expect(sales_log_address_manually_entered.town_or_city).to eq("Testford") - expect(sales_log_address_manually_entered.postcode_full).to eq("SW1 1AA") - - task.invoke - - lettings_log_address_manually_entered.reload - sales_log_address_manually_entered.reload - - expect(lettings_log_address_manually_entered.manual_address_entry_selected).to be true - expect(lettings_log_address_manually_entered.address_line1).to eq("1 Test Street") - expect(lettings_log_address_manually_entered.address_line2).to eq("Testville") - expect(lettings_log_address_manually_entered.town_or_city).to eq("Testford") - expect(lettings_log_address_manually_entered.postcode_full).to eq("SW1 1AA") - - expect(sales_log_address_manually_entered.manual_address_entry_selected).to be true - expect(sales_log_address_manually_entered.address_line1).to eq("1 Test Street") - expect(sales_log_address_manually_entered.address_line2).to eq("Testville") - expect(sales_log_address_manually_entered.town_or_city).to eq("Testford") - expect(sales_log_address_manually_entered.postcode_full).to eq("SW1 1AA") - end - end - end - end - - describe "bulk_update:update_postcode_full_preexisting_manual_entry_logs" do - let(:task) { Rake::Task["bulk_update:update_postcode_full_preexisting_manual_entry_logs"] } - - let(:lettings_log_to_fix) do - build(:lettings_log, :inprogress_without_address_fields, startdate: Time.zone.local(2024, 6, 1), updated_at: Time.zone.parse("2025-03-19 16:30:00")) - end - - let(:bu_lettings_log_to_fix) do - build(:lettings_log, :inprogress_without_address_fields, startdate: Time.zone.local(2024, 6, 1), creation_method: "bulk upload", updated_at: Time.zone.parse("2025-03-19 16:30:00")) - end - - let(:lettings_log_not_to_fix) do - build(:lettings_log, :inprogress_without_address_fields, startdate: Time.zone.local(2024, 6, 1), updated_at: Time.zone.parse("2025-03-19 15:30:00")) - end - - before do - lettings_log_to_fix.manual_address_entry_selected = true - lettings_log_to_fix.address_line1 = "1 Test Street" - lettings_log_to_fix.address_line2 = "Testville" - lettings_log_to_fix.town_or_city = "Testford" - lettings_log_to_fix.postcode_full = nil - lettings_log_to_fix.address_line1_input = "1 Test Street" - lettings_log_to_fix.postcode_full_input = "SW1 2BB" - lettings_log_to_fix.save!(validate: false) - - bu_lettings_log_to_fix.manual_address_entry_selected = true - bu_lettings_log_to_fix.address_line1 = "1 Test Street" - bu_lettings_log_to_fix.address_line2 = "Testville" - bu_lettings_log_to_fix.town_or_city = "Testford" - bu_lettings_log_to_fix.postcode_full = nil - bu_lettings_log_to_fix.address_line1_as_entered = "1 Test Street" - bu_lettings_log_to_fix.postcode_full_as_entered = "SW1 2BB" - bu_lettings_log_to_fix.save!(validate: false) - - lettings_log_not_to_fix.postcode_full = nil - lettings_log_not_to_fix.save!(validate: false) - end - - context "when running the task" do - it "updates logs that meet the criteria" do - expect(lettings_log_to_fix.postcode_full).to be_nil - expect(lettings_log_to_fix.address_line1).to eq("1 Test Street") - expect(lettings_log_to_fix.address_line2).to eq("Testville") - expect(lettings_log_to_fix.town_or_city).to eq("Testford") - expect(lettings_log_to_fix.address_line1_input).to eq("1 Test Street") - expect(lettings_log_to_fix.postcode_full_input).to eq("SW1 2BB") - - expect(bu_lettings_log_to_fix.postcode_full).to be_nil - expect(bu_lettings_log_to_fix.address_line1_input).to be_nil - expect(bu_lettings_log_to_fix.address_line1).to eq("1 Test Street") - expect(bu_lettings_log_to_fix.address_line2).to eq("Testville") - expect(bu_lettings_log_to_fix.town_or_city).to eq("Testford") - expect(bu_lettings_log_to_fix.address_line1_as_entered).to eq("1 Test Street") - expect(bu_lettings_log_to_fix.postcode_full_as_entered).to eq("SW1 2BB") - - task.invoke - - lettings_log_to_fix.reload - bu_lettings_log_to_fix.reload - - expect(lettings_log_to_fix.postcode_full).to eq(lettings_log_to_fix.postcode_full_input) - expect(lettings_log_to_fix.postcode_full).to eq("SW1 2BB") - expect(bu_lettings_log_to_fix.postcode_full).to eq(bu_lettings_log_to_fix.postcode_full_as_entered) - expect(bu_lettings_log_to_fix.postcode_full).to eq("SW1 2BB") - end - - it "does not update logs that do not meet the criteria" do - task.invoke - - lettings_log_not_to_fix.reload - - expect(lettings_log_not_to_fix.postcode_full).to be_nil - end - end - end -end