split up and refactor TicketController#update

close and open actions for plain opening and closing the tickets
respond_with so fields are not cleared on invalid update

the custom actions are not strictly restful. But adding a subresource felt like too much overhead and is conceptually hard to grasp (so we destroy the openess of the ticket to close it?).

8 files changed, 78 insertions, 50 deletions

diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb
index 19663c3..bb98277 100644
--- a/engines/support/app/controllers/tickets_controller.rb
+++ b/engines/support/app/controllers/tickets_controller.rb
@@ -5,8 +5,8 @@ class TicketsController < ApplicationController
   #has_scope :open, :type => boolean
 
   before_filter :require_login, :only => [:index]
-  before_filter :fetch_ticket, :only => [:show, :update, :destroy]
-  before_filter :require_ticket_access, :only => [:show, :update, :destroy]
+  before_filter :fetch_ticket, except: [:new, :create, :index]
+  before_filter :require_ticket_access, except: [:new, :create, :index]
   before_filter :fetch_user
   before_filter :set_title
 
@@ -37,33 +37,32 @@ class TicketsController < ApplicationController
     end
   end
 
-  def update
-    if params[:button] == 'close'
-      @ticket.is_open = false
-      @ticket.save
-      redirect_to_tickets
-    elsif params[:button] == 'open'
-      @ticket.is_open = true
-      @ticket.save
-      redirect_to auto_ticket_path(@ticket)
-    else
-      @ticket.attributes = cleanup_ticket_params(params[:ticket])
+  def close
+    @ticket.close
+    @ticket.save
+    redirect_to redirection_path
+  end
 
-      if params[:button] == 'reply_and_close'
-        @ticket.close
-      end
+  def open
+    @ticket.reopen
+    @ticket.save
+    redirect_to redirection_path
+  end
 
-      if @ticket.comments_changed?
-        @ticket.comments.last.posted_by = current_user.id
-        @ticket.comments.last.private = false unless admin?
-      end
+  def update
+    @ticket.attributes = cleanup_ticket_params(params[:ticket])
 
-      if @ticket.changed? and @ticket.save
-        redirect_to_tickets
-      else
-        redirect_to auto_ticket_path(@ticket)
-      end
+    if params[:button] == 'reply_and_close'
+      @ticket.close
+    end
+
+    if @ticket.comments_changed?
+      @ticket.comments.last.posted_by = current_user.id
+      @ticket.comments.last.private = false unless admin?
     end
+
+    @ticket.save
+    respond_with @ticket, location: redirection_path
   end
 
   def index
@@ -90,19 +89,14 @@ class TicketsController < ApplicationController
   private
 
   #
-  # redirects to ticket index, if appropriate.
-  # otherwise, just redirects to @ticket
+  # ticket index, if appropriate.
+  # otherwise, just @ticket
   #
-  def redirect_to_tickets
-    if logged_in?
-      if params[:button] == t(:reply_and_close)
-        redirect_to auto_tickets_path
-      else
-        redirect_to auto_ticket_path(@ticket)
-      end
+  def redirection_path
+    if logged_in? && params[:button] == t(:reply_and_close)
+      auto_tickets_path
     else
-      # if we are not logged in, there is no index to view
-      redirect_to auto_ticket_path(@ticket)
+      auto_ticket_path(@ticket)
     end
   end
 
diff --git a/engines/support/app/models/ticket.rb b/engines/support/app/models/ticket.rb
index bf5df53..161507c 100644
--- a/engines/support/app/models/ticket.rb
+++ b/engines/support/app/models/ticket.rb
@@ -39,6 +39,8 @@ class Ticket < CouchRest::Model::Base
   # * valid email address
   validates :email, :allow_blank => true, :format => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/
 
+  #  validates :comments, presence: true
+
   def self.search(options = {})
     @selection = TicketSelection.new(options)
     @selection.tickets
