From 72e500aa295fe703c23994ad68e93993b02fda12 Mon Sep 17 00:00:00 2001 From: Brian Palmer Date: Thu, 18 Jul 2013 12:19:35 -0600 Subject: [PATCH] use the database lower() function instead of ruby downcase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes CNVS-6974 test plan: * create a user with Ĭńşŧřůćƭǜȑȩ in their name, enroll them in a course * search for that user in the course roster, they should be found Change-Id: Ia21d4d5163eec950cd45950123c1a273673a8c9a Reviewed-on: https://gerrit.instructure.com/22470 Tested-by: Jenkins Reviewed-by: Cody Cutrer Reviewed-by: Jacob Fugal QA-Review: Jeremy Putnam Product-Review: Brian Palmer --- app/models/communication_channel.rb | 4 ++-- app/models/enrollment.rb | 4 ++-- app/models/pseudonym.rb | 2 +- lib/user_search.rb | 4 ++-- spec/lib/user_search_spec.rb | 17 ++++++++++++----- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/app/models/communication_channel.rb b/app/models/communication_channel.rb index 690c0b5f4ae..526054264f6 100644 --- a/app/models/communication_channel.rb +++ b/app/models/communication_channel.rb @@ -125,7 +125,7 @@ class CommunicationChannel < ActiveRecord::Base return if path.nil? return if retired? return unless user_id - conditions = ["LOWER(path)=? AND user_id=? AND path_type=? AND workflow_state IN('unconfirmed', 'active')", path.mb_chars.downcase, user_id, path_type] + conditions = ["LOWER(path)=LOWER(?) AND user_id=? AND path_type=? AND workflow_state IN('unconfirmed', 'active')", path, user_id, path_type] unless new_record? conditions.first << " AND id<>?" conditions << id @@ -224,7 +224,7 @@ class CommunicationChannel < ActiveRecord::Base if %{mysql mysql2}.include?(connection_pool.spec.config[:adapter]) where(:path => path) else - where("LOWER(communication_channels.path)=?", path.try(:downcase)) + where("LOWER(communication_channels.path)=LOWER(?)", path) end } diff --git a/app/models/enrollment.rb b/app/models/enrollment.rb index 5b456d9bf5f..c23c95ec27b 100644 --- a/app/models/enrollment.rb +++ b/app/models/enrollment.rb @@ -875,13 +875,13 @@ class Enrollment < ActiveRecord::Base if Rails.version < '3.0' { :joins => { :user => :communication_channels }, - :conditions => ["users.workflow_state='creation_pending' AND communication_channels.workflow_state='unconfirmed' AND path_type='email' AND LOWER(path)=?", email.downcase], + :conditions => ["users.workflow_state='creation_pending' AND communication_channels.workflow_state='unconfirmed' AND path_type='email' AND LOWER(path)=LOWER(?)", email], :select => 'enrollments.*', :readonly => false } else joins(:user => :communication_channels). - where("users.workflow_state='creation_pending' AND communication_channels.workflow_state='unconfirmed' AND path_type='email' AND LOWER(path)=?", email.downcase). + where("users.workflow_state='creation_pending' AND communication_channels.workflow_state='unconfirmed' AND path_type='email' AND LOWER(path)=LOWER(?)", email). select("enrollments.*"). readonly(false) end diff --git a/app/models/pseudonym.rb b/app/models/pseudonym.rb index 7463f8fb34e..ddc4ac38792 100644 --- a/app/models/pseudonym.rb +++ b/app/models/pseudonym.rb @@ -113,7 +113,7 @@ class Pseudonym < ActiveRecord::Base if %w{mysql mysql2}.include?(connection_pool.spec.config[:adapter]) where(:unique_id => unique_id) else - where("LOWER(#{quoted_table_name}.unique_id)=?", unique_id.mb_chars.downcase) + where("LOWER(#{quoted_table_name}.unique_id)=LOWER(?)", unique_id) end } diff --git a/lib/user_search.rb b/lib/user_search.rb index 9b76b893b52..b9ed8b695c8 100644 --- a/lib/user_search.rb +++ b/lib/user_search.rb @@ -26,7 +26,7 @@ module UserSearch def self.like_string_for(search_term) pattern_type = (gist_search_enabled? ? :full : :right) - wildcard_pattern(search_term, :type => pattern_type) + wildcard_pattern(search_term, :type => pattern_type, :case_sensitive => false) end def self.scope_for(context, searcher, options={}) @@ -84,7 +84,7 @@ module UserSearch end def self.like_condition(value) - ActiveRecord::Base.like_condition(value) + ActiveRecord::Base.like_condition(value, 'lower(?)') end def self.wildcard_pattern(value, options) diff --git a/spec/lib/user_search_spec.rb b/spec/lib/user_search_spec.rb index c8291113f25..e5d37f8c58e 100644 --- a/spec/lib/user_search_spec.rb +++ b/spec/lib/user_search_spec.rb @@ -1,9 +1,11 @@ +# encoding: utf-8 + require File.expand_path( '../spec_helper' , File.dirname(__FILE__)) describe UserSearch do describe '.for_user_in_context' do - let(:search_names) { ['Rose Tyler', 'Martha Jones', 'Rosemary Giver', 'Martha Stewart', 'Tyler Pickett', 'Jon Stewart', 'Stewart Little'] } + let(:search_names) { ['Rose Tyler', 'Martha Jones', 'Rosemary Giver', 'Martha Stewart', 'Tyler Pickett', 'Jon Stewart', 'Stewart Little', 'Ĭńşŧřůćƭǜȑȩ Person'] } let(:course) { Course.create! } let(:users) { UserSearch.for_user_in_context('Stewart', course, user).to_a } let(:names) { users.map(&:name) } @@ -26,6 +28,15 @@ describe UserSearch do describe 'with gist setting enabled' do before { Setting.set('user_search_with_gist', 'true') } + it "searches case-insensitively" do + UserSearch.for_user_in_context("steWArt", course, user).size.should == 3 + end + + it "uses postgres lower(), not ruby downcase()" do + # ruby 1.9 downcase doesn't handle the downcasing of many multi-byte characters correctly + UserSearch.for_user_in_context('Ĭńşŧřůćƭǜȑȩ', course, user).size.should == 1 + end + it 'returns an enumerable' do users.size.should == 3 end @@ -180,10 +191,6 @@ describe UserSearch do end describe '.like_string_for' do - it 'lowercases the term' do - UserSearch.like_string_for("MickyMouse").should =~ /mickymouse/ - end - it 'uses a prefix if gist is not configured' do Setting.set('user_search_with_gist', 'false') UserSearch.like_string_for("word").should == 'word%'