diff options
-rw-r--r-- | config/locales/en.yml | 1 | ||||
-rw-r--r-- | help/app/controllers/tickets_controller.rb | 67 | ||||
-rw-r--r-- | help/app/models/ticket.rb | 15 | ||||
-rw-r--r-- | help/app/models/ticket_comment.rb | 5 | ||||
-rw-r--r-- | help/app/views/tickets/_comment.html.haml | 34 | ||||
-rw-r--r-- | help/app/views/tickets/_new_comment.html.haml | 2 | ||||
-rw-r--r-- | help/app/views/tickets/_ticket.html.haml | 22 | ||||
-rw-r--r-- | help/app/views/tickets/_ticket_data.html.haml | 14 | ||||
-rw-r--r-- | help/app/views/tickets/new.html.haml | 3 | ||||
-rw-r--r-- | users/app/controllers/users_controller.rb | 9 | ||||
-rw-r--r-- | users/app/helpers/users_helper.rb | 6 | ||||
-rw-r--r-- | users/app/models/user.rb | 4 | ||||
-rw-r--r-- | users/app/views/emails/_email.html.haml | 5 | ||||
-rw-r--r-- | users/app/views/users/_user.html.haml | 2 | ||||
-rw-r--r-- | users/app/views/users/show.html.haml | 31 | ||||
-rw-r--r-- | users/config/locales/en.yml | 1 | ||||
-rw-r--r-- | users/config/routes.rb | 10 | ||||
-rw-r--r-- | users/test/functional/users_controller_test.rb | 72 | ||||
-rw-r--r-- | users/test/integration/api/account_flow_test.rb | 11 | ||||
-rwxr-xr-x | users/test/integration/api/python/flow_with_srp.py | 2 |
20 files changed, 239 insertions, 77 deletions
diff --git a/config/locales/en.yml b/config/locales/en.yml index 179c14c..fc61c31 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3,3 +3,4 @@ en: hello: "Hello world" + no_such_thing: "No such %{thing}." diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb index d4aa378..b613088 100644 --- a/help/app/controllers/tickets_controller.rb +++ b/help/app/controllers/tickets_controller.rb @@ -6,6 +6,7 @@ class TicketsController < ApplicationController before_filter :set_strings before_filter :authorize, :only => [:index] + before_filter :fetch_ticket, :only => [:show, :update, :destroy] # don't now have an edit method def new @ticket = Ticket.new @@ -31,52 +32,44 @@ class TicketsController < ApplicationController =begin def edit - @ticket = Ticket.find(params[:id]) @ticket.comments.build # build ticket comments? end =end def show - @ticket = Ticket.find(params[:id]) @comment = TicketComment.new 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]) - if !ticket_access_denied? - if params[:post] #currently changes to title or is_open status - @ticket.attributes = params[:post] - # TODO: do we want to keep the history of title changes? one possibility was adding a comment that said something like 'user changed the title from a to b' - - 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.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 || !logged_in? - respond_with @ticket - else #for closed tickets with authenticated users, 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??? + if params[:post] #currently changes to title or is_open status + @ticket.attributes = params[:post] + # TODO: do we want to keep the history of title changes? one possibility was adding a comment that said something like 'user changed the title from a to b' + + 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.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 || !logged_in? + respond_with @ticket + else #for closed tickets with authenticated users, 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 @@ -88,7 +81,6 @@ class TicketsController < ApplicationController def destroy # should we allow non-admins to delete their own tickets? i don't think necessary. - @ticket = Ticket.find(params[:id]) @ticket.destroy if admin? redirect_to tickets_path end @@ -99,16 +91,19 @@ class TicketsController < ApplicationController @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 set_strings @post_reply_str = 'Post reply' #t :post_reply @reply_close_str = 'Reply and close' #t :reply_and_close end + def fetch_ticket + @ticket = Ticket.find(params[:id]) + if !@ticket and admin? + redirect_to tickets_path, :alert => t(:no_such_thing, :thing => 'ticket') + return + end + access_denied unless ticket_access? + 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 a27a9d4..ed1ff9d 100644 --- a/help/app/models/ticket.rb +++ b/help/app/models/ticket.rb @@ -16,7 +16,7 @@ class Ticket < CouchRest::Model::Base #belongs_to :user #from leap_web_users. doesn't necessarily belong to a user though property :created_by, String, :protected => true #Integer #nil unless user was authenticated for ticket creation, #THIS should not be changed after being set - #property :regarding_user, String#Integer # form cannot be submitted if they type in a username w/out corresponding ID. this field can be nil. for authenticated ticket creation by non-admins, should this just automatically be set to be same as created_by? or maybe we don't use this field unless created_by is nil? + property :regarding_user, String#Integer # form cannot be submitted if they type in a username w/out corresponding ID. this field can be nil. for authenticated ticket creation by non-admins, should this just automatically be set to be same as created_by? or maybe we don't use this field unless created_by is nil? #also, both created_by and regarding_user could be nil---say user forgets username, or has general question property :title, String property :email, String #verify @@ -30,7 +30,7 @@ class Ticket < CouchRest::Model::Base timestamps! #before_validation :set_created_by, :set_code, :set_email, :on => :create - before_validation :set_email, :on => :create + before_validation :set_email, :set_regarding_user, :on => :create #named_scope :open, :conditions => {:is_open => true} #?? @@ -94,6 +94,10 @@ class Ticket < CouchRest::Model::Base # in controller set to be current users email if that exists end + def set_regarding_user + self.regarding_user = nil if self.regarding_user == "" + end + #not saving with close and reopen, as we will save in update when they are called. #TODO: not sure if we should bother with these: def close @@ -132,6 +136,13 @@ class Ticket < CouchRest::Model::Base end end + def created_by_user + User.find(self.created_by) + end + + def regarding_user_actual_user + User.find_by_login(self.regarding_user) + end =begin def validate if email_address and not email_address.strip =~ RFC822::EmailAddress diff --git a/help/app/models/ticket_comment.rb b/help/app/models/ticket_comment.rb index 49e5c6c..1df7eec 100644 --- a/help/app/models/ticket_comment.rb +++ b/help/app/models/ticket_comment.rb @@ -7,6 +7,7 @@ class TicketComment property :posted_at, Time#, :protected => true #property :posted_verified, TrueClass, :protected => true #should be true if current_user is set when the comment is created property :body, String + property :private, TrueClass # private comments are only viewable by admins # ? timestamps! validates :body, :presence => true @@ -21,6 +22,10 @@ class TicketComment !!posted_by end + def posted_by_user + User.find(self.posted_by) + end + =begin #TODO. #this is resetting all comments associated with the ticket: diff --git a/help/app/views/tickets/_comment.html.haml b/help/app/views/tickets/_comment.html.haml index 1d8ee41..501ceec 100644 --- a/help/app/views/tickets/_comment.html.haml +++ b/help/app/views/tickets/_comment.html.haml @@ -1,16 +1,20 @@ - # style is super ugly but just for now -%tr - %td - - if commenter = User.find(comment.posted_by) - %b - = 'Posted by' + (commenter.is_admin? ? ' admin' : '') + ':' - = commenter.login - - else - %b - Unauthenticated post - .pull-right - %b - Posted at: - = comment.posted_at.to_s(:short) - %br - = comment.body +- if admin? or !comment.private # only show comment if user is admin or comment is not private + %tr + %td + - if comment.posted_by_user + %b + = 'Posted by' + (comment.posted_by_user.is_admin? ? ' admin' : '') + ':' + = comment.posted_by_user.login + - else + %b + Unauthenticated post + - if comment.private + (Private comment) + .pull-right + %b + Posted at: + = comment.posted_at.to_s(:short) + %br + = comment.body + diff --git a/help/app/views/tickets/_new_comment.html.haml b/help/app/views/tickets/_new_comment.html.haml index 31d134f..96388ea 100644 --- a/help/app/views/tickets/_new_comment.html.haml +++ b/help/app/views/tickets/_new_comment.html.haml @@ -1,2 +1,4 @@ = f.simple_fields_for :comments, @comment do |c| = c.input :body, :label => 'Comment', :as => :text, :input_html => {:class => "span9", :rows=>4} + - if admin? + = c.input :private, :as => :boolean, :label => false, :inline_label => true diff --git a/help/app/views/tickets/_ticket.html.haml b/help/app/views/tickets/_ticket.html.haml index 3edfa8b..7b37652 100644 --- a/help/app/views/tickets/_ticket.html.haml +++ b/help/app/views/tickets/_ticket.html.haml @@ -1,13 +1,17 @@ +- updated_at_text = 'updated: ' + ticket.updated_at.to_s(:long) %tr %td %b = link_to ticket.title, ticket - %br - %small - created: - = ticket.created_at.to_s(:short) - updated: - = ticket.updated_at.to_s(:short) - %small.pull-right - comments by: - = ticket.commenters + - if params[:controller] == 'tickets' + %br + %small + created: + = ticket.created_at.to_s(:long) + = updated_at_text + %small.pull-right + comments by: + = ticket.commenters + - else + %small + = updated_at_text
\ No newline at end of file diff --git a/help/app/views/tickets/_ticket_data.html.haml b/help/app/views/tickets/_ticket_data.html.haml index fee080f..d68d3e9 100644 --- a/help/app/views/tickets/_ticket_data.html.haml +++ b/help/app/views/tickets/_ticket_data.html.haml @@ -1,10 +1,20 @@ .spam12 %b Created by: - - if User.find(@ticket.created_by) - = User.find(@ticket.created_by).login + - if @ticket.created_by_user + = link_to @ticket.created_by_user.login, edit_user_path(@ticket.created_by_user) #todo: won't want edit path - else Unauthenticated ticket creator + - if @ticket.regarding_user + %b + Regarding user: + - if admin? + - if @ticket.regarding_user_actual_user + = link_to @ticket.regarding_user_actual_user.login, edit_user_path(@ticket.regarding_user_actual_user) #todo: won't want edit path + - else + = @ticket.regarding_user + ' (no such user)' + - else # a non-admin is viewing the ticket, so they shouldn't see confirmation of whether the regarding_user exists or not. + = @ticket.regarding_user - if @ticket.email %b email: diff --git a/help/app/views/tickets/new.html.haml b/help/app/views/tickets/new.html.haml index ee7adb2..1aa689b 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -3,9 +3,8 @@ = simple_form_for @ticket, :validate => true, :html => {:class => 'form-horizontal'} do |f| = f.input :title = f.input :email if !current_user #hmm--might authenticated users want to submit an alternate email? + = f.input :regarding_user = render :partial => 'new_comment', :locals => {:f => f} - = # regarding_user if not logged in - = # email if not logged in .form-actions = f.button :submit, :class => 'btn-primary' = link_to t(:cancel), tickets_path, :class => :btn diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 75ae2da..6cb438b 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -2,7 +2,9 @@ class UsersController < ApplicationController skip_before_filter :verify_authenticity_token, :only => [:create] - before_filter :fetch_user, :only => [:edit, :update, :destroy] + + before_filter :authorize, :only => [:show, :edit, :update, :destroy] + before_filter :fetch_user, :only => [:show, :edit, :update, :destroy] before_filter :set_anchor, :only => [:edit, :update] before_filter :authorize_admin, :only => [:index] @@ -49,7 +51,12 @@ class UsersController < ApplicationController protected def fetch_user + # authorize filter has been checked first, so won't get here unless authenticated @user = User.find_by_param(params[:id]) + if !@user and admin? + redirect_to users_path, :alert => t(:no_such_thing, :thing => 'user') + return + end access_denied unless admin? or (@user == current_user) end diff --git a/users/app/helpers/users_helper.rb b/users/app/helpers/users_helper.rb index 45ca0e9..5f68085 100644 --- a/users/app/helpers/users_helper.rb +++ b/users/app/helpers/users_helper.rb @@ -30,4 +30,10 @@ module UsersHelper classes.compact end + def user_field(field) + value = @user.send(field) + value = value.to_s(:long) if field.end_with? '_at' + value || 'not set' + end + end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 63f4d0f..292fb13 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -86,6 +86,10 @@ class User < CouchRest::Model::Base email_aliases.build(attrs.values.first) if attrs end + def most_recent_tickets(count=3) + Ticket.for_user(self).limit(count).all #defaults to having most recent updated first + end + protected ## diff --git a/users/app/views/emails/_email.html.haml b/users/app/views/emails/_email.html.haml index f5eb2d0..e96385d 100644 --- a/users/app/views/emails/_email.html.haml +++ b/users/app/views/emails/_email.html.haml @@ -1,5 +1,6 @@ %li.pull-right %code= email - = link_to(user_email_alias_path(@user, email), :method => :delete) do - %i.icon-remove + - if params[:action] == 'edit' + = link_to(user_email_alias_path(@user, email), :method => :delete) do + %i.icon-remove .clearfix diff --git a/users/app/views/users/_user.html.haml b/users/app/views/users/_user.html.haml index 7db0041..ca03d34 100644 --- a/users/app/views/users/_user.html.haml +++ b/users/app/views/users/_user.html.haml @@ -1,5 +1,5 @@ %tr - %td= user.login + %td= link_to user.login, user %td= time_ago_in_words(user.created_at) + " ago" %td = link_to edit_user_path(user), :class => "btn btn-mini btn-primary" do diff --git a/users/app/views/users/show.html.haml b/users/app/views/users/show.html.haml new file mode 100644 index 0000000..a1eeccb --- /dev/null +++ b/users/app/views/users/show.html.haml @@ -0,0 +1,31 @@ +.span8.offset1 + %h2= @user.login + .small + = link_to 'edit', edit_user_path(@user) + %dl.offset1 + - fields = ['login', 'email', 'created_at', 'updated_at', 'email_forward'] + - fields.each do |field| + %dt + = field.titleize + %dd + = user_field(field) + %dt + =t :email_aliases + %dd + - aliases = @user.email_aliases + - if aliases.present? + %ul.pull-left.unstyled + = render aliases + - else + =t :none + .clearfix + %dt + =t :most_recently_updated_tickets + %dd + - tix = @user.most_recent_tickets + - if tix.present? + %table + %tbody + = render @user.most_recent_tickets + - else + =t :none
\ No newline at end of file diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index 3c71e7e..7a6ab90 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -1,4 +1,5 @@ en: + none: "None." signup: "Sign up" signup_message: "Please create an account." cancel: "Cancel" diff --git a/users/config/routes.rb b/users/config/routes.rb index 8985502..0c2d8d9 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -1,8 +1,12 @@ Rails.application.routes.draw do - scope "/1", :module => "V1", defaults: {format: 'json'} do - resources :sessions, :only => [:new, :create, :update, :destroy] - resources :users, :only => [:create] + constraints :subdomain => "api" do + namespace "api", { module: "V1", + path: "/1/", + defaults: {format: 'json'} } do + resources :sessions, :only => [:new, :create, :update, :destroy] + resources :users, :only => [:create] + end end get "login" => "sessions#new", :as => "login" diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 8f1ee15..9fb06c9 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -9,13 +9,85 @@ class UsersControllerTest < ActionController::TestCase assert_response :success end + test "failed show without login" do + user = FactoryGirl.build(:user) + user.save + get :show, :id => user.id + assert_response :redirect + assert_redirected_to login_path + user.destroy + end + + test "user can see user" do + user = find_record :user, + :email => nil, + :email_forward => nil, + :email_aliases => [], + :created_at => Time.now, + :updated_at => Time.now, + :most_recent_tickets => [] + login user + get :show, :id => user.id + assert_response :success + end + + test "admin can see other user" do + user = find_record :user, + :email => nil, + :email_forward => nil, + :email_aliases => [], + :created_at => Time.now, + :updated_at => Time.now, + :most_recent_tickets => [] + login :is_admin? => true + get :show, :id => user.id + assert_response :success + + end + + test "user cannot see other user" do + user = find_record :user, + :email => nil, + :email_forward => nil, + :email_aliases => [], + :created_at => Time.now, + :updated_at => Time.now, + :most_recent_tickets => [] + login + get :show, :id => user.id + assert_response :redirect + assert_access_denied + end + + test "show for non-existing user" do + nonid = 'thisisnotanexistinguserid' + + # when unauthenticated: + get :show, :id => nonid + assert_access_denied(true, false) + + # when authenticated but not admin: + login + get :show, :id => nonid + assert_access_denied + + # when authenticated as admin: + login :is_admin? => true + get :show, :id => nonid + assert_response :redirect + assert_equal({:alert => "No such user."}, flash.to_hash) + assert_redirected_to users_path + end + test "should create new user" do user_attribs = record_attributes_for :user user = User.new(user_attribs) User.expects(:create).with(user_attribs).returns(user) + post :create, :user => user_attribs, :format => :json + assert_nil session[:user_id] assert_json_response user assert_response :success diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb index b9e2a4e..268fb5e 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -22,7 +22,7 @@ class AccountFlowTest < ActiveSupport::TestCase :password_verifier => @srp.verifier.to_s(16), :password_salt => @srp.salt.to_s(16) } - post '/1/users.json', :user => @user_params + post 'http://api.lvh.me:3000/1/users.json', :user => @user_params @user = User.find_by_param(@login) end @@ -33,7 +33,10 @@ class AccountFlowTest < ActiveSupport::TestCase # this test wraps the api and implements the interface the ruby-srp client. def handshake(login, aa) - post "/1/sessions.json", :login => login, 'A' => aa.to_s(16), :format => :json + post "http://api.lvh.me:3000/1/sessions.json", + :login => login, + 'A' => aa.to_s(16), + :format => :json response = JSON.parse(last_response.body) if response['errors'] raise RECORD_NOT_FOUND.new(response['errors']) @@ -43,7 +46,9 @@ class AccountFlowTest < ActiveSupport::TestCase end def validate(m) - put "/1/sessions/" + @login + '.json', :client_auth => m.to_s(16), :format => :json + put "http://api.lvh.me:3000/1/sessions/" + @login + '.json', + :client_auth => m.to_s(16), + :format => :json return JSON.parse(last_response.body) end diff --git a/users/test/integration/api/python/flow_with_srp.py b/users/test/integration/api/python/flow_with_srp.py index f28aeda..df83dfb 100755 --- a/users/test/integration/api/python/flow_with_srp.py +++ b/users/test/integration/api/python/flow_with_srp.py @@ -16,7 +16,7 @@ def id_generator(size=6, chars=string.ascii_uppercase + string.digits): return ''.join(random.choice(chars) for x in range(size)) # using globals for a start -server = 'http://localhost:3000/1' +server = 'http://api.lvh.me:3000/1' login = id_generator() password = id_generator() + id_generator() |