Fix watcher handling on unsaved objects (#39186).

Patch by Holger Just.


git-svn-id: https://svn.redmine.org/redmine/trunk@22349 e93f8b46-1217-0410-a6f0-8f06a7374b81

2 files changed, 53 insertions, 6 deletions

diff --git a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
index 907535ec7..1a5888464 100644
--- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
+++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
@@ -11,6 +11,7 @@ module Redmine
       module ClassMethods
         def acts_as_watchable(options = {})
           return if self.included_modules.include?(Redmine::Acts::Watchable::InstanceMethods)
+
           class_eval do
             has_many :watchers, :as => :watchable, :dependent => :delete_all
             has_many :watcher_users, :through => :watchers, :source => :user, :validate => false
@@ -44,17 +45,26 @@ module Redmine
 
         # Adds user as a watcher
         def add_watcher(user)
-          # Rails does not reset the has_many :through association
-          watcher_users.reset
-          self.watchers << Watcher.new(:user => user)
+          if persisted?
+            # Rails does not reset the has_many :through association
+            watcher_users.reset
+            self.watchers << Watcher.new(:user => user)
+          else
+            self.watcher_users << user
+          end
         end
 
         # Removes user from the watchers list
         def remove_watcher(user)
           return nil unless user && (user.is_a?(User) || user.is_a?(Group))
-          # Rails does not reset the has_many :through association
-          watcher_users.reset
-          watchers.where(:user_id => user.id).delete_all
+
+          if persisted?
+            # Rails does not reset the has_many :through association
+            watcher_users.reset
+            watchers.where(:user_id => user.id).delete_all
+          else
+            watcher_users.delete(user)
+          end
         end
 
         # Adds/removes watcher
diff --git a/test/unit/watcher_test.rb b/test/unit/watcher_test.rb
index ced499712..a5c00f946 100644
--- a/test/unit/watcher_test.rb
+++ b/test/unit/watcher_test.rb
@@ -126,6 +126,43 @@ class WatcherTest < ActiveSupport::TestCase
     end
   end
 
+  def test_add_watcher_with_unsaved_object
+    issue = Issue.new(project: Project.find(1), tracker_id: 1, subject: "test", author: User.find(2))
+    assert_not issue.persisted?
+
+    issue.add_watcher(@user)
+    assert issue.watched_by?(@user)
+
+    assert_equal [@user.id], issue.watcher_user_ids
+    assert_equal [@user], issue.watcher_users
+
+    assert_equal [nil], issue.watcher_ids
+    assert_equal 1, issue.watchers.size
+
+    issue.save!
+    assert 1, Watcher.where(watchable: issue).count
+  end
+
+  def test_remove_watcher_with_unsaved_object
+    issue = Issue.new(project: Project.find(1), tracker_id: 1, subject: "test", author: User.find(2))
+    assert_not issue.persisted?
+
+    issue.add_watcher(@user)
+    assert_equal [@user], issue.watcher_users
+
+    issue.remove_watcher(@user)
+    assert_not issue.watched_by?(@user)
+
+    assert_equal [], issue.watcher_user_ids
+    assert_equal [], issue.watcher_users
+
+    assert_equal [], issue.watcher_ids
+    assert_equal [], issue.watchers
+
+    issue.save!
+    assert 0, Watcher.where(watchable: issue).count
+  end
+
   def test_addable_watcher_users_should_not_include_user_that_cannot_view_the_object
     issue = Issue.new(:project => Project.find(1), :is_private => true)
     assert_nil issue.addable_watcher_users.detect {|user| user.is_a?(User) && !issue.visible?(user)}