[PATCH] Sprint status is updated separate from the details page #82

[Darryl L. Pierce]

When the user selects to set the status, they're brought to a page and
asked to confirm the change.

Signed-off-by: Darryl L. Pierce <mcpierce(a)gmail.com&gt;
---
 app/controllers/sprints_controller.rb      |   35 ++++++++++++++
 app/models/sprint.rb                       |   19 +++++++-
 app/models/user_story.rb                   |    5 ++
 app/views/sprints/show.html.erb            |   29 +++++++-----
 app/views/sprints/status.html.erb          |   37 +++++++++++++++
 config/routes.rb                           |    9 +++-
 public/images/icons/sprint_cancel.png      |  Bin 0 -> 700 bytes
 public/images/icons/sprint_complete.png    |  Bin 0 -> 623 bytes
 public/images/icons/sprint_start.png       |  Bin 0 -> 670 bytes
 public/images/icons/story_view.png         |  Bin 0 -> 465 bytes
 test/functional/sprints_controller_test.rb |   67 ++++++++++++++++++++++++++++
 11 files changed, 188 insertions(+), 13 deletions(-)
 create mode 100644 app/views/sprints/status.html.erb
 create mode 100644 public/images/icons/sprint_cancel.png
 create mode 100644 public/images/icons/sprint_complete.png
 create mode 100644 public/images/icons/sprint_start.png
 create mode 100644 public/images/icons/story_view.png

diff --git a/app/controllers/sprints_controller.rb
b/app/controllers/sprints_controller.rb
index 09ebe5b..c836acd 100644
--- a/app/controllers/sprints_controller.rb
+++ b/app/controllers/sprints_controller.rb
@@ -20,6 +20,7 @@ class SprintsController < ApplicationController
   before_filter :authenticated, :except => [:index, :show]
   before_filter :load_product
   before_filter :load_sprint, :except => [:index, :new, :create]
+  before_filter :get_status, :only => [:status]
 
   # GET /products/1/sprints
   def index
@@ -104,6 +105,29 @@ class SprintsController < ApplicationController
     end
   end
 
+  # PUT /products/1/sprints/1/status
+  def status
+    respond_to do |format|
+      if @sprint.can_edit?(@user)
+	if @sprint.allowed_status?(@status)
+	  Sprint.transaction do
+	    @sprint.status = @status
+	    @sprint.save!
+
+	    flash[:message] = "Sprint moved to #{(a)sprint.status_text}.&quot;
+	    format.html { redirect_to product_sprint_path(@product, @sprint) }
+	  end
+	else
+	  flash[:error] = "You cannot move the sprint to #{(a)sprint.status_text}.&quot;
+	  format.html { redirect_to product_sprint_path(@product, @sprint) }
+	end
+      else
+	flash[:error] = "You are not allowed to change this sprint's status."
+	format.html { redirect_to product_sprint_path(@product, @sprint) }
+      end
+    end
+  end
+
   # DELETE /products/1/sprints/1/destroy
   def destroy
     respond_to do |format|
@@ -216,4 +240,15 @@ class SprintsController < ApplicationController
       end
     end
   end
+
+  def get_status
+    @status = params[:status].to_i
+
+    unless @status && (0...Sprint::STATUS_TEXT.size).include?(@status)
+      flash[:error] = "Missing or invalid status."
+      respond_to do |format|
+	format.html { redirect_to product_sprint_path(@product, @sprint) }
+      end
+    end
+  end
 end
diff --git a/app/models/sprint.rb b/app/models/sprint.rb
index 2638613..1fffb5b 100644
--- a/app/models/sprint.rb
+++ b/app/models/sprint.rb
@@ -124,7 +124,24 @@ class Sprint < ActiveRecord::Base
 
   # Returns whether the specified user is allowed to populate this sprint.
   def can_populate?(user)
-    user && (user.id == product.owner.id)
+    user && (user.id == product.owner.id) && pending?
+  end
+
+  # Returns whether the sprint can be moved to the given status.
+  def allowed_status?(status)
+    case self.status
+    when STATUS_PLANNED:
+	return true if status == STATUS_ACTIVE
+    when STATUS_ACTIVE:
+	if(status == STATUS_PLANNED && actual_hours == 0) ||
+	    ([STATUS_CANCELED, STATUS_CLOSED].include?(status))
+	  return true
+	end
+    when STATUS_CLOSED:   return true if status == STATUS_ACTIVE
+    when STATUS_CANCELED: return true if status == STATUS_ACTIVE
+    end
+
+    return false
   end
 
   # Returns whether the sprint is pending.
