From ae0fda1e042dddfcdedc600fe7c7d3922243fcaf Mon Sep 17 00:00:00 2001
From: Kat <katrina@kosiak.co.uk>
Date: Tue, 7 May 2024 14:28:19 +0100
Subject: [PATCH] Add cancel, conditionally display send email

---
 .../lettings_logs_filters_controller.rb       | 10 +-
 app/views/filters/_checkbox_filter.html.erb   |  1 +
 app/views/filters/_select_filter.html.erb     |  1 +
 .../lettings_log_filters/assigned_to.html.erb |  6 +-
 .../lettings_log_filters/managed_by.html.erb  |  5 +-
 .../lettings_log_filters/needstype.html.erb   |  6 +-
 .../lettings_log_filters/owned_by.html.erb    |  5 +-
 .../lettings_log_filters/status.html.erb      |  5 +-
 .../lettings_log_filters/years.html.erb       |  6 +-
 app/views/logs/download_csv.html.erb          | 11 ++-
 spec/features/lettings_log_spec.rb            | 93 +++++++++++++++++++
 11 files changed, 136 insertions(+), 13 deletions(-)

diff --git a/app/controllers/lettings_logs_filters_controller.rb b/app/controllers/lettings_logs_filters_controller.rb
index 90c0f42ea..a18b9602c 100644
--- a/app/controllers/lettings_logs_filters_controller.rb
+++ b/app/controllers/lettings_logs_filters_controller.rb
@@ -6,20 +6,22 @@ class LettingsLogsFiltersController < ApplicationController
     define_method(filter) do
       @filter_form = Forms::FilterForm.new
       @filter_type = "lettings_logs"
+      @search_term = params["search"]
+      @codes_only = params["codes_only"]
       render "filters/lettings_log_filters/#{filter}"
     end
   end
 
   def update
     @filter_form = Forms::FilterForm.new(filter_form_params)
+    @filter_type = "lettings_logs"
+    @search_term = params["search"]
+    @codes_only = params["codes_only"]
 
     if @filter_form.valid?
       session_filters
-      redirect_to csv_download_lettings_logs_path(search: "", codes_only: true)
+      redirect_to csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only)
     else
-      @filter_type = "lettings_logs"
-      @search_term = params["search"]
-      @codes_only = params["codes_only"]
       render "filters/lettings_log_filters/years", status: :unprocessable_entity
     end
   end
diff --git a/app/views/filters/_checkbox_filter.html.erb b/app/views/filters/_checkbox_filter.html.erb
index d08c42a77..c862875ef 100644
--- a/app/views/filters/_checkbox_filter.html.erb
+++ b/app/views/filters/_checkbox_filter.html.erb
@@ -2,6 +2,7 @@
   <% options.map do |key, option| %>
     <%= f.govuk_check_box category, key.to_s,
     label: { text: option },
+    name: "#{category}[]",
     checked: filter_selected?(category, key, @filter_type),
     size: "s" %>
   <% end %>
diff --git a/app/views/filters/_select_filter.html.erb b/app/views/filters/_select_filter.html.erb
index e31326347..9e85bbf9b 100644
--- a/app/views/filters/_select_filter.html.erb
+++ b/app/views/filters/_select_filter.html.erb
@@ -1,5 +1,6 @@
 <%= f.govuk_select(category.to_sym,
   label: { text: label, hidden: secondary },
+  name: "#{category}[]",
   "data-controller": "accessible-autocomplete conditional-filter") do %>
     <% collection.each do |answer| %>
       <option value="<%= answer.id %>"
diff --git a/app/views/filters/lettings_log_filters/assigned_to.html.erb b/app/views/filters/lettings_log_filters/assigned_to.html.erb
index 4da199f33..703c5c994 100644
--- a/app/views/filters/lettings_log_filters/assigned_to.html.erb
+++ b/app/views/filters/lettings_log_filters/assigned_to.html.erb
@@ -18,5 +18,9 @@
                 label: "Assigned to",
                 category: "assigned_to",
               } %>
-  <%= f.govuk_submit "Save changes", class: "govuk-!-margin-bottom-0" %>
+
+  <div class="govuk-button-group">
+    <%= f.govuk_submit "Save changes" %>
+    <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %>
+  </div>
 <% end %>
