Moved new project member to its own action MemberController#new.

git-svn-id: http://svn.redmine.org/redmine/trunk@13600 e93f8b46-1217-0410-a6f0-8f06a7374b81

12 files changed, 79 insertions, 47 deletions

diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb
index ca5002af8..111cabc3d 100644
--- a/app/controllers/members_controller.rb
+++ b/app/controllers/members_controller.rb
@@ -17,9 +17,9 @@
 
 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 :find_model_object, :except => [:index, :new, :create, :autocomplete]
+  before_filter :find_project_from_association, :except => [:index, :new, :create, :autocomplete]
+  before_filter :find_project_by_project_id, :only => [:index, :new, :create, :autocomplete]
   before_filter :authorize
   accept_api_auth :index, :show, :create, :update, :destroy
 
@@ -46,6 +46,10 @@ class MembersController < ApplicationController
     end
   end
 
+  def new
+    @member = Member.new
+  end
+
   def create
     members = []
     if params[:membership]
diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
index 36f823569..9036c6db0 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -19,12 +19,15 @@
 
 module MembersHelper
   def render_principals_for_new_members(project)
-    scope = Principal.visible.sorted.not_member_of(project).like(params[:q])
+    scope = Principal.active.visible.sorted.not_member_of(project).like(params[:q])
     principal_count = scope.count
     principal_pages = Redmine::Pagination::Paginator.new principal_count, 100, params['page']
     principals = scope.offset(principal_pages.offset).limit(principal_pages.per_page).to_a
 
