Project

General

Profile

Actions

Defect #31287

closed

Ordering wiki pages should not be case sensitive

Added by hyper loop almost 5 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Wiki
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

There is a tricky problems for the ordering of WikiPages under Postgresql, the title is designed to be case insensitive.
https://github.com/redmine/redmine/blob/24ccf9d981d907ec7c8fa3901d0a79bc7f641825/app/models/wiki_page.rb#L49

But when it comes to order the wiki pages, it’s ordered in a case sensitive way, which results in the three WikiPages titled with ["Aac", "Aace_", "ABc", "Acc”] is ordered as `["ABc", "Aac", "Aace_", "Acc"] `, which is not correct since `"Aac", “Aace_"` should precede `"ABc”`.

A quick fix would be change the

 order(:title)
to
order('lower(title)’)
, patch is attached.

The order works well under mysql since mysql will order regardless of the case. But I think it should also work properly under Postgresql.


Files

patch.rb (566 Bytes) patch.rb hyper loop, 2019-05-01 22:01
patch.diff (566 Bytes) patch.diff hyper loop, 2019-05-01 22:03
patch2.diff (520 Bytes) patch2.diff hyper loop, 2019-05-01 22:14
fixed-31287.patch (4.15 KB) fixed-31287.patch Yuichi HARADA, 2019-06-14 02:22
sqlite.png (18 KB) sqlite.png Go MAEDA, 2020-06-18 01:40
Actions #1

Updated by hyper loop almost 5 years ago

Actions #2

Updated by hyper loop almost 5 years ago

Similar for

 has_many :pages, lambda {order('title')}, :class_name => 'WikiPage', :dependent => :destroy
https://github.com/redmine/redmine/blob/24ccf9d981d907ec7c8fa3901d0a79bc7f641825/app/models/wiki.rb#L23, which should be
  has_many :pages, lambda {order('lower(title)')}, :class_name => 'WikiPage', :dependent => :destroy 

patch is attached too.

Actions #3

Updated by Go MAEDA almost 5 years ago

  • Target version set to Candidate for next major release
Actions #4

Updated by Yuichi HARADA almost 5 years ago

A warning occurred when I created a test and applied patch2.diff to the trunk.

diff --git a/test/unit/wiki_test.rb b/test/unit/wiki_test.rb
index b4ecd29bb..9184d4efc 100644
--- a/test/unit/wiki_test.rb
+++ b/test/unit/wiki_test.rb
@@ -52,6 +52,16 @@ class WikiTest < ActiveSupport::TestCase
     assert_equal page, wiki.find_page('ANOTHER page')
   end

+  def test_ordering_pages_should_not_be_case_sensitive
+    wiki = Wiki.find(1)
+    wiki.pages.destroy_all
+    %w(Acc ABc Aace_ Aac).each do |title|
+      wiki.pages.create(:title => title)
+    end
+    wiki.reload
+    assert_equal %w(Aac Aace_ ABc Acc), wiki.pages.pluck(:title)
+  end
+
   def test_find_page_with_cyrillic_characters
     wiki = Wiki.find(1)
     page = WikiPage.find(10)
$ bundle exec rake test TEST=test/unit/wiki_test.rb:56
Run options: --seed 43297

# Running:

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from block in <class:Wiki> at ./app/models/wiki.rb:23)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from block in <class:Wiki> at ./app/models/wiki.rb:23)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from test_ordering_pages_should_not_be_case_sensitive at ./test/unit/wiki_test.rb:57)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from initialize at ./app/models/wiki_page.rb:74)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from initialize at ./app/models/wiki_page.rb:74)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from initialize at ./app/models/wiki_page.rb:74)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from initialize at ./app/models/wiki_page.rb:74)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from block in <class:Wiki> at ./app/models/wiki.rb:23)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from block in <class:Wiki> at ./app/models/wiki.rb:23)
DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "LOWER(title)". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql(). (called from test_ordering_pages_should_not_be_case_sensitive at ./test/unit/wiki_test.rb:62)
.

Finished in 0.137138s, 7.2919 runs/s, 7.2919 assertions/s.
1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
$

I made a patch based on patch.diff and patch2.diff .

Actions #5

Updated by Go MAEDA almost 5 years ago

  • Category set to Wiki
  • Target version changed from Candidate for next major release to 4.1.0

Setting the target version to 4.1.0.

Actions #6

Updated by Anonymous over 4 years ago

+1, but will commit happen? Why are we waiting?

Actions #7

Updated by Go MAEDA over 4 years ago

  • Target version changed from 4.1.0 to 4.2.0
Actions #8

Updated by Go MAEDA almost 4 years ago

  • File sqlite.png sqlite.png added
  • Subject changed from Case sensitivity for title of WikiPage under postgresql to Ordering wiki pages should not be case sensitive

Not only PostgreSQL but SQLite is also affected.

Actions #9

Updated by Go MAEDA almost 4 years ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch. Thank you.

Actions

Also available in: Atom PDF