Project

General

Profile

Actions
Copy link

Defect #25867

open

Assignable users should respect database collation

Added by Pavel Rosický over 6 years ago. Updated about 2 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
Database
Target version:
6.0.0
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Affected version:

Description

mysql collation: utf8_general_ci
['a','u','č'] should be sorted as ['a','č','u'] but because of ruby sort it's reordered back as ['a','u','č']

  def assignable_users
    users = project.assignable_users(tracker).to_a
    users << author if author && author.active?
    if assigned_to_id_was.present? && assignee = Principal.find_by_id(assigned_to_id_was)
      users << assignee
    end

    users.uniq.sort
  end

I can provide a patch, are you interested or is it desired behaviour?

Environment (not important, all redmine versions and databases are affected):
  Redmine version                3.3.3.devel.16557
  Ruby version                   2.1.5-p273 (2014-11-13) [x64-mingw32]
  Rails version                  4.2.8
  Environment                    production
  Database adapter               Mysql2
SCM:
  Subversion                     1.9.5
  Git                            2.11.0
  Filesystem
Redmine plugins:
  no plugin installed

Files

Download all files
issue_test.rb.patch (883 Bytes) issue_test.rb.patch Pavel Rosický, 2017-05-18 21:57
issue.rb.patch (1.16 KB) issue.rb.patch Pavel Rosický, 2017-05-18 21:57
25867.patch (2.01 KB) 25867.patch Go MAEDA, 2022-12-08 09:50
Actions
Copy link
 #2

Updated by Pavel Rosický over 5 years ago

It's been a year and the problem is still reproducible.

Actions
Copy link
 #3

Updated by Go MAEDA about 5 years ago

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

Updated by Go MAEDA 12 months ago

Updated the patch for the current trunk (r21987).

Actions
Copy link
 #5

Updated by Go MAEDA 12 months ago

  • Target version changed from Candidate for next major release to 5.1.0

Setting the target version to 5.1.0.

Actions
Copy link
 #6

Updated by Holger Just 12 months ago

In the updated patch, you removed the line return @assignable_users unless @assignable_users.nil? from the original patch in #25867#note-1. Without this line, the caching of the result in @assignable_users becomes useless.

I'm actually unsure if it's worthwhile to introduce caching here at all. I tend to say: we do not need it as the method does not appear to be regularly called multiple times per request. As such, I think, we can get rid of the caching and its associated possibility for inconsistencies. Turns out, it is called multiple times in the issues/_attributes.html.erb partial. Thus, we still might want caching... In any case though, we should either remove the instance variable caching completely, or use it if present.

As a slight improvement, it might also be useful to also remove the to_a at the end and to return a query object. That way, callers might chain other query refinements to it without affecting the current use-case.

Finally, it might also be useful to extract the fetching of the (unsorted) user ids into a separate method, e.g. assignable_user_ids, which might make checks such as those in the Issue model to check if the assignee is allowed less expensive by avoiding the final fetch of the Principal objects. Only these ids might then possible be cached?

Actions
Copy link
 #7

Updated by Go MAEDA about 2 months ago

  • Target version changed from 5.1.0 to 6.0.0
Actions
Copy link

Also available in: Atom PDF