Skip to content

Commit

Permalink
Stories no longer need to be top-level issues
Browse files Browse the repository at this point in the history
  • Loading branch information
friflaj committed May 9, 2011
1 parent dde73ff commit d08928c
Show file tree
Hide file tree
Showing 22 changed files with 125 additions and 121 deletions.
13 changes: 10 additions & 3 deletions app/controllers/rb_taskboards_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ class RbTaskboardsController < RbApplicationController
def show
@statuses = Tracker.find_by_id(Task.tracker).issue_statuses
@story_ids = @sprint.stories.map{|s| s.id}
@last_updated = Task.find(:first,
:conditions => ["parent_id in (?)", @story_ids],
:order => "updated_on DESC")

if @sprint.stories.size == 0
@last_updated = nil
else
@last_updated = Task.find(:first,
:conditions => ['tracker_id = ? and fixed_version_id = ?',
Task.tracker, @sprint.stories[0].fixed_version_id],
:order => "updated_on DESC")
end

respond_to do |format|
format.html { render :layout => "rb" }
end
Expand Down
10 changes: 5 additions & 5 deletions app/models/story.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,13 @@ class Story < Issue
def self.condition(project_id, sprint_id, extras=[])
if sprint_id.nil?
c = ["
parent_id is NULL
and project_id = ?
project_id = ?
and tracker_id in (?)
and fixed_version_id is NULL
and is_closed = ?", project_id, Story.trackers, false]
else
c = ["
parent_id is NULL
and project_id = ?
project_id = ?
and tracker_id in (?)
and fixed_version_id = ?",
project_id, Story.trackers, sprint_id]
Expand Down Expand Up @@ -69,10 +67,12 @@ def self.find_all_updated_since(since, project_id)
:order => "updated_on ASC")
end

def self.trackers
def self.trackers(type = :array)
trackers = Setting.plugin_redmine_backlogs[:story_trackers]
return [] if trackers.blank?

return trackers.join(',') if type == :string

return trackers.map { |tracker| Integer(tracker) }
end

