Makes user unwatch what he can no longer view after its permissions have changed (#3589).

A rake task (redmine:watchers:prune) is also added to prune existing watchers.

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

app/models/board.rb

@@ -27,6 +27,10 @@ class Board < ActiveRecord::Base
   validates_length_of :name, :maximum => 30
   validates_length_of :description, :maximum => 255
   
+  def visible?(user=User.current)
+    !user.nil? && user.allowed_to?(:view_messages, project)
+  end
+  
   def to_s
     name
   end

app/models/member.rb

@@ -24,6 +24,8 @@ class Member < ActiveRecord::Base
 
   validates_presence_of :principal, :project
   validates_uniqueness_of :user_id, :scope => :project_id
+
+  after_destroy :unwatch_from_permission_change
   
   def name
     self.user.name
@@ -39,7 +41,11 @@ class Member < ActiveRecord::Base
     # Add new roles
     new_role_ids.each {|id| member_roles << MemberRole.new(:role_id => id) }
     # Remove roles (Rails' #role_ids= will not trigger MemberRole#on_destroy)
-    member_roles.select {|mr| !ids.include?(mr.role_id)}.each(&:destroy)
+    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
   
   def <=>(member)
@@ -63,4 +69,13 @@ class Member < ActiveRecord::Base
   def validate
     errors.add_to_base "Role can't be blank" 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

app/models/member_role.rb

@@ -49,6 +49,11 @@ class MemberRole < ActiveRecord::Base
   end
   
   def remove_role_from_group_users
-    MemberRole.find(:all, :conditions => { :inherited_from => id }).each(&:destroy)
+    MemberRole.find(:all, :conditions => { :inherited_from => id }).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

app/models/message.rb

@@ -42,6 +42,10 @@ class Message < ActiveRecord::Base
   
   after_create :add_author_as_watcher
   
+  def visible?(user=User.current)
+    !user.nil? && user.allowed_to?(:view_messages, project)
+  end
+  
   def validate_on_create
     # Can not reply to a locked topic
     errors.add_to_base 'Topic is locked' if root.locked? && self != root

app/models/watcher.rb

@@ -21,10 +21,45 @@ class Watcher < ActiveRecord::Base
   
   validates_presence_of :user
   validates_uniqueness_of :user_id, :scope => [:watchable_type, :watchable_id]
+
+  # Unwatch things that users are no longer allowed to view
+  def self.prune(options={})
+    if options.has_key?(:user)
+      prune_single_user(options[:user], options)
+    else
+      pruned = 0
+      User.find(:all, :conditions => "id IN (SELECT DISTINCT user_id FROM #{table_name})").each do |user|
+        pruned += prune_single_user(user, options)
+      end
+      pruned
+    end
+  end
   
   protected
   
   def validate
     errors.add :user_id, :invalid unless user.nil? || user.active?
   end
+  
+  private
+  
+  def self.prune_single_user(user, options={})
+    return unless user.is_a?(User)
+    pruned = 0
+    find(:all, :conditions => {:user_id => user.id}).each do |watcher|
+      next if watcher.watchable.nil?
+      
+      if options.has_key?(:project)
+        next unless watcher.watchable.respond_to?(:project) && watcher.watchable.project == options[:project]
+      end
+      
+      if watcher.watchable.respond_to?(:visible?)
+        unless watcher.watchable.visible?(user)
+          watcher.destroy
+          pruned += 1
+        end
+      end
+    end
+    pruned
+  end
 end

app/models/wiki.rb

@@ -25,6 +25,10 @@ class Wiki < ActiveRecord::Base
   validates_presence_of :start_page
   validates_format_of :start_page, :with => /^[^,\.\/\?\;\|\:]*$/
   
+  def visible?(user=User.current)
+    !user.nil? && user.allowed_to?(:view_wiki_pages, project)
+  end
+  
   # find the page with the given title
   # if page doesn't exist, return a new page
   def find_or_new_page(title)

app/models/wiki_page.rb

@@ -40,6 +40,10 @@ class WikiPage < ActiveRecord::Base
   validates_format_of :title, :with => /^[^,\.\/\?\;\|\s]*$/
   validates_uniqueness_of :title, :scope => :wiki_id, :case_sensitive => false
   validates_associated :content
+  
+  def visible?(user=User.current)
+    !user.nil? && user.allowed_to?(:view_wiki_pages, project)
+  end
 
   def title=(value)
     value = Wiki.titleize(value)

lib/tasks/watchers.rake

@@ -0,0 +1,9 @@
+desc 'Removes watchers from what they can no longer view.'
+
+namespace :redmine do
+  namespace :watchers do
+    task :prune => :environment do
+      Watcher.prune
+    end
+  end
+end

test/fixtures/enabled_modules.yml

@@ -59,3 +59,7 @@ enabled_modules_015:
   name: wiki
   project_id: 2
   id: 15
+enabled_modules_016: 
+  name: boards
+  project_id: 2
+  id: 16

test/fixtures/messages.yml

@@ -66,3 +66,14 @@ messages_006:
   author_id: 3
   parent_id: 4
   board_id: 1
+messages_007: 
+  created_on: <%= 2.days.ago.to_date.to_s(:db) %>
+  updated_on: <%= 2.days.ago.to_date.to_s(:db) %>
+  subject: 'Message on a private project'
+  id: 7
+  replies_count: 0
+  last_reply_id: 
+  content: "This is a private message"
+  author_id: 1
+  parent_id: 
+  board_id: 3

test/unit/member_test.rb

@@ -18,7 +18,7 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class MemberTest < ActiveSupport::TestCase
-  fixtures :users, :projects, :roles, :members, :member_roles
+  fixtures :all
 
   def setup
     @jsmith = Member.find(1)
@@ -68,4 +68,70 @@ class MemberTest < ActiveSupport::TestCase
     
     assert_raise(ActiveRecord::RecordNotFound) { Member.find(@jsmith.id) }
   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_id => 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

test/unit/watcher_test.rb

@@ -1,5 +1,5 @@
-# redMine - project management software
-# Copyright (C) 2006-2007  Jean-Philippe Lang
+# Redmine - project management software
+# Copyright (C) 2006-2009  Jean-Philippe Lang
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -18,7 +18,11 @@
 require File.dirname(__FILE__) + '/../test_helper'
 
 class WatcherTest < ActiveSupport::TestCase
-  fixtures :issues, :users
+  fixtures :projects, :users, :members, :member_roles, :roles, :enabled_modules,
+           :issues,
+           :boards, :messages,
+           :wikis, :wiki_pages,
+           :watchers
 
   def setup
     @user = User.find(1)
@@ -66,4 +70,36 @@ class WatcherTest < ActiveSupport::TestCase
     @issue.reload
     assert_equal 1, @issue.remove_watcher(@user)  
   end
+  
+  def test_prune
+    Watcher.delete_all("user_id = 9")
+    user = User.find(9)
+    
+    # public
+    Watcher.create!(:watchable => Issue.find(1), :user => user)
+    Watcher.create!(:watchable => Issue.find(2), :user => user)
+    Watcher.create!(:watchable => Message.find(1), :user => user)
+    Watcher.create!(:watchable => Wiki.find(1), :user => user)
+    Watcher.create!(:watchable => WikiPage.find(2), :user => user)
+    
+    # private project (id: 2)
+    Member.create!(:project => Project.find(2), :principal => user, :role_ids => [1])
+    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)
+    
+    assert_no_difference 'Watcher.count' do
+      Watcher.prune(:user => User.find(9))
+    end
+    
+    Member.delete_all
+    
+    assert_difference 'Watcher.count', -4 do
+      Watcher.prune(:user => User.find(9))
+    end
+    
+    assert Issue.find(1).watched_by?(user)
+    assert !Issue.find(4).watched_by?(user)
+  end
 end