diff --git a/app/models/user_story.rb b/app/models/user_story.rb
index f8b3963..ba6c648 100644
--- a/app/models/user_story.rb
+++ b/app/models/user_story.rb
@@ -51,4 +51,9 @@ class UserStory < ActiveRecord::Base
   def can_be_deleted?
     backlog_items.empty?
   end
+
+  # Returns whether the user story is closed.
+  def closed?
+    closed
+  end
 end
diff --git a/app/views/sprints/show.html.erb b/app/views/sprints/show.html.erb
index 0b826c6..7f2c911 100644
--- a/app/views/sprints/show.html.erb
+++ b/app/views/sprints/show.html.erb
@@ -45,17 +45,24 @@
     <tr>
       <td class="label">Status:</td>
       <td class="value">
-        <% if @sprint.can_edit?(@user) %>
-          <% 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 %>
-            <%= submit_tag "Apply" %>
-          <% end %>
-        <% else %>
-          <%= Sprint::STATUS_TEXT[@sprint.status] %>
-        <% end %>
-      </td>
+	<%= @sprint.status_text %>
+	<% if @sprint.can_edit?(@user) %>
+	[
+	  <%= link_to(image_tag("icons/sprint_plan.png", :title => "Move
sprint back to planning..."),
+	    status_product_sprint_path(@product, @sprint, :status =>
Sprint::STATUS_PLANNED),
+            :method => :put) if @sprint.allowed_status?(Sprint::STATUS_PLANNED) %>
+	  <%= link_to(image_tag("icons/sprint_start.png", :title => "Start
this sprint..."),
+	    status_product_sprint_path(@product, @sprint, :status => Sprint::STATUS_ACTIVE),
+            :method => :put) if @sprint.allowed_status?(Sprint::STATUS_ACTIVE) %>
+	  <%= link_to(image_tag("icons/sprint_complete.png", :title =>
"Mark this sprint as completed..."),
+	    status_product_sprint_path(@product, @sprint, :status => Sprint::STATUS_CLOSED),
+            :method => :put) if @sprint.allowed_status?(Sprint::STATUS_CLOSED) %>
+	  <%= link_to(image_tag("icons/sprint_cancel.png", :title =>
"Cancel this sprint..."),
+	    status_product_sprint_path(@product, @sprint, :status =>
Sprint::STATUS_CANCELED),
+            :method => :put) if @sprint.allowed_status?(Sprint::STATUS_CANCELED)
%>
+	]
+	<% end %>
+	</td>
     </tr>
 
     <% if @sprint.can_view_burndown? %>
diff --git a/app/views/sprints/status.html.erb b/app/views/sprints/status.html.erb
new file mode 100644
index 0000000..4a32fd9
--- /dev/null
+++ b/app/views/sprints/status.html.erb
@@ -0,0 +1,37 @@
+<% form_for(:sprint, @sprint, :url => update_status_product_sprint_path(@product,
@sprint),
+  :html => {:method => :put}) do |form| %>
+
+  <fieldset id="status">
+    <legend>Update Sprint Status</legend>
+
+    <table class="edit">
+      <colgroup>
+        <col class="label" />
+        <col class="value" />
+      </colgroup>
+
+      <tbody>
+        <tr>
+          <td class="label-required">Title</td>
+          <td class="value">
+            <%= form.text_field :title %>
+            <%= error_message_on(:sprint, :title) %>
+          </td>
+        </tr>
+
+        <tr>
+          <td class="label-required">Change status to</td>
+          <td class="value"><%= "#{@status_text}"
%></td>
+        </tr>
+
+        <tr>
+          <td class="buttons" colspan="2">
+            <%= submit_tag "Save" %>
+          </td>
+        </tr>
+      </tbody>
+    </table>
+
+  </fieldset>
+
+<% end %>
diff --git a/config/routes.rb b/config/routes.rb
index 503e7e8..f37405b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -20,7 +20,14 @@ ActionController::Routing::Routes.draw do |map|
   map.resources :products do |product|
     product.resources :roles
     product.resources :stories
-    product.resources(:sprints, :member => {:plan => :get, :populate => :post,
:roles => :get}) do |sprint|
+    product.resources(:sprints, :member =>
+		      {
+			:plan          => :get,
+			:populate      => :post,
+			:roles         => :get,
+			:status        => :put
+		      }
+		      ) do |sprint|
       sprint.resources :items, :member =>
         {
         :accept   => :get,
diff --git a/public/images/icons/sprint_cancel.png
b/public/images/icons/sprint_cancel.png
new file mode 100644
index 0000000000000000000000000000000000000000..0cfd585963d255190b8855a7689e8da1c4d7cf6b
GIT binary patch
literal 700
zcmV;t0z>_YP)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00004XF*Lt006JZ
zHwB960000PbVXQnQ*UN;cVTj606}DLVr3vnZDD6+Qe|Oed2z{QJOBU!PDw;TR5;6}
zl08pUK@^6c-MfIG7zy&ZdO=W0RM5go6AL;JW9dJzHAaJtjTV}aNP&t#jK9E25`KYJ
z7-190qI|g^MC3yjcJIA2b7#h4m*qn(j3+rcCzCnP`_AM<thM}4#10;;z8Sh6DK2^%
ziAG(kwZIsx(Ir;M1)a$}vRV(<KX@qr2v`|>*?Fk0YVb%?UEFajs1S?+YtYiPrjx0+
z+4<Bxpm%2`vh#$MS1&q>YbyJXwz!SX#yqTlhtNQ%Ku9=RNm$j)&+(}lZ!UGGp|@|O
z09YA#-dR#rIaGe;MBLe!ht*}!c?U}6YT!dfHDO%~>xtx&Klk-^WB==sC_vP4ddg4L
z#GN10u$+QGf$!(i3&8VpF6O6+ef~&gQ#>AVqCJH_utvKMAuOeG%3%mn<<%9)yb~#4
zHc70e5sYyQ03$?zFUko7D1Bg1=6jXvg#bUm1b(pVKuC*}koEKGdj<=zd<p?lVuY}}
z02!2`6>M#RWsl+kfRf;OU^G_BQh+Fc$z&F_AHuQYu(b)<Z0@4xK{j7~54O@!dlDrz
zdV71Z08s1e-K%5C<<PwV6}7OL&Ox&ILC?pe)0lb}|Ke4?jgBveYir&67HRs{%^PJ6
zZeR7KbB#q~?_p*Ntij5VPzbEsjJkY{l@Ft{naPQ}ZHX_#`v3q_9qnD^r<(?<&!3GK
z8HH8~V@z(R46J^dAd^VkZ=0Nb_S-7&M6&%#ms585NhIdDHVlQ)nus+e+V>aq=H_Fx
idDl8IBmWBc*Z2i=4uSP&;Q8VJ0000<MNUMnLSTZ|>p<iH

literal 0
HcmV?d00001

diff --git a/public/images/icons/sprint_complete.png
b/public/images/icons/sprint_complete.png
new file mode 100644
index 0000000000000000000000000000000000000000..0156c266e4e1fbcd1263fab2c2dd1f3712553d14
GIT binary patch
literal 623
zcmV-#0+9WQP)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00004XF*Lt006JZ
zHwB960000PbVXQnQ*UN;cVTj606}DLVr3vnZDD6+Qe|Oed2z{QJOBU!0ZBwbR5;6}
zlif?xQ5eU)^D^-Kemg@D!Y+!S3;%@vf@v>Wtv3@TMo36t?!us(ut2(qE`#u;Y2=pT
zq@2~2T~yXyeA(Qqy5;+9o4@n>4e8r+MnSPzjxHXahjaKm&-eRqP6dF9|A~@^;+?rt
z=H0jE_T4vTwd@82LFU{PN7lpd$oktP$3Gho==lu{2>(S8IKK0m@1<YyO?kI9VLH0$
zHr14Dke%Y+Gaio2BoF0Nk03)2cS29x1A#6Au`zB!kph~=3A`n1nuh$>#{_mk%-f(Z
z+<+0WLyL4{ZOnvyEl7}LBImWjh~I^nZG{LlL-;RKT!Im`W9MTdQZ5xqr;6nfwX(<p
z$xIhy(^L_-S^zcQd1&6Vq~;oO!43iyXORN(=9b<vGZ&Cg_Cn0HK`v19og?@Rv@aG?
zWF;H7vGLx5_;UrZVWp}_!Rn~`Fzde#J=jj5gijY>&09}<xQX>?3!+0xOT@w2!VY%$
zj|Qoisx?Xewm&q%SbjiFY^0(I!gmV#q77T0t|0EJMfe#*$tA9gs83{GdZFA;x{o96
zB^%K$7%@AxKer&}ti|FWs}2pYzTg0Ry6;OQZ&K!w#ON`Q%*5Js144tWhd>1Tnd7f7
zk-!t=?~tr%T0tsJ(--<#x2)&;s)i5r7k+}jpgM|^Wqqb{;s;8N-ZpQB0Ez$r002ov
JPDHLkV1fyHAhiGh

literal 0
HcmV?d00001

diff --git a/public/images/icons/sprint_start.png b/public/images/icons/sprint_start.png
new file mode 100644
index 0000000000000000000000000000000000000000..b88c8578956ceec4ff17f81995b8652f6aa2b58d
GIT binary patch
literal 670
zcmV;P0%84$P)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00004XF*Lt006JZ
zHwB960000PbVXQnQ*UN;cVTj606}DLVr3vnZDD6+Qe|Oed2z{QJOBU!FiAu~R5;6(
zlFdt$Q543Xd*AoIGtP{SV`!p;RDLwHXb}ZrB5nE)LV~1K+h`F41>rx?szq&Dw38OK
zY!^{rCAFy_2z8TV&4=Ube7+y|oYO*02OOyb5BD7I^ZdAQt`ZS+tMaFrb6^=AxbXHx
zH;=|4CCm%L{PZwSS3v3G^sH+#W3JcR_xs(&`Tqt8^J9}d0<R|rCcamAect<LKyh(u
z-Xya*?IZvI#_Kg=`Po5URsKAa5tggMd<86Lg=N(AWK)TBKRBv@gF2Kug0^2o*!0^%
zQV`;=!u&>vU#im5^f#04JL4qMaI^<NL$QE@B5bn&i7f;fXfpzdEhq|M!vJyELfEQR
zl!&1Cerm<b^q)*-kAa)3;BX6UEhM(!9Z5_OQgBj}2pXGc*Q>seoYDXwB>7;oyw=|M
z1!ayym?6XvqV3ae_f95{py8ukt2TxB^!VIzRRh4#rNu~y^X+P>L{SXo3_|Qqm>9wY
zz(9!5s#OBElpmj4DRyjO`0`RiEIkUg%7D)8y}}Ye3}prow;JG>UQOIs{kfZSJ9bYz
zskMPbH9)1H6FDf)1=ZKVfe+;jf`a(O{!9meiN~~d0iA$0qX=t0D6Ydx4#RO76h@#R
z9_k7Z;$fv6G>QeZ{Yu0n&xL4%!?l}UPj4!j&Vs@?dl=y8#_IQ`5I-5a_T$dJtJ_~5
z4&186>klZh{hfba<gb<&Ca&+F57N^8<m`vL#@n6$02)Sf{tFKxDF6Tf07*qoM6N<$
Eg88vBhX4Qo

literal 0
HcmV?d00001

diff --git a/public/images/icons/story_view.png b/public/images/icons/story_view.png
new file mode 100644
index 0000000000000000000000000000000000000000..3bc0bd32fceb21d70368f7842a00a53d6369ba48
GIT binary patch
literal 465
zcmV;?0WSWDP)<h;3K|Lk000e1NJLTq000mG000mO1^@s6AM^iV00004XF*Lt006JZ
zHwB960000PbVXQnQ*UN;cVTj606}DLVr3vnZDD6+Qe|Oed2z{QJOBUzV@X6oR5;6}
zlif?gU=+sv7bPi)=!3$7q?JQqQvwMoh(bhK+J{k6%rsgQVNn!z(HBXQkd^mAH(hwA
z^Yotc=4S08nh!j@F!ub;!`WU30603a!#SmnFKE?TaOVx=ZRYd~NSn_P*eHX4{Rzai
z68Mrum`pr?uyhCB_zlR(s~YAA5Xn~cSpFUUYax26696aMv1k4Q2q33l!H$M!&p1HZ
zs?dXQ!8A(HJcd!rMboIk$jLW=EvyAdm3{&e#VDQ4W|M-siV5fsA9Db1`>zJNu3H-P
zO&@UpeyZQXi7jKe-Hk?r-sue;aDce_XqkvXP+W#F_*ot`jB?BS93Uw71|U^ZjLH<w
zh;-sq3Vy5@0GB_@0Tc0CO9QIe)}UUmTN-qU84mEqu5JAXPM->`yP%FO7U<6!nLCG}
z$SDlW<k^-FX;JQ=20hXqbO&;*_AZ;O0?VLP0(5*EI|Y0J3Jfj)lKW#`00000NkvXX
Hu0mjf{J_IH

literal 0
HcmV?d00001

diff --git a/test/functional/sprints_controller_test.rb
b/test/functional/sprints_controller_test.rb
index 3bb0eda..836f093 100644
--- a/test/functional/sprints_controller_test.rb
+++ b/test/functional/sprints_controller_test.rb
@@ -474,4 +474,71 @@ class SprintsControllerTest < ActionController::TestCase
     assert
BacklogItem.find_by_sprint_id_and_user_story_id(@pending_sprint.id,(a)user_story.id),
       "Backlog item should have been created"
   end
+
+  # Ensures that an anonymous user can't change a sprint's status.
+  def test_status_as_anonymous
+    put :status
+
+    assert_redirected_to login_path
+  end
+
+  # Ensures that a valid product is required.
+  def test_status_with_invalid_product
+    put :status, {}, {:user_id => @owner.id}
+
+    assert_redirected_to products_url
+  end
+
+  # Ensures that a valid sprint is required.
+  def test_status_with_invalid_sprint
+    put :status, {:product_id => @product.id}, {:user_id => @owner.id}
+
+    assert_redirected_to product_sprints_path(@product)
+  end
+
+  # Ensures that non-product owners can't alter a sprint's status.
+  def test_status_as_nonowner
+    put :status,
+      {:product_id => @product.id, :id => @active_sprint.id},
+      {:user_id => @nonowner.id}
+
+    assert_redirected_to product_sprint_path(@product, @active_sprint)
+  end
+
+  # Ensures that a status must be presented.
+  def test_status_without_state
+    put :status,
+      {:product_id => @product.id, :id => @active_sprint.id},
+      {:user_id => @owner.id}
+
+    assert_redirected_to product_sprint_path(@product, @active_sprint)
+  end
+
+  # Ensures that if a status is not allowed then the sprint's not updated.
+  def test_status_with_disallowed_status
+    put :status,
+      {:product_id => @product.id, :id => @active_sprint.id, :status =>
Sprint::STATUS_PLANNED},
+      {:user_id => @owner.id}
+
+    assert_redirected_to product_sprint_path(@product, @active_sprint)
+    assert_equal @active_sprint.status, Sprint.find_by_id((a)active_sprint.id).status,
+      "Sprint status should not have been updated."
+  end
+
+  # Ensures that a status can be updated as allowed.
+  def test_status
+    put :status,
+      {:product_id => @product.id, :id => @active_sprint.id, :status =>
Sprint::STATUS_CLOSED},
+      {:user_id => @owner.id}
+
+    assert_redirected_to product_sprint_path(@product, @active_sprint)
+    result = Sprint.find_by_id((a)active_sprint.id)
+    assert result.closed?, "Sprint status was not updated."
+    # ensure all backlog items were closed
+    result.backlog_items.each do |item|
+      if item.completed?
+	assert item.user_story.closed?, "User story should have been closed."
+      end
+    end
+  end
 end
-- 
1.6.0.4

[LAN-SUN-LUK Benjamin]

ACK - Ok it works ;). After you push, I will work on #71.

--
Benjamin LAN-SUN-LUK



Le 08/12/08 20:03, « Darryl L. Pierce » <mcpierce(a)gmail.com&gt; a écrit :

When the user selects to set the status, they're brought to a page and
asked to confirm the change.

_______________________________________________
projxp-devel mailing list
projxp-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/projxp-devel

[Darryl L. Pierce]

On Mon, Dec 8, 2008 at 7:01 PM, LAN-SUN-LUK Benjamin
<Benjamin.LAN-SUN-LUK(a)supinfo.com&gt; wrote:
>  ACK – Ok it works ;). After you push, I will work on #71. 
Cool, thanks for testing. This is now pushed.

-- 
Darryl L. Pierce <mcpierce(a)gmail.com&gt;
Visit the Infobahn Offramp: <http://mcpierce.multiply.com>
"Bury me next to my wife. Nothing too fancy..." - Ulysses S. Grant