Merge branch 'ticket/unstable/802-group-watchers' into unstable

This commit is contained in:
Eric Davis 2011-12-27 18:09:25 -08:00
commit 7bce7f7b07
12 changed files with 187 additions and 81 deletions

View File

@ -48,8 +48,14 @@ class AutoCompletesController < ApplicationController
end
@removed_users ||= []
if params[:include_groups]
user_finder = Principal
else
user_finder = User
end
@users = User.active.like(params[:q]).find(:all, :limit => 100) - @removed_users
@users = user_finder.active.like(params[:q]).find(:all, :limit => 100) - @removed_users
render :layout => false
end

View File

@ -53,7 +53,7 @@ class WatchersController < ApplicationController
end
def destroy
@watched.set_watcher(User.find(params[:user_id]), false) if request.post?
@watched.set_watcher(Principal.find(params[:user_id]), false) if request.post?
respond_to do |format|
format.html { redirect_to :back }
format.js do

View File

@ -912,6 +912,12 @@ module ApplicationHelper
# +user+ can be a User or a string that will be scanned for an email address (eg. 'joe <joe@foo.bar>')
def avatar(user, options = { })
if Setting.gravatar_enabled?
if user.is_a?(Group)
size = options[:size] || 50
size = "#{size}x#{size}" # image_tag uses WxH
options[:class] ||= 'gravatar'
return image_tag("group.png", options.merge(:size => size))
end
options.merge!({:ssl => (defined?(request) && request.ssl?), :default => Setting.gravatar_default})
email = nil
if user.respond_to?(:mail)

View File

@ -22,6 +22,11 @@ class Group < Principal
validates_uniqueness_of :lastname, :case_sensitive => false
validates_length_of :lastname, :maximum => 30
# Returns an array of all of the email addresses of the group's users
def mails
users.collect(&:mail)
end
def to_s
lastname.to_s
end

View File

@ -48,6 +48,82 @@ class Principal < ActiveRecord::Base
end
end
def active?
true
end
def logged?
true # TODO: should all principals default to logged or not?
end
# Return true if the user is allowed to do the specified action on a specific context
# Action can be:
# * a parameter-like Hash (eg. :controller => 'projects', :action => 'edit')
# * a permission Symbol (eg. :edit_project)
# Context can be:
# * a project : returns true if user is allowed to do the specified action on this project
# * a group of projects : returns true if user is allowed on every project
# * nil with options[:global] set : check if user has at least one role allowed for this action,
# or falls back to Non Member / Anonymous permissions depending if the user is logged
def allowed_to?(action, context, options={})
if context && context.is_a?(Project)
# No action allowed on archived projects
return false unless context.active?
# No action allowed on disabled modules
return false unless context.allows_to?(action)
# Admin users are authorized for anything else
return true if admin?
roles = roles_for_project(context)
return false unless roles
roles.detect {|role| (context.is_public? || role.member?) && role.allowed_to?(action)}
elsif context && context.is_a?(Array)
# Authorize if user is authorized on every element of the array
context.map do |project|
allowed_to?(action,project,options)
end.inject do |memo,allowed|
memo && allowed
end
elsif options[:global]
# Admin users are always authorized
return true if admin?
# authorize if user has at least one role that has this permission
roles = memberships.collect {|m| m.roles}.flatten.uniq
roles.detect {|r| r.allowed_to?(action)} || (self.logged? ? Role.non_member.allowed_to?(action) : Role.anonymous.allowed_to?(action))
else
false
end
end
# Is the user allowed to do the specified action on any project?
# See allowed_to? for the actions and valid options.
def allowed_to_globally?(action, options)
allowed_to?(action, nil, options.reverse_merge(:global => true))
end
# Return user's roles for project
def roles_for_project(project)
roles = []
# No role on archived projects
return roles unless project && project.active?
if logged?
# Find project membership
membership = memberships.detect {|m| m.project_id == project.id}
if membership
roles = membership.roles
else
@role_non_member ||= Role.non_member
roles << @role_non_member
end
else
@role_anonymous ||= Role.anonymous
roles << @role_anonymous
end
roles
end
protected
# Make sure we don't try to insert NULL values (see #4632)

View File

@ -345,27 +345,6 @@ class User < Principal
!logged?
end
# Return user's roles for project
def roles_for_project(project)
roles = []
# No role on archived projects
return roles unless project && project.active?
if logged?
# Find project membership
membership = memberships.detect {|m| m.project_id == project.id}
if membership
roles = membership.roles
else
@role_non_member ||= Role.non_member
roles << @role_non_member
end
else
@role_anonymous ||= Role.anonymous
roles << @role_anonymous
end
roles
end
# Return true if the user is a member of project
def member_of?(project)
!roles_for_project(project).detect {|role| role.member?}.nil?
@ -388,53 +367,6 @@ class User < Principal
@projects_by_role
end
# Return true if the user is allowed to do the specified action on a specific context
# Action can be:
# * a parameter-like Hash (eg. :controller => 'projects', :action => 'edit')
# * a permission Symbol (eg. :edit_project)
# Context can be:
# * a project : returns true if user is allowed to do the specified action on this project
# * a group of projects : returns true if user is allowed on every project
# * nil with options[:global] set : check if user has at least one role allowed for this action,
# or falls back to Non Member / Anonymous permissions depending if the user is logged
def allowed_to?(action, context, options={})
if context && context.is_a?(Project)
# No action allowed on archived projects
return false unless context.active?
# No action allowed on disabled modules
return false unless context.allows_to?(action)
# Admin users are authorized for anything else
return true if admin?
roles = roles_for_project(context)
return false unless roles
roles.detect {|role| (context.is_public? || role.member?) && role.allowed_to?(action)}
elsif context && context.is_a?(Array)
# Authorize if user is authorized on every element of the array
context.map do |project|
allowed_to?(action,project,options)
end.inject do |memo,allowed|
memo && allowed
end
elsif options[:global]
# Admin users are always authorized
return true if admin?
# authorize if user has at least one role that has this permission
roles = memberships.collect {|m| m.roles}.flatten.uniq
roles.detect {|r| r.allowed_to?(action)} || (self.logged? ? Role.non_member.allowed_to?(action) : Role.anonymous.allowed_to?(action))
else
false
end
end
# Is the user allowed to do the specified action on any project?
# See allowed_to? for the actions and valid options.
def allowed_to_globally?(action, options)
allowed_to?(action, nil, options.reverse_merge(:global => true))
end
safe_attributes 'login',
'firstname',
'lastname',

