Merge branch 'tc-add-custom-emoji-creator' into 'master'

Add creator to custom emoji

See merge request gitlab-org/gitlab!53879

app/graphql/mutations/custom_emoji/create.rb

@@ -31,6 +31,7 @@ module Mutations
         group = authorized_find!(group_path: group_path)
         # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37911#note_444682238
         args[:external] = true
+        args[:creator] = current_user
 
         custom_emoji = group.custom_emoji.create(args)
 

app/models/custom_emoji.rb

@@ -6,6 +6,7 @@ class CustomEmoji < ApplicationRecord
   belongs_to :namespace, inverse_of: :custom_emoji
 
   belongs_to :group, -> { where(type: 'Group') }, foreign_key: 'namespace_id'
+  belongs_to :creator, class_name: "User", inverse_of: :created_custom_emoji
 
   # For now only external emoji are supported. See https://gitlab.com/gitlab-org/gitlab/-/issues/230467
   validates :external, inclusion: { in: [true] }
@@ -15,6 +16,7 @@ class CustomEmoji < ApplicationRecord
   validate :valid_emoji_name
 
   validates :group, presence: true
+  validates :creator, presence: true
   validates :name,
     uniqueness: { scope: [:namespace_id, :name] },
     presence: true,

app/models/user.rb

@@ -179,6 +179,7 @@ class User < ApplicationRecord
   has_many :merge_request_reviewers, inverse_of: :reviewer
   has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue
   has_many :assigned_merge_requests, class_name: "MergeRequest", through: :merge_request_assignees, source: :merge_request
+  has_many :created_custom_emoji, class_name: 'CustomEmoji', inverse_of: :creator
 
   has_many :bulk_imports
 

changelogs/unreleased/tc-add-custom-emoji-creator.yml

+---
+title: Add creator to custom emoji
+merge_request: 53879
+author:
+type: changed

db/migrate/20210205134213_add_creator_id_to_custom_emoji.rb

+# frozen_string_literal: true
+
+class AddCreatorIdToCustomEmoji < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    # Custom Emoji is at the moment behind a default-disabled feature flag. It
+    # will be unlikely there are any records in this table, but to able to
+    # ensure a not-null constraint delete any existing rows.
+    # Roll-out issue: https://gitlab.com/gitlab-org/gitlab/-/issues/231317
+    execute 'DELETE FROM custom_emoji'
+
+    add_reference :custom_emoji, # rubocop:disable Migration/AddReference
+                  :creator,
+                  index: true,
+                  null: false, # rubocop:disable Rails/NotNullColumn
+                  foreign_key: false # FK is added in 20210219100137
+  end
+
+  def down
+    remove_reference :custom_emoji, :creator
+  end
+end

db/migrate/20210219100137_add_creator_foreign_key_to_custom_emoji.rb

+# frozen_string_literal: true
+
+class AddCreatorForeignKeyToCustomEmoji < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  FK_NAME = 'fk_custom_emoji_creator_id'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :custom_emoji, :users,
+                               on_delete: :cascade,
+                               column: :creator_id,
+                               name: FK_NAME
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :custom_emoji, name: FK_NAME
+    end
+  end
+end

db/schema_migrations/20210205134213

+7c368cad497ccfd86c6a92e2edfec1d2a16879eb749184b1d20c5ab4c702b974
\ No newline at end of file

db/schema_migrations/20210219100137

+be2ddc15e16d7d59bd050a60faaa0b6941d94ba7c47a88be473bcf3a17bb2763
\ No newline at end of file

db/structure.sql

@@ -11609,6 +11609,7 @@ CREATE TABLE custom_emoji (
     name text NOT NULL,
     file text NOT NULL,
     external boolean DEFAULT true NOT NULL,
+    creator_id bigint NOT NULL,
     CONSTRAINT check_8c586dd507 CHECK ((char_length(name) <= 36)),
     CONSTRAINT check_dd5d60f1fb CHECK ((char_length(file) <= 255))
 );
