Merge pull request #73 from azul/bugfix/3623-teardown-test-data-properly

Bugfix/3623 teardown test data properly

18 files changed, 135 insertions, 79 deletions

diff --git a/billing/test/functional/customer_controller_test.rb b/billing/test/functional/customer_controller_test.rb
index d62ee95..878ed48 100644
--- a/billing/test/functional/customer_controller_test.rb
+++ b/billing/test/functional/customer_controller_test.rb
@@ -21,7 +21,9 @@ class CustomerControllerTest < ActionController::TestCase
   end
 
   test "edit uses params[:id]" do
-    customer = FactoryGirl.create :customer_with_payment_info
+    user = find_record :user
+    customer = stub_record :customer_with_payment_info, user: user
+    Customer.stubs(:find_by_user_id).with(user.id).returns(customer)
     login customer.user
     get :edit, id: customer.user.id
 
@@ -50,7 +52,10 @@ class CustomerControllerTest < ActionController::TestCase
   end
 
   test "customer update" do
-    customer = FactoryGirl.create :customer_with_payment_info
+    user = find_record :user
+    customer = stub_record :customer_with_payment_info, user: user
+    customer.expects(:save)
+    Customer.stubs(:find_by_user_id).with(user.id).returns(customer)
     login customer.user
     Braintree::TransparentRedirect.expects(:confirm).
       returns(success_response(customer))
@@ -91,7 +96,9 @@ class CustomerControllerTest < ActionController::TestCase
   end
 
   test "failed user update with stubbing" do
-    customer = FactoryGirl.create :customer_with_payment_info
+    user = find_record :user
+    customer = stub_record :customer_with_payment_info, user: user
+    Customer.stubs(:find_by_user_id).with(user.id).returns(customer)
     login customer.user
     Braintree::TransparentRedirect.expects(:confirm).returns(failure_response)
     post :confirm, bla: :blub
diff --git a/billing/test/functional/payments_controller_test.rb b/billing/test/functional/payments_controller_test.rb
index 8f3bfe7..055a990 100644
--- a/billing/test/functional/payments_controller_test.rb
+++ b/billing/test/functional/payments_controller_test.rb
@@ -17,7 +17,9 @@ class PaymentsControllerTest < ActionController::TestCase
   end
 
   test "payment when authenticated as customer" do
-    customer = FactoryGirl.create :customer_with_payment_info
+    user = find_record :user
+    customer = stub_record :customer_with_payment_info, user: user
+    Customer.stubs(:find_by_user_id).with(user.id).returns(customer)
     login customer.user
     get :new
     assert_not_nil assigns(:tr_data)
diff --git a/billing/test/integration/admin_customer_test.rb b/billing/test/integration/admin_customer_test.rb
index 5169d85..58a7557 100644
--- a/billing/test/integration/admin_customer_test.rb
+++ b/billing/test/integration/admin_customer_test.rb
@@ -2,7 +2,7 @@ require 'test_helper'
 require 'fake_braintree'
 require 'capybara/rails'
 
-class CustomerCreationTest < ActionDispatch::IntegrationTest
+class AdminCustomerTest < ActionDispatch::IntegrationTest
   include Warden::Test::Helpers
   include Capybara::DSL
 
diff --git a/billing/test/integration/customer_creation_test.rb b/billing/test/integration/customer_creation_test.rb
index 5e3734c..aabd9b6 100644
--- a/billing/test/integration/customer_creation_test.rb
+++ b/billing/test/integration/customer_creation_test.rb
@@ -13,6 +13,7 @@ class CustomerCreationTest < ActionDispatch::IntegrationTest
   end
 
   teardown do
+    @user.destroy
     Warden.test_reset!
   end
 
diff --git a/billing/test/unit/customer_test.rb b/billing/test/unit/customer_test.rb
index 2ea0b5e..6156f87 100644
--- a/billing/test/unit/customer_test.rb
+++ b/billing/test/unit/customer_test.rb
@@ -1,9 +1,11 @@
 require 'test_helper'
 
 class CustomerTest < ActiveSupport::TestCase
+  include StubRecordHelper
 
   setup do