View File

@ -14,8 +14,8 @@
class Watcher < ActiveRecord::Base
belongs_to :watchable, :polymorphic => true
belongs_to :user
belongs_to :user, :class_name => 'Principal', :foreign_key => 'user_id'
validates_presence_of :user
validates_uniqueness_of :user_id, :scope => [:watchable_type, :watchable_id]

View File

@ -12,12 +12,12 @@
:object_id => watched},
:method => :post,
:html => {:id => 'new-watcher-form', :style => 'display:none;'}) do |f| %>
<% users = User.active.find(:all, :limit => 25) - watched.watcher_users %>
<% users = Principal.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),
:url => auto_complete_users_path(:remove_watchers => watched.id, :klass => watched.class, :include_groups => true),
:with => 'q')
%>

View File

@ -92,6 +92,20 @@ class AutoCompletesControllerTest < ActionController::TestCase
end
end
context "including groups" do
setup do
@group = Group.generate(:lastname => 'Complete Group').reload
get :users, :q => 'complete', :include_groups => true
end
should_respond_with :success
should "include matching groups" do
assert_select "input[type=checkbox][value=?]", @group.id
end
end
context "restrict by removing group members" do
setup do
@group = Group.first

View File

@ -132,6 +132,21 @@ class WatchersControllerTest < ActionController::TestCase
assert Issue.find(2).watched_by?(User.find(7))
end
context "POST :new" do
should "add groups" do
@group = Group.generate!.reload
Member.generate!(:project => Project.find(1), :roles => [Role.find(1)], :principal => @group)
@request.session[:user_id] = 2
assert_difference('Watcher.count') do
xhr :post, :new, :object_type => 'issue', :object_id => '2', :user_ids => [@group.id.to_s]
assert_response :success
assert_select_rjs :replace_html, 'watchers'
end
assert Issue.find(2).watched_by?(@group)
end
end
def test_remove_watcher
@request.session[:user_id] = 2
assert_difference('Watcher.count', -1) do
@ -141,4 +156,23 @@ class WatchersControllerTest < ActionController::TestCase
end
assert !Issue.find(2).watched_by?(User.find(3))
end
context "POST :destroy" do
should "remove a group" do
@group = Group.generate!.reload
Member.generate!(:project => Project.find(1), :roles => [Role.find(1)], :principal => @group)
assert Issue.find(2).add_watcher(@group)
assert Issue.find(2).watched_by?(@group)
@request.session[:user_id] = 2
assert_difference('Watcher.count', -1) do
xhr :post, :destroy, :object_type => 'issue', :object_id => '2', :user_id => @group.id.to_s
assert_response :success
assert_select_rjs :replace_html, 'watchers'
end
assert !Issue.find(2).watched_by?(@group)
end
end
end

View File

@ -14,11 +14,7 @@
require File.expand_path('../../test_helper', __FILE__)
class WatcherTest < ActiveSupport::TestCase
fixtures :projects, :users, :members, :member_roles, :roles, :enabled_modules,
:issues,
:boards, :messages,
:wikis, :wiki_pages,
:watchers
fixtures :all
def setup
@user = User.find(1)
@ -125,4 +121,34 @@ class WatcherTest < ActiveSupport::TestCase
assert Issue.find(1).watched_by?(user)
assert !Issue.find(4).watched_by?(user)
end
context "group watch" do
setup do
@group = Group.generate!
Member.generate!(:project => Project.find(1), :roles => [Role.find(1)], :principal => @group)
@group.users << @user = User.find(1)
@group.users << @user2 = User.find(2)
end
should "be valid" do
assert @issue.add_watcher(@group)
@issue.reload
assert @issue.watchers.detect { |w| w.user == @group }
end
should "add all group members to recipients" do
@issue.watchers.delete_all
@issue.reload
assert @issue.watcher_recipients.empty?
assert @issue.add_watcher(@group)
@user.save
@issue.reload
assert @issue.watcher_recipients.include?(@user.mail)
assert @issue.watcher_recipients.include?(@user2.mail)
end
end
end

View File

@ -42,7 +42,7 @@ module Redmine
# Removes user from the watchers list
def remove_watcher(user)
return nil unless user && user.is_a?(User)
return nil unless user && user.is_a?(Principal)
Watcher.delete_all "watchable_type = '#{self.class}' AND watchable_id = #{self.id} AND user_id = #{user.id}"
end
@ -64,7 +64,14 @@ module Redmine
if respond_to?(:visible?)
notified.reject! {|user| !visible?(user)}
end
notified.collect(&:mail).compact
notified.collect {|w|
if w.respond_to?(:mail) && w.mail.present? # Single mail
w.mail
elsif w.respond_to?(:mails) && w.mails.present? # Multiple mail
w.mails
end
}.flatten.compact
end
module ClassMethods; end