diff options
39 files changed, 403 insertions, 274 deletions
diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index cb5fa1b..bb3b8bc 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -1,16 +1,25 @@ +// +// import custom scss, content to be set in deployment. +// +@import "tail"; + +// +// import bootstrap. +// @import "bootstrap"; body { padding: 40px; } @import "bootstrap-responsive"; - - table.table-hover .btn { opacity: 0; } - table.table-hover tr:hover .btn { opacity: 1; } +@import "bootstrap-editable"; -@import "bootstrap-editable";
\ No newline at end of file +// +// import custom scss, content to be set in deployment. +// +@import "tail"; diff --git a/app/assets/stylesheets/head.scss b/app/assets/stylesheets/head.scss new file mode 100644 index 0000000..431565f --- /dev/null +++ b/app/assets/stylesheets/head.scss @@ -0,0 +1,3 @@ +// +// put custom scss here that goes before all the others +// diff --git a/app/assets/stylesheets/tail.scss b/app/assets/stylesheets/tail.scss new file mode 100644 index 0000000..541c2df --- /dev/null +++ b/app/assets/stylesheets/tail.scss @@ -0,0 +1,3 @@ +// +// put custom scss here that goes after all the others +// diff --git a/common_dependencies.rb b/common_dependencies.rb index a6691cf..1650fee 100644 --- a/common_dependencies.rb +++ b/common_dependencies.rb @@ -4,3 +4,8 @@ group :test do gem 'mocha', '~> 0.13.0', :require => false end +group :test, :development do + gem 'faker' + gem 'factory_girl_rails' +end + diff --git a/core/lib/extensions/couchrest.rb b/core/lib/extensions/couchrest.rb index a8da23b..5938df4 100644 --- a/core/lib/extensions/couchrest.rb +++ b/core/lib/extensions/couchrest.rb @@ -1,8 +1,23 @@ -class CouchRest::Model::Designs::View - - # so we can called Ticket.method.descending or Ticket.method.ascending - def ascending - self - end +module CouchRest::Model::Designs + + class View + + # so we can called Ticket.method.descending or Ticket.method.ascending + def ascending + self + end + end + + class DesignMapper + def load_views(dir) + Dir.glob("#{dir}/*.js") do |js| + name = File.basename(js, '.js') + file = File.open(js, 'r') + view name.to_sym, + :map => file.read, + :reduce => "function(key, values, rereduce) { return sum(values); }" + end + end + end end diff --git a/help/app/designs/ticket/by_includes_post_by.js b/help/app/designs/ticket/by_includes_post_by.js new file mode 100644 index 0000000..2eeac89 --- /dev/null +++ b/help/app/designs/ticket/by_includes_post_by.js @@ -0,0 +1,13 @@ +// TODO: This view is only used in tests--should we keep it? +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); + } + }); + } +} diff --git a/help/app/designs/ticket/by_includes_post_by_and_created_at.js b/help/app/designs/ticket/by_includes_post_by_and_created_at.js new file mode 100644 index 0000000..72169b0 --- /dev/null +++ b/help/app/designs/ticket/by_includes_post_by_and_created_at.js @@ -0,0 +1,12 @@ +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); + } + }); + } +} diff --git a/help/app/designs/ticket/by_includes_post_by_and_is_open_and_created_at.js b/help/app/designs/ticket/by_includes_post_by_and_is_open_and_created_at.js new file mode 100644 index 0000000..33dfe0b --- /dev/null +++ b/help/app/designs/ticket/by_includes_post_by_and_is_open_and_created_at.js @@ -0,0 +1,12 @@ +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); + } + }); + } +} diff --git a/help/app/designs/ticket/by_includes_post_by_and_is_open_and_updated_at.js b/help/app/designs/ticket/by_includes_post_by_and_is_open_and_updated_at.js new file mode 100644 index 0000000..3bd2a74 --- /dev/null +++ b/help/app/designs/ticket/by_includes_post_by_and_is_open_and_updated_at.js @@ -0,0 +1,12 @@ +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); + } + }); + } +} diff --git a/help/app/designs/ticket/by_includes_post_by_and_updated_at.js b/help/app/designs/ticket/by_includes_post_by_and_updated_at.js new file mode 100644 index 0000000..2b4304f --- /dev/null +++ b/help/app/designs/ticket/by_includes_post_by_and_updated_at.js @@ -0,0 +1,12 @@ +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); + } + }); + } +} diff --git a/help/app/models/ticket.rb b/help/app/models/ticket.rb index 262d5a4..ed1ff9d 100644 --- a/help/app/models/ticket.rb +++ b/help/app/models/ticket.rb @@ -47,84 +47,7 @@ class Ticket < CouchRest::Model::Base view :by_is_open_and_created_at view :by_is_open_and_updated_at - - #TODO: This view is only used in tests--should we keep it? - view :by_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 :by_includes_post_by_and_is_open_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 :by_includes_post_by_and_is_open_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 :by_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 :by_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); }" - + load_views(Rails.root.join('help', 'app', 'designs', 'ticket')) end validates :title, :presence => true @@ -132,7 +55,7 @@ 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 + validates :email, :allow_blank => true, :format => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/ #TODO: #def set_created_by diff --git a/help/app/views/tickets/_comment.html.haml b/help/app/views/tickets/_comment.html.haml index 01cf01a..501ceec 100644 --- a/help/app/views/tickets/_comment.html.haml +++ b/help/app/views/tickets/_comment.html.haml @@ -7,7 +7,8 @@ = 'Posted by' + (comment.posted_by_user.is_admin? ? ' admin' : '') + ':' = comment.posted_by_user.login - else - Unauthenticated post + %b + Unauthenticated post - if comment.private (Private comment) .pull-right @@ -15,4 +16,5 @@ Posted at: = comment.posted_at.to_s(:short) %br - = comment.body
\ No newline at end of file + = comment.body + diff --git a/help/app/views/tickets/_new_comment.html.haml b/help/app/views/tickets/_new_comment.html.haml index 8d40bb6..96388ea 100644 --- a/help/app/views/tickets/_new_comment.html.haml +++ b/help/app/views/tickets/_new_comment.html.haml @@ -1,4 +1,4 @@ = f.simple_fields_for :comments, @comment do |c| - = c.input :body, :label => 'Comment', :as => :text, :input_html => {:class => "span12", :rows=>4} + = c.input :body, :label => 'Comment', :as => :text, :input_html => {:class => "span9", :rows=>4} - if admin? = c.input :private, :as => :boolean, :label => false, :inline_label => true diff --git a/help/app/views/tickets/_ticket.html.haml b/help/app/views/tickets/_ticket.html.haml index 3edfa8b..7b37652 100644 --- a/help/app/views/tickets/_ticket.html.haml +++ b/help/app/views/tickets/_ticket.html.haml @@ -1,13 +1,17 @@ +- updated_at_text = 'updated: ' + ticket.updated_at.to_s(:long) %tr %td %b = link_to ticket.title, ticket - %br - %small - created: - = ticket.created_at.to_s(:short) - updated: - = ticket.updated_at.to_s(:short) - %small.pull-right - comments by: - = ticket.commenters + - if params[:controller] == 'tickets' + %br + %small + created: + = ticket.created_at.to_s(:long) + = updated_at_text + %small.pull-right + comments by: + = ticket.commenters + - else + %small + = updated_at_text
\ No newline at end of file diff --git a/help/app/views/tickets/_ticket_data.html.haml b/help/app/views/tickets/_ticket_data.html.haml index 2261d8f..d68d3e9 100644 --- a/help/app/views/tickets/_ticket_data.html.haml +++ b/help/app/views/tickets/_ticket_data.html.haml @@ -32,4 +32,4 @@ = button_to 'Close', {:post => {:is_open => false}}, :method => :put, :class => 'btn btn-small' - else = 'closed' - = button_to 'Open', {:post => {:is_open => true}}, :method => :put, :class => 'btn btn-small'
\ No newline at end of file + = button_to 'Open', {:post => {:is_open => true}}, :method => :put, :class => 'btn btn-small' diff --git a/help/app/views/tickets/new.html.haml b/help/app/views/tickets/new.html.haml index af8baad..1aa689b 100644 --- a/help/app/views/tickets/new.html.haml +++ b/help/app/views/tickets/new.html.haml @@ -1,6 +1,6 @@ .span12 %h2=t :new_ticket - = simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test + = simple_form_for @ticket, :validate => true, :html => {:class => 'form-horizontal'} do |f| = f.input :title = f.input :email if !current_user #hmm--might authenticated users want to submit an alternate email? = f.input :regarding_user diff --git a/help/app/views/tickets/show.html.haml b/help/app/views/tickets/show.html.haml index 3f00b35..a69048b 100644 --- a/help/app/views/tickets/show.html.haml +++ b/help/app/views/tickets/show.html.haml @@ -8,7 +8,7 @@ = render(:partial => "comment", :collection => @ticket.comments) = #render @ticket.comments should work if view is in /app/views/comments/_comment - = simple_form_for(@ticket, :html => {:novalidate => true}) do |f| #turn off html5 validations to test + = simple_form_for @ticket, :html => {:class => 'form-horizontal'} do |f| # don't need validations so long as this is so simple = render :partial => 'new_comment', :locals => {:f => f} .span10.offset3 = f.button :submit, @post_reply_str, :class => 'btn-primary' diff --git a/help/test/factories.rb b/help/test/factories.rb new file mode 100644 index 0000000..5b38952 --- /dev/null +++ b/help/test/factories.rb @@ -0,0 +1,10 @@ +FactoryGirl.define do + + factory :ticket do + title { Faker::Lorem.sentence } + comments_attributes do + { "0" => { "body" => Faker::Lorem.sentences.join(" ") } } + end + end + +end diff --git a/help/test/functional/tickets_controller_test.rb b/help/test/functional/tickets_controller_test.rb index 7060936..0c462a9 100644 --- a/help/test/functional/tickets_controller_test.rb +++ b/help/test/functional/tickets_controller_test.rb @@ -3,21 +3,17 @@ require 'test_helper' class TicketsControllerTest < ActionController::TestCase 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"}}}) + @user = FactoryGirl.create :user + @other_user = FactoryGirl.create :user end teardown do - User.find_by_login('first_test').destroy - User.find_by_login('different').destroy - Ticket.find('stubtestticketid').destroy - Ticket.find('stubtestticketid2').destroy + @user.destroy + @other_user.destroy end test "should get index if logged in" do - login :is_admin? => false + login get :index assert_response :success assert_not_nil assigns(:tickets) @@ -35,29 +31,32 @@ class TicketsControllerTest < ActionController::TestCase assert_response :success end - test "ticket show access" do - ticket = Ticket.first - ticket.created_by = nil # TODO: hacky, but this makes sure this ticket is an unauthenticated one - ticket.save + test "unauthenticated tickets are visible" do + ticket = find_record :ticket, :created_by => nil get :show, :id => ticket.id assert_response :success + end - ticket.created_by = User.last.id - ticket.save + test "user tickets are not visible without login" do + ticket = find_record :ticket, :created_by => @user.id get :show, :id => ticket.id assert_response :redirect assert_redirected_to login_url + end - login(User.last) + test "user tickets are visible to creator" do + ticket = find_record :ticket, :created_by => @user.id + login @user get :show, :id => ticket.id assert_response :success + end - login(User.first) #assumes User.first != User.last: - assert_not_equal User.first, User.last + test "user tickets are not visible to other user" do + ticket = find_record :ticket, :created_by => @user.id + login @other_user get :show, :id => ticket.id assert_response :redirect assert_redirected_to root_url - end test "should create unauthenticated ticket" do @@ -80,7 +79,7 @@ class TicketsControllerTest < ActionController::TestCase params = {:title => "auth ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}} - login :email => "test@email.net" + login :email => Faker::Internet.user_name + '@' + APP_CONFIG[:domain] assert_difference('Ticket.count') do post :create, :ticket => params @@ -99,11 +98,9 @@ class TicketsControllerTest < ActionController::TestCase 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 + ticket = FactoryGirl.create :ticket, :created_by => nil - assert_difference('Ticket.find("stubtestticketid").comments.count') do + assert_difference('Ticket.find(ticket.id).comments.count') do put :update, :id => ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } end @@ -111,24 +108,24 @@ class TicketsControllerTest < ActionController::TestCase 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) + assigns(:ticket).destroy 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 + ticket = FactoryGirl.create :ticket, :created_by => @current_user.id #they should be able to comment if it is their ticket: - assert_difference('Ticket.find("stubtestticketid").comments.count') do + assert_difference('Ticket.find(ticket.id).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 + assigns(:ticket).destroy end @@ -136,17 +133,13 @@ class TicketsControllerTest < ActionController::TestCase 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 + ticket = FactoryGirl.create :ticket, :created_by => @other_user.id # 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 + assert_equal ticket.comments.map(&:body), assigns(:ticket).comments.map(&:body) end @@ -155,14 +148,10 @@ class TicketsControllerTest < ActionController::TestCase 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 + ticket = FactoryGirl.create :ticket, :created_by => @other_user.id #admin should be able to comment: - assert_difference('Ticket.find("stubtestticketid").comments.count') do + assert_difference('Ticket.find(ticket.id).comments.count') do put :update, :id => ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } end @@ -170,9 +159,11 @@ class TicketsControllerTest < ActionController::TestCase 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 "tickets by admin" do + ticket = FactoryGirl.create :ticket, :created_by => @other_user.id login :is_admin? => true, :email => nil @@ -189,17 +180,18 @@ class TicketsControllerTest < ActionController::TestCase test "admin_status mine vs all" do - testticket = Ticket.create :title => 'temp testytest' + testticket = FactoryGirl.create :ticket 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) + testticket.destroy end test "commenting on a ticket adds to tickets that are mine" do - testticket = Ticket.create :title => 'temp testytest' + testticket = FactoryGirl.create :ticket login :is_admin? => true, :email => nil get :index, {:admin_status => "mine", :open_status => "open"} @@ -216,6 +208,7 @@ class TicketsControllerTest < ActionController::TestCase end test "admin ticket ordering" do + tickets = FactoryGirl.create_list :ticket, 2 login :is_admin? => true, :email => nil get :index, {:admin_status => "all", :open_status => "open", :sort_order => 'created_at_desc'} @@ -234,33 +227,36 @@ class TicketsControllerTest < ActionController::TestCase assert_not_equal first_tick, assigns(:all_tickets).first assert_not_equal last_tick, assigns(:all_tickets).last + tickets.each {|ticket| ticket.destroy} end test "tickets for regular user" do - login :is_admin? => false, :email => nil + login + ticket = FactoryGirl.create :ticket + other_ticket = FactoryGirl.create :ticket - put :update, :id => 'stubtestticketid',:ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}} } + put :update, :id => ticket.id, + :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')) + assert assigns(:all_tickets).include?(ticket) + assert !assigns(:all_tickets).include?(other_ticket) # 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"}}} + put :update, :id => other_ticket.id, :ticket => {:comments_attributes => {"0" => {"body" =>"NEWER comment"}}} get :index, {:open_status => "open"} end - assert assigns(:all_tickets).include?(Ticket.find('stubtestticketid2')) + assert assigns(:all_tickets).include?(other_ticket) # 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 + other_ticket.reload + other_ticket.close + other_ticket.save get :index, {:open_status => "open"} end @@ -268,16 +264,18 @@ class TicketsControllerTest < ActionController::TestCase # 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')) + assert !assigns(:all_tickets).include?(ticket) + assert assigns(:all_tickets).include?(other_ticket) 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 assigns(:all_tickets).include?(ticket) + assert assigns(:all_tickets).include?(other_ticket) assert_equal assigns(:all_tickets).count, number_closed_tickets + number_open_tickets + assigns(:all_tickets).each {|t| t.destroy} + end end diff --git a/help/test/unit/ticket_comment_test.rb b/help/test/unit/ticket_comment_test.rb index 1fe1fe2..44865ed 100644 --- a/help/test/unit/ticket_comment_test.rb +++ b/help/test/unit/ticket_comment_test.rb @@ -21,11 +21,11 @@ class TicketCommentTest < ActiveSupport::TestCase #comment.ticket = testticket #Ticket.find_by_title("testing") #assert_equal testticket.title, comment.ticket.title - + #tc.ticket = Ticket.find_by_title("test title") #tc.ticket.title end - + =begin test "create authenticated comment" do User.current = 4 @@ -49,6 +49,7 @@ class TicketCommentTest < ActiveSupport::TestCase testticket.comments << comment2 #this should validate comment2 testticket.valid? assert_equal testticket.comments.count, 2 + testticket.reload.destroy # where should posted_at be set? #assert_not_nil comment.posted_at #assert_not_nil testticket.comments.last.posted_at diff --git a/help/test/unit/ticket_test.rb b/help/test/unit/ticket_test.rb index ac6426e..ce35e1d 100644 --- a/help/test/unit/ticket_test.rb +++ b/help/test/unit/ticket_test.rb @@ -32,6 +32,7 @@ class TicketTest < ActiveSupport::TestCase #p t.email_address #p t.email_address.strip =~ RFC822::EmailAddress assert !t.valid? + t.reload.destroy end test "creation validated" do diff --git a/test/factories.rb b/test/factories.rb new file mode 100644 index 0000000..6c671f8 --- /dev/null +++ b/test/factories.rb @@ -0,0 +1,3 @@ +Dir.glob(Rails.root.join('**','test','factories.rb')) do |factory_file| + require factory_file +end 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/designs/user/by_email_alias.js b/users/app/designs/user/by_email_alias.js new file mode 100644 index 0000000..508a002 --- /dev/null +++ b/users/app/designs/user/by_email_alias.js @@ -0,0 +1,8 @@ +function(doc) { + if (doc.type != 'User') { + return; + } + doc.email_aliases.forEach(function(alias){ + emit(alias.email, 1); + }); +} diff --git a/users/app/designs/user/by_email_or_alias.js b/users/app/designs/user/by_email_or_alias.js new file mode 100644 index 0000000..71fd0ea --- /dev/null +++ b/users/app/designs/user/by_email_or_alias.js @@ -0,0 +1,11 @@ +function(doc) { + if (doc.type != 'User') { + return; + } + if (doc.email) { + emit(doc.email, 1); + } + doc.email_aliases.forEach(function(alias){ + emit(alias.email, 1); + }); +} 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 2a8a57b..1e8ee0e 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -18,8 +18,8 @@ class User < CouchRest::Model::Base :if => :serverside? validates :login, - :format => { :with => /\A[A-Za-z\d_]+\z/, - :message => "Only letters, digits and _ allowed" } + :format => { :with => /\A[A-Za-z\d_\.]+\z/, + :message => "Only letters, digits, . and _ allowed" } validates :password_salt, :password_verifier, :format => { :with => /\A[\dA-Fa-f]+\z/, :message => "Only hex numbers allowed" } @@ -46,52 +46,19 @@ class User < CouchRest::Model::Base timestamps! design do + load_views(Rails.root.join('users', 'app', 'designs', 'user')) 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 alias_method :find_by_param, :find - - # valid set of attributes for testing - def valid_attributes_hash - { :login => "me", - :password_verifier => "1234ABCD", - :password_salt => "4321AB" } - end - end - alias_method :to_param, :id + def to_param + self.id + end def to_json(options={}) { @@ -127,6 +94,10 @@ class User < CouchRest::Model::Base email_aliases.build(attrs.values.first) if attrs end + def most_recent_tickets(count=3) + Ticket.for_user(self).limit(count).all #defaults to having most recent updated first + end + protected ## diff --git a/users/app/views/emails/_email.html.haml b/users/app/views/emails/_email.html.haml index 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/_user.html.haml b/users/app/views/users/_user.html.haml index 7db0041..ca03d34 100644 --- a/users/app/views/users/_user.html.haml +++ b/users/app/views/users/_user.html.haml @@ -1,5 +1,5 @@ %tr - %td= user.login + %td= link_to user.login, user %td= time_ago_in_words(user.created_at) + " ago" %td = link_to edit_user_path(user), :class => "btn btn-mini btn-primary" do diff --git a/users/app/views/users/show.html.haml b/users/app/views/users/show.html.haml new file mode 100644 index 0000000..a1eeccb --- /dev/null +++ b/users/app/views/users/show.html.haml @@ -0,0 +1,31 @@ +.span8.offset1 + %h2= @user.login + .small + = link_to 'edit', edit_user_path(@user) + %dl.offset1 + - fields = ['login', 'email', 'created_at', 'updated_at', 'email_forward'] + - fields.each do |field| + %dt + = field.titleize + %dd + = user_field(field) + %dt + =t :email_aliases + %dd + - aliases = @user.email_aliases + - if aliases.present? + %ul.pull-left.unstyled + = render aliases + - else + =t :none + .clearfix + %dt + =t :most_recently_updated_tickets + %dd + - tix = @user.most_recent_tickets + - if tix.present? + %table + %tbody + = render @user.most_recent_tickets + - else + =t :none
\ No newline at end of file diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index 3c71e7e..7a6ab90 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -1,4 +1,5 @@ en: + none: "None." signup: "Sign up" signup_message: "Please create an account." cancel: "Cancel" diff --git a/users/test/factories.rb b/users/test/factories.rb new file mode 100644 index 0000000..4bf7e62 --- /dev/null +++ b/users/test/factories.rb @@ -0,0 +1,20 @@ +FactoryGirl.define do + + factory :user do + login { Faker::Internet.user_name } + password_verifier "1234ABCD" + password_salt "4321AB" + + factory :user_with_settings do + email_forward { Faker::Internet.email } + email { Faker::Internet.user_name + '@' + APP_CONFIG[:domain] } + email_aliases_attributes do + {:a => Faker::Internet.user_name + '@' + APP_CONFIG[:domain]} + end + end + + factory :admin_user do + is_admin? true + end + end +end diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb index 1fa1462..46db4d1 100644 --- a/users/test/functional/users_controller_test.rb +++ b/users/test/functional/users_controller_test.rb @@ -9,11 +9,63 @@ 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 "admin can see other user" do + user = find_record :user, + :email => nil, + :email_forward => nil, + :email_aliases => [], + :created_at => Time.now, + :updated_at => Time.now, + :most_recent_tickets => [] + login :is_admin? => true + get :show, :id => user.id + assert_response :success + + end + + test "user cannot see other user" do + user = find_record :user, + :email => nil, + :email_forward => nil, + :email_aliases => [], + :created_at => Time.now, + :updated_at => Time.now, + :most_recent_tickets => [] + login + get :show, :id => user.id + assert_response :redirect + assert_access_denied + end + + test "should create new user" do - user = stub_record User - User.expects(:create).with(user.params).returns(user) + user_attribs = record_attributes_for :user + user = User.new(user_attribs) + User.expects(:create).with(user_attribs).returns(user) + + + post :create, :user => user_attribs, :format => :json - post :create, :user => user.params, :format => :json assert_nil session[:user_id] assert_json_response user @@ -21,23 +73,20 @@ class UsersControllerTest < ActionController::TestCase end test "should redirect to signup form on failed attempt" do - params = User.valid_attributes_hash.slice(:login) - user = User.new(params) - params.stringify_keys! + user_attribs = record_attributes_for :user + user_attribs.slice!('login') + user = User.new(user_attribs) assert !user.valid? - User.expects(:create).with(params).returns(user) + User.expects(:create).with(user_attribs).returns(user) - post :create, :user => params, :format => :json + post :create, :user => user_attribs, :format => :json assert_json_error user.errors.messages assert_response 422 end test "should get edit view" do - user = find_record User, - :email => nil, - :email_forward => nil, - :email_aliases => [] + user = find_record :user login user get :edit, :id => user.id @@ -46,14 +95,14 @@ class UsersControllerTest < ActionController::TestCase end test "user can change settings" do - user = find_record User - user.expects(:attributes=).with(user.params) + user = find_record :user + changed_attribs = record_attributes_for :user_with_settings + user.expects(:attributes=).with(changed_attribs) user.expects(:changed?).returns(true) user.expects(:save).returns(true) - user.stubs(:email_aliases).returns([]) login user - put :update, :user => user.params, :id => user.id, :format => :json + put :update, :user => changed_attribs, :id => user.id, :format => :json assert_equal user, assigns[:user] assert_response 204 @@ -61,14 +110,15 @@ class UsersControllerTest < ActionController::TestCase end test "admin can update user" do - user = find_record User - user.expects(:attributes=).with(user.params) + user = find_record :user + changed_attribs = record_attributes_for :user_with_settings + user.expects(:attributes=).with(changed_attribs.stringify_keys) user.expects(:changed?).returns(true) user.expects(:save).returns(true) user.stubs(:email_aliases).returns([]) login :is_admin? => true - put :update, :user => user.params, :id => user.id, :format => :json + put :update, :user => changed_attribs, :id => user.id, :format => :json assert_equal user, assigns[:user] assert_response 204 @@ -76,7 +126,7 @@ class UsersControllerTest < ActionController::TestCase end test "admin can destroy user" do - user = find_record User + user = find_record :user user.expects(:destroy) login :is_admin? => true @@ -87,7 +137,7 @@ class UsersControllerTest < ActionController::TestCase end test "user can cancel account" do - user = find_record User + user = find_record :user user.expects(:destroy) login user @@ -98,7 +148,7 @@ class UsersControllerTest < ActionController::TestCase end test "non-admin can't destroy user" do - user = stub_record User + user = find_record :user login delete :destroy, :id => user.id diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb index 7636f2b..b9e2a4e 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -12,10 +12,6 @@ class AccountFlowTest < ActiveSupport::TestCase OUTER_APP end - def teardown - Warden.test_reset! - end - def setup @login = "integration_test_user" User.find_by_login(@login).tap{|u| u.destroy if u} @@ -31,7 +27,8 @@ class AccountFlowTest < ActiveSupport::TestCase end def teardown - @user.destroy if @user # make sure we can run this test again + @user.destroy if @user + Warden.test_reset! end # this test wraps the api and implements the interface the ruby-srp client. diff --git a/users/test/support/auth_test_helper.rb b/users/test/support/auth_test_helper.rb index c9f5612..c0fcf3a 100644 --- a/users/test/support/auth_test_helper.rb +++ b/users/test/support/auth_test_helper.rb @@ -10,10 +10,10 @@ module AuthTestHelper end def login(user_or_method_hash = {}) - @current_user = stub_record(User, user_or_method_hash) - unless @current_user.respond_to? :is_admin? - @current_user.stubs(:is_admin?).returns(false) + if user_or_method_hash.respond_to?(:reverse_merge) + user_or_method_hash.reverse_merge! :is_admin? => false end + @current_user = stub_record(:user, user_or_method_hash, true) request.env['warden'] = stub :user => @current_user return @current_user end diff --git a/users/test/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb index 1be419a..168a827 100644 --- a/users/test/support/stub_record_helper.rb +++ b/users/test/support/stub_record_helper.rb @@ -4,10 +4,12 @@ module StubRecordHelper # return the record given. # If no record is given but a hash or nil will create a stub based on # that instead and returns the stub. - def find_record(klass, record_or_method_hash = {}) - record = stub_record(klass, record_or_method_hash) - finder = klass.respond_to?(:find_by_param) ? :find_by_param : :find_by_id - klass.expects(finder).with(record.to_param).returns(record) + def find_record(factory, attribs_hash = {}) + attribs_hash.reverse_merge!(:id => Random.rand(10000).to_s) + record = stub_record factory, attribs_hash + klass = record.class + finder = klass.respond_to?(:find_by_param) ? :find_by_param : :find + klass.expects(finder).with(record.to_param.to_s).returns(record) return record end @@ -17,25 +19,28 @@ module StubRecordHelper # If the second parameter is a record we return the record itself. # This way you can build functions that either take a record or a # method hash to stub from. See find_record for an example. - def stub_record(klass, record_or_method_hash = {}, persisted = true) + def stub_record(factory, record_or_method_hash = {}, persisted=false) if record_or_method_hash && !record_or_method_hash.is_a?(Hash) return record_or_method_hash end - stub record_params_for(klass, record_or_method_hash, persisted) + FactoryGirl.build_stubbed(factory).tap do |record| + if persisted or record.persisted? + record_or_method_hash.reverse_merge! :created_at => Time.now, + :updated_at => Time.now, :id => Random.rand(100000).to_s + end + record.stubs(record_or_method_hash) if record_or_method_hash.present? + end end - def record_params_for(klass, params = {}, persisted = true) - if klass.respond_to?(:valid_attributes_hash) - params.reverse_merge!(klass.valid_attributes_hash) + # returns deep stringified attributes so they can be compared to + # what the controller receives as params + def record_attributes_for(factory, attribs_hash = nil) + FactoryGirl.attributes_for(factory, attribs_hash).tap do |attribs| + attribs.keys.each do |key| + val = attribs.delete(key) + attribs[key.to_s] = val.is_a?(Hash) ? val.stringify_keys! : val + end end - params[:params] = params.stringify_keys - params.reverse_merge! :id => "A123", - :to_param => "A123", - :class => klass, - :to_key => ['123'], - :to_json => %Q({"stub":"#{klass.name}"}), - :new_record? => !persisted, - :persisted? => persisted end end diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb index 88f97f4..e3f060d 100644 --- a/users/test/unit/email_aliases_test.rb +++ b/users/test/unit/email_aliases_test.rb @@ -3,12 +3,8 @@ 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) - @attribs.merge!(:login => "other_user") - User.find_by_login(@attribs[:login]).try(:destroy) - @other_user = User.create(@attribs) + @user = FactoryGirl.build :user + @other_user = FactoryGirl.build :user @alias = "valid_alias@#{APP_CONFIG[:domain]}" User.find_by_email_or_alias(@alias).try(:destroy) end diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb index 060ced5..d7ef1f8 100644 --- a/users/test/unit/email_test.rb +++ b/users/test/unit/email_test.rb @@ -3,13 +3,8 @@ 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) + @user = FactoryGirl.build :user + @other_user = FactoryGirl.build :user @email_string = "valid_alias@#{APP_CONFIG[:domain]}" User.find_by_email_or_alias(@email_string).try(:destroy) end diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb index 0c79f1f..917728b 100644 --- a/users/test/unit/user_test.rb +++ b/users/test/unit/user_test.rb @@ -4,9 +4,7 @@ class UserTest < ActiveSupport::TestCase include SRP::Util setup do - @attribs = User.valid_attributes_hash - User.find_by_login(@attribs[:login]).try(:destroy) - @user = User.new(@attribs) + @user = FactoryGirl.build(:user) end test "test set of attributes should be valid" do @@ -49,13 +47,14 @@ 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? + test 'normal user is no admin' do assert !@user.is_admin? end + test 'user with login in APP_CONFIG is an admin' do + admin_login = APP_CONFIG['admins'].first + @user.login = admin_login + assert @user.is_admin? + end + end |