Refactor how Rails handles filter categories passed to and from Vue

CHANGELOG.md

@@ -19,6 +19,8 @@ This changelog also serves to acknowledge the incredible people who've contribut
 ### Other bugfixes
 * Fix breakage in claiming a contribution #946
 
+### Development notes
+* Rework how contribution filtering works under the hood #546
 
 ## [0.5.0] - 2021-04-08
 ### Enhancements

app/blueprints/contribution_blueprint.rb

@@ -16,13 +16,19 @@ class ContributionBlueprint < Blueprinter::Base
     contribution.created_at.to_f * 1000 # Javascript wants miliseconds, not seconds
   end
   field :type, name: :contribution_type
-  field :profile_path do |contribution, options|
-    options[:profile_path]&.call(contribution.person_id)
-  end
+
   field :view_path do |contribution, options|
-    options[:view_path]&.call(contribution.id)
+    routes.contribution_path(contribution.id) if options[:show_view_path]
   end
   field :match_path do |contribution, options|
-    options[:match_path]&.call(contribution.id)
+    routes.match_listing_listing_path(contribution.id) if options[:show_match_path]
+  end
+
+  class << self
+    private
+
+    def routes
+      Rails.application.routes.url_helpers
+    end
   end
 end

app/blueprints/filter_option_blueprint.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+# FilterOptionsBlueprint emits a structure that the browse Vue page can turn
+# into checkboxes or other form UI elements to filter a list of contributions
+#
+# We assume that there's a top level type of filter (e.g ContactMethodFilter)
+# that then references a series of these FilterOptionBlueprint objects for
+# each element (e.g. each available contact method)
+class FilterOptionBlueprint < Blueprinter::Base
+  # The identifier here needs to be in a format that the BrowseFilter can then
+  # interpret and use to filter results
+  identifier :id do |filter_option, _options|
+    "#{filter_option.class}[#{filter_option.id}]"
+  end
+
+  # This is currently used as a display name so people can understand
+  # which each option represents
+  field :name
+end

app/controllers/contributions_controller.rb

@@ -3,11 +3,21 @@
 class ContributionsController < ApplicationController
   before_action :authenticate_user!, unless: :peer_to_peer_mode?
   skip_after_action :verify_policy_scoped
-
   # FIXME: this should probably be wrapped by a policy scope?
+  # Nomenclature note:
+  # Filter —
+  #   An object that handles the logic or action of filtering
+  # Filter Grouping —
+  #   A higher-level category or other grouping of filter options. Example: Contact Method can be
+  # a *filter grouping* that then has *filter options* for things like "email" or "text message"
+  # Filter Option —
+  #   An individual item that can be chosen to change what's filtered. Each *filter option* is
+  # associated to one and only one *filter grouping*
+
   def index
-    @filter_types = FilterTypeBlueprint.render([ContributionType, Category, ServiceArea, UrgencyLevel, ContactMethod])
-    filter = BrowseFilter.new(filter_params)
+    @filter_groupings = BrowseFilter.filter_groupings_json
+    # The BrowserFilter takes the result of the parameters from the filter checkboxes and returns a list of contributions
+    filter = BrowseFilter.new(allowed_params)
     @contributions = ContributionBlueprint.render(filter.contributions, contribution_blueprint_options)
     respond_to do |format|
       format.html
@@ -46,21 +56,11 @@ def peer_to_peer_mode?
   end
 
   def contribution_blueprint_options
-    options = {}
-    options[:view_path] = ->(id) { contribution_path(id) }
-    options
-  end
-
-  def filter_params
-    return {} unless allowed_params&.to_h.any?
-
-    allowed_params.to_h.filter { |key, _v| BrowseFilter::ALLOWED_PARAMS.keys.include? key }.tap do |hash|
-      hash.each_key { |key| hash[key] = hash[key].keys }
-    end
+    {show_view_path: true}
   end
 
   def allowed_params
-    @allowed_params ||= params.permit(:format, **BrowseFilter::ALLOWED_PARAMS)
+    params.permit(:format, BrowseFilter::ALLOWED_PARAMS)
   end
 
   def contribution

app/filters/base_filter.rb

@@ -0,0 +1,25 @@
+# This class has information about how we sort and filter groupings
+# and is capable of accepting a model scope and adding additional
+# `where` clauses to it in order to further filter the model scope
+
+# BaseFilter is more like an abstract parent class, but it also serves
+# as a NullObject version of the filter grouping, if you want one
+class BaseFilter
+  # This method should be overridden to return a hash with the following keys:
+  # * :name => a short string that the user will see that describes what type of filters these are
+  # * :filters => the output a call to FilterOptionBlueprint.render_as_hash that represent each filter option to check or uncheck
+  def self.filter_grouping
+    {}
+  end
+
+  attr_reader :parameters
+
+  def initialize(parameters)
+    @parameters = parameters
+  end
+
+  # By default, return the model scope unfiltered
+  def filter(scope)
+    scope
+  end
+end

