From 3a4a8144d57c52d3bde49eb40cde49fb75de4ee8 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Fri, 13 Dec 2024 08:35:52 +0000
Subject: [PATCH] CLDC-3646 Do not update status for pending logs (#2851)

* Do not update status for pending logs

* Update some tests

* skip_update_status when unpending logs
---
 app/models/log.rb                             |  6 +++-
 .../bulk_upload/lettings/log_creator.rb       |  2 --
 app/services/bulk_upload/sales/log_creator.rb |  2 --
 lib/tasks/correct_rent_type_value.rake        |  1 -
 ..._sales_over_retirement_age_validation.rake |  1 -
 .../lib/tasks/correct_rent_type_value_spec.rb |  2 --
 ...les_over_retirement_age_validation_spec.rb |  2 --
 spec/models/lettings_log_spec.rb              | 29 +++++++++++++++++--
 spec/models/log_spec.rb                       | 11 +++++++
 spec/models/user_spec.rb                      | 11 ++-----
 ...upload_lettings_results_controller_spec.rb |  2 +-
 spec/requests/form_controller_spec.rb         |  1 -
 .../requests/lettings_logs_controller_spec.rb |  5 +---
 .../requests/organisations_controller_spec.rb |  6 ++--
 spec/requests/sales_logs_controller_spec.rb   |  1 -
 .../lettings/year2023/row_parser_spec.rb      |  1 -
 .../lettings/year2024/row_parser_spec.rb      |  1 -
 spec/services/bulk_upload/processor_spec.rb   |  2 +-
 .../sales/year2023/row_parser_spec.rb         |  1 -
 .../sales/year2024/row_parser_spec.rb         |  1 -
 .../lettings_log_export_service_spec.rb       |  1 -
 21 files changed, 51 insertions(+), 38 deletions(-)

diff --git a/app/models/log.rb b/app/models/log.rb
index bcbea9c92..d2c7890e1 100644
--- a/app/models/log.rb
+++ b/app/models/log.rb
@@ -317,7 +317,11 @@ private
   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?
diff --git a/app/services/bulk_upload/lettings/log_creator.rb b/app/services/bulk_upload/lettings/log_creator.rb
index 1fd0d45b6..0df59b310 100644
--- a/app/services/bulk_upload/lettings/log_creator.rb
+++ b/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!
diff --git a/app/services/bulk_upload/sales/log_creator.rb b/app/services/bulk_upload/sales/log_creator.rb
index ea337d220..69f1580a0 100644
--- a/app/services/bulk_upload/sales/log_creator.rb
+++ b/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!
diff --git a/lib/tasks/correct_rent_type_value.rake b/lib/tasks/correct_rent_type_value.rake
index 1d43383c7..e3b166d04 100644
--- a/lib/tasks/correct_rent_type_value.rake
+++ b/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
diff --git a/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation.rake b/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation.rake
index e827e1c10..b1cbd56f0 100644
--- a/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation.rake
+++ b/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}"
diff --git a/spec/lib/tasks/correct_rent_type_value_spec.rb b/spec/lib/tasks/correct_rent_type_value_spec.rb
index adccda405..d4e015b5d 100644
--- a/spec/lib/tasks/correct_rent_type_value_spec.rb
+++ b/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")
diff --git a/spec/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation_spec.rb b/spec/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation_spec.rb
index b76380f90..cd5e0255c 100644
--- a/spec/lib/tasks/recalculate_status_after_sales_over_retirement_age_validation_spec.rb
+++ b/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
 
diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb
index 11e663469..17cad1c5e 100644
--- a/spec/models/lettings_log_spec.rb
+++ b/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
diff --git a/spec/models/log_spec.rb b/spec/models/log_spec.rb
index 76fadad6b..c5f133730 100644
--- a/spec/models/log_spec.rb
+++ b/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
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 53561f3e9..cd79977cc 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -726,15 +726,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")
diff --git a/spec/requests/bulk_upload_lettings_results_controller_spec.rb b/spec/requests/bulk_upload_lettings_results_controller_spec.rb
index afbd84df9..b23cb2c17 100644
--- a/spec/requests/bulk_upload_lettings_results_controller_spec.rb
+++ b/spec/requests/bulk_upload_lettings_results_controller_spec.rb
@@ -68,7 +68,7 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do
           get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary"
 
           expect(response.body).to include("This error report is out of date.")
-          expect(response.body).to include("Some logs in this upload are assigned to #{user.name}, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
+          expect(CGI.unescapeHTML(response.body)).to include("Some logs in this upload are assigned to #{user.name}, who has moved to a different organisation since this file was uploaded. Reupload the file to get an accurate error report.")
         end
       end
 
diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb
index fbbd5fc85..0cbec27d2 100644
--- a/spec/requests/form_controller_spec.rb
+++ b/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
 
diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb
index d84a6d714..b902305ad 100644
--- a/spec/requests/lettings_logs_controller_spec.rb
+++ b/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
 
@@ -1161,11 +1159,10 @@ RSpec.describe LettingsLogsController, type: :request do
 
             context "with bulk_upload_id filter" do
               let(:bulk_upload) { create(:bulk_upload, :lettings, user:) }
-              let(:lettings_log) { create(:lettings_log, :completed, age1: nil, bulk_upload:, assigned_to: user, creation_method: "bulk upload") }
+              let(:lettings_log) { create(:lettings_log, :completed, bulk_upload:, assigned_to: user, creation_method: "bulk upload") }
 
               before do
                 lettings_log.status = "completed"
-                lettings_log.skip_update_status = true
                 lettings_log.save!(validate: false)
               end
 
diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb
index 795ab5ab6..af9027753 100644
--- a/spec/requests/organisations_controller_spec.rb
+++ b/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: {}
@@ -2013,7 +2013,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
 
@@ -2077,7 +2077,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
 
diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb
index 6c882cc87..840613458 100644
--- a/spec/requests/sales_logs_controller_spec.rb
+++ b/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
 
diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
index 591ffdd53..acaba83e9 100644
--- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
+++ b/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
 
diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
index 13ab91370..8ffae6bd2 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/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
 
diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb
index 0368635e7..468d5fef8 100644
--- a/spec/services/bulk_upload/processor_spec.rb
+++ b/spec/services/bulk_upload/processor_spec.rb
@@ -281,7 +281,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")
diff --git a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb
index 2994a418e..8347647cb 100644
--- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb
+++ b/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
 
diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
index 7e969c10e..b449c57b0 100644
--- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
+++ b/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
 
diff --git a/spec/services/exports/lettings_log_export_service_spec.rb b/spec/services/exports/lettings_log_export_service_spec.rb
index c0dde5771..1db9cf4d9 100644
--- a/spec/services/exports/lettings_log_export_service_spec.rb
+++ b/spec/services/exports/lettings_log_export_service_spec.rb
@@ -62,7 +62,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",