Save mentions when markdown columns are directly saved to DB

When markdown columns are saved directly to DB, model callbacks
are skiped and we are not saving user mentions. We can check if
markdown fields are for a mentionable model and save mentions
for that case.

app/models/concerns/cache_markdown_field.rb

@@ -70,8 +70,11 @@ module CacheMarkdownField
 
   def refresh_markdown_cache!
     updates = refresh_markdown_cache
-
-    save_markdown(updates)
+    if save_markdown(updates)
+      # save_markdown updates DB columns directly, so reload to let store_mentions! be able to parse mentions
+      # otherwise we end up in a loop
+      reset.store_mentions! if is_a?(Mentionable) && mentionable_attrs_changed?(updates.keys)
+    end
   end
 
   def cached_html_up_to_date?(markdown_field)
@@ -106,7 +109,18 @@ module CacheMarkdownField
   def updated_cached_html_for(markdown_field)
     return unless cached_markdown_fields.markdown_fields.include?(markdown_field)
 
-    refresh_markdown_cache! if attribute_invalidated?(cached_markdown_fields.html_field(markdown_field))
+    if attribute_invalidated?(cached_markdown_fields.html_field(markdown_field))
+      # * If html is invalid and there is a change in markdown field, then just refresh html for the
+      # corresponding markdown field.
+      # * Else if the the markdown did not changee it means it is freshly loaded from DB
+      # but its corresponding html field is invalid, so we refresh and update it in DB.
+      # * The change in html field will trigger also a mentions parsing as well as an update if needed.
+      if changed_attributes[markdown_field]
+        refresh_markdown_cache
+      else
+        refresh_markdown_cache!
+      end
+    end
 
     cached_html_for(markdown_field)
   end
@@ -140,6 +154,16 @@ module CacheMarkdownField
     nil
   end
 
+  private
+
+  def mentionable_attrs_changed?(updated_fields)
+    return false unless is_a?(Mentionable)
+
+    self.class.mentionable_attrs.any? do |attr|
+      updated_fields.include?(cached_markdown_fields.html_field(attr.first))
+    end
+  end
+
   included do
     cattr_reader :cached_markdown_fields do
       Gitlab::MarkdownCache::FieldData.new

spec/models/concerns/cache_markdown_field_spec.rb

@@ -145,7 +145,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do
       it 'saves the changes' do
         expect(thing)
           .to receive(:save_markdown)
-          .with("description_html" => updated_html, "title_html" => "", "cached_markdown_version" => cache_version)
+                .with("description_html" => updated_html, "title_html" => "", "cached_markdown_version" => cache_version)
 
         thing.refresh_markdown_cache!
       end
@@ -279,10 +279,69 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do
     end
   end
 
+  shared_examples 'a class with mentionable markdown fields' do
+    context 'when klass is a Mentionable' do
+      before do
+        klass.send(:include, Mentionable)
+        klass.send(:attr_mentionable, :description)
+      end
+
+      describe '#updated_cached_html_for' do
+        let(:thing) { klass.new(description: markdown, description_html: html, title: markdown, title_html: html, cached_markdown_version: cache_version) }
+
+        context 'when cache is outdated' do
+          before do
+            thing.cached_markdown_version += 1
+            allow(thing).to receive(:reset).and_return(thing)
+            allow(thing).to receive(:save_markdown).and_return(true)
+          end
+
+          context 'when the markdown field also a mentionable attribute' do
+            it 'calls #store_mentions!' do
+              expect(thing).to receive(:store_mentions!)
+
+              expect(thing.updated_cached_html_for(:description)).to eq(html)
+            end
+          end
+
+          context 'when the markdown field is not mentionable attribute' do
+            it 'does not call #store_mentions!' do
+              expect(thing).not_to receive(:store_mentions!)
+              expect(thing).to receive(:refresh_markdown_cache!)
+
+              thing.updated_cached_html_for(:title)
+            end
+          end
+        end
+
+        context 'when the markdown field does not exist' do
+          it 'does not call #store_mentions!' do
+            expect(thing).not_to receive(:store_mentions!)
+
+            thing.updated_cached_html_for(:something)
+          end
+        end
+
+        context 'when the markdown cache is up to date' do
+          before do
+            thing.try(:save)
+          end
+
+          it 'does not call #store_mentions!' do
+            expect(thing).not_to receive(:store_mentions!)
+
+            thing.updated_cached_html_for(:description)
+          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) }
@@ -337,5 +396,6 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do
     let(:klass) { other_class }
 
     it_behaves_like 'a class with cached markdown fields'
+    it_behaves_like 'a class with mentionable markdown fields'
   end
 end