-    @customer = FactoryGirl.build(:customer)
+    @user = find_record :user
+    @customer = FactoryGirl.build(:customer, user: @user)
   end
 
   test "test set of attributes should be valid" do
diff --git a/billing/test/unit/customer_with_payment_info_test.rb b/billing/test/unit/customer_with_payment_info_test.rb
index ca89e65..0589a59 100644
--- a/billing/test/unit/customer_with_payment_info_test.rb
+++ b/billing/test/unit/customer_with_payment_info_test.rb
@@ -2,9 +2,11 @@ require 'test_helper'
 require 'fake_braintree'
 
 class CustomerWithPaymentInfoTest < ActiveSupport::TestCase
+  include StubRecordHelper
 
   setup do
-    @customer = FactoryGirl.build(:customer_with_payment_info)
+    @user = find_record :user
+    @customer = FactoryGirl.build(:customer_with_payment_info, user: @user)
   end
 
   test "has payment_info" do
@@ -28,6 +30,7 @@ class CustomerWithPaymentInfoTest < ActiveSupport::TestCase
     assert_equal 'Spender', @customer.last_name
     assert_equal 1, @customer.credit_cards.size
     assert_equal Hash.new, @customer.custom_fields
+    @customer.destroy
   end
 
   test "sets default_credit_card" do
diff --git a/help/app/models/ticket_comment.rb b/help/app/models/ticket_comment.rb
index 13bea2b..bed5237 100644
--- a/help/app/models/ticket_comment.rb
+++ b/help/app/models/ticket_comment.rb
@@ -1,5 +1,5 @@
 class TicketComment
-  include CouchRest::Model::Embeddable 
+  include CouchRest::Model::Embeddable
 
   #belongs_to :ticket #is this best way to do it? will want to access all of a tickets comments, so maybe this isn't the way?
   property :posted_by, String#, :protected => true #Integer#this should be current_user if that is set, meaning the user is logged in  #cannot have it be protected and set via comments_attributes=. also, if it is protected and we set in the tickets_controller, it gets unset. TODO---is this okay to have it not protected and manually check it? We do not users to be able to set this.
@@ -23,17 +23,17 @@ class TicketComment
   end
 
   def posted_by_user
-    User.find(self.posted_by)
+    User.find(posted_by) if posted_by
   end
 
 =begin
-  #TODO. 
+  #TODO.
   #this is resetting all comments associated with the ticket:
   def set_time
     self.posted_at = Time.now
   end
 =end
-  
+
 =begin
   def set_posted_by
     self.posted_by = User.current if User.current
diff --git a/help/test/functional/tickets_controller_test.rb b/help/test/functional/tickets_controller_test.rb
index 6479ba4..3747ad0 100644
--- a/help/test/functional/tickets_controller_test.rb
+++ b/help/test/functional/tickets_controller_test.rb
@@ -2,16 +2,6 @@ require 'test_helper'
 
 class TicketsControllerTest < ActionController::TestCase
 
-  setup do
-    @user = FactoryGirl.create :user
-    @other_user = FactoryGirl.create :user
-  end
-
-  teardown do
-    @user.destroy
-    @other_user.destroy
-  end
-
   test "should get index if logged in" do
     login
     get :index
@@ -38,22 +28,25 @@ class TicketsControllerTest < ActionController::TestCase
   end
 
   test "user tickets are not visible without login" do
-    ticket = find_record :ticket, :created_by => @user.id
+    user = find_record :user
+    ticket = find_record :ticket, :created_by => user.id
     get :show, :id => ticket.id
     assert_response :redirect
     assert_redirected_to login_url
   end
 
   test "user tickets are visible to creator" do
-    ticket = find_record :ticket, :created_by => @user.id
-    login @user
+    user = find_record :user
+    ticket = find_record :ticket, :created_by => user.id
+    login user
     get :show, :id => ticket.id
     assert_response :success
   end
 
-  test "user tickets are not visible to other user" do
-    ticket = find_record :ticket, :created_by => @user.id
-    login @other_user
+  test "other users tickets are not visible" do
+    other_user = find_record :user
+    ticket = find_record :ticket, :created_by => other_user.id
+    login
     get :show, :id => ticket.id
     assert_response :redirect
     assert_redirected_to root_url
