From 2ff2e8f4c6e949d8a4dae353e056708923271e43 Mon Sep 17 00:00:00 2001 From: Vasilii Iakliushin Date: Wed, 26 Feb 2025 13:18:37 +0100 Subject: [PATCH] Soft deprecate widgets responses **Problem** `commits_without_merge_commits` and `default_merge_commit_message` fields require access to commit messages. But these fields are not used by the frontend anymore. **Solution** Make both fields to return empty values when the feature flag "disable_widget_responses" is enabled. --- ...merge_request_poll_cached_widget_entity.rb | 6 ++++- .../merge_request_poll_widget_entity.rb | 6 ++++- .../disable_widget_responses.yml | 9 ++++++++ ..._request_poll_cached_widget_entity_spec.rb | 23 ++++++++++++++----- .../merge_request_poll_widget_entity_spec.rb | 18 ++++++++++++--- 5 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/disable_widget_responses.yml diff --git a/app/serializers/merge_request_poll_cached_widget_entity.rb b/app/serializers/merge_request_poll_cached_widget_entity.rb index df119d8516b795..a246caf0faf932 100644 --- a/app/serializers/merge_request_poll_cached_widget_entity.rb +++ b/app/serializers/merge_request_poll_cached_widget_entity.rb @@ -29,7 +29,11 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity end expose :commits_without_merge_commits, using: MergeRequestWidgetCommitEntity do |merge_request| - merge_request.recent_commits.without_merge_commits + if Feature.enabled?(:disable_widget_responses, merge_request.target_project) + [] + else + merge_request.recent_commits.without_merge_commits + end end expose :diff_head_sha do |merge_request| diff --git a/app/serializers/merge_request_poll_widget_entity.rb b/app/serializers/merge_request_poll_widget_entity.rb index 3617713d03d70b..eeac341aa72cb6 100644 --- a/app/serializers/merge_request_poll_widget_entity.rb +++ b/app/serializers/merge_request_poll_widget_entity.rb @@ -29,7 +29,11 @@ class MergeRequestPollWidgetEntity < Grape::Entity end expose :default_merge_commit_message_with_description do |merge_request| - merge_request.default_merge_commit_message(include_description: true) + if Feature.enabled?(:disable_widget_responses, merge_request.target_project) + '' + else + merge_request.default_merge_commit_message(include_description: true) + end end expose :only_allow_merge_if_pipeline_succeeds do |merge_request| diff --git a/config/feature_flags/gitlab_com_derisk/disable_widget_responses.yml b/config/feature_flags/gitlab_com_derisk/disable_widget_responses.yml new file mode 100644 index 00000000000000..7292300c20186e --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/disable_widget_responses.yml @@ -0,0 +1,9 @@ +--- +name: disable_widget_responses +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/520302 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/182725 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/521327 +milestone: '17.10' +group: group::source code +type: gitlab_com_derisk +default_enabled: false diff --git a/spec/serializers/merge_request_poll_cached_widget_entity_spec.rb b/spec/serializers/merge_request_poll_cached_widget_entity_spec.rb index 458d9ecd916773..da20e56f2922cc 100644 --- a/spec/serializers/merge_request_poll_cached_widget_entity_spec.rb +++ b/spec/serializers/merge_request_poll_cached_widget_entity_spec.rb @@ -174,13 +174,23 @@ def find_matching_commit(short_id) resource.commits.find { |c| c.short_id == short_id } end - it 'does not include merge commits' do - commits_in_widget = subject[:commits_without_merge_commits] + it 'returns empty array' do + expect(subject[:commits_without_merge_commits]).to eq([]) + end + + context 'when "disable_widget_responses" is disabled' do + before do + stub_feature_flags(disable_widget_responses: false) + end + + it 'does not include merge commits' do + commits_in_widget = subject[:commits_without_merge_commits] - expect(commits_in_widget.length).to be < resource.commits.length - expect(commits_in_widget.length).to eq(resource.commits.without_merge_commits.length) - commits_in_widget.each do |c| - expect(find_matching_commit(c[:short_id]).merge_commit?).to eq(false) + expect(commits_in_widget.length).to be < resource.commits.length + expect(commits_in_widget.length).to eq(resource.commits.without_merge_commits.length) + commits_in_widget.each do |c| + expect(find_matching_commit(c[:short_id]).merge_commit?).to eq(false) + end end end end @@ -210,6 +220,7 @@ def find_matching_commit(short_id) context 'when merge request is mergeable' do before do stub_const('MergeRequestDiff::COMMITS_SAFE_SIZE', 20) + stub_feature_flags(disable_widget_responses: false) end it 'has default_squash_commit_message and commits_without_merge_commits' do diff --git a/spec/serializers/merge_request_poll_widget_entity_spec.rb b/spec/serializers/merge_request_poll_widget_entity_spec.rb index 7101f465de4a71..15388937676949 100644 --- a/spec/serializers/merge_request_poll_widget_entity_spec.rb +++ b/spec/serializers/merge_request_poll_widget_entity_spec.rb @@ -17,9 +17,21 @@ described_class.new(resource, { request: request }.merge(options)).as_json end - it 'has default_merge_commit_message_with_description' do - expect(subject[:default_merge_commit_message_with_description]) - .to eq(resource.default_merge_commit_message(include_description: true)) + describe '#default_merge_commit_message_with_description' do + it 'returns empty string' do + expect(subject[:default_merge_commit_message_with_description]).to eq('') + end + + context 'when "disable_widget_responses" is disabled' do + before do + stub_feature_flags(disable_widget_responses: false) + end + + it 'has default_merge_commit_message_with_description' do + expect(subject[:default_merge_commit_message_with_description]) + .to eq(resource.default_merge_commit_message(include_description: true)) + end + end end it { is_expected.to include(ff_only_enabled: false) } -- GitLab