diff --git a/app/controllers/duplicate_logs_controller.rb b/app/controllers/duplicate_logs_controller.rb index acea5a158..f708691c0 100644 --- a/app/controllers/duplicate_logs_controller.rb +++ b/app/controllers/duplicate_logs_controller.rb @@ -1,6 +1,7 @@ class DuplicateLogsController < ApplicationController before_action :authenticate_user! before_action :find_resource_by_named_id + before_action :find_duplicate_logs def show if @log @@ -19,6 +20,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 +36,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", diff --git a/app/policies/lettings_log_policy.rb b/app/policies/lettings_log_policy.rb index cd81c184d..d9ac8a845 100644 --- a/app/policies/lettings_log_policy.rb +++ b/app/policies/lettings_log_policy.rb @@ -21,8 +21,4 @@ class LettingsLogPolicy # Data providers can only delete the log if it is assigned to them log.created_by == user end - - def delete_duplicates? - user.support? || log.owning_organisation == user.organisation || log.managing_organisation == user.organisation - end end diff --git a/app/policies/sales_log_policy.rb b/app/policies/sales_log_policy.rb index 86069a818..de34527ff 100644 --- a/app/policies/sales_log_policy.rb +++ b/app/policies/sales_log_policy.rb @@ -21,8 +21,4 @@ class SalesLogPolicy # Data providers can only delete the log if it is assigned to them log.created_by == user end - - def delete_duplicates? - user.support? || log.owning_organisation == user.organisation - end end diff --git a/app/views/logs/delete_duplicates.html.erb b/app/views/logs/delete_duplicates.html.erb index c4aebe946..b9a678c3f 100644 --- a/app/views/logs/delete_duplicates.html.erb +++ b/app/views/logs/delete_duplicates.html.erb @@ -18,7 +18,7 @@ <% @duplicate_logs.each do |duplicate_log| %>
  • - <%= govuk_link_to "Log #{duplicate_log.id}", duplicate_log.lettings? ? lettings_log_path(duplicate_log) : sales_log_path(duplicate_log) %> + <%= govuk_link_to "Log #{duplicate_log.id}", url_for(duplicate_log) %>
  • <% end %> diff --git a/spec/requests/duplicate_logs_controller_spec.rb b/spec/requests/duplicate_logs_controller_spec.rb index dc0b173f4..bf7892b83 100644 --- a/spec/requests/duplicate_logs_controller_spec.rb +++ b/spec/requests/duplicate_logs_controller_spec.rb @@ -205,5 +205,193 @@ RSpec.describe DuplicateLogsController, type: :request do end end end + + describe "GET sales delete-duplicates" do + let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { create(:user, :data_coordinator) } + let!(:sales_log) do + create(:sales_log, :completed, owning_organisation: user.organisation) + end + let(:id) { sales_log.id } + + let(:request) { get "/sales-logs/#{id}/delete-duplicates", headers: } + + 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) do + duplicate = sales_log.dup + duplicate.id = nil + duplicate.save! + duplicate + end + + 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_path(id)) # update with correct path when known + end + end + + context "when there are multiple duplicate logs being deleted" do + let!(:duplicate_log) do + duplicate = sales_log.dup + duplicate.id = nil + duplicate.save! + duplicate + end + let!(:duplicate_log_2) do + duplicate = sales_log.dup + duplicate.id = nil + duplicate.save! + duplicate + end + + 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_path(id)) # update with correct path when known + 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(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + let(:user) { create(:user, :data_coordinator) } + let!(:lettings_log) do + create(:lettings_log, :completed, owning_organisation: user.organisation) + end + let(:id) { lettings_log.id } + let(:request) { get "/lettings-logs/#{id}/delete-duplicates", headers: } + + 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) do + duplicate = lettings_log.dup + duplicate.id = nil + duplicate.save! + duplicate + end + + 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_path(id)) # update with correct path when known + end + end + + context "when there are multiple duplicate logs being deleted" do + let!(:duplicate_log) do + duplicate = lettings_log.dup + duplicate.id = nil + duplicate.save! + duplicate + end + let!(:duplicate_log_2) do + duplicate = lettings_log.dup + duplicate.id = nil + duplicate.save! + duplicate + end + + 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_path(id)) # update with correct path when known + 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 end end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 72883dac9..2fc1ef461 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -1667,98 +1667,4 @@ RSpec.describe LettingsLogsController, type: :request do end end end - - describe "GET delete-duplicates" do - let(:headers) { { "Accept" => "text/html" } } - let(:page) { Capybara::Node::Simple.new(response.body) } - let(:user) { create(:user, :data_coordinator) } - let!(:lettings_log) do - create(:lettings_log, :completed, owning_organisation: user.organisation) - end - let(:id) { lettings_log.id } - let(:request) { get "/lettings-logs/#{id}/delete-duplicates", headers: } - - 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) do - duplicate = lettings_log.dup - duplicate.id = nil - duplicate.save! - duplicate - end - - 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_path(id)) # update with correct path when known - end - end - - context "when there are multiple duplicate logs being deleted" do - let!(:duplicate_log) do - duplicate = lettings_log.dup - duplicate.id = nil - duplicate.save! - duplicate - end - let!(:duplicate_log_2) do - duplicate = lettings_log.dup - duplicate.id = nil - duplicate.save! - duplicate - end - - 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_path(id)) # update with correct path when known - 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(:unauthorized) - end - end - end end diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 904294b5f..5d24e6b01 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -872,98 +872,4 @@ RSpec.describe SalesLogsController, type: :request do end end end - - describe "GET delete-duplicates" do - let(:headers) { { "Accept" => "text/html" } } - let(:page) { Capybara::Node::Simple.new(response.body) } - let(:user) { create(:user, :data_coordinator) } - let!(:sales_log) do - create(:sales_log, :completed, owning_organisation: user.organisation) - end - let(:id) { sales_log.id } - - let(:request) { get "/sales-logs/#{id}/delete-duplicates", headers: } - - 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) do - duplicate = sales_log.dup - duplicate.id = nil - duplicate.save! - duplicate - end - - 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_path(id)) # update with correct path when known - end - end - - context "when there are multiple duplicate logs being deleted" do - let!(:duplicate_log) do - duplicate = sales_log.dup - duplicate.id = nil - duplicate.save! - duplicate - end - let!(:duplicate_log_2) do - duplicate = sales_log.dup - duplicate.id = nil - duplicate.save! - duplicate - end - - 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_path(id)) # update with correct path when known - 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(:unauthorized) - end - end - end end