From 4f51cf4da80554bfbf24ffbcc0de9786777fec94 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 3 Dec 2024 08:18:19 +0000 Subject: [PATCH 01/19] Bump rails-html-sanitizer from 1.6.0 to 1.6.1 (#2847) Bumps [rails-html-sanitizer](https://github.com/rails/rails-html-sanitizer) from 1.6.0 to 1.6.1. - [Release notes](https://github.com/rails/rails-html-sanitizer/releases) - [Changelog](https://github.com/rails/rails-html-sanitizer/blob/main/CHANGELOG.md) - [Commits](https://github.com/rails/rails-html-sanitizer/compare/v1.6.0...v1.6.1) --- updated-dependencies: - dependency-name: rails-html-sanitizer dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Gemfile.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1a98463b5..d2a5de05d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -246,7 +246,7 @@ GEM listen (3.9.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - loofah (2.22.0) + loofah (2.23.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) mail (2.8.1) @@ -274,11 +274,11 @@ GEM net-smtp (0.5.0) net-protocol nio4r (2.7.3) - nokogiri (1.16.7-arm64-darwin) + nokogiri (1.16.8-arm64-darwin) racc (~> 1.4) - nokogiri (1.16.7-x86_64-darwin) + nokogiri (1.16.8-x86_64-darwin) racc (~> 1.4) - nokogiri (1.16.7-x86_64-linux) + nokogiri (1.16.8-x86_64-linux) racc (~> 1.4) notifications-ruby-client (6.0.0) jwt (>= 1.5, < 3) @@ -345,9 +345,9 @@ GEM activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.6.0) + rails-html-sanitizer (1.6.1) loofah (~> 2.21) - nokogiri (~> 1.14) + nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) rails_admin (3.1.3) activemodel-serializers-xml (>= 1.0) kaminari (>= 0.14, < 2.0) From a137999d294585a6b8cd1995556619326709da45 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 3 Dec 2024 08:19:59 +0000 Subject: [PATCH 02/19] Add addresses flow feature tests and update the flow a bit (#2805) --- .../lettings_log_variables.rb | 9 + .../derived_variables/sales_log_variables.rb | 9 + app/models/log.rb | 4 +- spec/features/lettings_log_spec.rb | 348 ++++++++++++++++++ spec/features/sales_log_spec.rb | 348 ++++++++++++++++++ 5 files changed, 717 insertions(+), 1 deletion(-) diff --git a/app/models/derived_variables/lettings_log_variables.rb b/app/models/derived_variables/lettings_log_variables.rb index 4692a0e6b..ced530b17 100644 --- a/app/models/derived_variables/lettings_log_variables.rb +++ b/app/models/derived_variables/lettings_log_variables.rb @@ -84,6 +84,15 @@ module DerivedVariables::LettingsLogVariables if uprn_known&.zero? self.uprn = nil + if uprn_known_was == 1 + self.address_line1 = nil + self.address_line2 = nil + self.town_or_city = nil + self.county = nil + self.postcode_known = nil + self.postcode_full = nil + self.la = nil + end end if uprn_known == 1 && uprn_confirmed&.zero? diff --git a/app/models/derived_variables/sales_log_variables.rb b/app/models/derived_variables/sales_log_variables.rb index 4f4c3105d..6e12ec488 100644 --- a/app/models/derived_variables/sales_log_variables.rb +++ b/app/models/derived_variables/sales_log_variables.rb @@ -53,6 +53,15 @@ module DerivedVariables::SalesLogVariables if uprn_known&.zero? self.uprn = nil + if uprn_known_was == 1 + self.address_line1 = nil + self.address_line2 = nil + self.town_or_city = nil + self.county = nil + self.pcodenk = nil + self.postcode_full = nil + self.la = nil + end end if uprn_known == 1 && uprn_confirmed&.zero? diff --git a/app/models/log.rb b/app/models/log.rb index dd4301550..bcbea9c92 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -126,9 +126,11 @@ class Log < ApplicationRecord end def address_options - return @address_options if @address_options + return @address_options if @address_options && @last_searched_address_string == address_string if [address_line1_input, postcode_full_input].all?(&:present?) + @last_searched_address_string = address_string + service = AddressClient.new(address_string) service.call if service.result.blank? || service.error.present? diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index efb7e7665..b10dfc6e5 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -729,5 +729,353 @@ RSpec.describe "Lettings Log Features" do expect(duplicate_log.duplicate_set_id).to be_nil end end + + context "when filling out address fields" do + let(:lettings_log) { create(:lettings_log, :setup_completed, assigned_to: user) } + + before do + body = { + results: [ + { + DPA: { + "POSTCODE": "AA1 1AA", + "POST_TOWN": "Bristol", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=111") + .to_return(status: 200, body:, headers: {}) + + body = { results: [{ DPA: { UPRN: "111" } }] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?query=Address+line+1%2C+AA1+1AA&key=OS_DATA_KEY&maxresults=10&minmatch=0.4") + .to_return(status: 200, body:, headers: {}) + + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/AA11AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"AA1 1AA\",\"admin_district\":\"Westminster\",\"codes\":{\"admin_district\":\"E09000033\"}}}", headers: {}) + + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/AA12AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"AA1 2AA\",\"admin_district\":\"Wigan\",\"codes\":{\"admin_district\":\"E08000010\"}}}", headers: {}) + + body = { results: [] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?query=Address+line+1%2C+AA1+1AB&key=OS_DATA_KEY&maxresults=10&minmatch=0.4") + .to_return(status: 200, body:, headers: {}) + + visit("/lettings-logs/#{lettings_log.id}/uprn") + end + + context "and uprn is known and answered" do + before do + choose "Yes" + fill_in("lettings_log[uprn]", with: "111") + click_button("Save and continue") + end + + context "and uprn is confirmed" do + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(1) # yes + expect(lettings_log.uprn).to eq("111") + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 1AA") + expect(lettings_log.address_line1).to eq("Some Place") + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq("Bristol") + expect(lettings_log.address_line1_input).to eq(nil) + expect(lettings_log.postcode_full_input).to eq(nil) + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E09000033") + + choose "Yes" + click_button("Save and continue") + + lettings_log.reload + expect(lettings_log.uprn_known).to eq(1) # yes + expect(lettings_log.uprn).to eq("111") + expect(lettings_log.uprn_confirmed).to eq(1) # yes + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 1AA") + expect(lettings_log.address_line1).to eq("Some Place") + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq("Bristol") + expect(lettings_log.address_line1_input).to eq(nil) + expect(lettings_log.postcode_full_input).to eq(nil) + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E09000033") + end + + context "and changes to uprn not known" do + it "sets correct address fields" do + visit("/lettings-logs/#{lettings_log.id}/uprn") + + choose "No" + click_button("Save and continue") + + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(nil) + expect(lettings_log.postcode_full).to eq(nil) + expect(lettings_log.address_line1).to eq(nil) + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq(nil) + expect(lettings_log.address_line1_input).to eq(nil) + expect(lettings_log.postcode_full_input).to eq(nil) + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq(nil) + end + end + end + + context "and uprn is not confirmed" do + before do + choose "No, I want to search for the address instead" + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(nil) + expect(lettings_log.postcode_full).to eq(nil) + expect(lettings_log.address_line1).to eq(nil) + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq(nil) + expect(lettings_log.address_line1_input).to eq(nil) + expect(lettings_log.postcode_full_input).to eq(nil) + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq(nil) + end + end + end + + context "and uprn is not known" do + before do + choose "No" + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(nil) + expect(lettings_log.postcode_full).to eq(nil) + expect(lettings_log.address_line1).to eq(nil) + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq(nil) + expect(lettings_log.address_line1_input).to eq(nil) + expect(lettings_log.postcode_full_input).to eq(nil) + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq(nil) + end + + context "and the address is not found" do + it "sets correct address fields" do + fill_in("lettings_log[address_line1_input]", with: "Address line 1") + fill_in("lettings_log[postcode_full_input]", with: "AA1 1AB") + click_button("Search") + + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(nil) + expect(lettings_log.postcode_full).to eq(nil) + expect(lettings_log.address_line1).to eq(nil) + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq(nil) + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AB") + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq(nil) + + click_button("Confirm and continue") + + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(nil) + expect(lettings_log.postcode_full).to eq(nil) + expect(lettings_log.address_line1).to eq(nil) + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq(nil) + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AB") + expect(lettings_log.address_search_value_check).to eq(0) + expect(lettings_log.la).to eq(nil) + end + end + + context "and address is found, re-searched and not found" do + before do + fill_in("lettings_log[address_line1_input]", with: "Address line 1") + fill_in("lettings_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + visit("/lettings-logs/#{lettings_log.id}/address-matcher") + + fill_in("lettings_log[address_line1_input]", with: "Address line 1") + fill_in("lettings_log[postcode_full_input]", with: "AA1 1AB") + click_button("Search") + end + + it "routes to the correct page" do + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/no-address-found") + end + end + + context "and the user selects 'address_not_listed'" do + before do + fill_in("lettings_log[address_line1_input]", with: "Address line 1") + fill_in("lettings_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + choose "The address is not listed, I want to enter the address manually" + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq("uprn_not_listed") + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 1AA") + expect(lettings_log.address_line1).to eq("Address line 1") + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq(nil) + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AA") + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E09000033") + end + + context "and the user enters a new address manually" do + context "without changing a valid postcode" do + before do + fill_in("lettings_log[town_or_city]", with: "Town") + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq("uprn_not_listed") + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 1AA") + expect(lettings_log.address_line1).to eq("Address line 1") + expect(lettings_log.address_line2).to eq("") + expect(lettings_log.town_or_city).to eq("Town") + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AA") + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E09000033") + end + end + + context "with changing the postcode" do + before do + fill_in("lettings_log[town_or_city]", with: "Town") + fill_in("lettings_log[postcode_full]", with: "AA12AA") + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(0) # no + expect(lettings_log.uprn).to eq(nil) + expect(lettings_log.uprn_confirmed).to eq(nil) + expect(lettings_log.uprn_selection).to eq("uprn_not_listed") + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 2AA") + expect(lettings_log.address_line1).to eq("Address line 1") + expect(lettings_log.address_line2).to eq("") + expect(lettings_log.town_or_city).to eq("Town") + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AA") + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E08000010") + end + end + end + end + + context "and the user selects 'address_not_listed' and then changes their mind and selects an address" do + before do + fill_in("lettings_log[address_line1_input]", with: "Address line 1") + fill_in("lettings_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + choose "The address is not listed, I want to enter the address manually" + click_button("Save and continue") + + visit("/lettings-logs/#{lettings_log.id}/uprn-selection") + choose("lettings-log-uprn-selection-111-field", allow_label_click: true) + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(1) + expect(lettings_log.uprn).to eq("111") + expect(lettings_log.uprn_confirmed).to eq(1) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 1AA") + expect(lettings_log.address_line1).to eq("Some Place") + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq("Bristol") + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AA") + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E09000033") + end + end + + context "and possible addresses found and selected" do + before do + fill_in("lettings_log[address_line1_input]", with: "Address line 1") + fill_in("lettings_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + choose("lettings-log-uprn-selection-111-field", allow_label_click: true) + click_button("Save and continue") + end + + it "sets correct address fields" do + lettings_log.reload + expect(lettings_log.uprn_known).to eq(1) + expect(lettings_log.uprn).to eq("111") + expect(lettings_log.uprn_confirmed).to eq(1) + expect(lettings_log.uprn_selection).to eq(nil) + expect(lettings_log.postcode_known).to eq(1) + expect(lettings_log.postcode_full).to eq("AA1 1AA") + expect(lettings_log.address_line1).to eq("Some Place") + expect(lettings_log.address_line2).to eq(nil) + expect(lettings_log.town_or_city).to eq("Bristol") + expect(lettings_log.address_line1_input).to eq("Address line 1") + expect(lettings_log.postcode_full_input).to eq("AA1 1AA") + expect(lettings_log.address_search_value_check).to eq(nil) + expect(lettings_log.la).to eq("E09000033") + end + end + end + end end end diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb index 879f2b5c8..d418bcb37 100644 --- a/spec/features/sales_log_spec.rb +++ b/spec/features/sales_log_spec.rb @@ -310,6 +310,354 @@ RSpec.describe "Sales Log Features" do expect(page).to have_current_path("/sales-logs/bulk-uploads") end end + + context "when filling out address fields" do + let(:sales_log) { create(:sales_log, :shared_ownership_setup_complete, assigned_to: user) } + + before do + body = { + results: [ + { + DPA: { + "POSTCODE": "AA1 1AA", + "POST_TOWN": "Bristol", + "ORGANISATION_NAME": "Some place", + }, + }, + ], + }.to_json + + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/uprn?dataset=DPA,LPI&key=OS_DATA_KEY&uprn=111") + .to_return(status: 200, body:, headers: {}) + + body = { results: [{ DPA: { UPRN: "111" } }] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?query=Address+line+1%2C+AA1+1AA&key=OS_DATA_KEY&maxresults=10&minmatch=0.4") + .to_return(status: 200, body:, headers: {}) + + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/AA11AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"AA1 1AA\",\"admin_district\":\"Westminster\",\"codes\":{\"admin_district\":\"E09000033\"}}}", headers: {}) + + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/AA12AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"AA1 2AA\",\"admin_district\":\"Wigan\",\"codes\":{\"admin_district\":\"E08000010\"}}}", headers: {}) + + body = { results: [] }.to_json + WebMock.stub_request(:get, "https://api.os.uk/search/places/v1/find?query=Address+line+1%2C+AA1+1AB&key=OS_DATA_KEY&maxresults=10&minmatch=0.4") + .to_return(status: 200, body:, headers: {}) + + visit("/sales-logs/#{sales_log.id}/uprn") + end + + context "and uprn is known and answered" do + before do + choose "Yes" + fill_in("sales_log[uprn]", with: "111") + click_button("Save and continue") + end + + context "and uprn is confirmed" do + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(1) # yes + expect(sales_log.uprn).to eq("111") + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 1AA") + expect(sales_log.address_line1).to eq("Some Place") + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq("Bristol") + expect(sales_log.address_line1_input).to eq(nil) + expect(sales_log.postcode_full_input).to eq(nil) + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E09000033") + + choose "Yes" + click_button("Save and continue") + + sales_log.reload + expect(sales_log.uprn_known).to eq(1) # yes + expect(sales_log.uprn).to eq("111") + expect(sales_log.uprn_confirmed).to eq(1) # yes + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 1AA") + expect(sales_log.address_line1).to eq("Some Place") + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq("Bristol") + expect(sales_log.address_line1_input).to eq(nil) + expect(sales_log.postcode_full_input).to eq(nil) + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E09000033") + end + + context "and changes to uprn not known" do + it "sets correct address fields" do + visit("/sales-logs/#{sales_log.id}/uprn") + + choose "No" + click_button("Save and continue") + + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(nil) + expect(sales_log.postcode_full).to eq(nil) + expect(sales_log.address_line1).to eq(nil) + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq(nil) + expect(sales_log.address_line1_input).to eq(nil) + expect(sales_log.postcode_full_input).to eq(nil) + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq(nil) + end + end + end + + context "and uprn is not confirmed" do + before do + choose "No, I want to search for the address instead" + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(nil) + expect(sales_log.postcode_full).to eq(nil) + expect(sales_log.address_line1).to eq(nil) + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq(nil) + expect(sales_log.address_line1_input).to eq(nil) + expect(sales_log.postcode_full_input).to eq(nil) + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq(nil) + end + end + end + + context "and uprn is not known" do + before do + choose "No" + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(nil) + expect(sales_log.postcode_full).to eq(nil) + expect(sales_log.address_line1).to eq(nil) + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq(nil) + expect(sales_log.address_line1_input).to eq(nil) + expect(sales_log.postcode_full_input).to eq(nil) + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq(nil) + end + + context "and the address is not found" do + it "sets correct address fields" do + fill_in("sales_log[address_line1_input]", with: "Address line 1") + fill_in("sales_log[postcode_full_input]", with: "AA1 1AB") + click_button("Search") + + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(nil) + expect(sales_log.postcode_full).to eq(nil) + expect(sales_log.address_line1).to eq(nil) + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq(nil) + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AB") + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq(nil) + + click_button("Confirm and continue") + + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(nil) + expect(sales_log.postcode_full).to eq(nil) + expect(sales_log.address_line1).to eq(nil) + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq(nil) + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AB") + expect(sales_log.address_search_value_check).to eq(0) + expect(sales_log.la).to eq(nil) + end + end + + context "and address is found, re-searched and not found" do + before do + fill_in("sales_log[address_line1_input]", with: "Address line 1") + fill_in("sales_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + visit("/sales-logs/#{sales_log.id}/address-matcher") + + fill_in("sales_log[address_line1_input]", with: "Address line 1") + fill_in("sales_log[postcode_full_input]", with: "AA1 1AB") + click_button("Search") + end + + it "routes to the correct page" do + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/no-address-found") + end + end + + context "and the user selects 'address_not_listed'" do + before do + fill_in("sales_log[address_line1_input]", with: "Address line 1") + fill_in("sales_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + choose "The address is not listed, I want to enter the address manually" + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq("uprn_not_listed") + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 1AA") + expect(sales_log.address_line1).to eq("Address line 1") + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq(nil) + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AA") + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E09000033") + end + + context "and the user enters a new address manually" do + context "without changing a valid postcode" do + before do + fill_in("sales_log[town_or_city]", with: "Town") + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq("uprn_not_listed") + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 1AA") + expect(sales_log.address_line1).to eq("Address line 1") + expect(sales_log.address_line2).to eq("") + expect(sales_log.town_or_city).to eq("Town") + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AA") + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E09000033") + end + end + + context "with changing the postcode" do + before do + fill_in("sales_log[town_or_city]", with: "Town") + fill_in("sales_log[postcode_full]", with: "AA12AA") + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(0) # no + expect(sales_log.uprn).to eq(nil) + expect(sales_log.uprn_confirmed).to eq(nil) + expect(sales_log.uprn_selection).to eq("uprn_not_listed") + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 2AA") + expect(sales_log.address_line1).to eq("Address line 1") + expect(sales_log.address_line2).to eq("") + expect(sales_log.town_or_city).to eq("Town") + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AA") + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E08000010") + end + end + end + end + + context "and the user selects 'address_not_listed' and then changes their mind and selects an address" do + before do + fill_in("sales_log[address_line1_input]", with: "Address line 1") + fill_in("sales_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + choose "The address is not listed, I want to enter the address manually" + click_button("Save and continue") + + visit("/sales-logs/#{sales_log.id}/uprn-selection") + choose("sales-log-uprn-selection-111-field", allow_label_click: true) + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(1) + expect(sales_log.uprn).to eq("111") + expect(sales_log.uprn_confirmed).to eq(1) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 1AA") + expect(sales_log.address_line1).to eq("Some Place") + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq("Bristol") + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AA") + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E09000033") + end + end + + context "and possible addresses found and selected" do + before do + fill_in("sales_log[address_line1_input]", with: "Address line 1") + fill_in("sales_log[postcode_full_input]", with: "AA1 1AA") + click_button("Search") + choose("sales-log-uprn-selection-111-field", allow_label_click: true) + click_button("Save and continue") + end + + it "sets correct address fields" do + sales_log.reload + expect(sales_log.uprn_known).to eq(1) + expect(sales_log.uprn).to eq("111") + expect(sales_log.uprn_confirmed).to eq(1) + expect(sales_log.uprn_selection).to eq(nil) + expect(sales_log.pcodenk).to eq(0) + expect(sales_log.postcode_full).to eq("AA1 1AA") + expect(sales_log.address_line1).to eq("Some Place") + expect(sales_log.address_line2).to eq(nil) + expect(sales_log.town_or_city).to eq("Bristol") + expect(sales_log.address_line1_input).to eq("Address line 1") + expect(sales_log.postcode_full_input).to eq("AA1 1AA") + expect(sales_log.address_search_value_check).to eq(nil) + expect(sales_log.la).to eq("E09000033") + end + end + end + end end context "when a log becomes a duplicate" do From 84177349f0d13b12463a07cfdf953c37d42df075 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 4 Dec 2024 12:21:09 +0000 Subject: [PATCH 03/19] Refactor exports to have years (#2849) * Refactor exports to have years * Update existing exports * Fix flaky test * Remove a comment --- app/models/export.rb | 3 ++ app/services/exports/export_service.rb | 10 ++--- .../exports/lettings_log_export_service.rb | 22 +++++----- .../exports/organisation_export_service.rb | 12 +++--- app/services/exports/user_export_service.rb | 12 +++--- app/services/exports/xml_export_service.rb | 20 ++++----- .../20241204100518_add_year_to_export.rb | 5 +++ db/schema.rb | 3 +- lib/tasks/data_export.rake | 6 +-- lib/tasks/set_export_collection_years.rake | 8 ++++ spec/helpers/tab_nav_helper_spec.rb | 2 +- spec/lib/tasks/data_export_spec.rb | 6 +-- .../tasks/set_export_collection_years_spec.rb | 41 +++++++++++++++++++ .../lettings_log_export_service_spec.rb | 18 ++++---- 14 files changed, 111 insertions(+), 57 deletions(-) create mode 100644 db/migrate/20241204100518_add_year_to_export.rb create mode 100644 lib/tasks/set_export_collection_years.rake create mode 100644 spec/lib/tasks/set_export_collection_years_spec.rb diff --git a/app/models/export.rb b/app/models/export.rb index 2ee04fe3d..27e574c72 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -1,2 +1,5 @@ class Export < ApplicationRecord + scope :lettings, -> { where(collection: "lettings") } + scope :organisations, -> { where(collection: "organisations") } + scope :users, -> { where(collection: "users") } end diff --git a/app/services/exports/export_service.rb b/app/services/exports/export_service.rb index bf98a4edf..40c10055b 100644 --- a/app/services/exports/export_service.rb +++ b/app/services/exports/export_service.rb @@ -7,7 +7,7 @@ module Exports @logger = logger end - def export_xml(full_update: false, collection: nil) + def export_xml(full_update: false, collection: nil, year: nil) start_time = Time.zone.now daily_run_number = get_daily_run_number lettings_archives_for_manifest = {} @@ -21,12 +21,12 @@ module Exports when "organisations" organisations_archives_for_manifest = get_organisation_archives(start_time, full_update) else - lettings_archives_for_manifest = get_lettings_archives(start_time, full_update, collection) + lettings_archives_for_manifest = get_lettings_archives(start_time, full_update, year) end else users_archives_for_manifest = get_user_archives(start_time, full_update) organisations_archives_for_manifest = get_organisation_archives(start_time, full_update) - lettings_archives_for_manifest = get_lettings_archives(start_time, full_update, collection) + lettings_archives_for_manifest = get_lettings_archives(start_time, full_update, year) end write_master_manifest(daily_run_number, lettings_archives_for_manifest.merge(users_archives_for_manifest).merge(organisations_archives_for_manifest)) @@ -70,9 +70,9 @@ module Exports organisations_export_service.export_xml_organisations(full_update:) end - def get_lettings_archives(start_time, full_update, collection) + def get_lettings_archives(start_time, full_update, collection_year) lettings_export_service = Exports::LettingsLogExportService.new(@storage_service, start_time) - lettings_export_service.export_xml_lettings_logs(full_update:, collection_year: collection) + lettings_export_service.export_xml_lettings_logs(full_update:, collection_year:) end end end diff --git a/app/services/exports/lettings_log_export_service.rb b/app/services/exports/lettings_log_export_service.rb index 97f495e0c..a8877eac3 100644 --- a/app/services/exports/lettings_log_export_service.rb +++ b/app/services/exports/lettings_log_export_service.rb @@ -5,11 +5,11 @@ module Exports def export_xml_lettings_logs(full_update: false, collection_year: nil) archives_for_manifest = {} - collection_years_to_export(collection_year).each do |collection| - recent_export = Export.where(collection:).order("started_at").last - base_number = Export.where(empty_export: false, collection:).maximum(:base_number) || 1 - export = build_export_run(collection, base_number, full_update) - archives = write_export_archive(export, collection, recent_export, full_update) + collection_years_to_export(collection_year).each do |year| + recent_export = Export.lettings.where(year:).order("started_at").last + base_number = Export.lettings.where(empty_export: false, year:).maximum(:base_number) || 1 + export = build_export_run("lettings", base_number, full_update, year) + archives = write_export_archive(export, year, recent_export, full_update) archives_for_manifest.merge!(archives) @@ -22,21 +22,21 @@ module Exports private - def get_archive_name(collection, base_number, increment) - return unless collection + def get_archive_name(year, base_number, increment) + return unless year base_number_str = "f#{base_number.to_s.rjust(4, '0')}" increment_str = "inc#{increment.to_s.rjust(4, '0')}" - "core_#{collection}_#{collection + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase + "core_#{year}_#{year + 1}_apr_mar_#{base_number_str}_#{increment_str}".downcase end - def retrieve_resources(recent_export, full_update, collection) + def retrieve_resources(recent_export, full_update, year) if !full_update && recent_export params = { from: recent_export.started_at, to: @start_time } - LettingsLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(collection) + LettingsLog.exportable.where("(updated_at >= :from AND updated_at <= :to) OR (values_updated_at IS NOT NULL AND values_updated_at >= :from AND values_updated_at <= :to)", params).filter_by_year(year) else params = { to: @start_time } - LettingsLog.exportable.where("updated_at <= :to", params).filter_by_year(collection) + LettingsLog.exportable.where("updated_at <= :to", params).filter_by_year(year) end end diff --git a/app/services/exports/organisation_export_service.rb b/app/services/exports/organisation_export_service.rb index 71eccf60a..2b2da71c8 100644 --- a/app/services/exports/organisation_export_service.rb +++ b/app/services/exports/organisation_export_service.rb @@ -5,9 +5,9 @@ module Exports def export_xml_organisations(full_update: false) collection = "organisations" - recent_export = Export.where(collection:).order("started_at").last + recent_export = Export.organisations.order("started_at").last - base_number = Export.where(empty_export: false, collection:).maximum(:base_number) || 1 + base_number = Export.organisations.where(empty_export: false).maximum(:base_number) || 1 export = build_export_run(collection, base_number, full_update) archives_for_manifest = write_export_archive(export, collection, recent_export, full_update) @@ -19,15 +19,13 @@ module Exports private - def get_archive_name(collection, base_number, increment) - return unless collection - + def get_archive_name(_year, base_number, increment) base_number_str = "f#{base_number.to_s.rjust(4, '0')}" increment_str = "inc#{increment.to_s.rjust(4, '0')}" - "#{collection}_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase + "organisations_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase end - def retrieve_resources(recent_export, full_update, _collection) + def retrieve_resources(recent_export, full_update, _year) if !full_update && recent_export params = { from: recent_export.started_at, to: @start_time } Organisation.where("(updated_at >= :from AND updated_at <= :to)", params) diff --git a/app/services/exports/user_export_service.rb b/app/services/exports/user_export_service.rb index 907a1cc86..aaa30c424 100644 --- a/app/services/exports/user_export_service.rb +++ b/app/services/exports/user_export_service.rb @@ -5,9 +5,9 @@ module Exports def export_xml_users(full_update: false) collection = "users" - recent_export = Export.where(collection:).order("started_at").last + recent_export = Export.users.order("started_at").last - base_number = Export.where(empty_export: false, collection:).maximum(:base_number) || 1 + base_number = Export.users.where(empty_export: false).maximum(:base_number) || 1 export = build_export_run(collection, base_number, full_update) archives_for_manifest = write_export_archive(export, collection, recent_export, full_update) @@ -19,15 +19,13 @@ module Exports private - def get_archive_name(collection, base_number, increment) - return unless collection - + def get_archive_name(_year, base_number, increment) base_number_str = "f#{base_number.to_s.rjust(4, '0')}" increment_str = "inc#{increment.to_s.rjust(4, '0')}" - "#{collection}_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase + "users_2024_2025_apr_mar_#{base_number_str}_#{increment_str}".downcase end - def retrieve_resources(recent_export, full_update, _collection) + def retrieve_resources(recent_export, full_update, _year) if !full_update && recent_export params = { from: recent_export.started_at, to: @start_time } User.where("(updated_at >= :from AND updated_at <= :to)", params) diff --git a/app/services/exports/xml_export_service.rb b/app/services/exports/xml_export_service.rb index 009a1b306..d499dd518 100644 --- a/app/services/exports/xml_export_service.rb +++ b/app/services/exports/xml_export_service.rb @@ -11,9 +11,9 @@ module Exports private - def build_export_run(collection, base_number, full_update) - @logger.info("Building export run for #{collection}") - previous_exports_with_data = Export.where(collection:, empty_export: false) + def build_export_run(collection, base_number, full_update, year = nil) + @logger.info("Building export run for #{[collection, year].join(' ')}") + previous_exports_with_data = Export.where(collection:, year:, empty_export: false) increment_number = previous_exports_with_data.where(base_number:).maximum(:increment_number) || 1 @@ -25,16 +25,16 @@ module Exports end if previous_exports_with_data.empty? - return Export.new(collection:, base_number:, started_at: @start_time) + return Export.new(collection:, year:, base_number:, started_at: @start_time) end - Export.new(collection:, started_at: @start_time, base_number:, increment_number:) + Export.new(collection:, year:, started_at: @start_time, base_number:, increment_number:) end - def write_export_archive(export, collection, recent_export, full_update) - archive = get_archive_name(collection, export.base_number, export.increment_number) # archive name would be the same for all logs because they're already filtered by year (?) + def write_export_archive(export, year, recent_export, full_update) + archive = get_archive_name(year, export.base_number, export.increment_number) - initial_count = retrieve_resources(recent_export, full_update, collection).count + initial_count = retrieve_resources(recent_export, full_update, year).count @logger.info("Creating #{archive} - #{initial_count} resources") return {} if initial_count.zero? @@ -46,12 +46,12 @@ module Exports loop do slice = if last_processed_marker.present? - retrieve_resources(recent_export, full_update, collection) + retrieve_resources(recent_export, full_update, year) .where("created_at > ?", last_processed_marker) .order(:created_at) .limit(MAX_XML_RECORDS).to_a else - retrieve_resources(recent_export, full_update, collection) + retrieve_resources(recent_export, full_update, year) .order(:created_at) .limit(MAX_XML_RECORDS).to_a end diff --git a/db/migrate/20241204100518_add_year_to_export.rb b/db/migrate/20241204100518_add_year_to_export.rb new file mode 100644 index 000000000..784754888 --- /dev/null +++ b/db/migrate/20241204100518_add_year_to_export.rb @@ -0,0 +1,5 @@ +class AddYearToExport < ActiveRecord::Migration[7.0] + def change + add_column :exports, :year, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 0cdc15e9f..d31a54da2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_11_22_154743) do +ActiveRecord::Schema[7.0].define(version: 2024_12_04_100518) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -113,6 +113,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_11_22_154743) do t.integer "increment_number", default: 1, null: false t.boolean "empty_export", default: false, null: false t.string "collection" + t.integer "year" end create_table "la_rent_ranges", force: :cascade do |t| diff --git a/lib/tasks/data_export.rake b/lib/tasks/data_export.rake index 7a9a90bc8..43ef06374 100644 --- a/lib/tasks/data_export.rake +++ b/lib/tasks/data_export.rake @@ -7,13 +7,13 @@ namespace :core do end desc "Export all data XMLs for import into Central Data System (CDS)" - task :full_data_export_xml, %i[collection] => :environment do |_task, args| + task :full_data_export_xml, %i[collection year] => :environment do |_task, args| collection = args[:collection].presence - collection = collection.to_i if collection.present? && collection.scan(/\D/).empty? + year = args[:year]&.to_i storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["EXPORT_BUCKET"]) export_service = Exports::ExportService.new(storage_service) - export_service.export_xml(full_update: true, collection:) + export_service.export_xml(full_update: true, collection:, year:) end end diff --git a/lib/tasks/set_export_collection_years.rake b/lib/tasks/set_export_collection_years.rake new file mode 100644 index 000000000..badaab0eb --- /dev/null +++ b/lib/tasks/set_export_collection_years.rake @@ -0,0 +1,8 @@ +desc "Set export collection years for lettings exports" +task set_export_collection_years: :environment do + Export.where(collection: %w[2022 2023 2024 2025]).find_each do |export| + export.year = export.collection.to_i + export.collection = "lettings" + export.save! + end +end diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb index 89f867775..bd29f4c46 100644 --- a/spec/helpers/tab_nav_helper_spec.rb +++ b/spec/helpers/tab_nav_helper_spec.rb @@ -9,7 +9,7 @@ RSpec.describe TabNavHelper do describe "#user_cell" do it "returns user link and email separated by a newline character" do expected_html = "#{current_user.name}\nUser #{current_user.email}" - expect(user_cell(current_user)).to match(expected_html) + expect(CGI.unescapeHTML(user_cell(current_user))).to match(expected_html) end end diff --git a/spec/lib/tasks/data_export_spec.rb b/spec/lib/tasks/data_export_spec.rb index 8b5dd5fbe..2f6127512 100644 --- a/spec/lib/tasks/data_export_spec.rb +++ b/spec/lib/tasks/data_export_spec.rb @@ -30,7 +30,7 @@ describe "rake core:data_export", type: task do context "with all available years" do it "calls the export service" do - expect(export_service).to receive(:export_xml).with(full_update: true, collection: nil) + expect(export_service).to receive(:export_xml).with(full_update: true, collection: nil, year: nil) task.invoke end @@ -38,9 +38,9 @@ describe "rake core:data_export", type: task do context "with a specific collection" do it "calls the export service" do - expect(export_service).to receive(:export_xml).with(full_update: true, collection: 2022) + expect(export_service).to receive(:export_xml).with(full_update: true, collection: "lettings", year: 2022) - task.invoke("2022") + task.invoke("lettings", "2022") end end end diff --git a/spec/lib/tasks/set_export_collection_years_spec.rb b/spec/lib/tasks/set_export_collection_years_spec.rb new file mode 100644 index 000000000..76ff779b0 --- /dev/null +++ b/spec/lib/tasks/set_export_collection_years_spec.rb @@ -0,0 +1,41 @@ +require "rails_helper" +require "rake" + +RSpec.describe "set_export_collection_years" do + describe ":set_export_collection_years", type: :task do + subject(:task) { Rake::Task["set_export_collection_years"] } + + before do + Rake.application.rake_require("tasks/set_export_collection_years") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the rake task is run" do + let!(:lettings_export_2023) { Export.create(collection: "2023", year: nil, started_at: Time.zone.now) } + let!(:lettings_export_2024) { Export.create(collection: "2024", year: nil, started_at: Time.zone.now) } + let!(:updated_lettings_export) { Export.create(collection: "lettings", year: 2023, started_at: Time.zone.now) } + let!(:organisations_export) { Export.create(collection: "organisations", year: nil, started_at: Time.zone.now) } + let!(:users_export) { Export.create(collection: "users", year: nil, started_at: Time.zone.now) } + + it "correctly updates collection years" do + task.invoke + + expect(lettings_export_2023.reload.collection).to eq("lettings") + expect(lettings_export_2023.year).to eq(2023) + + expect(lettings_export_2024.reload.collection).to eq("lettings") + expect(lettings_export_2024.year).to eq(2024) + + expect(updated_lettings_export.reload.collection).to eq("lettings") + expect(updated_lettings_export.year).to eq(2023) + + expect(organisations_export.reload.collection).to eq("organisations") + expect(organisations_export.year).to eq(nil) + + expect(users_export.reload.collection).to eq("users") + expect(users_export.year).to eq(nil) + end + end + end +end diff --git a/spec/services/exports/lettings_log_export_service_spec.rb b/spec/services/exports/lettings_log_export_service_spec.rb index 6a07af8dd..8c123b47e 100644 --- a/spec/services/exports/lettings_log_export_service_spec.rb +++ b/spec/services/exports/lettings_log_export_service_spec.rb @@ -207,15 +207,15 @@ RSpec.describe Exports::LettingsLogExportService do it "generates multiple ZIP export files with the expected filenames" do expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) expect(storage_service).to receive(:write_file).with(expected_zip_filename2, any_args) - expect(Rails.logger).to receive(:info).with("Building export run for 2021") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2021") expect(Rails.logger).to receive(:info).with("Creating core_2021_2022_apr_mar_f0001_inc0001 - 1 resources") expect(Rails.logger).to receive(:info).with("Added core_2021_2022_apr_mar_f0001_inc0001_pt001.xml") expect(Rails.logger).to receive(:info).with("Writing core_2021_2022_apr_mar_f0001_inc0001.zip") - expect(Rails.logger).to receive(:info).with("Building export run for 2022") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2022") expect(Rails.logger).to receive(:info).with("Creating core_2022_2023_apr_mar_f0001_inc0001 - 1 resources") expect(Rails.logger).to receive(:info).with("Added core_2022_2023_apr_mar_f0001_inc0001_pt001.xml") expect(Rails.logger).to receive(:info).with("Writing core_2022_2023_apr_mar_f0001_inc0001.zip") - expect(Rails.logger).to receive(:info).with("Building export run for 2023") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2023") expect(Rails.logger).to receive(:info).with("Creating core_2023_2024_apr_mar_f0001_inc0001 - 0 resources") export_service.export_xml_lettings_logs @@ -223,7 +223,7 @@ RSpec.describe Exports::LettingsLogExportService do it "generates zip export files only for specified year" do expect(storage_service).to receive(:write_file).with(expected_zip_filename2, any_args) - expect(Rails.logger).to receive(:info).with("Building export run for 2022") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2022") expect(Rails.logger).to receive(:info).with("Creating core_2022_2023_apr_mar_f0001_inc0001 - 1 resources") expect(Rails.logger).to receive(:info).with("Added core_2022_2023_apr_mar_f0001_inc0001_pt001.xml") expect(Rails.logger).to receive(:info).with("Writing core_2022_2023_apr_mar_f0001_inc0001.zip") @@ -236,21 +236,21 @@ RSpec.describe Exports::LettingsLogExportService do let(:expected_zip_filename2) { "core_2022_2023_apr_mar_f0001_inc0001.zip" } before do - Export.new(started_at: Time.zone.yesterday, base_number: 7, increment_number: 3, collection: 2021).save! + Export.new(started_at: Time.zone.yesterday, base_number: 7, increment_number: 3, collection: "lettings", year: 2021).save! end it "generates multiple ZIP export files with different base numbers in the filenames" do expect(storage_service).to receive(:write_file).with(expected_zip_filename, any_args) expect(storage_service).to receive(:write_file).with(expected_zip_filename2, any_args) - expect(Rails.logger).to receive(:info).with("Building export run for 2021") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2021") expect(Rails.logger).to receive(:info).with("Creating core_2021_2022_apr_mar_f0007_inc0004 - 1 resources") expect(Rails.logger).to receive(:info).with("Added core_2021_2022_apr_mar_f0007_inc0004_pt001.xml") expect(Rails.logger).to receive(:info).with("Writing core_2021_2022_apr_mar_f0007_inc0004.zip") - expect(Rails.logger).to receive(:info).with("Building export run for 2022") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2022") expect(Rails.logger).to receive(:info).with("Creating core_2022_2023_apr_mar_f0001_inc0001 - 1 resources") expect(Rails.logger).to receive(:info).with("Added core_2022_2023_apr_mar_f0001_inc0001_pt001.xml") expect(Rails.logger).to receive(:info).with("Writing core_2022_2023_apr_mar_f0001_inc0001.zip") - expect(Rails.logger).to receive(:info).with("Building export run for 2023") + expect(Rails.logger).to receive(:info).with("Building export run for lettings 2023") expect(Rails.logger).to receive(:info).with("Creating core_2023_2024_apr_mar_f0001_inc0001 - 0 resources") export_service.export_xml_lettings_logs @@ -329,7 +329,7 @@ RSpec.describe Exports::LettingsLogExportService do context "when this is a second export (partial)" do before do start_time = Time.zone.local(2022, 6, 1) - Export.new(started_at: start_time, collection: 2021).save! + Export.new(started_at: start_time, collection: "lettings", year: 2021).save! end it "does not add any entry for the master manifest (no lettings logs)" do From 9343d22768d2496912f76a9fdcbcb8be6a60b124 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:00:55 +0000 Subject: [PATCH 04/19] CLDC-3335 Adjust BU errors for addresses (#2829) * Adjust BU errors for addresses * typo * Update error messages --- .../lettings/year2024/row_parser.rb | 20 +++++++++++++------ .../bulk_upload/sales/year2024/row_parser.rb | 20 +++++++++++++------ .../lettings/2024/bulk_upload.en.yml | 2 ++ .../validations/sales/2024/bulk_upload.en.yml | 2 ++ .../lettings/year2024/row_parser_spec.rb | 16 +++++++-------- .../sales/year2024/row_parser_spec.rb | 18 ++++++++--------- 6 files changed, 49 insertions(+), 29 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index ef7433614..6315274ce 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -607,14 +607,22 @@ private def validate_uprn_exists_if_any_key_address_fields_are_blank if field_16.blank? && !key_address_fields_provided? - errors.add(:field_16, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "UPRN.")) + %i[field_17 field_19 field_21 field_22].each do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.address.not_answered")) if send(field).blank? + end + errors.add(:field_16, I18n.t("#{ERROR_BASE_KEY}.address.not_answered", question: "UPRN.")) end end def validate_address_option_found if log.uprn.nil? && field_16.blank? && key_address_fields_provided? + error_message = if log.address_options_present? + I18n.t("#{ERROR_BASE_KEY}.address.not_determined") + else + I18n.t("#{ERROR_BASE_KEY}.address.not_found") + end %i[field_17 field_18 field_19 field_20 field_21 field_22].each do |field| - errors.add(field, I18n.t("#{ERROR_BASE_KEY}.address.not_found")) + errors.add(field, error_message) if errors[field].blank? end end end @@ -625,19 +633,19 @@ private def validate_address_fields if field_16.blank? || log.errors.attribute_names.include?(:uprn) - if field_17.blank? + if field_17.blank? && errors[:field_17].blank? errors.add(:field_17, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "address line 1.")) end - if field_19.blank? + if field_19.blank? && errors[:field_19].blank? errors.add(:field_19, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "town or city.")) end - if field_21.blank? + if field_21.blank? && errors[:field_21].blank? errors.add(:field_21, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "part 1 of postcode.")) end - if field_22.blank? + if field_22.blank? && errors[:field_22].blank? errors.add(:field_22, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "part 2 of postcode.")) end end diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index fdef65aec..0e7b72d6f 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -605,14 +605,22 @@ private def validate_uprn_exists_if_any_key_address_fields_are_blank if field_22.blank? && !key_address_fields_provided? - errors.add(:field_22, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "UPRN.")) + %i[field_23 field_25 field_27 field_28].each do |field| + errors.add(field, I18n.t("#{ERROR_BASE_KEY}.address.not_answered")) if send(field).blank? + end + errors.add(:field_22, I18n.t("#{ERROR_BASE_KEY}.address.not_answered", question: "UPRN.")) end end def validate_address_option_found if log.uprn.nil? && field_22.blank? && key_address_fields_provided? + error_message = if log.address_options_present? + I18n.t("#{ERROR_BASE_KEY}.address.not_determined") + else + I18n.t("#{ERROR_BASE_KEY}.address.not_found") + end %i[field_23 field_24 field_25 field_26 field_27 field_28].each do |field| - errors.add(field, I18n.t("#{ERROR_BASE_KEY}.address.not_found")) + errors.add(field, error_message) if errors[field].blank? end end end @@ -623,19 +631,19 @@ private def validate_address_fields if field_22.blank? || log.errors.attribute_names.include?(:uprn) - if field_23.blank? + if field_23.blank? && errors[:field_23].blank? errors.add(:field_23, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "address line 1.")) end - if field_25.blank? + if field_25.blank? && errors[:field_25].blank? errors.add(:field_25, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "town or city.")) end - if field_27.blank? + if field_27.blank? && errors[:field_27].blank? errors.add(:field_27, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "part 1 of postcode.")) end - if field_28.blank? + if field_28.blank? && errors[:field_28].blank? errors.add(:field_28, I18n.t("#{ERROR_BASE_KEY}.not_answered", question: "part 2 of postcode.")) end end diff --git a/config/locales/validations/lettings/2024/bulk_upload.en.yml b/config/locales/validations/lettings/2024/bulk_upload.en.yml index 76985ee32..a16090543 100644 --- a/config/locales/validations/lettings/2024/bulk_upload.en.yml +++ b/config/locales/validations/lettings/2024/bulk_upload.en.yml @@ -50,6 +50,8 @@ en: invalid: "Age of person %{person_num} must be a number or the letter R" address: not_found: "We could not find this address. Check the address data in your CSV file is correct and complete, or select the correct address using the CORE site." + not_determined: "There are multiple matches for this address. Either select the correct address manually or correct the UPRN in the CSV file." + not_answered: "Enter either the UPRN or the full address." nationality: invalid: "Select a valid nationality." charges: diff --git a/config/locales/validations/sales/2024/bulk_upload.en.yml b/config/locales/validations/sales/2024/bulk_upload.en.yml index fc68662d1..2621386c1 100644 --- a/config/locales/validations/sales/2024/bulk_upload.en.yml +++ b/config/locales/validations/sales/2024/bulk_upload.en.yml @@ -40,5 +40,7 @@ en: buyer_cannot_be_over_16_and_child: "Buyer 2's age cannot be 16 or over if their working situation is child under 16." address: not_found: "We could not find this address. Check the address data in your CSV file is correct and complete, or select the correct address using the CORE site." + not_determined: "There are multiple matches for this address. Either select the correct address manually or correct the UPRN in the CSV file." + not_answered: "Enter either the UPRN or the full address." nationality: invalid: "Select a valid nationality." 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 e9047b2ae..13ab91370 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -1658,11 +1658,11 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do it "adds appropriate errors to UPRN and key address fields" do parser.valid? - expect(parser.errors[:field_16]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "UPRN.")]) - expect(parser.errors[:field_17]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "address line 1.")]) - expect(parser.errors[:field_19]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "town or city.")]) - expect(parser.errors[:field_21]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "part 1 of postcode.")]) - expect(parser.errors[:field_22]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "part 2 of postcode.")]) + expect(parser.errors[:field_16]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_17]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_19]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_21]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_22]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) end end @@ -1671,8 +1671,8 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do it "adds errors to UPRN and the missing key address field" do parser.valid? - expect(parser.errors[:field_16]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "UPRN.")]) - expect(parser.errors[:field_17]).to eql([I18n.t("validations.lettings.2024.bulk_upload.not_answered", question: "address line 1.")]) + expect(parser.errors[:field_16]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_17]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_answered")]) expect(parser.errors[:field_19]).to be_empty expect(parser.errors[:field_21]).to be_empty expect(parser.errors[:field_22]).to be_empty @@ -1721,7 +1721,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do parser.valid? expect(parser.errors[:field_16]).to be_empty %i[field_17 field_18 field_19 field_20 field_21 field_22].each do |field| - expect(parser.errors[field]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_found")]) + expect(parser.errors[field]).to eql([I18n.t("validations.lettings.2024.bulk_upload.address.not_determined")]) end 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 e4e2eb6f5..7e969c10e 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -310,7 +310,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do it "only has one error added to the field" do parser.valid? - expect(parser.errors[:field_23]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "address line 1.")]) + expect(parser.errors[:field_23]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) end end @@ -1042,11 +1042,11 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do it "adds appropriate errors to UPRN and key address fields" do parser.valid? - expect(parser.errors[:field_22]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "UPRN.")]) - expect(parser.errors[:field_23]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "address line 1.")]) - expect(parser.errors[:field_25]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "town or city.")]) - expect(parser.errors[:field_27]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "part 1 of postcode.")]) - expect(parser.errors[:field_28]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "part 2 of postcode.")]) + expect(parser.errors[:field_22]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_23]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_25]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_27]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_28]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) end end @@ -1055,8 +1055,8 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do it "adds errors to UPRN and the missing key address field" do parser.valid? - expect(parser.errors[:field_22]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "UPRN.")]) - expect(parser.errors[:field_23]).to eql([I18n.t("validations.sales.2024.bulk_upload.not_answered", question: "address line 1.")]) + expect(parser.errors[:field_22]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) + expect(parser.errors[:field_23]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_answered")]) expect(parser.errors[:field_25]).to be_empty expect(parser.errors[:field_27]).to be_empty expect(parser.errors[:field_28]).to be_empty @@ -1105,7 +1105,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do parser.valid? expect(parser.errors[:field_22]).to be_empty %i[field_23 field_24 field_25 field_26 field_27 field_28].each do |field| - expect(parser.errors[field]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_found")]) + expect(parser.errors[field]).to eql([I18n.t("validations.sales.2024.bulk_upload.address.not_determined")]) end end end From 918cf0f08f832aadf15cad9a51c541b7eb8554d7 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:54:53 +0000 Subject: [PATCH 05/19] CLDC-2434 Fix test warnings and update some flaky tests (#2850) * Remove test warnings * Fix some flaky tests --- spec/requests/lettings_logs_controller_spec.rb | 2 +- .../organisation_relationships_controller_spec.rb | 8 ++++---- spec/requests/organisations_controller_spec.rb | 6 +++--- spec/requests/sales_logs_controller_spec.rb | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 3d2c27400..e795b76ea 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -1483,7 +1483,7 @@ RSpec.describe LettingsLogsController, type: :request do end context "when viewing a collection of logs affected by deactivated location" do - let!(:affected_lettings_logs) { FactoryBot.create_list(:lettings_log, 3, unresolved: true, assigned_to: user) } + let!(:affected_lettings_logs) { FactoryBot.create_list(:lettings_log, 3, unresolved: true, assigned_to: user, tenancycode: "affected tenancycode", propcode: "affected propcode") } let!(:other_user_affected_lettings_log) { FactoryBot.create(:lettings_log, unresolved: true) } let!(:non_affected_lettings_logs) { FactoryBot.create_list(:lettings_log, 4, assigned_to: user) } let(:other_user) { FactoryBot.create(:user, organisation: user.organisation) } diff --git a/spec/requests/organisation_relationships_controller_spec.rb b/spec/requests/organisation_relationships_controller_spec.rb index feb687e0c..79706fff9 100644 --- a/spec/requests/organisation_relationships_controller_spec.rb +++ b/spec/requests/organisation_relationships_controller_spec.rb @@ -40,7 +40,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do it "shows a table of stock owners" do expected_html = " Date: Wed, 4 Dec 2024 16:05:05 +0000 Subject: [PATCH 06/19] CLDC-3314: Skip BU error for ownershipsch in validate_discounted_ownership_value (#2837) --- .../validations/sales/sale_information_validations.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb index 3825271c5..4b8948053 100644 --- a/app/models/validations/sales/sale_information_validations.rb +++ b/app/models/validations/sales/sale_information_validations.rb @@ -64,7 +64,7 @@ module Validations::Sales::SaleInformationValidations if over_tolerance?(record.mortgage_deposit_and_grant_total, record.value_with_discount, tolerance, strict: !record.discount.nil?) && record.discounted_ownership_sale? deposit_and_grant_sentence = record.grant.present? ? ", cash deposit (#{record.field_formatted_as_currency('deposit')}), and grant (#{record.field_formatted_as_currency('grant')})" : " and cash deposit (#{record.field_formatted_as_currency('deposit')})" discount_sentence = record.discount.present? ? " (#{record.field_formatted_as_currency('value')}) subtracted by the sum of the full purchase price (#{record.field_formatted_as_currency('value')}) multiplied by the percentage discount (#{record.discount}%)" : "" - %i[mortgageused mortgage value deposit ownershipsch discount grant].each do |field| + %i[mortgageused mortgage value deposit discount grant].each do |field| record.errors.add field, I18n.t("validations.sales.sale_information.#{field}.discounted_ownership_value", mortgage: record.mortgage&.positive? ? " (#{record.field_formatted_as_currency('mortgage')})" : "", deposit_and_grant_sentence:, @@ -72,6 +72,12 @@ module Validations::Sales::SaleInformationValidations discount_sentence:, value_with_discount: record.field_formatted_as_currency("value_with_discount")).html_safe end + record.errors.add :ownershipsch, :skip_bu_error, message: I18n.t("validations.sales.sale_information.ownershipsch.discounted_ownership_value", + mortgage: record.mortgage&.positive? ? " (#{record.field_formatted_as_currency('mortgage')})" : "", + deposit_and_grant_sentence:, + mortgage_deposit_and_grant_total: record.field_formatted_as_currency("mortgage_deposit_and_grant_total"), + discount_sentence:, + value_with_discount: record.field_formatted_as_currency("value_with_discount")).html_safe end end From 1b2100bc8fc3729ce409ed5aba7a9a7c38925c9f Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:06:25 +0000 Subject: [PATCH 07/19] CLDC-3093: Update content inconsistencies and typos (#2846) * Update copy * Update sale_information.en.yml * Update sale_information.en.yml * Fixes * Update test * Update test * Update test --- app/models/form/sales/questions/equity.rb | 2 +- app/models/form/sales/questions/value.rb | 2 +- .../lettings/household_characteristics.en.yml | 38 ++++++------ .../2023/lettings/income_and_benefits.en.yml | 2 +- .../sales/household_characteristics.en.yml | 58 +++++++++---------- .../sales/income_benefits_and_savings.en.yml | 12 ++-- .../forms/2023/sales/sale_information.en.yml | 24 ++++---- .../lettings/household_characteristics.en.yml | 8 +-- .../2024/lettings/income_and_benefits.en.yml | 2 +- .../sales/household_characteristics.en.yml | 58 +++++++++---------- .../sales/income_benefits_and_savings.en.yml | 8 +-- .../forms/2024/sales/sale_information.en.yml | 24 ++++---- .../lettings/household_characteristics.en.yml | 2 +- .../2025/lettings/income_and_benefits.en.yml | 2 +- .../sales/household_characteristics.en.yml | 20 +++---- .../sales/income_benefits_and_savings.en.yml | 2 +- .../forms/2025/sales/sale_information.en.yml | 8 +-- config/locales/forms/2025/sales/setup.en.yml | 2 +- spec/models/form/sales/pages/equity_spec.rb | 4 ++ .../pages/value_shared_ownership_spec.rb | 4 ++ .../form/sales/questions/equity_spec.rb | 4 ++ 21 files changed, 149 insertions(+), 137 deletions(-) diff --git a/app/models/form/sales/questions/equity.rb b/app/models/form/sales/questions/equity.rb index e39e77ebb..74f5e9970 100644 --- a/app/models/form/sales/questions/equity.rb +++ b/app/models/form/sales/questions/equity.rb @@ -2,7 +2,7 @@ class Form::Sales::Questions::Equity < ::Form::Question def initialize(id, hsh, page) super @id = "equity" - @copy_key = "sales.sale_information.equity.#{page.id}" + @copy_key = form.start_year_2025_or_later? ? "sales.sale_information.equity.#{page.id}" : "sales.sale_information.equity" @type = "numeric" @min = 0 @max = 100 diff --git a/app/models/form/sales/questions/value.rb b/app/models/form/sales/questions/value.rb index ad021e920..ce8d07e28 100644 --- a/app/models/form/sales/questions/value.rb +++ b/app/models/form/sales/questions/value.rb @@ -2,7 +2,7 @@ class Form::Sales::Questions::Value < ::Form::Question def initialize(id, hsh, page) super @id = "value" - @copy_key = "sales.sale_information.value.#{page.id}" + @copy_key = form.start_year_2025_or_later? ? "sales.sale_information.value.#{page.id}" : "sales.sale_information.value" @type = "numeric" @min = 0 @step = 1 diff --git a/config/locales/forms/2023/lettings/household_characteristics.en.yml b/config/locales/forms/2023/lettings/household_characteristics.en.yml index 1f175adbc..3de0745b1 100644 --- a/config/locales/forms/2023/lettings/household_characteristics.en.yml +++ b/config/locales/forms/2023/lettings/household_characteristics.en.yml @@ -19,23 +19,23 @@ en: page_header: "" age1_known: check_answer_label: "" - hint_text: "The ’lead’ or ’main’ tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." + hint_text: "The lead tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." question_text: "Do you know the lead tenant’s age?" age1: check_answer_label: "Lead tenant’s age" hint_text: "" question_text: "Age" - + sex1: page_header: "" check_answer_label: "Lead tenant’s gender identity" hint_text: "The lead tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." question_text: "Which of these best describes the lead tenant’s gender identity?" - + ethnic_group: page_header: "" check_answer_label: "Lead tenant’s ethnic group" - hint_text: "The lead tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." + hint_text: "The lead tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." question_text: "What is the lead tenant’s ethnic group?" ethnic: @@ -97,13 +97,13 @@ en: question_text: "Do you know person 2’s age?" age2: check_answer_label: "Person 2’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex2: page_header: "" check_answer_label: "Person 2’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 2’s gender identity?" ecstat2: @@ -132,13 +132,13 @@ en: question_text: "Do you know person 3’s age?" age3: check_answer_label: "Person 3’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex3: page_header: "" check_answer_label: "Person 3’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 3’s gender identity?" ecstat3: @@ -167,13 +167,13 @@ en: question_text: "Do you know person 4’s age?" age4: check_answer_label: "Person 4’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex4: page_header: "" check_answer_label: "Person 4’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 4’s gender identity?" ecstat4: @@ -202,13 +202,13 @@ en: question_text: "Do you know person 5’s age?" age5: check_answer_label: "Person 5’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex5: page_header: "" check_answer_label: "Person 5’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 5’s gender identity?" ecstat5: @@ -237,13 +237,13 @@ en: question_text: "Do you know person 6’s age?" age6: check_answer_label: "Person 6’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex6: page_header: "" check_answer_label: "Person 6’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 6’s gender identity?" ecstat6: @@ -272,13 +272,13 @@ en: question_text: "Do you know person 7’s age?" age7: check_answer_label: "Person 7’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex7: page_header: "" check_answer_label: "Person 7’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 7’s gender identity?" ecstat7: @@ -307,17 +307,17 @@ en: question_text: "Do you know person 8’s age?" age8: check_answer_label: "Person 8’s age" - hint_text: "" + hint_text: "" question_text: "Age" sex8: page_header: "" check_answer_label: "Person 8’s gender identity" - hint_text: "" + hint_text: "" question_text: "Which of these best describes person 8’s gender identity?" ecstat8: page_header: "" check_answer_label: "Person 8’s working situation" hint_text: "" - question_text: "Which of these best describes person 8’s working situation?" \ No newline at end of file + question_text: "Which of these best describes person 8’s working situation?" diff --git a/config/locales/forms/2023/lettings/income_and_benefits.en.yml b/config/locales/forms/2023/lettings/income_and_benefits.en.yml index bc19c7954..dc5ee5a53 100644 --- a/config/locales/forms/2023/lettings/income_and_benefits.en.yml +++ b/config/locales/forms/2023/lettings/income_and_benefits.en.yml @@ -103,6 +103,6 @@ en: hint_text: "You only need to give an approximate figure." question_text: "Can you estimate the outstanding amount?" tshortfall: - check_answer_label: "Estimated outstanding amountt" + check_answer_label: "Estimated outstanding amount" hint_text: "Also known as the ‘outstanding amount’." question_text: "Estimated outstanding amount" diff --git a/config/locales/forms/2023/sales/household_characteristics.en.yml b/config/locales/forms/2023/sales/household_characteristics.en.yml index a49a817dc..ed03bc698 100644 --- a/config/locales/forms/2023/sales/household_characteristics.en.yml +++ b/config/locales/forms/2023/sales/household_characteristics.en.yml @@ -37,7 +37,7 @@ en: check_answer_label: "Buyer 1’s age" hint_text: "" question_text: "Age" - + sex1: page_header: "" check_answer_label: "Buyer 1’s gender identity" @@ -49,7 +49,7 @@ en: check_answer_label: "Buyer 1’s ethnic group" hint_text: "Buyer 1 is the person in the household who does the most paid work. If it’s a joint purchase and the buyers do the same amount of paid work, buyer 1 is whoever is the oldest." question_text: "What is buyer 1’s ethnic group?" - + ethnic: ethnic_background_black: page_header: "" @@ -103,9 +103,9 @@ en: question_text: "What is buyer 2's relationship to buyer 1?" person: page_header: "" - check_answer_label: "Person 2’s relationship to Buyer 1" + check_answer_label: "Person 2’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 2’s relationship to Buyer 1?" + question_text: "What is person 2’s relationship to buyer 1?" age2: buyer: @@ -139,14 +139,14 @@ en: page_header: "" check_answer_label: "Person 2’s gender identity" hint_text: "" - question_text: "Which of these best describes Person 2’s gender identity?" + question_text: "Which of these best describes person 2’s gender identity?" ethnic_group2: page_header: "" check_answer_label: "Buyer 2’s ethnic group" hint_text: "" question_text: "What is buyer 2’s ethnic group?" - + ethnicbuy2: ethnic_background_black: page_header: "" @@ -179,7 +179,7 @@ en: check_answer_label: "Buyer 2’s nationality" hint_text: "" question_text: "What is buyer 2’s nationality?" - + ecstat2: buyer: page_header: "" @@ -190,14 +190,14 @@ en: page_header: "" check_answer_label: "Person 2’s working situation" hint_text: "" - question_text: "Which of these best describes Person 2’s working situation?" + question_text: "Which of these best describes person 2’s working situation?" buy2livein: page_header: "" check_answer_label: "Will buyer 2 live in the property?" hint_text: "" question_text: "Will buyer 2 live in the property?" - + hholdcount: joint_purchase: page_header: "" @@ -224,9 +224,9 @@ en: relat3: page_header: "" - check_answer_label: "Person 3’s relationship to Buyer 1" + check_answer_label: "Person 3’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 3’s relationship to Buyer 1?" + question_text: "What is person 3’s relationship to buyer 1?" age3: page_header: "" @@ -238,18 +238,18 @@ en: check_answer_label: "Person 3’s age" hint_text: "" question_text: "Age" - + sex3: page_header: "" check_answer_label: "Person 3’s gender identity" hint_text: "" - question_text: "Which of these best describes Person 3’s gender identity?" + question_text: "Which of these best describes person 3’s gender identity?" ecstat3: page_header: "" check_answer_label: "Person 3’s working situation" hint_text: "" - question_text: "Which of these best describes Person 3’s working situation?" + question_text: "Which of these best describes person 3’s working situation?" details_known_4: page_header: "" @@ -259,9 +259,9 @@ en: relat4: page_header: "" - check_answer_label: "Person 4’s relationship to Buyer 1" + check_answer_label: "Person 4’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 4’s relationship to Buyer 1?" + question_text: "What is person 4’s relationship to buyer 1?" age4: page_header: "" @@ -273,18 +273,18 @@ en: check_answer_label: "Person 4’s age" hint_text: "" question_text: "Age" - + sex4: page_header: "" check_answer_label: "Person 4’s gender identity" hint_text: "" - question_text: "Which of these best describes Person 4’s gender identity?" + question_text: "Which of these best describes person 4’s gender identity?" ecstat4: page_header: "" check_answer_label: "Person 4’s working situation" hint_text: "" - question_text: "Which of these best describes Person 4’s working situation?" + question_text: "Which of these best describes person 4’s working situation?" details_known_5: page_header: "" @@ -294,9 +294,9 @@ en: relat5: page_header: "" - check_answer_label: "Person 5’s relationship to Buyer 1" + check_answer_label: "Person 5’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 5’s relationship to Buyer 1?" + question_text: "What is person 5’s relationship to buyer 1?" age5: page_header: "" @@ -308,18 +308,18 @@ en: check_answer_label: "Person 5’s age" hint_text: "" question_text: "Age" - + sex5: page_header: "" check_answer_label: "Person 5’s gender identity" hint_text: "" - question_text: "Which of these best describes Person 5’s gender identity?" + question_text: "Which of these best describes person 5’s gender identity?" ecstat5: page_header: "" check_answer_label: "Person 5’s working situation" hint_text: "" - question_text: "Which of these best describes Person 5’s working situation?" + question_text: "Which of these best describes person 5’s working situation?" details_known_6: page_header: "" @@ -329,9 +329,9 @@ en: relat6: page_header: "" - check_answer_label: "Person 6’s relationship to Buyer 1" + check_answer_label: "Person 6’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 6’s relationship to Buyer 1?" + question_text: "What is person 6’s relationship to buyer 1?" age6: page_header: "" @@ -343,15 +343,15 @@ en: check_answer_label: "Person 6’s age" hint_text: "" question_text: "Age" - + sex6: page_header: "" check_answer_label: "Person 6’s gender identity" hint_text: "" - question_text: "Which of these best describes Person 6’s gender identity?" + question_text: "Which of these best describes person 6’s gender identity?" ecstat6: page_header: "" check_answer_label: "Person 6’s working situation" hint_text: "" - question_text: "Which of these best describes Person 6’s working situation?" + question_text: "Which of these best describes person 6’s working situation?" diff --git a/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml b/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml index 49262d7a1..689c611cb 100644 --- a/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml +++ b/config/locales/forms/2023/sales/income_benefits_and_savings.en.yml @@ -13,7 +13,7 @@ en: check_answer_label: "Buyer 1’s gross annual income" hint_text: "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." question_text: "Buyer 1’s gross annual income" - + inc1mort: page_header: "" check_answer_label: "Buyer 1’s income used for mortgage application" @@ -33,7 +33,7 @@ en: inc2mort: page_header: "" - check_answer_label: "Buyer 2’s income used for mortgage application" + check_answer_label: "Buyer 2’s income used for mortgage application" hint_text: "" question_text: "Was buyer 2’s income used for a mortgage application?" @@ -48,14 +48,14 @@ en: check_answer_label: "Housing-related benefits buyer received before buying this property" hint_text: "" question_text: "Was the buyer receiving any of these housing-related benefits immediately before buying this property?" - + savings: joint_purchase: page_header: "" savingsnk: - check_answer_label: "Buyers’ total savings known?" + check_answer_label: "Buyers’ total savings known?" hint_text: "" - question_text: "Do you know how much the 'buyers' had in savings before they paid any deposit for the property?" + question_text: "Do you know how much the buyers had in savings before they paid any deposit for the property?" savings: check_answer_label: "Buyers’ total savings before any deposit paid" hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." @@ -87,4 +87,4 @@ en: page_header: "" check_answer_label: "Previous property shared ownership?" hint_text: "For any buyer" - question_text: "Was the previous property under shared ownership?" \ No newline at end of file + question_text: "Was the previous property under shared ownership?" diff --git a/config/locales/forms/2023/sales/sale_information.en.yml b/config/locales/forms/2023/sales/sale_information.en.yml index 318d7c7db..24f767026 100644 --- a/config/locales/forms/2023/sales/sale_information.en.yml +++ b/config/locales/forms/2023/sales/sale_information.en.yml @@ -28,7 +28,7 @@ en: staircasing: page_header: "" check_answer_label: "Staircasing transaction" - hint_text: "A staircasing transaction is when the household purchases more shares in their property, increasing the proportion they own and decreasing the proportion the housing association owns. Once the household purchases 100% of the shares, they own the property" + hint_text: "A staircasing transaction is when the household purchases more shares in their property, increasing the proportion they own and decreasing the proportion the housing association owns. Once the household purchases 100% of the shares, they own the property." question_text: "Is this a staircasing transaction?" about_staircasing: page_header: "About the staircasing transaction" @@ -69,7 +69,7 @@ en: check_answer_label: "Household rehoused under a local authority nominations agreement?" hint_text: "A local authority nominations agreement is a written agreement between a local authority and private registered provider (PRP) that some or all of its sales vacancies are offered to local authorities for rehousing" question_text: "Was the household rehoused under a 'local authority nominations agreement'?" - + soctenant: joint_purchase: page_header: "" @@ -81,11 +81,11 @@ en: check_answer_label: "Buyer was a registered provider, housing association or local authority tenant immediately before this sale?" hint_text: "" question_text: "Was the buyer a private registered provider, housing association or local authority tenant immediately before this sale?" - + frombeds: - page_header: "About the buyers’ previous property" + page_header: "" check_answer_label: "Number of bedrooms in previous property" - hint_text: "For bedsits enter 1" + hint_text: "A bedsit has 1 bedroom." question_text: "How many bedrooms did the property have?" fromprop: @@ -113,13 +113,13 @@ en: question_text: "What was the initial percentage equity stake purchased?" mortgageused: - page_header: "Mortgage Amount" + page_header: "" check_answer_label: "Mortgage used" hint_text: "" question_text: "Was a mortgage used for the purchase of this property?" mortgage: - page_header: "Mortgage Amount" + page_header: "" check_answer_label: "Mortgage amount" hint_text: "Enter the amount of mortgage agreed with the mortgage lender. Exclude any deposits or cash payments. Numeric in pounds. Rounded to the nearest pound." question_text: "What is the mortgage amount?" @@ -135,7 +135,7 @@ en: check_answer_label: "Other Mortgage Lender" hint_text: "" question_text: "What is the other mortgage lender?" - + mortlen: page_header: "" check_answer_label: "Length of mortgage" @@ -147,7 +147,7 @@ en: check_answer_label: "Any other borrowing?" hint_text: "" question_text: "Does this include any extra borrowing?" - + deposit: page_header: "About the deposit" check_answer_label: "Deposit amount" @@ -165,7 +165,7 @@ en: check_answer_label: "Monthly rent" hint_text: "Amount paid before any charges" question_text: "What is the basic monthly rent?" - + leaseholdcharges: page_header: "" has_mscharge: @@ -199,10 +199,10 @@ en: check_answer_label: "Percentage discount" hint_text: "For Right to Buy (RTB), Preserved Right to Buy (PRTB), and Voluntary Right to Buy (VRTB)

If discount capped, enter capped %

If the property is being sold to an existing tenant under the RTB, PRTB, or VRTB schemes, enter the % discount from the full market value that is being given." question_text: "What was the percentage discount?" - + grant: page_header: "About the price of the property" check_answer_label: "Amount of any loan, grant or subsidy" hint_text: "For all schemes except Right to Buy (RTB), Preserved Right to Buy (PRTB), Voluntary Right to Buy (VRTB) and Rent to Buy" question_text: "What was the amount of any loan, grant, discount or subsidy given?" - \ No newline at end of file + diff --git a/config/locales/forms/2024/lettings/household_characteristics.en.yml b/config/locales/forms/2024/lettings/household_characteristics.en.yml index 04a311f06..0537a82bd 100644 --- a/config/locales/forms/2024/lettings/household_characteristics.en.yml +++ b/config/locales/forms/2024/lettings/household_characteristics.en.yml @@ -13,19 +13,19 @@ en: page_header: "" age1_known: check_answer_label: "" - hint_text: "The ’lead’ or ’main’ tenant is the person in the household who does the most paid work. If several people do the same amount of paid work, the lead tenant is whoever is the oldest." + hint_text: "The lead tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." question_text: "Do you know the lead tenant’s age?" age1: check_answer_label: "Lead tenant’s age" hint_text: "" question_text: "Age" - + sex1: page_header: "" check_answer_label: "Lead tenant’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." question_text: "Which of these best describes the lead tenant’s gender identity?" - + ethnic_group: page_header: "" check_answer_label: "Lead tenant’s ethnic group" @@ -319,4 +319,4 @@ en: page_header: "" check_answer_label: "Person 8’s working situation" hint_text: "" - question_text: "Which of these best describes person 8’s working situation?" \ No newline at end of file + question_text: "Which of these best describes person 8’s working situation?" diff --git a/config/locales/forms/2024/lettings/income_and_benefits.en.yml b/config/locales/forms/2024/lettings/income_and_benefits.en.yml index bb3cc320e..8e364f611 100644 --- a/config/locales/forms/2024/lettings/income_and_benefits.en.yml +++ b/config/locales/forms/2024/lettings/income_and_benefits.en.yml @@ -103,6 +103,6 @@ en: hint_text: "You only need to give an approximate figure." question_text: "Can you estimate the outstanding amount?" tshortfall: - check_answer_label: "Estimated outstanding amountt" + check_answer_label: "Estimated outstanding amount" hint_text: "Also known as the ‘outstanding amount’." question_text: "Estimated outstanding amount" diff --git a/config/locales/forms/2024/sales/household_characteristics.en.yml b/config/locales/forms/2024/sales/household_characteristics.en.yml index 22f9427e8..5b06639fe 100644 --- a/config/locales/forms/2024/sales/household_characteristics.en.yml +++ b/config/locales/forms/2024/sales/household_characteristics.en.yml @@ -13,7 +13,7 @@ en: check_answer_label: "Buyer 1’s age" hint_text: "" question_text: "Age" - + sex1: page_header: "" check_answer_label: "Buyer 1’s gender identity" @@ -25,7 +25,7 @@ en: check_answer_label: "Buyer 1’s ethnic group" hint_text: "" question_text: "What is buyer 1’s ethnic group?" - + ethnic: ethnic_background_black: page_header: "" @@ -85,9 +85,9 @@ en: question_text: "What is buyer 2's relationship to buyer 1?" person: page_header: "" - check_answer_label: "Person 2’s relationship to Buyer 1" + check_answer_label: "Person 2’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 2’s relationship to Buyer 1?" + question_text: "What is person 2’s relationship to buyer 1?" age2: buyer: @@ -121,14 +121,14 @@ en: page_header: "" check_answer_label: "Person 2’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 2’s gender identity?" + question_text: "Which of these best describes person 2’s gender identity?" ethnic_group2: page_header: "" check_answer_label: "Buyer 2’s ethnic group" hint_text: "" question_text: "What is buyer 2’s ethnic group?" - + ethnicbuy2: ethnic_background_black: page_header: "" @@ -167,7 +167,7 @@ en: check_answer_label: "Buyer 2’s nationality" hint_text: "" question_text: "Enter a nationality" - + ecstat2: buyer: page_header: "" @@ -178,14 +178,14 @@ en: page_header: "" check_answer_label: "Person 2’s working situation" hint_text: "" - question_text: "Which of these best describes Person 2’s working situation?" + question_text: "Which of these best describes person 2’s working situation?" buy2livein: page_header: "" check_answer_label: "Will buyer 2 live in the property?" hint_text: "" question_text: "Will buyer 2 live in the property?" - + hholdcount: joint_purchase: page_header: "" @@ -212,9 +212,9 @@ en: relat3: page_header: "" - check_answer_label: "Person 3’s relationship to Buyer 1" + check_answer_label: "Person 3’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 3’s relationship to Buyer 1?" + question_text: "What is person 3’s relationship to buyer 1?" age3: page_header: "" @@ -226,18 +226,18 @@ en: check_answer_label: "Person 3’s age" hint_text: "" question_text: "Age" - + sex3: page_header: "" check_answer_label: "Person 3’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 3’s gender identity?" + question_text: "Which of these best describes person 3’s gender identity?" ecstat3: page_header: "" check_answer_label: "Person 3’s working situation" hint_text: "" - question_text: "Which of these best describes Person 3’s working situation?" + question_text: "Which of these best describes person 3’s working situation?" details_known_4: page_header: "" @@ -247,9 +247,9 @@ en: relat4: page_header: "" - check_answer_label: "Person 4’s relationship to Buyer 1" + check_answer_label: "Person 4’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 4’s relationship to Buyer 1?" + question_text: "What is person 4’s relationship to buyer 1?" age4: page_header: "" @@ -261,18 +261,18 @@ en: check_answer_label: "Person 4’s age" hint_text: "" question_text: "Age" - + sex4: page_header: "" check_answer_label: "Person 4’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 4’s gender identity?" + question_text: "Which of these best describes person 4’s gender identity?" ecstat4: page_header: "" check_answer_label: "Person 4’s working situation" hint_text: "" - question_text: "Which of these best describes Person 4’s working situation?" + question_text: "Which of these best describes person 4’s working situation?" details_known_5: page_header: "" @@ -282,9 +282,9 @@ en: relat5: page_header: "" - check_answer_label: "Person 5’s relationship to Buyer 1" + check_answer_label: "Person 5’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 5’s relationship to Buyer 1?" + question_text: "What is person 5’s relationship to buyer 1?" age5: page_header: "" @@ -296,18 +296,18 @@ en: check_answer_label: "Person 5’s age" hint_text: "" question_text: "Age" - + sex5: page_header: "" check_answer_label: "Person 5’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 5’s gender identity?" + question_text: "Which of these best describes person 5’s gender identity?" ecstat5: page_header: "" check_answer_label: "Person 5’s working situation" hint_text: "" - question_text: "Which of these best describes Person 5’s working situation?" + question_text: "Which of these best describes person 5’s working situation?" details_known_6: page_header: "" @@ -317,9 +317,9 @@ en: relat6: page_header: "" - check_answer_label: "Person 6’s relationship to Buyer 1" + check_answer_label: "Person 6’s relationship to buyer 1" hint_text: "" - question_text: "What is Person 6’s relationship to Buyer 1?" + question_text: "What is person 6’s relationship to buyer 1?" age6: page_header: "" @@ -331,15 +331,15 @@ en: check_answer_label: "Person 6’s age" hint_text: "" question_text: "Age" - + sex6: page_header: "" check_answer_label: "Person 6’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 6’s gender identity?" + question_text: "Which of these best describes person 6’s gender identity?" ecstat6: page_header: "" check_answer_label: "Person 6’s working situation" hint_text: "" - question_text: "Which of these best describes Person 6’s working situation?" + question_text: "Which of these best describes person 6’s working situation?" diff --git a/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml b/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml index 0654c9e1f..8d5ec0772 100644 --- a/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml +++ b/config/locales/forms/2024/sales/income_benefits_and_savings.en.yml @@ -13,7 +13,7 @@ en: check_answer_label: "Buyer 1’s gross annual income" hint_text: "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." question_text: "Buyer 1’s gross annual income" - + inc1mort: page_header: "" check_answer_label: "Buyer 1’s income used for mortgage application" @@ -48,14 +48,14 @@ en: check_answer_label: "Housing-related benefits buyer received before buying this property" hint_text: "" question_text: "Was the buyer receiving any of these housing-related benefits immediately before buying this property?" - + savings: joint_purchase: page_header: "" savingsnk: check_answer_label: "Buyers’ total savings known?" hint_text: "" - question_text: "Do you know how much the 'buyers' had in savings before they paid any deposit for the property?" + question_text: "Do you know how much the buyers had in savings before they paid any deposit for the property?" savings: check_answer_label: "Buyers’ total savings before any deposit paid" hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." @@ -87,4 +87,4 @@ en: page_header: "" check_answer_label: "Previous property shared ownership?" hint_text: "For any buyer" - question_text: "Was the previous property under shared ownership?" \ No newline at end of file + question_text: "Was the previous property under shared ownership?" diff --git a/config/locales/forms/2024/sales/sale_information.en.yml b/config/locales/forms/2024/sales/sale_information.en.yml index d0031416e..92acca9cd 100644 --- a/config/locales/forms/2024/sales/sale_information.en.yml +++ b/config/locales/forms/2024/sales/sale_information.en.yml @@ -28,7 +28,7 @@ en: staircasing: page_header: "" check_answer_label: "Staircasing transaction" - hint_text: "A staircasing transaction is when the household purchases more shares in their property, increasing the proportion they own and decreasing the proportion the housing association owns. Once the household purchases 100% of the shares, they own the property" + hint_text: "A staircasing transaction is when the household purchases more shares in their property, increasing the proportion they own and decreasing the proportion the housing association owns. Once the household purchases 100% of the shares, they own the property." question_text: "Is this a staircasing transaction?" about_staircasing: page_header: "About the staircasing transaction" @@ -73,7 +73,7 @@ en: check_answer_label: "Household rehoused under a local authority nominations agreement?" hint_text: "A local authority nominations agreement is a written agreement between a local authority and private registered provider (PRP) that some or all of its sales vacancies are offered to local authorities for rehousing" question_text: "Was the household rehoused under a 'local authority nominations agreement'?" - + soctenant: joint_purchase: page_header: "" @@ -85,11 +85,11 @@ en: check_answer_label: "Buyer was a registered provider, housing association or local authority tenant immediately before this sale?" hint_text: "" question_text: "Was the buyer a private registered provider, housing association or local authority tenant immediately before this sale?" - + frombeds: - page_header: "About the buyers’ previous property" + page_header: "" check_answer_label: "Number of bedrooms in previous property" - hint_text: "For bedsits enter 1" + hint_text: "A bedsit has 1 bedroom." question_text: "How many bedrooms did the property have?" fromprop: @@ -117,13 +117,13 @@ en: question_text: "What was the initial percentage equity stake purchased?" mortgageused: - page_header: "Mortgage Amount" + page_header: "" check_answer_label: "Mortgage used" hint_text: "" question_text: "Was a mortgage used for the purchase of this property?" mortgage: - page_header: "Mortgage Amount" + page_header: "" check_answer_label: "Mortgage amount" hint_text: "Enter the amount of mortgage agreed with the mortgage lender. Exclude any deposits or cash payments. Numeric in pounds. Rounded to the nearest pound." question_text: "What is the mortgage amount?" @@ -139,7 +139,7 @@ en: check_answer_label: "Other Mortgage Lender" hint_text: "" question_text: "What is the other mortgage lender?" - + mortlen: page_header: "" check_answer_label: "Length of mortgage" @@ -151,7 +151,7 @@ en: check_answer_label: "Any other borrowing?" hint_text: "" question_text: "Does this include any extra borrowing?" - + deposit: page_header: "About the deposit" check_answer_label: "Deposit amount" @@ -169,7 +169,7 @@ en: check_answer_label: "Monthly rent" hint_text: "Amount paid before any charges" question_text: "What is the basic monthly rent?" - + leaseholdcharges: page_header: "" has_mscharge: @@ -198,10 +198,10 @@ en: check_answer_label: "Percentage discount" hint_text: "For Right to Buy (RTB), Preserved Right to Buy (PRTB), and Voluntary Right to Buy (VRTB)

If discount capped, enter capped %

If the property is being sold to an existing tenant under the RTB, PRTB, or VRTB schemes, enter the % discount from the full market value that is being given." question_text: "What was the percentage discount?" - + grant: page_header: "About the price of the property" check_answer_label: "Amount of any loan, grant or subsidy" hint_text: "For all schemes except Right to Buy (RTB), Preserved Right to Buy (PRTB), Voluntary Right to Buy (VRTB) and Rent to Buy" question_text: "What was the amount of any loan, grant, discount or subsidy given?" - \ No newline at end of file + diff --git a/config/locales/forms/2025/lettings/household_characteristics.en.yml b/config/locales/forms/2025/lettings/household_characteristics.en.yml index 1aef9297a..b6d7ad7e9 100644 --- a/config/locales/forms/2025/lettings/household_characteristics.en.yml +++ b/config/locales/forms/2025/lettings/household_characteristics.en.yml @@ -13,7 +13,7 @@ en: page_header: "" age1_known: check_answer_label: "" - hint_text: "The ’lead’ or ’main’ tenant is the person in the household who does the most paid work. If several people do the same amount of paid work, the lead tenant is whoever is the oldest." + hint_text: "The lead tenant is the person in the household who does the most paid work. If several people do the same paid work, the lead tenant is whoever is the oldest." question_text: "Do you know the lead tenant’s age?" age1: check_answer_label: "Lead tenant’s age" diff --git a/config/locales/forms/2025/lettings/income_and_benefits.en.yml b/config/locales/forms/2025/lettings/income_and_benefits.en.yml index 55e193ff7..5b8ed26c0 100644 --- a/config/locales/forms/2025/lettings/income_and_benefits.en.yml +++ b/config/locales/forms/2025/lettings/income_and_benefits.en.yml @@ -103,6 +103,6 @@ en: hint_text: "You only need to give an approximate figure." question_text: "Can you estimate the outstanding amount?" tshortfall: - check_answer_label: "Estimated outstanding amountt" + check_answer_label: "Estimated outstanding amount" hint_text: "Also known as the ‘outstanding amount’." question_text: "Estimated outstanding amount" diff --git a/config/locales/forms/2025/sales/household_characteristics.en.yml b/config/locales/forms/2025/sales/household_characteristics.en.yml index 3f9f503be..a217c578c 100644 --- a/config/locales/forms/2025/sales/household_characteristics.en.yml +++ b/config/locales/forms/2025/sales/household_characteristics.en.yml @@ -121,7 +121,7 @@ en: page_header: "" check_answer_label: "Person 2’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 2’s gender identity?" + question_text: "Which of these best describes person 2’s gender identity?" ethnic_group2: page_header: "" @@ -178,7 +178,7 @@ en: page_header: "" check_answer_label: "Person 2’s working situation" hint_text: "" - question_text: "Which of these best describes Person 2’s working situation?" + question_text: "Which of these best describes person 2’s working situation?" buy2livein: page_header: "" @@ -231,13 +231,13 @@ en: page_header: "" check_answer_label: "Person 3’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 3’s gender identity?" + question_text: "Which of these best describes person 3’s gender identity?" ecstat3: page_header: "" check_answer_label: "Person 3’s working situation" hint_text: "" - question_text: "Which of these best describes Person 3’s working situation?" + question_text: "Which of these best describes person 3’s working situation?" details_known_4: page_header: "" @@ -266,13 +266,13 @@ en: page_header: "" check_answer_label: "Person 4’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 4’s gender identity?" + question_text: "Which of these best describes person 4’s gender identity?" ecstat4: page_header: "" check_answer_label: "Person 4’s working situation" hint_text: "" - question_text: "Which of these best describes Person 4’s working situation?" + question_text: "Which of these best describes person 4’s working situation?" details_known_5: page_header: "" @@ -301,13 +301,13 @@ en: page_header: "" check_answer_label: "Person 5’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 5’s gender identity?" + question_text: "Which of these best describes person 5’s gender identity?" ecstat5: page_header: "" check_answer_label: "Person 5’s working situation" hint_text: "" - question_text: "Which of these best describes Person 5’s working situation?" + question_text: "Which of these best describes person 5’s working situation?" details_known_6: page_header: "" @@ -336,10 +336,10 @@ en: page_header: "" check_answer_label: "Person 6’s gender identity" hint_text: "This should be however they personally choose to identify from the options below. This may or may not be the same as their biological sex or the sex they were assigned at birth." - question_text: "Which of these best describes Person 6’s gender identity?" + question_text: "Which of these best describes person 6’s gender identity?" ecstat6: page_header: "" check_answer_label: "Person 6’s working situation" hint_text: "" - question_text: "Which of these best describes Person 6’s working situation?" + question_text: "Which of these best describes person 6’s working situation?" diff --git a/config/locales/forms/2025/sales/income_benefits_and_savings.en.yml b/config/locales/forms/2025/sales/income_benefits_and_savings.en.yml index 20beb0b85..b38c203f7 100644 --- a/config/locales/forms/2025/sales/income_benefits_and_savings.en.yml +++ b/config/locales/forms/2025/sales/income_benefits_and_savings.en.yml @@ -55,7 +55,7 @@ en: savingsnk: check_answer_label: "Buyers’ total savings known?" hint_text: "" - question_text: "Do you know how much the 'buyers' had in savings before they paid any deposit for the property?" + question_text: "Do you know how much the buyers had in savings before they paid any deposit for the property?" savings: check_answer_label: "Buyers’ total savings before any deposit paid" hint_text: "Include any savings, investments, ISAs, premium bonds, shares, or money held in a bank or building society account." diff --git a/config/locales/forms/2025/sales/sale_information.en.yml b/config/locales/forms/2025/sales/sale_information.en.yml index 0535caca1..278c8a235 100644 --- a/config/locales/forms/2025/sales/sale_information.en.yml +++ b/config/locales/forms/2025/sales/sale_information.en.yml @@ -105,9 +105,9 @@ en: question_text: "Was the buyer a private registered provider, housing association or local authority tenant immediately before this sale?" frombeds: - page_header: "About the buyers’ previous property" + page_header: "" check_answer_label: "Number of bedrooms in previous property" - hint_text: "For bedsits enter 1" + hint_text: "A bedsit has 1 bedroom." question_text: "How many bedrooms did the property have?" fromprop: @@ -145,13 +145,13 @@ en: question_text: "What was the percentage shared purchased in the initial transaction?" mortgageused: - page_header: "Mortgage Amount" + page_header: "" check_answer_label: "Mortgage used?" hint_text: "" question_text: "Was a mortgage used for the purchase of this property?" mortgage: - page_header: "Mortgage Amount" + page_header: "" check_answer_label: "Mortgage amount" hint_text: "Enter the amount of mortgage agreed with the mortgage lender. Exclude any deposits or cash payments. Numeric in pounds. Rounded to the nearest pound." question_text: "What is the mortgage amount?" diff --git a/config/locales/forms/2025/sales/setup.en.yml b/config/locales/forms/2025/sales/setup.en.yml index 6f7c5da98..c4cc9a42c 100644 --- a/config/locales/forms/2025/sales/setup.en.yml +++ b/config/locales/forms/2025/sales/setup.en.yml @@ -42,7 +42,7 @@ en: staircasing: page_header: "" check_answer_label: "Staircasing transaction" - hint_text: "A staircasing transaction is when the household purchases more shares in their property, increasing the proportion they own and decreasing the proportion the housing association owns. Once the household purchases 100% of the shares, they own the property" + hint_text: "A staircasing transaction is when the household purchases more shares in their property, increasing the proportion they own and decreasing the proportion the housing association owns. Once the household purchases 100% of the shares, they own the property." question_text: "Is this a staircasing transaction?" type: diff --git a/spec/models/form/sales/pages/equity_spec.rb b/spec/models/form/sales/pages/equity_spec.rb index 83a5dfaa3..e27538263 100644 --- a/spec/models/form/sales/pages/equity_spec.rb +++ b/spec/models/form/sales/pages/equity_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Form::Sales::Pages::Equity, type: :model do let(:page_definition) { nil } let(:subsection) { instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1))) } + before do + allow(page.subsection.form).to receive(:start_year_2025_or_later?).and_return(false) + end + it "has correct subsection" do expect(page.subsection).to eq(subsection) end diff --git a/spec/models/form/sales/pages/value_shared_ownership_spec.rb b/spec/models/form/sales/pages/value_shared_ownership_spec.rb index eb1b1099f..82e6c1055 100644 --- a/spec/models/form/sales/pages/value_shared_ownership_spec.rb +++ b/spec/models/form/sales/pages/value_shared_ownership_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Form::Sales::Pages::ValueSharedOwnership, type: :model do let(:page_definition) { nil } let(:subsection) { instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1))) } + before do + allow(page.subsection.form).to receive(:start_year_2025_or_later?).and_return(false) + end + it "has correct subsection" do expect(page.subsection).to eq(subsection) end diff --git a/spec/models/form/sales/questions/equity_spec.rb b/spec/models/form/sales/questions/equity_spec.rb index 5083af9e8..3e6b9d85a 100644 --- a/spec/models/form/sales/questions/equity_spec.rb +++ b/spec/models/form/sales/questions/equity_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Form::Sales::Questions::Equity, type: :model do let(:question_definition) { nil } let(:page) { instance_double(Form::Page, id: "initial_equity", subsection: instance_double(Form::Subsection, form: instance_double(Form, start_date: Time.zone.local(2023, 4, 1)))) } + before do + allow(page.subsection.form).to receive(:start_year_2025_or_later?).and_return(false) + end + it "has correct page" do expect(question.page).to eq(page) end From ef49307d9bbda4d44366bb5feefd4dd4869ad237 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:16:55 +0000 Subject: [PATCH 08/19] CLDC-3361 Update scheme back link paths (#2802) * Update scheme back link paths * Correclty route if has other client group changed from no to yes --- app/controllers/schemes_controller.rb | 4 +- app/helpers/schemes_helper.rb | 18 +++++ app/views/schemes/confirm_secondary.html.erb | 2 +- app/views/schemes/details.html.erb | 2 +- .../schemes/primary_client_group.html.erb | 8 +- .../schemes/secondary_client_group.html.erb | 2 +- app/views/schemes/support.html.erb | 2 +- spec/requests/schemes_controller_spec.rb | 74 ++++++++++++++++++- 8 files changed, 98 insertions(+), 14 deletions(-) diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb index 3dc642345..a76c9db52 100644 --- a/app/controllers/schemes_controller.rb +++ b/app/controllers/schemes_controller.rb @@ -150,7 +150,7 @@ class SchemesController < ApplicationController @scheme.update!(secondary_client_group: nil) if @scheme.has_other_client_group == "No" if scheme_params[:confirmed] == "true" || @scheme.confirmed? if check_answers && should_direct_via_secondary_client_group_page?(page) - redirect_to scheme_secondary_client_group_path(@scheme, referrer: "check-answers") + redirect_to scheme_secondary_client_group_path(@scheme, referrer: "has-other-client-group") else @scheme.locations.update!(confirmed: true) flash[:notice] = if scheme_previously_confirmed @@ -162,7 +162,7 @@ class SchemesController < ApplicationController end elsif check_answers if should_direct_via_secondary_client_group_page?(page) - redirect_to scheme_secondary_client_group_path(@scheme, referrer: "check-answers") + redirect_to scheme_secondary_client_group_path(@scheme, referrer: "has-other-client-group") else redirect_to scheme_check_answers_path(@scheme) end diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index 12d86aba8..f262c08ff 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -101,6 +101,24 @@ module SchemesHelper organisation.owned_schemes.duplicate_sets.any? || organisation.owned_schemes.any? { |scheme| scheme.locations.duplicate_sets.any? } end + def scheme_back_button_path(scheme, current_page) + return scheme_check_answers_path(scheme) if request.params[:referrer] == "check-answers" + return scheme_confirm_secondary_client_group_path(scheme, referrer: "check-answers") if request.params[:referrer] == "has-other-client-group" + + case current_page + when "details" + schemes_path + when "primary_client_group" + scheme_details_path(scheme) + when "confirm_secondary_client_group" + scheme_primary_client_group_path(scheme) + when "secondary_client_group" + scheme_confirm_secondary_client_group_path(scheme) + when "support" + scheme.has_other_client_group == "Yes" ? scheme_secondary_client_group_path(scheme) : scheme_confirm_secondary_client_group_path(scheme) + end + end + private ActivePeriod = Struct.new(:from, :to) diff --git a/app/views/schemes/confirm_secondary.html.erb b/app/views/schemes/confirm_secondary.html.erb index 9b715c264..9feca87bc 100644 --- a/app/views/schemes/confirm_secondary.html.erb +++ b/app/views/schemes/confirm_secondary.html.erb @@ -1,7 +1,7 @@ <% content_for :title, "Does this scheme provide for another client group?" %> <% content_for :before_content do %> - <%= govuk_back_link(href: :back) %> + <%= govuk_back_link(href: scheme_back_button_path(@scheme, "confirm_secondary_client_group")) %> <% end %> <%= render partial: "organisations/headings", locals: { main: "Does this scheme provide for another client group?", sub: @scheme.service_name } %> diff --git a/app/views/schemes/details.html.erb b/app/views/schemes/details.html.erb index 4b23ab016..d1943cabe 100644 --- a/app/views/schemes/details.html.erb +++ b/app/views/schemes/details.html.erb @@ -1,7 +1,7 @@ <% content_for :title, "Create a new supported housing scheme" %> <% content_for :before_content do %> - <%= govuk_back_link(href: :back) %> + <%= govuk_back_link(href: scheme_back_button_path(@scheme, "details")) %> <% end %> <%= form_for(@scheme, method: :patch) do |f| %> diff --git a/app/views/schemes/primary_client_group.html.erb b/app/views/schemes/primary_client_group.html.erb index 55dc504b4..68da2f35c 100644 --- a/app/views/schemes/primary_client_group.html.erb +++ b/app/views/schemes/primary_client_group.html.erb @@ -1,13 +1,7 @@ <% content_for :title, "What client group is this scheme intended for?" %> -<% if request.referer&.include?("new") %> - <% back_button_path = scheme_details_path(@scheme) %> -<% else %> - <% back_button_path = :back %> -<% end %> - <% content_for :before_content do %> - <%= govuk_back_link(href: back_button_path) %> + <%= govuk_back_link(href: scheme_back_button_path(@scheme, "primary_client_group")) %> <% end %> <%= form_for(@scheme, method: :patch) do |f| %> diff --git a/app/views/schemes/secondary_client_group.html.erb b/app/views/schemes/secondary_client_group.html.erb index cd50283e3..8200702d2 100644 --- a/app/views/schemes/secondary_client_group.html.erb +++ b/app/views/schemes/secondary_client_group.html.erb @@ -1,7 +1,7 @@ <% content_for :title, "What is the other client group?" %> <% content_for :before_content do %> - <%= govuk_back_link(href: :back) %> + <%= govuk_back_link(href: scheme_back_button_path(@scheme, "secondary_client_group")) %> <% end %> <%= form_for(@scheme, method: :patch) do |f| %> diff --git a/app/views/schemes/support.html.erb b/app/views/schemes/support.html.erb index a5d30ed7f..eca72014f 100644 --- a/app/views/schemes/support.html.erb +++ b/app/views/schemes/support.html.erb @@ -1,7 +1,7 @@ <% content_for :title, "What support does this scheme provide?" %> <% content_for :before_content do %> - <%= govuk_back_link(href: :back) %> + <%= govuk_back_link(href: scheme_back_button_path(@scheme, "support")) %> <% end %> <%= form_for(@scheme, method: :patch) do |f| %> diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 83ba11fd9..2eb2330c8 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -2035,6 +2035,17 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content("What client group is this scheme intended for?") end + it "has correct back link" do + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/details") + end + + context "and accessed from check answers" do + it "has correct back link" do + get "/schemes/#{scheme.id}/primary-client-group?referrer=check-answers" + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/check-answers") + end + end + context "when attempting to access primary-client-group scheme page for another organisation" do before do get "/schemes/#{another_scheme.id}/primary-client-group" @@ -2112,6 +2123,17 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content("Does this scheme provide for another client group?") end + it "has correct back link" do + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/primary-client-group") + end + + context "and accessed from check answers" do + it "has correct back link" do + get "/schemes/#{scheme.id}/confirm-secondary-client-group?referrer=check-answers" + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/check-answers") + end + end + context "when attempting to access confirm-secondary-client-group scheme page for another organisation" do before do get "/schemes/#{another_scheme.id}/confirm-secondary-client-group" @@ -2189,6 +2211,24 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content("What is the other client group?") end + it "has correct back link" do + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/confirm-secondary-client-group") + end + + context "and accessed from check answers" do + it "has correct back link" do + get "/schemes/#{scheme.id}/secondary-client-group?referrer=check-answers" + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/check-answers") + end + end + + context "and accessed from has other client group" do + it "has correct back link" do + get "/schemes/#{scheme.id}/secondary-client-group?referrer=has-other-client-group" + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/confirm-secondary-client-group?referrer=check-answers") + end + end + context "when attempting to access secondary-client-group scheme page for another organisation" do before do get "/schemes/#{another_scheme.id}/secondary-client-group" @@ -2258,7 +2298,7 @@ RSpec.describe SchemesController, type: :request do context "when signed in as a data coordinator" do let(:user) { create(:user, :data_coordinator) } - let(:scheme) { create(:scheme, owning_organisation: user.organisation, confirmed: nil) } + let(:scheme) { create(:scheme, owning_organisation: user.organisation, confirmed: nil, has_other_client_group: "Yes") } let(:another_scheme) { create(:scheme, confirmed: nil) } before do @@ -2271,6 +2311,27 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content("What support does this scheme provide?") end + context "when scheme has secondary client group" do + it "has correct back link" do + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/secondary-client-group") + end + end + + context "when scheme has no secondary client group" do + let(:scheme) { create(:scheme, owning_organisation: user.organisation, confirmed: nil, has_other_client_group: "No") } + + it "has correct back link" do + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/confirm-secondary-client-group") + end + end + + context "and accessed from check answers" do + it "has correct back link" do + get "/schemes/#{scheme.id}/support?referrer=check-answers" + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/check-answers") + end + end + context "when attempting to access secondary-client-group scheme page for another organisation" do before do get "/schemes/#{another_scheme.id}/support" @@ -2433,6 +2494,17 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content("Create a new supported housing scheme") end + it "has correct back link" do + expect(page).to have_link("Back", href: "/schemes") + end + + context "and accessed from check answers" do + it "has correct back link" do + get "/schemes/#{scheme.id}/details?referrer=check-answers" + expect(page).to have_link("Back", href: "/schemes/#{scheme.id}/check-answers") + end + end + context "when attempting to access check-answers scheme page for another organisation" do before do get "/schemes/#{another_scheme.id}/details" From 46331b92adcb0854a12aa0eff76dc8bd227553e6 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Thu, 5 Dec 2024 09:29:01 +0000 Subject: [PATCH 09/19] CLDC-3268: Update github action dependencies (#2852) --- .github/workflows/aws_deploy.yml | 22 ++++++-------- .../workflows/review_teardown_pipeline.yml | 6 ++-- .github/workflows/staging_pipeline.yml | 30 +++++++++---------- 3 files changed, 27 insertions(+), 31 deletions(-) diff --git a/.github/workflows/aws_deploy.yml b/.github/workflows/aws_deploy.yml index c72c9d874..5af3c2d08 100644 --- a/.github/workflows/aws_deploy.yml +++ b/.github/workflows/aws_deploy.yml @@ -41,19 +41,17 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: ${{ env.aws_region }} role-to-assume: ${{ env.app_repo_role }} - name: Login to Amazon ECR id: ecr-login - uses: aws-actions/amazon-ecr-login@v1 - with: - mask-password: "true" + uses: aws-actions/amazon-ecr-login@v2 - name: Check if image with tag already exists run: | @@ -81,16 +79,14 @@ jobs: steps: - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: ${{ env.aws_region }} role-to-assume: ${{ env.app_repo_role }} - name: Login to Amazon ECR id: ecr-login - uses: aws-actions/amazon-ecr-login@v1 - with: - mask-password: "true" + uses: aws-actions/amazon-ecr-login@v2 - name: Get timestamp id: timestamp @@ -112,7 +108,7 @@ jobs: echo "image=$registry/$repository:$readable_tag" >> $GITHUB_ENV - name: Configure AWS credentials for environment - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: ${{ env.aws_region }} role-to-assume: arn:aws:iam::${{ inputs.aws_account_id }}:role/${{ inputs.aws_role_prefix }}-deployment @@ -133,7 +129,7 @@ jobs: image: ${{ env.image }} - name: Update ad hoc task definition - uses: aws-actions/amazon-ecs-deploy-task-definition@v1 + uses: aws-actions/amazon-ecs-deploy-task-definition@v2 with: task-definition: ${{ steps.ad-hoc-task-def.outputs.task-definition }} @@ -185,7 +181,7 @@ jobs: image: ${{ env.image }} - name: Deploy updated application - uses: aws-actions/amazon-ecs-deploy-task-definition@v1 + uses: aws-actions/amazon-ecs-deploy-task-definition@v2 with: cluster: ${{ inputs.aws_task_prefix }}-app service: ${{ inputs.aws_task_prefix }}-app @@ -207,7 +203,7 @@ jobs: image: ${{ env.image }} - name: Deploy updated sidekiq - uses: aws-actions/amazon-ecs-deploy-task-definition@v1 + uses: aws-actions/amazon-ecs-deploy-task-definition@v2 with: cluster: ${{ inputs.aws_task_prefix }}-app service: ${{ inputs.aws_task_prefix }}-sidekiq diff --git a/.github/workflows/review_teardown_pipeline.yml b/.github/workflows/review_teardown_pipeline.yml index 4303e4063..8925b3340 100644 --- a/.github/workflows/review_teardown_pipeline.yml +++ b/.github/workflows/review_teardown_pipeline.yml @@ -25,13 +25,13 @@ jobs: steps: - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: ${{ env.aws_region }} role-to-assume: ${{ env.app_repo_role }} - name: Configure AWS credentials for review environment - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: ${{ env.aws_region }} role-to-assume: arn:aws:iam::${{ env.aws_account_id }}:role/${{ env.aws_role_prefix }}-deployment @@ -46,7 +46,7 @@ jobs: network=$(aws ecs describe-services --cluster $cluster --services $service --query services[0].networkConfiguration) overrides='{ "containerOverrides" : [{ "name" : "app", "command" : ["bundle", "exec", "rake", "db:drop"]}]}' arn=$(aws ecs run-task --cluster $cluster --task-definition $ad_hoc_task_definition --network-configuration "$network" --overrides "$overrides" --group migrations --launch-type FARGATE --query tasks[0].taskArn) - echo "Waiting for db prepare task to complete" + echo "Waiting for db drop task to complete" temp=${arn##*/} id=${temp%*\"} aws ecs wait tasks-stopped --cluster $cluster --tasks $id diff --git a/.github/workflows/staging_pipeline.yml b/.github/workflows/staging_pipeline.yml index e5447f4ef..aff0fe5f0 100644 --- a/.github/workflows/staging_pipeline.yml +++ b/.github/workflows/staging_pipeline.yml @@ -55,7 +55,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -63,7 +63,7 @@ jobs: bundler-cache: true - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: yarn node-version: 20 @@ -113,7 +113,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -121,7 +121,7 @@ jobs: bundler-cache: true - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: yarn node-version: 20 @@ -171,7 +171,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -179,7 +179,7 @@ jobs: bundler-cache: true - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: yarn node-version: 20 @@ -230,7 +230,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -238,7 +238,7 @@ jobs: bundler-cache: true - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: yarn node-version: 20 @@ -289,7 +289,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -297,7 +297,7 @@ jobs: bundler-cache: true - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: yarn node-version: 20 @@ -320,7 +320,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -328,7 +328,7 @@ jobs: bundler-cache: true - name: Set up Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: yarn node-version: 20 @@ -347,7 +347,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -379,13 +379,13 @@ jobs: steps: - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: ${{ env.aws_region }} role-to-assume: ${{ env.app_repo_role }} - name: Configure AWS credentials for the environment - uses: aws-actions/configure-aws-credentials@v3 + uses: aws-actions/configure-aws-credentials@v4 with: aws-region: eu-west-2 role-to-assume: arn:aws:iam::107155005276:role/core-staging-deployment From 5fa69ce0b62c844ad122de5adae813ec71deac74 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Thu, 5 Dec 2024 11:38:14 +0000 Subject: [PATCH 10/19] CLDC-2128: Validate supplied year in bulk upload pages (#2839) * CLDC-2128: Validate supplied year in bulk upload pages * Fix lint * Refactor --- .../bulk_upload_lettings_logs_controller.rb | 23 +++- .../bulk_upload_sales_logs_controller.rb | 19 ++- .../forms/bulk_upload_lettings/needstype.rb | 39 ------ .../forms/needstype.erb | 23 ---- ...lk_upload_lettings_logs_controller_spec.rb | 111 ++++++++++++++++++ .../bulk_upload_sales_logs_controller_spec.rb | 111 ++++++++++++++++++ 6 files changed, 257 insertions(+), 69 deletions(-) delete mode 100644 app/models/forms/bulk_upload_lettings/needstype.rb delete mode 100644 app/views/bulk_upload_lettings_logs/forms/needstype.erb diff --git a/app/controllers/bulk_upload_lettings_logs_controller.rb b/app/controllers/bulk_upload_lettings_logs_controller.rb index a8ad14f9e..39bc05f7e 100644 --- a/app/controllers/bulk_upload_lettings_logs_controller.rb +++ b/app/controllers/bulk_upload_lettings_logs_controller.rb @@ -1,6 +1,7 @@ class BulkUploadLettingsLogsController < ApplicationController before_action :authenticate_user! - before_action :validate_data_protection_agrement_signed! + before_action :validate_data_protection_agreement_signed! + before_action :validate_year!, except: %w[start] def start if have_choice_of_year? @@ -24,12 +25,26 @@ class BulkUploadLettingsLogsController < ApplicationController private - def validate_data_protection_agrement_signed! + def validate_data_protection_agreement_signed! return if @current_user.organisation.data_protection_confirmed? redirect_to lettings_logs_path end + def validate_year! + return if params[:id] == "year" + return if params[:id] == "guidance" && params.dig(:form, :year).nil? + + allowed_years = [current_year] + allowed_years.push(current_year - 1) if FormHandler.instance.lettings_in_crossover_period? + allowed_years.push(current_year + 1) if FeatureToggle.allow_future_form_use? + + provided_year = params.dig(:form, :year)&.to_i + return if allowed_years.include?(provided_year) + + render_not_found + end + def current_year FormHandler.instance.current_collection_start_year end @@ -48,8 +63,6 @@ private Forms::BulkUploadLettings::PrepareYourFile.new(form_params) when "guidance" Forms::BulkUploadLettings::Guidance.new(form_params.merge(referrer: params[:referrer])) - when "needstype" - Forms::BulkUploadLettings::Needstype.new(form_params) when "upload-your-file" Forms::BulkUploadLettings::UploadYourFile.new(form_params.merge(current_user:)) when "checking-file" @@ -60,6 +73,6 @@ private end def form_params - params.fetch(:form, {}).permit(:year, :needstype, :file, :organisation_id) + params.fetch(:form, {}).permit(:year, :file, :organisation_id) end end diff --git a/app/controllers/bulk_upload_sales_logs_controller.rb b/app/controllers/bulk_upload_sales_logs_controller.rb index 2fd312d10..cb04cea95 100644 --- a/app/controllers/bulk_upload_sales_logs_controller.rb +++ b/app/controllers/bulk_upload_sales_logs_controller.rb @@ -1,6 +1,7 @@ class BulkUploadSalesLogsController < ApplicationController before_action :authenticate_user! - before_action :validate_data_protection_agrement_signed! + before_action :validate_data_protection_agreement_signed! + before_action :validate_year!, except: %w[start] def start if have_choice_of_year? @@ -24,12 +25,26 @@ class BulkUploadSalesLogsController < ApplicationController private - def validate_data_protection_agrement_signed! + def validate_data_protection_agreement_signed! return if @current_user.organisation.data_protection_confirmed? redirect_to sales_logs_path end + def validate_year! + return if params[:id] == "year" + return if params[:id] == "guidance" && params.dig(:form, :year).nil? + + allowed_years = [current_year] + allowed_years.push(current_year - 1) if FormHandler.instance.sales_in_crossover_period? + allowed_years.push(current_year + 1) if FeatureToggle.allow_future_form_use? + + provided_year = params.dig(:form, :year)&.to_i + return if allowed_years.include?(provided_year) + + render_not_found + end + def current_year FormHandler.instance.current_collection_start_year end diff --git a/app/models/forms/bulk_upload_lettings/needstype.rb b/app/models/forms/bulk_upload_lettings/needstype.rb deleted file mode 100644 index b15c05b52..000000000 --- a/app/models/forms/bulk_upload_lettings/needstype.rb +++ /dev/null @@ -1,39 +0,0 @@ -module Forms - module BulkUploadLettings - class Needstype - include ActiveModel::Model - include ActiveModel::Attributes - include Rails.application.routes.url_helpers - - attribute :needstype, :integer - attribute :year, :integer - attribute :organisation_id, :integer - - validates :needstype, presence: true - - def view_path - "bulk_upload_lettings_logs/forms/needstype" - end - - def options - [OpenStruct.new(id: 1, name: "General needs"), OpenStruct.new(id: 2, name: "Supported housing")] - end - - def back_path - bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, needstype:, organisation_id: }.compact) - end - - def next_path - bulk_upload_lettings_log_path(id: "upload-your-file", form: { year:, needstype:, organisation_id: }.compact) - end - - def year_combo - "#{year} to #{year + 1}" - end - - def save! - true - end - end - end -end diff --git a/app/views/bulk_upload_lettings_logs/forms/needstype.erb b/app/views/bulk_upload_lettings_logs/forms/needstype.erb deleted file mode 100644 index 644dd9f5f..000000000 --- a/app/views/bulk_upload_lettings_logs/forms/needstype.erb +++ /dev/null @@ -1,23 +0,0 @@ -<% content_for :before_content do %> - <%= govuk_back_link href: @form.back_path %> -<% end %> - -
-
- <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "needstype"), method: :patch do |f| %> - <%= f.govuk_error_summary %> - <%= f.hidden_field :year %> - <%= f.hidden_field :organisation_id %> - - <%= f.govuk_collection_radio_buttons :needstype, - @form.options, - :id, - :name, - hint: { text: I18n.t("hints.bulk_upload.needstype") }, - legend: { text: "What is the needs type?", size: "l" }, - caption: { text: "Upload lettings logs in bulk (#{@form.year_combo})", size: "l" } %> - - <%= f.govuk_submit %> - <% end %> -
-
diff --git a/spec/requests/bulk_upload_lettings_logs_controller_spec.rb b/spec/requests/bulk_upload_lettings_logs_controller_spec.rb index 18b208e74..c9a22768d 100644 --- a/spec/requests/bulk_upload_lettings_logs_controller_spec.rb +++ b/spec/requests/bulk_upload_lettings_logs_controller_spec.rb @@ -73,5 +73,116 @@ RSpec.describe BulkUploadLettingsLogsController, type: :request do expect(response.body).to include("How to upload logs in bulk") end end + + context "when no year is specified" do + it "shows guidance page with links defaulting to the current year" do + get "/lettings-logs/bulk-upload-logs/guidance" + + expect(response.body).to include("Download the lettings bulk upload template (#{current_collection_start_year} to #{current_collection_start_year + 1})") + end + end + + context "when an invalid year is specified" do + it "shows not found" do + get "/lettings-logs/bulk-upload-logs/guidance?form%5Byear%5D=10000" + + expect(response).to be_not_found + end + end + end + + describe "GET /lettings-logs/bulk-upload-logs/year" do + it "does not require a year to be specified" do + get "/lettings-logs/bulk-upload-logs/year" + + expect(response).to be_ok + end + end + + pages_requiring_year_specification = %w[prepare-your-file upload-your-file checking-file] + pages_requiring_year_specification.each do |page_id| + describe "GET /lettings-logs/bulk-upload-logs/#{page_id}" do + context "when no year is provided" do + it "returns not found" do + get "/lettings-logs/bulk-upload-logs/#{page_id}" + + expect(response).to be_not_found + end + end + + context "when requesting the previous year in a crossover period" do + before do + allow(FormHandler.instance).to receive(:lettings_in_crossover_period?).and_return(true) + end + + it "succeeds" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year - 1}" + + expect(response).to be_ok + end + end + + context "when requesting the previous year outside a crossover period" do + before do + allow(FormHandler.instance).to receive(:lettings_in_crossover_period?).and_return(false) + end + + it "returns not found" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year - 1}" + + expect(response).to be_not_found + end + end + + context "when requesting the current year" do + it "succeeds" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year}" + + expect(response).to be_ok + end + end + + if page_id != "prepare-your-file" + context "when requesting the next year with future form use toggled on" do + before do + allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(true) + end + + it "succeeds" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year + 1}" + + expect(response).to be_ok + end + end + end + + context "when requesting the next year with future form use toggled off" do + before do + allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(false) + end + + it "returns not found" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year + 1}" + + expect(response).to be_not_found + end + end + + context "when requesting a far future year" do + it "returns not found" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=9990" + + expect(response).to be_not_found + end + end + + context "when requesting a nonsense value for year" do + it "returns not found" do + get "/lettings-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=thisisnotayear" + + expect(response).to be_not_found + end + end + end end end diff --git a/spec/requests/bulk_upload_sales_logs_controller_spec.rb b/spec/requests/bulk_upload_sales_logs_controller_spec.rb index 7cd6d4be8..4c20482be 100644 --- a/spec/requests/bulk_upload_sales_logs_controller_spec.rb +++ b/spec/requests/bulk_upload_sales_logs_controller_spec.rb @@ -73,5 +73,116 @@ RSpec.describe BulkUploadSalesLogsController, type: :request do expect(response.body).to include("How to upload logs in bulk") end end + + context "when no year is specified" do + it "shows guidance page with links defaulting to the current year" do + get "/sales-logs/bulk-upload-logs/guidance" + + expect(response.body).to include("Download the sales bulk upload template (#{current_collection_start_year} to #{current_collection_start_year + 1})") + end + end + + context "when an invalid year is specified" do + it "shows not found" do + get "/sales-logs/bulk-upload-logs/guidance?form%5Byear%5D=10000" + + expect(response).to be_not_found + end + end + end + + describe "GET /sales-logs/bulk-upload-logs/year" do + it "does not require a year to be specified" do + get "/sales-logs/bulk-upload-logs/year" + + expect(response).to be_ok + end + end + + pages_requiring_year_specification = %w[prepare-your-file upload-your-file checking-file] + pages_requiring_year_specification.each do |page_id| + describe "GET /sales-logs/bulk-upload-logs/#{page_id}" do + context "when no year is provided" do + it "returns not found" do + get "/sales-logs/bulk-upload-logs/#{page_id}" + + expect(response).to be_not_found + end + end + + context "when requesting the previous year in a crossover period" do + before do + allow(FormHandler.instance).to receive(:sales_in_crossover_period?).and_return(true) + end + + it "succeeds" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year - 1}" + + expect(response).to be_ok + end + end + + context "when requesting the previous year outside a crossover period" do + before do + allow(FormHandler.instance).to receive(:sales_in_crossover_period?).and_return(false) + end + + it "returns not found" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year - 1}" + + expect(response).to be_not_found + end + end + + context "when requesting the current year" do + it "succeeds" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year}" + + expect(response).to be_ok + end + end + + if page_id != "prepare-your-file" + context "when requesting the next year with future form use toggled on" do + before do + allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(true) + end + + it "succeeds" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year + 1}" + + expect(response).to be_ok + end + end + end + + context "when requesting the next year with future form use toggled off" do + before do + allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(false) + end + + it "returns not found" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=#{current_collection_start_year + 1}" + + expect(response).to be_not_found + end + end + + context "when requesting a far future year" do + it "returns not found" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=9990" + + expect(response).to be_not_found + end + end + + context "when requesting a nonsense value for year" do + it "returns not found" do + get "/sales-logs/bulk-upload-logs/#{page_id}?form%5Byear%5D=thisisnotayear" + + expect(response).to be_not_found + end + end + end end end From 3b8858f59455e46022254314f199e3e7d78d7b7a Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:30:53 +0000 Subject: [PATCH 11/19] If scheme is incomplete and deactivated treat is as deactivated (#2842) --- app/models/scheme.rb | 2 +- spec/models/scheme_spec.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 1cd56ac7d..a9b479342 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -329,9 +329,9 @@ class Scheme < ApplicationRecord def status_at(date) return :deleted if discarded_at.present? - return :incomplete unless confirmed && locations.confirmed.any? return :deactivated if owning_organisation.status_at(date) == :deactivated || owning_organisation.status_at(date) == :merged || (open_deactivation&.deactivation_date.present? && date >= open_deactivation.deactivation_date) + return :incomplete unless confirmed && locations.confirmed.any? return :deactivating_soon if open_deactivation&.deactivation_date.present? && date < open_deactivation.deactivation_date return :reactivating_soon if last_deactivation_before(date)&.reactivation_date.present? && date < last_deactivation_before(date).reactivation_date return :activating_soon if startdate.present? && date < startdate diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb index 65174388d..21b60cd52 100644 --- a/spec/models/scheme_spec.rb +++ b/spec/models/scheme_spec.rb @@ -390,6 +390,13 @@ RSpec.describe Scheme, type: :model do scheme.startdate = Time.zone.today + 2.weeks expect(scheme.status).to eq(:activating_soon) end + + it "returns deactivated if scheme is deactivated and incomplete" do + scheme.update!(support_type: nil, confirmed: nil) + FactoryBot.create(:scheme_deactivation_period, deactivation_date: Time.zone.yesterday, scheme:) + scheme.reload + expect(scheme.status).to eq(:deactivated) + end end context "when there have been previous deactivations" do From b8a576d87f0f39caa27068bf490c9ae9601ab27e Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:18:12 +0000 Subject: [PATCH 12/19] CLDC-3048 Fix pluralisation for 1 result (#2841) * Fix pluralisation for 1 result * Fix world * Update tests --- .../search_result_caption_component.html.erb | 2 +- app/views/bulk_upload_shared/uploads.html.erb | 2 +- app/views/locations/index.html.erb | 2 +- app/views/logs/_log_list.html.erb | 2 +- app/views/logs/index.html.erb | 2 +- app/views/logs/update_logs.html.erb | 2 +- .../managing_agents.html.erb | 4 +-- .../stock_owners.html.erb | 4 +-- .../organisations/_organisation_list.html.erb | 2 +- app/views/organisations/index.html.erb | 2 +- app/views/organisations/logs.html.erb | 2 +- app/views/schemes/_scheme_list.html.erb | 2 +- app/views/users/_user_list.html.erb | 2 +- .../search_result_caption_component_spec.rb | 26 ++++++++++++++++++- .../requests/lettings_logs_controller_spec.rb | 4 +-- ...anisation_relationships_controller_spec.rb | 12 ++++----- .../requests/organisations_controller_spec.rb | 8 +++--- spec/requests/sales_logs_controller_spec.rb | 4 +-- 18 files changed, 54 insertions(+), 30 deletions(-) diff --git a/app/components/search_result_caption_component.html.erb b/app/components/search_result_caption_component.html.erb index b8a9382b7..b2a28a505 100644 --- a/app/components/search_result_caption_component.html.erb +++ b/app/components/search_result_caption_component.html.erb @@ -7,7 +7,7 @@ <%= count %> <%= item_label.pluralize(count) %> matching filters
<% else %> - <%= count %> total <%= item %> + <%= count %> total <%= item.pluralize(count) %> <% end %> diff --git a/app/views/bulk_upload_shared/uploads.html.erb b/app/views/bulk_upload_shared/uploads.html.erb index 958887453..a9d134c60 100644 --- a/app/views/bulk_upload_shared/uploads.html.erb +++ b/app/views/bulk_upload_shared/uploads.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "uploads") %> +<% item_label = format_label(@pagy.count, "upload") %> <% title = format_title(@searched, bulk_upload_title(controller.controller_name), current_user, item_label, @pagy.count, nil) %> <% content_for :title, title %> diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 23550f894..8ef5bcb56 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -32,7 +32,7 @@ count: @pagy.count, item_label:, total_count: @total_count, - item: "locations", + item: "location", filters_count: applied_filters_count(@filter_type), )) %> <% end %> diff --git a/app/views/logs/_log_list.html.erb b/app/views/logs/_log_list.html.erb index b5290c117..24714f247 100644 --- a/app/views/logs/_log_list.html.erb +++ b/app/views/logs/_log_list.html.erb @@ -1,7 +1,7 @@

