revert 2310

app/components/check_answers_summary_list_card_component.rb

@@ -35,13 +35,17 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base
 private
 
   def unanswered_value
-    if log.creation_method_bulk_upload?
+    if bulk_uploaded?
       "<span class=\"app-!-colour-red\">You still need to answer this question</span>".html_safe
     else
       "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
     end
   end
 
+  def bulk_uploaded?
+    log.bulk_upload
+  end
+
   def number_of_buyers
     log[:jointpur] == 1 ? 2 : 1
   end

app/helpers/check_answers_helper.rb

@@ -54,10 +54,14 @@ private
   end
 
   def unanswered_value(log:)
-    if log.creation_method_bulk_upload?
+    if bulk_uploaded?(log:)
       "<span class=\"app-!-colour-red\">You still need to answer this question</span>".html_safe
     else
       "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
     end
   end
+
+  def bulk_uploaded?(log:)
+    log.bulk_upload
+  end
 end

app/helpers/log_actions_helper.rb

@@ -16,7 +16,7 @@ private
 
   def back_button_for(log)
     if log.completed?
-      if log.creation_method_bulk_upload?
+      if log.bulk_uploaded?
         if log.lettings?
           govuk_button_link_to "Back to uploaded logs", resume_bulk_upload_lettings_result_path(log.bulk_upload)
         else

app/models/log.rb

@@ -24,7 +24,7 @@ class Log < ApplicationRecord
     "single log" => 1,
     "bulk upload" => 2,
   }.freeze
-  enum creation_method: CREATION_METHOD, _prefix: true
+  enum creation_method: CREATION_METHOD
 
   scope :visible, -> { where(status: %w[not_started in_progress completed]) }
   scope :exportable, -> { where(status: %w[not_started in_progress completed deleted]) }
@@ -184,6 +184,10 @@ 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

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

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

app/services/imports/lettings_logs_field_import_service.rb

@@ -10,8 +10,6 @@ module Imports
         import_from(folder, :update_lettings_allocation)
       when "offered"
         import_from(folder, :update_offered)
-      when "creation_method"
-        import_from(folder, :update_creation_method)
       else
         raise "Updating #{field} is not supported by the field import service"
       end
@@ -38,24 +36,6 @@ module Imports
       end
     end
 
-    def update_creation_method(xml_doc)
-      old_id = meta_field_value(xml_doc, "document-id")
-      log = LettingsLog.find_by(old_id:)
-
-      return @logger.warn "lettings log with old id #{old_id} not found" unless log
-
-      upload_id = meta_field_value(xml_doc, "upload-id")
-
-      if upload_id.nil?
-        @logger.info "lettings log with old id #{old_id} entered manually, no need for update"
-      elsif log.creation_method_bulk_upload?
-        @logger.info "lettings log #{log.id} creation method already set to bulk upload, no need for update"
-      else
-        log.creation_method_bulk_upload!
-        @logger.info "lettings log #{log.id} creation method set to bulk upload"
-      end
-    end
-
     def update_lettings_allocation(xml_doc)
       return if meta_field_value(xml_doc, "form-name").include?("Sales")
 
@@ -130,5 +110,31 @@ module Imports
         @logger.warn("Could not find record matching legacy ID #{old_id}")
       end
     end
+
+    def compose_date(xml_doc, day_str, month_str, year_str)
+      day = Integer(field_value(xml_doc, "xmlns", day_str), exception: false)
+      month = Integer(field_value(xml_doc, "xmlns", month_str), exception: false)
+      year = Integer(field_value(xml_doc, "xmlns", year_str), exception: false)
+      if day.nil? || month.nil? || year.nil?
+        nil
+      else
+        Time.zone.local(year, month, day)
+      end
+    end
+
+    def string_or_nil(xml_doc, attribute)
+      str = field_value(xml_doc, "xmlns", attribute)
+      str.presence
+    end
+
+    # Unsafe: A string that has more than just the integer value
+    def unsafe_string_as_integer(xml_doc, attribute)
+      str = string_or_nil(xml_doc, attribute)
+      if str.nil?
+        nil
+      else
+        str.to_i
+      end
+    end
   end
 end

