Merge branch 'master' into fix-679-leads-to-accounts

CHANGELOG.md

@@ -10,6 +10,9 @@ Unreleased (0.18.0)
 #### Mininium ruby version
 #665 Support for Ruby 2.3 has been dropped, with test coverage for 2.4 and 2.5 enabled.
 
+#### Swap to FactoryBot
+If you consume fat free crm as an engine and re-use any factories, you'll need to [upgrade to FactoryBot](https://github.com/thoughtbot/factory_bot/blob/4-9-0-stable/UPGRADE_FROM_FACTORY_GIRL.md).
+
 #### Removed methods
 `Lead.update_with_permissions` is removed, use user_ids and group_ids inside attributes instead and call lead.update_with_account_and_lead_counters
 `FatFreeCRM::Permissions.save_with_permissions` is removed, use user_ids and group_ids inside attributes and call save

Gemfile

@@ -66,7 +66,7 @@ group :development, :test do
   gem 'headless'
   gem 'byebug'
   gem 'pry-rails' unless ENV["CI"]
-  gem 'factory_girl_rails', '~> 4.7.0' # 4.8.0+ stubbed models are not allowed to access the database - User#destroyed?()
+  gem 'factory_bot_rails'
   gem 'rubocop', '~> 0.52.0' # Pinned because upgrades require regenerating rubocop_todo.yml
   gem 'rainbow'
 end

Gemfile.lock

@@ -125,10 +125,10 @@ GEM
     email_reply_parser_ffcrm (0.5.0)
     erubi (1.7.0)
     execjs (2.7.0)
-    factory_girl (4.7.0)
+    factory_bot (4.8.2)
       activesupport (>= 3.0.0)
-    factory_girl_rails (4.7.0)
-      factory_girl (~> 4.7.0)
+    factory_bot_rails (4.8.2)
+      factory_bot (~> 4.8.2)
       railties (>= 3.0.0)
     ffaker (2.8.0)
     ffi (1.9.18)
@@ -423,7 +423,7 @@ DEPENDENCIES
   dynamic_form
   email_reply_parser_ffcrm
   execjs
-  factory_girl_rails (~> 4.7.0)
+  factory_bot_rails
   ffaker (>= 2)
   font-awesome-rails
   guard

lib/fat_free_crm/engine.rb

@@ -13,8 +13,8 @@ class Engine < ::Rails::Engine
     config.active_record.observers = %i[lead_observer opportunity_observer
                                         task_observer entity_observer]
 
-    initializer "model_core.factories", after: "factory_girl.set_factory_paths" do
-      FactoryGirl.definition_file_paths << File.expand_path('../../../spec/factories', __FILE__) if defined?(FactoryGirl)
+    initializer "model_core.factories", after: "factory_bot.set_factory_paths" do
+      FactoryBot.definition_file_paths << File.expand_path('../../../spec/factories', __FILE__) if defined?(FactoryBot)
     end
 
     initializer :append_migrations do |app|

spec/controllers/admin/users_controller_spec.rb

@@ -18,7 +18,7 @@
   #----------------------------------------------------------------------------
   describe "GET index" do
     it "assigns all users as @users and renders [index] template" do
-      @users = [current_user, FactoryGirl.create(:user)]
+      @users = [current_user, create(:user)]
 
       get :index
       expect(assigns[:users].first).to eq(@users.last) # get_users() sorts by id DESC
@@ -27,8 +27,8 @@
     end
 
     it "performs lookup using query string" do
-      @amy = FactoryGirl.create(:user, username: "amy_anderson")
-      @bob = FactoryGirl.create(:user, username: "bob_builder")
+      @amy = create(:user, username: "amy_anderson")
+      @bob = create(:user, username: "bob_builder")
 
       get :index, params: { query: "amy_anderson" }
       expect(assigns[:users]).to eq([@amy])
@@ -42,7 +42,7 @@
   #----------------------------------------------------------------------------
   describe "GET show" do
     it "assigns the requested user as @user and renders [show] template" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
 
       get :show, params: { id: @user.id }
       expect(assigns[:user]).to eq(@user)
@@ -65,7 +65,7 @@
   #----------------------------------------------------------------------------
   describe "GET edit" do
     it "assigns the requested user as @user and renders [edit] template" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
 
       get :edit, params: { id: @user.id }, xhr: true
       expect(assigns[:user]).to eq(@user)
@@ -74,15 +74,15 @@
     end
 
     it "assigns the previous user as @previous when necessary" do
