Browse Source

Set duplicate_set_id straight on logs

pull/2141/head
Kat 2 years ago
parent
commit
2d7e7337c7
  1. 8
      app/controllers/delete_logs_controller.rb
  2. 29
      app/controllers/form_controller.rb
  3. 18
      app/models/duplicate_log_reference.rb
  4. 2
      app/models/lettings_log.rb
  5. 5
      app/models/log.rb
  6. 2
      app/models/sales_log.rb
  7. 1
      app/services/exports/lettings_log_export_service.rb
  8. 11
      db/migrate/20240110101500_create_duplicate_log_references.rb
  9. 6
      db/migrate/20240118183843_add_duplicate_set_id.rb
  10. 14
      db/schema.rb
  11. 10
      lib/tasks/set_duplicate_references.rake
  12. 9
      spec/factories/duplicate_log_reference.rb
  13. 19
      spec/features/form/form_navigation_spec.rb
  14. 37
      spec/features/lettings_log_spec.rb
  15. 40
      spec/features/sales_log_spec.rb
  16. 2
      spec/fixtures/exports/general_needs_log.xml
  17. 2
      spec/fixtures/exports/general_needs_log_23_24.xml
  18. 2
      spec/fixtures/exports/supported_housing_logs.xml
  19. 123
      spec/lib/tasks/set_duplicate_references_spec.rb
  20. 89
      spec/models/duplicate_log_reference_spec.rb
  21. 28
      spec/requests/form_controller_spec.rb
  22. 4
      spec/services/csv/lettings_log_csv_service_spec.rb
  23. 4
      spec/services/csv/sales_log_csv_service_spec.rb
  24. 6
      spec/services/exports/lettings_log_export_service_spec.rb

8
app/controllers/delete_logs_controller.rb

@ -28,9 +28,9 @@ class DeleteLogsController < ApplicationController
discard logs
if request.referer&.include?("delete-duplicates")
logs.each do |log|
log.duplicate_log_references.destroy_all
log.update!(duplicate_set_id: nil)
end
LettingsLog.find(params["remaining_log_id"]).duplicate_log_references.destroy_all
LettingsLog.find(params["remaining_log_id"]).update!(duplicate_set_id: nil)
redirect_to lettings_log_duplicate_logs_path(lettings_log_id: params["remaining_log_id"], original_log_id: params["original_log_id"], referrer: params[:referrer], organisation_id: params[:organisation_id]), notice: I18n.t("notification.duplicate_logs_deleted", count: logs.count, log_ids: duplicate_log_ids(logs))
else
@ -62,9 +62,9 @@ class DeleteLogsController < ApplicationController
discard logs
if request.referer&.include?("delete-duplicates")
logs.each do |log|
log.duplicate_log_references.destroy_all
log.update!(duplicate_set_id: nil)
end
SalesLog.find(params["remaining_log_id"]).duplicate_log_references.destroy_all
SalesLog.find(params["remaining_log_id"]).update!(duplicate_set_id: nil)
redirect_to sales_log_duplicate_logs_path(sales_log_id: params["remaining_log_id"], original_log_id: params["original_log_id"], referrer: params[:referrer], organisation_id: params[:organisation_id]), notice: I18n.t("notification.duplicate_logs_deleted", count: logs.count, log_ids: duplicate_log_ids(logs))
else

29
app/controllers/form_controller.rb

@ -175,12 +175,12 @@ private
if dynamic_duplicates.count.positive?
saved_duplicates = @log.duplicates
unless saved_duplicates == dynamic_duplicates
duplicate_log_reference = DuplicateLogReference.find_or_create_by!(log_id: @log.id, log_type: @log.class.name)
@log.update!(duplicate_set_id: new_duplicate_set_id) if @log.duplicate_set_id.blank?
dynamic_duplicates.each do |duplicate|
DuplicateLogReference.find_or_create_by!(log_id: duplicate.id, log_type: @log.class.name, duplicate_set_id: duplicate_log_reference.duplicate_set_id)
duplicate.update!(duplicate_set_id: @log.duplicate_set_id) if duplicate.duplicate_set_id != @log.duplicate_set_id
end
return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
end
return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
end
end
@ -257,12 +257,12 @@ private
if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).count.positive?
unless current_user.send(class_name.pluralize).duplicate_logs(@log).count.positive?
remove_fixed_duplicate_log_references(@log)
remove_fixed_duplicate_set_ids(@log)
flash[:notice] = deduplication_success_banner
end
send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id])
else
remove_fixed_duplicate_log_references(original_log)
remove_fixed_duplicate_set_ids(original_log)
flash[:notice] = deduplication_success_banner
send("#{class_name}_duplicate_logs_path", "#{class_name}_id".to_sym => from_referrer_query("first_remaining_duplicate_id"), original_log_id: from_referrer_query("original_log_id"), referrer: params[:referrer], organisation_id: params[:organisation_id])
end
@ -284,12 +284,19 @@ private
I18n.t("notification.duplicate_logs.deduplication_success_banner", log_link: deduplicated_log_link, changed_question_label:).html_safe
end
def remove_fixed_duplicate_log_references(log)
duplicate_log_reference = DuplicateLogReference.find_by(log_id: log.id, log_type: log.class.name)
return unless duplicate_log_reference
def remove_fixed_duplicate_set_ids(log)
duplicate_set_id = log.duplicate_set_id
return unless duplicate_set_id
duplicate_set_id = duplicate_log_reference.duplicate_set_id
duplicate_log_reference.destroy! if duplicate_log_reference.present?
DuplicateLogReference.find_by(duplicate_set_id:).destroy! if DuplicateLogReference.where(duplicate_set_id:).count == 1
log.update!(duplicate_set_id: nil)
LettingsLog.find_by(duplicate_set_id:)&.update!(duplicate_set_id: nil) if log.lettings? && LettingsLog.where(duplicate_set_id:).count == 1
SalesLog.find_by(duplicate_set_id:)&.update!(duplicate_set_id: nil) if log.sales? && SalesLog.where(duplicate_set_id:).count == 1
end
def new_duplicate_set_id
loop do
duplicate_set_id = SecureRandom.random_number(1_000_000)
return duplicate_set_id unless LettingsLog.exists?(duplicate_set_id:)
end
end
end

