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

.rubocop_todo.yml

@@ -1,11 +1,18 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2018-01-07 23:59:28 +0900 using RuboCop version 0.52.1.
+# on 2018-01-20 22:00:49 +1030 using RuboCop version 0.52.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
+# Offense count: 1
+# Configuration parameters: Include.
+# Include: **/Gemfile, **/gems.rb
+Bundler/DuplicatedGem:
+  Exclude:
+    - 'Gemfile'
+
 # Offense count: 1
 # Cop supports --auto-correct.
 Layout/EmptyLinesAroundArguments:

CHANGELOG.md

@@ -10,6 +10,11 @@ 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.
 
+#### 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
+`FatFreeCRM::Permissions.update_with_permissions` is removed, use user_ids and group_ids inside attributes and call update_attributes
+
 #### Other changes
 TBA - https://github.com/fatfreecrm/fat_free_crm/milestone/6
 

Gemfile

@@ -4,13 +4,17 @@ source 'https://rubygems.org'
 
 # Uncomment the database that you have configured in config/database.yml
 # ----------------------------------------------------------------------
-db_drivers = {
-  "mysql" => "mysql2",
-  "sqlite" => "sqlite3",
-  "postgres" => "pg"
-}
 
-gem db_drivers[ENV['CI'] && ENV['DB']] || 'pg'
+case ENV['CI'] && ENV['DB']
+when 'sqlite'
+  gem 'sqlite3'
+when 'mysql'
+  gem 'mysql2'
+when 'postgres'
+  gem 'pg', '~> 0.21.0' # Pinned, see https://github.com/fatfreecrm/fat_free_crm/pull/689
+else
+  gem 'pg', '~> 0.21.0'
+end
 
 # Removes a gem dependency
 def remove(name)

Gemfile.lock

@@ -194,7 +194,7 @@ GEM
     minitest (5.11.1)
     money (6.10.1)
       i18n (>= 0.6.4, < 1.0)
-    msgpack (1.2.0)
+    msgpack (1.2.2)
     nenv (0.3.0)
     net-scp (1.2.1)
       net-ssh (>= 2.6.5)
@@ -437,7 +437,7 @@ DEPENDENCIES
   nokogiri (>= 1.8.1)
   paper_trail (~> 6.0.0)
   paperclip
-  pg
+  pg (~> 0.21.0)
   premailer
   pry-rails
   puma
@@ -474,4 +474,4 @@ DEPENDENCIES
   zeus
 
 BUNDLED WITH
-   1.16.0
+   1.16.1

app/models/entities/lead.rb

@@ -106,13 +106,6 @@ def save_with_account_and_permissions(params)
     end
   end
 
-  # Deprecated: see update_with_lead_counters
-  #----------------------------------------------------------------------------
-  def update_with_permissions(attributes, _users = nil)
-    ActiveSupport::Deprecation.warn "lead.update_with_account_and_lead_counters is deprecated and may be removed from future releases, use user_ids and group_ids inside attributes instead and call lead.update_with_lead_counters"
-    update_with_account_and_lead_counters(attributes)
-  end
-
   # Update lead attributes taking care of campaign lead counters when necessary.
   #----------------------------------------------------------------------------
   def update_with_account_and_lead_counters(attributes)

lib/fat_free_crm/permissions.rb

@@ -76,20 +76,6 @@ def remove_permissions
         permissions_to_remove.each { |p| permissions.delete(p); p.destroy }
       end
 
-      # Save the model along with its permissions if any.
-      #--------------------------------------------------------------------------
-      def save_with_permissions(_users = nil)
-        ActiveSupport::Deprecation.warn "save_with_permissions is deprecated and may be removed from future releases, use user_ids and group_ids inside attributes instead."
-        save
-      end
-
-      # Update the model along with its permissions if any.
-      #--------------------------------------------------------------------------
-      def update_with_permissions(attributes, _users = nil)
-        ActiveSupport::Deprecation.warn "update_with_permissions is deprecated and may be removed from future releases, use user_ids and group_ids inside attributes instead."
-        update_attributes(attributes)
-      end
-
       # Save the model copying other model's permissions.
       #--------------------------------------------------------------------------
       def save_with_model_permissions(model)

