baarkerlounger
/
submit-social-housing-lettings-and-sales-data
mirror of https://github.com/communitiesuk/submit-social-housing-lettings-and-sales-data.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Allow confirming duplicate schemes

pull/2655/head
Kat 2 years ago committed by kosiakkatrina
parent
9bc8845421
commit
8395c94117
8 changed files with 214 additions and 101 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 46
      app/controllers/organisations_controller.rb
  2. 4
      app/policies/organisation_policy.rb
  3. 189
      app/views/organisations/duplicate_schemes.html.erb
  4. 2
      config/locales/en.yml
  5. 1
      config/routes.rb
  6. 5
      db/migrate/20240920144611_add_schemes_deduplicated_at.rb
  7. 3
      db/schema.rb
  8. 65
      spec/requests/organisations_controller_spec.rb

46
app/controllers/organisations_controller.rb
Unescape View File

@ -47,17 +47,7 @@ class OrganisationsController < ApplicationController
def duplicate_schemes def duplicate_schemes
authorize @organisation authorize @organisation
duplicate_scheme_sets = @organisation.owned_schemes.duplicate_sets get_duplicate_schemes_and_locations
@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
def show def show
@ -311,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
@ -341,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
@ -360,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

4
app/policies/organisation_policy.rb
Unescape View File

@ -38,4 +38,8 @@ class OrganisationPolicy
def duplicate_schemes? def duplicate_schemes?
user.support? || (user.data_coordinator? && user.organisation == organisation) user.support? || (user.data_coordinator? && user.organisation == organisation)
end end
def confirm_duplicate_schemes?
duplicate_schemes?
end
end end

189
app/views/organisations/duplicate_schemes.html.erb
Unescape View File

