comprehensive reworking after code review

ensure that we are not passing lists of ids through params in the query string, risking overflowing the maximum URL length,
adjust tests accordingly,
do not attempt to reuse the same table for sales and lettings

app/controllers/lettings_logs_controller.rb

@@ -16,7 +16,7 @@ class LettingsLogsController < LogsController
         all_logs = current_user.lettings_logs.visible
         unpaginated_filtered_logs = filter_manager.filtered_logs(all_logs, search_term, session_filters)
 
-        @search_term = search_term
+        @delete_logs_path = delete_logs_lettings_logs_path(search: search_term)
         @pagy, @logs = pagy(unpaginated_filtered_logs)
         @searched = search_term.presence
         @total_count = all_logs.size
@@ -112,6 +112,41 @@ class LettingsLogsController < LogsController
     end
   end
 
+  def delete_lettings_logs
+    @delete_logs_form = delete_logs_form
+  end
+
+  def delete_lettings_logs_with_selected_ids
+    selected_ids = params.require(:selected_ids).split.map(&:to_i)
+    @delete_logs_form = delete_logs_form(selected_ids:)
+    render :delete_lettings_logs
+  end
+
+  def delete_logs_confirmation
+    default_attributes = {
+      current_user:,
+      log_filters: @session_filters,
+      log_type: :lettings,
+    }
+    form_attributes = params.require(:forms_delete_logs_form).permit(:search_term, selected_ids: [])
+    attributes = form_attributes.merge(default_attributes)
+    attributes[:selected_ids] = [] unless attributes.key? :selected_ids
+    @delete_logs_form = Forms::DeleteLogsForm.new(attributes)
+    unless @delete_logs_form.valid?
+      render :delete_lettings_logs
+    end
+  end
+
+  def delete_logs
+    logs = LettingsLog.find(params.require(:ids))
+    logs.each do |log|
+      authorize log, :destroy?
+      log.discard!
+    end
+
+    redirect_to lettings_logs_path, notice: I18n.t("notification.logs_deleted", count: logs.count)
+  end
+
 private
 
   def session_filters
@@ -128,6 +163,10 @@ private
     )
   end
 
+  def delete_logs_form(selected_ids: nil)
+    Forms::DeleteLogsForm.new(current_user:, search_term:, log_filters: @session_filters, log_type: :lettings, selected_ids:)
+  end
+
   def authenticate_scope!
     head :unauthorized and return if codes_only_export? && !current_user.support?
   end

app/frontend/styles/application.scss

