jro_apps/redmine Commit - r5200:e3dae9ddbf46

Cleaner way to handle the replacement of watch links (#8071)....

Jean-Philippe Lang -

r5200:e3dae9ddbf46

parent child

Context file:

r5200:e3dae9ddbf46

Collapse all files

app/controllers/watchers_controller.rb +5 -13

-            # redMine - project management software
+            # Redmine - project management software
-            # Copyright (C) 2006-2007  Jean-Philippe Lang
+            # Copyright (C) 2006-2011  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.
             class WatchersController < ApplicationController
               before_filter :find_project
               before_filter :require_login, :check_project_privacy, :only => [:watch, :unwatch]
               before_filter :authorize, :only => [:new, :destroy]
               verify :method => :post,
                      :only => [ :watch, :unwatch ],
                      :render => { :nothing => true, :status => :method_not_allowed }
               def watch
                 if @watched.respond_to?(:visible?) && !@watched.visible?(User.current)
                   render_403
                 else
                   set_watcher(User.current, true)
                 end
               end
               def unwatch
                 set_watcher(User.current, false)
               end
               def new
                 @watcher = Watcher.new(params[:watcher])
                 @watcher.watchable = @watched
                 @watcher.save if request.post?
                 respond_to do |format|
                   format.html { redirect_to :back }
                   format.js do
                     render :update do |page|
                       page.replace_html 'watchers', :partial => 'watchers/watchers', :locals => {:watched => @watched}
                     end
                   end
                 end
               rescue ::ActionController::RedirectBackError
                 render :text => 'Watcher added.', :layout => true
               end
               def destroy
                 @watched.set_watcher(User.find(params[:user_id]), false) if request.post?
                 respond_to do |format|
                   format.html { redirect_to :back }
                   format.js do
                     render :update do |page|
                       page.replace_html 'watchers', :partial => 'watchers/watchers', :locals => {:watched => @watched}
                     end
                   end
                 end
               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
               rescue
                 render_404
               end
               def set_watcher(user, watching)
                 @watched.set_watcher(user, watching)
-                if params[:replace].present?
-                  if params[:replace].is_a? Array
-                    replace_ids = params[:replace]
-                  else
-                    replace_ids = [params[:replace]]
-                  end
-                else
-                  replace_ids = ['watcher']
-                end
                 respond_to do |format|
                   format.html { redirect_to :back }
                   format.js do
                     render(:update) do |page|
-                      replace_ids.each do |replace_id|
+                      c = watcher_css(@watched)
-                        page.replace_html replace_id, watcher_link(@watched, user, :replace => replace_ids)
+                      page.select(".#{c}").each do |item|
+                        page.replace_html item, watcher_link(@watched, user)
                       end
                     end
                   end
                 end
               rescue ::ActionController::RedirectBackError
                 render :text => (watching ? 'Watcher added.' : 'Watcher removed.'), :layout => true
               end
             end

app/helpers/watchers_helper.rb +12 -17

-            # redMine - project management software
+            # Redmine - project management software
-            # Copyright (C) 2006-2007  Jean-Philippe Lang
+            # Copyright (C) 2006-2011  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.
             module WatchersHelper
-              # Valid options
+              def watcher_tag(object, user, options={})
-              # * :id - the element id
+                content_tag("span", watcher_link(object, user), :class => watcher_css(object))
-              # * :replace - a string or array of element ids that will be
-              #   replaced
-              def watcher_tag(object, user, options={:replace => 'watcher'})
-                id = options[:id]
-                id ||= options[:replace] if options[:replace].is_a? String
-                content_tag("span", watcher_link(object, user, options), :id => id)
               end
-              # Valid options
+              def watcher_link(object, user)
-              # * :replace - a string or array of element ids that will be
-              #   replaced
-              def watcher_link(object, user, options={:replace => 'watcher'})
                 return '' unless user && user.logged? && object.respond_to?('watched_by?')
                 watched = object.watched_by?(user)
                 url = {:controller => 'watchers',
                        :action => (watched ? 'unwatch' : 'watch'),
                        :object_type => object.class.to_s.underscore,
-                       :object_id => object.id,
+                       :object_id => object.id}
-                       :replace => options[:replace]}
                 link_to_remote((watched ? l(:button_unwatch) : l(:button_watch)),
                                {:url => url},
                                :href => url_for(url),
                                :class => (watched ? 'icon icon-fav' : 'icon icon-fav-off'))
               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"
