From e7684fc2ddb20efcfc28228480420049f03c3169 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:10:52 +0100 Subject: [PATCH] CLDC-2495 Add delete duplicate logs page (#1752) * CLDC-2494: WIP * CLDC-2494: wip * CLDC-2494: page work in progress * cleanup * Add a path for duplicate logs * Display all duplicate logs * Move a test * Display duplicate check answers for logs * Add buttons to delete duplicates * Add a route for sales logs duplicates * Update duplicated page to work for sales logs * lint * Add auth * Rebase updates * Update displayed questions * Add delete duplicates page * Update page content for when there are multiple duplicates * Add auth to the path * Update delete button to use delete controller * Update page for sales logs * Update styling * Update success banner for deleting duplicates * Typo * Render not found if there are no duplicate logs * rebase changes * Rebase changes * Update back and cancel links * Update the duplicate journey after deleting logs * Update change button routing * Refactor tests * Add content for no duplicates * Update params * Refactor tests and paths * Update params to include original_log_id from the beginning * Rename file * lint * Add full stop after deletion messages * Add caption and update duplicate log IDs in the banner * lint * Styling --------- Co-authored-by: Aaron Spencer --- app/controllers/delete_logs_controller.rb | 18 +- app/controllers/duplicate_logs_controller.rb | 28 +- app/controllers/form_controller.rb | 17 +- app/controllers/lettings_logs_controller.rb | 2 +- app/controllers/sales_logs_controller.rb | 2 +- app/helpers/duplicate_logs_helper.rb | 25 ++ .../_duplicate_log_check_answers.erb | 2 +- app/views/duplicate_logs/show.erb | 20 -- app/views/duplicate_logs/show.html.erb | 27 ++ app/views/logs/delete_duplicates.html.erb | 40 +++ config/locales/en.yml | 7 +- config/routes.rb | 2 + spec/factories/lettings_log.rb | 2 + spec/factories/sales_log.rb | 1 + spec/features/lettings_log_spec.rb | 60 ++++ spec/features/sales_log_spec.rb | 63 ++++ spec/requests/delete_logs_controller_spec.rb | 8 +- .../duplicate_logs_controller_spec.rb | 313 +++++++++++++----- spec/requests/form_controller_spec.rb | 4 +- .../requests/lettings_logs_controller_spec.rb | 2 +- spec/requests/sales_logs_controller_spec.rb | 2 +- 21 files changed, 512 insertions(+), 133 deletions(-) create mode 100644 app/helpers/duplicate_logs_helper.rb delete mode 100644 app/views/duplicate_logs/show.erb create mode 100644 app/views/duplicate_logs/show.html.erb create mode 100644 app/views/logs/delete_duplicates.html.erb diff --git a/app/controllers/delete_logs_controller.rb b/app/controllers/delete_logs_controller.rb index 0b4dc3ba5..5f4c97cbd 100644 --- a/app/controllers/delete_logs_controller.rb +++ b/app/controllers/delete_logs_controller.rb @@ -26,8 +26,11 @@ class DeleteLogsController < ApplicationController def discard_lettings_logs logs = LettingsLog.find(params.require(:ids)) discard logs - - redirect_to lettings_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count) + if request.referer&.include?("delete-duplicates") + redirect_to lettings_log_duplicate_logs_path(lettings_log_id: params["remaining_log_id"], original_log_id: params["original_log_id"]), notice: I18n.t("notification.duplicate_logs_deleted", count: logs.count, log_ids: duplicate_log_ids(logs)) + else + redirect_to lettings_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count) + end end def delete_sales_logs @@ -52,8 +55,11 @@ class DeleteLogsController < ApplicationController def discard_sales_logs logs = SalesLog.find(params.require(:ids)) discard logs - - redirect_to sales_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count) + if request.referer&.include?("delete-duplicates") + redirect_to sales_log_duplicate_logs_path(sales_log_id: params["remaining_log_id"], original_log_id: params["original_log_id"]), notice: I18n.t("notification.duplicate_logs_deleted", count: logs.count, log_ids: duplicate_log_ids(logs)) + else + redirect_to sales_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count) + end end def delete_lettings_logs_for_organisation @@ -193,4 +199,8 @@ private log.discard! end end + + def duplicate_log_ids(logs) + logs.map { |log| "Log #{log.id}" }.to_sentence(last_word_connector: " and ") + end end diff --git a/app/controllers/duplicate_logs_controller.rb b/app/controllers/duplicate_logs_controller.rb index 21e853bf9..b084a8124 100644 --- a/app/controllers/duplicate_logs_controller.rb +++ b/app/controllers/duplicate_logs_controller.rb @@ -1,14 +1,11 @@ class DuplicateLogsController < ApplicationController before_action :authenticate_user! before_action :find_resource_by_named_id + before_action :find_duplicate_logs + before_action :find_original_log_id def show if @log - @duplicate_logs = if @log.lettings? - current_user.lettings_logs.duplicate_logs(@log) - else - current_user.sales_logs.duplicate_logs(@log) - end @all_duplicates = [@log, *@duplicate_logs] @duplicate_check_questions = duplicate_check_question_ids.map { |question_id| question = @log.form.get_question(question_id, @log) @@ -19,6 +16,12 @@ class DuplicateLogsController < ApplicationController end end + def delete_duplicates + return render_not_found unless @log && @duplicate_logs.any? + + render "logs/delete_duplicates" + end + private def find_resource_by_named_id @@ -29,6 +32,16 @@ private end end + def find_duplicate_logs + return unless @log + + @duplicate_logs = if @log.lettings? + current_user.lettings_logs.duplicate_logs(@log) + else + current_user.sales_logs.duplicate_logs(@log) + end + end + def duplicate_check_question_ids if @log.lettings? ["owning_organisation_id", @@ -47,4 +60,9 @@ private %w[owning_organisation_id saledate purchid age1 sex1 ecstat1 postcode_full] end end + + def find_original_log_id + query_params = URI.parse(request.url).query + @original_log_id = CGI.parse(query_params)["original_log_id"][0]&.to_i if query_params.present? + end end diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index f016291ff..b7ab5de29 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -141,9 +141,16 @@ private return unless query_params parsed_params = CGI.parse(query_params) - return unless parsed_params["referrer"] + parsed_params["referrer"]&.first + end + + def original_duplicate_log_id_from_query + query_params = URI.parse(request.url).query - parsed_params["referrer"][0] + return unless query_params + + parsed_params = CGI.parse(query_params) + parsed_params["original_log_id"]&.first end def previous_interruption_screen_page_id @@ -158,10 +165,10 @@ private if FeatureToggle.deduplication_flow_enabled? if @log.lettings? if current_user.lettings_logs.duplicate_logs(@log).count.positive? - return send("lettings_log_duplicate_logs_path", @log) + return send("lettings_log_duplicate_logs_path", @log, original_log_id: @log.id) end elsif current_user.sales_logs.duplicate_logs(@log).count.positive? - return send("sales_log_duplicate_logs_path", @log) + return send("sales_log_duplicate_logs_path", @log, original_log_id: @log.id) end end @@ -177,7 +184,7 @@ private end end if previous_interruption_screen_page_id.present? - return send("#{@log.class.name.underscore}_#{previous_interruption_screen_page_id}_path", @log, { referrer: previous_interruption_screen_referrer }.compact) + return send("#{@log.class.name.underscore}_#{previous_interruption_screen_page_id}_path", @log, { referrer: previous_interruption_screen_referrer, original_log_id: original_duplicate_log_id_from_query }.compact) end redirect_path = form.next_page_redirect_path(@page, @log, current_user) diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index b29e72b5d..64e07efcd 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -77,7 +77,7 @@ class LettingsLogsController < LogsController @log.discard! - redirect_to lettings_logs_path, notice: "Log #{@log.id} has been deleted" + redirect_to lettings_logs_path, notice: "Log #{@log.id} has been deleted." end def delete_confirmation diff --git a/app/controllers/sales_logs_controller.rb b/app/controllers/sales_logs_controller.rb index 7c5083265..f7c69ec87 100644 --- a/app/controllers/sales_logs_controller.rb +++ b/app/controllers/sales_logs_controller.rb @@ -51,7 +51,7 @@ class SalesLogsController < LogsController @log.discard! - redirect_to sales_logs_path, notice: "Log #{@log.id} has been deleted" + redirect_to sales_logs_path, notice: "Log #{@log.id} has been deleted." end def delete_confirmation diff --git a/app/helpers/duplicate_logs_helper.rb b/app/helpers/duplicate_logs_helper.rb new file mode 100644 index 000000000..b73fc5cf3 --- /dev/null +++ b/app/helpers/duplicate_logs_helper.rb @@ -0,0 +1,25 @@ +module DuplicateLogsHelper + include GovukLinkHelper + + def duplicate_logs_continue_button(all_duplicates, duplicate_log, original_log_id) + if all_duplicates.count > 1 + return govuk_button_link_to "Keep this log and delete duplicates", url_for( + controller: "duplicate_logs", + action: "delete_duplicates", + "#{duplicate_log.class.name.underscore}_id": duplicate_log.id, + original_log_id:, + ) + end + + if original_log_id == duplicate_log.id + govuk_button_link_to "Back to Log #{duplicate_log.id}", send("#{duplicate_log.class.name.underscore}_path", duplicate_log) + else + type = duplicate_log.lettings? ? "lettings" : "sales" + govuk_button_link_to "Back to #{type} logs", url_for(duplicate_log.class) + end + end + + def duplicate_logs_action_href(log, page_id, original_log_id) + send("#{log.model_name.param_key}_#{page_id}_path", log, referrer: "interruption_screen", original_log_id:) + end +end diff --git a/app/views/duplicate_logs/_duplicate_log_check_answers.erb b/app/views/duplicate_logs/_duplicate_log_check_answers.erb index 15d4caf73..a5abaea16 100644 --- a/app/views/duplicate_logs/_duplicate_log_check_answers.erb +++ b/app/views/duplicate_logs/_duplicate_log_check_answers.erb @@ -29,7 +29,7 @@ <% row.action( text: question.action_text(@log), - href: action_href(@log, question.page.id), + href: duplicate_logs_action_href(@log, question.page.id, @original_log_id), visually_hidden_text: question.check_answer_label.to_s.downcase, ) %> <% end %> diff --git a/app/views/duplicate_logs/show.erb b/app/views/duplicate_logs/show.erb deleted file mode 100644 index 04da14593..000000000 --- a/app/views/duplicate_logs/show.erb +++ /dev/null @@ -1,20 +0,0 @@ -<% content_for :title, "Check duplicate logs" %> -
-
- <%= govuk_panel( - classes: "app-panel--interruption", - ) do %> -

These logs are duplicates

-

These logs have the same values for the following fields. Choose one to keep or correct the answers.

- <% end %> - - <% @all_duplicates.each_with_index do |log, index| %> - <%= render partial: "duplicate_log", locals: { log: log } %> - <%= render partial: "duplicate_log_check_answers", locals: { log: log } %> - <%= govuk_button_link_to "Keep this log and delete duplicates", "#" %> - <% if index < @all_duplicates.count - 1 %> -
- <% end %> - <% end %> -
-
diff --git a/app/views/duplicate_logs/show.html.erb b/app/views/duplicate_logs/show.html.erb new file mode 100644 index 000000000..1a7dd8e86 --- /dev/null +++ b/app/views/duplicate_logs/show.html.erb @@ -0,0 +1,27 @@ +<% content_for :title, "Check duplicate logs" %> +
+
+ <% if @all_duplicates.count > 1 %> + <%= govuk_panel( + classes: "app-panel--interruption", + ) do %> +

These logs are duplicates

+

These logs have the same values for the following fields. Choose one to keep or correct the answers.

+ <% end %> + <% else %> +

Make sure these answers are correct

+

+ This log had the same answers but it is no longer a duplicate. Make sure the answers are correct. +

+ <% end %> + + <% @all_duplicates.each_with_index do |log, index| %> + <%= render partial: "duplicate_log", locals: { log: } %> + <%= render partial: "duplicate_log_check_answers", locals: { log: } %> + <%= duplicate_logs_continue_button(@all_duplicates, log, @original_log_id) %> + <% if index < @all_duplicates.count - 1 %> +
+ <% end %> + <% end %> +
+
diff --git a/app/views/logs/delete_duplicates.html.erb b/app/views/logs/delete_duplicates.html.erb new file mode 100644 index 000000000..5ba4459e2 --- /dev/null +++ b/app/views/logs/delete_duplicates.html.erb @@ -0,0 +1,40 @@ +<% content_for :before_content do %> + <% content_for :title, "Are you sure you want to delete #{@duplicate_logs.count == 1 ? 'this duplicate log' : 'these duplicate logs'}?" %> + <%= govuk_back_link href: @log.lettings? ? lettings_log_duplicate_logs_path(@log) : sales_log_duplicate_logs_path(@log) %> +<% end %> + +
+
+ Delete duplicate logs +

+ <%= content_for(:title) %> +

+ + <%= govuk_warning_text(text: "You will not be able to undo this action.") %> + +

+ <%= @duplicate_logs.count == 1 ? "This log" : "These logs" %> will be deleted: +

+
    + <% @duplicate_logs.each do |duplicate_log| %> +
  • + + <%= govuk_link_to "Log #{duplicate_log.id}", url_for(duplicate_log) %> + +
    • + <% end %> +
+ +
+ <%= govuk_button_to @duplicate_logs.count == 1 ? "Delete this log" : "Delete these logs", + send("delete_logs_#{@log.class.name.underscore}s_path"), + method: "delete", + params: { ids: @duplicate_logs.map(&:id), original_log_id: @original_log_id, remaining_log_id: @log.id } %> + <%= govuk_button_link_to( + "Cancel", + send("#{@log.class.name.underscore}_duplicate_logs_path", @log), + secondary: true, + ) %> +
+
+
diff --git a/config/locales/en.yml b/config/locales/en.yml index ec73f2906..e3507b9e5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -180,8 +180,11 @@ en: notification: logs_deleted: - one: "%{count} log has been deleted" - other: "%{count} logs have been deleted" + one: "%{count} log has been deleted." + other: "%{count} logs have been deleted." + duplicate_logs_deleted: + one: "%{log_ids} has been deleted." + other: "%{log_ids} have been deleted." validations: organisation: diff --git a/config/routes.rb b/config/routes.rb index 4da2e007e..d509fc392 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -173,6 +173,7 @@ Rails.application.routes.draw do resources :lettings_logs, path: "/lettings-logs" do get "delete-confirmation", to: "lettings_logs#delete_confirmation" get "duplicate-logs", to: "duplicate_logs#show" + get "delete-duplicates", to: "duplicate_logs#delete_duplicates" collection do post "bulk-upload", to: "bulk_upload#bulk_upload" @@ -239,6 +240,7 @@ Rails.application.routes.draw do resources :sales_logs, path: "/sales-logs" do get "delete-confirmation", to: "sales_logs#delete_confirmation" get "duplicate-logs", to: "duplicate_logs#show" + get "delete-duplicates", to: "duplicate_logs#delete_duplicates" collection do get "csv-download", to: "sales_logs#download_csv" diff --git a/spec/factories/lettings_log.rb b/spec/factories/lettings_log.rb index 9cf6232d4..ae203b79e 100644 --- a/spec/factories/lettings_log.rb +++ b/spec/factories/lettings_log.rb @@ -39,6 +39,8 @@ FactoryBot.define do needstype { 1 } tenancycode { "same tenancy code" } postcode_full { "A1 1AA" } + uprn_known { 0 } + declaration { 1 } age1 { 18 } sex1 { "M" } ecstat1 { 0 } diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index dbfc8e418..54687c928 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -46,6 +46,7 @@ FactoryBot.define do ecstat1 { 1 } postcode_full { "A1 1AA" } privacynotice { 1 } + uprn_known { 0 } end trait :completed do purchid { rand(999_999_999).to_s } diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index f74091f5b..da1f89a23 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -434,5 +434,65 @@ RSpec.describe "Lettings Log Features" do expect(page).to have_current_path("/lettings-logs") end end + + context "when a log becomes a duplicate" do + let(:lettings_log) { create(:lettings_log, :duplicate, owning_organisation: user.organisation, created_by: user) } + let!(:duplicate_log) { create(:lettings_log, :duplicate, owning_organisation: user.organisation, created_by: user) } + + before do + lettings_log.update!(tenancycode: "different") + visit("/lettings-logs/#{lettings_log.id}/tenant-code") + fill_in("lettings-log-tenancycode-field", with: duplicate_log.tenancycode) + click_button("Save and continue") + end + + it "allows keeping the original log and deleting duplicates" do + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + click_link("Keep this log and delete duplicates", href: "/lettings-logs/#{lettings_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + click_button "Delete this log" + duplicate_log.reload + expect(duplicate_log.deleted?).to be true + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + expect(page).to have_content("Log #{duplicate_log.id} has been deleted.") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + expect(page).not_to have_content("These logs are duplicates") + expect(page).not_to have_link("Keep this log and delete duplicates") + expect(page).to have_link("Back to Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}") + end + + it "allows changing answers on remaining original log" do + click_link("Keep this log and delete duplicates", href: "/lettings-logs/#{lettings_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + click_button "Delete this log" + click_link("Change", href: "/lettings-logs/#{lettings_log.id}/tenant-code?original_log_id=#{lettings_log.id}&referrer=interruption_screen") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + expect(page).to have_link("Back to Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}") + end + + it "allows keeping the duplicate log and deleting the original one" do + expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + click_link("Keep this log and delete duplicates", href: "/lettings-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + expect(page).to have_current_path("/lettings-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + click_button "Delete this log" + lettings_log.reload + expect(lettings_log.status).to eq("deleted") + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + expect(page).to have_content("Log #{lettings_log.id} has been deleted.") + expect(page).to have_current_path("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + expect(page).not_to have_content("These logs are duplicates") + expect(page).not_to have_link("Keep this log and delete duplicates") + expect(page).to have_link("Back to lettings logs", href: "/lettings-logs") + end + + it "allows changing answers to remaining duplicate log" do + click_link("Keep this log and delete duplicates", href: "/lettings-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + click_button "Delete this log" + click_link("Change", href: "/lettings-logs/#{duplicate_log.id}/tenant-code?original_log_id=#{lettings_log.id}&referrer=interruption_screen") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") + expect(page).to have_link("Back to lettings logs", href: "/lettings-logs") + end + end end end diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb index 9a2fec455..e88ecd0e0 100644 --- a/spec/features/sales_log_spec.rb +++ b/spec/features/sales_log_spec.rb @@ -182,4 +182,67 @@ RSpec.describe "Sales Log Features" do end end end + + context "when a log becomes a duplicate" do + let(:user) { create(:user, :data_coordinator) } + let(:sales_log) { create(:sales_log, :duplicate, created_by: user) } + let!(:duplicate_log) { create(:sales_log, :duplicate, created_by: user) } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + sales_log.update!(purchid: "different") + visit("/sales-logs/#{sales_log.id}/purchaser-code") + fill_in("sales-log-purchid-field", with: duplicate_log.purchid) + click_button("Save and continue") + end + + it "allows keeping the original log and deleting duplicates" do + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + click_link("Keep this log and delete duplicates", href: "/sales-logs/#{sales_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + click_button "Delete this log" + duplicate_log.reload + expect(duplicate_log.deleted?).to be true + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + expect(page).to have_content("Log #{duplicate_log.id} has been deleted.") + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + expect(page).not_to have_content("These logs are duplicates") + expect(page).not_to have_link("Keep this log and delete duplicates") + expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}") + end + + it "allows changing answer on remaining original log" do + click_link("Keep this log and delete duplicates", href: "/sales-logs/#{sales_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + click_button "Delete this log" + click_link("Change", href: "/sales-logs/#{sales_log.id}/purchaser-code?original_log_id=#{sales_log.id}&referrer=interruption_screen") + click_button("Save and continue") + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + expect(page).to have_link("Back to Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}") + end + + it "allows keeping the duplicate log and deleting the original one" do + expect(page).to have_current_path("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + click_link("Keep this log and delete duplicates", href: "/sales-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + expect(page).to have_current_path("/sales-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + click_button "Delete this log" + sales_log.reload + expect(sales_log.status).to eq("deleted") + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + expect(page).to have_content("Log #{sales_log.id} has been deleted.") + expect(page).to have_current_path("/sales-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + expect(page).not_to have_content("These logs are duplicates") + expect(page).not_to have_link("Keep this log and delete duplicates") + expect(page).to have_link("Back to sales logs", href: "/sales-logs") + end + + it "allows changing answers on remaining duplicate log" do + click_link("Keep this log and delete duplicates", href: "/sales-logs/#{duplicate_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + click_button "Delete this log" + click_link("Change", href: "/sales-logs/#{duplicate_log.id}/purchaser-code?original_log_id=#{sales_log.id}&referrer=interruption_screen") + click_button("Save and continue") + expect(page).to have_current_path("/sales-logs/#{duplicate_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") + expect(page).to have_link("Back to sales logs", href: "/sales-logs") + end + end end diff --git a/spec/requests/delete_logs_controller_spec.rb b/spec/requests/delete_logs_controller_spec.rb index 9a90a9d87..5d3f703bf 100644 --- a/spec/requests/delete_logs_controller_spec.rb +++ b/spec/requests/delete_logs_controller_spec.rb @@ -230,7 +230,7 @@ RSpec.describe "DeleteLogs", type: :request do expect(response).to redirect_to lettings_logs_path follow_redirect! expect(page).to have_selector(".govuk-notification-banner--success") - expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted") + expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted.") end end @@ -470,7 +470,7 @@ RSpec.describe "DeleteLogs", type: :request do expect(response).to redirect_to sales_logs_path follow_redirect! expect(page).to have_selector(".govuk-notification-banner--success") - expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted") + expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted.") end end @@ -714,7 +714,7 @@ RSpec.describe "DeleteLogs", type: :request do expect(response).to redirect_to lettings_logs_organisation_path(id: organisation) follow_redirect! expect(page).to have_selector(".govuk-notification-banner--success") - expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted") + expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted.") end end @@ -939,7 +939,7 @@ RSpec.describe "DeleteLogs", type: :request do expect(response).to redirect_to sales_logs_organisation_path(id: organisation) follow_redirect! expect(page).to have_selector(".govuk-notification-banner--success") - expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted") + expect(page).to have_selector(".govuk-notification-banner--success", text: "2 logs have been deleted.") end end end diff --git a/spec/requests/duplicate_logs_controller_spec.rb b/spec/requests/duplicate_logs_controller_spec.rb index 3ba94c218..1de8af9ae 100644 --- a/spec/requests/duplicate_logs_controller_spec.rb +++ b/spec/requests/duplicate_logs_controller_spec.rb @@ -2,110 +2,251 @@ require "rails_helper" RSpec.describe DuplicateLogsController, type: :request do let(:page) { Capybara::Node::Simple.new(response.body) } - let(:user) { create(:user) } - - context "when a user is signed in" do - let(:lettings_log) do - create( - :lettings_log, - :completed, - created_by: user, - ) + let(:user) { create(:user, :data_coordinator) } + let(:lettings_log) { create(:lettings_log, :duplicate, created_by: user) } + let(:sales_log) { create(:sales_log, :duplicate, created_by: user) } + + describe "GET show" do + context "when user is not signed in" do + it "redirects to sign in page" do + get "/lettings-logs/#{lettings_log.id}/duplicate-logs" + expect(response).to redirect_to("/account/sign-in") + end end - let(:sales_log) do - create( - :sales_log, - :completed, - created_by: user, - ) + + context "when the user is from different organisation" do + let(:other_user) { create(:user) } + + before do + allow(other_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in other_user + end + + it "renders page not found" do + get "/lettings-logs/#{lettings_log.id}/duplicate-logs" + expect(response).to have_http_status(:not_found) + end end - describe "GET" do - context "when user is not signed in" do - it "redirects to sign in page" do - get "/lettings-logs/#{lettings_log.id}/duplicate-logs" - expect(response).to redirect_to("/account/sign-in") - end + context "when user is signed in" do + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user end - context "when the user is from different organisation" do - let(:other_user) { create(:user) } + context "with multiple duplicate lettings logs" do + let(:duplicate_logs) { create_list(:lettings_log, 2, :completed) } before do - allow(other_user).to receive(:need_two_factor_authentication?).and_return(false) - sign_in other_user + allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs) + get "/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}" + end + + it "displays links to all the duplicate logs" do + expect(page).to have_link("Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}") + expect(page).to have_link("Log #{duplicate_logs.first.id}", href: "/lettings-logs/#{duplicate_logs.first.id}") + expect(page).to have_link("Log #{duplicate_logs.second.id}", href: "/lettings-logs/#{duplicate_logs.second.id}") + end + + it "displays check your answers for each log with correct questions" do + expect(page).to have_content("Q5 - Tenancy start date", count: 3) + expect(page).to have_content("Q7 - Tenant code", count: 3) + expect(page).to have_content("Q12 - Postcode", count: 3) + expect(page).to have_content("Q32 - Lead tenant’s age", count: 3) + expect(page).to have_content("Q33 - Lead tenant’s gender identity", count: 3) + expect(page).to have_content("Q37 - Lead tenant’s working situation", count: 3) + expect(page).to have_content("Household rent and charges", count: 3) + expect(page).to have_link("Change", count: 21) end - it "renders page not found" do - get "/lettings-logs/#{lettings_log.id}/duplicate-logs" - expect(response).to have_http_status(:not_found) + it "displays buttons to delete" do + expect(page).to have_link("Keep this log and delete duplicates", count: 3) + expect(page).to have_link("Keep this log and delete duplicates", href: "/lettings-logs/#{lettings_log.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + expect(page).to have_link("Keep this log and delete duplicates", href: "/lettings-logs/#{duplicate_logs.first.id}/delete-duplicates?original_log_id=#{lettings_log.id}") + expect(page).to have_link("Keep this log and delete duplicates", href: "/lettings-logs/#{duplicate_logs.second.id}/delete-duplicates?original_log_id=#{lettings_log.id}") end end - context "when user is signed in" do + context "with multiple duplicate sales logs" do + let(:duplicate_logs) { create_list(:sales_log, 2, :completed) } + before do - allow(user).to receive(:need_two_factor_authentication?).and_return(false) - sign_in user + allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs) + get "/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}" end - context "with multiple duplicate lettings logs" do - let(:duplicate_logs) { create_list(:lettings_log, 2, :completed) } - - before do - allow(LettingsLog).to receive(:duplicate_logs).and_return(duplicate_logs) - get "/lettings-logs/#{lettings_log.id}/duplicate-logs" - end - - it "displays links to all the duplicate logs" do - expect(page).to have_link("Log #{lettings_log.id}", href: "/lettings-logs/#{lettings_log.id}") - expect(page).to have_link("Log #{duplicate_logs.first.id}", href: "/lettings-logs/#{duplicate_logs.first.id}") - expect(page).to have_link("Log #{duplicate_logs.second.id}", href: "/lettings-logs/#{duplicate_logs.second.id}") - end - - it "displays check your answers for each log with correct questions" do - expect(page).to have_content("Q5 - Tenancy start date", count: 3) - expect(page).to have_content("Q7 - Tenant code", count: 3) - expect(page).to have_content("Q12 - Postcode", count: 3) - expect(page).to have_content("Q32 - Lead tenant’s age", count: 3) - expect(page).to have_content("Q33 - Lead tenant’s gender identity", count: 3) - expect(page).to have_content("Q37 - Lead tenant’s working situation", count: 3) - expect(page).to have_content("Household rent and charges", count: 3) - expect(page).to have_link("Change", count: 21) - end - - it "displays buttons to delete" do - expect(page).to have_link("Keep this log and delete duplicates", count: 3) - end + it "displays links to all the duplicate logs" do + expect(page).to have_link("Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}") + expect(page).to have_link("Log #{duplicate_logs.first.id}", href: "/sales-logs/#{duplicate_logs.first.id}") + expect(page).to have_link("Log #{duplicate_logs.second.id}", href: "/sales-logs/#{duplicate_logs.second.id}") end - context "with multiple duplicate sales logs" do - let(:duplicate_logs) { create_list(:sales_log, 2, :completed) } - - before do - allow(SalesLog).to receive(:duplicate_logs).and_return(duplicate_logs) - get "/sales-logs/#{sales_log.id}/duplicate-logs" - end - - it "displays links to all the duplicate logs" do - expect(page).to have_link("Log #{sales_log.id}", href: "/sales-logs/#{sales_log.id}") - expect(page).to have_link("Log #{duplicate_logs.first.id}", href: "/sales-logs/#{duplicate_logs.first.id}") - expect(page).to have_link("Log #{duplicate_logs.second.id}", href: "/sales-logs/#{duplicate_logs.second.id}") - end - - it "displays check your answers for each log with correct questions" do - expect(page).to have_content("Q1 - Sale completion date", count: 3) - expect(page).to have_content("Q2 - Purchaser code", count: 3) - expect(page).to have_content("Q20 - Lead buyer’s age", count: 3) - expect(page).to have_content("Q21 - Buyer 1’s gender identity", count: 3) - expect(page).to have_content("Q25 - Buyer 1's working situation", count: 3) - expect(page).to have_content("Q15 - Postcode", count: 3) - expect(page).to have_link("Change", count: 18) - end - - it "displays buttons to delete" do - expect(page).to have_link("Keep this log and delete duplicates", count: 3) - end + it "displays check your answers for each log with correct questions" do + expect(page).to have_content("Q1 - Sale completion date", count: 3) + expect(page).to have_content("Q2 - Purchaser code", count: 3) + expect(page).to have_content("Q20 - Lead buyer’s age", count: 3) + expect(page).to have_content("Q21 - Buyer 1’s gender identity", count: 3) + expect(page).to have_content("Q25 - Buyer 1's working situation", count: 3) + expect(page).to have_content("Q15 - Postcode", count: 3) + expect(page).to have_link("Change", count: 18) end + + it "displays buttons to delete" do + expect(page).to have_link("Keep this log and delete duplicates", count: 3) + expect(page).to have_link("Keep this log and delete duplicates", href: "/sales-logs/#{sales_log.id}/delete-duplicates?original_log_id=#{sales_log.id}") + expect(page).to have_link("Keep this log and delete duplicates", href: "/sales-logs/#{duplicate_logs.first.id}/delete-duplicates?original_log_id=#{sales_log.id}") + expect(page).to have_link("Keep this log and delete duplicates", href: "/sales-logs/#{duplicate_logs.second.id}/delete-duplicates?original_log_id=#{sales_log.id}") + end + end + end + end + + describe "GET sales delete-duplicates" do + let(:headers) { { "Accept" => "text/html" } } + let(:id) { sales_log.id } + let(:request) { get "/sales-logs/#{id}/delete-duplicates?original_log_id=#{id}" } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + context "when there are no duplicate logs" do + it "renders not found" do + request + expect(response).to have_http_status(:not_found) + end + end + + context "when there is 1 duplicate log being deleted" do + let!(:duplicate_log) { create(:sales_log, :duplicate, created_by: user) } + + it "renders page" do + request + expect(response).to have_http_status(:ok) + + expect(page).to have_content("Are you sure you want to delete this duplicate log?") + expect(page).to have_button(text: "Delete this log") + expect(page).to have_link(text: "Log #{duplicate_log.id}", href: sales_log_path(duplicate_log.id)) + expect(page).not_to have_link(text: "Log #{id}", href: sales_log_path(id)) + expect(page).to have_link(text: "Cancel", href: sales_log_duplicate_logs_path(id)) + expect(page).to have_link(text: "Back", href: sales_log_duplicate_logs_path(id)) + end + end + + context "when there are multiple duplicate logs being deleted" do + let!(:duplicate_log) { create(:sales_log, :duplicate, created_by: user) } + let!(:duplicate_log_2) { create(:sales_log, :duplicate, created_by: user) } + + it "renders page" do + request + expect(response).to have_http_status(:ok) + + expect(page).to have_content("Are you sure you want to delete these duplicate logs?") + expect(page).to have_content("These logs will be deleted:") + expect(page).to have_button(text: "Delete these logs") + expect(page).to have_link(text: "Log #{duplicate_log.id}", href: sales_log_path(duplicate_log.id)) + expect(page).to have_link(text: "Log #{duplicate_log_2.id}", href: sales_log_path(duplicate_log_2.id)) + expect(page).to have_link(text: "Cancel", href: sales_log_duplicate_logs_path(id)) + expect(page).to have_link(text: "Back", href: sales_log_duplicate_logs_path(id)) + end + end + + context "when log does not exist" do + let(:id) { -1 } + + it "returns 404" do + request + expect(response).to have_http_status(:not_found) + end + end + + context "when user is not authorised" do + let(:other_user) { create(:user) } + + before do + allow(other_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in other_user + end + + it "returns 404" do + request + expect(response).to have_http_status(:not_found) + end + end + end + + describe "GET lettings delete-duplicates" do + let(:id) { lettings_log.id } + let(:request) { get "/lettings-logs/#{id}/delete-duplicates?original_log_id=#{id}" } + + before do + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + end + + context "when there are no duplicate logs" do + it "renders page not found" do + request + expect(response).to have_http_status(:not_found) + end + end + + context "when there is 1 duplicate log being deleted" do + let!(:duplicate_log) { create(:lettings_log, :duplicate, created_by: user) } + + it "renders page" do + request + expect(response).to have_http_status(:ok) + + expect(page).to have_content("Are you sure you want to delete this duplicate log?") + expect(page).to have_content("This log will be deleted:") + expect(page).to have_button(text: "Delete this log") + expect(page).to have_link(text: "Log #{duplicate_log.id}", href: lettings_log_path(duplicate_log.id)) + expect(page).not_to have_link(text: "Log #{id}", href: lettings_log_path(id)) + expect(page).to have_link(text: "Cancel", href: lettings_log_duplicate_logs_path(id)) + expect(page).to have_link(text: "Back", href: lettings_log_duplicate_logs_path(id)) + end + end + + context "when there are multiple duplicate logs being deleted" do + let!(:duplicate_log) { create(:lettings_log, :duplicate, created_by: user) } + let!(:duplicate_log_2) { create(:lettings_log, :duplicate, created_by: user) } + + it "renders page" do + request + expect(response).to have_http_status(:ok) + + expect(page).to have_content("Are you sure you want to delete these duplicate logs?") + expect(page).to have_content("These logs will be deleted:") + expect(page).to have_button(text: "Delete these logs") + expect(page).to have_link(text: "Log #{duplicate_log.id}", href: lettings_log_path(duplicate_log.id)) + expect(page).to have_link(text: "Log #{duplicate_log_2.id}", href: lettings_log_path(duplicate_log_2.id)) + expect(page).to have_link(text: "Cancel", href: lettings_log_duplicate_logs_path(id)) + expect(page).to have_link(text: "Back", href: lettings_log_duplicate_logs_path(id)) + end + end + + context "when log does not exist" do + let(:id) { -1 } + + it "returns 404" do + request + expect(response).to have_http_status(:not_found) + end + end + + context "when user is not authorised" do + let(:other_user) { create(:user) } + + before do + allow(other_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in other_user + end + + it "returns 404" do + request + expect(response).to have_http_status(:not_found) end end end diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 78ca078ca..8f16efcca 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -568,7 +568,7 @@ RSpec.describe FormController, type: :request do end it "redirects to the duplicate logs page" do - expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs") + expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/duplicate-logs?original_log_id=#{lettings_log.id}") follow_redirect! expect(page).to have_content("These logs are duplicates") end @@ -601,7 +601,7 @@ RSpec.describe FormController, type: :request do end it "redirects to the duplicate logs page" do - expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs") + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/duplicate-logs?original_log_id=#{sales_log.id}") follow_redirect! expect(page).to have_content("These logs are duplicates") end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 688639339..e6faf2b57 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -1432,7 +1432,7 @@ RSpec.describe LettingsLogsController, type: :request do delete_request expect(response).to redirect_to(lettings_logs_path) follow_redirect! - expect(page).to have_content("Log #{id} has been deleted") + expect(page).to have_content("Log #{id} has been deleted.") end it "marks the log as deleted" do diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index d481752b3..cba885d55 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -904,7 +904,7 @@ RSpec.describe SalesLogsController, type: :request do delete_request expect(response).to redirect_to(sales_logs_path) follow_redirect! - expect(page).to have_content("Log #{id} has been deleted") + expect(page).to have_content("Log #{id} has been deleted.") end it "marks the log as deleted" do