Merge branch 'master' into feature/unauthenticated_tickets

Conflicts:
	help/app/views/tickets/_comment.html.haml
	help/app/views/tickets/_new_comment.html.haml

17 files changed, 203 insertions, 112 deletions

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