From f9feaf4a577292d0dcec94b3ffe080c34ab456b3 Mon Sep 17 00:00:00 2001
From: carolynbarker <8038496+carolynbarker@users.noreply.github.com>
Date: Tue, 25 Mar 2025 18:24:32 +0000
Subject: [PATCH 1/2] CLDC-3938 Log invalid BU row numbers (#3018)
* log row numbers of any logs still invalid after blanking non setup fields
* include first invalid row in sentry message
---------
Co-authored-by: Carolyn <carolyn.barker@softwire.com>
---
app/services/bulk_upload/lettings/validator.rb | 8 +++++---
app/services/bulk_upload/sales/validator.rb | 9 +++++----
2 files changed, 10 insertions(+), 7 deletions(-)
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
From f3c253221624538dc0d0c2ac4458124d84c58c8a Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Wed, 26 Mar 2025 09:53:45 +0000
Subject: [PATCH 2/2] Remove update manual address entry task (#3016)
* Delete task
* Delete test
---
...ddress_entry_selected_prexisting_logs.rake | 207 -----------------
...ess_entry_selected_prexisting_logs_spec.rb | 211 ------------------
2 files changed, 418 deletions(-)
delete mode 100644 lib/tasks/update_manual_address_entry_selected_prexisting_logs.rake
delete mode 100644 spec/lib/tasks/update_manual_address_entry_selected_prexisting_logs_spec.rb
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