spec/lib/permissions_spec.rb

@@ -110,24 +110,6 @@
     end
   end
 
-  describe "save_with_permissions" do
-    it "should raise deprecation warning and call save" do
-      entity = UserWithPermission.new
-      expect(ActiveSupport::Deprecation).to receive(:warn)
-      expect(entity).to receive(:save)
-      entity.save_with_permissions
-    end
-  end
-
-  describe "update_with_permissions" do
-    it "should raise deprecation warning and call update_attributes" do
-      entity = UserWithPermission.new
-      expect(ActiveSupport::Deprecation).to receive(:warn)
-      expect(entity).to receive(:update_attributes).with({})
-      entity.update_with_permissions({})
-    end
-  end
-
   describe "save_with_model_permissions" do
     it "should copy permissions from original model" do
       entity = UserWithPermission.new

spec/models/entities/account_spec.rb

@@ -30,10 +30,8 @@
 require 'spec_helper'
 
 describe Account do
-  let(:current_user) { FactoryGirl.create(:user) }
-
   it "should create a new instance given valid attributes" do
-    Account.create!(name: "Test Account", user: FactoryGirl.create(:user))
+    Account.create!(name: "Test Account")
   end
 
   describe "Attach" do
@@ -42,7 +40,7 @@
     end
 
     it "should return nil when attaching existing asset" do
-      @task = FactoryGirl.create(:task, asset: @account, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @account)
       @contact = FactoryGirl.create(:contact)
       @account.contacts << @contact
       @opportunity = FactoryGirl.create(:opportunity)
@@ -54,7 +52,7 @@
     end
 
     it "should return non-empty list of attachments when attaching new asset" do
-      @task = FactoryGirl.create(:task, user: current_user)
+      @task = FactoryGirl.create(:task)
       @contact = FactoryGirl.create(:contact)
       @opportunity = FactoryGirl.create(:opportunity)
 
@@ -70,7 +68,7 @@
     end
 
     it "should discard a task" do
-      @task = FactoryGirl.create(:task, asset: @account, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @account)
       expect(@account.tasks.count).to eq(1)
 
       @account.discard!(@task)
@@ -103,15 +101,15 @@
 
   describe "Exportable" do
     describe "assigned account" do
-      let(:account1) { FactoryGirl.build(:account, user: FactoryGirl.create(:user), assignee: FactoryGirl.create(:user)) }
+      let(:account1) { FactoryGirl.build(:account, assignee: FactoryGirl.create(:user)) }
       let(:account2) { FactoryGirl.build(:account, user: FactoryGirl.create(:user, first_name: nil, last_name: nil), assignee: FactoryGirl.create(:user, first_name: nil, last_name: nil)) }
       it_should_behave_like("exportable") do
         let(:exported) { [account1, account2] }
       end
     end
 
     describe "unassigned account" do
-      let(:account1) { FactoryGirl.build(:account, user: FactoryGirl.create(:user), assignee: nil) }
+      let(:account1) { FactoryGirl.build(:account, assignee: nil) }
       let(:account2) { FactoryGirl.build(:account, user: FactoryGirl.create(:user, first_name: nil, last_name: nil), assignee: nil) }
       it_should_behave_like("exportable") do
         let(:exported) { [account1, account2] }
@@ -139,13 +137,14 @@
 
   describe "scopes" do
     context "visible_on_dashboard" do
-      before :each do
+      before do
+        @another_user = FactoryGirl.create(:user)
         @user = FactoryGirl.create(:user)
         @a1 = FactoryGirl.create(:account, user: @user)