@@ -79,7 +72,7 @@ class TicketsControllerTest < ActionController::TestCase
 
     params = {:title => "auth ticket test title", :comments_attributes => {"0" => {"body" =>"body of test ticket"}}}
 
-    login :email => Faker::Internet.user_name + '@' + APP_CONFIG[:domain]
+    login
 
     assert_difference('Ticket.count') do
       post :create, :ticket => params
@@ -89,7 +82,7 @@ class TicketsControllerTest < ActionController::TestCase
 
     assert_not_nil assigns(:ticket).created_by
     assert_equal assigns(:ticket).created_by, @current_user.id
-    assert_equal assigns(:ticket).email, @current_user.email_address.email
+    assert_equal assigns(:ticket).email, @current_user.email_address
 
     assert_equal 1, assigns(:ticket).comments.count
     assert_not_nil assigns(:ticket).comments.first.posted_by
@@ -114,7 +107,7 @@ class TicketsControllerTest < ActionController::TestCase
 
   test "add comment to own authenticated ticket" do
 
-    login User.last
+    login
     ticket = FactoryGirl.create :ticket, :created_by => @current_user.id
 
     #they should be able to comment if it is their ticket:
@@ -132,8 +125,9 @@ class TicketsControllerTest < ActionController::TestCase
 
   test "cannot comment if it is not your ticket" do
 
+    other_user = find_record :user
     login :is_admin? => false, :email => nil
-    ticket = FactoryGirl.create :ticket, :created_by => @other_user.id
+    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
@@ -146,9 +140,10 @@ class TicketsControllerTest < ActionController::TestCase
 
   test "admin add comment to authenticated ticket" do
 
+    other_user = find_record :user
     login :is_admin? => true
 
-    ticket = FactoryGirl.create :ticket, :created_by => @other_user.id
+    ticket = FactoryGirl.create :ticket, :created_by => other_user.id
 
     #admin should be able to comment:
     assert_difference('Ticket.find(ticket.id).comments.count') do
@@ -163,9 +158,10 @@ class TicketsControllerTest < ActionController::TestCase
   end
 
   test "tickets by admin" do
-    ticket = FactoryGirl.create :ticket, :created_by => @other_user.id
+    other_user = find_record :user
+    ticket = FactoryGirl.create :ticket, :created_by => other_user.id
 
-    login :is_admin? => true, :email => nil
+    login :is_admin? => true
 
     get :index, {:admin_status => "all", :open_status => "open"}
     assert assigns(:all_tickets).count > 1
diff --git a/users/app/controllers/v1/users_controller.rb b/users/app/controllers/v1/users_controller.rb
index 4f82572..01a1a2f 100644
--- a/users/app/controllers/v1/users_controller.rb
+++ b/users/app/controllers/v1/users_controller.rb
@@ -19,23 +19,20 @@ module V1
     end
 
     def create
-      @user = signup_service.register(params[:user])
+      @user = Account.create(params[:user])
       respond_with @user # return ID instead?
     end
 
     def update
-      account_settings.update params[:user]
+      account.update params[:user]
       respond_with @user
     end
 
     protected
 
-    def account_settings
-      AccountSettings.new(@user)
+    def account
+      Account.new(@user)
     end
 
-    def signup_service
-      SignupService.new
-    end
   end
 end
diff --git a/users/app/models/account_settings.rb b/users/app/models/account.rb
index 27fa227..5368a1b 100644
--- a/users/app/models/account_settings.rb
+++ b/users/app/models/account.rb
@@ -1,9 +1,21 @@
-class AccountSettings
+#
+# A Composition of a User record and it's identity records.
+#
+class Account
 
-  def initialize(user)
+  attr_reader :user
+
+  def initialize(user = nil)
     @user = user
   end
 
+  # Returns the user record so it can be used in views.
+  def self.create(attrs)
+    @user = User.create(attrs).tap do |user|
+      Identity.create_for user
+    end
+  end
+
   def update(attrs)
     if attrs[:password_verifier].present?
       update_login(attrs[:login])