18
app/models/duplicate_log_reference.rb

@ -1,18 +0,0 @@
class DuplicateLogReference < ApplicationRecord
belongs_to :log, polymorphic: true
before_create :set_default_duplicate_set_id
private
def set_default_duplicate_set_id
self.duplicate_set_id ||= generate_new_id
end
def generate_new_id
loop do
duplicate_set_id = SecureRandom.random_number(1_000_000)
return duplicate_set_id unless DuplicateLogReference.exists?(duplicate_set_id:)
end
end
end

2
app/models/lettings_log.rb

@ -646,7 +646,7 @@ class LettingsLog < Log
end
def duplicates
LettingsLog.joins(:duplicate_log_references).where(duplicate_log_references: { duplicate_set_id: duplicate_log_references&.first&.duplicate_set_id }).where.not(id:)
LettingsLog.where.not(duplicate_set_id: nil).where(duplicate_set_id:).where.not(id:)
end
private

5
app/models/log.rb

@ -7,7 +7,6 @@ class Log < ApplicationRecord
belongs_to :created_by, class_name: "User", optional: true
belongs_to :updated_by, class_name: "User", optional: true
belongs_to :bulk_upload, optional: true
has_many :duplicate_log_references, as: :log
before_save :update_status!
@ -210,10 +209,6 @@ class Log < ApplicationRecord
end
end
def duplicate_set_id
duplicate_log_references.first&.duplicate_set_id
end
private
# Handle logs that are older than previous collection start date

2
app/models/sales_log.rb

@ -458,6 +458,6 @@ class SalesLog < Log
end
def duplicates
SalesLog.joins(:duplicate_log_references).where(duplicate_log_references: { duplicate_set_id: duplicate_log_references&.first&.duplicate_set_id }).where.not(id:)
SalesLog.where.not(duplicate_set_id: nil).where(duplicate_set_id:).where.not(id:)
end
end

1
app/services/exports/lettings_log_export_service.rb

@ -226,7 +226,6 @@ module Exports
attribute_hash["relat#{index}"] = "R"
attribute_hash["ecstat#{index}"] = 10
end
attribute_hash["duplicate_set_id"] = lettings_log.duplicate_set_id
attribute_hash
end

11
db/migrate/20240110101500_create_duplicate_log_references.rb

@ -1,11 +0,0 @@
class CreateDuplicateLogReferences < ActiveRecord::Migration[7.0]
def change
create_table :duplicate_log_references do |t|
t.integer :duplicate_set_id
t.integer :log_id
t.string :log_type
t.timestamps
end
end
end

6
db/migrate/20240118183843_add_duplicate_set_id.rb

@ -0,0 +1,6 @@
class AddDuplicateSetId < ActiveRecord::Migration[7.0]
def change
add_column :lettings_logs, :duplicate_set_id, :integer
add_column :sales_logs, :duplicate_set_id, :integer
end
end

14
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: 2024_01_10_101500) do
ActiveRecord::Schema[7.0].define(version: 2024_01_18_183843) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -63,14 +63,6 @@ ActiveRecord::Schema[7.0].define(version: 2024_01_10_101500) do
t.index ["organisation_id"], name: "index_data_protection_confirmations_on_organisation_id"
end
create_table "duplicate_log_references", force: :cascade do |t|
t.integer "duplicate_set_id"
t.integer "log_id"
t.string "log_type"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end
create_table "la_rent_ranges", force: :cascade do |t|
t.integer "ranges_rent_id"
t.integer "lettype"
@ -310,6 +302,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_01_10_101500) do
t.integer "supcharg_value_check"
t.integer "scharge_value_check"
t.integer "pscharge_value_check"
t.integer "duplicate_set_id"
t.index ["bulk_upload_id"], name: "index_lettings_logs_on_bulk_upload_id"
t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id"
t.index ["location_id"], name: "index_lettings_logs_on_location_id"
@ -471,6 +464,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_01_10_101500) do
t.string "reader_type", null: false
t.bigint "reader_id"
t.datetime "timestamp", precision: nil, null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["readable_type", "readable_id"], name: "index_read_marks_on_readable_type_and_readable_id"
t.index ["reader_id", "reader_type", "readable_type", "readable_id"], name: "read_marks_reader_readable_index", unique: true
t.index ["reader_type", "reader_id"], name: "index_read_marks_on_reader_type_and_reader_id"
@ -656,6 +651,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_01_10_101500) do
t.integer "old_form_id"
t.datetime "values_updated_at"
t.bigint "managing_organisation_id"
t.integer "duplicate_set_id"
t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id"
t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id"
t.index ["managing_organisation_id"], name: "index_sales_logs_on_managing_organisation_id"

10
lib/tasks/set_duplicate_references.rake

