Skip to content

Commit

Permalink
[F] Improve filterable / ordering
Browse files Browse the repository at this point in the history
* Add support for a default scope and auto-detecting based on whether
  or not the model is orderable, and has default scope method defined
* Detect whether the order param was applied, and if not, apply the
  default scope
* Expose the ability to set the default scope for projects in settings
  using the same values as ProjectCollections
* Clean up some of the project collection sorting logic not to be so
  dependent on raw strings and instead actual programmatic calls
* Use sort title for project collections and universally when sorting
  projects by title
* Blacklist some params that should not be included in filters ever
  for any reason

Resolves RET-1727
  • Loading branch information
scryptmouse authored and zdavis committed Aug 20, 2024
1 parent 6fbab30 commit 89c01a5
Show file tree
Hide file tree
Showing 20 changed files with 352 additions and 44 deletions.
1 change: 1 addition & 0 deletions api/app/models/application_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ApplicationRecord < ActiveRecord::Base
include ClassyEnum::ActiveRecord
include ArelHelpers
include DetectsSpam
include LazyOrdering
include SliceWith
include ValuesAt
include WithAdvisoryLock::Concern
Expand Down
34 changes: 33 additions & 1 deletion api/app/models/concerns/filterable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,21 @@
#
# @see Filtering::Apply
# @see Filtering::Applicator
# @see Filtering::Config
module Filterable
extend ActiveSupport::Concern

class_methods do
include LazyOrdering

included do
extend Dry::Core::ClassAttributes

defines :filtering_config, type: Filtering::Types.Instance(Filtering::Config)

filtering_config Filtering::Config.new(self)
end

module ClassMethods
# @param [Hash] params
# @param [ActiveRecord::Relation] scope
# @param [User, nil] user
Expand All @@ -23,5 +34,26 @@ def filtered(params, scope: all, user: nil, skip_pagination: false)
def apply_filtering_loads
all
end

# @param [Hash] options (@see Filtering::Config for options)
# @return [void]
def configure_filtering!(**options)
filtering_config Filtering::Config.new(self, **options)
end

def filtering_recalculate_available_scopes!
filtering_config.recalculate_available_scopes!
rescue NameError
# :nocov:
# intentionally left blank, only occurs in CI
# :nocov:
end

# @api private
def singleton_method_added(name)
super

filtering_recalculate_available_scopes!
end
end
end
55 changes: 55 additions & 0 deletions api/app/models/concerns/lazy_ordering.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# frozen_string_literal: true

module LazyOrdering
extend ActiveSupport::Concern

SimpleSortDirection = Dry::Types["coercible.string"].default("asc").enum("asc", "desc").fallback("asc")

module ClassMethods
# @param [Symbol] column
# @return [ActiveRecord::Relation]
def lazily_order(column, raw_direction = :asc)
base = lazy_order_scope_for column

expr = lazy_order_expr_for(column, raw_direction: raw_direction)

base.order(expr)
end

def lazy_order_column_for(column)
method_name = :"#{column}_order_column"

if respond_to?(method_name)
public_send(method_name)
else
column
end.then { |expr| arel_attrify(expr) }
end

def lazy_order_expr_for(column, raw_direction: "asc")
direction = ::Filtering::Types::SortDirection[raw_direction]

method_name = :"#{column}_order_expression"

if respond_to?(method_name)
public_send(method_name, direction: direction)
else
attr = lazy_order_column_for column

attr.public_send(direction)
end
end

# @param [Symbol] column
# @return [ActiveRecord::Relation]
def lazy_order_scope_for(column)
scope_name = :"prepare_order_for_#{column}"

if respond_to?(scope_name)
all.public_send(scope_name)
else
all
end
end
end
end
15 changes: 15 additions & 0 deletions api/app/models/concerns/project_ordering.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

# Shared logic for
module ProjectOrdering
extend ActiveSupport::Concern

ALLOWED_SORT_KEYS = %w(created_at updated_at publication_date title).freeze
ALLOWED_SORT_DIRECTIONS = %w(asc desc).freeze

ALLOWED_SORT_MAPPING = Filtering::SortMapping.from(*ALLOWED_SORT_KEYS)

ALLOWED_SORT_VALUES = ALLOWED_SORT_MAPPING.keys.freeze

