[refs #114] fix emails uniqueness

app/models/comment.rb

@@ -28,7 +28,7 @@ class Comment < ActiveRecord::Base
       self.commentable.subscribes.create(:user_id => self.user_id) if !self.commentable.subscribes.exists?(:user_id => self.user_id)
     elsif self.commentable.class == Grit::Commit
       recipients = self.project.relations.by_role('admin').where(:object_type => 'User').map &:object # admins
-      recipients << self.user << UserEmail.where(:email => self.commentable.committer.email).first.try(:user) # commentor and committer
+      recipients << self.user << UserEmail.where(:email_lower => self.commentable.committer.email.downcase).first.try(:user) # commentor and committer
       recipients << self.project.owner if self.project.owner_type == 'User' # project owner
       recipients.compact.uniq.each {|user| Subscribe.subscribe_user_to_commit(self, user.id)}
     end

app/models/subscribe.rb

@@ -42,7 +42,7 @@ class Subscribe < ActiveRecord::Base
 
   def self.subscribed_to_commit?(project, user, commentable)
     is_commentor = (Comment.where(:commentable_type => commentable.class.name, :commentable_id => commentable.id).exists?(:user_id => user.id))
-    is_committer = (user.emails.exists? :email => commentable.committer.email)
+    is_committer = (user.emails.exists? :email_lower => commentable.committer.email.downcase)
     return false if user.subscribes.where(:subscribeable_id => commentable.id, :subscribeable_type => commentable.class.name,
                      :project_id => project.id, :status => Subscribe::OFF).first.present?
     (project.owner?(user) && user.notifier.new_comment_commit_repo_owner) or

app/models/user.rb

@@ -59,8 +59,8 @@ class User < ActiveRecord::Base
     def find_for_database_authentication(warden_conditions)
       conditions = warden_conditions.dup
       login = conditions.delete(:login)
-      where(conditions).where("lower(uname) = :value OR " +
-        "exists (select null from user_emails m where m.user_id = m.user_id and lower(m.email) = :value)",
+      where(conditions).where("(lower(uname) = :value OR \
+        exists (select null from user_emails m where m.user_id = m.user_id and m.email_lower = :value))",
         {:value => login.downcase}).first
     end
 

app/models/user_email.rb

@@ -3,7 +3,14 @@ class UserEmail < ActiveRecord::Base
 
   belongs_to :user
 
-  validates :email, :uniqueness => true
+  validates :email_lower, :uniqueness => true
   validates :email, :presence => true
 
+  before_validation :set_lower
+
+  private
+
+  def set_lower
+    self.email_lower = self.email.downcase
+  end
 end

db/migrate/20120129120025_add_email_lower_to_user_emails.rb

@@ -0,0 +1,18 @@
+class AddEmailLowerToUserEmails < ActiveRecord::Migration
+  def self.up
+    add_column :user_emails, :email_lower, :string
+    remove_index :user_emails, :email
+
+    UserEmail.reset_column_information
+    UserEmail.update_all("email_lower = lower(email)")
+
+    change_column :user_emails, :email_lower, :string, :null => false
+    add_index :user_emails, :email_lower
+  end
+
+  def self.down
+    remove_column :user_emails, :email_lower
+    add_index :user_emails, :email
+    remove_index :user_emails, :email_lower
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20120123161250) do
+ActiveRecord::Schema.define(:version => 20120129120025) do
 
   create_table "arches", :force => true do |t|
     t.string   "name",       :null => false
@@ -167,13 +167,6 @@ ActiveRecord::Schema.define(:version => 20120123161250) do
 
   add_index "issues", ["project_id", "serial_id"], :name => "index_issues_on_project_id_and_serial_id", :unique => true
 
-  create_table "permissions", :force => true do |t|
-    t.integer  "right_id"
-    t.integer  "role_id"
-    t.datetime "created_at"
-    t.datetime "updated_at"
-  end
-
   create_table "platforms", :force => true do |t|
     t.string   "description"
     t.string   "name"
@@ -270,27 +263,6 @@ ActiveRecord::Schema.define(:version => 20120123161250) do
     t.string   "owner_type"
   end
 
-  create_table "rights", :force => true do |t|
-    t.string   "name",       :null => false
-    t.string   "controller", :null => false
-    t.string   "action",     :null => false
-    t.datetime "created_at"
-    t.datetime "updated_at"
-  end
-
-  create_table "role_lines", :force => true do |t|
-    t.integer  "role_id"
-    t.integer  "relation_id"
-    t.datetime "created_at"
-    t.datetime "updated_at"
-  end
-
-  create_table "roles", :force => true do |t|
-    t.string   "name"
-    t.datetime "created_at"
-    t.datetime "updated_at"
-  end
-
   create_table "rpms", :force => true do |t|
     t.string   "name",       :null => false
     t.integer  "arch_id",    :null => false
@@ -331,18 +303,18 @@ ActiveRecord::Schema.define(:version => 20120123161250) do
     t.string   "email",       :null => false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.string   "email_lower", :null => false
   end
 
-  add_index "user_emails", ["email"], :name => "index_user_emails_on_email"
+  add_index "user_emails", ["email_lower"], :name => "index_user_emails_on_email_lower"
   add_index "user_emails", ["user_id"], :name => "index_user_emails_on_user_id"
 
   create_table "users", :force => true do |t|
     t.string   "name"
     t.string   "email",                                 :default => "",   :null => false
     t.string   "encrypted_password",     :limit => 128, :default => "",   :null => false
-    t.string   "password_salt",                       :default => "",   :null => false
     t.string   "reset_password_token"
-    t.string   "remember_token"
+    t.datetime "reset_password_sent_at"
     t.datetime "remember_created_at"
     t.datetime "created_at"
     t.datetime "updated_at"

spec/models/user_email_spec.rb

@@ -13,5 +13,16 @@ describe UserEmail do
       @stranger.emails.create(:email => @user.emails.first.email)
       @stranger.emails.exists?(:email => @user.emails.first.email).should be_false
     end
+
+    it 'should not create duplicate lowercase emails' do
+      @stranger = Factory(:user)
+      @stranger.emails.create(:email => @user.email.upcase)
+      @stranger.emails.exists?(:email => @user.email.upcase).should be_false
+    end
+
+    it 'should not create too many emails' do
+      15.times {|i| @user.emails.create(:email => Factory.next(:email))}
+      @user.emails.count.should be_equal(UserEmail::MAX_EMAILS)
+    end
   end
 end