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 6eeee6511..46bd31b5f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -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/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/controllers/bulk_upload_lettings_logs_controller.rb b/app/controllers/bulk_upload_lettings_logs_controller.rb index 465bbc5f6..a8ad14f9e 100644 --- a/app/controllers/bulk_upload_lettings_logs_controller.rb +++ b/app/controllers/bulk_upload_lettings_logs_controller.rb @@ -4,9 +4,9 @@ class BulkUploadLettingsLogsController < ApplicationController def start if have_choice_of_year? - redirect_to bulk_upload_lettings_log_path(id: "year") + redirect_to bulk_upload_lettings_log_path(id: "year", form: { organisation_id: params[:organisation_id] }.compact) else - redirect_to bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: current_year }) + redirect_to bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: current_year, organisation_id: params[:organisation_id] }.compact) end end @@ -60,6 +60,6 @@ private end def form_params - params.fetch(:form, {}).permit(:year, :needstype, :file) + params.fetch(:form, {}).permit(:year, :needstype, :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 56bd1d4de..2fd312d10 100644 --- a/app/controllers/bulk_upload_sales_logs_controller.rb +++ b/app/controllers/bulk_upload_sales_logs_controller.rb @@ -4,9 +4,9 @@ class BulkUploadSalesLogsController < ApplicationController def start if have_choice_of_year? - redirect_to bulk_upload_sales_log_path(id: "year") + redirect_to bulk_upload_sales_log_path(id: "year", form: { organisation_id: params[:organisation_id] }.compact) else - redirect_to bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: current_year }) + redirect_to bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: current_year, organisation_id: params[:organisation_id] }.compact) end end @@ -58,6 +58,6 @@ private end def form_params - params.fetch(:form, {}).permit(:year, :file) + params.fetch(:form, {}).permit(:year, :file, :organisation_id) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index bb119b29e..5a9203da8 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -2,6 +2,7 @@ module ApplicationHelper include Pagy::Frontend def browser_title(title, pagy, *resources) + title = sanitize(title)&.gsub("&", "&") if resources.any? { |r| r.present? && r.errors.present? } "Error: #{[title, t('service_name'), 'GOV.UK'].select(&:present?).join(' - ')}" else diff --git a/app/models/forms/bulk_upload_lettings/checking_file.rb b/app/models/forms/bulk_upload_lettings/checking_file.rb index 8cd9ee696..3b43cb269 100644 --- a/app/models/forms/bulk_upload_lettings/checking_file.rb +++ b/app/models/forms/bulk_upload_lettings/checking_file.rb @@ -6,13 +6,18 @@ module Forms include Rails.application.routes.url_helpers attribute :year, :integer + attribute :organisation_id, :integer def view_path "bulk_upload_lettings_logs/forms/checking_file" end def back_path - bulk_upload_lettings_log_path(id: "start") + if organisation_id.present? + lettings_logs_organisation_path(organisation_id) + else + bulk_upload_lettings_log_path(id: "start") + end end def year_combo diff --git a/app/models/forms/bulk_upload_lettings/guidance.rb b/app/models/forms/bulk_upload_lettings/guidance.rb index b6cf5bf74..24bc531f2 100644 --- a/app/models/forms/bulk_upload_lettings/guidance.rb +++ b/app/models/forms/bulk_upload_lettings/guidance.rb @@ -7,6 +7,7 @@ module Forms attribute :year, :integer attribute :referrer + attribute :organisation_id, :integer def view_path "bulk_upload_shared/guidance" @@ -15,7 +16,7 @@ module Forms def back_path case referrer when "prepare-your-file" - bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: }) + bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact) when "home" root_path else diff --git a/app/models/forms/bulk_upload_lettings/needstype.rb b/app/models/forms/bulk_upload_lettings/needstype.rb index 0cadd647c..32ed0acf3 100644 --- a/app/models/forms/bulk_upload_lettings/needstype.rb +++ b/app/models/forms/bulk_upload_lettings/needstype.rb @@ -7,6 +7,7 @@ module Forms attribute :needstype, :integer attribute :year, :integer + attribute :organisation_id, :integer validates :needstype, presence: true @@ -19,11 +20,11 @@ module Forms end def back_path - bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, needstype: }) + 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: }) + bulk_upload_lettings_log_path(id: "upload-your-file", form: { year:, needstype:, organisation_id: }.compact) end def year_combo diff --git a/app/models/forms/bulk_upload_lettings/prepare_your_file.rb b/app/models/forms/bulk_upload_lettings/prepare_your_file.rb index b5069d908..159436ce1 100644 --- a/app/models/forms/bulk_upload_lettings/prepare_your_file.rb +++ b/app/models/forms/bulk_upload_lettings/prepare_your_file.rb @@ -7,6 +7,7 @@ module Forms attribute :year, :integer attribute :needstype, :integer + attribute :organisation_id, :integer def view_path case year @@ -19,15 +20,16 @@ module Forms def back_path if have_choice_of_year? - Rails.application.routes.url_helpers.bulk_upload_lettings_log_path(id: "year", form: { year: }) + Rails.application.routes.url_helpers.bulk_upload_lettings_log_path(id: "year", form: { year:, organisation_id: }.compact) + elsif organisation_id.present? + lettings_logs_organisation_path(organisation_id) else Rails.application.routes.url_helpers.lettings_logs_path end end def next_path - page_id = year == 2022 ? "needstype" : "upload-your-file" - bulk_upload_lettings_log_path(id: page_id, form: { year:, needstype: }) + bulk_upload_lettings_log_path(id: "upload-your-file", form: { year:, needstype:, organisation_id: }.compact) end def legacy_template_path diff --git a/app/models/forms/bulk_upload_lettings/upload_your_file.rb b/app/models/forms/bulk_upload_lettings/upload_your_file.rb index 9ccec7622..954c6e6e6 100644 --- a/app/models/forms/bulk_upload_lettings/upload_your_file.rb +++ b/app/models/forms/bulk_upload_lettings/upload_your_file.rb @@ -11,6 +11,7 @@ module Forms attribute :needstype, :integer attribute :file attribute :current_user + attribute :organisation_id, :integer validates :file, presence: true validate :validate_file_is_csv @@ -20,8 +21,7 @@ module Forms end def back_path - page_id = year == 2022 ? "needstype" : "prepare-your-file" - bulk_upload_lettings_log_path(id: page_id, form: { year:, needstype: }) + bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, needstype:, organisation_id: }.compact) end def year_combo @@ -29,7 +29,7 @@ module Forms end def next_path - bulk_upload_lettings_log_path(id: "checking-file", form: { year: }) + bulk_upload_lettings_log_path(id: "checking-file", form: { year:, organisation_id: }.compact) end def save! @@ -39,6 +39,7 @@ module Forms year:, needstype:, filename: file.original_filename, + organisation_id: (organisation_id if current_user.support?) || current_user.organisation_id, ) storage_service.write_file(bulk_upload.identifier, File.read(file.path)) diff --git a/app/models/forms/bulk_upload_lettings/year.rb b/app/models/forms/bulk_upload_lettings/year.rb index e199559fa..396b91a79 100644 --- a/app/models/forms/bulk_upload_lettings/year.rb +++ b/app/models/forms/bulk_upload_lettings/year.rb @@ -6,6 +6,7 @@ module Forms include Rails.application.routes.url_helpers attribute :year, :integer + attribute :organisation_id, :integer validates :year, presence: true @@ -20,11 +21,15 @@ module Forms end def back_path - lettings_logs_path + if organisation_id.present? + lettings_logs_organisation_path(organisation_id) + else + lettings_logs_path + end end def next_path - bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year: }) + bulk_upload_lettings_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact) end def save! diff --git a/app/models/forms/bulk_upload_sales/checking_file.rb b/app/models/forms/bulk_upload_sales/checking_file.rb index a37be3ccb..243d64070 100644 --- a/app/models/forms/bulk_upload_sales/checking_file.rb +++ b/app/models/forms/bulk_upload_sales/checking_file.rb @@ -6,13 +6,18 @@ module Forms include Rails.application.routes.url_helpers attribute :year, :integer + attribute :organisation_id, :integer def view_path "bulk_upload_sales_logs/forms/checking_file" end def back_path - bulk_upload_sales_log_path(id: "start") + if organisation_id.present? + sales_logs_organisation_path(organisation_id) + else + bulk_upload_sales_log_path(id: "start") + end end def year_combo diff --git a/app/models/forms/bulk_upload_sales/guidance.rb b/app/models/forms/bulk_upload_sales/guidance.rb index 28ca6c3b5..ef792a3e4 100644 --- a/app/models/forms/bulk_upload_sales/guidance.rb +++ b/app/models/forms/bulk_upload_sales/guidance.rb @@ -7,6 +7,7 @@ module Forms attribute :year, :integer attribute :referrer + attribute :organisation_id, :integer def view_path "bulk_upload_shared/guidance" @@ -15,7 +16,7 @@ module Forms def back_path case referrer when "prepare-your-file" - bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: }) + bulk_upload_sales_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact) when "home" root_path else diff --git a/app/models/forms/bulk_upload_sales/prepare_your_file.rb b/app/models/forms/bulk_upload_sales/prepare_your_file.rb index 387f6239f..4bf0797a8 100644 --- a/app/models/forms/bulk_upload_sales/prepare_your_file.rb +++ b/app/models/forms/bulk_upload_sales/prepare_your_file.rb @@ -6,6 +6,7 @@ module Forms include Rails.application.routes.url_helpers attribute :year, :integer + attribute :organisation_id, :integer def view_path case year @@ -18,14 +19,16 @@ module Forms def back_path if have_choice_of_year? - Rails.application.routes.url_helpers.bulk_upload_sales_log_path(id: "year", form: { year: }) + Rails.application.routes.url_helpers.bulk_upload_sales_log_path(id: "year", form: { year: }.compact) + elsif organisation_id.present? + sales_logs_organisation_path(organisation_id) else Rails.application.routes.url_helpers.sales_logs_path end end def next_path - bulk_upload_sales_log_path(id: "upload-your-file", form: { year: }) + bulk_upload_sales_log_path(id: "upload-your-file", form: { year:, organisation_id: }.compact) end def legacy_template_path diff --git a/app/models/forms/bulk_upload_sales/upload_your_file.rb b/app/models/forms/bulk_upload_sales/upload_your_file.rb index de650c831..5a0114caf 100644 --- a/app/models/forms/bulk_upload_sales/upload_your_file.rb +++ b/app/models/forms/bulk_upload_sales/upload_your_file.rb @@ -10,6 +10,7 @@ module Forms attribute :year, :integer attribute :file attribute :current_user + attribute :organisation_id, :integer validates :file, presence: true validate :validate_file_is_csv @@ -19,7 +20,7 @@ module Forms end def back_path - bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: }) + bulk_upload_sales_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact) end def year_combo @@ -27,7 +28,7 @@ module Forms end def next_path - bulk_upload_sales_log_path(id: "checking-file", form: { year: }) + bulk_upload_sales_log_path(id: "checking-file", form: { year:, organisation_id: }.compact) end def save! @@ -36,6 +37,7 @@ module Forms log_type: BulkUpload.log_types[:sales], year:, filename: file.original_filename, + organisation_id: (organisation_id if current_user.support?) || current_user.organisation_id, ) storage_service.write_file(bulk_upload.identifier, File.read(file.path)) diff --git a/app/models/forms/bulk_upload_sales/year.rb b/app/models/forms/bulk_upload_sales/year.rb index 85959c4e5..1de8dec04 100644 --- a/app/models/forms/bulk_upload_sales/year.rb +++ b/app/models/forms/bulk_upload_sales/year.rb @@ -6,6 +6,7 @@ module Forms include Rails.application.routes.url_helpers attribute :year, :integer + attribute :organisation_id, :integer validates :year, presence: true @@ -20,11 +21,15 @@ module Forms end def back_path - sales_logs_path + if organisation_id.present? + sales_logs_organisation_path(organisation_id) + else + sales_logs_path + end end def next_path - bulk_upload_sales_log_path(id: "prepare-your-file", form: { year: }) + bulk_upload_sales_log_path(id: "prepare-your-file", form: { year:, organisation_id: }.compact) end def save! 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/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 8fc913055..dc0b2dca3 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -432,6 +432,7 @@ class BulkUpload::Lettings::Year2024::RowParser validate :validate_assigned_to_exists, on: :after_log validate :validate_assigned_to_related, on: :after_log + validate :validate_assigned_to_when_support, on: :after_log validate :validate_all_charges_given, on: :after_log, if: proc { is_carehome.zero? } validate :validate_address_option_found, on: :after_log, unless: -> { supported_housing? } @@ -582,6 +583,12 @@ private end end + def validate_assigned_to_when_support + if field_3.blank? && bulk_upload.user.support? + errors.add(:field_3, category: :setup, message: I18n.t("validations.not_answered", question: "what is the CORE username of the account this letting log should be assigned to?")) + end + end + def validate_assigned_to_related return unless assigned_to return if assigned_to.organisation == owning_organisation || assigned_to.organisation == managing_organisation @@ -891,12 +898,17 @@ private end def validate_owning_org_permitted - if owning_organisation && !bulk_upload.user.organisation.affiliated_stock_owners.include?(owning_organisation) - block_log_creation! + return unless owning_organisation + return if bulk_upload_organisation.affiliated_stock_owners.include?(owning_organisation) - if errors[:field_1].blank? - errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup) - end + block_log_creation! + + return if errors[:field_1].present? + + if bulk_upload.user.support? + errors.add(:field_1, "This owning organisation is not affiliated with #{bulk_upload_organisation.name}", category: :setup) + else + errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup) end end @@ -1137,7 +1149,7 @@ private attributes["renewal"] = renewal attributes["scheme"] = scheme attributes["location"] = location - attributes["assigned_to"] = assigned_to || bulk_upload.user + attributes["assigned_to"] = assigned_to || (bulk_upload.user.support? ? nil : bulk_upload.user) attributes["created_by"] = bulk_upload.user attributes["needstype"] = field_4 attributes["rent_type"] = RENT_TYPE_BU_MAPPING[field_11] @@ -1625,4 +1637,8 @@ private def reason_is_other? field_98 == 20 end + + def bulk_upload_organisation + Organisation.find(bulk_upload.organisation_id) + 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 8be08d62f..0b7a70c27 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -462,6 +462,7 @@ class BulkUpload::Sales::Year2024::RowParser validate :validate_assigned_to_exists, on: :after_log validate :validate_assigned_to_related, on: :after_log + validate :validate_assigned_to_when_support, on: :after_log validate :validate_managing_org_related, on: :after_log validate :validate_relevant_collection_window, on: :after_log validate :validate_incomplete_soft_validations, on: :after_log @@ -925,7 +926,7 @@ private attributes["owning_organisation"] = owning_organisation attributes["managing_organisation"] = managing_organisation - attributes["assigned_to"] = assigned_to || bulk_upload.user + attributes["assigned_to"] = assigned_to || (bulk_upload.user.support? ? nil : bulk_upload.user) attributes["created_by"] = bulk_upload.user attributes["hhregres"] = field_72 attributes["hhregresstill"] = field_73 @@ -1302,12 +1303,17 @@ private end def validate_owning_org_permitted - if owning_organisation && !bulk_upload.user.organisation.affiliated_stock_owners.include?(owning_organisation) - block_log_creation! + return unless owning_organisation + return if bulk_upload_organisation.affiliated_stock_owners.include?(owning_organisation) - if errors[:field_1].blank? - errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup) - end + block_log_creation! + + return if errors[:field_1].present? + + if bulk_upload.user.support? + errors.add(:field_1, "This owning organisation is not affiliated with #{bulk_upload_organisation.name}", category: :setup) + else + errors.add(:field_1, "You do not have permission to add logs for this owning organisation", category: :setup) end end @@ -1319,6 +1325,12 @@ private end end + def validate_assigned_to_when_support + if field_3.blank? && bulk_upload.user.support? + errors.add(:field_3, category: :setup, message: I18n.t("validations.not_answered", question: "what is the CORE username of the account this sales log should be assigned to?")) + end + end + def validate_assigned_to_related return unless assigned_to return if assigned_to.organisation == owning_organisation || assigned_to.organisation == managing_organisation @@ -1492,4 +1504,8 @@ private def valid_nationality_options %w[0] + GlobalConstants::COUNTRIES_ANSWER_OPTIONS.keys # 0 is "Prefers not to say" end + + def bulk_upload_organisation + Organisation.find(bulk_upload.organisation_id) + end end diff --git a/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb b/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb index 7524072a5..2e6d4cd10 100644 --- a/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb +++ b/app/views/bulk_upload_lettings_logs/forms/checking_file.html.erb @@ -6,6 +6,7 @@
<%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "prepare-your-file"), method: :patch do |f| %> <%= f.hidden_field :year %> + <%= f.hidden_field :organisation_id %> Upload lettings logs in bulk (<%= @form.year_combo %>)