app/filters/category_filter.rb

@@ -0,0 +1,17 @@
+class CategoryFilter < BaseFilter
+  def self.filter_grouping
+    {
+      name: "Categories",
+      # Currently only filtering by top-level categories
+      filter_options: FilterOptionBlueprint.render_as_hash(Category.roots)
+    }
+  end
+
+  def filter(scope)
+    return super unless parameters
+    scope.tagged_with(
+      Category.roots.where(id: parameters.keys).pluck('name'),
+      any: true
+    )
+  end
+end

app/filters/contact_method_filter.rb

@@ -0,0 +1,13 @@
+class ContactMethodFilter < BaseFilter
+  def self.filter_grouping
+    {
+      name: 'Contact Methods',
+      filter_options: FilterOptionBlueprint.render_as_hash(ContactMethod.enabled.distinct(:name))
+    }
+  end
+
+  def filter(scope)
+    return super unless parameters
+    scope.joins(:person).where(people: {preferred_contact_method: parameters.keys})
+  end
+end

app/filters/contribution_type_filter.rb

@@ -0,0 +1,24 @@
+class ContributionTypeFilter < BaseFilter
+  def self.filter_grouping
+    {name: 'Contribution Types', filter_options: [
+      {id: 'ContributionType[Ask]', name: 'Ask'},
+      {id: 'ContributionType[Offer]', name: 'Offer'}
+    ]}
+  end
+  ALL_ALLOWED_TYPES = ['Ask', 'Offer'].freeze
+
+  def filter(scope)
+    raise NotImplementedError.new(
+      # So far the best solution I've found for filtering scopes by contribution types would require
+      # using SQL UNIONs, which have no good support in Rails
+      "Can't filter an existing scope by contribution type. Use the `ContributionTypeFilter#scopes` method generate scopes for other filters"
+    )
+  end
+
+  def scopes
+    classes = parameters.blank? ? ALL_ALLOWED_TYPES : parameters.keys
+    classes.intersection(ALL_ALLOWED_TYPES).map do |type|
+      type.constantize.matchable
+    end
+  end
+end

app/filters/service_area_filter.rb

@@ -0,0 +1,13 @@
+class ServiceAreaFilter < BaseFilter
+  def self.filter_grouping
+    {
+      name: 'Service Areas',
+      filter_options: FilterOptionBlueprint.render_as_hash(ServiceArea.i18n)
+    }
+  end
+
+  def filter(scope)
+    return super unless parameters
+    scope.where(service_area_id: parameters.keys)
+  end
+end

app/javascript/pages/Browse.vue

@@ -11,7 +11,7 @@
       <section class="mt-3 mb-3">
         <div class="card">
           <div class="card-content">
-            <Filters :filterTypes="filterTypes" v-model="activeFilters" />
+            <Filters :filterGroupings="filterGroupings" v-model="activeFilters" />
           </div>
         </div>
       </section>