@@ -44,7 +44,6 @@ $govuk-breakpoints: (
 @import "search";
 @import "sub-navigation";
 @import "errors";
-@import "log-list";
 
 // App utilities
 .app-\!-colour-muted {

app/helpers/log_list_helper.rb

@@ -1,9 +1,9 @@
 module LogListHelper
-  def display_delete_logs?
-    if @current_user.data_provider?
+  def display_delete_logs?(current_user, search_term)
+    if current_user.data_provider?
       filter_selected?("user", "yours")
     else
-      any_filter_selected? || @searched.present?
+      any_filter_selected? || search_term.present?
     end
   end
 end

app/models/forms/delete_logs_form.rb

@@ -0,0 +1,38 @@
+module Forms
+  class DeleteLogsForm
+    include Modules::LogsFilter
+    include ActiveModel::Model
+    include ActiveModel::Validations
+
+    attr_reader :logs, :log_type, :selected_ids, :search_term
+
+    validate :at_least_one_log_selected
+
+    def initialize(attributes)
+      @log_type = attributes[:log_type]
+      @search_term = attributes[:search_term]
+      @current_user = attributes[:current_user]
+      @logs = FilterService.filter_logs(all_logs, @search_term, attributes[:log_filters], nil, @current_user)
+      @selected_ids = attributes[:selected_ids] || @logs.map(&:id)
+    end
+
+    def log_count
+      @logs.count
+    end
+
+  private
+
+    def at_least_one_log_selected
+      if selected_ids.blank? || selected_ids.reject(&:blank?).blank?
+        errors.add(:log_ids, "Select at least one log to delete or press cancel to return")
+      end
+    end
+
+    def all_logs
+      case @log_type
+      when :lettings then @current_user.lettings_logs.visible
+      when :sales then @current_user.sales_logs.visible
+      end
+    end
+  end
+end

app/views/logs/_log_list.html.erb

@@ -10,8 +10,8 @@
   <% end %>
 </div>
 <div class="govuk-grid-column-one-quarter govuk-!-text-align-right">
-  <% if display_delete_logs? %>
-    <%= govuk_link_to "Delete logs", "#", class: "app-!-colour-red" %>
+  <% if logs&.any? && display_delete_logs?(@current_user, @searched) %>
+    <%= govuk_link_to "Delete logs", delete_logs_path, class: "app-!-colour-red" %>
   <% end %>
 </div>
 </div>

app/views/logs/delete_lettings_logs.html.erb

@@ -0,0 +1,48 @@
+<% title = "Delete logs" %>
+<% content_for :title, title %>
+<% content_for :before_content do %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<h1 class="govuk-heading-l">
+  <span class="govuk-caption-l"><%= title %></span>
+  Review the logs you want to delete
+</h1>
+<p>You've selected <%= @delete_logs_form.log_count %> <%= "log".pluralize(@delete_logs_form.log_count) %> to delete</p>
+
+<%= form_with model: @delete_logs_form, url: delete_logs_confirmation_lettings_logs_path do |f| %>
+  <%= f.hidden_field :search_term, value: @delete_logs_form.search_term %>
+  <%= f.govuk_error_summary %>
+  <%= govuk_table do |table| %>
+    <% table.head do |head| %>
+      <% head.row do |row| %>
+        <% row.cell(header: true, text: "Log ID") %>
+        <% row.cell(header: true, text: "Tenancy code") %>
+        <% row.cell(header: true, text: "Property reference") %>
+        <% row.cell(header: true, text: "Status") %>
+        <% row.cell(header: true, text: "Delete?") %>
+      <% end %>
+    <% end %>
+    <% table.body do |body| %>
+      <% f.govuk_check_boxes_fieldset :selected_ids, small: true do %>
+        <% @delete_logs_form.logs.each do |log| %>
+          <% body.row do |row| %>
+            <% row.cell(text: log.id) %>
+            <% row.cell(text: log.tenancycode) %>
+            <% row.cell(text: log.propcode) %>
+            <% row.cell(text: status_tag(log.status)) %>
+            <% row.cell do %>
+              <% f.govuk_check_box :selected_ids, log.id,
+                label: { text: log.id, hidden: true },
+                size: "s",
+                checked: @delete_logs_form.selected_ids.include?(log.id) %>
+            <% end %>
+          <% end %>
+        <% end %>
+      <% end %>
+    <% end %>
+  <% end %>
+  <%= f.govuk_submit "Continue" do %>
+    <%= govuk_button_link_to "Cancel", lettings_logs_path(search: @delete_logs_form.search_term), secondary: true %>
+  <% end %>
+<% end %>

app/views/logs/delete_logs_confirmation.html.erb

@@ -0,0 +1,25 @@
+<% title = "Delete logs" %>
+<% content_for :title, title %>
+<% content_for :before_content do %>
+  <%= govuk_back_link(href: :back) %>
+<% end %>
+
+<h1 class="govuk-heading-l">
+  <span class="govuk-caption-l"><%= title %></span>
+  Are you sure you want to delete these logs?
+</h1>
+<% log_count = @delete_logs_form.selected_ids.count %>
+<p>You've selected <%= log_count %> <%= "log".pluralize(log_count) %> to delete</p>
+
+<%= govuk_warning_text(icon_fallback_text: "Danger") do %>
+  You will not be able to undo this action
+<% end %>
+
+<div class="govuk-button-group">
+  <%= govuk_button_to "Delete logs", delete_logs_lettings_logs_path, method: "delete", params: { ids: @delete_logs_form.selected_ids } %>
+  <%= form_with url: delete_logs_lettings_logs_path do |f| %>
+    <%= f.hidden_field :selected_ids, value: @delete_logs_form.selected_ids %>
+    <%= f.hidden_field :search, value: @delete_logs_form.search_term %>
+    <%= f.govuk_submit "Cancel", secondary: true %>
+  <% end %>
+</div>

app/views/logs/index.html.erb

@@ -68,8 +68,9 @@
                  searched: @searched,
                  item_label:,
                  total_count: @total_count,
-                 csv_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: false),
-                 csv_codes_only_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: true),
+                 csv_download_url: csv_download_url_for_controller(controller:, search: @searched, codes_only: false),
+                 csv_codes_only_download_url: csv_download_url_for_controller(controller:, search: @searched, codes_only: true),
+                 delete_logs_path: @delete_logs_path,
                } %>
     <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
   </div>

