jro_apps/redmine Commit - r13465:5d2eea14893b

Cleans up parent project assignment in ProjectsController....

Jean-Philippe Lang -

r13465:5d2eea14893b

parent child

Context file:

r13465:5d2eea14893b

Collapse all files

app/controllers/projects_controller.rb +3 -23

                  @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
                    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?
                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)
                  # 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

app/models/project.rb +44 -38

                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|
                # 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)
                # 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
                  '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) }
                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
                  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

test/object_helpers.rb +4 -2

                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

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