-        @a2 = FactoryGirl.create(:account, user: @user, assignee: FactoryGirl.create(:user))
-        @a3 = FactoryGirl.create(:account, user: FactoryGirl.create(:user), assignee: @user)
-        @a4 = FactoryGirl.create(:account, user: FactoryGirl.create(:user), assignee: FactoryGirl.create(:user))
-        @a5 = FactoryGirl.create(:account, user: FactoryGirl.create(:user), assignee: @user)
+        @a2 = FactoryGirl.create(:account, user: @user, assignee: @another_user)
+        @a3 = FactoryGirl.create(:account, assignee: @user)
+        @a4 = FactoryGirl.create(:account, assignee: @another_user)
+        @a5 = FactoryGirl.create(:account, assignee: @user)
       end
 
       it "should show accounts which have been created by the user and are unassigned" do

spec/models/entities/campaign_spec.rb

@@ -34,10 +34,8 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 
 describe Campaign do
-  let(:current_user) { FactoryGirl.create(:user) }
-
   it "should create a new instance given valid attributes" do
-    Campaign.create!(name: "Campaign", user: FactoryGirl.create(:user))
+    Campaign.create!(name: "Campaign")
   end
 
   describe "Attach" do
@@ -46,17 +44,13 @@
     end
 
     it "should return nil when attaching existing asset" do
-      @task = FactoryGirl.create(:task, asset: @campaign, user: current_user)
-      @lead = FactoryGirl.create(:lead, campaign: @campaign)
-      @opportunity = FactoryGirl.create(:opportunity, campaign: @campaign)
-
-      expect(@campaign.attach!(@task)).to eq(nil)
-      expect(@campaign.attach!(@lead)).to eq(nil)
-      expect(@campaign.attach!(@opportunity)).to eq(nil)
+      expect(@campaign.attach!(create(:task, asset: @campaign))).to eq(nil)
+      expect(@campaign.attach!(create(:lead, campaign: @campaign))).to eq(nil)
+      expect(@campaign.attach!(create(:opportunity, campaign: @campaign))).to eq(nil)
     end
 
     it "should return non-empty list of attachments when attaching new asset" do
-      @task = FactoryGirl.create(:task, user: current_user)
+      @task = FactoryGirl.create(:task)
       @lead = FactoryGirl.create(:lead)
       @opportunity = FactoryGirl.create(:opportunity)
 
@@ -67,9 +61,8 @@
 
     it "should increment leads count when attaching a new lead" do
       @leads_count = @campaign.leads_count
-      @lead = FactoryGirl.create(:lead)
 
-      @campaign.attach!(@lead)
+      @campaign.attach!(FactoryGirl.create(:lead))
       expect(@campaign.reload.leads_count).to eq(@leads_count + 1)
     end
 
@@ -87,7 +80,7 @@
     end
 
     it "should discard a task" do
-      @task = FactoryGirl.create(:task, asset: @campaign, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @campaign)
       expect(@campaign.tasks.count).to eq(1)
 
       @campaign.discard!(@task)

spec/models/entities/contact_spec.rb

@@ -41,16 +41,13 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 
 describe Contact do
-  let(:current_user) { FactoryGirl.create(:user) }
-
   it "should create a new instance given valid attributes" do
     Contact.create!(first_name: "Billy", last_name: "Bones")
   end
 
   describe "Update existing contact" do
     before(:each) do
-      @account = FactoryGirl.create(:account)
-      @contact = FactoryGirl.create(:contact, account: @account)
+      @contact = create(:contact, account: create(:account))
     end
 
     it "should create new account if requested so" do
@@ -117,7 +114,7 @@
     end
 
     it "should return nil when attaching existing asset" do
-      @task = FactoryGirl.create(:task, asset: @contact, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @contact)
       @opportunity = FactoryGirl.create(:opportunity)
       @contact.opportunities << @opportunity
 
@@ -126,7 +123,7 @@
     end
 
     it "should return non-empty list of attachments when attaching new asset" do
-      @task = FactoryGirl.create(:task, user: current_user)
+      @task = FactoryGirl.create(:task)
       @opportunity = FactoryGirl.create(:opportunity)
 
       expect(@contact.attach!(@task)).to eq([@task])
