diff --git a/app/controllers/auto_completes_controller.rb b/app/controllers/auto_completes_controller.rb index becc957d..27226ea8 100644 --- a/app/controllers/auto_completes_controller.rb +++ b/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 diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 29b642d2..987f9838 100644 --- a/app/controllers/groups_controller.rb +++ b/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) diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index e9479f43..5d5d2d50 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -34,9 +34,12 @@ class WatchersController < ApplicationController end def new - @watcher = Watcher.new(params[:watcher]) - @watcher.watchable = @watched - @watcher.save if request.post? + 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 diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 83007e15..f4396b56 100644 --- a/app/helpers/application_helper.rb +++ b/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 << "\n" + s << "\n" end s end diff --git a/app/views/groups/autocomplete_for_user.html.erb b/app/views/auto_completes/users.html.erb similarity index 100% rename from app/views/groups/autocomplete_for_user.html.erb rename to app/views/auto_completes/users.html.erb diff --git a/app/views/groups/_users.html.erb b/app/views/groups/_users.html.erb index 33091ea3..2fa91cb5 100644 --- a/app/views/groups/_users.html.erb +++ b/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') %> diff --git a/app/views/watchers/_watchers.rhtml b/app/views/watchers/_watchers.rhtml index 4da8c0dd..741366cd 100644 --- a/app/views/watchers/_watchers.rhtml +++ b/app/views/watchers/_watchers.rhtml @@ -1,24 +1,31 @@
-<%= link_to_remote l(:button_add), - :url => {:controller => 'watchers', - :action => 'new', - :object_type => watched.class.name.underscore, - :object_id => watched} if User.current.allowed_to?(:add_issue_watchers, @project) %> +<%= link_to_function(l(:button_add), "$('new-watcher-form').toggle();") if User.current.allowed_to?(:add_issue_watchers, @project) %>

<%= l(:label_issue_watchers) %> (<%= watched.watcher_users.size %>)

-<% unless @watcher.nil? %> - <% remote_form_for(:watcher, @watcher, +<% 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| %> -

<%= f.select :user_id, (watched.addable_watcher_users.collect {|m| [m.name, m.id]}), :prompt => "--- #{l(:actionview_instancetag_blank_option)} ---" %> + :html => {:id => 'new-watcher-form', :style => 'display:none;'}) do |f| %> + <% users = User.active.find(:all, :limit => 25) - watched.watcher_users %> +

<%= label_tag "user_search", l(:label_user_search) %><%= text_field_tag 'user_search', nil, :style => "width:98%;" %>

+ <%= observe_field(:user_search, + :frequency => 0.5, + :update => :users, + :url => auto_complete_users_path(:remove_watchers => watched.id, :klass => watched.class), + :with => 'q') + %> + +
+ <%= principals_check_box_tags 'user_ids[]', users %> +
- <%= submit_tag l(:button_add) %> +

<%= submit_tag l(:button_add) %> <%= toggle_link l(:button_cancel), 'new-watcher-form'%>

<% end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 89fb761c..f012ce2a 100644 --- a/config/routes.rb +++ b/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' diff --git a/test/functional/auto_completes_controller_test.rb b/test/functional/auto_completes_controller_test.rb index 88ed0f1e..4ee6482d 100644 --- a/test/functional/auto_completes_controller_test.rb +++ b/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 diff --git a/test/functional/groups_controller_test.rb b/test/functional/groups_controller_test.rb index da656e06..c123c48b 100644 --- a/test/functional/groups_controller_test.rb +++ b/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 diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb index 7023908f..858e807e 100644 --- a/test/functional/watchers_controller_test.rb +++ b/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 diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 6ae25c47..dc9f0532 100644 --- a/test/integration/routing_test.rb +++ b/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 diff --git a/test/unit/member_test.rb b/test/unit/member_test.rb index 43dd1a92..b2b16f9a 100644 --- a/test/unit/member_test.rb +++ b/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