Commit 69065bae authored by Alex Pooley's avatar Alex Pooley

Merge branch '320757-remove-context-commits-ff' into 'master'

Remove context_commits feature flag

See merge request gitlab-org/gitlab!80881
parents ba2015cd 960ccee0
...@@ -111,9 +111,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic ...@@ -111,9 +111,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
allow_tree_conflicts: display_merge_conflicts_in_diff? allow_tree_conflicts: display_merge_conflicts_in_diff?
) )
if @merge_request.project.context_commits_enabled? options[:context_commits] = @merge_request.recent_context_commits
options[:context_commits] = @merge_request.recent_context_commits
end
render json: DiffsSerializer.new(request).represent(diffs, options) render json: DiffsSerializer.new(request).represent(diffs, options)
end end
......
...@@ -219,8 +219,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -219,8 +219,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end end
def context_commits def context_commits
return render_404 unless project.context_commits_enabled?
# Get commits from repository # Get commits from repository
# or from cache if already merged # or from cache if already merged
commits = ContextCommitsFinder.new(project, @merge_request, { search: params[:search], limit: params[:limit], offset: params[:offset] }).execute commits = ContextCommitsFinder.new(project, @merge_request, { search: params[:search], limit: params[:limit], offset: params[:offset] }).execute
......
...@@ -1012,10 +1012,6 @@ class Project < ApplicationRecord ...@@ -1012,10 +1012,6 @@ class Project < ApplicationRecord
Feature.enabled?(:unlink_fork_network_upon_visibility_decrease, self, default_enabled: true) Feature.enabled?(:unlink_fork_network_upon_visibility_decrease, self, default_enabled: true)
end end
def context_commits_enabled?
Feature.enabled?(:context_commits, self.group, default_enabled: :yaml)
end
# LFS and hashed repository storage are required for using Design Management. # LFS and hashed repository storage are required for using Design Management.
def design_management_enabled? def design_management_enabled?
lfs_enabled? && hashed_storage?(:repository) lfs_enabled? && hashed_storage?(:repository)
......
...@@ -23,7 +23,7 @@ class DiffsEntity < Grape::Entity ...@@ -23,7 +23,7 @@ class DiffsEntity < Grape::Entity
CommitEntity.represent(options[:commit], commit_options(options)) CommitEntity.represent(options[:commit], commit_options(options))
end end
expose :context_commits, using: API::Entities::Commit, if: -> (diffs, options) { merge_request&.project&.context_commits_enabled? } do |diffs| expose :context_commits, using: API::Entities::Commit do |diffs|
options[:context_commits] options[:context_commits]
end end
...@@ -89,7 +89,7 @@ class DiffsEntity < Grape::Entity ...@@ -89,7 +89,7 @@ class DiffsEntity < Grape::Entity
project_blob_path(merge_request.project, merge_request.diff_head_sha) project_blob_path(merge_request.project, merge_request.diff_head_sha)
end end
expose :context_commits_diff, if: -> (_) { merge_request&.project&.context_commits_enabled? } do |diffs, options| expose :context_commits_diff do |diffs, options|
next unless merge_request.context_commits_diff.commits_count > 0 next unless merge_request.context_commits_diff.commits_count > 0
ContextCommitsDiffEntity.represent( ContextCommitsDiffEntity.represent(
......
...@@ -22,7 +22,7 @@ ...@@ -22,7 +22,7 @@
- if context_commits.present? - if context_commits.present?
%li.commit-header.js-commit-header %li.commit-header.js-commit-header
%span.font-weight-bold= n_("%d previously merged commit", "%d previously merged commits", context_commits.count) % context_commits.count %span.font-weight-bold= n_("%d previously merged commit", "%d previously merged commits", context_commits.count) % context_commits.count
- if project.context_commits_enabled? && can_update_merge_request - if can_update_merge_request
%button.gl-button.btn.btn-default.ml-3.add-review-item-modal-trigger{ type: "button", data: { context_commits_empty: 'false' } } %button.gl-button.btn.btn-default.ml-3.add-review-item-modal-trigger{ type: "button", data: { context_commits_empty: 'false' } }
= _('Add/remove') = _('Add/remove')
...@@ -40,7 +40,7 @@ ...@@ -40,7 +40,7 @@
.gl-alert-content .gl-alert-content
= n_('%s additional commit has been omitted to prevent performance issues.', '%s additional commits have been omitted to prevent performance issues.', hidden) % number_with_delimiter(hidden) = n_('%s additional commit has been omitted to prevent performance issues.', '%s additional commits have been omitted to prevent performance issues.', hidden) % number_with_delimiter(hidden)
- if project.context_commits_enabled? && can_update_merge_request && context_commits&.empty? - if can_update_merge_request && context_commits&.empty?
%button.gl-button.btn.btn-default.mt-3.add-review-item-modal-trigger{ type: "button", data: { context_commits_empty: 'true' } } %button.gl-button.btn.btn-default.mt-3.add-review-item-modal-trigger{ type: "button", data: { context_commits_empty: 'true' } }
= _('Add previously merged commits') = _('Add previously merged commits')
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
= custom_icon ('illustration_no_commits') = custom_icon ('illustration_no_commits')
%h4 %h4
= _('There are no commits yet.') = _('There are no commits yet.')
- if @project&.context_commits_enabled? && can_update_merge_request - if can_update_merge_request
%p %p
= _('Push commits to the source branch or add previously merged commits to review them.') = _('Push commits to the source branch or add previously merged commits to review them.')
%button.btn.gl-button.btn-confirm.add-review-item-modal-trigger{ type: "button", data: { commits_empty: 'true', context_commits_empty: 'true' } } %button.btn.gl-button.btn-confirm.add-review-item-modal-trigger{ type: "button", data: { commits_empty: 'true', context_commits_empty: 'true' } }
...@@ -14,5 +14,5 @@ ...@@ -14,5 +14,5 @@
%ol#commits-list.list-unstyled %ol#commits-list.list-unstyled
= render "projects/commits/commits", merge_request: @merge_request = render "projects/commits/commits", merge_request: @merge_request
- if @project&.context_commits_enabled? && can_update_merge_request && @merge_request.iid - if can_update_merge_request && @merge_request.iid
.add-review-item-modal-wrapper{ data: { context_commits_path: context_commits_project_json_merge_request_url(@merge_request&.project, @merge_request, :json), target_branch: @merge_request.target_branch, merge_request_iid: @merge_request.iid, project_id: @merge_request.project.id } } .add-review-item-modal-wrapper{ data: { context_commits_path: context_commits_project_json_merge_request_url(@merge_request&.project, @merge_request, :json), target_branch: @merge_request.target_branch, merge_request_iid: @merge_request.iid, project_id: @merge_request.project.id } }
---
name: context_commits
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23701
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320757
milestone: '12.8'
type: development
group: group::code review
default_enabled: true
...@@ -31,15 +31,7 @@ To seamlessly navigate among commits in a merge request: ...@@ -31,15 +31,7 @@ To seamlessly navigate among commits in a merge request:
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29274) in GitLab 13.12 [with a flag](../../../administration/feature_flags.md) named `context_commits`. Enabled by default. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/29274) in GitLab 13.12 [with a flag](../../../administration/feature_flags.md) named `context_commits`. Enabled by default.
> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/320757) in GitLab 14.8. > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/320757) in GitLab 14.8.
> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/320757) in GitLab 14.9. [Feature flag `context_commits`](https://gitlab.com/gitlab-org/gitlab/-/issues/320757) removed.
WARNING:
This feature is in [beta](../../../policy/alpha-beta-support.md#beta-features)
and is [incomplete](https://gitlab.com/groups/gitlab-org/-/epics/1192).
FLAG:
On self-managed GitLab, by default this feature is available. To hide the feature,
ask an administrator to [disable the feature flag](../../../administration/feature_flags.md) named `context_commits`.
On GitLab.com, this feature is available.
When reviewing a merge request, it helps to have more context about the changes When reviewing a merge request, it helps to have more context about the changes
made. That includes unchanged lines in unchanged files, and previous commits made. That includes unchanged lines in unchanged files, and previous commits
......
...@@ -304,10 +304,6 @@ module API ...@@ -304,10 +304,6 @@ module API
end end
get ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review, urgency: :high do get ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review, urgency: :high do
merge_request = find_merge_request_with_access(params[:merge_request_iid]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
project = merge_request.project
not_found! unless project.context_commits_enabled?
context_commits = context_commits =
paginate(merge_request.merge_request_context_commits).map(&:to_commit) paginate(merge_request.merge_request_context_commits).map(&:to_commit)
...@@ -328,9 +324,6 @@ module API ...@@ -328,9 +324,6 @@ module API
end end
merge_request = find_merge_request_with_access(params[:merge_request_iid]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
project = merge_request.project
not_found! unless project.context_commits_enabled?
authorize!(:update_merge_request, merge_request) authorize!(:update_merge_request, merge_request)
...@@ -351,9 +344,6 @@ module API ...@@ -351,9 +344,6 @@ module API
delete ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review do delete ':id/merge_requests/:merge_request_iid/context_commits', feature_category: :code_review do
commit_ids = params[:commits] commit_ids = params[:commits]
merge_request = find_merge_request_with_access(params[:merge_request_iid]) merge_request = find_merge_request_with_access(params[:merge_request_iid])
project = merge_request.project
not_found! unless project.context_commits_enabled?
authorize!(:destroy_merge_request, merge_request) authorize!(:destroy_merge_request, merge_request)
project = merge_request.target_project project = merge_request.target_project
......
...@@ -7991,47 +7991,6 @@ RSpec.describe Project, factory_default: :keep do ...@@ -7991,47 +7991,6 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
describe '#context_commits_enabled?' do
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, namespace: group) }
subject(:result) { project.context_commits_enabled? }
context 'when context_commits feature flag is enabled' do
before do
stub_feature_flags(context_commits: true)
end
it { is_expected.to be_truthy }
end
context 'when context_commits feature flag is disabled' do
before do
stub_feature_flags(context_commits: false)
end
it { is_expected.to be_falsey }
end
context 'when context_commits feature flag is enabled on project group' do
before do
stub_feature_flags(context_commits: group)
end
it { is_expected.to be_truthy }
end
context 'when context_commits feature flag is enabled on another group' do
let(:another_group) { create(:group) }
before do
stub_feature_flags(context_commits: another_group)
end
it { is_expected.to be_falsey }
end
end
describe '.not_hidden' do describe '.not_hidden' do
it 'lists projects that are not hidden' do it 'lists projects that are not hidden' do
project = create(:project) project = create(:project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment