Merge branch 'ticket/unstable/800-non-member-watch' into unstable

app/controllers/auto_completes_controller.rb

@@ -13,7 +13,7 @@
 #++
 
 class AutoCompletesController < ApplicationController
-  before_filter :find_project
+  before_filter :find_project, :only => :issues
 
   def issues
     @issues = []
@@ -33,6 +33,26 @@ class AutoCompletesController < ApplicationController
     render :layout => false
   end
 
+  def users
+    if params[:remove_group_members].present?
+      @group = Group.find(params[:remove_group_members])
+      @removed_users = @group.users
+    end
+
+    if params[:remove_watchers].present? && params[:klass].present?
+      watcher_class = params[:klass].constantize
+      if watcher_class.included_modules.include?(Redmine::Acts::Watchable) # check class is a watching class
+        @object = watcher_class.find(params[:remove_watchers])
+        @removed_users = @object.watcher_users
+      end
+    end
+
+    @removed_users ||= []
+    
+    @users = User.active.like(params[:q]).find(:all, :limit => 100) - @removed_users
+    render :layout => false
+  end
+  
   private
 
   def find_project

app/controllers/groups_controller.rb

@@ -126,12 +126,6 @@ class GroupsController < ApplicationController
     end
   end
 
-  def autocomplete_for_user
-    @group = Group.find(params[:id])
-    @users = User.active.not_in_group(@group).like(params[:q]).all(:limit => 100)
-    render :layout => false
-  end
-
   def edit_membership
     @group = Group.find(params[:id])
     @membership = Member.edit_membership(params[:membership_id], params[:membership], @group)

app/controllers/watchers_controller.rb

@@ -34,9 +34,12 @@ class WatchersController < ApplicationController
   end
 
   def new
-    @watcher = Watcher.new(params[:watcher])
+    params[:user_ids].each do |user_id|
+      @watcher = Watcher.new((params[:watcher] || {}).merge({:user_id => user_id}))
       @watcher.watchable = @watched
       @watcher.save if request.post?
+    end if params[:user_ids].present?
+    
     respond_to do |format|
       format.html { redirect_to :back }
       format.js do

app/helpers/application_helper.rb

@@ -285,7 +285,7 @@ module ApplicationHelper
   def principals_check_box_tags(name, principals)
     s = ''
     principals.sort.each do |principal|
-      s << "<label>#{ check_box_tag name, principal.id, false } #{h principal}</label>\n"
+      s << "<label style='display:block;'>#{ check_box_tag name, principal.id, false } #{h principal}</label>\n"
     end
     s
   end

app/views/groups/_users.html.erb

@@ -33,7 +33,7 @@
 		<%= observe_field(:user_search,
                  :frequency => 0.5,
                  :update => :users,
-                 :url => { :controller => 'groups', :action => 'autocomplete_for_user', :id => @group },
+                 :url => auto_complete_users_path(:remove_group_members => @group),
                  :with => 'q')
                   %>
 

app/views/watchers/_watchers.rhtml

@@ -1,24 +1,31 @@
 <div class="contextual">
-<%= link_to_remote l(:button_add),
+<%= link_to_function(l(:button_add), "$('new-watcher-form').toggle();") if User.current.allowed_to?(:add_issue_watchers, @project) %>
-                   :url => {:controller => 'watchers',
-                            :action => 'new',
-                            :object_type => watched.class.name.underscore,
-                            :object_id => watched} if User.current.allowed_to?(:add_issue_watchers, @project) %>
 </div>
 
 <h3><%= l(:label_issue_watchers) %> (<%= watched.watcher_users.size %>)</h3>
 
-<% unless @watcher.nil? %>
+<% if User.current.allowed_to?(:add_issue_watchers, @project) %>
 	<% remote_form_for(:watcher, @watcher, 
                 	   :url => {:controller => 'watchers',
               	              :action => 'new',
             	                :object_type => watched.class.name.underscore,
          	                    :object_id => watched},
        	             :method => :post,
-      	             :html => {:id => 'new-watcher-form'}) do |f| %>
+      	             :html => {:id => 'new-watcher-form', :style => 'display:none;'}) do |f| %>
-		<p><%= f.select :user_id, (watched.addable_watcher_users.collect {|m| [m.name, m.id]}), :prompt => "--- #{l(:actionview_instancetag_blank_option)} ---" %>
+    <% users = User.active.find(:all, :limit => 25) - watched.watcher_users %>
+		<p><%= label_tag "user_search", l(:label_user_search) %><%= text_field_tag 'user_search', nil, :style => "width:98%;" %></p>
+		<%= observe_field(:user_search,
+                 :frequency => 0.5,
+                 :update => :users,
+                 :url => auto_complete_users_path(:remove_watchers => watched.id, :klass => watched.class),
+                 :with => 'q')
+                  %>
 		
-		<%= submit_tag l(:button_add) %>
+		<div id="users">
+			<%= principals_check_box_tags 'user_ids[]', users %>
+		</div>
+
+		<p><%= submit_tag l(:button_add) %>
 		<%= toggle_link l(:button_cancel), 'new-watcher-form'%></p>
 	<% end %>
 <% end %>

config/routes.rb

@@ -87,6 +87,7 @@ ActionController::Routing::Routes.draw do |map|
 
   # Misc issue routes. TODO: move into resources
   map.auto_complete_issues '/issues/auto_complete', :controller => 'auto_completes', :action => 'issues'
