diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index df1728b7b..ebe38a895 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -92,7 +92,7 @@ class LettingsLogsController < LogsController unpaginated_filtered_logs = filter_manager.filtered_logs(current_user.lettings_logs, search_term, session_filters) - render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, codes_only: codes_only_export?, session_filters: } + render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, codes_only: codes_only_export?, session_filters:, filter_type: "lettings_logs" } end def email_csv diff --git a/app/controllers/sales_logs_controller.rb b/app/controllers/sales_logs_controller.rb index 25cf405a7..e86d841ce 100644 --- a/app/controllers/sales_logs_controller.rb +++ b/app/controllers/sales_logs_controller.rb @@ -62,9 +62,11 @@ class SalesLogsController < LogsController end def download_csv + redirect_to filters_years_sales_logs_path(search: search_term, codes_only: codes_only_export?) and return if session_filters["years"].blank? || session_filters["years"].count != 1 + unpaginated_filtered_logs = filter_manager.filtered_logs(current_user.sales_logs, search_term, session_filters) - render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_sales_logs_path, codes_only: codes_only_export? } + render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_sales_logs_path, codes_only: codes_only_export?, session_filters:, filter_type: "sales_logs" } end def email_csv diff --git a/app/controllers/sales_logs_filters_controller.rb b/app/controllers/sales_logs_filters_controller.rb new file mode 100644 index 000000000..415e774f6 --- /dev/null +++ b/app/controllers/sales_logs_filters_controller.rb @@ -0,0 +1,46 @@ +class SalesLogsFiltersController < ApplicationController + before_action :session_filters, if: :current_user, only: %i[update] + before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[update] + + %w[years status assigned_to owned_by managed_by].each do |filter| + define_method(filter) do + @filter_form = Forms::FilterForm.new + @filter_type = "sales_logs" + @search_term = params["search"] + @codes_only = params["codes_only"] + render "filters/sales_log_filters/#{filter}" + end + end + + def update + @filter_form = Forms::FilterForm.new(filter_form_params) + @filter_type = "sales_logs" + @search_term = params["search"] + @codes_only = params["codes_only"] + + if @filter_form.valid? + session_filters + redirect_to csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only) + else + render "filters/sales_log_filters/years", status: :unprocessable_entity + end + end +end + +private + +def filter_form_params + filter_params = params.permit(:years, :status, :assigned_to, :user, :owning_organisation_select, :owning_organisation, :managing_organisation_select, :managing_organisation) + filter_params[:years] = session_filters["years"] if filter_params[:years].blank? + filter_params +end + +def session_filters + params["forms_filter_form"].each { |key, value| params[key] = value } if params["forms_filter_form"].present? + params["years"] = [params["years"]] if params["years"].present? + filter_manager.session_filters +end + +def filter_manager + FilterManager.new(current_user:, session:, params:, filter_type: "sales_logs") +end diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 878c16f97..5ee36e5a9 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -165,6 +165,14 @@ module FiltersHelper filters_count(applied_filters(filter_type)) end + def check_your_answers_filters_list(session_filters, filter_type) + if filter_type == "lettings_logs" + check_your_answers_lettings_filters_list(session_filters) + else + check_your_answers_sales_filters_list(session_filters) + end + end + def check_your_answers_lettings_filters_list(session_filters) [ { label: "Collection year", value: formatted_years_filter(session_filters), path: filters_years_lettings_logs_path }, @@ -176,6 +184,16 @@ module FiltersHelper ] end + def check_your_answers_sales_filters_list(session_filters) + [ + { label: "Collection year", value: formatted_years_filter(session_filters), path: filters_years_sales_logs_path }, + { label: "Status", value: formatted_status_filter(session_filters), path: filters_status_sales_logs_path }, + { label: "Assigned to", value: formatted_assigned_to_filter(session_filters), path: filters_assigned_to_sales_logs_path }, + { label: "Owned by", value: formatted_owned_by_filter(session_filters), path: filters_owned_by_sales_logs_path }, + { label: "Managed by", value: formatted_managed_by_filter(session_filters), path: filters_managed_by_sales_logs_path }, + ] + end + private def applied_filters(filter_type) 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 c8bfdc96e..900732a69 100644 --- a/app/views/filters/lettings_log_filters/managed_by.html.erb +++ b/app/views/filters/lettings_log_filters/managed_by.html.erb @@ -7,13 +7,13 @@ label: "Specific managing organisation", conditional_filter: { type: "select", - label: user_or_org_lettings_path? ? "Managed by" : "Reported by", + label: "Managed by", category: "managing_organisation", options: managing_organisation_filter_options(current_user), }, }, }, - label: user_or_org_lettings_path? ? "Managed by" : "Reported by", + label: "Managed by", category: "managing_organisation_select", } %>
diff --git a/app/views/filters/sales_log_filters/assigned_to.html.erb b/app/views/filters/sales_log_filters/assigned_to.html.erb new file mode 100644 index 000000000..56d45e619 --- /dev/null +++ b/app/views/filters/sales_log_filters/assigned_to.html.erb @@ -0,0 +1,26 @@ +<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> + <%= render partial: "filters/radio_filter", + locals: { + f:, + options: { + "all": { label: "Any user" }, + "you": { label: "You" }, + "specific_user": { + label: "Specific user", + conditional_filter: { + type: "select", + label: "User", + category: "user", + options: assigned_to_filter_options(current_user), + }, + }, + }, + label: "Assigned to", + category: "assigned_to", + } %> + +
+ <%= f.govuk_submit "Save changes" %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> +
+<% end %> diff --git a/app/views/filters/sales_log_filters/managed_by.html.erb b/app/views/filters/sales_log_filters/managed_by.html.erb new file mode 100644 index 000000000..98de78864 --- /dev/null +++ b/app/views/filters/sales_log_filters/managed_by.html.erb @@ -0,0 +1,23 @@ +<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> + <%= render partial: "filters/radio_filter", locals: { + f:, + options: { + "all": { label: "Any managing organisation" }, + "specific_org": { + label: "Specific managing organisation", + conditional_filter: { + type: "select", + label: "Reported by", + category: "managing_organisation", + options: managing_organisation_filter_options(current_user), + }, + }, + }, + label: "Reported by", + category: "managing_organisation_select", + } %> +
+ <%= f.govuk_submit "Save changes" %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> +
+<% end %> diff --git a/app/views/filters/sales_log_filters/owned_by.html.erb b/app/views/filters/sales_log_filters/owned_by.html.erb new file mode 100644 index 000000000..86143c0ad --- /dev/null +++ b/app/views/filters/sales_log_filters/owned_by.html.erb @@ -0,0 +1,23 @@ +<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> + <%= render partial: "filters/radio_filter", locals: { + f:, + options: { + "all": { label: "Any owning organisation" }, + "specific_org": { + label: "Specific owning organisation", + conditional_filter: { + type: "select", + label: "Owning Organisation", + category: "owning_organisation", + options: owning_organisation_filter_options(current_user), + }, + }, + }, + label: "Owned by", + category: "owning_organisation_select", + } %> +
+ <%= f.govuk_submit "Save changes" %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> +
+<% end %> diff --git a/app/views/filters/sales_log_filters/status.html.erb b/app/views/filters/sales_log_filters/status.html.erb new file mode 100644 index 000000000..3a7dea248 --- /dev/null +++ b/app/views/filters/sales_log_filters/status.html.erb @@ -0,0 +1,13 @@ +<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> + <%= render partial: "filters/checkbox_filter", + locals: { + f:, + options: status_filters, + label: "Status", + category: "status", + } %> +
+ <%= f.govuk_submit "Save changes" %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> +
+<% end %> diff --git a/app/views/filters/sales_log_filters/years.html.erb b/app/views/filters/sales_log_filters/years.html.erb new file mode 100644 index 000000000..f258004c5 --- /dev/null +++ b/app/views/filters/sales_log_filters/years.html.erb @@ -0,0 +1,16 @@ +<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> + <%= f.govuk_error_summary %> + + <%= render partial: "filters/radio_filter", + locals: { + f:, + options: collection_year_radio_options, + label: "Which financial year do you want to download data for?", + category: "years", + } %> + +
+ <%= f.govuk_submit "Save changes" %> + <%= govuk_button_link_to "Cancel", sales_logs_path, secondary: true %> +
+<% end %> diff --git a/app/views/logs/download_csv.html.erb b/app/views/logs/download_csv.html.erb index dc8764928..43efcdac4 100644 --- a/app/views/logs/download_csv.html.erb +++ b/app/views/logs/download_csv.html.erb @@ -22,7 +22,7 @@ <%= govuk_summary_list do |summary_list| %> - <% check_your_answers_lettings_filters_list(session_filters).each do |filter| %> + <% check_your_answers_filters_list(session_filters, filter_type).each do |filter| %> <% summary_list.with_row do |row| %> <% row.with_key { filter[:label] } %> <% row.with_value do %> diff --git a/config/routes.rb b/config/routes.rb index c8605c856..af3422431 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -284,6 +284,11 @@ Rails.application.routes.draw do post "delete-logs-confirmation", to: "delete_logs#delete_sales_logs_confirmation" delete "delete-logs", to: "delete_logs#discard_sales_logs" + %w[years status assigned-to owned-by managed-by].each do |filter| + get "filters/#{filter}", to: "sales_logs_filters##{filter.underscore}" + end + post "filters", to: "sales_logs_filters#update" + resources :bulk_upload_sales_logs, path: "bulk-upload-logs" do collection do get :start diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb index 8a0dab68d..2fb68d854 100644 --- a/spec/features/sales_log_spec.rb +++ b/spec/features/sales_log_spec.rb @@ -139,6 +139,100 @@ RSpec.describe "Sales Log Features" do end end + context "when downloading logs" do + let(:user) { create(:user, :support) } + let(:other_user) { create(:user, organisation: user.organisation) } + + context "when I am signed in" do + before do + create(:sales_log, :in_progress, owning_organisation: user.organisation, assigned_to: user) + create(:sales_log, :in_progress, owning_organisation: user.organisation, assigned_to: other_user) + allow(user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in user + visit("/sales-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("/sales-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("/sales-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("/sales-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("/sales-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("/sales-logs") + end + end + + context "when one year filter is selected" 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("/sales-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: "/sales-logs/filters/assigned-to?search=1&codes_only=true") + + choose("forms-filter-form-assigned-to-you-field", allow_label_click: true) + click_button("Save changes") + + expect(page).to have_content("You've selected 1 logs") + + click_link("Change", href: "/sales-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: "/sales-logs/filters/assigned-to?search=1&codes_only=false") + + click_link(text: "Cancel") + expect(page).to have_current_path("/sales-logs/csv-download?codes_only=false&search=1") + end + end + end + end + context "when signed in as a support user" do let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index ddc8b0199..f636a9c23 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -1402,12 +1402,13 @@ RSpec.describe LettingsLogsController, type: :request do let(:search_term) { "foo" } before do + create(:lettings_log, :setup_completed, assigned_to: user, owning_organisation: user.organisation, tenancycode: search_term) sign_in user end context "when there is 1 year selected in the filters" do before do - get "/lettings-logs/csv-download?years[]=2021&search=#{search_term}&codes_only=false", headers: + get "/lettings-logs/csv-download?years[]=2024&search=#{search_term}&codes_only=false", headers: end it "returns http success" do @@ -1783,29 +1784,33 @@ RSpec.describe LettingsLogsController, type: :request do let(:headers) { { "Accept" => "text/html" } } before do + create(:lettings_log, :setup_completed, assigned_to: user, tenancycode: search_term) allow(user).to receive(:need_two_factor_authentication?).and_return(false) sign_in user end it "renders a page with the correct header" do - get "/lettings-logs/csv-download?years[]=2021&codes_only=false", headers:, params: {} + get "/lettings-logs/csv-download?years[]=2024&codes_only=false", headers:, params: {} header = page.find_css("h1") expect(header.text).to include("Download CSV") end it "renders a form with the correct target containing a button with the correct text" do - get "/lettings-logs/csv-download?years[]=2021&codes_only=false", headers:, params: {} + get "/lettings-logs/csv-download?years[]=2024&codes_only=false", headers:, params: {} form = page.find("form.button_to") expect(form[:method]).to eq("post") expect(form[:action]).to eq("/lettings-logs/email-csv") expect(form).to have_button("Send email") end - it "when query string contains search parameter, form contains hidden field with correct value" do - search_term = "blam" - get "/lettings-logs/csv-download?years[]=2021&codes_only=false&search=#{search_term}", headers:, params: {} - hidden_field = page.find("form.button_to").find_field("search", type: "hidden") - expect(hidden_field.value).to eq(search_term) + context "when query string contains search parameter" do + let(:search_term) { "blam" } + + it "contains hidden field with correct value" do + get "/lettings-logs/csv-download?years[]=2024&codes_only=false&search=#{search_term}", headers:, params: {} + hidden_field = page.find("form.button_to").find_field("search", type: "hidden") + expect(hidden_field.value).to eq(search_term) + end end context "when the user is a data coordinator" do @@ -1813,7 +1818,7 @@ RSpec.describe LettingsLogsController, type: :request do it "when codes_only query parameter is false, form contains hidden field with correct value" do codes_only = false - get "/lettings-logs/csv-download?years[]=2021&codes_only=#{codes_only}", headers:, params: {} + get "/lettings-logs/csv-download?years[]=2024&codes_only=#{codes_only}", headers:, params: {} hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") expect(hidden_field.value).to eq(codes_only.to_s) end @@ -1828,7 +1833,7 @@ RSpec.describe LettingsLogsController, type: :request do context "when the user is a data provider" do it "when codes_only query parameter is false, form contains hidden field with correct value" do codes_only = false - get "/lettings-logs/csv-download?years[]=2021&codes_only=#{codes_only}", headers:, params: {} + get "/lettings-logs/csv-download?years[]=2024&codes_only=#{codes_only}", headers:, params: {} hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") expect(hidden_field.value).to eq(codes_only.to_s) end @@ -1845,14 +1850,14 @@ RSpec.describe LettingsLogsController, type: :request do it "when codes_only query parameter is false, form contains hidden field with correct value" do codes_only = false - get "/lettings-logs/csv-download?years[]=2021&codes_only=#{codes_only}", headers:, params: {} + get "/lettings-logs/csv-download?years[]=2024&codes_only=#{codes_only}", headers:, params: {} hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") expect(hidden_field.value).to eq(codes_only.to_s) end it "when codes_only query parameter is true, form contains hidden field with correct value" do codes_only = true - get "/lettings-logs/csv-download?years[]=2021&codes_only=#{codes_only}", headers:, params: {} + get "/lettings-logs/csv-download?years[]=2024&codes_only=#{codes_only}", headers:, params: {} hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden") expect(hidden_field.value).to eq(codes_only.to_s) end diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 8c8fad320..e9652228b 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -965,24 +965,70 @@ RSpec.describe SalesLogsController, type: :request do let(:codes_only) { false } before do + create(:sales_log, :in_progress, assigned_to: user, purchid: search_term) allow(user).to receive(:need_two_factor_authentication?).and_return(false) sign_in user - get "/sales-logs/csv-download?search=#{search_term}&codes_only=#{codes_only}", headers: end - it "returns http success" do - expect(response).to have_http_status(:success) + context "when there is 1 year selected in the filters" do + before do + get "/sales-logs/csv-download?years[]=2023&search=#{search_term}&codes_only=#{codes_only}", headers: + end + + it "returns http success" do + expect(response).to have_http_status(:success) + end + + it "shows a confirmation button" do + expect(page).to have_button("Send email") + end + + it "allows updating log filters" do + expect(page).to have_content("Check your filters") + expect(page).to have_link("Change", count: 5) + expect(page).to have_link("Change", href: "/sales-logs/filters/years?search=#{search_term}&codes_only=false") + expect(page).to have_link("Change", href: "/sales-logs/filters/assigned-to?search=#{search_term}&codes_only=false") + expect(page).to have_link("Change", href: "/sales-logs/filters/owned-by?search=#{search_term}&codes_only=false") + expect(page).to have_link("Change", href: "/sales-logs/filters/managed-by?search=#{search_term}&codes_only=false") + expect(page).to have_link("Change", href: "/sales-logs/filters/status?search=#{search_term}&codes_only=false") + end + + it "has a hidden field with the search term" do + expect(page).to have_field("search", type: "hidden", with: search_term) + end end - it "shows a confirmation button" do - expect(page).to have_button("Send email") + context "when there are no years selected in the filters" do + before do + get "/sales-logs/csv-download?search=#{search_term}&codes_only=false", headers: + end + + it "redirects to the year filter question" do + expect(response).to redirect_to("/sales-logs/filters/years?codes_only=false&search=#{search_term}") + follow_redirect! + expect(page).to have_content("Which financial year do you want to download data for?") + expect(page).to have_button("Save changes") + end end - it "has a hidden field with the search term" do - expect(page).to have_field("search", type: "hidden", with: search_term) + context "when there are multiple years selected in the filters" do + before do + get "/sales-logs/csv-download?years[]=2021&years[]=2022&search=#{search_term}&codes_only=false", headers: + end + + it "redirects to the year filter question" do + expect(response).to redirect_to("/sales-logs/filters/years?codes_only=false&search=#{search_term}") + follow_redirect! + expect(page).to have_content("Which financial year do you want to download data for?") + expect(page).to have_button("Save changes") + end end context "when user is not support" do + before do + get "/sales-logs/csv-download?years[]=2023&search=#{search_term}&codes_only=#{codes_only}", headers: + end + context "and export type is not codes only" do it "has a hidden field with the export type" do expect(page).to have_field("codes_only", type: "hidden", with: codes_only) @@ -1001,6 +1047,10 @@ RSpec.describe SalesLogsController, type: :request do context "when user is support" do let(:user) { FactoryBot.create(:user, :support) } + before do + get "/sales-logs/csv-download?years[]=2023&search=#{search_term}&codes_only=#{codes_only}", headers: + end + context "and export type is not codes only" do it "has a hidden field with the export type" do expect(page).to have_field("codes_only", type: "hidden", with: codes_only)