DEFAULT_COLLECTION_SORT_VALUE = "created_at_desc"
end
41 changes: 39 additions & 2 deletions api/app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class Project < ApplicationRecord
include Metadata
include HasFormattedAttributes
include HasSortTitle
include ProjectOrdering
include WithPermittedUsers
include Sluggable
include SearchIndexable
Expand Down Expand Up @@ -76,7 +77,7 @@ class Project < ApplicationRecord
has_many :favorites, as: :favoritable, dependent: :destroy, inverse_of: :favoritable
has_many :events, -> { order "events.created_at DESC" }, dependent: :destroy,
inverse_of: :project
has_many :resources, -> { with_order }, dependent: :destroy, inverse_of: :project
has_many :resources, -> { in_default_order }, dependent: :destroy, inverse_of: :project
has_many :resource_collections, dependent: :destroy, inverse_of: :project
has_many :collection_resources, through: :resource_collections, inverse_of: :project
has_many :project_subjects, dependent: :destroy, inverse_of: :project
Expand Down Expand Up @@ -191,7 +192,11 @@ class Project < ApplicationRecord
.where(is_same_project.and(in_same_collection))
end)

scope :with_order, ->(by = nil) { by.present? ? order(by) : order(:sort_title, :title) }
scope :in_default_order, -> { in_specific_order(Settings.default_project_sort, mode: :standard) }

scope :in_title_order, -> { lazily_order(:title, :asc) }

scope :with_order, ->(by = nil) { by.present? ? order(by) : in_default_order }

scope :by_standalone_mode_enforced, ->(enforced) { to_boolean(enforced) ? standalone_enforced : standalone_unforced }

Expand Down Expand Up @@ -461,6 +466,38 @@ def build_update_ability_scope_for(user = nil)
where arel_with_roles_for(user, RoleName.for_project_update)
end

# @param [:collection, :standard] mode
def in_default_order_for(mode)
case mode
when :collection
lazily_order(:created_at, :desc)
else
lazily_order(:title, :asc)
end
end

# @param [String] value (@see ProjectOrdering::ALLOWED_SORT_VALUES)
# @return [ActiveRecord::Relation]
def in_specific_order(value, mode: :standard)
return in_default_order_for(mode) unless value.in?(ProjectOrdering::ALLOWED_SORT_VALUES)

mapping = ProjectOrdering::ALLOWED_SORT_MAPPING.fetch(value)

lazily_order(mapping.property, mapping.direction).then do |scope|
next scope if mapping.property == "title"

scope.order(arel_table[:title].asc.nulls_last)
end
end

# @param ["asc", "desc"] direction
# @return [Arel]
def title_order_expression(direction: "asc")
direction = Filtering::Types::SortDirection[direction]

%i[sort_title title].map { |attr| arel_table[attr].public_send(direction) }
end

private

# rubocop:disable Metrics/AbcSize
Expand Down
41 changes: 21 additions & 20 deletions api/app/models/project_collection.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
class ProjectCollection < ApplicationRecord

# Constants
ALLOWED_SORT_KEYS = %w(created_at updated_at publication_date title).freeze
ALLOWED_SORT_DIRECTIONS = %w(asc desc).freeze
# frozen_string_literal: true

# Concerns
class ProjectCollection < ApplicationRecord
include Authority::Abilities
include Attachments
include Entitleable
include HasFormattedAttributes
include Filterable
include Attachments
include TrackedCreator
include Authority::Abilities
include HasFormattedAttributes
include ProjectOrdering
include SerializedAbilitiesFor
include Taggable
include Sluggable
include TrackedCreator
include Taggable
include WithProjectCollectionLayout

# Attachments
Expand Down Expand Up @@ -43,11 +40,11 @@ class ProjectCollection < ApplicationRecord
scope :with_projects, lambda { |presence|
where(id: CollectionProject.select(:project_collection_id)) if presence.present?
}
scope :with_order, lambda { |by|
return order(position: :asc) unless by.present?

order(by)
}
scope :in_default_order, -> { order(position: :asc) }

scope :with_order, ->(by) { by.present? ? order(by) : in_default_order }

