Browse Source

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
pull/2681/head^2
kosiakkatrina 3 months ago committed by GitHub
parent
commit
d16f71981a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 40
      app/controllers/organisations_controller.rb
  2. 8
      app/helpers/schemes_helper.rb
  3. 8
      app/policies/organisation_policy.rb
  4. 138
      app/views/organisations/duplicate_schemes.html.erb
  5. 10
      app/views/organisations/schemes.html.erb
  6. 2
      config/locales/en.yml
  7. 2
      config/routes.rb
  8. 5
      db/migrate/20240920144611_add_schemes_deduplicated_at.rb
  9. 1
      db/schema.rb
  10. 117
      spec/helpers/schemes_helper_spec.rb
  11. 150
      spec/requests/organisations_controller_spec.rb

40
app/controllers/organisations_controller.rb

@ -44,6 +44,12 @@ class OrganisationsController < ApplicationController
redirect_to schemes_csv_confirmation_organisation_path redirect_to schemes_csv_confirmation_organisation_path
end end
def duplicate_schemes
authorize @organisation
get_duplicate_schemes_and_locations
end
def show def show
redirect_to details_organisation_path(@organisation) redirect_to details_organisation_path(@organisation)
end end
@ -295,6 +301,22 @@ class OrganisationsController < ApplicationController
render json: org_data.to_json render json: org_data.to_json
end 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 private
def filter_type def filter_type
@ -325,6 +347,10 @@ private
params.require(:organisation).permit(rent_periods: [], all_rent_periods: []) params.require(:organisation).permit(rent_periods: [], all_rent_periods: [])
end end
def scheme_duplicates_checked_params
params.require(:organisation).permit(:scheme_duplicates_checked)
end
def codes_only_export? def codes_only_export?
params.require(:codes_only) == "true" params.require(:codes_only) == "true"
end end
@ -344,4 +370,18 @@ private
def find_resource def find_resource
@organisation = Organisation.find(params[:id]) @organisation = Organisation.find(params[:id])
end 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 end

8
app/helpers/schemes_helper.rb

@ -85,6 +85,14 @@ module SchemesHelper
end end
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 private
ActivePeriod = Struct.new(:from, :to) ActivePeriod = Struct.new(:from, :to)

8
app/policies/organisation_policy.rb

@ -34,4 +34,12 @@ class OrganisationPolicy
editable_sales_logs = organisation.sales_logs.visible.after_date(editable_from_date) editable_sales_logs = organisation.sales_logs.visible.after_date(editable_from_date)
organisation.sales_logs.visible.where(saledate: nil).any? || editable_sales_logs.any? organisation.sales_logs.visible.where(saledate: nil).any? || editable_sales_logs.any?
end end
def duplicate_schemes?
user.support? || (user.data_coordinator? && user.organisation == organisation)
end
def confirm_duplicate_schemes?
duplicate_schemes?
end
end end