app/services/imports/lettings_logs_import_service.rb

@@ -66,7 +66,6 @@ module Imports
       attributes["startdate"] = compose_date(xml_doc, "DAY", "MONTH", "YEAR")
       attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID")
       attributes["managing_organisation_id"] = find_organisation_id(xml_doc, "MANINGORGID")
-      attributes["creation_method"] = creation_method(xml_doc)
       attributes["joint"] = unsafe_string_as_integer(xml_doc, "joint")
       attributes["startertenancy"] = unsafe_string_as_integer(xml_doc, "_2a")
       attributes["tenancy"] = unsafe_string_as_integer(xml_doc, "Q2b")

app/services/imports/logs_import_service.rb

@@ -11,7 +11,11 @@ module Imports
     # Unsafe: A string that has more than just the integer value
     def unsafe_string_as_integer(xml_doc, attribute)
       str = string_or_nil(xml_doc, attribute)
-      str&.to_i
+      if str.nil?
+        nil
+      else
+        str.to_i
+      end
     end
 
     def compose_date(xml_doc, day_str, month_str, year_str)
@@ -25,11 +29,6 @@ module Imports
       end
     end
 
-    def creation_method(xml_doc)
-      upload_id = meta_field_value(xml_doc, "upload-id")
-      upload_id.present? ? "bulk upload" : "single log"
-    end
-
     def find_organisation_id(xml_doc, id_field)
       old_visible_id = string_or_nil(xml_doc, id_field)
       organisation = Organisation.find_by(old_visible_id:)

app/services/imports/sales_logs_field_import_service.rb

@@ -1,32 +0,0 @@
-module Imports
-  class SalesLogsFieldImportService < LogsImportService
-    def update_field(field, folder)
-      case field
-      when "creation_method"
-        import_from(folder, :update_creation_method)
-      else
-        raise "Updating #{field} is not supported by the field import service"
-      end
-    end
-
-  private
-
-    def update_creation_method(xml_doc)
-      old_id = meta_field_value(xml_doc, "document-id")
-      log = SalesLog.find_by(old_id:)
-
-      return @logger.warn "sales log with old id #{old_id} not found" unless log
-
-      upload_id = meta_field_value(xml_doc, "upload-id")
-
-      if upload_id.nil?
-        @logger.info "sales log with old id #{old_id} entered manually, no need for update"
-      elsif log.creation_method_bulk_upload?
-        @logger.info "sales log #{log.id} creation method already set to bulk upload, no need for update"
-      else
-        log.creation_method_bulk_upload!
-        @logger.info "sales log #{log.id} creation method set to bulk upload"
-      end
-    end
-  end
-end

app/services/imports/sales_logs_import_service.rb

@@ -31,7 +31,6 @@ module Imports
       attributes["owning_organisation_id"] = find_organisation_id(xml_doc, "OWNINGORGID")
       attributes["type"] = unsafe_string_as_integer(xml_doc, "DerSaleType")
       attributes["old_id"] = meta_field_value(xml_doc, "document-id")
-      attributes["creation_method"] = creation_method(xml_doc)
       attributes["created_at"] = Time.zone.parse(meta_field_value(xml_doc, "created-date"))
       attributes["updated_at"] = Time.zone.parse(meta_field_value(xml_doc, "modified-date"))
       attributes["purchid"] = string_or_nil(xml_doc, "PurchaserCode")

lib/tasks/creation_method.rake

@@ -1,5 +0,0 @@
-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).find_each(&:creation_method_bulk_upload!)
-  SalesLog.where.not(bulk_upload_id: nil).find_each(&:creation_method_bulk_upload!)
-end

spec/components/check_answers_summary_list_card_component_spec.rb

