Merge tag '0.5.2'

20 files changed, 495 insertions, 317 deletions

diff --git a/engines/support/app/controllers/tickets_controller.rb b/engines/support/app/controllers/tickets_controller.rb
index 99357ab..1ccbd16 100644
--- a/engines/support/app/controllers/tickets_controller.rb
+++ b/engines/support/app/controllers/tickets_controller.rb
@@ -4,10 +4,10 @@ class TicketsController < ApplicationController
   respond_to :html, :json
   #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_user
+  before_filter :require_login, :only => [:index]
+  before_filter :fetch_ticket, except: [:new, :create, :index]
+  before_filter :require_ticket_access, except: [:new, :create]
   before_filter :set_title
 
   def new
@@ -23,13 +23,13 @@ class TicketsController < ApplicationController
     @ticket.comments.last.posted_by = current_user.id
     @ticket.comments.last.private = false unless admin?
     @ticket.created_by = current_user.id
-    if @ticket.save
-      flash[:notice] = t(:thing_was_successfully_created, :thing => t(:ticket))
-      if !logged_in?
-        flash[:notice] += " " + t(:access_ticket_text, :full_url => ticket_url(@ticket.id))
-      end
+    flash_for @ticket
+    if @ticket.save && !logged_in?
+      flash[:success] += t 'tickets.access_ticket_text',
+        full_url: ticket_url(@ticket.id),
+        default: ""
     end
-    respond_with(@ticket, :location => auto_ticket_path(@ticket))
+    respond_with @ticket, :location => auto_ticket_path(@ticket)
   end
 
   def show
@@ -40,35 +40,33 @@ 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
-        flash[:notice] = t(:changes_saved)
-        redirect_to_tickets
-      else
-        flash[:error] = @ticket.errors.full_messages.join(". ") if @ticket.changed?
-        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
+
+    flash_for @ticket, with_errors: true
+    @ticket.save
+    respond_with @ticket, location: redirection_path
   end
 
   def index
@@ -85,25 +83,20 @@ class TicketsController < ApplicationController
   protected
 
   def set_title
-    @title = t(:tickets)
+    @title = t("layouts.title.tickets")
   end
 
   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
 
@@ -136,14 +129,24 @@ class TicketsController < ApplicationController
   end
 
   def ticket_access?
-    admin? or
-      @ticket.created_by.blank? or
-      current_user.id == @ticket.created_by
+    admin? or (
+      @ticket &&
+      @ticket.created_by.blank?
+    ) or (
+      @ticket &&
+      @ticket.created_by == current_user.id
+    ) or (
+      @ticket.nil? &&
+      @user &&
+      @user.id == current_user.id
+    )
   end
 
   def fetch_user
     if params[:user_id]
       @user = User.find(params[:user_id])
+    else
+      @user = current_user
     end
   end
 
@@ -153,7 +156,7 @@ class TicketsController < ApplicationController
   def search_options(params)
     params.merge(
       :admin_status => params[:user_id] ? 'mine' : 'all',
-      :user_id      => @user ? @user.id : current_user.id,
+      :user_id      => @user.id,
       :is_admin     => admin?
     )
   end
diff --git a/engines/support/app/helpers/tickets_helper.rb b/engines/support/app/helpers/tickets_helper.rb
index 7af50d6..11b02e4 100644
--- a/engines/support/app/helpers/tickets_helper.rb
+++ b/engines/support/app/helpers/tickets_helper.rb
@@ -35,13 +35,7 @@ module TicketsHelper
   #
 
   def link_to_status(new_status)
-    if new_status == "open"
-      label = t(:open_tickets)
-    elsif new_status == "closed"
-      label = t(:closed_tickets)
-    elsif new_status == "all"
-      label = t(:all_tickets)
-    end
+    label = t(:".#{new_status}", cascade: true)
     link_to label, auto_tickets_path(:open_status => new_status, :sort_order => search_order)
   end
 
@@ -62,11 +56,7 @@ module TicketsHelper
       direction = 'desc'
     end
 
-    if order_field == 'updated'
-      label = t(:updated)
-    elsif order_field == 'created'
-      label = t(:created)
-    end
+    label = t(:".#{order_field}", cascade: true)
 
     link_to auto_tickets_path(:sort_order => order_field + '_at_' + direction, :open_status => search_status) do
       arrow + label
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/models/ticket_comment.rb b/engines/support/app/models/ticket_comment.rb
index bed5237..2c5df41 100644
--- a/engines/support/app/models/ticket_comment.rb
+++ b/engines/support/app/models/ticket_comment.rb
@@ -18,6 +18,11 @@ class TicketComment
   #  view :by_body
   #end
 