138
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 %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop">
<h1 class="govuk-heading-xl"><%= title %></h1>
<p class="govuk-body">Since your organisation recently merged, we’ve reviewed your schemes for possible duplicates.</p>
<p class="govuk-body">These sets of schemes and locations might be duplicates because they have the same answers for certain fields.</p>
<h2 class="govuk-heading-m">What you need to do</h2>
<ul class="govuk-list govuk-list--bullet">
<li>Review each set of schemes or locations and decide if they are duplicates.</li>
<li>If they are, choose one to keep and deactivate the others on the date your organisation merged.</li>
<li>When you have resolved all duplicates, confirm below.</li>
</ul>
<p class="govuk-body">If you need help with this, <%= govuk_link_to "contact the helpdesk (opens in a new tab)", GlobalConstants::HELPDESK_URL, target: "#" %>.</p>
<% if @duplicate_schemes.any? %>
<h2 class="govuk-heading-m"><%= @duplicate_schemes.count == 1 ? "This set" : "These #{@duplicate_schemes.count} sets" %> of schemes might have duplicates</h2>
<%= govuk_details(summary_text: "Why are these schemes identified as duplicates?") do %>
<p class="govuk-body">
These schemes have the same answers for the following fields:
</p>
<ul class="govuk-list govuk-list--bullet">
<li>Type of scheme</li>
<li>Registered under Care Standards Act 2000</li>
<li>Housing stock owned by</li>
<li>Support services provided by</li>
<li>Primary client group</li>
<li>Has another client group</li>
<li>Secondary client group</li>
<li>Level of support given</li>
<li>Intended length of stay</li>
</ul>
<% end %>
<%= govuk_table do |table| %>
<%= table.with_head do |head| %>
<% head.with_row do |row| %>
<% row.with_cell(header: true, text: "Schemes") %>
<% end %>
<%= table.with_body do |body| %>
<% @duplicate_schemes.each do |duplicate_set| %>
<% body.with_row do |row| %>
<% row.with_cell do %>
<ol class="govuk-list govuk-list--number">
<% duplicate_set.each do |scheme| %>
<li>
<%= govuk_link_to scheme.service_name, scheme %>
</li>
<% end %>
</ol>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
<% if @duplicate_locations.any? %>
<h2 class="govuk-heading-m"><%= @duplicate_locations.count == 1 ? "This set" : "These #{@duplicate_locations.count} sets" %> of locations might have duplicates</h2>
<%= govuk_details(summary_text: "Why are these locations identified as duplicates?") do %>
<p class="govuk-body">
These locations belong to the same scheme and have the same answers for the following fields:
</p>
<ul class="govuk-list govuk-list--bullet">
<li>Postcode</li>
<li>Mobility standards</li>
</ul>
<% end %>
<%= govuk_table do |table| %>
<%= table.with_head do |head| %>
<% head.with_row do |row| %>
<% row.with_cell(header: true, text: "Locations") %>
<% row.with_cell(header: true, text: "Scheme") %>
<% end %>
<%= table.with_body do |body| %>
<% @duplicate_locations.each do |duplicate_set| %>
<% body.with_row do |row| %>
<% row.with_cell do %>
<ol class="govuk-list govuk-list--number">
<% duplicate_set[:locations].each do |location| %>
<li>
<%= govuk_link_to location.name, scheme_location_path(location) %>
</li>
<% end %>
</ol>
<% end %>
<% row.with_cell do %>
<%= govuk_link_to duplicate_set[:scheme].service_name, duplicate_set[:scheme] %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
<% end %>
<%= f.govuk_check_boxes_fieldset :scheme_duplicates_checked,
legend: { text: "Have you resolved all duplicates?" } do %>
<%= f.govuk_check_box :scheme_duplicates_checked,
true,
false,
multiple: false,
checked: false,
label: { text: "Yes, none of the schemes and locations above are duplicates" } %>
<% end %>
<%= f.govuk_submit "Confirm" %>
</div>
</div>
<% end %>

10
app/views/organisations/schemes.html.erb

@ -11,6 +11,16 @@
) %> ) %>
<h2 class="govuk-visually-hidden">Supported housing schemes</h2> <h2 class="govuk-visually-hidden">Supported housing schemes</h2>
<% end %> <% end %>
<% if display_duplicate_schemes_banner?(@organisation, current_user) %>
<%= govuk_notification_banner(title_text: "Important") do %>
<p class="govuk-notification-banner__heading govuk-!-width-full" style="max-width: fit-content">
Some schemes and locations might be duplicates.
<p>
<%= govuk_link_to "Review possible duplicates", href: schemes_duplicates_organisation_path(@organisation) %>
<% end %>
<% end %>
<div class="app-filter-layout" data-controller="filter-layout"> <div class="app-filter-layout" data-controller="filter-layout">
<% if SchemePolicy.new(current_user, nil).create? %> <% 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 } %>

2
config/locales/en.yml

@ -37,6 +37,7 @@ en:
updated: "Organisation details updated." updated: "Organisation details updated."
reactivated: "%{organisation} has been reactivated." reactivated: "%{organisation} has been reactivated."
deactivated: "%{organisation} has been deactivated." deactivated: "%{organisation} has been deactivated."
duplicate_schemes_confirmed: "You’ve confirmed the remaining schemes and locations are not duplicates."
user: user:
create_password: "Create a password to finish setting up your account." create_password: "Create a password to finish setting up your account."
reset_password: "Reset your password." reset_password: "Reset your password."
@ -229,6 +230,7 @@ en:
blank: "You must choose a managing agent." blank: "You must choose a managing agent."
already_added: "You have already added this managing agent." already_added: "You have already added this managing agent."
merged: "That organisation has already been merged. Select a different organisation." merged: "That organisation has already been merged. Select a different organisation."
scheme_duplicates_not_resolved: "You must resolve all duplicates or indicate that there are no duplicates"
not_answered: "You must answer %{question}" not_answered: "You must answer %{question}"
invalid_option: "Enter a valid value for %{question}" invalid_option: "Enter a valid value for %{question}"
invalid_number: "Enter a number for %{question}" invalid_number: "Enter a number for %{question}"

2
config/routes.rb

