Skip to content

Commit

Permalink
Merge pull request #15675 from opf/fix/replace_merge_in_query
Browse files Browse the repository at this point in the history
Fix/replace merge in query
  • Loading branch information
klaustopher authored May 28, 2024
2 parents 9b10bd6 + 047306a commit 41a1a95
Show file tree
Hide file tree
Showing 62 changed files with 359 additions and 292 deletions.
23 changes: 10 additions & 13 deletions app/models/queries/base_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,31 +187,28 @@ def context
self
end

def apply_filters(scope)
filters.each do |filter|
scope = scope.merge(filter.scope)
def apply_filters(query_scope)
filters.inject(query_scope) do |scope, filter|
filter.apply_to(scope)
end

scope
end

def apply_orders(scope)
build_orders.each do |order|
scope = scope.merge(order.scope)
def apply_orders(query_scope)
query_scope = build_orders.inject(query_scope) do |scope, order|
order.apply_to(scope)
end

# To get deterministic results, especially when paginating (limit + offset)
# an order needs to be prepended that is ensured to be
# different between all elements.
# Without such a criteria, results can occur on multiple pages.
already_ordered_by_id?(scope) ? scope : scope.order(id: :desc)
already_ordered_by_id?(query_scope) ? query_scope : query_scope.order(id: :desc)
end

def apply_group_by(scope)
return scope if group_by.nil?
def apply_group_by(query_scope)
return query_scope if group_by.nil?

scope
.merge(group_by.scope)
group_by.apply_to(query_scope)
.order(group_by.name)
end

Expand Down
12 changes: 6 additions & 6 deletions app/models/queries/filters/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,12 @@ def default_operator
type_strategy.default_operator_class
end

def scope
scope = model.where(where)
scope = scope.from(from) if from
scope = scope.joins(joins) if joins
scope = scope.left_outer_joins(left_outer_joins) if left_outer_joins
scope
def apply_to(query_scope)
query_scope = query_scope.where(where)
query_scope = query_scope.from(from) if from
query_scope = query_scope.joins(joins) if joins
query_scope = query_scope.left_outer_joins(left_outer_joins) if left_outer_joins
query_scope
end

def self.key
Expand Down
2 changes: 1 addition & 1 deletion app/models/queries/filters/empty_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def human_name
# deactivating superclass validation
def validate_inclusion_of_operator; end

def scope
def apply_to(_query_scope)
context.default_scope
end

Expand Down
13 changes: 3 additions & 10 deletions app/models/queries/filters/not_existing_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,9 @@ def to_hash
}
end

def scope
# TODO: remove switch once the WP query is a
# subclass of Queries::Base
model = if context.respond_to?(:model)
context.model
else
WorkPackage
end

model.unscoped
def apply_to(query_scope)
# No change to the query scope whatsoever since the filter does not exist.
query_scope
end

def attributes_hash
Expand Down
7 changes: 3 additions & 4 deletions app/models/queries/group_bys/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ def association_class
nil
end

def scope
scope = model
scope = model.joins(joins) if joins
group_by scope
def apply_to(query_scope)
query_scope = query_scope.joins(joins) if joins
group_by query_scope
end

def name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ def self.key

private

def order
def order(scope)
order_string = "groups_users.lastname"

order_string += " DESC" if direction == :desc

model.order(order_string)
scope.order(order_string)
end

def joins
Expand Down
4 changes: 2 additions & 2 deletions app/models/queries/individual_principals/orders/name_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ def self.key

private

def order
ordered = model.order_by_name
def order(scope)
ordered = scope.order_by_name

if direction == :desc
ordered = ordered.reverse_order
Expand Down
7 changes: 1 addition & 6 deletions app/models/queries/members/filters/group_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ class Queries::Members::Filters::GroupFilter < Queries::Members::Filters::Member
include Queries::Filters::Shared::GroupFilter

def joins
nil
end

def scope
scope = model.joins(:principal)
scope.where(where)
:principal
end
end
4 changes: 2 additions & 2 deletions app/models/queries/members/orders/email_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ def joins

private

def order
def order(scope)
with_raise_on_invalid do
order_string = "NULLIF(mail, '')"
order_string += " DESC" if direction == :desc
order_string += " NULLS LAST"

model.order(Arel.sql(order_string))
scope.order(Arel.sql(order_string))
end
end
end
4 changes: 2 additions & 2 deletions app/models/queries/members/orders/name_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def joins

protected

def order
model.merge Principal.ordered_by_name(desc: direction == :desc)
def order(scope)
scope.merge Principal.ordered_by_name(desc: direction == :desc)
end
end
4 changes: 2 additions & 2 deletions app/models/queries/notifications/orders/project_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ def joins

