diff --git a/app/controllers/lettings_logs_filters_controller.rb b/app/controllers/lettings_logs_filters_controller.rb index 97a3aa965..12496b733 100644 --- a/app/controllers/lettings_logs_filters_controller.rb +++ b/app/controllers/lettings_logs_filters_controller.rb @@ -1,42 +1,35 @@ class LettingsLogsFiltersController < 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] + before_action :session_filters, if: :current_user + before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user %w[years status needstype assigned_to owned_by managed_by].each do |filter| 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"] + %w[status needstype assigned_to owned_by managed_by].each do |filter| + define_method("update_#{filter}") do + @filter_type = "lettings_logs" + + redirect_to csv_download_lettings_logs_path(search: params["search"], codes_only: params["codes_only"]) + end + end - if @filter_form.valid? - session_filters - redirect_to csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only) + def update_years + @filter_type = "lettings_logs" + if params["years"].nil? + redirect_to filters_years_lettings_logs_path(search: params["search"], codes_only: params["codes_only"], error: "Please select a year") else - render "filters/lettings_log_filters/years", status: :unprocessable_entity + redirect_to csv_download_lettings_logs_path(search: params["search"], codes_only: params["codes_only"]) end end end private -def filter_form_params - filter_params = params.permit(:years, :status, :needstypes, :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 diff --git a/app/controllers/sales_logs_filters_controller.rb b/app/controllers/sales_logs_filters_controller.rb index 415e774f6..b2b65bb1a 100644 --- a/app/controllers/sales_logs_filters_controller.rb +++ b/app/controllers/sales_logs_filters_controller.rb @@ -1,42 +1,35 @@ 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] + before_action :session_filters, if: :current_user + before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user %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"] + %w[status assigned_to owned_by managed_by].each do |filter| + define_method("update_#{filter}") do + @filter_type = "sales_logs" + + redirect_to csv_download_sales_logs_path(search: params["search"], codes_only: params["codes_only"]) + end + end - if @filter_form.valid? - session_filters - redirect_to csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only) + def update_years + @filter_type = "sales_logs" + if params["years"].nil? + redirect_to filters_years_sales_logs_path(search: params["search"], codes_only: params["codes_only"], error: "Please select a year") else - render "filters/sales_log_filters/years", status: :unprocessable_entity + redirect_to csv_download_sales_logs_path(search: params["search"], codes_only: params["codes_only"]) 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 diff --git a/app/models/forms/filter_form.rb b/app/models/forms/filter_form.rb deleted file mode 100644 index 6d624ac31..000000000 --- a/app/models/forms/filter_form.rb +++ /dev/null @@ -1,10 +0,0 @@ -module Forms - class FilterForm - include ActiveModel::Model - include ActiveModel::Validations - - attr_accessor :years, :status, :needstypes, :assigned_to, :user, :owning_organisation_select, :owning_organisation, :managing_organisation_select, :managing_organisation - - validates :years, presence: true - end -end 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 703c5c994..bbd2ce15d 100644 --- a/app/views/filters/lettings_log_filters/assigned_to.html.erb +++ b/app/views/filters/lettings_log_filters/assigned_to.html.erb @@ -1,4 +1,4 @@ -<%= form_with model: @filter_form, url: filters_lettings_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_assigned_to_lettings_logs_path do |f| %> <%= render partial: "filters/radio_filter", locals: { f:, @@ -17,10 +17,15 @@ }, label: "Assigned to", category: "assigned_to", + size: "l", } %> + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %>
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: request.params["search"], codes_only: request.params["codes_only"]), secondary: true %>
<% 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 900732a69..43f87f655 100644 --- a/app/views/filters/lettings_log_filters/managed_by.html.erb +++ b/app/views/filters/lettings_log_filters/managed_by.html.erb @@ -1,4 +1,4 @@ -<%= form_with model: @filter_form, url: filters_lettings_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_managed_by_lettings_logs_path do |f| %> <%= render partial: "filters/radio_filter", locals: { f:, options: { @@ -15,9 +15,16 @@ }, label: "Managed by", category: "managing_organisation_select", + size: "l", } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: request.params["search"], codes_only: request.params["codes_only"]), secondary: true %>
<% end %> diff --git a/app/views/filters/lettings_log_filters/needstype.html.erb b/app/views/filters/lettings_log_filters/needstype.html.erb index 61920bb90..fcbb7e8e6 100644 --- a/app/views/filters/lettings_log_filters/needstype.html.erb +++ b/app/views/filters/lettings_log_filters/needstype.html.erb @@ -1,14 +1,20 @@ -<%= form_with model: @filter_form, url: filters_lettings_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_needstype_lettings_logs_path do |f| %> <%= render partial: "filters/checkbox_filter", locals: { f:, options: needstype_filters, label: "Needs type", category: "needstypes", + size: "l", } %> + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: request.params["search"], codes_only: request.params["codes_only"]), secondary: true %>
<% 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 59b59d10a..d4dea76d3 100644 --- a/app/views/filters/lettings_log_filters/owned_by.html.erb +++ b/app/views/filters/lettings_log_filters/owned_by.html.erb @@ -1,4 +1,4 @@ -<%= form_with model: @filter_form, url: filters_lettings_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_owned_by_lettings_logs_path do |f| %> <%= render partial: "filters/radio_filter", locals: { f:, options: { @@ -15,9 +15,16 @@ }, label: "Owned by", category: "owning_organisation_select", + size: "l", } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: request.params["search"], codes_only: request.params["codes_only"]), secondary: true %>
<% end %> diff --git a/app/views/filters/lettings_log_filters/status.html.erb b/app/views/filters/lettings_log_filters/status.html.erb index e49062835..a09342369 100644 --- a/app/views/filters/lettings_log_filters/status.html.erb +++ b/app/views/filters/lettings_log_filters/status.html.erb @@ -1,13 +1,20 @@ -<%= form_with model: @filter_form, url: filters_lettings_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_status_lettings_logs_path do |f| %> <%= render partial: "filters/checkbox_filter", locals: { f:, options: status_filters, label: "Status", category: "status", + size: "l", } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_lettings_logs_path(search: request.params["search"], codes_only: request.params["codes_only"]), secondary: true %>
<% end %> diff --git a/app/views/filters/lettings_log_filters/years.html.erb b/app/views/filters/lettings_log_filters/years.html.erb index 3c0b9f9f1..d82f0f237 100644 --- a/app/views/filters/lettings_log_filters/years.html.erb +++ b/app/views/filters/lettings_log_filters/years.html.erb @@ -1,5 +1,20 @@ -<%= form_with model: @filter_form, url: filters_lettings_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> - <%= f.govuk_error_summary %> +<%= form_with html: { method: :get }, url: filters_update_years_lettings_logs_path do |f| %> + <% if params["error"].present? %> +
+
+