@ -178,6 +178,8 @@ Rails.application.routes.draw do
get "schemes/csv-download", to: "organisations#download_schemes_csv" get "schemes/csv-download", to: "organisations#download_schemes_csv"
post "schemes/email-csv", to: "organisations#email_schemes_csv" post "schemes/email-csv", to: "organisations#email_schemes_csv"
get "schemes/csv-confirmation", to: "schemes#csv_confirmation" get "schemes/csv-confirmation", to: "schemes#csv_confirmation"
get "schemes/duplicates", to: "organisations#duplicate_schemes"
post "schemes/duplicates", to: "organisations#confirm_duplicate_schemes"
get "stock-owners", to: "organisation_relationships#stock_owners" get "stock-owners", to: "organisation_relationships#stock_owners"
get "stock-owners/add", to: "organisation_relationships#add_stock_owner" get "stock-owners/add", to: "organisation_relationships#add_stock_owner"
get "stock-owners/remove", to: "organisation_relationships#remove_stock_owner" get "stock-owners/remove", to: "organisation_relationships#remove_stock_owner"

5
db/migrate/20240920144611_add_schemes_deduplicated_at.rb

@ -0,0 +1,5 @@
class AddSchemesDeduplicatedAt < ActiveRecord::Migration[7.0]
def change
add_column :organisations, :schemes_deduplicated_at, :datetime
end
end

1
db/schema.rb

@ -514,6 +514,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_23_145326) do
t.bigint "absorbing_organisation_id" t.bigint "absorbing_organisation_id"
t.datetime "available_from" t.datetime "available_from"
t.datetime "discarded_at" t.datetime "discarded_at"
t.datetime "schemes_deduplicated_at"
t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id" t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id"
t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true
end end

117
spec/helpers/schemes_helper_spec.rb

@ -118,4 +118,121 @@ RSpec.describe SchemesHelper do
end end
end end
end end
describe "display_duplicate_schemes_banner?" do
let(:organisation) { create(:organisation) }
let(:current_user) { create(:user, :support) }
context "when organisation has not absorbed other organisations" do
context "and it has duplicate schemes" do
before do
create_list(:scheme, 2, :duplicate, owning_organisation: organisation)
end
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
end
context "when organisation has absorbed other organisations in open collection year" do
before do
build(:organisation, merge_date: Time.zone.yesterday, absorbing_organisation_id: organisation.id).save(validate: false)
end
context "and it has duplicate schemes" do
before do
create_list(:scheme, 2, :duplicate, owning_organisation: organisation)
end
it "displays the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_truthy
end
context "and organisation has confirmed duplicate schemes after the most recent merge" do
before do
organisation.update!(schemes_deduplicated_at: Time.zone.today)
end
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
context "and organisation has confirmed duplicate schemes before the most recent merge" do
before do
organisation.update!(schemes_deduplicated_at: Time.zone.today - 2.days)
end
it "displays the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_truthy
end
end
end
context "and it has duplicate locations" do
let(:scheme) { create(:scheme, owning_organisation: organisation) }
before do
create_list(:location, 2, postcode: "AB1 2CD", mobility_type: "A", scheme:)
end
it "displays the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_truthy
end
end
context "and it has no duplicate schemes or locations" do
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
context "and it is viewed by data provider" do
let(:current_user) { create(:user, :data_provider) }
before do
create_list(:scheme, 2, :duplicate, owning_organisation: organisation)
end
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
end
context "when organisation has absorbed other organisations in closed collection year" do
before do
build(:organisation, merge_date: Time.zone.today - 2.years, absorbing_organisation_id: organisation.id).save(validate: false)
end
context "and it has duplicate schemes" do
before do
create_list(:scheme, 2, :duplicate, owning_organisation: organisation)
end
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
context "and it has duplicate locations" do
let(:scheme) { create(:scheme, owning_organisation: organisation) }
before do
create(:location, postcode: "AB1 2CD", mobility_type: "A", scheme:)
end
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
context "and it has no duplicate schemes or locations" do
it "does not display the banner" do
expect(display_duplicate_schemes_banner?(organisation, current_user)).to be_falsey
end
end
end
end
end end

150
spec/requests/organisations_controller_spec.rb

