Merge branch 'tc-custom-emoji-fixes' into 'master'

Fixes for custom emoji

See merge request gitlab-org/gitlab!84304

app/models/award_emoji.rb

@@ -19,6 +19,8 @@ class AwardEmoji < ApplicationRecord
 
   participant :user
 
+  delegate :resource_parent, to: :awardable, allow_nil: true
+
   scope :downvotes, -> { named(DOWNVOTE_NAME) }
   scope :upvotes, -> { named(UPVOTE_NAME) }
   scope :named, -> (names) { where(name: names) }
@@ -61,7 +63,9 @@ class AwardEmoji < ApplicationRecord
   end
 
   def url
-    awardable.try(:namespace)&.custom_emoji&.by_name(name)&.first&.url
+    return if TanukiEmoji.find_by_alpha_code(name)
+
+    CustomEmoji.for_resource(resource_parent).by_name(name).select(:url).first&.url
   end
 
   def expire_cache

app/models/custom_emoji.rb

@@ -28,6 +28,19 @@ class CustomEmoji < ApplicationRecord
 
   alias_attribute :url, :file # this might need a change in https://gitlab.com/gitlab-org/gitlab/-/issues/230467
 
+  # Find custom emoji for the given resource.
+  # A resource can be either a Project or a Group, or anything responding to #root_ancestor.
+  # Usually it's the return value of #resource_parent on any model.
+  scope :for_resource, -> (resource) do
+    return none if resource.nil?
+
+    namespace = resource.root_ancestor
+
+    return none if namespace.nil? || Feature.disabled?(:custom_emoji, namespace)
+
+    namespace.custom_emoji
+  end
+
   private
 
   def valid_emoji_name

app/models/snippet.rb

@@ -40,6 +40,7 @@ class Snippet < ApplicationRecord
 
   belongs_to :author, class_name: 'User'
   belongs_to :project
+  alias_method :resource_parent, :project
 
   has_many :notes, as: :noteable, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :user_mentions, class_name: "SnippetUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent

app/validators/gitlab/emoji_name_validator.rb

@@ -24,11 +24,9 @@ module Gitlab
     end
 
     def valid_custom_emoji?(record, value)
-      namespace = record.try(:awardable).try(:namespace)
+      resource = record.try(:resource_parent)
 
-      return unless namespace
-
-      namespace.custom_emoji&.by_name(value.to_s)&.any?
+      CustomEmoji.for_resource(resource).by_name(value.to_s).any?
     end
   end
 end

ee/spec/models/ee/award_emoji_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe AwardEmoji do
+  describe 'validations' do
+    context 'custom emoji' do
+      let_it_be(:user) { create(:user) }
+      let_it_be(:group) { create(:group) }
+      let_it_be(:emoji) { create(:custom_emoji, name: 'partyparrot', namespace: group) }
+
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'accepts custom emoji on epic' do
+        epic = create(:epic, group: group)
+        new_award = build(:award_emoji, user: user, awardable: epic, name: emoji.name)
+
+        expect(new_award).to be_valid
+      end
+
+      it 'accepts custom emoji on subgroup epic' do
+        subgroup = create(:group, parent: group)
+        epic = create(:epic, group: subgroup)
+        new_award = build(:award_emoji, user: user, awardable: epic, name: emoji.name)
+
+        expect(new_award).to be_valid
+      end
+    end
+  end
+end

lib/banzai/filter/custom_emoji_filter.rb

@@ -8,8 +8,7 @@ module Banzai
       IGNORED_ANCESTOR_TAGS = %w(pre code tt).to_set
 
       def call
-        return doc unless context[:project]
-        return doc unless Feature.enabled?(:custom_emoji, context[:project])
+        return doc unless resource_parent
 
         doc.xpath('descendant-or-self::text()').each do |node|
           content = node.to_html
@@ -50,12 +49,12 @@ module Banzai
 
       def has_custom_emoji?
         strong_memoize(:has_custom_emoji) do
