Browse Source

Refactor user_spec.rb

Ryan Lue 6 years ago
parent
commit
ad19b532a9

+ 1 - 1
spec/factories/token.rb

@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :token do
-    user_id { FactoryBot.create(:user).id }
+    user
   end
 
   factory :token_password_reset, parent: :token do

+ 107 - 101
spec/models/concerns/has_groups_examples.rb

@@ -1,12 +1,6 @@
-# Requires: let(:group_access_instance) { ... }
-# Requires: let(:new_group_access_instance) { ... }
-RSpec.shared_examples 'HasGroups' do
-
+RSpec.shared_examples 'HasGroups' do |group_access_factory:|
   context 'group' do
-    let(:group_access_instance_inactive) do
-      group_access_instance.update!(active: false)
-      group_access_instance
-    end
+    subject { create(group_access_factory) }
     let(:group_full) { create(:group) }
     let(:group_read) { create(:group) }
     let(:group_inactive) { create(:group, active: false) }
@@ -22,7 +16,7 @@ RSpec.shared_examples 'HasGroups' do
       end
 
       it 'instance responds to group_through_identifier method' do
-        expect(group_access_instance).to respond_to(described_class.group_through_identifier)
+        expect(subject).to respond_to(described_class.group_through_identifier)
       end
     end
 
@@ -40,19 +34,19 @@ RSpec.shared_examples 'HasGroups' do
     context '#groups' do
 
       it 'responds to groups' do
-        expect(group_access_instance).to respond_to(:groups)
+        expect(subject).to respond_to(:groups)
       end
 
       context '#groups.access' do
 
         it 'responds to groups.access' do
-          expect(group_access_instance.groups).to respond_to(:access)
+          expect(subject.groups).to respond_to(:access)
         end
 
         context 'result' do
 
           before(:each) do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name     => 'full',
               group_read.name     => 'read',
               group_inactive.name => 'change',
@@ -60,23 +54,23 @@ RSpec.shared_examples 'HasGroups' do
           end
 
           it 'returns all related Groups' do
-            expect(group_access_instance.groups.access.size).to eq(3)
+            expect(subject.groups.access.size).to eq(3)
           end
 
           it 'adds join table attribute(s like) access' do
-            expect(group_access_instance.groups.access.first).to respond_to(:access)
+            expect(subject.groups.access.first).to respond_to(:access)
           end
 
           it 'filters for given access parameter' do
-            expect(group_access_instance.groups.access('read')).to include(group_read)
+            expect(subject.groups.access('read')).to include(group_read)
           end
 
           it 'filters for given access list parameter' do
-            expect(group_access_instance.groups.access('read', 'change')).to include(group_read, group_inactive)
+            expect(subject.groups.access('read', 'change')).to include(group_read, group_inactive)
           end
 
           it 'always includes full access groups' do
-            expect(group_access_instance.groups.access('read')).to include(group_full)
+            expect(subject.groups.access('read')).to include(group_full)
           end
         end
       end
@@ -85,11 +79,11 @@ RSpec.shared_examples 'HasGroups' do
     context '#group_access?' do
 
       it 'responds to group_access?' do
-        expect(group_access_instance).to respond_to(:group_access?)
+        expect(subject).to respond_to(:group_access?)
       end
 
       before(:each) do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
       end
@@ -107,61 +101,65 @@ RSpec.shared_examples 'HasGroups' do
       end
 
       it 'prevents inactive Group' do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_inactive.name => 'read',
         }
 
-        expect(group_access_instance.group_access?(group_inactive.id, 'read')).to be false
+        expect(subject.group_access?(group_inactive.id, 'read')).to be false
       end
 
       it 'prevents inactive instances' do
-        group_access_instance_inactive.group_names_access_map = {
+        subject.update!(active: false)
+
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
 
-        expect(group_access_instance_inactive.group_access?(group_read.id, 'read')).to be false
+        expect(subject.group_access?(group_read.id, 'read')).to be false
       end
     end
 
     context '#group_ids_access' do
 
       it 'responds to group_ids_access' do
-        expect(group_access_instance).to respond_to(:group_ids_access)
+        expect(subject).to respond_to(:group_ids_access)
       end
 
       before(:each) do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
       end
 
       it 'lists only active Group IDs' do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_read.name     => 'read',
           group_inactive.name => 'read',
         }
 
-        result = group_access_instance.group_ids_access('read')
+        result = subject.group_ids_access('read')
         expect(result).not_to include(group_inactive.id)
       end
 
       it "doesn't list for inactive instances" do
-        group_access_instance_inactive.group_names_access_map = {
+        subject.update!(active: false)
+
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
 
-        expect(group_access_instance_inactive.group_ids_access('read')).to be_empty
+        expect(subject.group_ids_access('read')).to be_empty
       end
 
       context 'single access' do
 
         it 'lists access Group IDs' do
-          result = group_access_instance.group_ids_access('read')
+          result = subject.group_ids_access('read')
           expect(result).to include(group_read.id)
         end
 
         it "doesn't list for no access" do
-          result = group_access_instance.group_ids_access('change')
+          result = subject.group_ids_access('change')
           expect(result).not_to include(group_read.id)
         end
       end
@@ -169,12 +167,12 @@ RSpec.shared_examples 'HasGroups' do
       context 'access list' do
 
         it 'lists access Group IDs' do
-          result = group_access_instance.group_ids_access(%w[read change])
+          result = subject.group_ids_access(%w[read change])
           expect(result).to include(group_read.id)
         end
 
         it "doesn't list for no access" do
-          result = group_access_instance.group_ids_access(%w[change create])
+          result = subject.group_ids_access(%w[change create])
           expect(result).not_to include(group_read.id)
         end
       end
@@ -183,19 +181,19 @@ RSpec.shared_examples 'HasGroups' do
     context '#groups_access' do
 
       it 'responds to groups_access' do
-        expect(group_access_instance).to respond_to(:groups_access)
+        expect(subject).to respond_to(:groups_access)
       end
 
       it 'wraps #group_ids_access' do
-        expect(group_access_instance).to receive(:group_ids_access)
-        group_access_instance.groups_access('read')
+        expect(subject).to receive(:group_ids_access)
+        subject.groups_access('read')
       end
 
       it 'returns Groups' do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
-        result = group_access_instance.groups_access('read')
+        result = subject.groups_access('read')
         expect(result).to include(group_read)
       end
     end
@@ -203,14 +201,14 @@ RSpec.shared_examples 'HasGroups' do
     context '#group_names_access_map=' do
 
       it 'responds to group_names_access_map=' do
-        expect(group_access_instance).to respond_to(:group_names_access_map=)
+        expect(subject).to respond_to(:group_names_access_map=)
       end
 
       context 'existing instance' do
 
         it 'stores Hash with String values' do
           expect do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name => 'full',
               group_read.name => 'read',
             }
@@ -221,7 +219,7 @@ RSpec.shared_examples 'HasGroups' do
 
         it 'stores Hash with Array<String> values' do
           expect do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name => 'full',
               group_read.name => %w[read change],
             }
@@ -231,13 +229,13 @@ RSpec.shared_examples 'HasGroups' do
         end
 
         it 'allows empty Hash value' do