+  # translations are in the same scope as those of a "proper" couchrest model
+  def self.i18n_scope
+    "couchrest"
+  end
+
   def is_comment_validated?
     !!posted_by
   end
diff --git a/engines/support/app/views/tickets/_comment.html.haml b/engines/support/app/views/tickets/_comment.html.haml
index 778ca13..65ec394 100644
--- a/engines/support/app/views/tickets/_comment.html.haml
+++ b/engines/support/app/views/tickets/_comment.html.haml
@@ -1,4 +1,5 @@
-- if admin? or !comment.private # only show comment if user is admin or comment is not private
+- # only show comment if user is admin or comment is not private
+- if admin? or !comment.private
   %tr
     %td.user
       %div
@@ -17,4 +18,4 @@
           %span.label.label-important
             = t(:private)
     %td.comment
-      = simple_format(comment.body)
\ No newline at end of file
+      = simple_format(comment.body)
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..889dac2 100644
--- a/engines/support/app/views/tickets/_edit_form.html.haml
+++ b/engines/support/app/views/tickets/_edit_form.html.haml
@@ -17,34 +17,30 @@
     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'
+        %b{style: 'padding:10px'}= t("tickets.status.open")
+        = f.button :loading, t("tickets.action.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'
-    %span.label.label-clear= t(:created_by_on, :user => created_by, :time => @ticket.created_at.to_s(:short)).html_safe
+        %b{style: 'padding:10px'}= t("tickets.status.closed")
+        = f.button :loading, t("tickets.action.open"), class: 'btn-mini'
+    %span.label.label-clear
+      = t("tickets.created_by_on", user: created_by, time: @ticket.created_at.to_s(:short), cascade: true).html_safe
 = simple_form_for @ticket do |f|
   = hidden_ticket_fields
-  %div= t(:subject)
-  = f.text_field :subject, :class => 'large full-width'
+  = f.input :subject, input_html: {:class => 'large full-width'}
   .row-fluid
     .span4
-      %div= t(:status)
-      = f.select :is_open, [[t(:open), "true"], [t(:closed), "false"]]
+      = f.input :is_open, as: :select, collection: [:true, :false], include_blank: false
     .span4
-      %div= t(:email)
-      = f.text_field :email
+      = f.input :email
     .span4
-      %div
-        = t(:regarding_account)
-        = regarding_user_link
-      = f.text_field :regarding_user
-  = f.button :loading, t(:save), :value => 'save'
+      = f.input :regarding_user, label: Ticket.human_attribute_name(:regarding_user) + regarding_user_link
+  = f.button :loading
   - if admin?
-    = link_to t(:destroy), auto_ticket_path(@ticket),  :confirm => t(:are_you_sure), :method => :delete, :class => 'btn'
+    = destroy_btn t(".destroy", cascade: true), auto_ticket_path(@ticket)
diff --git a/engines/support/app/views/tickets/_new_comment_form.html.haml b/engines/support/app/views/tickets/_new_comment_form.html.haml
index 40c737f..711421d 100644
--- a/engines/support/app/views/tickets/_new_comment_form.html.haml
+++ b/engines/support/app/views/tickets/_new_comment_form.html.haml
@@ -7,7 +7,7 @@
     = c.input :body, :label => false, :as => :text, :input_html => {:class => "full-width", :rows=> 5}
     - if admin?
       = c.input :private, :as => :boolean, :label => false, :inline_label => true
-  = f.button :loading, t(:post_reply), class: 'btn-primary', value: 'post_reply'
+  = f.button :loading, t(".post_reply"), class: 'btn-primary', value: 'post_reply'
   - if logged_in? && @ticket.is_open
-    = f.button :loading, t(:reply_and_close), value: 'reply_and_close'
-    = link_to t(:cancel), auto_tickets_path, :class => :btn
+    = f.button :loading, t(".reply_and_close"), value: 'reply_and_close'
+    = btn t(".cancel"), auto_tickets_path
diff --git a/engines/support/app/views/tickets/_tabs.html.haml b/engines/support/app/views/tickets/_tabs.html.haml
index 445a909..7872bb5 100644
--- a/engines/support/app/views/tickets/_tabs.html.haml
+++ b/engines/support/app/views/tickets/_tabs.html.haml
@@ -3,21 +3,17 @@
 -#
 - unless action?(:new) or action?(:create)
   %ul.nav.nav-pills.pull-right.slim
-    %li{:class=> ("active" if search_order.start_with? 'created_at')}
-      = link_to_order('created')
-    %li{:class=> ("active" if search_order.start_with? 'updated_at')}
-      = link_to_order('updated')
+    - %w(created updated).each do |order|
+      %li{:class=> ("active" if search_order.start_with? order)}
+        = link_to_order(order)
 
 -#
 -# STATUS FILTER TABS
 -#
 %ul.nav.nav-tabs
   - if logged_in?
-    %li{:class => ("active" if search_status == 'open')}
-      = link_to_status 'open'
-    %li{:class => ("active" if search_status == 'closed')}
-      = link_to_status 'closed'
-    %li{:class => ("active" if search_status == 'all')}
-      = link_to_status 'all'
+    - %w(open closed all).each do |status|
+      %li{:class => ("active" if search_status == status)}
+        = link_to_status status
   %li{:class => ("active" if action?(:new) || action?(:create))}
-    = link_to icon(:plus, :black) + t(:new_ticket), auto_new_ticket_path
+    = link_to icon(:plus, :black) + t(".new", cascade: true), auto_new_ticket_path
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..03bda7d
--- /dev/null
+++ b/engines/support/app/views/tickets/edit.html.haml
@@ -0,0 +1,6 @@
+- @show_navigation = params[:user_id].present?
+- @comment = TicketComment.new
+
+.ticket
+  = render 'tickets/edit_form'
+  = render 'tickets/comments'
diff --git a/engines/support/app/views/tickets/index.html.haml b/engines/support/app/views/tickets/index.html.haml
index a4df6e3..526cd6d 100644
--- a/engines/support/app/views/tickets/index.html.haml
+++ b/engines/support/app/views/tickets/index.html.haml
@@ -5,15 +5,15 @@
 %table.table.table-striped.table-bordered
   %thead
     %tr
-      %th= t(:subject)
-      %th= t(:created)
-      %th= t(:updated)
-      %th= t(:voices)
+      %th= t(".subject")
+      %th= t(".created")
+      %th= t(".updated")
+      %th= t(".voices")
   %tbody
     - if @tickets.any?
       = render @tickets.all
     - else
       %tr
-        %td{:colspan=>4}= t(:none)
+        %td{:colspan=>4}= t(".none")
 
 = paginate @tickets
diff --git a/engines/support/app/views/tickets/new.html.haml b/engines/support/app/views/tickets/new.html.haml
index 3de5fe9..d3580f9 100644
--- a/engines/support/app/views/tickets/new.html.haml
+++ b/engines/support/app/views/tickets/new.html.haml
@@ -11,7 +11,7 @@
   = f.input :email
   = f.input :regarding_user
   = f.simple_fields_for :comments, @comment do |c|
-    = c.input :body, :label => t(:description), :as => :text, :input_html => {:class => "full-width", :rows=> 5}
+    = c.input :body, :as => :text, :input_html => {:class => "full-width", :rows=> 5}
     - if admin?
       = c.input :private, :as => :boolean, :label => false, :inline_label => true
   = f.button :wrapped, cancel: (logged_in? ? auto_tickets_path : home_path)
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/locales/en.yml b/engines/support/config/locales/en.yml
index 342adea..8d2af67 100644
--- a/engines/support/config/locales/en.yml
+++ b/engines/support/config/locales/en.yml
@@ -1,22 +1,105 @@
 en:
-  access_ticket_text: >
-    You can later access this ticket at the URL %{full_url}. You might want to bookmark this page to find it again.
-    Anybody with this URL will be able to access this ticket, so if you are on a shared computer you might want to
-    remove it from the browser history.
-  support_tickets: "Support Tickets"
-  all_tickets: "All Tickets"
-  my_tickets: "My Tickets"
-  open_tickets: "Open Tickets"
-  closed_tickets: "Closed Tickets"
-  new_ticket: "New Ticket"
-  tickets: "Tickets"
-  subject: "Subject"
-  destroy: "Destroy"
-  open: "Open"
-  closed: "Closed"
-  close: "Close"
-  post_reply: "Post Reply"
-  reply_and_close: "Reply and Close"
-  description: "Description"
-  ticket: "Ticket"
-  regarding_account: "Regarding Account"
\ No newline at end of file
+  support_tickets: "Support"
+  # translations used in the layout views or @title
+  layouts:
+    # fallback for all translations of "tickets" nested below:
+    tickets: "Tickets" 
+    title:
+      tickets: "Tickets"
+    header:
+      tickets: "Tickets"
+    navigation:
+      tickets: "Support Tickets"
+  # Translations used in the views inside the tickets directory
+  tickets:
+    # If these do not exist they will be looked up in the global scope.
+    all: "All Tickets"
+    open: "Open Tickets"
+    closed: "Closed Tickets"
+    new: "New Ticket"
+    created: "Created at"
+    updated: "Updated at"
+    subject: "couchrest.models.tickets.attributes.subject"
+    status:
+      open: "Open"
+      closed: "Closed"
+    action:
+      open: "Open"
+      close: "Close"
+    confirm:
+      destroy:
+        are_you_sure: "Are you sure you want to destroy this ticket?"
+    # If you want to be more specific you can use the partial as a scope:
+    tabs:
+      all: "All Tickets"
+      open: "Open Tickets"
+      closed: "Closed Tickets"
+    index:
+      none: "No tickets have been found."
+      voices: "Voices"
+    destroy: "Destroy"
+    post_reply: "Post Reply"
+    reply_and_close: "Reply and Close"
+    access_ticket_text: >
+      You can later access this ticket at the URL %{full_url}. You might want to bookmark this page to find it again.
+      Anybody with this URL will be able to access this ticket, so if you are on a shared computer you might want to
+      remove it from the browser history.
+  # rails i18n
+  helpers:
+    # translations used for submit buttons. simple form picks these up
+    submit:
+      ticket:
+        create: "Submit Ticket"
+        update: "Update Ticket"
+  # translations for the model and its attributes
+  # serve as fallback for simpleform labels
+  couchrest:
+    models:
+      ticket: "Ticket"
+      ticket_comment: "Comment"
+    attributes:
+      ticket:
+        # these will fallback to translations in the "attributes" scope
+        subject: "Subject"
+        email: "Email"
+        regarding_user: "Regarding User"
+        regarding_account: "Regarding Account"
+        is_open: "Status"
+      ticket_comment:
+        body: "Description"
+        private: "private"
+  simple_form:
+    # labels next to the field
+    labels:
+      # these will fallback to the human_name translations of the model
+      ticket:
+        # these will fallback to translations in "simple_form.labels.defaults"
+        regarding_:
+        # you can be specific about translations for a given action:
+        #edit:
+        #  regaring_user:
+        email: "Email"
+        comments:
+          # these will fall back to "simple_form.labels.comments"
+          body: "Description"
+        # comments: ~
+    options:
+      ticket:
+        is_open:
+          "true": "Open"
+          "false": "Closed"
+    # mouse over hints for the given fields
+    hints:
+      ticket: 
+        email: "Please provide an email address so we can get back to you."
+        # these will fallback to translations in "simple_form.hints.defaults"
+    # placeholders inside the fields before anything was typed
+    #placeholders:
+    # ticket: ~
+        # these will fallback to translations in "simple_form.placeholders.defaults"
+
+  # these are generic defaults. They should be moved into the toplevel
+    #  attributes:
+    #simple_form:
+    #labels:
+    # defaults:
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/factories.rb b/engines/support/test/factories.rb
index be04f15..bcf41e8 100644
--- a/engines/support/test/factories.rb
+++ b/engines/support/test/factories.rb
@@ -6,13 +6,23 @@ FactoryGirl.define do
 
     factory :ticket_with_comment do
       comments_attributes do
-        { "0" => { "body" => Faker::Lorem.sentences.join(" ") } }
+        { "0" => {
+          "body" => Faker::Lorem.sentences.join(" "),
+          "posted_by" => created_by
+        } }
       end
     end
 
     factory :ticket_with_creator do
       created_by { FactoryGirl.create(:user).id }
     end
+
+  end
+
+  # TicketComments can not be saved. so only use this with build
+  # and add to a ticket afterwards
+  factory :ticket_comment do
+    body { Faker::Lorem.sentences.join(" ") }
   end
 
 end
diff --git a/engines/support/test/functional/ticket_comments_test.rb b/engines/support/test/functional/ticket_comments_test.rb
new file mode 100644
index 0000000..5cbe233
--- /dev/null
+++ b/engines/support/test/functional/ticket_comments_test.rb
@@ -0,0 +1,101 @@
+require 'test_helper'
+
+class TicketsCommentsTest < ActionController::TestCase
+  tests TicketsController
+
+  teardown do
+    # destroy all tickets that were created during the test
+    Ticket.all.each{|t| t.destroy}
+  end
+
+  test "add comment to unauthenticated ticket" do
+    ticket = FactoryGirl.create :ticket, :created_by => nil
+
+    assert_difference('Ticket.find(ticket.id).comments.count') do
+      put :update, :id => ticket.id,
+        :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
+    end
+
+    assert_equal ticket, assigns(:ticket) # still same ticket, with different comments
+    assert_not_equal ticket.comments, assigns(:ticket).comments # ticket == assigns(:ticket), but they have different comments (which we want)
+
+  end
+
+
+  test "add comment to own authenticated ticket" do
+
+    login
+    ticket = FactoryGirl.create :ticket, :created_by => @current_user.id
+
+    #they should be able to comment if it is their ticket:
+    assert_difference('Ticket.find(ticket.id).comments.count') do
+      put :update, :id => ticket.id,
+        :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
+    end
+    assert_not_equal ticket.comments, assigns(:ticket).comments
+    assert_not_nil assigns(:ticket).comments.last.posted_by
+    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
+
+  end
+
+
+  test "cannot comment if it is another users ticket" do
+    other_user = find_record :user
+    login :is_admin? => false, :email => nil
+    ticket = FactoryGirl.create :ticket, :created_by => other_user.id
+    # they should *not* be able to comment if it is not their ticket
+    put :update, :id => ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"not allowed comment"}} }
+    assert_response :redirect
+    assert_access_denied
+
+    assert_equal ticket.comments.map(&:body), assigns(:ticket).comments.map(&:body)
+  end
+
+  test "authenticated comment on an anonymous ticket adds to my tickets" do
+    login
+    ticket = FactoryGirl.create :ticket
+    other_ticket = FactoryGirl.create :ticket
+    put :update, :id => ticket.id,
+      :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
+    assert_not_nil assigns(:ticket).comments.last.posted_by
+    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
+    visible_tickets = Ticket.search admin_status: 'mine',
+      user_id: @current_user.id, is_admin: false
+    assert_equal [ticket], visible_tickets.all
+  end
+
+
+
+  test "admin add comment to authenticated ticket" do
+
+    other_user = find_record :user
+    login :is_admin? => true
+
+    ticket = FactoryGirl.create :ticket, :created_by => other_user.id
+
+    #admin should be able to comment:
+    assert_difference('Ticket.find(ticket.id).comments.count') do
+      put :update, :id => ticket.id,
+        :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
+    end
+    assert_not_equal ticket.comments, assigns(:ticket).comments
+    assert_not_nil assigns(:ticket).comments.last.posted_by
+    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
+  end
+
+  test "commenting on a ticket adds to tickets that are mine" do
+    testticket = FactoryGirl.create :ticket
+    user = find_record :admin_user
+    login user
+    get :index, {:user_id => user.id, :open_status => "open"}
+    assert_difference('assigns[:all_tickets].count') do
+      put :update, :id => testticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}}
+      get :index, {:user_id => user.id, :open_status => "open"}
+    end
+
+    assert assigns(:all_tickets).include?(assigns(:ticket))
+    assert_not_nil assigns(:ticket).comments.last.posted_by
+    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
+  end
+
+end
diff --git a/engines/support/test/functional/tickets_controller_test.rb b/engines/support/test/functional/tickets_controller_test.rb
index fc4a6f8..ebaa3a4 100644
--- a/engines/support/test/functional/tickets_controller_test.rb
+++ b/engines/support/test/functional/tickets_controller_test.rb
@@ -1,5 +1,14 @@
 require 'test_helper'
 
