diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c981244e1..8a6d16f20 100644 --- 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 d68cb9bbb..07ba23d0a 100644 --- 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 = '' + project_tree_options_for_select(project.possible_parents, :selected => project.parent) + options = '' + 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 42387c2d2..62c761d1a 100644 --- 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 e69dfedb0..0e286fcae 100644 --- a/app/views/projects/_form.rhtml +++ b/app/views/projects/_form.rhtml @@ -4,7 +4,7 @@

<%= f.text_field :name, :required => true %>
<%= l(:text_caracters_maximum, 30) %>

-<% if User.current.admin? && !@project.possible_parents.empty? %> +<% unless @project.allowed_parents.empty? %>

<%= parent_project_select_tag(@project) %>

<% end %> diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index dcbcb77eb..6704d4fa8 100644 --- 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 2c1d2601a..b4bb4cc11 100644 --- 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