-      @user = FactoryGirl.create(:user)
-      @previous = FactoryGirl.create(:user)
+      @user = create(:user)
+      @previous = create(:user)
 
       get :edit, params: { id: @user.id, previous: @previous.id }, xhr: true
       expect(assigns[:previous]).to eq(@previous)
     end
 
     it "reloads current page with the flash message if user got deleted" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
       @user.destroy
 
       get :edit, params: { id: @user.id }, xhr: true
@@ -91,8 +91,8 @@
     end
 
     it "notifies the view if previous user got deleted" do
-      @user = FactoryGirl.create(:user)
-      @previous = FactoryGirl.create(:user)
+      @user = create(:user)
+      @previous = create(:user)
       @previous.destroy
 
       get :edit, params: { id: @user.id, previous: @previous.id }, xhr: true
@@ -114,7 +114,7 @@
       end
 
       it "assigns a newly created user as @user and renders [create] template" do
-        @user = FactoryGirl.build(:user, username: @username, email: @email)
+        @user = build(:user, username: @username, email: @email)
         allow(User).to receive(:new).and_return(@user)
 
         post :create, params: { user: { username: @username, email: @email, password: @password, password_confirmation: @password } }, xhr: true
@@ -137,7 +137,7 @@
 
     describe "with invalid params" do
       it "assigns a newly created but unsaved user as @user and re-renders [create] template" do
-        @user = FactoryGirl.build(:user, username: "", email: "")
+        @user = build(:user, username: "", email: "")
         allow(User).to receive(:new).and_return(@user)
 
         post :create, params: { user: {} }, xhr: true
@@ -153,7 +153,7 @@
   describe "PUT update" do
     describe "with valid params" do
       it "updates the requested user, assigns it to @user, and renders [update] template" do
-        @user = FactoryGirl.create(:user, username: "flip", email: "flip@example.com")
+        @user = create(:user, username: "flip", email: "flip@example.com")
 
         put :update, params: { id: @user.id, user: { username: "flop", email: "flop@example.com" } }, xhr: true
         expect(assigns[:user]).to eq(@user.reload)
@@ -162,7 +162,7 @@
       end
 
       it "reloads current page is the user got deleted" do
-        @user = FactoryGirl.create(:user)
+        @user = create(:user)
         @user.destroy
 
         put :update, params: { id: @user.id, user: { username: "flop", email: "flop@example.com" } }, xhr: true
@@ -171,15 +171,15 @@
       end
 
       it "assigns admin rights when requested so" do
-        @user = FactoryGirl.create(:user, admin: false)
+        @user = create(:user, admin: false)
         put :update, params: { id: @user.id, user: { admin: "1", username: @user.username, email: @user.email } }, xhr: true
         expect(assigns[:user]).to eq(@user.reload)
         expect(assigns[:user].admin).to eq(true)
         expect(response).to render_template("admin/users/update")
       end
 
       it "revokes admin rights when requested so" do
-        @user = FactoryGirl.create(:user, admin: true)
+        @user = create(:user, admin: true)
         put :update, params: { id: @user.id, user: { admin: "0", username: @user.username, email: @user.email } }, xhr: true
         expect(assigns[:user]).to eq(@user.reload)
         expect(assigns[:user].admin).to eq(false)
@@ -189,7 +189,7 @@
 
     describe "with invalid params" do
       it "doesn't update the requested user, but assigns it to @user and renders [update] template" do
-        @user = FactoryGirl.create(:user, username: "flip", email: "flip@example.com")
+        @user = create(:user, username: "flip", email: "flip@example.com")
 
         put :update, params: { id: @user.id, user: {} }, xhr: true
         expect(assigns[:user]).to eq(@user.reload)
@@ -203,15 +203,15 @@
   #----------------------------------------------------------------------------
   describe "GET confirm" do
     it "assigns the requested user as @user and renders [confirm] template" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
 
       get :confirm, params: { id: @user.id }, xhr: true
       expect(assigns[:user]).to eq(@user)
       expect(response).to render_template("admin/users/confirm")
     end
 
     it "reloads current page is the user got deleted" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
       @user.destroy
 
       get :confirm, params: { id: @user.id }, xhr: true
@@ -225,16 +225,16 @@
   #----------------------------------------------------------------------------
   describe "DELETE destroy" do
     it "destroys the requested user and renders [destroy] template" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
 
       delete :destroy, params: { id: @user.id }, xhr: true
       expect { User.find(@user.id) }.to raise_error(ActiveRecord::RecordNotFound)
       expect(response).to render_template("admin/users/destroy")
     end
 
     it "handles the case when the requested user can't be deleted" do
-      @user = FactoryGirl.create(:user)
-      @account = FactoryGirl.create(:account, user: @user) # Plant artifact to prevent the user from being deleted.
+      @user = create(:user)
+      @account = create(:account, user: @user) # Plant artifact to prevent the user from being deleted.
 
       delete :destroy, params: { id: @user.id }, xhr: true
       expect(flash[:warning]).not_to eq(nil)