+#
+# Tests for the basic actions in the TicketsController
+#
+# Also see
+# TicketCommentsTest
+# TicketsListTest
+#
+# for detailed functional tests for comments and index action.
+#
 class TicketsControllerTest < ActionController::TestCase
 
   teardown do
@@ -36,8 +45,7 @@ class TicketsControllerTest < ActionController::TestCase
     user = find_record :user
     ticket = find_record :ticket, :created_by => user.id
     get :show, :id => ticket.id
-    assert_response :redirect
-    assert_redirected_to login_url
+    assert_login_required
   end
 
   test "user tickets are visible to creator" do
@@ -48,13 +56,19 @@ class TicketsControllerTest < ActionController::TestCase
     assert_response :success
   end
 
-  test "other users tickets are not visible" do
+  test "ticket of other user is not visible" do
     other_user = find_record :user
     ticket = find_record :ticket, :created_by => other_user.id
     login
     get :show, :id => ticket.id
-    assert_response :redirect
-    assert_redirected_to home_url
+    assert_access_denied
+  end
+
+  test "ticket list of other user is not visible" do
+    other_user = find_record :user
+    login
+    get :index, :user_id => other_user.id
+    assert_access_denied
   end
 
   test "should create unauthenticated ticket" do
@@ -104,180 +118,20 @@ class TicketsControllerTest < ActionController::TestCase
     assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id
   end
 
