Thank you for your suggestion.
I have sended the new path, but I'm not sure that the test are correct.
--
Benjamin LAN-SUN-LUK
Le 05/12/08 15:13, « Darryl Pierce » <mcpierce(a)gmail.com> a écrit :
On Fri, Dec 5, 2008 at 8:10 AM, LAN-SUN-LUK Benjamin
<Benjamin.LAN-SUN-LUK(a)supinfo.com> wrote:
Signed-off-by: Benjamin LAN-SUN-LUK <benjamin.lan-sun-luk(a)supinfo.com>
---
NAK.
There's no unit test to confirm and ensure the change works. We need
to have a test in place that tries to move a sprint with work history
back to planned to confirm that the user can't do that.
More comments below.
app/controllers/sprints_controller.rb | 22 ++++++++++++++--------
app/models/sprint.rb | 10 ++++++++++
app/views/sprints/show.html.erb | 2 +-
3 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/app/controllers/sprints_controller.rb
b/app/controllers/sprints_controller.rb
index 09ebe5b..f8f5881 100644
--- a/app/controllers/sprints_controller.rb
+++ b/app/controllers/sprints_controller.rb
@@ -87,15 +87,21 @@ class SprintsController < ApplicationController
def update
respond_to do |format|
if @sprint.can_edit?(@user)
- @sprint.update_attributes(params[:sprint])
-
- if @sprint.save
- flash[:message] = "Sprint updated successfully."
- format.html { redirect_to params[:url] ? params[:url] :
product_sprints_path(@product) }
+ unless @sprint.allowed_status?(params[:sprint][:status] ||
@sprint.status)
+ flash[:error] = "You cannot change the status because you got
tasks against the sprint"
Make the error message here to "Sprints with work history cannot be
moved to the planned state."
+ format.html { redirect_to :action => :show }
else
- @title = "Sprint #{(a)sprint.id} (Edit)"
- @sprint.valid?
- format.html { render :action => :edit }
+
+ @sprint.update_attributes(params[:sprint])
+
+ 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}."
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index 2638613..da092ee 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -171,4 +171,14 @@ class Sprint < ActiveRecord::Base
return false
end
+
+ # Check if the sprint is active and have tasks againsts him.
+ def has_activity?
+ status == STATUS_ACTIVE && !actual_hours.zero?
+ end
+
+ # Check if the sprint can move to this new status
+ def allowed_status?(new_status)
+ !(has_activity? && new_status.to_i == STATUS_PLANNED)
+ end
end
diff --git a/app/views/sprints/show.html.erb
b/app/views/sprints/show.html.erb
index 0b826c6..56e255e 100644
--- a/app/views/sprints/show.html.erb
+++ b/app/views/sprints/show.html.erb
@@ -49,7 +49,7 @@
<% form_for(:sprint, @sprint, :url =>
product_sprint_path(@product, @sprint),
:html => {:method => :put}) do |form| %>
<%= hidden_field_tag :url, product_sprint_url(@product,
@sprint) %>
- <%= select :sprint, :status, Sprint::STATUS_TEXT %>
+ <%= select :sprint, :status, Sprint::STATUS_TEXT -
@sprint.has_activity? ? [Sprint::STATUS_TEXT[Sprint::STATUS_PLANNED]] : []
%>
This line causes:
49: <% form_for(:sprint, @sprint, :url =>
product_sprint_path(@product, @sprint),
50: :html => {:method => :put}) do |form| %>
51: <%= hidden_field_tag :url,
product_sprint_url(@product, @sprint) %>
52: <%= select :sprint, :status, Sprint::STATUS_TEXT -
@sprint.has_activity? ? [Sprint::STATUS_TEXT[Sprint::STATUS_PLANNED]]
: [] %>
53: <%= submit_tag "Apply" %>
54: <% end %>
55: <% else %>
app/views/sprints/show.html.erb:52:in `-'
app/views/sprints/show.html.erb:52:in
`_run_erb_47app47views47sprints47show46html46erb'
/usr/lib/ruby/gems/1.8/gems/actionpack-2.1.1/lib/action_view/helpers/form_helper.rb:317:in
`fields_for'
/usr/lib/ruby/gems/1.8/gems/actionpack-2.1.1/lib/action_view/helpers/form_helper.rb:253:in
`form_for'
app/views/sprints/show.html.erb:49:in
`_run_erb_47app47views47sprints47show46html46erb'
The error message is "can't convert false into Array ". However, with
the follow change for line 52:
<%= select :sprint, :status, Sprint::STATUS_TEXT -
(@sprint.has_activity? ?
[Sprint::STATUS_TEXT[Sprint::STATUS_PLANNED],Sprint::STATUS_PLANNED] :
[]) %>
it works as desired.
<%= submit_tag "Apply" %>
<% end %>
<% else %>
--
1.6.0.2
--
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
_______________________________________________
projxp-devel mailing list
projxp-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/projxp-devel