@@ -247,7 +247,7 @@
   #----------------------------------------------------------------------------
   describe "POST auto_complete" do
     before(:each) do
-      @auto_complete_matches = [FactoryGirl.create(:user, first_name: "Hello")]
+      @auto_complete_matches = [create(:user, first_name: "Hello")]
     end
 
     it_should_behave_like("auto complete")
@@ -258,7 +258,7 @@
   #----------------------------------------------------------------------------
   describe "PUT suspend" do
     it "suspends the requested user" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
 
       put :suspend, params: { id: @user.id }, xhr: true
       expect(assigns[:user].suspended?).to eq(true)
@@ -274,7 +274,7 @@
     end
 
     it "reloads current page is the user got deleted" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
       @user.destroy
 
       put :suspend, params: { id: @user.id }, xhr: true
@@ -288,15 +288,15 @@
   #----------------------------------------------------------------------------
   describe "PUT reactivate" do
     it "re-activates the requested user" do
-      @user = FactoryGirl.create(:user, suspended_at: Time.now.yesterday)
+      @user = create(:user, suspended_at: Time.now.yesterday)
 
       put :reactivate, params: { id: @user.id }, xhr: true
       expect(assigns[:user].suspended?).to eq(false)
       expect(response).to render_template("admin/users/reactivate")
     end
 
     it "reloads current page is the user got deleted" do
-      @user = FactoryGirl.create(:user)
+      @user = create(:user)
       @user.destroy
 
       put :reactivate, params: { id: @user.id }, xhr: true

spec/controllers/authentications_controller_spec.rb

@@ -34,7 +34,7 @@
 
     describe "user must not be logged in" do
       before(:each) do
-        @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass")
+        @user = create(:user, username: "user", password: "pass", password_confirmation: "pass")
         allow(@controller).to receive(:current_user).and_return(@user)
       end
 
@@ -74,7 +74,7 @@
       end
 
       it "displays welcome message and redirects to the home page" do
-        @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass", login_count: 0)
+        @user = create(:user, username: "user", password: "pass", password_confirmation: "pass", login_count: 0)
         allow(@authentication).to receive(:user).and_return(@user)
 
         post :create, params: { authentication: @login }
@@ -84,7 +84,7 @@
       end
 
       it "displays last login time if it's not the first login" do
-        @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass", login_count: 42)
+        @user = create(:user, username: "user", password: "pass", password_confirmation: "pass", login_count: 42)
         allow(@authentication).to receive(:user).and_return(@user)
 
         post :create, params: { authentication: @login }
@@ -96,7 +96,7 @@
     describe "authenticaion failure" do
       describe "user is not suspended" do
         it "redirects to login page if username or password are invalid" do
-          @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass")
+          @user = create(:user, username: "user", password: "pass", password_confirmation: "pass")
           allow(@authentication).to receive(:user).and_return(@user)
           allow(@authentication).to receive(:save).and_return(false) # <--- Authentication failure.
           allow(Authentication).to receive(:new).and_return(@authentication)
@@ -115,7 +115,7 @@
 
         # This tests :before_save update_info callback in Authentication model.
         it "keeps user login attributes intact" do
-          @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass", suspended_at: Date.yesterday, login_count: 0, last_login_at: nil, last_login_ip: nil)
+          @user = create(:user, username: "user", password: "pass", password_confirmation: "pass", suspended_at: Date.yesterday, login_count: 0, last_login_at: nil, last_login_ip: nil)
           allow(@authentication).to receive(:user).and_return(@user)
 
           post :create, params: { authentication: @login }
@@ -125,7 +125,7 @@
         end
 
         it "redirects to login page if user is suspended" do
-          @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass", suspended_at: Date.yesterday)
+          @user = create(:user, username: "user", password: "pass", password_confirmation: "pass", suspended_at: Date.yesterday)
           allow(@authentication).to receive(:user).and_return(@user)
 
           post :create, params: { authentication: @login }
@@ -136,7 +136,7 @@
 
         it "redirects to login page with the message if signup needs approval and user hasn't been activated yet" do
           allow(Setting).to receive(:user_signup).and_return(:needs_approval)
-          @user = FactoryGirl.create(:user, username: "user", password: "pass", password_confirmation: "pass", suspended_at: Date.yesterday, login_count: 0)
+          @user = create(:user, username: "user", password: "pass", password_confirmation: "pass", suspended_at: Date.yesterday, login_count: 0)
           allow(@authentication).to receive(:user).and_return(@user)
 
           post :create, params: { authentication: @login }