Do not update status for pending logs

app/models/bulk_upload.rb

@@ -116,7 +116,6 @@ class BulkUpload < ApplicationRecord
 
   def unpend
     logs.find_each do |log|
-      log.skip_update_status = true
       log.status = log.status_cache
       log.save!
     end

app/models/log.rb

@@ -57,7 +57,7 @@ class Log < ApplicationRecord
   scope :filter_by_owning_organisation_text_search, ->(param, _user) { where(owning_organisation: Organisation.search_by(param)) }
   scope :filter_by_managing_organisation_text_search, ->(param, _user) { where(managing_organisation: Organisation.search_by(param)) }
 
-  attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :select_best_address_match, :skip_dpo_validation
+  attr_accessor :skip_update_uprn_confirmed, :select_best_address_match, :skip_dpo_validation
 
   delegate :present?, to: :address_options, prefix: true
 
@@ -315,9 +315,11 @@ private
   end
 
   def update_status!
-    return if skip_update_status
-
-    self.status = calculate_status
+    if status == "pending"
+      self.status_cache = calculate_status
+    else
+      self.status = calculate_status
+    end
   end
 
   def all_subsections_completed?

app/services/bulk_upload/lettings/log_creator.rb

@@ -16,9 +16,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.skip_update_status = true
       row_parser.log.status = "pending"
-      row_parser.log.status_cache = row_parser.log.calculate_status
 
       begin
         row_parser.log.save!

app/services/bulk_upload/sales/log_creator.rb

@@ -15,9 +15,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.skip_update_status = true
       row_parser.log.status = "pending"
-      row_parser.log.status_cache = row_parser.log.calculate_status
 
       begin
         row_parser.log.save!

lib/tasks/correct_rent_type_value.rake

@@ -9,7 +9,6 @@ task correct_rent_type_value: :environment do
 
       new_rent_type_value = BulkUpload::Lettings::Year2024::RowParser::RENT_TYPE_BU_MAPPING[rent_type_at_upload]
       log.rent_type = new_rent_type_value
-      log.skip_update_status = true if log.status == "pending"
       if log.save
         Rails.logger.info("Log #{log.id} rent_type updated from #{rent_type_at_upload} to #{log.rent_type}")
       else

lib/tasks/recalculate_status_after_sales_over_retirement_age_validation.rake

@@ -3,7 +3,6 @@ task recalculate_status_over_retirement: :environment do
   validation_trigger_condition = "(ecstat1 != 5 AND age1 > 66) OR (ecstat2 != 5 AND age2 > 66) OR (ecstat3 != 5 AND age3 > 66) OR (ecstat4 != 5 AND age4 > 66) OR (ecstat5 != 5 AND age5 > 66) OR (ecstat6 != 5 AND age6 > 66)"
   SalesLog.filter_by_year(2024).where(status: "pending", status_cache: "completed").where(validation_trigger_condition).find_each do |log|
     log.status_cache = log.calculate_status
-    log.skip_update_status = true
 
     unless log.save
       Rails.logger.info "Could not save changes to pending sales log #{log.id}"

spec/lib/tasks/correct_rent_type_value_spec.rb

@@ -35,7 +35,6 @@ RSpec.describe "correct_rent_type_value" do
 
         it "updates the rent_type value on a pending log where it was set to 1 on create" do
           log = build(:lettings_log, :completed, rent_type: 1, bulk_upload:, status: "pending")
-          log.skip_update_status = true
           log.save!
           initial_updated_at = log.updated_at
           expect(log.status).to eq("pending")
@@ -166,7 +165,6 @@ RSpec.describe "correct_rent_type_value" do
 
         it "updates the rent_type value on a pending log where it was set to 2 on create" do
           log = build(:lettings_log, :completed, rent_type: 2, bulk_upload:, status: "pending")
-          log.skip_update_status = true
           log.save!
           initial_updated_at = log.updated_at
           expect(log.status).to eq("pending")

spec/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation_spec.rb

@@ -17,7 +17,6 @@ RSpec.describe "recalculate_status_after_sales_over_retirement_age_validation" d
 
         before do
           log.status = "completed"
-          log.skip_update_status = true
           log.save!
         end
 
@@ -34,7 +33,6 @@ RSpec.describe "recalculate_status_after_sales_over_retirement_age_validation" d
         before do
           log.status = "pending"
           log.status_cache = "completed"
-          log.skip_update_status = true
           log.save!
         end
 

spec/models/lettings_log_spec.rb