app/views/organisations/logs.html.erb

@@ -34,6 +34,7 @@
                  total_count: @total_count,
                  csv_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: false),
                  csv_codes_only_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: true),
+                 delete_logs_path: "#",
                } %>
     <%= render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
   </div>

config/locales/en.yml

@@ -175,6 +175,11 @@ en:
             new_organisation_telephone_number:
               blank: "Enter a valid telephone number"
 
+  notification:
+    logs_deleted:
+      one: "%{count} log has been deleted"
+      other: "%{count} logs have been deleted"
+
   validations:
     organisation:
       data_sharing_agreement_not_signed: Your organisation must accept the Data Sharing Agreement before you can create any logs.

config/routes.rb

@@ -170,6 +170,11 @@ Rails.application.routes.draw do
       post "email-csv", to: "lettings_logs#email_csv"
       get "csv-confirmation", to: "lettings_logs#csv_confirmation"
 
+      get "delete-logs", to: "lettings_logs#delete_lettings_logs"
+      post "delete-logs", to: "lettings_logs#delete_lettings_logs_with_selected_ids"
+      post "delete-logs-confirmation", to: "lettings_logs#delete_logs_confirmation"
+      delete "delete-logs", to: "lettings_logs#delete_logs"
+
       resources :bulk_upload_lettings_logs, path: "bulk-upload-logs", only: %i[show update] do
         collection do
           get :start
@@ -227,6 +232,10 @@ Rails.application.routes.draw do
       post "email-csv", to: "sales_logs#email_csv"
       get "csv-confirmation", to: "sales_logs#csv_confirmation"
 
+      get "delete-logs", to: "sales_logs#delete_logs_list"
+      get "delete-logs-confirmation", to: "sales_logs#delete_logs_confirmation"
+      delete "delete-logs", to: "sales_logs#delete_logs"
+
       resources :bulk_upload_sales_logs, path: "bulk-upload-logs" do
         collection do
           get :start

spec/features/lettings_log_spec.rb

@@ -245,13 +245,13 @@ RSpec.describe "Lettings Log Features" do
         expect(rows.count).to be 2
         id_to_delete, id_to_keep = rows.map { |row| row.first("td").text.to_i }
         expect([id_to_delete, id_to_keep]).to match_array [lettings_log_1.id, lettings_log_2.id]
-        check "forms-delete-logs-form-logs-to-delete-#{id_to_delete}-field"
-        uncheck "forms-delete-logs-form-logs-to-delete-#{id_to_keep}-field"
+        check "forms-delete-logs-form-selected-ids-#{id_to_delete}-field"
+        uncheck "forms-delete-logs-form-selected-ids-#{id_to_keep}-field"
         click_button "Continue"
 
-        expect(page).to have_current_path delete_logs_confirmation_lettings_logs_path, ignore_query: true
+        expect(page).to have_current_path delete_logs_confirmation_lettings_logs_path
         expect(page.text).to include "You've selected 1 log to delete"
-        expect(page.find("input#ids", visible: false).value.to_i).to be id_to_delete
+        expect(page.find("form.button_to")[:action]).to eq delete_logs_lettings_logs_path
         click_button "Delete logs"
 
         expect(page).to have_current_path lettings_logs_path

spec/requests/lettings_logs_controller_spec.rb

@@ -1845,21 +1845,35 @@ RSpec.describe LettingsLogsController, type: :request do
   describe "GET delete-logs" do
     let(:page) { Capybara::Node::Simple.new(response.body) }
     let(:user) { create(:user, name: "Richard MacDuff") }
