summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/views/home/index.html.haml11
-rw-r--r--app/views/layouts/_messages.html.haml5
-rw-r--r--app/views/layouts/application.html.haml2
-rw-r--r--help/app/controllers/tickets_controller.rb115
-rw-r--r--help/app/models/ticket.rb30
-rw-r--r--help/app/views/tickets/index.html.haml10
-rw-r--r--help/app/views/tickets/show.html.haml47
-rw-r--r--help/config/routes.rb2
-rw-r--r--help/test/functional/tickets_controller_test.rb41
-rw-r--r--users/app/controllers/controller_extension/authentication.rb4
-rw-r--r--users/test/functional/application_controller_test.rb2
-rw-r--r--users/test/support/auth_test_helper.rb6
-rw-r--r--users/test/unit/user_test.rb11
13 files changed, 231 insertions, 55 deletions
diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml
index 9e68674..dd7e5aa 100644
--- a/app/views/home/index.html.haml
+++ b/app/views/home/index.html.haml
@@ -1,11 +1,14 @@
+%h1 spacer for firefox
+%h1 spacer for firefox
Try to fetch a
= link_to "cert", cert_path
%p
-Try to create a
+Create a
= link_to "ticket", new_ticket_path
-%p
-See all
-= link_to "tickets", tickets_path
+- if logged_in?
+ %p
+ See all
+ = link_to "tickets", tickets_path
diff --git a/app/views/layouts/_messages.html.haml b/app/views/layouts/_messages.html.haml
new file mode 100644
index 0000000..80e34d4
--- /dev/null
+++ b/app/views/layouts/_messages.html.haml
@@ -0,0 +1,5 @@
+- flash.each do |name, msg|
+ - if msg.is_a?(String)
+ %div{:class => "alert alert-#{name == :notice ? "success" : "error"}"}
+ %a.close{"data-dismiss" => "alert"} ×
+ = content_tag :div, msg, :id => "flash_#{name}"
diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml
index a57d65e..e3b7977 100644
--- a/app/views/layouts/application.html.haml
+++ b/app/views/layouts/application.html.haml
@@ -18,6 +18,6 @@
.content
.row
.span12
- //= render 'layouts/messages'
+ = render 'layouts/messages' # TODO: In firefox, these are hidden by header
= yield
%footer
diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb
index 4c7415b..b76b051 100644
--- a/help/app/controllers/tickets_controller.rb
+++ b/help/app/controllers/tickets_controller.rb
@@ -3,6 +3,8 @@ class TicketsController < ApplicationController
respond_to :html #, :json
#has_scope :open, :type => boolean
+ before_filter :set_strings
+
def new
@ticket = Ticket.new
@ticket.comments.build
@@ -17,8 +19,10 @@ class TicketsController < ApplicationController
else
@ticket.comments.last.posted_by = nil #hacky, but protecting this attribute doesn't work right, so this should make sure it isn't set.
end
-
flash[:notice] = 'Ticket was successfully created.' if @ticket.save
+ if !logged_in?
+ flash[:notice] = flash[:notice] + ' You can later access this ticket at the url ' + request.protocol + request.host_with_port + ticket_path(@ticket.id) + '. 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' #todo
+ end
respond_with(@ticket)
end
@@ -33,34 +37,119 @@ class TicketsController < ApplicationController
def show
@ticket = Ticket.find(params[:id])
+ if !@ticket
+ redirect_to tickets_path, :alert => "No such ticket"
+ return
+ end
+ authorize_ticket_access
# @ticket.comments.build
# build ticket comments?
end
def update
@ticket = Ticket.find(params[:id])
- @ticket.attributes = params[:ticket]
- @ticket.comments.last.posted_by = (current_user ? current_user.id : nil) #protecting posted_by isn't working, so this should protect it.
-
- if @ticket.save
- flash[:notice] = 'Ticket was successfully updated.'
- respond_with @ticket
- else
- #redirect_to [:show, @ticket] #
- flash[:alert] = 'Ticket has not been changed'
- redirect_to @ticket
- #respond_with(@ticket) # why does this go to edit?? redirect???
+ if ticket_access?
+ if status = params[:change_status] #close or open button was pressed
+ @ticket.close if params[:change_status] == 'close'
+ @ticket.reopen if params[:change_status] == 'open'
+ else
+ params[:ticket][:comments_attributes] = nil if params[:ticket][:comments_attributes].values.first[:body].blank? #unset comments hash if no new comment was typed
+ @ticket.attributes = params[:ticket] #this will call comments_attributes=
+ # @ticket.is_open = false if params[:commit] == @reply_close_str #this overrides is_open selection
+ @ticket.close if params[:commit] == @reply_close_str #this overrides is_open selection
+
+ # what if there is an update and no new comment? Confirm that there is a new comment to update posted_by:
+ @ticket.comments.last.posted_by = (current_user ? current_user.id : nil) if @ticket.comments_changed? #protecting posted_by isn't working, so this should protect it.
+ end
+ if @ticket.changed? and @ticket.save
+ flash[:notice] = 'Ticket was successfully updated.'
+ if @ticket.is_open
+ respond_with @ticket
+ else #for closed tickets, redirect to index.
+ redirect_to tickets_path
+ end
+ else
+ #redirect_to [:show, @ticket] #
+ flash[:alert] = 'Ticket has not been changed'
+ redirect_to @ticket
+ #respond_with(@ticket) # why does this go to edit?? redirect???
+ end
end
end
def index
# @tickets = Ticket.by_title #not actually what we will want
- respond_with(@tickets = Ticket.all) #we'll want only tickets that this user can access
+ #we'll want only tickets that this user can access
+ # @tickets = Ticket.by_is_open.key(params[:status])
+
+ #TODO: we will need pagination
+
+ #below is obviously too messy and not what we want, but wanted to get basic functionality there
+ if admin?
+ # todo: for admins, might want option to see tickets they have already posted to. want to use something like tickets_by_admin
+ if params[:status] == 'open'
+ @tickets = Ticket.by_is_open.key(true)
+ elsif params[:status] == 'closed'
+ @tickets = Ticket.by_is_open.key(false)
+ elsif params[:status] == 'open tickets I admin' #TODO: obviously temp hack
+ @tickets = tickets_by_admin(current_user.id)
+ elsif params[:status] == 'all tickets I admin' #TODO: obviously temp hack
+ @tickets = tickets_by_admin(current_user.id, false)
+ else
+ @tickets = Ticket.all
+ end
+ elsif logged_in?
+ #TODO---if, when logged in, user accessed unauthenticated ticket, then seems okay to list it in their list of tickets. Thus, include all tickets that the user has posted to, not just those that they created.
+ if params[:status] == 'open'
+ @tickets = Ticket.by_is_open_and_created_by.key([true, current_user.id]).all
+ elsif params[:status] == 'closed'
+ @tickets = Ticket.by_is_open_and_created_by.key([false, current_user.id]).all
+ else
+ @tickets = Ticket.by_created_by.key(current_user.id).all
+ end
+ else
+ access_denied
+ return
+ end
+
+ respond_with(@tickets)
+ end
+
+ def destroy
+ @ticket = Ticket.find(params[:id])
+ @ticket.destroy if admin?
+ redirect_to tickets_path
end
private
+ def ticket_access?
+ @ticket and (admin? or !@ticket.created_by or (current_user and current_user.id == @ticket.created_by))
+ end
+
+ def authorize_ticket_access
+ access_denied unless ticket_access?
+ end
+
+ def tickets_by_admin(id=current_user.id, just_open=true)
+ admin_tickets = []
+ tickets = Ticket.all
+ tickets.each do |ticket|
+ ticket.comments.each do |comment|
+ if comment.posted_by == id and (!just_open or ticket.is_open)
+ admin_tickets << ticket
+ break
+ end
+ end
+ end
+ admin_tickets
+ end
+
+ def set_strings
+ @post_reply_str = 'Post reply' #t :post_reply
+ @reply_close_str = 'Reply and close' #t :reply_and_close
+ end
# not using now, as we are using comment_attributes= from the Ticket model
=begin
def add_comment
diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb
index f38fed2..dc2f51b 100644
--- a/help/app/models/ticket.rb
+++ b/help/app/models/ticket.rb
@@ -23,20 +23,24 @@ class Ticket < CouchRest::Model::Base
#property :user_verified, TrueClass, :default => false #will be true exactly when user is set
#admins
- property :code, String, :protected => true # only should be set if created_by is nil
+ #property :code, String, :protected => true # only should be set if created_by is nil #instead we will just use couchdb ID
property :is_open, TrueClass, :default => true
property :comments, [TicketComment]
timestamps!
#before_validation :set_created_by, :set_code, :set_email, :on => :create
- before_validation :set_code, :set_email, :on => :create
+ before_validation :set_email, :on => :create
#named_scope :open, :conditions => {:is_open => true} #??
design do
view :by_title
+ view :by_is_open
+ view :by_created_by
+ view :by_is_open_and_created_by
+
end
validates :title, :presence => true
@@ -55,10 +59,12 @@ class Ticket < CouchRest::Model::Base
!!created_by
end
- def set_code
+=begin
+ def set_code #let's not use this---can use same show url
# ruby 1.9 provides url-safe option---this is not necessarily url-safe
self.code = SecureRandom.hex(8) if !is_creator_validated?
end
+=end
def set_email
@@ -66,23 +72,25 @@ class Ticket < CouchRest::Model::Base
# in controller set to be current users email if that exists
end
+ #not saving with close and reopen, as we will save in update when they are called.
def close
self.is_open = false
- save
+ #save
end
def reopen
self.is_open = true
- save
+ #save
end
def comments_attributes=(attributes)
-
- comment = TicketComment.new(attributes.values.first) #TicketComment.new(attributes)
- #comment.posted_by = User.current.id if User.current #we want to avoid User.current, and current_user won't work here. instead will set in tickets_controller
- comment.posted_at = Time.now
- comments << comment
-
+ if attributes # could be empty as we will empty if nothing was typed in
+ comment = TicketComment.new(attributes.values.first) #TicketComment.new(attributes)
+ #comment.posted_by = User.current.id if User.current #we want to avoid User.current, and current_user won't work here. instead will set in tickets_controller
+ # what about: comment.posted_by = self.updated_by (will need to add ticket.updated_by)
+ comment.posted_at = Time.now
+ comments << comment
+ end
end
=begin
diff --git a/help/app/views/tickets/index.html.haml b/help/app/views/tickets/index.html.haml
index 6db2140..5e35b12 100644
--- a/help/app/views/tickets/index.html.haml
+++ b/help/app/views/tickets/index.html.haml
@@ -1,9 +1,15 @@
-%h2 tickets index (just as space)
+%h1 tickets index (just as space)
Create a
= link_to "new ticket", new_ticket_path
= # below shouldn't be unless logged in
%h2 Tickets
-= # want to have selection option to see tickets, that are open, closed or all
+= form_tag(tickets_path, :method => :get) do # want to redo as ajax, and make sure it displays the selected option
+ - options = ["all", "open", "closed"]
+ - if admin?
+ - options << "open tickets I admin" # obviously not what we will want
+ - options << "all tickets I admin" # obviously not what we will want
+ = select_tag :status, options_for_select(options, :selected => params[:status]|| "all")
+ = submit_tag "filter"
- @tickets.each do |ticket|
%p
= link_to ticket.title, ticket
diff --git a/help/app/views/tickets/show.html.haml b/help/app/views/tickets/show.html.haml
index a9b994e..9b12f34 100644
--- a/help/app/views/tickets/show.html.haml
+++ b/help/app/views/tickets/show.html.haml
@@ -1,26 +1,39 @@
-- if flash[:notice]
- =flash[:notice]
-- if flash[:alert]
- =flash[:alert]
+%h1 tickets show (just as space for firefox)
+%h1 tickets show (just as space for firefox)
%h2= @ticket.title
-is open?
-= @ticket.is_open
-- if @ticket.code
- code:
- = @ticket.code
- if @ticket.email
email:
= @ticket.email
-- if User.find(@ticket.created_by)
- Created by
- = User.find(@ticket.created_by).login
-- else
- Unauthenticated ticket creator
+%li
+ - if User.find(@ticket.created_by)
+ Created by
+ = User.find(@ticket.created_by).login
+ - else
+ Unauthenticated ticket creator
+%li
+ = "status:"
+ - if @ticket.is_open
+ = 'open'
+ = #link_to 'close', ticket_path, :method => :put
+ = #button_to 'close', ticket_path, :method => :put
+ = button_to 'close', {:change_status => :close}, :method => :put
+ - else
+ = 'closed'
+ = button_to 'open', {:change_status => :open}, :method => :put
= render(:partial => "comment", :collection => @ticket.comments)
+= #render @ticket.comments should work if view is in /app/views/comments/_comment
-= simple_form_for (@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test
+= simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test
= f.simple_fields_for :comments, TicketComment.new do |c|
= c.input :body, :label => 'Comment', :as => :text
= #render :partial => 'new_comment'
- = f.button :submit
- = link_to t(:cancel), tickets_path, :class => :btn \ No newline at end of file
+ = #f.label :is_open
+ = #f.select :is_open, [true, false] #remove
+ = f.button :submit, @post_reply_str
+ - if @ticket.is_open
+ = f.button :submit, @reply_close_str
+= #link_to t(:destroy), ticket_path, :confirm => 'are you sure?', :method => :delete, :class => :btn if admin? # for link_to to work with delete, need to figure out jquery interaction correctly. see http://stackoverflow.com/questions/3774925/delete-link-sends-get-instead-of-delete-in-rails-3-view etc..
+= button_to 'destroy', ticket_path, :confirm => 'are you sure?', :method => :delete if admin?
+= # TODO want to have button to close
+= # TODO if admin, have button to delete
+= link_to t(:cancel), tickets_path, :class => :btn
diff --git a/help/config/routes.rb b/help/config/routes.rb
index 5e57e02..86a9201 100644
--- a/help/config/routes.rb
+++ b/help/config/routes.rb
@@ -1,5 +1,5 @@
Rails.application.routes.draw do
- resources :tickets, :only => [:new, :create, :index, :show, :update]
+ resources :tickets, :only => [:new, :create, :index, :show, :update, :destroy]
#resources :ticket, :only => [:show]
end
diff --git a/help/test/functional/tickets_controller_test.rb b/help/test/functional/tickets_controller_test.rb
index 6bdb6c7..bb17acc 100644
--- a/help/test/functional/tickets_controller_test.rb
+++ b/help/test/functional/tickets_controller_test.rb
@@ -2,7 +2,10 @@ require 'test_helper'
class TicketsControllerTest < ActionController::TestCase
- test "should get index" do
+ test "should get index if logged in" do
+ #todo: should redo this and actually authorize
+ user = User.last
+ session[:user_id] = user.id
get :index
assert_response :success
assert_not_nil assigns(:tickets)
@@ -28,6 +31,8 @@ class TicketsControllerTest < ActionController::TestCase
assert_nil assigns(:ticket).created_by
assert_equal 1, assigns(:ticket).comments.count
+ assigns(:ticket).destroy # destroys without checking permission. is that okay?
+
end
@@ -48,17 +53,49 @@ class TicketsControllerTest < ActionController::TestCase
assert_equal @current_user.email, ticket.email
assert_equal 1, assigns(:ticket).comments.count
+ assigns(:ticket).destroy # ?
end
- test "add comment to ticket" do
+ test "add comment to unauthenticated ticket" do
ticket = Ticket.last
+ ticket.created_by = nil # TODO: hacky, but this makes sure this ticket is an unauthenticated one
+ ticket.save
+# comment_count = t.comments.count
+# put :update, :id => t.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
+# assert_equal(comment_count + 1, assigns(:ticket).comments.count)
+ #assert_difference block isn't working
assert_difference('Ticket.last.comments.count') do
put :update, :id => ticket.id,
:ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} }
end
assert_equal ticket, assigns(:ticket)
+
+ test "add comment to authenticated ticket" do
+
+
+ params = {:title => "ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}}
+
+ #todo: should redo this and actually authorize
+ user = User.last
+ session[:user_id] = user.id
+
+ post :create, :ticket => params
+ t = assigns(:ticket)
+
+ comment_count = t.comments.count
+ debugger
+ put :update, :id => t.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } # this isn't working
+ assert_equal(comment_count + 1, t.comments.count)
+
+ #comment_count = t.comments.count
+ # now log out: and retry
+ #session[:user_id] = nil
+ #put :update, :id => t.id, :ticket => {:comments_attributes => {"0" => {"body" =>"EVEN NEWER comment"}} } # should fail
+# assert_equal(comment_count, t.comments.count)
+ #assert_difference block isn't working
+ t.destroy
end
end
diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb
index 87f7921..1726278 100644
--- a/users/app/controllers/controller_extension/authentication.rb
+++ b/users/app/controllers/controller_extension/authentication.rb
@@ -20,7 +20,9 @@ module ControllerExtension::Authentication
end
def access_denied
- redirect_to login_url, :alert => "Not authorized"
+ # TODO: should we redirect to the root_url in either case, and have the root_url include the login screen (and also ability to create unauthenticated tickets) when no user is logged in?
+ redirect_to login_url, :alert => "Not authorized" if !logged_in?
+ redirect_to root_url, :alert => "Not authorized" if logged_in?
end
def admin?
diff --git a/users/test/functional/application_controller_test.rb b/users/test/functional/application_controller_test.rb
index 857bae5..94b77bd 100644
--- a/users/test/functional/application_controller_test.rb
+++ b/users/test/functional/application_controller_test.rb
@@ -9,7 +9,7 @@ class ApplicationControllerTest < ActionController::TestCase
def test_authorize_redirect
@controller.send(:authorize)
- assert_access_denied
+ assert_access_denied(true, false)
end
def test_authorized
diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb
index f211597..795a977 100644
--- a/users/test/support/auth_test_helper.rb
+++ b/users/test/support/auth_test_helper.rb
@@ -15,10 +15,12 @@ module AuthTestHelper
return @current_user
end
- def assert_access_denied(denied = true)
+ def assert_access_denied(denied = true, logged_in = true)
if denied
assert_equal({:alert => "Not authorized"}, flash.to_hash)
- assert_redirected_to login_path
+ # todo: eventually probably eliminate separate conditions
+ assert_redirected_to login_path if !logged_in
+ assert_redirected_to root_path if logged_in
else
assert flash[:alert].blank?
end
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index f057ca7..9977fca 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -48,4 +48,15 @@ class UserTest < ActiveSupport::TestCase
assert_equal client_rnd, srp_session.aa
end
+ test 'is user an admin' do
+ admin_login = APP_CONFIG['admins'].first
+ attribs = User.valid_attributes_hash
+ attribs[:login] = admin_login
+ admin_user = User.new(attribs)
+ assert admin_user.is_admin?
+ assert !@user.is_admin?
+
+ end
+
+
end