Allow non admin users to add subprojects (#2963).

Subprojects can be added to the projects for which the user has add_project permission.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3059 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index c981244e107eed0a686dca7c10409f2bead768f1..8a6d16f20bf77ee417dc476d01eeff73fa1f637f 100644 (file)
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -75,7 +75,7 @@ class ProjectsController < ApplicationController
     else
       @project.enabled_module_names = params[:enabled_modules]
       if @project.save
-        @project.set_parent!(params[:project]['parent_id']) if User.current.admin? && params[:project].has_key?('parent_id')
+        @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
         # Add current user as a project member if he is not admin
         unless User.current.admin?
           r = Role.givable.find_by_id(Setting.new_project_user_role_id.to_i) || Role.givable.first
@@ -106,7 +106,7 @@ class ProjectsController < ApplicationController
       @project = Project.new(params[:project])
       @project.enabled_module_names = params[:enabled_modules]
       if @project.copy(@source_project, :only => params[:only])
-        @project.set_parent!(params[:project]['parent_id']) if User.current.admin? && params[:project].has_key?('parent_id')
+        @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
         flash[:notice] = l(:notice_successful_create)
         redirect_to :controller => 'admin', :action => 'projects'
       end              
@@ -158,7 +158,7 @@ class ProjectsController < ApplicationController
     if request.post?
       @project.attributes = params[:project]
       if @project.save
-        @project.set_parent!(params[:project]['parent_id']) if User.current.admin? && params[:project].has_key?('parent_id')
+        @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id')
         flash[:notice] = l(:notice_successful_update)
         redirect_to :action => 'settings', :id => @project
       else

diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index d68cb9bbb2624be67c6596b334535c4bb327f51f..07ba23d0a8552b14a54b586d3bf81ece1e8491d8 100644 (file)
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -36,7 +36,7 @@ module ProjectsHelper
   end
   
   def parent_project_select_tag(project)
-    options = '<option></option>' + project_tree_options_for_select(project.possible_parents, :selected => project.parent)
+    options = '<option></option>' + project_tree_options_for_select(project.allowed_parents, :selected => project.parent)
     content_tag('select', options, :name => 'project[parent_id]')
   end
   

diff --git a/app/models/project.rb b/app/models/project.rb
index 42387c2d2e3f2b75d5d2609e14b4be3e74bb5722..62c761d1aa04bab55b409d385313af84f21210cc 100644 (file)
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -148,14 +148,16 @@ class Project < ActiveRecord::Base
     else
       statements << "1=0"
       if user.logged?
-        statements << "#{Project.table_name}.is_public = #{connection.quoted_true}" if Role.non_member.allowed_to?(permission)
+        if Role.non_member.allowed_to?(permission) && !options[:member]
+          statements << "#{Project.table_name}.is_public = #{connection.quoted_true}"
+        end
         allowed_project_ids = user.memberships.select {|m| m.roles.detect {|role| role.allowed_to?(permission)}}.collect {|m| m.project_id}
         statements << "#{Project.table_name}.id IN (#{allowed_project_ids.join(',')})" if allowed_project_ids.any?
-      elsif Role.anonymous.allowed_to?(permission)
-        # anonymous user allowed on public project
-        statements << "#{Project.table_name}.is_public = #{connection.quoted_true}" 
       else
-        # anonymous user is not authorized
+        if Role.anonymous.allowed_to?(permission) && !options[:member]
+          # anonymous user allowed on public project
+          statements << "#{Project.table_name}.is_public = #{connection.quoted_true}"
+        end 
       end
     end
     statements.empty? ? base_statement : "((#{base_statement}) AND (#{statements.join(' OR ')}))"
@@ -253,8 +255,34 @@ class Project < ActiveRecord::Base
   end
   
   # Returns an array of projects the project can be moved to
-  def possible_parents
-    @possible_parents ||= (Project.active.find(:all) - self_and_descendants)
+  # by the current user
+  def allowed_parents
+    return @allowed_parents if @allowed_parents
+    @allowed_parents = (Project.find(:all, :conditions => Project.allowed_to_condition(User.current, :add_project, :member => true)) - self_and_descendants)
+    unless parent.nil? || @allowed_parents.empty? || @allowed_parents.include?(parent)
+      @allowed_parents << parent
+    end
+    @allowed_parents
+  end
+  
+  # Sets the parent of the project with authorization check
+  def set_allowed_parent!(p)
+    unless p.nil? || p.is_a?(Project)
+      if p.to_s.blank?
+        p = nil
+      else
+        p = Project.find_by_id(p)
+        return false unless p
+      end
+    end
+    if p.nil?
+      if !new_record? && allowed_parents.empty?
+        return false
+      end
+    elsif !allowed_parents.include?(p)
+      return false
+    end
+    set_parent!(p)
   end
   
   # Sets the parent of the project

diff --git a/app/views/projects/_form.rhtml b/app/views/projects/_form.rhtml
index e69dfedb00abd93e93e61b69f3b627169d662689..0e286fcaed7da7dc701003ef4ca2894a93627472 100644 (file)
--- a/app/views/projects/_form.rhtml
+++ b/app/views/projects/_form.rhtml
@@ -4,7 +4,7 @@
 <!--[form:project]-->
 <p><%= f.text_field :name, :required => true %><br /><em><%= l(:text_caracters_maximum, 30) %></em></p>
 
-<% if User.current.admin? && !@project.possible_parents.empty? %>
+<% unless @project.allowed_parents.empty? %>
     <p><label><%= l(:field_parent) %></label><%= parent_project_select_tag(@project) %></p>
 <% end %>
 

diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb
index dcbcb77eb39c50475d0ca69e560328d1003871f3..6704d4fa80f8cc71b9efcf3a064123e97ad6167c 100644 (file)
--- a/test/functional/projects_controller_test.rb
+++ b/test/functional/projects_controller_test.rb
@@ -117,6 +117,23 @@ class ProjectsControllerTest < ActionController::TestCase
     assert_kind_of Project, project
     assert_equal 'weblog', project.description 
     assert_equal true, project.is_public?
+    assert_nil project.parent
+  end
+  
+  def test_post_add_subproject
+    @request.session[:user_id] = 1
+    post :add, :project => { :name => "blog", 
+                             :description => "weblog",
+                             :identifier => "blog",
+                             :is_public => 1,
+                             :custom_field_values => { '3' => 'Beta' },
+                             :parent_id => 1
+                            }
+    assert_redirected_to '/projects/blog/settings'
+    
+    project = Project.find_by_name('blog')
+    assert_kind_of Project, project
+    assert_equal Project.find(1), project.parent
   end
   
   def test_post_add_by_non_admin

diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb
index 2c1d2601a5d5ca2714f0a914b0f3031776a1e4bb..b4bb4cc11bb94e87cfaf16494c2cfc84eb209eff 100644 (file)
--- a/test/unit/project_test.rb
+++ b/test/unit/project_test.rb
@@ -26,6 +26,7 @@ class ProjectTest < ActiveSupport::TestCase
   def setup
     @ecookbook = Project.find(1)
     @ecookbook_sub1 = Project.find(3)
+    User.current = nil
   end
   
   should_validate_presence_of :name
@@ -236,6 +237,14 @@ class ProjectTest < ActiveSupport::TestCase
     assert_equal [5, 6, 3, 4], d.collect(&:id)
   end
   
+  def test_allowed_parents_should_be_empty_for_non_member_user
+    Role.non_member.add_permission!(:add_project)
+    user = User.find(9)
+    assert user.memberships.empty?
+    User.current = user
+    assert Project.new.allowed_parents.empty?
+  end
+  
   def test_users_by_role
     users_by_role = Project.find(1).users_by_role
     assert_kind_of Hash, users_by_role