-    let(:log_1) { create(:lettings_log, :in_progress) }
-    let(:log_2) { create(:lettings_log, :completed) }
+    let!(:log_1) { create(:lettings_log, :in_progress, created_by: user) }
+    let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
 
     before do
       allow(user).to receive(:need_two_factor_authentication?).and_return(false)
       sign_in user
-      create(:lettings_log, :in_progress)
+      allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
     end
 
-    it "requires log_ids to be provided" do
-      expect { get delete_logs_lettings_logs_path }.to raise_error(ActionController::ParameterMissing)
+    it "calls the filter service with the filters in the session and the search term from the query params" do
+      search = "Schrödinger's cat"
+      logs_filters = {
+        "years" => [""],
+        "status" => ["", "in_progress"],
+        "user" => "all",
+      }
+      get lettings_logs_path(logs_filters) # adds the filters to the session
+
+      expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
+        expect(arg1).to contain_exactly(log_1, log_2)
+        expect(arg2).to eq search
+        expect(arg3).to eq logs_filters
+      }.and_return LettingsLog.all
+
+      get delete_logs_lettings_logs_path(search:)
     end
 
-    it "displays the logs for the ids provided" do
-      get delete_logs_lettings_logs_path(log_ids: [log_1.id, log_2.id])
+    it "displays the logs returned by the filter service" do
+      get delete_logs_lettings_logs_path
 
       table_body_rows = page.find_all("tbody tr")
       expect(table_body_rows.count).to be 2
@@ -1868,14 +1882,60 @@ RSpec.describe LettingsLogsController, type: :request do
     end
 
     it "checks all checkboxes by default" do
-      get delete_logs_lettings_logs_path(log_ids: [log_1.id, log_2.id])
+      get delete_logs_lettings_logs_path
 
       checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
+      expect(checkboxes.count).to be 2
       expect(checkboxes).to all be_checked
     end
+  end
+
+  describe "POST delete-logs" do
+    let(:page) { Capybara::Node::Simple.new(response.body) }
+    let(:user) { create(:user, name: "Richard MacDuff") }
+    let!(:log_1) { create(:lettings_log, :in_progress, created_by: user) }
+    let!(:log_2) { create(:lettings_log, :completed, created_by: user) }
+    let(:selected_ids) { log_1.id }
+
+    before do
+      allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+      sign_in user
+      allow(FilterService).to receive(:filter_logs).and_return LettingsLog.all
+    end
+
+    it "throws an error if selected ids are not provided" do
+      expect { post delete_logs_lettings_logs_path }.to raise_error ActionController::ParameterMissing
+    end
+
+    it "calls the filter service with the filters in the session and the search term from the query params" do
+      search = "Schrödinger's cat"
+      logs_filters = {
+        "years" => [""],
+        "status" => ["", "in_progress"],
+        "user" => "all",
+      }
+      get lettings_logs_path(logs_filters) # adds the filters to the session
+
+      expect(FilterService).to receive(:filter_logs) { |arg1, arg2, arg3, _arg4, _arg5|
+        expect(arg1).to contain_exactly(log_1, log_2)
+        expect(arg2).to eq search
+        expect(arg3).to eq logs_filters
+      }.and_return LettingsLog.all
+
+      post delete_logs_lettings_logs_path(search:, selected_ids:)
+    end
+
+    it "displays the logs returned by the filter service" do
+      post delete_logs_lettings_logs_path(selected_ids:)
+
+      table_body_rows = page.find_all("tbody tr")
+      expect(table_body_rows.count).to be 2
+      ids_in_table = table_body_rows.map { |row| row.first("td").text }
+      expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
+    end
 
     it "only checks the selected checkboxes when selected_ids provided" do
-      get delete_logs_lettings_logs_path(log_ids: [log_1.id, log_2.id], selected_ids: [log_1.id])
+      post delete_logs_lettings_logs_path(selected_ids:)
 
       checkboxes = page.find_all("tbody tr").map { |row| row.find("input") }
       checkbox_expected_checked = checkboxes.find { |cb| cb.value == log_1.id.to_s }
@@ -1885,7 +1945,7 @@ RSpec.describe LettingsLogsController, type: :request do
     end
   end
 
-  describe "GET delete-logs-confirmation" do
+  describe "POST delete-logs-confirmation" do
     let(:page) { Capybara::Node::Simple.new(response.body) }
     let(:user) { create(:user, name: "Urban Chronotis") }
     let(:log_1) { create(:lettings_log, :in_progress) }
