From f11f14dc504f2e6540538c2e6bac4ccf4af9b5e0 Mon Sep 17 00:00:00 2001 From: Arthur Campbell Date: Mon, 3 Jul 2023 16:59:38 +0100 Subject: [PATCH] use the methods dynamically created by active record in all relevant places, removing obsolete methods in teh process. various tests tweaked to suppor this change. rake task from another ticket folded into this ticket to prevent merge conflicts --- app/components/check_answers_summary_list_card_component.rb | 6 +----- app/helpers/check_answers_helper.rb | 6 +----- app/helpers/log_actions_helper.rb | 2 +- app/models/log.rb | 6 +----- app/services/bulk_upload/lettings/log_creator.rb | 2 +- app/services/bulk_upload/sales/log_creator.rb | 2 +- app/services/imports/lettings_logs_field_import_service.rb | 4 ++-- app/services/imports/sales_logs_field_import_service.rb | 4 ++-- lib/tasks/creation_method.rake | 5 +++++ .../check_answers_summary_list_card_component_spec.rb | 3 +-- spec/helpers/check_answers_helper_spec.rb | 3 +-- .../imports/lettings_logs_field_import_service_spec.rb | 2 +- .../imports/sales_logs_field_import_service_spec.rb | 2 +- spec/views/logs/edit.html.erb_spec.rb | 4 ++-- 14 files changed, 21 insertions(+), 30 deletions(-) create mode 100644 lib/tasks/creation_method.rake diff --git a/app/components/check_answers_summary_list_card_component.rb b/app/components/check_answers_summary_list_card_component.rb index c9409462a..d4e7650de 100644 --- a/app/components/check_answers_summary_list_card_component.rb +++ b/app/components/check_answers_summary_list_card_component.rb @@ -35,17 +35,13 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base private def unanswered_value - if bulk_uploaded? + if log.creation_method_bulk_upload? "You still need to answer this question".html_safe else "You didn’t answer this question".html_safe end end - def bulk_uploaded? - log.bulk_upload - end - def number_of_buyers log[:jointpur] == 1 ? 2 : 1 end diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 7d55594ee..d235dbc6f 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -54,14 +54,10 @@ private end def unanswered_value(log:) - if bulk_uploaded?(log:) + if log.creation_method_bulk_upload? "You still need to answer this question".html_safe else "You didn’t answer this question".html_safe end end - - def bulk_uploaded?(log:) - log.bulk_upload - end end diff --git a/app/helpers/log_actions_helper.rb b/app/helpers/log_actions_helper.rb index 8f02ccbf5..d6949e0cb 100644 --- a/app/helpers/log_actions_helper.rb +++ b/app/helpers/log_actions_helper.rb @@ -16,7 +16,7 @@ private def back_button_for(log) if log.completed? - if log.bulk_uploaded? + if log.creation_method_bulk_upload? if log.lettings? govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(log.bulk_upload) else diff --git a/app/models/log.rb b/app/models/log.rb index 3a6361e36..699bbae3f 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -24,7 +24,7 @@ class Log < ApplicationRecord "single log" => 1, "bulk upload" => 2, }.freeze - enum creation_method: CREATION_METHOD + enum creation_method: CREATION_METHOD, _prefix: true scope :visible, -> { where(status: %w[not_started in_progress completed]) } scope :exportable, -> { where(status: %w[not_started in_progress completed deleted]) } @@ -184,10 +184,6 @@ class Log < ApplicationRecord end end - def bulk_uploaded? - bulk_upload_id.present? - end - def collection_closed_for_editing? form.edit_end_date < Time.zone.now || older_than_previous_collection_year? end diff --git a/app/services/bulk_upload/lettings/log_creator.rb b/app/services/bulk_upload/lettings/log_creator.rb index 02852a2a5..99cfd4390 100644 --- a/app/services/bulk_upload/lettings/log_creator.rb +++ b/app/services/bulk_upload/lettings/log_creator.rb @@ -14,7 +14,7 @@ class BulkUpload::Lettings::LogCreator row_parser.log.blank_invalid_non_setup_fields! row_parser.log.bulk_upload = bulk_upload - row_parser.log.creation_method = "bulk upload" + row_parser.log.creation_method_bulk_upload! row_parser.log.skip_update_status = true row_parser.log.status = "pending" row_parser.log.status_cache = row_parser.log.calculate_status diff --git a/app/services/bulk_upload/sales/log_creator.rb b/app/services/bulk_upload/sales/log_creator.rb index 5aa9d01c8..3028914b9 100644 --- a/app/services/bulk_upload/sales/log_creator.rb +++ b/app/services/bulk_upload/sales/log_creator.rb @@ -14,7 +14,7 @@ class BulkUpload::Sales::LogCreator row_parser.log.blank_invalid_non_setup_fields! row_parser.log.bulk_upload = bulk_upload - row_parser.log.creation_method = "bulk upload" + row_parser.log.creation_method_bulk_upload! row_parser.log.skip_update_status = true row_parser.log.status = "pending" row_parser.log.status_cache = row_parser.log.calculate_status diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb index 61fe3e829..9148c99b7 100644 --- a/app/services/imports/lettings_logs_field_import_service.rb +++ b/app/services/imports/lettings_logs_field_import_service.rb @@ -49,10 +49,10 @@ module Imports when "Manual Entry" @logger.info "lettings log with old id #{old_id} entered manually, no need for update" when "Bulk Upload" - if log.creation_method == "bulk upload" + if log.creation_method_bulk_upload? @logger.info "lettings log #{log.id} creation method already set to bulk upload, no need for update" else - log.update!(creation_method: "bulk upload") + log.creation_method_bulk_upload! @logger.info "lettings log #{log.id} creation method set to bulk upload" end end diff --git a/app/services/imports/sales_logs_field_import_service.rb b/app/services/imports/sales_logs_field_import_service.rb index 5aafd29e3..d34aa878a 100644 --- a/app/services/imports/sales_logs_field_import_service.rb +++ b/app/services/imports/sales_logs_field_import_service.rb @@ -22,10 +22,10 @@ module Imports when "Manual Entry" @logger.info "sales log with old id #{old_id} entered manually, no need for update" when "Bulk Upload" - if log.creation_method == "bulk upload" + if log.creation_method_bulk_upload? @logger.info "sales log #{log.id} creation method already set to bulk upload, no need for update" else - log.update!(creation_method: "bulk upload") + log.creation_method_bulk_upload! @logger.info "sales log #{log.id} creation method set to bulk upload" end end diff --git a/lib/tasks/creation_method.rake b/lib/tasks/creation_method.rake new file mode 100644 index 000000000..7dde5ad80 --- /dev/null +++ b/lib/tasks/creation_method.rake @@ -0,0 +1,5 @@ +desc "set creation method to bulk upload if a log has a bulk upload id" +task set_creation_method: :environment do + LettingsLog.where.not(bulk_upload_id: nil).each(&:creation_method_bulk_upload!) + SalesLog.where.not(bulk_upload_id: nil).each(&:creation_method_bulk_upload!) +end diff --git a/spec/components/check_answers_summary_list_card_component_spec.rb b/spec/components/check_answers_summary_list_card_component_spec.rb index 842f55e14..3dcafe104 100644 --- a/spec/components/check_answers_summary_list_card_component_spec.rb +++ b/spec/components/check_answers_summary_list_card_component_spec.rb @@ -40,8 +40,7 @@ RSpec.describe CheckAnswersSummaryListCardComponent, type: :component do context "when log was created via a bulk upload and has an unanswered question" do subject(:component) { described_class.new(questions:, log:, user:) } - let(:bulk_upload) { build(:bulk_upload, :lettings) } - let(:log) { build(:lettings_log, :in_progress, bulk_upload:, age2: 99, startdate: Time.zone.local(2021, 5, 1)) } + let(:log) { build(:lettings_log, :in_progress, creation_method: "bulk upload", age2: 99, startdate: Time.zone.local(2021, 5, 1)) } it "displays tweaked copy in red" do expect(rendered).to have_selector("span", class: "app-!-colour-red", text: "You still need to answer this question") diff --git a/spec/helpers/check_answers_helper_spec.rb b/spec/helpers/check_answers_helper_spec.rb index a31c6bf16..f633801cb 100644 --- a/spec/helpers/check_answers_helper_spec.rb +++ b/spec/helpers/check_answers_helper_spec.rb @@ -39,8 +39,7 @@ RSpec.describe CheckAnswersHelper do describe "#get_answer_label" do context "when unanswered and bulk upload" do let(:question) { log.form.questions.sample } - let(:bulk_upload) { build(:bulk_upload, :sales) } - let(:log) { build(:sales_log, bulk_upload:) } + let(:log) { build(:sales_log, creation_method: "bulk upload") } it "is red" do expect(get_answer_label(question, log)).to include("red") diff --git a/spec/services/imports/lettings_logs_field_import_service_spec.rb b/spec/services/imports/lettings_logs_field_import_service_spec.rb index b151e1bf0..7a9767aa2 100644 --- a/spec/services/imports/lettings_logs_field_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_field_import_service_spec.rb @@ -106,7 +106,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do let(:lettings_log_id) { "166fc004-392e-47a8-acb8-1c018734882b" } it "logs that bulk upload id does not need setting" do - lettings_log.update!(creation_method: "single log") + lettings_log.creation_method_single_log! expect(logger).to receive(:info).with(/lettings log \d+ creation method set to bulk upload/) expect { import_service.update_field(field, remote_folder) }.to change { lettings_log.reload.creation_method }.to "bulk upload" end diff --git a/spec/services/imports/sales_logs_field_import_service_spec.rb b/spec/services/imports/sales_logs_field_import_service_spec.rb index 36d83b578..b5bdb22a4 100644 --- a/spec/services/imports/sales_logs_field_import_service_spec.rb +++ b/spec/services/imports/sales_logs_field_import_service_spec.rb @@ -57,7 +57,7 @@ RSpec.describe Imports::SalesLogsFieldImportService do let(:sales_log_filename) { "shared_ownership_sales_log2" } it "logs that bulk upload id does not need setting" do - sales_log.update!(creation_method: "single log") + sales_log.creation_method_single_log! expect(logger).to receive(:info).with(/sales log \d+ creation method set to bulk upload/) expect { import_service.update_field(field, remote_folder) }.to change { sales_log.reload.creation_method }.to "bulk upload" end diff --git a/spec/views/logs/edit.html.erb_spec.rb b/spec/views/logs/edit.html.erb_spec.rb index 6ac12230d..e28e0bc4b 100644 --- a/spec/views/logs/edit.html.erb_spec.rb +++ b/spec/views/logs/edit.html.erb_spec.rb @@ -69,7 +69,7 @@ RSpec.describe "logs/edit.html.erb" do context "when lettings log is bulk uploaded" do let(:bulk_upload) { create(:bulk_upload, :lettings) } - let(:log) { create(:lettings_log, :completed, bulk_upload:) } + let(:log) { create(:lettings_log, :completed, bulk_upload:, creation_method: "bulk upload") } it "has link 'Back to uploaded logs'" do render @@ -90,7 +90,7 @@ RSpec.describe "logs/edit.html.erb" do context "when sales log is bulk uploaded" do let(:bulk_upload) { create(:bulk_upload, :sales) } - let(:log) { create(:sales_log, :completed, bulk_upload:) } + let(:log) { create(:sales_log, :completed, bulk_upload:, creation_method: "bulk upload") } it "has link 'Back to uploaded logs'" do render