Merge pull request #603 from opf/bugfix/416_possible_watchers

Bugfix/416 possible watchers

app/models/work_package.rb

@@ -681,6 +681,13 @@ class WorkPackage < ActiveRecord::Base
     work_package
   end
 
+  # Override of acts_as_watchable#possible_watcher_users
+  # Restricts the result to project members if the project is private
+  def possible_watcher_users
+    users = project.is_public? ? User.not_builtin : project.users
+    users.select {|user| possible_watcher?(user)}
+  end
+
   protected
 
   def recalculate_attributes_for(work_package_id)

doc/CHANGELOG.md

@@ -29,6 +29,7 @@ See doc/COPYRIGHT.rdoc for more details.
 
 # Changelog
 
+* `#416`  Fix: Too many users selectable as watchers
 * `#2697` Fix: Missing migration of planning element watchers
 * `#2564` Support custom fields in REST API v2 for work packages and projects
 * `#2567` [Timelines] Select2 selection shows double escaped character
@@ -82,7 +83,7 @@ See doc/COPYRIGHT.rdoc for more details.
 * `#2597` [Roadmap] Missing english/german closed percentage label
 * `#2604` [Migration] Attachable journals incorrect
 * `#2608` [Activity] Clicking on Atom-feed in activity leads to 500 error
-* Add rake task for changing timestamps in the database to UTC
+* `#2598` Add rake task for changing timestamps in the database to UTC
 
 ## 3.0.0pre23
 

spec/factories/role_factory.rb

@@ -36,12 +36,14 @@ FactoryGirl.define do
       name "Non member"
       builtin Role::BUILTIN_NON_MEMBER
       assignable false
+      initialize_with { Role.find_or_create_by_name(name) }
     end
 
     factory :anonymous_role do
       name "Anonymous"
       builtin Role::BUILTIN_ANONYMOUS
       assignable false
+      initialize_with { Role.find_or_create_by_name(name) }
     end
   end
 end

spec/models/work_package/work_package_acts_as_watchable_spec.rb

@@ -46,13 +46,6 @@ describe WorkPackage do
     let!(:admin){ FactoryGirl.create(:admin) }
     let!(:anonymous_user){ FactoryGirl.create(:anonymous) }
 
-    shared_examples 'it provides possible watchers' do
-      example 'and contains exactly those users who are allowed to view work packages' do
-        users_allowed_to_view_work_packages = User.all.select{ |u| u.allowed_to?(:view_work_packages, project) }
-        work_package.possible_watcher_users.sort.should == Array.wrap(users_allowed_to_view_work_packages).sort
-      end
-    end
-
     shared_context 'non member role has the permission to view work packages' do
       let(:non_member_role) { Role.find_by_name('Non member') }
 
@@ -66,7 +59,10 @@ describe WorkPackage do
     end
 
     context 'when it is a public project' do
-      it_behaves_like 'it provides possible watchers'
+      it 'contains non-anonymous users who are allowed to view work packages' do
+        users_allowed_to_view_work_packages = User.not_builtin.select{ |u| u.allowed_to?(:view_work_packages, project) }
+        work_package.possible_watcher_users.sort.should == users_allowed_to_view_work_packages.sort
+      end
 
       it { should include(admin) }
       it { should include(project_member) }
@@ -93,11 +89,14 @@ describe WorkPackage do
         work_package.reload
       end
 
-      it_behaves_like 'it provides possible watchers'
+      it 'contains project members who are allowed to view work packages' do
+        users_allowed_to_view_work_packages = project.users.select{ |u| u.allowed_to?(:view_work_packages, project) }
+        work_package.possible_watcher_users.sort.should == users_allowed_to_view_work_packages.sort
+      end
 
       it { should include(project_member) }
-      it { should include(admin) }
 
+      it { should_not include(admin) }
       it { should_not include(non_member_user) }
       it { should_not include(anonymous_user) }
     end