@@ -48,12 +48,12 @@ export default {
   },
   props: {
     contributions: {type: Array},
-    filterTypes: {type: Array, default: ()=>[]},
+    filterGroupings: {type: Array, default: ()=>[]},
     initialFilters: {
       type: Array,
       default: function () {
         return [].concat(
-          ...this.filterTypes.map((fType) => fType.filters.map((filter) => filter.id))
+          ...this.filterGroupings.map((fGrouping) => fGrouping.filter_options.map((filter_option) => filter_option.id))
         )
       },
     },

app/javascript/pages/browse/ContributionFetcher.js

@@ -14,7 +14,7 @@ export default class {
       .catch((error) => ({error: error, data: fallback}))
   }
   parsedFilters(filters = []) {
-    return filters.map((filter) => '' + filter + '=1').join('&')
+    return filters.map((filterOption) => '' + filterOption + '=1').join('&')
   }
   defaultFetch() {
     return typeof window === 'undefined' ? this.noopFetch : (args) => window.fetch(args)

app/javascript/pages/browse/Filters.vue

@@ -17,32 +17,32 @@
       <div class="columns">
         <b-collapse
           class="column"
-          v-for="(type, index) of filterTypes"
+          v-for="(filterGrouping, index) of filterGroupings"
           :key="index"
           :open="initialOpenStatus(index)"
         >
           <span class="subtitle is-5" slot="trigger" slot-scope="props">
-            {{ type.name }} <a>{{ props.open ? '-' : '+' }}</a>
+            {{ filterGrouping.name }} <a>{{ props.open ? '-' : '+' }}</a>
           </span>
           <b-checkbox
-            :id="`toggle-filters-${type.name}`"
-            @input="toggleFilters(type.filters)"
-            :value="filterTypeSelectAllValue(type.filters)"
-            :indeterminate="indeterminate(type.filters)"
+            :id="`toggle-filters-${filterGrouping.name}`"
+            @input="toggleFilters(filterGrouping.filter_options)"
+            :value="filterGroupingSelectAllValue(filterGrouping.filter_options)"
+            :indeterminate="indeterminate(filterGrouping.filter_options)"
           >
             Select all
           </b-checkbox>
           <ul class="mt-1">
-            <li v-for="filter of type.filters" :key="filter.id">
+            <li v-for="filterOption of filterGrouping.filter_options" :key="filterOption.id">
               <b-checkbox
-                :native-value="filter.id"
+                :native-value="filterOption.id"
                 :value="currentFilters"
                 @input="$emit('change', $event)"
               >
-                {{ filter.name }}
+                {{ filterOption.name }}
                 <MappedIconList
-                  :iconTypes="[{id: filter.name, name: filter.name}]"
-                  v-if="showIconsForFilter(filter.name)"
+                  :iconTypes="[{id: filterOption.name, name: filterOption.name}]"
+                  v-if="showIconsForFilter(filterOption.name)"
                   class="is-inline"
                 />
               </b-checkbox>
@@ -57,12 +57,12 @@
 <script>
 import MappedIconList from 'components/MappedIconList'
 
-// TODO: consider extracting a FilterType component.
+// TODO: consider extracting a FilterGrouping component.
 // see this comment: https://github.com/rubyforgood/mutual-aid/pull/799#pullrequestreview-554188100
 export default {
   components: {MappedIconList},
   props: {
-    filterTypes: {type: Array, default: () => []},
+    filterGroupings: {type: Array, default: () => []},
     currentFilters: {type: Array, default: () => []},
   },
   model: {
@@ -87,22 +87,22 @@ export default {
     showIconsForFilter(filterName) {
       return !!this.hasFilterIcons[filterName]
     },
-    currentFiltersForFilterType(filterIds) {
-      return this.currentFilters.filter((el) => filterIds.includes(el))
+    currentFiltersOptionsForFilterGrouping(filterOptionIds) {
+      return this.currentFilters.filter((el) => filterOptionIds.includes(el))
     },
-    filterTypeSelectAllValue(filters) {
+    filterGroupingSelectAllValue(filters) {
       let filterIds = filters.map((f) => f.id)
-      return this.currentFiltersForFilterType(filterIds).length == filterIds.length
+      return this.currentFiltersOptionsForFilterGrouping(filterIds).length == filterIds.length
     },
     indeterminate(filters) {
       let filterIds = filters.map((f) => f.id)
-      return this.currentFiltersForFilterType(filterIds).length == 0
+      return this.currentFiltersOptionsForFilterGrouping(filterIds).length == 0
         ? false
-        : this.currentFiltersForFilterType(filterIds).length < filterIds.length
+        : this.currentFiltersOptionsForFilterGrouping(filterIds).length < filterIds.length
     },
     toggleFilters(filters) {
       let filterIds = filters.map((f) => f.id)
-      if (this.currentFiltersForFilterType(filterIds).length < filterIds.length) {
+      if (this.currentFiltersOptionsForFilterGrouping(filterIds).length < filterIds.length) {
         this.$emit('change', [...new Set([...this.currentFilters, ...filterIds])])
       } else {
         this.$emit(
@@ -121,16 +121,16 @@ export default {
   },
   data() {
     return {
-      hasFilterIcons: this.filterTypes
-        .map((type) => type.filters)
-        .reduce(function (memo, filters) {
-          filters.map(function (filter) {
-            memo[filter.name] = MappedIconList.iconNameMapping[filter.name]
+      hasFilterIcons: this.filterGroupings
+        .map((fGrouping) => fGrouping.filter_options)
+        .reduce(function (memo, filterOptions) {
+          filterOptions.map(function (fOption) {
+            memo[fOption.name] = MappedIconList.iconNameMapping[fOption.name]
           })
           return memo
         }, {}),
       allFilters: [].concat(
-        ...this.filterTypes.map((fType) => fType.filters.map((filter) => filter.id))
+        ...this.filterGroupings.map((fGrouping) => fGrouping.filter_options.map((fOption) => fOption.id))
       ),
     }
   },