rebase changes

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",

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

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

app/views/logs/delete_duplicates.html.erb

@@ -18,7 +18,7 @@
       <% @duplicate_logs.each do |duplicate_log| %>
         <li>
           <strong>
-            <%= 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) %>
           </strong>
         </li>
       <% end %>

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

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

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