diff --git a/app/views/filters/lettings_log_filters/managed_by.html.erb b/app/views/filters/lettings_log_filters/managed_by.html.erb
index caa249df6..c8bfdc96e 100644
--- a/app/views/filters/lettings_log_filters/managed_by.html.erb
+++ b/app/views/filters/lettings_log_filters/managed_by.html.erb
@@ -16,5 +16,8 @@
     label: user_or_org_lettings_path? ? "Managed by" : "Reported by",
     category: "managing_organisation_select",
   } %>
-  <%= f.govuk_submit "Save changes", class: "govuk-!-margin-bottom-0" %>
+  <div class="govuk-button-group">
+    <%= f.govuk_submit "Save changes" %>
+    <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %>
+  </div>
 <% end %>
diff --git a/app/views/filters/lettings_log_filters/needstype.html.erb b/app/views/filters/lettings_log_filters/needstype.html.erb
index 025a7f08c..61920bb90 100644
--- a/app/views/filters/lettings_log_filters/needstype.html.erb
+++ b/app/views/filters/lettings_log_filters/needstype.html.erb
@@ -6,5 +6,9 @@
                          label: "Needs type",
                          category: "needstypes",
                        } %>
-  <%= f.govuk_submit "Save changes", class: "govuk-!-margin-bottom-0" %>
+
+  <div class="govuk-button-group">
+    <%= f.govuk_submit "Save changes" %>
+    <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %>
+  </div>
 <% end %>
diff --git a/app/views/filters/lettings_log_filters/owned_by.html.erb b/app/views/filters/lettings_log_filters/owned_by.html.erb
index 8644b23b2..59b59d10a 100644
--- a/app/views/filters/lettings_log_filters/owned_by.html.erb
+++ b/app/views/filters/lettings_log_filters/owned_by.html.erb
@@ -16,5 +16,8 @@
     label: "Owned by",
     category: "owning_organisation_select",
   } %>
-  <%= f.govuk_submit "Save changes", class: "govuk-!-margin-bottom-0" %>
+  <div class="govuk-button-group">
+    <%= f.govuk_submit "Save changes" %>
+    <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %>
+  </div>
 <% end %>
diff --git a/app/views/filters/lettings_log_filters/status.html.erb b/app/views/filters/lettings_log_filters/status.html.erb
index 65c70e667..e49062835 100644
--- a/app/views/filters/lettings_log_filters/status.html.erb
+++ b/app/views/filters/lettings_log_filters/status.html.erb
@@ -6,5 +6,8 @@
                 label: "Status",
                 category: "status",
               } %>
-  <%= f.govuk_submit "Save changes", class: "govuk-!-margin-bottom-0" %>
+  <div class="govuk-button-group">
+    <%= f.govuk_submit "Save changes" %>
+    <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %>
+  </div>
 <% end %>
diff --git a/app/views/filters/lettings_log_filters/years.html.erb b/app/views/filters/lettings_log_filters/years.html.erb
index e059b0ae4..3c0b9f9f1 100644
--- a/app/views/filters/lettings_log_filters/years.html.erb
+++ b/app/views/filters/lettings_log_filters/years.html.erb
@@ -8,5 +8,9 @@
         label: "Which financial year do you want to download data for?",
         category: "years",
       } %>
-  <%= f.govuk_submit "Save changes", class: "govuk-!-margin-bottom-0" %>
+
+  <div class="govuk-button-group">
+    <%= f.govuk_submit "Save changes" %>
+    <%= govuk_button_link_to "Cancel", lettings_logs_path, secondary: true %>
+  </div>
 <% end %>
diff --git a/app/views/logs/download_csv.html.erb b/app/views/logs/download_csv.html.erb
index 3ef435616..dc8764928 100644
--- a/app/views/logs/download_csv.html.erb
+++ b/app/views/logs/download_csv.html.erb
@@ -9,7 +9,11 @@
     <h1 class="govuk-heading-l">Download CSV</h1>
 
     <p class="govuk-body">We'll send a secure download link to your email address <strong><%= @current_user.email %></strong>.</p>