We’re checking the file

diff --git a/app/views/bulk_upload_lettings_logs/forms/needstype.erb b/app/views/bulk_upload_lettings_logs/forms/needstype.erb index 6deec7e1d..644dd9f5f 100644 --- a/app/views/bulk_upload_lettings_logs/forms/needstype.erb +++ b/app/views/bulk_upload_lettings_logs/forms/needstype.erb @@ -7,6 +7,7 @@ <%= 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, diff --git a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb index 45ce4b385..7657396f1 100644 --- a/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb +++ b/app/views/bulk_upload_lettings_logs/forms/prepare_your_file_2024.html.erb @@ -6,6 +6,7 @@
<%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "prepare-your-file"), method: :patch do |f| %> <%= f.hidden_field :year %> + <%= f.hidden_field :organisation_id %> Upload lettings logs in bulk (<%= @form.year_combo %>)

Prepare your file

diff --git a/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb b/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb index bbafc28b3..2814eb1ea 100644 --- a/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb +++ b/app/views/bulk_upload_lettings_logs/forms/upload_your_file.html.erb @@ -7,6 +7,7 @@ <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "upload-your-file"), method: :patch do |f| %> <%= f.hidden_field :year %> <%= f.hidden_field :needstype %> + <%= f.hidden_field :organisation_id %> <%= f.govuk_error_summary %> diff --git a/app/views/bulk_upload_lettings_logs/forms/year.html.erb b/app/views/bulk_upload_lettings_logs/forms/year.html.erb index 8ba1c280f..4fef16dd7 100644 --- a/app/views/bulk_upload_lettings_logs/forms/year.html.erb +++ b/app/views/bulk_upload_lettings_logs/forms/year.html.erb @@ -4,6 +4,7 @@ <%= form_with model: @form, scope: :form, url: bulk_upload_lettings_log_path(id: "year"), method: :patch do |f| %> <%= f.govuk_error_summary %> + <%= f.hidden_field :organisation_id %> <%= f.govuk_collection_radio_buttons :year, @form.options, diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb index 30a6fd585..15c486b91 100644 --- a/app/views/bulk_upload_lettings_results/show.html.erb +++ b/app/views/bulk_upload_lettings_results/show.html.erb @@ -25,4 +25,4 @@
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %> +<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %> diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb index b144793bf..8a59e8999 100644 --- a/app/views/bulk_upload_lettings_results/summary.html.erb +++ b/app/views/bulk_upload_lettings_results/summary.html.erb @@ -29,4 +29,4 @@ <% end %>
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %> +<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %> diff --git a/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb b/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb index c47a97d5e..8bcef5cad 100644 --- a/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb +++ b/app/views/bulk_upload_sales_logs/forms/checking_file.html.erb @@ -6,6 +6,7 @@
<%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "prepare-your-file"), method: :patch do |f| %> <%= f.hidden_field :year %> + <%= f.hidden_field :organisation_id %><%= f.hidden_field :organisation_id %> Upload sales logs in bulk (<%= @form.year_combo %>)

