From 09b3cb2166ed99b81d6cc0b0b9e5934e5c677de7 Mon Sep 17 00:00:00 2001 From: Kat Date: Mon, 15 Jul 2024 09:22:17 +0100 Subject: [PATCH] Track updated bulk uploads --- app/models/bulk_upload.rb | 2 +- app/services/bulk_upload/sales/log_creator.rb | 2 +- .../20240715072421_add_rent_fix_status.rb | 5 ++ db/schema.rb | 3 +- lib/tasks/correct_noint_value.rake | 24 ----- lib/tasks/correct_rent_type_value.rake | 3 +- spec/factories/bulk_upload.rb | 2 +- spec/lib/tasks/correct_noint_value_spec.rb | 87 ------------------- .../lib/tasks/correct_rent_type_value_spec.rb | 32 ++++++- 9 files changed, 40 insertions(+), 120 deletions(-) create mode 100644 db/migrate/20240715072421_add_rent_fix_status.rb delete mode 100644 lib/tasks/correct_noint_value.rake delete mode 100644 spec/lib/tasks/correct_noint_value_spec.rb diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index a1b656f49..7af43ee28 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -1,6 +1,6 @@ class BulkUpload < ApplicationRecord enum log_type: { lettings: "lettings", sales: "sales" } - enum noint_fix_status: { not_applied: "not_applied", applied: "applied", not_needed: "not_needed" } + enum rent_type_fix_status: { not_applied: "not_applied", applied: "applied", not_needed: "not_needed" } belongs_to :user diff --git a/app/services/bulk_upload/sales/log_creator.rb b/app/services/bulk_upload/sales/log_creator.rb index 9f3a00fc9..aeb3f66c4 100644 --- a/app/services/bulk_upload/sales/log_creator.rb +++ b/app/services/bulk_upload/sales/log_creator.rb @@ -7,7 +7,7 @@ class BulkUpload::Sales::LogCreator end def call - @bulk_upload.update!(noint_fix_status: BulkUpload.noint_fix_statuses[:not_needed]) + @bulk_upload.update!(rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_needed]) row_parsers.each do |row_parser| row_parser.valid? diff --git a/db/migrate/20240715072421_add_rent_fix_status.rb b/db/migrate/20240715072421_add_rent_fix_status.rb new file mode 100644 index 000000000..18733499c --- /dev/null +++ b/db/migrate/20240715072421_add_rent_fix_status.rb @@ -0,0 +1,5 @@ +class AddRentFixStatus < ActiveRecord::Migration[7.0] + def change + add_column :bulk_uploads, :rent_type_fix_status, :string, default: "not_applied" + end +end diff --git a/db/schema.rb b/db/schema.rb index 66693435f..8aeca4b3d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_06_10_142812) do +ActiveRecord::Schema[7.0].define(version: 2024_07_15_072421) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -42,6 +42,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_06_10_142812) do t.text "choice" t.integer "total_logs_count" t.string "noint_fix_status", default: "not_applied" + t.string "rent_type_fix_status", default: "not_applied" t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true t.index ["user_id"], name: "index_bulk_uploads_on_user_id" end diff --git a/lib/tasks/correct_noint_value.rake b/lib/tasks/correct_noint_value.rake deleted file mode 100644 index 354daa8c1..000000000 --- a/lib/tasks/correct_noint_value.rake +++ /dev/null @@ -1,24 +0,0 @@ -desc "Alter noint values for bulk uploaded sales logs where these have not been set in the service" -task correct_noint_value: :environment do - update_counts = { - in_progress: 0, - completed: 0, - pending: 0, - deleted: 0, - } - affected_uploads = BulkUpload.where(log_type: "sales", noint_fix_status: BulkUpload.noint_fix_statuses[:not_applied]) - affected_uploads.each do |upload| - upload.logs.where(noint: 2).each do |log| - noint_at_upload = log.versions.length == 1 ? log.noint : log.versions.first.next.reify.noint - next unless noint_at_upload == 2 - - Rails.logger.info("Updating noint value on log #{log.id}, owning org #{log.owning_organisation_id}") - update_counts[log.status.to_sym] += 1 - log.noint = 1 - log.skip_update_status = true - log.save! - end - upload.update!(noint_fix_status: BulkUpload.noint_fix_statuses[:applied]) - end - Rails.logger.info("Logs updated; #{update_counts}") -end diff --git a/lib/tasks/correct_rent_type_value.rake b/lib/tasks/correct_rent_type_value.rake index 8ecf01c1f..1d43383c7 100644 --- a/lib/tasks/correct_rent_type_value.rake +++ b/lib/tasks/correct_rent_type_value.rake @@ -1,6 +1,6 @@ desc "Alter rent_type values for bulk uploaded lettings logs for 2024 where they were not mapped correctly" task correct_rent_type_value: :environment do - affected_uploads = BulkUpload.where(log_type: "lettings", year: 2024) + affected_uploads = BulkUpload.where(log_type: "lettings", year: 2024, rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_applied]) affected_uploads.each do |upload| upload.logs.where.not(rent_type: nil).each do |log| current_rent_type = log.rent_type @@ -16,5 +16,6 @@ task correct_rent_type_value: :environment do Rails.logger.error("Log #{log.id} rent_type could not be updated from #{rent_type_at_upload} to #{log.rent_type}. Error: #{log.errors.full_messages.join(', ')}") end end + upload.update!(rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:applied]) end end diff --git a/spec/factories/bulk_upload.rb b/spec/factories/bulk_upload.rb index f7d81ab7a..c848fb071 100644 --- a/spec/factories/bulk_upload.rb +++ b/spec/factories/bulk_upload.rb @@ -8,7 +8,7 @@ FactoryBot.define do identifier { SecureRandom.uuid } sequence(:filename) { |n| "bulk-upload-#{n}.csv" } needstype { 1 } - noint_fix_status { BulkUpload.noint_fix_statuses.values.sample } + rent_type_fix_status { BulkUpload.rent_type_fix_statuses.values.sample } trait(:sales) do log_type { BulkUpload.log_types[:sales] } diff --git a/spec/lib/tasks/correct_noint_value_spec.rb b/spec/lib/tasks/correct_noint_value_spec.rb deleted file mode 100644 index 51dd7779c..000000000 --- a/spec/lib/tasks/correct_noint_value_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -require "rails_helper" -require "rake" - -RSpec.describe "correct_noint_value" do - describe ":correct_noint_value", type: :task do - subject(:task) { Rake::Task["correct_noint_value"] } - - before do - Rake.application.rake_require("tasks/correct_noint_value") - Rake::Task.define_task(:environment) - task.reenable - end - - context "when the rake task is run" do - context "and there is a sales bulk upload with the fix needed" do - let(:bulk_upload) { create(:bulk_upload, :sales, noint_fix_status: BulkUpload.noint_fix_statuses[:not_applied]) } - - before do - bulk_upload.save! - end - - it "updates the noint value on a log with noint = 2 where it was set to 2 on create" do - log = create(:sales_log, :completed, noint: 2, bulk_upload:) - - task.invoke - log.reload - - expect(log.noint).to be(1) - end - - it "updates the noint value on a log with noint = 2 where it was set to 2 on create and other fields have since changed" do - log = create(:sales_log, :in_progress, noint: 2, bulk_upload:) - log.update!(status: Log.statuses[:completed]) - - task.invoke - log.reload - - expect(log.noint).to be(1) - end - - it "does not update the noint value on a log that has noint = 1" do - log = create(:sales_log, :completed, noint: 2, bulk_upload:) - log.update!(noint: 1) - - task.invoke - log.reload - - expect(log.noint).to be(1) - end - - it "does not update the noint value on a log with noint = 2 where noint was nil on create" do - log = create(:sales_log, :completed, noint: nil, bulk_upload:) - log.update!(noint: 2) - - task.invoke - log.reload - - expect(log.noint).to be(2) - end - - it "updates the noint_fix_status value on the bulk upload" do - task.invoke - bulk_upload.reload - - expect(bulk_upload.noint_fix_status).to eq(BulkUpload.noint_fix_statuses[:applied]) - end - end - - context "and there is a sales bulk upload with the fix marked as not needed" do - let(:bulk_upload) { create(:bulk_upload, :sales, noint_fix_status: BulkUpload.noint_fix_statuses[:not_needed]) } - - before do - bulk_upload.save! - end - - it "does not update the noint values on logs" do - log = create(:sales_log, :completed, noint: 2, bulk_upload:) - - task.invoke - log.reload - - expect(log.noint).to be(2) - end - end - end - end -end diff --git a/spec/lib/tasks/correct_rent_type_value_spec.rb b/spec/lib/tasks/correct_rent_type_value_spec.rb index 593327baf..a217c516a 100644 --- a/spec/lib/tasks/correct_rent_type_value_spec.rb +++ b/spec/lib/tasks/correct_rent_type_value_spec.rb @@ -13,8 +13,8 @@ RSpec.describe "correct_rent_type_value" do context "when the rake task is run" do context "and rent_type is 1" do - let(:bulk_upload) { create(:bulk_upload, :lettings, year: 2024) } - let(:bulk_upload_2023) { create(:bulk_upload, :lettings, year: 2023) } + let(:bulk_upload) { create(:bulk_upload, :lettings, year: 2024, rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_applied]) } + let(:bulk_upload_2023) { create(:bulk_upload, :lettings, year: 2023, rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_applied]) } before do bulk_upload.save! @@ -30,6 +30,7 @@ RSpec.describe "correct_rent_type_value" do expect(log.rent_type).to be(0) expect(log.status).to eq("completed") expect(log.updated_at).not_to eq(initial_updated_at) + expect(bulk_upload.reload.rent_type_fix_status).to eq(BulkUpload.rent_type_fix_statuses[:applied]) end it "updates the rent_type value on a pending log where it was set to 1 on create" do @@ -45,6 +46,7 @@ RSpec.describe "correct_rent_type_value" do expect(log.rent_type).to be(0) expect(log.status).to eq("pending") expect(log.updated_at).not_to eq(initial_updated_at) + expect(bulk_upload.reload.rent_type_fix_status).to eq(BulkUpload.rent_type_fix_statuses[:applied]) end it "updates the rent_type value on a deleted log where it was set to 1 on create" do @@ -58,6 +60,7 @@ RSpec.describe "correct_rent_type_value" do expect(log.rent_type).to be(0) expect(log.status).to eq("deleted") expect(log.updated_at).not_to eq(initial_updated_at) + expect(bulk_upload.reload.rent_type_fix_status).to eq(BulkUpload.rent_type_fix_statuses[:applied]) end it "updates the rent_type value on a log where it was set to 1 on create and other fields have since changed" do @@ -71,6 +74,7 @@ RSpec.describe "correct_rent_type_value" do expect(log.rent_type).to be(0) expect(log.status).to eq("completed") expect(log.updated_at).not_to eq(initial_updated_at) + expect(bulk_upload.reload.rent_type_fix_status).to eq(BulkUpload.rent_type_fix_statuses[:applied]) end it "does not update the rent_type value on a log if it has since been changed" do @@ -119,11 +123,31 @@ RSpec.describe "correct_rent_type_value" do expect(log.rent_type).to be(1) expect(log.updated_at).to eq(initial_updated_at) end + + context "when the rent_type_fix_status is not_needed" do + let(:bulk_upload) { create(:bulk_upload, :lettings, year: 2024, rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_needed]) } + + before do + bulk_upload.save! + end + + it "does not update the rent_type values on logs" do + log = create(:lettings_log, :completed, rent_type: 1, bulk_upload:) + initial_updated_at = log.updated_at + + task.invoke + log.reload + + expect(log.rent_type).to be(1) + expect(log.updated_at).to eq(initial_updated_at) + expect(bulk_upload.reload.rent_type_fix_status).to eq(BulkUpload.rent_type_fix_statuses[:not_needed]) + end + end end context "and rent_type is 2" do - let(:bulk_upload) { create(:bulk_upload, :lettings, year: 2024) } - let(:bulk_upload_2023) { create(:bulk_upload, :lettings, year: 2023) } + let(:bulk_upload) { create(:bulk_upload, :lettings, year: 2024, rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_applied]) } + let(:bulk_upload_2023) { create(:bulk_upload, :lettings, year: 2023, rent_type_fix_status: BulkUpload.rent_type_fix_statuses[:not_applied]) } before do bulk_upload.save!