CLDC-3496 Move period creation to log (#2490)

* Move period creation to log

* Allow adding rent periods to organisations

* Update more tests

spec/factories/lettings_log.rb

@@ -6,6 +6,13 @@ FactoryBot.define do
     managing_organisation { assigned_to.organisation }
     created_at { Time.zone.today }
     updated_at { Time.zone.today }
+
+    before(:create) do |log, _evaluator|
+      if log.period && !log.managing_organisation.organisation_rent_periods.exists?(rent_period: log.period)
+        log.managing_organisation.organisation_rent_periods << build(:organisation_rent_period, rent_period: log.period)
+      end
+    end
+
     trait :setup_completed do
       startdate_today
       renewal { 0 }

spec/factories/organisation.rb

@@ -15,14 +15,12 @@ FactoryBot.define do
     end
 
     transient do
-      skip_rent_period_creation { false }
+      rent_periods { [] }
     end
 
-    after(:build) do |organisation, evaluator|
+    after(:create) do |organisation, evaluator|
-      unless evaluator.skip_rent_period_creation
+      evaluator.rent_periods.each do |rent_period|
-        (1..11).each do |rent_period|
+        organisation.organisation_rent_periods << build(:organisation_rent_period, rent_period:)
-          organisation.organisation_rent_periods << build(:organisation_rent_period, organisation:, rent_period:)
-        end
       end
     end
 

spec/helpers/organisations_helper_spec.rb

@@ -3,7 +3,7 @@ require "rails_helper"
 RSpec.describe OrganisationsHelper do
   include TagHelper
   describe "display_organisation_attributes" do
-    let(:organisation) { create(:organisation, skip_rent_period_creation: true) }
+    let(:organisation) { create(:organisation) }
 
     it "has the correct values" do
       expect(display_organisation_attributes(organisation)).to eq(

spec/models/form/lettings/questions/period_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Form::Lettings::Questions::Period, type: :model do
   end
 
   context "when managing organisation has rent periods" do
-    let(:managing_organisation) { create(:organisation, skip_rent_period_creation: true) }
+    let(:managing_organisation) { create(:organisation) }
     let(:log) { create(:lettings_log, managing_organisation:) }
 
     before do

spec/models/lettings_log_derived_fields_spec.rb

@@ -2,7 +2,7 @@ require "rails_helper"
 require "shared/shared_examples_for_derived_fields"
 
 RSpec.describe LettingsLog, type: :model do
-  let(:organisation) { build(:organisation, name: "derived fields org", skip_rent_period_creation: true) }
+  let(:organisation) { build(:organisation, name: "derived fields org") }
   let(:user) { build(:user, organisation:) }
   let(:log) { build(:lettings_log, :startdate_today, assigned_to: user) }
 
@@ -956,7 +956,7 @@ RSpec.describe LettingsLog, type: :model do
   end
 
   describe "variables dependent on whether a letting is a renewal" do
-    let(:organisation) { create(:organisation, skip_rent_period_creation: true) }
+    let(:organisation) { create(:organisation) }
     let(:user) { create(:user, organisation:) }
     let(:startdate) { Time.zone.today }
     let(:persisted_renewal_lettings_log) { create(:lettings_log, :setup_completed, startdate:, renewal: 1, assigned_to: user) }

spec/models/organisation_spec.rb

@@ -109,7 +109,7 @@ RSpec.describe Organisation, type: :model do
     end
 
     context "with associated rent periods" do
-      let(:organisation) { create(:organisation, skip_rent_period_creation: true) }
+      let(:organisation) { create(:organisation) }
       let(:period_1_label) { "Every minute" }
       let(:fake_rent_periods) do
         {

spec/models/validations/financial_validations_spec.rb

@@ -139,7 +139,7 @@ RSpec.describe Validations::FinancialValidations do
   end
 
   describe "rent period validations" do
-    let(:organisation) { create(:organisation, skip_rent_period_creation: true) }
+    let(:organisation) { create(:organisation) }
     let(:user) { create(:user, organisation:) }
     let(:record) { create(:lettings_log, owning_organisation: organisation, managing_organisation: organisation, assigned_to: user) }
     let(:used_period) { 2 }

spec/requests/OrganisationsController/organisations_controller_rent_periods_spec.rb

@@ -53,7 +53,7 @@ RSpec.describe OrganisationsController, type: :request do
   end
 
   describe "#edit" do
-    let(:organisation) { create(:organisation, skip_rent_period_creation: true) }
+    let(:organisation) { create(:organisation) }
     let(:fake_rent_periods) do
       {
         "1" => { "value" => "Every minute" },
@@ -83,7 +83,7 @@ RSpec.describe OrganisationsController, type: :request do
   end
 
   describe "#update" do
-    let(:organisation) { create(:organisation, skip_rent_period_creation: true) }
+    let(:organisation) { create(:organisation) }
     let(:initially_checked_rent_period_id) { "1" }
     let(:initially_unchecked_rent_period_id) { "2" }
     let(:params) do

spec/requests/lettings_logs_controller_spec.rb

@@ -1,7 +1,7 @@
 require "rails_helper"
 
 RSpec.describe LettingsLogsController, type: :request do
-  let(:user) { FactoryBot.create(:user) }
+  let(:user) { FactoryBot.create(:user, organisation: create(:organisation, rent_periods: [2])) }
   let(:owning_organisation) { user.organisation }
   let(:managing_organisation) { owning_organisation }
   let(:api_username) { "test_user" }

spec/services/bulk_upload/lettings/log_creator_spec.rb

@@ -3,7 +3,7 @@ require "rails_helper"
 RSpec.describe BulkUpload::Lettings::LogCreator do
   subject(:service) { described_class.new(bulk_upload:, path: "") }
 
-  let(:owning_org) { create(:organisation, old_visible_id: 123) }
+  let(:owning_org) { create(:organisation, old_visible_id: 123, rent_periods: [2]) }
   let(:user) { create(:user, organisation: owning_org) }
 
   let(:bulk_upload) { create(:bulk_upload, :lettings, user:) }

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

@@ -10,7 +10,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
   let(:user) { create(:user, organisation: owning_org) }
 
   let(:owning_org) { create(:organisation, :with_old_visible_id) }
-  let(:managing_org) { create(:organisation, :with_old_visible_id) }
+  let(:managing_org) { create(:organisation, :with_old_visible_id, rent_periods: [4]) }
   let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
   let(:location) { create(:location, :with_old_visible_id, scheme:) }
 

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

@@ -10,7 +10,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
   let(:user) { create(:user, organisation: owning_org) }
 
   let(:owning_org) { create(:organisation, :with_old_visible_id) }
-  let(:managing_org) { create(:organisation, :with_old_visible_id) }
+  let(:managing_org) { create(:organisation, :with_old_visible_id, rent_periods: [4, 1]) }
   let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) }
   let(:location) { create(:location, :with_old_visible_id, scheme:) }
 

spec/services/bulk_upload/processor_spec.rb

@@ -5,7 +5,7 @@ RSpec.describe BulkUpload::Processor do
 
   let(:bulk_upload) { create(:bulk_upload, :lettings, user:) }
   let(:user) { create(:user, organisation: owning_org) }
-  let(:owning_org) { create(:organisation, old_visible_id: 123) }
+  let(:owning_org) { create(:organisation, old_visible_id: 123, rent_periods: [2]) }
   let(:mock_validator) do
     instance_double(
       BulkUpload::Lettings::Validator,

spec/services/merge/merge_organisations_service_spec.rb

@@ -75,8 +75,8 @@ RSpec.describe Merge::MergeOrganisationsService do
       end
 
       context "and merging organisation rent periods" do
-        let(:absorbing_organisation) { create(:organisation, holds_own_stock: false, name: "absorbing org", skip_rent_period_creation: true) }
+        let(:absorbing_organisation) { create(:organisation, holds_own_stock: false, name: "absorbing org") }
-        let(:merging_organisation) { create(:organisation, holds_own_stock: true, name: "fake org", skip_rent_period_creation: true) }
+        let(:merging_organisation) { create(:organisation, holds_own_stock: true, name: "fake org") }
 
         before do
           OrganisationRentPeriod.create!(organisation: absorbing_organisation, rent_period: 1)
@@ -1160,8 +1160,8 @@ RSpec.describe Merge::MergeOrganisationsService do
       end
 
       context "and merging organisation rent periods" do
-        let(:new_absorbing_organisation) { create(:organisation, :without_dpc, holds_own_stock: false, skip_rent_period_creation: true) }
+        let(:new_absorbing_organisation) { create(:organisation, :without_dpc, holds_own_stock: false) }
-        let(:merging_organisation) { create(:organisation, holds_own_stock: true, name: "fake org", skip_rent_period_creation: true) }
+        let(:merging_organisation) { create(:organisation, holds_own_stock: true, name: "fake org") }
 
         before do
           OrganisationRentPeriod.create!(organisation: new_absorbing_organisation, rent_period: 1)