We’re checking the file

diff --git a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb index 81c947e15..2a64de689 100644 --- a/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb +++ b/app/views/bulk_upload_sales_logs/forms/prepare_your_file_2024.html.erb @@ -6,6 +6,7 @@
<%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "prepare-your-file"), method: :patch do |f| %> <%= f.hidden_field :year %> + <%= f.hidden_field :organisation_id %> Upload sales logs in bulk (<%= @form.year_combo %>)

Prepare your file

diff --git a/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb b/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb index 36e7240dc..75350ae9e 100644 --- a/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb +++ b/app/views/bulk_upload_sales_logs/forms/upload_your_file.html.erb @@ -6,6 +6,7 @@
<%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "upload-your-file"), method: :patch do |f| %> <%= f.hidden_field :year %> + <%= f.hidden_field :organisation_id %> <%= f.govuk_error_summary %> diff --git a/app/views/bulk_upload_sales_logs/forms/year.html.erb b/app/views/bulk_upload_sales_logs/forms/year.html.erb index d8aa09172..2aaa7c0f8 100644 --- a/app/views/bulk_upload_sales_logs/forms/year.html.erb +++ b/app/views/bulk_upload_sales_logs/forms/year.html.erb @@ -4,6 +4,7 @@ <%= form_with model: @form, scope: :form, url: bulk_upload_sales_log_path(id: "year"), method: :patch do |f| %> <%= f.govuk_error_summary %> + <%= f.hidden_field :organisation_id %> <%= f.govuk_collection_radio_buttons :year, @form.options, diff --git a/app/views/bulk_upload_sales_results/show.html.erb b/app/views/bulk_upload_sales_results/show.html.erb index 0d645db33..776fdfa2f 100644 --- a/app/views/bulk_upload_sales_results/show.html.erb +++ b/app/views/bulk_upload_sales_results/show.html.erb @@ -25,4 +25,4 @@
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path %> +<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %> diff --git a/app/views/bulk_upload_sales_results/summary.html.erb b/app/views/bulk_upload_sales_results/summary.html.erb index 0fa51b9dc..0e423621d 100644 --- a/app/views/bulk_upload_sales_results/summary.html.erb +++ b/app/views/bulk_upload_sales_results/summary.html.erb @@ -29,4 +29,4 @@ <% end %>
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path %> +<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %> diff --git a/app/views/logs/_create_for_org_actions.html.erb b/app/views/logs/_create_for_org_actions.html.erb index f6ed9ad82..0564e678a 100644 --- a/app/views/logs/_create_for_org_actions.html.erb +++ b/app/views/logs/_create_for_org_actions.html.erb @@ -1,10 +1,12 @@
<% if @organisation.data_protection_confirmed? %> <% if current_page?(controller: 'organisations', action: 'lettings_logs') %> - <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post) %> - <% end %> + <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %> + <%= govuk_button_link_to "Upload lettings logs in bulk", bulk_upload_lettings_log_path(id: "start", organisation_id: @organisation.id), secondary: true %> +<% end %> <% if current_page?(controller: 'organisations', action: 'sales_logs') %> - <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post) %> - <% end %> + <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %> + <%= govuk_button_link_to "Upload sales logs in bulk", bulk_upload_sales_log_path(id: "start", organisation_id: @organisation.id), secondary: true %> +<% end %> <% end %>
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/db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb b/db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb new file mode 100644 index 000000000..d9f4f16da --- /dev/null +++ b/db/migrate/20240905092332_add_organisation_id_to_bulk_uploads.rb @@ -0,0 +1,5 @@ +class AddOrganisationIdToBulkUploads < ActiveRecord::Migration[7.0] + def change + add_column :bulk_uploads, :organisation_id, :integer + end +end diff --git a/db/migrate/20240923145326_add_validation_checked_field.rb b/db/migrate/20240923145326_add_validation_checked_field.rb new file mode 100644 index 000000000..899992974 --- /dev/null +++ b/db/migrate/20240923145326_add_validation_checked_field.rb @@ -0,0 +1,5 @@ +class AddValidationCheckedField < ActiveRecord::Migration[7.0] + def change + add_column :log_validations, :checked, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 80463eaad..4d6f18b84 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_09_18_112702) do +ActiveRecord::Schema[7.0].define(version: 2024_09_23_145326) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -42,6 +42,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_18_112702) do t.text "choice" t.integer "total_logs_count" t.string "rent_type_fix_status", default: "not_applied" + t.integer "organisation_id" t.integer "moved_user_id" t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true t.index ["user_id"], name: "index_bulk_uploads_on_user_id" @@ -410,6 +411,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_18_112702) do t.string "other_validated_models" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "checked" end create_table "logs_exports", force: :cascade do |t| 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/bulk_upload.rb b/spec/factories/bulk_upload.rb index c848fb071..cefe95c2b 100644 --- a/spec/factories/bulk_upload.rb +++ b/spec/factories/bulk_upload.rb @@ -9,6 +9,7 @@ FactoryBot.define do sequence(:filename) { |n| "bulk-upload-#{n}.csv" } needstype { 1 } rent_type_fix_status { BulkUpload.rent_type_fix_statuses.values.sample } + organisation_id { user.organisation_id } trait(:sales) do log_type { BulkUpload.log_types[:sales] } 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/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 1867285eb..3d65cda87 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -139,6 +139,10 @@ RSpec.describe "User Features" do expect(page).to have_button("Create a new lettings log for this organisation") end + it "shows a upload lettings logs in bulk link" do + expect(page).to have_link("Upload lettings logs in bulk") + end + context "when creating a log for that organisation" do it "pre-fills the value for owning organisation for that log" do click_button("Create a new lettings log for this organisation") @@ -230,6 +234,10 @@ RSpec.describe "User Features" do expect(page).to have_button("Create a new sales log for this organisation") end + it "shows a upload sales logs in bulk link" do + expect(page).to have_link("Upload sales logs in bulk") + end + context "when creating a log for that organisation" do it "pre-fills the value for owning organisation for that log" do click_button("Create a new sales log for this organisation") 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/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb index 3faa0a699..68de5d22b 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -815,6 +815,23 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do end end + context "when blank and bulk upload user is support" do + let(:bulk_upload) { create(:bulk_upload, :sales, user: create(:user, :support), year: 2024) } + + let(:attributes) { setup_section_params.merge(bulk_upload:, field_3: nil) } + + it "is not permitted" do + parser.valid? + expect(parser.errors[:field_3]).to be_present + expect(parser.errors[:field_3]).to include("You must answer what is the CORE username of the account this letting log should be assigned to?") + end + + it "blocks log creation" do + parser.valid? + expect(parser).to be_block_log_creation + end + end + context "when user could not be found" do let(:attributes) { { bulk_upload:, field_3: "idonotexist@example.com" } } @@ -1439,6 +1456,43 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do expect(parser.errors[:field_10]).to include(/Enter a date when the owning and managing organisation was active/) end end + + context "when user is an unaffiliated non-support user and bulk upload organisation is affiliated with the owning organisation" do + let(:affiliated_org) { create(:organisation, :with_old_visible_id) } + let(:unaffiliated_user) { create(:user, organisation: create(:organisation)) } + let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } } + let(:organisation_id) { unaffiliated_user.organisation_id } + + before do + create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org) + bulk_upload.update!(organisation_id:, user: unaffiliated_user) + end + + it "blocks log creation and adds an error to field_1" do + parser = described_class.new(attributes) + parser.valid? + expect(parser).to be_block_log_creation + expect(parser.errors[:field_1]).to include("You do not have permission to add logs for this owning organisation") + end + end + + context "when user is an unaffiliated support user and bulk upload organisation is affiliated with the owning organisation" do + let(:affiliated_org) { create(:organisation, :with_old_visible_id) } + let(:unaffiliated_support_user) { create(:user, :support, organisation: create(:organisation)) } + let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } } + let(:organisation_id) { affiliated_org.id } + + before do + create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org) + bulk_upload.update!(organisation_id:, user: unaffiliated_support_user) + end + + it "does not block log creation and does not add an error to field_1" do + parser = described_class.new(attributes) + parser.valid? + expect(parser.errors[:field_1]).not_to include("You do not have permission to add logs for this owning organisation") + end + end end describe "#field_2" do # managing org 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 e428f7792..ca8f29e92 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -554,6 +554,43 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do expect(parser.errors[:field_6]).to include(/Enter a date when the owning organisation was active/) end end + + context "when user is an unaffiliated non-support user and bulk upload organisation is affiliated with the owning organisation" do + let(:affiliated_org) { create(:organisation, :with_old_visible_id) } + let(:unaffiliated_user) { create(:user, organisation: create(:organisation)) } + let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } } + let(:organisation_id) { unaffiliated_user.organisation_id } + + before do + create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org) + bulk_upload.update!(organisation_id:, user: unaffiliated_user) + end + + it "blocks log creation and adds an error to field_1" do + parser = described_class.new(attributes) + parser.valid? + expect(parser).to be_block_log_creation + expect(parser.errors[:field_1]).to include("You do not have permission to add logs for this owning organisation") + end + end + + context "when user is an unaffiliated support user and bulk upload organisation is affiliated with the owning organisation" do + let(:affiliated_org) { create(:organisation, :with_old_visible_id) } + let(:unaffiliated_support_user) { create(:user, :support, organisation: create(:organisation)) } + let(:attributes) { { bulk_upload:, field_1: affiliated_org.old_visible_id } } + let(:organisation_id) { affiliated_org.id } + + before do + create(:organisation_relationship, parent_organisation: owning_org, child_organisation: affiliated_org) + bulk_upload.update!(organisation_id:, user: unaffiliated_support_user) + end + + it "does not block log creation and does not add an error to field_1" do + parser = described_class.new(attributes) + parser.valid? + expect(parser.errors[:field_1]).not_to include("You do not have permission to add logs for this owning organisation") + end + end end describe "#field_3" do # username for assigned_to @@ -576,6 +613,23 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do end end + context "when blank and bulk upload user is support" do + let(:bulk_upload) { create(:bulk_upload, :sales, user: create(:user, :support), year: 2024) } + + let(:attributes) { setup_section_params.merge(bulk_upload:, field_3: nil) } + + it "is not permitted" do + parser.valid? + expect(parser.errors[:field_3]).to be_present + expect(parser.errors[:field_3]).to include("You must answer what is the CORE username of the account this sales log should be assigned to?") + end + + it "blocks log creation" do + parser.valid? + expect(parser).to be_block_log_creation + end + end + context "when user could not be found" do let(:attributes) { { bulk_upload:, field_3: "idonotexist@example.com" } }