Schemes are attached to the organisation that owns the property. Check you have correctly answered question 1 "Which organisation owns this property?"
If your organisation’s schemes were migrated from old CORE, they may have new names and codes. Search by postcode to find your scheme.
-
<%= govuk_link_to("View your organisation’s schemes", schemes_path) %>
+
<%= govuk_link_to("View your organisation’s schemes", clear_filters_url(filter_type: "schemes")) %>
<%= govuk_link_to("Read more about how schemes have changed", scheme_changes_path) %>
<% end %>
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index da80693ec..2d43e9b02 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -99,6 +99,20 @@ RSpec.describe "Schemes scheme Features" do
expect(page).not_to have_link("Clear", href: "/clear-filters?filter_type=schemes")
end
end
+
+ context "when on the scheme question page" do
+ let(:lettings_log) { FactoryBot.create(:lettings_log, assigned_to: user, needstype: 2) }
+
+ it "open from guidance page with filters cleared" do
+ expect(page).to have_text("2 filters applied")
+ visit("/lettings-logs/#{lettings_log.id}/scheme")
+ find(".govuk-details__summary").click
+ expect(page).to have_link("View your organisation’s schemes")
+ click_link "View your organisation’s schemes"
+ expect(page).to have_current_path("/organisations/#{user.organisation.id}/schemes")
+ expect(page).to have_text("No filters applied")
+ end
+ end
end
end
end
From 72cbc7d6fd2dab86f7f1ba67662307b207c406db Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Fri, 2 Aug 2024 14:31:17 +0100
Subject: [PATCH 02/10] Update rexml to 3.3.3 (#2544)
---
Gemfile.lock | 2 +-
docs/Gemfile.lock | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Gemfile.lock b/Gemfile.lock
index c8d1edd1a..7fdef3710 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -379,7 +379,7 @@ GEM
responders (3.1.1)
actionpack (>= 5.2)
railties (>= 5.2)
- rexml (3.3.2)
+ rexml (3.3.3)
strscan
roo (2.10.1)
nokogiri (~> 1)
diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock
index ee982b784..849732bc5 100644
--- a/docs/Gemfile.lock
+++ b/docs/Gemfile.lock
@@ -226,7 +226,7 @@ GEM
rb-fsevent (0.11.2)
rb-inotify (0.10.1)
ffi (~> 1.0)
- rexml (3.3.2)
+ rexml (3.3.3)
strscan
rouge (3.26.0)
ruby2_keywords (0.0.5)
From 6a42978fabc51d9cfcc98c5a5757f6a94bd9df22 Mon Sep 17 00:00:00 2001
From: Rachael Booth
Date: Fri, 2 Aug 2024 17:31:28 +0100
Subject: [PATCH 03/10] CLDC-3546: Add error to managing org when it does not
have relevant rent period (#2538)
* CLDC-3546: Add error to managing org when it does not have relevant rent period
* Add test for :skip_bu_error
* Fix managing org question id
---
app/models/validations/financial_validations.rb | 7 ++++++-
.../bulk_upload/lettings/year2024/row_parser.rb | 1 +
config/locales/en.yml | 4 +++-
.../models/validations/financial_validations_spec.rb | 8 +++++++-
.../bulk_upload/lettings/year2024/row_parser_spec.rb | 12 ++++++++++++
5 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb
index 5e7d92d2c..53e50a92f 100644
--- a/app/models/validations/financial_validations.rb
+++ b/app/models/validations/financial_validations.rb
@@ -149,7 +149,12 @@ module Validations::FinancialValidations
unless record.managing_organisation.rent_periods.include? record.period
record.errors.add :period, :wrong_rent_period, message: I18n.t(
- "validations.financial.rent_period.invalid_for_org",
+ "validations.financial.rent_period.invalid_for_org.period",
+ org_name: record.managing_organisation.name,
+ rent_period: record.form.get_question("period", record).label_from_value(record.period).downcase,
+ )
+ record.errors.add :managing_organisation_id, :skip_bu_error, message: I18n.t(
+ "validations.financial.rent_period.invalid_for_org.managing_org",
org_name: record.managing_organisation.name,
rent_period: record.form.get_question("period", record).label_from_value(record.period).downcase,
)
diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb
index b912c167b..2e54f5c3f 100644
--- a/app/services/bulk_upload/lettings/year2024/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb
@@ -467,6 +467,7 @@ class BulkUpload::Lettings::Year2024::RowParser
fields.each do |field|
next if errors.include?(field)
+ next if error.type == :skip_bu_error
question = log.form.get_question(error.attribute, log)
diff --git a/config/locales/en.yml b/config/locales/en.yml
index a70e7082b..b6ba761f2 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -436,7 +436,9 @@ en:
under_10: "Enter a total charge that is at least £10.00 per week"
less_than_shortfall: "The total charge must be more than the outstanding amount"
rent_period:
- invalid_for_org: "%{org_name} does not use %{rent_period} as a rent period. Choose another rent period, or a data coordinator can add rent periods to your organisation"
+ invalid_for_org:
+ period: "%{org_name} does not use %{rent_period} as a rent period. Choose another rent period, or a data coordinator can add rent periods to your organisation"
+ managing_org: "%{org_name} does not use %{rent_period} as a rent period. Set another rent period on this log, or a data coordinator can add rent periods to this organisation"
carehome:
out_of_range: "Household rent and other charges must be between %{min_chcharge} and %{max_chcharge} if paying %{period}"
not_provided: "Enter how much rent and other charges the household pays %{period}"
diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb
index c743381f7..a6eefa708 100644
--- a/spec/models/validations/financial_validations_spec.rb
+++ b/spec/models/validations/financial_validations_spec.rb
@@ -165,7 +165,13 @@ RSpec.describe Validations::FinancialValidations do
financial_validator.validate_rent_period(record)
expect(record.errors["period"])
.to include(match I18n.t(
- "validations.financial.rent_period.invalid_for_org",
+ "validations.financial.rent_period.invalid_for_org.period",
+ org_name: user.organisation.name,
+ rent_period: "every 4 weeks",
+ ))
+ expect(record.errors["managing_organisation_id"])
+ .to include(match I18n.t(
+ "validations.financial.rent_period.invalid_for_org.managing_org",
org_name: user.organisation.name,
rent_period: "every 4 weeks",
))
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 fbff000fd..7cae5eeae 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -750,6 +750,18 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
expect(parser.errors[:field_15]).to eq(["You must answer tenant has seen the privacy notice"])
end
end
+
+ context "when there is a :skip_bu_error error" do
+ let(:managing_org) { create(:organisation, :with_old_visible_id, rent_periods: [4, 1]) }
+ let(:attributes) { valid_attributes.merge({ field_123: 3, field_128: 80 }) }
+
+ it "does not add that error" do
+ parser.valid?
+
+ expect(parser.log.errors.map(&:attribute).sort).to eql(%i[managing_organisation_id period])
+ expect(parser.errors.map(&:attribute)).to eql(%i[field_123])
+ end
+ end
end
describe "#validate_nulls" do
From e66d5ea1d0c380c7f7a550becec43b96518df762 Mon Sep 17 00:00:00 2001
From: Rachael Booth
Date: Fri, 2 Aug 2024 17:46:29 +0100
Subject: [PATCH 04/10] CLDC-3566: Clear duplicate set ids when a log ceases to
be a duplicate through normal update flow (#2534)
* CLDC-3566: Clear duplicate set ids when a log ceases to be a duplicate in normal updates
* Refactor
* Ensure post not run twice in form controller tests
* Remove deduplication flow feature toggle
* Ensure class_name is defined in redirect path function
* Add tests for sales logs
---
app/controllers/form_controller.rb | 74 +++---
app/services/feature_toggle.rb | 4 -
spec/requests/form_controller_spec.rb | 311 +++++++++++++++++++++++---
3 files changed, 311 insertions(+), 78 deletions(-)
diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb
index d84f8642a..8ed367e93 100644
--- a/app/controllers/form_controller.rb
+++ b/app/controllers/form_controller.rb
@@ -26,6 +26,8 @@ class FormController < ApplicationController
flash[:notice] = "You have successfully updated #{updated_question_string}"
end
+ update_duplication_tracking
+
pages_requiring_update = pages_requiring_update(shown_page_ids_with_unanswered_questions_before_update)
redirect_to(successful_redirect_path(pages_requiring_update))
else
@@ -192,24 +194,43 @@ private
params[@log.model_name.param_key]["interruption_page_referrer_type"].presence
end
- def successful_redirect_path(pages_to_check)
- if FeatureToggle.deduplication_flow_enabled?
- if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
- return correcting_duplicate_logs_redirect_path
+ def update_duplication_tracking
+ class_name = @log.class.name.underscore
+ dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log)
+
+ if dynamic_duplicates.any?
+ saved_duplicates = @log.duplicates
+ if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates)
+ duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
+ update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
+ saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
end
+ else
+ remove_fixed_duplicate_set_ids(@log)
+ end
+ end
+
+ def successful_redirect_path(pages_to_check)
+ class_name = @log.class.name.underscore
+
+ if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
+ original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
- dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log)
- if dynamic_duplicates.any?
- saved_duplicates = @log.duplicates
- if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates)
- duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
- update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
- saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
+ if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any?
+ if @log.duplicates.none?
+ flash[:notice] = deduplication_success_banner
end
- return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
+ return send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id])
+ else
+ flash[:notice] = deduplication_success_banner
+ return 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
end
+ if @log.duplicates.any?
+ return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
+ end
+
if is_referrer_type?("check_answers")
next_page_id = form.next_page_id(@page, @log, current_user)
next_page = form.get_page(next_page_id)
@@ -315,35 +336,6 @@ private
CONFIRMATION_PAGE_IDS = %w[uprn_confirmation uprn_selection].freeze
- def correcting_duplicate_logs_redirect_path
- class_name = @log.class.name.underscore
-
- original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
- dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log)
-
- if dynamic_duplicates.any?
- saved_duplicates = @log.duplicates
- if duplicates_changed?(dynamic_duplicates, saved_duplicates)
- duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
- update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
- saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
- end
- else
- remove_fixed_duplicate_set_ids(@log)
- end
-
- if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any?
- if dynamic_duplicates.none?
- 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_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
- end
-
def deduplication_success_banner
deduplicated_log_link = "Log #{@log.id}"
changed_labels = {
diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index 810bfb845..91989ff86 100644
--- a/app/services/feature_toggle.rb
+++ b/app/services/feature_toggle.rb
@@ -11,10 +11,6 @@ class FeatureToggle
!Rails.env.development?
end
- def self.deduplication_flow_enabled?
- true
- end
-
def self.duplicate_summary_enabled?
true
end
diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb
index 020dba3a4..80157e992 100644
--- a/spec/requests/form_controller_spec.rb
+++ b/spec/requests/form_controller_spec.rb
@@ -763,39 +763,162 @@ RSpec.describe FormController, type: :request do
}
end
- before do
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
+ context "when the log will not be a duplicate" do
+ before do
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
- it "re-renders the same page with errors if validation fails" do
- expect(response).to have_http_status(:redirect)
- end
+ it "re-renders the same page with errors if validation fails" do
+ expect(response).to have_http_status(:redirect)
+ end
- it "only updates answers that apply to the page being submitted" do
- lettings_log.reload
- expect(lettings_log.age1).to eq(answer)
- expect(lettings_log.age2).to be nil
+ it "only updates answers that apply to the page being submitted" do
+ lettings_log.reload
+ expect(lettings_log.age1).to eq(answer)
+ expect(lettings_log.age2).to be nil
+ end
+
+ it "tracks who updated the record" do
+ lettings_log.reload
+ whodunnit_actor = lettings_log.versions.last.actor
+ expect(whodunnit_actor).to be_a(User)
+ expect(whodunnit_actor.id).to eq(user.id)
+ end
end
- it "tracks who updated the record" do
- lettings_log.reload
- whodunnit_actor = lettings_log.versions.last.actor
- expect(whodunnit_actor).to be_a(User)
- expect(whodunnit_actor.id).to eq(user.id)
+ context "when the answer makes the log a duplicate" do
+ context "with one other log" do
+ let(:new_duplicate) { create(:lettings_log) }
+
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "sets a new duplicate set id on both logs" do
+ lettings_log.reload
+ new_duplicate.reload
+ expect(lettings_log.duplicate_set_id).not_to be_nil
+ expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+ end
+
+ it "redirects to the duplicate logs page" do
+ expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
+ follow_redirect!
+ expect(page).to have_content("These logs are duplicates")
+ end
+ end
+
+ context "with a set of other logs" do
+ let(:duplicate_set_id) { 100 }
+ let(:new_duplicates) { create_list(:lettings_log, 2, duplicate_set_id:) }
+
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicates.pluck(:id)))
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "sets the logs duplicate set id to that of the set it is now part of" do
+ lettings_log.reload
+ expect(lettings_log.duplicate_set_id).to eql(duplicate_set_id)
+ new_duplicates.each do |log|
+ log.reload
+ expect(log.duplicate_set_id).to eql(duplicate_set_id)
+ end
+ end
+
+ it "redirects to the duplicate logs page" do
+ expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
+ follow_redirect!
+ expect(page).to have_content("These logs are duplicates")
+ end
+ end
+
+ context "when the log was previously in a different duplicate set" do
+ context "with a single other log" do
+ let(:old_duplicate_set_id) { 110 }
+ let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
+ let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+ let(:new_duplicate) { create(:lettings_log) }
+
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ lettings_log.reload
+ old_duplicate.reload
+ new_duplicate.reload
+ expect(old_duplicate.duplicate_set_id).to be_nil
+ expect(lettings_log.duplicate_set_id).not_to be_nil
+ expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+ end
+ end
+
+ context "with multiple other logs" do
+ let(:old_duplicate_set_id) { 120 }
+ let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
+ let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+ let(:new_duplicate) { create(:lettings_log) }
+
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ lettings_log.reload
+ new_duplicate.reload
+ old_duplicates.each do |log|
+ log.reload
+ expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+ end
+ expect(lettings_log.duplicate_set_id).not_to be_nil
+ expect(lettings_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
+ expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+ end
+ end
+ end
end
- context "and duplicate logs" do
- let(:duplicate_logs) { create_list(:lettings_log, 2) }
+ context "when the answer makes the log stop being a duplicate" do
+ context "when the log had one duplicate" do
+ let(:old_duplicate_set_id) { 130 }
+ let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
+ let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs)
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ lettings_log.reload
+ old_duplicate.reload
+ expect(old_duplicate.duplicate_set_id).to be_nil
+ expect(lettings_log.duplicate_set_id).to be_nil
+ end
end
- it "redirects to the duplicate logs page" do
- expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
- follow_redirect!
- expect(page).to have_content("These logs are duplicates")
+ context "when the log had multiple duplicates" do
+ let(:old_duplicate_set_id) { 140 }
+ let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
+ let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ lettings_log.reload
+ old_duplicates.each do |log|
+ log.reload
+ expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+ end
+ expect(lettings_log.duplicate_set_id).to be_nil
+ end
end
end
end
@@ -816,19 +939,141 @@ RSpec.describe FormController, type: :request do
},
}
end
+ let(:page_id) { "buyer_1_age" }
- context "and duplicate logs" do
- let!(:duplicate_logs) { create_list(:sales_log, 2) }
+ context "when the answer makes the log a duplicate" do
+ context "with one other log" do
+ let(:new_duplicate) { create(:sales_log) }
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs)
- post "/sales-logs/#{sales_log.id}/buyer-1-age", params:
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
+ post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "sets a new duplicate set id on both logs" do
+ sales_log.reload
+ new_duplicate.reload
+ expect(sales_log.duplicate_set_id).not_to be_nil
+ expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+ end
+
+ it "redirects to the duplicate logs page" do
+ expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
+ follow_redirect!
+ expect(page).to have_content("These logs are duplicates")
+ end
end
- it "redirects to the duplicate logs page" do
- expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
- follow_redirect!
- expect(page).to have_content("These logs are duplicates")
+ context "with a set of other logs" do
+ let(:duplicate_set_id) { 100 }
+ let(:new_duplicates) { create_list(:sales_log, 2, duplicate_set_id:) }
+
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicates.pluck(:id)))
+ post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "sets the logs duplicate set id to that of the set it is now part of" do
+ sales_log.reload
+ expect(sales_log.duplicate_set_id).to eql(duplicate_set_id)
+ new_duplicates.each do |log|
+ log.reload
+ expect(log.duplicate_set_id).to eql(duplicate_set_id)
+ end
+ end
+
+ it "redirects to the duplicate logs page" do
+ expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
+ follow_redirect!
+ expect(page).to have_content("These logs are duplicates")
+ end
+ end
+
+ context "when the log was previously in a different duplicate set" do
+ context "with a single other log" do
+ let(:old_duplicate_set_id) { 110 }
+ let!(:old_duplicate) { create(:sales_log, duplicate_set_id: old_duplicate_set_id) }
+ let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+ let(:new_duplicate) { create(:sales_log) }
+
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
+ post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ sales_log.reload
+ old_duplicate.reload
+ new_duplicate.reload
+ expect(old_duplicate.duplicate_set_id).to be_nil
+ expect(sales_log.duplicate_set_id).not_to be_nil
+ expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+ end
+ end
+
+ context "with multiple other logs" do
+ let(:old_duplicate_set_id) { 120 }
+ let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) }
+ let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+ let(:new_duplicate) { create(:sales_log) }
+
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
+ post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ sales_log.reload
+ new_duplicate.reload
+ old_duplicates.each do |log|
+ log.reload
+ expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+ end
+ expect(sales_log.duplicate_set_id).not_to be_nil
+ expect(sales_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
+ expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
+ end
+ end
+ end
+ end
+
+ context "when the answer makes the log stop being a duplicate" do
+ context "when the log had one duplicate" do
+ let(:old_duplicate_set_id) { 130 }
+ let!(:old_duplicate) { create(:sales_log, duplicate_set_id: old_duplicate_set_id) }
+ let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
+ post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ sales_log.reload
+ old_duplicate.reload
+ expect(old_duplicate.duplicate_set_id).to be_nil
+ expect(sales_log.duplicate_set_id).to be_nil
+ end
+ end
+
+ context "when the log had multiple duplicates" do
+ let(:old_duplicate_set_id) { 140 }
+ let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) }
+ let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
+ post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
+ end
+
+ it "updates the relevant duplicate set ids" do
+ sales_log.reload
+ old_duplicates.each do |log|
+ log.reload
+ expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
+ end
+ expect(sales_log.duplicate_set_id).to be_nil
+ end
end
end
end
From c3b85f2b5036b25b87b9261ed1e14ccb5af4c8c3 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Wed, 7 Aug 2024 13:26:40 +0100
Subject: [PATCH 05/10] CLDC-3411: Correct 24/25 bulk upload error messages on
lettings and sales (#2553)
---
app/models/form/question.rb | 3 ++-
.../bulk_upload/lettings/year2024/row_parser.rb | 10 ++++------
config/locales/en.yml | 8 ++++----
.../models/form/lettings/questions/declaration_spec.rb | 4 ++--
.../models/form/sales/questions/privacy_notice_spec.rb | 8 ++++----
.../bulk_upload/lettings/year2023/row_parser_spec.rb | 2 +-
.../bulk_upload/lettings/year2024/row_parser_spec.rb | 2 +-
7 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/app/models/form/question.rb b/app/models/form/question.rb
index 960af8909..1fbced841 100644
--- a/app/models/form/question.rb
+++ b/app/models/form/question.rb
@@ -208,7 +208,8 @@ class Form::Question
end
def unanswered_error_message
- I18n.t("validations.not_answered", question: error_display_label.downcase)
+ question_text = error_display_label.presence || "this question"
+ I18n.t("validations.not_answered", question: question_text.downcase)
end
def suffix_label(log)
diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb
index 2e54f5c3f..e523c11c4 100644
--- a/app/services/bulk_upload/lettings/year2024/row_parser.rb
+++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb
@@ -44,7 +44,7 @@ class BulkUpload::Lettings::Year2024::RowParser
field_39: "If 'Other', what is the type of tenancy?",
field_40: "What is the length of the fixed-term tenancy to the nearest year?",
field_41: "Is this letting sheltered accommodation?",
- field_15: "Has tenant seen the DLUHC privacy notice?",
+ field_15: "Has tenant seen the MHCLG privacy notice?",
field_42: "What is the lead tenant's age?",
field_43: "Which of these best describes the lead tenant's gender identity?",
field_44: "Which of these best describes the lead tenant's ethnic background?",
@@ -806,16 +806,14 @@ private
if setup_question?(question)
fields.each do |field|
- if errors.select { |e| fields.include?(e.attribute) }.none?
- question_text = question.error_display_label.presence || "this question"
- errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase), category: :setup) if field.present?
+ if errors.select { |e| fields.include?(e.attribute) }.none? && field.present?
+ errors.add(field, question.unanswered_error_message, category: :setup)
end
end
else
fields.each do |field|
unless errors.any? { |e| fields.include?(e.attribute) }
- question_text = question.error_display_label.presence || "this question"
- errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase))
+ errors.add(field, question.unanswered_error_message)
end
end
end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index b6ba761f2..4dfe72872 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -589,13 +589,13 @@ en:
declaration:
missing:
- pre_2024: "You must show the MHCLG privacy notice to the tenant before you can submit this log."
- post_2024: "You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log."
+ pre_2024: "You must show the MHCLG privacy notice to the tenant before you can submit this log"
+ post_2024: "You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log"
privacynotice:
missing:
- pre_2024: "You must show the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log."
- post_2024: "You must show or give access to the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log."
+ pre_2024: "You must show the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log"
+ post_2024: "You must show or give access to the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log"
scheme:
toggle_date:
diff --git a/spec/models/form/lettings/questions/declaration_spec.rb b/spec/models/form/lettings/questions/declaration_spec.rb
index ab9bcebef..7e4d47249 100644
--- a/spec/models/form/lettings/questions/declaration_spec.rb
+++ b/spec/models/form/lettings/questions/declaration_spec.rb
@@ -63,7 +63,7 @@ RSpec.describe Form::Lettings::Questions::Declaration, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the tenant before you can submit this log.")
+ expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the tenant before you can submit this log")
end
end
@@ -87,7 +87,7 @@ RSpec.describe Form::Lettings::Questions::Declaration, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log.")
+ expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log")
end
end
end
diff --git a/spec/models/form/sales/questions/privacy_notice_spec.rb b/spec/models/form/sales/questions/privacy_notice_spec.rb
index 4be918c5c..bbaabb6a5 100644
--- a/spec/models/form/sales/questions/privacy_notice_spec.rb
+++ b/spec/models/form/sales/questions/privacy_notice_spec.rb
@@ -60,7 +60,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyer before you can submit this log.")
+ expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyer before you can submit this log")
end
end
@@ -78,7 +78,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyers before you can submit this log.")
+ expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyers before you can submit this log")
end
end
end
@@ -100,7 +100,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyer before you can submit this log.")
+ expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyer before you can submit this log")
end
end
@@ -118,7 +118,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyers before you can submit this log.")
+ expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyers before you can submit this log")
end
end
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 a8cfd0a79..e38396328 100644
--- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb
@@ -686,7 +686,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
it "cannot be nulled" do
parser.valid?
- expect(parser.errors[:field_45]).to eq(["You must show the MHCLG privacy notice to the tenant before you can submit this log."])
+ expect(parser.errors[:field_45]).to eq(["You must show the MHCLG privacy notice to the tenant before you can submit this log"])
end
end
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 7cae5eeae..405bb292d 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -747,7 +747,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
it "cannot be nulled" do
parser.valid?
- expect(parser.errors[:field_15]).to eq(["You must answer tenant has seen the privacy notice"])
+ expect(parser.errors[:field_15]).to eq(["You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log"])
end
end
From 1da04df30fe63890fa69c319d1db646ea4a83957 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Thu, 8 Aug 2024 09:50:47 +0100
Subject: [PATCH 06/10] CLDC-3411 correct 24 25 bulk upload error messages on
lettings and sales (#2556)
* Update error messages and sales parser
---
app/services/bulk_upload/sales/year2024/row_parser.rb | 4 ++--
config/locales/en.yml | 4 ++--
.../form/lettings/questions/declaration_spec.rb | 2 +-
.../form/sales/questions/privacy_notice_spec.rb | 4 ++--
.../bulk_upload/lettings/year2024/row_parser_spec.rb | 2 +-
.../bulk_upload/sales/year2024/row_parser_spec.rb | 11 ++++++++++-
6 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb
index 68f638e17..d39715f30 100644
--- a/app/services/bulk_upload/sales/year2024/row_parser.rb
+++ b/app/services/bulk_upload/sales/year2024/row_parser.rb
@@ -1364,13 +1364,13 @@ private
if setup_question?(question)
fields.each do |field|
unless errors.any? { |e| fields.include?(e.attribute) }
- errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase), category: :setup)
+ errors.add(field, question.unanswered_error_message, category: :setup)
end
end
else
fields.each do |field|
unless errors.any? { |e| fields.include?(e.attribute) }
- errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase))
+ errors.add(field, question.unanswered_error_message)
end
end
end
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4dfe72872..a53b375fe 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -590,12 +590,12 @@ en:
declaration:
missing:
pre_2024: "You must show the MHCLG privacy notice to the tenant before you can submit this log"
- post_2024: "You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log"
+ post_2024: "You must show or give the tenant access to the MHCLG privacy notice before you can submit this log"
privacynotice:
missing:
pre_2024: "You must show the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log"
- post_2024: "You must show or give access to the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log"
+ post_2024: "You must show or give the %{buyer_or_buyers} access to the MHCLG privacy notice before you can submit this log"
scheme:
toggle_date:
diff --git a/spec/models/form/lettings/questions/declaration_spec.rb b/spec/models/form/lettings/questions/declaration_spec.rb
index 7e4d47249..c64e7f3c8 100644
--- a/spec/models/form/lettings/questions/declaration_spec.rb
+++ b/spec/models/form/lettings/questions/declaration_spec.rb
@@ -87,7 +87,7 @@ RSpec.describe Form::Lettings::Questions::Declaration, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log")
+ expect(question.unanswered_error_message).to eq("You must show or give the tenant access to the MHCLG privacy notice before you can submit this log")
end
end
end
diff --git a/spec/models/form/sales/questions/privacy_notice_spec.rb b/spec/models/form/sales/questions/privacy_notice_spec.rb
index bbaabb6a5..9fb81057a 100644
--- a/spec/models/form/sales/questions/privacy_notice_spec.rb
+++ b/spec/models/form/sales/questions/privacy_notice_spec.rb
@@ -100,7 +100,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyer before you can submit this log")
+ expect(question.unanswered_error_message).to eq("You must show or give the buyer access to the MHCLG privacy notice before you can submit this log")
end
end
@@ -118,7 +118,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
end
it "returns correct unanswered_error_message" do
- expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyers before you can submit this log")
+ expect(question.unanswered_error_message).to eq("You must show or give the buyers access to the MHCLG privacy notice before you can submit this log")
end
end
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 405bb292d..25529cc19 100644
--- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb
@@ -747,7 +747,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
it "cannot be nulled" do
parser.valid?
- expect(parser.errors[:field_15]).to eq(["You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log"])
+ expect(parser.errors[:field_15]).to eq(["You must show or give the tenant access to the MHCLG privacy notice before you can submit this log"])
end
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 115995f3e..656ff7a2f 100644
--- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
+++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb
@@ -1042,12 +1042,21 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
describe "#field_18" do # data protection
let(:attributes) { setup_section_params.merge({ field_18: nil }) }
+ before do
+ parser.valid?
+ end
+
context "when not answered" do
it "returns a setup error" do
- parser.valid?
expect(parser.errors.where(:field_18, category: :setup)).to be_present
end
end
+
+ context "when the privacy notice is not accepted" do
+ it "cannot be nulled" do
+ expect(parser.errors[:field_18]).to eq(["You must show or give the buyer access to the MHCLG privacy notice before you can submit this log"])
+ end
+ end
end
[
From 979e2a6ba229ebf14239296644b143af4bddf45f Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 8 Aug 2024 14:28:37 +0100
Subject: [PATCH 07/10] Filter out location questions before checking the
routing (#2557)
---
app/models/lettings_log.rb | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index eab05c997..e7a72350d 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -581,14 +581,22 @@ class LettingsLog < Log
def non_location_setup_questions_completed?
form.setup_sections.all? do |section|
section.subsections.all? do |subsection|
- relevant_qs = subsection.applicable_questions(self).reject { |q| optional_fields.include?(q.id) || %w[scheme_id location_id].include?(q.id) }
- relevant_qs.all? do |question|
+ relevant_qs = subsection.questions.reject { |q| optional_fields.include?(q.id) || %w[scheme_id location_id].include?(q.id) }
+ relevant_applicable_qs = select_applicable_questions(self, relevant_qs)
+ relevant_applicable_qs.all? do |question|
question.completed?(self)
end
end
end
end
+ # this is the same as the subsection method, but only for given questions
+ def select_applicable_questions(log, questions)
+ questions.select do |q|
+ (q.displayed_to_user?(log) && !q.derived?(log)) || q.is_derived_or_has_inferred_check_answers_value?(log)
+ end
+ end
+
def resolve!
update(unresolved: false)
end
From c7bf7a037d451bda8f8ec8e10f313c3f5b19b30d Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Thu, 8 Aug 2024 14:54:39 +0100
Subject: [PATCH 08/10] CLDC-2588-Separate soft and hard validation errors in
bulk upload (#2545)
* Separate soft and hard validation errors in bulk upload
---
.../bulk_upload_error_row_component.html.erb | 50 ++++++++++++++++---
app/frontend/styles/application.scss | 14 ++++++
.../summary.html.erb | 2 +-
.../summary.html.erb | 2 +-
.../bulk_upload_error_row_component_spec.rb | 49 ++++++++++++++++++
.../show.html.erb_spec.rb | 2 +-
.../summary.html.erb_spec.rb | 2 +-
.../show.html.erb_spec.rb | 3 +-
.../summary.html.erb_spec.rb | 2 +-
9 files changed, 112 insertions(+), 14 deletions(-)
diff --git a/app/components/bulk_upload_error_row_component.html.erb b/app/components/bulk_upload_error_row_component.html.erb
index 907c73298..4e9ff1390 100644
--- a/app/components/bulk_upload_error_row_component.html.erb
+++ b/app/components/bulk_upload_error_row_component.html.erb
@@ -1,13 +1,16 @@
<%= govuk_table do |table| %>
<%= table.with_head do |head| %>
<% head.with_row do |row| %>
@@ -18,16 +21,49 @@
<% end %>
<%= table.with_body do |body| %>
- <% bulk_upload_errors.each do |error| %>
+ <% critical_errors.each do |error| %>
<% body.with_row do |row| %>
- <% row.with_cell(header: true, text: error.cell) %>
- <% row.with_cell(text: question_for_field(error.field)) %>
- <% row.with_cell(text: error.error) %>
- <% row.with_cell(text: error.field.humanize) %>
+ <% row.with_cell(text: error.cell) %>
+ <% row.with_cell(text: question_for_field(error.field)) %>
+ <% row.with_cell(text: error.error, html_attributes: { class: "govuk-!-font-weight-bold" }) %>
+ <% row.with_cell(text: error.field.humanize) %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
+
+ <% if potential_errors.any? %>
+
Potential errors
+
The following groups of cells might have conflicting data. Check the answers and fix any incorrect data.
If the answers are correct, fix the critical errors and reupload the file. You'll need to confirm that the following data is correct when the file only contains potential errors.
+ <%= govuk_table do |table| %>
+ <%= table.with_head do |head| %>
+ <% head.with_row do |row| %>
+ <% row.with_cell(header: true, text: "Cell") %>
+ <% row.with_cell(header: true, text: "Question") %>
+ <% row.with_cell(header: true, text: "Potential error") %>
+ <% row.with_cell(header: true, text: "Specification") %>
+ <% end %>
+ <% end %>
+
+ <%= table.with_body do |body| %>
+ <% potential_errors.group_by(&:error).each do |error_message, errors| %>
+ <% errors.each_with_index do |error, index| %>
+ <% row_class = "grouped-rows" %>
+ <% row_class += " first-row" if index.zero? %>
+ <% row_class += " last-row" if index == errors.size - 1 %>
+ <% body.with_row(html_attributes: { class: row_class }) do |row| %>
+ <% row.with_cell(text: error.cell) %>
+ <% row.with_cell(text: question_for_field(error.field)) %>
+ <% if index == 0 %>
+ <% row.with_cell(text: error_message, rowspan: errors.size, html_attributes: { class: "govuk-!-font-weight-bold grouped-multirow-cell" }) %>
+ <% end %>
+ <% row.with_cell(text: error.field.humanize) %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <% end %>
+ <% end %>
Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again
- We could not create logs from your bulk upload. Below is a list of everything that you need to fix your spreadsheet. You can download the <%= govuk_link_to "specification", Forms::BulkUploadLettings::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
+ We could not create logs from your bulk upload because of the following errors. Download the <%= govuk_link_to "specification", Forms::BulkUploadLettings::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again
- We could not create logs from your bulk upload. Below is a list of everything that you need to fix your spreadsheet. You can download the <%= govuk_link_to "specification", Forms::BulkUploadSales::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
+ We could not create logs from your bulk upload because of the following errors. Download the <%= govuk_link_to "specification", Forms::BulkUploadSales::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
diff --git a/spec/components/bulk_upload_error_row_component_spec.rb b/spec/components/bulk_upload_error_row_component_spec.rb
index 01af041cb..b593a0048 100644
--- a/spec/components/bulk_upload_error_row_component_spec.rb
+++ b/spec/components/bulk_upload_error_row_component_spec.rb
@@ -108,4 +108,53 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
expect(result).to have_content(expected)
end
end
+
+ context "when there are potential errors" do
+ let(:row) { rand(9_999) }
+ let(:tenant_code) { SecureRandom.hex(4) }
+ let(:property_ref) { SecureRandom.hex(4) }
+ let(:purchaser_code) { nil }
+ let(:category) { "soft_validation" }
+ let(:field_46) { 40 }
+ let(:field_50) { 5 }
+ let(:error) { "You told us this person is aged 40 years and retired." }
+ let(:bulk_upload) { create(:bulk_upload, :lettings) }
+ let(:bulk_upload_errors) do
+ [
+ FactoryBot.build(
+ :bulk_upload_error,
+ bulk_upload:,
+ row:,
+ tenant_code:,
+ property_ref:,
+ purchaser_code:,
+ field: :field_46,
+ error:,
+ category:,
+ ),
+ FactoryBot.build(
+ :bulk_upload_error,
+ bulk_upload:,
+ row:,
+ tenant_code:,
+ property_ref:,
+ purchaser_code:,
+ field: :field_50,
+ error:,
+ category:,
+ ),
+ ]
+ end
+
+ it "renders the potential errors section" do
+ result = render_inline(described_class.new(bulk_upload_errors:))
+ expect(result).to have_content("Potential errors")
+ end
+
+ it "renders the potential error message" do
+ expected = error
+ result = render_inline(described_class.new(bulk_upload_errors:))
+ expect(result).to have_content(expected, count: 1)
+ end
+ end
end
diff --git a/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb b/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb
index 3dea3ce92..637dfa3a9 100644
--- a/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb
+++ b/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb
@@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_lettings_results/show.html.erb" do
fragment = Capybara::Node::Simple.new(rendered)
- expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+ expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
end
end
end
diff --git a/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb b/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb
index 05e2dce85..ac0c1f82d 100644
--- a/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb
+++ b/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb
@@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_lettings_results/summary.html.erb" do
fragment = Capybara::Node::Simple.new(rendered)
- expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+ expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
end
end
end
diff --git a/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb b/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb
index 785d830b3..dc6751dc8 100644
--- a/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb
+++ b/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb
@@ -32,8 +32,7 @@ RSpec.describe "bulk_upload_sales_results/show.html.erb" do
render
fragment = Capybara::Node::Simple.new(rendered)
-
- expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+ expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
end
end
end
diff --git a/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb b/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb
index 6c00e5adf..b3d9aa006 100644
--- a/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb
+++ b/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb
@@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_sales_results/summary.html.erb" do
fragment = Capybara::Node::Simple.new(rendered)
- expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+ expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
end
end
end
From 53ae216bae9f7d7be5b563fe9f02e8634f61c3a8 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Fri, 9 Aug 2024 15:59:12 +0100
Subject: [PATCH 09/10] Revert "CLDC-3566: Clear duplicate set ids when a log
ceases to be a duplicate through normal update flow (#2534)" (#2563)
This reverts commit e66d5ea1d0c380c7f7a550becec43b96518df762.
---
app/controllers/form_controller.rb | 74 +++---
app/services/feature_toggle.rb | 4 +
spec/requests/form_controller_spec.rb | 311 +++-----------------------
3 files changed, 78 insertions(+), 311 deletions(-)
diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb
index 8ed367e93..d84f8642a 100644
--- a/app/controllers/form_controller.rb
+++ b/app/controllers/form_controller.rb
@@ -26,8 +26,6 @@ class FormController < ApplicationController
flash[:notice] = "You have successfully updated #{updated_question_string}"
end
- update_duplication_tracking
-
pages_requiring_update = pages_requiring_update(shown_page_ids_with_unanswered_questions_before_update)
redirect_to(successful_redirect_path(pages_requiring_update))
else
@@ -194,43 +192,24 @@ private
params[@log.model_name.param_key]["interruption_page_referrer_type"].presence
end
- def update_duplication_tracking
- class_name = @log.class.name.underscore
- dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log)
-
- if dynamic_duplicates.any?
- saved_duplicates = @log.duplicates
- if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates)
- duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
- update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
- saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
- end
- else
- remove_fixed_duplicate_set_ids(@log)
- end
- end
-
def successful_redirect_path(pages_to_check)
- class_name = @log.class.name.underscore
-
- if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
- original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
+ if FeatureToggle.deduplication_flow_enabled?
+ if is_referrer_type?("duplicate_logs") || is_referrer_type?("duplicate_logs_banner")
+ return correcting_duplicate_logs_redirect_path
+ end
- if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any?
- if @log.duplicates.none?
- flash[:notice] = deduplication_success_banner
+ dynamic_duplicates = @log.lettings? ? current_user.lettings_logs.duplicate_logs(@log) : current_user.sales_logs.duplicate_logs(@log)
+ if dynamic_duplicates.any?
+ saved_duplicates = @log.duplicates
+ if saved_duplicates.none? || duplicates_changed?(dynamic_duplicates, saved_duplicates)
+ duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
+ update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
+ saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
end
- return send("#{class_name}_duplicate_logs_path", original_log, original_log_id: original_log.id, referrer: params[:referrer], organisation_id: params[:organisation_id])
- else
- flash[:notice] = deduplication_success_banner
- return 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])
+ return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
end
end
- if @log.duplicates.any?
- return send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: @log.id)
- end
-
if is_referrer_type?("check_answers")
next_page_id = form.next_page_id(@page, @log, current_user)
next_page = form.get_page(next_page_id)
@@ -336,6 +315,35 @@ private
CONFIRMATION_PAGE_IDS = %w[uprn_confirmation uprn_selection].freeze
+ def correcting_duplicate_logs_redirect_path
+ class_name = @log.class.name.underscore
+
+ original_log = current_user.send(class_name.pluralize).find_by(id: from_referrer_query("original_log_id"))
+ dynamic_duplicates = current_user.send(class_name.pluralize).duplicate_logs(@log)
+
+ if dynamic_duplicates.any?
+ saved_duplicates = @log.duplicates
+ if duplicates_changed?(dynamic_duplicates, saved_duplicates)
+ duplicate_set_id = dynamic_duplicates.first.duplicate_set_id || new_duplicate_set_id(@log)
+ update_logs_with_duplicate_set_id(@log, dynamic_duplicates, duplicate_set_id)
+ saved_duplicates.first.update!(duplicate_set_id: nil) if saved_duplicates.count == 1
+ end
+ else
+ remove_fixed_duplicate_set_ids(@log)
+ end
+
+ if original_log.present? && current_user.send(class_name.pluralize).duplicate_logs(original_log).any?
+ if dynamic_duplicates.none?
+ 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_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
+ end
+
def deduplication_success_banner
deduplicated_log_link = "Log #{@log.id}"
changed_labels = {
diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index 91989ff86..810bfb845 100644
--- a/app/services/feature_toggle.rb
+++ b/app/services/feature_toggle.rb
@@ -11,6 +11,10 @@ class FeatureToggle
!Rails.env.development?
end
+ def self.deduplication_flow_enabled?
+ true
+ end
+
def self.duplicate_summary_enabled?
true
end
diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb
index 80157e992..020dba3a4 100644
--- a/spec/requests/form_controller_spec.rb
+++ b/spec/requests/form_controller_spec.rb
@@ -763,162 +763,39 @@ RSpec.describe FormController, type: :request do
}
end
- context "when the log will not be a duplicate" do
- before do
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
-
- it "re-renders the same page with errors if validation fails" do
- expect(response).to have_http_status(:redirect)
- end
-
- it "only updates answers that apply to the page being submitted" do
- lettings_log.reload
- expect(lettings_log.age1).to eq(answer)
- expect(lettings_log.age2).to be nil
- end
-
- it "tracks who updated the record" do
- lettings_log.reload
- whodunnit_actor = lettings_log.versions.last.actor
- expect(whodunnit_actor).to be_a(User)
- expect(whodunnit_actor.id).to eq(user.id)
- end
+ before do
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
end
- context "when the answer makes the log a duplicate" do
- context "with one other log" do
- let(:new_duplicate) { create(:lettings_log) }
-
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
-
- it "sets a new duplicate set id on both logs" do
- lettings_log.reload
- new_duplicate.reload
- expect(lettings_log.duplicate_set_id).not_to be_nil
- expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
- end
-
- it "redirects to the duplicate logs page" do
- expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
- follow_redirect!
- expect(page).to have_content("These logs are duplicates")
- end
- end
-
- context "with a set of other logs" do
- let(:duplicate_set_id) { 100 }
- let(:new_duplicates) { create_list(:lettings_log, 2, duplicate_set_id:) }
-
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicates.pluck(:id)))
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
-
- it "sets the logs duplicate set id to that of the set it is now part of" do
- lettings_log.reload
- expect(lettings_log.duplicate_set_id).to eql(duplicate_set_id)
- new_duplicates.each do |log|
- log.reload
- expect(log.duplicate_set_id).to eql(duplicate_set_id)
- end
- end
-
- it "redirects to the duplicate logs page" do
- expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
- follow_redirect!
- expect(page).to have_content("These logs are duplicates")
- end
- end
-
- context "when the log was previously in a different duplicate set" do
- context "with a single other log" do
- let(:old_duplicate_set_id) { 110 }
- let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
- let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
- let(:new_duplicate) { create(:lettings_log) }
-
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
-
- it "updates the relevant duplicate set ids" do
- lettings_log.reload
- old_duplicate.reload
- new_duplicate.reload
- expect(old_duplicate.duplicate_set_id).to be_nil
- expect(lettings_log.duplicate_set_id).not_to be_nil
- expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
- end
- end
+ it "re-renders the same page with errors if validation fails" do
+ expect(response).to have_http_status(:redirect)
+ end
- context "with multiple other logs" do
- let(:old_duplicate_set_id) { 120 }
- let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
- let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
- let(:new_duplicate) { create(:lettings_log) }
-
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.where(id: new_duplicate.id))
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
-
- it "updates the relevant duplicate set ids" do
- lettings_log.reload
- new_duplicate.reload
- old_duplicates.each do |log|
- log.reload
- expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
- end
- expect(lettings_log.duplicate_set_id).not_to be_nil
- expect(lettings_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
- expect(lettings_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
- end
- end
- end
+ it "only updates answers that apply to the page being submitted" do
+ lettings_log.reload
+ expect(lettings_log.age1).to eq(answer)
+ expect(lettings_log.age2).to be nil
end
- context "when the answer makes the log stop being a duplicate" do
- context "when the log had one duplicate" do
- let(:old_duplicate_set_id) { 130 }
- let!(:old_duplicate) { create(:lettings_log, duplicate_set_id: old_duplicate_set_id) }
- let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
+ it "tracks who updated the record" do
+ lettings_log.reload
+ whodunnit_actor = lettings_log.versions.last.actor
+ expect(whodunnit_actor).to be_a(User)
+ expect(whodunnit_actor.id).to eq(user.id)
+ end
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
+ context "and duplicate logs" do
+ let(:duplicate_logs) { create_list(:lettings_log, 2) }
- it "updates the relevant duplicate set ids" do
- lettings_log.reload
- old_duplicate.reload
- expect(old_duplicate.duplicate_set_id).to be_nil
- expect(lettings_log.duplicate_set_id).to be_nil
- end
+ before do
+ allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs)
+ post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
end
- context "when the log had multiple duplicates" do
- let(:old_duplicate_set_id) { 140 }
- let!(:old_duplicates) { create_list(:lettings_log, 2, duplicate_set_id: old_duplicate_set_id) }
- let(:lettings_log) { create(:lettings_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
-
- before do
- allow(LettingsLog).to receive(:duplicate_logs).and_return(LettingsLog.none)
- post "/lettings-logs/#{lettings_log.id}/#{page_id.dasherize}", params:
- end
-
- it "updates the relevant duplicate set ids" do
- lettings_log.reload
- old_duplicates.each do |log|
- log.reload
- expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
- end
- expect(lettings_log.duplicate_set_id).to be_nil
- end
+ it "redirects to the duplicate logs page" do
+ expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}")
+ follow_redirect!
+ expect(page).to have_content("These logs are duplicates")
end
end
end
@@ -939,141 +816,19 @@ RSpec.describe FormController, type: :request do
},
}
end
- let(:page_id) { "buyer_1_age" }
-
- context "when the answer makes the log a duplicate" do
- context "with one other log" do
- let(:new_duplicate) { create(:sales_log) }
-
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
- post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
- end
-
- it "sets a new duplicate set id on both logs" do
- sales_log.reload
- new_duplicate.reload
- expect(sales_log.duplicate_set_id).not_to be_nil
- expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
- end
-
- it "redirects to the duplicate logs page" do
- expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
- follow_redirect!
- expect(page).to have_content("These logs are duplicates")
- end
- end
-
- context "with a set of other logs" do
- let(:duplicate_set_id) { 100 }
- let(:new_duplicates) { create_list(:sales_log, 2, duplicate_set_id:) }
-
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicates.pluck(:id)))
- post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
- end
-
- it "sets the logs duplicate set id to that of the set it is now part of" do
- sales_log.reload
- expect(sales_log.duplicate_set_id).to eql(duplicate_set_id)
- new_duplicates.each do |log|
- log.reload
- expect(log.duplicate_set_id).to eql(duplicate_set_id)
- end
- end
-
- it "redirects to the duplicate logs page" do
- expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
- follow_redirect!
- expect(page).to have_content("These logs are duplicates")
- end
- end
-
- context "when the log was previously in a different duplicate set" do
- context "with a single other log" do
- let(:old_duplicate_set_id) { 110 }
- let!(:old_duplicate) { create(:sales_log, duplicate_set_id: old_duplicate_set_id) }
- let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
- let(:new_duplicate) { create(:sales_log) }
-
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
- post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
- end
-
- it "updates the relevant duplicate set ids" do
- sales_log.reload
- old_duplicate.reload
- new_duplicate.reload
- expect(old_duplicate.duplicate_set_id).to be_nil
- expect(sales_log.duplicate_set_id).not_to be_nil
- expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
- end
- end
-
- context "with multiple other logs" do
- let(:old_duplicate_set_id) { 120 }
- let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) }
- let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
- let(:new_duplicate) { create(:sales_log) }
-
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.where(id: new_duplicate.id))
- post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
- end
-
- it "updates the relevant duplicate set ids" do
- sales_log.reload
- new_duplicate.reload
- old_duplicates.each do |log|
- log.reload
- expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
- end
- expect(sales_log.duplicate_set_id).not_to be_nil
- expect(sales_log.duplicate_set_id).not_to eql(old_duplicate_set_id)
- expect(sales_log.duplicate_set_id).to eql(new_duplicate.duplicate_set_id)
- end
- end
- end
- end
-
- context "when the answer makes the log stop being a duplicate" do
- context "when the log had one duplicate" do
- let(:old_duplicate_set_id) { 130 }
- let!(:old_duplicate) { create(:sales_log, duplicate_set_id: old_duplicate_set_id) }
- let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
- post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
- end
+ context "and duplicate logs" do
+ let!(:duplicate_logs) { create_list(:sales_log, 2) }
- it "updates the relevant duplicate set ids" do
- sales_log.reload
- old_duplicate.reload
- expect(old_duplicate.duplicate_set_id).to be_nil
- expect(sales_log.duplicate_set_id).to be_nil
- end
+ before do
+ allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs)
+ post "/sales-logs/#{sales_log.id}/buyer-1-age", params:
end
- context "when the log had multiple duplicates" do
- let(:old_duplicate_set_id) { 140 }
- let!(:old_duplicates) { create_list(:sales_log, 2, duplicate_set_id: old_duplicate_set_id) }
- let(:sales_log) { create(:sales_log, assigned_to: user, duplicate_set_id: old_duplicate_set_id) }
-
- before do
- allow(SalesLog).to receive(:duplicate_logs).and_return(SalesLog.none)
- post "/sales-logs/#{sales_log.id}/#{page_id.dasherize}", params:
- end
-
- it "updates the relevant duplicate set ids" do
- sales_log.reload
- old_duplicates.each do |log|
- log.reload
- expect(log.duplicate_set_id).to eql(old_duplicate_set_id)
- end
- expect(sales_log.duplicate_set_id).to be_nil
- end
+ it "redirects to the duplicate logs page" do
+ expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}")
+ follow_redirect!
+ expect(page).to have_content("These logs are duplicates")
end
end
end
From 72890e516e6888c8a0253d4c1c51e2b6cc33717f Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Tue, 13 Aug 2024 12:01:37 +0100
Subject: [PATCH 10/10] CLDC-3564 Update filter search (#2535)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Add search controller
* Update on confirm
* lint
* Update search endpoint
* remove assigned to filter options
* Explicitly define host in the url
* Update search controller
* Allow searching organisations
* Allow filtering by user without js
* Allow filtering by org without js
* Add filter_type to method calls
* Update filter helpers
* Hide text search input when js is enabled
* Some feature test updates
* fix model test
* Delete a random file 👀
* lint
* Update more tests
* Update inner text for filter
* Keep csv filters the same
* Clear free text filters for csv dowloads
* User path helper
* Update scheme filters and log scopes
* Update which users we can filter by
---
app/controllers/organisations_controller.rb | 15 +-
app/controllers/users_controller.rb | 11 +
app/frontend/controllers/index.js | 3 +
app/frontend/controllers/search_controller.js | 46 +++
app/frontend/modules/search.js | 49 +++
app/helpers/filters_helper.rb | 82 ++++-
app/models/lettings_log.rb | 4 +
app/models/log.rb | 3 +
app/models/organisation.rb | 1 +
app/models/user.rb | 5 +-
app/services/filter_manager.rb | 11 +
app/views/filters/_radio_filter.html.erb | 1 +
.../filters/_text_select_filter.html.erb | 20 ++
app/views/filters/assigned_to.html.erb | 3 +-
app/views/filters/managed_by.html.erb | 3 +-
app/views/filters/owned_by.html.erb | 3 +-
app/views/logs/_log_filters.html.erb | 15 +-
app/views/schemes/_scheme_filters.html.erb | 2 +-
config/routes.rb | 8 +
spec/features/lettings_log_spec.rb | 4 +-
spec/features/organisation_spec.rb | 6 +-
spec/features/user_spec.rb | 7 +-
spec/helpers/filters_helper_spec.rb | 298 ++++++++++++++++--
spec/models/user_spec.rb | 6 +-
.../requests/organisations_controller_spec.rb | 45 +++
spec/requests/users_controller_spec.rb | 61 ++++
26 files changed, 643 insertions(+), 69 deletions(-)
create mode 100644 app/frontend/controllers/search_controller.js
create mode 100644 app/views/filters/_text_select_filter.html.erb
diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb
index dfe50fa22..9d3e63b33 100644
--- a/app/controllers/organisations_controller.rb
+++ b/app/controllers/organisations_controller.rb
@@ -4,8 +4,8 @@ class OrganisationsController < ApplicationController
include DuplicateLogsHelper
before_action :authenticate_user!
- before_action :find_resource, except: %i[index new create]
- before_action :authenticate_scope!, except: [:index]
+ before_action :find_resource, except: %i[index new create search]
+ before_action :authenticate_scope!, except: %i[index search]
before_action :session_filters, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_lettings_csv download_lettings_csv email_sales_csv download_sales_csv]
before_action :session_filters, only: %i[users schemes email_schemes_csv download_schemes_csv]
before_action -> { filter_manager.serialize_filters_to_session }, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_lettings_csv download_lettings_csv email_sales_csv download_sales_csv]
@@ -280,6 +280,17 @@ class OrganisationsController < ApplicationController
render "schemes/changes"
end
+ def search
+ org_options = current_user.support? ? Organisation.all : Organisation.affiliated_organisations(current_user.organisation)
+ organisations = org_options.search_by(params["query"]).limit(20)
+
+ org_data = organisations.each_with_object({}) do |org, hash|
+ hash[org.id] = { value: org.name }
+ end
+
+ render json: org_data.to_json
+ end
+
private
def filter_type
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 3fe3b1813..2f7bb2bd6 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -32,6 +32,17 @@ class UsersController < ApplicationController
end
end
+ def search
+ user_options = current_user.support? ? User.all : User.own_and_managing_org_users(current_user.organisation)
+ users = user_options.search_by(params["query"]).limit(20)
+
+ user_data = users.each_with_object({}) do |user, hash|
+ hash[user.id] = { value: user.name, hint: user.email }
+ end
+
+ render json: user_data.to_json
+ end
+
def resend_invite
@user.send_confirmation_instructions
flash[:notice] = "Invitation sent to #{@user.email}"
diff --git a/app/frontend/controllers/index.js b/app/frontend/controllers/index.js
index ece539d15..ef29b99ca 100644
--- a/app/frontend/controllers/index.js
+++ b/app/frontend/controllers/index.js
@@ -13,6 +13,8 @@ import GovukfrontendController from './govukfrontend_controller.js'
import NumericQuestionController from './numeric_question_controller.js'
+import SearchController from './search_controller.js'
+
import FilterLayoutController from './filter_layout_controller.js'
application.register('accessible-autocomplete', AccessibleAutocompleteController)
application.register('conditional-filter', ConditionalFilterController)
@@ -20,3 +22,4 @@ application.register('conditional-question', ConditionalQuestionController)
application.register('govukfrontend', GovukfrontendController)
application.register('numeric-question', NumericQuestionController)
application.register('filter-layout', FilterLayoutController)
+application.register('search', SearchController)
diff --git a/app/frontend/controllers/search_controller.js b/app/frontend/controllers/search_controller.js
new file mode 100644
index 000000000..565b9d0d1
--- /dev/null
+++ b/app/frontend/controllers/search_controller.js
@@ -0,0 +1,46 @@
+import { Controller } from '@hotwired/stimulus'
+import accessibleAutocomplete from 'accessible-autocomplete'
+import 'accessible-autocomplete/dist/accessible-autocomplete.min.css'
+import { searchSuggestion, fetchAndPopulateSearchResults, confirmSelectedOption, searchableName } from '../modules/search'
+
+const options = []
+const populateOptions = (results, selectEl) => {
+ selectEl.innerHTML = ''
+
+ Object.keys(results).forEach((key) => {
+ const option = document.createElement('option')
+ option.value = key
+ option.innerHTML = results[key].value
+ if (results[key].hint) { option.setAttribute('data-hint', results[key].hint) }
+ option.setAttribute('text', searchableName(results[key]))
+ selectEl.appendChild(option)
+ options.push(option)
+ })
+}
+
+export default class extends Controller {
+ connect () {
+ const selectEl = this.element
+ const matches = /^(\w+)\[(\w+)\]$/.exec(selectEl.name)
+ const rawFieldName = matches ? `${matches[1]}[${matches[2]}_raw]` : ''
+ const searchUrl = JSON.parse(this.element.dataset.info).search_url
+
+ document.querySelectorAll('.non-js-text-search-input-field').forEach((el) => {
+ el.style.display = 'none'
+ })
+
+ accessibleAutocomplete.enhanceSelectElement({
+ defaultValue: '',
+ selectElement: selectEl,
+ minLength: 1,
+ source: (query, populateResults) => {
+ fetchAndPopulateSearchResults(query, populateResults, searchUrl, populateOptions, selectEl)
+ },
+ autoselect: true,
+ placeholder: 'Start typing to search',
+ templates: { suggestion: (value) => searchSuggestion(value, options) },
+ name: rawFieldName,
+ onConfirm: (val) => confirmSelectedOption(selectEl, val)
+ })
+ }
+}
diff --git a/app/frontend/modules/search.js b/app/frontend/modules/search.js
index 71944746e..efdf7b9d0 100644
--- a/app/frontend/modules/search.js
+++ b/app/frontend/modules/search.js
@@ -117,6 +117,22 @@ export const suggestion = (value, options) => {
}
}
+export const searchSuggestion = (value, options) => {
+ try {
+ const option = options.find((o) => o.getAttribute('text') === value)
+ if (option) {
+ const result = enhanceOption(option)
+ const html = result.append ? `${result.text}${result.append}` : `${result.text}`
+ return result.hint ? `${html}
${result.hint}
` : html
+ } else {
+ return 'No results found'
+ }
+ } catch (error) {
+ console.error('Error fetching user option:', error)
+ return value
+ }
+}
+
export const enhanceOption = (option) => {
return {
text: option.text,
@@ -128,6 +144,39 @@ export const enhanceOption = (option) => {
}
}
+export const fetchAndPopulateSearchResults = async (query, populateResults, relativeUrlRoute, populateOptions, selectEl) => {
+ if (/\S/.test(query)) {
+ const results = await fetchUserOptions(query, relativeUrlRoute)
+ populateOptions(results, selectEl)
+ populateResults(Object.values(results).map((o) => searchableName(o)))
+ }
+}
+
+export const fetchUserOptions = async (query, searchUrl) => {
+ try {
+ const response = await fetch(`${searchUrl}?query=${encodeURIComponent(query)}`)
+ const results = await response.json()
+ return results
+ } catch (error) {
+ console.error('Error fetching user options:', error)
+ return []
+ }
+}
+
export const getSearchableName = (option) => {
return option.getAttribute('data-hint') ? option.text + ' ' + option.getAttribute('data-hint') : option.text
}
+
+export const searchableName = (option) => {
+ return option.hint ? option.value + ' ' + option.hint : option.value
+}
+
+export const confirmSelectedOption = (selectEl, val) => {
+ const arrayOfOptions = Array.from(selectEl.options).filter(function (option, index, arr) { return option.value !== '' })
+
+ const selectedOption = [].filter.call(
+ arrayOfOptions,
+ (option) => option.getAttribute('text') === val
+ )[0]
+ if (selectedOption) selectedOption.selected = true
+}
diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb
index 38c15b82b..5f8488bc9 100644
--- a/app/helpers/filters_helper.rb
+++ b/app/helpers/filters_helper.rb
@@ -11,8 +11,8 @@ module FiltersHelper
return true if !selected_filters.key?("owning_organisation") && filter == "owning_organisation_select" && value == :all
return true if !selected_filters.key?("managing_organisation") && filter == "managing_organisation_select" && value == :all
- return true if selected_filters["owning_organisation"].present? && filter == "owning_organisation_select" && value == :specific_org
- return true if selected_filters["managing_organisation"].present? && filter == "managing_organisation_select" && value == :specific_org
+ return true if (selected_filters["owning_organisation"].present? || selected_filters["owning_organisation_text_search"].present?) && filter == "owning_organisation_select" && value == :specific_org
+ return true if (selected_filters["managing_organisation"].present? || selected_filters["managing_organisation_text_search"].present?) && filter == "managing_organisation_select" && value == :specific_org
return false if selected_filters[filter].blank?
@@ -84,16 +84,54 @@ module FiltersHelper
JSON.parse(session[session_name_for(filter_type)])[filter] || ""
end
- def owning_organisation_filter_options(user)
+ def all_owning_organisation_filter_options(user)
organisation_options = user.support? ? Organisation.all : ([user.organisation] + user.organisation.stock_owners + user.organisation.absorbed_organisations).uniq
[OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) }
end
- def assigned_to_filter_options(user)
+ def owning_organisation_filter_options(user, filter_type)
+ if applied_filters(filter_type)["owning_organisation"].present?
+ organisation_id = applied_filters(filter_type)["owning_organisation"]
+
+ org = if user.support?
+ Organisation.where(id: organisation_id)&.first
+ else
+ Organisation.affiliated_organisations(user.organisation).where(id: organisation_id)&.first
+ end
+ return [OpenStruct.new(id: org.id, name: org.name)] if org.present?
+ end
+
+ [OpenStruct.new(id: "", name: "Select an option")]
+ end
+
+ def assigned_to_csv_filter_options(user)
user_options = user.support? ? User.all : (user.organisation.users + user.organisation.managing_agents.flat_map(&:users) + user.organisation.stock_owners.flat_map(&:users)).uniq
[OpenStruct.new(id: "", name: "Select an option", hint: "")] + user_options.map { |user_option| OpenStruct.new(id: user_option.id, name: user_option.name, hint: user_option.email) }
end
+ def assigned_to_filter_options(filter_type)
+ if applied_filters(filter_type)["assigned_to"] == "specific_user" && applied_filters(filter_type)["user"].present?
+ user_id = applied_filters(filter_type)["user"]
+ selected_user = if current_user.support?
+ User.where(id: user_id)&.first
+ else
+ User.own_and_managing_org_users(current_user.organisation).where(id: user_id)&.first
+ end
+
+ return [OpenStruct.new(id: selected_user.id, name: selected_user.name, hint: selected_user.email)] if selected_user.present?
+ end
+ [OpenStruct.new(id: "", name: "Select an option", hint: "")]
+ end
+
+ def filter_search_url(category)
+ case category
+ when :user
+ search_users_path
+ when :owning_organisation, :managing_organisation
+ search_organisations_path
+ end
+ end
+
def collection_year_options
years = {
current_collection_start_year.to_s => year_combo(current_collection_start_year),
@@ -125,11 +163,26 @@ module FiltersHelper
end
end
- def managing_organisation_filter_options(user)
+ def managing_organisation_csv_filter_options(user)
organisation_options = user.support? ? Organisation.all : ([user.organisation] + user.organisation.managing_agents + user.organisation.absorbed_organisations).uniq
[OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) }
end
+ def managing_organisation_filter_options(user, filter_type)
+ if applied_filters(filter_type)["managing_organisation"].present?
+ organisation_id = applied_filters(filter_type)["managing_organisation"]
+
+ org = if user.support?
+ Organisation.where(id: organisation_id)&.first
+ else
+ Organisation.affiliated_organisations(user.organisation).where(id: organisation_id)&.first
+ end
+ return [OpenStruct.new(id: org.id, name: org.name)] if org.present?
+ end
+
+ [OpenStruct.new(id: "", name: "Select an option")]
+ end
+
def show_scheme_managing_org_filter?(user)
org = user.organisation
@@ -176,8 +229,8 @@ module FiltersHelper
{ id: "status", label: "Status", value: formatted_status_filter(session_filters) },
filter_type == "lettings_logs" ? { id: "needstype", label: "Needs type", value: formatted_needstype_filter(session_filters) } : nil,
{ id: "assigned_to", label: "Assigned to", value: formatted_assigned_to_filter(session_filters) },
- { id: "owned_by", label: "Owned by", value: formatted_owned_by_filter(session_filters) },
- { id: "managed_by", label: "Managed by", value: formatted_managed_by_filter(session_filters) },
+ { id: "owned_by", label: "Owned by", value: formatted_owned_by_filter(session_filters, filter_type) },
+ { id: "managed_by", label: "Managed by", value: formatted_managed_by_filter(session_filters, filter_type) },
].compact
end
@@ -221,7 +274,7 @@ private
filters.each.sum do |category, category_filters|
if %w[years status needstypes bulk_upload_id].include?(category)
category_filters.count(&:present?)
- elsif %w[user owning_organisation managing_organisation].include?(category)
+ elsif %w[user owning_organisation managing_organisation user_text_search owning_organisation_text_search managing_organisation_text_search].include?(category)
1
else
0
@@ -256,26 +309,27 @@ private
return "All" if session_filters["assigned_to"].include?("all")
return "You" if session_filters["assigned_to"].include?("you")
- selected_user_option = assigned_to_filter_options(current_user).find { |x| x.id == session_filters["user"].to_i }
+ User.own_and_managing_org_users(current_user.organisation).find(session_filters["user"].to_i).name
+ selected_user_option = User.own_and_managing_org_users(current_user.organisation).find(session_filters["user"].to_i)
return unless selected_user_option
- "#{selected_user_option.name} (#{selected_user_option.hint})"
+ "#{selected_user_option.name} (#{selected_user_option.email})"
end
- def formatted_owned_by_filter(session_filters)
+ def formatted_owned_by_filter(session_filters, filter_type)
return "All" if params["id"].blank? && (session_filters["owning_organisation"].blank? || session_filters["owning_organisation"]&.include?("all"))
session_org_id = session_filters["owning_organisation"] || params["id"]
- selected_owning_organisation_option = owning_organisation_filter_options(current_user).find { |org| org.id == session_org_id.to_i }
+ selected_owning_organisation_option = owning_organisation_filter_options(current_user, filter_type).find { |org| org.id == session_org_id.to_i }
return unless selected_owning_organisation_option
selected_owning_organisation_option&.name
end
- def formatted_managed_by_filter(session_filters)
+ def formatted_managed_by_filter(session_filters, filter_type)
return "All" if session_filters["managing_organisation"].blank? || session_filters["managing_organisation"].include?("all")
- selected_managing_organisation_option = managing_organisation_filter_options(current_user).find { |org| org.id == session_filters["managing_organisation"].to_i }
+ selected_managing_organisation_option = managing_organisation_filter_options(current_user, filter_type).find { |org| org.id == session_filters["managing_organisation"].to_i }
return unless selected_managing_organisation_option
selected_managing_organisation_option&.name
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index e7a72350d..10ab612cd 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -132,6 +132,10 @@ class LettingsLog < Log
illness_type_10: false)
}
+ scope :filter_by_user_text_search, ->(param, user) { where(assigned_to: user.support? ? User.search_by(param) : User.own_and_managing_org_users(user.organisation).search_by(param)) }
+ scope :filter_by_owning_organisation_text_search, ->(param, _user) { where(owning_organisation: Organisation.search_by(param)) }
+ scope :filter_by_managing_organisation_text_search, ->(param, _user) { where(managing_organisation: Organisation.search_by(param)) }
+
AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze
OPTIONAL_FIELDS = %w[tenancycode propcode chcharge].freeze
RENT_TYPE_MAPPING_LABELS = { 1 => "Social Rent", 2 => "Affordable Rent", 3 => "Intermediate Rent" }.freeze
diff --git a/app/models/log.rb b/app/models/log.rb
index c095a8276..3a6c1e982 100644
--- a/app/models/log.rb
+++ b/app/models/log.rb
@@ -53,6 +53,9 @@ class Log < ApplicationRecord
scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) }
scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) }
scope :filter_by_managing_organisation, ->(managing_organisation, _user = nil) { where(managing_organisation:) }
+ scope :filter_by_user_text_search, ->(param, user) { where(assigned_to: user.support? ? User.search_by(param) : User.own_and_managing_org_users(user.organisation).search_by(param)) }
+ scope :filter_by_owning_organisation_text_search, ->(param, _user) { where(owning_organisation: Organisation.search_by(param)) }
+ scope :filter_by_managing_organisation_text_search, ->(param, _user) { where(managing_organisation: Organisation.search_by(param)) }
attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :select_best_address_match, :skip_dpo_validation
diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index 8f77df166..65b35c24e 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -18,6 +18,7 @@ class Organisation < ApplicationRecord
belongs_to :absorbing_organisation, class_name: "Organisation", optional: true
has_many :absorbed_organisations, class_name: "Organisation", foreign_key: "absorbing_organisation_id"
scope :visible, -> { where(discarded_at: nil) }
+ scope :affiliated_organisations, ->(organisation) { where(id: (organisation.child_organisations + [organisation] + organisation.parent_organisations + organisation.absorbed_organisations).map(&:id)) }
def affiliated_stock_owners
ids = []
diff --git a/app/models/user.rb b/app/models/user.rb
index d25faaa53..c79ceb0d9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -77,6 +77,7 @@ class User < ApplicationRecord
scope :deactivated, -> { where(active: false) }
scope :active_status, -> { where(active: true).where.not(last_sign_in_at: nil) }
scope :visible, -> { where(discarded_at: nil) }
+ scope :own_and_managing_org_users, ->(organisation) { where(organisation: organisation.child_organisations + [organisation]) }
def lettings_logs
if support?
@@ -209,9 +210,9 @@ class User < ApplicationRecord
def logs_filters(specific_org: false)
if (support? && !specific_org) || organisation.has_managing_agents? || organisation.has_stock_owners?
- %w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id]
+ %w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id user_text_search owning_organisation_text_search managing_organisation_text_search]
else
- %w[years status needstypes assigned_to user bulk_upload_id]
+ %w[years status needstypes assigned_to user bulk_upload_id user_text_search]
end
end
diff --git a/app/services/filter_manager.rb b/app/services/filter_manager.rb
index 8d661b3fa..9f68a097c 100644
--- a/app/services/filter_manager.rb
+++ b/app/services/filter_manager.rb
@@ -24,6 +24,9 @@ class FilterManager
next if category == "owning_organisation" && all_orgs
next if category == "managing_organisation" && all_orgs
next if category == "assigned_to"
+ next if category == "user_text_search" && filters["assigned_to"] != "specific_user"
+ next if category == "owning_organisation_text_search" && all_orgs
+ next if category == "managing_organisation_text_search" && all_orgs
logs = logs.public_send("filter_by_#{category}", values, user)
end
@@ -94,11 +97,19 @@ class FilterManager
new_filters[filter] = params[filter] if params[filter].present?
end
+ if params["action"] == "download_csv"
+ new_filters["assigned_to"] = "all" if new_filters["assigned_to"] == "specific_user" && new_filters["user_text_search"].present?
+ new_filters["owning_organisation_select"] = "all" if new_filters["owning_organisation_select"] == "specific_organisation" && new_filters["owning_organisation_text_search"].present?
+ new_filters["managing_organisation_select"] = "all" if new_filters["managing_organisation_select"] == "specific_organisation" && new_filters["managing_organisation_text_search"].present?
+ end
new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all"
new_filters = new_filters.except("managing_organisation") if params["managing_organisation_select"] == "all"
new_filters = new_filters.except("user") if params["assigned_to"] == "all"
new_filters["user"] = current_user.id.to_s if params["assigned_to"] == "you"
+ new_filters = new_filters.except("user_text_search") if params["assigned_to"] == "all" || params["assigned_to"] == "you"
+ new_filters = new_filters.except("owning_organisation_text_search") if params["owning_organisation_select"] == "all"
+ new_filters = new_filters.except("managing_organisation_text_search") if params["managing_organisation_select"] == "all"
end
if (filter_type.include?("schemes") || filter_type.include?("users") || filter_type.include?("scheme_locations")) && params["status"].present?
diff --git a/app/views/filters/_radio_filter.html.erb b/app/views/filters/_radio_filter.html.erb
index 6eb902dba..e4d23573c 100644
--- a/app/views/filters/_radio_filter.html.erb
+++ b/app/views/filters/_radio_filter.html.erb
@@ -10,6 +10,7 @@
collection: option[:conditional_filter][:options],
category: option[:conditional_filter][:category],
label: option[:conditional_filter][:label],
+ caption_text: option[:conditional_filter][:caption_text],
secondary: true,
hint_text: option[:conditional_filter][:hint_text],
} %>
diff --git a/app/views/filters/_text_select_filter.html.erb b/app/views/filters/_text_select_filter.html.erb
new file mode 100644
index 000000000..ecc997bba
--- /dev/null
+++ b/app/views/filters/_text_select_filter.html.erb
@@ -0,0 +1,20 @@
+
+<%= f.govuk_text_field "#{category}_text_search".to_sym,
+ label: { text: label, hidden: secondary },
+ "data-controller": "search conditional-filter",
+ caption: { text: caption_text },
+ "data-info": { search_url: filter_search_url(category.to_sym) }.to_json,
+ value: selected_option("#{category}_text_search", @filter_type) %>
+
+<%= f.govuk_select(category.to_sym,
+ label: { text: label, hidden: secondary },
+ "data-controller": "search conditional-filter",
+ "hidden": true,
+ "data-info": { search_url: filter_search_url(category.to_sym) }.to_json) do %>
+ <% collection.each do |answer| %>
+
+ <% end %>
+ <% end %>
diff --git a/app/views/filters/assigned_to.html.erb b/app/views/filters/assigned_to.html.erb
index 9f0582fbb..778d63c8a 100644
--- a/app/views/filters/assigned_to.html.erb
+++ b/app/views/filters/assigned_to.html.erb
@@ -11,7 +11,8 @@
type: "select",
label: "User",
category: "user",
- options: assigned_to_filter_options(current_user),
+ caption_text: "User's name or email",
+ options: assigned_to_csv_filter_options(current_user),
},
},
},
diff --git a/app/views/filters/managed_by.html.erb b/app/views/filters/managed_by.html.erb
index e3d849c9b..5d4b684f3 100644
--- a/app/views/filters/managed_by.html.erb
+++ b/app/views/filters/managed_by.html.erb
@@ -9,7 +9,8 @@
type: "select",
label: "Managed by",
category: "managing_organisation",
- options: managing_organisation_filter_options(current_user),
+ options: managing_organisation_csv_filter_options(current_user),
+ caption_text: "Organisation name",
},
},
},
diff --git a/app/views/filters/owned_by.html.erb b/app/views/filters/owned_by.html.erb
index 7acfd459c..271b68de9 100644
--- a/app/views/filters/owned_by.html.erb
+++ b/app/views/filters/owned_by.html.erb
@@ -9,7 +9,8 @@
type: "select",
label: "Owning Organisation",
category: "owning_organisation",
- options: owning_organisation_filter_options(current_user),
+ options: all_owning_organisation_filter_options(current_user),
+ caption_text: "Organisation name",
},
},
},
diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb
index aaef70377..3beab4b6b 100644
--- a/app/views/logs/_log_filters.html.erb
+++ b/app/views/logs/_log_filters.html.erb
@@ -66,10 +66,11 @@
"specific_user": {
label: "Specific user",
conditional_filter: {
- type: "select",
+ type: "text_select",
label: "User",
category: "user",
- options: assigned_to_filter_options(current_user),
+ options: assigned_to_filter_options(@filter_type),
+ caption_text: "User's name or email",
},
},
},
@@ -86,10 +87,11 @@
"specific_org": {
label: "Specific owning organisation",
conditional_filter: {
- type: "select",
+ type: "text_select",
label: "Owning Organisation",
category: "owning_organisation",
- options: owning_organisation_filter_options(current_user),
+ options: owning_organisation_filter_options(current_user, @filter_type),
+ caption_text: "Organisation name",
},
},
},
@@ -107,10 +109,11 @@
"specific_org": {
label: "Specific managing organisation",
conditional_filter: {
- type: "select",
+ type: "text_select",
label: user_or_org_lettings_path? ? "Managed by" : "Reported by",
category: "managing_organisation",
- options: managing_organisation_filter_options(current_user),
+ options: managing_organisation_filter_options(current_user, @filter_type),
+ caption_text: "Organisation name",
},
},
},
diff --git a/app/views/schemes/_scheme_filters.html.erb b/app/views/schemes/_scheme_filters.html.erb
index ca0538463..51687a096 100644
--- a/app/views/schemes/_scheme_filters.html.erb
+++ b/app/views/schemes/_scheme_filters.html.erb
@@ -35,7 +35,7 @@
type: "select",
label: "Owning Organisation",
category: "owning_organisation",
- options: owning_organisation_filter_options(current_user),
+ options: all_owning_organisation_filter_options(current_user),
},
},
},
diff --git a/config/routes.rb b/config/routes.rb
index e0d9631e9..faea457fe 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -125,6 +125,10 @@ Rails.application.routes.draw do
get "edit-dpo", to: "users#dpo"
get "edit-key-contact", to: "users#key_contact"
+ collection do
+ get :search
+ end
+
member do
get "deactivate", to: "users#deactivate"
get "reactivate", to: "users#reactivate"
@@ -191,6 +195,10 @@ Rails.application.routes.draw do
get "delete-confirmation", to: "organisations#delete_confirmation"
delete "delete", to: "organisations#delete"
end
+
+ collection do
+ get :search
+ end
end
resources :merge_requests, path: "/merge-request" do
diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb
index 2b977fdd7..ac9a1e4a8 100644
--- a/spec/features/lettings_log_spec.rb
+++ b/spec/features/lettings_log_spec.rb
@@ -89,9 +89,9 @@ RSpec.describe "Lettings Log Features" do
check("In progress")
choose("You")
choose("Specific owning organisation")
- select(stock_owner_1.name, from: "owning_organisation")
+ fill_in("owning-organisation-text-search-field", with: "stock")
choose("Specific managing organisation")
- select(managing_agent_1.name, from: "managing_organisation")
+ fill_in("managing-organisation-text-search-field", with: "managing")
click_button("Apply filters")
end
diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb
index 65f787c2a..1867285eb 100644
--- a/spec/features/organisation_spec.rb
+++ b/spec/features/organisation_spec.rb
@@ -199,14 +199,14 @@ RSpec.describe "User Features" do
it "can filter lettings logs by year" do
check("years-2022-field")
click_button("Apply filters")
- expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2022&status[]=&needstypes[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=&managing_organisation_select=all&managing_organisation=")
+ expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2022&status[]=&needstypes[]=&assigned_to=all&user_text_search=&user=&owning_organisation_select=all&owning_organisation_text_search=&owning_organisation=&managing_organisation_select=all&managing_organisation_text_search=&managing_organisation=")
expect(page).not_to have_link first_log.id.to_s, href: "/lettings-logs/#{first_log.id}"
end
it "can filter lettings logs by needstype" do
check("needstypes-1-field")
click_button("Apply filters")
- expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&status[]=&needstypes[]=&needstypes[]=1&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=&managing_organisation_select=all&managing_organisation=")
+ expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&status[]=&needstypes[]=&needstypes[]=1&assigned_to=all&user_text_search=&user=&owning_organisation_select=all&owning_organisation_text_search=&owning_organisation=&managing_organisation_select=all&managing_organisation_text_search=&managing_organisation=")
other_general_needs_logs.each do |general_needs_log|
expect(page).to have_link general_needs_log.id.to_s, href: "/lettings-logs/#{general_needs_log.id}"
end
@@ -245,7 +245,7 @@ RSpec.describe "User Features" do
end
check("years-2022-field")
click_button("Apply filters")
- expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2022&status[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=&managing_organisation_select=all&managing_organisation=")
+ expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2022&status[]=&assigned_to=all&user_text_search=&user=&owning_organisation_select=all&owning_organisation_text_search=&owning_organisation=&managing_organisation_select=all&managing_organisation_text_search=&managing_organisation=")
expect(page).not_to have_link first_log.id.to_s, href: "/sales-logs/#{first_log.id}"
end
end
diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb
index 169465cb1..c30abe1e9 100644
--- a/spec/features/user_spec.rb
+++ b/spec/features/user_spec.rb
@@ -796,12 +796,13 @@ RSpec.describe "User Features" do
visit("/lettings-logs")
choose("owning-organisation-select-specific-org-field", allow_label_click: true)
expect(page).to have_field("owning-organisation-field", with: "")
- find("#owning-organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter)
+ find("#owning-organisation-field").click.native.send_keys("F", "i", "l", "t")
+ select(parent_organisation.name, from: "owning-organisation-field-select", visible: false)
click_button("Apply filters")
- expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&%5Bneedstypes%5D%5B%5D=&assigned_to=all&owning_organisation_select=specific_org&owning_organisation=#{parent_organisation.id}&managing_organisation_select=all")
+ expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&%5Bneedstypes%5D%5B%5D=&assigned_to=all&user_text_search=&owning_organisation_select=specific_org&owning_organisation_text_search=&owning_organisation=#{parent_organisation.id}&managing_organisation_select=all&managing_organisation_text_search=")
choose("owning-organisation-select-all-field", allow_label_click: true)
click_button("Apply filters")
- expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&%5Bneedstypes%5D%5B%5D=&assigned_to=all&owning_organisation_select=all&managing_organisation_select=all")
+ expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&%5Bneedstypes%5D%5B%5D=&assigned_to=all&user_text_search=&owning_organisation_select=all&owning_organisation_text_search=&managing_organisation_select=all&managing_organisation_text_search=")
end
end
end
diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb
index f04157521..c57f92311 100644
--- a/spec/helpers/filters_helper_spec.rb
+++ b/spec/helpers/filters_helper_spec.rb
@@ -175,27 +175,146 @@ RSpec.describe FiltersHelper do
context "with a support user" do
let(:user) { FactoryBot.create(:user, :support, organisation: child_organisation) }
- it "returns a list of all organisations" do
- expect(owning_organisation_filter_options(user)).to match_array([
- OpenStruct.new(id: "", name: "Select an option"),
- OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
- OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
- OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
- OpenStruct.new(id: 99, name: "Other organisation"),
- ])
+ context "when no organisation is selected in the filters" do
+ it "returns an empty list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
+ end
+
+ context "when a specific child organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": child_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
+ ])
+ end
+ end
+
+ context "when a specific parent organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": parent_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
+ ])
+ end
+ end
+
+ context "when a specific absorbed organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": absorbed_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
+ ])
+ end
+ end
+
+ context "when a specific non related organisation is selected in the filters" do
+ let(:unrelated_organisation) { create(:organisation, name: "Unrelated organisation") }
+
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": unrelated_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: unrelated_organisation.id, name: "Unrelated organisation"),
+ ])
+ end
+ end
+
+ context "when a non existing organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": 143_542_542 }.to_json
+ end
+
+ it "returns an empty list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
end
end
context "with a data coordinator user" do
let(:user) { FactoryBot.create(:user, :data_coordinator, organisation: child_organisation) }
- it "returns a list of parent orgs and your own organisation" do
- expect(owning_organisation_filter_options(user.reload)).to eq([
- OpenStruct.new(id: "", name: "Select an option"),
- OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
- OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
- OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
- ])
+ context "when no organisation is selected in the filters" do
+ it "returns an empty list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
+ end
+
+ context "when a specific child organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": child_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
+ ])
+ end
+ end
+
+ context "when a specific parent organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": parent_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
+ ])
+ end
+ end
+
+ context "when a specific absorbed organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": absorbed_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
+ ])
+ end
+ end
+
+ context "when a specific non related organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": create(:organisation).id }.to_json
+ end
+
+ it "returns an empty list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
+ end
+
+ context "when a non existing organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "owning_organisation": 143_542_542 }.to_json
+ end
+
+ it "returns an empty list" do
+ expect(owning_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
end
end
end
@@ -214,27 +333,146 @@ RSpec.describe FiltersHelper do
context "with a support user" do
let(:user) { FactoryBot.create(:user, :support, organisation: parent_organisation) }
- it "returns a list of all organisations" do
- expect(managing_organisation_filter_options(user)).to eq([
- OpenStruct.new(id: "", name: "Select an option"),
- OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
- OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
- OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
- OpenStruct.new(id: 99, name: "Other organisation"),
- ])
+ context "when no organisation is selected in the filters" do
+ it "returns an empty list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
+ end
+
+ context "when a specific child organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": child_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
+ ])
+ end
+ end
+
+ context "when a specific parent organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": parent_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
+ ])
+ end
+ end
+
+ context "when a specific absorbed organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": absorbed_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
+ ])
+ end
+ end
+
+ context "when a specific non related organisation is selected in the filters" do
+ let(:unrelated_organisation) { create(:organisation, name: "Unrelated organisation") }
+
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": unrelated_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: unrelated_organisation.id, name: "Unrelated organisation"),
+ ])
+ end
+ end
+
+ context "when a non existing organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": 143_542_542 }.to_json
+ end
+
+ it "returns an empty list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
end
end
context "with a data coordinator user" do
let(:user) { FactoryBot.create(:user, :data_coordinator, organisation: parent_organisation) }
- it "returns a list of child orgs and your own organisation" do
- expect(managing_organisation_filter_options(user.reload)).to eq([
- OpenStruct.new(id: "", name: "Select an option"),
- OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
- OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
- OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
- ])
+ context "when no organisation is selected in the filters" do
+ it "returns an empty list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
+ end
+
+ context "when a specific child organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": child_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: child_organisation.id, name: "Child organisation"),
+ ])
+ end
+ end
+
+ context "when a specific parent organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": parent_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
+ ])
+ end
+ end
+
+ context "when a specific absorbed organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": absorbed_organisation.id }.to_json
+ end
+
+ it "returns the selected organisation in the list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"),
+ ])
+ end
+ end
+
+ context "when a specific non related organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": create(:organisation).id }.to_json
+ end
+
+ it "returns an empty list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
+ end
+
+ context "when a non existing organisation is selected in the filters" do
+ before do
+ session[:lettings_logs_filters] = { "managing_organisation": 143_542_542 }.to_json
+ end
+
+ it "returns an empty list" do
+ expect(managing_organisation_filter_options(user.reload, "lettings_logs")).to eq([
+ OpenStruct.new(id: "", name: "Select an option"),
+ ])
+ end
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index edb998ac3..6a04e9a0b 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -164,7 +164,7 @@ RSpec.describe User, type: :model do
end
it "can filter lettings logs by user, year and status" do
- expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user bulk_upload_id])
+ expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user bulk_upload_id user_text_search])
end
end
@@ -174,7 +174,7 @@ RSpec.describe User, type: :model do
end
it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do
- expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id])
+ expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id managing_organisation_text_search owning_organisation_text_search user_text_search])
end
end
end
@@ -215,7 +215,7 @@ RSpec.describe User, type: :model do
end
it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do
- expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user owning_organisation managing_organisation bulk_upload_id])
+ expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user owning_organisation managing_organisation bulk_upload_id managing_organisation_text_search owning_organisation_text_search user_text_search])
end
end
diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb
index 13879a38c..d3e8a8155 100644
--- a/spec/requests/organisations_controller_spec.rb
+++ b/spec/requests/organisations_controller_spec.rb
@@ -75,6 +75,13 @@ RSpec.describe OrganisationsController, type: :request do
end
end
end
+
+ describe "#search" do
+ it "redirects to the sign in page" do
+ get "/organisations/search"
+ expect(response).to redirect_to("/account/sign-in")
+ end
+ end
end
context "when user is signed in" do
@@ -807,6 +814,25 @@ RSpec.describe OrganisationsController, type: :request do
end
end
end
+
+ describe "#search" do
+ let(:parent_organisation) { create(:organisation, name: "parent test organisation") }
+ let(:child_organisation) { create(:organisation, name: "child test organisation") }
+
+ before do
+ user.organisation.update!(name: "test organisation")
+ create(:organisation_relationship, parent_organisation: user.organisation, child_organisation:)
+ create(:organisation_relationship, child_organisation: user.organisation, parent_organisation:)
+ create(:organisation, name: "other organisation test organisation")
+ end
+
+ it "only searches within the current user's organisation, managing agents and stock owners" do
+ get "/organisations/search", headers:, params: { query: "test organisation" }
+ result = JSON.parse(response.body)
+ expect(result.count).to eq(3)
+ expect(result.keys).to match_array([user.organisation.id.to_s, parent_organisation.id.to_s, child_organisation.id.to_s])
+ end
+ end
end
context "with a data provider user" do
@@ -2077,6 +2103,25 @@ RSpec.describe OrganisationsController, type: :request do
end
end
end
+
+ describe "#search" do
+ let(:parent_organisation) { create(:organisation, name: "parent test organisation") }
+ let(:child_organisation) { create(:organisation, name: "child test organisation") }
+ let!(:other_organisation) { create(:organisation, name: "other organisation test organisation") }
+
+ before do
+ user.organisation.update!(name: "test organisation")
+ create(:organisation_relationship, parent_organisation: user.organisation, child_organisation:)
+ create(:organisation_relationship, child_organisation: user.organisation, parent_organisation:)
+ end
+
+ it "searches within all the organisations" do
+ get "/organisations/search", headers:, params: { query: "test organisation" }
+ result = JSON.parse(response.body)
+ expect(result.count).to eq(4)
+ expect(result.keys).to match_array([user.organisation.id.to_s, parent_organisation.id.to_s, child_organisation.id.to_s, other_organisation.id.to_s])
+ end
+ end
end
end
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index bb0a1cca3..8e87f7f28 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -117,6 +117,13 @@ RSpec.describe UsersController, type: :request do
expect(response).to redirect_to("/account/sign-in")
end
end
+
+ describe "#search" do
+ it "redirects to the sign in page" do
+ get "/users/search"
+ expect(response).to redirect_to("/account/sign-in")
+ end
+ end
end
context "when user is signed in as a data provider" do
@@ -404,6 +411,25 @@ RSpec.describe UsersController, type: :request do
expect(response).to have_http_status(:unauthorized)
end
end
+
+ describe "#search" do
+ let(:parent_relationship) { create(:organisation_relationship, parent_organisation: user.organisation) }
+ let(:child_relationship) { create(:organisation_relationship, child_organisation: user.organisation) }
+ let!(:org_user) { create(:user, organisation: user.organisation, name: "test_name") }
+ let!(:managing_user) { create(:user, organisation: parent_relationship.child_organisation, name: "managing_agent_test_name") }
+
+ before do
+ create(:user, organisation: child_relationship.parent_organisation, name: "stock_owner_test_name")
+ create(:user, name: "other_organisation_test_name")
+ end
+
+ it "only searches within the current user's organisation and managing agents" do
+ get "/users/search", headers:, params: { query: "test_name" }
+ result = JSON.parse(response.body)
+ expect(result.count).to eq(2)
+ expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s])
+ end
+ end
end
context "when user is signed in as a data coordinator" do
@@ -1174,6 +1200,25 @@ RSpec.describe UsersController, type: :request do
expect(response).to have_http_status(:unauthorized)
end
end
+
+ describe "#search" do
+ let(:parent_relationship) { create(:organisation_relationship, parent_organisation: user.organisation) }
+ let(:child_relationship) { create(:organisation_relationship, child_organisation: user.organisation) }
+ let!(:org_user) { create(:user, organisation: user.organisation, email: "test_name@example.com") }
+ let!(:managing_user) { create(:user, organisation: parent_relationship.child_organisation, email: "managing_agent_test_name@example.com") }
+
+ before do
+ create(:user, email: "other_organisation_test_name@example.com")
+ create(:user, organisation: child_relationship.parent_organisation, email: "stock_owner_test_name@example.com")
+ end
+
+ it "only searches within the current user's organisation and managing agents" do
+ get "/users/search", headers:, params: { query: "test_name" }
+ result = JSON.parse(response.body)
+ expect(result.count).to eq(2)
+ expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s])
+ end
+ end
end
context "when user is signed in as a support user" do
@@ -2111,6 +2156,22 @@ RSpec.describe UsersController, type: :request do
expect(page).not_to have_link("User to be deleted")
end
end
+
+ describe "#search" do
+ let(:parent_relationship) { create(:organisation_relationship, parent_organisation: user.organisation) }
+ let(:child_relationship) { create(:organisation_relationship, child_organisation: user.organisation) }
+ let!(:org_user) { create(:user, organisation: user.organisation, name: "test_name") }
+ let!(:managing_user) { create(:user, organisation: child_relationship.parent_organisation, name: "stock_owner_test_name") }
+ let!(:owner_user) { create(:user, organisation: parent_relationship.child_organisation, name: "managing_agent_test_name") }
+ let!(:other_user) { create(:user, name: "other_organisation_test_name") }
+
+ it "searches all users" do
+ get "/users/search", headers:, params: { query: "test_name" }
+ result = JSON.parse(response.body)
+ expect(result.count).to eq(4)
+ expect(result.keys).to match_array([org_user.id.to_s, managing_user.id.to_s, owner_user.id.to_s, other_user.id.to_s])
+ end
+ end
end
describe "title link" do