Added actions to create new backlog items, plus unit tests to enforce conditions. Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com&gt; --- app/controllers/items_controller.rb | 47 +++++++++++++++- app/models/backlog_item.rb | 5 ++ app/models/sprint.rb | 23 ++++++-- app/models/user_story.rb | 7 +++ app/views/items/_edit.html.erb | 48 +++++++++++++++++ app/views/items/new.html.erb | 1 + app/views/sprints/show.html.erb | 5 ++ test/functional/items_controller_test.rb | 86 ++++++++++++++++++++++++++++++ test/unit/sprint_test.rb | 8 +++ 9 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 app/views/items/_edit.html.erb create mode 100644 app/views/items/new.html.erb diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb index eb92906..0b35ebe 100644 --- a/app/controllers/items_controller.rb +++ b/app/controllers/items_controller.rb @@ -17,17 +17,19 @@ # +ItemsController+ handles CRUD operations on instances of +BacklogItem+. class ItemsController < ApplicationController - before_filter :unsupported, :only => [:new, :edit, :create, :update] + before_filter :unsupported, :only => [:edit, :update] before_filter :authenticated, :except => [:index, :show] - before_filter :load_backlog_item, :except => [:index] + before_filter :load_sprint, :only => [:new, :create] + before_filter :load_backlog_item, :except => [:index, :new, :create] before_filter :verify_can_delete, :only => [:destroy] + before_filter :verify_can_create, :only => [:new, :create] before_filter :verify_sprint_is_active, :only => [:accept, :estimate, :drop, :complete, :reopen, :blocked, :update_blocked] before_filter :verify_is_member, :only => [:accept, :drop, :complete, :reopen] before_filter :verify_is_owner, :only => [:drop, :complete, :estimate, :blocked, :update_blocked] before_filter :verify_can_mark_blocked, :only => [:blocked, :update_blocked] before_filter :load_message, :only => [:blocked, :update_blocked] before_filter :load_hours, :only => [:estimate] - before_filter :path_to_list, :only => [:index] + before_filter :path_to_list, :only => [:index, :new] before_filter :path_to_one, :only => [:show] # GET /items @@ -49,6 +51,32 @@ class ItemsController < ApplicationController end end + # GET /items/new?sprint=1 + def new + add_breadcrumb("New") + @backlog_item = BacklogItem.new(:sprint => @sprint) + @backlog_item.discovered = true + end + + # POST /items/new + def create + BacklogItem.transaction do + @backlog_item = BacklogItem.new(params[:backlog_item]) + @backlog_item.sprint = @sprint + @backlog_item.discovered = true + + respond_to do |format| + if @backlog_item.save + flash[:message] = "Backlog item created." + format.html {redirect_to item_path(@backlog_item)} + else + @backlog_item.valid? + format.html {render :action => :new} + end + end + end + end + # DELETE /items/1 def destroy UserStory.transaction do @@ -198,10 +226,19 @@ class ItemsController < ApplicationController report_error "That function is not supported for backlog items." end + def load_sprint + @sprint = Sprint.find_by_id(params[:sprint]) + @team_lead = @sprint.team_lead if @sprint + @user_stories = UserStory.for_product(@sprint.product).not_in_sprint((a)sprint) + + report_error "Missing or invalid sprint." unless @sprint + end + def load_backlog_item @backlog_item = BacklogItem.find_by_id(params[:id]) @tasks = @backlog_item.tasks if @backlog_item @sprint = @backlog_item.sprint if @backlog_item + @team_lead = @sprint.team_lead if @backlog_item report_error "Missing or invalid backlog item." unless @backlog_item && (@backlog_item.sprint_id == @sprint.id) end @@ -221,6 +258,10 @@ class ItemsController < ApplicationController report_error "You are not allowed to delete this item." unless @backlog_item.can_delete?(@user) end + def verify_can_create + report_error "You are not allowed to create or edit backlog items." unless @sprint.can_add_backlog_items?(@user) + end + def verify_sprint_is_active report_error "This sprint is not in the active state." unless @sprint.active? end diff --git a/app/models/backlog_item.rb b/app/models/backlog_item.rb index 07dc2b4..6b0b0b2 100644 --- a/app/models/backlog_item.rb +++ b/app/models/backlog_item.rb @@ -139,6 +139,11 @@ class BacklogItem < ActiveRecord::Base self.owner = nil end + # Returns whether the item was discovered. + def discovered? + discovered + end + # Returns whether the given user can modify this backlog item. def can_edit?(user) (owner?(user) || product_owner?(user)) && (state != STATE_COMPLETED) diff --git a/app/models/sprint.rb b/app/models/sprint.rb index a26a994..7b855a1 100644 --- a/app/models/sprint.rb +++ b/app/models/sprint.rb @@ -131,20 +131,17 @@ class Sprint < ActiveRecord::Base # Returns whether the specified user can modify this sprint. def can_edit?(user) - user && ((user.id == product.owner.id) || (user.id == team_lead.id)) + is_product_owner(user) || is_team_lead(user) end # Returns whether the user can delete the current sprint. def can_delete?(user) - user && (user.id == product.owner_id) + is_product_owner(user) end # Returns whether the specified user is allowed to populate this sprint. def can_populate?(user) - user && - ((user.id == product.owner.id) || - (user.id == team_lead_id)) && - pending? + (is_product_owner(user) || is_team_lead(user)) && pending? end # Returns whether the sprint can be moved to the given status. @@ -184,6 +181,12 @@ class Sprint < ActiveRecord::Base status != STATUS_PLANNED end + # Rethers whether the sprint can have backlog items added, and whether the + # specified user can add one. + def can_add_backlog_items?(user) + (is_product_owner(user) || is_team_lead(user)) && status == STATUS_ACTIVE + end + # Returns the data as of the given day into the sprint. def burndown_data data = [] @@ -220,6 +223,14 @@ class Sprint < ActiveRecord::Base private + def is_product_owner(user) + user != nil && user.id == product.owner_id + end + + def is_team_lead(user) + user != nil && user.id == team_lead_id + end + # When the user close a sprint, every user stories are close if the related # backlog item are completed. def close_user_stories diff --git a/app/models/user_story.rb b/app/models/user_story.rb index 4007790..57a20d6 100644 --- a/app/models/user_story.rb +++ b/app/models/user_story.rb @@ -42,6 +42,9 @@ class UserStory < ActiveRecord::Base named_scope :for_product, lambda { |product_id| { :conditions => product_id ? ["product_id = ?", product_id] : [] } } + named_scope :not_in_sprint, lambda { |sprint_id| + {:conditions => ['id not in (select user_story_id from backlog_items where sprint_id = ?)', sprint_id]} + } # Returns whether the user can edit this user story. def can_edit?(user) @@ -62,4 +65,8 @@ class UserStory < ActiveRecord::Base def closed? closed end + + def selectable_title + return "#{title} (#{priority})" + end end diff --git a/app/views/items/_edit.html.erb b/app/views/items/_edit.html.erb new file mode 100644 index 0000000..04ab68a --- /dev/null +++ b/app/views/items/_edit.html.erb @@ -0,0 +1,48 @@ +<div id="content-no-sidebar"> + + <% html = @backlog_item.new_record? ? {:method => :post} : {:method => :put} %> + <% form_for :backlog_item, @backlog_item, :url => item_path(@backlog_item), :html => html do |form| %> + + <%= hidden_field_tag :sprint, @sprint.id %> + + <table class="edit"> + <caption><%= &quot;#{(a)backlog_item.new_record? ? 'Create' : 'Edit'} A Backlog Item." %></caption> + + <tbody> + <tr> + <td class="label-required">For user story:</td> + <td class="value"> + <%= collection_select(:backlog_item, :user_story_id, @user_stories, + :id, :selectable_title) %> + <%= error_message_on(:backlog_item, :user_story_id) %> + </td> + </tr> + + <tr> + <td class="label-required">Estimated hours</td> + <td class="value"> + <%= form.text_field :estimated_hours, :size => 3, :maxlength => 5 %> + <%= error_message_on(:backlog_item, :estimated_hours) %> + </td> + </tr> + + <tr> + <td class="label"></td> + <td class="value"> + <%= form.check_box :discovered, :disabled => true %> + <%= form.label :discovered, "This item was discovered after the sprint started." %> + </td> + </tr> + + <tr> + <td class="buttons" colspan="2"> + <%= submit_tag &quot;#{(a)backlog_item.new_record? ? 'Save' : 'Update'}" %> + </td> + </tr> + </tbody> + + </table> + + <% end %> + +</div> diff --git a/app/views/items/new.html.erb b/app/views/items/new.html.erb new file mode 100644 index 0000000..0b10edb --- /dev/null +++ b/app/views/items/new.html.erb @@ -0,0 +1 @@ +<%= render :partial => 'edit', :locals => {:backlog_item => @backlog_item, :sprint => @sprint} %> diff --git a/app/views/sprints/show.html.erb b/app/views/sprints/show.html.erb index f94af3a..ce58061 100644 --- a/app/views/sprints/show.html.erb +++ b/app/views/sprints/show.html.erb @@ -34,6 +34,11 @@ plan_sprint_path(@sprint), :class => "command" %> <% end %> + <% if @sprint.can_add_backlog_items?(@user) %> + <%= link_to "Add backlog item", + new_item_path(:sprint => @sprint), :class => "command" %> + <% end %> + <div class="header"><span class="title">Change Sprint Status</span></div> <% if @sprint.allowed_status?(Sprint::STATUS_PLANNED) %> diff --git a/test/functional/items_controller_test.rb b/test/functional/items_controller_test.rb index 972f2b7..1960059 100644 --- a/test/functional/items_controller_test.rb +++ b/test/functional/items_controller_test.rb @@ -30,12 +30,19 @@ class ItemsControllerTest < ActionController::TestCase @other_product = products(:projxp_web_services) raise "Product and other cannot be the same!" if @product.id == @other_product.id + @user_story = user_stories(:freerange_user_story) + raise "User story not found!" unless @user_story + @owner = @product.owner @nonowner = users(:mcpierce) raise "Owner and nonowner cannot be the same person." if @owner.id == @nonowner.id @sprint = sprints(:active_sprint) raise "Product and sprint are mismatched!" unless @product.id == @sprint.product_id + @team_lead = @sprint.team_lead + + @non_team_lead = users(:jdonuts) + raise "Non-team lead cannot be the team lead!" if @team_lead.id == @non_team_lead.id @pending_sprint = sprints(:inactive_sprint) raise "Sprint should be pending!" unless @pending_sprint.pending? @@ -60,6 +67,8 @@ class ItemsControllerTest < ActionController::TestCase raise "Item must be marked as blocked." unless @blocked_item.blocked @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} end # Ensures that viewing an index works as expected. @@ -94,6 +103,83 @@ class ItemsControllerTest < ActionController::TestCase assert assigns['tasks'], "Failed to load the tasks for this item." end + # Ensures that anonymous users cannot create a new backlog item. + def test_new_as_anonymous + get :new + + assert_redirected_to login_path + end + + # Ensures that sprint must be specified. + def test_new_with_invalid_sprint + get :new, {}, {:user_id => @owner.id} + + assert_redirected_to error_path + end + + # Ensures that only the team lead can add a backlog item. + def test_new_as_not_team_lead + get :new, {:sprint => @sprint.id}, {:user_id => @non_team_lead.id} + + assert_redirected_to error_path + end + + # Ensures that starting a new item works as expected. + def test_new + get :new, {:sprint => @sprint.id}, {:user_id => @team_lead.id} + + assert_response :success + assert assigns['backlog_item'], "Failed to create a backlog item." + 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." + end + + # Ensures anonymous users can't create backlog items. + def test_create_as_anonymous + post :create + + assert_redirected_to login_path + end + + # Ensures that a valids print must be defined. + def test_create_with_invalid_sprint + post :create, {}, {:user_id => @team_lead.id} + + assert_redirected_to error_path + end + + # Ensures that only the team lead can add a backlog item. + def test_create_as_non_team_lead + post :create, {:sprint => @sprint.id},{:user_id => @non_team_lead.id} + + assert_redirected_to error_path + end + + # Ensures that, if the backlog item is malformed, the user is reshown the edit page. + def test_create_with_invalid_item + @new_backlog_item[:user_story_id] = nil + post :create, {:sprint => @sprint.id, :backlog_item => @new_backlog_item}, {:user_id => @sprint.team_lead_id} + + 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." + end + + # Ensures that creating an item works as expected. + def test_create + post :create, {:sprint => @sprint.id, :backlog_item => @new_backlog_item}, {:user_id => @sprint.team_lead_id} + + result = BacklogItem.find_by_sprint_id_and_user_story_id((a)sprint.id, @user_story.id) + assert result, "Failed to create backlog item." + assert_redirected_to item_path(result) + assert result.discovered?, "Item was not marked as discovered." + end + # Ensures that anonymous users can't delete. def test_destroy_as_anonymous delete :destroy diff --git a/test/unit/sprint_test.rb b/test/unit/sprint_test.rb index a3ca8b5..c57cde6 100644 --- a/test/unit/sprint_test.rb +++ b/test/unit/sprint_test.rb @@ -46,6 +46,9 @@ class SprintTest < ActiveSupport::TestCase @team_lead = @existing_sprint.team_lead @owner = @product.owner raise "Team lead and product owner cannot be the same user!" if @team_lead.id == @owner.id + + @closed_sprint = sprints(:closed_sprint) + raise "Sprint must be closed!" unless @closed_sprint.closed? end # Ensures that a sprint has to have a product. @@ -146,4 +149,9 @@ class SprintTest < ActiveSupport::TestCase def test_edit_as_product_owner flunk "Product owners must be allowed to edit sprints." unless @existing_sprint.can_edit?(@owner) end + + # Ensures that a sprint that a sprint that's not active cannot add a backlog item. + def test_can_add_backlog_items_for_inactive_sprint + flunk "Inactive sprints cannot add backlog items." if @closed_sprint.can_add_backlog_items?((a)closed_sprint.team_lead) + end end -- 1.6.0.6