Merge pull request #74 from azul/refactor/finding-users

there's no need for User#find_by_param. clean it up

9 files changed, 16 insertions, 50 deletions

diff --git a/billing/app/controllers/billing_base_controller.rb b/billing/app/controllers/billing_base_controller.rb
index 06820a6..c250831 100644
--- a/billing/app/controllers/billing_base_controller.rb
+++ b/billing/app/controllers/billing_base_controller.rb
@@ -6,12 +6,12 @@ class BillingBaseController < ApplicationController
   # required for navigation to work.
   def assign_user
     if params[:user_id]
-      @user = User.find_by_param(params[:user_id])
+      @user = User.find(params[:user_id])
     elsif params[:action] == "confirm" or params[:action] == "destroy" # confirms and subscription deletes will come back with different ID set, so check for this first
       # This is only for cases where an admin cannot apply action for customer, but should be all confirms
       @user = current_user
     elsif params[:id]
-      @user = User.find_by_param(params[:id])
+      @user = User.find(params[:id])
     else
       # TODO
       # hacky, what are cases where @user hasn't yet been set? certainly some cases with subscriptions and payments
diff --git a/help/app/controllers/tickets_controller.rb b/help/app/controllers/tickets_controller.rb
index a03ef22..a669e19 100644
--- a/help/app/controllers/tickets_controller.rb
+++ b/help/app/controllers/tickets_controller.rb
@@ -138,7 +138,7 @@ class TicketsController < ApplicationController
 
   def fetch_user
     if params[:user_id]
-      @user = User.find_by_param(params[:user_id])
+      @user = User.find(params[:user_id])
     end
   end
 
diff --git a/users/app/controllers/users_base_controller.rb b/users/app/controllers/users_base_controller.rb
index dc2fa16..9becf0d 100644
--- a/users/app/controllers/users_base_controller.rb
+++ b/users/app/controllers/users_base_controller.rb
@@ -7,7 +7,7 @@ class UsersBaseController < ApplicationController
   protected
 
   def fetch_user
-    @user = User.find_by_param(params[:user_id] || params[:id])
+    @user = User.find(params[:user_id] || params[:id])
     if !@user && admin?
       redirect_to users_url, :alert => t(:no_such_thing, :thing => 'user')
     elsif !admin? && @user != current_user
diff --git a/users/app/models/token.rb b/users/app/models/token.rb
index 514b97f..3de0059 100644
--- a/users/app/models/token.rb
+++ b/users/app/models/token.rb
@@ -2,14 +2,10 @@ class Token < CouchRest::Model::Base
 
   use_database :tokens
 
-  property :user_id, String, accessible: false
+  belongs_to :user
 
   validates :user_id, presence: true
 
-  def user
-    User.find(self.user_id)
-  end
-
   def initialize(*args)
     super
     self.id = SecureRandom.urlsafe_base64(32).gsub(/^_*/, '')
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index c1988f3..8874966 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -34,14 +34,6 @@ class User < CouchRest::Model::Base
     view :by_created_at
   end # end of design
 