+ There is a problem +

+ +
+
+ <% end %> <%= render partial: "filters/radio_filter", locals: { @@ -7,8 +22,14 @@ options: collection_year_radio_options, label: "Which financial year do you want to download data for?", category: "years", + size: "l", } %> + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> <%= govuk_button_link_to "Cancel", lettings_logs_path, secondary: true %> diff --git a/app/views/filters/sales_log_filters/assigned_to.html.erb b/app/views/filters/sales_log_filters/assigned_to.html.erb index 56d45e619..569f6fa59 100644 --- a/app/views/filters/sales_log_filters/assigned_to.html.erb +++ b/app/views/filters/sales_log_filters/assigned_to.html.erb @@ -1,4 +1,4 @@ -<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_assigned_to_sales_logs_path do |f| %> <%= render partial: "filters/radio_filter", locals: { f:, @@ -17,10 +17,16 @@ }, label: "Assigned to", category: "assigned_to", + size: "l", } %> + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: request.params["search"], codes_only: request.params["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 index 98de78864..d4c29a429 100644 --- a/app/views/filters/sales_log_filters/managed_by.html.erb +++ b/app/views/filters/sales_log_filters/managed_by.html.erb @@ -1,4 +1,4 @@ -<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_manaed_by_sales_logs_path do |f| %> <%= render partial: "filters/radio_filter", locals: { f:, options: { @@ -15,9 +15,16 @@ }, label: "Reported by", category: "managing_organisation_select", + size: "l", } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: request.params["search"], codes_only: request.params["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 index 86143c0ad..0481fcc7c 100644 --- a/app/views/filters/sales_log_filters/owned_by.html.erb +++ b/app/views/filters/sales_log_filters/owned_by.html.erb @@ -1,4 +1,4 @@ -<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_owned_by_sales_logs_path do |f| %> <%= render partial: "filters/radio_filter", locals: { f:, options: { @@ -15,9 +15,16 @@ }, label: "Owned by", category: "owning_organisation_select", + size: "l", } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: request.params["search"], codes_only: request.params["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 index 3a7dea248..476df83b1 100644 --- a/app/views/filters/sales_log_filters/status.html.erb +++ b/app/views/filters/sales_log_filters/status.html.erb @@ -1,13 +1,20 @@ -<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> +<%= form_with html: { method: :get }, url: filters_update_status_sales_logs_path do |f| %> <%= render partial: "filters/checkbox_filter", locals: { f:, options: status_filters, label: "Status", category: "status", + size: "l", } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> - <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: @search_term, codes_only: @codes_only), secondary: true %> + <%= govuk_button_link_to "Cancel", csv_download_sales_logs_path(search: request.params["search"], codes_only: request.params["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 index f258004c5..2f1e339b8 100644 --- a/app/views/filters/sales_log_filters/years.html.erb +++ b/app/views/filters/sales_log_filters/years.html.erb @@ -1,5 +1,19 @@ -<%= form_with model: @filter_form, url: filters_sales_logs_path(search: @search_term, codes_only: @codes_only) do |f| %> - <%= f.govuk_error_summary %> +<%= form_with html: { method: :get }, url: filters_update_years_sales_logs_path do |f| %> <% if params["error"].present? %> +
+
+

+ There is a problem +

+ +
+
+ <% end %> <%= render partial: "filters/radio_filter", locals: { @@ -7,8 +21,14 @@ options: collection_year_radio_options, label: "Which financial year do you want to download data for?", category: "years", + size: "l", } %> + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + <%= f.hidden_field :codes_only, value: request.params["codes_only"] %> +
<%= f.govuk_submit "Save changes" %> <%= govuk_button_link_to "Cancel", sales_logs_path, secondary: true %> diff --git a/config/routes.rb b/config/routes.rb index af3422431..99d41e809 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -216,8 +216,8 @@ Rails.application.routes.draw do %w[years status needstype assigned-to owned-by managed-by].each do |filter| get "filters/#{filter}", to: "lettings_logs_filters##{filter.underscore}" + get "filters/update_#{filter}", to: "lettings_logs_filters#update_#{filter.underscore}" end - post "filters", to: "lettings_logs_filters#update" resources :bulk_upload_lettings_logs, path: "bulk-upload-logs", only: %i[show update] do collection do @@ -286,8 +286,8 @@ Rails.application.routes.draw do %w[years status assigned-to owned-by managed-by].each do |filter| get "filters/#{filter}", to: "sales_logs_filters##{filter.underscore}" + get "filters/update_#{filter}", to: "sales_logs_filters#update_#{filter.underscore}" end - post "filters", to: "sales_logs_filters#update" resources :bulk_upload_sales_logs, path: "bulk-upload-logs" do collection do diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index d0d7c0942..f4611cc35 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -133,10 +133,8 @@ RSpec.describe "Lettings Log Features" do 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-#{lettings_log.form.start_date.year}-field", allow_label_click: true) + choose("years-#{lettings_log.form.start_date.year}-field", allow_label_click: true) click_button("Save changes") expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=false&search=1") @@ -148,10 +146,9 @@ RSpec.describe "Lettings Log Features" do 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) + choose("years-#{lettings_log.form.start_date.year}-field", allow_label_click: true) click_button("Save changes") expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=true&search=1") @@ -183,14 +180,14 @@ RSpec.describe "Lettings Log Features" do click_link("Change", href: "/lettings-logs/filters/needstype?search=1&codes_only=true") expect(page).to have_current_path("/lettings-logs/filters/needstype?search=1&codes_only=true") - check("forms-filter-form-needstypes-1-field", allow_label_click: true) + check("needstypes-1-field", allow_label_click: true) click_button("Save changes") expect(page).to have_current_path("/lettings-logs/csv-download?codes_only=true&search=1") 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) + check("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") diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb index 2fb68d854..1409982a8 100644 --- a/spec/features/sales_log_spec.rb +++ b/spec/features/sales_log_spec.rb @@ -159,10 +159,9 @@ RSpec.describe "Sales Log Features" do 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) + choose("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") @@ -174,10 +173,9 @@ RSpec.describe "Sales Log Features" do 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) + choose("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") @@ -209,13 +207,13 @@ RSpec.describe "Sales Log Features" do 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) + choose("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) + check("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")