@ -2,19 +2,19 @@ desc "Set duplicate references for sales and lettings logs"
task set_duplicate_references: :environment do
SalesLog.filter_by_year(2023).duplicate_sets.each do |duplicate_set|
duplicate_set_id = generate_new_duplicate_set_id
next if duplicate_set.any? { |log_id| DuplicateLogReference.exists?(log_id:, log_type: "SalesLog") }
next if duplicate_set.any? { |_log_id| SalesLog.exists?(duplicate_set_id:) }
duplicate_set.each do |log_id|
DuplicateLogReference.create(log_id:, log_type: "SalesLog", duplicate_set_id:)
SalesLog.find(log_id).update!(duplicate_set_id:)
end
end
LettingsLog.filter_by_year(2023).duplicate_sets.each do |duplicate_set|
duplicate_set_id = generate_new_duplicate_set_id
next if duplicate_set.any? { |log_id| DuplicateLogReference.exists?(log_id:, log_type: "LettingsLog") }
next if duplicate_set.any? { |_log_id| LettingsLog.exists?(duplicate_set_id:) }
duplicate_set.each do |log_id|
DuplicateLogReference.create(log_id:, log_type: "LettingsLog", duplicate_set_id:)
LettingsLog.find(log_id).update!(duplicate_set_id:)
end
end
end
@ -22,6 +22,6 @@ end
def generate_new_duplicate_set_id
loop do
duplicate_set_id = SecureRandom.random_number(1_000_000)
return duplicate_set_id unless DuplicateLogReference.exists?(duplicate_set_id:)
return duplicate_set_id unless LettingsLog.exists?(duplicate_set_id:)
end
end

9
spec/factories/duplicate_log_reference.rb

@ -1,9 +0,0 @@
FactoryBot.define do
factory :duplicate_log_reference do
log_id { 1 }
log_type { "SalesLog" }
duplicate_set_id { nil }
created_at { Time.zone.today }
updated_at { Time.zone.today }
end
end

19
spec/features/form/form_navigation_spec.rb

@ -178,28 +178,25 @@ RSpec.describe "Form Navigation" do
end
describe "fixing duplicate logs" do
let(:lettings_log) { create(:lettings_log, :duplicate, created_by: user) }
let(:second_log) { create(:lettings_log, :duplicate, created_by: user) }
before do
create(:duplicate_log_reference, log_id: id, log_type: "LettingsLog")
create(:duplicate_log_reference, log_id: second_log.id, log_type: "LettingsLog")
end
let!(:lettings_log) { create(:lettings_log, :duplicate, created_by: user, duplicate_set_id: 1) }
let!(:second_log) { create(:lettings_log, :duplicate, created_by: user, duplicate_set_id: 1) }
it "shows a correct cancel link" do
expect(DuplicateLogReference.count).to eq(2)
expect(lettings_log.duplicates.count).to eq(1)
visit("lettings-logs/#{id}/tenant-code-test?first_remaining_duplicate_id=#{second_log.id}&original_log_id=#{id}&referrer=duplicate_logs")
click_link(text: "Cancel")
expect(page).to have_current_path("/lettings-logs/#{id}/duplicate-logs?original_log_id=#{id}")
expect(DuplicateLogReference.count).to eq(2)
lettings_log.reload
expect(lettings_log.duplicates.count).to eq(1)
end
it "shows a correct Save Changes buttons" do
expect(DuplicateLogReference.count).to eq(2)
expect(lettings_log.duplicates.count).to eq(1)
visit("lettings-logs/#{id}/tenant-code-test?first_remaining_duplicate_id=#{id}&original_log_id=#{id}&referrer=duplicate_logs")
click_button(text: "Save changes")
expect(page).to have_current_path("/lettings-logs/#{id}/duplicate-logs?original_log_id=#{id}&referrer=duplicate_logs")
expect(DuplicateLogReference.count).to eq(2)
lettings_log.reload
expect(lettings_log.duplicates.count).to eq(1)
end
end
end

37
spec/features/lettings_log_spec.rb