protected

def order
def order(scope)
order_string = "projects.name"
order_string += " DESC" if direction == :desc

model.order(order_string)
scope.order(order_string)
end
end
14 changes: 7 additions & 7 deletions app/models/queries/orders/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ def self.key
raise NotImplementedError
end

def scope
scope = order
scope = scope.joins(joins) if joins
scope = scope.left_outer_joins(left_outer_joins) if left_outer_joins
scope
def apply_to(query_scope)
query_scope = order(query_scope)
query_scope = query_scope.joins(joins) if joins
query_scope = query_scope.left_outer_joins(left_outer_joins) if left_outer_joins
query_scope
end

def name
Expand All @@ -68,8 +68,8 @@ def available?

private

def order
model.order(name => direction)
def order(scope)
scope.order(name => direction)
end

def joins
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,24 @@ def self.key
:access_to_anything_in_project
end

def scope
def apply_to(query_scope)
case operator
when "="
visible_scope.in_anything_in_project(values)
query_scope.visible.in_anything_in_project(values)
when "!"
visible_scope.not_in_anything_in_project(values)
query_scope.visible.not_in_anything_in_project(values)
when "*"
member_included_scope.where.not(members: { id: nil })
member_included_scope(query_scope).where.not(members: { id: nil })
when "!*"
member_included_scope.where.not(id: Member.distinct(:user_id).select(:user_id))
member_included_scope(query_scope).where.not(id: Member.distinct(:user_id).select(:user_id))
end
end

private

def visible_scope
Principal.visible(User.current)
end

def member_included_scope
visible_scope
def member_included_scope(scope)
scope
.visible
.includes(:members)
.merge(Member.where.not(project: nil))
end
Expand Down
19 changes: 8 additions & 11 deletions app/models/queries/principals/filters/member_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,24 @@ def self.key
:member
end

def scope
def apply_to(query_scope)
case operator
when "="
visible_scope.in_project(values)
query_scope.visible.in_project(values)
when "!"
visible_scope.not_in_project(values)
query_scope.visible.not_in_project(values)
when "*"
member_included_scope.where.not(members: { id: nil })
member_included_scope(query_scope).where.not(members: { id: nil })
when "!*"
member_included_scope.where.not(id: Member.distinct(:user_id).select(:user_id))
member_included_scope(query_scope).where.not(id: Member.distinct(:user_id).select(:user_id))
end
end

private

def visible_scope
Principal.visible(User.current)
end

def member_included_scope
visible_scope
def member_included_scope(scope)
scope
.visible
.includes(:members)
.merge(Member.of_any_project)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,15 @@ def key
:mentionable_on_work_package
end

def scope
def apply_to(query_scope)
case operator
when "="
principals_with_a_membership
query_scope.where(id: principals_with_a_membership)
when "!"
visible_scope.where.not(id: principals_with_a_membership.select(:id))
query_scope.where(id: visible_scope.where.not(id: principals_with_a_membership.select(:id)))
end
end

def where
"1=1"
end

private

def principals_with_a_membership
Expand Down
6 changes: 3 additions & 3 deletions app/models/queries/principals/filters/type_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ def self.key
:type
end

def scope
def apply_to(query_scope)
if operator == "="
Principal.where(type: values)
query_scope.where(type: values)
else
Principal.where.not(type: values)
query_scope.where.not(type: values)
end
end
end
4 changes: 2 additions & 2 deletions app/models/queries/principals/orders/name_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def self.key

protected

def order
model.ordered_by_name(desc: direction == :desc)
def order(scope)
scope.ordered_by_name(desc: direction == :desc)
end
end
10 changes: 7 additions & 3 deletions app/models/queries/projects/filters/ancestor_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,23 @@ module Queries
module Projects
module Filters
class AncestorFilter < ::Queries::Projects::Filters::ProjectFilter
def scope
def apply_to(_query_scope)
case operator
when "="
Project
super
.where(exists_condition.exists)
when "!"
Project
super
.where.not(exists_condition.exists)
else
raise "unsupported operator"
end
end

def where
nil
end

def type
:list
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,21 @@ def available?
User.current.admin?
end

def scope
def apply_to(_query_scope)
case operator
when "="
model.with_available_custom_fields(values)
super.with_available_custom_fields(values)
when "!"
model.without_available_custom_fields(values)
super.without_available_custom_fields(values)
else
raise "unsupported operator"
end
end

def where
nil
end

def human_name
I18n.t(:label_available_project_attributes)
end
Expand Down
Loading

0 comments on commit 41a1a95

Please sign in to comment.