scope :after_homepage_start_date, lambda { |date|
no_date = arel_table[:homepage_end_date].eq(nil)
after_date = arel_table[:homepage_start_date].lteq(date)
Expand Down Expand Up @@ -81,12 +78,16 @@ class ProjectCollection < ApplicationRecord
allow_nil: true
validate :valid_homepage_start_date!, :valid_homepage_end_date!

# Introspection helper
#
# @api private
# @return
def project_sorting
column, _delimiter, direction = sort_order.rpartition "_"
column = "created_at" unless ALLOWED_SORT_KEYS.include? column
direction = "desc" unless ALLOWED_SORT_DIRECTIONS.include? direction
Project.in_specific_order(sort_order, mode: :collection).order_values.each_with_object([]) do |ordering, tuples|
next unless ordering.respond_to?(:expr) && ordering.respond_to?(:direction)

"projects.#{column} #{direction}, projects.title asc NULLS LAST"
tuples << [ordering.try(:expr).try(:name).to_s, ordering.try(:direction).to_s]
end
end

def manually_sorted?
Expand Down Expand Up @@ -120,7 +121,7 @@ def valid_homepage_end_date!
def reset_sort_order!
return unless smart? && manually_sorted?

self.sort_order = "created_at_desc"
self.sort_order = DEFAULT_COLLECTION_SORT_VALUE
end

def cache_collection_projects!
Expand Down
3 changes: 2 additions & 1 deletion api/app/models/reading_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class ReadingGroup < ApplicationRecord

scope :by_keyword, ->(value) { build_keyword_scope(value) if value.present? }
scope :with_sort_order, ->(value) { build_sort_order_scope(value) }
scope :with_order, ->(by = nil) { by.present? ? order(by) : order(created_at: :desc) }
scope :in_default_order, -> { order(created_at: :desc) }
scope :with_order, ->(by = nil) { by.present? ? order(by) : in_default_order }
scope :non_public, -> { where.not(privacy: "public") }
scope :visible_to_public, -> { where(privacy: "public") }
scope :visible_to, ->(user) do
Expand Down
8 changes: 4 additions & 4 deletions api/app/models/resource.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# A resource is any asset our source document that is associated with a text.
class Resource < ApplicationRecord

Expand Down Expand Up @@ -106,11 +108,9 @@ class Resource < ApplicationRecord
.where("collection_resources.resource_collection_id = ?", id)
.order("collection_resources.position ASC")
}
scope :with_order, lambda { |by = nil|
return order(:sort_title, :created_at) unless by.present?

order(by)
}
scope :in_default_order, -> { order(sort_title: :asc, created_at: :asc) }
scope :with_order, ->(by = nil) { by.present? ? order(by) : in_default_order }

# Callbacks
before_validation :update_kind, :set_fingerprint!
Expand Down
4 changes: 4 additions & 0 deletions api/app/models/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ def current
end
end

def default_project_sort
current.general.default_project_sort
end

# Check if we {.update_from_environment? should update from the environment}
# and {#update_from_environment! do so}.
# @return [void]
Expand Down
8 changes: 2 additions & 6 deletions api/app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,9 @@ class User < ApplicationRecord
with_parsed_name :first_name, :last_name
has_secure_password

# Scopes
scope :by_email, ->(email) { where(arel_table[:email].matches("#{email}%")) if email.present? }
scope :with_order, ->(by) do
return order(:last_name, :first_name) unless by.present?

order(by)
end
scope :in_default_order, -> { order(:last_name, :first_name) }
scope :with_order, ->(by) { by.present? ? order(by) : in_default_order }
scope :by_role, ->(role) { RoleName[role].then { |r| with_role(r.to_sym) if r.present? } }
scope :by_cached_role, ->(*role) { where(role: role) }
scope :email_confirmed, -> { where.not(email_confirmed_at: nil) }
Expand Down
4 changes: 2 additions & 2 deletions api/app/operations/filtering/apply.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ class Apply
# @param [Boolean] skip_pagination
# @return [Searchkick::Relation]
# @return [ActiveRecord::Relation] with kaminari data from {.by_pagination}.
def call(raw_params, scope:, user:, skip_pagination: false)
Filtering::Applicator.new(raw_params, scope: scope, user: user, skip_pagination: skip_pagination).call
def call(raw_params, scope:, user:, model: scope.model, skip_pagination: false)
Filtering::Applicator.new(raw_params, model: model, scope: scope, user: user, skip_pagination: skip_pagination).call
end
end
end
1 change: 1 addition & 0 deletions api/app/serializers/v1/setting_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class SettingSerializer < ManifoldSerializer

contact_email: Types::Serializer::Email.optional,
copyright: Types::String.optional,
default_project_sort: Types::String.optional,
default_publisher: Types::String.optional,
default_publisher_place: Types::String.optional,
facebook: Types::String.optional,
Expand Down
Loading

0 comments on commit 89c01a5

Please sign in to comment.