From 856ef810b4854c5524843f7f4f69157b6223bfc7 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 10 Feb 2013 10:31:12 +0000 Subject: [PATCH] Bulk watch/unwatch issues from the context menu (#7159). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11339 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/watchers_controller.rb | 36 ++++++----- app/helpers/watchers_helper.rb | 31 +++++---- app/views/context_menus/issues.html.erb | 7 ++- app/views/watchers/_new.html.erb | 10 +-- lib/redmine.rb | 2 +- test/functional/watchers_controller_test.rb | 49 +++++++++++++-- test/unit/helpers/watchers_helper_test.rb | 69 +++++++++++++++++++++ 7 files changed, 164 insertions(+), 40 deletions(-) create mode 100644 test/unit/helpers/watchers_helper_test.rb diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index 00d36f90c..2f55de2f0 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -16,23 +16,19 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class WatchersController < ApplicationController - before_filter :find_project - before_filter :require_login, :check_project_privacy, :only => [:watch, :unwatch] - before_filter :authorize, :only => [:new, :destroy] - accept_api_auth :create, :destroy + before_filter :require_login, :find_watchables, :only => [:watch, :unwatch] def watch - if @watched.respond_to?(:visible?) && !@watched.visible?(User.current) - render_403 - else - set_watcher(User.current, true) - end + set_watcher(@watchables, User.current, true) end def unwatch - set_watcher(User.current, false) + set_watcher(@watchables, User.current, false) end + before_filter :find_project, :authorize, :only => [:new, :create, :append, :destroy, :autocomplete_for_user] + accept_api_auth :create, :destroy + def new end @@ -77,7 +73,8 @@ class WatchersController < ApplicationController render :layout => false end -private + private + def find_project if params[:object_type] && params[:object_id] klass = Object.const_get(params[:object_type].camelcase) @@ -91,11 +88,22 @@ private render_404 end - def set_watcher(user, watching) - @watched.set_watcher(user, watching) + def find_watchables + klass = Object.const_get(params[:object_type].camelcase) rescue nil + if klass && klass.respond_to?('watched_by') + @watchables = klass.find_all_by_id(Array.wrap(params[:object_id])) + raise Unauthorized if @watchables.any? {|w| w.respond_to?(:visible?) && !w.visible?} + end + render_404 unless @watchables.present? + end + + def set_watcher(watchables, user, watching) + watchables.each do |watchable| + watchable.set_watcher(user, watching) + end respond_to do |format| format.html { redirect_to_referer_or {render :text => (watching ? 'Watcher added.' : 'Watcher removed.'), :layout => true}} - format.js { render :partial => 'set_watcher', :locals => {:user => user, :watched => @watched} } + format.js { render :partial => 'set_watcher', :locals => {:user => user, :watched => watchables} } end end end diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb index 5b3b1b7b4..3c27fc02d 100644 --- a/app/helpers/watchers_helper.rb +++ b/app/helpers/watchers_helper.rb @@ -24,21 +24,28 @@ module WatchersHelper watcher_link(object, user) end - def watcher_link(object, user) - return '' unless user && user.logged? && object.respond_to?('watched_by?') - watched = object.watched_by?(user) - css = [watcher_css(object), watched ? 'icon icon-fav' : 'icon icon-fav-off'].join(' ') - url = {:controller => 'watchers', - :action => (watched ? 'unwatch' : 'watch'), - :object_type => object.class.to_s.underscore, - :object_id => object.id} - link_to((watched ? l(:button_unwatch) : l(:button_watch)), url, - :remote => true, :method => 'post', :class => css) + def watcher_link(objects, user) + return '' unless user && user.logged? + objects = Array.wrap(objects) + + watched = objects.any? {|object| object.watched_by?(user)} + css = [watcher_css(objects), watched ? 'icon icon-fav' : 'icon icon-fav-off'].join(' ') + text = watched ? l(:button_unwatch) : l(:button_watch) + url = { + :controller => 'watchers', + :action => (watched ? 'unwatch' : 'watch'), + :object_type => objects.first.class.to_s.underscore, + :object_id => (objects.size == 1 ? objects.first.id : objects.map(&:id).sort) + } + + link_to text, url, :remote => true, :method => 'post', :class => css end # Returns the css class used to identify watch links for a given +object+ - def watcher_css(object) - "#{object.class.to_s.underscore}-#{object.id}-watcher" + def watcher_css(objects) + objects = Array.wrap(objects) + id = (objects.size == 1 ? objects.first.id : 'bulk') + "#{objects.first.class.to_s.underscore}-#{id}-watcher" end # Returns a comma separated list of users watching the given object diff --git a/app/views/context_menus/issues.html.erb b/app/views/context_menus/issues.html.erb index 2163d95c1..b30c3a81f 100644 --- a/app/views/context_menus/issues.html.erb +++ b/app/views/context_menus/issues.html.erb @@ -117,10 +117,11 @@ <% end %> +<% if User.current.logged? %> +
  • <%= watcher_link(@issues, User.current) %>
    • +<% end %> + <% if @issue.present? %> - <% if User.current.logged? %> -
  • <%= watcher_link(@issue, User.current) %>
    • - <% end %> <% if @can[:log_time] -%>
  • <%= context_menu_link l(:button_log_time), new_issue_time_entry_path(@issue), :class => 'icon-time-add' %>
    • diff --git a/app/views/watchers/_new.html.erb b/app/views/watchers/_new.html.erb index eba0665b4..9fdf79262 100644 --- a/app/views/watchers/_new.html.erb +++ b/app/views/watchers/_new.html.erb @@ -2,8 +2,9 @@ <%= form_tag({:controller => 'watchers', :action => (watched ? 'create' : 'append'), - :object_type => watched.class.name.underscore, - :object_id => watched}, + :object_type => (watched && watched.class.name.underscore), + :object_id => watched, + :project_id => @project}, :remote => true, :method => :post, :id => 'new-watcher-form') do %> @@ -11,8 +12,9 @@

    <%= label_tag 'user_search', l(:label_user_search) %><%= text_field_tag 'user_search', nil %>

    <%= javascript_tag "observeSearchfield('user_search', 'users_for_watcher', '#{ escape_javascript url_for(:controller => 'watchers', :action => 'autocomplete_for_user', - :object_type => watched.class.name.underscore, - :object_id => watched) }')" %> + :object_type => (watched && watched.class.name.underscore), + :object_id => watched, + :project_id => @project) }')" %>
    <%= principals_check_box_tags 'watcher[user_ids][]', (watched ? watched.addable_watcher_users : User.active.all(:limit => 100)) %> diff --git a/lib/redmine.rb b/lib/redmine.rb index ad248c14f..63fbdeb23 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -127,7 +127,7 @@ Redmine::AccessControl.map do |map| map.permission :save_queries, {:queries => [:new, :create, :edit, :update, :destroy]}, :require => :loggedin # Watchers map.permission :view_issue_watchers, {}, :read => true - map.permission :add_issue_watchers, {:watchers => :new} + map.permission :add_issue_watchers, {:watchers => [:new, :create, :append, :autocomplete_for_user]} map.permission :delete_issue_watchers, {:watchers => :destroy} end diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb index abce6d558..44bda3d99 100644 --- a/test/functional/watchers_controller_test.rb +++ b/test/functional/watchers_controller_test.rb @@ -25,7 +25,7 @@ class WatchersControllerTest < ActionController::TestCase User.current = nil end - def test_watch + def test_watch_a_single_object @request.session[:user_id] = 3 assert_difference('Watcher.count') do xhr :post, :watch, :object_type => 'issue', :object_id => '1' @@ -35,6 +35,27 @@ class WatchersControllerTest < ActionController::TestCase assert Issue.find(1).watched_by?(User.find(3)) end + def test_watch_a_collection_with_a_single_object + @request.session[:user_id] = 3 + assert_difference('Watcher.count') do + xhr :post, :watch, :object_type => 'issue', :object_id => ['1'] + assert_response :success + assert_include '$(".issue-1-watcher")', response.body + end + assert Issue.find(1).watched_by?(User.find(3)) + end + + def test_watch_a_collection_with_multiple_objects + @request.session[:user_id] = 3 + assert_difference('Watcher.count', 2) do + xhr :post, :watch, :object_type => 'issue', :object_id => ['1', '3'] + assert_response :success + assert_include '$(".issue-bulk-watcher")', response.body + end + assert Issue.find(1).watched_by?(User.find(3)) + assert Issue.find(3).watched_by?(User.find(3)) + end + def test_watch_should_be_denied_without_permission Role.find(2).remove_permission! :view_issues @request.session[:user_id] = 3 @@ -70,6 +91,20 @@ class WatchersControllerTest < ActionController::TestCase assert !Issue.find(1).watched_by?(User.find(3)) end + def test_unwatch_a_collection_with_multiple_objects + @request.session[:user_id] = 3 + Watcher.create!(:user_id => 3, :watchable => Issue.find(1)) + Watcher.create!(:user_id => 3, :watchable => Issue.find(3)) + + assert_difference('Watcher.count', -2) do + xhr :post, :unwatch, :object_type => 'issue', :object_id => ['1', '3'] + assert_response :success + assert_include '$(".issue-bulk-watcher")', response.body + end + assert !Issue.find(1).watched_by?(User.find(3)) + assert !Issue.find(3).watched_by?(User.find(3)) + end + def test_new @request.session[:user_id] = 2 xhr :get, :new, :object_type => 'issue', :object_id => '2' @@ -77,7 +112,7 @@ class WatchersControllerTest < ActionController::TestCase assert_match /ajax-modal/, response.body end - def test_new_for_new_record_with_id + def test_new_for_new_record_with_project_id @request.session[:user_id] = 2 xhr :get, :new, :project_id => 1 assert_response :success @@ -85,7 +120,7 @@ class WatchersControllerTest < ActionController::TestCase assert_match /ajax-modal/, response.body end - def test_new_for_new_record_with_identifier + def test_new_for_new_record_with_project_identifier @request.session[:user_id] = 2 xhr :get, :new, :project_id => 'ecookbook' assert_response :success @@ -117,7 +152,8 @@ class WatchersControllerTest < ActionController::TestCase end def test_autocomplete_on_watchable_creation - xhr :get, :autocomplete_for_user, :q => 'mi' + @request.session[:user_id] = 2 + xhr :get, :autocomplete_for_user, :q => 'mi', :project_id => 'ecookbook' assert_response :success assert_select 'input', :count => 4 assert_select 'input[name=?][value=1]', 'watcher[user_ids][]' @@ -127,7 +163,8 @@ class WatchersControllerTest < ActionController::TestCase end def test_autocomplete_on_watchable_update - xhr :get, :autocomplete_for_user, :q => 'mi', :object_id => '2' , :object_type => 'issue' + @request.session[:user_id] = 2 + xhr :get, :autocomplete_for_user, :q => 'mi', :object_id => '2' , :object_type => 'issue', :project_id => 'ecookbook' assert_response :success assert_select 'input', :count => 3 assert_select 'input[name=?][value=2]', 'watcher[user_ids][]' @@ -139,7 +176,7 @@ class WatchersControllerTest < ActionController::TestCase def test_append @request.session[:user_id] = 2 assert_no_difference 'Watcher.count' do - xhr :post, :append, :watcher => {:user_ids => ['4', '7']} + xhr :post, :append, :watcher => {:user_ids => ['4', '7']}, :project_id => 'ecookbook' assert_response :success assert_include 'watchers_inputs', response.body assert_include 'issue[watcher_user_ids][]', response.body diff --git a/test/unit/helpers/watchers_helper_test.rb b/test/unit/helpers/watchers_helper_test.rb new file mode 100644 index 000000000..1f541d709 --- /dev/null +++ b/test/unit/helpers/watchers_helper_test.rb @@ -0,0 +1,69 @@ +# Redmine - project management software +# Copyright (C) 2006-2013 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 +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.expand_path('../../../test_helper', __FILE__) + +class WatchersHelperTest < ActionView::TestCase + include WatchersHelper + include Redmine::I18n + + fixtures :users, :issues + + def setup + super + set_language_if_valid('en') + User.current = nil + end + + test '#watcher_link with a non-watched object' do + expected = link_to( + "Watch", + "/watchers/watch?object_id=1&object_type=issue", + :remote => true, :method => 'post', :class => "issue-1-watcher icon icon-fav-off" + ) + assert_equal expected, watcher_link(Issue.find(1), User.find(1)) + end + + test '#watcher_link with a single objet array' do + expected = link_to( + "Watch", + "/watchers/watch?object_id=1&object_type=issue", + :remote => true, :method => 'post', :class => "issue-1-watcher icon icon-fav-off" + ) + assert_equal expected, watcher_link([Issue.find(1)], User.find(1)) + end + + test '#watcher_link with a multiple objets array' do + expected = link_to( + "Watch", + "/watchers/watch?object_id%5B%5D=1&object_id%5B%5D=3&object_type=issue", + :remote => true, :method => 'post', :class => "issue-bulk-watcher icon icon-fav-off" + ) + assert_equal expected, watcher_link([Issue.find(1), Issue.find(3)], User.find(1)) + end + + test '#watcher_link with a watched object' do + Watcher.create!(:watchable => Issue.find(1), :user => User.find(1)) + + expected = link_to( + "Unwatch", + "/watchers/unwatch?object_id=1&object_type=issue", + :remote => true, :method => 'post', :class => "issue-1-watcher icon icon-fav" + ) + assert_equal expected, watcher_link(Issue.find(1), User.find(1)) + end +end