diff --git a/app/models/user.rb b/app/models/user.rb index b039ba0b32..f69604199b 100644 --- a/app/models/user.rb +++ b/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 diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 3f824f9fe8..ee5df0ee7a 100644 --- a/test/unit/user_test.rb +++ b/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