@@ -5,8 +5,8 @@ require "shared/shared_log_examples"
 # rubocop:disable RSpec/MessageChain
 RSpec.describe LettingsLog do
   let(:different_managing_organisation) { create(:organisation) }
-  let(:assigned_to_user) { create(:user) }
-  let(:owning_organisation) { assigned_to_user.organisation }
+  let(:owning_organisation) { create(:organisation, rent_periods: [2]) }
+  let(:assigned_to_user) { create(:user, organisation: owning_organisation) }
   let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") }
 
   around do |example|
@@ -220,6 +220,31 @@ RSpec.describe LettingsLog do
         expect(completed_lettings_log.completed?).to be(true)
       end
     end
+
+    it "sets the correct status for a completed lettings log" do
+      complete_lettings_log = build(:lettings_log, :completed, assigned_to: assigned_to_user)
+      complete_lettings_log.save!
+      expect(complete_lettings_log.reload.status).to eq "completed"
+    end
+
+    it "returns the correct status for an in progress lettings log" do
+      in_progress_lettings_log = build(:lettings_log, :in_progress, assigned_to: assigned_to_user)
+      in_progress_lettings_log.save!
+      expect(in_progress_lettings_log.reload.status).to eq "in_progress"
+    end
+
+    it "recalculates the status if it's currently set incorrectly" do
+      complete_lettings_log = build(:lettings_log, :completed, assigned_to: assigned_to_user, status: "in_progress")
+      complete_lettings_log.save!
+      expect(complete_lettings_log.reload.status).to eq "completed"
+    end
+
+    it "recalculates status_cache if the log is pending" do
+      complete_lettings_log = build(:lettings_log, :completed, assigned_to: assigned_to_user, status_cache: "in_progress", status: "pending")
+      complete_lettings_log.save!
+      expect(complete_lettings_log.reload.status_cache).to eq "completed"
+      expect(complete_lettings_log.status).to eq "pending"
+    end
   end
 
   describe "weekly_net_income" do

spec/models/log_spec.rb

@@ -29,6 +29,17 @@ RSpec.describe Log, type: :model do
       in_progress_lettings_log = build(:lettings_log, :in_progress, assigned_to: user)
       expect(in_progress_lettings_log.calculate_status).to eq "in_progress"
     end
+
+    it "recalculates the status if it's currently set incorrectly" do
+      complete_lettings_log = build(:lettings_log, :completed, assigned_to: user, status: "in_progress")
+      expect(complete_lettings_log.calculate_status).to eq "completed"
+    end
+
+    it "recalculates status_cache if the log is pending" do
+      complete_lettings_log = build(:lettings_log, :completed, assigned_to: user, status_cache: "in_progress", status: "pending")
+      expect(complete_lettings_log.calculate_status).to eq "completed"
+      expect(complete_lettings_log.calculate_status).to eq "completed"
+    end
   end
 
   describe "#blank_invalid_non_setup_fields!" do

spec/models/user_spec.rb

@@ -725,15 +725,8 @@ RSpec.describe User, type: :model do
       context "and the user has pending logs assigned to them" do
         let(:lettings_bu) { create(:bulk_upload, :lettings) }
         let(:sales_bu) { create(:bulk_upload, :sales) }
-        let!(:pending_lettings_log) { build(:lettings_log, status: "pending", assigned_to: user, bulk_upload: lettings_bu) }
-        let!(:pending_sales_log) { build(:sales_log, status: "pending", assigned_to: user, bulk_upload: sales_bu) }
-
-        before do
-          pending_lettings_log.skip_update_status = true
-          pending_lettings_log.save!
-          pending_sales_log.skip_update_status = true
-          pending_sales_log.save!
-        end
+        let!(:pending_lettings_log) { create(:lettings_log, status: "pending", assigned_to: user, bulk_upload: lettings_bu) }
+        let!(:pending_sales_log) { create(:sales_log, status: "pending", assigned_to: user, bulk_upload: sales_bu) }
 
         it "sets choice for fixing the logs to cancelled-by-moved-user" do
           user.reassign_logs_and_update_organisation(new_organisation, "reassign_all")

spec/requests/form_controller_spec.rb

@@ -512,7 +512,6 @@ RSpec.describe FormController, type: :request do
               owning_organisation: organisation,
               assigned_to: user,
               status: "pending",
-              skip_update_status: true,
             )
           end
 

spec/requests/lettings_logs_controller_spec.rb

@@ -244,7 +244,6 @@ RSpec.describe LettingsLogsController, type: :request do
         assigned_to: user,
         tenancycode: "LC999",
         status: "pending",
-        skip_update_status: true,
       )
     end
 
