7 files changed, 197 insertions, 108 deletions

diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb
index 37e2f5fb8d4..6e62d4ef31b 100644
--- a/spec/models/concerns/cache_markdown_field_spec.rb
+++ b/spec/models/concerns/cache_markdown_field_spec.rb
@@ -233,7 +233,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do
         end
 
         it 'calls #refresh_markdown_cache!' do
-          expect(thing).to receive(:refresh_markdown_cache!)
+          expect(thing).to receive(:refresh_markdown_cache)
 
           expect(thing.updated_cached_html_for(:description)).to eq(html)
         end
@@ -279,10 +279,101 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do
     end
   end
 
+  shared_examples 'a class with mentionable markdown fields' do
+    let(:mentionable) { klass.new(description: markdown, description_html: html, title: markdown, title_html: html, cached_markdown_version: cache_version) }
+
+    context 'when klass is a Mentionable', :aggregate_failures do
+      before do
+        klass.send(:include, Mentionable)
+        klass.send(:attr_mentionable, :description)
+      end
+
+      describe '#mentionable_attributes_changed?' do
+        message = Struct.new(:text)
+
+        let(:changes) do
+          msg = message.new('test')
+
+          changes = {}
+          changes[msg] = ['', 'some message']
+          changes[:random_sym_key] = ['', 'some message']
+          changes["description"] = ['', 'some message']
+          changes
+        end
+
+        it 'returns true with key string' do
+          changes["description_html"] = ['', 'some message']
+
+          allow(mentionable).to receive(:saved_changes).and_return(changes)
+
+          expect(mentionable.send(:mentionable_attributes_changed?)).to be true
+        end
+
+        it 'returns false with key symbol' do
+          changes[:description_html] = ['', 'some message']
+          allow(mentionable).to receive(:saved_changes).and_return(changes)
+
+          expect(mentionable.send(:mentionable_attributes_changed?)).to be false
+        end
+
+        it 'returns false when no attr_mentionable keys' do
+          allow(mentionable).to receive(:saved_changes).and_return(changes)
+
+          expect(mentionable.send(:mentionable_attributes_changed?)).to be false
+        end
+      end
+
+      describe '#save' do
+        context 'when cache is outdated' do
+          before do
+            thing.cached_markdown_version += 1
+          end
+
+          context 'when the markdown field also a mentionable attribute' do
+            let(:thing) { klass.new(description: markdown, description_html: html, cached_markdown_version: cache_version) }
+
+            it 'calls #store_mentions!' do
+              expect(thing).to receive(:mentionable_attributes_changed?).and_return(true)
+              expect(thing).to receive(:store_mentions!)
+
+              thing.try(:save)
+
+              expect(thing.description_html).to eq(html)
+            end
+          end
+
+          context 'when the markdown field is not mentionable attribute' do
+            let(:thing) { klass.new(title: markdown, title_html: html, cached_markdown_version: cache_version) }
+
+            it 'does not call #store_mentions!' do
+              expect(thing).not_to receive(:store_mentions!)
+              expect(thing).to receive(:refresh_markdown_cache)
+
+              thing.try(:save)
+
+              expect(thing.title_html).to eq(html)
+            end
+          end
+        end
+
+        context 'when the markdown field does not exist' do
+          let(:thing) { klass.new(cached_markdown_version: cache_version) }
+
+          it 'does not call #store_mentions!' do
+            expect(thing).not_to receive(:store_mentions!)
+
+            thing.try(:save)
+          end
+        end
+      end
+    end
+  end
+
   context 'for Active record classes' do
     let(:klass) { ar_class }
 
     it_behaves_like 'a class with cached markdown fields'
+    it_behaves_like 'a class with mentionable markdown fields'
 
     describe '#attribute_invalidated?' do
       let(:thing) { klass.create!(description: markdown, description_html: html, cached_markdown_version: cache_version) }
