From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 23 Jun 2018 05:19:07 +0000 (+0000)
Subject: Dangerous query method deprecation warnings (#23630).
X-Git-Tag: 4.0.0~245
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d8a202471586677cf1863de390389058b1f0a0c0;p=redmine.git

Dangerous query method deprecation warnings (#23630).

git-svn-id: http://svn.redmine.org/redmine/trunk@17411 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

diff --git a/app/models/query.rb b/app/models/query.rb
index 53a245a8d..115e1bb7c 100644
--- a/app/models/query.rb
+++ b/app/models/query.rb
@@ -780,14 +780,16 @@ class Query < ActiveRecord::Base
   end
 
   def sort_clause
-    sort_criteria.sort_clause(sortable_columns)
+    if clause = sort_criteria.sort_clause(sortable_columns)
+      clause.map {|c| Arel.sql c}
+    end
   end
 
   # Returns the SQL sort order that should be prepended for grouping
   def group_by_sort_order
     if column = group_by_column
       order = (sort_criteria.order_for(column.name) || column.default_order || 'asc').try(:upcase)
-      Array(column.sortable).map {|s| "#{s} #{order}"}
+      Array(column.sortable).map {|s| Arel.sql("#{s} #{order}")}
     end
   end
 
diff --git a/app/models/version.rb b/app/models/version.rb
index 53d1d7758..5626750f8 100644
--- a/app/models/version.rb
+++ b/app/models/version.rb
@@ -317,7 +317,7 @@ class Version < ActiveRecord::Base
 
   def self.fields_for_order_statement(table=nil)
     table ||= table_name
-    ["(CASE WHEN #{table}.effective_date IS NULL THEN 1 ELSE 0 END)", "#{table}.effective_date", "#{table}.name", "#{table}.id"]
+    [Arel.sql("(CASE WHEN #{table}.effective_date IS NULL THEN 1 ELSE 0 END)"), "#{table}.effective_date", "#{table}.name", "#{table}.id"]
   end
 
   scope :sorted, lambda { order(fields_for_order_statement) }
diff --git a/lib/redmine/field_format.rb b/lib/redmine/field_format.rb
index bcf6ccc3d..c4960c0f7 100644
--- a/lib/redmine/field_format.rb
+++ b/lib/redmine/field_format.rb
@@ -323,7 +323,7 @@ module Redmine
       # Returns nil if the custom field can not be used for sorting.
       def order_statement(custom_field)
         # COALESCE is here to make sure that blank and NULL values are sorted equally
-        "COALESCE(#{join_alias custom_field}.value, '')"
+        Arel.sql "COALESCE(#{join_alias custom_field}.value, '')"
       end
 
       # Returns a GROUP BY clause that can used to group by custom value
@@ -461,7 +461,7 @@ module Redmine
         # Make the database cast values into numeric
         # Postgresql will raise an error if a value can not be casted!
         # CustomValue validations should ensure that it doesn't occur
-        "CAST(CASE #{join_alias custom_field}.value WHEN '' THEN '0' ELSE #{join_alias custom_field}.value END AS decimal(30,3))"
+        Arel.sql "CAST(CASE #{join_alias custom_field}.value WHEN '' THEN '0' ELSE #{join_alias custom_field}.value END AS decimal(30,3))"
       end
 
       # Returns totals for the given scope
@@ -747,7 +747,7 @@ module Redmine
       end
 
       def group_statement(custom_field)
-        "COALESCE(#{join_alias custom_field}.value, '')"
+        Arel.sql "COALESCE(#{join_alias custom_field}.value, '')"
       end
 
       def join_for_order_statement(custom_field)
diff --git a/lib/redmine/sort_criteria.rb b/lib/redmine/sort_criteria.rb
index 3d440f010..8193b9b91 100644
--- a/lib/redmine/sort_criteria.rb
+++ b/lib/redmine/sort_criteria.rb
@@ -97,7 +97,7 @@ module Redmine
       if criterion =~ / (asc|desc)$/i
         criterion
       else
-        "#{criterion} #{order.to_s.upcase}"
+        Arel.sql "#{criterion} #{order.to_s.upcase}"
       end
     end
   end
diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index dd5ef3629..5b9886dd9 100644
--- a/test/unit/query_test.rb
+++ b/test/unit/query_test.rb
@@ -1483,7 +1483,8 @@ class QueryTest < ActiveSupport::TestCase
     c = q.available_columns.find {|col| col.is_a?(QueryCustomFieldColumn) && col.custom_field.field_format == 'string' }
     assert c
     assert c.sortable
-    issues = q.issues(:order => "#{c.sortable} ASC")
+    q.sort_criteria = [[c.name.to_s, 'asc']]
+    issues = q.issues
     values = issues.collect {|i| i.custom_value_for(c.custom_field).to_s}
     assert !values.empty?
     assert_equal values.sort, values
@@ -1494,7 +1495,8 @@ class QueryTest < ActiveSupport::TestCase
     c = q.available_columns.find {|col| col.is_a?(QueryCustomFieldColumn) && col.custom_field.field_format == 'string' }
     assert c
     assert c.sortable
-    issues = q.issues(:order => "#{c.sortable} DESC")
+    q.sort_criteria = [[c.name.to_s, 'desc']]
+    issues = q.issues
     values = issues.collect {|i| i.custom_value_for(c.custom_field).to_s}
     assert !values.empty?
     assert_equal values.sort.reverse, values
@@ -1505,7 +1507,8 @@ class QueryTest < ActiveSupport::TestCase
     c = q.available_columns.find {|col| col.is_a?(QueryCustomFieldColumn) && col.custom_field.field_format == 'float' }
     assert c
     assert c.sortable
-    issues = q.issues(:order => "#{c.sortable} ASC")
+    q.sort_criteria = [[c.name.to_s, 'asc']]
+    issues = q.issues
     values = issues.collect {|i| begin; Kernel.Float(i.custom_value_for(c.custom_field).to_s); rescue; nil; end}.compact
     assert !values.empty?
     assert_equal values.sort, values
@@ -1703,9 +1706,9 @@ class QueryTest < ActiveSupport::TestCase
 
   def test_issue_ids
     q = IssueQuery.new(:name => '_')
-    order = "issues.subject, issues.id"
-    issues = q.issues(:order => order)
-    assert_equal issues.map(&:id), q.issue_ids(:order => order)
+    q.sort_criteria = ['subject', 'id']
+    issues = q.issues
+    assert_equal issues.map(&:id), q.issue_ids
   end
 
   def test_label_for