From 6cdf3bbf6397c60d7b29ee1f2350ec2ef469f06e Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 8 Oct 2024 09:38:52 +0100 Subject: [PATCH 1/5] Correctly caption message to sentry (#2679) --- app/services/bulk_upload/sales/validator.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index 777424349..a473a6461 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -43,12 +43,12 @@ class BulkUpload::Sales::Validator return false if any_setup_errors? if row_parsers.any?(&:block_log_creation?) - Sentry.capture_exception("Bulk upload log creation blocked: #{bulk_upload.id}.") + Sentry.capture_message("Bulk upload log creation blocked: #{bulk_upload.id}.") return false end if any_logs_already_exist? && FeatureToggle.bulk_upload_duplicate_log_check_enabled? - Sentry.capture_exception("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") + Sentry.capture_message("Bulk upload log creation blocked due to duplicate logs: #{bulk_upload.id}.") return false end @@ -57,7 +57,7 @@ class BulkUpload::Sales::Validator end if any_logs_invalid? - Sentry.capture_exception("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") + Sentry.capture_message("Bulk upload log creation blocked due to invalid logs after blanking non setup fields: #{bulk_upload.id}.") return false end From d16f71981ab7e30d9f1a744de1de9746b60cf34b Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 8 Oct 2024 14:48:00 +0100 Subject: [PATCH 2/5] CLDC-3619 Add duplicate schemes after merge view (#2655) * Add duplicate sets scope to schemes * Add rake task to write duplicate scheme sets * Add duplicate sets scope to locations * Add rake task to write duplicate locations * lint * Update location duplicate count * Add scheme_id back to DUPLICATE_LOCATION_ATTRIBUTES * Add display duplicate schemes banner method * Add banner to the schemes page * Add page content * Allow confirming duplicate schemes * Update display_duplicate_schemes_banner? method * Make title dynamic --- app/controllers/organisations_controller.rb | 40 +++++ app/helpers/schemes_helper.rb | 8 + app/policies/organisation_policy.rb | 8 + .../organisations/duplicate_schemes.html.erb | 138 ++++++++++++++++ app/views/organisations/schemes.html.erb | 10 ++ config/locales/en.yml | 2 + config/routes.rb | 2 + ...40920144611_add_schemes_deduplicated_at.rb | 5 + db/schema.rb | 1 + spec/helpers/schemes_helper_spec.rb | 117 ++++++++++++++ .../requests/organisations_controller_spec.rb | 150 ++++++++++++++++++ 11 files changed, 481 insertions(+) create mode 100644 app/views/organisations/duplicate_schemes.html.erb create mode 100644 db/migrate/20240920144611_add_schemes_deduplicated_at.rb diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 044bab74d..61cd43674 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -44,6 +44,12 @@ class OrganisationsController < ApplicationController redirect_to schemes_csv_confirmation_organisation_path end + def duplicate_schemes + authorize @organisation + + get_duplicate_schemes_and_locations + end + def show redirect_to details_organisation_path(@organisation) end @@ -295,6 +301,22 @@ class OrganisationsController < ApplicationController render json: org_data.to_json end + def confirm_duplicate_schemes + authorize @organisation + + if scheme_duplicates_checked_params[:scheme_duplicates_checked] == "true" + @organisation.schemes_deduplicated_at = Time.zone.now + if @organisation.save + flash[:notice] = I18n.t("organisation.duplicate_schemes_confirmed") + redirect_to schemes_organisation_path(@organisation) + end + else + @organisation.errors.add(:scheme_duplicates_checked, I18n.t("validations.organisation.scheme_duplicates_not_resolved")) + get_duplicate_schemes_and_locations + render :duplicate_schemes, status: :unprocessable_entity + end + end + private def filter_type @@ -325,6 +347,10 @@ private params.require(:organisation).permit(rent_periods: [], all_rent_periods: []) end + def scheme_duplicates_checked_params + params.require(:organisation).permit(:scheme_duplicates_checked) + end + def codes_only_export? params.require(:codes_only) == "true" end @@ -344,4 +370,18 @@ private def find_resource @organisation = Organisation.find(params[:id]) end + + def get_duplicate_schemes_and_locations + duplicate_scheme_sets = @organisation.owned_schemes.duplicate_sets + @duplicate_schemes = duplicate_scheme_sets.map { |set| set.map { |id| @organisation.owned_schemes.find(id) } } + @duplicate_locations = [] + @organisation.owned_schemes.each do |scheme| + duplicate_location_sets = scheme.locations.duplicate_sets + next unless duplicate_location_sets.any? + + duplicate_location_sets.each do |duplicate_set| + @duplicate_locations << { scheme: scheme, locations: duplicate_set.map { |id| scheme.locations.find(id) } } + end + end + end end diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb index 0e318d283..7efb9fffd 100644 --- a/app/helpers/schemes_helper.rb +++ b/app/helpers/schemes_helper.rb @@ -85,6 +85,14 @@ module SchemesHelper end end + def display_duplicate_schemes_banner?(organisation, current_user) + return unless organisation.absorbed_organisations.merged_during_open_collection_period.any? + return unless current_user.data_coordinator? || current_user.support? + return if organisation.schemes_deduplicated_at.present? && organisation.schemes_deduplicated_at > organisation.absorbed_organisations.map(&:merge_date).max + + organisation.owned_schemes.duplicate_sets.any? || organisation.owned_schemes.any? { |scheme| scheme.locations.duplicate_sets.any? } + end + private ActivePeriod = Struct.new(:from, :to) diff --git a/app/policies/organisation_policy.rb b/app/policies/organisation_policy.rb index 9c27d6e91..9c5fc4449 100644 --- a/app/policies/organisation_policy.rb +++ b/app/policies/organisation_policy.rb @@ -34,4 +34,12 @@ class OrganisationPolicy editable_sales_logs = organisation.sales_logs.visible.after_date(editable_from_date) organisation.sales_logs.visible.where(saledate: nil).any? || editable_sales_logs.any? end + + def duplicate_schemes? + user.support? || (user.data_coordinator? && user.organisation == organisation) + end + + def confirm_duplicate_schemes? + duplicate_schemes? + end end diff --git a/app/views/organisations/duplicate_schemes.html.erb b/app/views/organisations/duplicate_schemes.html.erb new file mode 100644 index 000000000..427cf427c --- /dev/null +++ b/app/views/organisations/duplicate_schemes.html.erb @@ -0,0 +1,138 @@ +<% content_for :before_content do %> + <%= govuk_back_link href: schemes_organisation_path(@organisation) %> +<% end %> +<%= form_with model: @organisation, url: schemes_duplicates_organisation_path(@organisation), method: "post" do |f| %> + <%= f.govuk_error_summary %> + + <% if @duplicate_schemes.any? %> + <% if @duplicate_locations.any? %> + <% title = "Review these sets of schemes and locations" %> + <% else %> + <% title = "Review these sets of schemes" %> + <% end %> + <% else %> + <% title = "Review these sets of locations" %> + <% end %> + + <% content_for :title, title %> + + <% if current_user.support? %> + <%= render SubNavigationComponent.new( + items: secondary_items(request.path, @organisation.id), + ) %> + <% end %> + +
Since your organisation recently merged, we’ve reviewed your schemes for possible duplicates.
+These sets of schemes and locations might be duplicates because they have the same answers for certain fields.
+If you need help with this, <%= govuk_link_to "contact the helpdesk (opens in a new tab)", GlobalConstants::HELPDESK_URL, target: "#" %>.
+ + <% if @duplicate_schemes.any? %> ++ These schemes have the same answers for the following fields: +
++ These locations belong to the same scheme and have the same answers for the following fields: +
++ <%= govuk_link_to "Review possible duplicates", href: schemes_duplicates_organisation_path(@organisation) %> + <% end %> +<% end %> +
c3u&}6ZK7L(I{tH)+isvOqOqrX2j=Y;ynSnDOin}!nVV>E$;M8N5;x{i
z7nFhf>uxhaV{c0r5f K9*1=2%MS*UX8>gv>PfkRxrx9%uk}gl)$yjv^vUk^
zx+el-9GA6gF+q$6Aw%}m4~G>tfnVQKdUIV1`0mL2a@UlgZIwob$6W)PgOBfG$G;J~
z>f2W5NMxUAzCbHn+OCLN;DbQ9^|n{~-^EfTxpf;--u+2 G!*3R08MLJ!7p_R#lwtx(Fro B?ZO>*vwk`p<0L+C=8?QgW@o+xz?yV+NT6YQiEWsC`w?@3(ip+l|6igoons(dUGU
z8p=y-vISP8V{vCsE~7+Cufdz9+2!+0lQ|S;&pT_sjY-UY`fV}P^8w9{TVoF`#fku9
zr8}xzFnUpCbg3RKgo|f8_vl4D!$O4xzN^Ahdy-vHMTOl_uI%*!Lwq)QfEG#0YOcwo%vlnOQy7n~drZNt|M{)G?!5rk*o6SV(
zI=EDRLJHuY(XrW{;Ef(BC|DMKd&=EgLt}(_rAG|AKg_8IS;mB9=H?R}kY2oSF=YkW
zv1f<b#6={NT5k&OQ%(di-#LMRBp^1z5H9*u_;f&8L-$)gWe2f1M%QA6b`f0<$p(
zEgi(b7eKY_S}QmneeHm}ZTDvB>^8b~MeHR0C&eI6FE`7VR6r-NS*?
z(O?(E`ev~Ulaz&E#ZM?+b|8~8nN{hHa%r*j6en?M5P<7?OyBg*T3Ii>*WmZf=&a)$
zk)wg)Q)zO@{NCE{m2WRF&CX-dJ3?PY@KUt07hh!j4(FNpyy`&`XbvLU999VQh6Oq~
z??yqoGAh4T-;@tnaBj#6R