@@ -40,7 +40,8 @@ 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(:log) { build(:lettings_log, :in_progress, creation_method: "bulk upload", age2: 99, startdate: Time.zone.local(2021, 5, 1)) }
+        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)) }
 
         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")

spec/fixtures/imports/logs/166fc004-392e-47a8-acb8-1c018734882b.xml

@@ -9,8 +9,7 @@
     <meta:modified-date>2022-04-12T14:10:59.953121Z</meta:modified-date>
     <meta:status>submitted-valid</meta:status>
     <meta:reporting-year>2021</meta:reporting-year>
-    <meta:upload-method>Bulk Upload</meta:upload-method>
+    <meta:upload-method>Manual Entry</meta:upload-method>
-    <meta:upload-id>8dda8f1a-f5a1-4827-8d82-dd7fd9258eab</meta:upload-id>
     <meta:schema assert-valid="true"/>
     <meta:rules assert-valid="true"/>
   </meta:metadata>

spec/fixtures/imports/sales_logs/shared_ownership_sales_log2.xml

@@ -9,8 +9,7 @@
     <meta:modified-date>2023-02-22T11:00:06.575832Z</meta:modified-date>
     <meta:status>submitted-valid</meta:status>
     <meta:reporting-year>2022</meta:reporting-year>
-    <meta:upload-method>Bulk Upload</meta:upload-method>
+    <meta:upload-method>Manual Entry</meta:upload-method>
-    <meta:upload-id>8dda8f1a-f5a1-4827-8d82-dd7fd9258eab</meta:upload-id>
     <meta:schema assert-valid="true"/>
     <meta:rules assert-valid="true"/>
   </meta:metadata>

spec/helpers/check_answers_helper_spec.rb

@@ -39,7 +39,8 @@ RSpec.describe CheckAnswersHelper do
   describe "#get_answer_label" do
     context "when unanswered and bulk upload" do
       let(:question) { log.form.questions.sample }
-      let(:log) { build(:sales_log, creation_method: "bulk upload") }
+      let(:bulk_upload) { build(:bulk_upload, :sales) }
+      let(:log) { build(:sales_log, bulk_upload:) }
 
       it "is red" do
         expect(get_answer_label(question, log)).to include("red")

spec/services/bulk_upload/lettings/log_creator_spec.rb

@@ -31,7 +31,7 @@ RSpec.describe BulkUpload::Lettings::LogCreator do
       it "sets the creation method" do
         service.call
 
-        expect(LettingsLog.last.creation_method_bulk_upload?).to be true
+        expect(LettingsLog.last.creation_method).to eq "bulk upload"
       end
     end
 

spec/services/bulk_upload/sales/log_creator_spec.rb

@@ -38,7 +38,7 @@ RSpec.describe BulkUpload::Sales::LogCreator do
       it "sets the creation method" do
         service.call
 
-        expect(SalesLog.last.creation_method_bulk_upload?).to be true
+        expect(SalesLog.last.creation_method).to eq "bulk upload"
       end
     end
 

spec/services/imports/lettings_logs_field_import_service_spec.rb

@@ -10,7 +10,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
   let(:fixture_directory) { "spec/fixtures/imports/logs" }
 
   let(:lettings_log_id) { "0ead17cb-1668-442d-898c-0d52879ff592" }
-  let(:lettings_log_file) { File.open("#{fixture_directory}/#{lettings_log_id}.xml") }
+  let(:lettings_log_file) { open_file(fixture_directory, lettings_log_id) }
   let(:lettings_log_xml) { Nokogiri::XML(lettings_log_file) }
   let(:remote_folder) { "lettings_logs" }
   let(:old_user_id) { "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa" }
@@ -21,6 +21,12 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
       Singleton.__init__(FormHandler)
       example.run
     end
+    Timecop.return
+    Singleton.__init__(FormHandler)
+  end
+
+  def open_file(directory, filename)
+    File.open("#{directory}/#{filename}.xml")
   end
 
   before do