diff --git a/engines/support/app/views/tickets/_comments.html.haml b/engines/support/app/views/tickets/_comments.html.haml
new file mode 100644
index 0000000..0a3b345
--- /dev/null
+++ b/engines/support/app/views/tickets/_comments.html.haml
@@ -0,0 +1,8 @@
+%table.table.table-striped.table-bordered
+  %tbody
+    = render :partial => 'tickets/comment', :collection => @ticket.comments
+    %tr
+      %td.user
+        = current_user.login || t(:anonymous)
+      %td.comment
+        = render 'tickets/new_comment_form'
diff --git a/engines/support/app/views/tickets/_edit_form.html.haml b/engines/support/app/views/tickets/_edit_form.html.haml
index b8da779..22815f2 100644
--- a/engines/support/app/views/tickets/_edit_form.html.haml
+++ b/engines/support/app/views/tickets/_edit_form.html.haml
@@ -17,17 +17,18 @@
     regarding_user_link = ''
   end
 
-= simple_form_for @ticket do |f|
+- url = url_for([@ticket.is_open? ? :close : :open, @ticket])
+= simple_form_for @ticket, url: url do |f|
   = hidden_ticket_fields
   %p.first
     - if @ticket.is_open?
       %span.label.label-info
         %b{style: 'padding:10px'}= t(:open)
-        = f.button :loading, t(:close), value: 'close', class: 'btn-mini'
+        = f.button :loading, t(:close), class: 'btn-mini'
     - else
       %span.label.label-success
         %b{style: 'padding:10px'}= t(:closed)
-        = f.button :loading, t(:open), value: 'open', class: 'btn-mini'
+        = f.button :loading, t(:open), class: 'btn-mini'
     %span.label.label-clear= t(:created_by_on, :user => created_by, :time => @ticket.created_at.to_s(:short)).html_safe
 = simple_form_for @ticket do |f|
   = hidden_ticket_fields
diff --git a/engines/support/app/views/tickets/edit.html.haml b/engines/support/app/views/tickets/edit.html.haml
new file mode 100644
index 0000000..99afa2a
--- /dev/null
+++ b/engines/support/app/views/tickets/edit.html.haml
@@ -0,0 +1,5 @@
+- @show_navigation = params[:user_id].present?
+
+.ticket
+  = render 'tickets/edit_form'
+  = render 'tickets/comments'
diff --git a/engines/support/app/views/tickets/show.html.haml b/engines/support/app/views/tickets/show.html.haml
index 4f3c127..99afa2a 100644
--- a/engines/support/app/views/tickets/show.html.haml
+++ b/engines/support/app/views/tickets/show.html.haml
@@ -2,11 +2,4 @@
 
 .ticket
   = render 'tickets/edit_form'
-  %table.table.table-striped.table-bordered
-    %tbody
-      = render :partial => 'tickets/comment', :collection => @ticket.comments
-      %tr
-        %td.user
-          = current_user.login || t(:anonymous)
-        %td.comment
-          = render 'tickets/new_comment_form'
+  = render 'tickets/comments'
diff --git a/engines/support/config/routes.rb b/engines/support/config/routes.rb
index 23e0c11..ca471b3 100644
--- a/engines/support/config/routes.rb
+++ b/engines/support/config/routes.rb
@@ -1,8 +1,16 @@
 Rails.application.routes.draw do
-  scope "(:locale)", :locale => MATCH_LOCALE do
-    resources :tickets, :except => :edit
+  scope "(:locale)", locale: MATCH_LOCALE do
+
+    resources :tickets, except: :edit do
+      member do
+        put 'open'
+        put 'close'
+      end
+    end
+
     resources :users do
-      resources :tickets, :except => :edit
+      resources :tickets, except: :edit
     end
+
   end
 end
diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb
index e103d01..1d074cc 100644
--- a/engines/support/test/functional/tickets_controller_test.rb
+++ b/engines/support/test/functional/tickets_controller_test.rb
@@ -112,5 +112,22 @@ class TicketsControllerTest < ActionController::TestCase
     assert_not_nil assigns(:ticket).comments.first.posted_by
     assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id
   end
+
+  test "close ticket" do
+    login
+    open_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    post :close, id: open_ticket.id
+    assert !open_ticket.reload.is_open
+  end
+
+  test "reopen ticket" do
+    login
+    open_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id, is_open: false
+    post :open, id: open_ticket.id
+    assert open_ticket.reload.is_open
+  end
+
 end