-  test "add comment to unauthenticated ticket" do
-    ticket = FactoryGirl.create :ticket, :created_by => nil
-
-    assert_difference('Ticket.find(ticket.id).comments.count') do
-      put :update, :id => ticket.id,
-        :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
-    end
-
-    assert_equal ticket, assigns(:ticket) # still same ticket, with different comments
-    assert_not_equal ticket.comments, assigns(:ticket).comments # ticket == assigns(:ticket), but they have different comments (which we want)
-
-  end
-
-
-  test "add comment to own authenticated ticket" do
-
+  test "close ticket" do
     login
-    ticket = FactoryGirl.create :ticket, :created_by => @current_user.id
-
-    #they should be able to comment if it is their ticket:
-    assert_difference('Ticket.find(ticket.id).comments.count') do
-      put :update, :id => ticket.id,
-        :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
-    end
-    assert_not_equal ticket.comments, assigns(:ticket).comments
-    assert_not_nil assigns(:ticket).comments.last.posted_by
-    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
-
-  end
-
-
-  test "cannot comment if it is not your ticket" do
-
-    other_user = find_record :user
-    login :is_admin? => false, :email => nil
-    ticket = FactoryGirl.create :ticket, :created_by => other_user.id
-    # they should *not* be able to comment if it is not their ticket
-    put :update, :id => ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"not allowed comment"}} }
-    assert_response :redirect
-    assert_access_denied
-
-    assert_equal ticket.comments.map(&:body), assigns(:ticket).comments.map(&:body)
-
-  end
-
-
-  test "admin add comment to authenticated ticket" do
-
-    other_user = find_record :user
-    login :is_admin? => true
-
-    ticket = FactoryGirl.create :ticket, :created_by => other_user.id
-
-    #admin should be able to comment:
-    assert_difference('Ticket.find(ticket.id).comments.count') do
-      put :update, :id => ticket.id,
-        :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
-    end
-    assert_not_equal ticket.comments, assigns(:ticket).comments
-    assert_not_nil assigns(:ticket).comments.last.posted_by
-    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
-  end
-
-  test "tickets by admin" do
-      other_user = find_record :user
-      ticket = FactoryGirl.create :ticket, :created_by => other_user.id
-
-      login :is_admin? => true
-
-      get :index, {:admin_status => "all", :open_status => "open"}
-      assert assigns(:all_tickets).count > 0
-
-      # if we close one ticket, the admin should have 1 less open ticket
-      assert_difference('assigns[:all_tickets].count', -1) do
-        assigns(:tickets).first.close
-        assigns(:tickets).first.save
-        get :index, {:admin_status => "all", :open_status => "open"}
-      end
-  end
-
-
-  test "admin_status mine vs all" do
-    testticket = FactoryGirl.create :ticket
-    user = find_record :user
-    login :is_admin? => true, :email => nil
-
-    get :index, {:open_status => "open"}
-    assert assigns(:all_tickets).include?(testticket)
-    get :index, {:user_id => user.id, :open_status => "open"}
-    assert !assigns(:all_tickets).include?(testticket)
+    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 "commenting on a ticket adds to tickets that are mine" do
-    testticket = FactoryGirl.create :ticket
-    user = find_record :admin_user
-    login user
-    get :index, {:user_id => user.id, :open_status => "open"}
-    assert_difference('assigns[:all_tickets].count') do
-      put :update, :id => testticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}}
-      get :index, {:user_id => user.id, :open_status => "open"}
-    end
-
-    assert assigns(:all_tickets).include?(assigns(:ticket))
-    assert_not_nil assigns(:ticket).comments.last.posted_by
-    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
-  end
-
-  test "admin ticket ordering" do
-    tickets = FactoryGirl.create_list :ticket, 2
-
-    login :is_admin? => true, :email => nil
-    get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_desc'}
-
-    # this will consider all tickets, not just those on first page
-    first_tick = assigns(:all_tickets).all.first
-    last_tick = assigns(:all_tickets).all.last
-    assert first_tick.created_at > last_tick.created_at
-
-    # and now reverse order:
-    get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_asc'}
-
-    assert_equal first_tick, assigns(:all_tickets).last
-    assert_equal last_tick, assigns(:all_tickets).first
-
-    assert_not_equal first_tick, assigns(:all_tickets).first
-    assert_not_equal last_tick, assigns(:all_tickets).last
-
-  end
-
-  test "tickets for regular user" do
+  test "reopen ticket" do
     login
