From 685a3a7546d1a28c400726ec6d6ce655fd0ba6e5 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 22 Mar 2023 10:55:51 +0000 Subject: [PATCH] integrate new bulk upload approve journey --- .../bulk_upload_lettings_resume_controller.rb | 2 +- .../bulk_upload_lettings_resume/confirm.rb | 11 ++++ .../bulk_upload_lettings_resume/fix_choice.rb | 4 ++ .../bulk_upload/lettings/validator.rb | 1 - app/services/bulk_upload/processor.rb | 26 +++++++-- ..._upload_lettings_resume_controller_spec.rb | 24 ++++++--- .../bulk_upload/lettings/validator_spec.rb | 53 ------------------- 7 files changed, 56 insertions(+), 65 deletions(-) diff --git a/app/controllers/bulk_upload_lettings_resume_controller.rb b/app/controllers/bulk_upload_lettings_resume_controller.rb index 21edd9d41..4c21d39e2 100644 --- a/app/controllers/bulk_upload_lettings_resume_controller.rb +++ b/app/controllers/bulk_upload_lettings_resume_controller.rb @@ -16,7 +16,7 @@ class BulkUploadLettingsResumeController < ApplicationController def update @bulk_upload = current_user.bulk_uploads.find(params[:id]) - if form.valid? # && form.save! + if form.valid? && form.save! redirect_to form.next_path else render form.view_path diff --git a/app/models/forms/bulk_upload_lettings_resume/confirm.rb b/app/models/forms/bulk_upload_lettings_resume/confirm.rb index e5f246008..7760ab2e8 100644 --- a/app/models/forms/bulk_upload_lettings_resume/confirm.rb +++ b/app/models/forms/bulk_upload_lettings_resume/confirm.rb @@ -14,6 +14,17 @@ module Forms def back_path page_bulk_upload_lettings_resume_path(bulk_upload, page: "fix-choice") end + + def next_path + resume_bulk_upload_lettings_result_path(bulk_upload) + end + + def save! + processor = BulkUpload::Processor.new(bulk_upload:) + processor.approve + + true + end end end end diff --git a/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb b/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb index 40dcd99cd..5513434de 100644 --- a/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb +++ b/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb @@ -44,6 +44,10 @@ module Forms "For this many errors we recommend to upload logs and fix errors on site as you can easily see the questions and select the appropriate answer." end end + + def save! + true + end end end end diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index bc415f3dd..15aef3c4d 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -44,7 +44,6 @@ class BulkUpload::Lettings::Validator def create_logs? return false if any_setup_errors? - return false if over_column_error_threshold? return false if row_parsers.any?(&:block_log_creation?) row_parsers.all? { |row_parser| row_parser.log.valid? } diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 2d27464f0..3ba6fb4d8 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -15,11 +15,16 @@ class BulkUpload::Processor if validator.any_setup_errors? send_setup_errors_mail elsif validator.create_logs? - create_logs - send_fix_errors_mail if created_logs_but_incompleted? - send_success_mail if created_logs_and_all_completed? + if bulk_upload.bulk_upload_errors.count.zero? + create_logs + + send_fix_errors_mail if created_logs_but_incompleted? + send_success_mail if created_logs_and_all_completed? + else + send_how_fix_upload_mail + end else - send_correct_and_upload_again_mail + send_correct_and_upload_again_mail # summary/full report end rescue StandardError => e Sentry.capture_exception(e) @@ -28,8 +33,21 @@ class BulkUpload::Processor downloader.delete_local_file! end + def approve + download + create_logs + ensure + downloader.delete_local_file! + end + private + def send_how_fix_upload_mail + BulkUploadMailer + .send_how_fix_upload_mail(bulk_upload:) + .deliver_later + end + def send_setup_errors_mail BulkUploadMailer .send_bulk_upload_failed_file_setup_error_mail(bulk_upload:) diff --git a/spec/requests/bulk_upload_lettings_resume_controller_spec.rb b/spec/requests/bulk_upload_lettings_resume_controller_spec.rb index a14594c13..5529a13db 100644 --- a/spec/requests/bulk_upload_lettings_resume_controller_spec.rb +++ b/spec/requests/bulk_upload_lettings_resume_controller_spec.rb @@ -59,14 +59,26 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do end describe "GET /lettings-logs/bulk-upload-resume/:ID/confirm" do - context "without previous answer" do - it "renders page" do - get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/confirm" + it "renders page" do + get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/confirm" - expect(response).to be_successful + expect(response).to be_successful - expect(response.body).to include("Are you sure") - end + expect(response.body).to include("Are you sure") + end + end + + describe "PATCH /lettings-logs/bulk-upload-resume/:ID/confirm" do + let(:mock_processor) { instance_double(BulkUpload::Processor, approve: nil) } + + it "approves logs for creation" do + allow(BulkUpload::Processor).to receive(:new).with(bulk_upload:).and_return(mock_processor) + + patch "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/confirm" + + expect(mock_processor).to have_received(:approve) + + expect(response).to redirect_to("/lettings-logs/bulk-upload-results/#{bulk_upload.id}/resume") end end end diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index 1b77f8f43..e40db4b87 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -280,58 +280,5 @@ RSpec.describe BulkUpload::Lettings::Validator do end end end - - context "when a column has error rate above absolute threshold" do - before do - stub_const("BulkUpload::Lettings::Validator::COLUMN_ABSOLUTE_ERROR_THRESHOLD", 1) - end - - context "when a column is over 60% error threshold" do - let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } - let(:log_2) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - let(:log_3) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - let(:log_4) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - let(:log_5) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - - before do - file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_3, line_ending: "\r\n", col_offset: 0).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_4, line_ending: "\r\n", col_offset: 0).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_5, line_ending: "\r\n", col_offset: 0).to_2022_csv_row) - file.close - end - - it "returns false" do - validator.call - expect(validator).not_to be_create_logs - end - end - - context "when a column is under 60% error threshold" do - let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user) } - let(:log_2) { build(:lettings_log, :completed, renttype: 1, created_by: user) } - let(:log_3) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - let(:log_4) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - let(:log_5) { build(:lettings_log, renttype: 2, created_by: user, builtype: nil, startdate: Time.zone.local(2022, 5, 1)) } - - before do - overrides = { age1: 50, age2: "R", age3: "R", age4: "4", age5: "R", age6: "R", age7: "R", age8: "R" } - - file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0, overrides:).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_2, line_ending: "\r\n", col_offset: 0, overrides:).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_3, line_ending: "\r\n", col_offset: 0, overrides:).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_4, line_ending: "\r\n", col_offset: 0, overrides:).to_2022_csv_row) - file.write(BulkUpload::LogToCsv.new(log: log_5, line_ending: "\r\n", col_offset: 0, overrides:).to_2022_csv_row) - - file.close - end - - it "returns true" do - validator.call - expect(validator).to be_create_logs - end - end - end end end