Commit 55d586db authored by Marcia Ramos's avatar Marcia Ramos

Merge branch 'remove-merge-request-reviewer-ff' into 'master'

[RUN AS-IF-FOSS] Remove merge_request_reviewers feature flag [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!52468
parents e6019b97 690c47d0
...@@ -12,7 +12,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap ...@@ -12,7 +12,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
before_action :build_merge_request, except: [:create] before_action :build_merge_request, except: [:create]
before_action do before_action do
push_frontend_feature_flag(:merge_request_reviewers, @project, default_enabled: true)
push_frontend_feature_flag(:mr_collapsed_approval_rules, @project) push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
push_frontend_feature_flag(:reviewer_approval_rules, @project, default_enabled: :yaml) push_frontend_feature_flag(:reviewer_approval_rules, @project, default_enabled: :yaml)
end end
......
...@@ -52,7 +52,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -52,7 +52,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end end
before_action do before_action do
push_frontend_feature_flag(:merge_request_reviewers, @project, default_enabled: true)
push_frontend_feature_flag(:mr_collapsed_approval_rules, @project) push_frontend_feature_flag(:mr_collapsed_approval_rules, @project)
push_frontend_feature_flag(:reviewer_approval_rules, @project, default_enabled: :yaml) push_frontend_feature_flag(:reviewer_approval_rules, @project, default_enabled: :yaml)
end end
......
...@@ -243,7 +243,7 @@ module Types ...@@ -243,7 +243,7 @@ module Types
end end
def reviewers def reviewers
object.reviewers if object.allows_reviewers? object.reviewers
end end
end end
end end
......
...@@ -174,15 +174,9 @@ module MergeRequestsHelper ...@@ -174,15 +174,9 @@ module MergeRequestsHelper
end end
end end
def merge_request_reviewers_enabled?
Feature.enabled?(:merge_request_reviewers, default_enabled: :yaml)
end
private private
def review_requested_merge_requests_count def review_requested_merge_requests_count
return 0 unless merge_request_reviewers_enabled?
current_user.review_requested_open_merge_requests_count current_user.review_requested_open_merge_requests_count
end end
......
...@@ -1784,7 +1784,7 @@ class MergeRequest < ApplicationRecord ...@@ -1784,7 +1784,7 @@ class MergeRequest < ApplicationRecord
end end
def allows_reviewers? def allows_reviewers?
Feature.enabled?(:merge_request_reviewers, project, default_enabled: true) true
end end
def allows_multiple_reviewers? def allows_multiple_reviewers?
......
...@@ -10,7 +10,7 @@ class MergeRequestBasicEntity < Grape::Entity ...@@ -10,7 +10,7 @@ class MergeRequestBasicEntity < Grape::Entity
expose :milestone, using: API::Entities::Milestone expose :milestone, using: API::Entities::Milestone
expose :labels, using: LabelEntity expose :labels, using: LabelEntity
expose :assignees, using: API::Entities::UserBasic expose :assignees, using: API::Entities::UserBasic
expose :reviewers, if: -> (m) { m.allows_reviewers? }, using: API::Entities::UserBasic expose :reviewers, using: API::Entities::UserBasic
expose :task_status, :task_status_short expose :task_status, :task_status_short
expose :lock_version, :lock_version expose :lock_version, :lock_version
end end
...@@ -5,7 +5,7 @@ class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity ...@@ -5,7 +5,7 @@ class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity
MergeRequestUserEntity.represent(merge_request.assignees, options.merge(merge_request: merge_request)) MergeRequestUserEntity.represent(merge_request.assignees, options.merge(merge_request: merge_request))
end end
expose :reviewers, if: -> (m) { m.allows_reviewers? } do |merge_request, options| expose :reviewers do |merge_request, options|
MergeRequestUserEntity.represent(merge_request.reviewers, options.merge(merge_request: merge_request)) MergeRequestUserEntity.represent(merge_request.reviewers, options.merge(merge_request: merge_request))
end end
end end
...@@ -108,7 +108,7 @@ module MergeRequests ...@@ -108,7 +108,7 @@ module MergeRequests
def filter_reviewer(merge_request) def filter_reviewer(merge_request)
return if params[:reviewer_ids].blank? return if params[:reviewer_ids].blank?
unless can_admin_issuable?(merge_request) && merge_request.allows_reviewers? unless can_admin_issuable?(merge_request)
params.delete(:reviewer_ids) params.delete(:reviewer_ids)
return return
......
...@@ -47,11 +47,10 @@ ...@@ -47,11 +47,10 @@
%span.badge.badge-pill.issues-count.green-badge{ class: ('hidden' if issues_count == 0) } %span.badge.badge-pill.issues-count.green-badge{ class: ('hidden' if issues_count == 0) }
= number_with_delimiter(issues_count) = number_with_delimiter(issues_count)
- if header_link?(:merge_requests) - if header_link?(:merge_requests)
- reviewers_enabled = merge_request_reviewers_enabled? = nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter dropdown" }) do
= nav_link(path: 'dashboard#merge_requests', html_options: { class: "user-counter #{reviewers_enabled ? 'dropdown' : ''}" }) do
= link_to assigned_mrs_dashboard_path, class: 'dashboard-shortcuts-merge_requests', title: _('Merge requests'), aria: { label: _('Merge requests') }, = link_to assigned_mrs_dashboard_path, class: 'dashboard-shortcuts-merge_requests', title: _('Merge requests'), aria: { label: _('Merge requests') },
data: { qa_selector: 'merge_requests_shortcut_button', data: { qa_selector: 'merge_requests_shortcut_button',
toggle: reviewers_enabled ? "dropdown" : "tooltip", toggle: "dropdown",
placement: 'bottom', placement: 'bottom',
track_label: 'main_navigation', track_label: 'main_navigation',
track_event: 'click_merge_link', track_event: 'click_merge_link',
...@@ -60,9 +59,7 @@ ...@@ -60,9 +59,7 @@
= sprite_icon('git-merge') = sprite_icon('git-merge')
%span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if user_merge_requests_counts[:total] == 0) } %span.badge.badge-pill.merge-requests-count.js-merge-requests-count{ class: ('hidden' if user_merge_requests_counts[:total] == 0) }
= number_with_delimiter(user_merge_requests_counts[:total]) = number_with_delimiter(user_merge_requests_counts[:total])
- if reviewers_enabled
= sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0!') = sprite_icon('chevron-down', css_class: 'caret-down gl-mx-0!')
- if reviewers_enabled
.dropdown-menu.dropdown-menu-right .dropdown-menu.dropdown-menu-right
%ul %ul
%li.dropdown-header %li.dropdown-header
......
...@@ -55,7 +55,7 @@ ...@@ -55,7 +55,7 @@
- if merge_request.assignees.any? - if merge_request.assignees.any?
%li.gl-display-flex.gl-align-items-center %li.gl-display-flex.gl-align-items-center
= render 'shared/issuable/assignees', project: merge_request.project, issuable: merge_request = render 'shared/issuable/assignees', project: merge_request.project, issuable: merge_request
- if Feature.enabled?(:merge_request_reviewers, @project, default_enabled: true) && merge_request.reviewers.any? - if merge_request.reviewers.any?
%li.gl-display-flex.issuable-reviewers %li.gl-display-flex.issuable-reviewers
= render 'shared/issuable/reviewers', project: merge_request.project, issuable: merge_request = render 'shared/issuable/reviewers', project: merge_request.project, issuable: merge_request
= render 'projects/merge_requests/approvals_count', merge_request: merge_request = render 'projects/merge_requests/approvals_count', merge_request: merge_request
......
...@@ -26,7 +26,7 @@ ...@@ -26,7 +26,7 @@
.block.assignee.qa-assignee-block .block.assignee.qa-assignee-block
= render "shared/issuable/sidebar_assignees", issuable_sidebar: issuable_sidebar, assignees: assignees, signed_in: signed_in = render "shared/issuable/sidebar_assignees", issuable_sidebar: issuable_sidebar, assignees: assignees, signed_in: signed_in
- if Feature.enabled?(:merge_request_reviewers, @project, default_enabled: true) && reviewers - if reviewers
.block.reviewer.qa-reviewer-block .block.reviewer.qa-reviewer-block
= render "shared/issuable/sidebar_reviewers", issuable_sidebar: issuable_sidebar, reviewers: reviewers, signed_in: signed_in = render "shared/issuable/sidebar_reviewers", issuable_sidebar: issuable_sidebar, reviewers: reviewers, signed_in: signed_in
......
---
title: Remove merge_request_reviewers feature flag
merge_request: 52468
author:
type: removed
---
name: merge_request_reviewers
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40488
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/245190
milestone: '13.4'
type: development
group: group::code review
default_enabled: true
...@@ -454,10 +454,10 @@ Parameters: ...@@ -454,10 +454,10 @@ Parameters:
| `author_id` | integer | no | Returns merge requests created by the given user `id`. Mutually exclusive with `author_username`. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13060) in GitLab 9.5)_. | | `author_id` | integer | no | Returns merge requests created by the given user `id`. Mutually exclusive with `author_username`. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13060) in GitLab 9.5)_. |
| `author_username` | string | no | Returns merge requests created by the given `username`. Mutually exclusive with `author_id`. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13060) in GitLab 12.10)_. | | `author_username` | string | no | Returns merge requests created by the given `username`. Mutually exclusive with `author_id`. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13060) in GitLab 12.10)_. |
| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13060) in GitLab 9.5)_. | | `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13060) in GitLab 9.5)_. |
| `approver_ids` **(PREMIUM)** | integer array | no | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. | | `approver_ids` **(PREMIUM))** | integer array | no | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. |
| `approved_by_ids` **(PREMIUM)** | integer array | no | Returns merge requests which have been approved by all the users with the given `id`s (Max: 5). `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. | | `approved_by_ids` **(PREMIUM)** | integer array | no | Returns merge requests which have been approved by all the users with the given `id`s (Max: 5). `None` returns merge requests with no approvals. `Any` returns merge requests with an approval. |
| `reviewer_id` | integer | no | Returns merge requests which have the user as a [reviewer](../user/project/merge_requests/getting_started.md#enable-or-disable-merge-request-reviewers) with the given user `id`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_username`. | | `reviewer_id` | integer | no | Returns merge requests which have the user as a [reviewer](../user/project/merge_requests/getting_started.md#reviewer) with the given user `id`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_username`. |
| `reviewer_username` | string | no | Returns merge requests which have the user as a [reviewer](../user/project/merge_requests/getting_started.md#enable-or-disable-merge-request-reviewers) with the given `username`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_id`. | | `reviewer_username` | string | no | Returns merge requests which have the user as a [reviewer](../user/project/merge_requests/getting_started.md#reviewer) with the given `username`. `None` returns merge requests with no reviewers. `Any` returns merge requests with any reviewer. Mutually exclusive with `reviewer_id`. |
| `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14016) in GitLab 10.0)_. | | `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14016) in GitLab 10.0)_. |
| `source_branch` | string | no | Return merge requests with the given source branch. | | `source_branch` | string | no | Return merge requests with the given source branch. |
| `target_branch` | string | no | Return merge requests with the given target branch. | | `target_branch` | string | no | Return merge requests with the given target branch. |
......
...@@ -115,16 +115,9 @@ It is also possible to manage multiple assignees: ...@@ -115,16 +115,9 @@ It is also possible to manage multiple assignees:
### Reviewer ### Reviewer
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216054) in GitLab 13.5. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/216054) in GitLab 13.5.
> - It was [deployed behind a feature flag](../../../user/feature_flags.md), disabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/245190) in GitLab 13.9.
> - [Became enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49787) on GitLab 13.7.
> - It's enabled on GitLab.com.
> - It's recommended for production use.
> - It can be enabled or disabled for a single project.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-merge-request-reviewers). **(FREE SELF)**
WARNING: WARNING:
This feature might not be available to you. Check the **version history** note above for details.
Requesting a code review is an important part of contributing code. However, deciding who should review Requesting a code review is an important part of contributing code. However, deciding who should review
your code and asking for a review are no easy tasks. Using the "assignee" field for both authors and your code and asking for a review are no easy tasks. Using the "assignee" field for both authors and
reviewers makes it hard for others to determine who's doing what on a merge request. reviewers makes it hard for others to determine who's doing what on a merge request.
...@@ -137,31 +130,6 @@ This makes it easy to determine the relevant roles for the users involved in the ...@@ -137,31 +130,6 @@ This makes it easy to determine the relevant roles for the users involved in the
To request it, open the **Reviewers** drop-down box to search for the user you wish to get a review from. To request it, open the **Reviewers** drop-down box to search for the user you wish to get a review from.
#### Enable or disable Merge Request Reviewers **(FREE SELF)**
Merge Request Reviewers is under development but ready for production use.
It is deployed behind a feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
can opt to disable it.
To enable it:
```ruby
# For the instance
Feature.enable(:merge_request_reviewers)
# For a single project
Feature.enable(:merge_request_reviewers, Project.find(<project id>))
```
To disable it:
```ruby
# For the instance
Feature.disable(:merge_request_reviewers)
# For a single project
Feature.disable(:merge_request_reviewers, Project.find(<project id>))
```
#### Approval Rule information for Reviewers **(PREMIUM)** #### Approval Rule information for Reviewers **(PREMIUM)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/233736) in GitLab 13.8. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/233736) in GitLab 13.8.
......
...@@ -28,7 +28,7 @@ export default { ...@@ -28,7 +28,7 @@ export default {
return s__('ApprovalRule|Approval rules'); return s__('ApprovalRule|Approval rules');
}, },
isCollapseFeatureEnabled() { isCollapseFeatureEnabled() {
return this.glFeatures.mergeRequestReviewers && this.glFeatures.mrCollapsedApprovalRules; return this.glFeatures.mrCollapsedApprovalRules;
}, },
hasOptionalRules() { hasOptionalRules() {
return this.rules.every((r) => r.approvalsRequired === 0); return this.rules.every((r) => r.approvalsRequired === 0);
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
- return unless issuable.is_a?(MergeRequest) - return unless issuable.is_a?(MergeRequest)
- return unless issuable.approval_feature_available? - return unless issuable.approval_feature_available?
- if !Feature.enabled?(:merge_request_reviewers, @project, default_enabled: true) || !Feature.enabled?(:mr_collapsed_approval_rules, @project) - if !Feature.enabled?(:mr_collapsed_approval_rules, @project)
.form-group.row .form-group.row
.col-sm-2.col-form-label .col-sm-2.col-form-label
= form.label :approver_ids, "Approval rules" = form.label :approver_ids, "Approval rules"
......
...@@ -15,7 +15,6 @@ module EE ...@@ -15,7 +15,6 @@ module EE
types MergeRequest types MergeRequest
condition do condition do
quick_action_target.allows_multiple_reviewers? && quick_action_target.allows_multiple_reviewers? &&
::Feature.enabled?(:merge_request_reviewers, project, default_enabled: :yaml) &&
quick_action_target.persisted? && quick_action_target.persisted? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
end end
......
...@@ -14,8 +14,8 @@ RSpec.describe 'Merge request > User sets approvers', :js do ...@@ -14,8 +14,8 @@ RSpec.describe 'Merge request > User sets approvers', :js do
context 'with feature flag off' do context 'with feature flag off' do
let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
def visit_mr(merge_request_reviewers: false, mr_collapsed_approval_rules: false) def visit_mr(mr_collapsed_approval_rules: false)
stub_feature_flags(merge_request_reviewers: merge_request_reviewers, mr_collapsed_approval_rules: mr_collapsed_approval_rules) stub_feature_flags(mr_collapsed_approval_rules: mr_collapsed_approval_rules)
project.add_developer(user) project.add_developer(user)
sign_in(user) sign_in(user)
visit edit_project_merge_request_path(project, merge_request) visit edit_project_merge_request_path(project, merge_request)
...@@ -25,18 +25,8 @@ RSpec.describe 'Merge request > User sets approvers', :js do ...@@ -25,18 +25,8 @@ RSpec.describe 'Merge request > User sets approvers', :js do
expect(page).to have_button('Add approval rule') expect(page).to have_button('Add approval rule')
end end
it 'does not hide approval rules inside collapse when only merge_request_reviewers is off' do
visit_mr(merge_request_reviewers: false, mr_collapsed_approval_rules: true)
non_collapse_approval_rules
end
it 'does not hide approval rules inside collapse when mr_collapsed_approval_rules is off' do it 'does not hide approval rules inside collapse when mr_collapsed_approval_rules is off' do
visit_mr(merge_request_reviewers: true, mr_collapsed_approval_rules: false) visit_mr(mr_collapsed_approval_rules: false)
non_collapse_approval_rules
end
it 'does not hide approval rules inside collapse when merge_request_reviewers and mr_collapsed_approval_rules are off' do
visit_mr(merge_request_reviewers: false, mr_collapsed_approval_rules: false)
non_collapse_approval_rules non_collapse_approval_rules
end end
end end
......
...@@ -16,13 +16,12 @@ describe('EE Approvals MREditApp', () => { ...@@ -16,13 +16,12 @@ describe('EE Approvals MREditApp', () => {
let store; let store;
let axiosMock; let axiosMock;
const factory = (mergeRequestReviewers = false, mrCollapsedApprovalRules = false) => { const factory = (mrCollapsedApprovalRules = false) => {
wrapper = mount(MREditApp, { wrapper = mount(MREditApp, {
localVue, localVue,
store: new Vuex.Store(store), store: new Vuex.Store(store),
provide: { provide: {
glFeatures: { glFeatures: {
mergeRequestReviewers,
mrCollapsedApprovalRules, mrCollapsedApprovalRules,
}, },
}, },
......
...@@ -276,7 +276,6 @@ RSpec.describe QuickActions::InterpretService do ...@@ -276,7 +276,6 @@ RSpec.describe QuickActions::InterpretService do
context 'reassign_reviewer command' do context 'reassign_reviewer command' do
let(:content) { "/reassign_reviewer @#{current_user.username}" } let(:content) { "/reassign_reviewer @#{current_user.username}" }
context "if the 'merge_request_reviewers' feature flag is on" do
context 'unlicensed' do context 'unlicensed' do
before do before do
stub_licensed_features(multiple_merge_request_reviewers: false) stub_licensed_features(multiple_merge_request_reviewers: false)
...@@ -297,19 +296,6 @@ RSpec.describe QuickActions::InterpretService do ...@@ -297,19 +296,6 @@ RSpec.describe QuickActions::InterpretService do
end end
end end
context "if the 'merge_request_reviewers' feature flag is off" do
before do
stub_feature_flags(merge_request_reviewers: false)
end
it 'does not recognize /reassign_reviewer @user' do
_, updates = service.execute(content, merge_request)
expect(updates).to be_empty
end
end
end
context 'iteration command' do context 'iteration command' do
let_it_be(:iteration) { create(:iteration, group: group) } let_it_be(:iteration) { create(:iteration, group: group) }
......
...@@ -27,7 +27,7 @@ module API ...@@ -27,7 +27,7 @@ module API
expose(:downvotes) { |merge_request, options| issuable_metadata.downvotes } expose(:downvotes) { |merge_request, options| issuable_metadata.downvotes }
expose :author, :assignees, :assignee, using: Entities::UserBasic expose :author, :assignees, :assignee, using: Entities::UserBasic
expose :reviewers, if: -> (merge_request, _) { merge_request.allows_reviewers? }, using: Entities::UserBasic expose :reviewers, using: Entities::UserBasic
expose :source_project_id, :target_project_id expose :source_project_id, :target_project_id
expose :labels do |merge_request, options| expose :labels do |merge_request, options|
if options[:with_labels_details] if options[:with_labels_details]
......
...@@ -181,7 +181,6 @@ module Gitlab ...@@ -181,7 +181,6 @@ module Gitlab
end end
types MergeRequest types MergeRequest
condition do condition do
Feature.enabled?(:merge_request_reviewers, project, default_enabled: :yaml) &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
end end
parse_params do |reviewer_param| parse_params do |reviewer_param|
...@@ -221,7 +220,6 @@ module Gitlab ...@@ -221,7 +220,6 @@ module Gitlab
types MergeRequest types MergeRequest
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
Feature.enabled?(:merge_request_reviewers, project, default_enabled: :yaml) &&
quick_action_target.reviewers.any? && quick_action_target.reviewers.any? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
end end
......
...@@ -20,14 +20,4 @@ RSpec.describe 'Merge request > User edits MR' do ...@@ -20,14 +20,4 @@ RSpec.describe 'Merge request > User edits MR' do
include_context 'merge request edit context' include_context 'merge request edit context'
it_behaves_like 'an editable merge request' it_behaves_like 'an editable merge request'
end end
context 'when merge_request_reviewers is turned off' do
before do
stub_feature_flags(merge_request_reviewers: false)
end
it 'does not render reviewers dropdown' do
expect(page).not_to have_selector('.js-reviewer-search')
end
end
end end
...@@ -30,19 +30,6 @@ RSpec.describe 'User views an open merge request' do ...@@ -30,19 +30,6 @@ RSpec.describe 'User views an open merge request' do
end end
end end
context 'when merge_request_reviewers is turned off' do
let(:project) { create(:project, :public, :repository) }
before do
stub_feature_flags(merge_request_reviewers: false)
visit(merge_request_path(merge_request))
end
it 'has reviewers in sidebar' do
expect(page).not_to have_css('.reviewer')
end
end
context 'when a merge request has repository', :js do context 'when a merge request has repository', :js do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
......
...@@ -40,9 +40,8 @@ RSpec.describe 'Merge requests > User lists merge requests' do ...@@ -40,9 +40,8 @@ RSpec.describe 'Merge requests > User lists merge requests' do
updated_at: 10.seconds.ago) updated_at: 10.seconds.ago)
end end
context 'when merge_request_reviewers is turned on' do context 'merge request reviewers' do
before do before do
stub_feature_flags(merge_request_reviewers: true)
visit_merge_requests(project, reviewer_id: user.id) visit_merge_requests(project, reviewer_id: user.id)
end end
...@@ -62,15 +61,6 @@ RSpec.describe 'Merge requests > User lists merge requests' do ...@@ -62,15 +61,6 @@ RSpec.describe 'Merge requests > User lists merge requests' do
end end
end end
context 'when merge_request_reviewers is turned false' do
it 'has no reviewers in MR list' do
stub_feature_flags(merge_request_reviewers: false)
visit_merge_requests(project, reviewer_id: user.id)
expect(page).not_to have_css('.issuable-reviewers')
end
end
it 'filters on no assignee' do it 'filters on no assignee' do
visit_merge_requests(project, assignee_id: IssuableFinder::Params::FILTER_NONE) visit_merge_requests(project, assignee_id: IssuableFinder::Params::FILTER_NONE)
......
...@@ -89,15 +89,5 @@ RSpec.describe MergeRequestsHelper do ...@@ -89,15 +89,5 @@ RSpec.describe MergeRequestsHelper do
total: user.assigned_open_merge_requests_count + user.review_requested_open_merge_requests_count total: user.assigned_open_merge_requests_count + user.review_requested_open_merge_requests_count
) )
end end
context 'when merge_request_reviewers is disabled' do
before do
stub_feature_flags(merge_request_reviewers: false)
end
it 'returns review_requested as 0' do
expect(subject[:review_requested]).to eq(0)
end
end
end end
end end
...@@ -42,9 +42,7 @@ RSpec.describe ::API::Entities::MergeRequestBasic do ...@@ -42,9 +42,7 @@ RSpec.describe ::API::Entities::MergeRequestBasic do
end end
context 'reviewers' do context 'reviewers' do
context "when merge_request_reviewers FF is enabled" do
before do before do
stub_feature_flags(merge_request_reviewers: true)
merge_request.reviewers = [user] merge_request.reviewers = [user]
end end
...@@ -54,17 +52,4 @@ RSpec.describe ::API::Entities::MergeRequestBasic do ...@@ -54,17 +52,4 @@ RSpec.describe ::API::Entities::MergeRequestBasic do
expect(result['reviewers'][0]['username']).to eq user.username expect(result['reviewers'][0]['username']).to eq user.username
end end
end end
context "when merge_request_reviewers FF is disabled" do
before do
stub_feature_flags(merge_request_reviewers: false)
end
it 'does not include reviewers' do
result = Gitlab::Json.parse(present(merge_request).to_json)
expect(result.keys).not_to include('reviewers')
end
end
end
end end
...@@ -1244,9 +1244,7 @@ RSpec.describe Issue do ...@@ -1244,9 +1244,7 @@ RSpec.describe Issue do
end end
describe '#allows_reviewers?' do describe '#allows_reviewers?' do
it 'returns false as issues do not support reviewers feature' do it 'returns false as we do not support reviewers on issues yet' do
stub_feature_flags(merge_request_reviewers: true)
issue = build_stubbed(:issue) issue = build_stubbed(:issue)
expect(issue.allows_reviewers?).to be(false) expect(issue.allows_reviewers?).to be(false)
......
...@@ -4567,17 +4567,7 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -4567,17 +4567,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
describe '#allows_reviewers?' do describe '#allows_reviewers?' do
it 'returns false without merge_request_reviewers feature' do it 'returns true' do
stub_feature_flags(merge_request_reviewers: false)
merge_request = build_stubbed(:merge_request)
expect(merge_request.allows_reviewers?).to be(false)
end
it 'returns true with merge_request_reviewers feature' do
stub_feature_flags(merge_request_reviewers: true)
merge_request = build_stubbed(:merge_request) merge_request = build_stubbed(:merge_request)
expect(merge_request.allows_reviewers?).to be(true) expect(merge_request.allows_reviewers?).to be(true)
......
...@@ -66,14 +66,6 @@ RSpec.describe 'getting merge request information nested in a project' do ...@@ -66,14 +66,6 @@ RSpec.describe 'getting merge request information nested in a project' do
expect(graphql_data_at(:project, :merge_request, :reviewers, :nodes)).to match_array(expected) expect(graphql_data_at(:project, :merge_request, :reviewers, :nodes)).to match_array(expected)
expect(graphql_data_at(:project, :merge_request, :participants, :nodes)).to include(*expected) expect(graphql_data_at(:project, :merge_request, :participants, :nodes)).to include(*expected)
end end
it 'suppresses reviewers if reviewers are not allowed' do
stub_feature_flags(merge_request_reviewers: false)
post_graphql(query, current_user: current_user)
expect(graphql_data_at(:project, :merge_request, :reviewers)).to be_nil
end
end end
it 'includes diff stats' do it 'includes diff stats' do
......
...@@ -265,18 +265,6 @@ RSpec.describe 'getting merge request listings nested in a project' do ...@@ -265,18 +265,6 @@ RSpec.describe 'getting merge request listings nested in a project' do
}) })
end end
context 'the feature flag is disabled' do
before do
stub_feature_flags(merge_request_reviewers: false)
end
it 'does not return reviewers' do
execute_query
expect(results).to all(match a_hash_including('reviewers' => be_nil))
end
end
include_examples 'N+1 query check' include_examples 'N+1 query check'
end end
end end
......
...@@ -20,23 +20,11 @@ RSpec.describe MergeRequestBasicEntity do ...@@ -20,23 +20,11 @@ RSpec.describe MergeRequestBasicEntity do
let(:params) { { reviewers: [reviewer] } } let(:params) { { reviewers: [reviewer] } }
let(:reviewer) { build(:user) } let(:reviewer) { build(:user) }
context 'when merge_request_reviewers feature is disabled' do
it 'does not contain assignees attributes' do
stub_feature_flags(merge_request_reviewers: false)
expect(subject[:reviewers]).to be_nil
end
end
context 'when merge_request_reviewers feature is enabled' do
it 'contains reviewers attributes' do it 'contains reviewers attributes' do
stub_feature_flags(merge_request_reviewers: true)
expect(subject[:reviewers].count).to be 1 expect(subject[:reviewers].count).to be 1
expect(subject[:reviewers].first.keys).to include( expect(subject[:reviewers].first.keys).to include(
:id, :name, :username, :state, :avatar_url, :web_url :id, :name, :username, :state, :avatar_url, :web_url
) )
end end
end end
end
end end
...@@ -35,23 +35,11 @@ RSpec.describe MergeRequestSidebarExtrasEntity do ...@@ -35,23 +35,11 @@ RSpec.describe MergeRequestSidebarExtrasEntity do
end end
describe '#reviewers' do describe '#reviewers' do
context 'when merge_request_reviewers feature is disabled' do
it 'does not contain reviewers attributes' do
stub_feature_flags(merge_request_reviewers: false)
expect(entity[:reviewers]).to be_nil
end
end
context 'when merge_request_reviewers feature is enabled' do
it 'contains reviewers attributes' do it 'contains reviewers attributes' do
stub_feature_flags(merge_request_reviewers: true)
expect(entity[:reviewers].count).to be 1 expect(entity[:reviewers].count).to be 1
expect(entity[:reviewers].first.keys).to include( expect(entity[:reviewers].first.keys).to include(
:id, :name, :username, :state, :avatar_url, :web_url, :can_merge :id, :name, :username, :state, :avatar_url, :web_url, :can_merge
) )
end end
end end
end
end end
...@@ -142,30 +142,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do ...@@ -142,30 +142,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
context 'with reviewers' do context 'with reviewers' do
let(:opts) { { reviewer_ids: [user2.id] } } let(:opts) { { reviewer_ids: [user2.id] } }
context 'when merge_request_reviewers feature is disabled' do
before do
stub_feature_flags(merge_request_reviewers: false)
end
it 'does not create a system note about merge_request review request' do
note = find_note('review requested from')
expect(note).to be_nil
end
it 'does not update the tracking' do
expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
.not_to receive(:track_users_review_requested)
update_merge_request(reviewer_ids: [user.id])
end
end
context 'when merge_request_reviewers feature is enabled' do
before(:context) do
stub_feature_flags(merge_request_reviewers: true)
end
it 'creates system note about merge_request review request' do it 'creates system note about merge_request review request' do
note = find_note('requested review from') note = find_note('requested review from')
...@@ -181,7 +157,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do ...@@ -181,7 +157,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
update_merge_request(reviewer_ids: [user.id]) update_merge_request(reviewer_ids: [user.id])
end end
end end
end
it 'creates a resource label event' do it 'creates a resource label event' do
event = merge_request.resource_label_events.last event = merge_request.resource_label_events.last
......
...@@ -879,7 +879,6 @@ RSpec.describe QuickActions::InterpretService do ...@@ -879,7 +879,6 @@ RSpec.describe QuickActions::InterpretService do
let(:issuable) { issue } let(:issuable) { issue }
end end
context 'when the merge_request_reviewers flag is enabled' do
describe 'assign_reviewer command' do describe 'assign_reviewer command' do
let(:content) { "/assign_reviewer @#{developer.username}" } let(:content) { "/assign_reviewer @#{developer.username}" }
let(:issuable) { merge_request } let(:issuable) { merge_request }
...@@ -999,27 +998,6 @@ RSpec.describe QuickActions::InterpretService do ...@@ -999,27 +998,6 @@ RSpec.describe QuickActions::InterpretService do
it_behaves_like 'unassign_reviewer command' it_behaves_like 'unassign_reviewer command'
end end
end end
end
context 'when the merge_request_reviewers flag is disabled' do
before do
stub_feature_flags(merge_request_reviewers: false)
end
describe 'assign_reviewer command' do
it_behaves_like 'empty command' do
let(:content) { "/assign_reviewer @#{developer.username}" }
let(:issuable) { merge_request }
end
end
describe 'unassign_reviewer command' do
it_behaves_like 'empty command' do
let(:content) { "/unassign_reviewer @#{developer.username}" }
let(:issuable) { merge_request }
end
end
end
context 'unassign command' do context 'unassign command' do
let(:content) { '/unassign' } let(:content) { '/unassign' }
......
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