@@ -1894,9 +1954,8 @@ RSpec.describe LettingsLogsController, type: :request do
     let(:params) do
       {
         forms_delete_logs_form: {
-          log_type: :lettings,
-          log_ids: [log_1, log_2, log_3].map(&:id).join(" "),
-          logs_to_delete: [log_1, log_2].map(&:id),
+          search_term: "milk",
+          selected_ids: [log_1, log_2].map(&:id),
         },
       }
     end
@@ -1904,11 +1963,11 @@ RSpec.describe LettingsLogsController, type: :request do
     before do
       allow(user).to receive(:need_two_factor_authentication?).and_return(false)
       sign_in user
-      get delete_logs_confirmation_lettings_logs_path, params: params
+      post delete_logs_confirmation_lettings_logs_path, params: params
     end
 
     it "requires delete logs form data to be provided" do
-      expect { get delete_logs_confirmation_lettings_logs_path }.to raise_error(ActionController::ParameterMissing)
+      expect { post delete_logs_confirmation_lettings_logs_path }.to raise_error(ActionController::ParameterMissing)
     end
 
     it "shows the correct title" do
@@ -1923,15 +1982,14 @@ RSpec.describe LettingsLogsController, type: :request do
       let(:params) do
         {
           forms_delete_logs_form: {
-            log_type: :lettings,
-            log_ids: [log_1, log_2, log_3].map(&:id).join(" "),
-            logs_to_delete: [log_1.id],
+            search_term: "milk",
+            selected_ids: [log_1].map(&:id),
           },
         }
       end
 
       it "shows the correct information text to the user in the singular" do
-        get delete_logs_confirmation_lettings_logs_path, params: params
+        post delete_logs_confirmation_lettings_logs_path, params: params
 
         expect(page).to have_selector("p", text: "You've selected 1 log to delete")
       end
@@ -1948,19 +2006,22 @@ RSpec.describe LettingsLogsController, type: :request do
     it "the delete logs button submits the correct data to the correct path" do
       form_containing_button = page.find("form.button_to")
 
-      expect(form_containing_button[:action]).to eq delete_logs_lettings_logs_path(ids: params[:forms_delete_logs_form][:logs_to_delete])
-      expect(form_containing_button[:method]).to eq "post"
+      expect(form_containing_button[:action]).to eq delete_logs_lettings_logs_path
+      expect(form_containing_button).to have_field "_method", type: :hidden, with: "delete"
+      expect(form_containing_button).to have_field "ids[]", type: :hidden, with: log_1.id
+      expect(form_containing_button).to have_field "ids[]", type: :hidden, with: log_2.id
     end
 
     it "shows a cancel button with the correct style" do
-      expect(page).to have_selector("a.govuk-button.govuk-button--secondary", text: "Cancel")
+      expect(page).to have_selector("button.govuk-button--secondary", text: "Cancel")
     end
 
     it "the cancel button submits the correct data to the correct path" do
-      cancel_button = page.find("a.govuk-button.govuk-button--secondary", text: "Cancel")
-      expected_path = delete_logs_lettings_logs_path(log_ids: params[:forms_delete_logs_form][:log_ids].split, selected_ids: params[:forms_delete_logs_form][:logs_to_delete])
-
-      expect(cancel_button[:href]).to eq expected_path
+      form_containing_cancel = page.find_all("form").find { |form| form.has_selector?("button.govuk-button--secondary") }
+      expect(form_containing_cancel).to have_field("selected_ids", type: :hidden, with: [log_1, log_2].map(&:id).join(" "))
+      expect(form_containing_cancel).to have_field("search", type: :hidden, with: "milk")
+      expect(form_containing_cancel[:method]).to eq "post"
+      expect(form_containing_cancel[:action]).to eq delete_logs_lettings_logs_path
     end
 
     context "when no logs are selected" do
@@ -1974,7 +2035,7 @@ RSpec.describe LettingsLogsController, type: :request do
       end
 
       before do
-        get delete_logs_confirmation_lettings_logs_path, params: params
+        post delete_logs_confirmation_lettings_logs_path, params: params
       end
 
       it "renders the list of logs table again" do
