If a user is not a sprint team member then they cannot accept sprint items. Had to change the user in roles_controller_test to fix a test that broke as a result of this patch. Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com&gt; --- app/models/backlog_item.rb | 6 +++--- app/models/sprint.rb | 9 +++++++++ app/models/sprint_member.rb | 2 +- test/fixtures/product_roles.yml | 7 +++++++ test/functional/roles_controller_test.rb | 4 ++-- test/unit/backlog_item_test.rb | 20 +++++++++++++++++--- 6 files changed, 39 insertions(+), 9 deletions(-) diff --git a/app/models/backlog_item.rb b/app/models/backlog_item.rb index 37a81e9..0604575 100644 --- a/app/models/backlog_item.rb +++ b/app/models/backlog_item.rb @@ -135,7 +135,7 @@ class BacklogItem < ActiveRecord::Base # Returns whether the user can accept this item. def can_accept?(user) - user && sprint.product.is_member?(user) && + user && sprint.is_team_member?(user) && [STATE_PENDING, STATE_DROPPED]. include?(state) && sprint.active? end @@ -184,10 +184,10 @@ class BacklogItem < ActiveRecord::Base end def product_owner?(user) - user && (sprint.product.owner.id == user.id) + user && (sprint.product.owner_id == user.id) end def team_lead?(user) - user && (sprint.team_lead.id == user.id) + user && (sprint.team_lead_id == user.id) end end diff --git a/app/models/sprint.rb b/app/models/sprint.rb index b50938f..c8c4285 100644 --- a/app/models/sprint.rb +++ b/app/models/sprint.rb @@ -204,6 +204,15 @@ class Sprint < ActiveRecord::Base return false end + # Returns whether the user is a team member. + def is_team_member?(user) + if (user != nil) && members + return members.include?(user) + end + + false + end + private # When the user close a sprint, every user stories are close if the related diff --git a/app/models/sprint_member.rb b/app/models/sprint_member.rb index 20f8f72..4482d7e 100644 --- a/app/models/sprint_member.rb +++ b/app/models/sprint_member.rb @@ -17,7 +17,7 @@ # A +SprintMember+ maps a user's membership to a sprint. class SprintMember < ActiveRecord::Base - validates_presence_of :sprint_id, :message => "A sprint is required." + validates_presence_of :sprint, :message => "A sprint is required." validates_presence_of :member_id, :message => "A member is required." validates_uniqueness_of :member_id, :scope => [:sprint_id] diff --git a/test/fixtures/product_roles.yml b/test/fixtures/product_roles.yml index 6c4897f..27ec3ce 100644 --- a/test/fixtures/product_roles.yml +++ b/test/fixtures/product_roles.yml @@ -12,6 +12,13 @@ team_lead_projxp: pending: false is_approved: true +celliot_projxp: + product_id: <%= Fixtures.identify(:projxp_web) %> + user_id: <%= Fixtures.identify(:celliot) %> + role_id: <%= Fixtures.identify(:developer) %> + pending: false + is_approved: true + jdonuts_projxp: product_id: <%= Fixtures.identify(:projxp_web) %> user_id: <%= Fixtures.identify(:jdonuts) %> diff --git a/test/functional/roles_controller_test.rb b/test/functional/roles_controller_test.rb index a08e708..0eb0d3d 100644 --- a/test/functional/roles_controller_test.rb +++ b/test/functional/roles_controller_test.rb @@ -37,8 +37,8 @@ class RolesControllerTest < ActionController::TestCase raise "Pending roles should not be active!" if @pending_role.active? @pending_user = @pending_role.user - @user_with_no_role = users(:celliot) - rails "User should have no roles!" if ProductRole.find_by_product_id_and_user_id((a)product.id, @user_with_no_role.id) + @user_with_no_role = users(:teatime_owner) + raise "User should have no roles!" if ProductRole.find_by_product_id_and_user_id((a)product.id, @user_with_no_role.id) @new_role = { :user_id => @user_with_no_role.id, diff --git a/test/unit/backlog_item_test.rb b/test/unit/backlog_item_test.rb index bce39c6..f83932a 100644 --- a/test/unit/backlog_item_test.rb +++ b/test/unit/backlog_item_test.rb @@ -24,8 +24,11 @@ class BacklogItemTest < ActiveSupport::TestCase fixtures :users def setup + @sprint = sprints(:active_sprint) + @product = @sprint.product + @item = BacklogItem.new( - :sprint_id => sprints(:active_sprint).id, + :sprint_id => @sprint.id, :user_story_id => user_stories(:create_login).id, :estimated_hours => 5.0) @@ -39,8 +42,13 @@ class BacklogItemTest < ActiveSupport::TestCase @closed_backlog_item = backlog_items(:closed_backlog_item) raise "Closed items must be closed!" unless @closed_backlog_item.completed? + @member = users(:mcpierce) + raise "Member is not a sprint team member!" unless @sprint.is_team_member?(@member) + @nonmember = users(:celliot) - raise "Nonmember cannot be on the product team!" if @closed_backlog_item.sprint.product.is_member?(@nonmember) + raise "Nonmember cannot be on the sprint team!" if @sprint.is_team_member?(@nonmember) + raise "Nonmember must be on the product team!" unless @product.is_member?(@nonmember) + raise "Nonmember cannot be the team lead!" if @sprint.team_lead_id == @nonmember.id @item_on_inactive_sprint = backlog_items(:inactive_sprint_backlog_item) raise "Item should not have an owner!" if @item_on_inactive_sprint.owner @@ -91,9 +99,15 @@ class BacklogItemTest < ActiveSupport::TestCase 'Remaining hours is incorrectly reported.' end + # Ensure a non-member cannot accept a backlog item. + def test_account_as_nonmember + assert !@unowned_backlog_item.can_accept?((a)nonmember), + "Non-members cannot accept backlog items." + end + # Ensures that a user can accept an unowned backlog item. def test_accept_an_unowned_item - assert @unowned_backlog_item.can_accept?(@owner), + assert @unowned_backlog_item.can_accept?(@member), "Users must be able to accept backlog items." end -- 1.6.0.6