Move investments search form partial to a component

As a bonus, we now have a few less helper methods :).

app/assets/stylesheets/admin.scss

@@ -956,34 +956,6 @@ table {
 // 10. Budgets
 // -----------------
 
-.advanced-filters {
-  margin: $line-height 0;
-
-  @include breakpoint(medium) {
-    margin: calc(#{$line-height} / 2) 0 0;
-  }
-}
-
-.advanced-filters-content {
-  background: $highlight;
-  clear: both;
-  margin: $line-height 0;
-  padding: calc(#{$line-height} / 2);
-
-  .filter {
-    display: inline-block;
-    margin: 0 calc(#{$line-height} / 2);
-
-    label {
-      font-weight: normal;
-      margin: 0;
-    }
-  }
-
-  .button {
-    margin-top: calc(#{$line-height} / 2);
-  }
-}
 
 .columns-selector {
 

app/assets/stylesheets/admin/budget_investments/search_form.scss

@@ -0,0 +1,30 @@
+.admin-budget-investments-search-form {
+  .advanced-filters {
+    margin: $line-height 0;
+
+    @include breakpoint(medium) {
+      margin: calc(#{$line-height} / 2) 0 0;
+    }
+  }
+
+  .advanced-filters-content {
+    background: $highlight;
+    clear: both;
+    margin: $line-height 0;
+    padding: calc(#{$line-height} / 2);
+
+    .filter {
+      display: inline-block;
+      margin: 0 calc(#{$line-height} / 2);
+
+      label {
+        font-weight: normal;
+        margin: 0;
+      }
+    }
+
+    .button {
+      margin-top: calc(#{$line-height} / 2);
+    }
+  }
+}

app/components/admin/budget_investments/search_form_component.html.erb

@@ -1,5 +1,5 @@
 <% init_advanced_menu %>
-<%= form_tag(admin_budget_budget_investments_path(budget: @budget), method: :get, enforce_utf8: false) do %>
+<%= form_tag(admin_budget_budget_investments_path(budget), method: :get, enforce_utf8: false, class: "admin-budget-investments-search-form") do %>
   <div class="small-12 column">
     <%= link_to "#advanced_filters_content",
                 data: { toggle: "advanced_filters" },
@@ -34,28 +34,28 @@
 
   <div class="small-12 medium-3 column">
     <%= select_tag :administrator_id,
-                   options_for_select(admin_select_options(@budget), params[:administrator_id]),
+                   options_for_select(admin_select_options, params[:administrator_id]),
                    { prompt: t("admin.budget_investments.index.administrator_filter_all") } %>
   </div>
   <div class="small-12 medium-3 column">
     <%= select_tag :valuator_or_group_id,
-                   options_for_select(valuator_or_group_select_options(@budget), params[:valuator_or_group_id]),
+                   options_for_select(valuator_or_group_select_options, params[:valuator_or_group_id]),
                    { prompt: t("admin.budget_investments.index.valuator_filter_all") } %>
   </div>
   <div class="small-12 medium-3 column">
     <%= select_tag :heading_id,
-                   options_for_select(budget_heading_select_options(@budget), params[:heading_id]),
+                   options_for_select(budget_heading_select_options(budget), params[:heading_id]),
                    { prompt: t("admin.budget_investments.index.heading_filter_all") } %>
   </div>
   <div class="small-12 medium-3 column">
     <%= select_tag :tag_name,
-                   options_for_select(investment_tags_select_options(@budget, "valuation_tags"), params[:tag_name]),
+                   options_for_select(investment_tags_select_options("valuation_tags"), params[:tag_name]),
                    { prompt: t("admin.budget_investments.index.tags_filter_all") } %>
   </div>
 
   <div class="small-12 medium-3 column">
     <%= select_tag :milestone_tag_name,
-                   options_for_select(investment_tags_select_options(@budget, "milestone_tags"), params[:milestone_tag_name]),
+                   options_for_select(investment_tags_select_options("milestone_tags"), params[:milestone_tag_name]),
                    { prompt: t("admin.budget_investments.index.milestone_tags_filter_all") } %>
   </div>
 

app/components/admin/budget_investments/search_form_component.rb

@@ -0,0 +1,45 @@
+class Admin::BudgetInvestments::SearchFormComponent < ApplicationComponent
+  attr_reader :budget
+  use_helpers :budget_heading_select_options
+
+  def initialize(budget)
+    @budget = budget
+  end
+
+  private
+
+    def init_advanced_menu
+      params[:advanced_filters] = [] unless params[:advanced_filters]
+    end
+
+    def advanced_menu_visibility
+      if params[:advanced_filters].empty? &&
+         params["min_total_supports"].blank? &&
+         params["max_total_supports"].blank?
+        "hide"
+      else
+        ""
+      end
+    end
+
+    def admin_select_options
+      budget.administrators.with_user.map { |v| [v.description_or_name, v.id] }.sort_by { |a| a[0] }
+    end
+
+    def valuator_or_group_select_options
+      valuator_group_select_options + valuator_select_options
+    end
+
+    def valuator_group_select_options
+      ValuatorGroup.order("name ASC").map { |g| [g.name, "group_#{g.id}"] }
+    end
+
+    def valuator_select_options
+      budget.valuators.order("description ASC").order("users.email ASC").includes(:user)
+            .map { |v| [v.description_or_email, "valuator_#{v.id}"] }
+    end
+
+    def investment_tags_select_options(context)
+      budget.investments.tags_on(context).order(:name).pluck(:name)
+    end
+end

app/helpers/admin_budget_investments_helper.rb

@@ -1,36 +0,0 @@
-module AdminBudgetInvestmentsHelper
-  def advanced_menu_visibility
-    if params[:advanced_filters].empty? &&
-       params["min_total_supports"].blank? &&
-       params["max_total_supports"].blank?
-      "hide"
-    else
-      ""
-    end
-  end
-
-  def init_advanced_menu
-    params[:advanced_filters] = [] unless params[:advanced_filters]
-  end
-
-  def admin_select_options(budget)
-    budget.administrators.with_user.map { |v| [v.description_or_name, v.id] }.sort_by { |a| a[0] }
-  end
-
-  def valuator_or_group_select_options(budget)
-    valuator_group_select_options + valuator_select_options(budget)
-  end
-
-  def valuator_select_options(budget)
-    budget.valuators.order("description ASC").order("users.email ASC").includes(:user)
-          .map { |v| [v.description_or_email, "valuator_#{v.id}"] }
-  end
-
-  def valuator_group_select_options
-    ValuatorGroup.order("name ASC").map { |g| [g.name, "group_#{g.id}"] }
-  end
-
-  def investment_tags_select_options(budget, context)
-    budget.investments.tags_on(context).order(:name).pluck(:name)
-  end
-end

app/views/admin/budget_investments/index.html.erb

@@ -8,7 +8,7 @@
 
 <h2 class="inline-block"><%= @budget.name %> - <%= t("admin.budget_investments.index.title") %></h2>
 
-<%= render "search_form" %>
+<%= render Admin::BudgetInvestments::SearchFormComponent.new(@budget) %>
 
 <%= render "/shared/filter_subnav", i18n_namespace: "admin.budget_investments.index" %>
 

spec/components/admin/budget_investments/search_form_component_spec.rb

@@ -1,19 +1,23 @@
 require "rails_helper"
 
-describe AdminBudgetInvestmentsHelper do
+describe Admin::BudgetInvestments::SearchFormComponent do
   describe "#admin_select_options" do
     it "includes administrators assigned to the budget" do
       admin = create(:administrator, user: create(:user, username: "Winston"))
       budget = create(:budget, administrators: [admin])
 
-      expect(admin_select_options(budget)).to eq([["Winston", admin.id]])
+      render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget)
+
+      expect(page).to have_select options: ["All administrators", "Winston"]
     end
 
     it "does not include other administrators" do
       create(:administrator, user: create(:user, username: "Winston"))
       budget = create(:budget, administrators: [])
 
-      expect(admin_select_options(budget)).to be_empty
+      render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget)
+
+      expect(page).to have_select options: ["All administrators"]
     end
   end
 
@@ -22,14 +26,18 @@ describe AdminBudgetInvestmentsHelper do
       valuator = create(:valuator, description: "Kodogo")
       budget = create(:budget, valuators: [valuator])
 
-      expect(valuator_select_options(budget)).to eq([["Kodogo", "valuator_#{valuator.id}"]])
+      render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget)
+
+      expect(page).to have_select options: ["All valuators", "Kodogo"]
     end
 
     it "does not include other valuators" do
       create(:valuator, description: "Kodogo")
       budget = create(:budget, valuators: [])
 
-      expect(valuator_select_options(budget)).to be_empty
+      render_inline Admin::BudgetInvestments::SearchFormComponent.new(budget)
+
+      expect(page).to have_select options: ["All valuators"]
     end
   end
 end