diff options
| -rw-r--r-- | app/assets/javascripts/application.js | 5 | ||||
| -rw-r--r-- | app/views/home/index.html.haml | 11 | ||||
| -rw-r--r-- | app/views/layouts/_messages.html.haml | 5 | ||||
| -rw-r--r-- | app/views/layouts/application.html.haml | 5 | ||||
| -rw-r--r-- | core/lib/leap_web_core/dependencies.rb | 1 | ||||
| -rw-r--r-- | core/lib/leap_web_core/ui_dependencies.rb | 1 | ||||
| -rw-r--r-- | help/app/controllers/tickets_controller.rb | 111 | ||||
| -rw-r--r-- | help/app/models/ticket.rb | 43 | ||||
| -rw-r--r-- | help/app/views/tickets/index.html.haml | 45 | ||||
| -rw-r--r-- | help/app/views/tickets/show.html.haml | 47 | ||||
| -rw-r--r-- | help/config/routes.rb | 2 | ||||
| -rw-r--r-- | help/test/functional/tickets_controller_test.rb | 160 | ||||
| -rw-r--r-- | ui_dependencies.rb | 1 | ||||
| -rw-r--r-- | users/app/controllers/controller_extension/authentication.rb | 4 | ||||
| -rw-r--r-- | users/test/functional/application_controller_test.rb | 2 | ||||
| -rw-r--r-- | users/test/support/auth_test_helper.rb | 6 | ||||
| -rw-r--r-- | users/test/unit/user_test.rb | 11 |
17 files changed, 385 insertions, 75 deletions
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index f7ca1ec..23d7fef 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -16,5 +16,10 @@ //= require users //= require_tree . //= require bootstrap +//= require jquery.pjax + +$(function() { + $('a:not([data-remote]):not([data-behavior]):not([data-skip-pjax])').pjax('[data-pjax-container]'); +}); //= require rails.validations //= require rails.validations.simple_form 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..b0644f2 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -18,6 +18,7 @@ .content .row .span12 - //= render 'layouts/messages' - = yield + = render 'layouts/messages' # TODO: In firefox, these are hidden by header + %div{"data-pjax-container" => ""} + = yield %footer diff --git a/core/lib/leap_web_core/dependencies.rb b/core/lib/leap_web_core/dependencies.rb index 7f6ca87..00ef515 100644 --- a/core/lib/leap_web_core/dependencies.rb +++ b/core/lib/leap_web_core/dependencies.rb @@ -11,6 +11,7 @@ module LeapWebCore "haml" => "~> 3.1.7", "bootstrap-sass" => "~> 2.0.4", "jquery-rails" => nil, + "pjax_rails" => nil, "simple_form" => nil } diff --git a/core/lib/leap_web_core/ui_dependencies.rb b/core/lib/leap_web_core/ui_dependencies.rb index e0a0b86..8ca9b91 100644 --- a/core/lib/leap_web_core/ui_dependencies.rb +++ b/core/lib/leap_web_core/ui_dependencies.rb @@ -2,6 +2,7 @@ require "haml" require "bootstrap-sass" require "jquery-rails" require "simple_form" +require "pjax_rails" if Rails.env == "development" require "haml-rails" diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb index 4c7415b..2f26d24 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 @@ -10,15 +12,17 @@ class TicketsController < ApplicationController def create @ticket = Ticket.new(params[:ticket]) - if current_user + if logged_in? @ticket.created_by = current_user.id @ticket.email = current_user.email if current_user.email @ticket.comments.last.posted_by = current_user.id 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,111 @@ class TicketsController < ApplicationController def show @ticket = Ticket.find(params[:id]) + if !@ticket + redirect_to tickets_path, :alert => "No such ticket" + return + end + ticket_access_denied? #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_denied? + 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 + #TODO: we will need pagination + + if admin? + if params[:admin_status] == 'mine' + @tickets = tickets_by_admin(current_user.id) + elsif params[:open_status] == 'open' + @tickets = Ticket.by_is_open.key(true) + elsif params[:open_status] == 'closed' + @tickets = Ticket.by_is_open.key(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[:open_status] == 'open' + @tickets = Ticket.by_is_open_and_created_by.key([true, current_user.id]).all + elsif params[:open_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 ticket_access_denied? + access_denied unless ticket_access? + end + + def tickets_by_admin(id=current_user.id) + admin_tickets = [] + tickets = Ticket.all + tickets.each do |ticket| + ticket.comments.each do |comment| + if comment.posted_by == id and (params[:open_status] != 'open' or ticket.is_open) and (params[:open_status] != 'closed' or !ticket.is_open) #limit based on whether the ticket is open if open_status is set to open or closed + 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..3b66fe4 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,38 @@ 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) + def commenters + commenters = [] + self.comments.each do |comment| + if comment.posted_by + user = User.find(comment.posted_by) + commenters << user.login if user and !commenters.include?(user.login) + else + commenters << 'unauthenticated user' if !commenters.include?('unauthenticated user') #todo don't hardcode string 'unauthenticated user' + end + end + commenters.join(', ') + end - 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 - + def comments_attributes=(attributes) + 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..95b65c0 100644 --- a/help/app/views/tickets/index.html.haml +++ b/help/app/views/tickets/index.html.haml @@ -1,10 +1,39 @@ -%h2 tickets index (just as space) +%h1 tickets index (just as space) +%h1 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 -- @tickets.each do |ticket| - %p - = link_to ticket.title, ticket -= #render(:partial => "ticket", :collection => @tickets) + += #%div{"data-pjax-container" => ""} # not sure how to get this working right +.row + .span2 + - if admin? + %h4 whose tickets + %ul.nav.nav-pills + %li{:class => ("active" if params[:admin_status] == 'mine')} + = link_to 'only tickets i admin', {:admin_status => 'mine', :open_status => params[:open_status]} + %li{:class => ("active" if params[:admin_status] != 'mine')} + = link_to 'all', {:admin_status => 'all', :open_status => params[:open_status]} + .span10 + %ul.nav.nav-tabs + %li{:class => ("active" if params[:open_status] != 'closed' and params[:open_status] != 'all')} + = link_to 'open issues', {:open_status => 'open', :admin_status => params[:admin_status]} + %li{:class => ("active" if params[:open_status] == 'closed')} + = link_to 'closed issues', {:open_status => 'closed', :admin_status => params[:admin_status]} + = #%a{:href => "#"} closed issue + %li{:class => ("active" if params[:open_status] == 'all')} + = link_to 'open & closed issues', {:open_status => 'all', :admin_status => params[:admin_status]} + + - @tickets.each do |ticket| + %p + = link_to ticket.title, ticket + comments by: + = ticket.commenters + + +%div{"data-pjax-container" => ""} + / PJAX updates will go here + hmmm + + 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..35901ca 100644 --- a/help/test/functional/tickets_controller_test.rb +++ b/help/test/functional/tickets_controller_test.rb @@ -2,7 +2,8 @@ require 'test_helper' class TicketsControllerTest < ActionController::TestCase - test "should get index" do + test "should get index if logged in" do + login(User.last) get :index assert_response :success assert_not_nil assigns(:tickets) @@ -14,51 +15,184 @@ class TicketsControllerTest < ActionController::TestCase assert_response :success end + test "ticket show access" do + ticket = Ticket.first + ticket.created_by = nil # TODO: hacky, but this makes sure this ticket is an unauthenticated one + ticket.save + get :show, :id => ticket.id + assert_response :success + + ticket.created_by = User.last.id + ticket.save + get :show, :id => ticket.id + assert_response :redirect + assert_redirected_to login_url + + login(User.last) + get :show, :id => ticket.id + assert_response :success + + login(User.first) #assumes User.first != User.last: + assert_not_equal User.first, User.last + get :show, :id => ticket.id + assert_response :redirect + assert_redirected_to root_url + + end test "should create unauthenticated ticket" do - params = {:title => "ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} + params = {:title => "unauth ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} assert_difference('Ticket.count') do post :create, :ticket => params end assert_response :redirect - #assert_equal assigns(:ticket).email, User.current.email - #assert_equal User.find(assigns(:ticket).created_by).login, User.current.login assert_nil assigns(:ticket).created_by assert_equal 1, assigns(:ticket).comments.count - end + assert_nil assigns(:ticket).comments.first.posted_by + assigns(:ticket).destroy # destroys without checking permission. is that okay? + end test "should create authenticated ticket" do - params = {:title => "ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} + params = {:title => "auth ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} login User.last - assert_difference('Ticket.count') do post :create, :ticket => params end assert_response :redirect - ticket = assigns(:ticket) - assert ticket - assert_equal @current_user.id, ticket.created_by - assert_equal @current_user.email, ticket.email + assert_not_nil assigns(:ticket).created_by + assert_equal assigns(:ticket).created_by, @current_user.id + assert_equal assigns(:ticket).email, @current_user.email + assert_equal 1, assigns(:ticket).comments.count + assert_not_nil assigns(:ticket).comments.first.posted_by + assert_equal assigns(:ticket).comments.first.posted_by, @current_user.id + assigns(:ticket).destroy + end + + 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 + 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) # 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 ticket" do + test "add comment to own authenticated ticket" do + + login(User.last) ticket = Ticket.last + ticket.created_by = User.last.id # TODO: hacky, but confirms it is their ticket + ticket.save + #they should be able to comment if it is their ticket: 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) + 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 + + login(User.last) # assumes User.last is not admin + assert !@current_user.is_admin? + + ticket = Ticket.last + + assert_not_nil User.first.id + ticket.created_by = User.first.id #assumes User.first != User.last: + assert_not_equal User.first, User.last + ticket.save + # they should *not* be able to comment if it is not their ticket + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + assert_response :redirect + assert_access_denied + assert_equal ticket.comments, assigns(:ticket).comments + + end + + + test "admin add comment to authenticated ticket" do + + admin_login = APP_CONFIG['admins'].first + admin_user = User.find_by_login(admin_login) #assumes that there is an admin login + login(admin_user) + + ticket = Ticket.last + assert_not_nil User.last.id + ticket.created_by = User.last.id # TODO: hacky, but confirms it somebody elses ticket. assumes last user is not admin user: + assert_not_equal User.last, admin_user + ticket.save + + #admin should be able to comment: + assert_difference('Ticket.last.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 + + admin_login = APP_CONFIG['admins'].first + admin_user = User.find_by_login(admin_login) #assumes that there is an admin login + login(admin_user) + + post :create, :ticket => {:title => "test tick", :comments_attributes => {"0" => {"body" =>"body of test tick"}}} + post :create, :ticket => {:title => "another test tick", :comments_attributes => {"0" => {"body" =>"body of another test tick"}}} + + assert_not_nil assigns(:ticket).created_by + assert_equal assigns(:ticket).created_by, admin_user.id + + get :index, {:status => "open tickets I admin"} + assert assigns(:tickets).count > 1 # at least 2 tickets + + # if we close one ticket, the admin should have 1 less open ticket they admin + assert_difference('assigns[:tickets].count', -1) do + assigns(:ticket).close + assigns(:ticket).save + get :index, {:status => "open tickets I admin"} + end + assigns(:ticket).destroy + + testticket = Ticket.create :title => 'testytest' + assert !assigns(:tickets).include?(testticket) + + # admin should have one more ticket if a new tick gets an admin comment + assert_difference('assigns[:tickets].count') do + put :update, :id => testticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} + get :index, {:status => "open tickets I admin"} + end + + assert assigns(:tickets).include?(assigns(:ticket)) + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, admin_user.id + + assigns(:ticket).destroy + + end + end + diff --git a/ui_dependencies.rb b/ui_dependencies.rb index 44f8f32..a05a8f9 100644 --- a/ui_dependencies.rb +++ b/ui_dependencies.rb @@ -2,6 +2,7 @@ gem "haml", "~> 3.1.7" gem "bootstrap-sass", "~> 2.1.0" gem "jquery-rails" gem "simple_form" +gem "pjax_rails" gem 'client_side_validations' gem 'client_side_validations-simple_form' 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 |