@@ -77,51 +83,7 @@ RSpec.describe Imports::LettingsLogsFieldImportService do
     end
   end
 
-  context "when updating creation method" do
+  context "when updating letings allocation values" do
-    let(:field) { "creation_method" }
-    let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) }
-
-    before do
-      Imports::LettingsLogsImportService.new(storage_service, logger).create_logs(fixture_directory)
-      lettings_log_file.rewind
-    end
-
-    context "and the log was manually entered" do
-      it "logs that bulk upload id does not need setting" do
-        expect(logger).to receive(:info).with("lettings log with old id #{lettings_log_id} entered manually, no need for update")
-        expect { import_service.update_field(field, remote_folder) }.not_to(change { lettings_log.reload.creation_method })
-      end
-    end
-
-    context "and the log was bulk uploaded and the creation method is already correct" do
-      let(:lettings_log_id) { "166fc004-392e-47a8-acb8-1c018734882b" }
-
-      it "logs that bulk upload id does not need setting" do
-        expect(logger).to receive(:info).with(/lettings log \d+ creation method already set to bulk upload, no need for update/)
-        expect { import_service.update_field(field, remote_folder) }.not_to(change { lettings_log.reload.creation_method })
-      end
-    end
-
-    context "and the log was bulk uploaded and the creation method requires updating" do
-      let(:lettings_log_id) { "166fc004-392e-47a8-acb8-1c018734882b" }
-
-      it "logs that bulk upload id does not need setting" do
-        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
-    end
-
-    context "and the log was not previously imported" do
-      it "logs a warning that the log has not been found in the db" do
-        lettings_log.destroy!
-        expect(logger).to receive(:warn).with("lettings log with old id #{lettings_log_id} not found")
-        import_service.update_field(field, remote_folder)
-      end
-    end
-  end
-
-  context "when updating lettings allocation values" do
     let(:field) { "lettings_allocation" }
     let(:lettings_log) { LettingsLog.find_by(old_id: lettings_log_id) }
 

spec/services/imports/lettings_logs_import_service_spec.rb

@@ -478,29 +478,6 @@ RSpec.describe Imports::LettingsLogsImportService do
         end
       end
 
-      context "when the log being imported was manually entered" do
-        it "sets the creation method correctly" do
-          lettings_log_service.send(:create_log, lettings_log_xml)
-
-          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
-          expect(lettings_log.creation_method_single_log?).to be true
-        end
-      end
-
-      context "when the log being imported was bulk uploaded" do
-        before do
-          metadata = lettings_log_xml.at_xpath("//meta:metadata", { "meta" => "http://data.gov.uk/core/metadata" })
-          metadata << "<meta:upload-id>#{SecureRandom.uuid}</meta:upload-id>"
-        end
-
-        it "sets the creation method correctly" do
-          lettings_log_service.send(:create_log, lettings_log_xml)
-
-          lettings_log = LettingsLog.find_by(old_id: lettings_log_id)
-          expect(lettings_log.creation_method_bulk_upload?).to be true
-        end
-      end
-
       context "and income over the max" do
         before do
           lettings_log_xml.at_xpath("//xmlns:Q8Money").content = "25000"

spec/services/imports/sales_logs_field_import_service_spec.rb

