diff --git a/CHANGELOG.md b/CHANGELOG.md index 77139e5a9..ca0d6b367 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/blueprints/contribution_blueprint.rb b/app/blueprints/contribution_blueprint.rb index 657012c8e..eabba3bf4 100644 --- a/app/blueprints/contribution_blueprint.rb +++ b/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 diff --git a/app/blueprints/filter_option_blueprint.rb b/app/blueprints/filter_option_blueprint.rb new file mode 100644 index 000000000..9f9a62c38 --- /dev/null +++ b/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 diff --git a/app/blueprints/filter_type_blueprint.rb b/app/blueprints/filter_type_blueprint.rb deleted file mode 100644 index 3ced91bea..000000000 --- a/app/blueprints/filter_type_blueprint.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -# FilterTypeBlueprint -# -# The `.render` method will accept a class (usually an ActiveRecord::Model) or an array of classes -# and serialize the classes consistent with what the Filter component in Vue will expect. -# -# The class must have a `.as_filter_types` method or scope that returns a list of instances that -# respond to `#name` and `#id`. - -class FilterTypeBlueprint < Blueprinter::Base - identifier :name do |klass, _options| - klass.to_s.titleize.pluralize - end - - association :as_filter_types, name: :filters, blueprint: SubfilterTypeBlueprint -end \ No newline at end of file diff --git a/app/blueprints/subfilter_type_blueprint.rb b/app/blueprints/subfilter_type_blueprint.rb deleted file mode 100644 index 3a11c2105..000000000 --- a/app/blueprints/subfilter_type_blueprint.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -class SubfilterTypeBlueprint < Blueprinter::Base - identifier :id do |category, _options| - "#{category.class.to_s}[#{category.id}]" - end - field :name -end diff --git a/app/controllers/contributions_controller.rb b/app/controllers/contributions_controller.rb index 5bc79d2ff..43db3dee6 100644 --- a/app/controllers/contributions_controller.rb +++ b/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 diff --git a/app/filters/base_filter.rb b/app/filters/base_filter.rb new file mode 100644 index 000000000..093bc99d2 --- /dev/null +++ b/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 diff --git a/app/filters/category_filter.rb b/app/filters/category_filter.rb new file mode 100644 index 000000000..6f745171c --- /dev/null +++ b/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 diff --git a/app/filters/contact_method_filter.rb b/app/filters/contact_method_filter.rb new file mode 100644 index 000000000..f62dd7f69 --- /dev/null +++ b/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 diff --git a/app/filters/contribution_type_filter.rb b/app/filters/contribution_type_filter.rb new file mode 100644 index 000000000..569887caf --- /dev/null +++ b/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 diff --git a/app/filters/service_area_filter.rb b/app/filters/service_area_filter.rb new file mode 100644 index 000000000..4e6271ef2 --- /dev/null +++ b/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 diff --git a/app/javascript/pages/Browse.vue b/app/javascript/pages/Browse.vue index bb3258825..0f2723baf 100644 --- a/app/javascript/pages/Browse.vue +++ b/app/javascript/pages/Browse.vue @@ -11,7 +11,7 @@
- +
@@ -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)) ) }, }, diff --git a/app/javascript/pages/browse/ContributionFetcher.js b/app/javascript/pages/browse/ContributionFetcher.js index a95d17421..9740c3d70 100644 --- a/app/javascript/pages/browse/ContributionFetcher.js +++ b/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) diff --git a/app/javascript/pages/browse/Filters.vue b/app/javascript/pages/browse/Filters.vue index 1c05052fb..d11afa9bd 100644 --- a/app/javascript/pages/browse/Filters.vue +++ b/app/javascript/pages/browse/Filters.vue @@ -17,32 +17,32 @@
- {{ type.name }} {{ props.open ? '-' : '+' }} + {{ filterGrouping.name }} {{ props.open ? '-' : '+' }} Select all