@@ -21970,6 +21971,8 @@ CREATE INDEX index_csv_issue_imports_on_project_id ON csv_issue_imports USING bt
 
 CREATE INDEX index_csv_issue_imports_on_user_id ON csv_issue_imports USING btree (user_id);
 
+CREATE INDEX index_custom_emoji_on_creator_id ON custom_emoji USING btree (creator_id);
+
 CREATE UNIQUE INDEX index_custom_emoji_on_namespace_id_and_name ON custom_emoji USING btree (namespace_id, name);
 
 CREATE UNIQUE INDEX index_daily_build_group_report_results_unique_columns ON ci_daily_build_group_report_results USING btree (project_id, ref_path, date, group_name);
@@ -24689,6 +24692,9 @@ ALTER TABLE ONLY todos
 ALTER TABLE ONLY geo_event_log
     ADD CONSTRAINT fk_cff7185ad2 FOREIGN KEY (reset_checksum_event_id) REFERENCES geo_reset_checksum_events(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY custom_emoji
+    ADD CONSTRAINT fk_custom_emoji_creator_id FOREIGN KEY (creator_id) REFERENCES users(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY bulk_import_entities
     ADD CONSTRAINT fk_d06d023c30 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

spec/factories/custom_emoji.rb

@@ -6,5 +6,6 @@ FactoryBot.define do
     namespace
     group
     file { 'https://gitlab.com/images/partyparrot.png' }
+    creator { namespace.owner }
   end
 end

spec/graphql/mutations/custom_emoji/create_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::CustomEmoji::Create do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+  let(:args) { { group_path: group.full_path, name: 'tanuki', url: 'https://about.gitlab.com/images/press/logo/png/gitlab-icon-rgb.png' } }
+
+  before do
+    group.add_developer(user)
+  end
+
+  describe '#resolve' do
+    subject(:resolve) { described_class.new(object: nil, context: { current_user: user }, field: nil).resolve(**args) }
+
+    it 'creates the custom emoji' do
+      expect { resolve }.to change(CustomEmoji, :count).by(1)
+    end
+
+    it 'sets the creator to be the user who added the emoji' do
+      resolve
+
+      expect(CustomEmoji.last.creator).to eq(user)
+    end
+  end
+end

spec/models/custom_emoji_spec.rb

@@ -4,8 +4,10 @@ require 'spec_helper'
 
 RSpec.describe CustomEmoji do
   describe 'Associations' do
-    it { is_expected.to belong_to(:namespace) }
+    it { is_expected.to belong_to(:namespace).inverse_of(:custom_emoji) }
+    it { is_expected.to belong_to(:creator).inverse_of(:created_custom_emoji) }
     it { is_expected.to have_db_column(:file) }
+    it { is_expected.to validate_presence_of(:creator) }
     it { is_expected.to validate_length_of(:name).is_at_most(36) }
     it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to have_db_column(:external) }
@@ -36,7 +38,7 @@ RSpec.describe CustomEmoji do
       new_emoji = build(:custom_emoji, name: old_emoji.name, namespace: old_emoji.namespace, group: group)
 
       expect(new_emoji).not_to be_valid
-      expect(new_emoji.errors.messages).to eq(name: ["has already been taken"])
+      expect(new_emoji.errors.messages).to include(name: ["has already been taken"])
     end
 
     it 'disallows non http and https file value' do

spec/models/user_spec.rb

@@ -101,6 +101,7 @@ RSpec.describe User do
     it { is_expected.to have_many(:reviews).inverse_of(:author) }
     it { is_expected.to have_many(:merge_request_assignees).inverse_of(:assignee) }
     it { is_expected.to have_many(:merge_request_reviewers).inverse_of(:reviewer) }
+    it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) }
 
     describe "#user_detail" do
       it 'does not persist `user_detail` by default' do