From 9bd2463ecf0b1492ac83b5c941fa6f75befc37ad Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 27 Nov 2023 15:21:27 +0000 Subject: [PATCH 1/5] feat: use latest merge date/start date where available for availability text for schemes and locations --- app/models/location.rb | 4 +--- app/models/scheme.rb | 4 ++-- app/services/merge/merge_organisations_service.rb | 4 ++-- db/migrate/20231127120659_add_merge_date_to_schemes.rb | 5 +++++ db/migrate/20231127125729_add_merge_date_to_locations.rb | 5 +++++ db/schema.rb | 4 +++- 6 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20231127120659_add_merge_date_to_schemes.rb create mode 100644 db/migrate/20231127125729_add_merge_date_to_locations.rb diff --git a/app/models/location.rb b/app/models/location.rb index 65e0466b3..eceed7251 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -122,9 +122,7 @@ class Location < ApplicationRecord end def available_from - return startdate if startdate.present? - - FormHandler.instance.earliest_open_collection_start_date(now: created_at) + [merge_date, startdate].compact.max || FormHandler.instance.earliest_open_collection_start_date(now: created_at) end def open_deactivation diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 9d760998d..a01678981 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -243,7 +243,7 @@ class Scheme < ApplicationRecord end def validate_confirmed - required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units deactivation_date deactivation_date_type] + required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units deactivation_date deactivation_date_type merge_date] if confirmed == true required_attributes.any? do |attribute| @@ -262,7 +262,7 @@ class Scheme < ApplicationRecord end def available_from - FormHandler.instance.earliest_open_collection_start_date(now: created_at) + merge_date || FormHandler.instance.earliest_open_collection_start_date(now: created_at) end def open_deactivation diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index 46426a119..e0417704c 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -68,9 +68,9 @@ private merging_organisation.owned_schemes.each do |scheme| next if scheme.deactivated? - new_scheme = Scheme.create!(scheme.attributes.except("id", "owning_organisation_id", "old_id", "old_visible_id").merge(owning_organisation: @absorbing_organisation)) + new_scheme = Scheme.create!(scheme.attributes.except("id", "owning_organisation_id", "old_id", "old_visible_id").merge(owning_organisation: @absorbing_organisation, merge_date: @merge_date)) scheme.locations.each do |location| - new_scheme.locations << Location.new(location.attributes.except("id", "scheme_id", "old_id", "old_visible_id")) unless location.deactivated? + new_scheme.locations << Location.new(location.attributes.except("id", "scheme_id", "old_id", "old_visible_id").merge(merge_date: @merge_date)) unless location.deactivated? end @merged_schemes[merging_organisation.name] << { name: new_scheme.service_name, code: new_scheme.id } SchemeDeactivationPeriod.create!(scheme:, deactivation_date: @merge_date) diff --git a/db/migrate/20231127120659_add_merge_date_to_schemes.rb b/db/migrate/20231127120659_add_merge_date_to_schemes.rb new file mode 100644 index 000000000..b1dd4de94 --- /dev/null +++ b/db/migrate/20231127120659_add_merge_date_to_schemes.rb @@ -0,0 +1,5 @@ +class AddMergeDateToSchemes < ActiveRecord::Migration[7.0] + def change + add_column :schemes, :merge_date, :datetime + end +end diff --git a/db/migrate/20231127125729_add_merge_date_to_locations.rb b/db/migrate/20231127125729_add_merge_date_to_locations.rb new file mode 100644 index 000000000..5a8a24dff --- /dev/null +++ b/db/migrate/20231127125729_add_merge_date_to_locations.rb @@ -0,0 +1,5 @@ +class AddMergeDateToLocations < ActiveRecord::Migration[7.0] + def change + add_column :locations, :merge_date, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 7eda570d1..55b1d48dd 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_10_23_142854) do +ActiveRecord::Schema[7.0].define(version: 2023_11_27_125729) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -355,6 +355,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_10_23_142854) do t.datetime "startdate" t.string "location_admin_district" t.boolean "confirmed" + t.datetime "merge_date" t.index ["old_id"], name: "index_locations_on_old_id", unique: true t.index ["scheme_id"], name: "index_locations_on_scheme_id" end @@ -659,6 +660,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_10_23_142854) do t.string "old_visible_id" t.integer "total_units" t.boolean "confirmed" + t.datetime "merge_date" t.index ["owning_organisation_id"], name: "index_schemes_on_owning_organisation_id" end From 5b27b6fff48915657820759a42d6965a345351cb Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 27 Nov 2023 16:50:28 +0000 Subject: [PATCH 2/5] feat: add tests for availability text --- spec/helpers/locations_helper_spec.rb | 29 +++++++++++++++++++++++++++ spec/helpers/schemes_helper_spec.rb | 11 ++++++++++ 2 files changed, 40 insertions(+) diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb index da4b17322..56e805c8a 100644 --- a/spec/helpers/locations_helper_spec.rb +++ b/spec/helpers/locations_helper_spec.rb @@ -214,6 +214,35 @@ RSpec.describe LocationsHelper do expect(availability_attribute).to eq("Active from 1 April 2022") end + + context "when location was merged" do + context "when there is no startdate" do + it "displays merge date as availability date" do + location.update!(merge_date: Time.zone.local(2022, 4, 16)) + availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] + + expect(availability_attribute).to eq("Active from 16 April 2022") + end + end + + context "when the startdate is before the merge date" do + it "displays merge date as availability date" do + location.update!(startdate: Time.zone.local(2022, 4, 3), merge_date: Time.zone.local(2022, 4, 16)) + availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] + + expect(availability_attribute).to eq("Active from 16 April 2022") + end + end + + context "when the startdate is after the merge date" do + it "displays startdate as availability date" do + location.update!(startdate: Time.zone.local(2022, 5, 3), merge_date: Time.zone.local(2022, 4, 16)) + availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] + + expect(availability_attribute).to eq("Active from 3 May 2022") + end + end + end end context "with previous deactivations" do diff --git a/spec/helpers/schemes_helper_spec.rb b/spec/helpers/schemes_helper_spec.rb index 6aaa10260..7a52230ab 100644 --- a/spec/helpers/schemes_helper_spec.rb +++ b/spec/helpers/schemes_helper_spec.rb @@ -303,6 +303,17 @@ RSpec.describe SchemesHelper do expect(display_scheme_attributes(scheme)).to eq(attributes) end end + + context "when scheme was merged from another organisation" do + before do + FactoryBot.create(:location, scheme:) + scheme.merge_date = Time.zone.local(2023, 1, 5) + end + + it "returns correct availability" do + expect(display_scheme_attributes(scheme)).to include({ name: "Availability", value: "Active from 5 January 2023" }) + end + end end describe "edit_scheme_text" do From bde6f35254e6f456377805bb401405aa1b8bec4b Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Dec 2023 11:15:21 +0000 Subject: [PATCH 3/5] feat: use startdate instead to track merges on schemes and locations --- app/models/location.rb | 6 ++-- app/models/scheme.rb | 5 ++-- .../merge/merge_organisations_service.rb | 4 +-- ...0231127120659_add_merge_date_to_schemes.rb | 5 ---- ...31127125729_add_merge_date_to_locations.rb | 5 ---- db/schema.rb | 5 ++-- spec/helpers/locations_helper_spec.rb | 28 +++---------------- spec/helpers/schemes_helper_spec.rb | 2 +- 8 files changed, 15 insertions(+), 45 deletions(-) delete mode 100644 db/migrate/20231127120659_add_merge_date_to_schemes.rb delete mode 100644 db/migrate/20231127125729_add_merge_date_to_locations.rb diff --git a/app/models/location.rb b/app/models/location.rb index eceed7251..64272d549 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -21,9 +21,9 @@ class Location < ApplicationRecord scope :search_by_postcode, ->(postcode) { where("REPLACE(postcode, ' ', '') ILIKE ?", "%#{postcode.delete(' ')}%") } scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by, ->(param) { search_by_name(param).or(search_by_postcode(param)) } - scope :started, -> { where("startdate <= ?", Time.zone.today).or(where(startdate: nil)) } + scope :started, -> { where("locations.startdate <= ?", Time.zone.today).or(where(startdate: nil)) } scope :active, -> { where(confirmed: true).and(started) } - scope :started_in_2_weeks, -> { where("startdate <= ?", Time.zone.today + 2.weeks).or(where(startdate: nil)) } + scope :started_in_2_weeks, -> { where("locations.startdate <= ?", Time.zone.today + 2.weeks).or(where(startdate: nil)) } scope :active_in_2_weeks, -> { where(confirmed: true).and(started_in_2_weeks) } scope :confirmed, -> { where(confirmed: true) } scope :unconfirmed, -> { where.not(confirmed: true) } @@ -122,7 +122,7 @@ class Location < ApplicationRecord end def available_from - [merge_date, startdate].compact.max || FormHandler.instance.earliest_open_collection_start_date(now: created_at) + startdate || FormHandler.instance.earliest_open_collection_start_date(now: created_at) end def open_deactivation diff --git a/app/models/scheme.rb b/app/models/scheme.rb index a01678981..a8a17ebdb 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -243,7 +243,7 @@ class Scheme < ApplicationRecord end def validate_confirmed - required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units deactivation_date deactivation_date_type merge_date] + required_attributes = attribute_names - %w[id created_at updated_at old_id old_visible_id confirmed end_date sensitive secondary_client_group total_units deactivation_date deactivation_date_type startdate] if confirmed == true required_attributes.any? do |attribute| @@ -262,7 +262,7 @@ class Scheme < ApplicationRecord end def available_from - merge_date || FormHandler.instance.earliest_open_collection_start_date(now: created_at) + startdate || FormHandler.instance.earliest_open_collection_start_date(now: created_at) end def open_deactivation @@ -282,6 +282,7 @@ class Scheme < ApplicationRecord return :deactivated if open_deactivation&.deactivation_date.present? && date >= open_deactivation.deactivation_date return :deactivating_soon if open_deactivation&.deactivation_date.present? && date < open_deactivation.deactivation_date return :reactivating_soon if last_deactivation_before(date)&.reactivation_date.present? && date < last_deactivation_before(date).reactivation_date + return :activating_soon if startdate.present? && date < startdate :active end diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index 8dcef90e1..a6a148b21 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -69,9 +69,9 @@ private merging_organisation.owned_schemes.each do |scheme| next if scheme.deactivated? - new_scheme = Scheme.create!(scheme.attributes.except("id", "owning_organisation_id", "old_id", "old_visible_id").merge(owning_organisation: @absorbing_organisation, merge_date: @merge_date)) + new_scheme = Scheme.create!(scheme.attributes.except("id", "owning_organisation_id", "old_id", "old_visible_id").merge(owning_organisation: @absorbing_organisation, startdate: @merge_date)) scheme.locations.each do |location| - new_scheme.locations << Location.new(location.attributes.except("id", "scheme_id", "old_id", "old_visible_id").merge(merge_date: @merge_date)) unless location.deactivated? + new_scheme.locations << Location.new(location.attributes.except("id", "scheme_id", "old_id", "old_visible_id").merge(startdate: [location&.startdate, @merge_date].compact.max)) unless location.deactivated? end @merged_schemes[merging_organisation.name] << { name: new_scheme.service_name, code: new_scheme.id } SchemeDeactivationPeriod.create!(scheme:, deactivation_date: @merge_date) diff --git a/db/migrate/20231127120659_add_merge_date_to_schemes.rb b/db/migrate/20231127120659_add_merge_date_to_schemes.rb deleted file mode 100644 index b1dd4de94..000000000 --- a/db/migrate/20231127120659_add_merge_date_to_schemes.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddMergeDateToSchemes < ActiveRecord::Migration[7.0] - def change - add_column :schemes, :merge_date, :datetime - end -end diff --git a/db/migrate/20231127125729_add_merge_date_to_locations.rb b/db/migrate/20231127125729_add_merge_date_to_locations.rb deleted file mode 100644 index 5a8a24dff..000000000 --- a/db/migrate/20231127125729_add_merge_date_to_locations.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddMergeDateToLocations < ActiveRecord::Migration[7.0] - def change - add_column :locations, :merge_date, :datetime - end -end diff --git a/db/schema.rb b/db/schema.rb index 55b1d48dd..ba197acb6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_11_27_125729) do +ActiveRecord::Schema[7.0].define(version: 2023_12_04_101105) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -355,7 +355,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_11_27_125729) do t.datetime "startdate" t.string "location_admin_district" t.boolean "confirmed" - t.datetime "merge_date" t.index ["old_id"], name: "index_locations_on_old_id", unique: true t.index ["scheme_id"], name: "index_locations_on_scheme_id" end @@ -660,7 +659,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_11_27_125729) do t.string "old_visible_id" t.integer "total_units" t.boolean "confirmed" - t.datetime "merge_date" + t.datetime "startdate" t.index ["owning_organisation_id"], name: "index_schemes_on_owning_organisation_id" end diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb index 56e805c8a..fec1ce53b 100644 --- a/spec/helpers/locations_helper_spec.rb +++ b/spec/helpers/locations_helper_spec.rb @@ -216,31 +216,11 @@ RSpec.describe LocationsHelper do end context "when location was merged" do - context "when there is no startdate" do - it "displays merge date as availability date" do - location.update!(merge_date: Time.zone.local(2022, 4, 16)) - availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] - - expect(availability_attribute).to eq("Active from 16 April 2022") - end - end - - context "when the startdate is before the merge date" do - it "displays merge date as availability date" do - location.update!(startdate: Time.zone.local(2022, 4, 3), merge_date: Time.zone.local(2022, 4, 16)) - availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] - - expect(availability_attribute).to eq("Active from 16 April 2022") - end - end - - context "when the startdate is after the merge date" do - it "displays startdate as availability date" do - location.update!(startdate: Time.zone.local(2022, 5, 3), merge_date: Time.zone.local(2022, 4, 16)) - availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] + it "displays merge date as availability date" do + location.update!(startdate: Time.zone.local(2022, 4, 16)) + availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value] - expect(availability_attribute).to eq("Active from 3 May 2022") - end + expect(availability_attribute).to eq("Active from 16 April 2022") end end end diff --git a/spec/helpers/schemes_helper_spec.rb b/spec/helpers/schemes_helper_spec.rb index 7a52230ab..ff3030932 100644 --- a/spec/helpers/schemes_helper_spec.rb +++ b/spec/helpers/schemes_helper_spec.rb @@ -307,7 +307,7 @@ RSpec.describe SchemesHelper do context "when scheme was merged from another organisation" do before do FactoryBot.create(:location, scheme:) - scheme.merge_date = Time.zone.local(2023, 1, 5) + scheme.startdate = Time.zone.local(2023, 1, 5) end it "returns correct availability" do From bb2fd35c940a9e8c1f5f5965c61a7784c392add5 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Dec 2023 11:29:42 +0000 Subject: [PATCH 4/5] feat: test startdates are set correctly on merged schemes and locations --- .../merge/merge_organisations_service_spec.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index b6168d57a..1c82318e5 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -305,6 +305,9 @@ RSpec.describe Merge::MergeOrganisationsService do context "and merging organisation schemes and locations" do let!(:scheme) { create(:scheme, owning_organisation: merging_organisation) } let!(:location) { create(:location, scheme:) } + let!(:location_without_startdate) { create(:location, scheme:, startdate: nil) } + let!(:location_with_past_startdate) { create(:location, scheme:, startdate: Time.zone.today - 2.months) } + let!(:location_with_future_startdate) { create(:location, scheme:, startdate: Time.zone.today + 2.months) } let!(:deactivated_location) { create(:location, scheme:) } let!(:deactivated_scheme) { create(:scheme, owning_organisation: merging_organisation) } let!(:owned_lettings_log) { create(:lettings_log, :sh, scheme:, location:, startdate: Time.zone.tomorrow, owning_organisation: merging_organisation) } @@ -329,8 +332,12 @@ RSpec.describe Merge::MergeOrganisationsService do absorbing_organisation.reload expect(absorbing_organisation.owned_schemes.count).to eq(1) expect(absorbing_organisation.owned_schemes.first.service_name).to eq(scheme.service_name) - expect(absorbing_organisation.owned_schemes.first.locations.count).to eq(1) - expect(absorbing_organisation.owned_schemes.first.locations.first.postcode).to eq(location.postcode) + expect(absorbing_organisation.owned_schemes.first.startdate).to eq(Time.zone.yesterday) + expect(absorbing_organisation.owned_schemes.first.locations.count).to eq(4) + expect(absorbing_organisation.owned_schemes.first.locations.map(&:postcode)).to match_array([location, location_without_startdate, location_with_past_startdate, location_with_future_startdate].map(&:postcode)) + expect(absorbing_organisation.owned_schemes.first.locations.find_by(postcode: location_without_startdate.postcode).startdate).to eq(Time.zone.yesterday) + expect(absorbing_organisation.owned_schemes.first.locations.find_by(postcode: location_with_past_startdate.postcode).startdate).to eq(Time.zone.yesterday) + expect(absorbing_organisation.owned_schemes.first.locations.find_by(postcode: location_with_future_startdate.postcode).startdate).to eq(Time.zone.today + 2.months) expect(scheme.scheme_deactivation_periods.count).to eq(1) expect(scheme.scheme_deactivation_periods.first.deactivation_date.to_date).to eq(Time.zone.yesterday) end From 7aa46ecb56bac07a741a0b45e504a9e63c1c2480 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire Date: Mon, 4 Dec 2023 13:42:32 +0000 Subject: [PATCH 5/5] feat: add migration file --- db/migrate/20231204101105_add_startdate_to_schemes.rb | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 db/migrate/20231204101105_add_startdate_to_schemes.rb diff --git a/db/migrate/20231204101105_add_startdate_to_schemes.rb b/db/migrate/20231204101105_add_startdate_to_schemes.rb new file mode 100644 index 000000000..9299cb80a --- /dev/null +++ b/db/migrate/20231204101105_add_startdate_to_schemes.rb @@ -0,0 +1,5 @@ +class AddStartdateToSchemes < ActiveRecord::Migration[7.0] + def change + add_column :schemes, :startdate, :datetime + end +end