-    s = content_tag('div', principals_check_box_tags('membership[user_ids][]', principals), :id => 'principals')
+    s = content_tag('div',
+      content_tag('div', principals_check_box_tags('membership[user_ids][]', principals), :id => 'principals'),
+      :class => 'objects-selection'
+    )
 
     links = pagination_links_full(principal_pages, principal_count, :per_page_links => false) {|text, parameters, options|
       link_to text, autocomplete_project_memberships_path(project, parameters.merge(:q => params[:q], :format => 'js')), :remote => true
diff --git a/app/views/members/_new_form.html.erb b/app/views/members/_new_form.html.erb
new file mode 100644
index 000000000..9140da7c4
--- /dev/null
+++ b/app/views/members/_new_form.html.erb
@@ -0,0 +1,16 @@
+<fieldset class="box">
+  <legend><%= label_tag("principal_search", l(:label_principal_search)) %></legend>
+  <p><%= text_field_tag('principal_search', nil) %></p>
+  <%= javascript_tag "observeSearchfield('principal_search', null, '#{ escape_javascript autocomplete_project_memberships_path(@project, :format => 'js') }')" %>
+  <div id="principals_for_new_member">
+    <%= render_principals_for_new_members(@project) %>
+  </div>
+</fieldset>
+<fieldset class="box">
+  <legend><%= l(:label_role_plural) %> <%= toggle_checkboxes_link('.roles-selection input') %></legend>
+  <div class="roles-selection">
+    <% Role.givable.all.each do |role| %>
+      <label><%= check_box_tag 'membership[role_ids][]', role.id, false, :id => nil %> <%= role %></label>
+    <% end %>
+  </div>
+</fieldset>
diff --git a/app/views/members/_new_modal.html.erb b/app/views/members/_new_modal.html.erb
new file mode 100644
index 000000000..0338a953c
--- /dev/null
+++ b/app/views/members/_new_modal.html.erb
@@ -0,0 +1,9 @@
+<h3 class="title"><%= l(:label_member_new) %></h3>
+
+<%= form_for @member, :as => :membership, :url => project_memberships_path(@project), :remote => true, :method => :post do |f| %>
+  <%= render :partial => 'new_form' %>
+  <p class="buttons">
+    <%= submit_tag l(:button_add), :id => 'member-add-submit' %>
+    <%= submit_tag l(:button_cancel), :name => nil, :onclick => "hideModal(this);", :type => 'button' %>
+  </p>
+<% end %>
diff --git a/app/views/members/create.js.erb b/app/views/members/create.js.erb
index 11c468a76..d716743ef 100644
--- a/app/views/members/create.js.erb
+++ b/app/views/members/create.js.erb
@@ -2,10 +2,11 @@ $('#tab-content-members').html('<%= escape_javascript(render :partial => 'projec
 hideOnLoad();
 
 <% if @members.present? && @members.all? {|m| m.valid? } %>
+  hideModal();
   <% @members.each do |member| %>
     $("#member-<%= member.id %>").effect("highlight");
   <% end %>
-<% else %>
+<% elsif @members.present? %>
   <% errors = @members.collect {|m| m.errors.full_messages}.flatten.uniq.join(', ') %>
   alert('<%= raw(escape_javascript(l(:notice_failed_to_save_members, :errors => errors))) %>');
 <% end %>
diff --git a/app/views/members/new.html.erb b/app/views/members/new.html.erb
new file mode 100644
index 000000000..97dd380c7
--- /dev/null
+++ b/app/views/members/new.html.erb
@@ -0,0 +1,6 @@
+<h2><%= l(:label_member_new) %></h2>
+
+<%= form_for @member, :as => :membership, :url => project_memberships_path(@project), :method => :post do |f| %>
+  <%= render :partial => 'new_form' %>
+  <p><%= submit_tag l(:button_add), :name => nil %></p>
+<% end %>
diff --git a/app/views/members/new.js.erb b/app/views/members/new.js.erb
new file mode 100644
index 000000000..2d60e296a
--- /dev/null
+++ b/app/views/members/new.js.erb
@@ -0,0 +1,2 @@
+$('#ajax-modal').html('<%= escape_javascript(render :partial => 'members/new_modal') %>');
+showModal('ajax-modal', '700px');
diff --git a/app/views/principal_memberships/_new_form.html.erb b/app/views/principal_memberships/_new_form.html.erb
index 2a0d50dbc..c49d84993 100644
--- a/app/views/principal_memberships/_new_form.html.erb
+++ b/app/views/principal_memberships/_new_form.html.erb
@@ -1,6 +1,6 @@
 <fieldset class="box">
   <legend><%= l(:label_project_plural) %> <%= toggle_checkboxes_link('.projects-selection input:enabled') %></legend>
-  <div style="max-height:300px; overflow:auto;">
+  <div class="objects-selection">
     <div class="projects-selection">
     <%= render_project_nested_lists(@projects) do |p| %>
       <label>
diff --git a/app/views/projects/settings/_members.html.erb b/app/views/projects/settings/_members.html.erb
index 392d1d4b5..569684a58 100644
--- a/app/views/projects/settings/_members.html.erb
+++ b/app/views/projects/settings/_members.html.erb
@@ -1,8 +1,8 @@
-<%= error_messages_for 'member' %>
 <% roles = Role.find_all_givable
    members = @project.member_principals.includes(:member_roles, :roles, :principal).to_a.sort %>
 
-<div class="splitcontentleft">
+<p><%= link_to l(:label_member_new), new_project_membership_path(@project), :remote => true, :class => "icon icon-add" %></p>
+
 <% if members.any? %>
 <table class="list members">
   <thead>
@@ -62,31 +62,3 @@
 <% else %>
 <p class="nodata"><%= l(:label_no_data) %></p>
 <% end %>
-</div>
-
-<div class="splitcontentright">
-<% if roles.any? %>
-  <%= form_for(@member,
-               {:as => :membership, :url => project_memberships_path(@project),
-                :remote => true, :method => :post}) 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>
-      <%= javascript_tag "observeSearchfield('principal_search', null, '#{ escape_javascript autocomplete_project_memberships_path(@project, :format => 'js') }')" %>
-      <div id="principals_for_new_member">
-        <%= render_principals_for_new_members(@project) %>
-      </div>
-      <p>
-        <%= l(:label_role_plural) %>:
-        <% roles.each do |role| %>
-          <label><%= check_box_tag 'membership[role_ids][]', role.id, false, :id => nil %> <%= role %></label>
-        <% end %>
-      </p>
-      <p><%= submit_tag l(:button_add), :id => 'member-add-submit' %></p>
-    </fieldset>
-  <% end %>
-<% end %>
-</div>
diff --git a/lib/redmine.rb b/lib/redmine.rb
index 9fad74f42..bf7732352 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -82,7 +82,7 @@ Redmine::AccessControl.map do |map|
   map.permission :close_project, {:projects => [:close, :reopen]}, :require => :member, :read => true
   map.permission :select_project_modules, {:projects => :modules}, :require => :member
   map.permission :view_members, {:members => [:index, :show]}, :public => true, :read => true
-  map.permission :manage_members, {:projects => :settings, :members => [:index, :show, :create, :update, :destroy, :autocomplete]}, :require => :member
+  map.permission :manage_members, {:projects => :settings, :members => [:index, :show, :new, :create, :update, :destroy, :autocomplete]}, :require => :member
   map.permission :manage_versions, {:projects => :settings, :versions => [:new, :create, :edit, :update, :close_completed, :destroy]}, :require => :member
   map.permission :add_subprojects, {:projects => [:new, :create]}, :require => :member
 
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
index e60cf8d6d..ebe588bc3 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -97,6 +97,7 @@ html>body #content { min-height: 600px; }
 div.modal { border-radius:5px; background:#fff; z-index:50; padding:4px;}
 div.modal h3.title {display:none;}
 div.modal p.buttons {text-align:right; margin-bottom:0;}
+div.modal .box p {margin: 0.3em 0;}
 
 input#openid_url { background: url(../images/openid-bg.gif) no-repeat; background-color: #fff; background-position: 0 50%; padding-left: 18px; }
 
@@ -599,14 +600,12 @@ select.bool_cf {width:auto !important;}
 
 #tab-content-modules fieldset p {margin:3px 0 4px 0;}
 
-#tab-content-members .splitcontentleft, #tab-content-memberships .splitcontentleft, #tab-content-users .splitcontentleft {width: 64%;}
-#tab-content-members .splitcontentright, #tab-content-memberships .splitcontentright, #tab-content-users .splitcontentright {width: 34%;}
-#tab-content-members fieldset, #tab-content-memberships fieldset, #tab-content-users fieldset {padding:1em; margin-bottom: 1em;}
-#tab-content-members fieldset legend, #tab-content-memberships fieldset legend, #tab-content-users fieldset legend {font-weight: bold;}
-#tab-content-members fieldset label, #tab-content-memberships fieldset label, #tab-content-users fieldset label {display: block;}
-#tab-content-members #principals, #tab-content-users #principals {max-height: 400px; overflow: auto;}
-
-#tab-content-memberships .splitcontentright select {width:90%}
+#tab-content-users .splitcontentleft {width: 64%;}
+#tab-content-users .splitcontentright {width: 34%;}
+#tab-content-users fieldset {padding:1em; margin-bottom: 1em;}
+#tab-content-users fieldset legend {font-weight: bold;}
+#tab-content-users fieldset label {display: block;}
+#tab-content-users #principals {max-height: 400px; overflow: auto;}
 
 #users_for_watcher {height: 200px; overflow:auto;}
 #users_for_watcher label {display: block;}
@@ -626,7 +625,16 @@ input.autocomplete.ajax-loading {
 
 .role-visibility {padding-left:2em;}
 
-.projects-selection {
+.objects-selection {
+  height: 300px;
+  overflow: auto;
+}
+
+.objects-selection label {
+  display: block;
+}
+
+.objects-selection>div {
   column-count: auto;
   column-width: 200px;
   -webkit-column-count: auto;
diff --git a/test/functional/members_controller_test.rb b/test/functional/members_controller_test.rb
index a1ab6f93a..be8b7a343 100644
--- a/test/functional/members_controller_test.rb
+++ b/test/functional/members_controller_test.rb
@@ -25,6 +25,17 @@ class MembersControllerTest < ActionController::TestCase
     @request.session[:user_id] = 2
   end
 
+  def test_new
+    get :new, :project_id => 1
+    assert_response :success
+  end
+
+  def test_xhr_new
+    xhr :get, :new, :project_id => 1
+    assert_response :success
+    assert_equal 'text/javascript', response.content_type
+  end
+
   def test_create
     assert_difference 'Member.count' do
       post :create, :project_id => 1, :membership => {:role_ids => [1], :user_id => 7}