@@ -140,7 +137,7 @@
     end
 
     it "should discard a task" do
-      @task = FactoryGirl.create(:task, asset: @contact, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @contact)
       expect(@contact.tasks.count).to eq(1)
 
       @contact.discard!(@task)
@@ -161,15 +158,15 @@
 
   describe "Exportable" do
     describe "assigned contact" do
-      let(:contact1) { FactoryGirl.build(:contact, user: FactoryGirl.create(:user), assignee: FactoryGirl.create(:user)) }
+      let(:contact1) { FactoryGirl.build(:contact, assignee: FactoryGirl.create(:user)) }
       let(:contact2) { FactoryGirl.build(:contact, user: FactoryGirl.create(:user, first_name: nil, last_name: nil), assignee: FactoryGirl.create(:user, first_name: nil, last_name: nil)) }
       it_should_behave_like("exportable") do
         let(:exported) { [contact1, contact2] }
       end
     end
 
     describe "unassigned contact" do
-      let(:contact1) { FactoryGirl.build(:contact, user: FactoryGirl.create(:user), assignee: nil) }
+      let(:contact1) { FactoryGirl.build(:contact, assignee: nil) }
       let(:contact2) { FactoryGirl.build(:contact, user: FactoryGirl.create(:user, first_name: nil, last_name: nil), assignee: nil) }
       it_should_behave_like("exportable") do
         let(:exported) { [contact1, contact2] }
@@ -182,7 +179,7 @@
   end
 
   describe "text_search" do
-    before(:each) do
+    before do
       @contact = FactoryGirl.create(:contact, first_name: "Bob", last_name: "Dillion", email: 'bob_dillion@example.com', phone: '+1 123 456 789')
     end
 

spec/models/entities/lead_spec.rb

@@ -41,8 +41,6 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 
 describe Lead do
-  let(:current_user) { FactoryGirl.create(:user) }
-
   it "should create a new instance given valid attributes" do
     Lead.create!(first_name: "Billy", last_name: "Bones")
   end
@@ -53,13 +51,13 @@
     end
 
     it "should return nil when attaching existing task" do
-      @task = FactoryGirl.create(:task, asset: @lead, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @lead)
 
       expect(@lead.attach!(@task)).to eq(nil)
     end
 
     it "should return non-empty list of tasks when attaching new task" do
-      @task = FactoryGirl.create(:task, user: current_user)
+      @task = FactoryGirl.create(:task)
 
       expect(@lead.attach!(@task)).to eq([@task])
     end
@@ -71,7 +69,7 @@
     end
 
     it "should discard a task" do
-      @task = FactoryGirl.create(:task, asset: @lead, user: current_user)
+      @task = FactoryGirl.create(:task, asset: @lead)
       expect(@lead.tasks.count).to eq(1)
 
       @lead.discard!(@task)
@@ -82,15 +80,15 @@
 
   describe "Exportable" do
     describe "assigned lead" do
-      let(:lead1) { FactoryGirl.build(:lead, user: FactoryGirl.create(:user), assignee: FactoryGirl.create(:user)) }
+      let(:lead1) { FactoryGirl.build(:lead, assignee: FactoryGirl.create(:user)) }
       let(:lead2) { FactoryGirl.build(:lead, user: FactoryGirl.create(:user, first_name: nil, last_name: nil), assignee: FactoryGirl.create(:user, first_name: nil, last_name: nil)) }
       it_should_behave_like("exportable") do
         let(:exported) { [lead1, lead2] }
       end
     end
 
     describe "unassigned lead" do
-      let(:lead1) { FactoryGirl.build(:lead, user: FactoryGirl.create(:user), assignee: nil) }
+      let(:lead1) { FactoryGirl.build(:lead, assignee: nil) }
       let(:lead2) { FactoryGirl.build(:lead, user: FactoryGirl.create(:user, first_name: nil, last_name: nil), assignee: nil) }
       it_should_behave_like("exportable") do
         let(:exported) { [lead1, lead2] }