respond to PO feedback

the log id in the delte logs table should be a link to the log
the delete logs button should be visible when the user is in a bulk upload journey
updated associated tests

app/helpers/filters_helper.rb

@@ -17,9 +17,10 @@ module FiltersHelper
 
     filters = JSON.parse(filters_json)
     filters["user"] == "yours" ||
-      filters["organisation"]&.present? ||
+      filters["organisation"].present? ||
       filters["status"]&.compact_blank&.any? ||
-      filters["years"]&.compact_blank&.any?
+      filters["years"]&.compact_blank&.any? ||
+      filters["bulk_upload_id"].present?
   end
 
   def status_filters

app/views/logs/_delete_logs_table_lettings.html.erb

@@ -12,7 +12,9 @@
     <% 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 do %>
+            <%= govuk_link_to log.id, url_for(log) %>
+          <% end %>
           <% row.cell text: log.tenancycode %>
           <% row.cell text: log.propcode %>
           <% row.cell text: status_tag(log.status) %>

app/views/logs/_delete_logs_table_sales.html.erb

@@ -12,7 +12,9 @@
     <% 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 do %>
+            <%= govuk_link_to log.id, url_for(log) %>
+          <% end %>
           <% row.cell text: log.purchid %>
           <% row.cell text: log.saledate&.to_formatted_s(:govuk_date) %>
           <% row.cell text: status_tag(log.status) %>

spec/helpers/filters_helper_spec.rb

@@ -124,6 +124,14 @@ RSpec.describe FiltersHelper do
       end
     end
 
+    context "when the user is currently in a bulk upload journey" do
+      let(:filters) { { "bulk_upload_id" => "3456" } }
+
+      it "returns true" do
+        expect(result).to be true
+      end
+    end
+
     context "when a range of filters are applied" do
       let(:filters) do
         {

spec/requests/delete_logs_controller_spec.rb

@@ -40,7 +40,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
       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 }
+      ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
       expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
     end
 
@@ -89,7 +89,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
       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 }
+      ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
       expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
     end
 
@@ -280,7 +280,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
       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 }
+      ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
       expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
     end
 
@@ -329,7 +329,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
       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 }
+      ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
       expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
     end
 
@@ -524,7 +524,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
         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 }
+        ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
         expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
       end
 
@@ -573,7 +573,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
         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 }
+        ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
         expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
       end
 
@@ -749,7 +749,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
         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 }
+        ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
         expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
       end
 
@@ -798,7 +798,7 @@ RSpec.describe "DeleteLogs", type: :request do
 
         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 }
+        ids_in_table = table_body_rows.map { |row| row.first("td").text.strip }
         expect(ids_in_table).to match_array [log_1.id.to_s, log_2.id.to_s]
       end
 

spec/views/logs/delete_logs_spec.rb

@@ -64,9 +64,16 @@ RSpec.describe "logs/delete_logs.html.erb" do
     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]
+      row_data = fragment.find_all("table tbody tr td").map(&:text)[0...-1].map(&:strip)
       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 "links to the relevant log on each row" do
+      render
+      fragment = Capybara::Node::Simple.new(rendered)
+      first_cell = fragment.first("table tbody tr td")
+      expect(first_cell).to have_link lettings_log_1.id.to_s, href: lettings_log_path(lettings_log_1)
+    end
   end
 
   context "when the table contains sales logs" do
@@ -90,9 +97,16 @@ RSpec.describe "logs/delete_logs.html.erb" do
     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]
+      row_data = fragment.find_all("table tbody tr td").map(&:text)[0...-1].map(&:strip)
       expect(row_data).to eq [sales_log.id.to_s, sales_log.purchid, sales_log.saledate.to_formatted_s(:govuk_date), sales_log.status.humanize.capitalize]
     end
+
+    it "links to the relevant log on each row" do
+      render
+      fragment = Capybara::Node::Simple.new(rendered)
+      first_cell = fragment.first("table tbody tr td")
+      expect(first_cell).to have_link sales_log.id.to_s, href: sales_log_path(sales_log)
+    end
   end
 
   it "shows a checkbox with the correct hidden label in the final cell of each row" do