@ -459,9 +459,11 @@ RSpec.describe "Lettings Log Features" do
end
it "allows keeping the original log and deleting duplicates" do
expect(DuplicateLogReference.count).to eq(2)
lettings_log.reload
duplicate_log.reload
expect(lettings_log.duplicates.count).to eq(1)
expect(lettings_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicate_set_id).not_to be_nil
expect(lettings_log.duplicates).to include(duplicate_log)
expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
@ -477,8 +479,13 @@ RSpec.describe "Lettings Log Features" do
expect(page).not_to have_link("Keep this log and delete duplicates")
expect(page).to have_link("Back to Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}")
expect(DuplicateLogReference.count).to eq(0)
lettings_log.reload
duplicate_log.reload
expect(lettings_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
it "allows changing answers on remaining original log" do
@ -491,9 +498,11 @@ RSpec.describe "Lettings Log Features" do
end
it "allows keeping the duplicate log and deleting the original one" do
expect(DuplicateLogReference.count).to eq(2)
lettings_log.reload
duplicate_log.reload
expect(duplicate_log.duplicates.count).to eq(1)
expect(lettings_log.duplicates.count).to eq(1)
expect(lettings_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicates).to include(lettings_log)
expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
@ -509,8 +518,13 @@ RSpec.describe "Lettings Log Features" do
expect(page).not_to have_link("Keep this log and delete duplicates")
expect(page).to have_link("Back to lettings logs", href: "/lettings-logs")
expect(DuplicateLogReference.count).to eq(0)
lettings_log.reload
duplicate_log.reload
expect(lettings_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
it "allows changing answers to remaining duplicate log" do
@ -523,9 +537,11 @@ RSpec.describe "Lettings Log Features" do
end
it "allows deduplicating logs by changing the answers on the duplicate log" do
expect(DuplicateLogReference.count).to eq(2)
lettings_log.reload
duplicate_log.reload
expect(lettings_log.duplicates.count).to eq(1)
expect(lettings_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicate_set_id).not_to be_nil
expect(lettings_log.duplicates).to include(duplicate_log)
expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
@ -538,8 +554,13 @@ RSpec.describe "Lettings Log Features" do
expect(page).to have_content("Log #{duplicate_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the tenant code.")
expect(DuplicateLogReference.count).to eq(0)
lettings_log.reload
duplicate_log.reload
expect(lettings_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
it "allows deduplicating logs by changing the answers on the original log" do
@ -551,8 +572,8 @@ RSpec.describe "Lettings Log Features" do
expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
expect(page).to have_content("Log #{lettings_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the tenant code.")
expect(DuplicateLogReference.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicate_set_id).to be_nil
end
end
end

40
spec/features/sales_log_spec.rb

@ -198,9 +198,12 @@ RSpec.describe "Sales Log Features" do
end
it "allows keeping the original log and deleting duplicates" do
expect(DuplicateLogReference.count).to eq(2)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(1)
expect(duplicate_log.duplicates.count).to eq(1)
expect(sales_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicate_set_id).not_to be_nil
expect(sales_log.duplicates).to include(duplicate_log)
expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
@ -216,8 +219,13 @@ RSpec.describe "Sales Log Features" do
expect(page).not_to have_link("Keep this log and delete duplicates")
expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}")
expect(DuplicateLogReference.count).to eq(0)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
it "allows changing answer on remaining original log" do
@ -230,9 +238,12 @@ RSpec.describe "Sales Log Features" do
end
it "allows keeping the duplicate log and deleting the original one" do
expect(DuplicateLogReference.count).to eq(2)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(1)
expect(duplicate_log.duplicates.count).to eq(1)
expect(sales_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicates).to include(sales_log)
expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
@ -248,8 +259,13 @@ RSpec.describe "Sales Log Features" do
expect(page).not_to have_link("Keep this log and delete duplicates")
expect(page).to have_link("Back to sales logs", href: "/sales-logs")
expect(DuplicateLogReference.count).to eq(0)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
it "allows changing answers on remaining duplicate log" do
@ -262,9 +278,12 @@ RSpec.describe "Sales Log Features" do
end
it "allows deduplicating logs by changing the answers on the duplicate log" do
expect(DuplicateLogReference.count).to eq(2)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(1)
expect(duplicate_log.duplicates.count).to eq(1)
expect(sales_log.duplicate_set_id).not_to be_nil
expect(duplicate_log.duplicate_set_id).not_to be_nil
expect(sales_log.duplicates).to include(duplicate_log)
expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
@ -277,8 +296,13 @@ RSpec.describe "Sales Log Features" do
expect(page).to have_content("Log #{duplicate_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the purchaser code.")
expect(DuplicateLogReference.count).to eq(0)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
it "allows deduplicating logs by changing the answers on the original log" do
@ -291,8 +315,10 @@ RSpec.describe "Sales Log Features" do
expect(page).to have_content("Log #{sales_log.id} is no longer a duplicate and has been removed from the list")
expect(page).to have_content("You changed the purchaser code.")
expect(DuplicateLogReference.count).to eq(0)
expect(sales_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_log.duplicate_set_id).to be_nil
end
end
end

2
spec/fixtures/exports/general_needs_log.xml vendored

@ -144,6 +144,7 @@
<county/>
<discarded_at/>
<creation_method>1</creation_method>
<duplicate_set_id/>
<formid>{id}</formid>
<owningorgid>{owning_org_id}</owningorgid>
<owningorgname>DLUHC</owningorgname>
@ -156,7 +157,6 @@
<log_id>{log_id}</log_id>
<created_by>test1@example.com</created_by>
<amended_by/>
<duplicate_set_id/>
<providertype>1</providertype>
</form>
</forms>

2
spec/fixtures/exports/general_needs_log_23_24.xml vendored

@ -145,6 +145,7 @@
<county/>
<discarded_at/>
<creation_method>1</creation_method>
<duplicate_set_id/>
<formid>{id}</formid>
<owningorgid>{owning_org_id}</owningorgid>
<owningorgname>DLUHC</owningorgname>
@ -157,7 +158,6 @@
<log_id>{log_id}</log_id>
<created_by>test1@example.com</created_by>
<amended_by/>
<duplicate_set_id/>
<providertype>1</providertype>
</form>
</forms>

2
spec/fixtures/exports/supported_housing_logs.xml vendored

@ -143,6 +143,7 @@
<county/>
<discarded_at/>
<creation_method>1</creation_method>
<duplicate_set_id/>
<formid>{id}</formid>
<owningorgid>{owning_org_id}</owningorgid>
<owningorgname>DLUHC</owningorgname>
@ -172,7 +173,6 @@
<units>20</units>
<location_code>{location_id}</location_code>
<location_status>active</location_status>
<duplicate_set_id/>
<providertype>1</providertype>
</form>
</forms>

123
spec/lib/tasks/set_duplicate_references_spec.rb

@ -20,15 +20,14 @@ RSpec.describe "set_duplicate_references" do
let!(:sales_log_without_duplicates) { create(:sales_log, created_by: user) }
it "creates duplicate references for sales logs" do
expect(DuplicateLogReference.count).to eq(0)
expect(sales_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_log_references).to be_empty
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_sales_log.duplicates.count).to eq(0)
expect(duplicate_sales_log.duplicate_log_references).to be_empty
expect(duplicate_sales_log.duplicate_set_id).to be_nil
expect(second_duplicate_sales_log.duplicates.count).to eq(0)
expect(second_duplicate_sales_log.duplicate_log_references).to be_empty
expect(second_duplicate_sales_log.duplicate_set_id).to be_nil
expect(sales_log_without_duplicates.duplicates.count).to eq(0)
expect(sales_log_without_duplicates.duplicate_log_references).to be_empty
expect(sales_log_without_duplicates.duplicate_set_id).to be_nil
task.invoke
sales_log.reload
@ -36,32 +35,13 @@ RSpec.describe "set_duplicate_references" do
second_duplicate_sales_log.reload
sales_log_without_duplicates.reload
expect(DuplicateLogReference.count).to eq(3)
expect(sales_log.duplicates.count).to eq(2)
expect(sales_log.duplicate_log_references.count).to eq(1)
expect(duplicate_sales_log.duplicates.count).to eq(2)
expect(duplicate_sales_log.duplicate_log_references.count).to eq(1)
expect(second_duplicate_sales_log.duplicates.count).to eq(2)
expect(second_duplicate_sales_log.duplicate_log_references.count).to eq(1)
expect(sales_log_without_duplicates.duplicates.count).to eq(0)
expect(sales_log_without_duplicates.duplicate_log_references).to be_empty
expect(sales_log.duplicate_log_references.count).to eq(1)
expect(sales_log.duplicate_set_id).to eq(duplicate_sales_log.duplicate_set_id)
expect(sales_log.duplicate_set_id).to eq(second_duplicate_sales_log.duplicate_set_id)
end
it "does not create the references twice" do
expect(DuplicateLogReference.count).to eq(0)
task.invoke
expect(DuplicateLogReference.count).to eq(3)
task.reenable
task.invoke
expect(DuplicateLogReference.count).to eq(3)
end
end
context "and there are sales duplicates in multiple organisations" do
@ -75,41 +55,35 @@ RSpec.describe "set_duplicate_references" do
let!(:other_sales_log_without_duplicates) { create(:sales_log, created_by: other_user) }
it "creates separate duplicate references for sales logs" do
expect(DuplicateLogReference.count).to eq(0)
expect(sales_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_log_references).to be_empty
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_sales_log.duplicates.count).to eq(0)
expect(duplicate_sales_log.duplicate_log_references).to be_empty
expect(duplicate_sales_log.duplicate_set_id).to be_nil
expect(sales_log_without_duplicates.duplicates.count).to eq(0)
expect(sales_log_without_duplicates.duplicate_log_references).to be_empty
expect(sales_log_without_duplicates.duplicate_set_id).to be_nil
expect(other_sales_log.duplicates.count).to eq(0)
expect(other_sales_log.duplicate_log_references).to be_empty
expect(other_sales_log.duplicate_set_id).to be_nil
expect(other_duplicate_sales_log.duplicates.count).to eq(0)
expect(other_duplicate_sales_log.duplicate_log_references).to be_empty
expect(other_duplicate_sales_log.duplicate_set_id).to be_nil
expect(other_sales_log_without_duplicates.duplicates.count).to eq(0)
expect(other_sales_log_without_duplicates.duplicate_log_references).to be_empty
expect(other_sales_log_without_duplicates.duplicate_set_id).to be_nil
task.invoke
sales_log.reload
duplicate_sales_log.reload
sales_log_without_duplicates.reload
other_sales_log.reload
other_duplicate_sales_log.reload
expect(DuplicateLogReference.count).to eq(4)
expect(sales_log.duplicates.count).to eq(1)
expect(sales_log.duplicate_log_references.count).to eq(1)
expect(duplicate_sales_log.duplicates.count).to eq(1)
expect(duplicate_sales_log.duplicate_log_references.count).to eq(1)
expect(sales_log_without_duplicates.duplicates.count).to eq(0)
expect(sales_log_without_duplicates.duplicate_log_references).to be_empty
expect(sales_log.duplicate_set_id).to eq(duplicate_sales_log.duplicate_set_id)
expect(other_sales_log.duplicates.count).to eq(1)
expect(other_sales_log.duplicate_log_references.count).to eq(1)
expect(other_duplicate_sales_log.duplicates.count).to eq(1)
expect(other_duplicate_sales_log.duplicate_log_references.count).to eq(1)
expect(other_sales_log_without_duplicates.duplicates.count).to eq(0)
expect(other_sales_log_without_duplicates.duplicate_log_references).to be_empty
expect(other_sales_log.duplicate_set_id).to eq(other_duplicate_sales_log.duplicate_set_id)
expect(other_sales_log.duplicate_set_id).not_to eq(sales_log.duplicate_set_id)
end
@ -128,21 +102,19 @@ RSpec.describe "set_duplicate_references" do
end
it "does not create duplicate references for sales logs" do
expect(DuplicateLogReference.count).to eq(0)
expect(sales_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_log_references).to be_empty
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_sales_log.duplicates.count).to eq(0)
expect(duplicate_sales_log.duplicate_log_references).to be_empty
expect(duplicate_sales_log.duplicate_set_id).to be_nil
task.invoke
sales_log.reload
duplicate_sales_log.reload
expect(DuplicateLogReference.count).to eq(0)
expect(sales_log.duplicates.count).to eq(0)
expect(sales_log.duplicate_log_references).to be_empty
expect(sales_log.duplicate_set_id).to be_nil
expect(duplicate_sales_log.duplicates.count).to eq(0)
expect(duplicate_sales_log.duplicate_log_references).to be_empty
expect(duplicate_sales_log.duplicate_set_id).to be_nil
end
end
@ -154,15 +126,14 @@ RSpec.describe "set_duplicate_references" do
let!(:lettings_log_without_duplicates) { create(:lettings_log, created_by: user) }
it "creates duplicate references for lettings logs" do
expect(DuplicateLogReference.count).to eq(0)
expect(lettings_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_log_references).to be_empty
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_lettings_log.duplicates.count).to eq(0)
expect(duplicate_lettings_log.duplicate_log_references).to be_empty
expect(duplicate_lettings_log.duplicate_set_id).to be_nil
expect(second_duplicate_lettings_log.duplicates.count).to eq(0)
expect(second_duplicate_lettings_log.duplicate_log_references).to be_empty
expect(second_duplicate_lettings_log.duplicate_set_id).to be_nil
expect(lettings_log_without_duplicates.duplicates.count).to eq(0)
expect(lettings_log_without_duplicates.duplicate_log_references).to be_empty
expect(lettings_log_without_duplicates.duplicate_set_id).to be_nil
task.invoke
lettings_log.reload
@ -170,32 +141,14 @@ RSpec.describe "set_duplicate_references" do
second_duplicate_lettings_log.reload
lettings_log_without_duplicates.reload
expect(DuplicateLogReference.count).to eq(3)
expect(lettings_log.duplicates.count).to eq(2)
expect(lettings_log.duplicate_log_references.count).to eq(1)
expect(duplicate_lettings_log.duplicates.count).to eq(2)
expect(duplicate_lettings_log.duplicate_log_references.count).to eq(1)
expect(second_duplicate_lettings_log.duplicates.count).to eq(2)
expect(second_duplicate_lettings_log.duplicate_log_references.count).to eq(1)
expect(lettings_log_without_duplicates.duplicates.count).to eq(0)
expect(lettings_log_without_duplicates.duplicate_log_references).to be_empty
expect(lettings_log.duplicate_log_references.count).to eq(1)
expect(lettings_log_without_duplicates.duplicate_set_id).to be_nil
expect(lettings_log.duplicate_set_id).to eq(duplicate_lettings_log.duplicate_set_id)
expect(lettings_log.duplicate_set_id).to eq(second_duplicate_lettings_log.duplicate_set_id)
end
it "does not create the references twice" do
expect(DuplicateLogReference.count).to eq(0)
task.invoke
expect(DuplicateLogReference.count).to eq(3)
task.reenable
task.invoke
expect(DuplicateLogReference.count).to eq(3)
end
end
context "and there are lettings duplicates in multiple organisations" do
@ -209,41 +162,37 @@ RSpec.describe "set_duplicate_references" do
let!(:other_lettings_log_without_duplicates) { create(:lettings_log, created_by: other_user) }
it "creates separate duplicate references for lettings logs" do
expect(DuplicateLogReference.count).to eq(0)
expect(lettings_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_log_references).to be_empty
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_lettings_log.duplicates.count).to eq(0)
expect(duplicate_lettings_log.duplicate_log_references).to be_empty
expect(duplicate_lettings_log.duplicate_set_id).to be_nil
expect(lettings_log_without_duplicates.duplicates.count).to eq(0)
expect(lettings_log_without_duplicates.duplicate_log_references).to be_empty
expect(lettings_log_without_duplicates.duplicate_set_id).to be_nil
expect(other_lettings_log.duplicates.count).to eq(0)
expect(other_lettings_log.duplicate_log_references).to be_empty
expect(other_lettings_log.duplicate_set_id).to be_nil
expect(other_duplicate_lettings_log.duplicates.count).to eq(0)
expect(other_duplicate_lettings_log.duplicate_log_references).to be_empty
expect(other_duplicate_lettings_log.duplicate_set_id).to be_nil
expect(other_lettings_log_without_duplicates.duplicates.count).to eq(0)
expect(other_lettings_log_without_duplicates.duplicate_log_references).to be_empty
expect(other_lettings_log_without_duplicates.duplicate_set_id).to be_nil
task.invoke
lettings_log.reload
duplicate_lettings_log.reload
lettings_log_without_duplicates.reload
other_lettings_log.reload
other_duplicate_lettings_log.reload
expect(DuplicateLogReference.count).to eq(4)
expect(lettings_log.duplicates.count).to eq(1)
expect(lettings_log.duplicate_log_references.count).to eq(1)
expect(duplicate_lettings_log.duplicates.count).to eq(1)
expect(duplicate_lettings_log.duplicate_log_references.count).to eq(1)
expect(lettings_log_without_duplicates.duplicates.count).to eq(0)
expect(lettings_log_without_duplicates.duplicate_log_references).to be_empty
expect(lettings_log_without_duplicates.duplicate_set_id).to be_nil
expect(lettings_log.duplicate_set_id).to eq(duplicate_lettings_log.duplicate_set_id)
expect(other_lettings_log.duplicates.count).to eq(1)
expect(other_lettings_log.duplicate_log_references.count).to eq(1)
expect(other_duplicate_lettings_log.duplicates.count).to eq(1)
expect(other_duplicate_lettings_log.duplicate_log_references.count).to eq(1)
expect(other_lettings_log_without_duplicates.duplicates.count).to eq(0)
expect(other_lettings_log_without_duplicates.duplicate_log_references).to be_empty
expect(other_lettings_log_without_duplicates.duplicate_set_id).to be_nil
expect(other_lettings_log.duplicate_set_id).to eq(other_duplicate_lettings_log.duplicate_set_id)
expect(other_lettings_log.duplicate_set_id).not_to eq(lettings_log.duplicate_set_id)
end
@ -262,21 +211,19 @@ RSpec.describe "set_duplicate_references" do
end
it "does not create duplicate references for lettings logs" do
expect(DuplicateLogReference.count).to eq(0)
expect(lettings_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_log_references).to be_empty
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_lettings_log.duplicates.count).to eq(0)
expect(duplicate_lettings_log.duplicate_log_references).to be_empty
expect(duplicate_lettings_log.duplicate_set_id).to be_nil
task.invoke
lettings_log.reload
duplicate_lettings_log.reload
expect(DuplicateLogReference.count).to eq(0)
expect(lettings_log.duplicates.count).to eq(0)
expect(lettings_log.duplicate_log_references).to be_empty
expect(lettings_log.duplicate_set_id).to be_nil
expect(duplicate_lettings_log.duplicates.count).to eq(0)
expect(duplicate_lettings_log.duplicate_log_references).to be_empty
expect(duplicate_lettings_log.duplicate_set_id).to be_nil
end
end
end