Expand Down
23 changes: 14 additions & 9 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def self.create_with_relationships(params, user_id, project_id, is_impediment =
end

# TODO: there's an assumption here that impediments always have the
# task-tracker as their tracker.
# task-tracker as their tracker, and are top-level issues.
def self.find_all_updated_since(since, project_id, find_impediments = false)
find(:all,
:conditions => ["project_id = ? AND updated_on > ? AND tracker_id in (?) and parent_id IS #{ find_impediments ? '' : 'NOT' } NULL", project_id, Time.parse(since), tracker],
Expand All @@ -46,13 +46,13 @@ def self.find_all_updated_since(since, project_id, find_impediments = false)

def self.tasks_for(story_id)
tasks = []
Task.find(:all,
:conditions => ['tracker_id = ? and not parent_id is NULL and root_id = ?', Task.tracker, story_id],
:order => :lft
).each_with_index {|task, i|
task.rank = i + 1
tasks << task
}
story = Story.find_by_id(story_id)
if Story.trackers.include?(story.tracker_id)
story.descendants.each_with_index {|task, i|
task.rank = i + 1
tasks << task
}
end
return tasks
end

Expand Down Expand Up @@ -121,7 +121,12 @@ def rank=(r)
end

def rank
@rank ||= Issue.count(:conditions => ['tracker_id = ? and not parent_id is NULL and root_id = ? and lft <= ?', Task.tracker, story_id, self.lft])
s = self.story
return nil if !s

@rank ||= Issue.count(
:conditions => ['tracker_id = ? and root_id = ? and lft > ? and lft <= ?',
Task.tracker, s.root_id, s.lft, self.lft])
return @rank
end
end
3 changes: 3 additions & 0 deletions app/views/rb_impediments/_impediment.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
<div class="remaining_hours editable" fieldname="remaining_hours"><%= remaining_hours(impediment) %></div>
<div class="indicator"> </div>
<div class="meta">
<!-- friflaj:
TODO: I don't get this -- impediments aren't tied to a story, and Task explicitly assumes they're top-level issues
-->
<div class="story_id"><%= impediment.parent_id %></div>
<div class="status_id"><%= impediment.status_id %></div>
<%= render :partial => "shared/model_errors", :object => impediment.errors %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/rb_tasks/_task.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<div class="remaining_hours editable" fieldname="remaining_hours"><%= remaining_hours(task) %></div>
<div class="indicator"> </div>
<div class="meta">
<div class="story_id"><%= task.parent_id %></div>
<div class="story_id"><%= task.story ? task.story.id : '' %></div>
<div class="status_id"><%= task.status_id %></div>
<div class="previous"><%= task.right_sibling.blank? ? '' : task.right_sibling.id %></div>
<%= render :partial => "shared/model_errors", :object => task.errors %>
Expand Down
4 changes: 2 additions & 2 deletions db/migrate/013_fill_position.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
class FillPosition < ActiveRecord::Migration
def self.up
pos = execute "select project_id, max(position) from issues where parent_id is null group by project_id"
pos = execute "select project_id, max(position) from issues where tracker_id in (#{Story.trackers(:string)}) group by project_id"
pos.each do |row|
project_id = row[0].to_i
position = row[1].to_i

Story.find(:all, :conditions => ["project_id = ? and parent_id is null and position is null", project_id], :order => "created_on").each do |story|
Story.find(:all, :conditions => ["project_id = ? and tracker_id in (#{Story.trackers(:string)}) and position is null", project_id], :order => "created_on").each do |story|
position += 1

story.position = position
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/014_fix_positions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def self.up
end

ActiveRecord::Base.transaction do
Story.find(:all, :conditions => "parent_id is NULL", :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
Story.find(:all, :conditions => ["tracker_id in (?)", Story.trackers], :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
s.position=i+1
s.save!
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/015_order_tasks_using_tree.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class OrderTasksUsingTree < ActiveRecord::Migration
def self.up
last_task = {}
ActiveRecord::Base.transaction do
Task.find(:all, :conditions => "not parent_id is NULL", :order => "project_id ASC, parent_id ASC, position ASC").each do |t|
Task.find(:all, :conditions => ["tracker_id = ?", Task.tracker], :order => "project_id ASC, parent_id ASC, position ASC").each do |t|
begin
t.move_after last_task[t.parent_id] if last_task[t.parent_id]
rescue
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/016_remove_task_position.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def self.up
# this intentionally loads tasks as stories so we can issue
# remove_from_list, which does more than just nilling the
# position
Story.find(:all, :conditions => "id <> root_id and not position is null").each do |t|
Story.find(:all, :conditions => "tracker_id = #{Task.tracker}").each do |t|
t.remove_from_list
end
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/018_fix_story_positions_again.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class FixStoryPositionsAgain < ActiveRecord::Migration
def self.up
ActiveRecord::Base.transaction do
Story.find(:all, :conditions => "parent_id is NULL", :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
Story.find(:all, :conditions => ["tracker_id = ?", Task.tracker], :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
s.position=i+1
s.save!
end
Expand Down
27 changes: 20 additions & 7 deletions db/migrate/020_flatten_story_positions.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
class FlattenStoryPositions < ActiveRecord::Migration
def self.up
# stupid "Attempted to update a stale object" errors
# Rails doesn't support temp tables, mysql doesn't support update
# from same-table subselect

# this makes sure everything has an unique id that won't conflict
# with what we're about to do next
execute "update issues set position = -id"

Story.find(:all, :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
execute "update issues set position=#{i+1} where id=#{s.id}"
create_table :backlogs_tmp_issue_position do |t|
t.column :id, :integer, :null => false
t.column :position, :integer, :null => false
end

execute "insert into backlogs_tmp_issue_position
select story.id, count(*) + 1
from issues story
join issues pred on
(pred.project_id < story.project_id)
or
(pred.project_id = story.project_id and pred.fixed_version_id < story.fixed_version_id)
or
(pred.project_id = story.project_id and pred.fixed_version_id = story.fixed_version_id and pred.position < story.position)
group by story.id"

execute "update issues set position = (select position from backlogs_tmp_issue_position where backlogs_tmp_issue_position.id = issues.id)"

drop_table :backlogs_tmp_issue_position
end

def self.down
Expand Down
1 change: 1 addition & 0 deletions features/step_definitions/_given_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@

Given /^I have selected card label stock (.+)$/ do |stock|
Setting.plugin_redmine_backlogs = Setting.plugin_redmine_backlogs.merge( {:card_spec => stock } )
Cards::TaskboardCards.selected_label.should_not be_nil
end

Given /^I have set my API access key$/ do
Expand Down
4 changes: 2 additions & 2 deletions features/step_definitions/_then_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@
Then /^the story named (.+) should have (\d+) task named (.+)$/ do |story_subject, count, task_subject|
stories = Story.find(:all, :conditions => { :subject => story_subject })
stories.length.should == 1
tasks = Task.find(:all, :conditions => { :parent_id => stories.first.id })

tasks = stories.first.descendants
tasks.length.should == 1

tasks.first.subject.should == task_subject
Expand Down
8 changes: 4 additions & 4 deletions features/step_definitions/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,31 +40,31 @@ def login_as_product_owner
visit url_for(:controller => 'account', :action=>'login')
fill_in 'username', :with => 'jsmith'
fill_in 'password', :with => 'jsmith'
click_button 'Login »'
page.find(:xpath, '//input[@name="login"]').click
@user = User.find(:first, :conditions => "login='jsmith'")
end

def login_as_scrum_master
visit url_for(:controller => 'account', :action=>'login')
fill_in 'username', :with => 'jsmith'
fill_in 'password', :with => 'jsmith'
click_button 'Login »'
page.find(:xpath, '//input[@name="login"]').click
@user = User.find(:first, :conditions => "login='jsmith'")
end

def login_as_team_member
visit url_for(:controller => 'account', :action=>'login')
fill_in 'username', :with => 'jsmith'
fill_in 'password', :with => 'jsmith'
click_button 'Login »'
page.find(:xpath, '//input[@name="login"]').click
@user = User.find(:first, :conditions => "login='jsmith'")
end

def login_as_admin
visit url_for(:controller => 'account', :action=>'login')
fill_in 'username', :with => 'admin'
fill_in 'password', :with => 'admin'
click_button 'Login »'
page.find(:xpath, '//input[@name="login"]').click
@user = User.find(:first, :conditions => "login='admin'")
end

Expand Down
2 changes: 1 addition & 1 deletion features/team_member.feature
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Feature: Team Member
And I set the subject of the task to A Whole New Task
When I create the task
Then the request should complete successfully
And the 1st task for Story 1 should be A Whole New Task
And the 2nd task for Story 1 should be A Whole New Task

Scenario: Update a task for a story
Given I am viewing the taskboard for Sprint 001
Expand Down
12 changes: 8 additions & 4 deletions lib/backlogs_issue_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,20 @@ def journalized_update_attribute(attrib, v)
end

def is_story?
return (Story.trackers.include?(self.tracker_id) and self.root?)
return Story.trackers.include?(self.tracker_id)
end

def is_task?
return (self.parent_id && self.tracker_id == Task.tracker)
end

def story
return Issue.find(:first,
:conditions => [ "id = ? and tracker_id in (?)", self.root_id, Story.trackers ])
# the self.id test verifies we're not looking at a new,
# unsaved issue object
return nil unless self.id && self.is_task?

return Issue.find(:first, :order => 'lft DESC',
:conditions => [ "root_id = ? and lft < ? and tracker_id in (?)", self.root_id, self.lft, Story.trackers ])
end

def blocks
Expand Down Expand Up @@ -144,7 +148,7 @@ def backlogs_after_save
connection.execute "update issues set tracker_id = #{connection.quote(Task.tracker)}, fixed_version_id = #{connection.quote(story.fixed_version_id)} where id = #{connection.quote(self.id)}"
end

touched_sprints = [self.root_id, self.root_id_was].compact.uniq.collect{|s| Story.find(s).fixed_version}.compact
touched_sprints = [self.parent_id, self.parent_id_was].compact.uniq.collect{|t| Task.find(t).story }.compact.uniq.collect{|s| s.fixed_version}.compact
end
end

Expand Down
5 changes: 2 additions & 3 deletions lib/backlogs_project_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ def scrum_statistics
@scrum_statistics[:error, :product_backlog, :is_empty] = (self.status == Project::STATUS_ACTIVE && backlog.length == 0)
@scrum_statistics[:error, :product_backlog, :unsized] = backlog.inject(false) {|unsized, story| unsized || story.story_points.blank? }

@scrum_statistics[:error, :sprint, :unsized] = Issue.exists?(["story_points is null and parent_id is null and fixed_version_id in (?) and tracker_id in (?)", all_sprints.collect{|s| s.id}, Story.trackers])
@scrum_statistics[:error, :sprint, :unestimated] = Issue.exists?(["estimated_hours is null and not parent_id is null and fixed_version_id in (?) and tracker_id = ?", all_sprints.collect{|s| s.id}, Task.tracker])
@scrum_statistics[:error, :sprint, :unsized] = Issue.exists?(["story_points is null and fixed_version_id in (?) and tracker_id in (?)", all_sprints.collect{|s| s.id}, Story.trackers])
@scrum_statistics[:error, :sprint, :unestimated] = Issue.exists?(["estimated_hours is null and fixed_version_id in (?) and tracker_id = ?", all_sprints.collect{|s| s.id}, Task.tracker])
@scrum_statistics[:error, :sprint, :notes_missing] = closed_sprints.inject(false){|missing, sprint| missing || !sprint.has_wiki_page}

@scrum_statistics[:error, :inactive] = (self.status == Project::STATUS_ACTIVE && !(active_sprint && active_sprint.activity))
Expand Down Expand Up @@ -191,7 +191,6 @@ def scrum_statistics
and not (estimated_hours is null or estimated_hours = 0)
and fixed_version_id in (#{sprint_ids})
and project_id = #{self.id}
and not parent_id is null
and tracker_id in (#{story_trackers})
"

Expand Down
35 changes: 22 additions & 13 deletions lib/backlogs_query_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,35 @@ def self.included(base) # :nodoc:
base.add_available_column(QueryColumn.new(:remaining_hours, :sortable => "#{Issue.table_name}.remaining_hours"))
base.add_available_column(QueryColumn.new(:velocity_based_estimate))

# couldn't get HAVING to work, so a subselect will have to
# do
story_sql = "from issues story
where
story.root_id = issues.root_id
and story.lft in (
select max(story_lft.lft)
from issues story_lft
where story_lft.root_id = issues.root_id
and story_lft.tracker_id in (#{Story.trackers(:string)})
and issues.lft >= story_lft.lft and issues.rgt <= story_lft.rgt
)"

base.add_available_column(QueryColumn.new(:position,
:sortable => [
# sprint startdate
"coalesce((select sprint_start_date from versions where versions.id = issues.fixed_version_id), '1900-01-01')",

# sprint name, in case start dates are the same
"(select name from versions where versions.id = issues.fixed_version_id)",
# sprint id, in case start dates are the same
"(select id from versions where versions.id = issues.fixed_version_id)",

# make sure stories with NULL
# position sort-last
"(select case when root.position is null then 1 else 0 end from issues root where issues.root_id = root.id)",
# make sure stories with NULL position sort-last
"(select case when story.position is null then 1 else 0 end #{story_sql})",

# story position
"(select root.position from issues root where issues.root_id = root.id)",
"(select story.position #{story_sql})",

# story ID, in case positions
# are the same (SHOULD NOT HAPPEN!).
# DO NOT CHANGE; root_id is the only field that sorts the same for stories _and_
# the tasks it has.
"issues.root_id",
# story ID, in case story positions are the same (SHOULD NOT HAPPEN!).
"(select story.id #{story_sql})",

# order in task tree
"issues.lft"
Expand Down Expand Up @@ -78,9 +87,9 @@ def sql_for_field_with_backlogs_issue_type(field, operator, v, db_table, db_fiel
selected_values.each { |val|
case val
when "story"
sql << "(#{db_table}.tracker_id in (#{story_trackers}) and #{db_table}.parent_id is NULL)"
sql << "(#{db_table}.tracker_id in (#{story_trackers}))"
when "task"
sql << "(#{db_table}.tracker_id = #{Task.tracker} and not #{db_table}.parent_id is NULL)"
sql << "(#{db_table}.tracker_id = #{Task.tracker})"
when "impediment"
sql << "(#{db_table}.id in (
select issue_from_id
Expand Down
2 changes: 1 addition & 1 deletion lib/cards.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def card(issue, type)

@pdf.start_new_page if row == 1 and col == 1 and @cards != 1

parent_story = issue.story
parent_story = (type == :task ? issue.story : issue)

# card bounds
@pdf.bounding_box self.top_left(row, col), :width => @width, :height => @height do
Expand Down
4 changes: 2 additions & 2 deletions lib/tasks/fix_positions.rake
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ namespace :redmine do
namespace :backlogs do
desc "Remove duplicate positions in the issues table"
task :fix_positions => :environment do
Story.find(:all, :conditions => "parent_id is NULL", :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
Story.find(:all, :conditions => ["tracker_id in (?)", Story.trackers], :order => "project_id ASC, fixed_version_id ASC, position ASC").each_with_index do |s,i|
s.position=i+1
s.save!
end
end
end
end
end
Loading

0 comments on commit d08928c

Please sign in to comment.