From 8e3d1b694ab47317638b474082cb70e08a8d02e7 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 13 Mar 2010 14:56:49 +0000 Subject: [PATCH] Adds subtasking (#443) including: * priority, start/due dates, progress, estimate, spent time roll-up to parent issues * descendant issues tree displayed on the issue view with context menu support * issue tree display on the gantt chart * issue tree copy on project copy * unlimited nesting Defining subtasks requires the new permission 'Manage subtasks'. Subtasks can not belong to a different project than the parent task. Implementation is based on scoped nested sets for fast reads and updates. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3573 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 23 +- app/controllers/timelog_controller.rb | 8 +- app/helpers/issues_helper.rb | 28 ++ app/models/issue.rb | 259 ++++++++++++--- app/models/issue_relation.rb | 1 + app/models/project.rb | 12 +- app/views/issues/_attributes.rhtml | 10 +- app/views/issues/_form.rhtml | 10 + app/views/issues/auto_complete.html.erb | 9 + app/views/issues/gantt.rhtml | 15 +- app/views/issues/show.rhtml | 20 +- config/locales/en.yml | 4 + config/locales/fr.yml | 4 + ...13132032_add_issues_nested_sets_columns.rb | 17 + lib/redmine.rb | 3 +- lib/redmine/default_data/loader.rb | 1 + lib/redmine/helpers/gantt.rb | 53 +++- public/images/task_parent_end.png | Bin 0 -> 165 bytes public/javascripts/application.js | 12 + public/stylesheets/application.css | 25 +- test/fixtures/enumerations.yml | 5 + test/fixtures/issues.yml | 39 +++ test/fixtures/roles.yml | 2 + test/functional/issues_controller_test.rb | 44 ++- test/unit/issue_nested_set_test.rb | 300 ++++++++++++++++++ test/unit/issue_test.rb | 31 +- .../lib/acts_as_customizable.rb | 7 + .../lib/awesome_nested_set.rb | 2 +- 28 files changed, 853 insertions(+), 91 deletions(-) create mode 100644 app/views/issues/auto_complete.html.erb create mode 100644 db/migrate/20100313132032_add_issues_nested_sets_columns.rb create mode 100644 public/images/task_parent_end.png create mode 100644 test/unit/issue_nested_set_test.rb diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index cd61fdc37..4a8c56ea9 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -21,7 +21,7 @@ class IssuesController < ApplicationController before_filter :find_issue, :only => [:show, :edit, :update, :reply] before_filter :find_issues, :only => [:bulk_edit, :move, :destroy] - before_filter :find_project, :only => [:new, :update_form, :preview] + before_filter :find_project, :only => [:new, :update_form, :preview, :auto_complete] before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu] before_filter :find_optional_project, :only => [:index, :changes, :gantt, :calendar] accept_key_auth :index, :show, :changes @@ -164,7 +164,8 @@ class IssuesController < ApplicationController call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue}) respond_to do |format| format.html { - redirect_to(params[:continue] ? { :action => 'new', :tracker_id => @issue.tracker } : + redirect_to(params[:continue] ? { :action => 'new', :issue => {:tracker_id => @issue.tracker, + :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} } : { :action => 'show', :id => @issue }) } format.xml { render :action => 'show', :status => :created, :location => url_for(:controller => 'issues', :action => 'show', :id => @issue) } @@ -247,6 +248,7 @@ class IssuesController < ApplicationController # Bulk edit a set of issues def bulk_edit + @issues.sort! if request.post? attributes = (params[:issue] || {}).reject {|k,v| v.blank?} attributes.keys.each {|k| attributes[k] = '' if attributes[k] == 'none'} @@ -254,6 +256,7 @@ class IssuesController < ApplicationController unsaved_issue_ids = [] @issues.each do |issue| + issue.reload journal = issue.init_journal(User.current, params[:notes]) issue.safe_attributes = attributes call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue }) @@ -271,6 +274,7 @@ class IssuesController < ApplicationController end def move + @issues.sort! @copy = params[:copy_options] && params[:copy_options][:copy] @allowed_projects = [] # find projects to which the user is allowed to move the issue @@ -289,6 +293,7 @@ class IssuesController < ApplicationController unsaved_issue_ids = [] moved_issues = [] @issues.each do |issue| + issue.reload changed_attributes = {} [:assigned_to_id, :status_id, :start_date, :due_date].each do |valid_attribute| unless params[valid_attribute].blank? @@ -297,7 +302,7 @@ class IssuesController < ApplicationController end issue.init_journal(User.current) call_hook(:controller_issues_move_before_save, { :params => params, :issue => issue, :target_project => @target_project, :copy => !!@copy }) - if r = issue.move_to(@target_project, new_tracker, {:copy => @copy, :attributes => changed_attributes}) + if r = issue.move_to_project(@target_project, new_tracker, {:copy => @copy, :attributes => changed_attributes}) moved_issues << r else unsaved_issue_ids << issue.id @@ -456,6 +461,18 @@ class IssuesController < ApplicationController render :partial => 'common/preview' end + def auto_complete + @issues = [] + q = params[:q].to_s + if q.match(/^\d+$/) + @issues << @project.issues.visible.find_by_id(q.to_i) + end + unless q.blank? + @issues += @project.issues.visible.find(:all, :conditions => ["LOWER(#{Issue.table_name}.subject) LIKE ?", "%#{q.downcase}%"], :limit => 10) + end + render :layout => false + end + private def find_issue @issue = Issue.find(params[:id], :include => [:project, :tracker, :status, :author, :priority, :category]) diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index c08815890..160c19974 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -94,7 +94,7 @@ class TimelogController < ApplicationController elsif @issue.nil? sql_condition = @project.project_condition(Setting.display_subprojects_issues?) else - sql_condition = "#{TimeEntry.table_name}.issue_id = #{@issue.id}" + sql_condition = "#{Issue.table_name}.root_id = #{@issue.root_id} AND #{Issue.table_name}.lft >= #{@issue.lft} AND #{Issue.table_name}.rgt <= #{@issue.rgt}" end sql = "SELECT #{sql_select}, tyear, tmonth, tweek, spent_on, SUM(hours) AS hours" @@ -166,7 +166,7 @@ class TimelogController < ApplicationController elsif @issue.nil? cond << @project.project_condition(Setting.display_subprojects_issues?) else - cond << ["#{TimeEntry.table_name}.issue_id = ?", @issue.id] + cond << "#{Issue.table_name}.root_id = #{@issue.root_id} AND #{Issue.table_name}.lft >= #{@issue.lft} AND #{Issue.table_name}.rgt <= #{@issue.rgt}" end retrieve_date_range @@ -176,7 +176,7 @@ class TimelogController < ApplicationController respond_to do |format| format.html { # Paginate results - @entry_count = TimeEntry.count(:include => :project, :conditions => cond.conditions) + @entry_count = TimeEntry.count(:include => [:project, :issue], :conditions => cond.conditions) @entry_pages = Paginator.new self, @entry_count, per_page_option, params['page'] @entries = TimeEntry.find(:all, :include => [:project, :activity, :user, {:issue => :tracker}], @@ -184,7 +184,7 @@ class TimelogController < ApplicationController :order => sort_clause, :limit => @entry_pages.items_per_page, :offset => @entry_pages.current.offset) - @total_hours = TimeEntry.sum(:hours, :include => :project, :conditions => cond.conditions).to_f + @total_hours = TimeEntry.sum(:hours, :include => [:project, :issue], :conditions => cond.conditions).to_f render :layout => !request.xhr? } diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 8124add12..db21e9ab7 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -30,6 +30,34 @@ module IssuesHelper "#{@cached_label_assigned_to}: #{issue.assigned_to}
" + "#{@cached_label_priority}: #{issue.priority.name}" end + + def render_issue_subject_with_tree(issue) + s = '' + issue.ancestors.each do |ancestor| + s << '
' + content_tag('p', link_to_issue(ancestor)) + end + s << '
' + content_tag('h3', h(issue.subject)) + s << '
' * (issue.ancestors.size + 1) + s + end + + def render_descendants_tree(issue) + s = '
' + ancestors = [] + issue.descendants.sort_by(&:lft).each do |child| + level = child.level - issue.level - 1 + s << content_tag('tr', + content_tag('td', check_box_tag("ids[]", child.id, false, :id => nil)) + + content_tag('td', link_to_issue(child), :class => 'subject', + :style => "padding-left: #{level * 20}px") + + content_tag('td', h(child.status)) + + content_tag('td', link_to_user(child.assigned_to)) + + content_tag('td', progress_bar(child.done_ratio, :width => '80px')), + :class => "issue-#{child.id} hascontextmenu") + end + s << '
' + s + end def render_custom_fields_rows(issue) return if issue.custom_field_values.empty? diff --git a/app/models/issue.rb b/app/models/issue.rb index 6f78ff1e8..c39c9ce8f 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -32,6 +32,7 @@ class Issue < ActiveRecord::Base has_many :relations_from, :class_name => 'IssueRelation', :foreign_key => 'issue_from_id', :dependent => :delete_all has_many :relations_to, :class_name => 'IssueRelation', :foreign_key => 'issue_to_id', :dependent => :delete_all + acts_as_nested_set :scope => 'root_id' acts_as_attachable :after_remove => :attachment_removed acts_as_customizable acts_as_watchable @@ -68,7 +69,9 @@ class Issue < ActiveRecord::Base before_create :default_assign before_save :reschedule_following_issues, :close_duplicates, :update_done_ratio_from_issue_status - after_save :create_journal + after_save :update_nested_set_attributes, :update_parent_attributes, :create_journal + after_destroy :destroy_children + after_destroy :update_parent_attributes # Returns true if usr or current user is allowed to view the issue def visible?(usr=nil) @@ -90,60 +93,75 @@ class Issue < ActiveRecord::Base def copy_from(arg) issue = arg.is_a?(Issue) ? arg : Issue.find(arg) - self.attributes = issue.attributes.dup.except("id", "created_on", "updated_on") - self.custom_values = issue.custom_values.collect {|v| v.clone} + self.attributes = issue.attributes.dup.except("id", "root_id", "parent_id", "lft", "rgt", "created_on", "updated_on") + self.custom_field_values = issue.custom_field_values.inject({}) {|h,v| h[v.custom_field_id] = v.value; h} self.status = issue.status self end # Moves/copies an issue to a new project and tracker # Returns the moved/copied issue on success, false on failure - def move_to(new_project, new_tracker = nil, options = {}) - options ||= {} - issue = options[:copy] ? self.clone : self + def move_to_project(*args) ret = Issue.transaction do - if new_project && issue.project_id != new_project.id - # delete issue relations - unless Setting.cross_project_issue_relations? - issue.relations_from.clear - issue.relations_to.clear - end - # issue is moved to another project - # reassign to the category with same name if any - new_category = issue.category.nil? ? nil : new_project.issue_categories.find_by_name(issue.category.name) - issue.category = new_category - # Keep the fixed_version if it's still valid in the new_project - unless new_project.shared_versions.include?(issue.fixed_version) - issue.fixed_version = nil - end - issue.project = new_project - end - if new_tracker - issue.tracker = new_tracker + move_to_project_without_transaction(*args) || raise(ActiveRecord::Rollback) + end || false + end + + def move_to_project_without_transaction(new_project, new_tracker = nil, options = {}) + options ||= {} + issue = options[:copy] ? self.class.new.copy_from(self) : self + + if new_project && issue.project_id != new_project.id + # delete issue relations + unless Setting.cross_project_issue_relations? + issue.relations_from.clear + issue.relations_to.clear end - if options[:copy] - issue.custom_field_values = self.custom_field_values.inject({}) {|h,v| h[v.custom_field_id] = v.value; h} - issue.status = if options[:attributes] && options[:attributes][:status_id] - IssueStatus.find_by_id(options[:attributes][:status_id]) - else - self.status - end + # issue is moved to another project + # reassign to the category with same name if any + new_category = issue.category.nil? ? nil : new_project.issue_categories.find_by_name(issue.category.name) + issue.category = new_category + # Keep the fixed_version if it's still valid in the new_project + unless new_project.shared_versions.include?(issue.fixed_version) + issue.fixed_version = nil end - # Allow bulk setting of attributes on the issue - if options[:attributes] - issue.attributes = options[:attributes] + issue.project = new_project + if issue.parent && issue.parent.project_id != issue.project_id + issue.parent_issue_id = nil end - if issue.save - unless options[:copy] - # Manually update project_id on related time entries - TimeEntry.update_all("project_id = #{new_project.id}", {:issue_id => id}) + end + if new_tracker + issue.tracker = new_tracker + issue.reset_custom_values! + end + if options[:copy] + issue.custom_field_values = self.custom_field_values.inject({}) {|h,v| h[v.custom_field_id] = v.value; h} + issue.status = if options[:attributes] && options[:attributes][:status_id] + IssueStatus.find_by_id(options[:attributes][:status_id]) + else + self.status + end + end + # Allow bulk setting of attributes on the issue + if options[:attributes] + issue.attributes = options[:attributes] + end + if issue.save + unless options[:copy] + # Manually update project_id on related time entries + TimeEntry.update_all("project_id = #{new_project.id}", {:issue_id => id}) + + issue.children.each do |child| + unless child.move_to_project_without_transaction(new_project) + # Move failed and transaction was rollback'd + return false + end end - true - else - raise ActiveRecord::Rollback end + else + return false end - ret ? issue : false + issue end def priority_id=(pid) @@ -177,6 +195,7 @@ class Issue < ActiveRecord::Base SAFE_ATTRIBUTES = %w( tracker_id status_id + parent_issue_id category_id assigned_to_id priority_id @@ -203,6 +222,19 @@ class Issue < ActiveRecord::Base attrs.delete('status_id') end end + + unless leaf? + attrs.reject! {|k,v| %w(priority_id done_ratio start_date due_date estimated_hours).include?(k)} + end + + if attrs.has_key?('parent_issue_id') + if !user.allowed_to?(:manage_subtasks, project) + attrs.delete('parent_issue_id') + elsif !attrs['parent_issue_id'].blank? + attrs.delete('parent_issue_id') unless Issue.visible(user).exists?(attrs['parent_issue_id']) + end + end + self.attributes = attrs end @@ -249,6 +281,22 @@ class Issue < ActiveRecord::Base errors.add :tracker_id, :inclusion end end + + # Checks parent issue assignment + if @parent_issue + if @parent_issue.project_id != project_id + errors.add :parent_issue_id, :not_same_project + elsif !new_record? + # moving an existing issue + if @parent_issue.root_id != root_id + # we can always move to another tree + elsif move_possible?(@parent_issue) + # move accepted inside tree + else + errors.add :parent_issue_id, :not_a_valid_parent + end + end + end end # Set the done_ratio using the status if that setting is set. This will keep the done_ratios @@ -340,13 +388,13 @@ class Issue < ActiveRecord::Base notified.collect(&:mail) end - # Returns the total number of hours spent on this issue. + # Returns the total number of hours spent on this issue and its descendants # # Example: - # spent_hours => 0 - # spent_hours => 50 + # spent_hours => 0.0 + # spent_hours => 50.2 def spent_hours - @spent_hours ||= time_entries.sum(:hours) || 0 + @spent_hours ||= self_and_descendants.sum("#{TimeEntry.table_name}.hours", :include => :time_entries).to_f || 0.0 end def relations @@ -386,6 +434,16 @@ class Issue < ActiveRecord::Base @soonest_start ||= relations_to.collect{|relation| relation.successor_soonest_start}.compact.min end + def <=>(issue) + if issue.nil? + -1 + elsif root_id != issue.root_id + (root_id || 0) <=> (issue.root_id || 0) + else + (lft || 0) <=> (issue.lft || 0) + end + end + def to_s "#{tracker} ##{id}: #{subject}" end @@ -442,6 +500,24 @@ class Issue < ActiveRecord::Base Issue.update_versions(["#{Version.table_name}.project_id IN (?) OR #{Issue.table_name}.project_id IN (?)", moved_project_ids, moved_project_ids]) end + def parent_issue_id=(arg) + parent_issue_id = arg.blank? ? nil : arg.to_i + if parent_issue_id && @parent_issue = Issue.find_by_id(parent_issue_id) + @parent_issue.id + else + @parent_issue = nil + nil + end + end + + def parent_issue_id + if instance_variable_defined? :@parent_issue + @parent_issue.nil? ? nil : @parent_issue.id + else + parent_id + end + end + # Extracted from the ReportsController. def self.by_tracker(project) count_and_group_by(:project => project, @@ -495,6 +571,95 @@ class Issue < ActiveRecord::Base private + def update_nested_set_attributes + if root_id.nil? + # issue was just created + self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id) + set_default_left_and_right + Issue.update_all("root_id = #{root_id}, lft = #{lft}, rgt = #{rgt}", ["id = ?", id]) + if @parent_issue + move_to_child_of(@parent_issue) + end + reload + elsif parent_issue_id != parent_id + # moving an existing issue + if @parent_issue && @parent_issue.root_id == root_id + # inside the same tree + move_to_child_of(@parent_issue) + else + # to another tree + unless root? + move_to_right_of(root) + reload + end + old_root_id = root_id + self.root_id = (@parent_issue.nil? ? id : @parent_issue.root_id ) + target_maxright = nested_set_scope.maximum(right_column_name) || 0 + offset = target_maxright + 1 - lft + Issue.update_all("root_id = #{root_id}, lft = lft + #{offset}, rgt = rgt + #{offset}", + ["root_id = ? AND lft >= ? AND rgt <= ? ", old_root_id, lft, rgt]) + self[left_column_name] = lft + offset + self[right_column_name] = rgt + offset + if @parent_issue + move_to_child_of(@parent_issue) + end + end + reload + # delete invalid relations of all descendants + self_and_descendants.each do |issue| + issue.relations.each do |relation| + relation.destroy unless relation.valid? + end + end + end + remove_instance_variable(:@parent_issue) if instance_variable_defined?(:@parent_issue) + end + + def update_parent_attributes + if parent_id && p = Issue.find_by_id(parent_id) + # priority = highest priority of children + if priority_position = p.children.maximum("#{IssuePriority.table_name}.position", :include => :priority) + p.priority = IssuePriority.find_by_position(priority_position) + end + + # start/due dates = lowest/highest dates of children + p.start_date = p.children.minimum(:start_date) + p.due_date = p.children.maximum(:due_date) + if p.start_date && p.due_date && p.due_date < p.start_date + p.start_date, p.due_date = p.due_date, p.start_date + end + + # done ratio = weighted average ratio of leaves + unless Issue.use_status_for_done_ratio? && p.status && p.status.default_done_ratio? + leaves_count = p.leaves.count + if leaves_count > 0 + average = p.leaves.average(:estimated_hours).to_f + if average == 0 + average = 1 + end + done = p.leaves.sum("COALESCE(estimated_hours, #{average}) * (CASE WHEN is_closed = #{connection.quoted_true} THEN 100 ELSE COALESCE(done_ratio, 0) END)", :include => :status).to_f + progress = done / (average * leaves_count) + p.done_ratio = progress.round + end + end + + # estimate = sum of leaves estimates + p.estimated_hours = p.leaves.sum(:estimated_hours).to_f + p.estimated_hours = nil if p.estimated_hours == 0.0 + + # ancestors will be recursively updated + p.save(false) + end + end + + def destroy_children + unless leaf? + children.each do |child| + child.destroy + end + end + end + # Update issues so their versions are not pointing to a # fixed_version that is not shared with the issue's project def self.update_versions(conditions=nil) @@ -562,7 +727,7 @@ class Issue < ActiveRecord::Base def create_journal if @current_journal # attributes changes - (Issue.column_names - %w(id description lock_version created_on updated_on)).each {|c| + (Issue.column_names - %w(id description root_id lft rgt lock_version created_on updated_on)).each {|c| @current_journal.details << JournalDetail.new(:property => 'attr', :prop_key => c, :old_value => @issue_before_change.send(c), diff --git a/app/models/issue_relation.rb b/app/models/issue_relation.rb index 6dd4ce4be..2c110a72f 100644 --- a/app/models/issue_relation.rb +++ b/app/models/issue_relation.rb @@ -48,6 +48,7 @@ class IssueRelation < ActiveRecord::Base errors.add :issue_to_id, :invalid if issue_from_id == issue_to_id errors.add :issue_to_id, :not_same_project unless issue_from.project_id == issue_to.project_id || Setting.cross_project_issue_relations? errors.add_to_base :circular_dependency if issue_to.all_dependent_issues.include? issue_from + errors.add_to_base :cant_link_an_issue_with_a_descendant if issue_from.is_descendant_of?(issue_to) || issue_from.is_ancestor_of?(issue_to) end end diff --git a/app/models/project.rb b/app/models/project.rb index d5941e095..8d6303224 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -557,9 +557,12 @@ class Project < ActiveRecord::Base # value. Used to map the two togeather for issue relations. issues_map = {} - project.issues.each do |issue| + # Get issues sorted by root_id, lft so that parent issues + # get copied before their children + project.issues.find(:all, :order => 'root_id, lft').each do |issue| new_issue = Issue.new new_issue.copy_from(issue) + new_issue.project = self # Reassign fixed_versions by name, since names are unique per # project and the versions for self are not yet saved if issue.fixed_version @@ -570,6 +573,13 @@ class Project < ActiveRecord::Base if issue.category new_issue.category = self.issue_categories.select {|c| c.name == issue.category.name}.first end + # Parent issue + if issue.parent_id + if copied_parent = issues_map[issue.parent_id] + new_issue.parent_issue_id = copied_parent.id + end + end + self.issues << new_issue issues_map[issue.id] = new_issue end diff --git a/app/views/issues/_attributes.rhtml b/app/views/issues/_attributes.rhtml index acbd9101a..455eb77b2 100644 --- a/app/views/issues/_attributes.rhtml +++ b/app/views/issues/_attributes.rhtml @@ -7,7 +7,7 @@

<%= @issue.status.name %>

<% end %> -

<%= f.select :priority_id, (@priorities.collect {|p| [p.name, p.id]}), :required => true %>

+

<%= f.select :priority_id, (@priorities.collect {|p| [p.name, p.id]}), {:required => true}, :disabled => !@issue.leaf? %>

<%= f.select :assigned_to_id, (@issue.assignable_users.collect {|m| [m.name, m.id]}), :include_blank => true %>

<% unless @project.issue_categories.empty? %>

<%= f.select :category_id, (@project.issue_categories.collect {|c| [c.name, c.id]}), :include_blank => true %> @@ -31,10 +31,10 @@

-

<%= f.text_field :start_date, :size => 10 %><%= calendar_for('issue_start_date') %>

-

<%= f.text_field :due_date, :size => 10 %><%= calendar_for('issue_due_date') %>

-

<%= f.text_field :estimated_hours, :size => 3 %> <%= l(:field_hours) %>

-<% if Issue.use_field_for_done_ratio? %> +

<%= f.text_field :start_date, :size => 10, :disabled => !@issue.leaf? %><%= calendar_for('issue_start_date') if @issue.leaf? %>

+

<%= f.text_field :due_date, :size => 10, :disabled => !@issue.leaf? %><%= calendar_for('issue_due_date') if @issue.leaf? %>

+

<%= f.text_field :estimated_hours, :size => 3, :disabled => !@issue.leaf? %> <%= l(:field_hours) %>

+<% if @issue.leaf? && Issue.use_field_for_done_ratio? %>

<%= f.select :done_ratio, ((0..10).to_a.collect {|r| ["#{r*10} %", r*10] }) %>

<% end %>
diff --git a/app/views/issues/_form.rhtml b/app/views/issues/_form.rhtml index 032d08205..72fd6221d 100644 --- a/app/views/issues/_form.rhtml +++ b/app/views/issues/_form.rhtml @@ -5,6 +5,16 @@ :with => "Form.serialize('issue-form')" %>

<%= f.text_field :subject, :size => 80, :required => true %>

+ +<% unless (@issue.new_record? && @issue.parent_issue_id.nil?) || !User.current.allowed_to?(:manage_subtasks, @project) %> +

<%= f.text_field :parent_issue_id, :size => 10 %>

+
+<%= javascript_tag "observeParentIssueField('#{url_for(:controller => :issues, + :action => :auto_complete, + :id => @issue, + :project_id => @project) }')" %> +<% end %> +

<%= f.text_area :description, :cols => 60, :rows => (@issue.description.blank? ? 10 : [[10, @issue.description.length / 50].max, 100].min), diff --git a/app/views/issues/auto_complete.html.erb b/app/views/issues/auto_complete.html.erb new file mode 100644 index 000000000..b130159eb --- /dev/null +++ b/app/views/issues/auto_complete.html.erb @@ -0,0 +1,9 @@ +

diff --git a/app/views/issues/gantt.rhtml b/app/views/issues/gantt.rhtml index 8d6677d9c..f0c6f4622 100644 --- a/app/views/issues/gantt.rhtml +++ b/app/views/issues/gantt.rhtml @@ -79,8 +79,10 @@ t_height = g_height + headers_height # Tasks subjects # top = headers_height + 8 -@gantt.events.each do |i| %> -
+@gantt.events.each do |i| +left = 4 + (i.is_a?(Issue) ? i.level * 16 : 0) + %> +
<% if i.is_a? Issue %> <%= h("#{i.project} -") unless @project && @project == i.project %> <%= link_to_issue i %> @@ -189,15 +191,16 @@ top = headers_height + 10 i_width = ((i_end_date - i_start_date + 1)*zoom).floor - 2 # total width of the issue (- 2 for left and right borders) d_width = ((i_done_date - i_start_date)*zoom).floor - 2 # done width l_width = i_late_date ? ((i_late_date - i_start_date+1)*zoom).floor - 2 : 0 # delay width + css = "task " + (i.leaf? ? 'leaf' : 'parent') %> -
 
+
 
<% if l_width > 0 %> -
 
+
 
<% end %> <% if d_width > 0 %> -
 
+
 
<% end %> -
+
<%= i.status.name %> <%= (i.done_ratio).to_i %>%
diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml index b78908b00..c07fa921b 100644 --- a/app/views/issues/show.rhtml +++ b/app/views/issues/show.rhtml @@ -4,7 +4,10 @@
<%= avatar(@issue.author, :size => "50") %> -

<%=h @issue.subject %>

+ +
+<%= render_issue_subject_with_tree(@issue) %> +

<%= authoring @issue.created_on, @issue.author %>. <% if @issue.created_on != @issue.updated_on %> @@ -56,6 +59,17 @@ <%= call_hook(:view_issues_show_description_bottom, :issue => @issue) %> +<% if !@issue.leaf? || User.current.allowed_to?(:manage_subtasks, @project) %> +

+
+
+ <%= link_to(l(:button_add), {:controller => 'issues', :action => 'new', :project_id => @project, :issue => {:parent_issue_id => @issue}}) if User.current.allowed_to?(:manage_subtasks, @project) %> +
+

<%=l(:label_subtask_pural)%>

+<%= render_descendants_tree(@issue) unless @issue.leaf? %> +
+<% end %> + <% if authorize_for('issue_relations', 'new') || @issue.relations.any? %>
@@ -113,4 +127,8 @@ <% content_for :header_tags do %> <%= auto_discovery_link_tag(:atom, {:format => 'atom', :key => User.current.rss_key}, :title => "#{@issue.project} - #{@issue.tracker} ##{@issue.id}: #{@issue.subject}") %> <%= stylesheet_link_tag 'scm' %> + <%= javascript_include_tag 'context_menu' %> + <%= stylesheet_link_tag 'context_menu' %> <% end %> + +<%= javascript_tag "new ContextMenu('#{url_for(:controller => 'issues', :action => 'context_menu')}')" %> \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index 95dd45244..f636c9d99 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -112,6 +112,7 @@ en: greater_than_start_date: "must be greater than start date" not_same_project: "doesn't belong to the same project" circular_dependency: "This relation would create a circular dependency" + cant_link_an_issue_with_a_descendant: "An issue can not be linked to one of its subtasks" actionview_instancetag_blank_option: Please select @@ -277,6 +278,7 @@ en: field_content: Content field_group_by: Group results by field_sharing: Sharing + field_parent_issue: Parent task setting_app_title: Application title setting_app_subtitle: Application subtitle @@ -386,6 +388,7 @@ en: permission_delete_messages: Delete messages permission_delete_own_messages: Delete own messages permission_export_wiki_pages: Export wiki pages + permission_manage_subtasks: Manage subtasks project_module_issue_tracking: Issue tracking project_module_time_tracking: Time tracking @@ -750,6 +753,7 @@ en: label_missing_api_access_key: Missing an API access key label_api_access_key_created_on: "API access key created {{value}} ago" label_profile: Profile + label_subtask_plural: Subtasks button_login: Login button_submit: Submit diff --git a/config/locales/fr.yml b/config/locales/fr.yml index ad665235e..06f645600 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -136,6 +136,7 @@ fr: greater_than_start_date: "doit être postérieure à la date de début" not_same_project: "n'appartient pas au même projet" circular_dependency: "Cette relation créerait une dépendance circulaire" + cant_link_an_issue_with_a_descendant: "Une demande ne peut pas être liée à l'une de ses sous-tâches" actionview_instancetag_blank_option: Choisir @@ -299,6 +300,7 @@ fr: field_group_by: Grouper par field_sharing: Partage field_active: Actif + field_parent_issue: Tâche parente setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application @@ -408,6 +410,7 @@ fr: permission_delete_own_messages: Supprimer ses propres messages permission_export_wiki_pages: Exporter les pages permission_manage_project_activities: Gérer les activités + permission_manage_subtasks: Gérer les sous-tâches project_module_issue_tracking: Suivi des demandes project_module_time_tracking: Suivi du temps passé @@ -765,6 +768,7 @@ fr: label_close_versions: Fermer les versions terminées label_revision_id: Revision {{value}} label_profile: Profil + label_subtask_pural: Sous-tâches button_login: Connexion button_submit: Soumettre diff --git a/db/migrate/20100313132032_add_issues_nested_sets_columns.rb b/db/migrate/20100313132032_add_issues_nested_sets_columns.rb new file mode 100644 index 000000000..e7d03e980 --- /dev/null +++ b/db/migrate/20100313132032_add_issues_nested_sets_columns.rb @@ -0,0 +1,17 @@ +class AddIssuesNestedSetsColumns < ActiveRecord::Migration + def self.up + add_column :issues, :parent_id, :integer, :default => nil + add_column :issues, :root_id, :integer, :default => nil + add_column :issues, :lft, :integer, :default => nil + add_column :issues, :rgt, :integer, :default => nil + + Issue.update_all("parent_id = NULL, root_id = id, lft = 1, rgt = 2") + end + + def self.down + remove_column :issues, :parent_id + remove_column :issues, :root_id + remove_column :issues, :lft + remove_column :issues, :rgt + end +end diff --git a/lib/redmine.rb b/lib/redmine.rb index 1205f6fa4..df57b08dc 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -47,13 +47,14 @@ Redmine::AccessControl.map do |map| map.permission :manage_categories, {:projects => :settings, :issue_categories => [:new, :edit, :destroy]}, :require => :member # Issues map.permission :view_issues, {:projects => :roadmap, - :issues => [:index, :changes, :show, :context_menu], + :issues => [:index, :changes, :show, :context_menu, :auto_complete], :versions => [:show, :status_by], :queries => :index, :reports => [:issue_report, :issue_report_details]} map.permission :add_issues, {:issues => [:new, :update_form]} map.permission :edit_issues, {:issues => [:edit, :update, :reply, :bulk_edit, :update_form]} map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]} + map.permission :manage_subtasks, {} map.permission :add_issue_notes, {:issues => [:edit, :update, :reply]} map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin diff --git a/lib/redmine/default_data/loader.rb b/lib/redmine/default_data/loader.rb index 7baeb976c..5eed3d9b0 100644 --- a/lib/redmine/default_data/loader.rb +++ b/lib/redmine/default_data/loader.rb @@ -53,6 +53,7 @@ module Redmine :add_issues, :edit_issues, :manage_issue_relations, + :manage_subtasks, :add_issue_notes, :save_queries, :view_gantt, diff --git a/lib/redmine/helpers/gantt.rb b/lib/redmine/helpers/gantt.rb index e24be1cae..330b58ee7 100644 --- a/lib/redmine/helpers/gantt.rb +++ b/lib/redmine/helpers/gantt.rb @@ -52,8 +52,29 @@ module Redmine @date_to = (@date_from >> @months) - 1 end + def events=(e) - @events = e.sort {|x,y| x.start_date <=> y.start_date } + @events = e + # Adds all ancestors + root_ids = e.select {|i| i.is_a?(Issue) && i.parent_id? }.collect(&:root_id).uniq + if root_ids.any? + # Retrieves all nodes + parents = Issue.find_all_by_root_id(root_ids, :conditions => ["rgt - lft > 1"]) + # Only add ancestors + @events += parents.select {|p| @events.detect {|i| i.is_a?(Issue) && p.is_ancestor_of?(i)}} + end + @events.uniq! + # Sort issues by hierarchy and start dates + @events.sort! {|x,y| + if x.is_a?(Issue) && y.is_a?(Issue) + gantt_issue_compare(x, y, @events) + else + gantt_start_compare(x, y) + end + } + # Removes issues that have no start or end date + @events.reject! {|i| i.is_a?(Issue) && (i.start_date.nil? || i.due_before.nil?) } + @events end def params @@ -218,6 +239,36 @@ module Redmine imgl.format = format imgl.to_blob end if Object.const_defined?(:Magick) + + private + + def gantt_issue_compare(x, y, issues) + if x.parent_id == y.parent_id + gantt_start_compare(x, y) + elsif x.is_ancestor_of?(y) + -1 + elsif y.is_ancestor_of?(x) + 1 + else + ax = issues.select {|i| i.is_a?(Issue) && i.is_ancestor_of?(x) && !i.is_ancestor_of?(y) }.sort_by(&:lft).first + ay = issues.select {|i| i.is_a?(Issue) && i.is_ancestor_of?(y) && !i.is_ancestor_of?(x) }.sort_by(&:lft).first + if ax.nil? && ay.nil? + gantt_start_compare(x, y) + else + gantt_issue_compare(ax || x, ay || y, issues) + end + end + end + + def gantt_start_compare(x, y) + if x.start_date.nil? + -1 + elsif y.start_date.nil? + 1 + else + x.start_date <=> y.start_date + end + end end end end diff --git a/public/images/task_parent_end.png b/public/images/task_parent_end.png new file mode 100644 index 0000000000000000000000000000000000000000..fc920564345aa04a2572bf729daac9e635d4cc04 GIT binary patch literal 165 zcmeAS@N?(olHy`uVBq!ia0vp^96&6ZRaQU(D&A+G=b|9}4cc~4IdP=FYq zdG+rnpgP8qAirP+hi5m^fE;B{7sn8e>&XF3Ol(RVimIxvY;LNmjBIR*u54^|!hAYK3f literal 0 HcmV?d00001 diff --git a/public/javascripts/application.js b/public/javascripts/application.js index 787b9904c..4cd34c2f2 100644 --- a/public/javascripts/application.js +++ b/public/javascripts/application.js @@ -194,6 +194,18 @@ function randomKey(size) { return key; } +function observeParentIssueField(url) { + new Ajax.Autocompleter('issue_parent_issue_id', + 'parent_issue_candidates', + url, + { minChars: 3, + frequency: 0.5, + paramName: 'q', + updateElement: function(value) { + document.getElementById('issue_parent_issue_id').value = value.id; + }}); +} + /* shows and hides ajax indicator */ Ajax.Responders.register({ onCreate: function(){ diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index ec31c95c3..cd8e98699 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -237,6 +237,13 @@ p.breadcrumb { font-size: 0.9em; margin: 4px 0 4px 0;} p.subtitle { font-size: 0.9em; margin: -6px 0 12px 0; font-style: italic; } p.footnote { font-size: 0.9em; margin-top: 0px; margin-bottom: 0px; } +div.issue div.subject div div { padding-left: 16px; } +div.issue div.subject p {margin: 0; margin-bottom: 0.1em; font-size: 90%; color: #999;} +div.issue div.subject>div>p { margin-top: 0.5em; } +div.issue div.subject h3 {margin: 0; margin-bottom: 0.1em;} + +#issue_tree table.issues { border: 0; } + fieldset.collapsible { border-width: 1px 0 0 0; font-size: 0.9em; } fieldset.collapsible legend { padding-left: 16px; background: url(../images/arrow_expanded.png) no-repeat 0% 40%; cursor:pointer; } fieldset.collapsible.collapsed legend { background-image: url(../images/arrow_collapsed.png); } @@ -588,8 +595,7 @@ button.tab-right { /***** Auto-complete *****/ div.autocomplete { position:absolute; - width:250px; - background-color:white; + width:400px; margin:0; padding:0; } @@ -598,23 +604,26 @@ div.autocomplete ul { margin:0; padding:0; } -div.autocomplete ul li.selected { background-color: #ffb;} div.autocomplete ul li { list-style-type:none; display:block; - margin:0; + margin:-1px 0 0 0; padding:2px; cursor:pointer; font-size: 90%; - border-bottom: 1px solid #ccc; + border: 1px solid #ccc; border-left: 1px solid #ccc; border-right: 1px solid #ccc; + background-color:white; } +div.autocomplete ul li.selected { background-color: #ffb;} div.autocomplete ul li span.informal { font-size: 80%; color: #aaa; } +#parent_issue_candidates ul li {width: 500px;} + /***** Diff *****/ .diff_out { background: #fcc; } .diff_in { background: #cfc; } @@ -741,6 +750,12 @@ background-image:url('../images/close_hl.png'); .task_late { background:#f66 url(../images/task_late.png); border: 1px solid #f66; } .task_done { background:#66f url(../images/task_done.png); border: 1px solid #66f; } .task_todo { background:#aaa url(../images/task_todo.png); border: 1px solid #aaa; } + +.task_todo.parent { background: #888; border: 1px solid #888; height: 6px;} +.task_late.parent, .task_done.parent { height: 3px;} +.task_todo.parent .left { position: absolute; background: url(../images/task_parent_end.png) no-repeat 0 0; width: 8px; height: 16px; margin-left: -5px; left: 0px; top: -1px;} +.task_todo.parent .right { position: absolute; background: url(../images/task_parent_end.png) no-repeat 0 0; width: 8px; height: 16px; margin-right: -5px; right: 0px; top: -1px;} + .milestone { background-image:url(../images/milestone.png); background-repeat: no-repeat; border: 0; } /***** Icons *****/ diff --git a/test/fixtures/enumerations.yml b/test/fixtures/enumerations.yml index 650a48c3e..c4a25d954 100644 --- a/test/fixtures/enumerations.yml +++ b/test/fixtures/enumerations.yml @@ -19,27 +19,32 @@ enumerations_004: id: 4 type: IssuePriority active: true + position: 1 enumerations_005: name: Normal id: 5 type: IssuePriority is_default: true active: true + position: 2 enumerations_006: name: High id: 6 type: IssuePriority active: true + position: 3 enumerations_007: name: Urgent id: 7 type: IssuePriority active: true + position: 4 enumerations_008: name: Immediate id: 8 type: IssuePriority active: true + position: 5 enumerations_009: name: Design id: 9 diff --git a/test/fixtures/issues.yml b/test/fixtures/issues.yml index 4b61b41a3..6ec671e58 100644 --- a/test/fixtures/issues.yml +++ b/test/fixtures/issues.yml @@ -15,6 +15,9 @@ issues_001: status_id: 1 start_date: <%= 1.day.ago.to_date.to_s(:db) %> due_date: <%= 10.day.from_now.to_date.to_s(:db) %> + root_id: 1 + lft: 1 + rgt: 2 issues_002: created_on: 2006-07-19 21:04:21 +02:00 project_id: 1 @@ -31,6 +34,9 @@ issues_002: status_id: 2 start_date: <%= 2.day.ago.to_date.to_s(:db) %> due_date: + root_id: 2 + lft: 1 + rgt: 2 issues_003: created_on: 2006-07-19 21:07:27 +02:00 project_id: 1 @@ -47,6 +53,9 @@ issues_003: status_id: 1 start_date: <%= 1.day.from_now.to_date.to_s(:db) %> due_date: <%= 40.day.ago.to_date.to_s(:db) %> + root_id: 3 + lft: 1 + rgt: 2 issues_004: created_on: <%= 5.days.ago.to_date.to_s(:db) %> project_id: 2 @@ -61,6 +70,9 @@ issues_004: assigned_to_id: 2 author_id: 2 status_id: 1 + root_id: 4 + lft: 1 + rgt: 2 issues_005: created_on: <%= 5.days.ago.to_date.to_s(:db) %> project_id: 3 @@ -75,6 +87,9 @@ issues_005: assigned_to_id: author_id: 2 status_id: 1 + root_id: 5 + lft: 1 + rgt: 2 issues_006: created_on: <%= 1.minute.ago.to_date.to_s(:db) %> project_id: 5 @@ -91,6 +106,9 @@ issues_006: status_id: 1 start_date: <%= Date.today.to_s(:db) %> due_date: <%= 1.days.from_now.to_date.to_s(:db) %> + root_id: 6 + lft: 1 + rgt: 2 issues_007: created_on: <%= 10.days.ago.to_date.to_s(:db) %> project_id: 1 @@ -108,6 +126,9 @@ issues_007: start_date: <%= 10.days.ago.to_s(:db) %> due_date: <%= Date.today.to_s(:db) %> lock_version: 0 + root_id: 7 + lft: 1 + rgt: 2 issues_008: created_on: <%= 10.days.ago.to_date.to_s(:db) %> project_id: 1 @@ -125,6 +146,9 @@ issues_008: start_date: due_date: lock_version: 0 + root_id: 8 + lft: 1 + rgt: 2 issues_009: created_on: <%= 1.minute.ago.to_date.to_s(:db) %> project_id: 5 @@ -141,6 +165,9 @@ issues_009: status_id: 1 start_date: <%= Date.today.to_s(:db) %> due_date: <%= 1.days.from_now.to_date.to_s(:db) %> + root_id: 9 + lft: 1 + rgt: 2 issues_010: created_on: <%= 1.minute.ago.to_date.to_s(:db) %> project_id: 5 @@ -157,6 +184,9 @@ issues_010: status_id: 1 start_date: <%= Date.today.to_s(:db) %> due_date: <%= 1.days.from_now.to_date.to_s(:db) %> + root_id: 10 + lft: 1 + rgt: 2 issues_011: created_on: <%= 3.days.ago.to_date.to_s(:db) %> project_id: 1 @@ -173,6 +203,9 @@ issues_011: status_id: 5 start_date: <%= 1.day.ago.to_date.to_s(:db) %> due_date: + root_id: 11 + lft: 1 + rgt: 2 issues_012: created_on: <%= 3.days.ago.to_date.to_s(:db) %> project_id: 1 @@ -189,6 +222,9 @@ issues_012: status_id: 5 start_date: <%= 1.day.ago.to_date.to_s(:db) %> due_date: + root_id: 12 + lft: 1 + rgt: 2 issues_013: created_on: <%= 5.days.ago.to_date.to_s(:db) %> project_id: 3 @@ -203,3 +239,6 @@ issues_013: assigned_to_id: author_id: 2 status_id: 1 + root_id: 13 + lft: 1 + rgt: 2 diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml index e3dc3645f..f63203ae3 100644 --- a/test/fixtures/roles.yml +++ b/test/fixtures/roles.yml @@ -14,6 +14,7 @@ roles_001: - :add_issues - :edit_issues - :manage_issue_relations + - :manage_subtasks - :add_issue_notes - :move_issues - :delete_issues @@ -66,6 +67,7 @@ roles_002: - :add_issues - :edit_issues - :manage_issue_relations + - :manage_subtasks - :add_issue_notes - :move_issues - :delete_issues diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index b2f75eaba..11d35f500 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -476,7 +476,7 @@ class IssuesControllerTest < ActionController::TestCase :subject => 'This is first issue', :priority_id => 5}, :continue => '' - assert_redirected_to :controller => 'issues', :action => 'new', :tracker_id => 3 + assert_redirected_to :controller => 'issues', :action => 'new', :issue => {:tracker_id => 3} end def test_post_new_without_custom_fields_param @@ -533,6 +533,20 @@ class IssuesControllerTest < ActionController::TestCase assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail) end + def test_post_new_subissue + @request.session[:user_id] = 2 + + assert_difference 'Issue.count' do + post :new, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is a child issue', + :parent_issue_id => 2} + end + issue = Issue.find_by_subject('This is a child issue') + assert_not_nil issue + assert_equal Issue.find(2), issue.parent + end + def test_post_new_should_send_a_notification ActionMailer::Base.deliveries.clear @request.session[:user_id] = 2 @@ -1214,6 +1228,34 @@ class IssuesControllerTest < ActionController::TestCase :attributes => { :href => '#', :class => 'icon-del disabled' } end + + def test_auto_complete_routing + assert_routing( + {:method => :get, :path => '/issues/auto_complete'}, + :controller => 'issues', :action => 'auto_complete' + ) + end + + def test_auto_complete_should_not_be_case_sensitive + get :auto_complete, :project_id => 'ecookbook', :q => 'ReCiPe' + assert_response :success + assert_not_nil assigns(:issues) + assert assigns(:issues).detect {|issue| issue.subject.match /recipe/} + end + + def test_auto_complete_should_return_issue_with_given_id + get :auto_complete, :project_id => 'subproject1', :q => '13' + assert_response :success + assert_not_nil assigns(:issues) + assert assigns(:issues).include?(Issue.find(13)) + end + + def test_destroy_routing + assert_recognizes( #TODO: use DELETE on issue URI (need to change forms) + {:controller => 'issues', :action => 'destroy', :id => '1'}, + {:method => :post, :path => '/issues/1/destroy'} + ) + end def test_destroy_issue_with_no_time_entries assert_nil TimeEntry.find_by_issue_id(2) diff --git a/test/unit/issue_nested_set_test.rb b/test/unit/issue_nested_set_test.rb new file mode 100644 index 000000000..d82a42d28 --- /dev/null +++ b/test/unit/issue_nested_set_test.rb @@ -0,0 +1,300 @@ +# redMine - project management software +# Copyright (C) 2006-2007 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.dirname(__FILE__) + '/../test_helper' + +class IssueNestedSetTest < ActiveSupport::TestCase + fixtures :projects, :users, :members, :member_roles, :roles, + :trackers, :projects_trackers, + :versions, + :issue_statuses, :issue_categories, :issue_relations, :workflows, + :enumerations, + :issues, + :custom_fields, :custom_fields_projects, :custom_fields_trackers, :custom_values, + :time_entries + + self.use_transactional_fixtures = false + + def test_create_root_issue + issue1 = create_issue! + issue2 = create_issue! + issue1.reload + issue2.reload + + assert_equal [issue1.id, nil, 1, 2], [issue1.root_id, issue1.parent_id, issue1.lft, issue1.rgt] + assert_equal [issue2.id, nil, 1, 2], [issue2.root_id, issue2.parent_id, issue2.lft, issue2.rgt] + end + + def test_create_child_issue + parent = create_issue! + child = create_issue!(:parent_issue_id => parent.id) + parent.reload + child.reload + + assert_equal [parent.id, nil, 1, 4], [parent.root_id, parent.parent_id, parent.lft, parent.rgt] + assert_equal [parent.id, parent.id, 2, 3], [child.root_id, child.parent_id, child.lft, child.rgt] + end + + def test_creating_a_child_in_different_project_should_not_validate + issue = create_issue! + child = Issue.new(:project_id => 2, :tracker_id => 1, :author_id => 1, :subject => 'child', :parent_issue_id => issue.id) + assert !child.save + assert_not_nil child.errors.on(:parent_issue_id) + end + + def test_move_a_root_to_child + parent1 = create_issue! + parent2 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + + parent2.parent_issue_id = parent1.id + parent2.save! + child.reload + parent1.reload + parent2.reload + + assert_equal [parent1.id, 1, 6], [parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [parent1.id, 4, 5], [parent2.root_id, parent2.lft, parent2.rgt] + assert_equal [parent1.id, 2, 3], [child.root_id, child.lft, child.rgt] + end + + def test_move_a_child_to_root + parent1 = create_issue! + parent2 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + + child.parent_issue_id = nil + child.save! + child.reload + parent1.reload + parent2.reload + + assert_equal [parent1.id, 1, 2], [parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [parent2.id, 1, 2], [parent2.root_id, parent2.lft, parent2.rgt] + assert_equal [child.id, 1, 2], [child.root_id, child.lft, child.rgt] + end + + def test_move_a_child_to_another_issue + parent1 = create_issue! + parent2 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + + child.parent_issue_id = parent2.id + child.save! + child.reload + parent1.reload + parent2.reload + + assert_equal [parent1.id, 1, 2], [parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [parent2.id, 1, 4], [parent2.root_id, parent2.lft, parent2.rgt] + assert_equal [parent2.id, 2, 3], [child.root_id, child.lft, child.rgt] + end + + def test_move_a_child_with_descendants_to_another_issue + parent1 = create_issue! + parent2 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + grandchild = create_issue!(:parent_issue_id => child.id) + + parent1.reload + parent2.reload + child.reload + grandchild.reload + + assert_equal [parent1.id, 1, 6], [parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [parent2.id, 1, 2], [parent2.root_id, parent2.lft, parent2.rgt] + assert_equal [parent1.id, 2, 5], [child.root_id, child.lft, child.rgt] + assert_equal [parent1.id, 3, 4], [grandchild.root_id, grandchild.lft, grandchild.rgt] + + child.reload.parent_issue_id = parent2.id + child.save! + child.reload + grandchild.reload + parent1.reload + parent2.reload + + assert_equal [parent1.id, 1, 2], [parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [parent2.id, 1, 6], [parent2.root_id, parent2.lft, parent2.rgt] + assert_equal [parent2.id, 2, 5], [child.root_id, child.lft, child.rgt] + assert_equal [parent2.id, 3, 4], [grandchild.root_id, grandchild.lft, grandchild.rgt] + end + + def test_move_a_child_with_descendants_to_another_project + parent1 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + grandchild = create_issue!(:parent_issue_id => child.id) + + assert child.reload.move_to_project(Project.find(2)) + child.reload + grandchild.reload + parent1.reload + + assert_equal [1, parent1.id, 1, 2], [parent1.project_id, parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [2, child.id, 1, 4], [child.project_id, child.root_id, child.lft, child.rgt] + assert_equal [2, child.id, 2, 3], [grandchild.project_id, grandchild.root_id, grandchild.lft, grandchild.rgt] + end + + def test_invalid_move_to_another_project + parent1 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + grandchild = create_issue!(:parent_issue_id => child.id, :tracker_id => 2) + Project.find(2).tracker_ids = [1] + + parent1.reload + assert_equal [1, parent1.id, 1, 6], [parent1.project_id, parent1.root_id, parent1.lft, parent1.rgt] + + # child can not be moved to Project 2 because its child is on a disabled tracker + assert_equal false, Issue.find(child.id).move_to_project(Project.find(2)) + child.reload + grandchild.reload + parent1.reload + + # no change + assert_equal [1, parent1.id, 1, 6], [parent1.project_id, parent1.root_id, parent1.lft, parent1.rgt] + assert_equal [1, parent1.id, 2, 5], [child.project_id, child.root_id, child.lft, child.rgt] + assert_equal [1, parent1.id, 3, 4], [grandchild.project_id, grandchild.root_id, grandchild.lft, grandchild.rgt] + end + + def test_moving_an_issue_to_a_descendant_should_not_validate + parent1 = create_issue! + parent2 = create_issue! + child = create_issue!(:parent_issue_id => parent1.id) + grandchild = create_issue!(:parent_issue_id => child.id) + + child.reload + child.parent_issue_id = grandchild.id + assert !child.save + assert_not_nil child.errors.on(:parent_issue_id) + end + + def test_moving_an_issue_should_keep_valid_relations_only + issue1 = create_issue! + issue2 = create_issue! + issue3 = create_issue!(:parent_issue_id => issue2.id) + issue4 = create_issue! + r1 = IssueRelation.create!(:issue_from => issue1, :issue_to => issue2, :relation_type => IssueRelation::TYPE_PRECEDES) + r2 = IssueRelation.create!(:issue_from => issue1, :issue_to => issue3, :relation_type => IssueRelation::TYPE_PRECEDES) + r3 = IssueRelation.create!(:issue_from => issue2, :issue_to => issue4, :relation_type => IssueRelation::TYPE_PRECEDES) + issue2.reload + issue2.parent_issue_id = issue1.id + issue2.save! + assert !IssueRelation.exists?(r1.id) + assert !IssueRelation.exists?(r2.id) + assert IssueRelation.exists?(r3.id) + end + + def test_destroy_should_destroy_children + issue1 = create_issue! + issue2 = create_issue! + issue3 = create_issue!(:parent_issue_id => issue2.id) + issue4 = create_issue!(:parent_issue_id => issue1.id) + issue2.reload.destroy + issue1.reload + issue4.reload + assert !Issue.exists?(issue2.id) + assert !Issue.exists?(issue3.id) + assert_equal [issue1.id, 1, 4], [issue1.root_id, issue1.lft, issue1.rgt] + assert_equal [issue1.id, 2, 3], [issue4.root_id, issue4.lft, issue4.rgt] + end + + def test_parent_priority_should_be_the_highest_child_priority + parent = create_issue!(:priority => IssuePriority.find_by_name('Normal')) + # Create children + child1 = create_issue!(:priority => IssuePriority.find_by_name('High'), :parent_issue_id => parent.id) + assert_equal 'High', parent.reload.priority.name + child2 = create_issue!(:priority => IssuePriority.find_by_name('Immediate'), :parent_issue_id => child1.id) + assert_equal 'Immediate', child1.reload.priority.name + assert_equal 'Immediate', parent.reload.priority.name + child3 = create_issue!(:priority => IssuePriority.find_by_name('Low'), :parent_issue_id => parent.id) + assert_equal 'Immediate', parent.reload.priority.name + # Destroy a child + child1.destroy + assert_equal 'Low', parent.reload.priority.name + # Update a child + child3.reload.priority = IssuePriority.find_by_name('Normal') + child3.save! + assert_equal 'Normal', parent.reload.priority.name + end + + def test_parent_dates_should_be_lowest_start_and_highest_due_dates + parent = create_issue! + create_issue!(:start_date => '2010-01-25', :due_date => '2010-02-15', :parent_issue_id => parent.id) + create_issue!( :due_date => '2010-02-13', :parent_issue_id => parent.id) + create_issue!(:start_date => '2010-02-01', :due_date => '2010-02-22', :parent_issue_id => parent.id) + parent.reload + assert_equal Date.parse('2010-01-25'), parent.start_date + assert_equal Date.parse('2010-02-22'), parent.due_date + end + + def test_parent_done_ratio_should_be_average_done_ratio_of_leaves + parent = create_issue! + create_issue!(:done_ratio => 20, :parent_issue_id => parent.id) + assert_equal 20, parent.reload.done_ratio + create_issue!(:done_ratio => 70, :parent_issue_id => parent.id) + assert_equal 45, parent.reload.done_ratio + + child = create_issue!(:done_ratio => 0, :parent_issue_id => parent.id) + assert_equal 30, parent.reload.done_ratio + + create_issue!(:done_ratio => 30, :parent_issue_id => child.id) + assert_equal 30, child.reload.done_ratio + assert_equal 40, parent.reload.done_ratio + end + + def test_parent_done_ratio_should_be_weighted_by_estimated_times_if_any + parent = create_issue! + create_issue!(:estimated_hours => 10, :done_ratio => 20, :parent_issue_id => parent.id) + assert_equal 20, parent.reload.done_ratio + create_issue!(:estimated_hours => 20, :done_ratio => 50, :parent_issue_id => parent.id) + assert_equal (50 * 20 + 20 * 10) / 30, parent.reload.done_ratio + end + + def test_parent_estimate_should_be_sum_of_leaves + parent = create_issue! + create_issue!(:estimated_hours => nil, :parent_issue_id => parent.id) + assert_equal nil, parent.reload.estimated_hours + create_issue!(:estimated_hours => 5, :parent_issue_id => parent.id) + assert_equal 5, parent.reload.estimated_hours + create_issue!(:estimated_hours => 7, :parent_issue_id => parent.id) + assert_equal 12, parent.reload.estimated_hours + end + + def test_project_copy_should_copy_issue_tree + p = Project.create!(:name => 'Tree copy', :identifier => 'tree-copy', :tracker_ids => [1, 2]) + i1 = create_issue!(:project_id => p.id, :subject => 'i1') + i2 = create_issue!(:project_id => p.id, :subject => 'i2', :parent_issue_id => i1.id) + i3 = create_issue!(:project_id => p.id, :subject => 'i3', :parent_issue_id => i1.id) + i4 = create_issue!(:project_id => p.id, :subject => 'i4', :parent_issue_id => i2.id) + i5 = create_issue!(:project_id => p.id, :subject => 'i5') + c = Project.new(:name => 'Copy', :identifier => 'copy', :tracker_ids => [1, 2]) + c.copy(p, :only => 'issues') + c.reload + + assert_equal 5, c.issues.count + ic1, ic2, ic3, ic4, ic5 = c.issues.find(:all, :order => 'subject') + assert ic1.root? + assert_equal ic1, ic2.parent + assert_equal ic1, ic3.parent + assert_equal ic2, ic4.parent + assert ic5.root? + end + + # Helper that creates an issue with default attributes + def create_issue!(attributes={}) + Issue.create!({:project_id => 1, :tracker_id => 1, :author_id => 1, :subject => 'test'}.merge(attributes)) + end +end diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index cf5369726..de53125f5 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -329,7 +329,7 @@ class IssueTest < ActiveSupport::TestCase def test_move_to_another_project_with_same_category issue = Issue.find(1) - assert issue.move_to(Project.find(2)) + assert issue.move_to_project(Project.find(2)) issue.reload assert_equal 2, issue.project_id # Category changes @@ -340,7 +340,7 @@ class IssueTest < ActiveSupport::TestCase def test_move_to_another_project_without_same_category issue = Issue.find(2) - assert issue.move_to(Project.find(2)) + assert issue.move_to_project(Project.find(2)) issue.reload assert_equal 2, issue.project_id # Category cleared @@ -350,7 +350,7 @@ class IssueTest < ActiveSupport::TestCase def test_move_to_another_project_should_clear_fixed_version_when_not_shared issue = Issue.find(1) issue.update_attribute(:fixed_version_id, 1) - assert issue.move_to(Project.find(2)) + assert issue.move_to_project(Project.find(2)) issue.reload assert_equal 2, issue.project_id # Cleared fixed_version @@ -360,7 +360,7 @@ class IssueTest < ActiveSupport::TestCase def test_move_to_another_project_should_keep_fixed_version_when_shared_with_the_target_project issue = Issue.find(1) issue.update_attribute(:fixed_version_id, 4) - assert issue.move_to(Project.find(5)) + assert issue.move_to_project(Project.find(5)) issue.reload assert_equal 5, issue.project_id # Keep fixed_version @@ -370,7 +370,7 @@ class IssueTest < ActiveSupport::TestCase def test_move_to_another_project_should_clear_fixed_version_when_not_shared_with_the_target_project issue = Issue.find(1) issue.update_attribute(:fixed_version_id, 1) - assert issue.move_to(Project.find(5)) + assert issue.move_to_project(Project.find(5)) issue.reload assert_equal 5, issue.project_id # Cleared fixed_version @@ -380,7 +380,7 @@ class IssueTest < ActiveSupport::TestCase def test_move_to_another_project_should_keep_fixed_version_when_shared_systemwide issue = Issue.find(1) issue.update_attribute(:fixed_version_id, 7) - assert issue.move_to(Project.find(2)) + assert issue.move_to_project(Project.find(2)) issue.reload assert_equal 2, issue.project_id # Keep fixed_version @@ -392,7 +392,7 @@ class IssueTest < ActiveSupport::TestCase target = Project.find(2) target.tracker_ids = [3] target.save - assert_equal false, issue.move_to(target) + assert_equal false, issue.move_to_project(target) issue.reload assert_equal 1, issue.project_id end @@ -401,7 +401,7 @@ class IssueTest < ActiveSupport::TestCase issue = Issue.find(1) copy = nil assert_difference 'Issue.count' do - copy = issue.move_to(issue.project, nil, :copy => true) + copy = issue.move_to_project(issue.project, nil, :copy => true) end assert_kind_of Issue, copy assert_equal issue.project, copy.project @@ -412,8 +412,9 @@ class IssueTest < ActiveSupport::TestCase issue = Issue.find(1) copy = nil assert_difference 'Issue.count' do - copy = issue.move_to(Project.find(3), Tracker.find(2), :copy => true) + copy = issue.move_to_project(Project.find(3), Tracker.find(2), :copy => true) end + copy.reload assert_kind_of Issue, copy assert_equal Project.find(3), copy.project assert_equal Tracker.find(2), copy.tracker @@ -421,7 +422,7 @@ class IssueTest < ActiveSupport::TestCase assert_nil copy.custom_value_for(2) end - context "#move_to" do + context "#move_to_project" do context "as a copy" do setup do @issue = Issue.find(1) @@ -429,24 +430,24 @@ class IssueTest < ActiveSupport::TestCase end should "allow assigned_to changes" do - @copy = @issue.move_to(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:assigned_to_id => 3}}) + @copy = @issue.move_to_project(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:assigned_to_id => 3}}) assert_equal 3, @copy.assigned_to_id end should "allow status changes" do - @copy = @issue.move_to(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:status_id => 2}}) + @copy = @issue.move_to_project(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:status_id => 2}}) assert_equal 2, @copy.status_id end should "allow start date changes" do date = Date.today - @copy = @issue.move_to(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:start_date => date}}) + @copy = @issue.move_to_project(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:start_date => date}}) assert_equal date, @copy.start_date end should "allow due date changes" do date = Date.today - @copy = @issue.move_to(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:due_date => date}}) + @copy = @issue.move_to_project(Project.find(3), Tracker.find(2), {:copy => true, :attributes => {:due_date => date}}) assert_equal date, @copy.due_date end @@ -457,7 +458,7 @@ class IssueTest < ActiveSupport::TestCase issue = Issue.find(12) assert issue.recipients.include?(issue.author.mail) # move the issue to a private project - copy = issue.move_to(Project.find(5), Tracker.find(2), :copy => true) + copy = issue.move_to_project(Project.find(5), Tracker.find(2), :copy => true) # author is not a member of project anymore assert !copy.recipients.include?(copy.author.mail) end diff --git a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb index a5598dbd2..01a61bf48 100644 --- a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb +++ b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb @@ -77,6 +77,13 @@ module Redmine @custom_field_values = nil end + def reset_custom_values! + @custom_field_values = nil + @custom_field_values_changed = true + values = custom_values.inject({}) {|h,v| h[v.custom_field_id] = v.value; h} + custom_values.each {|cv| cv.destroy unless custom_field_values.include?(cv)} + end + module ClassMethods end end diff --git a/vendor/plugins/awesome_nested_set/lib/awesome_nested_set.rb b/vendor/plugins/awesome_nested_set/lib/awesome_nested_set.rb index 6be3c5e42..9c4ee0f22 100644 --- a/vendor/plugins/awesome_nested_set/lib/awesome_nested_set.rb +++ b/vendor/plugins/awesome_nested_set/lib/awesome_nested_set.rb @@ -256,7 +256,7 @@ module CollectiveIdea #:nodoc: end def leaf? - right - left == 1 + new_record? || (right - left == 1) end # Returns true is this is a child node -- 2.39.5