diff --git a/.github/workflows/staging_pipeline.yml b/.github/workflows/staging_pipeline.yml index 415a943d1..8c0553741 100644 --- a/.github/workflows/staging_pipeline.yml +++ b/.github/workflows/staging_pipeline.yml @@ -14,6 +14,11 @@ defaults: run: shell: bash +env: + app_repo_role: arn:aws:iam::815624722760:role/core-application-repo + aws_region: eu-west-2 + repository: core + jobs: test: name: Tests @@ -364,3 +369,48 @@ jobs: environment: staging permissions: id-token: write + + performance: + needs: [aws_deploy] + runs-on: ubuntu-latest + permissions: + id-token: write + + steps: + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@v3 + 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 + with: + aws-region: eu-west-2 + role-to-assume: arn:aws:iam::107155005276:role/core-staging-deployment + role-chaining: true + + - name: Run Performance Test + env: + ad_hoc_task_definition: core-staging-ad-hoc + cluster: core-staging-app + service: core-staging-app + run: | + echo $cluster + network=$(aws ecs describe-services --cluster $cluster --services $service --query services[0].networkConfiguration) + overrides='{ + "containerOverrides": [{ + "name": "app", + "command": ["bash", "-c", "export email=$STAGING_PERFORMANCE_TEST_EMAIL && export password=$STAGING_PERFORMANCE_TEST_PASSWORD && sh ./lib/tasks/performance_test.sh"] + }] + }' + arn=$(aws ecs run-task --cluster $cluster --task-definition $ad_hoc_task_definition --network-configuration "$network" --overrides "$overrides" --group performance --launch-type FARGATE --query tasks[0].taskArn) + + echo "Waiting for performance tests to run" + task_id=${arn##*/} + task_id=${task_id%*\"} + + aws ecs wait tasks-stopped --cluster $cluster --tasks $task_id + + code=$(aws ecs describe-tasks --cluster $cluster --tasks $task_id --query "tasks[0].containers[0].exitCode") + if [ "$code == 0" ]; then exit 0; else exit 1; fi diff --git a/Dockerfile b/Dockerfile index a26c80ee9..46bd31b5f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,7 +10,7 @@ RUN apk add --update --no-cache tzdata && \ # build-base: compilation tools for bundle # yarn: node package manager # postgresql-dev: postgres driver and libraries -RUN apk add --no-cache build-base=0.5-r3 busybox=1.36.1-r7 nodejs-current=20.8.1-r0 yarn=1.22.19-r0 postgresql13-dev=13.15-r0 git=2.40.3-r0 bash=5.2.15-r5 +RUN apk add --no-cache build-base=0.5-r3 busybox=1.36.1-r7 nodejs-current=20.8.1-r0 yarn=1.22.19-r0 postgresql13-dev=13.16-r0 git=2.40.3-r0 bash=5.2.15-r5 # Bundler version should be the same version as what the Gemfile.lock was bundled with RUN gem install bundler:2.3.14 --no-document @@ -31,6 +31,9 @@ EXPOSE ${PORT} RUN adduser --system --no-create-home nonroot +RUN apk add curl +RUN apk add apache2-utils + FROM base as test RUN bundle config set without "" @@ -67,6 +70,9 @@ RUN mkdir -p tmp log RUN chown -R nonroot tmp log RUN chown nonroot db/schema.rb +RUN mkdir -p performance_test +RUN chown -R nonroot performance_test + USER nonroot CMD bundle exec rails s -e ${RAILS_ENV} -p ${PORT} --binding=0.0.0.0 diff --git a/Gemfile.lock b/Gemfile.lock index cdf923ae0..43d9374d1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -314,7 +314,7 @@ GEM byebug (~> 11.0) pry (>= 0.13, < 0.15) public_suffix (5.0.4) - puma (5.6.8) + puma (5.6.9) nio4r (~> 2.0) pundit (2.3.1) activesupport (>= 3.0.0) diff --git a/app/components/bulk_upload_error_row_component.html.erb b/app/components/bulk_upload_error_row_component.html.erb index 9447c9d37..65e38613f 100644 --- a/app/components/bulk_upload_error_row_component.html.erb +++ b/app/components/bulk_upload_error_row_component.html.erb @@ -9,24 +9,27 @@
<% potential_errors, critical_errors = bulk_upload_errors.partition { |error| error.category == "soft_validation" } %> -

Critical errors

-

These errors must be fixed to complete your logs.

- <%= govuk_table do |table| %> - <%= table.with_head do |head| %> - <% head.with_row do |row| %> - <% row.with_cell(header: true, text: "Cell") %> - <% row.with_cell(header: true, text: "Question") %> - <% row.with_cell(header: true, text: "Error") %> - <% row.with_cell(header: true, text: "Specification") %> - <% end %> - <%= table.with_body do |body| %> - <% critical_errors.each do |error| %> - <% body.with_row do |row| %> - <% row.with_cell(text: error.cell) %> - <% row.with_cell(text: question_for_field(error.field), html_attributes: { class: "govuk-!-width-one-half" }) %> - <% row.with_cell(text: error.error.html_safe, html_attributes: { class: "govuk-!-font-weight-bold govuk-!-width-one-half" }) %> - <% row.with_cell(text: error.field.humanize) %> + <% if critical_errors.any? %> +

Critical errors

+

These errors must be fixed to complete your logs.

+ <%= govuk_table do |table| %> + <%= table.with_head do |head| %> + <% head.with_row do |row| %> + <% row.with_cell(header: true, text: "Cell") %> + <% row.with_cell(header: true, text: "Question") %> + <% row.with_cell(header: true, text: "Error") %> + <% row.with_cell(header: true, text: "Specification") %> + <% end %> + + <%= table.with_body do |body| %> + <% critical_errors.each do |error| %> + <% body.with_row do |row| %> + <% row.with_cell(text: error.cell) %> + <% row.with_cell(text: question_for_field(error.field), html_attributes: { class: "govuk-!-width-one-half" }) %> + <% row.with_cell(text: error.error.html_safe, html_attributes: { class: "govuk-!-font-weight-bold govuk-!-width-one-half" }) %> + <% row.with_cell(text: error.field.humanize) %> + <% end %> <% end %> <% end %> <% end %> diff --git a/app/models/location.rb b/app/models/location.rb index 19cf5e211..03af24a94 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -121,6 +121,30 @@ class Location < ApplicationRecord scope :visible, -> { where(discarded_at: nil) } + scope :duplicate_sets, lambda { + scope = visible + .group(*DUPLICATE_LOCATION_ATTRIBUTES) + .where.not(scheme_id: nil) + .where.not(postcode: nil) + .where.not(mobility_type: nil) + .having( + "COUNT(*) > 1", + ) + scope.pluck("ARRAY_AGG(id)") + } + + scope :duplicate_sets_within_given_schemes, lambda { + scope = visible + .group(*DUPLICATE_LOCATION_ATTRIBUTES - %w[scheme_id]) + .where.not(postcode: nil) + .where.not(mobility_type: nil) + .having( + "COUNT(*) > 1", + ) + scope.pluck("ARRAY_AGG(id)") + } + + DUPLICATE_LOCATION_ATTRIBUTES = %w[scheme_id postcode mobility_type].freeze LOCAL_AUTHORITIES = LocalAuthority.all.map { |la| [la.name, la.code] }.to_h enum local_authorities: LOCAL_AUTHORITIES diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 6d3524723..2c73acc06 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -103,6 +103,22 @@ class Scheme < ApplicationRecord scope :visible, -> { where(discarded_at: nil) } + scope :duplicate_sets, lambda { + scope = visible + .group(*DUPLICATE_SCHEME_ATTRIBUTES) + .where.not(scheme_type: nil) + .where.not(registered_under_care_act: nil) + .where.not(primary_client_group: nil) + .where.not(has_other_client_group: nil) + .where.not(secondary_client_group: nil).or(where(has_other_client_group: 0)) + .where.not(support_type: nil) + .where.not(intended_stay: nil) + .having( + "COUNT(*) > 1", + ) + scope.pluck("ARRAY_AGG(id)") + } + validate :validate_confirmed validate :validate_owning_organisation @@ -192,6 +208,8 @@ class Scheme < ApplicationRecord "Missing": "X", }.freeze + DUPLICATE_SCHEME_ATTRIBUTES = %w[scheme_type registered_under_care_act primary_client_group secondary_client_group has_other_client_group support_type intended_stay].freeze + enum arrangement_type: ARRANGEMENT_TYPE, _suffix: true def self.find_by_id_on_multiple_fields(scheme_id, location_id) diff --git a/app/views/schemes/index.html.erb b/app/views/schemes/index.html.erb index d0ae44072..edaed6212 100644 --- a/app/views/schemes/index.html.erb +++ b/app/views/schemes/index.html.erb @@ -6,7 +6,9 @@ <%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Supported housing schemes", sub: nil } : { main: "Supported housing schemes", sub: current_user.organisation.name } %> <% if SchemePolicy.new(current_user, nil).create? %> - <%= govuk_button_link_to "Create a new supported housing scheme", new_scheme_path, html: { method: :post } %> +
+ <%= govuk_button_link_to "Create a new supported housing scheme", new_scheme_path, html: { method: :post } %> +
<% end %>
<%= render partial: "schemes/scheme_filters" %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 7da3deda1..4e5053563 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -6,7 +6,9 @@ <%= render partial: "organisations/headings", locals: current_user.support? ? { main: "Users", sub: nil } : { main: "Users", sub: current_user.organisation.name } %> <% if current_user.data_coordinator? || current_user.support? %> - <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %> +
+ <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %> +
<% end %>
<%= render partial: "users/user_filters" %> diff --git a/lib/tasks/count_duplicates.rake b/lib/tasks/count_duplicates.rake new file mode 100644 index 000000000..e65688b4d --- /dev/null +++ b/lib/tasks/count_duplicates.rake @@ -0,0 +1,63 @@ +namespace :count_duplicates do + desc "Count the number of duplicate schemes per organisation" + task scheme_duplicates_per_org: :environment do + duplicates_csv = CSV.generate(headers: true) do |csv| + csv << ["Organisation id", "Number of duplicate sets", "Total duplicate schemes"] + + Organisation.visible.each do |organisation| + if organisation.owned_schemes.duplicate_sets.count.positive? + csv << [organisation.id, organisation.owned_schemes.duplicate_sets.count, organisation.owned_schemes.duplicate_sets.sum(&:size)] + end + end + end + + filename = "scheme-duplicates-#{Time.zone.now}.csv" + storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"]) + storage_service.write_file(filename, "#{duplicates_csv}") + + url = storage_service.get_presigned_url(filename, 72.hours.to_i) + Rails.logger.info("Download URL: #{url}") + end + + desc "Count the number of duplicate locations per organisation" + task location_duplicates_per_org: :environment do + duplicates_csv = CSV.generate(headers: true) do |csv| + csv << ["Organisation id", "Duplicate sets within individual schemes", "Duplicate locations within individual schemes", "All duplicate sets", "All duplicates"] + + Organisation.visible.each do |organisation| + duplicate_sets_within_individual_schemes = [] + + organisation.owned_schemes.each do |scheme| + duplicate_sets_within_individual_schemes += scheme.locations.duplicate_sets + end + duplicate_locations_within_individual_schemes = duplicate_sets_within_individual_schemes.flatten + + duplicate_sets_within_duplicate_schemes = [] + if organisation.owned_schemes.duplicate_sets.count.positive? + organisation.owned_schemes.duplicate_sets.each do |duplicate_set| + duplicate_sets_within_duplicate_schemes += Location.where(scheme_id: duplicate_set).duplicate_sets_within_given_schemes + end + duplicate_locations_within_duplicate_schemes_ids = duplicate_sets_within_duplicate_schemes.flatten + + duplicate_sets_within_individual_schemes_without_intersecting_sets = duplicate_sets_within_individual_schemes.reject { |set| set.any? { |id| duplicate_sets_within_duplicate_schemes.any? { |duplicate_set| duplicate_set.include?(id) } } } + all_duplicate_sets_count = (duplicate_sets_within_individual_schemes_without_intersecting_sets + duplicate_sets_within_duplicate_schemes).count + all_duplicate_locations_count = (duplicate_locations_within_duplicate_schemes_ids + duplicate_locations_within_individual_schemes).uniq.count + else + all_duplicate_sets_count = duplicate_sets_within_individual_schemes.count + all_duplicate_locations_count = duplicate_locations_within_individual_schemes.count + end + + if all_duplicate_locations_count.positive? + csv << [organisation.id, duplicate_sets_within_individual_schemes.count, duplicate_locations_within_individual_schemes.count, all_duplicate_sets_count, all_duplicate_locations_count] + end + end + end + + filename = "location-duplicates-#{Time.zone.now}.csv" + storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["BULK_UPLOAD_BUCKET"]) + storage_service.write_file(filename, "#{duplicates_csv}") + + url = storage_service.get_presigned_url(filename, 72.hours.to_i) + Rails.logger.info("Download URL: #{url}") + end +end diff --git a/lib/tasks/performance_test.sh b/lib/tasks/performance_test.sh new file mode 100644 index 000000000..6fdc4248a --- /dev/null +++ b/lib/tasks/performance_test.sh @@ -0,0 +1,124 @@ +cd performance_test + +# Lettings logs page +echo "Get token" +TOKEN=$(curl -c token_cookies.txt -s https://staging.submit-social-housing-data.levellingup.gov.uk/account/sign-in | grep ' performance_lettings_test_results.txt +file="performance_lettings_test_results.txt" + +failed_requests=$(grep "Failed requests:" "$file" | awk '{print $3}') +non_2xx_responses=$(grep "Non-2xx responses:" "$file" | awk '{print $3}') +time_per_request_all=$(grep "Time per request:" "$file" | awk 'NR==2{print $4}') +requests_per_second=$(grep "Requests per second:" "$file" | awk '{print $4}') + + +if [ "$failed_requests" -gt 0 ]; then + echo "Lettings logs: Performance test failed - $failed_requests failed requests" + exit 1 +fi + +if [ "$non_2xx_responses" -ne 0 ] && [ -n "$non_2xx_responses" ]; then + echo "Lettings logs: Performance test failed: There were $non_2xx_responses non-2xx responses." + exit 1 +fi + +if (( $(echo "$time_per_request_all > 250" | bc -l) )); then + echo "Lettings logs: Performance test failed - Time per request across all concurrent requests is more than 250 ms: $time_per_request_all ms" + exit 1 +fi + +if (( $(echo "$requests_per_second < 5" | bc -l) )); then + echo "Lettings logs: Performance test failed - Requests per second is less than 5: $requests_per_second" + exit 1 +fi + +echo "Lettings logs page test passed: No failed requests and no non-2xx responses." + + +# Sales logs page +echo "Running sales logs page performance test..." +ab -n 50 -c 50 -l -C "$COOKIES" 'https://staging.submit-social-housing-data.levellingup.gov.uk/sales-logs?years[]=2024&status[]=completed' > performance_sales_test_results.txt +file="performance_sales_test_results.txt" + +failed_requests=$(grep "Failed requests:" "$file" | awk '{print $3}') +non_2xx_responses=$(grep "Non-2xx responses:" "$file" | awk '{print $3}') +time_per_request_all=$(grep "Time per request:" "$file" | awk 'NR==2{print $4}') +requests_per_second=$(grep "Requests per second:" "$file" | awk '{print $4}') + + +if [ "$failed_requests" -gt 0 ]; then + echo "Sales logs: Performance test failed - $failed_requests failed requests" + exit 1 +fi + +if [ "$non_2xx_responses" -ne 0 ] && [ -n "$non_2xx_responses" ]; then + echo "Sales logs: Performance test failed: There were $non_2xx_responses non-2xx responses." + exit 1 +fi + +if (( $(echo "$time_per_request_all > 250" | bc -l) )); then + echo "Sales logs: Performance test failed - Time per request across all concurrent requests is more than 250 ms: $time_per_request_all ms" + exit 1 +fi + +if (( $(echo "$requests_per_second < 5" | bc -l) )); then + echo "Sales logs: Performance test failed - Requests per second is less than 5: $requests_per_second" + exit 1 +fi + +echo "Sales logs page test passed: No failed requests and no non-2xx responses." + + +# Post data to a log test +page_content=$(curl -b login_cookies.txt -s 'https://staging.submit-social-housing-data.levellingup.gov.uk/lettings-logs?years[]=2024&status[]=completed') +completed_log_link=$(echo "$page_content" | sed -n 's/.* 500" | bc -l) )); then + echo "Update logs: Performance test failed - Time per request across all concurrent requests is more than 500 ms: $time_per_request_all ms" + exit 1 +fi + +if (( $(echo "$requests_per_second < 3" | bc -l) )); then + echo "Update logs: Performance test failed - Requests per second is less than 3: $requests_per_second" + exit 1 +fi + +echo "Update logs test passed: No failed requests and request times as expected." + +echo "All tests passed" +exit 0 diff --git a/spec/factories/scheme.rb b/spec/factories/scheme.rb index 7983bfa0a..5f4ad30bc 100644 --- a/spec/factories/scheme.rb +++ b/spec/factories/scheme.rb @@ -32,5 +32,14 @@ FactoryBot.define do confirmed { false } support_type { nil } end + trait :duplicate do + scheme_type { 4 } + registered_under_care_act { 1 } + primary_client_group { "O" } + secondary_client_group { "H" } + has_other_client_group { 1 } + support_type { 2 } + intended_stay { "M" } + end end end diff --git a/spec/lib/tasks/count_duplicates_spec.rb b/spec/lib/tasks/count_duplicates_spec.rb new file mode 100644 index 000000000..99da5b2fb --- /dev/null +++ b/spec/lib/tasks/count_duplicates_spec.rb @@ -0,0 +1,111 @@ +require "rails_helper" +require "rake" + +RSpec.describe "count_duplicates" do + before do + allow(Storage::S3Service).to receive(:new).and_return(storage_service) + allow(storage_service).to receive(:write_file) + allow(storage_service).to receive(:get_presigned_url).and_return(test_url) + end + + describe "count_duplicates:scheme_duplicates_per_org", type: :task do + subject(:task) { Rake::Task["count_duplicates:scheme_duplicates_per_org"] } + + let(:storage_service) { instance_double(Storage::S3Service) } + let(:test_url) { "test_url" } + + before do + Rake.application.rake_require("tasks/count_duplicates") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the rake task is run" do + context "and there are no duplicate schemes" do + before do + create(:organisation) + end + + it "creates a csv with headers only" do + expect(storage_service).to receive(:write_file).with(/scheme-duplicates-.*\.csv/, "\uFEFFOrganisation id,Number of duplicate sets,Total duplicate schemes\n") + expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}") + task.invoke + end + end + + context "and there are duplicate schemes" do + let(:organisation) { create(:organisation) } + let(:organisation2) { create(:organisation) } + + before do + create_list(:scheme, 2, :duplicate, owning_organisation: organisation) + create_list(:scheme, 3, :duplicate, primary_client_group: "I", owning_organisation: organisation) + create_list(:scheme, 5, :duplicate, owning_organisation: organisation2) + end + + it "creates a csv with correct duplicate numbers" do + expect(storage_service).to receive(:write_file).with(/scheme-duplicates-.*\.csv/, "\uFEFFOrganisation id,Number of duplicate sets,Total duplicate schemes\n#{organisation.id},2,5\n#{organisation2.id},1,5\n") + expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}") + task.invoke + end + end + end + end + + describe "count_duplicates:location_duplicates_per_org", type: :task do + subject(:task) { Rake::Task["count_duplicates:location_duplicates_per_org"] } + + let(:storage_service) { instance_double(Storage::S3Service) } + let(:test_url) { "test_url" } + + before do + Rake.application.rake_require("tasks/count_duplicates") + Rake::Task.define_task(:environment) + task.reenable + end + + context "when the rake task is run" do + context "and there are no duplicate locations" do + before do + create(:organisation) + end + + it "creates a csv with headers only" do + expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, "\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates\n") + expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}") + task.invoke + end + end + + context "and there are duplicate locations" do + let(:organisation) { create(:organisation) } + let(:scheme_a) { create(:scheme, :duplicate, owning_organisation: organisation) } + let(:scheme_b) { create(:scheme, :duplicate, owning_organisation: organisation) } + let(:scheme_c) { create(:scheme, owning_organisation: organisation) } + let(:organisation2) { create(:organisation) } + let(:scheme2) { create(:scheme, owning_organisation: organisation2) } + let(:scheme3) { create(:scheme, owning_organisation: organisation2) } + + before do + create_list(:location, 2, postcode: "A1 1AB", mobility_type: "M", scheme: scheme_a) # Location A + create_list(:location, 1, postcode: "A1 1AB", mobility_type: "A", scheme: scheme_a) # Location B + + create_list(:location, 1, postcode: "A1 1AB", mobility_type: "M", scheme: scheme_b) # Location A + create_list(:location, 1, postcode: "A1 1AB", mobility_type: "A", scheme: scheme_b) # Location B + create_list(:location, 2, postcode: "A1 1AB", mobility_type: "N", scheme: scheme_b) # Location C + + create_list(:location, 2, postcode: "A1 1AB", mobility_type: "A", scheme: scheme_c) # Location B + + create_list(:location, 5, postcode: "A1 1AB", mobility_type: "M", scheme: scheme2) + create_list(:location, 2, postcode: "A1 1AB", mobility_type: "M", scheme: scheme3) + end + + it "creates a csv with correct duplicate numbers" do + expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, "\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates\n#{organisation.id},3,6,4,9\n#{organisation2.id},2,7,2,7\n") + expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}") + task.invoke + end + end + end + end +end diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb index 4856c5662..79265d361 100644 --- a/spec/models/location_spec.rb +++ b/spec/models/location_spec.rb @@ -831,6 +831,76 @@ RSpec.describe Location, type: :model do expect(described_class.active.count).to eq(2) end end + + context "when getting list of duplicate locations" do + let!(:scheme) { create(:scheme) } + let!(:location) { create(:location, postcode: "AB1 2CD", mobility_type: "M", scheme:) } + let!(:duplicate_location) { create(:location, postcode: "AB1 2CD", mobility_type: "M", scheme:) } + let(:duplicate_sets) { described_class.duplicate_sets } + + it "returns a list of duplicates for the same scheme" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id) + end + + context "when there is a deleted duplicate location" do + before do + create(:location, postcode: "AB1 2CD", mobility_type: "M", discarded_at: Time.zone.now, scheme:) + end + + it "does not return the deleted location as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id) + end + end + + context "when there is a location with a different postcode" do + before do + create(:location, postcode: "A1 1AB", mobility_type: "M", scheme:) + end + + it "does not return a location with a different postcode as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id) + end + end + + context "when there is a location with a different mobility_type" do + before do + create(:location, postcode: "AB1 2CD", mobility_type: "A", scheme:) + end + + it "does not return a location with a different mobility_type as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id) + end + end + + context "when there is a location with a different scheme" do + before do + create(:location, postcode: "AB1 2CD", mobility_type: "M") + end + + it "does not return a location with a different scheme as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(location.id, duplicate_location.id) + end + end + + context "when there is a location with nil values for duplicate check fields" do + before do + [location, duplicate_location].each do |l| + l.postcode = nil + l.mobility_type = nil + l.save!(validate: false) + end + end + + it "does not return a location with nil values as a duplicate" do + expect(duplicate_sets).to be_empty + end + end + end end describe "status" do diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb index 9b3db15a4..5ca529d3e 100644 --- a/spec/models/scheme_spec.rb +++ b/spec/models/scheme_spec.rb @@ -208,6 +208,135 @@ RSpec.describe Scheme, type: :model do end end end + + context "when getting list of duplicate schemes" do + let(:organisation) { create(:organisation) } + let!(:scheme) { create(:scheme, :duplicate, owning_organisation: organisation) } + let!(:duplicate_scheme) { create(:scheme, :duplicate, owning_organisation: organisation) } + let(:duplicate_sets) { described_class.duplicate_sets } + + it "returns a list of duplicates in the same organisation" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + + context "when there is a deleted duplicate scheme" do + before do + create(:scheme, :duplicate, discarded_at: Time.zone.now) + end + + it "does not return the deleted scheme as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different scheme_type" do + before do + create(:scheme, :duplicate, scheme_type: 7) + end + + it "does not return a scheme with a different scheme_type as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different registered_under_care_act" do + before do + create(:scheme, :duplicate, registered_under_care_act: 2) + end + + it "does not return a scheme with a different registered_under_care_act as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different primary_client_group" do + before do + create(:scheme, :duplicate, primary_client_group: "H") + end + + it "does not return a scheme with a different primary_client_group as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different secondary_client_group" do + before do + create(:scheme, :duplicate, secondary_client_group: "O") + end + + it "does not return a scheme with a different secondary_client_group as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different has_other_client_group" do + before do + create(:scheme, :duplicate, has_other_client_group: 0) + end + + it "does not return a scheme with a different has_other_client_group as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different support_type" do + before do + create(:scheme, :duplicate, support_type: 4) + end + + it "does not return a scheme with a different support_type as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with a different intended_stay" do + before do + create(:scheme, :duplicate, intended_stay: "P") + end + + it "does not return a scheme with a different intended_stay as a duplicate" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + + context "when there is a scheme with nil values for duplicate check fields" do + before do + [scheme, duplicate_scheme].each do |s| + s.scheme_type = nil + s.registered_under_care_act = nil + s.primary_client_group = nil + s.secondary_client_group = nil + s.has_other_client_group = nil + s.support_type = nil + s.intended_stay = nil + s.save!(validate: false) + end + end + + it "does not return a scheme with nil values as a duplicate" do + expect(duplicate_sets).to be_empty + end + end + + context "when there are duplicate schemes without secondary client group" do + let!(:scheme) { create(:scheme, :duplicate, owning_organisation: organisation, secondary_client_group: nil, has_other_client_group: 0) } + let!(:duplicate_scheme) { create(:scheme, :duplicate, owning_organisation: organisation, secondary_client_group: nil, has_other_client_group: 0) } + + it "does not returns the duplicates" do + expect(duplicate_sets.count).to eq(1) + expect(duplicate_sets.first).to contain_exactly(scheme.id, duplicate_scheme.id) + end + end + end end end diff --git a/spec/services/postcode_service_spec.rb b/spec/services/postcode_service_spec.rb index abc61129f..4df809606 100644 --- a/spec/services/postcode_service_spec.rb +++ b/spec/services/postcode_service_spec.rb @@ -12,14 +12,10 @@ describe PostcodeService do end describe "lookup" do - before do - Excon.defaults[:mock] = true - Excon.defaults[:stubs] = :local - end - context "when the request returns a success response" do before do - Excon.stub({}, { body: '{"result": { "admin_district": "District", "codes": { "admin_district": "123" } } }', status: 200 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"postcode\":\"A00 0AA\",\"admin_district\":\"District\",\"codes\":{\"admin_district\":\"123\"}}}", headers: {}) end it "returns the admin district and admin district code" do @@ -31,7 +27,8 @@ describe PostcodeService do context "when the request returns a not found response" do before do - Excon.stub({}, { status: 404 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 404, body: "{\"status\":404,\"error\":\"Postcode not found\"}", headers: {}) end it "returns nil" do @@ -47,7 +44,8 @@ describe PostcodeService do context "when the request returns an error response" do before do - Excon.stub({}, { body: "This is an error message that is not valid json", status: 500 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 500, body: "This is an error message that is not valid json", headers: {}) end it "returns nil" do @@ -63,7 +61,8 @@ describe PostcodeService do context "when the request returns a success response that causes later errors" do before do - Excon.stub({}, { body: '{"result": { "admin_district": "District" } }', status: 200 }) + WebMock.stub_request(:get, "https://api.postcodes.io/postcodes/A000AA") + .to_return(status: 200, body: "{\"status\":200,\"result\":{\"admin_district\":\"Westminster\"", headers: {}) end it "returns nil" do diff --git a/yarn.lock b/yarn.lock index ea0395016..65de3d8d2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1792,10 +1792,10 @@ bl@^4.1.0: inherits "^2.0.4" readable-stream "^3.4.0" -body-parser@1.20.2, body-parser@^1.20.2: - version "1.20.2" - resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.2.tgz#6feb0e21c4724d06de7ff38da36dad4f57a747fd" - integrity sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA== +body-parser@1.20.3, body-parser@^1.20.2: + version "1.20.3" + resolved "https://registry.yarnpkg.com/body-parser/-/body-parser-1.20.3.tgz#1953431221c6fb5cd63c4b36d53fab0928e548c6" + integrity sha512-7rAxByjUMqQ3/bHJy7D6OGXvx/MMc4IqBn/X0fcM1QUcAItpZrBEYhWGem+tzXH90c+G01ypMcYJBO9Y30203g== dependencies: bytes "3.1.2" content-type "~1.0.5" @@ -1805,7 +1805,7 @@ body-parser@1.20.2, body-parser@^1.20.2: http-errors "2.0.0" iconv-lite "0.4.24" on-finished "2.4.1" - qs "6.11.0" + qs "6.13.0" raw-body "2.5.2" type-is "~1.6.18" unpipe "1.0.0" @@ -2488,6 +2488,11 @@ encodeurl@~1.0.1, encodeurl@~1.0.2: resolved "https://registry.yarnpkg.com/encodeurl/-/encodeurl-1.0.2.tgz#ad3ff4c86ec2d029322f5a02c3a9a606c95b3f59" integrity sha512-TPJXq8JqFaVYm2CWmPvnP2Iyo4ZSM7/QKcSmuMLDObfpH5fi7RUGmd/rTDf+rut/saiDiQEeVTNgAmJEdAOx0w== +encodeurl@~2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/encodeurl/-/encodeurl-2.0.0.tgz#7b8ea898077d7e409d3ac45474ea38eaf0857a58" + integrity sha512-Q0n9HRi4m6JuGIV1eFlmvJB7ZEVxu93IrMyiMsGC0lrMJMWzRgx6WGquyfQgZVb31vhGgXnfmPNNXmxnOkRBrg== + engine.io-client@~6.5.2: version "6.5.4" resolved "https://registry.yarnpkg.com/engine.io-client/-/engine.io-client-6.5.4.tgz#b8bc71ed3f25d0d51d587729262486b4b33bd0d0" @@ -2953,36 +2958,36 @@ express-session@^1.18.0: uid-safe "~2.1.5" express@^4.18.2: - version "4.19.2" - resolved "https://registry.yarnpkg.com/express/-/express-4.19.2.tgz#e25437827a3aa7f2a827bc8171bbbb664a356465" - integrity sha512-5T6nhjsT+EOMzuck8JjBHARTHfMht0POzlA60WV2pMD3gyXw2LZnZ+ueGdNxG+0calOJcWKbpFcuzLZ91YWq9Q== + version "4.21.0" + resolved "https://registry.yarnpkg.com/express/-/express-4.21.0.tgz#d57cb706d49623d4ac27833f1cbc466b668eb915" + integrity sha512-VqcNGcj/Id5ZT1LZ/cfihi3ttTn+NJmkli2eZADigjq29qTlWi/hAQ43t/VLPq8+UX06FCEx3ByOYet6ZFblng== dependencies: accepts "~1.3.8" array-flatten "1.1.1" - body-parser "1.20.2" + body-parser "1.20.3" content-disposition "0.5.4" content-type "~1.0.4" cookie "0.6.0" cookie-signature "1.0.6" debug "2.6.9" depd "2.0.0" - encodeurl "~1.0.2" + encodeurl "~2.0.0" escape-html "~1.0.3" etag "~1.8.1" - finalhandler "1.2.0" + finalhandler "1.3.1" fresh "0.5.2" http-errors "2.0.0" - merge-descriptors "1.0.1" + merge-descriptors "1.0.3" methods "~1.1.2" on-finished "2.4.1" parseurl "~1.3.3" - path-to-regexp "0.1.7" + path-to-regexp "0.1.10" proxy-addr "~2.0.7" - qs "6.11.0" + qs "6.13.0" range-parser "~1.2.1" safe-buffer "5.2.1" - send "0.18.0" - serve-static "1.15.0" + send "0.19.0" + serve-static "1.16.2" setprototypeof "1.2.0" statuses "2.0.1" type-is "~1.6.18" @@ -3106,13 +3111,13 @@ finalhandler@1.1.0: statuses "~1.3.1" unpipe "~1.0.0" -finalhandler@1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.2.0.tgz#7d23fe5731b207b4640e4fcd00aec1f9207a7b32" - integrity sha512-5uXcUVftlQMFnWC9qu/svkWv3GTd2PfUhK/3PLkYNAe7FbqJMt3515HaxE6eRL74GdsriiwujiawdaB1BpEISg== +finalhandler@1.3.1: + version "1.3.1" + resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.3.1.tgz#0c575f1d1d324ddd1da35ad7ece3df7d19088019" + integrity sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ== dependencies: debug "2.6.9" - encodeurl "~1.0.2" + encodeurl "~2.0.0" escape-html "~1.0.3" on-finished "2.4.1" parseurl "~1.3.3" @@ -4224,10 +4229,10 @@ meow@^13.2.0: resolved "https://registry.yarnpkg.com/meow/-/meow-13.2.0.tgz#6b7d63f913f984063b3cc261b6e8800c4cd3474f" integrity sha512-pxQJQzB6djGPXh08dacEloMFopsOqGVRKFPYvPOt9XDZ1HasbgDZA74CJGreSU4G3Ak7EFJGoiH2auq+yXISgA== -merge-descriptors@1.0.1: - version "1.0.1" - resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.1.tgz#b00aaa556dd8b44568150ec9d1b953f3f90cbb61" - integrity sha512-cCi6g3/Zr1iqQi6ySbseM1Xvooa98N0w31jzUYrXPX2xqObmFGHJ0tQ5u74H3mVh7wLouTseZyYIq39g8cNp1w== +merge-descriptors@1.0.3: + version "1.0.3" + resolved "https://registry.yarnpkg.com/merge-descriptors/-/merge-descriptors-1.0.3.tgz#d80319a65f3c7935351e5cfdac8f9318504dbed5" + integrity sha512-gaNvAS7TZ897/rVaZ0nMtAyxNyi/pdbjbAwUpFQpN70GqnVfOiXpeUUMKRBmzXaSQ8DdTX4/0ms62r2K+hE6mQ== merge-stream@^2.0.0: version "2.0.0" @@ -4625,10 +4630,10 @@ path-parse@^1.0.7: resolved "https://registry.yarnpkg.com/path-parse/-/path-parse-1.0.7.tgz#fbc114b60ca42b30d9daf5858e4bd68bbedb6735" integrity sha512-LDJzPVEEEPR+y48z93A0Ed0yXb8pAByGWo/k5YYdYgpY2/2EsOsksJrq7lOHxryrVOn1ejG6oAp8ahvOIQD8sw== -path-to-regexp@0.1.7: - version "0.1.7" - resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.7.tgz#df604178005f522f15eb4490e7247a1bfaa67f8c" - integrity sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ== +path-to-regexp@0.1.10: + version "0.1.10" + resolved "https://registry.yarnpkg.com/path-to-regexp/-/path-to-regexp-0.1.10.tgz#67e9108c5c0551b9e5326064387de4763c4d5f8b" + integrity sha512-7lf7qcQidTku0Gu3YDPc8DJ1q7OOucfa/BSsIwjuh56VU7katFvuM8hULfkwB3Fns/rsVF7PwPKVw1sl5KQS9w== path-type@^4.0.0: version "4.0.0" @@ -4802,14 +4807,7 @@ punycode@^2.1.0: resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.3.1.tgz#027422e2faec0b25e1549c3e1bd8309b9133b6e5" integrity sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg== -qs@6.11.0: - version "6.11.0" - resolved "https://registry.yarnpkg.com/qs/-/qs-6.11.0.tgz#fd0d963446f7a65e1367e01abd85429453f0c37a" - integrity sha512-MvjoMCJwEarSbUYk5O+nmoSzSutSsTwF85zcHPQ9OrlFoZOYIjaqBAJIqIXjptyD5vThxGq52Xu/MaJzRkIk4Q== - dependencies: - side-channel "^1.0.4" - -qs@^6.4.0: +qs@6.13.0, qs@^6.4.0: version "6.13.0" resolved "https://registry.yarnpkg.com/qs/-/qs-6.13.0.tgz#6ca3bd58439f7e245655798997787b0d88a51906" integrity sha512-+38qI9SOr8tfZ4QmJNplMUxqjbe7LKvvZgWdExBOmd+egZTtjLB67Gu0HRX3u/XOq7UU2Nx6nsjvS16Z9uwfpg== @@ -5209,10 +5207,10 @@ send@0.16.2: range-parser "~1.2.0" statuses "~1.4.0" -send@0.18.0: - version "0.18.0" - resolved "https://registry.yarnpkg.com/send/-/send-0.18.0.tgz#670167cc654b05f5aa4a767f9113bb371bc706be" - integrity sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg== +send@0.19.0: + version "0.19.0" + resolved "https://registry.yarnpkg.com/send/-/send-0.19.0.tgz#bbc5a388c8ea6c048967049dbeac0e4a3f09d7f8" + integrity sha512-dW41u5VfLXu8SJh5bwRmyYUbAoSB3c9uQh6L8h/KtsFREPWpbX1lrljJo186Jc4nmci/sGUZ9a0a0J2zgfq2hw== dependencies: debug "2.6.9" depd "2.0.0" @@ -5258,15 +5256,15 @@ serve-static@1.13.2: parseurl "~1.3.2" send "0.16.2" -serve-static@1.15.0: - version "1.15.0" - resolved "https://registry.yarnpkg.com/serve-static/-/serve-static-1.15.0.tgz#faaef08cffe0a1a62f60cad0c4e513cff0ac9540" - integrity sha512-XGuRDNjXUijsUL0vl6nSD7cwURuzEgglbOaFuZM9g3kwDXOWVTck0jLzjPzGD+TazWbboZYu52/9/XPdUgne9g== +serve-static@1.16.2: + version "1.16.2" + resolved "https://registry.yarnpkg.com/serve-static/-/serve-static-1.16.2.tgz#b6a5343da47f6bdd2673848bf45754941e803296" + integrity sha512-VqpjJZKadQB/PEbEwvFdO43Ax5dFBZ2UECszz8bQ7pi7wt//PWe1P6MN7eCnjsatYtBT6EuiClbjSWP2WrIoTw== dependencies: - encodeurl "~1.0.2" + encodeurl "~2.0.0" escape-html "~1.0.3" parseurl "~1.3.3" - send "0.18.0" + send "0.19.0" server-destroy@1.0.1: version "1.0.1"