-    ticket = FactoryGirl.create :ticket
-    other_ticket = FactoryGirl.create :ticket
-
-    put :update, :id => ticket.id,
-      :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
-    assert_not_nil assigns(:ticket).comments.last.posted_by
-    assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id
-
-    get :index, {:open_status => "open"}
-    assert assigns(:all_tickets).count > 0
-    assert assigns(:all_tickets).include?(ticket)
-    assert !assigns(:all_tickets).include?(other_ticket)
-
-    # user should have one more ticket if a new tick gets a comment by this user
-    assert_difference('assigns[:all_tickets].count') do
-      put :update, :id => other_ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}}
-      get :index, {:open_status => "open"}
-    end
-    assert assigns(:all_tickets).include?(other_ticket)
-
-   # if we close one ticket, the user should have 1 less open ticket
-    assert_difference('assigns[:all_tickets].count', -1) do
-      other_ticket.reload
-      other_ticket.close
-      other_ticket.save
-      get :index, {:open_status => "open"}
-    end
-
-    number_open_tickets = assigns(:all_tickets).count
-
-    # look at closed tickets:
-    get :index, {:open_status => "closed"}
-    assert !assigns(:all_tickets).include?(ticket)
-    assert assigns(:all_tickets).include?(other_ticket)
-    number_closed_tickets = assigns(:all_tickets).count
-
-    # all tickets should equal closed + open
-    get :index, {:open_status => "all"}
-    assert assigns(:all_tickets).include?(ticket)
-    assert assigns(:all_tickets).include?(other_ticket)
-    assert_equal assigns(:all_tickets).count, number_closed_tickets + number_open_tickets
-
-
+    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
diff --git a/engines/support/test/functional/tickets_list_test.rb b/engines/support/test/functional/tickets_list_test.rb
new file mode 100644
index 0000000..4c4cdef
--- /dev/null
+++ b/engines/support/test/functional/tickets_list_test.rb
@@ -0,0 +1,122 @@
+require 'test_helper'
+
+class TicketsListTest < ActionController::TestCase
+  tests TicketsController
+
+  teardown do
+    # destroy all records that were created during the test
+    Ticket.all.each{|t| t.destroy}
+    User.all.each{|u| u.account.destroy}
+  end
+
+
+  test "tickets by admin" do
+      other_user = find_record :user
+      ticket = FactoryGirl.create :ticket, :created_by => other_user.id
+
+      login :is_admin? => true
+
+      get :index, {:admin_status => "all", :open_status => "open"}
+      assert assigns(:all_tickets).count > 0
+
+      # if we close one ticket, the admin should have 1 less open ticket
+      assert_difference('assigns[:all_tickets].count', -1) do
+        assigns(:tickets).first.close
+        assigns(:tickets).first.save
+        get :index, {:admin_status => "all", :open_status => "open"}
+      end
+  end
+
+
+  test "admin_status mine vs all" do
+    testticket = FactoryGirl.create :ticket
+    user = find_record :user
+    login :is_admin? => true, :email => nil
+
+    get :index, {:open_status => "open"}
+    assert assigns(:all_tickets).include?(testticket)
+    get :index, {:user_id => user.id, :open_status => "open"}
+    assert !assigns(:all_tickets).include?(testticket)
+  end
+
+  test "admin ticket ordering" do
+    tickets = FactoryGirl.create_list :ticket, 2
+
+    login :is_admin? => true, :email => nil
+    get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_desc'}
+
+    # this will consider all tickets, not just those on first page
+    first_tick = assigns(:all_tickets).all.first
+    last_tick = assigns(:all_tickets).all.last
+    assert first_tick.created_at > last_tick.created_at
+
+    # and now reverse order:
+    get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_asc'}
+
+    assert_equal first_tick, assigns(:all_tickets).last
+    assert_equal last_tick, assigns(:all_tickets).first
+
+    assert_not_equal first_tick, assigns(:all_tickets).first
+    assert_not_equal last_tick, assigns(:all_tickets).last
+
+  end
+
+  test "own tickets include tickets commented upon" do
+    login
+    ticket = FactoryGirl.create :ticket
+    other_ticket = FactoryGirl.create :ticket
+    comment = FactoryGirl.build(:ticket_comment, posted_by: @current_user.id)
+    ticket.comments << comment
+    ticket.save
+
+    get :index, {:open_status => "open"}
+    assert assigns(:all_tickets).count > 0
+    assert assigns(:all_tickets).include?(ticket)
+    assert !assigns(:all_tickets).include?(other_ticket)
+  end
+
+  test "list all tickets created by user" do
+    login
+    ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    other_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    get :index, {:open_status => "open"}
+    assert_equal 2, assigns[:all_tickets].count
+  end
+
+  test "closing ticket removes from open tickets list" do
+    login
+    ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    other_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    other_ticket.reload
+    other_ticket.close
+    other_ticket.save
+    get :index, {:open_status => "open"}
+    assert_equal 1, assigns[:all_tickets].count
+  end
+
+  test "list closed tickets only" do
+    login
+    open_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    closed_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id, is_open: false
+    get :index, {:open_status => "closed"}
+    assert_equal [closed_ticket], assigns(:all_tickets).all
+  end
+
+  test "list all tickets inludes closed + open" do
+    login
+    open_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id
+    closed_ticket = FactoryGirl.create :ticket_with_comment,
+      created_by: @current_user.id, is_open: false
+    get :index, {:open_status => "all"}
+    assert_equal 2, assigns(:all_tickets).count
+    assert assigns(:all_tickets).include?(open_ticket)
+    assert assigns(:all_tickets).include?(closed_ticket)
+  end
+end
diff --git a/engines/support/test/integration/create_ticket_test.rb b/engines/support/test/integration/create_ticket_test.rb
index 0f8453c..90e9a8a 100644
--- a/engines/support/test/integration/create_ticket_test.rb
+++ b/engines/support/test/integration/create_ticket_test.rb
@@ -7,7 +7,7 @@ class CreateTicketTest < BrowserIntegrationTest
     click_on 'Get Help'
     fill_in 'Subject', with: 'test ticket'
     fill_in 'Description', with: 'description of the problem goes here'
