From 5d2eea14893b509f0f9044a53270bd84b4e06acb Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Wed, 7 Jan 2015 22:19:57 +0000 Subject: [PATCH] Cleans up parent project assignment in ProjectsController. git-svn-id: http://svn.redmine.org/redmine/trunk@13847 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/projects_controller.rb | 26 +------- app/models/project.rb | 82 ++++++++++++++------------ test/object_helpers.rb | 6 +- 3 files changed, 51 insertions(+), 63 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 747212927..85848f513 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -74,8 +74,7 @@ class ProjectsController < ApplicationController @project = Project.new @project.safe_attributes = params[:project] - if validate_parent_id && @project.save - @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id') + if @project.save unless User.current.admin? @project.add_default_member(User.current) end @@ -110,8 +109,7 @@ class ProjectsController < ApplicationController Mailer.with_deliveries(params[:notifications] == '1') do @project = Project.new @project.safe_attributes = params[:project] - if validate_parent_id && @project.copy(@source_project, :only => params[:only]) - @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id') + if @project.copy(@source_project, :only => params[:only]) flash[:notice] = l(:notice_successful_create) redirect_to settings_project_path(@project) elsif !@project.new_record? @@ -170,8 +168,7 @@ class ProjectsController < ApplicationController def update @project.safe_attributes = params[:project] - if validate_parent_id && @project.save - @project.set_allowed_parent!(params[:project]['parent_id']) if params[:project].has_key?('parent_id') + if @project.save respond_to do |format| format.html { flash[:notice] = l(:notice_successful_update) @@ -233,21 +230,4 @@ class ProjectsController < ApplicationController # hide project in layout @project = nil end - - private - - # Validates parent_id param according to user's permissions - # TODO: move it to Project model in a validation that depends on User.current - def validate_parent_id - return true if User.current.admin? - parent_id = params[:project] && params[:project][:parent_id] - if parent_id || @project.new_record? - parent = parent_id.blank? ? nil : Project.find_by_id(parent_id.to_i) - unless @project.allowed_parents.include?(parent) - @project.errors.add :parent_id, :invalid - return false - end - end - true - end end diff --git a/app/models/project.rb b/app/models/project.rb index 90ca7cbe4..afb0ee0c5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -80,9 +80,11 @@ class Project < ActiveRecord::Base validates_format_of :identifier, :with => /\A(?!\d+$)[a-z0-9\-_]*\z/, :if => Proc.new { |p| p.identifier_changed? } # reserved words validates_exclusion_of :identifier, :in => %w( new ) + validate :validate_parent after_save :update_inherited_members, :if => Proc.new {|project| project.inherit_members_changed?} after_save :remove_inherited_member_roles, :add_inherited_member_roles, :if => Proc.new {|project| project.parent_id_changed?} + after_update :update_versions_from_hierarchy_change, :if => Proc.new {|project| project.parent_id_changed?} before_destroy :delete_all_members scope :has_module, lambda {|mod| @@ -366,11 +368,11 @@ class Project < ActiveRecord::Base # Returns an array of projects the project can be moved to # by the current user - def allowed_parents + def allowed_parents(user=User.current) return @allowed_parents if @allowed_parents - @allowed_parents = Project.allowed_to(User.current, :add_subprojects).to_a + @allowed_parents = Project.allowed_to(user, :add_subprojects).to_a @allowed_parents = @allowed_parents - self_and_descendants - if User.current.allowed_to?(:add_project, nil, :global => true) || (!new_record? && parent.nil?) + if user.allowed_to?(:add_project, nil, :global => true) || (!new_record? && parent.nil?) @allowed_parents << nil end unless parent.nil? || @allowed_parents.empty? || @allowed_parents.include?(parent) @@ -381,48 +383,20 @@ class Project < ActiveRecord::Base # 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) + p = p.id if p.is_a?(Project) + send :safe_attributes, {:project_id => p} + save end # Sets the parent of the project # Argument can be either a Project, a String, a Fixnum or nil def set_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 == parent && !p.nil? - # Nothing to do - true - elsif p.nil? || (p.active? && move_possible?(p)) + if p.is_a?(Project) self.parent = p - save - p.reload if p - Issue.update_versions_from_hierarchy_change(self) - true else - # Can not move to the given target - false + self.parent_id = p end + save end # Returns an array of the trackers used by the project and its active sub projects @@ -688,7 +662,8 @@ class Project < ActiveRecord::Base 'custom_field_values', 'custom_fields', 'tracker_ids', - 'issue_custom_field_ids' + 'issue_custom_field_ids', + 'parent_id' safe_attributes 'enabled_module_names', :if => lambda {|project, user| project.new_record? || user.allowed_to?(:select_project_modules, project) } @@ -696,6 +671,23 @@ class Project < ActiveRecord::Base safe_attributes 'inherit_members', :if => lambda {|project, user| project.parent.nil? || project.parent.visible?(user)} + def safe_attributes=(attrs, user=User.current) + return unless attrs.is_a?(Hash) + attrs = attrs.deep_dup + + @unallowed_parent_id = nil + parent_id_param = attrs['parent_id'].to_s + if parent_id_param.blank? || parent_id_param != parent_id.to_s + p = parent_id_param.present? ? Project.find_by_id(parent_id_param) : nil + unless allowed_parents(user).include?(p) + attrs.delete('parent_id') + @unallowed_parent_id = true + end + end + + super(attrs, user) + end + # Returns an auto-generated project identifier based on the last identifier used def self.next_identifier p = Project.order('id DESC').first @@ -797,6 +789,20 @@ class Project < ActiveRecord::Base end end + def update_versions_from_hierarchy_change + Issue.update_versions_from_hierarchy_change(self) + end + + def validate_parent + if @unallowed_parent_id + errors.add(:parent_id, :invalid) + elsif parent_id_changed? + unless parent.nil? || (parent.active? && move_possible?(parent)) + errors.add(:parent_id, :invalid) + end + end + end + # Copies wiki from +project+ def copy_wiki(project) # Check that the source project has a wiki first diff --git a/test/object_helpers.rb b/test/object_helpers.rb index ca58638d9..2b1fa2f61 100644 --- a/test/object_helpers.rb +++ b/test/object_helpers.rb @@ -40,8 +40,10 @@ module ObjectHelpers end def Project.generate_with_parent!(parent, attributes={}) - project = Project.generate!(attributes) - project.set_parent!(parent) + project = Project.generate!(attributes) do |p| + p.parent = parent + end + parent.reload if parent project end -- 2.39.5