diff --git a/spec/models/concerns/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb
index 5fb7cdb4443..7cf7b825d7d 100644
--- a/spec/models/concerns/case_sensitivity_spec.rb
+++ b/spec/models/concerns/case_sensitivity_spec.rb
@@ -4,16 +4,16 @@ require 'spec_helper'
 
 RSpec.describe CaseSensitivity do
   describe '.iwhere' do
-    let(:connection) { ActiveRecord::Base.connection }
-    let(:model) do
+    let_it_be(:connection) { ActiveRecord::Base.connection }
+    let_it_be(:model) do
       Class.new(ActiveRecord::Base) do
         include CaseSensitivity
         self.table_name = 'namespaces'
       end
     end
 
-    let!(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') }
-    let!(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') }
+    let_it_be(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') }
+    let_it_be(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') }
 
     it 'finds a single instance by a single attribute regardless of case' do
       expect(model.iwhere(path: 'MODEL-1')).to contain_exactly(model_1)
@@ -28,6 +28,10 @@ RSpec.describe CaseSensitivity do
         .to contain_exactly(model_1)
     end
 
+    it 'finds instances by custom Arel attributes' do
+      expect(model.iwhere(model.arel_table[:path] => 'MODEL-1')).to contain_exactly(model_1)
+    end
+
     it 'builds a query using LOWER' do
       query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql
       expected_query = <<~QRY.strip
diff --git a/spec/models/concerns/ignorable_columns_spec.rb b/spec/models/concerns/ignorable_columns_spec.rb
index a5eff154a0b..c97dc606159 100644
--- a/spec/models/concerns/ignorable_columns_spec.rb
+++ b/spec/models/concerns/ignorable_columns_spec.rb
@@ -59,6 +59,14 @@ RSpec.describe IgnorableColumns do
       it_behaves_like 'storing removal information'
     end
 
+    context 'when called on a subclass without setting the ignored columns' do
+      let(:subclass) { Class.new(record_class) }
+
+      it 'does not raise Deadlock error' do
+        expect { subclass.ignored_columns_details }.not_to raise_error
+      end
+    end
+
     it 'defaults to empty Hash' do
       expect(subject.ignored_columns_details).to eq({})
     end
diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb
index 516c0fd75bc..3c095477ea9 100644
--- a/spec/models/concerns/mentionable_spec.rb
+++ b/spec/models/concerns/mentionable_spec.rb
@@ -29,42 +29,6 @@ RSpec.describe Mentionable do
       expect(mentionable.referenced_mentionables).to be_empty
     end
   end
-
-  describe '#any_mentionable_attributes_changed?' do
-    message = Struct.new(:text)
-
-    let(:mentionable) { Example.new }
-    let(:changes) do
-      msg = message.new('test')
-
-      changes = {}
-      changes[msg] = ['', 'some message']
-      changes[:random_sym_key] = ['', 'some message']
-      changes["random_string_key"] = ['', 'some message']
-      changes
-    end
-
-    it 'returns true with key string' do
-      changes["message"] = ['', 'some message']
-
-      allow(mentionable).to receive(:saved_changes).and_return(changes)
-
-      expect(mentionable.send(:any_mentionable_attributes_changed?)).to be true
-    end
-
-    it 'returns false with key symbol' do
-      changes[:message] = ['', 'some message']
-      allow(mentionable).to receive(:saved_changes).and_return(changes)
-
-      expect(mentionable.send(:any_mentionable_attributes_changed?)).to be false
-    end
-
-    it 'returns false when no attr_mentionable keys' do
-      allow(mentionable).to receive(:saved_changes).and_return(changes)
-
-      expect(mentionable.send(:any_mentionable_attributes_changed?)).to be false
-    end
-  end
 end
 
 RSpec.describe Issue, "Mentionable" do
diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb
index ba70ff563a8..2059e170446 100644
--- a/spec/models/concerns/project_features_compatibility_spec.rb
+++ b/spec/models/concerns/project_features_compatibility_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe ProjectFeaturesCompatibility do
   let(:project) { create(:project) }
   let(:features_enabled) { %w(issues wiki builds merge_requests snippets) }
-  let(:features) { features_enabled + %w(repository pages) }
+  let(:features) { features_enabled + %w(repository pages operations) }
 
   # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table
   # All those fields got moved to a new table called project_feature and are now integers instead of booleans
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index e4cf68663ef..6ab87053258 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -2,8 +2,69 @@
 
 require 'spec_helper'
 
+RSpec.shared_examples '.find_by_full_path' do
+  describe '.find_by_full_path', :aggregate_failures do
+    it 'finds records by their full path' do
+      expect(described_class.find_by_full_path(record.full_path)).to eq(record)
+      expect(described_class.find_by_full_path(record.full_path.upcase)).to eq(record)
+    end
+
+    it 'returns nil for unknown paths' do
+      expect(described_class.find_by_full_path('unknown')).to be_nil
+    end
+
+    it 'includes route information when loading a record' do
+      control_count = ActiveRecord::QueryRecorder.new do
+        described_class.find_by_full_path(record.full_path)
+      end.count
+
+      expect do
+        described_class.find_by_full_path(record.full_path).route
+      end.not_to exceed_all_query_limit(control_count)
+    end
+
+    context 'with redirect routes' do
+      let_it_be(:redirect_route) { create(:redirect_route, source: record) }
+
+      context 'without follow_redirects option' do
+        it 'does not find records by their redirected path' do
+          expect(described_class.find_by_full_path(redirect_route.path)).to be_nil
+          expect(described_class.find_by_full_path(redirect_route.path.upcase)).to be_nil
+        end
+      end
+
+      context 'with follow_redirects option set to true' do
+        it 'finds records by their canonical path' do
+          expect(described_class.find_by_full_path(record.full_path, follow_redirects: true)).to eq(record)
+          expect(described_class.find_by_full_path(record.full_path.upcase, follow_redirects: true)).to eq(record)
+        end
+
+        it 'finds records by their redirected path' do
+          expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to eq(record)
+          expect(described_class.find_by_full_path(redirect_route.path.upcase, follow_redirects: true)).to eq(record)
+        end
+
+        it 'returns nil for unknown paths' do
+          expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to be_nil
+        end
+      end
+    end
+  end
+end
+
+RSpec.describe Routable do
+  it_behaves_like '.find_by_full_path' do
+    let_it_be(:record) { create(:group) }
+  end
+
+  it_behaves_like '.find_by_full_path' do
+    let_it_be(:record) { create(:project) }
+  end
+end
+
 RSpec.describe Group, 'Routable' do
-  let!(:group) { create(:group, name: 'foo') }
+  let_it_be_with_reload(:group) { create(:group, name: 'foo') }
+  let_it_be(:nested_group) { create(:group, parent: group) }
 
   describe 'Validations' do
     it { is_expected.to validate_presence_of(:route) }
@@ -59,61 +120,20 @@ RSpec.describe Group, 'Routable' do
   end
 
   describe '.find_by_full_path' do
-    let!(:nested_group) { create(:group, parent: group) }
-
-    context 'without any redirect routes' do
-      it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
-      it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
-      it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
-      it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
-
-      it 'includes route information when loading a record' do
-        path = group.to_param
-        control_count = ActiveRecord::QueryRecorder.new { described_class.find_by_full_path(path) }.count
-
-        expect { described_class.find_by_full_path(path).route }.not_to exceed_all_query_limit(control_count)
-      end
+    it_behaves_like '.find_by_full_path' do
+      let_it_be(:record) { group }
     end
 
-    context 'with redirect routes' do
-      let!(:group_redirect_route) { group.redirect_routes.create!(path: 'bar') }
-      let!(:nested_group_redirect_route) { nested_group.redirect_routes.create!(path: nested_group.path.sub('foo', 'bar')) }
-
-      context 'without follow_redirects option' do
-        context 'with the given path not matching any route' do
-          it { expect(described_class.find_by_full_path('unknown')).to eq(nil) }
-        end
-
-        context 'with the given path matching the canonical route' do
-          it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) }
-          it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) }
-          it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) }
-        end
-
-        context 'with the given path matching a redirect route' do
-          it { expect(described_class.find_by_full_path(group_redirect_route.path)).to eq(nil) }
-          it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase)).to eq(nil) }
-          it { expect(described_class.find_by_full_path(nested_group_redirect_route.path)).to eq(nil) }
-        end
-      end
-
-      context 'with follow_redirects option set to true' do
-        context 'with the given path not matching any route' do
-          it { expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil) }
-        end
+    it_behaves_like '.find_by_full_path' do
+      let_it_be(:record) { nested_group }
+    end
 