-    click_on 'Create Ticket'
+    click_on 'Submit Ticket'
     assert page.has_content?("Ticket was successfully created.")
     assert page.has_content?("You can later access this ticket at the URL")
     assert page.has_content?(current_url)
@@ -20,12 +20,12 @@ class CreateTicketTest < BrowserIntegrationTest
     click_on 'Get Help'
     fill_in 'Subject', with: 'test ticket'
     fill_in 'Email', with: 'invalid data'
-    fill_in 'Regarding user', with: 'some user'
+    fill_in 'Regarding User', with: 'some user'
     fill_in 'Description', with: 'description of the problem goes here'
-    click_on 'Create Ticket'
+    click_on 'Submit Ticket'
     assert page.has_content?("is invalid")
     assert_equal 'invalid data', find_field('Email').value
-    assert_equal 'some user', find_field('Regarding user').value
+    assert_equal 'some user', find_field('Regarding User').value
   end
 
   test "prefills fields" do
@@ -35,7 +35,7 @@ class CreateTicketTest < BrowserIntegrationTest
     click_on "New Ticket"
     email = "#{@user.login}@#{APP_CONFIG[:domain]}"
     assert_equal email, find_field('Email').value
-    assert_equal @user.login, find_field('Regarding user').value
+    assert_equal @user.login, find_field('Regarding User').value
   end
 
   test "no prefill of email without email service" do
@@ -44,7 +44,7 @@ class CreateTicketTest < BrowserIntegrationTest
     click_on "Support Tickets"
     click_on "New Ticket"
     assert_equal "", find_field('Email').value
-    assert_equal @user.login, find_field('Regarding user').value
+    assert_equal @user.login, find_field('Regarding User').value
   end
 
   test "cleared email field should remain clear" do
@@ -55,7 +55,7 @@ class CreateTicketTest < BrowserIntegrationTest
     fill_in 'Subject', with: 'test ticket'
     fill_in 'Email', with: ''
     fill_in 'Description', with: 'description of the problem goes here'
-    click_on 'Create Ticket'
+    click_on 'Submit Ticket'
     ticket = Ticket.last
     assert_equal "", ticket.email
     ticket.destroy