diff options
44 files changed, 987 insertions, 115 deletions
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index f7ca1ec..3fd641c 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -16,5 +16,14 @@ //= require users //= require_tree . //= require bootstrap +//= require bootstrap-editable +//= require bootstrap-editable-rails +//= require bootstrap-editable-inline +//= require jquery.pjax +//= require tickets + +$(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/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 8dec07d..cb5fa1b 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -12,3 +12,5 @@ table.table-hover .btn { table.table-hover tr:hover .btn { opacity: 1; } + +@import "bootstrap-editable";
\ No newline at end of file diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml index 9e68674..c02dcad 100644 --- a/app/views/home/index.html.haml +++ b/app/views/home/index.html.haml @@ -1,11 +1,11 @@ - 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/application.html.haml b/app/views/layouts/application.html.haml index e6d22f0..bd03477 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -19,5 +19,6 @@ .row .span12 = render 'layouts/messages' + %div{"data-pjax-container" => ""} = yield %footer diff --git a/config/couchdb.yml b/config/couchdb.yml deleted file mode 100644 index 636f2f2..0000000 --- a/config/couchdb.yml +++ /dev/null @@ -1,9 +0,0 @@ -development: - prefix: "" - -production: - prefix: "" - -test: - prefix: "" - suffix: test 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/assets/javascripts/tickets.js b/help/app/assets/javascripts/tickets.js new file mode 100644 index 0000000..8f58e86 --- /dev/null +++ b/help/app/assets/javascripts/tickets.js @@ -0,0 +1,3 @@ +$(document).ready(function () { + $('#title').editable(); +});
\ No newline at end of file diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb index b5f3a63..db9bc82 100644 --- a/help/app/controllers/tickets_controller.rb +++ b/help/app/controllers/tickets_controller.rb @@ -1,8 +1,12 @@ class TicketsController < ApplicationController - respond_to :html #, :json + respond_to :html, :json #has_scope :open, :type => boolean + before_filter :set_strings + + before_filter :authorize, :only => [:index] + def new @ticket = Ticket.new @ticket.comments.build @@ -10,15 +14,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 +39,80 @@ 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_access_denied? + if params[:post] #currently changes to title or is_open status + if @ticket.update_attributes(params[:post]) #this saves ticket, so @ticket.changed? will be false + tick_updated = true + end + # 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' - 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??? + 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. + tick_updated = true if @ticket.changed? and @ticket.save + end + if tick_updated + 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 + @all_tickets = Ticket.for_user(current_user, params, admin?) #for tests, useful to have as separate variable + + #below works if @tickets is a CouchRest::Model::Designs::View, but not if it is an Array + @tickets = @all_tickets.page(params[:page]).per(10) + #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 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/helpers/tickets_helper.rb b/help/app/helpers/tickets_helper.rb new file mode 100644 index 0000000..bd2c069 --- /dev/null +++ b/help/app/helpers/tickets_helper.rb @@ -0,0 +1,43 @@ +module TicketsHelper + + def status + params[:open_status] || 'open' + end + + def admin + # do we not want this set for non-admins? the param will be viewable in the url + params[:admin_status] || 'all' + end + + def order + params[:sort_order] || 'updated_at_desc' + end + + def link_to_status(new_status) + label = new_status + ' issues' + link_to label, :open_status => new_status, :admin_status => admin, :sort_order => order + end + + def link_to_order(order_field) + if order.start_with?(order_field) + # link for currently-filtered field. Link to other direction of this field. + if order.end_with? 'asc' + direction = 'desc' + icon_direction = 'up' + else + direction = 'asc' + icon_direction = 'down' + end + arrow = content_tag(:i, '', class: 'icon-arrow-'+ icon_direction) + else + # for not-currently-filtered field, don't display an arrow, and link to descending direction + arrow = '' + direction = 'desc' + end + + link_to :sort_order => order_field + '_at_' + direction, :open_status => status, :admin_status => admin do + arrow + order_field + ' at' + end + end + +end diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb index f38fed2..781216e 100644 --- a/help/app/models/ticket.rb +++ b/help/app/models/ticket.rb @@ -20,23 +20,109 @@ class Ticket < CouchRest::Model::Base #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 - + #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 + #TODO--clean this all up + view :by_is_open + view :by_created_by + + view :by_updated_at + view :by_created_at + + view :by_is_open_and_created_by + view :by_is_open_and_created_at + view :by_is_open_and_updated_at + + view :includes_post_by, + :map => + "function(doc) { + var arr = {} + if (doc['type'] == 'Ticket' && doc.comments) { + doc.comments.forEach(function(comment){ + if (comment.posted_by && !arr[comment.posted_by]) { + //don't add duplicates + arr[comment.posted_by] = true; + emit(comment.posted_by, 1); + } + }); + } + }", :reduce => "function(k,v,r) { return sum(v); }" + + view :includes_post_by_and_open_status_and_updated_at, + :map => + "function(doc) { + var arr = {} + if (doc['type'] == 'Ticket' && doc.comments) { + doc.comments.forEach(function(comment){ + if (comment.posted_by && !arr[comment.posted_by]) { + //don't add duplicates + arr[comment.posted_by] = true; + emit([comment.posted_by, doc.is_open, doc.updated_at], 1); + } + }); + } + }", :reduce => "function(k,v,r) { return sum(v); }" + + view :includes_post_by_and_open_status_and_created_at, + :map => + "function(doc) { + var arr = {} + if (doc['type'] == 'Ticket' && doc.comments) { + doc.comments.forEach(function(comment){ + if (comment.posted_by && !arr[comment.posted_by]) { + //don't add duplicates + arr[comment.posted_by] = true; + emit([comment.posted_by, doc.is_open, doc.created_at], 1); + } + }); + } + }", :reduce => "function(k,v,r) { return sum(v); }" + + view :includes_post_by_and_updated_at, + :map => + "function(doc) { + var arr = {} + if (doc['type'] == 'Ticket' && doc.comments) { + doc.comments.forEach(function(comment){ + if (comment.posted_by && !arr[comment.posted_by]) { + //don't add duplicates + arr[comment.posted_by] = true; + emit([comment.posted_by, doc.updated_at], 1); + } + }); + } + }", :reduce => "function(k,v,r) { return sum(v); }" + + + view :includes_post_by_and_created_at, + :map => + "function(doc) { + var arr = {} + if (doc['type'] == 'Ticket' && doc.comments) { + doc.comments.forEach(function(comment){ + if (comment.posted_by && !arr[comment.posted_by]) { + //don't add duplicates + arr[comment.posted_by] = true; + emit([comment.posted_by, doc.created_at], 1); + } + }); + } + }", :reduce => "function(k,v,r) { return sum(v); }" + end validates :title, :presence => true @@ -45,20 +131,83 @@ class Ticket < CouchRest::Model::Base # html5 has built-in validation which isn't ideal, as it says 'please enter an email address' for invalid email addresses, which implies an email address is required, and it is not. validates :email, :format => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :if => :email #email address is optional - + #TODO: #def set_created_by # self.created_by = User.current if User.current #end - + + def self.for_user(user, options = {}, is_admin = false) + + # TODO: This is obviously super tedious. we will refactor later. + # TODO: thought i should reverse keys for descending, but that didn't work. look into whether that should be tweaked, and whether it works okay with pagination (seems to now...) + # TODO: Time.now + 2.days is to catch tickets created in future. shouldn't happen but does on my computer now, so this at least catches for now. + # TODO handle default values correctly: + options[:open_status] = 'open' if !options[:open_status] #hacky. redo this when handling defaults correctly + + if (is_admin && (options[:admin_status] != 'mine')) + # show all (selected) tickets to admin + if options[:open_status] == 'all' + if options[:sort_order] == 'created_at_desc' + Ticket.by_created_at.startkey(0).endkey(Time.now + 2.days).descending + elsif options[:sort_order] == 'updated_at_asc' + Ticket.by_updated_at.startkey(0).endkey(Time.now + 2.days) + elsif options[:sort_order] == 'created_at_asc' + Ticket.by_created_at.startkey(0).endkey(Time.now + 2.days) + else + Ticket.by_updated_at.startkey(0).endkey(Time.now + 2.days).descending + end + else + if options[:sort_order] == 'created_at_desc' + Ticket.by_is_open_and_created_at.startkey([(options[:open_status] == 'open'), 0]).endkey([(options[:open_status] == 'open'), Time.now + 2.days]).descending + elsif options[:sort_order] == 'updated_at_asc' + Ticket.by_is_open_and_updated_at.startkey([(options[:open_status] == 'open'), 0]).endkey([(options[:open_status] == 'open'), Time.now + 2.days]) + elsif options[:sort_order] == 'created_at_asc' + Ticket.by_is_open_and_created_at.startkey([(options[:open_status] == 'open'), 0]).endkey([(options[:open_status] == 'open'), Time.now + 2.days]) + else + Ticket.by_is_open_and_updated_at.startkey([(options[:open_status] == 'open'), 0]).endkey([(options[:open_status] == 'open'), Time.now + 2.days]).descending + end + end + else + # only show tickets this user has commented on, as user is non-admin or admin viewing only their tickets + if options[:open_status] == 'all' + if options[:sort_order] == 'created_at_desc' + Ticket.includes_post_by_and_created_at.startkey([user.id, 0]).endkey([user.id, Time.now + 2.days]).descending + elsif options[:sort_order] == 'updated_at_asc' + Ticket.includes_post_by_and_updated_at.startkey([user.id, 0]).endkey([user.id, Time.now + 2.days]) + elsif options[:sort_order] == 'created_at_asc' + Ticket.includes_post_by_and_created_at.startkey([user.id, 0]).endkey([user.id, Time.now + 2.days]) + else + Ticket.includes_post_by_and_updated_at.startkey([user.id, 0]).endkey([user.id, Time.now + 2.days]).descending + end + else + if options[:sort_order] == 'created_at_desc' + Ticket.includes_post_by_and_open_status_and_created_at.startkey([user.id, (options[:open_status] == 'open'), 0]).endkey([user.id, (options[:open_status] == 'open'), Time.now + 2.days]).descending + elsif options[:sort_order] == 'updated_at_asc' + Ticket.includes_post_by_and_open_status_and_updated_at.startkey([user.id, (options[:open_status] == 'open'), 0]).endkey([user.id, (options[:open_status] == 'open'), Time.now + 2.days]) + elsif options[:sort_order] == 'created_at_asc' + Ticket.includes_post_by_and_open_status_and_created_at.startkey([user.id, (options[:open_status] == 'open'), 0]).endkey([user.id, (options[:open_status] == 'open'), Time.now + 2.days]) + else + Ticket.includes_post_by_and_open_status_and_updated_at.startkey([user.id, (options[:open_status] == 'open'), 0]).endkey([user.id, (options[:open_status] == 'open'), Time.now + 2.days]).descending + end + end + end + end + + def self.tickets_by_commenter(user_id)#, options = {}) + Ticket.includes_post_by_and_updated_at.startkey([user_id, 0]).endkey([user_id, Time.now]) + end + def is_creator_validated? !!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 +215,42 @@ 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. + #TODO: not sure if we should bother with these: 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 + if user = User.find(comment.posted_by) + commenters << user.login if user and !commenters.include?(user.login) + else + commenters << 'unknown user' if !commenters.include?('unknown user') #todo don't hardcode string 'unknown user' + end + 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 @@ -91,5 +259,5 @@ class Ticket < CouchRest::Model::Base errors.add 'email', 'contains an invalid address' end end -=end +=end end diff --git a/help/app/views/tickets/_admin-nav.html.haml b/help/app/views/tickets/_admin-nav.html.haml new file mode 100644 index 0000000..0e45c40 --- /dev/null +++ b/help/app/views/tickets/_admin-nav.html.haml @@ -0,0 +1,5 @@ +%ul.nav.nav-pills.nav-stacked + %li{:class => ("active" if admin == 'mine')} + = link_to 'tickets i admin', {:admin_status => 'mine', :open_status => status, :sort_order => order} + %li{:class => ("active" if admin == 'all')} + = link_to 'all tickets', {:admin_status => 'all', :open_status => status, :sort_order => order} diff --git a/help/app/views/tickets/_new_comment.html.haml b/help/app/views/tickets/_new_comment.html.haml index a924dfd..b216311 100644 --- a/help/app/views/tickets/_new_comment.html.haml +++ b/help/app/views/tickets/_new_comment.html.haml @@ -1,3 +1,2 @@ -= #do we want this partial? not using it now -= simple_fields_for :comment do |c| += f.simple_fields_for :comments, comment_object do |c| = c.input :body, :label => 'Comment', :as => :text diff --git a/help/app/views/tickets/_order-nav.html.haml b/help/app/views/tickets/_order-nav.html.haml new file mode 100644 index 0000000..9e8bcee --- /dev/null +++ b/help/app/views/tickets/_order-nav.html.haml @@ -0,0 +1,5 @@ +%ul.nav.nav-pills.pull-right + %li{:class=> ("active" if order.start_with? 'created_at' )} + = link_to_order('created') + %li{:class=> ("active" if order.start_with? 'updated_at' )} + = link_to_order('updated') diff --git a/help/app/views/tickets/_status-nav.html.haml b/help/app/views/tickets/_status-nav.html.haml new file mode 100644 index 0000000..69f4248 --- /dev/null +++ b/help/app/views/tickets/_status-nav.html.haml @@ -0,0 +1,7 @@ +%ul.nav.nav-tabs + %li{:class => ("active" if status == 'open')} + = link_to_status 'open' + %li{:class => ("active" if status == 'closed')} + = link_to_status 'closed' + %li{:class => ("active" if status == 'all')} + = link_to_status 'all' diff --git a/help/app/views/tickets/_table-nav.html.haml b/help/app/views/tickets/_table-nav.html.haml new file mode 100644 index 0000000..635b59b --- /dev/null +++ b/help/app/views/tickets/_table-nav.html.haml @@ -0,0 +1,5 @@ +.row + .span6 + = render 'tickets/status-nav' + .span4 + = render 'tickets/order-nav' diff --git a/help/app/views/tickets/_ticket.html.haml b/help/app/views/tickets/_ticket.html.haml new file mode 100644 index 0000000..3edfa8b --- /dev/null +++ b/help/app/views/tickets/_ticket.html.haml @@ -0,0 +1,13 @@ +%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 diff --git a/help/app/views/tickets/index.html.haml b/help/app/views/tickets/index.html.haml index 6db2140..fdbeec5 100644 --- a/help/app/views/tickets/index.html.haml +++ b/help/app/views/tickets/index.html.haml @@ -1,10 +1,22 @@ -%h2 tickets index (just as space) +%h1 tickets index + 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? + = render 'tickets/admin-nav' + .span10 + = render 'tickets/table-nav' + %table.table-striped.table-bordered.table-hover{:style => "width:100%;"} + %tbody + = render @tickets.all + = paginate @tickets + +%div{"data-pjax-container" => ""} + / PJAX updates will go here + hmmm + + diff --git a/help/app/views/tickets/new.html.haml b/help/app/views/tickets/new.html.haml index 537b97f..0ee47ff 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -1,16 +1,9 @@ %h2=t :new_ticket = simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test - = #@ticket.errors.messages = f.input :title - = #f.input :email #if there is no current_user = f.input :email if !current_user #hmm--might authenticated users want to submit an alternate email? - - = f.simple_fields_for :comments do |c| - = c.input :body, :label => 'Comment', :as => :text - - = #render :partial => 'new_comment' #what we were using + = render :partial => 'new_comment', :locals => {:f => f, :comment_object => nil} = # regarding_user if not logged in = # email if not logged in - = #f.button :submit, :value => t(:submit), :class => 'btn-primary' = f.button :submit = link_to t(:cancel), tickets_path, :class => :btn diff --git a/help/app/views/tickets/show.html.haml b/help/app/views/tickets/show.html.haml index a9b994e..b9f2ce6 100644 --- a/help/app/views/tickets/show.html.haml +++ b/help/app/views/tickets/show.html.haml @@ -1,26 +1,34 @@ -- if flash[:notice] - =flash[:notice] -- if flash[:alert] - =flash[:alert] %h2= @ticket.title -is open? -= @ticket.is_open -- if @ticket.code - code: - = @ticket.code + +%a#title.editable.editable-click{"data-name" => "title", "data-resource" => "post", "data-type" => "text", "data-url" => ticket_path(@ticket.id), "data-pk" => @ticket.id, :href => "#"} + = @ticket.title + +%p - 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' + = button_to 'close', {:post => {:is_open => false}}, :method => :put + - else + = 'closed' + = button_to 'open', {:post => {:is_open => true}}, :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 - = 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 += simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test + = render :partial => 'new_comment', :locals => {:f => f, :comment_object => TicketComment.new} + = 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? += 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 b9e03ac..b29fa17 100644 --- a/help/test/functional/tickets_controller_test.rb +++ b/help/test/functional/tickets_controller_test.rb @@ -2,38 +2,83 @@ require 'test_helper' class TicketsControllerTest < ActionController::TestCase - test "should get index" do + setup do + User.create(User.valid_attributes_hash.merge({:login => 'first_test'})) + User.create(User.valid_attributes_hash.merge({:login => 'different'})) + Ticket.create( {:title => "stub test ticket", :id => 'stubtestticketid', :comments_attributes => {"0" => {"body" =>"body of stubbed test ticket"}}}) + Ticket.create( {:title => "stub test ticket two", :id => 'stubtestticketid2', :comments_attributes => {"0" => {"body" =>"body of second stubbed test ticket"}}}) + end + + teardown do + User.find_by_login('first_test').destroy + User.find_by_login('different').destroy + Ticket.find('stubtestticketid').destroy + Ticket.find('stubtestticketid2').destroy + end + + test "should get index if logged in" do + login :is_admin? => false get :index assert_response :success assert_not_nil assigns(:tickets) end + test "no index if not logged in" do + get :index + assert_response :redirect + assert_nil assigns(:tickets) + end + test "should get new" do get :new assert_equal Ticket, assigns(:ticket).class 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 :email => "test@email.net" @@ -42,23 +87,198 @@ class TicketsControllerTest < ActionController::TestCase 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.find('stubtestticketid') + ticket.created_by = nil # TODO: hacky, but this makes sure this ticket is an unauthenticated one + ticket.save + + assert_difference('Ticket.find("stubtestticketid").comments.count') do + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + end + + assert_equal ticket, assigns(:ticket) # still same ticket, with different comments + assert_not_equal ticket.comments, assigns(:ticket).comments # ticket == assigns(:ticket), but they have different comments (which we want) + + end + + + test "add comment to own authenticated ticket" do + + login User.last + ticket = Ticket.find('stubtestticketid') + ticket.created_by = @current_user.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.find("stubtestticketid").comments.count') do + put :update, :id => ticket.id, + :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + end + assert_not_equal ticket.comments, assigns(:ticket).comments + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + + end + + + test "cannot comment if it is not your ticket" do + + login :is_admin? => false, :email => nil + ticket = Ticket.first + + assert_not_nil User.first.id + ticket.created_by = User.first.id + 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" =>"not allowed comment"}} } + assert_response :redirect + assert_access_denied + + assert_equal ticket.comments, assigns(:ticket).comments + end - test "add comment to ticket" do - ticket = Ticket.last - assert_difference('Ticket.last.comments.count') do + test "admin add comment to authenticated ticket" do + + login :is_admin? => true + + ticket = Ticket.find('stubtestticketid') + assert_not_nil User.last.id + ticket.created_by = User.last.id # TODO: hacky, but confirms it somebody elses ticket: + assert_not_equal User.last.id, @current_user.id + ticket.save + + #admin should be able to comment: + assert_difference('Ticket.find("stubtestticketid").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 "tickets by admin" do + + login :is_admin? => true, :email => nil + + get :index, {:admin_status => "all", :open_status => "open"} + assert assigns(:all_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[:all_tickets].count', -1) do + assigns(:tickets).first.close + assigns(:tickets).first.save + get :index, {:admin_status => "all", :open_status => "open"} + end + end + + + test "admin_status mine vs all" do + testticket = Ticket.create :title => 'temp testytest' + login :is_admin? => true, :email => nil + + get :index, {:admin_status => "all", :open_status => "open"} + assert assigns(:all_tickets).include?(testticket) + get :index, {:admin_status => "mine", :open_status => "open"} + assert !assigns(:all_tickets).include?(testticket) + end + + test "commenting on a ticket adds to tickets that are mine" do + testticket = Ticket.create :title => 'temp testytest' + login :is_admin? => true, :email => nil + + get :index, {:admin_status => "mine", :open_status => "open"} + assert_difference('assigns[:all_tickets].count') do + put :update, :id => testticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} + get :index, {:admin_status => "mine", :open_status => "open"} + end + + assert assigns(:all_tickets).include?(assigns(:ticket)) + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + + assigns(:ticket).destroy + end + + test "admin ticket ordering" do + + login :is_admin? => true, :email => nil + get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_desc'} + + # this will consider all tickets, not just those on first page + first_tick = assigns(:all_tickets).all.first + last_tick = assigns(:all_tickets).all.last + assert first_tick.created_at > last_tick.created_at + + # and now reverse order: + get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_asc'} + + assert_equal first_tick, assigns(:all_tickets).last + assert_equal last_tick, assigns(:all_tickets).first + + assert_not_equal first_tick, assigns(:all_tickets).first + assert_not_equal last_tick, assigns(:all_tickets).last + + end + + test "tickets for regular user" do + login :is_admin? => false, :email => nil + + put :update, :id => 'stubtestticketid',:ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + assert_not_nil assigns(:ticket).comments.last.posted_by + assert_equal assigns(:ticket).comments.last.posted_by, @current_user.id + + get :index, {:open_status => "open"} + assert assigns(:all_tickets).count > 0 + assert assigns(:all_tickets).include?(Ticket.find('stubtestticketid')) + + assert !assigns(:all_tickets).include?(Ticket.find('stubtestticketid2')) + + # user should have one more ticket if a new tick gets a comment by this user + assert_difference('assigns[:all_tickets].count') do + put :update, :id => 'stubtestticketid2' , :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} + get :index, {:open_status => "open"} + end + assert assigns(:all_tickets).include?(Ticket.find('stubtestticketid2')) + + # if we close one ticket, the user should have 1 less open ticket + assert_difference('assigns[:all_tickets].count', -1) do + t = Ticket.find('stubtestticketid2') + t.close + t.save + get :index, {:open_status => "open"} + end + + number_open_tickets = assigns(:all_tickets).count + + # look at closed tickets: + get :index, {:open_status => "closed"} + assert assigns(:all_tickets).include?(Ticket.find('stubtestticketid2')) + assert !assigns(:all_tickets).include?(Ticket.find('stubtestticketid')) + number_closed_tickets = assigns(:all_tickets).count + + # all tickets should equal closed + open + get :index, {:open_status => "all"} + assert assigns(:all_tickets).include?(Ticket.find('stubtestticketid2')) + assert assigns(:all_tickets).include?(Ticket.find('stubtestticketid')) + assert_equal assigns(:all_tickets).count, number_closed_tickets + number_open_tickets end end + diff --git a/help/test/unit/ticket_test.rb b/help/test/unit/ticket_test.rb index 6b63a23..e93a121 100644 --- a/help/test/unit/ticket_test.rb +++ b/help/test/unit/ticket_test.rb @@ -23,7 +23,7 @@ class TicketTest < ActiveSupport::TestCase #user = LeapWebUsers::User.create #t.user = user - + #t.email = '' #invalid #assert !t.valid? #t.email = 'blah@blah.com, bb@jjj.org' @@ -40,7 +40,7 @@ class TicketTest < ActiveSupport::TestCase @sample.created_by = 22 #current_user assert @sample.is_creator_validated? end - + =begin # TODO: do once have current_user stuff in order test "code if & only if not creator-validated" do @@ -56,6 +56,38 @@ class TicketTest < ActiveSupport::TestCase end =end -end + test "finds open tickets sorted by created_at" do + tickets = Ticket.by_is_open_and_created_at. + startkey([true, 0]). + endkey([true, Time.now + 10.hours]) # some tickets were created in the future + assert_equal Ticket.by_is_open.key(true).count, tickets.count + end + + test "find tickets user commented on" do + + # clear old tickets just in case + # this will cause RestClient::ResourceNotFound errors if there are multiple copies of the same ticket returned + Ticket.includes_post_by.key('123').each {|t| t.destroy} + testticket = Ticket.create :title => "test retrieving commented tickets" + comment = TicketComment.new :body => "my email broke", :posted_by => "123" + assert_equal 0, testticket.comments.count + assert_equal [], Ticket.includes_post_by.key('123').all + testticket.comments << comment + testticket.save + assert_equal 1, testticket.reload.comments.count + assert_equal [testticket], Ticket.includes_post_by.key('123').all + + comment = TicketComment.new :body => "another comment", :posted_by => "123" + testticket.comments << comment + testticket.save + + # this will ensure that the ticket is only included once, even though the user has commented on the ticket twice: + assert_equal [testticket], Ticket.includes_post_by.key('123').all + + testticket.destroy + assert_equal [], Ticket.includes_post_by.key('123').all; + end + +end diff --git a/ui_dependencies.rb b/ui_dependencies.rb index 454e9a8..10eac67 100644 --- a/ui_dependencies.rb +++ b/ui_dependencies.rb @@ -2,8 +2,11 @@ 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' +gem 'kaminari', "0.13.0" # for pagination. trying 0.13.0 as there seem to be issues with 0.14.0 when using couchrest +gem 'bootstrap-editable-rails' group :assets do gem "haml-rails", "~> 0.3.4" diff --git a/users/app/assets/javascripts/users.js.coffee b/users/app/assets/javascripts/users.js.coffee index 0595292..0c1fb55 100644 --- a/users/app/assets/javascripts/users.js.coffee +++ b/users/app/assets/javascripts/users.js.coffee @@ -32,3 +32,7 @@ $(document).ready -> $('.user.form.change_password').submit srp.update $('.user.form.change_password').submit preventDefault $('.user.typeahead').typeahead({source: pollUsers}); + $('a[data-toggle="tab"]').on('shown', -> + $(ClientSideValidations.selectors.forms).validate() + ) + diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb index 6ac7a5b..f2184d9 100644 --- a/users/app/controllers/controller_extension/authentication.rb +++ b/users/app/controllers/controller_extension/authentication.rb @@ -24,7 +24,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/app/controllers/email_aliases_controller.rb b/users/app/controllers/email_aliases_controller.rb new file mode 100644 index 0000000..751df85 --- /dev/null +++ b/users/app/controllers/email_aliases_controller.rb @@ -0,0 +1,39 @@ +class EmailAliasesController < ApplicationController + + before_filter :fetch_user + + respond_to :html + + # get a list of email aliases for the given user? + def index + @aliases = @user.email_aliases + respond_with @aliases + end + + def create + @alias = @user.add_email_alias(params[:email_alias]) + flash[:notice] = t(:email_alias_created_successfully) unless @alias.errors + respond_with @alias, :location => edit_user_path(@user, :anchor => :email) + end + + def update + @alias = @user.get_email_alias(params[:id]) + @alias.set_email(params[:email_alias]) + flash[:notice] = t(:email_alias_updated_successfully) unless @alias.errors + respond_with @alias, :location => edit_user_path(@user, :anchor => :email) + end + + def destroy + @alias = @user.get_email_alias(params[:id]) + flash[:notice] = t(:email_alias_destroyed_successfully) + @alias.destroy + redirect_to edit_user_path(@user, :anchor => :email) + end + + protected + + def fetch_user + @user = User.find_by_param(params[:user_id]) + access_denied unless admin? or (@user == current_user) + end +end diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 4921a4a..811e8e5 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -30,8 +30,11 @@ class UsersController < ApplicationController end def update - if @user.update_attributes(params[:user]) + @user.attributes = params[:user] + if @user.changed? and @user.save flash[:notice] = t(:user_updated_successfully) + else + flash[:error] = @user.errors.full_messages end respond_with @user, :location => edit_user_path(@user) end diff --git a/users/app/helpers/email_aliases_helper.rb b/users/app/helpers/email_aliases_helper.rb new file mode 100644 index 0000000..b56b068 --- /dev/null +++ b/users/app/helpers/email_aliases_helper.rb @@ -0,0 +1,11 @@ +module EmailAliasesHelper + + def email_alias_form(options = {}) + simple_form_for [@user, EmailAlias.new()], + :html => {:class => "form-horizontal email-alias form"}, + :validate => true do |f| + yield f + end + end + +end diff --git a/users/app/models/email.rb b/users/app/models/email.rb new file mode 100644 index 0000000..4b01838 --- /dev/null +++ b/users/app/models/email.rb @@ -0,0 +1,17 @@ +class Email + include CouchRest::Model::Embeddable + + property :email, String + + validates :email, + :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + + def initialize(attributes = nil, &block) + attributes = {:email => attributes} if attributes.is_a? String + super(attributes, &block) + end + + def to_s + email + end +end diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb new file mode 100644 index 0000000..7cca4f4 --- /dev/null +++ b/users/app/models/local_email.rb @@ -0,0 +1,15 @@ +class LocalEmail < Email + + validate :unique_on_server + + def to_partial_path + "emails/email" + end + + def unique_on_server + has_email = User.find_by_email_or_alias(email) + if has_email && has_email != self.base_doc + errors.add(:email, "has already been taken") + end + end +end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 340ad9c..10f358d 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -3,11 +3,13 @@ class User < CouchRest::Model::Base use_database :users property :login, String, :accessible => true - property :email, String, :accessible => true - property :email_forward, String, :accessible => true property :password_verifier, String, :accessible => true property :password_salt, String, :accessible => true + property :email, String, :accessible => true + property :email_forward, String, :accessible => true + property :email_aliases, [LocalEmail] + validates :login, :password_salt, :password_verifier, :presence => true @@ -26,11 +28,48 @@ class User < CouchRest::Model::Base :confirmation => true, :format => { :with => /.{8}.*/, :message => "needs to be at least 8 characters long" } + # TODO: write a proper email validator to be used in the different places + validates :email, + :format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"} + + validates :email_forward, + :format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"} + + validate :no_duplicate_email_aliases + + validate :email_aliases_differ_from_email + timestamps! design do view :by_login view :by_created_at + view :by_email + view :by_email_alias, + :map => <<-EOJS + function(doc) { + if (doc.type != 'User') { + return; + } + doc.email_aliases.forEach(function(alias){ + emit(alias.email, doc); + }); + } + EOJS + view :by_email_or_alias, + :map => <<-EOJS + function(doc) { + if (doc.type != 'User') { + return; + } + if (doc.email) { + emit(doc.email, doc); + } + doc.email_aliases.forEach(function(alias){ + emit(alias.email, doc); + }); + } + EOJS end class << self @@ -75,6 +114,36 @@ class User < CouchRest::Model::Base APP_CONFIG['admins'].include? self.login end + def add_email_alias(email) + email = LocalEmail.new(email) unless email.is_a? Email + email_aliases << email + end + + # this currently only adds the first email address submitted. + # All the ui needs for now. + def email_aliases_attributes=(attrs) + if attrs && attrs.values.first + add_email_alias attrs.values.first + end + end + + ## + # Validation Functions + ## + + # TODO: How do we handle these errors? + def no_duplicate_email_aliases + if email_aliases.count != email_aliases.map(&:email).uniq.count + errors.add(:email_aliases, "include a duplicate") + end + end + + def email_aliases_differ_from_email + if email_aliases.map(&:email).include?(email) + errors.add(:email_aliases, "include the original email address") + end + end + protected def password password_verifier diff --git a/users/app/views/emails/_email.html.haml b/users/app/views/emails/_email.html.haml new file mode 100644 index 0000000..f182ed9 --- /dev/null +++ b/users/app/views/emails/_email.html.haml @@ -0,0 +1,4 @@ +%li.pull-right + %code= email + %i.icon-remove +.clearfix diff --git a/users/app/views/users/_email_aliases.html.haml b/users/app/views/users/_email_aliases.html.haml new file mode 100644 index 0000000..646480e --- /dev/null +++ b/users/app/views/users/_email_aliases.html.haml @@ -0,0 +1,6 @@ +.span6 + %ul.unstyled + =render @user.email_aliases +.clearfix += f.simple_fields_for :email_aliases, Email.new do |e| + = e.input :email, :placeholder => "alias@#{request.domain}" diff --git a/users/app/views/users/edit.html.haml b/users/app/views/users/edit.html.haml index b33c19b..92ab71b 100644 --- a/users/app/views/users/edit.html.haml +++ b/users/app/views/users/edit.html.haml @@ -14,3 +14,4 @@ .tab-pane#email = user_form_with 'email_field', :legend => :set_email_address = user_form_with 'email_forward_field', :legend => :forward_email + = user_form_with 'email_aliases', :legend => :add_email_alias diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index fe7e824..d068e70 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -12,6 +12,7 @@ en: set_email_address: "Set email address" forward_email: "Forward email" email_aliases: "Email aliases" + add_email_alias: "Add email alias" user_updated_successfully: "Settings have been updated successfully." user_created_successfully: "Successfully created your account." diff --git a/users/config/routes.rb b/users/config/routes.rb index 6de216f..3c5fb73 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -10,6 +10,8 @@ Rails.application.routes.draw do resources :sessions, :only => [:new, :create, :update, :destroy] get "signup" => "users#new", :as => "signup" - resources :users + resources :users do + resources :email_aliases + end end 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/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 1840a72..ce17500 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -1,7 +1,6 @@ require 'test_helper' class UsersControllerTest < ActionController::TestCase - include StubRecordHelper test "should get new" do get :new @@ -35,7 +34,10 @@ class UsersControllerTest < ActionController::TestCase end test "should get edit view" do - user = find_record User, :email => nil, :email_forward => nil + user = find_record User, + :email => nil, + :email_forward => nil, + :email_aliases => [] login user get :edit, :id => user.id @@ -45,7 +47,9 @@ class UsersControllerTest < ActionController::TestCase test "should process updated params" do user = find_record User - user.expects(:update_attributes).with(user.params).returns(true) + user.expects(:attributes=).with(user.params) + user.expects(:changed?).returns(true) + user.expects(:save).returns(true) login user put :update, :user => user.params, :id => user.id, :format => :json @@ -57,7 +61,9 @@ class UsersControllerTest < ActionController::TestCase test "admin can update user" do user = find_record User - user.expects(:update_attributes).with(user.params).returns(true) + user.expects(:attributes=).with(user.params) + user.expects(:changed?).returns(true) + user.expects(:save).returns(true) login :is_admin? => true put :update, :user => user.params, :id => user.id, :format => :json diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb index f3506ae..c9f5612 100644 --- a/users/test/support/auth_test_helper.rb +++ b/users/test/support/auth_test_helper.rb @@ -1,5 +1,4 @@ module AuthTestHelper - include StubRecordHelper extend ActiveSupport::Concern # Controller will fetch current user from warden. @@ -19,10 +18,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/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb index 2e1a533..1be419a 100644 --- a/users/test/support/stub_record_helper.rb +++ b/users/test/support/stub_record_helper.rb @@ -39,3 +39,7 @@ module StubRecordHelper end end + +class ActionController::TestCase + include StubRecordHelper +end diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb new file mode 100644 index 0000000..762aaea --- /dev/null +++ b/users/test/unit/email_aliases_test.rb @@ -0,0 +1,57 @@ +require 'test_helper' + +class EmailAliasTest < ActiveSupport::TestCase + + setup do + @attribs = User.valid_attributes_hash + User.find_by_login(@attribs[:login]).try(:destroy) + @user = User.new(@attribs) + end + + test "no email aliases set in the beginning" do + assert_equal [], @user.email_aliases + end + + test "adding email alias through params" do + email_alias = "valid_alias@domain.net" + @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + assert @user.changed? + assert @user.save + assert_equal email_alias, @user.email_aliases.first.to_s + end + + test "adding email alias directly" do + email_alias = "valid_alias@domain.net" + @user.add_email_alias(email_alias) + assert @user.changed? + assert @user.save + assert_equal email_alias, @user.reload.email_aliases.first.to_s + end + + test "duplicated email aliases are invalid" do + email_alias = "valid_alias@domain.net" + @user.add_email_alias(email_alias) + @user.save + # add again + @user.add_email_alias(email_alias) + assert @user.changed? + assert !@user.valid? + end + + test "email is invalid as email alias" do + email_alias = "valid_alias@domain.net" + @user.email = email_alias + @user.add_email_alias(email_alias) + assert @user.changed? + assert !@user.valid? + end + + test "find user by email alias" do + email_alias = "valid_alias@domain.net" + @user.add_email_alias(email_alias) + assert @user.save + assert_equal @user, User.find_by_email_or_alias(email_alias) + assert_equal @user, User.find_by_email_alias(email_alias) + assert_nil User.find_by_email(email_alias) + end +end diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb new file mode 100644 index 0000000..1e216d6 --- /dev/null +++ b/users/test/unit/email_test.rb @@ -0,0 +1,39 @@ +require 'test_helper' + +class EmailTest < ActiveSupport::TestCase + + setup do + # TODO build helper for this ... make_record(User) + @attribs = User.valid_attributes_hash + User.find_by_login(@attribs[:login]).try(:destroy) + @user = User.new(@attribs) + @attribs.merge!(:login => "other_user") + User.find_by_login(@attribs[:login]).try(:destroy) + @other_user = User.create(@attribs) + end + + teardown do + @user.destroy if @user.persisted? # just in case + @other_user.destroy + end + + + test "email aliases need to be unique" do + email_alias = "valid_alias@domain.net" + @other_user.add_email_alias email_alias + @other_user.save + @user.add_email_alias email_alias + assert @user.changed? + assert !@user.save + # TODO handle errors + end + + test "email aliases may not conflict with emails" do + email_alias = "valid_alias@domain.net" + @other_user.email = email_alias + @other_user.save + @user.add_email_alias email_alias + assert @user.changed? + assert !@user.save + end +end diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb index cce11c2..5d2a7ea 100644 --- a/users/test/unit/user_test.rb +++ b/users/test/unit/user_test.rb @@ -49,4 +49,22 @@ 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 + + test "find user by email" do + email = "tryto@find.me" + @user.email = email + @user.save + assert_equal @user, User.find_by_email(email) + assert_equal @user, User.find_by_email_or_alias(email) + assert_nil User.find_by_email_alias(email) + end end |