From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 2 Feb 2013 13:03:22 +0000 (+0000)
Subject: Don't remove watchers on permission change.
X-Git-Tag: 2.3.0~203
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5fea79504c7483ce589d2f91adf05760e1ba156f;p=redmine.git

Don't remove watchers on permission change.

This can be far too slow (especially with membership inheritance) and notifications are not sent to watchers that are not allowed to view the item. If we still want to remove watchers that are no longer able to view the watched items, the redmine:watchers:prune task can be called periodically.

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

diff --git a/app/models/member.rb b/app/models/member.rb
index 5ff31f316..b11086061 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -27,7 +27,6 @@ class Member < ActiveRecord::Base
   validate :validate_role
 
   before_destroy :set_issue_category_nil
-  after_destroy :unwatch_from_permission_change
 
   def role
   end
@@ -52,7 +51,6 @@ class Member < ActiveRecord::Base
     member_roles_to_destroy = member_roles.select {|mr| !ids.include?(mr.role_id)}
     if member_roles_to_destroy.any?
       member_roles_to_destroy.each(&:destroy)
-      unwatch_from_permission_change
     end
   end
 
@@ -112,13 +110,4 @@ class Member < ActiveRecord::Base
   def validate_role
     errors.add_on_empty :role if member_roles.empty? && roles.empty?
   end
-
-  private
-
-  # Unwatch things that the user is no longer allowed to view inside project
-  def unwatch_from_permission_change
-    if user
-      Watcher.prune(:user => user, :project => project)
-    end
-  end
 end
diff --git a/app/models/member_role.rb b/app/models/member_role.rb
index f344696be..29ad6563d 100644
--- a/app/models/member_role.rb
+++ b/app/models/member_role.rb
@@ -66,9 +66,6 @@ class MemberRole < ActiveRecord::Base
   def remove_inherited_roles
     MemberRole.where(:inherited_from => id).all.group_by(&:member).each do |member, member_roles|
       member_roles.each(&:destroy)
-      if member && member.user
-        Watcher.prune(:user => member.user, :project => member.project)
-      end
     end
   end
 end
diff --git a/test/unit/member_test.rb b/test/unit/member_test.rb
index 9d1432805..275e16599 100644
--- a/test/unit/member_test.rb
+++ b/test/unit/member_test.rb
@@ -122,70 +122,4 @@ class MemberTest < ActiveSupport::TestCase
     assert_equal -1, a <=> b
     assert_equal 1,  b <=> a
   end
-
-  context "removing permissions" do
-    setup do
-      Watcher.delete_all("user_id = 9")
-      user = User.find(9)
-      # public
-      Watcher.create!(:watchable => Issue.find(1), :user => user)
-      # private
-      Watcher.create!(:watchable => Issue.find(4), :user => user)
-      Watcher.create!(:watchable => Message.find(7), :user => user)
-      Watcher.create!(:watchable => Wiki.find(2), :user => user)
-      Watcher.create!(:watchable => WikiPage.find(3), :user => user)
-    end
-
-    context "of user" do
-      setup do
-        @member = Member.create!(:project => Project.find(2), :principal => User.find(9), :role_ids => [1, 2])
-      end
-
-      context "by deleting membership" do
-        should "prune watchers" do
-          assert_difference 'Watcher.count', -4 do
-            @member.destroy
-          end
-        end
-      end
-
-      context "by updating roles" do
-        should "prune watchers" do
-          Role.find(2).remove_permission! :view_wiki_pages
-          member = Member.first(:order => 'id desc')
-          assert_difference 'Watcher.count', -2 do
-            member.role_ids = [2]
-            member.save
-          end
-          assert !Message.find(7).watched_by?(@user)
-        end
-      end
-    end
-
-    context "of group" do
-      setup do
-        group = Group.find(10)
-        @member = Member.create!(:project => Project.find(2), :principal => group, :role_ids => [1, 2])
-        group.users << User.find(9)
-      end
-
-      context "by deleting membership" do
-        should "prune watchers" do
-          assert_difference 'Watcher.count', -4 do
-            @member.destroy
-          end
-        end
-      end
-
-      context "by updating roles" do
-        should "prune watchers" do
-          Role.find(2).remove_permission! :view_wiki_pages
-          assert_difference 'Watcher.count', -2 do
-            @member.role_ids = [2]
-            @member.save
-          end
-        end
-      end
-    end
-  end
 end