[PATCH] Backlog items that are over are hilighted. #102
by Darryl L. Pierce
When a backlog item is at or over its estimated hours, then it
is hilighted with a red background in all backlog item lists.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/views/items/_list.html.erb | 3 +++
public/stylesheets/lists.css | 10 ++++++++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/app/views/items/_list.html.erb b/app/views/items/_list.html.erb
index 2aa8bcd..bb574a3 100644
--- a/app/views/items/_list.html.erb
+++ b/app/views/items/_list.html.erb
@@ -48,6 +48,9 @@
<% else %>
<% @backlog_items.each_with_index do |item, index| %>
<% row_class = index%2 == 0 ? 'even' : 'odd' %>
+ <% unless item.completed? %>
+ <% row_class='overage' if item.actual_hours >= item.estimated_hours %>
+ <% end %>
<% sprint = item.sprint %>
<% product = sprint.product %>
diff --git a/public/stylesheets/lists.css b/public/stylesheets/lists.css
index f6aad1e..87b47b2 100644
--- a/public/stylesheets/lists.css
+++ b/public/stylesheets/lists.css
@@ -113,3 +113,13 @@ table.list tr.odd:hover {
background-color: #ffee87;
color: #000000;
}
+
+table.list tr.overage {
+ background-color: #7f0000;
+ color: #ffff00;
+ font-weight: bold;
+}
+
+table.list tr.overage a {
+ color: #ffffff;
+}
\ No newline at end of file
--
1.6.0.6
15 years
[PATCH] Backlog items that are over are hilighted. #102
by Darryl L. Pierce
When a backlog item is at or over its estimated hours, then it
is hilighted with a red background in all backlog item lists.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/views/items/_list.html.erb | 3 +++
public/stylesheets/lists.css | 10 ++++++++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/app/views/items/_list.html.erb b/app/views/items/_list.html.erb
index 2aa8bcd..bb574a3 100644
--- a/app/views/items/_list.html.erb
+++ b/app/views/items/_list.html.erb
@@ -48,6 +48,9 @@
<% else %>
<% @backlog_items.each_with_index do |item, index| %>
<% row_class = index%2 == 0 ? 'even' : 'odd' %>
+ <% unless item.completed? %>
+ <% row_class='overage' if item.actual_hours >= item.estimated_hours %>
+ <% end %>
<% sprint = item.sprint %>
<% product = sprint.product %>
diff --git a/public/stylesheets/lists.css b/public/stylesheets/lists.css
index f6aad1e..87b47b2 100644
--- a/public/stylesheets/lists.css
+++ b/public/stylesheets/lists.css
@@ -113,3 +113,13 @@ table.list tr.odd:hover {
background-color: #ffee87;
color: #000000;
}
+
+table.list tr.overage {
+ background-color: #7f0000;
+ color: #ffff00;
+ font-weight: bold;
+}
+
+table.list tr.overage a {
+ color: #ffffff;
+}
\ No newline at end of file
--
1.6.0.6
15 years
[PATCH] Only team members can accept backlog items. #143
by Darryl L. Pierce
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>
---
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
15 years
[PATCH] Sprints are composed of teams. #141
by Darryl L. Pierce
THIS PATCH REQUIRES A MIGRATION.
A new table and domain object, SprintMember, was created. This maps
users to the sprint for which they're a member.
Each sprint will now load its list of members and make them available
through the new "members" collection. Members can be added to the
membership list via the "sprint_members" association of Sprint.
When a sprint is edited, its list of current members is preselected in
the list that's displayed.
In the sprint list view there's a new column that shows the number of
team members on that sprint.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/controllers/application.rb | 1 +
app/controllers/sprints_controller.rb | 57 +++++++++++++++++++++-------
app/models/sprint.rb | 4 ++
app/models/sprint_member.rb | 27 +++++++++++++
app/views/sprints/_edit.html.erb | 26 ++++++++++--
app/views/sprints/_list.html.erb | 7 ++-
app/views/sprints/show.html.erb | 5 ++
db/migrate/027_create_sprint_members.rb | 33 ++++++++++++++++
test/fixtures/product_roles.yml | 7 +++
test/fixtures/sprint_members.yml | 3 +
test/functional/sprints_controller_test.rb | 56 +++++++++++++++++++++++++--
test/unit/sprint_member_test.rb | 56 +++++++++++++++++++++++++++
12 files changed, 256 insertions(+), 26 deletions(-)
create mode 100644 app/models/sprint_member.rb
create mode 100644 db/migrate/027_create_sprint_members.rb
create mode 100644 test/fixtures/sprint_members.yml
create mode 100644 test/unit/sprint_member_test.rb
diff --git a/app/controllers/application.rb b/app/controllers/application.rb
index 22c7661..17b7095 100644
--- a/app/controllers/application.rb
+++ b/app/controllers/application.rb
@@ -58,6 +58,7 @@ class ApplicationController < ActionController::Base
def handle_exceptions
yield
rescue Exception => error
+ puts "#{error.message}"
puts error.backtrace
erase_results
@title = "An Error Has Occurred."
diff --git a/app/controllers/sprints_controller.rb b/app/controllers/sprints_controller.rb
index f4e7557..528c1ac 100644
--- a/app/controllers/sprints_controller.rb
+++ b/app/controllers/sprints_controller.rb
@@ -21,6 +21,7 @@ class SprintsController < ApplicationController
before_filter :load_product
before_filter :load_sprint, :except => [:index, :new, :create]
before_filter :get_status, :only => [:status]
+ before_filter :prepare_for_edit, :only => [:new, :edit]
# GET /products/1/sprints
def index
@@ -69,14 +70,19 @@ class SprintsController < ApplicationController
if @product.can_create_sprints?(@user)
@sprint = Sprint.new(params[:sprint])
@sprint.product = @product
+ @selected = params[:selected]
+ @selected = Array.new unless @selected # if no members were selected, use an empty array
+ add_users_to_sprint(@selected)
- if @sprint.save
- flash[:message] = "Sprint successfully created!"
- format.html { redirect_to plan_product_sprint_path(@product,@sprint) }
- else
- @title = "New Sprint For #{(a)product.name}"
- @sprint.valid?
- format.html { render :action => :edit }
+ Sprint.transaction do
+ if @sprint.save
+ flash[:message] = "Sprint successfully created!"
+ format.html { redirect_to plan_product_sprint_path(@product,@sprint) }
+ else
+ @title = "New Sprint For #{(a)product.name}"
+ @sprint.valid?
+ format.html { render :action => :edit }
+ end
end
else
flash[:error] = "You are not allowed to create sprints for #{(a)product.name}."
@@ -90,14 +96,21 @@ class SprintsController < ApplicationController
respond_to do |format|
if @sprint.can_edit?(@user)
@sprint.update_attributes(params[:sprint])
+ # empty the member list
+ @sprint.sprint_members.clear
+ @selected = params[:selected]
+ @selected = Array.new unless @selected
+ add_users_to_sprint(@selected)
- if @sprint.save
- flash[:message] = "Sprint updated successfully."
- format.html { redirect_to params[:url] ? params[:url] : product_sprints_path(@product) }
- else
- @title = "Sprint #{(a)sprint.id} (Edit)"
- @sprint.valid?
- format.html { render :action => :edit }
+ Sprint.transaction do
+ if @sprint.save
+ flash[:message] = "Sprint updated successfully."
+ format.html { redirect_to params[:url] ? params[:url] : product_sprints_path(@product) }
+ else
+ @title = "Sprint #{(a)sprint.id} (Edit)"
+ @sprint.valid?
+ format.html { render :action => :edit }
+ end
end
else
flash[:error] = "You are now allowed to edit sprints for #{(a)product.name}."
@@ -220,6 +233,7 @@ class SprintsController < ApplicationController
def load_product
@product = Product.find_by_id(params[:product_id])
+ @members = @product.users if @product
unless @product
flash[:error] = "Missing or invalid product."
@@ -252,4 +266,19 @@ class SprintsController < ApplicationController
end
end
end
+
+ def prepare_for_edit
+ @selected = Array.new
+ if @sprint
+ @sprint.members.each { |member| @selected << member.id }
+ end
+ end
+
+ def add_users_to_sprint(selected)
+ selected.each do |id|
+ user = User.find_by_id(id)
+ raise "Invalid user: #{user.display_name} is not a member of the #{(a)product.name}." unless @product.is_member?(user)
+ @sprint.sprint_members << SprintMember.new(:sprint => @sprint, :member => user)
+ end
+ end
end
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index f2cb1f9..b50938f 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -64,6 +64,10 @@ class Sprint < ActiveRecord::Base
belongs_to :product
belongs_to :team_lead, :class_name => 'User', :foreign_key => :team_lead_id
+ # sprint membership
+ has_many :sprint_members, :dependent => :destroy
+ has_many :members, :through => :sprint_members
+
has_many :backlog_items,
:order => '(select priority from user_stories where id = user_story_id) ASC',
:dependent => :destroy
diff --git a/app/models/sprint_member.rb b/app/models/sprint_member.rb
new file mode 100644
index 0000000..4482d7e
--- /dev/null
+++ b/app/models/sprint_member.rb
@@ -0,0 +1,27 @@
+# sprint_member.rb
+# Copyright (C) 2009, Darryl L. Pierce <mcpierce(a)gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# A +SprintMember+ maps a user's membership to a sprint.
+class SprintMember < ActiveRecord::Base
+ 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]
+
+ belongs_to :sprint
+ belongs_to :member, :class_name => 'User', :foreign_key => 'member_id'
+end
diff --git a/app/views/sprints/_edit.html.erb b/app/views/sprints/_edit.html.erb
index 26ef5d8..f7b0b77 100644
--- a/app/views/sprints/_edit.html.erb
+++ b/app/views/sprints/_edit.html.erb
@@ -52,15 +52,31 @@
<%= error_message_on(:sprint, :goals) %>
</td>
</tr>
+ </tbody>
+ </table>
- <tr>
- <td class="buttons" colspan="2">
- <%= submit_tag "Save" %>
- </td>
- </tr>
+ <table class="list">
+ <thead>
+ <tr>
+ <th>Select Sprint Team Members</th>
+ </tr>
+ </thead>
+
+ <tbody>
+ <% @members.each_with_index do |member, index| %>
+ <% row_class = index%2 == 0 ? 'even' : 'odd' %>
+ <tr class="<%= row_class %>">
+ <td>
+ <%= check_box_tag "selected[]", member.id, @selected.include?(member.id) %>
+ <%= label_tag "selected[]", member.display_name %>
+ </td>
+ </tr>
+ <% end %>
</tbody>
</table>
+ <%= submit_tag "Save" %>
+
</fieldset>
<% end %>
diff --git a/app/views/sprints/_list.html.erb b/app/views/sprints/_list.html.erb
index 4d6f956..5d327bf 100644
--- a/app/views/sprints/_list.html.erb
+++ b/app/views/sprints/_list.html.erb
@@ -3,6 +3,7 @@
<col class="row_id" />
<col class="name" />
<col class="description" />
+ <col class="number" />
<col class="status" />
<col class="date" />
<col class="date" />
@@ -12,11 +13,12 @@
<thead>
<tr>
- <th class="title" colspan="8">Sprints</th>
+ <th class="title" colspan="9">Sprints</th>
<tr>
<th>#</th>
<th>Team Lead</th>
<th>Title</th>
+ <th>Members</th>
<th>Status</th>
<th>Starts</th>
<th>Ends</th>
@@ -28,7 +30,7 @@
<tbody>
<% if @sprints.empty? %>
<tr>
- <td colspan="8">No sprints found...</td>
+ <td colspan="9">No sprints found...</td>
</tr>
<% else %>
<% @sprints.each_with_index do |sprint, index| %>
@@ -37,6 +39,7 @@
<td><%= "#{sprint.id}" %></td>
<td><%= mail_to sprint.team_lead.email, sprint.team_lead.display_name %></td>
<td><%= sprint.title %></td>
+ <td><%= sprint.members.size %></td>
<td><%= sprint.status_text %></td>
<td><%= show_date(sprint.start) %></td>
<td><%= "#{show_date(sprint.end_date)} (#{sprint.duration} days)" %></td>
diff --git a/app/views/sprints/show.html.erb b/app/views/sprints/show.html.erb
index a5a3264..547a0f7 100644
--- a/app/views/sprints/show.html.erb
+++ b/app/views/sprints/show.html.erb
@@ -53,6 +53,11 @@
</tr>
<tr>
+ <td class="label">Team Size:</td>
+ <td class="value"><%= "#{(a)sprint.members.size} Developer#{(a)sprint.members.size ==1 ? '' : 's'}" %></td>
+ </tr>
+
+ <tr>
<td class="label">Status:</td>
<td class="value">
<%= @sprint.status_text %>
diff --git a/db/migrate/027_create_sprint_members.rb b/db/migrate/027_create_sprint_members.rb
new file mode 100644
index 0000000..c6ccce5
--- /dev/null
+++ b/db/migrate/027_create_sprint_members.rb
@@ -0,0 +1,33 @@
+# 027_create_sprint_members.rb
+# Copyright (C) 2009, Darryl L. Pierce <mcpierce(a)gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+class CreateSprintMembers < ActiveRecord::Migration
+ def self.up
+ create_table :sprint_members do |t|
+ t.integer :sprint_id, :null => false
+ t.integer :member_id, :null => false
+
+ t.timestamps
+ end
+
+ add_index :sprint_members, [:sprint_id, :member_id], :unique => true, :name => :single_membership
+ end
+
+ def self.down
+ drop_table :sprint_members
+ end
+end
diff --git a/test/fixtures/product_roles.yml b/test/fixtures/product_roles.yml
index 0eb7cba..6c4897f 100644
--- a/test/fixtures/product_roles.yml
+++ b/test/fixtures/product_roles.yml
@@ -5,6 +5,13 @@ mcpierce_projxp:
pending: false
is_approved: true
+team_lead_projxp:
+ product_id: <%= Fixtures.identify(:projxp_web) %>
+ user_id: <%= Fixtures.identify(:team_lead) %>
+ 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/fixtures/sprint_members.yml b/test/fixtures/sprint_members.yml
new file mode 100644
index 0000000..8cc07a2
--- /dev/null
+++ b/test/fixtures/sprint_members.yml
@@ -0,0 +1,3 @@
+mcpierce_active_sprint:
+ sprint_id: <%= Fixtures.identify(:active_sprint) %>
+ member_id: <%= Fixtures.identify(:mcpierce) %>
diff --git a/test/functional/sprints_controller_test.rb b/test/functional/sprints_controller_test.rb
index f39094a..b19b0c1 100644
--- a/test/functional/sprints_controller_test.rb
+++ b/test/functional/sprints_controller_test.rb
@@ -26,6 +26,9 @@ class SprintsControllerTest < ActionController::TestCase
@active_sprint = sprints(:active_sprint)
raise "An active sprint must be active!" unless @active_sprint.active?
+ @member = @active_sprint.members.first
+ raise "The active sprint has to have members!" unless @member
+
@pending_sprint = sprints(:inactive_sprint)
raise "Sprint must be pending!" unless @pending_sprint.pending?
raise "Sprints must be for the same product!" unless @pending_sprint.product_id == @active_sprint.product_id
@@ -38,6 +41,11 @@ class SprintsControllerTest < ActionController::TestCase
@nonowner = users(:mcpierce)
raise "Nonowner and owner must be different people!" if @owner.id == @nonowner.id
+ @product_nonmember = users(:celliot)
+ raise "Nonmembers cannot be members of the product team!" if @product.is_member?(@product_nonmember)
+ @other_member = users(:team_lead)
+ raise "Other member must be a part of the same product team!" unless @product.is_member?(@other_member)
+
@new_sprint = {
:title => "New Sprint",
:start => Date.today,
@@ -132,6 +140,7 @@ class SprintsControllerTest < ActionController::TestCase
assert_response :success
assert assigns['sprint'], "Failed to create a new sprint."
+ assert assigns['selected'], "Failed to create the default selection list."
assert_equal @product.id, assigns['sprint'].product_id,
"Did not set the product correctly."
end
@@ -176,6 +185,10 @@ class SprintsControllerTest < ActionController::TestCase
assert_response :success
assert assigns['sprint'], "Failed to load a sprint."
+ assert assigns['selected'], "Failed to load the selection list."
+ first = assigns['selected'].first
+ assert_equal assigns['selected'].first, @member.id,
+ "The selection list was not initialized correctly."
assert_equal @active_sprint.id, assigns['sprint'].id,
"Failed to load the right sprint."
end
@@ -215,16 +228,37 @@ class SprintsControllerTest < ActionController::TestCase
"Should not have saved the sprint."
end
+ # Ensures that, if no members were selected, the sprint is created with an empty membership.
+ def test_create_with_no_members
+ post :create,
+ {:product_id => @product.id, :sprint => @new_sprint},
+ {:user_id => @owner.id}
+
+ assert_redirected_to plan_product_sprint_path(@product, assigns['sprint'])
+ assert assigns['sprint'].members.empty?, "No members should be defined"
+ end
+
+ # Ensures that non-product members cannot be added to the sprint team.
+ def test_create_with_non_product_member
+ post :create,
+ {:product_id => @product.id, :sprint => @new_sprint, :selected => [@product_nonmember.id]},
+ {:user_id => @owner.id}
+
+ assert_redirected_to error_path
+ assert !Sprint.find_by_title(@new_sprint[:title]), "Sprint should not have been saved!"
+ end
+
# Ensures that creating a sprint works as expected.
def test_create
post :create,
- {:product_id => @product.id,
- :sprint => @new_sprint},
+ {:product_id => @product.id, :sprint => @new_sprint, :selected => [@member.id]},
{:user_id => @owner.id}
assert_redirected_to plan_product_sprint_path(@product,assigns['sprint'])
- assert Sprint.find_by_title(@new_sprint[:title]),
- "Failed to save the sprint."
+ result = Sprint.find_by_title(@new_sprint[:title])
+ assert result, "Failed to save the sprint."
+ assert_equal 1, result.members.size, "Did not populate the sprint team correctly."
+ assert_equal @member.id, result.members.first.id, "Did not put the right person in the team."
end
# Ensures that anonymous users cannot update a sprint.
@@ -270,18 +304,30 @@ class SprintsControllerTest < ActionController::TestCase
"Sprint should not have been updated."
end
+ # Ensures that non-product members cannot be added to the sprint team.
+ def test_update_with_non_product_member
+ put :update,
+ {:product_id => @product.id, :id => @active_sprint.id,
+ :sprint => {}, :selected => [@product_nonmember.id]},
+ {:user_id => @owner.id}
+
+ assert_redirected_to error_path
+ end
+
# Ensures that updating a sprint works as expected.
def test_update
sprint = {:title => "Do it!"}
put :update,
{:product_id => @product.id, :id => @active_sprint.id,
- :sprint => sprint},
+ :sprint => sprint, :selected => [@other_member.id]},
{:user_id => @owner.id}
assert_redirected_to product_sprints_path(@product)
result = Sprint.find_by_id((a)active_sprint.id)
assert_equal sprint[:title], result.title,
"Sprint should have been updated."
+ assert_equal 1, result.members.size, "Failed to populate the sprint."
+ assert_equal @other_member.id, result.members.first.id, "Failed to populate the sprint correctly."
end
# Ensures that updating with a specified return URL sends the browser back
diff --git a/test/unit/sprint_member_test.rb b/test/unit/sprint_member_test.rb
new file mode 100644
index 0000000..da4e3bc
--- /dev/null
+++ b/test/unit/sprint_member_test.rb
@@ -0,0 +1,56 @@
+# Copyright (C) 2009, Darryl L. Pierce <mcpierce(a)gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.dirname(__FILE__) + '/../test_helper'
+
+class SprintTeamTest < ActiveSupport::TestCase
+ fixtures :sprint_members
+ fixtures :sprints
+ fixtures :users
+
+ def setup
+ @sprint = sprints(:inactive_sprint)
+ @team_lead = @sprint.team_lead
+ @member = users(:mcpierce)
+
+ @membership = SprintMember.new(:sprint => @sprint, :member => @member)
+ @existing_membership = sprint_members(:mcpierce_active_sprint)
+ end
+
+ # Ensures that a membership requires a sprint reference.
+ def test_sprint_is_required
+ @membership.sprint = nil
+ flunk "Membership requires a sprint." if @membership.valid?
+ end
+
+ # Ensures that a membership requires a user reference.
+ def test_member_is_required
+ @membership.member = nil
+ flunk "Membership requires a member." if @membership.valid?
+ end
+
+ # Ensures that a well-formed membership is valid.
+ def test_valid
+ flunk "There is a general validation error." unless @membership.valid?
+ end
+
+ # Ensures that a user cannot have two memberships to the same sprint.
+ def test_unique_memberships
+ membership = SprintMember.new(:sprint => @existing_membership.sprint,
+ :member => @existing_membership.member)
+ flunk "There is a problem with uniqueness constraints." if membership.valid?
+ end
+end
--
1.6.0.6
15 years
[PATCH] Sprints are composed of teams. #141
by Darryl L. Pierce
THIS PATCH REQUIRES A MIGRATION.
A new table and domain object, SprintMember, was created. This maps
users to the sprint for which they're a member.
Each sprint will now load its list of members and make them available
through the new "members" collection. Members can be added to the
membership list via the "sprint_members" association of Sprint.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/models/sprint.rb | 4 ++
app/models/sprint_member.rb | 27 +++++++++++++++
db/migrate/027_create_sprint_members.rb | 33 ++++++++++++++++++
test/fixtures/sprint_members.yml | 3 ++
test/unit/sprint_member_test.rb | 56 +++++++++++++++++++++++++++++++
5 files changed, 123 insertions(+), 0 deletions(-)
create mode 100644 app/models/sprint_member.rb
create mode 100644 db/migrate/027_create_sprint_members.rb
create mode 100644 test/fixtures/sprint_members.yml
create mode 100644 test/unit/sprint_member_test.rb
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index f2cb1f9..b50938f 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -64,6 +64,10 @@ class Sprint < ActiveRecord::Base
belongs_to :product
belongs_to :team_lead, :class_name => 'User', :foreign_key => :team_lead_id
+ # sprint membership
+ has_many :sprint_members, :dependent => :destroy
+ has_many :members, :through => :sprint_members
+
has_many :backlog_items,
:order => '(select priority from user_stories where id = user_story_id) ASC',
:dependent => :destroy
diff --git a/app/models/sprint_member.rb b/app/models/sprint_member.rb
new file mode 100644
index 0000000..20f8f72
--- /dev/null
+++ b/app/models/sprint_member.rb
@@ -0,0 +1,27 @@
+# sprint_member.rb
+# Copyright (C) 2009, Darryl L. Pierce <mcpierce(a)gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# 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 :member_id, :message => "A member is required."
+
+ validates_uniqueness_of :member_id, :scope => [:sprint_id]
+
+ belongs_to :sprint
+ belongs_to :member, :class_name => 'User', :foreign_key => 'member_id'
+end
diff --git a/db/migrate/027_create_sprint_members.rb b/db/migrate/027_create_sprint_members.rb
new file mode 100644
index 0000000..c6ccce5
--- /dev/null
+++ b/db/migrate/027_create_sprint_members.rb
@@ -0,0 +1,33 @@
+# 027_create_sprint_members.rb
+# Copyright (C) 2009, Darryl L. Pierce <mcpierce(a)gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+class CreateSprintMembers < ActiveRecord::Migration
+ def self.up
+ create_table :sprint_members do |t|
+ t.integer :sprint_id, :null => false
+ t.integer :member_id, :null => false
+
+ t.timestamps
+ end
+
+ add_index :sprint_members, [:sprint_id, :member_id], :unique => true, :name => :single_membership
+ end
+
+ def self.down
+ drop_table :sprint_members
+ end
+end
diff --git a/test/fixtures/sprint_members.yml b/test/fixtures/sprint_members.yml
new file mode 100644
index 0000000..8cc07a2
--- /dev/null
+++ b/test/fixtures/sprint_members.yml
@@ -0,0 +1,3 @@
+mcpierce_active_sprint:
+ sprint_id: <%= Fixtures.identify(:active_sprint) %>
+ member_id: <%= Fixtures.identify(:mcpierce) %>
diff --git a/test/unit/sprint_member_test.rb b/test/unit/sprint_member_test.rb
new file mode 100644
index 0000000..da4e3bc
--- /dev/null
+++ b/test/unit/sprint_member_test.rb
@@ -0,0 +1,56 @@
+# Copyright (C) 2009, Darryl L. Pierce <mcpierce(a)gmail.com>
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation, either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+require File.dirname(__FILE__) + '/../test_helper'
+
+class SprintTeamTest < ActiveSupport::TestCase
+ fixtures :sprint_members
+ fixtures :sprints
+ fixtures :users
+
+ def setup
+ @sprint = sprints(:inactive_sprint)
+ @team_lead = @sprint.team_lead
+ @member = users(:mcpierce)
+
+ @membership = SprintMember.new(:sprint => @sprint, :member => @member)
+ @existing_membership = sprint_members(:mcpierce_active_sprint)
+ end
+
+ # Ensures that a membership requires a sprint reference.
+ def test_sprint_is_required
+ @membership.sprint = nil
+ flunk "Membership requires a sprint." if @membership.valid?
+ end
+
+ # Ensures that a membership requires a user reference.
+ def test_member_is_required
+ @membership.member = nil
+ flunk "Membership requires a member." if @membership.valid?
+ end
+
+ # Ensures that a well-formed membership is valid.
+ def test_valid
+ flunk "There is a general validation error." unless @membership.valid?
+ end
+
+ # Ensures that a user cannot have two memberships to the same sprint.
+ def test_unique_memberships
+ membership = SprintMember.new(:sprint => @existing_membership.sprint,
+ :member => @existing_membership.member)
+ flunk "There is a problem with uniqueness constraints." if membership.valid?
+ end
+end
--
1.6.0.6
15 years
Error in migrations...
by Darryl L. Pierce
Not sure where it went, but there was a migration previously that
added the admin_site column to the user_privileges table. While
working on sprint membership today I dropped and remigrated by tables
and that column was no longer found. So I went back and modified the
migration to create the user_privileges table and added back in the
column there.
--
Darryl L. Pierce <mcpierce(a)gmail.com>
Visit the Infobahn Offramp: <http://mcpierce.multiply.com>
"Bury me next to my wife. Nothing too fancy..." - Ulysses S. Grant
15 years
[PATCH] Team leads can edit their sprints. #132 #133 #134 #140
by Darryl L. Pierce
Added a team_lead fixture for use during tests. Added team leads to the
list of users who can edit a sprint. This solves multiple tickets
related to team leads change the sprint status.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/models/sprint.rb | 2 +-
doc/ChangeLog | 4 ++++
test/fixtures/sprints.yml | 6 +++---
test/fixtures/user_privileges.yml | 3 +++
test/fixtures/users.yml | 7 +++++++
test/unit/sprint_test.rb | 13 +++++++++++++
6 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index 596580e..f2cb1f9 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -122,7 +122,7 @@ class Sprint < ActiveRecord::Base
# Returns whether the specified user can modify this sprint.
def can_edit?(user)
- user && (user.id == product.owner.id)
+ user && ((user.id == product.owner.id) || (user.id == team_lead.id))
end
# Returns whether the user can delete the current sprint.
diff --git a/doc/ChangeLog b/doc/ChangeLog
index 7e2abdc..17fd679 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -5,4 +5,8 @@ Change Log (0.2.0):
* #100 - Team leads can reopen items. - Darryl L. Pierce <mcpierce(a)gmail.com>
* #125 - Added a system title to all pages. - Darryl L. Pierce <mcpierce(a)gmail.com>
* #131 - Admins can filter out unapproved projects. - Darryl L. Pierce <mcpierce(a)gmail.com>
+ * #132 - Team leads can start sprints. - Darryl L. Pierce <mcpierce(a)gmail.com>
+ * #133 - Team leads can cancel sprints - Darryl L. Pierce <mcpierce(a)gmail.com>
+ * #134 - Team leads can move a sprint back to planning. - Darryl L. Pierce <mcpierce(a)gmail.com>
* #136 - System title can be changed by the admin. - Darryl L. Pierce <mcpierce(a)gmail.com>
+ * #140 - Team leads can complete a sprint. - Darryl L. Pierce <mcpierce(a)gmail.com>
diff --git a/test/fixtures/sprints.yml b/test/fixtures/sprints.yml
index 704c0c0..e6263c3 100644
--- a/test/fixtures/sprints.yml
+++ b/test/fixtures/sprints.yml
@@ -5,7 +5,7 @@ active_sprint:
duration: 28
goals: Get things done.
status: <%= Sprint::STATUS_ACTIVE %>
- team_lead_id: <%= Fixtures.identify(:projxp_owner) %>
+ team_lead_id: <%= Fixtures.identify(:team_lead) %>
inactive_sprint:
product_id: <%= Fixtures.identify(:projxp_web) %>
@@ -14,7 +14,7 @@ inactive_sprint:
duration: 28
goals: Got stuff done.
status: <%= Sprint::STATUS_PLANNED %>
- team_lead_id: <%= Fixtures.identify(:projxp_owner) %>
+ team_lead_id: <%= Fixtures.identify(:team_lead) %>
closed_sprint:
product_id: <%= Fixtures.identify(:projxp_web) %>
@@ -23,4 +23,4 @@ closed_sprint:
duration: 28
goals: Get more stuff done.
status: <%= Sprint::STATUS_CLOSED %>
- team_lead_id: <%= Fixtures.identify(:projxp_owner) %>
+ team_lead_id: <%= Fixtures.identify(:team_lead) %>
diff --git a/test/fixtures/user_privileges.yml b/test/fixtures/user_privileges.yml
index 0ca79ac..1c9476c 100644
--- a/test/fixtures/user_privileges.yml
+++ b/test/fixtures/user_privileges.yml
@@ -17,6 +17,9 @@ projxp_owner_privileges:
admin_projects: false
admin_users: false
+team_lead:
+ user_id: <%= Fixtures.identify(:team_lead) %>
+
teatime_owner_privileges:
user_id: <%= Fixtures.identify(:teatime_owner) %>
admin_projects: false
diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml
index 9981281..2738441 100644
--- a/test/fixtures/users.yml
+++ b/test/fixtures/users.yml
@@ -21,6 +21,13 @@ projxp_owner:
salt: <%= SALT %>
hashed_password: <%= User.encrypted_password('bonk', SALT) %>
+team_lead:
+ email: team_lead(a)projxp.org
+ display_name: Team Lead
+ introduction: I'm the team lead.
+ salt: <%= SALT %>
+ hashed_password: <%= User.encrypted_password('team lead', SALT) %>
+
teatime_owner:
email: admin(a)teatime.org
display_name: Teatime Admin
diff --git a/test/unit/sprint_test.rb b/test/unit/sprint_test.rb
index 9150dea..43c2f47 100644
--- a/test/unit/sprint_test.rb
+++ b/test/unit/sprint_test.rb
@@ -43,6 +43,9 @@ class SprintTest < ActiveSupport::TestCase
@unhealthy_backlog = [ backlog_item ]
@existing_sprint = sprints(:active_sprint)
+ @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
end
# Ensures that a sprint has to have a product.
@@ -133,4 +136,14 @@ class SprintTest < ActiveSupport::TestCase
assert_equal @new_sprint.start + (@new_sprint.duration - 1), @new_sprint.end_date,
"End date calculation is wrong."
end
+
+ # Ensures that a team lead can edit a sprint.
+ def test_edit_as_team_lead
+ flunk "Team leads must be allowed to edit sprints." unless @existing_sprint.can_edit?(@team_lead)
+ end
+
+ # Ensures that a product owner can edit a sprint.
+ def test_edit_as_product_owner
+ flunk "Product owners must be allowed to edit sprints." unless @existing_sprint.can_edit?(@owner)
+ end
end
--
1.6.0.6
15 years
[PATCH] The project requestor is sent an email on approve or reject. #137
by Darryl L. Pierce
When the project is approved or rejected, an email is sent to the
project owner notifying them of the decision.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/controllers/projects_controller.rb | 2 ++
app/models/user_mailer.rb | 8 ++++++++
doc/ChangeLog | 1 +
test/functional/projects_controller_test.rb | 14 ++++++++++++++
4 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 7a29e96..4b561cc 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -132,6 +132,7 @@ class ProjectsController < ApplicationController
Project.transaction do
@project.approved = true
@project.save!
+ UserMailer.deliver_project_request(@project, true)
flash[:message] = "#{(a)project.name} has been approved."
respond_to do |format|
@@ -144,6 +145,7 @@ class ProjectsController < ApplicationController
def reject
Project.transaction do
@project.destroy
+ UserMailer.deliver_project_request(@project, false)
flash[:message] = "#{(a)project.name} has been rejected/deleted."
respond_to do |format|
diff --git a/app/models/user_mailer.rb b/app/models/user_mailer.rb
index 5609dc8..85b3e1d 100644
--- a/app/models/user_mailer.rb
+++ b/app/models/user_mailer.rb
@@ -40,6 +40,14 @@ class UserMailer < ActionMailer::Base
body :user => user, :new_password => new_password
end
+ # Sends an email to a project requestor about the disposition of the project.
+ def project_request(project, approved)
+ recipients project.owner.email
+ from MAIL_CONFIG[:from]
+ subject "Request project: #{project.name}"
+ body :project => project, :approved => approved)
+ end
+
# Sends an email to a product owner when a user requests a product role.
def product_role_request(requested_role)
recipients requested_role.product.owner.email
diff --git a/doc/ChangeLog b/doc/ChangeLog
index 7e2abdc..534ed3f 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -6,3 +6,4 @@ Change Log (0.2.0):
* #125 - Added a system title to all pages. - Darryl L. Pierce <mcpierce(a)gmail.com>
* #131 - Admins can filter out unapproved projects. - Darryl L. Pierce <mcpierce(a)gmail.com>
* #136 - System title can be changed by the admin. - Darryl L. Pierce <mcpierce(a)gmail.com>
+ * #137 - An email is sent to the project requestor - Darryl L. Pierce <mcpierce(a)gmail.com>
diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb
index e0a229c..81312f3 100644
--- a/test/functional/projects_controller_test.rb
+++ b/test/functional/projects_controller_test.rb
@@ -39,6 +39,14 @@ class ProjectsControllerTest < ActionController::TestCase
@admin = users(:project_admin)
raise "Nonadmin and admin cannot be the same user!" if @nonadmin.id == @admin.id
raise "Admin must have project admin rights!" unless @admin.can_create_projects?
+
+ # setup for testing mail
+ ActionMailer::Base.delivery_method = :test
+ ActionMailer::Base.perform_deliveries = true
+ ActionMailer::Base.deliveries = []
+
+ @emails = ActionMailer::Base.deliveries
+ @emails.clear
end
# Ensures that showing the project index works as expected.
@@ -325,6 +333,9 @@ class ProjectsControllerTest < ActionController::TestCase
assert_redirected_to project_path(@unapproved_project)
assert Project.find_by_id((a)unapproved_project.id).approved,
"Project should have been marked as approved."
+ email = @emails.first
+ assert email, "No email was generated."
+ assert_equal @unapproved_project.owner.email, email.to[0], "Email sent to wrong user."
end
# Ensures that anonymous users can't reject a project.
@@ -366,5 +377,8 @@ class ProjectsControllerTest < ActionController::TestCase
assert_redirected_to projects_path
assert_nil Project.find_by_id((a)unapproved_project.id),
"Rejected project should have been deleted."
+ email = @emails.first
+ assert email, "No email was generated."
+ assert_equal @unapproved_project.owner.email, email.to[0], "Email sent to wrong user."
end
end
--
1.6.0.6
15 years
[PATCH] Projects that are unapproved cannot have products. #139
by Darryl L. Pierce
If the project is not approved, then the add product link is not shown
on the page. If the controller method is invoked directly then an error
is shown.
Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com>
---
app/controllers/products_controller.rb | 17 +++++++++++++++--
app/views/products/_edit.html.erb | 2 +-
app/views/projects/show.html.erb | 2 +-
test/functional/products_controller_test.rb | 19 ++++++++++++++-----
4 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/app/controllers/products_controller.rb b/app/controllers/products_controller.rb
index 3d394c9..0eaae5b 100644
--- a/app/controllers/products_controller.rb
+++ b/app/controllers/products_controller.rb
@@ -20,8 +20,9 @@
#
class ProductsController < ApplicationController
before_filter :authenticated, :except => [:index, :show]
- before_filter :load_project, :only => [:new]
- before_filter :load_product, :only => [:show, :edit, :update, :join]
+ before_filter :load_project, :only => [:new, :create]
+ before_filter :is_approved, :only => [:new, :create]
+ before_filter :load_product, :only => [:show, :edit, :update, :join]
# GET /products
def index
@@ -95,6 +96,7 @@ class ProductsController < ApplicationController
if params[:product]
Product.transaction do
@product = Product.new(params[:product])
+ @product.project = @project
if @product.project.can_create_products?(@user)
if @product.save
@@ -168,6 +170,15 @@ class ProductsController < ApplicationController
end
end
+ def is_approved
+ unless @project.approved
+ flash[:error] = "A project must be approved before it can have products."
+ respond_to do |format|
+ format.html {redirect_to project_path(@project)}
+ end
+ end
+ end
+
def load_product
@product = Product.find_by_id(params[:id])
@@ -176,6 +187,8 @@ class ProductsController < ApplicationController
flash[:message] = 'Invalid product specified.'
format.html { redirect_to error_path }
end
+ else
+ @project = @product.project
end
end
end
diff --git a/app/views/products/_edit.html.erb b/app/views/products/_edit.html.erb
index e554591..06272d3 100644
--- a/app/views/products/_edit.html.erb
+++ b/app/views/products/_edit.html.erb
@@ -8,7 +8,7 @@
<% hidden_field_tag "id", @product.id %>
<% end %>
- <%= hidden_field :product, :project_id %>
+ <%= hidden_field_tag :project, @project.id %>
<table class="edit">
<colgroup>
diff --git a/app/views/projects/show.html.erb b/app/views/projects/show.html.erb
index 8d93f4c..8308d80 100644
--- a/app/views/projects/show.html.erb
+++ b/app/views/projects/show.html.erb
@@ -19,7 +19,7 @@
<%= link_to(image_tag("icons/edit.png", :title => "Edit this project..."),
edit_project_path(@project)) if @project.can_edit?(@user) %>
<%= link_to(image_tag("icons/new.png", :title => "Create a new product..."),
- new_product_path(:project => @project)) if @project.can_create_products?(@user) %>
+ new_product_path(:project => @project)) if @project.can_create_products?(@user) && @project.approved %>
</td>
</tr>
diff --git a/test/functional/products_controller_test.rb b/test/functional/products_controller_test.rb
index f690fe6..2ebbc1e 100644
--- a/test/functional/products_controller_test.rb
+++ b/test/functional/products_controller_test.rb
@@ -27,6 +27,8 @@ class ProductsControllerTest < ActionController::TestCase
def setup
@project = projects(:projxp)
+ @unapproved_project = projects(:unapproved_project)
+ raise "Unapproved project cannot be approved!" if @unapproved_project.approved
@project_owner = @project.owner
@non_project_owner = users(:mcpierce)
@@ -44,12 +46,10 @@ class ProductsControllerTest < ActionController::TestCase
@new_product = {
:name => 'product1',
- :project_id => @project.id,
:owner_id => @project_owner.id,
:description => 'This is a new product.'
}
@product_update = {
- :project_id => @project.id,
:id => @existing_product.id.to_f,
:name => @existing_product.name + " Gold"
}
@@ -63,7 +63,7 @@ class ProductsControllerTest < ActionController::TestCase
assert_response :success
assert assigns['products'], "Failed to load any products."
- assigns['products'].each do |product|
+ assigns['products'].each do |product|
assert_equal @project.id, product.project_id,
"Failed to load the right set of products."
end
@@ -142,6 +142,15 @@ class ProductsControllerTest < ActionController::TestCase
assert_redirected_to project_path(@project)
end
+ # Ensures that a product cannot be created for an unapproved project.
+ def test_new_for_unapproved_project
+ get :new,
+ {:project => @unapproved_project.id },
+ { :user_id => @unapproved_project.owner_id}
+
+ assert_redirected_to project_path(@unapproved_project)
+ end
+
# Ensures that creating a project works as expected.
def test_new
get :new,
@@ -227,7 +236,7 @@ class ProductsControllerTest < ActionController::TestCase
# Ensures that only the project owner can call the create method.
def test_create_as_non_project_owner
put :create,
- {:product => @new_product},
+ {:product => @new_product, :project => @project.id},
{:user_id => @non_project_owner.id}
assert_redirected_to project_path(@project)
@@ -236,7 +245,7 @@ class ProductsControllerTest < ActionController::TestCase
# Ensures that creating a product work as expected.
def test_create
put :create,
- {:product => @new_product},
+ {:product => @new_product, :project => @project.id},
{:user_id => @project_owner.id}
assert_redirected_to product_path(assigns['product'])
--
1.6.0.6
15 years