@@ -1,76 +0,0 @@
-require "rails_helper"
-
-RSpec.describe Imports::SalesLogsFieldImportService do
-  subject(:import_service) { described_class.new(storage_service, logger) }
-
-  let(:storage_service) { instance_double(Storage::S3Service) }
-  let(:logger) { instance_double(ActiveSupport::Logger) }
-
-  let(:fixture_directory) { "spec/fixtures/imports/sales_logs" }
-  let(:sales_log_filename) { "shared_ownership_sales_log" }
-  let(:sales_log_file) { File.open("#{fixture_directory}/#{sales_log_filename}.xml") }
-  let(:organisation) { create(:organisation, old_visible_id: "1") }
-  let(:old_user_id) { "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa" }
-
-  let(:remote_folder) { "sales_logs" }
-
-  before do
-    create(:user, old_user_id:, organisation:)
-
-    allow(storage_service)
-      .to receive(:list_files)
-      .and_return(["#{sales_log_filename}.xml"])
-    allow(storage_service)
-      .to receive(:get_file_io)
-      .with("#{sales_log_filename}.xml")
-      .and_return(sales_log_file)
-  end
-
-  context "when updating creation method" do
-    let(:field) { "creation_method" }
-    let(:sales_log) { SalesLog.find_by(old_id: sales_log_filename) }
-
-    before do
-      Imports::SalesLogsImportService.new(storage_service, logger).create_logs(fixture_directory)
-      sales_log_file.rewind
-    end
-
-    context "and the log was manually entered" do
-      let(:sales_log_filename) { "shared_ownership_sales_log" }
-
-      it "logs that bulk upload id does not need setting" do
-        expect(logger).to receive(:info).with("sales log with old id #{sales_log_filename} entered manually, no need for update")
-        expect { import_service.update_field(field, remote_folder) }.not_to(change { sales_log.reload.creation_method })
-      end
-    end
-
-    context "and the log was bulk uploaded and the creation method is already correct" do
-      let(:sales_log_filename) { "shared_ownership_sales_log2" }
-
-      it "logs that bulk upload id does not need setting" do
-        expect(logger).to receive(:info).with(/sales log \d+ creation method already set to bulk upload, no need for update/)
-        expect { import_service.update_field(field, remote_folder) }.not_to(change { sales_log.reload.creation_method })
-      end
-    end
-
-    context "and the log was bulk uploaded and the creation method requires updating" do
-      let(:sales_log_filename) { "shared_ownership_sales_log2" }
-
-      it "logs that bulk upload id does not need setting" do
-        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
-    end
-
-    context "and the log was not previously imported" do
-      let(:sales_log_filename) { "shared_ownership_sales_log" }
-
-      it "logs a warning that the log has not been found in the db" do
-        sales_log.destroy!
-        expect(logger).to receive(:warn).with("sales log with old id #{sales_log_filename} not found")
-        import_service.update_field(field, remote_folder)
-      end
-    end
-  end
-end

spec/services/imports/sales_logs_import_service_spec.rb

@@ -263,6 +263,8 @@ RSpec.describe Imports::SalesLogsImportService do
           Singleton.__init__(FormHandler)
           example.run
         end
+        Timecop.return
+        Singleton.__init__(FormHandler)
       end
 
       before do
@@ -1054,28 +1056,6 @@ RSpec.describe Imports::SalesLogsImportService do
         end
       end
 
-      context "when the log being imported was manually entered" do
-        let(:sales_log_id) { "shared_ownership_sales_log" }
-
-        it "sets the creation method correctly" do
-          sales_log_service.send(:create_log, sales_log_xml)
-
-          sales_log = SalesLog.find_by(old_id: sales_log_id)
-          expect(sales_log.creation_method_single_log?).to be true
-        end
-      end
-
-      context "when the log being imported was bulk uploaded" do
-        let(:sales_log_id) { "shared_ownership_sales_log2" }
-
-        it "sets the creation method correctly" do
-          sales_log_service.send(:create_log, sales_log_xml)
-
-          sales_log = SalesLog.find_by(old_id: sales_log_id)
-          expect(sales_log.creation_method_bulk_upload?).to be true
-        end
-      end
-
       context "when inferring age known" do
         let(:sales_log_id) { "discounted_ownership_sales_log" }
 

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:, creation_method: "bulk upload") }
+      let(:log) { create(:lettings_log, :completed, 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:, creation_method: "bulk upload") }
+      let(:log) { create(:sales_log, :completed, bulk_upload:) }
 
       it "has link 'Back to uploaded logs'" do
         render