-          group_access_instance.group_names_access_map = {
+          subject.group_names_access_map = {
             group_full.name => 'full',
             group_read.name => %w[read change],
           }
 
           expect do
-            group_access_instance.group_names_access_map = {}
+            subject.group_names_access_map = {}
           end.to change {
             described_class.group_through.klass.count
           }.by(-3)
@@ -249,13 +247,13 @@ RSpec.shared_examples 'HasGroups' do
           exception           = ActiveRecord::RecordInvalid
 
           expect do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name => invalid_combination,
             }
           end.to raise_error(exception)
 
           expect do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name => invalid_combination.reverse,
             }
           end.to raise_error(exception)
@@ -263,10 +261,11 @@ RSpec.shared_examples 'HasGroups' do
       end
 
       context 'new instance' do
+        subject { build(group_access_factory) }
 
         it "doesn't store directly" do
           expect do
-            new_group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name => 'full',
               group_read.name => 'read',
             }
@@ -277,12 +276,12 @@ RSpec.shared_examples 'HasGroups' do
 
         it 'stores after save' do
           expect do
-            new_group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_full.name => 'full',
               group_read.name => 'read',
             }
 
-            new_group_access_instance.save
+            subject.save
           end.to change {
             described_class.group_through.klass.count
           }.by(2)
@@ -290,9 +289,9 @@ RSpec.shared_examples 'HasGroups' do
 
         it 'allows empty Hash value' do
           expect do
-            new_group_access_instance.group_names_access_map = {}
+            subject.group_names_access_map = {}
 
-            new_group_access_instance.save
+            subject.save
           end.not_to change {
             described_class.group_through.klass.count
           }
@@ -303,7 +302,7 @@ RSpec.shared_examples 'HasGroups' do
     context '#group_names_access_map' do
 
       it 'responds to group_names_access_map' do
-        expect(group_access_instance).to respond_to(:group_names_access_map)
+        expect(subject).to respond_to(:group_names_access_map)
       end
 
       it 'returns instance Group name => access relations as Hash' do
@@ -312,44 +311,46 @@ RSpec.shared_examples 'HasGroups' do
           group_read.name => ['read'],
         }
 
-        group_access_instance.group_names_access_map = expected
+        subject.group_names_access_map = expected
 
-        expect(group_access_instance.group_names_access_map).to eq(expected)
+        expect(subject.group_names_access_map).to eq(expected)
       end
 
       it "doesn't map for inactive instances" do
-        group_access_instance_inactive.group_names_access_map = {
+        subject.update!(active: false)
+
+        subject.group_names_access_map = {
           group_full.name => ['full'],
           group_read.name => ['read'],
         }
 
-        expect(group_access_instance_inactive.group_names_access_map).to be_empty
+        expect(subject.group_names_access_map).to be_empty
       end
 
       it 'returns empty map if none is stored' do
 
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_full.name => 'full',
           group_read.name => 'read',
         }
 
-        group_access_instance.group_names_access_map = {}
+        subject.group_names_access_map = {}
 
-        expect(group_access_instance.group_names_access_map).to be_blank
+        expect(subject.group_names_access_map).to be_blank
       end
     end
 
     context '#group_ids_access_map=' do
 
       it 'responds to group_ids_access_map=' do
-        expect(group_access_instance).to respond_to(:group_ids_access_map=)
+        expect(subject).to respond_to(:group_ids_access_map=)
       end
 
       context 'existing instance' do
 
         it 'stores Hash with String values' do
           expect do