+              end
               # Returns a comma separated list of users watching the given object
               def watchers_list(object)
                 remove_allowed = User.current.allowed_to?("delete_#{object.class.name.underscore}_watchers".to_sym, object.project)
                 lis = object.watcher_users.collect do |user|
                   s = avatar(user, :size => "16").to_s + link_to_user(user, :class => 'user').to_s
                   if remove_allowed
                     url = {:controller => 'watchers',
                            :action => 'destroy',
                            :object_type => object.class.to_s.underscore,
                            :object_id => object.id,
                            :user_id => user}
                     s += ' ' + link_to_remote(image_tag('delete.png'),
                                               {:url => url},
                                               :href => url_for(url),
                                               :style => "vertical-align: middle",
                                               :class => "delete")
                   end
                   "<li>#{ s }</li>"
                 end
                 lis.empty? ? "" : "<ul>#{ lis.join("\n") }</ul>"
               end
             end

app/views/issues/_action_menu.rhtml +1 -2

             <div class="contextual">
             <%= link_to_if_authorized(l(:button_update), {:controller => 'issues', :action => 'edit', :id => @issue }, :onclick => 'showAndScrollTo("update", "notes"); return false;', :class => 'icon icon-edit', :accesskey => accesskey(:edit)) %>
             <%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'new', :issue_id => @issue}, :class => 'icon icon-time-add' %>
-            <% replace_watcher ||= 'watcher' %>
+            <%= watcher_tag(@issue, User.current) %>
-            <%= watcher_tag(@issue, User.current, {:id => replace_watcher, :replace => ['watcher','watcher2']}) %>
             <%= link_to_if_authorized l(:button_duplicate), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue }, :class => 'icon icon-duplicate' %>
             <%= link_to_if_authorized l(:button_copy), {:controller => 'issue_moves', :action => 'new', :id => @issue, :copy_options => {:copy => 't'}}, :class => 'icon icon-copy' %>
             <%= link_to_if_authorized l(:button_move), {:controller => 'issue_moves', :action => 'new', :id => @issue}, :class => 'icon icon-move' %>
             <%= link_to_if_authorized l(:button_delete), {:controller => 'issues', :action => 'destroy', :id => @issue}, :confirm => (@issue.leaf? ? l(:text_are_you_sure) : l(:text_are_you_sure_with_children)), :method => :post, :class => 'icon icon-del' %>
             </div>

