Validates user's mail_notification and turn options into strings (the attribute type).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4494 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/user.rb

@@ -35,13 +35,13 @@ class User < Principal
   }
 
   MAIL_NOTIFICATION_OPTIONS = [
-                               [:all, :label_user_mail_option_all],
-                               [:selected, :label_user_mail_option_selected],
-                               [:none, :label_user_mail_option_none],
-                               [:only_my_events, :label_user_mail_option_only_my_events],
-                               [:only_assigned, :label_user_mail_option_only_assigned],
-                               [:only_owner, :label_user_mail_option_only_owner]
-                              ]
+    ['all', :label_user_mail_option_all],
+    ['selected', :label_user_mail_option_selected],
+    ['only_my_events', :label_user_mail_option_only_my_events],
+    ['only_assigned', :label_user_mail_option_only_assigned],
+    ['only_owner', :label_user_mail_option_only_owner],
+    ['none', :label_user_mail_option_none]
+  ]
 
   has_and_belongs_to_many :groups, :after_add => Proc.new {|user, group| group.user_added(user)},
                                    :after_remove => Proc.new {|user, group| group.user_removed(user)}
@@ -73,6 +73,7 @@ class User < Principal
   validates_format_of :mail, :with => /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i, :allow_nil => true
   validates_length_of :mail, :maximum => 60, :allow_nil => true
   validates_confirmation_of :password, :allow_nil => true
+  validates_inclusion_of :mail_notification, :in => MAIL_NOTIFICATION_OPTIONS.collect(&:first), :allow_blank => true
 
   def before_create
     self.mail_notification = Setting.default_notification_option if self.mail_notification.blank?
@@ -265,7 +266,7 @@ class User < Principal
     # Note that @user.membership.size would fail since AR ignores
     # :include association option when doing a count
     if memberships.length < 1
-      MAIL_NOTIFICATION_OPTIONS.delete_if {|option| option.first == :selected}
+      MAIL_NOTIFICATION_OPTIONS.delete_if {|option| option.first == 'selected'}
     else
       MAIL_NOTIFICATION_OPTIONS
     end
@@ -411,26 +412,26 @@ class User < Principal
   #
   # TODO: only supports Issue events currently
   def notify_about?(object)
-    case mail_notification.to_sym
-    when :all
+    case mail_notification
+    when 'all'
       true
-    when :selected
+    when 'selected'
       # Handled by the Project
-    when :none
+    when 'none'
       false
-    when :only_my_events
+    when 'only_my_events'
       if object.is_a?(Issue) && (object.author == self || object.assigned_to == self)
         true
       else
         false
       end
-    when :only_assigned
+    when 'only_assigned'
       if object.is_a?(Issue) && object.assigned_to == self
         true
       else
         false
       end
-    when :only_owner
+    when 'only_owner'
       if object.is_a?(Issue) && object.author == self
         true
       else

test/unit/user_test.rb

@@ -115,12 +115,19 @@ class UserTest < ActiveSupport::TestCase
     assert Member.find_all_by_user_id(2).empty?
   end
   
-  def test_validate
+  def test_validate_login_presence
     @admin.login = ""
     assert !@admin.save
     assert_equal 1, @admin.errors.count
   end
   
+  def test_validate_mail_notification_inclusion
+    u = User.new
+    u.mail_notification = 'foo'
+    u.save
+    assert_not_nil u.errors.on(:mail_notification)
+  end
+  
   context "User#try_to_login" do
     should "fall-back to case-insensitive if user login is not found as-typed." do
       user = User.try_to_login("AdMin", "admin")
@@ -437,55 +444,49 @@ class UserTest < ActiveSupport::TestCase
       end
 
       should "be true for a user with :all" do
-        @author.update_attribute(:mail_notification, :all)
+        @author.update_attribute(:mail_notification, 'all')
         assert @author.notify_about?(@issue)
       end
       
       should "be false for a user with :none" do
-        @author.update_attribute(:mail_notification, :none)
+        @author.update_attribute(:mail_notification, 'none')
         assert ! @author.notify_about?(@issue)
       end
       
       should "be false for a user with :only_my_events and isn't an author, creator, or assignee" do
-        @user = User.generate_with_protected!(:mail_notification => :only_my_events)
+        @user = User.generate_with_protected!(:mail_notification => 'only_my_events')
         assert ! @user.notify_about?(@issue)
       end
       
       should "be true for a user with :only_my_events and is the author" do
-        @author.update_attribute(:mail_notification, :only_my_events)
+        @author.update_attribute(:mail_notification, 'only_my_events')
         assert @author.notify_about?(@issue)
       end
       
       should "be true for a user with :only_my_events and is the assignee" do
-        @assignee.update_attribute(:mail_notification, :only_my_events)
+        @assignee.update_attribute(:mail_notification, 'only_my_events')
         assert @assignee.notify_about?(@issue)
       end
       
       should "be true for a user with :only_assigned and is the assignee" do
-        @assignee.update_attribute(:mail_notification, :only_assigned)
+        @assignee.update_attribute(:mail_notification, 'only_assigned')
         assert @assignee.notify_about?(@issue)
       end
       
       should "be false for a user with :only_assigned and is not the assignee" do
-        @author.update_attribute(:mail_notification, :only_assigned)
+        @author.update_attribute(:mail_notification, 'only_assigned')
         assert ! @author.notify_about?(@issue)
       end
       
       should "be true for a user with :only_owner and is the author" do
-        @author.update_attribute(:mail_notification, :only_owner)
+        @author.update_attribute(:mail_notification, 'only_owner')
         assert @author.notify_about?(@issue)
       end
       
       should "be false for a user with :only_owner and is not the author" do
-        @assignee.update_attribute(:mail_notification, :only_owner)
+        @assignee.update_attribute(:mail_notification, 'only_owner')
         assert ! @assignee.notify_about?(@issue)
       end
-      
-      should "be false if the mail_notification is anything else" do
-        @assignee.update_attribute(:mail_notification, :somthing_else)
-        assert ! @assignee.notify_about?(@issue)
-      end
-      
     end
 
     context "other events" do