From 7bb1a16d4eee3eb3ef36c6f5b4fc7968e3da7dd6 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 28 Aug 2013 11:53:58 +0200 Subject: there's no need for User#find_by_param. clean it up --- help/app/controllers/tickets_controller.rb | 2 +- users/app/controllers/users_base_controller.rb | 2 +- users/app/models/token.rb | 6 +---- users/app/models/user.rb | 8 ------- users/test/support/stub_record_helper.rb | 7 +++--- users/test/unit/identity_test.rb | 7 +++--- users/test/unit/user_test.rb | 26 ++-------------------- .../warden_strategy_secure_remote_password_test.rb | 4 ++-- 8 files changed, 14 insertions(+), 48 deletions(-) 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"] -- cgit v1.2.3 From 77af7ac953fb22c181056e8c40c8749d12a63922 Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 30 Aug 2013 16:48:12 +0200 Subject: also replace find_by_param in billing engine --- billing/app/controllers/billing_base_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 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 -- cgit v1.2.3