89
spec/models/duplicate_log_reference_spec.rb

@ -1,89 +0,0 @@
require "rails_helper"
RSpec.describe DuplicateLogReference, type: :model do
context "when adding a new duplicate log" do
context "and duplicate_set_id is not given" do
let(:sales_log) { create(:sales_log) }
it "generates a new random duplicate_set_id" do
duplicate_log = described_class.create!(log_id: sales_log.id, log_type: "SalesLog")
expect(duplicate_log.duplicate_set_id).to be_a(Integer)
end
end
context "and duplicate_set_id is given" do
let(:sales_log) { create(:sales_log) }
it "adds correct duplicate_set_id" do
duplicate_log = described_class.create!(log_id: sales_log.id, log_type: "SalesLog", duplicate_set_id: 123_456)
expect(duplicate_log.duplicate_set_id).to eq(123_456)
end
end
context "and log does not exist" do
it "raises an error" do
expect { described_class.create!(log_id: 1, log_type: "SalesLog") }.to raise_error(ActiveRecord::RecordInvalid)
end
end
context "and log_type is invalid" do
let(:sales_log) { create(:sales_log) }
it "raises an error" do
expect { described_class.create!(log_id: sales_log.id, log_type: "SomethingElse") }.to raise_error(NameError)
end
end
end
context "when accessing all duplicates for a sales log" do
let(:sales_log) { create(:sales_log) }
context "and there are no duplicates" do
it "returns an empty array" do
expect(sales_log.duplicates).to eq([])
end
end
context "and there are duplicates" do
let(:other_sales_log) { create(:sales_log) }
before do
duplicate_log = described_class.create!(log_id: sales_log.id, log_type: "SalesLog")
described_class.create!(log_id: other_sales_log.id, log_type: "SalesLog", duplicate_set_id: duplicate_log.duplicate_set_id)
create(:sales_log)
create(:sales_log)
end
it "returns the correct duplicates" do
expect(sales_log.duplicates.count).to eq(1)
expect(sales_log.duplicates).to include(other_sales_log)
end
end
end
context "when accessing all duplicates for a lettings log" do
let(:lettings_log) { create(:lettings_log) }
context "and there are no duplicates" do
it "returns an empty array" do
expect(lettings_log.duplicates).to eq([])
end
end
context "and there are duplicates" do
let(:other_lettings_log) { create(:lettings_log) }
before do
duplicate_log = described_class.create!(log_id: lettings_log.id, log_type: "LettingsLog")
described_class.create!(log_id: other_lettings_log.id, log_type: "LettingsLog", duplicate_set_id: duplicate_log.duplicate_set_id)
create(:lettings_log)
create(:lettings_log)
end
it "returns the correct duplicates" do
expect(lettings_log.duplicates.count).to eq(1)
expect(lettings_log.duplicates).to include(other_lettings_log)
end
end
end
end