@ -1,114 +1,129 @@
<% content_for :before_content do %> <% content_for :before_content do %>
<%= govuk_back_link href: schemes_organisation_path(@organisation) %> <%= govuk_back_link href: schemes_organisation_path(@organisation) %>
<% end %> <% end %>
<%= form_with model: @organisation, url: schemes_duplicates_organisation_path(@organisation), method: "post" do |f| %>
<%= f.govuk_error_summary %>
<% title = "Review these sets of schemes and locations" %> <% title = "Review these sets of schemes and locations" %>
<% content_for :title, title %> <% content_for :title, title %>
<% if current_user.support? %> <% if current_user.support? %>
<%= render SubNavigationComponent.new( <%= render SubNavigationComponent.new(
items: secondary_items(request.path, @organisation.id), items: secondary_items(request.path, @organisation.id),
) %> ) %>
<% end %> <% end %>
<div class="govuk-grid-row"> <div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds-from-desktop"> <div class="govuk-grid-column-two-thirds-from-desktop">
<h1 class="govuk-heading-xl"><%= title %></h1> <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">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> <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> <h2 class="govuk-heading-m">What you need to do</h2>
<ul class="govuk-list govuk-list--bullet"> <ul class="govuk-list govuk-list--bullet">
<li>Review each set of schemes or locations and decide if they are duplicates.</li> <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>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> <li>When you have resolved all duplicates, confirm below.</li>
</ul> </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> <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? %> <% 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> <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 %> <%= govuk_details(summary_text: "Why are these schemes identified as duplicates?") do %>
<p class="govuk-body"> <p class="govuk-body">
These schemes have the same answers for the following fields: These schemes have the same answers for the following fields:
</p> </p>
<ul class="govuk-list govuk-list--bullet"> <ul class="govuk-list govuk-list--bullet">
<li>Type of scheme</li> <li>Type of scheme</li>
<li>Registered under Care Standards Act 2000</li> <li>Registered under Care Standards Act 2000</li>
<li>Housing stock owned by</li> <li>Housing stock owned by</li>
<li>Support services provided by</li> <li>Support services provided by</li>
<li>Primary client group</li> <li>Primary client group</li>
<li>Has another client group</li> <li>Has another client group</li>
<li>Secondary client group</li> <li>Secondary client group</li>
<li>Level of support given</li> <li>Level of support given</li>
<li>Intended length of stay</li> <li>Intended length of stay</li>
</ul> </ul>
<% end %> <% end %>
<%= govuk_table do |table| %> <%= govuk_table do |table| %>
<%= table.with_head do |head| %> <%= table.with_head do |head| %>
<% head.with_row do |row| %> <% head.with_row do |row| %>
<% row.with_cell(header: true, text: "Schemes") %> <% row.with_cell(header: true, text: "Schemes") %>
<% end %> <% end %>
<%= table.with_body do |body| %> <%= table.with_body do |body| %>
<% @duplicate_schemes.each do |duplicate_set| %> <% @duplicate_schemes.each do |duplicate_set| %>
<% body.with_row do |row| %> <% body.with_row do |row| %>
<% row.with_cell do %> <% row.with_cell do %>
<ol class="govuk-list govuk-list--number"> <ol class="govuk-list govuk-list--number">
<% duplicate_set.each do |scheme| %> <% duplicate_set.each do |scheme| %>
<li> <li>
<%= govuk_link_to scheme.service_name, scheme %> <%= govuk_link_to scheme.service_name, scheme %>
</li> </li>
<% end %> <% end %>
</ol> </ol>
<% end %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% end %>
<% if @duplicate_locations.any? %> <% 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> <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 %> <%= govuk_details(summary_text: "Why are these locations identified as duplicates?") do %>
<p class="govuk-body"> <p class="govuk-body">
These locations belong to the same scheme and have the same answers for the following fields: These locations belong to the same scheme and have the same answers for the following fields:
</p> </p>
<ul class="govuk-list govuk-list--bullet"> <ul class="govuk-list govuk-list--bullet">
<li>Postcode</li> <li>Postcode</li>
<li>Mobility standards</li> <li>Mobility standards</li>
</ul> </ul>
<% end %> <% end %>
<%= govuk_table do |table| %> <%= govuk_table do |table| %>
<%= table.with_head do |head| %> <%= table.with_head do |head| %>
<% head.with_row do |row| %> <% head.with_row do |row| %>
<% row.with_cell(header: true, text: "Locations") %> <% row.with_cell(header: true, text: "Locations") %>
<% row.with_cell(header: true, text: "Scheme") %> <% row.with_cell(header: true, text: "Scheme") %>
<% end %> <% end %>
<%= table.with_body do |body| %> <%= table.with_body do |body| %>
<% @duplicate_locations.each do |duplicate_set| %> <% @duplicate_locations.each do |duplicate_set| %>
<% body.with_row do |row| %> <% body.with_row do |row| %>
<% row.with_cell do %> <% row.with_cell do %>
<ol class="govuk-list govuk-list--number"> <ol class="govuk-list govuk-list--number">
<% duplicate_set[:locations].each do |location| %> <% duplicate_set[:locations].each do |location| %>
<li> <li>
<%= govuk_link_to location.name, scheme_location_path(location) %> <%= govuk_link_to location.name, scheme_location_path(location) %>
</li> </li>
<% end %> <% end %>
</ol> </ol>
<% end %> <% end %>
<% row.with_cell do %> <% row.with_cell do %>
<%= govuk_link_to duplicate_set[:scheme].service_name, duplicate_set[:scheme] %> <%= govuk_link_to duplicate_set[:scheme].service_name, duplicate_set[:scheme] %>
<% end %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% 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> </div>
</div> <% end %>

2
config/locales/en.yml
Unescape View File

@ -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}"

1
config/routes.rb
Unescape View File

@ -179,6 +179,7 @@ Rails.application.routes.draw do
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" 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
Unescape View File

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

3
db/schema.rb
Unescape View File

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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_20_144611) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -512,6 +512,7 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_18_112702) 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

65
spec/requests/organisations_controller_spec.rb
Unescape View File

@ -411,10 +411,10 @@ RSpec.describe OrganisationsController, type: :request do
context "with data coordinator user" do context "with data coordinator user" do
let(:user) { create(:user, :data_coordinator) } let(:user) { create(:user, :data_coordinator) }
let!(:schemes) { create_list(:scheme, 5, :duplicate) }
before do before do
sign_in user sign_in user
create_list(:scheme, 5, :duplicate)
get "/organisations/#{organisation.id}/schemes/duplicates", headers: get "/organisations/#{organisation.id}/schemes/duplicates", headers:
end end
@ -423,7 +423,7 @@ RSpec.describe OrganisationsController, type: :request do
end end
end end
context "with data coordinator user" do context "with data provider user" do
let(:user) { create(:user, :data_provider) } let(:user) { create(:user, :data_provider) }
before do before do
@ -2352,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

Write Preview
Loading…
Cancel
Save