@@ -2027,14 +2088,15 @@ RSpec.describe LettingsLogsController, type: :request do
       end
     end
 
-    context "when the user is not authorized to delete the logs provided" do
+    context "when the user is not authorized to delete all the logs provided" do
       let(:log_2) { create(:lettings_log, :completed) }
 
-      it "returns unauthorised and does not delete logs" do
+      it "returns unauthorised and only deletes logs for which the user is authorised" do
         delete delete_logs_lettings_logs_path, params: params
         expect(response).to have_http_status(:unauthorized)
         log_1.reload
-        expect(log_1.discarded_at).to be nil
+        expect(log_1.status).to eq "deleted"
+        expect(log_1.discarded_at).not_to be nil
         log_2.reload
         expect(log_2.discarded_at).to be nil
       end

spec/views/logs/delete_lettings_logs_spec.rb

@@ -0,0 +1,72 @@
+require "rails_helper"
+
+RSpec.describe "logs/delete_lettings_logs.html.erb" do
+  let(:user) { create(:user, :support, name: "Dirk Gently") }
+  let(:lettings_log_1) { create(:lettings_log, tenancycode: "Holistic", propcode: "Detective Agency", created_by: user) }
+  let(:lettings_logs) { [lettings_log_1] }
+  let(:delete_logs_form) { Forms::DeleteLogsForm.new(log_type: :lettings, current_user: user) }
+
+  before do
+    sign_in user
+    allow(FilterService).to receive(:filter_logs).and_return lettings_logs
+    assign(:delete_logs_form, delete_logs_form)
+  end
+
+  it "has the correct h1 content" do
+    render
+    fragment = Capybara::Node::Simple.new(rendered)
+    h1 = fragment.find("h1")
+    expect(h1.text).to include "Review the logs you want to delete"
+  end
+
+  context "when there is one log to delete" do
+    it "shows the informative text in the singular" do
+      render
+      fragment = Capybara::Node::Simple.new(rendered)
+      info_text = fragment.first("p").text
+      expect(info_text).to eq "You've selected 1 log to delete"
+    end
+  end
+
+  context "when there is more than one log to delete" do
+    let(:lettings_log_2) { create(:lettings_log, tenancycode: "01-354", propcode: "9112") }
+
+    before do
+      lettings_logs << lettings_log_2
+      allow(FilterService).to receive(:filter_logs).and_return lettings_logs
+      delete_logs_form = Forms::DeleteLogsForm.new(log_type: :lettings, current_user: user)
+      assign(:delete_logs_form, delete_logs_form)
+    end
+
+    it "shows the informative text in the plural" do
+      render
+      fragment = Capybara::Node::Simple.new(rendered)
+      info_text = fragment.first("p").text
+      expect(info_text).to eq "You've selected #{lettings_logs.count} logs to delete"
+    end
+  end
+
+  it "shows the correct headers in the table" do
+    render
+    fragment = Capybara::Node::Simple.new(rendered)
+    headers = fragment.find_all("table thead tr th").map(&:text)
+    expect(headers).to eq ["Log ID", "Tenancy code", "Property reference", "Status", "Delete?"]
+  end
+
+  it "shows the correct information in each row" do
+    render
+    fragment = Capybara::Node::Simple.new(rendered)
+    row_data = fragment.find_all("table tbody tr td").map(&:text)[0...-1]
+    expect(row_data).to eq [lettings_log_1.id.to_s, lettings_log_1.tenancycode, lettings_log_1.propcode, lettings_log_1.status.humanize.capitalize]
+  end
+
+  it "shows a checkbox with the correct hidden label in the final cell of each row" do
+    render
+    fragment = Capybara::Node::Simple.new(rendered)
+    final_cell = fragment.find_all("table tbody tr td")[-1]
+    expect(final_cell.find("input")[:type]).to eq "checkbox"
+    checkbox_label = final_cell.find("label span")
+    expect(checkbox_label.text).to eq lettings_log_1.id.to_s
+    expect(checkbox_label[:class]).to include "govuk-visually-hidden"
+  end
+end

spec/views/logs/delete_logs_spec.rb