@@ -12,6 +24,15 @@ class AccountSettings
     # TODO: move into identity controller
     update_pgp_key(attrs[:public_key]) if attrs.has_key? :public_key
     @user.save && save_identities
+    @user.refresh_identity
+  end
+
+  def destroy
+    return unless @user
+    Identity.by_user_id.key(@user.id).each do |identity|
+      identity.destroy
+    end
+    @user.destroy
   end
 
   protected
diff --git a/users/app/models/signup_service.rb b/users/app/models/signup_service.rb
deleted file mode 100644
index f316ca9..0000000
--- a/users/app/models/signup_service.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-class SignupService
-
-  def register(attrs)
-    User.create(attrs).tap do |user|
-      Identity.create_for user
-    end
-  end
-
-end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index 8874966..310eecd 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -86,6 +86,10 @@ class User < CouchRest::Model::Base
     @identity ||= Identity.for(self)
   end
 
+  def refresh_identity
+    @identity = Identity.for(self)
+  end
+
   protected
 
   ##
diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb
index 96ae48c..052de04 100644
--- a/users/test/functional/users_controller_test.rb
+++ b/users/test/functional/users_controller_test.rb
@@ -10,21 +10,14 @@ class UsersControllerTest < ActionController::TestCase
   end
 
   test "failed show without login" do
-    user = FactoryGirl.build(:user)
-    user.save
+    user = find_record :user
     get :show, :id => user.id
     assert_response :redirect
     assert_redirected_to login_path
-    user.destroy
   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
@@ -33,11 +26,6 @@ class UsersControllerTest < ActionController::TestCase
 
   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
@@ -47,11 +35,6 @@ class UsersControllerTest < ActionController::TestCase
 
   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
diff --git a/users/test/functional/v1/users_controller_test.rb b/users/test/functional/v1/users_controller_test.rb
index a330bf3..7cd9b0c 100644
--- a/users/test/functional/v1/users_controller_test.rb
+++ b/users/test/functional/v1/users_controller_test.rb
@@ -7,7 +7,7 @@ class V1::UsersControllerTest < ActionController::TestCase
     changed_attribs = record_attributes_for :user_with_settings
     account_settings = stub
     account_settings.expects(:update).with(changed_attribs)
-    AccountSettings.expects(:new).with(user).returns(account_settings)
+    Account.expects(:new).with(user).returns(account_settings)
 
     login user
     put :update, :user => changed_attribs, :id => user.id, :format => :json
@@ -22,7 +22,7 @@ class V1::UsersControllerTest < ActionController::TestCase
     changed_attribs = record_attributes_for :user_with_settings
     account_settings = stub
     account_settings.expects(:update).with(changed_attribs)
-    AccountSettings.expects(:new).with(user).returns(account_settings)
+    Account.expects(:new).with(user).returns(account_settings)
 
     login :is_admin? => true
     put :update, :user => changed_attribs, :id => user.id, :format => :json
@@ -41,7 +41,7 @@ class V1::UsersControllerTest < ActionController::TestCase
   test "should create new user" do
     user_attribs = record_attributes_for :user
     user = User.new(user_attribs)
-    User.expects(:create).with(user_attribs).returns(user)
+    Account.expects(:create).with(user_attribs).returns(user)
 
     post :create, :user => user_attribs, :format => :json
 
@@ -55,7 +55,7 @@ class V1::UsersControllerTest < ActionController::TestCase
     user_attribs.slice!('login')
     user = User.new(user_attribs)
     assert !user.valid?
-    User.expects(:create).with(user_attribs).returns(user)
+    Account.expects(:create).with(user_attribs).returns(user)
 
     post :create, :user => user_attribs, :format => :json
 
diff --git a/users/test/integration/browser/account_test.rb b/users/test/integration/browser/account_test.rb
index f3a78ed..8b214a4 100644
--- a/users/test/integration/browser/account_test.rb
+++ b/users/test/integration/browser/account_test.rb
@@ -12,6 +12,10 @@ class AccountTest < BrowserIntegrationTest
     click_on 'Logout'
     assert page.has_content?("Sign Up")
     assert_equal '/', current_path
