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&gt; --- 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}&quot; %><?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&gt; # @@ -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 = {&quot;#{(a)proposed_sprint.backlog_items.first.id}&quot; => "#{BacklogItemVote::ACK}"} + @estimates = {&quot;#{(a)proposed_sprint.backlog_items.first.id}&quot; => "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 = { - &quot;&#39;#{(a)user_story.id}&#39;&quot; => 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