From fae5250e52506356965a478518aa7960ada3ec61 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 24 Mar 2012 12:57:28 +0000 Subject: [PATCH] Ability to add non-member watchers on issue creation (#5159). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9254 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 1 + app/controllers/watchers_controller.rb | 34 ++++++++++++++++++--- app/helpers/watchers_helper.rb | 8 +++++ app/views/attachments/_form.html.erb | 5 ++- app/views/issues/new.html.erb | 13 +++++--- app/views/watchers/_new.html.erb | 4 +-- config/locales/en.yml | 1 + config/routes.rb | 2 ++ public/stylesheets/application.css | 6 ++++ test/functional/issues_controller_test.rb | 15 +++++++++ test/functional/watchers_controller_test.rb | 19 ++++++++++++ test/integration/routing/watchers_test.rb | 4 +++ 12 files changed, 98 insertions(+), 14 deletions(-) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 93a087361..f8d982c12 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -407,6 +407,7 @@ private @priorities = IssuePriority.active @allowed_statuses = @issue.new_statuses_allowed_to(User.current, true) + @available_watchers = (@issue.project.users.sort + @issue.watcher_users).uniq end def check_for_default_issue_status diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb index c51722b6a..4c45ff7b2 100644 --- a/app/controllers/watchers_controller.rb +++ b/app/controllers/watchers_controller.rb @@ -64,6 +64,23 @@ class WatchersController < ApplicationController render :text => 'Watcher added.', :layout => true end + def append + if params[:watcher].is_a?(Hash) + user_ids = params[:watcher][:user_ids] || [params[:watcher][:user_id]] + users = User.active.find_all_by_id(user_ids) + respond_to do |format| + format.js do + render :update do |page| + users.each do |user| + page.select("#issue_watcher_user_ids_#{user.id}").each(&:hide) + end + page.insert_html :bottom, 'watchers_inputs', :text => watchers_checkboxes(nil, users, true) + end + end + end + end + end + def destroy @watched.set_watcher(User.find(params[:user_id]), false) if request.post? respond_to do |format| @@ -77,16 +94,23 @@ class WatchersController < ApplicationController end def autocomplete_for_user - @users = User.active.like(params[:q]).find(:all, :limit => 100) - @watched.watcher_users + @users = User.active.like(params[:q]).find(:all, :limit => 100) + if @watched + @user -= @watched.watcher_users + end render :layout => false end private def find_project - klass = Object.const_get(params[:object_type].camelcase) - return false unless klass.respond_to?('watched_by') - @watched = klass.find(params[:object_id]) - @project = @watched.project + if params[:object_type] && params[:object_id] + klass = Object.const_get(params[:object_type].camelcase) + return false unless klass.respond_to?('watched_by') + @watched = klass.find(params[:object_id]) + @project = @watched.project + elsif params[:project_id] + @project = Project.visible.find(params[:project_id]) + end rescue render_404 end diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb index 74bb0e1ec..fb77a90ae 100644 --- a/app/helpers/watchers_helper.rb +++ b/app/helpers/watchers_helper.rb @@ -63,4 +63,12 @@ module WatchersHelper end (lis.empty? ? "" : "").html_safe end + + def watchers_checkboxes(object, users, checked=nil) + users.map do |user| + c = checked.nil? ? object.watched_by?(user) : checked + tag = check_box_tag 'issue[watcher_user_ids][]', user.id, c, :id => nil + content_tag 'label', "#{tag} #{h(user)}", :id => "issue_watcher_user_ids_#{user.id}", :class => "floating" + end.join + end end diff --git a/app/views/attachments/_form.html.erb b/app/views/attachments/_form.html.erb index 464d3a2d7..7eea17bb7 100644 --- a/app/views/attachments/_form.html.erb +++ b/app/views/attachments/_form.html.erb @@ -14,6 +14,5 @@ <%= link_to_function(image_tag('delete.png'), 'removeFileField(this)', :title => (l(:button_delete))) %> -<%= link_to l(:label_add_another_file), '#', :onclick => 'addFileField(); return false;' %> -(<%= l(:label_max_size) %>: <%= number_to_human_size(Setting.attachment_max_size.to_i.kilobytes) %>) - +<%= link_to l(:label_add_another_file), '#', :onclick => 'addFileField(); return false;', :class => 'add_attachment' %> +(<%= l(:label_max_size) %>: <%= number_to_human_size(Setting.attachment_max_size.to_i.kilobytes) %>) diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb index 11268214f..c95daa1e4 100644 --- a/app/views/issues/new.html.erb +++ b/app/views/issues/new.html.erb @@ -22,10 +22,15 @@ <% if @issue.safe_attribute? 'watcher_user_ids' -%>

- <% @issue.project.users.sort.each do |user| -%> - - <% end -%> -

+ + <%= watchers_checkboxes(@issue, @available_watchers) %> + + + <%= link_to_remote l(:label_search_for_watchers), + :url => {:controller => 'watchers', :action => 'new', :project_id => @issue.project}, + :method => 'get' %> + +

<% end %> diff --git a/app/views/watchers/_new.html.erb b/app/views/watchers/_new.html.erb index 9b4227ad7..c2c133ee7 100644 --- a/app/views/watchers/_new.html.erb +++ b/app/views/watchers/_new.html.erb @@ -1,7 +1,7 @@

<%= l(:permission_add_issue_watchers) %>

<% form_remote_tag :url => {:controller => 'watchers', - :action => 'create', + :action => (watched ? 'create' : 'append'), :object_type => watched.class.name.underscore, :object_id => watched}, :method => :post, @@ -22,7 +22,7 @@ :with => 'q') %>
- <%= principals_check_box_tags 'watcher[user_ids][]', watched.addable_watcher_users %> + <%= principals_check_box_tags 'watcher[user_ids][]', (watched ? watched.addable_watcher_users : User.active.all(:limit => 100)) %>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 21e12ba17..6180a860e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -844,6 +844,7 @@ en: label_copy_attachments: Copy attachments label_item_position: "%{position} of %{count}" label_completed_versions: Completed versions + label_search_for_watchers: Search for watchers to add button_login: Login button_submit: Submit diff --git a/config/routes.rb b/config/routes.rb index 3e98e9043..25a7ca59b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -124,6 +124,8 @@ ActionController::Routing::Routes.draw do |map| :conditions => {:method => :get} map.connect 'watchers', :controller=> 'watchers', :action => 'create', :conditions => {:method => :post} + map.connect 'watchers/append', :controller=> 'watchers', :action => 'append', + :conditions => {:method => :post} map.connect 'watchers/destroy', :controller=> 'watchers', :action => 'destroy', :conditions => {:method => :post} map.connect 'watchers/watch', :controller=> 'watchers', :action => 'watch', diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 937a69a83..72090235e 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -265,6 +265,12 @@ div.projects h3 { background: url(../images/projects.png) no-repeat 0% 50%; padd #watchers a.delete:hover {opacity: 1;} #watchers img.gravatar {margin: 0 4px 2px 0;} +span#watchers_inputs {overflow:auto; display:block;} +span.search_for_watchers {display:block;} +span.search_for_watchers, span.add_attachment {font-size:80%; line-height:2.5em;} +span.search_for_watchers a, span.add_attachment a {padding-left:16px; background: url(../images/bullet_add.png) no-repeat 0 50%; } + + .highlight { background-color: #FCFD8D;} .highlight.token-1 { background-color: #faa;} .highlight.token-2 { background-color: #afa;} diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 199e9317e..e91acb20c 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1680,6 +1680,21 @@ class IssuesControllerTest < ActionController::TestCase :value => 'Value for field 2'} end + def test_post_create_with_failure_should_preserve_watchers + assert !User.find(8).member_of?(Project.find(1)) + + @request.session[:user_id] = 2 + post :create, :project_id => 1, + :issue => {:tracker_id => 1, + :watcher_user_ids => ['3', '8']} + assert_response :success + assert_template 'new' + + assert_tag 'input', :attributes => {:name => 'issue[watcher_user_ids][]', :value => '2', :checked => nil} + assert_tag 'input', :attributes => {:name => 'issue[watcher_user_ids][]', :value => '3', :checked => 'checked'} + assert_tag 'input', :attributes => {:name => 'issue[watcher_user_ids][]', :value => '8', :checked => 'checked'} + end + def test_post_create_should_ignore_non_safe_attributes @request.session[:user_id] = 2 assert_nothing_raised do diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb index bfd93fe0d..03b1a977b 100644 --- a/test/functional/watchers_controller_test.rb +++ b/test/functional/watchers_controller_test.rb @@ -68,6 +68,13 @@ class WatchersControllerTest < ActionController::TestCase assert_select_rjs :replace_html, 'ajax-modal' end + def test_new_for_new_record + @request.session[:user_id] = 2 + xhr :get, :new, :project_id => 1 + assert_response :success + assert_select_rjs :replace_html, 'ajax-modal' + end + def test_create @request.session[:user_id] = 2 assert_difference('Watcher.count') do @@ -91,6 +98,18 @@ class WatchersControllerTest < ActionController::TestCase assert Issue.find(2).watched_by?(User.find(7)) end + def test_append + @request.session[:user_id] = 2 + assert_no_difference 'Watcher.count' do + xhr :post, :append, :watcher => {:user_ids => ['4', '7']} + assert_response :success + assert_select_rjs :insert_html, 'watchers_inputs' do + assert_select 'input[name=?][value=4]', 'issue[watcher_user_ids][]' + assert_select 'input[name=?][value=7]', 'issue[watcher_user_ids][]' + end + end + end + def test_remove_watcher @request.session[:user_id] = 2 assert_difference('Watcher.count', -1) do diff --git a/test/integration/routing/watchers_test.rb b/test/integration/routing/watchers_test.rb index 441d2533c..00410289b 100644 --- a/test/integration/routing/watchers_test.rb +++ b/test/integration/routing/watchers_test.rb @@ -23,6 +23,10 @@ class RoutingWatchersTest < ActionController::IntegrationTest { :method => 'get', :path => "/watchers/new" }, { :controller => 'watchers', :action => 'new' } ) + assert_routing( + { :method => 'post', :path => "/watchers/append" }, + { :controller => 'watchers', :action => 'append' } + ) assert_routing( { :method => 'post', :path => "/watchers" }, { :controller => 'watchers', :action => 'create' } -- 2.39.5