-    <p class="govuk-body">You've selected <%= count %> logs.</p>
+    <% if count.positive? %>
+      <p class="govuk-body">You've selected <%= count %> logs.</p>
+    <% else %>
+      <p class="govuk-body">You haven't selected any logs. Please check your filters</p>
+    <% end %>
 
     <div class="govuk-grid-row">
       <div class="govuk-grid-column-three-quarters-from-desktop">
@@ -38,7 +42,8 @@
         <% end %>
       </div>
     </div>
-
-    <%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term, codes_only: } %>
+    <% if count.positive? %>
+      <%= govuk_button_to "Send email", post_path, method: :post, params: { search: search_term, codes_only: } %>
+    <% end %>
   </div>
 </div>
diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb
index 546f924a8..25f0530f8 100644
--- a/spec/features/lettings_log_spec.rb
+++ b/spec/features/lettings_log_spec.rb
@@ -114,6 +114,99 @@ RSpec.describe "Lettings Log Features" do
     end
   end
 
+  context "when downloading logs" do
+    let(:user) { create(:user, :support, last_sign_in_at: Time.zone.now) }
+
+    context "when I am signed in" do
+      before do
+        create(:lettings_log, :setup_completed, owning_organisation: user.organisation, tenancycode: "1")
+        create(:lettings_log, :setup_completed, :sh, owning_organisation: user.organisation, tenancycode: "1")
+        allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+        sign_in user
+        visit("/lettings-logs?search=1")
+      end
+
+      context "when no filters are selected" do
+        it "requires year filter to be selected for non codes download" do
+          click_link("Download (CSV)")
+          expect(page).to have_current_path("/lettings-logs/filters/years?codes_only=false&search=1")
+          click_button("Save changes")
+
+          expect(page).to have_selector(".govuk-error-summary__title")
+          expect(page).to have_selector("#forms-filter-form-years-error")
+          expect(page).to have_content("There is a problem")
+
+          choose("forms-filter-form-years-2023-field", allow_label_click: true)
+          click_button("Save changes")
+
+          expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=false&search=1")
+        end
+
+        it "requires year filter to be selected for codes only download" do
+          click_link("Download (CSV, codes only)")
+          expect(page).to have_current_path("/lettings-logs/filters/years?codes_only=true&search=1")
+          click_button("Save changes")
+
+          expect(page).to have_selector(".govuk-error-summary__title")
+          expect(page).to have_selector("#forms-filter-form-years-error")
+          expect(page).to have_content("There is a problem")
+
+          choose("forms-filter-form-years-2023-field", allow_label_click: true)
+          click_button("Save changes")
+
+          expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=true&search=1")
+        end
+
+        it "allows cancelling the download without selecting the year filter" do
+          click_link("Download (CSV)")
+
+          click_link(text: "Cancel")
+          expect(page).to have_current_path("/lettings-logs")
+        end
+      end
+
+      context "when I have selected one year filter" do
+        before do
+          check("2024")
+          click_button("Apply filters")
+        end
+
+        it "allows changing the filters and downloading the csv data" do
+          click_link("Download (CSV)")
+
+          expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=false&search=1")
+          expect(page).to have_content("You've selected 2 logs")
+        end
+
+        it "allows updating filters" do
+          click_link("Download (CSV, codes only)")
+          expect(page).to have_content("You've selected 2 logs")
+          click_link("Change", href: "/lettings-logs/filters/needstype?search=1&codes_only=true")
+
+          check("forms-filter-form-needstypes-1-field", allow_label_click: true)
+          click_button("Save changes")
+
+          expect(page).to have_content("You've selected 1 logs")
+
+          click_link("Change", href: "/lettings-logs/filters/status?search=1&codes_only=true")
+          check("forms-filter-form-status-not-started-field", allow_label_click: true)
+          click_button("Save changes")
+
+          expect(page).to have_content("You haven't selected any logs. Please check your filters")
+          expect(page).not_to have_button("Send email")
+        end
+
+        it "routes back to the filters CYA when cancel is pressed" do
+          click_link("Download (CSV)")
+          click_link("Change", href: "/lettings-logs/filters/needstype?search=1&codes_only=false")
+
+          click_link(text: "Cancel")
+          expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=false&search=1")
+        end
+      end
+    end
+  end
+
   context "when the signed is user is a Support user" do
     let(:organisation) { create(:organisation, name: "User org") }
     let(:support_user) { create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) }