-            group_access_instance.group_ids_access_map = {
+            subject.group_ids_access_map = {
               group_full.id => 'full',
               group_read.id => 'read',
             }
@@ -360,7 +361,7 @@ RSpec.shared_examples 'HasGroups' do
 
         it 'stores Hash with Array<String> values' do
           expect do
-            group_access_instance.group_ids_access_map = {
+            subject.group_ids_access_map = {
               group_full.id => 'full',
               group_read.id => %w[read change],
             }
@@ -370,13 +371,13 @@ RSpec.shared_examples 'HasGroups' do
         end
 
         it 'allows empty Hash value' do
-          group_access_instance.group_ids_access_map = {
+          subject.group_ids_access_map = {
             group_full.id => 'full',
             group_read.id => %w[read change],
           }
 
           expect do
-            group_access_instance.group_ids_access_map = {}
+            subject.group_ids_access_map = {}
           end.to change {
             described_class.group_through.klass.count
           }.by(-3)
@@ -384,10 +385,11 @@ RSpec.shared_examples 'HasGroups' do
       end
 
       context 'new instance' do
+        subject { build(group_access_factory) }
 
         it "doesn't store directly" do
           expect do
-            new_group_access_instance.group_ids_access_map = {
+            subject.group_ids_access_map = {
               group_full.id => 'full',
               group_read.id => 'read',
             }
@@ -398,12 +400,12 @@ RSpec.shared_examples 'HasGroups' do
 
         it 'stores after save' do
           expect do
-            new_group_access_instance.group_ids_access_map = {
+            subject.group_ids_access_map = {
               group_full.id => 'full',
               group_read.id => 'read',
             }
 
-            new_group_access_instance.save
+            subject.save
           end.to change {
             described_class.group_through.klass.count
           }.by(2)
@@ -411,9 +413,9 @@ RSpec.shared_examples 'HasGroups' do
 
         it 'allows empty Hash value' do
           expect do
-            new_group_access_instance.group_ids_access_map = {}
+            subject.group_ids_access_map = {}
 
-            new_group_access_instance.save
+            subject.save
           end.not_to change {
             described_class.group_through.klass.count
           }
@@ -424,7 +426,7 @@ RSpec.shared_examples 'HasGroups' do
     context '#group_ids_access_map' do
 
       it 'responds to group_ids_access_map' do
-        expect(group_access_instance).to respond_to(:group_ids_access_map)
+        expect(subject).to respond_to(:group_ids_access_map)
       end
 
       it 'returns instance Group ID => access relations as Hash' do
@@ -433,30 +435,32 @@ RSpec.shared_examples 'HasGroups' do
           group_read.id => ['read'],
         }
 
-        group_access_instance.group_ids_access_map = expected
+        subject.group_ids_access_map = expected
 
-        expect(group_access_instance.group_ids_access_map).to eq(expected)
+        expect(subject.group_ids_access_map).to eq(expected)
       end
 
       it "doesn't map for inactive instances" do
-        group_access_instance_inactive.group_ids_access_map = {
+        subject.update!(active: false)
+
+        subject.group_ids_access_map = {
           group_full.id => ['full'],
           group_read.id => ['read'],
         }
 
-        expect(group_access_instance_inactive.group_ids_access_map).to be_empty
+        expect(subject.group_ids_access_map).to be_empty
       end
 
       it 'returns empty map if none is stored' do
 
-        group_access_instance.group_ids_access_map = {
+        subject.group_ids_access_map = {
           group_full.id => 'full',
           group_read.id => 'read',
         }
 
-        group_access_instance.group_ids_access_map = {}
+        subject.group_ids_access_map = {}
 
-        expect(group_access_instance.group_ids_access_map).to be_blank
+        expect(subject.group_ids_access_map).to be_blank
       end
     end
 
@@ -468,8 +472,8 @@ RSpec.shared_examples 'HasGroups' do
           group_read.id => ['read'],
         }
 
-        group_access_instance.associations_from_param(group_ids: expected)
-        expect(group_access_instance.group_ids_access_map).to eq(expected)
+        subject.associations_from_param(group_ids: expected)
+        expect(subject.group_ids_access_map).to eq(expected)
       end
 
       it 'handles groups parameter as group_names_access_map' do
@@ -478,8 +482,8 @@ RSpec.shared_examples 'HasGroups' do
           group_read.name => ['read'],
         }
 
-        group_access_instance.associations_from_param(groups: expected)
-        expect(group_access_instance.group_names_access_map).to eq(expected)
+        subject.associations_from_param(groups: expected)
+        expect(subject.group_names_access_map).to eq(expected)
       end
     end
 
@@ -491,9 +495,9 @@ RSpec.shared_examples 'HasGroups' do
           group_read.id => ['read'],
         }
 
-        group_access_instance.group_ids_access_map = expected
+        subject.group_ids_access_map = expected
 
-        result = group_access_instance.attributes_with_association_ids
+        result = subject.attributes_with_association_ids
         expect(result['group_ids']).to eq(expected)
       end
     end
@@ -506,9 +510,9 @@ RSpec.shared_examples 'HasGroups' do
           group_read.id => ['read'],
         }
 
-        group_access_instance.group_ids_access_map = expected
+        subject.group_ids_access_map = expected
 
-        result = group_access_instance.attributes_with_association_names
+        result = subject.attributes_with_association_names
         expect(result['group_ids']).to eq(expected)
       end
 
@@ -518,9 +522,9 @@ RSpec.shared_examples 'HasGroups' do
           group_read.name => ['read'],
         }
 
-        group_access_instance.group_names_access_map = expected
+        subject.group_names_access_map = expected
 
-        result = group_access_instance.attributes_with_association_names
+        result = subject.attributes_with_association_names
         expect(result['groups']).to eq(expected)
       end
     end
@@ -532,18 +536,20 @@ RSpec.shared_examples 'HasGroups' do
       end
 
       before(:each) do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
       end
 
       it 'lists only active instances' do
-        group_access_instance_inactive.group_names_access_map = {
+        subject.update!(active: false)
+
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
 
         result = described_class.group_access(group_read.id, 'read')
-        expect(result).not_to include(group_access_instance_inactive)
+        expect(result).not_to include(subject)
       end
 
       context 'Group ID parameter' do
@@ -571,24 +577,24 @@ RSpec.shared_examples 'HasGroups' do
       end
 
       it 'returns class instances' do
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_read.name => 'read',
         }
 
         result = described_class.group_access_ids(group_read, 'read')
-        expect(result).to include(group_access_instance.id)
+        expect(result).to include(subject.id)
       end
     end
 
     it 'destroys relations before instance gets destroyed' do
 
-      group_access_instance.group_names_access_map = {
+      subject.group_names_access_map = {
         group_full.name     => 'full',
         group_read.name     => 'read',
         group_inactive.name => 'change',
       }
       expect do
-        group_access_instance.destroy
+        subject.destroy
       end.to change {
         described_class.group_through.klass.count
       }.by(-3)
@@ -600,22 +606,22 @@ RSpec.shared_examples '#group_access? call' do
   context 'single access' do
 
     it 'checks positive' do
-      expect(group_access_instance.group_access?(group_parameter, 'read')).to be true
+      expect(subject.group_access?(group_parameter, 'read')).to be true
     end
 
     it 'checks negative' do
-      expect(group_access_instance.group_access?(group_parameter, 'change')).to be false
+      expect(subject.group_access?(group_parameter, 'change')).to be false
     end
   end
 
   context 'access list' do
 
     it 'checks positive' do
-      expect(group_access_instance.group_access?(group_parameter, %w[read change])).to be true
+      expect(subject.group_access?(group_parameter, %w[read change])).to be true
     end
 
     it 'checks negative' do
-      expect(group_access_instance.group_access?(group_parameter, %w[change create])).to be false
+      expect(subject.group_access?(group_parameter, %w[change create])).to be false
     end
   end
 end
@@ -624,22 +630,22 @@ RSpec.shared_examples '.group_access call' do
   context 'single access' do
 
     it 'lists access IDs' do
-      expect(described_class.group_access(group_parameter, 'read')).to include(group_access_instance)
+      expect(described_class.group_access(group_parameter, 'read')).to include(subject)
     end
 
     it 'excludes non access IDs' do
-      expect(described_class.group_access(group_parameter, 'change')).not_to include(group_access_instance)
+      expect(described_class.group_access(group_parameter, 'change')).not_to include(subject)
     end
   end
 
   context 'access list' do
 
     it 'lists access IDs' do
-      expect(described_class.group_access(group_parameter, %w[read change])).to include(group_access_instance)
+      expect(described_class.group_access(group_parameter, %w[read change])).to include(subject)
     end
 
     it 'excludes non access IDs' do
-      expect(described_class.group_access(group_parameter, %w[change create])).not_to include(group_access_instance)
+      expect(described_class.group_access(group_parameter, %w[change create])).not_to include(subject)
     end
   end
 end

+ 12 - 14
spec/models/concerns/has_groups_permissions_examples.rb

@@ -1,12 +1,10 @@
-# Requires: let(:group_access_no_permission_instance) { ... }
-RSpec.shared_examples 'HasGroups and Permissions' do
-
+RSpec.shared_examples 'HasGroups and Permissions' do |group_access_no_permission_factory:|
   context 'group' do
-
+    subject { build(group_access_no_permission_factory) }
     let(:group_read) { create(:group) }
 
     before(:each) do
-      group_access_no_permission_instance.group_names_access_map = {
+      subject.group_names_access_map = {
         group_read.name => 'read',
       }
     end
@@ -14,49 +12,49 @@ RSpec.shared_examples 'HasGroups and Permissions' do
     context '#group_access?' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.group_access?(group_read, 'read')).to be false
+        expect(subject.group_access?(group_read, 'read')).to be false
       end
     end
 
     context '#group_ids_access' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.group_ids_access('read')).to be_empty
+        expect(subject.group_ids_access('read')).to be_empty
       end
     end
 
     context '#groups_access' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.groups_access('read')).to be_empty
+        expect(subject.groups_access('read')).to be_empty
       end
     end
 
     context '#group_names_access_map' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.group_names_access_map).to be_empty
+        expect(subject.group_names_access_map).to be_empty
       end
     end
 
     context '#group_ids_access_map' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.group_ids_access_map).to be_empty
+        expect(subject.group_ids_access_map).to be_empty
       end
     end
 
     context '#attributes_with_association_ids' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.attributes_with_association_ids['group_ids']).to be_empty
+        expect(subject.attributes_with_association_ids['group_ids']).to be_empty
       end
     end
 
     context '#attributes_with_association_names' do
 
       it 'prevents instances without permissions' do
-        expect(group_access_no_permission_instance.attributes_with_association_names['group_ids']).to be_empty
+        expect(subject.attributes_with_association_names['group_ids']).to be_empty
       end
     end
 
