From 313000fec18d56fba8f720ac32ae3ca8a92e4e36 Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 24 Dec 2012 16:22:39 -0800 Subject: Rough functionality for unauthenticated tickets. --- help/app/models/ticket.rb | 2 +- help/app/views/tickets/_ticket_data.html.haml | 7 +++++++ help/app/views/tickets/new.html.haml | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb index fa056b4..fed2b8b 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 diff --git a/help/app/views/tickets/_ticket_data.html.haml b/help/app/views/tickets/_ticket_data.html.haml index 3d301be..80c0d74 100644 --- a/help/app/views/tickets/_ticket_data.html.haml +++ b/help/app/views/tickets/_ticket_data.html.haml @@ -5,6 +5,13 @@ = User.find(@ticket.created_by).login - else Unauthenticated ticket creator + - if @ticket.regarding_user + %b + Regarding user: + - if regarding_user = User.find_by_login(@ticket.regarding_user) + = link_to @ticket.regarding_user, edit_user_path(regarding_user) + - else + = @ticket.regarding_user + '(no such 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 750b990..255be65 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -3,6 +3,7 @@ = simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test = f.input :title = f.input :email if !current_user #hmm--might authenticated users want to submit an alternate email? + = f.input :regarding_user if !current_user = render :partial => 'new_comment', :locals => {:f => f} = # regarding_user if not logged in = # email if not logged in -- cgit v1.2.3 From 4f35e7555feae15fb86eb81a38d4ff39b97ab576 Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 24 Dec 2012 16:34:49 -0800 Subject: Link to edit page (as show doesn't now exist, and not clear if we will want it) of user who created ticket. --- help/app/views/tickets/_ticket_data.html.haml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/help/app/views/tickets/_ticket_data.html.haml b/help/app/views/tickets/_ticket_data.html.haml index 80c0d74..fd5d3bf 100644 --- a/help/app/views/tickets/_ticket_data.html.haml +++ b/help/app/views/tickets/_ticket_data.html.haml @@ -1,8 +1,11 @@ .spam12 %b Created by: - - if User.find(@ticket.created_by) - = User.find(@ticket.created_by).login + - if creator = User.find(@ticket.created_by) + - if !creator.is_admin? + = link_to creator.login, edit_user_path(creator) + - else + = creator.login - else Unauthenticated ticket creator - if @ticket.regarding_user -- cgit v1.2.3 From 6b0fba93fc6137e83fb66cc38098f5479d1ce485 Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 31 Dec 2012 15:20:31 -0800 Subject: remove commented-out lines. --- help/app/views/tickets/new.html.haml | 2 -- 1 file changed, 2 deletions(-) diff --git a/help/app/views/tickets/new.html.haml b/help/app/views/tickets/new.html.haml index 255be65..79a9af7 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -5,8 +5,6 @@ = f.input :email if !current_user #hmm--might authenticated users want to submit an alternate email? = f.input :regarding_user if !current_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 -- cgit v1.2.3 From ce8b283255e2be4c0f42b3223c3cf4ad33364933 Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 7 Jan 2013 12:25:12 -0800 Subject: Ticket comments can be private --- help/app/models/ticket_comment.rb | 1 + help/app/views/tickets/_comment.html.haml | 31 +++++++++++++++------------ help/app/views/tickets/_new_comment.html.haml | 2 ++ 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/help/app/models/ticket_comment.rb b/help/app/models/ticket_comment.rb index 49e5c6c..18da3e1 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 diff --git a/help/app/views/tickets/_comment.html.haml b/help/app/views/tickets/_comment.html.haml index 26794dc..ae7f1d4 100644 --- a/help/app/views/tickets/_comment.html.haml +++ b/help/app/views/tickets/_comment.html.haml @@ -1,15 +1,18 @@ - # 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 - 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 commenter = User.find(comment.posted_by) + %b + = 'Posted by' + (commenter.is_admin? ? ' admin' : '') + ':' + = commenter.login + - else + Unauthenticated post + - if comment.private + (Private comment) + .pull-right + %b + Posted at: + = comment.posted_at.to_s(:short) + %br + = comment.body \ No newline at end of file diff --git a/help/app/views/tickets/_new_comment.html.haml b/help/app/views/tickets/_new_comment.html.haml index 7307dad..8d40bb6 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 => "span12", :rows=>4} + - if admin? + = c.input :private, :as => :boolean, :label => false, :inline_label => true -- cgit v1.2.3 From 2599c7bac06ee55d58e492a47e09ee163e9582ba Mon Sep 17 00:00:00 2001 From: jessib Date: Tue, 8 Jan 2013 13:20:34 -0800 Subject: Adding show view for users. --- users/app/controllers/users_controller.rb | 2 +- users/app/helpers/users_helper.rb | 6 ++++++ users/app/models/user.rb | 4 ++++ users/app/views/users/_user.html.haml | 2 +- users/app/views/users/show.html.haml | 32 +++++++++++++++++++++++++++++++ 5 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 users/app/views/users/show.html.haml diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 79de630..eb93fcb 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -2,7 +2,7 @@ class UsersController < ApplicationController skip_before_filter :verify_authenticity_token, :only => [:create] - before_filter :fetch_user, :only => [: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] 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 1798ea4..4b6b06c 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -100,6 +100,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) #defaults to having most recent updated first + end + protected ## 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..9df029e --- /dev/null +++ b/users/app/views/users/show.html.haml @@ -0,0 +1,32 @@ +.span8.offset1 + %h2= @user.login + %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.empty? + none set + - else + %ul.unstyled + - aliases.each do |al| + %li + = al.email + %dt + =t :most_recently_updated_tickets + %dd + %ul + - @user.most_recent_tickets.each do |ticket| + %li + = link_to ticket.title, ticket + %small + updated: + = ticket.updated_at.to_s(:long) + + -- cgit v1.2.3 From f75005e1bc15101f37d377b7ed06f877dc206ee0 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 10 Jan 2013 10:17:31 +0100 Subject: moved api routes into their own namespace In case we need them at some point - now it's new_api_user_path instead of new_user_path for example. This way they should not conflict with the normal route generation --- users/config/routes.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/users/config/routes.rb b/users/config/routes.rb index 8985502..723d737 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", :path => nil do + scope "/1", :module => "V1", defaults: {format: 'json'} do + resources :sessions, :only => [:new, :create, :update, :destroy] + resources :users, :only => [:create] + end + end end get "login" => "sessions#new", :as => "login" -- cgit v1.2.3 From d81bf00ecd8bdfcddf50e4881428c917253326fe Mon Sep 17 00:00:00 2001 From: jessib Date: Thu, 10 Jan 2013 11:06:09 -0800 Subject: Add test for showing user. --- users/test/functional/users_controller_test.rb | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 1fa1462..1f6c868 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -9,12 +9,31 @@ class UsersControllerTest < ActionController::TestCase assert_response :success end + test "failed show without login" do + user = find_record User + get :show, :id => user.id + assert_response :redirect + assert_redirected_to login_path + 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 "should create new user" do user = stub_record User User.expects(:create).with(user.params).returns(user) post :create, :user => user.params, :format => :json - assert_nil session[:user_id] assert_json_response user assert_response :success -- cgit v1.2.3 From 2485527650c4832d764d318e91c10bafde8b8ae5 Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 14 Jan 2013 11:19:55 -0800 Subject: Some fixes to the how we keep track of information about users associated with a ticket. --- help/app/models/ticket.rb | 13 ++++++++++++- help/app/models/ticket_comment.rb | 4 ++++ help/app/views/tickets/_comment.html.haml | 6 +++--- help/app/views/tickets/_ticket_data.html.haml | 18 +++++++++--------- help/app/views/tickets/new.html.haml | 2 +- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb index fed2b8b..262d5a4 100644 --- a/help/app/models/ticket.rb +++ b/help/app/models/ticket.rb @@ -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} #?? @@ -171,6 +171,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 @@ -209,6 +213,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 18da3e1..1df7eec 100644 --- a/help/app/models/ticket_comment.rb +++ b/help/app/models/ticket_comment.rb @@ -22,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 ae7f1d4..01cf01a 100644 --- a/help/app/views/tickets/_comment.html.haml +++ b/help/app/views/tickets/_comment.html.haml @@ -2,10 +2,10 @@ - if admin? or !comment.private # only show comment if user is admin or comment is not private %tr %td - - if commenter = User.find(comment.posted_by) + - if comment.posted_by_user %b - = 'Posted by' + (commenter.is_admin? ? ' admin' : '') + ':' - = commenter.login + = 'Posted by' + (comment.posted_by_user.is_admin? ? ' admin' : '') + ':' + = comment.posted_by_user.login - else Unauthenticated post - if comment.private diff --git a/help/app/views/tickets/_ticket_data.html.haml b/help/app/views/tickets/_ticket_data.html.haml index fd5d3bf..2261d8f 100644 --- a/help/app/views/tickets/_ticket_data.html.haml +++ b/help/app/views/tickets/_ticket_data.html.haml @@ -1,20 +1,20 @@ .spam12 %b Created by: - - if creator = User.find(@ticket.created_by) - - if !creator.is_admin? - = link_to creator.login, edit_user_path(creator) - - else - = creator.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 regarding_user = User.find_by_login(@ticket.regarding_user) - = link_to @ticket.regarding_user, edit_user_path(regarding_user) - - else - = @ticket.regarding_user + '(no such 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 79a9af7..af8baad 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -3,7 +3,7 @@ = simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test = f.input :title = f.input :email if !current_user #hmm--might authenticated users want to submit an alternate email? - = f.input :regarding_user if !current_user + = f.input :regarding_user = render :partial => 'new_comment', :locals => {:f => f} .form-actions = f.button :submit, :class => 'btn-primary' -- cgit v1.2.3 From 8141876126aa25d713cf4b2c76c3ecff837c4ba7 Mon Sep 17 00:00:00 2001 From: jessib Date: Mon, 14 Jan 2013 12:39:59 -0800 Subject: Use partials for displaying details shown when viewing a user. Some of these partials have specific CSS for another use, so we will likely want to tweak this. --- help/app/views/tickets/_ticket.html.haml | 5 +++-- users/app/models/user.rb | 2 +- users/app/views/emails/_email.html.haml | 5 +++-- users/app/views/users/show.html.haml | 21 ++++++++------------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/help/app/views/tickets/_ticket.html.haml b/help/app/views/tickets/_ticket.html.haml index 3edfa8b..e02aaeb 100644 --- a/help/app/views/tickets/_ticket.html.haml +++ b/help/app/views/tickets/_ticket.html.haml @@ -1,3 +1,4 @@ += # TODO---this is now used in 2 places, and not sure we want the same CSS in both places %tr %td %b @@ -5,9 +6,9 @@ %br %small created: - = ticket.created_at.to_s(:short) + = ticket.created_at.to_s(:short) #todo doesn't show year updated: - = ticket.updated_at.to_s(:short) + = ticket.updated_at.to_s(:short) # doesn't show year %small.pull-right comments by: = ticket.commenters diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 42900ea..1e8ee0e 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -95,7 +95,7 @@ class User < CouchRest::Model::Base end def most_recent_tickets(count=3) - Ticket.for_user(self).limit(count) #defaults to having most recent updated first + 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 3feb6f0..948d847 100644 --- a/users/app/views/emails/_email.html.haml +++ b/users/app/views/emails/_email.html.haml @@ -1,6 +1,7 @@ - if email.valid? %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/show.html.haml b/users/app/views/users/show.html.haml index 9df029e..2b59c66 100644 --- a/users/app/views/users/show.html.haml +++ b/users/app/views/users/show.html.haml @@ -1,5 +1,7 @@ .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| @@ -14,19 +16,12 @@ - if aliases.empty? none set - else - %ul.unstyled - - aliases.each do |al| - %li - = al.email + .pull-left + = render aliases + .clearfix %dt =t :most_recently_updated_tickets %dd - %ul - - @user.most_recent_tickets.each do |ticket| - %li - = link_to ticket.title, ticket - %small - updated: - = ticket.updated_at.to_s(:long) - - + %table + %tbody + = render @user.most_recent_tickets \ No newline at end of file -- cgit v1.2.3 From e7d36df945792b292732e25e879a90577050a6c1 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 15 Jan 2013 11:06:46 +0100 Subject: minor: put emails in unstyled ul and simplify Just found out that render(@collection) returns nil for emtpy collections. So that is usefull for putting messages about the emtpy collection in an or clause. --- users/app/views/users/show.html.haml | 10 +++------- users/config/locales/en.yml | 1 + 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/users/app/views/users/show.html.haml b/users/app/views/users/show.html.haml index 2b59c66..d8b51e9 100644 --- a/users/app/views/users/show.html.haml +++ b/users/app/views/users/show.html.haml @@ -12,16 +12,12 @@ %dt =t :email_aliases %dd - - aliases = @user.email_aliases - - if aliases.empty? - none set - - else - .pull-left - = render aliases + %ul.pull-left.unstyled + = render(@user.email_aliases) || t(:none_set) .clearfix %dt =t :most_recently_updated_tickets %dd %table %tbody - = render @user.most_recent_tickets \ No newline at end of file + = render @user.most_recent_tickets diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index 3c71e7e..7aa23f1 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -1,4 +1,5 @@ en: + none_set: "None set." signup: "Sign up" signup_message: "Please create an account." cancel: "Cancel" -- cgit v1.2.3 From be8ee9fa669bc5554796be1fc99867fc99ba21bc Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 15 Jan 2013 11:28:37 +0100 Subject: reverted simplification - not good to have 'none set' in a %ul --- users/app/views/users/show.html.haml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/users/app/views/users/show.html.haml b/users/app/views/users/show.html.haml index d8b51e9..ec5cea6 100644 --- a/users/app/views/users/show.html.haml +++ b/users/app/views/users/show.html.haml @@ -12,8 +12,12 @@ %dt =t :email_aliases %dd - %ul.pull-left.unstyled - = render(@user.email_aliases) || t(:none_set) + - aliases = @user.email_aliases + - if aliases.present? + %ul.pull-left.unstyled + = render aliases + - else + =t :none_set .clearfix %dt =t :most_recently_updated_tickets -- cgit v1.2.3 From 9d53f7b2d1b34da6b6103e97bd6c931cedb23e9b Mon Sep 17 00:00:00 2001 From: jessib Date: Tue, 15 Jan 2013 11:03:02 -0800 Subject: Show different ticket characteristics when viewing the users versus when listing the tickets. Give a message if a user has no tickets. --- help/app/views/tickets/_ticket.html.haml | 23 +++++++++++++---------- users/app/views/users/show.html.haml | 12 ++++++++---- users/config/locales/en.yml | 2 +- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/help/app/views/tickets/_ticket.html.haml b/help/app/views/tickets/_ticket.html.haml index e02aaeb..7b37652 100644 --- a/help/app/views/tickets/_ticket.html.haml +++ b/help/app/views/tickets/_ticket.html.haml @@ -1,14 +1,17 @@ -= # TODO---this is now used in 2 places, and not sure we want the same CSS in both places +- 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) #todo doesn't show year - updated: - = ticket.updated_at.to_s(:short) # doesn't show year - %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/users/app/views/users/show.html.haml b/users/app/views/users/show.html.haml index ec5cea6..a1eeccb 100644 --- a/users/app/views/users/show.html.haml +++ b/users/app/views/users/show.html.haml @@ -17,11 +17,15 @@ %ul.pull-left.unstyled = render aliases - else - =t :none_set + =t :none .clearfix %dt =t :most_recently_updated_tickets %dd - %table - %tbody - = render @user.most_recent_tickets + - 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 7aa23f1..7a6ab90 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -1,5 +1,5 @@ en: - none_set: "None set." + none: "None." signup: "Sign up" signup_message: "Please create an account." cancel: "Cancel" -- cgit v1.2.3 From 3dc8886beb7d3689c87d9aa1e5ad2d4c6c5b4c55 Mon Sep 17 00:00:00 2001 From: jessib Date: Tue, 15 Jan 2013 11:55:12 -0800 Subject: Refactoring of tickets controller to fetch the ticket in a before filter for relevant actions. --- help/app/controllers/tickets_controller.rb | 67 ++++++++++++++---------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb index d4aa378..d47939e 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 => "No such 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 -- cgit v1.2.3 From e2021bdcc40b51ab5e571c97e882bba10dc80ad6 Mon Sep 17 00:00:00 2001 From: jessib Date: Tue, 15 Jan 2013 12:52:09 -0800 Subject: For both users and tickets, if the object is not found and the current user is an admin, they should see an alert that the object wasn't found, and be redirected to the current controller. If the object isn't found and the current user is not an admin, then we will continue to give an error about no access, so as not to leak information about what IDs do and don't exist. --- config/locales/en.yml | 1 + help/app/controllers/tickets_controller.rb | 2 +- users/app/controllers/users_controller.rb | 4 ++++ 3 files changed, 6 insertions(+), 1 deletion(-) 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 d47939e..b613088 100644 --- a/help/app/controllers/tickets_controller.rb +++ b/help/app/controllers/tickets_controller.rb @@ -99,7 +99,7 @@ class TicketsController < ApplicationController def fetch_ticket @ticket = Ticket.find(params[:id]) if !@ticket and admin? - redirect_to tickets_path, :alert => "No such ticket" + redirect_to tickets_path, :alert => t(:no_such_thing, :thing => 'ticket') return end access_denied unless ticket_access? diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 79de630..3d5a6a7 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -49,6 +49,10 @@ class UsersController < ApplicationController def fetch_user @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 -- cgit v1.2.3 From 1cf4cc5c8d571b571367a08f5e201be868289ed1 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 16 Jan 2013 13:16:38 +0100 Subject: using subdomain for api requests properly --- users/config/routes.rb | 10 +++++----- users/test/integration/api/account_flow_test.rb | 11 ++++++++--- users/test/integration/api/python/flow_with_srp.py | 2 +- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/users/config/routes.rb b/users/config/routes.rb index 723d737..0c2d8d9 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -1,11 +1,11 @@ Rails.application.routes.draw do constraints :subdomain => "api" do - namespace "api", :path => nil do - scope "/1", :module => "V1", defaults: {format: 'json'} do - resources :sessions, :only => [:new, :create, :update, :destroy] - resources :users, :only => [:create] - end + namespace "api", { module: "V1", + path: "/1/", + defaults: {format: 'json'} } do + resources :sessions, :only => [:new, :create, :update, :destroy] + resources :users, :only => [:create] end end 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() -- cgit v1.2.3 From dc16c6f8e5382f9e5470eb2a40081d41f4112437 Mon Sep 17 00:00:00 2001 From: jessib Date: Thu, 17 Jan 2013 11:25:39 -0800 Subject: Deal with corner case where we don't have authenticated user. Will write a test after merging in show view for users. --- users/app/controllers/users_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 3d5a6a7..b705f47 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -2,6 +2,7 @@ class UsersController < ApplicationController skip_before_filter :verify_authenticity_token, :only => [:create] + before_filter :authorize before_filter :fetch_user, :only => [:edit, :update, :destroy] before_filter :set_anchor, :only => [:edit, :update] before_filter :authorize_admin, :only => [:index] @@ -48,6 +49,7 @@ 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') -- cgit v1.2.3 From cce882a42cc0c139b75d932ea8ee42525e4fdb32 Mon Sep 17 00:00:00 2001 From: jessib Date: Thu, 17 Jan 2013 12:35:48 -0800 Subject: Should be able to create a user when not logged in. This isn't ready to merge, as there is an issue with logging in as an admin in the test. --- users/app/controllers/users_controller.rb | 2 +- users/test/functional/users_controller_test.rb | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index a8ba1ab..c0fe243 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -3,7 +3,7 @@ class UsersController < ApplicationController skip_before_filter :verify_authenticity_token, :only => [:create] - before_filter :authorize + 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] diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 46db4d1..8c584ef 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -10,10 +10,12 @@ class UsersControllerTest < ActionController::TestCase end test "failed show without login" do - user = find_record :user + 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 @@ -42,7 +44,7 @@ class UsersControllerTest < ActionController::TestCase assert_response :success end - + test "user cannot see other user" do user = find_record :user, :email => nil, @@ -57,6 +59,26 @@ class UsersControllerTest < ActionController::TestCase 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: + # TODO: THIS IS failing to login and have admin? return true in users_controller. Will look into it later. + 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 -- cgit v1.2.3 From 444dbca4054ccfb7a82bb4df2a6369959ef6c9b2 Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 18 Jan 2013 07:38:13 +0100 Subject: minor: smalles fix ever - is_admin? has a questionmark --- users/test/functional/users_controller_test.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 8c584ef..9fb06c9 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -72,8 +72,7 @@ class UsersControllerTest < ActionController::TestCase assert_access_denied # when authenticated as admin: - # TODO: THIS IS failing to login and have admin? return true in users_controller. Will look into it later. - login :is_admin => true + login :is_admin? => true get :show, :id => nonid assert_response :redirect assert_equal({:alert => "No such user."}, flash.to_hash) -- cgit v1.2.3