From 475d26a19b927018cf5af452e6848728412e173d Mon Sep 17 00:00:00 2001 From: Kat <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 2 Dec 2024 13:01:37 +0000 Subject: [PATCH] Find block log creation reason --- app/mailers/bulk_upload_mailer.rb | 2 + .../bulk_upload/lettings/validator.rb | 28 ++++++++----- app/services/bulk_upload/processor.rb | 40 +++++++++++++------ app/services/bulk_upload/sales/validator.rb | 37 +++++++++++++---- .../bulk_upload/lettings/validator_spec.rb | 10 ++--- spec/services/bulk_upload/processor_spec.rb | 32 +++++++++++++-- .../bulk_upload/sales/validator_spec.rb | 16 ++++---- 7 files changed, 119 insertions(+), 46 deletions(-) diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index e0115abb0..11aeccb15 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/app/mailers/bulk_upload_mailer.rb @@ -95,6 +95,8 @@ class BulkUploadMailer < NotifyMailer ) end + def send_correct_duplicates_and_upload_again_mail(bulk_upload:); end + def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:) bulk_upload_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? bulk_upload.sales? ? summary_bulk_upload_sales_result_url(bulk_upload) : summary_bulk_upload_lettings_result_url(bulk_upload) diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 116c3b745..85b0ea71b 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -40,29 +40,39 @@ class BulkUpload::Lettings::Validator end end - def create_logs? - return false if any_setup_errors? + # def create_logs? + # if block_log_creation_reason.present? + # case block_log_creation_reason + # when "row_parser_block_log_creation" + # Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") + # when "logs_invalid" + # Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") + # end + # return false + # end + + # true + # end + + def block_log_creation_reason + return "setup_errors" if any_setup_errors? if row_parsers.any?(&:block_log_creation?) Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") - return false + return "row_parser_block_log_creation" end if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled? - Sentry.capture_message("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") - return false + return "duplicate_logs" end row_parsers.each do |row_parser| 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}.") - return false + "logs_invalid" end - - true end def self.question_for_field(field) diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 38f67ede4..c54032fda 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -36,20 +36,28 @@ class BulkUpload::Processor if validator.any_setup_errors? send_setup_errors_mail - - elsif validator.create_logs? - create_logs - - if validator.soft_validation_errors_only? - send_check_soft_validations_mail - elsif created_logs_but_incompleted? - send_how_to_fix_upload_mail - elsif created_logs_and_all_completed? - bulk_upload.unpend - send_success_mail - end else - send_correct_and_upload_again_mail # summary/full report + block_creation_reason = validator.block_log_creation_reason + + if block_creation_reason.present? + case block_creation_reason + when "duplicate_logs" + send_correct_duplicates_and_upload_again_mail + else + send_correct_and_upload_again_mail # summary/full report + end + else + create_logs + + if validator.soft_validation_errors_only? + send_check_soft_validations_mail + elsif created_logs_but_incompleted? + send_how_to_fix_upload_mail + elsif created_logs_and_all_completed? + bulk_upload.unpend + send_success_mail + end + end end rescue StandardError => e Sentry.capture_exception(e) @@ -97,6 +105,12 @@ private .deliver_later end + def send_correct_duplicates_and_upload_again_mail + BulkUploadMailer + .send_correct_duplicates_and_upload_again_mail(bulk_upload:) + .deliver_later + end + def send_success_mail BulkUploadMailer .send_bulk_upload_complete_mail(user:, bulk_upload:) diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index 76fb6f1ae..4918ce51e 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -39,17 +39,42 @@ class BulkUpload::Sales::Validator end end - def create_logs? - return false if any_setup_errors? + # def create_logs? + # return false if any_setup_errors? + + # if row_parsers.any?(&:block_log_creation?) + # Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") + # return false + # end + + # if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled? + # Sentry.capture_message("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") + # return false + # end + + # row_parsers.each do |row_parser| + # 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}.") + # return false + # end + + # true + # end + + def block_log_creation_reason + return "setup_errors" if any_setup_errors? if row_parsers.any?(&:block_log_creation?) Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") - return false + return "row_parser_block_log_creation" end if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled? Sentry.capture_message("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") - return false + return "duplicate_logs" end row_parsers.each do |row_parser| @@ -58,10 +83,8 @@ class BulkUpload::Sales::Validator if any_logs_invalid? Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") - return false + "logs_invalid" end - - true end def any_setup_errors? diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index 897010de6..60eb8a955 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -232,7 +232,7 @@ RSpec.describe BulkUpload::Lettings::Validator do end end - describe "#create_logs?" do + describe "#block_log_creation_reason" do context "when a log has a clearable, non-setup error" do let(:log_1) { build(:lettings_log, :completed, period: 2, assigned_to: user) } let(:log_2) { build(:lettings_log, :completed, period: 2, assigned_to: user, age1: 5) } @@ -245,7 +245,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "returns false" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -261,7 +261,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "returns true" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -277,7 +277,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "will not create logs" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end @@ -291,7 +291,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "returns false" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index de0ed2dba..0368635e7 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -14,7 +14,7 @@ RSpec.describe BulkUpload::Processor do call: nil, total_logs_count: 1, any_setup_errors?: false, - create_logs?: true, + block_log_creation_reason: nil, soft_validation_errors_only?: false, ) end @@ -165,7 +165,7 @@ RSpec.describe BulkUpload::Processor do let(:log) { build(:lettings_log, :setup_completed, assigned_to: user) } before do - allow(mock_validator).to receive(:create_logs?).and_return(true) + allow(mock_validator).to receive(:block_log_creation_reason).and_return(nil) allow(mock_validator).to receive(:soft_validation_errors_only?).and_return(false) allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true) end @@ -198,7 +198,7 @@ RSpec.describe BulkUpload::Processor do context "when a bulk upload has logs with only soft validations triggered" do before do - allow(mock_validator).to receive(:create_logs?).and_return(true) + allow(mock_validator).to receive(:block_log_creation_reason).and_return(nil) allow(mock_validator).to receive(:soft_validation_errors_only?).and_return(true) allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true) end @@ -239,7 +239,7 @@ RSpec.describe BulkUpload::Processor do call: nil, total_logs_count: 1, any_setup_errors?: false, - create_logs?: false, + block_log_creation_reason: "row_parser_block_log_creation", ) end @@ -254,6 +254,30 @@ RSpec.describe BulkUpload::Processor do expect(mail_double).to have_received(:deliver_later) end end + + context "when upload has duplicate logs blocking log creation" do + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + call: nil, + total_logs_count: 1, + any_setup_errors?: false, + block_log_creation_reason: "duplicate_logs", + ) + end + + it "sends correct_and_upload_again_mail" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_correct_duplicates_and_upload_again_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_correct_duplicates_and_upload_again_mail) + expect(mail_double).to have_received(:deliver_later) + end + end end describe "#approve" do diff --git a/spec/services/bulk_upload/sales/validator_spec.rb b/spec/services/bulk_upload/sales/validator_spec.rb index c275ce681..968014e7c 100644 --- a/spec/services/bulk_upload/sales/validator_spec.rb +++ b/spec/services/bulk_upload/sales/validator_spec.rb @@ -204,7 +204,7 @@ RSpec.describe BulkUpload::Sales::Validator do end end - describe "#create_logs?" do + describe "#block_log_creation_reason" do context "when all logs are valid" do let(:log_1) { build(:sales_log, :completed, assigned_to: user) } let(:log_2) { build(:sales_log, :completed, assigned_to: user) } @@ -214,9 +214,9 @@ RSpec.describe BulkUpload::Sales::Validator do file.write(BulkUpload::SalesLogToCsv.new(log: log_2).to_csv_row) end - it "returns truthy" do + it "returns nil" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -229,9 +229,9 @@ RSpec.describe BulkUpload::Sales::Validator do file.write(BulkUpload::SalesLogToCsv.new(log: log_2).to_csv_row) end - it "returns truthy" do + it "returns nil" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -245,9 +245,9 @@ RSpec.describe BulkUpload::Sales::Validator do file.close end - it "returns false" do + it "returns the reason" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end @@ -262,7 +262,7 @@ RSpec.describe BulkUpload::Sales::Validator do it "will not create logs" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end end