app/views/issues/show.rhtml +1 -1

             <%= render :partial => 'action_menu' %>
             <h2><%= @issue.tracker.name %> #<%= @issue.id %></h2>
             <div class="<%= @issue.css_classes %> details">
                     <%= avatar(@issue.author, :size => "50") %>
             <div class="subject">
             <%= render_issue_subject_with_tree(@issue) %>
             </div>
                     <p class="author">
                     <%= authoring @issue.created_on, @issue.author %>.
                     <% if @issue.created_on != @issue.updated_on %>
                     <%= l(:label_updated_time, time_tag(@issue.updated_on)) %>.
                     <% end %>
                     </p>
             <table class="attributes">
             <tr>
                 <th class="status"><%=l(:field_status)%>:</th><td class="status"><%= @issue.status.name %></td>
                 <th class="start-date"><%=l(:field_start_date)%>:</th><td class="start-date"><%= format_date(@issue.start_date) %></td>
             </tr>
             <tr>
                 <th class="priority"><%=l(:field_priority)%>:</th><td class="priority"><%= @issue.priority.name %></td>
                 <th class="due-date"><%=l(:field_due_date)%>:</th><td class="due-date"><%= format_date(@issue.due_date) %></td>
             </tr>
             <tr>
                 <th class="assigned-to"><%=l(:field_assigned_to)%>:</th><td class="assigned-to"><%= avatar(@issue.assigned_to, :size => "14") %><%= @issue.assigned_to ? link_to_user(@issue.assigned_to) : "-" %></td>
                 <th class="progress"><%=l(:field_done_ratio)%>:</th><td class="progress"><%= progress_bar @issue.done_ratio, :width => '80px', :legend => "#{@issue.done_ratio}%" %></td>
             </tr>
             <tr>
                 <th class="category"><%=l(:field_category)%>:</th><td class="category"><%=h @issue.category ? @issue.category.name : "-" %></td>
                 <% if User.current.allowed_to?(:view_time_entries, @project) %>
                 <th class="spent-time"><%=l(:label_spent_time)%>:</th>
                 <td class="spent-time"><%= @issue.spent_hours > 0 ? (link_to l_hours(@issue.spent_hours), {:controller => 'timelog', :action => 'index', :project_id => @project, :issue_id => @issue}) : "-" %></td>
                 <% end %>
             </tr>
             <tr>
                 <th class="fixed-version"><%=l(:field_fixed_version)%>:</th><td class="fixed-version"><%= @issue.fixed_version ? link_to_version(@issue.fixed_version) : "-" %></td>
                 <% if @issue.estimated_hours %>
                 <th class="estimated-hours"><%=l(:field_estimated_hours)%>:</th><td class="estimated-hours"><%= l_hours(@issue.estimated_hours) %></td>
                 <% end %>
             </tr>
             <%= render_custom_fields_rows(@issue) %>
             <%= call_hook(:view_issues_show_details_bottom, :issue => @issue) %>
             </table>
             <% if @issue.description? || @issue.attachments.any? -%>
             <hr />
             <% if @issue.description? %>
             	<div class="contextual">
             	<%= link_to_remote_if_authorized(l(:button_quote), { :url => {:controller => 'journals', :action => 'new', :id => @issue} }, :class => 'icon icon-comment') %>
             	</div>
             	<p><strong><%=l(:field_description)%></strong></p>
             	<div class="wiki">
             	<%= textilizable @issue, :description, :attachments => @issue.attachments %>
             	</div>
             <% end %>
             <%= link_to_attachments @issue %>
             <% end -%>
             <%= call_hook(:view_issues_show_description_bottom, :issue => @issue) %>
             <% if !@issue.leaf? || User.current.allowed_to?(:manage_subtasks, @project) %>
             <hr />
             <div id="issue_tree">
             <div class="contextual">
               <%= link_to(l(:button_add), {:controller => 'issues', :action => 'new', :project_id => @project, :issue => {:parent_issue_id => @issue}}) if User.current.allowed_to?(:manage_subtasks, @project) %>
             </div>
             <p><strong><%=l(:label_subtask_plural)%></strong></p>
             <%= render_descendants_tree(@issue) unless @issue.leaf? %>
             </div>
             <% end %>
             <% if authorize_for('issue_relations', 'new') || @issue.relations.present? %>
             <hr />
             <div id="relations">
             <%= render :partial => 'relations' %>
             </div>
             <% end %>
             </div>
             <% if @changesets.present? %>
             <div id="issue-changesets">
             <h3><%=l(:label_associated_revisions)%></h3>
             <%= render :partial => 'changesets', :locals => { :changesets => @changesets} %>
             </div>
             <% end %>
             <% if @journals.present? %>
             <div id="history">
             <h3><%=l(:label_history)%></h3>
             <%= render :partial => 'history', :locals => { :issue => @issue, :journals => @journals } %>
             </div>
             <% end %>
             <div style="clear: both;"></div>
-            <%= render :partial => 'action_menu', :locals => {:replace_watcher => 'watcher2' } %>
+            <%= render :partial => 'action_menu' %>
             <div style="clear: both;"></div>
             <% if authorize_for('issues', 'edit') %>
               <div id="update" style="display:none;">
               <h3><%= l(:button_update) %></h3>
               <%= render :partial => 'edit' %>
               </div>
             <% end %>
             <% other_formats_links do |f| %>
             	<%= f.link_to 'Atom', :url => {:key => User.current.rss_key} %>
             	<%= f.link_to 'PDF' %>
             <% end %>
             <% html_title "#{@issue.tracker.name} ##{@issue.id}: #{@issue.subject}" %>
             <% content_for :sidebar do %>
               <%= render :partial => 'issues/sidebar' %>
               <% if User.current.allowed_to?(:add_issue_watchers, @project) ||
                 (@issue.watchers.present? && User.current.allowed_to?(:view_issue_watchers, @project)) %>
                 <div id="watchers">
                   <%= render :partial => 'watchers/watchers', :locals => {:watched => @issue} %>
                 </div>
               <% end %>
             <% end %>
             <% content_for :header_tags do %>
                 <%= auto_discovery_link_tag(:atom, {:format => 'atom', :key => User.current.rss_key}, :title => "#{@issue.project} - #{@issue.tracker} ##{@issue.id}: #{@issue.subject}") %>
                 <%= stylesheet_link_tag 'scm' %>
                 <%= javascript_include_tag 'context_menu' %>
                 <%= stylesheet_link_tag 'context_menu' %>
                 <%= stylesheet_link_tag 'context_menu_rtl' if l(:direction) == 'rtl' %>
             <% end %>
             <div id="context-menu" style="display: none;"></div>
             <%= javascript_tag "new ContextMenu('#{issues_context_menu_path}')" %>