+  map.auto_complete_users '/users/auto_complete', :controller => 'auto_completes', :action => 'users'
   map.preview_issue '/issues/preview/:id', :controller => 'previews', :action => 'issue' # TODO: would look nicer as /issues/:id/preview
   map.issues_context_menu '/issues/context_menu', :controller => 'context_menus', :action => 'issues'
   map.issue_changes '/issues/changes', :controller => 'journals', :action => 'index'

test/functional/auto_completes_controller_test.rb

@@ -63,4 +63,69 @@ class AutoCompletesControllerTest < ActionController::TestCase
     assert_response :success
     assert_equal [], assigns(:issues)
   end
+
+  context "GET :users" do
+    setup do
+      @login = User.generate!(:login => 'Acomplete')
+      @firstname = User.generate!(:firstname => 'Complete')
+      @lastname = User.generate!(:lastname => 'Complete')
+      @none = User.generate!(:login => 'hello', :firstname => 'ABC', :lastname => 'DEF')
+      @inactive = User.generate!(:firstname => 'Complete', :status => User::STATUS_LOCKED)
+    end
+
+    context "with no restrictions" do
+      setup do
+        get :users, :q => 'complete'
+      end
+      
+      should_respond_with :success
+      
+      should "render a list of matching users in checkboxes" do
+        assert_select "input[type=checkbox][value=?]", @login.id
+        assert_select "input[type=checkbox][value=?]", @firstname.id
+        assert_select "input[type=checkbox][value=?]", @lastname.id
+        assert_select "input[type=checkbox][value=?]", @none.id, :count => 0
+      end
+      
+      should "only show active users" do
+        assert_select "input[type=checkbox][value=?]", @inactive.id, :count => 0
+      end
+    end
+
+    context "restrict by removing group members" do
+      setup do
+        @group = Group.first
+        @group.users << @login
+        @group.users << @firstname
+        get :users, :q => 'complete', :remove_group_members => @group.id
+      end
+      
+      should_respond_with :success
+      
+      should "not include existing members of the Group" do
+        assert_select "input[type=checkbox][value=?]", @lastname.id
+
+        assert_select "input[type=checkbox][value=?]", @login.id, :count => 0
+        assert_select "input[type=checkbox][value=?]", @firstname.id, :count => 0
+      end
+    end
+    
+    context "restrict by removing issue watchers" do
+      setup do
+        @issue = Issue.find(2)
+        @issue.add_watcher(@login)
+        @issue.add_watcher(@firstname)
+        get :users, :q => 'complete', :remove_watchers => @issue.id, :klass => 'Issue'
+      end
+      
+      should_respond_with :success
+      
+      should "not include existing watchers" do
+        assert_select "input[type=checkbox][value=?]", @lastname.id
+
+        assert_select "input[type=checkbox][value=?]", @login.id, :count => 0
+        assert_select "input[type=checkbox][value=?]", @firstname.id, :count => 0
+      end
+    end
+  end
 end

test/functional/groups_controller_test.rb

@@ -100,13 +100,4 @@ class GroupsControllerTest < ActionController::TestCase
       post :destroy_membership, :id => 10, :membership_id => 6
     end
   end
-
-  def test_autocomplete_for_user
-    get :autocomplete_for_user, :id => 10, :q => 'mis'
-    assert_response :success
-    users = assigns(:users)
-    assert_not_nil users
-    assert users.any?
-    assert !users.include?(Group.find(10).users.first)
-  end
 end

test/functional/watchers_controller_test.rb

@@ -114,13 +114,24 @@ class WatchersControllerTest < ActionController::TestCase
   def test_new_watcher
     @request.session[:user_id] = 2
     assert_difference('Watcher.count') do
-      xhr :post, :new, :object_type => 'issue', :object_id => '2', :watcher => {:user_id => '4'}
+      xhr :post, :new, :object_type => 'issue', :object_id => '2', :user_ids => ['4']
       assert_response :success
       assert_select_rjs :replace_html, 'watchers'
     end
     assert Issue.find(2).watched_by?(User.find(4))
   end
 
+  def test_new_multiple_users
+    @request.session[:user_id] = 2
+    assert_difference('Watcher.count', 2) do
+      xhr :post, :new, :object_type => 'issue', :object_id => '2', :user_ids => ['4','7']
+      assert_response :success
+      assert_select_rjs :replace_html, 'watchers'
+    end
+    assert Issue.find(2).watched_by?(User.find(4))
+    assert Issue.find(2).watched_by?(User.find(7))
+  end
+
   def test_remove_watcher
     @request.session[:user_id] = 2
     assert_difference('Watcher.count', -1) do

test/integration/routing_test.rb

@@ -346,4 +346,9 @@ class RoutingTest < ActionController::IntegrationTest
   context "administration panel" do
     should_route :get, "/admin/projects", :controller => 'admin', :action => 'projects'
   end
+
+  context "auto_completes" do
+    should_route :get, "/users/auto_complete", :controller => 'auto_completes', :action => 'users'
+  end
+  
 end

test/unit/member_test.rb

@@ -89,6 +89,15 @@ class MemberTest < ActiveSupport::TestCase
             @member.destroy
           end
         end
+
+        should "not prune watchers if the user still has permission to watch as a non-member" do
+          @member_on_public_project = Member.create!(:project => Project.find(1), :principal => User.find(9), :role_ids => [1, 2])
+
+          assert_no_difference 'Watcher.count' do
+            @member_on_public_project.destroy
+          end
+        end
+        
       end
 
       context "by updating roles" do