Refactor

app/controllers/lettings_logs_filters_controller.rb

@@ -30,12 +30,16 @@ end
 private
 
 def filter_form_params
-  filter_params = params.permit(years: [], status: [], needstypes: [], assigned_to: [], owned_by: [], managed_by: [])
+  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 do |key, value|
+    params[key] = value
+  end
+  params["years"] = [params["years"]] if params["years"].present?
   filter_manager.session_filters
 end
 

app/helpers/filters_helper.rb

@@ -167,12 +167,12 @@ module FiltersHelper
 
   def check_your_answers_lettings_filters_list(session_filters)
     [
-      { label: "Collection year", value: session_filters["years"]&.to_sentence, path: filters_years_lettings_logs_path },
-      { label: "Status", value: session_filters["status"]&.to_sentence, path: filters_status_lettings_logs_path },
-      { label: "Needs type", value: session_filters["needstypes"]&.to_sentence, path: filters_needstype_lettings_logs_path },
-      { label: "Assigned to", value: session_filters["assigned_to"]&.to_s, path: filters_assigned_to_lettings_logs_path },
-      { label: "Owned by", value: session_filters["owning_organisation"]&.to_s, path: filters_owned_by_lettings_logs_path },
-      { label: "Managed by", value: session_filters["managing_organisation"]&.to_s, path: filters_managed_by_lettings_logs_path },
+      { label: "Collection year", value: formatted_years_filter(session_filters), path: filters_years_lettings_logs_path },
+      { label: "Status", value: formatted_status_filter(session_filters), path: filters_status_lettings_logs_path },
+      { label: "Needs type", value: formatted_needstype_filter(session_filters), path: filters_needstype_lettings_logs_path },
+      { label: "Assigned to", value: formatted_assigned_to_filter(session_filters), path: filters_assigned_to_lettings_logs_path },
+      { label: "Owned by", value: formatted_owned_by_filter(session_filters), path: filters_owned_by_lettings_logs_path },
+      { label: "Managed by", value: formatted_managed_by_filter(session_filters), path: filters_managed_by_lettings_logs_path },
     ]
   end
 
@@ -203,4 +203,47 @@ private
   def year_combo(year)
     "#{year}/#{year - 2000 + 1}"
   end
+
+  def formatted_years_filter(session_filters)
+    return unanswered_value if session_filters["years"].blank?
+
+    session_filters["years"].map { |year| year_combo(year.to_i) }.to_sentence
+  end
+
+  def formatted_status_filter(session_filters)
+    return unanswered_value if session_filters["status"].blank?
+
+    session_filters["status"].map { |status| status_filters[status] }.to_sentence
+  end
+
+  def formatted_needstype_filter(session_filters)
+    return unanswered_value if session_filters["needstypes"].blank?
+
+    session_filters["needstypes"].map { |needstype| needstype_filters[needstype] }.to_sentence
+  end
+
+  def formatted_assigned_to_filter(session_filters)
+    return unanswered_value if session_filters["assigned_to"].blank?
+    return "All" if session_filters["assigned_to"].include?("all")
+    return "You" if session_filters["assigned_to"].include?("you")
+
+    user = User.find(session_filters["user"].first)
+    "#{user.name} (#{user.email})"
+  end
+
+  def formatted_owned_by_filter(session_filters)
+    return "All" if session_filters["owning_organisation"].blank? || session_filters["owning_organisation"]&.include?("all")
+
+    Organisation.find(session_filters["owning_organisation"].first)&.name
+  end
+
+  def formatted_managed_by_filter(session_filters)
+    return "All" if session_filters["managing_organisation"].blank? || session_filters["managing_organisation"].include?("all")
+
+    Organisation.find(session_filters["managing_organisation"].first)&.name
+  end
+
+  def unanswered_value
+    "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
+  end
 end

app/models/forms/filter_form.rb

@@ -3,7 +3,7 @@ module Forms
     include ActiveModel::Model
     include ActiveModel::Validations
 
-    attr_accessor :years, :status, :needstypes, :assigned_to, :owned_by, :managed_by
+    attr_accessor :years, :status, :needstypes, :assigned_to, :user, :owning_organisation_select, :owning_organisation, :managing_organisation_select, :managing_organisation
 
     validates :years, presence: true
   end

app/services/filter_manager.rb

@@ -84,7 +84,7 @@ class FilterManager
   def deserialize_filters_from_session(specific_org)
     current_filters = session[session_name_for(filter_type)]
     new_filters = if current_filters.present?
-                    JSON.parse(current_filters).transform_values { |value| value.is_a?(Array) ? value.reject(&:empty?) : value }
+                    JSON.parse(current_filters).transform_values { |value| value.is_a?(Array) ? value.reject(&:blank?) : value }
                   else
                     {}
                   end

app/views/filters/_checkbox_filter.html.erb

@@ -2,7 +2,6 @@
   <% 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 %>

app/views/filters/_radio_filter.html.erb

@@ -3,7 +3,6 @@
     <%= f.govuk_radio_button category, key.to_s,
       label: { text: option[:label] },
       checked: filter_selected?(category, key, @filter_type),
-      name: "#{category}[]",
       size: "s" do %>
         <% if option[:conditional_filter] %>
           <%= render partial: "filters/#{option[:conditional_filter][:type]}_filter", locals: {

app/views/filters/_select_filter.html.erb

@@ -1,6 +1,5 @@
 <%= 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 %>"