@ -207,6 +207,24 @@ RSpec.describe OrganisationsController, type: :request do
expect(page).to have_title("#{user.organisation.name} (1 scheme matching ‘#{search_param}’) - Submit social housing lettings and sales data (CORE) - GOV.UK") expect(page).to have_title("#{user.organisation.name} (1 scheme matching ‘#{search_param}’) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end end
end end
context "when organisation has absorbed other organisations" do
before do
create(:organisation, merge_date: Time.zone.today, absorbing_organisation: organisation)
end
context "and it has duplicate schemes or locations" do
before do
create_list(:scheme, 2, :duplicate, owning_organisation: organisation)
get "/organisations/#{organisation.id}/schemes", headers:, params: {}
end
it "displays a banner with correct content" do
expect(page).to have_content("Some schemes and locations might be duplicates.")
expect(page).to have_link("Review possible duplicates", href: "/organisations/#{organisation.id}/schemes/duplicates")
end
end
end
end end
context "when data coordinator user" do context "when data coordinator user" do
@ -348,6 +366,77 @@ RSpec.describe OrganisationsController, type: :request do
end end
end end
describe "#duplicate_schemes" do
context "with support user" do
let(:user) { create(:user, :support) }
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
get "/organisations/#{organisation.id}/schemes/duplicates", headers:
end
context "with duplicate schemes and locations" do
let(:schemes) { create_list(:scheme, 5, :duplicate, owning_organisation: organisation) }
before do
create_list(:location, 2, scheme: schemes.first, postcode: "M1 1AA", mobility_type: "M")
create_list(:location, 2, scheme: schemes.first, postcode: "M1 1AA", mobility_type: "A")
get "/organisations/#{organisation.id}/schemes/duplicates", headers:
end
it "displays the duplicate schemes" do
expect(page).to have_content("This set of schemes might have duplicates")
end
it "displays the duplicate locations" do
expect(page).to have_content("These 2 sets of locations might have duplicates")
end
it "has page heading" do
expect(page).to have_content("Review these sets of schemes and locations")
end
end
context "without duplicate schemes and locations" do
it "does not display the schemes" do
expect(page).not_to have_content("schemes might have duplicates")
end
it "does not display the locations" do
expect(page).not_to have_content("locations might have duplicates")
end
end
end
context "with data coordinator user" do
let(:user) { create(:user, :data_coordinator) }
before do
sign_in user
create_list(:scheme, 5, :duplicate, owning_organisation: organisation)
get "/organisations/#{organisation.id}/schemes/duplicates", headers:
end
it "has page heading" do
expect(page).to have_content("Review these sets of schemes")
end
end
context "with data provider user" do
let(:user) { create(:user, :data_provider) }
before do
sign_in user
get "/organisations/#{organisation.id}/schemes/duplicates", headers:
end
it "be unauthorised" do
expect(response).to have_http_status(:unauthorized)
end
end
end
describe "#show" do describe "#show" do
context "with an organisation that the user belongs to" do context "with an organisation that the user belongs to" do
let(:set_time) {} let(:set_time) {}
@ -2263,4 +2352,65 @@ RSpec.describe OrganisationsController, type: :request do
end end
end end
end end
describe "POST #confirm_duplicate_schemes" do
let(:organisation) { create(:organisation) }
context "when not signed in" do
it "redirects to sign in" do
post "/organisations/#{organisation.id}/schemes/duplicates", headers: headers
expect(response).to redirect_to("/account/sign-in")
end
end
context "when signed in" do
before do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
sign_in user
end
context "when user is data provider" do
let(:user) { create(:user, role: "data_provider", organisation:) }
it "returns not found" do
post "/organisations/#{organisation.id}/schemes/duplicates", headers: headers
expect(response).to have_http_status(:unauthorized)
end
end
context "when user is coordinator" do
let(:user) { create(:user, role: "data_coordinator", organisation:) }
context "and the duplicate schemes have been confirmed" do
let(:params) { { "organisation": { scheme_duplicates_checked: "true" } } }
it "redirects to schemes page" do
post "/organisations/#{organisation.id}/schemes/duplicates", headers: headers, params: params
expect(response).to redirect_to("/organisations/#{organisation.id}/schemes")
expect(flash[:notice]).to eq("You’ve confirmed the remaining schemes and locations are not duplicates.")
end
it "updates schemes_deduplicated_at" do
expect(organisation.reload.schemes_deduplicated_at).to be_nil
post "/organisations/#{organisation.id}/schemes/duplicates", headers: headers, params: params
expect(organisation.reload.schemes_deduplicated_at).not_to be_nil
end
end
context "and the duplicate schemes have not been confirmed" do
let(:params) { { "organisation": { scheme_duplicates_checked: "" } } }
it "displays an error" do
post "/organisations/#{organisation.id}/schemes/duplicates", headers: headers, params: params
expect(response).to have_http_status(:unprocessable_entity)
expect(page).to have_content("You must resolve all duplicates or indicate that there are no duplicates")
end
end
end
end
end
end end

Loading…
Cancel
Save