@@ -64,7 +62,7 @@ RSpec.shared_examples 'HasGroups and Permissions' do
 
       it 'prevents instances without permissions' do
         result = described_class.group_access(group_read.id, 'read')
-        expect(result).not_to include(group_access_no_permission_instance)
+        expect(result).not_to include(subject)
       end
     end
 
@@ -72,7 +70,7 @@ RSpec.shared_examples 'HasGroups and Permissions' do
 
       it 'prevents instances without permissions' do
         result = described_class.group_access(group_read.id, 'read')
-        expect(result).not_to include(group_access_no_permission_instance.id)
+        expect(result).not_to include(subject.id)
       end
     end
   end

+ 46 - 51
spec/models/concerns/has_roles_examples.rb

@@ -1,13 +1,6 @@
-# Requires: let(:group_access_instance) { ... }
-# Requires: let(:new_group_access_instance) { ... }
-RSpec.shared_examples 'HasRoles' do
-
+RSpec.shared_examples 'HasRoles' do |group_access_factory:|
   context 'role' do
-
-    let(:group_access_instance_inactive) do
-      group_access_instance.update!(active: false)
-      group_access_instance
-    end
+    subject { create(group_access_factory) }
     let(:role) { create(:role) }
     let(:group_instance) { create(:group) }
     let(:group_role) { create(:group) }
@@ -16,7 +9,7 @@ RSpec.shared_examples 'HasRoles' do
     context '#role_access?' do
 
       it 'responds to role_access?' do
-        expect(group_access_instance).to respond_to(:role_access?)
+        expect(subject).to respond_to(:role_access?)
       end
 
       context 'active Role' do
@@ -25,8 +18,8 @@ RSpec.shared_examples 'HasRoles' do
             group_role.name => 'read',
           }
 
-          group_access_instance.roles.push(role)
-          group_access_instance.save
+          subject.roles.push(role)
+          subject.save
         end
 
         context 'Group ID parameter' do
@@ -46,7 +39,7 @@ RSpec.shared_examples 'HasRoles' do
             group_inactive.name => 'read',
           }
 
-          expect(group_access_instance.group_access?(group_inactive.id, 'read')).to be false
+          expect(subject.group_access?(group_inactive.id, 'read')).to be false
         end
       end
 
@@ -56,10 +49,10 @@ RSpec.shared_examples 'HasRoles' do
           group_role.name => 'read',
         }
 
-        group_access_instance.roles.push(role_inactive)
-        group_access_instance.save
+        subject.roles.push(role_inactive)
+        subject.save
 
-        expect(group_access_instance.group_access?(group_role.id, 'read')).to be false
+        expect(subject.group_access?(group_role.id, 'read')).to be false
       end
     end
 
@@ -70,8 +63,8 @@ RSpec.shared_examples 'HasRoles' do
           group_role.name => 'read',
         }
 
-        group_access_instance.roles.push(role)
-        group_access_instance.save
+        subject.roles.push(role)
+        subject.save
       end
 
       it 'responds to role_access_ids' do
@@ -79,16 +72,18 @@ RSpec.shared_examples 'HasRoles' do
       end
 
       it 'lists only active instance IDs' do
+        subject.update!(active: false)
+
         role.group_names_access_map = {
           group_role.name => 'read',
         }
 
-        group_access_instance_inactive.roles.push(role)
-        group_access_instance_inactive.save
-        group_access_instance_inactive.save
+        subject.roles.push(role)
+        subject.save
+        subject.save
 
         result = described_class.role_access_ids(group_role.id, 'read')
-        expect(result).not_to include(group_access_instance_inactive.id)
+        expect(result).not_to include(subject.id)
       end
 
       context 'Group ID parameter' do
@@ -111,10 +106,10 @@ RSpec.shared_examples 'HasRoles' do
           group_role.name => 'read',
         }
 
-        group_access_instance.roles.push(role)
-        group_access_instance.save
+        subject.roles.push(role)
+        subject.save
 
-        group_access_instance.group_names_access_map = {
+        subject.group_names_access_map = {
           group_instance.name => 'read',
         }
       end
@@ -122,13 +117,13 @@ RSpec.shared_examples 'HasRoles' do
       context '#group_access?' do
 
         it 'falls back to #role_access?' do
-          expect(group_access_instance).to receive(:role_access?)
-          group_access_instance.group_access?(group_role, 'read')
+          expect(subject).to receive(:role_access?)
+          subject.group_access?(group_role, 'read')
         end
 
         it "doesn't fall back to #role_access? if not needed" do
-          expect(group_access_instance).not_to receive(:role_access?)
-          group_access_instance.group_access?(group_instance, 'read')
+          expect(subject).not_to receive(:role_access?)
+          subject.group_access?(group_instance, 'read')
         end
       end
 
@@ -139,10 +134,10 @@ RSpec.shared_examples 'HasRoles' do
             group_role.name => 'read',
           }
 
-          group_access_instance.roles.push(role)
-          group_access_instance.save
+          subject.roles.push(role)
+          subject.save
 
-          group_access_instance.group_names_access_map = {
+          subject.group_names_access_map = {
             group_instance.name => 'read',
           }
         end
@@ -153,28 +148,28 @@ RSpec.shared_examples 'HasRoles' do
             group_inactive.name => 'read',
           }
 
-          result = group_access_instance.group_ids_access('read')
+          result = subject.group_ids_access('read')
           expect(result).not_to include(group_inactive.id)
         end
 
         context 'single access' do
 
           it 'lists access Group IDs' do
-            result = group_access_instance.group_ids_access('read')
+            result = subject.group_ids_access('read')
             expect(result).to include(group_role.id)
           end
 
           it "doesn't list for no access" do
-            result = group_access_instance.group_ids_access('change')
+            result = subject.group_ids_access('change')
             expect(result).not_to include(group_role.id)
           end
 
           it "doesn't contain duplicate IDs" do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_role.name => 'read',
             }
 
-            result = group_access_instance.group_ids_access('read')
+            result = subject.group_ids_access('read')
             expect(result.uniq).to eq(result)
           end
         end
@@ -182,21 +177,21 @@ RSpec.shared_examples 'HasRoles' do
         context 'access list' do
 
           it 'lists access Group IDs' do
-            result = group_access_instance.group_ids_access(%w[read change])
+            result = subject.group_ids_access(%w[read change])
             expect(result).to include(group_role.id)
           end
 
           it "doesn't list for no access" do
-            result = group_access_instance.group_ids_access(%w[change create])
+            result = subject.group_ids_access(%w[change create])
             expect(result).not_to include(group_role.id)
           end
 
           it "doesn't contain duplicate IDs" do