-          namespace&.custom_emoji&.any?
+          CustomEmoji.for_resource(resource_parent).any?
         end
       end
 
-      def namespace
-        context[:project].namespace.root_ancestor
+      def resource_parent
+        context[:project] || context[:group]
       end
 
       def custom_emoji_candidates
@@ -63,7 +62,8 @@ module Banzai
       end
 
       def all_custom_emoji
-        @all_custom_emoji ||= namespace.custom_emoji.by_name(custom_emoji_candidates).index_by(&:name)
+        @all_custom_emoji ||=
+          CustomEmoji.for_resource(resource_parent).by_name(custom_emoji_candidates).index_by(&:name)
       end
     end
   end

spec/models/award_emoji_spec.rb

@@ -59,17 +59,41 @@ RSpec.describe AwardEmoji do
       end
     end
 
-    it 'accepts custom emoji' do
-      user  = create(:user)
-      group = create(:group)
-      group.add_maintainer(user)
+    context 'custom emoji' do
+      let_it_be(:user) { create(:user) }
+      let_it_be(:group) { create(:group) }
+      let_it_be(:emoji) { create(:custom_emoji, name: 'partyparrot', namespace: group) }
 
-      project = create(:project, namespace: group)
-      issue = create(:issue, project: project)
-      emoji = create(:custom_emoji, name: 'partyparrot', namespace: group)
-      new_award = build(:award_emoji, user: user, awardable: issue, name: emoji.name)
+      before do
+        group.add_maintainer(user)
+      end
+
+      %i[issue merge_request note_on_issue snippet].each do |awardable_type|
+        let_it_be(:project) { create(:project, namespace: group) }
+        let(:awardable) { create(awardable_type, project: project) }
+
+        it "is accepted on #{awardable_type}" do
+          new_award = build(:award_emoji, user: user, awardable: awardable, name: emoji.name)
 
-      expect(new_award).to be_valid
+          expect(new_award).to be_valid
+        end
+      end
+
+      it 'is accepted on subgroup issue' do
+        subgroup = create(:group, parent: group)
+        project = create(:project, namespace: subgroup)
+        issue = create(:issue, project: project)
+        new_award = build(:award_emoji, user: user, awardable: issue, name: emoji.name)
+
+        expect(new_award).to be_valid
+      end
+
+      it 'is not supported on personal snippet (yet)' do
+        snippet = create(:personal_snippet)
+        new_award = build(:award_emoji, user: snippet.author, awardable: snippet, name: 'null')
+
+        expect(new_award).not_to be_valid
+      end
     end
   end
 
@@ -223,4 +247,47 @@ RSpec.describe AwardEmoji do
       end
     end
   end
+
+  describe '#url' do
+    let_it_be(:custom_emoji) { create(:custom_emoji) }
+    let_it_be(:project) { create(:project, namespace: custom_emoji.group) }
+    let_it_be(:issue) { create(:issue, project: project) }
+
+    def build_award(name)
+      build(:award_emoji, awardable: issue, name: name)
+    end
+
+    it 'is nil for built-in emoji' do
+      new_award = build_award('tada')
+
+      count = ActiveRecord::QueryRecorder.new do
+        expect(new_award.url).to be_nil
+      end.count
+      expect(count).to be_zero
+    end
+
+    it 'is nil for unrecognized emoji' do
+      new_award = build_award('null')
+
+      expect(new_award.url).to be_nil
+    end
+
+    it 'is set for custom emoji' do
+      new_award = build_award(custom_emoji.name)
+
+      expect(new_award.url).to eq(custom_emoji.url)
+    end
+
+    context 'feature flag disabled' do
+      before do
+        stub_feature_flags(custom_emoji: false)
+      end
+
+      it 'does not query' do
+        new_award = build_award(custom_emoji.name)
+
+        expect(ActiveRecord::QueryRecorder.new { new_award.url }.count).to be_zero
+      end
+    end
+  end
 end