+    assert user = User.find_by_login(username)
+    assert id = user.identity
+    id.destroy
+    user.destroy
   end
 
   # trying to seed an invalid A for srp login
@@ -24,6 +28,7 @@ class AccountTest < BrowserIntegrationTest
     click_on 'Log In'
     assert page.has_content?("Invalid random key")
     assert page.has_no_content?("Welcome")
+    user.destroy
   end
 
   test "reports internal server errors" do
diff --git a/users/test/unit/account_test.rb b/users/test/unit/account_test.rb
new file mode 100644
index 0000000..39969c0
--- /dev/null
+++ b/users/test/unit/account_test.rb
@@ -0,0 +1,45 @@
+require 'test_helper'
+
+class AccountTest < ActiveSupport::TestCase
+
+  test "create a new account" do
+    user = Account.create(FactoryGirl.attributes_for(:user))
+    assert user.valid?
+    assert user.persisted?
+    assert id = user.identity
+    assert_equal user.email_address, id.address
+    assert_equal user.email_address, id.destination
+    id.destroy
+    user.destroy
+  end
+
+  test "create and remove a user account" do
+    assert_no_difference "Identity.count" do
+      assert_no_difference "User.count" do
+        user = Account.create(FactoryGirl.attributes_for(:user))
+        Account.new(user).destroy
+      end
+    end
+  end
+
+  test "change username and create alias" do
+    user = Account.create(FactoryGirl.attributes_for(:user))
+    old_id = user.identity
+    old_email = user.email_address
+    Account.new(user).update(FactoryGirl.attributes_for(:user))
+    user.reload
+    old_id.reload
+    assert user.valid?
+    assert user.persisted?
+    assert id = user.identity
+    assert id.persisted?
+    assert_equal user.email_address, id.address
+    assert_equal user.email_address, id.destination
+    assert_equal user.email_address, old_id.destination
+    assert_equal old_email, old_id.address
+    old_id.destroy
+    id.destroy
+    user.destroy
+  end
+
+end
diff --git a/users/test/unit/identity_test.rb b/users/test/unit/identity_test.rb
index 472bc52..fa88315 100644
--- a/users/test/unit/identity_test.rb
+++ b/users/test/unit/identity_test.rb
@@ -7,9 +7,6 @@ class IdentityTest < ActiveSupport::TestCase
     @user = find_record :user
   end
 
-  teardown do
-  end
-
   test "initial identity for a user" do
     id = Identity.for(@user)
     assert_equal @user.email_address, id.address
@@ -36,7 +33,6 @@ class IdentityTest < ActiveSupport::TestCase
     assert_equal LocalEmail.new(alias_name), id.address
     assert_equal Email.new(forward_address), id.destination
     assert_equal @user, id.user
-    id.save
   end
 
   test "prevents duplicates" do
@@ -44,6 +40,7 @@ class IdentityTest < ActiveSupport::TestCase
     dup = Identity.build_for @user, address: alias_name, destination: forward_address
     assert !dup.valid?
     assert_equal ["This alias already exists"], dup.errors[:base]
+    id.destroy
   end
 
   test "validates availability" do
@@ -52,6 +49,7 @@ class IdentityTest < ActiveSupport::TestCase
     taken = Identity.build_for other_user, address: alias_name
     assert !taken.valid?
     assert_equal ["This email has already been taken"], taken.errors[:base]
+    id.destroy
   end
 
   test "setting and getting pgp key" do
@@ -69,6 +67,7 @@ class IdentityTest < ActiveSupport::TestCase
     assert result = view.rows.first
     assert_equal id.address, result["key"]
     assert_equal id.keys[:pgp], result["value"]
+    id.destroy
   end
 
   def alias_name
diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb
index 8efb0bd..c797623 100644
--- a/users/test/unit/user_test.rb
+++ b/users/test/unit/user_test.rb
@@ -50,8 +50,8 @@ class UserTest < ActiveSupport::TestCase
     other_user = FactoryGirl.create :user
     id = Identity.create_for other_user, address: @user.login
     assert !@user.valid?
-    other_user.destroy
     id.destroy
+    other_user.destroy
   end
 
   test "deprecated public key api still works" do