-            group_access_instance.group_names_access_map = {
+            subject.group_names_access_map = {
               group_role.name => 'read',
             }
 
-            result = group_access_instance.group_ids_access(%w[read create])
+            result = subject.group_ids_access(%w[read create])
             expect(result.uniq).to eq(result)
           end
         end
@@ -206,11 +201,11 @@ RSpec.shared_examples 'HasRoles' do
 
         it 'includes the result of .role_access_ids' do
           result = described_class.group_access_ids(group_role, 'read')
-          expect(result).to include(group_access_instance.id)
+          expect(result).to include(subject.id)
         end
 
         it "doesn't contain duplicate IDs" do
-          group_access_instance.group_names_access_map = {
+          subject.group_names_access_map = {
             group_role.name => 'read',
           }
 
@@ -226,22 +221,22 @@ RSpec.shared_examples '#role_access? call' do
   context 'single access' do
 
     it 'checks positive' do
-      expect(group_access_instance.role_access?(group_parameter, 'read')).to be true
+      expect(subject.role_access?(group_parameter, 'read')).to be true
     end
 
     it 'checks negative' do
-      expect(group_access_instance.role_access?(group_parameter, 'change')).to be false
+      expect(subject.role_access?(group_parameter, 'change')).to be false
     end
   end
 
   context 'access list' do
 
     it 'checks positive' do
-      expect(group_access_instance.role_access?(group_parameter, %w[read change])).to be true
+      expect(subject.role_access?(group_parameter, %w[read change])).to be true
     end
 
     it 'checks negative' do
-      expect(group_access_instance.role_access?(group_parameter, %w[change create])).to be false
+      expect(subject.role_access?(group_parameter, %w[change create])).to be false
     end
   end
 end
@@ -250,22 +245,22 @@ RSpec.shared_examples '.role_access_ids call' do
   context 'single access' do
 
     it 'lists access IDs' do
-      expect(described_class.role_access_ids(group_parameter, 'read')).to include(group_access_instance.id)
+      expect(described_class.role_access_ids(group_parameter, 'read')).to include(subject.id)
     end
 
     it 'excludes non access IDs' do
-      expect(described_class.role_access_ids(group_parameter, 'change')).not_to include(group_access_instance.id)
+      expect(described_class.role_access_ids(group_parameter, 'change')).not_to include(subject.id)
     end
   end
 
   context 'access list' do
 
     it 'lists access IDs' do
-      expect(described_class.role_access_ids(group_parameter, %w[read change])).to include(group_access_instance.id)
+      expect(described_class.role_access_ids(group_parameter, %w[read change])).to include(subject.id)
     end
 
     it 'excludes non access IDs' do
-      expect(described_class.role_access_ids(group_parameter, %w[change create])).not_to include(group_access_instance.id)
+      expect(described_class.role_access_ids(group_parameter, %w[change create])).not_to include(subject.id)
     end
   end
 end

+ 1 - 4
spec/models/role_spec.rb

@@ -2,10 +2,7 @@ require 'rails_helper'
 require 'models/concerns/has_groups_examples'
 
 RSpec.describe Role do
-  let(:group_access_instance) { create(:role) }
-  let(:new_group_access_instance) { build(:role) }
-
-  include_examples 'HasGroups'
+  include_examples 'HasGroups', group_access_factory: :role
 
   context '#validate_agent_limit_by_attributes' do
 

+ 297 - 396
spec/models/user_spec.rb

@@ -5,197 +5,268 @@ require 'models/concerns/has_groups_permissions_examples'
 require 'models/concerns/can_lookup_examples'
 
 RSpec.describe User do
-  let(:subject) { create(:user, user_attrs || {}) }
-  let(:group_access_instance) { create(:agent_user) }
-  let(:new_group_access_instance) { build(:agent_user) }
-  let(:group_access_no_permission_instance) { build(:user) }
-
-  include_examples 'HasGroups'
-  include_examples 'HasRoles'
-  include_examples 'HasGroups and Permissions'
+  include_examples 'HasGroups', group_access_factory: :agent_user
+  include_examples 'HasRoles', group_access_factory: :agent_user
+  include_examples 'HasGroups and Permissions', group_access_no_permission_factory: :user
   include_examples 'CanLookup'
 
-  let(:new_password) { 'N3W54V3PW!' }
+  subject(:user) { create(:user) }
 
-  context 'password' do
+  describe 'attributes' do
+    describe '#login_failed' do
+      before { user.update(login_failed: 1) }
 
-    it 'resets login_failed on password change' do
+      it 'resets failed login count when password is changed' do
+        expect { user.update(password: Faker::Internet.password) }
+          .to change { user.login_failed }.to(0)
+      end
+    end
 
-      failed_logins = (Setting.get('password_max_login_failed').to_i || 10) + 1
-      user          = create(:user, login_failed: failed_logins)
+    describe '#password' do
+      context 'when set to plaintext password' do
+        it 'hashes password before saving to DB' do
+          user.password = 'password'
 
-      expect do
-        user.password = new_password
-        user.save
-      end.to change { user.login_failed }.to(0)
-    end
-  end
+          expect { user.save }
+            .to change { user.password }.to(PasswordHash.crypt('password'))
+        end
+      end
+
+      context 'when set to SHA2 digest (to facilitate OTRS imports)' do
+        it 'does not re-hash before saving' do
+          user.password = "{sha2}#{Digest::SHA2.hexdigest('password')}"
+
+          expect { user.save }.not_to change { user.password }
+        end
+      end
 
-  context '#out_of_office_agent' do
+      context 'when set to Argon2 digest' do
+        it 'does not re-hash before saving' do
+          user.password = PasswordHash.crypt('password')
 
-    it 'responds to out_of_office_agent' do
-      user = create(:user)
-      expect(user).to respond_to(:out_of_office_agent)
+          expect { user.save }.not_to change { user.password }
+        end
+      end
     end
 
-    context 'replacement' do
+    describe '#phone' do
+      subject(:user) { create(:user, phone: orig_number) }
+
+      context 'when included on create' do
+        let(:orig_number) { '1234567890' }
+
+        it 'adds corresponding CallerId record' do
+          expect { user }
+            .to change { Cti::CallerId.where(caller_id: orig_number).count }.by(1)
+        end
+      end
+
+      context 'when added on update' do
+        let(:orig_number) { nil }
+        let(:new_number) { '1234567890' }
+
+        before { user } # create user
 
-      it 'finds assigned' do
-        user_replacement = create(:user)
+        it 'adds corresponding CallerId record' do
+          expect { user.update(phone: new_number) }
+            .to change { Cti::CallerId.where(caller_id: new_number).count }.by(1)
+        end
+      end
+
+      context 'when falsely added on update (change: [nil, ""])' do
+        let(:orig_number) { nil }
+        let(:new_number)  { '' }
+
+        before { user } # create user
+
+        it 'does not attempt to update CallerId record' do
+          allow(Cti::CallerId).to receive(:build).with(any_args)
+
+          expect(Cti::CallerId.where(object: 'User', o_id: user.id).count)
+            .to eq(0)
+
+          expect { user.update(phone: new_number) }
+            .to change { Cti::CallerId.where(object: 'User', o_id: user.id).count }.by(0)
+
+          expect(Cti::CallerId).not_to have_received(:build)
+        end
+      end
+
+      context 'when removed on update' do
+        let(:orig_number) { '1234567890' }
+        let(:new_number) { nil }
 
-        user_ooo = create(:user,
-                          out_of_office:                true,
-                          out_of_office_start_at:       Time.zone.yesterday,
-                          out_of_office_end_at:         Time.zone.tomorrow,
-                          out_of_office_replacement_id: user_replacement.id,)
+        before { user } # create user
 
-        expect(user_ooo.out_of_office_agent).to eq user_replacement
+        it 'removes corresponding CallerId record' do
+          expect { user.update(phone: nil) }
+            .to change { Cti::CallerId.where(caller_id: orig_number).count }.by(-1)
+        end
       end
 
-      it 'finds none for available users' do
-        user = create(:user)
-        expect(user.out_of_office_agent).to be nil
+      context 'when changed on update' do
+        let(:orig_number) { '1234567890' }
+        let(:new_number)  { orig_number.next }
+
+        before { user } # create user
+
+        it 'replaces CallerId record' do
+          # rubocop:disable Layout/MultilineMethodCallIndentation
+          expect { user.update(phone: new_number) }
+            .to change { Cti::CallerId.where(caller_id: orig_number).count }.by(-1)
+            .and change { Cti::CallerId.where(caller_id: new_number).count }.by(1)
+          # rubocop:enable Layout/MultilineMethodCallIndentation
+        end
       end
     end
   end
 
-  context '#max_login_failed?' do
+  describe '#max_login_failed?' do
+    it { is_expected.to respond_to(:max_login_failed?) }
 
-    it 'responds to max_login_failed?' do
-      user = create(:user)
-      expect(user).to respond_to(:max_login_failed?)
-    end
+    context 'with password_max_login_failed setting' do
+      before { Setting.set('password_max_login_failed', 5) }
+      before { user.update(login_failed: 5) }
 
-    it 'checks if a user has reached the maximum of failed logins' do
+      it 'returns true once user’s #login_failed count exceeds the setting' do
+        expect { user.update(login_failed: 6) }
+          .to change { user.max_login_failed? }.to(true)
+      end
+    end
 
-      user = create(:user)
-      expect(user.max_login_failed?).to be false
+    context 'without password_max_login_failed setting' do
+      before { Setting.set('password_max_login_failed', nil) }
+      before { user.update(login_failed: 0) }
 
-      user.login_failed = 999
-      user.save
-      expect(user.max_login_failed?).to be true
+      it 'defaults to 0' do
+        expect { user.update(login_failed: 1) }
+          .to change { user.max_login_failed? }.to(true)
+      end
     end
   end
 
-  context '.identify' do
+  describe '#out_of_office_agent' do
+    it { is_expected.to respond_to(:out_of_office_agent) }
 
-    it 'returns users found by login' do
-      user       = create(:user)
-      found_user = User.identify(user.login)
-      expect(found_user).to be_an(User)
-      expect(found_user.id).to eq user.id
+    context 'when user has no designated substitute' do
+      it 'returns nil' do
+        expect(user.out_of_office_agent).to be(nil)
+      end
     end
 
-    it 'returns users found by email' do
-      user       = create(:user)
-      found_user = User.identify(user.email)
-      expect(found_user).to be_an(User)
-      expect(found_user.id).to eq user.id
+    context 'when user has designated substitute, and is out of office' do
+      let(:substitute) { create(:user) }
+      subject(:user) do
+        create(:user,
+               out_of_office:                true,
+               out_of_office_start_at:       Time.zone.yesterday,
+               out_of_office_end_at:         Time.zone.tomorrow,
+               out_of_office_replacement_id: substitute.id,)
+      end
+
+      it 'returns the designated substitute' do
+        expect(user.out_of_office_agent).to eq(substitute)
+      end
     end
   end
 
-  context '.authenticate' do
+  describe '.authenticate' do
+    subject(:user) { create(:user, password: password) }
+    let(:password) { Faker::Internet.password }
 
-    it 'authenticates by username and password' do
-      password = 'zammad'
-      user     = create(:user, password: password)
-      result   = described_class.authenticate(user.login, password)
-      expect(result).to be_an(User)
+    context 'with valid credentials' do
+      it 'returns the matching user' do
+        expect(described_class.authenticate(user.login, password))
+          .to eq(user)
+      end
     end
 
-    context 'failure' do
+    context 'with valid credentials, but exceeding failed login limit' do
+      before { user.update(login_failed: 999) }
 
-      it 'increases login_failed on failed logins' do
-        user = create(:user)
-        expect do
-          described_class.authenticate(user.login, 'wrongpw')
-          user.reload
-        end
-          .to change { user.login_failed }.by(1)
+      it 'returns nil' do
+        expect(described_class.authenticate(user.login, password))
+          .to be(nil)
       end
+    end
 
-      it 'fails for unknown users' do
-        result = described_class.authenticate('john.doe', 'zammad')
-        expect(result).to be nil
+    context 'with valid user and invalid password' do
+      it 'increments failed login count' do
+        expect { described_class.authenticate(user.login, password.next) }
+          .to change { user.reload.login_failed }.by(1)
       end
 
-      it 'fails for inactive users' do
-        user   = create(:user, active: false)
-        result = described_class.authenticate(user.login, 'zammad')
-        expect(result).to be nil
+      it 'returns nil' do
+        expect(described_class.authenticate(user.login, password.next)).to be(nil)
       end
+    end
 
-      it 'fails for users with too many failed logins' do
-        user   = create(:user, login_failed: 999)
-        result = described_class.authenticate(user.login, 'zammad')
-        expect(result).to be nil
+    context 'with inactive user’s login' do
+      before { user.update(active: false) }
+
+      it 'returns nil' do
+        expect(described_class.authenticate(user.login, password)).to be(nil)
       end
+    end
 
-      it 'fails for wrong passwords' do
-        user   = create(:user)
-        result = described_class.authenticate(user.login, 'wrongpw')
-        expect(result).to be nil
+    context 'with non-existent user login' do
+      it 'returns nil' do
+        expect(described_class.authenticate('john.doe', password)).to be(nil)
       end
+    end
 
-      it 'fails for empty username parameter' do
-        result = described_class.authenticate('', 'zammad')
-        expect(result).to be nil
+    context 'with empty login string' do
+      it 'returns nil' do
+        expect(described_class.authenticate('', password)).to be(nil)
       end
+    end
 
-      it 'fails for empty password parameter' do
-        result = described_class.authenticate('username', '')
-        expect(result).to be nil
+    context 'with empty password string' do
+      it 'returns nil' do
+        expect(described_class.authenticate(user.login, '')).to be(nil)
       end
     end
   end
 
-  context '#by_reset_token' do
+  describe '#by_reset_token' do
+    let(:token) { create(:token_password_reset) }
+    subject(:user) { token.user }
 
-    it 'returns a User instance for existing tokens' do
-      token = create(:token_password_reset)
-      expect(described_class.by_reset_token(token.name)).to be_instance_of(described_class)
+    context 'with a valid token' do
+      it 'returns the matching user' do
+        expect(described_class.by_reset_token(token.name)).to eq(user)
+      end
     end
 
-    it 'returns nil for not existing tokens' do
-      expect(described_class.by_reset_token('not-existing')).to be nil
+    context 'with an invalid token' do
+      it 'returns nil' do
+        expect(described_class.by_reset_token('not-existing')).to be(nil)
+      end
     end
   end
 
-  context '#password_reset_via_token' do
+  describe '#password_reset_via_token' do
+    let!(:token) { create(:token_password_reset) }
+    subject(:user) { token.user }
 
     it 'changes the password of the token user and destroys the token' do
-      token = create(:token_password_reset)
-      user  = User.find(token.user_id)
-
-      expect do
-        described_class.password_reset_via_token(token.name, new_password)
-        user.reload
-      end.to change {
-        user.password
-      }.and change {
-        Token.count
-      }.by(-1)
+      expect { described_class.password_reset_via_token(token.name, Faker::Internet.password) }
+        .to change { user.reload.password }
+        .and change { Token.count }.by(-1)
     end
   end
 
-  context 'import' do
-
-    it "doesn't change imported passwords" do
-
-      # mock settings calls
-      expect(Setting).to receive(:get).with('import_mode').and_return(true)
-      allow(Setting).to receive(:get)
+  describe '.identify' do
+    it 'returns users by given login' do
+      expect(User.identify(user.login)).to eq(user)
+    end
 
-      user = build(:user, password: '{sha2}dd9c764fa7ea18cd992c8600006d3dc3ac983d1ba22e9ba2d71f6207456be0ba') # zammad
-      expect do
-        user.save
-      end.to_not change {
-        user.password
-      }
+    it 'returns users by given email' do
+      expect(User.identify(user.email)).to eq(user)
     end
   end
 
-  context '.access?' do
+  describe '#access?' do
 
     let(:role_with_admin_user_permissions) do
       create(:role).tap do |role|
@@ -533,7 +604,7 @@ RSpec.describe User do
     end
   end
 
-  context 'agent limit' do
+  describe 'system-wide agent limit' do
 
     def current_agent_count
       User.with_permissions('ticket.agent').count
@@ -542,323 +613,153 @@ RSpec.describe User do
     let(:agent_role) { Role.lookup(name: 'Agent') }
     let(:admin_role) { Role.lookup(name: 'Admin') }
 
-    context '#validate_agent_limit_by_role' do
+    describe '#validate_agent_limit_by_role' do
+      context 'for Integer value of system_agent_limit' do
+        context 'before exceeding the agent limit' do
+          before { Setting.set('system_agent_limit', current_agent_count + 1) }
 
-      context 'agent creation limit not reached' do
-
-        it 'grants agent creation' do
-          Setting.set('system_agent_limit', current_agent_count + 1)
-
-          expect do
-            create(:agent_user)
-          end.to change {
-            current_agent_count
-          }.by(1)
-        end
-
-        it 'grants role change' do
-          Setting.set('system_agent_limit', current_agent_count + 1)
-
-          future_agent = create(:customer_user)
-
-          expect do
-            future_agent.roles = [agent_role]
-          end.to change {
-            current_agent_count
-          }.by(1)
-        end
-
-        context 'role updates' do
-
-          it 'grants update by instances' do
-            Setting.set('system_agent_limit', current_agent_count + 1)
-
-            agent = create(:agent_user)
-
-            expect do
-              agent.roles = [
-                admin_role,
-                agent_role
-              ]
-              agent.save!
-            end.not_to raise_error
-          end
-
-          it 'grants update by id (Integer)' do
-            Setting.set('system_agent_limit', current_agent_count + 1)
-
-            agent = create(:agent_user)
-
-            expect do
-              agent.role_ids = [
-                admin_role.id,
-                agent_role.id
-              ]
-              agent.save!
-            end.not_to raise_error
+          it 'grants agent creation' do
+            expect { create(:agent_user) }
+              .to change { current_agent_count }.by(1)
           end
 
-          it 'grants update by id (String)' do
-            Setting.set('system_agent_limit', current_agent_count + 1)
+          it 'grants role change' do
+            future_agent = create(:customer_user)
 
-            agent = create(:agent_user)
-
-            expect do
-              agent.role_ids = [
-                admin_role.id.to_s,
-                agent_role.id.to_s
-              ]
-              agent.save!
-            end.not_to raise_error
+            expect { future_agent.roles = [agent_role] }
+              .to change { current_agent_count }.by(1)
           end
-        end
-      end
-
-      context 'agent creation limit surpassing prevention' do
-
-        it 'creation of new agents' do
-          Setting.set('system_agent_limit', current_agent_count + 2)
-
-          create_list(:agent_user, 2)
-
-          initial_agent_count = current_agent_count
-
-          expect do
-            create(:agent_user)
-          end.to raise_error(Exceptions::UnprocessableEntity)
-
-          expect(current_agent_count).to eq(initial_agent_count)
-        end
-
-        it 'prevents role change' do
-          Setting.set('system_agent_limit', current_agent_count)
 
-          future_agent = create(:customer_user)
+          describe 'role updates' do
+            let(:agent) { create(:agent_user) }
 
-          initial_agent_count = current_agent_count
+            it 'grants update by instances' do
+              expect { agent.roles = [admin_role, agent_role] }
+                .not_to raise_error
+            end
 
-          expect do
-            future_agent.roles = [agent_role]
-          end.to raise_error(Exceptions::UnprocessableEntity)
+            it 'grants update by id (Integer)' do
+              expect { agent.role_ids = [admin_role.id, agent_role.id] }
+                .not_to raise_error
+            end
 
-          expect(current_agent_count).to eq(initial_agent_count)
+            it 'grants update by id (String)' do
+              expect { agent.role_ids = [admin_role.id.to_s, agent_role.id.to_s] }
+                .not_to raise_error
+            end
+          end
         end
-      end
-    end
 
-    context '#validate_agent_limit_by_attributes' do
+        context 'when exceeding the agent limit' do
+          it 'creation of new agents' do
+            Setting.set('system_agent_limit', current_agent_count + 2)
 
-      context 'agent creation limit surpassing prevention' do
+            create_list(:agent_user, 2)
 
-        it 'prevents re-activation of agents' do
-          Setting.set('system_agent_limit', current_agent_count)
-
-          inactive_agent = create(:agent_user, active: false)
+            expect { create(:agent_user) }
+              .to raise_error(Exceptions::UnprocessableEntity)
+              .and change { current_agent_count }.by(0)
+          end
 
-          initial_agent_count = current_agent_count
+          it 'prevents role change' do
+            Setting.set('system_agent_limit', current_agent_count)
 
-          expect do
-            inactive_agent.update!(active: true)
-          end.to raise_error(Exceptions::UnprocessableEntity)
+            future_agent = create(:customer_user)
 
-          expect(current_agent_count).to eq(initial_agent_count)
+            expect { future_agent.roles = [agent_role] }
+              .to raise_error(Exceptions::UnprocessableEntity)
+              .and change { current_agent_count }.by(0)
+          end
         end
       end
-    end
-
-    context '#validate_agent_limit_by_role by string' do
-
-      context 'agent creation limit not reached' do
 
-        it 'grants agent creation' do
-          Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+      context 'for String value of system_agent_limit' do
+        context 'before exceeding the agent limit' do
+          before { Setting.set('system_agent_limit', (current_agent_count + 1).to_s) }
 
-          expect do
-            create(:agent_user)
-          end.to change {
-            current_agent_count
-          }.by(1)
-        end
-
-        it 'grants role change' do
-          Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
-
-          future_agent = create(:customer_user)
-
-          expect do
-            future_agent.roles = [agent_role]
-          end.to change {
-            current_agent_count
-          }.by(1)
-        end
-
-        context 'role updates' do
-
-          it 'grants update by instances' do
-            Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
-
-            agent = create(:agent_user)
-
-            expect do
-              agent.roles = [
-                admin_role,
-                agent_role
-              ]
-              agent.save!
-            end.not_to raise_error
+          it 'grants agent creation' do
+            expect { create(:agent_user) }
+              .to change { current_agent_count }.by(1)
           end
 
-          it 'grants update by id (Integer)' do
-            Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+          it 'grants role change' do
+            future_agent = create(:customer_user)
 
-            agent = create(:agent_user)
-
-            expect do
-              agent.role_ids = [
-                admin_role.id,
-                agent_role.id
-              ]
-              agent.save!
-            end.not_to raise_error
+            expect { future_agent.roles = [agent_role] }
+              .to change { current_agent_count }.by(1)
           end
 
-          it 'grants update by id (String)' do
-            Setting.set('system_agent_limit', (current_agent_count + 1).to_s)
+          describe 'role updates' do
+            let(:agent) { create(:agent_user) }
+
+            it 'grants update by instances' do
+              expect { agent.roles = [admin_role, agent_role] }
+                .not_to raise_error
+            end
 
-            agent = create(:agent_user)
+            it 'grants update by id (Integer)' do
+              expect { agent.role_ids = [admin_role.id, agent_role.id] }
+                .not_to raise_error
+            end
 
-            expect do
-              agent.role_ids = [
-                admin_role.id.to_s,
-                agent_role.id.to_s
-              ]
-              agent.save!
-            end.not_to raise_error
+            it 'grants update by id (String)' do
+              expect { agent.role_ids = [admin_role.id.to_s, agent_role.id.to_s] }
+                .not_to raise_error
+            end
           end
         end
-      end
-
-      context 'agent creation limit surpassing prevention' do
-
-        it 'creation of new agents' do
-          Setting.set('system_agent_limit', (current_agent_count + 2).to_s)
 
-          create_list(:agent_user, 2)
-
-          initial_agent_count = current_agent_count
-
-          expect do
-            create(:agent_user)
-          end.to raise_error(Exceptions::UnprocessableEntity)
-
-          expect(current_agent_count).to eq(initial_agent_count)
-        end
+        context 'when exceeding the agent limit' do
+          it 'creation of new agents' do
+            Setting.set('system_agent_limit', (current_agent_count + 2).to_s)
 
-        it 'prevents role change' do
-          Setting.set('system_agent_limit', current_agent_count.to_s)
+            create_list(:agent_user, 2)
 
-          future_agent = create(:customer_user)
+            expect { create(:agent_user) }
+              .to raise_error(Exceptions::UnprocessableEntity)
+              .and change { current_agent_count }.by(0)
+          end
 
-          initial_agent_count = current_agent_count
+          it 'prevents role change' do
+            Setting.set('system_agent_limit', current_agent_count.to_s)
 
-          expect do
-            future_agent.roles = [agent_role]
-          end.to raise_error(Exceptions::UnprocessableEntity)
+            future_agent = create(:customer_user)
 
-          expect(current_agent_count).to eq(initial_agent_count)
+            expect { future_agent.roles = [agent_role] }
+              .to raise_error(Exceptions::UnprocessableEntity)
+              .and change { current_agent_count }.by(0)
+          end
         end
       end
     end
 
-    context '#validate_agent_limit_by_attributes' do
-
-      context 'agent creation limit surpassing prevention' do
-
-        it 'prevents re-activation of agents' do
-          Setting.set('system_agent_limit', current_agent_count.to_s)
-
-          inactive_agent = create(:agent_user, active: false)
+    describe '#validate_agent_limit_by_attributes' do
+      context 'for Integer value of system_agent_limit' do
+        before { Setting.set('system_agent_limit', current_agent_count) }
 
-          initial_agent_count = current_agent_count
+        context 'when exceeding the agent limit' do
+          it 'prevents re-activation of agents' do
+            inactive_agent = create(:agent_user, active: false)
 
-          expect do
-            inactive_agent.update!(active: true)
-          end.to raise_error(Exceptions::UnprocessableEntity)
-
-          expect(current_agent_count).to eq(initial_agent_count)
+            expect { inactive_agent.update!(active: true) }
+              .to raise_error(Exceptions::UnprocessableEntity)
+              .and change { current_agent_count }.by(0)
+          end
         end
       end
-    end
-
-  end
-
-  context 'when phone attribute' do
-    let(:user_attrs) { { phone: orig_number } }
-
-    context 'included on create' do
-      let(:orig_number) { '1234567890' }
-
-      it 'adds corresponding CallerId record' do
-        expect { subject }
-          .to change { Cti::CallerId.where(caller_id: orig_number).count }.by(1)
-      end
-    end
-
-    context 'added on update' do
-      let(:orig_number) { nil }
-      let(:new_number) { '1234567890' }
 
-      before { subject } # create user
+      context 'for String value of system_agent_limit' do
+        before { Setting.set('system_agent_limit', current_agent_count.to_s) }
 
-      it 'adds corresponding CallerId record' do
-        expect { subject.update(phone: new_number) }
-          .to change { Cti::CallerId.where(caller_id: new_number).count }.by(1)
-      end
-    end
+        context 'when exceeding the agent limit' do
+          it 'prevents re-activation of agents' do
+            inactive_agent = create(:agent_user, active: false)
 
-    context 'falsely added on update (change: [nil, ""])' do
-      let(:orig_number) { nil }
-      let(:new_number)  { '' }
-
-      before { subject } # create user
-
-      it 'does not attempt to update CallerId record' do
-        allow(Cti::CallerId).to receive(:build).with(any_args)
-
-        expect(Cti::CallerId.where(object: 'User', o_id: subject.id).count)
-          .to eq(0)
-
-        expect { subject.update(phone: new_number) }
-          .to change { Cti::CallerId.where(object: 'User', o_id: subject.id).count }.by(0)
-
-        expect(Cti::CallerId).not_to have_received(:build)
-      end
-    end
-
-    context 'removed on update' do
-      let(:orig_number) { '1234567890' }
-      let(:new_number) { nil }
-
-      before { subject } # create user
-
-      it 'removes corresponding CallerId record' do
-        expect { subject.update(phone: nil) }
-          .to change { Cti::CallerId.where(caller_id: orig_number).count }.by(-1)
-      end
-    end
-
-    context 'changed on update' do
-      let(:orig_number) { '1234567890' }
-      let(:new_number)  { orig_number.next }
-
-      before { subject } # create user
-
-      it 'replaces CallerId record' do
-        # rubocop:disable Layout/MultilineMethodCallIndentation
-        expect { subject.update(phone: new_number) }
-          .to change { Cti::CallerId.where(caller_id: orig_number).count }.by(-1)
-          .and change { Cti::CallerId.where(caller_id: new_number).count }.by(1)
-        # rubocop:enable Layout/MultilineMethodCallIndentation
+            expect { inactive_agent.update!(active: true) }
+              .to raise_error(Exceptions::UnprocessableEntity)
+              .and change { current_agent_count }.by(0)
+          end
+        end
       end
     end
   end