-        context 'with the given path matching the canonical route' do
-          it { expect(described_class.find_by_full_path(group.to_param, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(group.to_param.upcase, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(nested_group.to_param, follow_redirects: true)).to eq(nested_group) }
-        end
+    it 'does not find projects with a matching path' do
+      project = create(:project)
+      redirect_route = create(:redirect_route, source: project)
 
-        context 'with the given path matching a redirect route' do
-          it { expect(described_class.find_by_full_path(group_redirect_route.path, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase, follow_redirects: true)).to eq(group) }
-          it { expect(described_class.find_by_full_path(nested_group_redirect_route.path, follow_redirects: true)).to eq(nested_group) }
-        end
-      end
+      expect(described_class.find_by_full_path(project.full_path)).to be_nil
+      expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil
     end
   end
 
@@ -131,8 +151,6 @@ RSpec.describe Group, 'Routable' do
     end
 
     context 'with valid paths' do
-      let!(:nested_group) { create(:group, parent: group) }
-
       it 'returns the projects matching the paths' do
         result = described_class.where_full_path_in([group.to_param, nested_group.to_param])
 
@@ -148,32 +166,36 @@ RSpec.describe Group, 'Routable' do
   end
 
   describe '#full_path' do
-    let(:group) { create(:group) }
-    let(:nested_group) { create(:group, parent: group) }
-
     it { expect(group.full_path).to eq(group.path) }
     it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") }
   end
 
   describe '#full_name' do
-    let(:group) { create(:group) }
-    let(:nested_group) { create(:group, parent: group) }
-
     it { expect(group.full_name).to eq(group.name) }
     it { expect(nested_group.full_name).to eq("#{group.name} / #{nested_group.name}") }
   end
 end
 
 RSpec.describe Project, 'Routable' do
-  describe '#full_path' do
-    let(:project) { build_stubbed(:project) }
+  let_it_be(:project) { create(:project) }
 
+  it_behaves_like '.find_by_full_path' do
+    let_it_be(:record) { project }
+  end
+
+  it 'does not find groups with a matching path' do
+    group = create(:group)
+    redirect_route = create(:redirect_route, source: group)
+
+    expect(described_class.find_by_full_path(group.full_path)).to be_nil
+    expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil
+  end
+
+  describe '#full_path' do
     it { expect(project.full_path).to eq "#{project.namespace.full_path}/#{project.path}" }
   end
 
   describe '#full_name' do
-    let(:project) { build_stubbed(:project) }
-
     it { expect(project.full_name).to eq "#{project.namespace.human_name} / #{project.name}" }
   end
 end
diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb
index 90e94b5dca9..d8b77e1cd0d 100644
--- a/spec/models/concerns/token_authenticatable_spec.rb
+++ b/spec/models/concerns/token_authenticatable_spec.rb
@@ -105,8 +105,8 @@ RSpec.describe PersonalAccessToken, 'TokenAuthenticatable' do
     it 'sets new token' do
       subject
 
-      expect(personal_access_token.token).to eq(token_value)
-      expect(personal_access_token.token_digest).to eq(Gitlab::CryptoHelper.sha256(token_value))
+      expect(personal_access_token.token).to eq("#{PersonalAccessToken.token_prefix}#{token_value}")
+      expect(personal_access_token.token_digest).to eq(Gitlab::CryptoHelper.sha256("#{PersonalAccessToken.token_prefix}#{token_value}"))
     end
   end