28
spec/requests/form_controller_spec.rb

@ -685,7 +685,7 @@ RSpec.describe FormController, type: :request do
end
context "and duplicate logs" do
let(:duplicate_logs) { create_list(:sales_log, 2) }
let!(:duplicate_logs) { create_list(:sales_log, 2) }
before do
allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs)
@ -882,8 +882,8 @@ RSpec.describe FormController, type: :request do
end
context "when the question was accessed from a duplicate logs screen" do
let(:lettings_log) { create(:lettings_log, :duplicate, created_by: user) }
let(:duplicate_log) { create(:lettings_log, :duplicate, created_by: user) }
let(:lettings_log) { create(:lettings_log, :duplicate, created_by: user, duplicate_set_id: 1) }
let(:duplicate_log) { create(:lettings_log, :duplicate, created_by: user, duplicate_set_id: 1) }
let(:referrer) { "/lettings-logs/#{lettings_log.id}/lead-tenant-age?referrer=duplicate_logs&first_remaining_duplicate_id=#{duplicate_log.id}&original_log_id=#{lettings_log.id}" }
let(:params) do
{
@ -897,14 +897,13 @@ RSpec.describe FormController, type: :request do
end
before do
duplicate_set_id = create(:duplicate_log_reference, log_id: lettings_log.id, log_type: "LettingsLog").duplicate_set_id
create(:duplicate_log_reference, log_id: duplicate_log.id, log_type: "LettingsLog", duplicate_set_id:)
post "/lettings-logs/#{lettings_log.id}/lead-tenant-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer })
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to redirect_to("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
expect(DuplicateLogReference.count).to eq(0)
expect(lettings_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
end
context "and the answer didn't change" do
@ -921,14 +920,15 @@ RSpec.describe FormController, type: :request do
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
expect(DuplicateLogReference.count).to eq(2)
expect(lettings_log.duplicates.count).to eq(1)
expect(duplicate_log.duplicates.count).to eq(1)
end
end
end
context "when the sales question was accessed from a duplicate logs screen" do
let(:sales_log) { create(:sales_log, :duplicate, created_by: user) }
let(:duplicate_log) { create(:sales_log, :duplicate, created_by: user) }
let!(:sales_log) { create(:sales_log, :duplicate, created_by: user, duplicate_set_id: 1) }
let!(:duplicate_log) { create(:sales_log, :duplicate, created_by: user, duplicate_set_id: 1) }
let(:referrer) { "/sales-logs/#{sales_log.id}/buyer-1-age?referrer=duplicate_logs&first_remaining_duplicate_id=#{duplicate_log.id}&original_log_id=#{sales_log.id}&referrer=duplicate_logs" }
let(:params) do
{
@ -942,14 +942,15 @@ RSpec.describe FormController, type: :request do
end
before do
duplicate_set_id = create(:duplicate_log_reference, log_id: sales_log.id, log_type: "SalesLog").duplicate_set_id
create(:duplicate_log_reference, log_id: duplicate_log.id, log_type: "SalesLog", duplicate_set_id:)
post "/sales-logs/#{sales_log.id}/buyer-1-age", params:, headers: headers.merge({ "HTTP_REFERER" => referrer })
end
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to redirect_to("/sales-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
expect(DuplicateLogReference.count).to eq(0)
sales_log.reload
duplicate_log.reload
expect(sales_log.duplicates.count).to eq(0)
expect(duplicate_log.duplicates.count).to eq(0)
end
context "and the answer didn't change" do
@ -966,7 +967,8 @@ RSpec.describe FormController, type: :request do
it "redirects back to the duplicates page for remaining duplicates" do
expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
expect(DuplicateLogReference.count).to eq(2)
expect(sales_log.duplicates.count).to eq(1)
expect(duplicate_log.duplicates.count).to eq(1)
end
end
end

4
spec/services/csv/lettings_log_csv_service_spec.rb

@ -171,7 +171,7 @@ RSpec.describe Csv::LettingsLogCsvService do
context "when the log has a duplicate log reference" do
before do
DuplicateLogReference.create!(log_id: log.id, log_type: "LettingsLog", duplicate_set_id: 12_312)
log.update!(duplicate_set_id: 12_312)
end
it "exports the id for under the heading 'duplicate_set_id'" do
@ -256,7 +256,7 @@ RSpec.describe Csv::LettingsLogCsvService do
context "when the log has a duplicate log reference" do
before do
DuplicateLogReference.create!(log_id: log.id, log_type: "LettingsLog", duplicate_set_id: 12_312)
log.update!(duplicate_set_id: 12_312)
end
it "exports the id for under the heading 'duplicate_set_id'" do

4
spec/services/csv/sales_log_csv_service_spec.rb

@ -166,7 +166,7 @@ RSpec.describe Csv::SalesLogCsvService do
context "when the log has a duplicate log reference" do
before do
DuplicateLogReference.create!(log_id: log.id, log_type: "SalesLog", duplicate_set_id: 12_312)
log.update!(duplicate_set_id: 12_312)
end
it "exports the id for under the heading 'duplicate_set_id'" do
@ -226,7 +226,7 @@ RSpec.describe Csv::SalesLogCsvService do
context "when the log has a duplicate log reference" do
before do
DuplicateLogReference.create!(log_id: log.id, log_type: "SalesLog", duplicate_set_id: 12_312)
log.update!(duplicate_set_id: 12_312)
end
it "exports the id for under the heading 'duplicate_set_id'" do

6
spec/services/exports/lettings_log_export_service_spec.rb

@ -415,11 +415,7 @@ RSpec.describe Exports::LettingsLogExportService do
end
context "and one lettings log with duplicate reference is available for export" do
let!(:lettings_log) { FactoryBot.create(:lettings_log, :completed, created_by: user, propcode: "123", ppostcode_full: "SE2 6RT", postcode_full: "NW1 5TY", tenancycode: "BZ737", startdate: Time.zone.local(2022, 2, 2, 10, 36, 49), voiddate: Time.zone.local(2019, 11, 3), mrcdate: Time.zone.local(2020, 5, 5, 10, 36, 49), tenancylength: 5, underoccupation_benefitcap: 4) }
before do
FactoryBot.create(:duplicate_log_reference, log_type: "LettingsLog", log_id: lettings_log.id, duplicate_set_id: 123)
end
let!(:lettings_log) { FactoryBot.create(:lettings_log, :completed, created_by: user, propcode: "123", ppostcode_full: "SE2 6RT", postcode_full: "NW1 5TY", tenancycode: "BZ737", startdate: Time.zone.local(2022, 2, 2, 10, 36, 49), voiddate: Time.zone.local(2019, 11, 3), mrcdate: Time.zone.local(2020, 5, 5, 10, 36, 49), tenancylength: 5, underoccupation_benefitcap: 4, duplicate_set_id: 123) }
def replace_duplicate_set_id(export_file)
export_file.sub!("<duplicate_set_id/>", "<duplicate_set_id>123</duplicate_set_id>")

Loading…
Cancel
Save