From 7ee01e6b8e75de45e2c2d59f638237bcb2bc3db9 Mon Sep 17 00:00:00 2001 From: Phil Hughes <me@iamphill.com> Date: Tue, 30 Jan 2024 15:21:11 +0000 Subject: [PATCH] Removes custom_emoji feature flag Changelog: other https://gitlab.com/gitlab-org/gitlab/-/issues/231317 --- app/assets/javascripts/emoji/index.js | 2 +- .../groups/custom_emoji_controller.rb | 4 --- app/finders/award_emojis_finder.rb | 7 ---- app/finders/groups/custom_emoji_finder.rb | 2 -- app/graphql/mutations/custom_emoji/create.rb | 4 --- app/graphql/mutations/custom_emoji/destroy.rb | 4 --- app/helpers/groups_helper.rb | 1 - app/models/custom_emoji.rb | 2 +- .../development/custom_emoji.yml | 8 ----- doc/api/graphql/custom_emoji.md | 5 +-- doc/user/emoji_reactions.md | 6 +--- lib/gitlab/gon_helper.rb | 1 - spec/finders/award_emojis_finder_spec.rb | 16 --------- spec/frontend/emoji/index_spec.js | 12 ------- spec/helpers/groups_helper_spec.rb | 36 +++++++------------ spec/models/award_emoji_spec.rb | 12 ------- spec/models/custom_emoji_spec.rb | 8 ----- .../api/graphql/custom_emoji_query_spec.rb | 11 ------ .../mutations/custom_emoji/create_spec.rb | 14 -------- .../mutations/custom_emoji/destroy_spec.rb | 14 -------- .../groups/custom_emoji_controller_spec.rb | 18 ++-------- 21 files changed, 20 insertions(+), 167 deletions(-) delete mode 100644 config/feature_flags/development/custom_emoji.yml diff --git a/app/assets/javascripts/emoji/index.js b/app/assets/javascripts/emoji/index.js index c4279e9d8e7a0..810a2fe9c4e3a 100644 --- a/app/assets/javascripts/emoji/index.js +++ b/app/assets/javascripts/emoji/index.js @@ -79,7 +79,7 @@ async function loadEmojiWithNames() { export async function loadCustomEmojiWithNames() { const emojiData = { emojis: {}, names: [] }; - if (document.body?.dataset?.groupFullPath && window.gon?.features?.customEmoji) { + if (document.body?.dataset?.groupFullPath) { const client = createApolloClient(); const { data } = await client.query({ query: customEmojiQuery, diff --git a/app/controllers/groups/custom_emoji_controller.rb b/app/controllers/groups/custom_emoji_controller.rb index f202c9febbac9..150c4ed9c1e16 100644 --- a/app/controllers/groups/custom_emoji_controller.rb +++ b/app/controllers/groups/custom_emoji_controller.rb @@ -4,9 +4,5 @@ module Groups class CustomEmojiController < Groups::ApplicationController feature_category :code_review_workflow urgency :low - - before_action do - render_404 unless Feature.enabled?(:custom_emoji) - end end end diff --git a/app/finders/award_emojis_finder.rb b/app/finders/award_emojis_finder.rb index 2f34e6ec4d3f1..1ca7eb61bba11 100644 --- a/app/finders/award_emojis_finder.rb +++ b/app/finders/award_emojis_finder.rb @@ -33,16 +33,9 @@ def by_awarded_by(awards) def validate_params return unless params.present? - validate_name_param unless Feature.enabled?(:custom_emoji) validate_awarded_by_param end - def validate_name_param - return unless params[:name] - - raise ArgumentError, 'Invalid name param' unless TanukiEmoji.find_by_alpha_code(params[:name].to_s) - end - def validate_awarded_by_param return unless params[:awarded_by] diff --git a/app/finders/groups/custom_emoji_finder.rb b/app/finders/groups/custom_emoji_finder.rb index 80a4e948f8b42..77c11b8747597 100644 --- a/app/finders/groups/custom_emoji_finder.rb +++ b/app/finders/groups/custom_emoji_finder.rb @@ -12,8 +12,6 @@ def initialize(group, params = {}) end def execute - return CustomEmoji.none if Feature.disabled?(:custom_emoji, group) - return CustomEmoji.for_resource(group) unless params[:include_ancestor_groups] CustomEmoji.for_namespaces(group_ids_for(group)) diff --git a/app/graphql/mutations/custom_emoji/create.rb b/app/graphql/mutations/custom_emoji/create.rb index 535ff44a7fd2d..269ea6c999981 100644 --- a/app/graphql/mutations/custom_emoji/create.rb +++ b/app/graphql/mutations/custom_emoji/create.rb @@ -28,10 +28,6 @@ class Create < BaseMutation description: 'Location of the emoji file.' def resolve(group_path:, **args) - if Feature.disabled?(:custom_emoji) - raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Custom emoji feature is disabled' - end - group = authorized_find!(group_path: group_path) # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37911#note_444682238 args[:external] = true diff --git a/app/graphql/mutations/custom_emoji/destroy.rb b/app/graphql/mutations/custom_emoji/destroy.rb index 0dd989ad841e1..4a573e463722c 100644 --- a/app/graphql/mutations/custom_emoji/destroy.rb +++ b/app/graphql/mutations/custom_emoji/destroy.rb @@ -17,10 +17,6 @@ class Destroy < BaseMutation description: 'Global ID of the custom emoji to destroy.' def resolve(id:) - if Feature.disabled?(:custom_emoji) - raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'Custom emoji feature is disabled' - end - custom_emoji = authorized_find!(id: id) custom_emoji.destroy! diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 7db833edff52c..9e3b856e9565e 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -196,7 +196,6 @@ def enabled_git_access_protocol_options_for_group end def new_custom_emoji_path(group) - return unless Feature.enabled?(:custom_emoji) return unless group return unless can?(current_user, :create_custom_emoji, group) diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb index a3318cd0bd856..aea8e2881cb72 100644 --- a/app/models/custom_emoji.rb +++ b/app/models/custom_emoji.rb @@ -48,7 +48,7 @@ class CustomEmoji < ApplicationRecord alias_attribute :url, :file # this might need a change in https://gitlab.com/gitlab-org/gitlab/-/issues/230467 scope :for_resource, -> (resource) do - return none if resource.nil? || Feature.disabled?(:custom_emoji, resource) + return none if resource.nil? return none unless resource.is_a?(Group) resource.custom_emoji diff --git a/config/feature_flags/development/custom_emoji.yml b/config/feature_flags/development/custom_emoji.yml deleted file mode 100644 index 8644340d2e681..0000000000000 --- a/config/feature_flags/development/custom_emoji.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: custom_emoji -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37911 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/231317 -milestone: '13.6' -type: development -group: group::project management -default_enabled: true diff --git a/doc/api/graphql/custom_emoji.md b/doc/api/graphql/custom_emoji.md index a22f1538c75d7..62d836f2406a7 100644 --- a/doc/api/graphql/custom_emoji.md +++ b/doc/api/graphql/custom_emoji.md @@ -13,10 +13,7 @@ DETAILS: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37911) in GitLab 13.6 [with a flag](../../administration/feature_flags.md) named `custom_emoji`. Disabled by default. > - Enabled on GitLab.com in GitLab 14.0. > - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138969) in GitLab 16.7. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, an administrator can [disable the feature flag](../../administration/feature_flags.md) named `custom_emoji`. -This feature is ready for production use. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/) in GitLab 16.9. Feature flag `custom_emoji` removed. To use [custom emoji](../../user/emoji_reactions.md) in comments and descriptions, you can add them to a top-level group using the GraphQL API. diff --git a/doc/user/emoji_reactions.md b/doc/user/emoji_reactions.md index 2f20640d4ebe6..983a097d88355 100644 --- a/doc/user/emoji_reactions.md +++ b/doc/user/emoji_reactions.md @@ -52,11 +52,7 @@ To remove an emoji reaction, select the emoji again. > - Enabled on GitLab.com in GitLab 14.0. > - UI to add emoji [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/333095) in GitLab 16.2. > - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138969) in GitLab 16.7. - -FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, an administrator can [disable the feature flag](../administration/feature_flags.md) named `custom_emoji`. -On GitLab.com, this feature is available. -This feature is ready for production use. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/) in GitLab 16.9. Feature flag `custom_emoji` removed. Custom emoji show in the emoji picker everywhere you can react with emoji. diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 707f5b1ef090e..3e75ada1f7fe1 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -79,7 +79,6 @@ def add_gon_variables push_frontend_feature_flag(:ui_for_organizations, current_user) # To be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/399248 push_frontend_feature_flag(:remove_monitor_metrics) - push_frontend_feature_flag(:custom_emoji) push_frontend_feature_flag(:encoding_logs_tree) push_frontend_feature_flag(:group_user_saml) end diff --git a/spec/finders/award_emojis_finder_spec.rb b/spec/finders/award_emojis_finder_spec.rb index 5c016d9e17754..5de5299beb5c7 100644 --- a/spec/finders/award_emojis_finder_spec.rb +++ b/spec/finders/award_emojis_finder_spec.rb @@ -12,23 +12,7 @@ let_it_be(:issue_2_thumbsup) { create(:award_emoji, name: 'thumbsup', awardable: issue_2) } let_it_be(:issue_2_thumbsdown) { create(:award_emoji, name: 'thumbsdown', awardable: issue_2) } - before do - stub_feature_flags(custom_emoji: false) - end - describe 'param validation' do - it 'raises an error if `name` is invalid' do - expect { described_class.new(issue_1, { name: 'invalid' }).execute }.to raise_error( - ArgumentError, - 'Invalid name param' - ) - end - - it 'does not raise an error if `name` is numeric' do - subject = described_class.new(issue_1, { name: 100 }) - expect { subject.execute }.not_to raise_error - end - it 'raises an error if `awarded_by` is invalid' do expectation = [ArgumentError, 'Invalid awarded_by param'] diff --git a/spec/frontend/emoji/index_spec.js b/spec/frontend/emoji/index_spec.js index 577b7bc726ea6..fd3d503c01e32 100644 --- a/spec/frontend/emoji/index_spec.js +++ b/spec/frontend/emoji/index_spec.js @@ -920,18 +920,6 @@ describe('emoji', () => { createMockEmojiClient(); }); - describe('flag disabled', () => { - beforeEach(() => { - window.gon = {}; - }); - - it('returns empty emoji data', async () => { - const result = await loadCustomEmojiWithNames(); - - expect(result).toEqual({ emojis: {}, names: [] }); - }); - }); - describe('when not in a group', () => { beforeEach(() => { delete document.body.dataset.groupFullPath; diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 6c4595ce4bb0a..d73fb0f2d4c6d 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -569,38 +569,28 @@ let_it_be(:group) { create(:group) } - context 'with feature flag disabled' do - before do - stub_feature_flags(custom_emoji: false) - end + context 'with nil group' do + let(:group) { nil } it { is_expected.to eq(nil) } end - context 'with feature flag enabled' do - context 'with nil group' do - let(:group) { nil } - - it { is_expected.to eq(nil) } + context 'with current_user who has no permissions' do + before do + allow(helper).to receive(:current_user).and_return(create(:user)) end - context 'with current_user who has no permissions' do - before do - allow(helper).to receive(:current_user).and_return(create(:user)) - end + it { is_expected.to eq(nil) } + end - it { is_expected.to eq(nil) } + context 'with current_user who has permissions' do + before do + user = create(:user) + group.add_owner(user) + allow(helper).to receive(:current_user).and_return(user) end - context 'with current_user who has permissions' do - before do - user = create(:user) - group.add_owner(user) - allow(helper).to receive(:current_user).and_return(user) - end - - it { is_expected.to eq(new_group_custom_emoji_path(group)) } - end + it { is_expected.to eq(new_group_custom_emoji_path(group)) } end end diff --git a/spec/models/award_emoji_spec.rb b/spec/models/award_emoji_spec.rb index a901453ba9fbe..542b1bae3eee4 100644 --- a/spec/models/award_emoji_spec.rb +++ b/spec/models/award_emoji_spec.rb @@ -329,18 +329,6 @@ def build_award(name) expect(new_award.url).to eq(custom_emoji.url) end 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 describe '#to_ability_name' do diff --git a/spec/models/custom_emoji_spec.rb b/spec/models/custom_emoji_spec.rb index cbdf05cf28f29..52b36d980497b 100644 --- a/spec/models/custom_emoji_spec.rb +++ b/spec/models/custom_emoji_spec.rb @@ -53,14 +53,6 @@ let_it_be(:group) { create(:group) } let_it_be(:custom_emoji) { create(:custom_emoji, namespace: group) } - context 'when custom_emoji feature flag is disabled' do - before do - stub_feature_flags(custom_emoji: false) - end - - it { expect(described_class.for_resource(group)).to eq([]) } - end - context 'when group is nil' do let_it_be(:group) { nil } diff --git a/spec/requests/api/graphql/custom_emoji_query_spec.rb b/spec/requests/api/graphql/custom_emoji_query_spec.rb index c89ad0002b477..7de5965f9534d 100644 --- a/spec/requests/api/graphql/custom_emoji_query_spec.rb +++ b/spec/requests/api/graphql/custom_emoji_query_spec.rb @@ -10,7 +10,6 @@ let_it_be(:custom_emoji) { create(:custom_emoji, group: group) } before do - stub_feature_flags(custom_emoji: true) group.add_developer(current_user) end @@ -35,16 +34,6 @@ def custom_emoji_query(group) expect(graphql_data['group']['customEmoji']['nodes'].first['name']).to eq(custom_emoji.name) end - it 'returns empty array when the custom_emoji feature flag is disabled' do - stub_feature_flags(custom_emoji: false) - - post_graphql(custom_emoji_query(group), current_user: current_user) - - expect(response).to have_gitlab_http_status(:ok) - expect(graphql_data['group']).to be_present - expect(graphql_data['group']['customEmoji']['nodes']).to eq([]) - end - it 'returns nil group when unauthorised' do user = create(:user) post_graphql(custom_emoji_query(group), current_user: user) diff --git a/spec/requests/api/graphql/mutations/custom_emoji/create_spec.rb b/spec/requests/api/graphql/mutations/custom_emoji/create_spec.rb index 19a52086f3482..32eb89709f187 100644 --- a/spec/requests/api/graphql/mutations/custom_emoji/create_spec.rb +++ b/spec/requests/api/graphql/mutations/custom_emoji/create_spec.rb @@ -39,19 +39,5 @@ expect(gql_response['customEmoji']['name']).to eq(attributes[:name]) expect(gql_response['customEmoji']['url']).to eq(attributes[:url]) end - - context 'when the custom_emoji feature flag is disabled' do - before do - stub_feature_flags(custom_emoji: false) - end - - it 'does nothing and returns and error' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - end.to not_change(CustomEmoji, :count) - - expect_graphql_errors_to_include('Custom emoji feature is disabled') - end - end end end diff --git a/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb b/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb index 2623d3d84105a..7e9c7d6571fea 100644 --- a/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb +++ b/spec/requests/api/graphql/mutations/custom_emoji/destroy_spec.rb @@ -68,20 +68,6 @@ end it_behaves_like 'deletes custom emoji' - - context 'when the custom_emoji feature flag is disabled' do - before do - stub_feature_flags(custom_emoji: false) - end - - it_behaves_like 'does not delete custom emoji' - - it 'returns an error' do - post_graphql_mutation(mutation, current_user: current_user) - - expect_graphql_errors_to_include('Custom emoji feature is disabled') - end - end end end end diff --git a/spec/requests/groups/custom_emoji_controller_spec.rb b/spec/requests/groups/custom_emoji_controller_spec.rb index d12cd8e42ac6f..96b6f54eeb3e8 100644 --- a/spec/requests/groups/custom_emoji_controller_spec.rb +++ b/spec/requests/groups/custom_emoji_controller_spec.rb @@ -6,22 +6,10 @@ let_it_be(:group) { create(:group) } describe 'GET #index' do - context 'with custom_emoji feature flag disabled' do - before do - stub_feature_flags(custom_emoji: false) - - get group_custom_emoji_index_url(group) - end - - it { expect(response).to have_gitlab_http_status(:not_found) } + before do + get group_custom_emoji_index_url(group) end - context 'with custom_emoji feature flag enabled' do - before do - get group_custom_emoji_index_url(group) - end - - it { expect(response).to have_gitlab_http_status(:ok) } - end + it { expect(response).to have_gitlab_http_status(:ok) } end end -- GitLab