test/functional/watchers_controller_test.rb +3 -24

             # Redmine - project management software
-            # Copyright (C) 2006-2008  Jean-Philippe Lang
+            # Copyright (C) 2006-2011  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__)
             require 'watchers_controller'
             # Re-raise errors caught by the controller.
             class WatchersController; def rescue_action(e) raise e end; end
             class WatchersControllerTest < ActionController::TestCase
               fixtures :projects, :users, :roles, :members, :member_roles, :enabled_modules,
                        :issues, :trackers, :projects_trackers, :issue_statuses, :enumerations, :watchers
               def setup
                 @controller = WatchersController.new
                 @request    = ActionController::TestRequest.new
                 @response   = ActionController::TestResponse.new
                 User.current = nil
               end
               def test_get_watch_should_be_invalid
                 @request.session[:user_id] = 3
                 get :watch, :object_type => 'issue', :object_id => '1'
                 assert_response 405
               end
               def test_watch
                 @request.session[:user_id] = 3
                 assert_difference('Watcher.count') do
                   xhr :post, :watch, :object_type => 'issue', :object_id => '1'
                   assert_response :success
-                  assert_select_rjs :replace_html, 'watcher'
+                  assert @response.body.include?('$$(".issue-1-watcher")')
                 end
                 assert Issue.find(1).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
                 assert_no_difference('Watcher.count') do
                   xhr :post, :watch, :object_type => 'issue', :object_id => '1'
                   assert_response 403
                 end
               end
-              def test_watch_with_multiple_replacements
-                @request.session[:user_id] = 3
-                assert_difference('Watcher.count') do
-                  xhr :post, :watch, :object_type => 'issue', :object_id => '1', :replace => ['watch_item_1','watch_item_2']
-                  assert_response :success
-                  assert_select_rjs :replace_html, 'watch_item_1'
-                  assert_select_rjs :replace_html, 'watch_item_2'
-                end
-              end
               def test_unwatch
                 @request.session[:user_id] = 3
                 assert_difference('Watcher.count', -1) do
                   xhr :post, :unwatch, :object_type => 'issue', :object_id => '2'
                   assert_response :success
-                  assert_select_rjs :replace_html, 'watcher'
+                  assert @response.body.include?('$$(".issue-2-watcher")')
-                end
-                assert !Issue.find(1).watched_by?(User.find(3))
-              end
-              def test_unwatch_with_multiple_replacements
-                @request.session[:user_id] = 3
-                assert_difference('Watcher.count', -1) do
-                  xhr :post, :unwatch, :object_type => 'issue', :object_id => '2', :replace => ['watch_item_1', 'watch_item_2']
-                  assert_response :success
-                  assert_select_rjs :replace_html, 'watch_item_1'
-                  assert_select_rjs :replace_html, 'watch_item_2'
                 end
                 assert !Issue.find(1).watched_by?(User.find(3))
               end
               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'}
                   assert_response :success
                   assert_select_rjs :replace_html, 'watchers'
                 end
                 assert Issue.find(2).watched_by?(User.find(4))
               end
               def test_remove_watcher
                 @request.session[:user_id] = 2
                 assert_difference('Watcher.count', -1) do
                   xhr :post, :destroy, :object_type => 'issue', :object_id => '2', :user_id => '3'
                   assert_response :success
                   assert_select_rjs :replace_html, 'watchers'
                 end
                 assert !Issue.find(2).watched_by?(User.find(3))
               end
             end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages