jro_apps/redmine Commit - r9008:6aad82e524c1

Fixed that the proposed users list may be empty when adding a project member (#10374)....

Jean-Philippe Lang -

r9008:6aad82e524c1

parent child

Context file:

r9008:6aad82e524c1

Collapse all files

app/controllers/members_controller.rb +1 -1

             # Redmine - project management software
             # 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 MembersController < ApplicationController
               model_object Member
               before_filter :find_model_object, :except => [:index, :create, :autocomplete]
               before_filter :find_project_from_association, :except => [:index, :create, :autocomplete]
               before_filter :find_project_by_project_id, :only => [:index, :create, :autocomplete]
               before_filter :authorize
               accept_api_auth :index, :show, :create, :update, :destroy
               def index
                 @offset, @limit = api_offset_and_limit
                 @member_count = @project.member_principals.count
                 @member_pages = Paginator.new self, @member_count, @limit, params['page']
                 @offset ||= @member_pages.current.offset
                 @members =  @project.member_principals.all(
                   :order => "#{Member.table_name}.id",
                   :limit  =>  @limit,
                   :offset =>  @offset
                 )
                 respond_to do |format|
                   format.html { head 406 }
                   format.api
                 end
               end
               def show
                 respond_to do |format|
                   format.html { head 406 }
                   format.api
                 end
               end
               def create
                 members = []
                 if params[:membership] && params[:membership][:user_ids]
                   attrs = params[:membership].dup
                   user_ids = attrs.delete(:user_ids)
                   user_ids.each do |user_id|
                     members << Member.new(attrs.merge(:user_id => user_id))
                   end
                 else
                   members << Member.new(params[:membership])
                 end
                 @project.members << members
                 respond_to do |format|
                   if members.present? && members.all? {|m| m.valid? }
                     format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project }
                     format.js {
                       render(:update) {|page|
                         page.replace_html "tab-content-members", :partial => 'projects/settings/members'
                         page << 'hideOnLoad()'
                         members.each {|member| page.visual_effect(:highlight, "member-#{member.id}") }
                       }
                     }
                     format.api {
                       @member = members.first
                       render :action => 'show', :status => :created, :location => membership_url(@member)
                     }
                   else
                     format.js {
                       render(:update) {|page|
                         errors = members.collect {|m|
                           m.errors.full_messages
                         }.flatten.uniq
                         page.alert(l(:notice_failed_to_save_members, :errors => errors.join(', ')))
                       }
                     }
                     format.api { render_validation_errors(members.first) }
                   end
                 end
               end
               def update
                 if params[:membership]
                   @member.role_ids = params[:membership][:role_ids]
                 end
                 saved = @member.save
                 respond_to do |format|
                   format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project }
                   format.js {
                     render(:update) {|page|
                       page.replace_html "tab-content-members", :partial => 'projects/settings/members'
                       page << 'hideOnLoad()'
                       page.visual_effect(:highlight, "member-#{@member.id}")
                     }
                   }
                   format.api {
                     if saved
                       head :ok
                     else
                       render_validation_errors(@member)
                     end
                   }
                 end
               end
               def destroy
                 if request.delete? && @member.deletable?
                   @member.destroy
                 end
                 respond_to do |format|
                   format.html { redirect_to :controller => 'projects', :action => 'settings', :tab => 'members', :id => @project }
                   format.js { render(:update) {|page|
                       page.replace_html "tab-content-members", :partial => 'projects/settings/members'
                       page << 'hideOnLoad()'
                     }
                   }
                   format.api {
                     if @member.destroyed?
                       head :ok
                     else
                       head :unprocessable_entity
                     end
                   }
                 end
               end
               def autocomplete
-                @principals = Principal.active.like(params[:q]).find(:all, :limit => 100) - @project.principals
+                @principals = Principal.active.not_member_of(@project).like(params[:q]).all(:limit => 100)
                 render :layout => false
               end
             end

app/models/principal.rb +11 0

             # Redmine - project management software
             # 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 Principal < ActiveRecord::Base
               set_table_name "#{table_name_prefix}users#{table_name_suffix}"
               has_many :members, :foreign_key => 'user_id', :dependent => :destroy
               has_many :memberships, :class_name => 'Member', :foreign_key => 'user_id', :include => [ :project, :roles ], :conditions => "#{Project.table_name}.status=#{Project::STATUS_ACTIVE}", :order => "#{Project.table_name}.name"
               has_many :projects, :through => :memberships
               has_many :issue_categories, :foreign_key => 'assigned_to_id', :dependent => :nullify
               # Groups and active users
               named_scope :active, :conditions => "#{Principal.table_name}.status = 1"
               named_scope :like, lambda {|q|
                 if q.blank?
                   {}
                 else
                   q = q.to_s.downcase
                   pattern = "%#{q}%"
                   sql = "LOWER(login) LIKE :p OR LOWER(firstname) LIKE :p OR LOWER(lastname) LIKE :p OR LOWER(mail) LIKE :p"
                   params = {:p => pattern}
                   if q =~ /^(.+)\s+(.+)$/
                     a, b = "#{$1}%", "#{$2}%"
                     sql << " OR (LOWER(firstname) LIKE :a AND LOWER(lastname) LIKE :b) OR (LOWER(firstname) LIKE :b AND LOWER(lastname) LIKE :a)"
                     params.merge!(:a => a, :b => b)
                   end
                   {:conditions => [sql, params]}
                 end
               }
               # Principals that are members of a collection of projects
               named_scope :member_of, lambda {|projects|
+                projects = [projects] unless projects.is_a?(Array)
                 if projects.empty?
                   {:conditions => "1=0"}
                 else
                   ids = projects.map(&:id)
                   {:conditions => ["#{Principal.table_name}.status = 1 AND #{Principal.table_name}.id IN (SELECT DISTINCT user_id FROM #{Member.table_name} WHERE project_id IN (?))", ids]}
                 end
               }
+              # Principals that are not members of projects
+              named_scope :not_member_of, lambda {|projects|
+                projects = [projects] unless projects.is_a?(Array)
+                if projects.empty?
+                  {:conditions => "1=0"}
+                else
+                  ids = projects.map(&:id)
+                  {:conditions => ["#{Principal.table_name}.id NOT IN (SELECT DISTINCT user_id FROM #{Member.table_name} WHERE project_id IN (?))", ids]}
+                end
+              }
               before_create :set_default_empty_values
               def name(formatter = nil)
                 to_s
               end
               def <=>(principal)
                 if principal.nil?
                   -1
                 elsif self.class.name == principal.class.name
                   self.to_s.downcase <=> principal.to_s.downcase
                 else
                   # groups after users
                   principal.class.name <=> self.class.name
                 end
               end
               protected
               # Make sure we don't try to insert NULL values (see #4632)
               def set_default_empty_values
                 self.login ||= ''
                 self.hashed_password ||= ''
                 self.firstname ||= ''
                 self.lastname ||= ''
                 self.mail ||= ''
                 true
               end
             end

app/views/projects/settings/_members.html.erb +1 -1

             <%= error_messages_for 'member' %>
             <% roles = Role.find_all_givable
                members = @project.member_principals.find(:all, :include => [:roles, :principal]).sort %>
             <div class="splitcontentleft">
             <% if members.any? %>
             <table class="list members">
               <thead><tr>
                 <th><%= l(:label_user) %> / <%= l(:label_group) %></th>
                 <th><%= l(:label_role_plural) %></th>
                 <th style="width:15%"></th>
                       <%= call_hook(:view_projects_settings_members_table_header, :project => @project) %>
               </tr></thead>
               <tbody>
               <% members.each do |member| %>
               <% next if member.new_record? %>
               <tr id="member-<%= member.id %>" class="<%= cycle 'odd', 'even' %> member">
               <td class="<%= member.principal.class.name.downcase %>"><%= link_to_user member.principal %></td>
               <td class="roles">
                 <span id="member-<%= member.id %>-roles"><%=h member.roles.sort.collect(&:to_s).join(', ') %></span>
                   <% remote_form_for(:membership, member, :url => membership_path(member),
                                                       :method => :put,
                                                       :html => { :id => "member-#{member.id}-roles-form", :class => 'hol' }) do |f| %>
                     <p><% roles.each do |role| %>
                     <label><%= check_box_tag 'membership[role_ids][]', role.id, member.roles.include?(role),
                                                                    :disabled => member.member_roles.detect {|mr| mr.role_id == role.id && !mr.inherited_from.nil?} %> <%=h role %></label><br />
                     <% end %></p>
                     <%= hidden_field_tag 'membership[role_ids][]', '' %>
                     <p><%= submit_tag l(:button_change), :class => "small" %>
                     <%= link_to_function l(:button_cancel),
                                          "$('member-#{member.id}-roles').show(); $('member-#{member.id}-roles-form').hide(); return false;"
                          %></p>
                   <% end %>
               </td>
               <td class="buttons">
                   <%= link_to_function l(:button_edit),
                                        "$('member-#{member.id}-roles').hide(); $('member-#{member.id}-roles-form').show(); return false;",
                                        :class => 'icon icon-edit' %>
                   <%= link_to_remote(
                         l(:button_delete),
                         { :url => membership_path(member),
                           :method => :delete,
                           :confirm => (!User.current.admin? && member.include?(User.current) ? l(:text_own_membership_delete_confirmation) : nil) },
                         :title => l(:button_delete),
                         :class => 'icon icon-del'
                       ) if member.deletable? %>
               </td>
               <%= call_hook(:view_projects_settings_members_table_row, { :project => @project, :member => member}) %>
               </tr>
             <% end; reset_cycle %>
               </tbody>
             </table>
             <% else %>
             <p class="nodata"><%= l(:label_no_data) %></p>
             <% end %>
             </div>
-            <% principals = Principal.active.find(:all, :limit => 100, :order => 'type, login, lastname ASC') - @project.principals %>
+            <% principals = Principal.active.not_member_of(@project).all(:limit => 100, :order => 'type, login, lastname ASC') %>
             <div class="splitcontentright">
             <% if roles.any? && principals.any? %>
               <% remote_form_for(:membership, @member, :url => project_memberships_path(@project), :method => :post,
                                  :loading => '$(\'member-add-submit\').disable();',
                                  :complete => 'if($(\'member-add-submit\')) $(\'member-add-submit\').enable();') do |f| %>
                 <fieldset><legend><%=l(:label_member_new)%></legend>
                 <p><%= label_tag "principal_search", l(:label_principal_search) %><%= text_field_tag 'principal_search', nil %></p>
                 <%= observe_field(:principal_search,
                              :frequency => 0.5,
                              :update => :principals,
                              :url => autocomplete_project_memberships_path(@project),
                              :method => :get,
                              :before => '$("principal_search").addClassName("ajax-loading")',
                              :complete => '$("principal_search").removeClassName("ajax-loading")',
                              :with => 'q')
                               %>
                 <div id="principals">
                   <%= principals_check_box_tags 'membership[user_ids][]', principals %>
                 </div>
                 <p><%= l(:label_role_plural) %>:
                 <% roles.each do |role| %>
                   <label><%= check_box_tag 'membership[role_ids][]', role.id %> <%=h role %></label>
                  <% end %></p>
                 <p><%= submit_tag l(:button_add), :id => 'member-add-submit' %></p>
                 </fieldset>
               <% end %>
             <% end %>
             </div>

test/unit/principal_test.rb +7 0

             # Redmine - project management software
             # 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__)
             class PrincipalTest < ActiveSupport::TestCase
+              fixtures :users, :projects, :members, :member_roles
               def test_active_scope_should_return_groups_and_active_users
                 result = Principal.active.all
                 assert_include Group.first, result
                 assert_not_nil result.detect {|p| p.is_a?(User)}
                 assert_nil result.detect {|p| p.is_a?(User) && !p.active?}
                 assert_nil result.detect {|p| p.is_a?(AnonymousUser)}
               end
               def test_member_of_scope_should_return_the_union_of_all_members
                 projects = Project.find_all_by_id(1, 2)
                 assert_equal projects.map(&:principals).flatten.sort, Principal.member_of(projects).sort
               end
+              def test_not_member_of_scope_should_return_users_that_have_no_memberships
+                projects = Project.find_all_by_id(1, 2)
+                expected = (Principal.all - projects.map(&:memberships).flatten.map(&:principal)).sort
+                assert_equal expected, Principal.not_member_of(projects).sort
+              end
               context "#like" do
                 setup do
                   Principal.generate!(:login => 'login')
                   Principal.generate!(:login => 'login2')
                   Principal.generate!(:firstname => 'firstname')
                   Principal.generate!(:firstname => 'firstname2')
                   Principal.generate!(:lastname => 'lastname')
                   Principal.generate!(:lastname => 'lastname2')
                   Principal.generate!(:mail => 'mail@example.com')
                   Principal.generate!(:mail => 'mail2@example.com')
                   @palmer = Principal.generate!(:firstname => 'David', :lastname => 'Palmer')
                 end
                 should "search login" do
                   results = Principal.like('login')
                   assert_equal 2, results.count
                   assert results.all? {|u| u.login.match(/login/) }
                 end
                 should "search firstname" do
                   results = Principal.like('firstname')
                   assert_equal 2, results.count
                   assert results.all? {|u| u.firstname.match(/firstname/) }
                 end
                 should "search lastname" do
                   results = Principal.like('lastname')
                   assert_equal 2, results.count
                   assert results.all? {|u| u.lastname.match(/lastname/) }
                 end
                 should "search mail" do
                   results = Principal.like('mail')
                   assert_equal 2, results.count
                   assert results.all? {|u| u.mail.match(/mail/) }
                 end
                 should "search firstname and lastname" do
                   results = Principal.like('david palm')
                   assert_equal 1, results.count
                   assert_equal @palmer, results.first
                 end
                 should "search lastname and firstname" do
                   results = Principal.like('palmer davi')
                   assert_equal 1, results.count
                   assert_equal @palmer, results.first
                 end
               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