Add letting log delete flow

app/controllers/lettings_logs_controller.rb

@@ -1,5 +1,5 @@
 class LettingsLogsController < LogsController
-  before_action :find_resource, except: %i[create index edit]
+  before_action :find_resource, except: %i[create index edit delete_confirmation]
   before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :authenticate_scope!, only: %i[download_csv email_csv]
@@ -68,17 +68,27 @@ class LettingsLogsController < LogsController
   end
 
   def destroy
-    if @log
+    render_not_found and return unless @log
-      if @log.delete
+
-        head :no_content
+    authorize @log, policy_class: LogPolicy
-      else
+
-        render json: { errors: @log.errors.messages }, status: :unprocessable_entity
+    if @log.delete
-      end
+      redirect_to lettings_logs_path, notice: "Log #{@log.id} has been deleted"
     else
-      render_not_found_json("Log", params[:id])
+      render_not_found
     end
   end
 
+  def delete_confirmation
+    @log = LettingsLog.visible.find_by(id: params[:lettings_log_id])
+
+    render_not_found and return unless @log
+
+    authorize @log, :destroy?, policy_class: LogPolicy
+
+    render "logs/delete_confirmation"
+  end
+
   def download_csv
     unpaginated_filtered_logs = filtered_logs(current_user.lettings_logs, search_term, @session_filters)
 

app/views/logs/delete_confirmation.html.erb

@@ -0,0 +1,31 @@
+<% content_for :before_content do %>
+  <% content_for :title, "Are you sure you want to delete this log?" %>
+  <%= govuk_back_link href: @log.lettings? ? lettings_logs_path : sales_logs_path %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <span class="govuk-caption-xl">Delete log <%= @log.id %></span>
+    <h1 class="govuk-heading-xl">
+      <%= content_for(:title) %>
+    </h1>
+
+    <%= govuk_warning_text(text: "You will not be able to undo this action.") %>
+
+    <div class="govuk-button-group">
+      <%= govuk_button_to(
+        "Delete this log",
+        @log.lettings? ? lettings_log_path(@log) : sales_log_path(@log),
+        class: "govuk-!-margin-right-6",
+        method: :delete
+      ) %>
+      <%= govuk_button_to(
+        "Cancel",
+        @log.lettings? ? lettings_log_path(@log) : sales_log_path(@log),
+        button: true,
+        method: :get,
+        secondary: true,
+      ) %>
+    </div>
+  </div>
+</div>

app/views/logs/edit.html.erb

@@ -51,5 +51,15 @@
         <%= govuk_button_link_to "Back to sales logs", sales_logs_path %>
       <% end %>
     <% end %>
+
+    <% if LogPolicy.new(current_user, @log).destroy? %>
+      <%= govuk_button_to(
+        "Delete log",
+        lettings_log_delete_confirmation_path(@log),
+        button: true,
+        method: :get,
+        warning: true,
+      ) %>
+    <% end %>
   </div>
 </div>

config/routes.rb

@@ -150,6 +150,8 @@ Rails.application.routes.draw do
   end
 
   resources :lettings_logs, path: "/lettings-logs" do
+    get "delete-confirmation", to: "lettings_logs#delete_confirmation"
+
     collection do
       post "bulk-upload", to: "bulk_upload#bulk_upload"
       get "bulk-upload", to: "bulk_upload#show"

spec/requests/lettings_logs_controller_spec.rb

@@ -1309,55 +1309,91 @@ RSpec.describe LettingsLogsController, type: :request do
   end
 
   describe "DELETE" do
+    let(:headers) { { "Accept" => "text/html" } }
+    let(:page) { Capybara::Node::Simple.new(response.body) }
+    let(:user) { create(:user, :support) }
     let!(:lettings_log) do
-      FactoryBot.create(:lettings_log, :in_progress)
+      create(:lettings_log, :completed)
     end
     let(:id) { lettings_log.id }
+    let(:delete_request) { delete "/lettings-logs/#{id}", headers: }
 
-    context "when deleting a lettings log" do
+    before do
-      before do
+      allow(user).to receive(:need_two_factor_authentication?).and_return(false)
-        delete "/lettings-logs/#{id}", headers:
+      sign_in user
-      end
+    end
 
-      it "returns http success" do
+    context "when delete permitted" do
-        expect(response).to have_http_status(:success)
+      it "redirects to lettings logs and shows message" do
+        delete_request
+        expect(response).to redirect_to(lettings_logs_path)
+        follow_redirect!
+        expect(page).to have_content("Log #{id} has been deleted")
       end
 
-      it "deletes the lettings log" do
+      it "deletes the log" do
-        expect { LettingsLog.find(id) }.to raise_error(ActiveRecord::RecordNotFound)
+        expect { delete_request }.to change { LettingsLog.exists?(id) }.from(true).to(false)
       end
+    end
 
-      context "with an invalid lettings log id" do
+    context "when log does not exist" do
-        let(:id) { (LettingsLog.order(:id).last&.id || 0) + 1 }
+      let(:id) { -1 }
 
-        it "returns 404" do
+      it "returns 404" do
-          expect(response).to have_http_status(:not_found)
+        delete_request
-        end
+        expect(response).to have_http_status(:not_found)
       end
+    end
 
-      context "with a request containing invalid credentials" do
+    context "when user not authorised" do
-        let(:basic_credentials) do
+      let(:user) { create(:user) }
-          ActionController::HttpAuthentication::Basic.encode_credentials(api_username, "Oops")
-        end
 
-        it "returns 401" do
+      it "returns 404" do
-          expect(response).to have_http_status(:unauthorized)
+        delete_request
-        end
+        expect(response).to have_http_status(:unauthorized)
       end
     end
+  end
 
-    context "when a lettings log deletion fails" do
+  describe "GET delete-confirmation" do
-      let(:mock_scope) { instance_double("LettingsLog::ActiveRecord_Relation", find_by: lettings_log) }
+    let(:headers) { { "Accept" => "text/html" } }
+    let(:page) { Capybara::Node::Simple.new(response.body) }
+    let(:user) { create(:user, :support) }
+    let!(:lettings_log) do
+      create(:lettings_log, :completed)
+    end
+    let(:id) { lettings_log.id }
+    let(:request) { get "/lettings-logs/#{id}/delete-confirmation", headers: }
 
-      before do
+    before do
-        allow(LettingsLog).to receive(:visible).and_return(mock_scope)
+      allow(user).to receive(:need_two_factor_authentication?).and_return(false)
-        allow(lettings_log).to receive(:delete).and_return(false)
+      sign_in user
+    end
 
-        delete "/lettings-logs/#{id}", headers:
+    context "when delete permitted" do
+      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 log?")
       end
+    end
 
-      it "returns an unprocessable entity 422" do
+    context "when log does not exist" do
-        expect(response).to have_http_status(:unprocessable_entity)
+      let(:id) { -1 }
+
+      it "returns 404" do
+        request
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
+    context "when user not authorised" do
+      let(:user) { create(:user) }
+
+      it "returns 404" do
+        request
+        expect(response).to have_http_status(:unauthorized)
       end
     end
   end