@@ -1,102 +0,0 @@
-require "rails_helper"
-
-RSpec.describe "logs/delete_logs.html.erb" do
-  let(:lettings_log_1) { create(:lettings_log, tenancycode: "Holistic", propcode: "Detective Agency") }
-  let(:lettings_logs) { [lettings_log_1] }
-  let(:delete_logs_form) { Forms::DeleteLogsForm.new(log_ids: lettings_logs.map(&:id), log_type: :lettings) }
-
-  before do
-    sign_in create(:user, :support, name: "Dirk Gently")
-    assign(:delete_logs_form, delete_logs_form)
-  end
-
-  it "has the correct h1 content" do
-    render
-    fragment = Capybara::Node::Simple.new(rendered)
-    h1 = fragment.find("h1")
-    expect(h1.text).to include "Review the logs you want to delete"
-  end
-
-  context "when there is one log to delete" do
-    it "shows the informative text in the singular" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      info_text = fragment.first("p").text
-      expect(info_text).to eq "You've selected 1 log to delete"
-    end
-  end
-
-  context "when there is more than one log to delete" do
-    let(:lettings_log_2) { create(:lettings_log, tenancycode: "01-354", propcode: "9112") }
-
-    before do
-      lettings_logs << lettings_log_2
-      delete_logs_form = Forms::DeleteLogsForm.new(log_ids: lettings_logs.map(&:id), log_type: :lettings)
-      assign(:delete_logs_form, delete_logs_form)
-    end
-
-    it "shows the informative text in the plural" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      info_text = fragment.first("p").text
-      expect(info_text).to eq "You've selected #{lettings_logs.count} logs to delete"
-    end
-  end
-
-  context "when the user is deletings lettings logs" do
-    it "shows the correct headers in the table" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      headers = fragment.find_all("table thead tr th").map(&:text)
-      expect(headers).to eq ["Log ID", "Tenancy code", "Property reference", "Status", "Delete?"]
-    end
-
-    it "shows the correct information in each row" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      row_data = fragment.find_all("table tbody tr td").map(&:text)[0...-1]
-      expect(row_data).to eq [lettings_log_1.id.to_s, lettings_log_1.tenancycode, lettings_log_1.propcode, lettings_log_1.status.humanize.capitalize]
-    end
-
-    it "shows a checkbox with the correct hidden label in the final cell of each row" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      final_cell = fragment.find_all("table tbody tr td")[-1]
-      expect(final_cell.find("input")[:type]).to eq "checkbox"
-      checkbox_label = final_cell.find("label span")
-      expect(checkbox_label.text).to eq lettings_log_1.id.to_s
-      expect(checkbox_label[:class]).to include "govuk-visually-hidden"
-    end
-  end
-
-  context "when the user is deletings sales logs" do
-    let(:organisation) { create(:organisation, name: "awgan-eye-zay-shun") }
-    let(:sales_log_1) { create(:sales_log, owning_organisation: organisation) }
-    let(:sales_logs) { [sales_log_1] }
-    let(:delete_logs_form) { Forms::DeleteLogsForm.new(log_ids: sales_logs.map(&:id), log_type: :sales) }
-
-    it "shows the correct headers in the table" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      headers = fragment.find_all("table thead tr th").map(&:text)
-      expect(headers).to eq ["Log ID", "Owning organisation", "Created", "Status", "Delete?"]
-    end
-
-    it "shows the correct information in each row" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      row_data = fragment.find_all("table tbody tr td").map(&:text)[0...-1]
-      expect(row_data).to eq [sales_log_1.id.to_s, sales_log_1.owning_organisation.name, sales_log_1.created_at.to_formatted_s(:govuk_date), sales_log_1.status.humanize.capitalize]
-    end
-
-    it "shows a checkbox with the correct hidden label in the final cell of each row" do
-      render
-      fragment = Capybara::Node::Simple.new(rendered)
-      final_cell = fragment.find_all("table tbody tr td")[-1]
-      expect(final_cell.find("input")[:type]).to eq "checkbox"
-      checkbox_label = final_cell.find("label span")
-      expect(checkbox_label.text).to eq sales_log_1.id.to_s
-      expect(checkbox_label[:class]).to include "govuk-visually-hidden"
-    end
-  end
-end