Skip to content

Commit

Permalink
Allow order fields to be subqueries with order
Browse files Browse the repository at this point in the history
Fields like last_compliance_status (on VmOrTemplate.yaml) contain
a limit and order.

This is because the column joins to a has_many and references
the most recent record.

ManageIQ/manageiq-ui-classic#5374
  • Loading branch information
kbrock committed Jul 8, 2019
1 parent 08c2cf2 commit e237d0a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
13 changes: 11 additions & 2 deletions lib/extensions/ar_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ def columns_for_distinct(columns, orders) #:nodoc:
order_columns = orders.reject(&:blank?).map { |s|
# Convert Arel node to string
unless s.is_a?(String)
if s.kind_of?(Arel::Nodes::Ordering)
s = s.expr
keep_order = true
end
if s.respond_to?(:to_sql)
s = s.to_sql
else # for Arel::Nodes::Attribute
Expand All @@ -17,9 +21,14 @@ def columns_for_distinct(columns, orders) #:nodoc:
s = collector.value
end
end
# If we haven't already removed the order clause,
# Remove any ASC/DESC modifiers
s.gsub(/\s+(?:ASC|DESC)\b/i, "")
.gsub(/\s+NULLS\s+(?:FIRST|LAST)\b/i, "")
if keep_order
s
else
s.gsub(/\s+(?:ASC|DESC)\b/i, "")
.gsub(/\s+NULLS\s+(?:FIRST|LAST)\b/i, "")
end
}.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" }

(order_columns << super).join(", ")
Expand Down
12 changes: 12 additions & 0 deletions spec/lib/extensions/ar_order_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
describe "ar_order extension" do
# includes a has_many AND references the has many
# this introduces a DISTINCT to the query.
# rails uses limited_ids_for (which calls columns_for_distinct) to run a quick query
#
# when we use a virtual attribute in the sort (and the attribute has a descending order)
# the string munging gives us issues.
it "supports order when distinct is present for has_many virtual column" do
expect do
VmOrTemplate.includes(:disks).references(:disks).order(:last_compliance_status).first
end.not_to raise_error
end

it "supports order when distinct is present for basic column" do
expect do
VmOrTemplate.includes(:disks).references(:disks).order(:id).first
Expand Down

0 comments on commit e237d0a

Please sign in to comment.