-  class << self
-    alias_method :find_by_param, :find
-  end
-
-  def to_param
-    self.id
-  end
-
   def to_json(options={})
     {
       :login => login,
diff --git a/users/test/support/stub_record_helper.rb b/users/test/support/stub_record_helper.rb
index 5bccb66..25138a0 100644
--- a/users/test/support/stub_record_helper.rb
+++ b/users/test/support/stub_record_helper.rb
@@ -1,7 +1,7 @@
 module StubRecordHelper
 
   #
-  # We will stub find_by_param or find_by_id to be called on klass and
+  # We will stub find when called on the records class and
   # return the record given.
   #
   # If no record is given but a hash or nil will create a stub based on
@@ -10,8 +10,9 @@ module StubRecordHelper
   def find_record(factory, record_or_attribs_hash = {})
     record = stub_record factory, record_or_attribs_hash, true
     klass = record.class
-    finder = klass.respond_to?(:find_by_param) ? :find_by_param : :find
-    klass.stubs(finder).with(record.to_param.to_s).returns(record)
+    # find is just an alias for get with CouchRest Model
+    klass.stubs(:get).with(record.to_param.to_s).returns(record)
+    klass.stubs(:find).with(record.to_param.to_s).returns(record)
     return record
   end
 
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index bf24f02..472bc52 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -1,13 +1,13 @@
 require 'test_helper'
 
 class IdentityTest < ActiveSupport::TestCase
+  include StubRecordHelper
 
   setup do
-    @user = FactoryGirl.create(:user)
+    @user = find_record :user
   end
 
   teardown do
-    @user.destroy
   end
 
   test "initial identity for a user" do
@@ -47,12 +47,11 @@ class IdentityTest < ActiveSupport::TestCase
   end
 
   test "validates availability" do
-    other_user = FactoryGirl.create(:user)
+    other_user = find_record :user
     id = Identity.create_for @user, address: alias_name, destination: forward_address
     taken = Identity.build_for other_user, address: alias_name
     assert !taken.valid?
     assert_equal ["This email has already been taken"], taken.errors[:base]
-    other_user.destroy
   end
 
   test "setting and getting pgp key" do
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index 89ee749..8efb0bd 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -22,16 +22,6 @@ class UserTest < ActiveSupport::TestCase
     assert !@user.valid?
   end
 
-  test "find_by_param gets User by id" do
-    @user.save
-    assert_equal @user, User.find_by_param(@user.id)
-    @user.destroy
-  end
-
-  test "to_param gives user id" do
-    assert_equal @user.id, @user.to_param
-  end
-
   test "verifier returns number for the hex in password_verifier" do
     assert_equal @user.password_verifier.hex, @user.verifier
   end
@@ -58,9 +48,10 @@ class UserTest < ActiveSupport::TestCase
 
   test "login needs to be unique amongst aliases" do
     other_user = FactoryGirl.create :user
-    Identity.create_for other_user, address: @user.login
+    id = Identity.create_for other_user, address: @user.login
     assert !@user.valid?
     other_user.destroy
+    id.destroy
   end
 
   test "deprecated public key api still works" do
@@ -69,17 +60,4 @@ class UserTest < ActiveSupport::TestCase
     assert_equal key, @user.public_key
   end
 
-  test "pgp key view" do
-    key = SecureRandom.base64(4096)
-    identity = Identity.create_for @user
-    identity.set_key('pgp', key)
-    identity.save
-
-    view = Identity.pgp_key_by_email.key(@user.email_address)
-
-    assert_equal 1, view.rows.count
-    assert result = view.rows.first
-    assert_equal @user.email_address, result["key"]
-    assert_equal key, result["value"]
-  end
 end
diff --git a/users/test/unit/warden_strategy_secure_remote_password_test.rb b/users/test/unit/warden_strategy_secure_remote_password_test.rb
index 319809a..e6fcfbe 100644
--- a/users/test/unit/warden_strategy_secure_remote_password_test.rb
+++ b/users/test/unit/warden_strategy_secure_remote_password_test.rb
@@ -21,7 +21,7 @@ class WardenStrategySecureRemotePasswordTest < ActiveSupport::TestCase
       returns(@server_handshake)
     @server_handshake.expects(:to_json).
      returns({'B' => @server_hex, 'salt' => @salt}.to_json)
-    User.expects(:find_by_param).with(@user.login).returns(@user)
+    User.expects(:find).with(@user.login).returns(@user)
     assert_equal @server_handshake, session[:handshake]
     assert_response :success
     assert_json_response :B => @server_hex, :salt => @salt
@@ -29,7 +29,7 @@ class WardenStrategySecureRemotePasswordTest < ActiveSupport::TestCase
 
   test "should report user not found" do
     unknown = "login_that_does_not_exist"
-    User.expects(:find_by_param).with(unknown).raises(RECORD_NOT_FOUND)
+    User.expects(:find).with(unknown).raises(RECORD_NOT_FOUND)
     post :create, :login => unknown
     assert_response :success
     assert_json_error "login" => ["unknown user"]