FIX: Add missing user_id args for ChatMessage.cook (#19508)

In both ChatMessage#rebake! and in ChatMessageProcessor
when we were calling ChatMessage.cook we were missing the
user_id to cook with, which causes missed hashtag cooks
because of missing permissions.

lib/pretty_text/helpers.rb

@@ -111,8 +111,13 @@ module PrettyText
     end
 
     def hashtag_lookup(slug, cooking_user_id, types_in_priority_order)
-      # This is _somewhat_ expected since we need to be able to cook posts
+      # NOTE: This is _somewhat_ expected since we need to be able to cook posts
       # etc. without a user sometimes, but it is still an edge case.
+      #
+      # The Discourse.system_user is usually an admin with access to _all_
+      # categories, however if the suppress_secured_categories_from_admin
+      # site setting is activated then this user will not be able to access
+      # secure categories, so hashtags that are secure will not render.
       if cooking_user_id.blank?
         cooking_user = Discourse.system_user
       else

plugins/chat/app/models/chat_message.rb

@@ -110,19 +110,20 @@ class ChatMessage < ActiveRecord::Base
   def cook
     ensure_last_editor_id
 
-    # A rule in our Markdown pipeline may have Guardian checks that require a
-    # user to be present. The last editing user of the message will be more
-    # generally up to date than the creating user. For example, we use
-    # this when cooking #hashtags to determine whether we should render
-    # the found hashtag based on whether the user can access the channel it
-    # is referencing.
     self.cooked = self.class.cook(self.message, user_id: self.last_editor_id)
     self.cooked_version = BAKED_VERSION
   end
 
   def rebake!(invalidate_oneboxes: false, priority: nil)
+    ensure_last_editor_id
+
     previous_cooked = self.cooked
-    new_cooked = self.class.cook(message, invalidate_oneboxes: invalidate_oneboxes)
+    new_cooked =
+      self.class.cook(
+        message,
+        invalidate_oneboxes: invalidate_oneboxes,
+        user_id: self.last_editor_id,
+      )
     update_columns(cooked: new_cooked, cooked_version: BAKED_VERSION)
     args = { chat_message_id: self.id }
     args[:queue] = priority.to_s if priority && priority != :normal
@@ -177,6 +178,12 @@ class ChatMessage < ActiveRecord::Base
   ]
 
   def self.cook(message, opts = {})
+    # A rule in our Markdown pipeline may have Guardian checks that require a
+    # user to be present. The last editing user of the message will be more
+    # generally up to date than the creating user. For example, we use
+    # this when cooking #hashtags to determine whether we should render
+    # the found hashtag based on whether the user can access the channel it
+    # is referencing.
     cooked =
       PrettyText.cook(
         message,

plugins/chat/lib/chat_message_processor.rb

@@ -10,7 +10,7 @@ class Chat::ChatMessageProcessor
     @size_cache = {}
     @opts = {}
 
-    cooked = ChatMessage.cook(chat_message.message)
+    cooked = ChatMessage.cook(chat_message.message, user_id: chat_message.last_editor_id)
     @doc = Loofah.fragment(cooked)
   end
 

plugins/chat/spec/lib/chat_message_processor_spec.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::ChatMessageProcessor do
+  fab!(:message) { Fabricate(:chat_message) }
+
+  it "cooks using the last_editor_id of the message" do
+    ChatMessage.expects(:cook).with(message.message, user_id: message.last_editor_id)
+    described_class.new(message)
+  end
+end

plugins/chat/spec/models/chat_message_spec.rb

@@ -5,6 +5,18 @@ require "rails_helper"
 describe ChatMessage do
   fab!(:message) { Fabricate(:chat_message, message: "hey friend, what's up?!") }
 
+  # TODO (martin) Remove this after https://github.com/discourse/discourse/pull/19491 is merged
+  def register_hashtag_contexts
+    # This is annoying, but we need to reset the hashtag data sources inbetween
+    # tests, and since this is normally done in plugin.rb with the plugin API
+    # there is not an easier way to do this.
+    HashtagAutocompleteService.register_data_source("channel", Chat::ChatChannelHashtagDataSource)
+    HashtagAutocompleteService.register_type_in_context("channel", "chat-composer", 200)
+    HashtagAutocompleteService.register_type_in_context("category", "chat-composer", 100)
+    HashtagAutocompleteService.register_type_in_context("tag", "chat-composer", 50)
+    HashtagAutocompleteService.register_type_in_context("channel", "topic-composer", 10)
+  end
+
   describe ".cook" do
     it "does not support HTML tags" do
       cooked = ChatMessage.cook("<h1>test</h1>")
@@ -234,6 +246,7 @@ describe ChatMessage do
       expect(cooked).to eq("<p><span class=\"mention\">@mention</span></p>")
     end
 
+    # TODO (martin) Remove this when enable_experimental_hashtag_autocomplete is default
     it "supports category-hashtag plugin" do
       category = Fabricate(:category)
 
@@ -244,6 +257,20 @@ describe ChatMessage do
       )
     end
 
+    it "supports hashtag-autocomplete plugin" do
+      register_hashtag_contexts
+      SiteSetting.enable_experimental_hashtag_autocomplete = true
+
+      category = Fabricate(:category)
+      user = Fabricate(:user)
+
+      cooked = ChatMessage.cook("##{category.slug}", user_id: user.id)
+
+      expect(cooked).to eq(
+        "<p><a class=\"hashtag-cooked\" href=\"#{category.url}\" data-type=\"category\" data-slug=\"#{category.slug}\"><svg class=\"fa d-icon d-icon-folder svg-icon svg-node\"><use href=\"#folder\"></use></svg><span>#{category.name}</span></a></p>",
+      )
+    end
+
     it "supports censored plugin" do
       watched_word = Fabricate(:watched_word, action: WatchedWord.actions[:censor])
 
@@ -484,4 +511,34 @@ describe ChatMessage do
       end
     end
   end
+
+  describe "#rebake!" do
+    fab!(:chat_message) { Fabricate(:chat_message) }
+
+    describe "hashtags" do
+      fab!(:category) { Fabricate(:category) }
+      fab!(:group) { Fabricate(:group) }
+      fab!(:secure_category) { Fabricate(:private_category, group: group) }
+
+      before do
+        register_hashtag_contexts
+        SiteSetting.enable_experimental_hashtag_autocomplete = true
+        SiteSetting.suppress_secured_categories_from_admin = true
+      end
+
+      it "keeps the same hashtags the user has permission to after rebake" do
+        group.add(chat_message.user)
+        chat_message.update!(
+          message: "this is the message ##{category.slug} ##{secure_category.slug} ##{chat_message.chat_channel.slug}",
+        )
+        chat_message.cook
+        chat_message.save!
+
+        expect(chat_message.reload.cooked).to include(secure_category.name)
+
+        chat_message.rebake!
+        expect(chat_message.reload.cooked).to include(secure_category.name)
+      end
+    end
+  end
 end