@@ -1107,7 +1106,6 @@ RSpec.describe LettingsLogsController, type: :request do
             :in_progress,
             assigned_to: user,
             status: "pending",
-            skip_update_status: true,
           )
         end
 
@@ -1165,7 +1163,6 @@ RSpec.describe LettingsLogsController, type: :request do
 
               before do
                 lettings_log.status = "completed"
-                lettings_log.skip_update_status = true
                 lettings_log.save!(validate: false)
               end
 

spec/requests/organisations_controller_spec.rb

@@ -1160,7 +1160,7 @@ RSpec.describe OrganisationsController, type: :request do
             create_list(:lettings_log, number_of_owned_org1_lettings_logs, assigned_to: user, owning_organisation: organisation, managing_organisation: child_organisation)
             create_list(:lettings_log, number_of_managed_org1_lettings_logs, assigned_to: user, owning_organisation: parent_organisation, managing_organisation: organisation)
             create_list(:lettings_log, number_of_owned_and_managed_org1_lettings_logs, assigned_to: user, owning_organisation: organisation, managing_organisation: organisation)
-            create(:lettings_log, assigned_to: user, status: "pending", skip_update_status: true)
+            create(:lettings_log, assigned_to: user, status: "pending")
             create_list(:lettings_log, number_of_org2_lettings_logs, assigned_to: nil, owning_organisation_id: unauthorised_organisation.id, managing_organisation_id: unauthorised_organisation.id)
 
             get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {}
@@ -2007,7 +2007,7 @@ RSpec.describe OrganisationsController, type: :request do
           let(:lettings_log_start_year) { lettings_logs[0].form.start_date.year }
 
           before do
-            create(:lettings_log, :in_progress, owning_organisation: organisation, status: "pending", skip_update_status: true)
+            create(:lettings_log, :in_progress, owning_organisation: organisation, status: "pending")
             create_list(:lettings_log, 2, :in_progress, owning_organisation: other_organisation)
           end
 
@@ -2071,7 +2071,7 @@ RSpec.describe OrganisationsController, type: :request do
 
           before do
             create_list(:sales_log, 2, :in_progress, owning_organisation: organisation)
-            create(:sales_log, :in_progress, owning_organisation: organisation, status: "pending", skip_update_status: true)
+            create(:sales_log, :in_progress, owning_organisation: organisation, status: "pending")
             create_list(:sales_log, 2, :in_progress, owning_organisation: other_organisation)
           end
 

spec/requests/sales_logs_controller_spec.rb

@@ -293,7 +293,6 @@ RSpec.describe SalesLogsController, type: :request do
               :sales_log,
               owning_organisation: organisation,
               status: "pending",
-              skip_update_status: true,
             )
           end
 

spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@@ -626,7 +626,6 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
           context "when a hidden log already exists in db" do
             before do
               parser.log.status = "pending"
-              parser.log.skip_update_status = true
               parser.log.save!
             end
 

spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb

@@ -677,7 +677,6 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
           context "when a hidden log already exists in db" do
             before do
               parser.log.status = "pending"
-              parser.log.skip_update_status = true
               parser.log.save!
             end
 

spec/services/bulk_upload/processor_spec.rb

@@ -257,7 +257,7 @@ RSpec.describe BulkUpload::Processor do
   end
 
   describe "#approve" do
-    let!(:log) { create(:lettings_log, :in_progress, bulk_upload:, status: "pending", skip_update_status: true, status_cache: "in_progress") }
+    let!(:log) { create(:lettings_log, :in_progress, bulk_upload:, status: "pending", status_cache: "in_progress") }
 
     it "makes pending logs no longer pending" do
       expect(log.status).to eql("pending")

spec/services/bulk_upload/sales/year2023/row_parser_spec.rb

@@ -694,7 +694,6 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do
     context "when a hidden log already exists in db" do
       before do
         parser.log.status = "pending"
-        parser.log.skip_update_status = true
         parser.log.save!
       end
 

spec/services/bulk_upload/sales/year2024/row_parser_spec.rb

@@ -824,7 +824,6 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
     context "when a hidden log already exists in db" do
       before do
         parser.log.status = "pending"
-        parser.log.skip_update_status = true
         parser.log.save!
       end
 

spec/services/exports/lettings_log_export_service_spec.rb

@@ -60,7 +60,6 @@ RSpec.describe Exports::LettingsLogExportService do
           :lettings_log,
           :completed,
           status: "pending",
-          skip_update_status: true,
           propcode: "123",
           ppostcode_full: "SE2 6RT",
           postcode_full: "NW1 5TY",