[PATCH] Development team provides backlog item estimates. #166
by Darryl L. Pierce
This patch requires a migration.
Added a new column. estimated_hours, to the backlog_item_votes table.
Removed the estimated_hours column from the backlog_items table. The
estimated hours will now be the largest hours estimate provided by the
development team.
Removed the estimated hours column from the sprint planning page. Also
dropped the votes column since it makes no sense to have that when items
are being added to a sprint.
Added a new column on the vote page to take the user's estimated hours.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/controllers/sprints_controller.rb | 90 +++++++++++++++-----------
app/models/backlog_item.rb | 21 +++---
app/models/backlog_item_vote.rb | 4 +
app/models/sprint.rb | 6 +-
app/views/sprints/plan.html.erb | 23 ++-----
app/views/sprints/show.html.erb | 2 +-
app/views/sprints/vote.html.erb | 4 +-
db/migrate/013_create_backlog_items.rb | 2 +-
db/migrate/024_create_backlog_item_votes.rb | 1 +
doc/ChangeLog | 1 +
test/fixtures/backlog_item_votes.yml | 2 +-
test/fixtures/backlog_items.yml | 12 ++--
test/fixtures/sprint_members.yml | 8 ++-
test/functional/items_controller_test.rb | 3 +-
test/functional/sprints_controller_test.rb | 94 ++++++++++++++++-----------
test/unit/backlog_item_test.rb | 16 ++---
test/unit/backlog_item_vote_test.rb | 13 +++-
17 files changed, 168 insertions(+), 134 deletions(-)
diff --git a/app/controllers/sprints_controller.rb b/app/controllers/sprints_controller.rb
index c88fc87..4c3d739 100644
--- a/app/controllers/sprints_controller.rb
+++ b/app/controllers/sprints_controller.rb
@@ -17,19 +17,18 @@
# +SprintsController+ handles performing CRUD operations on instances of
# +Sprint+.
class SprintsController < ApplicationController
- before_filter :authenticated, :except => [:index, :show, :members]
- before_filter :load_product, :only => [:new, :create]
- before_filter :load_sprint, :except => [:index, :new, :create]
- before_filter :verify_can_edit, :only => [:edit, :update]
- before_filter :verify_sprint_is_pending, :only => [:plan, :populate]
- before_filter :verify_can_change_state, :only => [:planning, :start, :complete, :cancel]
- before_filter :verify_can_vote, :only => [:vote, :cast_votes]
- before_filter :verify_planning, :only => [:vote, :cast_votes]
- before_filter :verify_cast_votes, :only => [:cast_votes]
- before_filter :load_user_votes, :only => [:vote, :cast_votes]
- before_filter :prepare_for_edit, :only => [:new, :create, :edit, :update, :plan, :populate]
- before_filter :path_to_list, :only => [:index, :new, :create]
- before_filter :path_to_one, :only => [:show, :edit, :update, :plan, :populate, :vote, :members]
+ before_filter :authenticated, :except => [:index, :show, :members]
+ before_filter :load_product, :only => [:new, :create]
+ before_filter :load_sprint, :except => [:index, :new, :create]
+ before_filter :verify_proposed, :only => [:plan, :populate, :vote, :cast_votes]
+ before_filter :verify_can_edit, :only => [:edit, :update]
+ before_filter :verify_can_change_state, :only => [:planning, :start, :complete, :cancel]
+ before_filter :verify_can_vote, :only => [:vote, :cast_votes]
+ before_filter :extract_votes_and_estimates, :only => [:vote]
+ before_filter :receive_votes_and_estimates, :only => [:cast_votes]
+ before_filter :prepare_for_edit, :only => [:new, :create, :edit, :update, :plan, :populate]
+ before_filter :path_to_list, :only => [:index, :new, :create]
+ before_filter :path_to_one, :only => [:show, :edit, :update, :plan, :populate, :vote, :members]
# GET /sprints
def index
@@ -174,11 +173,11 @@ class SprintsController < ApplicationController
def cast_votes
BacklogItemVote.transaction do
BacklogItemVote.for_sprint(@sprint).for_user((a)user).destroy_all
- @votes = params[:votes]
- @votes.each_pair do |item_id, vote|
- BacklogItemVote.create(:backlog_item_id => item_id, :user => @user, :vote => vote)
+ @user_votes.each do |vote|
+ vote.save!
end
respond_to do |format|
+ flash[:message] = "Your votes have been cast."
format.html {redirect_to sprint_path(@sprint)}
end
end
@@ -248,13 +247,12 @@ class SprintsController < ApplicationController
@title = "Sprint #{(a)sprint.id} (Planning)"
@user_stories = UserStory.find_all_by_product_id_and_closed((a)product.id, false)
@selected = []
- @estimates = {}
respond_to do |format|
format.html
end
else
- report_error "You are not allowed to plan this sprint."
+ report_error "You are not allowed to plan this sprint.", sprint_path(@sprint)
end
end
@@ -264,17 +262,12 @@ class SprintsController < ApplicationController
if @sprint.can_populate?(@user)
@sprint.backlog_items.clear
- @estimates = params[:estimates]
@selected = params[:selected]
if @selected
@selected.each do |selected|
- estimate = @estimates["'#{selected}'"]
- if estimate
- @sprint.backlog_items << BacklogItem.new(:sprint_id => @sprint.id,
- :user_story_id => selected,
- :estimated_hours => estimate)
- end
+ @sprint.backlog_items << BacklogItem.new(:sprint_id => @sprint.id,
+ :user_story_id => selected)
end
end
@@ -299,7 +292,7 @@ class SprintsController < ApplicationController
end
end
else
- report_error "You are not allowed to populate this sprint."
+ report_error "You are not allowed to populate this sprint.", sprint_path(@sprint)
end
end
@@ -330,10 +323,6 @@ class SprintsController < ApplicationController
report_error "You are not allowed to edit this sprint." unless @sprint.can_edit?(@user)
end
- def verify_sprint_is_pending
- report_error "This can only be done for a pending sprint." unless @sprint.planning?
- end
-
def verify_can_change_state
report_error "You are not allowed to change this sprint's state." unless @sprint.can_change_state?(@user)
end
@@ -342,25 +331,52 @@ class SprintsController < ApplicationController
report_error("You are not allowed to vote on this sprint.", sprint_path(@sprint)) unless @sprint.is_team_member?(@user)
end
- def verify_planning
- report_error("You can only vote on items while a sprint is in planning.", sprint_path(@sprint)) unless @sprint.planning?
+ def verify_proposed
+ report_error("You can only vote on items while a sprint is in planning.", sprint_path(@sprint)) unless @sprint.proposed?
end
def verify_cast_votes
params[:votes].each_pair do |item_id, vote|
item = BacklogItem.find_by_id(item_id)
if !item
- report_error("A vote was cast for a nonexistent backlog item.")
+ report_error("A vote was cast for a nonexistent backlog item.", sprint_path(@sprint))
elsif item.sprint_id != @sprint.id then
- report_error("The backlog item \"#{item.user_story.title}\" is not on this sprint.")
+ report_error("The backlog item \"#{item.user_story.title}\" is not on this sprint.", sprint_path(@sprint))
end
end
end
- def load_user_votes
+ def extract_votes_and_estimates
@votes = {}
- @sprint.backlog_items.each {|item| @votes[item.id] = BacklogItemVote::ABSTAIN}
- BacklogItemVote.for_sprint(@sprint).for_user((a)user).each {|vote| @votes[vote.backlog_item.id] = vote.vote}
+ @estimates = {}
+ @sprint.backlog_items.each do |item|
+ user_vote = BacklogItemVote.for_sprint(@sprint).for_user(@user).find(:first)
+
+ if user_vote
+ @votes[item.id] = user_vote.vote
+ @estimates[item.id] = user_vote.estimated_hours
+ else
+ @votes[item.id] = BacklogItemVote::ABSTAIN
+ @estimates[item.id] = 0.0
+ end
+ end
+ end
+
+ def receive_votes_and_estimates
+ @votes = params[:votes] || {}
+ @estimates = params[:estimates] || {}
+ @user_votes = []
+
+ # ensures that all votes are for valid backlog items
+ @votes.each_pair do |item_id, vote|
+ item = BacklogItem.find_by_id(item_id)
+
+ report_error("Invalid backlog item: id=#{item_id}") unless item
+ report_error("You cannot vote on an item on a different sprint.") unless (item.sprint_id == @sprint.id)
+ report_error("All accepted items must have an estimate.") if (vote == BacklogItemVote::ACK) && (@estimates[item_id] == nil)
+
+ @user_votes << BacklogItemVote.new(:user => @user, :backlog_item => item, :vote => vote, :estimated_hours => @estimates[item_id])
+ end
end
def prepare_for_edit
diff --git a/app/models/backlog_item.rb b/app/models/backlog_item.rb
index 0c41ff7..b5de6cf 100644
--- a/app/models/backlog_item.rb
+++ b/app/models/backlog_item.rb
@@ -18,15 +18,8 @@
# A +BacklogItem+ associates a +UserStory+ to a +Sprint+.
#
class BacklogItem < ActiveRecord::Base
- validates_presence_of :sprint_id,
- :message => 'You need to specify the sprint.'
-
- validates_presence_of :user_story_id,
- :message => 'You need to specify a user story.'
-
- validates_numericality_of :estimated_hours,
- :greater_than => 0,
- :message => 'Estimated hours must be numeric and greater than 0.'
+ validates_presence_of :sprint_id, :message => 'You need to specify the sprint.'
+ validates_presence_of :user_story_id, :message => 'You need to specify a user story.'
belongs_to :sprint
belongs_to :user_story
@@ -35,7 +28,7 @@ class BacklogItem < ActiveRecord::Base
has_many :tasks, :dependent => :destroy
has_many :remaining_hours_estimates
- has_many :votes, :class_name => 'BacklogItemVote'
+ has_many :votes, :class_name => 'BacklogItemVote', :order => 'estimated_hours DESC'
has_many :votes_in_favor, :class_name => 'BacklogItemVote', :conditions => "vote = #{BacklogItemVote::ACK}"
has_and_belongs_to_many :messages, :join_table => :backlog_items_messages
@@ -106,6 +99,14 @@ class BacklogItem < ActiveRecord::Base
STATE_TEXT[state]
end
+ # Estimated hours is the value for the highest estimate provided by
+ # the development team.
+ def estimated_hours
+ return 0 if votes.empty?
+
+ return votes.first.estimated_hours
+ end
+
def actual_hours
result = 0.0
diff --git a/app/models/backlog_item_vote.rb b/app/models/backlog_item_vote.rb
index 59a26bf..7ecd5e5 100644
--- a/app/models/backlog_item_vote.rb
+++ b/app/models/backlog_item_vote.rb
@@ -37,4 +37,8 @@ class BacklogItemVote < ActiveRecord::Base
NAK = -1
VOTES = {"Abstain" => ABSTAIN, "Accept" => ACK, "Reject" => NAK}
+
+ def validate
+ errors.add("estimated_hours", "An accepted item must have an hours estimate.") if (vote == ACK) && (estimated_hours <= 0)
+ end
end
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index 239102f..9660830 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -88,7 +88,7 @@ class Sprint < ActiveRecord::Base
named_scope :planned, {:conditions => ['state = ?', STATE_PLANNING]}
named_scope :active, {:conditions => ['state = ?', STATE_ACTIVE]}
- state_machine :initial => :planning do
+ state_machine :initial => :proposed do
state :proposed, :value => STATE_PROPOSED
state :planning, :value => STATE_PLANNING
state :active, :value => STATE_ACTIVE
@@ -180,12 +180,12 @@ class Sprint < ActiveRecord::Base
# Returns whether the specified user is allowed to populate this sprint.
def can_populate?(user)
- planning? && (is_product_owner(user) || is_team_lead?(user))
+ proposed? && (is_product_owner(user) || is_team_lead?(user))
end
# Returns whether a burndown chart can be viewed for this sprint.
def can_view_burndown?
- !planning?
+ !proposed? && !planning?
end
# Rethers whether the sprint can have backlog items added, and whether the
diff --git a/app/views/sprints/plan.html.erb b/app/views/sprints/plan.html.erb
index 4e8e20f..9e9c247 100644
--- a/app/views/sprints/plan.html.erb
+++ b/app/views/sprints/plan.html.erb
@@ -5,38 +5,27 @@
<caption><%= "Planning For: #{(a)sprint.title}" %><?caption>
<thead>
<tr>
+ <th scope="col"></th>
<th scope="col">#</th>
<th scope="col">Rank</th>
<th scope="col" class="Name">Title</th>
- <th scope="col">Estimate</th>
- <th scope="col">Votes</th>
</tr>
</thead>
<tbody>
<% @user_stories.each do |story| %>
<% backlog_item = @sprint.backlog_items.find_by_user_story_id(story.id) %>
<tr class="<%= cycle('odd', 'even') %>">
+ <td>
+ <%= check_box_tag "selected[]", story.id,
+ (backlog_item ||
+ (params[:selected].include?(story.id.to_s) if params[:selected])) %>
+ </td>
<td><%= story.id %></td>
<td><%= story.priority %></td>
<td class="name">
<%= link_to story.title, story_path(story), :target => "_other" %>
<%= RedCloth.new(get_first_sentence(story.description)).to_html %>
</td>
- <td>
- <%= check_box_tag "selected[]", story.id,
- (backlog_item ||
- (params[:selected].include?(story.id.to_s) if params[:selected])) %>
- <%= text_field_tag "estimates['#{story.id}']",
- (params[:estimates]["'#{story.id.to_s}'"] if params[:estimates]) ||
- (backlog_item.estimated_hours if backlog_item),
- :size => 4, :maxlength => 4%>
- </td>
- <td>
- <% if backlog_item %>
- <%= "#{backlog_item.votes_in_favor.size} of #{backlog_item.votes.size}" %>
- <% else %>
- 0
- <% end %>
</tr>
<% end %>
</tbody>
diff --git a/app/views/sprints/show.html.erb b/app/views/sprints/show.html.erb
index c1eed47..f5f572e 100644
--- a/app/views/sprints/show.html.erb
+++ b/app/views/sprints/show.html.erb
@@ -40,7 +40,7 @@
<% end %>
<% end %>
- <% if @sprint.is_team_member?(@user) && @sprint.planning? %>
+ <% if @sprint.is_team_member?(@user) && @sprint.proposed? %>
<%= link_to "Vote on sprint backlog",
vote_sprint_path(@sprint), :class => "command" %>
<% end %>
diff --git a/app/views/sprints/vote.html.erb b/app/views/sprints/vote.html.erb
index 4610a5a..5020388 100644
--- a/app/views/sprints/vote.html.erb
+++ b/app/views/sprints/vote.html.erb
@@ -12,6 +12,7 @@
<th scope="col" class="name">Title</th>
<th scope="col"># Votes</th>
<th scope="col">Vote</th>
+ <th scope="col">Estimate</th>
<th scope="col">Status</th>
</tr>
</thead>
@@ -22,7 +23,8 @@
<td><%= item.id %></td>
<td class="name"><%= item.user_story.title %></td>
<td><%= item.votes.size %></td>
- <td><%= select_tag "votes[#{item.id}]", options_for_select(BacklogItemVote::VOTES, @votes[item.id].to_i) %></td>
+ <td><%= select_tag "votes[#{item.id}]", options_for_select(BacklogItemVote::VOTES, @votes[item.id]) %></td>
+ <td><%= text_field_tag "estimates[#{item.id}]", @estimates[item.id], :size => 4, :maxlength => 4 %></td>
<td></td>
</tr>
diff --git a/db/migrate/013_create_backlog_items.rb b/db/migrate/013_create_backlog_items.rb
index 22ef1f2..d529b80 100644
--- a/db/migrate/013_create_backlog_items.rb
+++ b/db/migrate/013_create_backlog_items.rb
@@ -1,3 +1,4 @@
+
# 013_create_backlog_items.rb
# Copyright (C) 2008, Darryl L. Pierce <mcpierce(a)gmail.com>
#
@@ -22,7 +23,6 @@ class CreateBacklogItems < ActiveRecord::Migration
t.integer :owner_id, :null => true
t.integer :state, :null => false, :default => BacklogItem::STATE_PENDING
t.boolean :completed, :null => false, :default => false
- t.decimal :estimated_hours, :null => false, :default => 0.0, :precision => 5, :scale => 2
t.boolean :completed, :null => false, :default => false
t.boolean :discovered, :null => false, :default => false
t.boolean :blocked, :null => false, :default => false
diff --git a/db/migrate/024_create_backlog_item_votes.rb b/db/migrate/024_create_backlog_item_votes.rb
index d135b27..6ebae1a 100644
--- a/db/migrate/024_create_backlog_item_votes.rb
+++ b/db/migrate/024_create_backlog_item_votes.rb
@@ -21,6 +21,7 @@ class CreateBacklogItemVotes < ActiveRecord::Migration
t.integer :user_id, :null => false
t.integer :backlog_item_id, :null => false
t.integer :vote, :null => false, :default => 0
+ t.float :estimated_hours, :num => false, :default => 0.0
t.timestamps
end
diff --git a/doc/ChangeLog b/doc/ChangeLog
index eb357a1..799efae 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -7,6 +7,7 @@ Change Log (0.3.0):
* #162 - Users can reply to comments.
* #163 - Comments on user stories are shown on the details page.
* #164 - Users can vote on backlog items.
+ * #166 - The development team provides estimates for the sprint backlog.
* #167 - Blocker messages are included in the daily updates email.
* #168 - Users can post comments for epic stories.
* #169 - Comments on epics are shown on the details page.
diff --git a/test/fixtures/backlog_item_votes.yml b/test/fixtures/backlog_item_votes.yml
index bfff55d..3630a1b 100644
--- a/test/fixtures/backlog_item_votes.yml
+++ b/test/fixtures/backlog_item_votes.yml
@@ -5,5 +5,5 @@ planned_sprint_backlog_item_jdonuts_vote:
planned_sprint_backlog_item_mcpierce_vote:
user_id: <%= Fixtures.identify(:mcpierce) %>
- backlog_item_id: <%= Fixtures.identify(:planned_sprint_backlog_item) %>
+ backlog_item_id: <%= Fixtures.identify(:unowned_backlog_item) %>
vote: <%= BacklogItemVote::NAK %>
diff --git a/test/fixtures/backlog_items.yml b/test/fixtures/backlog_items.yml
index f7b3c50..fd0ee9b 100644
--- a/test/fixtures/backlog_items.yml
+++ b/test/fixtures/backlog_items.yml
@@ -2,46 +2,44 @@ owned_backlog_item:
sprint_id: <%= Fixtures.identify(:active_sprint) %>
user_story_id: <%= Fixtures.identify(:create_user_story) %>
owner_id: <%= Fixtures.identify(:mcpierce) %>
- estimated_hours: 5.0
state: <%= BacklogItem::STATE_ASSIGNED %>
unowned_backlog_item:
sprint_id: <%= Fixtures.identify(:active_sprint) %>
user_story_id: <%= Fixtures.identify(:create_login) %>
- estimated_hours: 3.5
state: <%= BacklogItem::STATE_PENDING %>
closed_backlog_item:
sprint_id: <%= Fixtures.identify(:active_sprint) %>
user_story_id: <%= Fixtures.identify(:create_logout) %>
owner_id: <%= Fixtures.identify(:mcpierce) %>
- estimated_hours: 4.0
state: <%= BacklogItem::STATE_COMPLETED %>
+proposed_sprint_backlog_item:
+ sprint_id: <%= Fixtures.identify(:proposed_sprint) %>
+ user_story_id: <%= Fixtures.identify(:create_login) %>
+ state: <%= BacklogItem::STATE_PENDING %>
+
planned_sprint_backlog_item:
sprint_id: <%= Fixtures.identify(:planned_sprint) %>
user_story_id: <%= Fixtures.identify(:create_update_profile) %>
- estimated_hours: 3.5
state: <%= BacklogItem::STATE_PENDING %>
owned_planned_sprint_backlog_item:
sprint_id: <%= Fixtures.identify(:planned_sprint) %>
user_story_id: <%= Fixtures.identify(:email_forgotten_password) %>
owner_id: <%= Fixtures.identify(:mcpierce) %>
- estimated_hours: 3.5
state: <%= BacklogItem::STATE_PENDING %>
closed_sprint_backlog_item:
sprint_id: <%= Fixtures.identify(:completed_sprint) %>
user_story_id: <%= Fixtures.identify(:email_forgotten_password) %>
owner_id: <%= Fixtures.identify(:mcpierce) %>
- estimated_hours: 4.5
state: <%= BacklogItem::STATE_COMPLETED %>
blocked_backlog_item:
sprint_id: <%= Fixtures.identify(:active_sprint) %>
user_story_id: <%= Fixtures.identify(:create_update_profile) %>
owner_id: <%= Fixtures.identify(:mcpierce) %>
- estimated_hours: 5.0
state: <%= BacklogItem::STATE_ASSIGNED %>
blocked: true
diff --git a/test/fixtures/sprint_members.yml b/test/fixtures/sprint_members.yml
index beec376..e475590 100644
--- a/test/fixtures/sprint_members.yml
+++ b/test/fixtures/sprint_members.yml
@@ -1,7 +1,11 @@
-mcpierce_active_sprint:
- sprint_id: <%= Fixtures.identify(:active_sprint) %>
+mcpierce_proposed_sprint:
+ sprint_id: <%= Fixtures.identify(:proposed_sprint) %>
member_id: <%= Fixtures.identify(:mcpierce) %>
mcpierce_planned_sprint:
sprint_id: <%= Fixtures.identify(:planned_sprint) %>
member_id: <%= Fixtures.identify(:mcpierce) %>
+
+mcpierce_active_sprint:
+ sprint_id: <%= Fixtures.identify(:active_sprint) %>
+ member_id: <%= Fixtures.identify(:mcpierce) %>
diff --git a/test/functional/items_controller_test.rb b/test/functional/items_controller_test.rb
index 54eed34..ca721a6 100644
--- a/test/functional/items_controller_test.rb
+++ b/test/functional/items_controller_test.rb
@@ -68,7 +68,7 @@ class ItemsControllerTest < ActionController::TestCase
@blocker_message = Message.blocker_messages((a)blocked_item).last
raise "The blocker message should have been loaded!" unless @blocker_message
- @new_backlog_item = {:user_story_id => @user_story.id, :estimated_hours => 4.5}
+ @new_backlog_item = {:user_story_id => @user_story.id}
end
# Ensures that viewing an index works as expected.
@@ -164,7 +164,6 @@ class ItemsControllerTest < ActionController::TestCase
assert_response :success
assert_template 'items/new'
assert assigns['backlog_item'], "Failed to maintain the backlog item."
- assert_equal @new_backlog_item[:estimated_hours], assigns['backlog_item'].estimated_hours, "Lost passed in values."
assert_equal @sprint.id, assigns['backlog_item'].sprint_id, "Did not set the right sprint."
assert assigns['backlog_item'].discovered?, "Item was not marked as discovered."
assert assigns['user_stories'], "Failed to load the list of user stories."
diff --git a/test/functional/sprints_controller_test.rb b/test/functional/sprints_controller_test.rb
index 946ccb2..505e7b7 100644
--- a/test/functional/sprints_controller_test.rb
+++ b/test/functional/sprints_controller_test.rb
@@ -40,7 +40,12 @@ class SprintsControllerTest < ActionController::TestCase
raise "Member must be a team member!" unless @planned_sprint.is_team_member?(@member)
raise "Nonmember cannot be a team member!" if @planned_sprint.is_team_member?(@nonmember)
- @votes = {(a)planned_sprint.backlog_items.first.id => BacklogItemVote::ACK}
+ @proposed_sprint_backlog_item = backlog_items(:proposed_sprint_backlog_item)
+ @proposed_sprint = @proposed_sprint_backlog_item.sprint
+ raise "Sprint must be in proposed state!" unless @proposed_sprint.proposed?
+
+ @votes = {"#{(a)proposed_sprint.backlog_items.first.id}" => "#{BacklogItemVote::ACK}"}
+ @estimates = {"#{(a)proposed_sprint.backlog_items.first.id}" => "5.0"}
@startable_sprint = sprints(:startable_sprint)
raise "Sprint must be in the planned start!" unless @startable_sprint.planning?
@@ -80,9 +85,6 @@ class SprintsControllerTest < ActionController::TestCase
}
@user_story = user_stories(:create_user_story_web_service_api)
- @estimates = {
- "'#{(a)user_story.id}'" => 3.5
- }
@selected = [
@user_story.id
@@ -475,6 +477,13 @@ class SprintsControllerTest < ActionController::TestCase
assert_redirected_to login_path
end
+ # Ensures that a valid sprint id is required.
+ def test_plan_with_invalid_id
+ get :plan, {}, {:user_id => @planned_sprint.team_lead_id}
+
+ assert_redirected_to error_path
+ end
+
# Ensures that cancelling a sprint works.
def test_cancel
count = @active_sprint.product.rss_entries.size
@@ -494,27 +503,26 @@ class SprintsControllerTest < ActionController::TestCase
end
# Ensures that you can only populate a pending sprint.
- def test_plan_requires_pending_sprint
+ def test_plan_requires_proposed_sprint
get :plan, {:id => @active_sprint.id}, {:user_id => @product.owner_id}
- assert_redirected_to error_path
+ assert_redirected_to sprint_path(@active_sprint)
end
# Ensures that only the owner can plan a sprint.
def test_plan_as_nonowner
- get :plan, {:id => @planned_sprint.id}, {:user_id => @nonowner.id}
+ get :plan, {:id => @proposed_sprint.id}, {:user_id => @nonowner.id}
- assert_redirected_to error_path
+ assert_redirected_to sprint_path(@proposed_sprint)
end
# Ensures that planning works.
def test_plan
- get :plan, { :id => @planned_sprint.id}, {:user_id => @planned_sprint.product.owner_id}
+ get :plan, { :id => @proposed_sprint.id}, {:user_id => @proposed_sprint.product.owner_id}
assert_response :success
assert assigns['user_stories'], "Failed to load any user stories."
assert assigns['selected'], "Failed to setup a selection map."
- assert assigns['estimates'], "Failed to setup the estimates list."
end
# Ensures an anonymous user can't populate a sprint.
@@ -535,36 +543,35 @@ class SprintsControllerTest < ActionController::TestCase
def test_populate_requires_pending_sprint
post :populate, { :id => @active_sprint.id}, {:user_id => @active_sprint.product.owner_id}
- assert_redirected_to error_path
+ assert_redirected_to sprint_path(@active_sprint)
end
# Ensures that only the owner can populate a sprint.
def test_populate_as_nonowner
- post :populate, { :id => @planned_sprint.id, :estimates => @estimates, :selected => @selected},
+ post :populate, { :id => @proposed_sprint.id, :estimates => @estimates, :selected => @selected},
{:user_id => @nonowner.id}
- assert_redirected_to error_path
- assert !BacklogItem.find_by_sprint_id_and_user_story_id(@planned_sprint.id,(a)user_story.id),
+ assert_redirected_to sprint_path(@proposed_sprint)
+ assert !BacklogItem.find_by_sprint_id_and_user_story_id(@proposed_sprint.id,(a)user_story.id),
"Backlog item should not have been created"
end
# Ensures that populating with no user stories is not an error. BUG#92
def test_populate_with_no_user_stories
- post :populate, { :id => @planned_sprint.id},{:user_id => @planned_sprint.product.owner_id}
+ post :populate, { :id => @proposed_sprint.id},{:user_id => @proposed_sprint.product.owner_id}
- assert_redirected_to sprint_path(@planned_sprint)
+ assert_redirected_to sprint_path(@proposed_sprint)
end
# Ensures that populating works.
def test_populate
- count = @planned_sprint.product.rss_entries.size
- post :populate, {:id => @planned_sprint.id,:estimates => @estimates, :selected => @selected},
- {:user_id => @planned_sprint.product.owner_id}
+ count = @proposed_sprint.product.rss_entries.size
+ post :populate, {:id => @proposed_sprint.id, :selected => @selected},{:user_id => @proposed_sprint.product.owner_id}
- assert_redirected_to sprint_path(@planned_sprint)
- assert BacklogItem.find_by_sprint_id_and_user_story_id(@planned_sprint.id,(a)user_story.id),
+ assert_redirected_to sprint_path(@proposed_sprint)
+ assert BacklogItem.find_by_sprint_id_and_user_story_id(@proposed_sprint.id,(a)user_story.id),
"Backlog item should have been created"
- product = Product.find_by_id((a)planned_sprint.product_id)
+ product = Product.find_by_id((a)proposed_sprint.product_id)
assert_equal count + 1, product.rss_entries.size, "Failed to add an RSS entry."
end
@@ -607,9 +614,9 @@ class SprintsControllerTest < ActionController::TestCase
# Ensures that only a team member can vote on sprint items.
def test_vote_with_nonmember
- get :vote, {:id => @planned_sprint.id}, {:user_id => @nonmember.id}
+ get :vote, {:id => @proposed_sprint.id}, {:user_id => @nonmember.id}
- assert_redirected_to sprint_path(@planned_sprint)
+ assert_redirected_to sprint_path(@proposed_sprint)
end
# Ensures that only a sprint in planning can get votes.
@@ -621,10 +628,11 @@ class SprintsControllerTest < ActionController::TestCase
# Ensures that the vote action works as expected.
def test_vote
- get :vote, {:id => @planned_sprint.id}, {:user_id => @member.id}
+ get :vote, {:id => @proposed_sprint.id}, {:user_id => @member.id}
assert_response :success
assert assigns['votes'], "Failed to setup the votes."
+ assert assigns['estimates'], "Failed to setup the estimates."
end
# Ensures that anonymous users cannot cast votes.
@@ -643,11 +651,11 @@ class SprintsControllerTest < ActionController::TestCase
# Ensures that only members can cast votes.
def test_cast_vote_as_nonmember
- count = BacklogItemVote.for_sprint((a)planned_sprint).size
- post :cast_votes, {:id => @planned_sprint.id}, {:user_id => @nonmember.id}
+ count = BacklogItemVote.for_sprint((a)proposed_sprint).size
+ post :cast_votes, {:id => @proposed_sprint.id}, {:user_id => @nonmember.id}
- assert_redirected_to sprint_path(@planned_sprint)
- assert_equal count, BacklogItemVote.for_sprint((a)planned_sprint).size, "No new votes should have been cast."
+ assert_redirected_to sprint_path(@proposed_sprint)
+ assert_equal count, BacklogItemVote.for_sprint((a)proposed_sprint).size, "No new votes should have been cast."
end
# Ensures that only sprints in planning can have votes.
@@ -661,27 +669,35 @@ class SprintsControllerTest < ActionController::TestCase
# Ensures that a vote cast for a non-existent backlog item fails.
def test_cast_vote_for_nonexistent_backlog_item
- count = BacklogItemVote.for_sprint((a)planned_sprint).size
- post :cast_votes, {:id => @planned_sprint.id, :votes => {9999 => BacklogItemVote::ACK}}, {:user_id => @member.id}
+ count = BacklogItemVote.for_sprint((a)proposed_sprint).size
+ post :cast_votes, {:id => @proposed_sprint.id, :votes => {9999 => BacklogItemVote::ACK}}, {:user_id => @member.id}
assert_redirected_to error_path
- assert_equal count, BacklogItemVote.for_sprint((a)planned_sprint).size, "Vote should not have been cast."
+ assert_equal count, BacklogItemVote.for_sprint((a)proposed_sprint).size, "Vote should not have been cast."
end
- # Ensures that a vote cast for an item on a different sprint fails.
+ # Ensures that a vote cast for an item on a different sprint fails.
def test_cast_vote_for_item_on_different_sprint
- count = BacklogItemVote.for_sprint((a)planned_sprint).size
- post :cast_votes, {:id => @planned_sprint.id, :votes => {(a)active_sprint.backlog_items.first.id => BacklogItemVote::ACK}}, {:user_id => @member.id}
+ count = BacklogItemVote.for_sprint((a)proposed_sprint).size
+ post :cast_votes, {:id => @proposed_sprint.id, :votes => {(a)active_sprint.backlog_items.first.id => BacklogItemVote::ACK}}, {:user_id => @member.id}
+
+ assert_redirected_to error_path
+ assert_equal count, BacklogItemVote.for_sprint((a)proposed_sprint).size, "Vote should not have been cast."
+ end
+
+ # Ensures that, when casting a positive vote, an estimate must be included.
+ def test_cast_vote_accept_with_no_estimate
+ post :cast_votes, {:id => @proposed_sprint.id, :votes => @votes}, {:user_id => @member.id}
assert_redirected_to error_path
- assert_equal count, BacklogItemVote.for_sprint((a)planned_sprint).size, "Vote should not have been cast."
end
# Ensures that casting votes works as expected.
def test_cast_vote
- post :cast_votes, {:id => @planned_sprint.id, :votes => @votes}, {:user_id => @member.id}
+ count = BacklogItemVote.for_sprint((a)proposed_sprint).size
+ post :cast_votes, {:id => @proposed_sprint.id, :votes => @votes, :estimates => @estimates}, {:user_id => @member.id}
- assert_redirected_to sprint_path(@planned_sprint)
- assert_equal 1, BacklogItemVote.for_user(@member).for_sprint((a)planned_sprint).size, "A new vote should have been cast."
+ assert_redirected_to sprint_path(@proposed_sprint)
+ assert_equal count + 1, BacklogItemVote.for_sprint((a)proposed_sprint).size, "A new vote should have been cast."
end
end
diff --git a/test/unit/backlog_item_test.rb b/test/unit/backlog_item_test.rb
index 1cc04d0..8ea0d22 100644
--- a/test/unit/backlog_item_test.rb
+++ b/test/unit/backlog_item_test.rb
@@ -33,8 +33,7 @@ class BacklogItemTest < ActiveSupport::TestCase
@new_item = BacklogItem.new(
:sprint_id => @sprint.id,
- :user_story_id => user_stories(:create_login).id,
- :estimated_hours => 5.0)
+ :user_story_id => user_stories(:create_login).id)
@owned_item = backlog_items(:owned_backlog_item)
raise "Owned item must have an owner!" unless @owned_item.owner
@@ -42,6 +41,7 @@ class BacklogItemTest < ActiveSupport::TestCase
@unowned_item = backlog_items(:unowned_backlog_item)
raise "Unowned items must not have an owner!" if @unowned_item.owner
+ raise "Unowned item must have estimates!" if @unowned_item.votes.empty?
@closed_item = backlog_items(:closed_backlog_item)
raise "Closed items must be closed!" unless @closed_item.completed?
@@ -87,22 +87,16 @@ class BacklogItemTest < ActiveSupport::TestCase
# Ensures that the default value for remaining hours is the original estimated
# hours.
def test_remaining_when_undefined
- @new_item.estimated_hours = 2.5
-
- assert_equal 2.5,
- @new_item.remaining_hours,
- 'Remaining hours should equal estimated hours.'
+ assert_equal @unowned_item.votes.first.estimated_hours, @unowned_item.remaining_hours,
+ 'Remaining hours should equal estimated hours.'
end
# Ensures that adding a new remaining hours estimation affects the hours
# reported.
def test_remaining_hours_when_updated
- @new_item.estimated_hours = 5.0
@new_item.remaining_hours_estimates << RemainingHoursEstimate.new(:hours => 2.5, :estimated_on => Date.today.next)
- assert_equal 2.5,
- @new_item.remaining_hours,
- 'Remaining hours is incorrectly reported.'
+ assert_equal 2.5, @new_item.remaining_hours, 'Remaining hours is incorrectly reported.'
end
# Ensure a non-member cannot accept a backlog item.
diff --git a/test/unit/backlog_item_vote_test.rb b/test/unit/backlog_item_vote_test.rb
index 6dd26f5..274c008 100644
--- a/test/unit/backlog_item_vote_test.rb
+++ b/test/unit/backlog_item_vote_test.rb
@@ -19,8 +19,10 @@ require File.dirname(__FILE__) + '/../test_helper'
class BacklogItemVoteTest < ActiveSupport::TestCase
def setup
- @new_vote = BacklogItemVote.new(:user_id => users(:mcpierce),
- :backlog_item_id => backlog_items(:planned_sprint_backlog_item))
+ @new_vote = BacklogItemVote.new(:user_id => users(:mcpierce),
+ :backlog_item_id => backlog_items(:planned_sprint_backlog_item),
+ :vote => BacklogItemVote::ACK,
+ :estimated_hours => 5.0)
end
# Ensures that a vote requires a user.
@@ -53,6 +55,13 @@ class BacklogItemVoteTest < ActiveSupport::TestCase
[-3, -2, 2, 3, 1.1].each {|vote| @new_vote.vote = vote; fails "#{vote} must be considered invalid!" if @new_vote.valid?}
end
+ # Ensures that, if an ACK is received, it must have a positive estimated value.
+ def test_valid_fails_without_estimate_on_ack
+ @new_vote.vote = BacklogItemVote::ACK
+ @new_vote.estimated_hours = 0
+ fail "An accept must have an estimate." if @new_vote.valid?
+ end
+
# Ensures that validation works as expected.
def test_valid
fail "There is a general validation error." unless @new_vote.valid?
--
1.6.2.5
14 years, 8 months
- 1
- 0
[PATCH] Sprints can be in the proposed state. #218
by Darryl L. Pierce
Changed state names from integers to strings. This gives more freedom in
modifying the states over time.
Added the new state to the state machine sprints. Added a test to ensure
that a sprint can be moved from proposed to planning state.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/models/sprint.rb | 19 +++++++++++++------
db/migrate/011_create_sprints.rb | 2 +-
test/fixtures/sprints.yml | 9 +++++++++
test/unit/sprint_test.rb | 13 +++++++++++++
4 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index 4cbc86f..239102f 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -73,12 +73,13 @@ class Sprint < ActiveRecord::Base
:dependent => :destroy
has_many :user_stories, :through => :backlog_items, :order => :priority
- STATE_PLANNING=0
- STATE_ACTIVE=1
- STATE_COMPLETED=2
- STATE_CANCELLED=3
+ STATE_PROPOSED='proposed'
+ STATE_PLANNING='planning'
+ STATE_ACTIVE='active'
+ STATE_COMPLETED='completed'
+ STATE_CANCELLED='cancelled'
- STATES = {STATE_PLANNING => "Planning", STATE_ACTIVE => "Active", :STATE_COMPLETED => "Completed", STATE_CANCELLED => "Cancelled"}
+ STATES = {STATE_PROPOSED => "Proposed", STATE_PLANNING => "Planning", STATE_ACTIVE => "Active", :STATE_COMPLETED => "Completed", STATE_CANCELLED => "Cancelled"}
named_scope :default, { :order => "starts ASC" }
named_scope :for_product, lambda { |product_id|
@@ -88,6 +89,7 @@ class Sprint < ActiveRecord::Base
named_scope :active, {:conditions => ['state = ?', STATE_ACTIVE]}
state_machine :initial => :planning do
+ state :proposed, :value => STATE_PROPOSED
state :planning, :value => STATE_PLANNING
state :active, :value => STATE_ACTIVE
state :completed, :value => STATE_COMPLETED
@@ -101,11 +103,16 @@ class Sprint < ActiveRecord::Base
end
end
+ event :unplan do
+ transition :planning => :proposed
+ end
+
event :plan do
transition :active => :planning
+ transition :proposed => :planning
def can_fire?(sprint)
- sprint.active? && Sprint.for_product(sprint.product).planned.empty?
+ sprint.proposed? || (sprint.active? && Sprint.for_product(sprint.product).planned.empty?)
end
end
diff --git a/db/migrate/011_create_sprints.rb b/db/migrate/011_create_sprints.rb
index ddec513..a3aba41 100644
--- a/db/migrate/011_create_sprints.rb
+++ b/db/migrate/011_create_sprints.rb
@@ -22,7 +22,7 @@ class CreateSprints < ActiveRecord::Migration
t.string :title, :null => false, :limit => 100
t.date :starts, :null => false
t.integer :duration, :null => false, :precision => 2, :scale => 0
- t.integer :state, :null => false, :default => 0
+ t.string :state, :null => false, :limit => 16
t.string :goals, :null => false, :limit => 1000
t.timestamps
diff --git a/test/fixtures/sprints.yml b/test/fixtures/sprints.yml
index d32b34f..06c700c 100644
--- a/test/fixtures/sprints.yml
+++ b/test/fixtures/sprints.yml
@@ -1,3 +1,12 @@
+proposed_sprint:
+ product_id: <%= Fixtures.identify(:projxp_web) %>
+ title: Stuff we're thinking about doing soon.
+ starts: <%= DateTime.now.to_s(:db) %>
+ duration: 28
+ goals: Don't know yet.
+ state: <%= Sprint::STATE_PROPOSED %>
+ team_lead_id: <%= Fixtures.identify(:team_lead) %>
+
planned_sprint:
product_id: <%= Fixtures.identify(:projxp_web) %>
title: Stuff we're going to do next.
diff --git a/test/unit/sprint_test.rb b/test/unit/sprint_test.rb
index 6c7c466..9bd309e 100644
--- a/test/unit/sprint_test.rb
+++ b/test/unit/sprint_test.rb
@@ -26,6 +26,9 @@ class SprintTest < ActiveSupport::TestCase
:goals => "To get stuff done!",
:team_lead => users(:mcpierce)
)
+ @proposed_sprint = sprints(:proposed_sprint)
+ raise "Sprint must be proposed!" unless @proposed_sprint.proposed?
+
@planned_sprint = sprints(:planned_sprint)
raise "Sprint must be in planning!" unless @planned_sprint.planning?
raise "Product must have an active sprint!" if Sprint.for_product((a)planned_sprint.product).active.empty?
@@ -116,6 +119,16 @@ class SprintTest < ActiveSupport::TestCase
end
end
+ # Ensures that a proposed sprint can be moved to the planning state.
+ def test_can_plan_a_proposed_sprint
+ flunk "A proposed sprint must be able to move to the planning state." unless @proposed_sprint.can_plan?
+ end
+
+ # Ensures that a planned sprint can be moved back to the proposed state.
+ def test_can_move_a_planned_sprint_to_proposed_state
+ flunk "A planned sprint should be able to go back to the proposed state." unless @planned_sprint.can_unplan?
+ end
+
# Ensures that nobody can edit a completed sprint.
def test_can_edit_with_completed_sprint
flunk "Completed sprints must not be edited by the team lead." if @completed_sprint.can_edit?((a)completed_sprint.team_lead)
--
1.6.2.5
14 years, 8 months
- 1
- 0
- ← Newer
- 1
- Older →