- <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "logs", filters_count: applied_filters_count(@filter_type))) %> + <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "log", filters_count: applied_filters_count(@filter_type))) %> <% if logs&.any? %> <%= govuk_link_to "Download (CSV)", csv_download_url, type: "text/csv", class: "govuk-!-margin-right-4", style: "white-space: nowrap" %> <% if @current_user.support? %> diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb index f142a2580..c51466097 100644 --- a/app/views/logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "logs") %> +<% item_label = format_label(@pagy.count, "log") %> <% title = format_title(@searched, "#{log_type_for_controller(controller).capitalize} logs", current_user, item_label, @pagy.count, nil) %> <% content_for :title, title %> diff --git a/app/views/logs/update_logs.html.erb b/app/views/logs/update_logs.html.erb index 1ab1fa31c..985adecf3 100644 --- a/app/views/logs/update_logs.html.erb +++ b/app/views/logs/update_logs.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "logs") %> +<% item_label = format_label(@pagy.count, "log") %> <% title = "Update logs" %> <% content_for :title, title %> diff --git a/app/views/organisation_relationships/managing_agents.html.erb b/app/views/organisation_relationships/managing_agents.html.erb index 726533e53..d09056d86 100644 --- a/app/views/organisation_relationships/managing_agents.html.erb +++ b/app/views/organisation_relationships/managing_agents.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "managing agents") %> +<% item_label = format_label(@pagy.count, "managing agent") %> <% if current_user.support? %> <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> @@ -44,7 +44,7 @@ pagy: @pagy, searched: @searched, item_label:, - search_item: "managing agents", + search_item: "managing agent", total_count: @total_count, remove_path: ->(org_id) { managing_agents_remove_organisation_path(target_organisation_id: org_id) }, } %> diff --git a/app/views/organisation_relationships/stock_owners.html.erb b/app/views/organisation_relationships/stock_owners.html.erb index 41b7af06d..3bbba6bf8 100644 --- a/app/views/organisation_relationships/stock_owners.html.erb +++ b/app/views/organisation_relationships/stock_owners.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "stock owners") %> +<% item_label = format_label(@pagy.count, "stock owner") %> <% if current_user.support? %> <%= render partial: "organisations/headings", locals: { main: @organisation.name, sub: nil } %> <%= render SubNavigationComponent.new(items: secondary_items(request.path, @organisation.id)) %> @@ -41,7 +41,7 @@ pagy: @pagy, searched: @searched, item_label:, - search_item: "stock owners", + search_item: "stock owner", total_count: @total_count, remove_path: ->(org_id) { stock_owners_remove_organisation_path(target_organisation_id: org_id) }, } %> diff --git a/app/views/organisations/_organisation_list.html.erb b/app/views/organisations/_organisation_list.html.erb index 67cc9c7a3..16309a5eb 100644 --- a/app/views/organisations/_organisation_list.html.erb +++ b/app/views/organisations/_organisation_list.html.erb @@ -1,7 +1,7 @@
<%= govuk_table do |table| %> <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "organisations", filters_count: applied_filters_count(@filter_type))) %> + <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "organisation", filters_count: applied_filters_count(@filter_type))) %> <% end %> <%= table.with_head do |head| %> <%= head.with_row do |row| %> diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb index 411d792c1..1de12ab77 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "organisations") %> +<% item_label = format_label(@pagy.count, "organisation") %> <% title = format_title(@searched, "Organisations", current_user, item_label, @pagy.count, nil) %> <% content_for :title, title %> diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb index e172a76a9..e318ff6ee 100644 --- a/app/views/organisations/logs.html.erb +++ b/app/views/organisations/logs.html.erb @@ -1,4 +1,4 @@ -<% item_label = format_label(@pagy.count, "logs") %> +<% item_label = format_label(@pagy.count, "log") %> <% title = format_title(@searched, action_name.humanize, current_user, item_label, @pagy.count, @organisation.name) %> <% content_for :title, title %> diff --git a/app/views/schemes/_scheme_list.html.erb b/app/views/schemes/_scheme_list.html.erb index 967295236..1c11e86d1 100644 --- a/app/views/schemes/_scheme_list.html.erb +++ b/app/views/schemes/_scheme_list.html.erb @@ -2,7 +2,7 @@ <%= govuk_table do |table| %> <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "schemes", filters_count: applied_filters_count(@filter_type))) %> + <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "scheme", filters_count: applied_filters_count(@filter_type))) %> <% if @schemes&.any? %> <%= govuk_link_to "Download schemes (CSV)", schemes_csv_download_url, type: "text/csv", class: "govuk-!-margin-right-4", style: "white-space: nowrap" %> <%= govuk_link_to "Download locations (CSV)", locations_csv_download_url, type: "text/csv", class: "govuk-!-margin-right-4", style: "white-space: nowrap" %> diff --git a/app/views/users/_user_list.html.erb b/app/views/users/_user_list.html.erb index 436c0def2..82a82b33b 100644 --- a/app/views/users/_user_list.html.erb +++ b/app/views/users/_user_list.html.erb @@ -1,7 +1,7 @@
<%= govuk_table do |table| %> <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "users", filters_count: applied_filters_count(@filter_type))) %> + <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "user", filters_count: applied_filters_count(@filter_type))) %> <% if current_user.support? %> <% query = searched.present? ? "?search=#{searched}" : nil %> <%= govuk_link_to "Download (CSV)", "#{request.path}.csv#{query}", type: "text/csv", style: "white-space: nowrap" %> diff --git a/spec/components/search_result_caption_component_spec.rb b/spec/components/search_result_caption_component_spec.rb index 25cbc1bdd..05ac09630 100644 --- a/spec/components/search_result_caption_component_spec.rb +++ b/spec/components/search_result_caption_component_spec.rb @@ -5,7 +5,7 @@ RSpec.describe SearchResultCaptionComponent, type: :component do let(:count) { 2 } let(:item_label) { "user" } let(:total_count) { 3 } - let(:item) { "schemes" } + let(:item) { "scheme" } let(:filters_count) { 1 } let(:result) { render_inline(described_class.new(searched:, count:, item_label:, total_count:, item:, filters_count:)) } @@ -21,6 +21,14 @@ RSpec.describe SearchResultCaptionComponent, type: :component do it "renders table caption including the search results and total" do expect(result.to_html).to eq("\n 2 users matching search
\n
\n") end + + context "with 1 result" do + let(:count) { 1 } + + it "renders table caption including the search results and total" do + expect(result.to_html).to eq("\n 1 user matching search
\n
\n") + end + end end context "when filter results are found" do @@ -29,6 +37,14 @@ RSpec.describe SearchResultCaptionComponent, type: :component do it "renders table caption including the search results and total" do expect(result.to_html).to eq("\n 2 users matching filters
\n
\n") end + + context "with 1 result" do + let(:count) { 1 } + + it "renders table caption including the search results and total" do + expect(result.to_html).to eq("\n 1 user matching filters
\n
\n") + end + end end context "when no search/filter is applied" do @@ -38,6 +54,14 @@ RSpec.describe SearchResultCaptionComponent, type: :component do it "renders table caption with total count only" do expect(result.to_html).to eq("\n \n 2 total schemes\n \n\n") end + + context "with 1 result" do + let(:count) { 1 } + + it "renders table caption with total count only" do + expect(result.to_html).to eq("\n \n 1 total scheme\n \n\n") + end + end end context "when nothing is found" do diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index e795b76ea..d84a6d714 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -759,7 +759,7 @@ RSpec.describe LettingsLogsController, type: :request do it "has search results in the title" do get "/lettings-logs?search=#{log_to_search.id}", headers:, params: {} - expect(page).to have_title("Lettings logs (1 logs matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") + expect(page).to have_title("Lettings logs (1 log matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") end it "shows lettings logs matching the id" do @@ -895,7 +895,7 @@ RSpec.describe LettingsLogsController, type: :request do end it "shows the total log count" do - expect(CGI.unescape_html(response.body)).to match("1 total logs") + expect(CGI.unescape_html(response.body)).to match("1 total log") end it "does not show the pagination links" do diff --git a/spec/requests/organisation_relationships_controller_spec.rb b/spec/requests/organisation_relationships_controller_spec.rb index 79706fff9..8e8dc4f2d 100644 --- a/spec/requests/organisation_relationships_controller_spec.rb +++ b/spec/requests/organisation_relationships_controller_spec.rb @@ -53,7 +53,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total stock owners") + expect(page).to have_content("1 total stock owner") end context "when adding a stock owner" do @@ -149,7 +149,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total managing agents") + expect(page).to have_content("1 total managing agent") end context "and current organisation is deactivated" do @@ -345,7 +345,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total stock owners") + expect(page).to have_content("1 total stock owner") end end @@ -481,7 +481,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total managing agents") + expect(page).to have_content("1 total managing agent") end end @@ -647,7 +647,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total stock owners") + expect(page).to have_content("1 total stock owner") end context "when adding a stock owner" do @@ -697,7 +697,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total managing agents") + expect(page).to have_content("1 total managing agent") end it "shows remove link(s)" do diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index ae3297d59..26723a563 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -1203,7 +1203,7 @@ RSpec.describe OrganisationsController, type: :request do it "has search results in the title" do get "/organisations/#{organisation.id}/lettings-logs?search=#{log_to_search.id}", headers: headers, params: {} - expect(page).to have_title("#{organisation.name} (1 logs matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") + expect(page).to have_title("#{organisation.name} (1 log matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") end it "has search term in the search box" do @@ -1352,7 +1352,7 @@ RSpec.describe OrganisationsController, type: :request do it "has search results in the title" do get "/organisations/#{organisation.id}/sales-logs?search=#{log_to_search.id}", headers: headers, params: {} - expect(page).to have_title("#{organisation.name} (1 logs matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") + expect(page).to have_title("#{organisation.name} (1 log matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") end it "shows sales logs matching the id" do @@ -1616,11 +1616,11 @@ RSpec.describe OrganisationsController, type: :request do end it "updates the table caption" do - expect(page).to have_content("1 organisations matching search") + expect(page).to have_content("1 organisation matching search") end it "has search in the title" do - expect(page).to have_title("Organisations (1 organisations matching ‘#{search_param}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") + expect(page).to have_title("Organisations (1 organisation matching ‘#{search_param}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") end context "when the search term matches more than 1 result" do diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index dc056c1ed..6c882cc87 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -610,7 +610,7 @@ RSpec.describe SalesLogsController, type: :request do it "has search results in the title" do get "/sales-logs?search=#{log_to_search.id}", headers: headers, params: {} - expect(page).to have_title("Sales logs (1 logs matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") + expect(page).to have_title("Sales logs (1 log matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") end it "shows sales logs matching the id" do @@ -692,7 +692,7 @@ RSpec.describe SalesLogsController, type: :request do end it "shows the total log count" do - expect(CGI.unescape_html(response.body)).to match("1 total logs") + expect(CGI.unescape_html(response.body)).to match("1 total log") end it "does not show the pagination links" do From af79b58741dc9e18098ccade83adedf9d1936e75 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Fri, 6 Dec 2024 16:51:49 +0000 Subject: [PATCH 13/19] Separate services tests to improve pipeline times (#2853) * Display slowest test * Attempt to improve test times on github * Run services tests in parallel * Try keeping services in Tests * Revert "Try keeping services in Tests" This reverts commit ef2e1bf4d255ed9a4bf87abda2b4d2b33787d406. * Fix parallel setup for services tests * Remove profiling --- .github/workflows/run_tests.yml | 411 +++++++++++++++++++++++++ .github/workflows/staging_pipeline.yml | 347 +-------------------- spec/db/seeds_spec.rb | 7 +- 3 files changed, 417 insertions(+), 348 deletions(-) create mode 100644 .github/workflows/run_tests.yml diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml new file mode 100644 index 000000000..1965a4034 --- /dev/null +++ b/.github/workflows/run_tests.yml @@ -0,0 +1,411 @@ +name: Run Tests + +on: + workflow_call: + pull_request: + types: + - opened + - synchronize + merge_group: + workflow_dispatch: + +defaults: + run: + shell: bash + +jobs: + test: + name: Tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:13.5 + env: + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + POSTGRES_DB: data_collector + ports: + - 5432:5432 + # Needed because the Postgres container does not provide a health check + # tmpfs makes database faster by using RAM + options: >- + --mount type=tmpfs,destination=/var/lib/postgresql/data + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.1.1 + DB_HOST: localhost + DB_DATABASE: data_collector + DB_USERNAME: postgres + DB_PASSWORD: password + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + PARALLEL_TEST_PROCESSORS: 4 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Create database + run: | + bundle exec rake parallel:setup + + - name: Compile assets + run: | + bundle exec rake assets:precompile + + - name: Run tests + run: | + bundle exec rake parallel:spec['spec\/(?!features|models|requests|services)'] + + feature_test: + name: Feature Tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:13.5 + env: + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + POSTGRES_DB: data_collector + ports: + - 5432:5432 + # Needed because the Postgres container does not provide a health check + # tmpfs makes database faster by using RAM + options: >- + --mount type=tmpfs,destination=/var/lib/postgresql/data + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.1.1 + DB_HOST: localhost + DB_DATABASE: data_collector + DB_USERNAME: postgres + DB_PASSWORD: password + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Create database + run: | + bundle exec rake db:prepare + + - name: Compile assets + run: | + bundle exec rake assets:precompile + + - name: Run tests + run: | + bundle exec rspec spec/features --fail-fast --exclude-pattern "spec/features/accessibility_spec.rb" + + model_test: + name: Model tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:13.5 + env: + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + POSTGRES_DB: data_collector + ports: + - 5432:5432 + # Needed because the Postgres container does not provide a health check + # tmpfs makes database faster by using RAM + options: >- + --mount type=tmpfs,destination=/var/lib/postgresql/data + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.1.1 + DB_HOST: localhost + DB_DATABASE: data_collector + DB_USERNAME: postgres + DB_PASSWORD: password + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Create database + run: | + bundle exec rake db:prepare + + - name: Compile assets + run: | + bundle exec rake assets:precompile + + - name: Run tests + run: | + bundle exec rspec spec/models --fail-fast + + requests_test: + name: Requests tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:13.5 + env: + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + POSTGRES_DB: data_collector + ports: + - 5432:5432 + # Needed because the Postgres container does not provide a health check + # tmpfs makes database faster by using RAM + options: >- + --mount type=tmpfs,destination=/var/lib/postgresql/data + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.1.1 + DB_HOST: localhost + DB_DATABASE: data_collector + DB_USERNAME: postgres + DB_PASSWORD: password + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + PARALLEL_TEST_PROCESSORS: 4 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Create database + run: | + bundle exec rake parallel:setup + + - name: Compile assets + run: | + bundle exec rake assets:precompile + + - name: Run tests + run: | + bundle exec rake parallel:spec['spec/requests'] + + services_test: + name: Services Tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:13.5 + env: + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + POSTGRES_DB: data_collector + ports: + - 5432:5432 + # Needed because the Postgres container does not provide a health check + # tmpfs makes database faster by using RAM + options: >- + --mount type=tmpfs,destination=/var/lib/postgresql/data + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.1.1 + DB_HOST: localhost + DB_DATABASE: data_collector + DB_USERNAME: postgres + DB_PASSWORD: password + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + PARALLEL_TEST_PROCESSORS: 4 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Create database + run: | + bundle exec rake parallel:setup + + - name: Compile assets + run: | + bundle exec rake assets:precompile + + - name: Run tests + run: | + bundle exec rake parallel:spec['spec\/services'] + + accessibility_test: + name: Accessibility tests + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:13.5 + env: + POSTGRES_PASSWORD: password + POSTGRES_USER: postgres + POSTGRES_DB: data_collector + ports: + - 5432:5432 + # Needed because the Postgres container does not provide a health check + # tmpfs makes database faster by using RAM + options: >- + --mount type=tmpfs,destination=/var/lib/postgresql/data + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + env: + RAILS_ENV: test + GEMFILE_RUBY_VERSION: 3.1.1 + DB_HOST: localhost + DB_DATABASE: data_collector + DB_USERNAME: postgres + DB_PASSWORD: password + RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} + PARALLEL_TEST_PROCESSORS: 4 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Create database + run: | + bundle exec rake parallel:setup + + - name: Compile assets + run: | + bundle exec rake assets:precompile + + - name: Run tests + run: | + bundle exec rspec spec/features/accessibility_spec.rb --fail-fast + + lint: + name: Lint + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + cache: yarn + node-version: 20 + + - name: Install packages and symlink local dependencies + run: | + yarn install --immutable --immutable-cache --check-cache + + - name: Lint + run: | + bundle exec rake lint + + audit: + name: Audit dependencies + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Audit + run: | + bundle exec bundler-audit diff --git a/.github/workflows/staging_pipeline.yml b/.github/workflows/staging_pipeline.yml index aff0fe5f0..a2e777db0 100644 --- a/.github/workflows/staging_pipeline.yml +++ b/.github/workflows/staging_pipeline.yml @@ -4,11 +4,6 @@ on: push: branches: - main - pull_request: - types: - - opened - - synchronize - merge_group: workflow_dispatch: defaults: @@ -21,347 +16,13 @@ env: repository: core jobs: - test: - name: Tests - runs-on: ubuntu-latest - - services: - postgres: - image: postgres:13.5 - env: - POSTGRES_PASSWORD: password - POSTGRES_USER: postgres - POSTGRES_DB: data_collector - ports: - - 5432:5432 - # Needed because the Postgres container does not provide a health check - # tmpfs makes database faster by using RAM - options: >- - --mount type=tmpfs,destination=/var/lib/postgresql/data - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - - env: - RAILS_ENV: test - GEMFILE_RUBY_VERSION: 3.1.1 - DB_HOST: localhost - DB_DATABASE: data_collector - DB_USERNAME: postgres - DB_PASSWORD: password - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} - PARALLEL_TEST_PROCESSORS: 4 - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - cache: yarn - node-version: 20 - - - name: Create database - run: | - bundle exec rake parallel:setup - - - name: Compile assets - run: | - bundle exec rake assets:precompile - - - name: Run tests - run: | - bundle exec rake parallel:spec['spec\/(?!features|models|requests)'] - - feature_test: - name: Feature Tests - runs-on: ubuntu-latest - - services: - postgres: - image: postgres:13.5 - env: - POSTGRES_PASSWORD: password - POSTGRES_USER: postgres - POSTGRES_DB: data_collector - ports: - - 5432:5432 - # Needed because the Postgres container does not provide a health check - # tmpfs makes database faster by using RAM - options: >- - --mount type=tmpfs,destination=/var/lib/postgresql/data - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - - env: - RAILS_ENV: test - GEMFILE_RUBY_VERSION: 3.1.1 - DB_HOST: localhost - DB_DATABASE: data_collector - DB_USERNAME: postgres - DB_PASSWORD: password - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - cache: yarn - node-version: 20 - - - name: Create database - run: | - bundle exec rake db:prepare - - - name: Compile assets - run: | - bundle exec rake assets:precompile - - - name: Run tests - run: | - bundle exec rspec spec/features --fail-fast --exclude-pattern "spec/features/accessibility_spec.rb" - - model_test: - name: Model tests - runs-on: ubuntu-latest - - services: - postgres: - image: postgres:13.5 - env: - POSTGRES_PASSWORD: password - POSTGRES_USER: postgres - POSTGRES_DB: data_collector - ports: - - 5432:5432 - # Needed because the Postgres container does not provide a health check - # tmpfs makes database faster by using RAM - options: >- - --mount type=tmpfs,destination=/var/lib/postgresql/data - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - - env: - RAILS_ENV: test - GEMFILE_RUBY_VERSION: 3.1.1 - DB_HOST: localhost - DB_DATABASE: data_collector - DB_USERNAME: postgres - DB_PASSWORD: password - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - cache: yarn - node-version: 20 - - - name: Create database - run: | - bundle exec rake db:prepare - - - name: Compile assets - run: | - bundle exec rake assets:precompile - - - name: Run tests - run: | - bundle exec rspec spec/models --fail-fast - - requests_test: - name: Requests tests - runs-on: ubuntu-latest - - services: - postgres: - image: postgres:13.5 - env: - POSTGRES_PASSWORD: password - POSTGRES_USER: postgres - POSTGRES_DB: data_collector - ports: - - 5432:5432 - # Needed because the Postgres container does not provide a health check - # tmpfs makes database faster by using RAM - options: >- - --mount type=tmpfs,destination=/var/lib/postgresql/data - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - - env: - RAILS_ENV: test - GEMFILE_RUBY_VERSION: 3.1.1 - DB_HOST: localhost - DB_DATABASE: data_collector - DB_USERNAME: postgres - DB_PASSWORD: password - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} - PARALLEL_TEST_PROCESSORS: 4 - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - cache: yarn - node-version: 20 - - - name: Create database - run: | - bundle exec rake parallel:setup - - - name: Compile assets - run: | - bundle exec rake assets:precompile - - - name: Run tests - run: | - bundle exec rake parallel:spec['spec/requests'] - - accessibility_test: - name: Accessibility tests - runs-on: ubuntu-latest - - services: - postgres: - image: postgres:13.5 - env: - POSTGRES_PASSWORD: password - POSTGRES_USER: postgres - POSTGRES_DB: data_collector - ports: - - 5432:5432 - # Needed because the Postgres container does not provide a health check - # tmpfs makes database faster by using RAM - options: >- - --mount type=tmpfs,destination=/var/lib/postgresql/data - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 - - env: - RAILS_ENV: test - GEMFILE_RUBY_VERSION: 3.1.1 - DB_HOST: localhost - DB_DATABASE: data_collector - DB_USERNAME: postgres - DB_PASSWORD: password - RAILS_MASTER_KEY: ${{ secrets.RAILS_MASTER_KEY }} - PARALLEL_TEST_PROCESSORS: 4 - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - cache: yarn - node-version: 20 - - - name: Create database - run: | - bundle exec rake parallel:setup - - - name: Compile assets - run: | - bundle exec rake assets:precompile - - - name: Run tests - run: | - bundle exec rspec spec/features/accessibility_spec.rb --fail-fast - - lint: - name: Lint - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Set up Node.js - uses: actions/setup-node@v4 - with: - cache: yarn - node-version: 20 - - - name: Install packages and symlink local dependencies - run: | - yarn install --immutable --immutable-cache --check-cache - - - name: Lint - run: | - bundle exec rake lint - - audit: - name: Audit dependencies - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Ruby - uses: ruby/setup-ruby@v1 - with: - bundler-cache: true - - - name: Audit - run: | - bundle exec bundler-audit + tests: + name: Run Tests + uses: ./.github/workflows/run_tests.yml aws_deploy: name: AWS Deploy - if: github.ref == 'refs/heads/main' - needs: [lint, test, feature_test, requests_test, model_test, audit] + needs: [tests] uses: ./.github/workflows/aws_deploy.yml with: aws_account_id: 107155005276 diff --git a/spec/db/seeds_spec.rb b/spec/db/seeds_spec.rb index 316f04ba6..6ae07ddb0 100644 --- a/spec/db/seeds_spec.rb +++ b/spec/db/seeds_spec.rb @@ -21,7 +21,8 @@ RSpec.describe "seeding process", type: task do allow(Rails.env).to receive(:review?).and_return(true) end - it "sets up correct data" do + # Doing this in one test should save ~2 minutes + it "sets up correct data idempotently" do expect { Rails.application.load_seed }.to change(User, :count) @@ -30,10 +31,6 @@ RSpec.describe "seeding process", type: task do .and change(Scheme, :count) .and change(Location, :count) .and change(LaRentRange, :count) - end - - it "is idempotent" do - Rails.application.load_seed expect { Rails.application.load_seed From 474ed4f348bfe85bb39ebbf0a0ae985b2e04ad81 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Mon, 9 Dec 2024 15:47:57 +0000 Subject: [PATCH 14/19] CLDC-3344: Sales CSV download ecstat2 child label bug fix (#2801) * Add missing answer option * Update tests * Allow user to change the age of a person if they're not actually under 16 without being blocked * Update displayed answer options * Add tests * Fix lint * Clear only when age is not known * Lint * Use existing method * Update tests * Revert changes in derived variables * Update sales_log_spec.rb --- .../questions/buyer2_working_situation.rb | 6 ++++ .../buyer2_working_situation_spec.rb | 28 +++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/app/models/form/sales/questions/buyer2_working_situation.rb b/app/models/form/sales/questions/buyer2_working_situation.rb index 38ab320b3..9105b8597 100644 --- a/app/models/form/sales/questions/buyer2_working_situation.rb +++ b/app/models/form/sales/questions/buyer2_working_situation.rb @@ -15,6 +15,10 @@ class Form::Sales::Questions::Buyer2WorkingSituation < ::Form::Question @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] end + def displayed_answer_options(_log, _user = nil) + answer_options.reject { |key, _| key == "9" } + end + def answer_options if form.start_year_2025_or_later? { @@ -26,6 +30,7 @@ class Form::Sales::Questions::Buyer2WorkingSituation < ::Form::Question "6" => { "value" => "Not seeking work" }, "7" => { "value" => "Full-time student" }, "8" => { "value" => "Unable to work due to long term sick or disability" }, + "9" => { "value" => "Child under 16" }, "0" => { "value" => "Other" }, "10" => { "value" => "Buyer prefers not to say" }, }.freeze @@ -41,6 +46,7 @@ class Form::Sales::Questions::Buyer2WorkingSituation < ::Form::Question "0" => { "value" => "Other" }, "10" => { "value" => "Buyer prefers not to say" }, "7" => { "value" => "Full-time student" }, + "9" => { "value" => "Child under 16" }, }.freeze end end diff --git a/spec/models/form/sales/questions/buyer2_working_situation_spec.rb b/spec/models/form/sales/questions/buyer2_working_situation_spec.rb index 7b825185c..c8ee349b6 100644 --- a/spec/models/form/sales/questions/buyer2_working_situation_spec.rb +++ b/spec/models/form/sales/questions/buyer2_working_situation_spec.rb @@ -36,6 +36,22 @@ RSpec.describe Form::Sales::Questions::Buyer2WorkingSituation, type: :model do "0" => { "value" => "Other" }, "10" => { "value" => "Buyer prefers not to say" }, "7" => { "value" => "Full-time student" }, + "9" => { "value" => "Child under 16" }, + }) + end + + it "has the correct displayed_answer_options" do + expect(question.displayed_answer_options(nil)).to eq({ + "1" => { "value" => "Full-time - 30 hours or more" }, + "2" => { "value" => "Part-time - Less than 30 hours" }, + "3" => { "value" => "In government training into work" }, + "4" => { "value" => "Jobseeker" }, + "6" => { "value" => "Not seeking work" }, + "8" => { "value" => "Unable to work due to long term sick or disability" }, + "5" => { "value" => "Retired" }, + "0" => { "value" => "Other" }, + "10" => { "value" => "Buyer prefers not to say" }, + "7" => { "value" => "Full-time student" }, }) end @@ -43,7 +59,11 @@ RSpec.describe Form::Sales::Questions::Buyer2WorkingSituation, type: :model do let(:form) { instance_double(Form, start_date: Time.zone.local(2024, 4, 1), start_year_2025_or_later?: false) } it "uses the old ordering for answer options" do - expect(question.answer_options.keys).to eq(%w[1 2 3 4 6 8 5 0 10 7]) + expect(question.answer_options.keys).to eq(%w[1 2 3 4 6 8 5 0 10 7 9]) + end + + it "uses the old ordering for displayed answer options" do + expect(question.displayed_answer_options(nil).keys).to eq(%w[1 2 3 4 6 8 5 0 10 7]) end end @@ -51,7 +71,11 @@ RSpec.describe Form::Sales::Questions::Buyer2WorkingSituation, type: :model do let(:form) { instance_double(Form, start_date: Time.zone.local(2025, 4, 1), start_year_2025_or_later?: true) } it "uses the new ordering for answer options" do - expect(question.answer_options.keys).to eq(%w[1 2 3 4 5 6 7 8 0 10]) + expect(question.answer_options.keys).to eq(%w[1 2 3 4 5 6 7 8 9 0 10]) + end + + it "uses the new ordering for displayed answer options" do + expect(question.displayed_answer_options(nil).keys).to eq(%w[1 2 3 4 5 6 7 8 0 10]) end end From 8594561a18eac0f351290662d370a390e29a9388 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:27:46 +0000 Subject: [PATCH 15/19] Stub some credentials (#2857) --- spec/mailers/devise_notify_mailer_spec.rb | 11 +++++++---- spec/models/user_spec.rb | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/mailers/devise_notify_mailer_spec.rb b/spec/mailers/devise_notify_mailer_spec.rb index 4ed209b24..7a7123be0 100644 --- a/spec/mailers/devise_notify_mailer_spec.rb +++ b/spec/mailers/devise_notify_mailer_spec.rb @@ -36,8 +36,11 @@ RSpec.describe DeviseNotifyMailer do end context "when the email domain is in the allowlist" do - let(:domain) { Rails.application.credentials[:email_allowlist].first } - let(:email) { "test@#{domain}" } + before do + allow(Rails.application.credentials).to receive(:[]).with(:email_allowlist).and_return(["example.com"]) + end + + let(:email) { "test@example.com" } it "does send emails" do expect(notify_client).to receive(:send_email).once @@ -48,10 +51,10 @@ RSpec.describe DeviseNotifyMailer do context "when notify mailer raises BadRequestError" do before do allow(notify_client).to receive(:send_email).and_raise(bad_request_error) + allow(Rails.application.credentials).to receive(:[]).with(:email_allowlist).and_return(["example.com"]) end - let(:domain) { Rails.application.credentials[:email_allowlist].first } - let(:email) { "test@#{domain}" } + let(:email) { "test@example.com" } it "does not raise an error" do expect { diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 51cfc00bd..53561f3e9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -300,6 +300,7 @@ RSpec.describe User, type: :model do context "when the user is in staging environment" do before do allow(Rails.env).to receive(:staging?).and_return(true) + allow(Rails.application.credentials).to receive(:[]).with(:staging_role_update_email_allowlist).and_return(["not_one_of_the_examples.com"]) end context "and the user is not in the staging role update email allowlist" do From e7490f1381dbad8180179cbd814bb90684f8fb8d Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 11 Dec 2024 11:45:53 +0000 Subject: [PATCH 16/19] CLDC-3779 Add duplicate logs email (#2843) * Find block log creation reason * Update mailer * Remove create_logs? method --- app/mailers/bulk_upload_mailer.rb | 21 ++++++++++ .../bulk_upload/lettings/validator.rb | 14 +++---- app/services/bulk_upload/processor.rb | 40 +++++++++++++------ app/services/bulk_upload/sales/validator.rb | 12 +++--- spec/mailers/bulk_upload_mailer_spec.rb | 25 ++++++++++++ .../bulk_upload/lettings/validator_spec.rb | 10 ++--- spec/services/bulk_upload/processor_spec.rb | 32 +++++++++++++-- .../bulk_upload/sales/validator_spec.rb | 16 ++++---- 8 files changed, 124 insertions(+), 46 deletions(-) diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index e0115abb0..7c62f71a1 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/app/mailers/bulk_upload_mailer.rb @@ -3,6 +3,7 @@ class BulkUploadMailer < NotifyMailer COMPLETE_TEMPLATE_ID = "83279578-c890-4168-838b-33c9cf0dc9f0".freeze FAILED_CSV_ERRORS_TEMPLATE_ID = "e27abcd4-5295-48c2-b127-e9ee4b781b75".freeze + FAILED_CSV_DUPLICATE_ERRORS_TEMPLATE_ID = "931d5bda-a08f-4de6-a455-38a63bff1956".freeze FAILED_FILE_SETUP_ERROR_TEMPLATE_ID = "24c9f4c7-96ad-470a-ba31-eb51b7cbafd9".freeze FAILED_SERVICE_ERROR_TEMPLATE_ID = "c3f6288c-7a74-4e77-99ee-6c4a0f6e125a".freeze HOW_TO_FIX_UPLOAD_TEMPLATE_ID = "21a07b26-f625-4846-9f4d-39e30937aa24".freeze @@ -95,6 +96,26 @@ class BulkUploadMailer < NotifyMailer ) end + def send_correct_duplicates_and_upload_again_mail(bulk_upload:) + summary_report_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? + bulk_upload.sales? ? summary_bulk_upload_sales_result_url(bulk_upload) : summary_bulk_upload_lettings_result_url(bulk_upload) + else + bulk_upload.sales? ? bulk_upload_sales_result_url(bulk_upload) : bulk_upload_lettings_result_url(bulk_upload) + end + + send_email( + bulk_upload.user.email, + FAILED_CSV_DUPLICATE_ERRORS_TEMPLATE_ID, + { + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), + year_combo: bulk_upload.year_combo, + lettings_or_sales: bulk_upload.log_type, + summary_report_link:, + }, + ) + end + def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:) bulk_upload_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? bulk_upload.sales? ? summary_bulk_upload_sales_result_url(bulk_upload) : summary_bulk_upload_lettings_result_url(bulk_upload) diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 116c3b745..291bf45e7 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -40,29 +40,25 @@ class BulkUpload::Lettings::Validator end end - def create_logs? - return false if any_setup_errors? + def block_log_creation_reason + return "setup_errors" if any_setup_errors? if row_parsers.any?(&:block_log_creation?) Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") - return false + return "row_parser_block_log_creation" end if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled? - Sentry.capture_message("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") - return false + return "duplicate_logs" end row_parsers.each do |row_parser| row_parser.log.blank_invalid_non_setup_fields! end - if any_logs_invalid? Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") - return false + "logs_invalid" end - - true end def self.question_for_field(field) diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 38f67ede4..c54032fda 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -36,20 +36,28 @@ class BulkUpload::Processor if validator.any_setup_errors? send_setup_errors_mail - - elsif validator.create_logs? - create_logs - - if validator.soft_validation_errors_only? - send_check_soft_validations_mail - elsif created_logs_but_incompleted? - send_how_to_fix_upload_mail - elsif created_logs_and_all_completed? - bulk_upload.unpend - send_success_mail - end else - send_correct_and_upload_again_mail # summary/full report + block_creation_reason = validator.block_log_creation_reason + + if block_creation_reason.present? + case block_creation_reason + when "duplicate_logs" + send_correct_duplicates_and_upload_again_mail + else + send_correct_and_upload_again_mail # summary/full report + end + else + create_logs + + if validator.soft_validation_errors_only? + send_check_soft_validations_mail + elsif created_logs_but_incompleted? + send_how_to_fix_upload_mail + elsif created_logs_and_all_completed? + bulk_upload.unpend + send_success_mail + end + end end rescue StandardError => e Sentry.capture_exception(e) @@ -97,6 +105,12 @@ private .deliver_later end + def send_correct_duplicates_and_upload_again_mail + BulkUploadMailer + .send_correct_duplicates_and_upload_again_mail(bulk_upload:) + .deliver_later + end + def send_success_mail BulkUploadMailer .send_bulk_upload_complete_mail(user:, bulk_upload:) diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index 76fb6f1ae..7ad9638d7 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -39,17 +39,17 @@ class BulkUpload::Sales::Validator end end - def create_logs? - return false if any_setup_errors? + def block_log_creation_reason + return "setup_errors" if any_setup_errors? if row_parsers.any?(&:block_log_creation?) Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") - return false + return "row_parser_block_log_creation" end if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled? Sentry.capture_message("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") - return false + return "duplicate_logs" end row_parsers.each do |row_parser| @@ -58,10 +58,8 @@ class BulkUpload::Sales::Validator if any_logs_invalid? Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") - return false + "logs_invalid" end - - true end def any_setup_errors? diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index c3225c937..910bca4a9 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -121,4 +121,29 @@ RSpec.describe BulkUploadMailer do mailer.send_check_soft_validations_mail(bulk_upload:) end end + + describe "#send_correct_duplicates_and_upload_again_mail" do + context "when 2 columns with errors" do + before do + create(:bulk_upload_error, bulk_upload:, col: "A") + create(:bulk_upload_error, bulk_upload:, col: "B") + end + + it "sends correctly formed email" do + expect(notify_client).to receive(:send_email).with( + email_address: user.email, + template_id: described_class::FAILED_CSV_DUPLICATE_ERRORS_TEMPLATE_ID, + personalisation: { + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), + year_combo: bulk_upload.year_combo, + lettings_or_sales: bulk_upload.log_type, + summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}", + }, + ) + + mailer.send_correct_duplicates_and_upload_again_mail(bulk_upload:) + end + end + end end diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index 897010de6..60eb8a955 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -232,7 +232,7 @@ RSpec.describe BulkUpload::Lettings::Validator do end end - describe "#create_logs?" do + describe "#block_log_creation_reason" do context "when a log has a clearable, non-setup error" do let(:log_1) { build(:lettings_log, :completed, period: 2, assigned_to: user) } let(:log_2) { build(:lettings_log, :completed, period: 2, assigned_to: user, age1: 5) } @@ -245,7 +245,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "returns false" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -261,7 +261,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "returns true" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -277,7 +277,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "will not create logs" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end @@ -291,7 +291,7 @@ RSpec.describe BulkUpload::Lettings::Validator do it "returns false" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index de0ed2dba..0368635e7 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -14,7 +14,7 @@ RSpec.describe BulkUpload::Processor do call: nil, total_logs_count: 1, any_setup_errors?: false, - create_logs?: true, + block_log_creation_reason: nil, soft_validation_errors_only?: false, ) end @@ -165,7 +165,7 @@ RSpec.describe BulkUpload::Processor do let(:log) { build(:lettings_log, :setup_completed, assigned_to: user) } before do - allow(mock_validator).to receive(:create_logs?).and_return(true) + allow(mock_validator).to receive(:block_log_creation_reason).and_return(nil) allow(mock_validator).to receive(:soft_validation_errors_only?).and_return(false) allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true) end @@ -198,7 +198,7 @@ RSpec.describe BulkUpload::Processor do context "when a bulk upload has logs with only soft validations triggered" do before do - allow(mock_validator).to receive(:create_logs?).and_return(true) + allow(mock_validator).to receive(:block_log_creation_reason).and_return(nil) allow(mock_validator).to receive(:soft_validation_errors_only?).and_return(true) allow(FeatureToggle).to receive(:bulk_upload_duplicate_log_check_enabled?).and_return(true) end @@ -239,7 +239,7 @@ RSpec.describe BulkUpload::Processor do call: nil, total_logs_count: 1, any_setup_errors?: false, - create_logs?: false, + block_log_creation_reason: "row_parser_block_log_creation", ) end @@ -254,6 +254,30 @@ RSpec.describe BulkUpload::Processor do expect(mail_double).to have_received(:deliver_later) end end + + context "when upload has duplicate logs blocking log creation" do + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + call: nil, + total_logs_count: 1, + any_setup_errors?: false, + block_log_creation_reason: "duplicate_logs", + ) + end + + it "sends correct_and_upload_again_mail" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_correct_duplicates_and_upload_again_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_correct_duplicates_and_upload_again_mail) + expect(mail_double).to have_received(:deliver_later) + end + end end describe "#approve" do diff --git a/spec/services/bulk_upload/sales/validator_spec.rb b/spec/services/bulk_upload/sales/validator_spec.rb index c275ce681..968014e7c 100644 --- a/spec/services/bulk_upload/sales/validator_spec.rb +++ b/spec/services/bulk_upload/sales/validator_spec.rb @@ -204,7 +204,7 @@ RSpec.describe BulkUpload::Sales::Validator do end end - describe "#create_logs?" do + describe "#block_log_creation_reason" do context "when all logs are valid" do let(:log_1) { build(:sales_log, :completed, assigned_to: user) } let(:log_2) { build(:sales_log, :completed, assigned_to: user) } @@ -214,9 +214,9 @@ RSpec.describe BulkUpload::Sales::Validator do file.write(BulkUpload::SalesLogToCsv.new(log: log_2).to_csv_row) end - it "returns truthy" do + it "returns nil" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -229,9 +229,9 @@ RSpec.describe BulkUpload::Sales::Validator do file.write(BulkUpload::SalesLogToCsv.new(log: log_2).to_csv_row) end - it "returns truthy" do + it "returns nil" do validator.call - expect(validator).to be_create_logs + expect(validator.block_log_creation_reason).to be_nil end end @@ -245,9 +245,9 @@ RSpec.describe BulkUpload::Sales::Validator do file.close end - it "returns false" do + it "returns the reason" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end @@ -262,7 +262,7 @@ RSpec.describe BulkUpload::Sales::Validator do it "will not create logs" do validator.call - expect(validator).not_to be_create_logs + expect(validator.block_log_creation_reason).to eq("setup_errors") end end end From 43ae7add3cd02ac2f6be10033c808d1d95b77f80 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:21:47 +0000 Subject: [PATCH 17/19] Patch rails (#2861) --- Gemfile | 2 +- Gemfile.lock | 124 +++++++++++++++++++++++++-------------------------- 2 files changed, 63 insertions(+), 63 deletions(-) diff --git a/Gemfile b/Gemfile index e9af29d55..027ed10d0 100644 --- a/Gemfile +++ b/Gemfile @@ -6,7 +6,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } ruby "3.1.4" # Bundle edge Rails instead: gem 'rails', github: 'rails/rails', branch: 'main' -gem "rails", "~> 7.0.8.5" +gem "rails", "~> 7.0.8.7" # Use postgresql as the database for Active Record gem "pg", "~> 1.1" # Use Puma as the app server diff --git a/Gemfile.lock b/Gemfile.lock index d2a5de05d..329c92051 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,71 +1,71 @@ GEM remote: https://rubygems.org/ specs: - actioncable (7.0.8.5) - actionpack (= 7.0.8.5) - activesupport (= 7.0.8.5) + actioncable (7.0.8.7) + actionpack (= 7.0.8.7) + activesupport (= 7.0.8.7) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (7.0.8.5) - actionpack (= 7.0.8.5) - activejob (= 7.0.8.5) - activerecord (= 7.0.8.5) - activestorage (= 7.0.8.5) - activesupport (= 7.0.8.5) + actionmailbox (7.0.8.7) + actionpack (= 7.0.8.7) + activejob (= 7.0.8.7) + activerecord (= 7.0.8.7) + activestorage (= 7.0.8.7) + activesupport (= 7.0.8.7) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.0.8.5) - actionpack (= 7.0.8.5) - actionview (= 7.0.8.5) - activejob (= 7.0.8.5) - activesupport (= 7.0.8.5) + actionmailer (7.0.8.7) + actionpack (= 7.0.8.7) + actionview (= 7.0.8.7) + activejob (= 7.0.8.7) + activesupport (= 7.0.8.7) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.0) - actionpack (7.0.8.5) - actionview (= 7.0.8.5) - activesupport (= 7.0.8.5) + actionpack (7.0.8.7) + actionview (= 7.0.8.7) + activesupport (= 7.0.8.7) rack (~> 2.0, >= 2.2.4) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (7.0.8.5) - actionpack (= 7.0.8.5) - activerecord (= 7.0.8.5) - activestorage (= 7.0.8.5) - activesupport (= 7.0.8.5) + actiontext (7.0.8.7) + actionpack (= 7.0.8.7) + activerecord (= 7.0.8.7) + activestorage (= 7.0.8.7) + activesupport (= 7.0.8.7) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.0.8.5) - activesupport (= 7.0.8.5) + actionview (7.0.8.7) + activesupport (= 7.0.8.7) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (7.0.8.5) - activesupport (= 7.0.8.5) + activejob (7.0.8.7) + activesupport (= 7.0.8.7) globalid (>= 0.3.6) - activemodel (7.0.8.5) - activesupport (= 7.0.8.5) + activemodel (7.0.8.7) + activesupport (= 7.0.8.7) activemodel-serializers-xml (1.0.2) activemodel (> 5.x) activesupport (> 5.x) builder (~> 3.1) - activerecord (7.0.8.5) - activemodel (= 7.0.8.5) - activesupport (= 7.0.8.5) - activestorage (7.0.8.5) - actionpack (= 7.0.8.5) - activejob (= 7.0.8.5) - activerecord (= 7.0.8.5) - activesupport (= 7.0.8.5) + activerecord (7.0.8.7) + activemodel (= 7.0.8.7) + activesupport (= 7.0.8.7) + activestorage (7.0.8.7) + actionpack (= 7.0.8.7) + activejob (= 7.0.8.7) + activerecord (= 7.0.8.7) + activesupport (= 7.0.8.7) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (7.0.8.5) + activesupport (7.0.8.7) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -149,7 +149,7 @@ GEM crass (1.0.6) cssbundling-rails (1.4.0) railties (>= 6.0.0) - date (3.3.4) + date (3.4.1) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) devise (4.9.3) @@ -258,13 +258,13 @@ GEM matrix (0.4.2) method_source (1.1.0) mini_mime (1.1.5) - minitest (5.25.1) + minitest (5.25.4) msgpack (1.7.2) multipart-post (2.4.1) nested_form (0.3.2) net-http (0.4.1) uri - net-imap (0.4.17) + net-imap (0.5.1) date net-protocol net-pop (0.1.2) @@ -273,12 +273,12 @@ GEM timeout net-smtp (0.5.0) net-protocol - nio4r (2.7.3) - nokogiri (1.16.8-arm64-darwin) + nio4r (2.7.4) + nokogiri (1.17.1-arm64-darwin) racc (~> 1.4) - nokogiri (1.16.8-x86_64-darwin) + nokogiri (1.17.1-x86_64-darwin) racc (~> 1.4) - nokogiri (1.16.8-x86_64-linux) + nokogiri (1.17.1-x86_64-linux) racc (~> 1.4) notifications-ruby-client (6.0.0) jwt (>= 1.5, < 3) @@ -327,20 +327,20 @@ GEM rack (>= 1.2.0) rack-test (2.1.0) rack (>= 1.3) - rails (7.0.8.5) - actioncable (= 7.0.8.5) - actionmailbox (= 7.0.8.5) - actionmailer (= 7.0.8.5) - actionpack (= 7.0.8.5) - actiontext (= 7.0.8.5) - actionview (= 7.0.8.5) - activejob (= 7.0.8.5) - activemodel (= 7.0.8.5) - activerecord (= 7.0.8.5) - activestorage (= 7.0.8.5) - activesupport (= 7.0.8.5) + rails (7.0.8.7) + actioncable (= 7.0.8.7) + actionmailbox (= 7.0.8.7) + actionmailer (= 7.0.8.7) + actionpack (= 7.0.8.7) + actiontext (= 7.0.8.7) + actionview (= 7.0.8.7) + activejob (= 7.0.8.7) + activemodel (= 7.0.8.7) + activerecord (= 7.0.8.7) + activestorage (= 7.0.8.7) + activesupport (= 7.0.8.7) bundler (>= 1.15.0) - railties (= 7.0.8.5) + railties (= 7.0.8.7) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -354,9 +354,9 @@ GEM nested_form (~> 0.3) rails (>= 6.0, < 8) turbo-rails (~> 1.0) - railties (7.0.8.5) - actionpack (= 7.0.8.5) - activesupport (= 7.0.8.5) + railties (7.0.8.7) + actionpack (= 7.0.8.7) + activesupport (= 7.0.8.7) method_source rake (>= 12.2) thor (~> 1.0) @@ -465,7 +465,7 @@ GEM thor (1.3.2) thread_safe (0.3.6) timecop (0.9.8) - timeout (0.4.1) + timeout (0.4.2) turbo-rails (1.5.0) actionpack (>= 6.0.0) activejob (>= 6.0.0) @@ -553,7 +553,7 @@ DEPENDENCIES rack (>= 2.2.6.3) rack-attack rack-mini-profiler (~> 2.0) - rails (~> 7.0.8.5) + rails (~> 7.0.8.7) rails_admin (~> 3.1) redcarpet (~> 3.6) redis (~> 4.8) From a7964e64588941c8800c2c9814a4775b5eab32b5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:51:59 +0000 Subject: [PATCH 18/19] Bump nanoid from 3.3.7 to 3.3.8 (#2863) Bumps [nanoid](https://github.com/ai/nanoid) from 3.3.7 to 3.3.8. - [Release notes](https://github.com/ai/nanoid/releases) - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md) - [Commits](https://github.com/ai/nanoid/compare/3.3.7...3.3.8) --- updated-dependencies: - dependency-name: nanoid dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 3e9afa0d8..94b220fe1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4335,9 +4335,9 @@ mute-stream@0.0.8: integrity sha512-nnbWWOkoWyUsTjKrhgD0dcz22mdkSnpYqbEjIm2nhwhuxlSkpywJmBo8h0ZqJdkp73mb90SssHkN4rsRaBAfAA== nanoid@^3.3.7: - version "3.3.7" - resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.7.tgz#d0c301a691bc8d54efa0a2226ccf3fe2fd656bd8" - integrity sha512-eSRppjcPIatRIMC1U6UngP8XFcz8MQWGQdt1MTBQ7NaAmvXDfvNxbvWV3x2y6CdEUciCSsDHDQZbhYaB8QEo2g== + version "3.3.8" + resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.8.tgz#b1be3030bee36aaff18bacb375e5cce521684baf" + integrity sha512-WNLf5Sd8oZxOm+TzppcYk8gVOgP+l58xNy58D0nbUnOxOWRWvlcCV4kUF7ltmI6PsrLl/BgKEyS4mqsGChFN0w== natural-compare@^1.4.0: version "1.4.0" From ca2d670116f1399fd48db3d07fd36c63ec963020 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:52:13 +0000 Subject: [PATCH 19/19] CLDC-3748 Check org name uniqueness (#2818) * Check org name uniqueness * Some tests * Do not update org name * Update some request specs * Remove unnecessary updates * Update error message --- app/controllers/organisations_controller.rb | 1 + app/models/organisation.rb | 1 + config/locales/en.yml | 1 + .../20241125153349_add_unique_index_to_org_name.rb | 5 +++++ db/schema.rb | 1 + spec/fixtures/exports/general_needs_log.xml | 4 ++-- spec/fixtures/exports/general_needs_log_23_24.xml | 4 ++-- spec/fixtures/exports/general_needs_log_24_25.xml | 4 ++-- spec/fixtures/exports/organisation.xml | 2 +- spec/fixtures/exports/supported_housing_logs.xml | 4 ++-- spec/fixtures/exports/user.xml | 2 +- .../lettings/questions/managing_organisation_spec.rb | 2 +- spec/models/organisation_spec.rb | 6 ++++++ .../organisation_relationships_controller_spec.rb | 8 ++++---- spec/requests/organisations_controller_spec.rb | 10 ++++++++-- .../exports/lettings_log_export_service_spec.rb | 2 ++ .../exports/organisation_export_service_spec.rb | 1 + spec/services/exports/user_export_service_spec.rb | 1 + 18 files changed, 42 insertions(+), 17 deletions(-) create mode 100644 db/migrate/20241125153349_add_unique_index_to_org_name.rb diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 8ffe426d7..93b667a99 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -155,6 +155,7 @@ class OrganisationsController < ApplicationController end redirect_to details_organisation_path(@organisation) else + @used_rent_periods = @organisation.lettings_logs.pluck(:period).uniq.compact.map(&:to_s) @rent_periods = helpers.rent_periods_with_checked_attr(checked_periods: selected_rent_periods) render :edit, status: :unprocessable_entity end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 23f91f1ad..69c80d198 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -58,6 +58,7 @@ class Organisation < ApplicationRecord alias_method :la?, :LA? validates :name, presence: { message: I18n.t("validations.organisation.name_missing") } + validates :name, uniqueness: { case_sensitive: false, message: I18n.t("validations.organisation.name_not_unique") } validates :provider_type, presence: { message: I18n.t("validations.organisation.provider_type_missing") } def self.find_by_id_on_multiple_fields(id) diff --git a/config/locales/en.yml b/config/locales/en.yml index 851a9ea2c..965c1f7a6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -235,6 +235,7 @@ en: organisation: data_sharing_agreement_not_signed: "Your organisation must accept the Data Sharing Agreement before you can create any logs." name_missing: "Enter the name of the organisation." + name_not_unique: "An organisation with this name already exists. Use the organisation that already exists or add a location or other identifier to the name. For example: Organisation name (City)." provider_type_missing: "Select the organisation type." stock_owner: blank: "You must choose a stock owner." diff --git a/db/migrate/20241125153349_add_unique_index_to_org_name.rb b/db/migrate/20241125153349_add_unique_index_to_org_name.rb new file mode 100644 index 000000000..a7a124183 --- /dev/null +++ b/db/migrate/20241125153349_add_unique_index_to_org_name.rb @@ -0,0 +1,5 @@ +class AddUniqueIndexToOrgName < ActiveRecord::Migration[7.0] + def change + add_index :organisations, :name, unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index d31a54da2..1a1c127c6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -546,6 +546,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_12_04_100518) do t.datetime "discarded_at" t.datetime "schemes_deduplicated_at" t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id" + t.index ["name"], name: "index_organisations_on_name", unique: true t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end diff --git a/spec/fixtures/exports/general_needs_log.xml b/spec/fixtures/exports/general_needs_log.xml index bacc7e9f0..0341dd2d4 100644 --- a/spec/fixtures/exports/general_needs_log.xml +++ b/spec/fixtures/exports/general_needs_log.xml @@ -147,10 +147,10 @@ {id} {owning_org_id} - MHCLG + {owning_org_name} 1234 {managing_org_id} - MHCLG + {managing_org_name} 1234 2022-05-01T00:00:00+01:00 2022-05-01T00:00:00+01:00 diff --git a/spec/fixtures/exports/general_needs_log_23_24.xml b/spec/fixtures/exports/general_needs_log_23_24.xml index 9635cd0e4..ef0c4066c 100644 --- a/spec/fixtures/exports/general_needs_log_23_24.xml +++ b/spec/fixtures/exports/general_needs_log_23_24.xml @@ -148,10 +148,10 @@ {id} {owning_org_id} - MHCLG + {owning_org_name} 1234 {managing_org_id} - MHCLG + {managing_org_name} 1234 2023-04-03T00:00:00+01:00 2023-04-03T00:00:00+01:00 diff --git a/spec/fixtures/exports/general_needs_log_24_25.xml b/spec/fixtures/exports/general_needs_log_24_25.xml index a665a284e..00d8bb1a5 100644 --- a/spec/fixtures/exports/general_needs_log_24_25.xml +++ b/spec/fixtures/exports/general_needs_log_24_25.xml @@ -161,10 +161,10 @@ la as entered {id} {owning_org_id} - MHCLG + {owning_org_name} 1234 {managing_org_id} - MHCLG + {managing_org_name} 1234 2024-04-03T00:00:00+01:00 2024-04-03T00:00:00+01:00 diff --git a/spec/fixtures/exports/organisation.xml b/spec/fixtures/exports/organisation.xml index 8d87da16c..70c699915 100644 --- a/spec/fixtures/exports/organisation.xml +++ b/spec/fixtures/exports/organisation.xml @@ -2,7 +2,7 @@
{id} - MHCLG + {name} 1 2 Marsham Street diff --git a/spec/fixtures/exports/supported_housing_logs.xml b/spec/fixtures/exports/supported_housing_logs.xml index 50649241b..310d5ba2b 100644 --- a/spec/fixtures/exports/supported_housing_logs.xml +++ b/spec/fixtures/exports/supported_housing_logs.xml @@ -146,10 +146,10 @@ {id} {owning_org_id} - MHCLG + {owning_org_name} 1234 {managing_org_id} - MHCLG + {managing_org_name} 1234 2022-05-01T00:00:00+01:00 2022-05-01T00:00:00+01:00 diff --git a/spec/fixtures/exports/user.xml b/spec/fixtures/exports/user.xml index 5652ac9c6..d29a33225 100644 --- a/spec/fixtures/exports/user.xml +++ b/spec/fixtures/exports/user.xml @@ -12,6 +12,6 @@ false false true - MHCLG + {organisation_name}
diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb index 776a873a6..00485e80f 100644 --- a/spec/models/form/lettings/questions/managing_organisation_spec.rb +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -185,7 +185,7 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do context "when organisation has merged" do let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) } let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) } - let!(:merged_deleted_org) { create(:organisation, name: "Merged org", holds_own_stock: false, discarded_at: Time.zone.yesterday) } + let!(:merged_deleted_org) { create(:organisation, name: "Merged org 2", holds_own_stock: false, discarded_at: Time.zone.yesterday) } let(:user) { create(:user, :data_coordinator, organisation: absorbing_org) } let(:log) do diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index 9b01845ae..b117feef7 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -19,6 +19,12 @@ RSpec.describe Organisation, type: :model do .to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Provider type #{I18n.t('validations.organisation.provider_type_missing')}") end + it "validates uniqueness of name" do + org = build(:organisation, name: organisation.name.downcase) + org.valid? + expect(org.errors[:name]).to include(I18n.t("validations.organisation.name_not_unique")) + end + context "with parent/child associations", :aggregate_failures do let!(:child_organisation) { create(:organisation, name: "MHCLG Child") } let!(:grandchild_organisation) { create(:organisation, name: "MHCLG Grandchild") } diff --git a/spec/requests/organisation_relationships_controller_spec.rb b/spec/requests/organisation_relationships_controller_spec.rb index 8e8dc4f2d..8733fba4b 100644 --- a/spec/requests/organisation_relationships_controller_spec.rb +++ b/spec/requests/organisation_relationships_controller_spec.rb @@ -58,7 +58,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do context "when adding a stock owner" do let!(:active_organisation) { FactoryBot.create(:organisation, name: "Active Org", active: true) } - let!(:inactive_organisation) { FactoryBot.create(:organisation, name: "Inactive LTD", active: false) } + let!(:inactive_organisation) { FactoryBot.create(:organisation, name: "Inactive LTD 2", active: false) } before do get "/organisations/#{organisation.id}/stock-owners/add", headers:, params: {} @@ -115,7 +115,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do let!(:managing_agent) { FactoryBot.create(:organisation) } let!(:other_org_managing_agent) { FactoryBot.create(:organisation, name: "Foobar LTD") } let!(:inactive_managing_agent) { FactoryBot.create(:organisation, name: "Inactive LTD", active: false) } - let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD") } + let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD 3") } before do FactoryBot.create(:organisation_relationship, parent_organisation: organisation, child_organisation: managing_agent) @@ -316,7 +316,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do context "with an organisation that the user belongs to" do let!(:stock_owner) { FactoryBot.create(:organisation) } let!(:other_org_stock_owner) { FactoryBot.create(:organisation, name: "Foobar LTD") } - let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD") } + let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD 2") } before do FactoryBot.create(:organisation_relationship, child_organisation: organisation, parent_organisation: stock_owner) @@ -452,7 +452,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do context "with an organisation that the user belongs to" do let!(:managing_agent) { FactoryBot.create(:organisation) } let!(:other_org_managing_agent) { FactoryBot.create(:organisation, name: "Foobar LTD") } - let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD") } + let!(:other_organisation) { FactoryBot.create(:organisation, name: "Foobar LTD 5") } before do FactoryBot.create(:organisation_relationship, parent_organisation: organisation, child_organisation: managing_agent) diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 26723a563..6c01e50bf 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -1555,7 +1555,10 @@ RSpec.describe OrganisationsController, type: :request do let(:total_organisations_count) { Organisation.all.count } before do - create_list(:organisation, 25) + build_list(:organisation, 25) do |organisation, index| + organisation.name = "Organisation #{index}" + organisation.save! + end get "/organisations" end @@ -1644,7 +1647,10 @@ RSpec.describe OrganisationsController, type: :request do let(:search_param) { "MHCLG" } before do - create_list(:organisation, 27, name: "MHCLG") + build_list(:organisation, 27) do |organisation, index| + organisation.name = "MHCLG #{index}" + organisation.save! + end get "/organisations?search=#{search_param}" end diff --git a/spec/services/exports/lettings_log_export_service_spec.rb b/spec/services/exports/lettings_log_export_service_spec.rb index 8c123b47e..c0dde5771 100644 --- a/spec/services/exports/lettings_log_export_service_spec.rb +++ b/spec/services/exports/lettings_log_export_service_spec.rb @@ -21,7 +21,9 @@ RSpec.describe Exports::LettingsLogExportService do def replace_entity_ids(lettings_log, export_template) export_template.sub!(/\{id\}/, (lettings_log["id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s) export_template.sub!(/\{owning_org_id\}/, (lettings_log["owning_organisation_id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s) + export_template.sub!(/\{owning_org_name\}/, lettings_log.owning_organisation.name) export_template.sub!(/\{managing_org_id\}/, (lettings_log["managing_organisation_id"] + Exports::LettingsLogExportService::LOG_ID_OFFSET).to_s) + export_template.sub!(/\{managing_org_name\}/, lettings_log.managing_organisation.name) export_template.sub!(/\{location_id\}/, (lettings_log["location_id"]).to_s) if lettings_log.needstype == 2 export_template.sub!(/\{scheme_id\}/, (lettings_log["scheme_id"]).to_s) if lettings_log.needstype == 2 export_template.sub!(/\{log_id\}/, lettings_log["id"].to_s) diff --git a/spec/services/exports/organisation_export_service_spec.rb b/spec/services/exports/organisation_export_service_spec.rb index 43ca19095..51c8fe8cf 100644 --- a/spec/services/exports/organisation_export_service_spec.rb +++ b/spec/services/exports/organisation_export_service_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Exports::OrganisationExportService do def replace_entity_ids(organisation, export_template) export_template.sub!(/\{id\}/, organisation["id"].to_s) + export_template.sub!(/\{name\}/, organisation["name"]) export_template.sub!(/\{dsa_signed_at\}/, organisation.data_protection_confirmation&.signed_at.to_s) export_template.sub!(/\{dpo_email\}/, organisation.data_protection_confirmation&.data_protection_officer_email) end diff --git a/spec/services/exports/user_export_service_spec.rb b/spec/services/exports/user_export_service_spec.rb index 8a0e22267..854dd1ce7 100644 --- a/spec/services/exports/user_export_service_spec.rb +++ b/spec/services/exports/user_export_service_spec.rb @@ -17,6 +17,7 @@ RSpec.describe Exports::UserExportService do def replace_entity_ids(user, export_template) export_template.sub!(/\{id\}/, user["id"].to_s) export_template.sub!(/\{organisation_id\}/, user["organisation_id"].to_s) + export_template.sub!(/\{organisation_name\}/, user.organisation.name) export_template.sub!(/\{email\}/, user["email"].to_s) end