Skip to content

G
gitlab-ce
Commit bbfe2b3c authored by charlie ablett's avatar charlie ablett
Browse files
Options

Merge branch 'remove-ff_external_status_checks-flag' into 'master' 

Remove ff_external_status_checks feature flag

See merge request gitlab-org/gitlab!65053
parents e8b67da0 22cccda5
Hide whitespace changes
Inline Side-by-side
Showing with 50 additions and 223 deletions
doc/api/status_checks.md
View file @ bbfe2b3c
......@@ -7,14 +7,8 @@ type: reference, api
# External Status Checks API **(ULTIMATE)**
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0.
> - It's [deployed behind a feature flag](../user/feature_flags.md), disabled by default.
> - It's disabled on GitLab.com.
> - It's not recommended for production use.
> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-external-status-checks).
WARNING:
This feature might not be available to you. Check the **version history** note above for details.
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0, disabled behind the `:ff_external_status_checks` feature flag.
> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/320783) in GitLab 14.1.
## List status checks for a merge request
......@@ -151,35 +145,6 @@ PUT /projects/:id/external_status_checks/:check_id
| `external_url` | string | no | URL of external status check resource |
| `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by |
## Enable or disable external status checks **(ULTIMATE SELF)**
External status checks are:
- Under development.
- Not ready for production use.
The feature is deployed behind a feature flag that is **disabled by default**.
[GitLab administrators with access to the GitLab Rails console](../user/feature_flags.md)
can enable it.
To enable it:
```ruby
# For the instance
Feature.enable(:ff_external_status_checks)
# For a single project
Feature.enable(:ff_external_status_checks, Project.find(<project id>))
```
To disable it:
```ruby
# For the instance
Feature.disable(:ff_external_status_checks)
# For a single project
Feature.disable(:ff_external_status_checks, Project.find(<project id>))
```
## Related links
- [External status checks](../user/project/merge_requests/status_checks.md).
doc/user/project/merge_requests/status_checks.md
View file @ bbfe2b3c
......@@ -8,11 +8,8 @@ disqus_identifier: 'https://docs.gitlab.com/ee/user/project/merge_requests/statu
# External Status Checks **(ULTIMATE)**
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0.
> - It's [deployed behind a feature flag](../../feature_flags.md), disabled by default.
> - It's disabled on GitLab.com.
> - It's not recommended for production use.
> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-status-checks). **(ULTIMATE SELF)**
> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3869) in GitLab 14.0, disabled behind the `:ff_external_status_checks` feature flag.
> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/320783) in GitLab 14.1.
WARNING:
This feature might not be available to you. Check the **version history** note above for details.
......@@ -116,7 +113,6 @@ the status check and it **will not** be recoverable.
## Status checks widget
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327634) in GitLab 14.1.
> - The [external status checks](#external-status-checks) feature must be [enabled](#enable-or-disable-status-checks) to see the status checks widget.
The status checks widget displays in merge requests and shows the status of external
status checks:
......@@ -188,31 +184,6 @@ You should:
- Check the [GitLab status page](https://status.gitlab.com/) if the problem persists,
to see if there is a wider outage.
## Enable or disable status checks **(ULTIMATE SELF)**
Status checks are under development and not ready for production use. It is
deployed behind a feature flag that is **disabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../../administration/feature_flags.md)
can enable it.
To enable it:
```ruby
# For the instance
Feature.enable(:ff_external_status_checks)
# For a single project
Feature.enable(:ff_external_status_checks, Project.find(<project id>))
```
To disable it:
```ruby
# For the instance
Feature.disable(:ff_external_status_checks)
# For a single project
Feature.disable(:ff_external_status_checks, Project.find(<project id>)
```
## Related links
- [External status checks API](../../../api/status_checks.md)
ee/app/presenters/ee/merge_request_presenter.rb
View file @ bbfe2b3c
......@@ -69,8 +69,7 @@ module EE
private
def expose_mr_status_checks?
::Feature.enabled?(:ff_external_status_checks, project, default_enabled: :yaml) &&
current_user.present? &&
current_user.present? &&
project.external_status_checks.applicable_to_branch(merge_request.target_branch).any?
end
......
ee/app/views/projects/_merge_request_settings.html.haml
View file @ bbfe2b3c
......@@ -8,7 +8,7 @@
= render_ce 'projects/merge_request_merge_checks_settings', project: @project, form: form
- if ::Feature.enabled?(:ff_external_status_checks, @project, default_enabled: :yaml) && @project.licensed_feature_available?(:external_status_checks)
- if @project.licensed_feature_available?(:external_status_checks)
= render_if_exists 'projects/merge_request_status_checks_settings'
= render 'projects/merge_request_merge_suggestions_settings', project: @project, form: form
......
ee/config/feature_flags/development/ff_external_status_checks.yml deleted 100644 → 0
View file @ e8b67da0
---
name: ff_external_status_checks
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/54002
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320783
milestone: '14.1'
type: development
group: group::compliance
default_enabled: false
ee/lib/api/status_checks.rb
View file @ bbfe2b3c
......@@ -13,8 +13,7 @@ module API
helpers do
def check_feature_enabled!
unauthorized! unless user_project.licensed_feature_available?(:external_status_checks) &&
Feature.enabled?(:ff_external_status_checks, user_project)
unauthorized! unless user_project.licensed_feature_available?(:external_status_checks)
end
end
......@@ -22,7 +21,6 @@ module API
segment ':id/external_status_checks' do
desc 'Create a new external status check' do
success ::API::Entities::ExternalStatusCheck
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end
params do
requires :name, type: String, desc: 'The name of the external status check'
......@@ -45,9 +43,7 @@ module API
render_api_error!(service.payload[:errors], service.http_status)
end
end
desc 'List project\'s external approval rules' do
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end
desc 'List project\'s external approval rules'
params do
use :pagination
end
......@@ -60,7 +56,6 @@ module API
segment ':check_id' do
desc 'Update an external approval rule' do
success ::API::Entities::ExternalStatusCheck
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end
params do
requires :check_id, type: Integer, desc: 'The ID of the external status check'
......@@ -85,9 +80,7 @@ module API
end
end
desc 'Delete an external status check' do
detail 'This feature is gated by the :ff_external_status_checks feature flag.'
end
desc 'Delete an external status check'
params do
requires :check_id, type: Integer, desc: 'The ID of the status check'
end
......@@ -106,7 +99,6 @@ module API
segment ':id/merge_requests/:merge_request_iid' do
desc 'Externally approve a merge request' do
detail 'This feature was introduced in 13.12 and is gated behind the :ff_external_status_checks feature flag.'
success Entities::MergeRequests::StatusCheckResponse
end
params do
......@@ -116,8 +108,6 @@ module API
requires :sha, type: String, desc: 'The current SHA at HEAD of the merge request.'
end
post 'status_check_responses' do
not_found! unless ::Feature.enabled?(:ff_external_status_checks, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request)
check_sha_param!(params, merge_request)
......@@ -127,12 +117,8 @@ module API
present(approval, with: Entities::MergeRequests::StatusCheckResponse)
end
desc 'List all status checks for a merge request and their state.' do
detail 'This feature was introduced in 13.12 and is gated behind the :ff_external_status_checks feature flag.'
end
desc 'List all status checks for a merge request and their state.'
get 'status_checks' do
not_found! unless ::Feature.enabled?(:ff_external_status_checks, user_project)
merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request)
present(paginate(user_project.external_status_checks.applicable_to_branch(merge_request.target_branch)), with: Entities::MergeRequests::StatusCheck, merge_request: merge_request, sha: merge_request.source_branch_sha)
......
ee/spec/features/merge_request/user_sees_status_checks_widget_spec.rb
View file @ bbfe2b3c
......@@ -29,37 +29,23 @@ RSpec.describe 'Merge request > User sees status checks widget', :js do
visit project_merge_request_path(project, merge_request)
end
context 'feature flag is enabled' do
before do
stub_feature_flags(ff_external_status_checks: true)
end
it 'shows the widget' do
expect(page).to have_content('Status checks 1 pending')
end
it 'shows the widget' do
expect(page).to have_content('Status checks 1 pending')
it 'shows the status check issues', :aggregate_failures do
within '[data-test-id="mr-status-checks"]' do
click_button 'Expand'
end
it 'shows the status check issues', :aggregate_failures do
within '[data-test-id="mr-status-checks"]' do
click_button 'Expand'
end
[check1, check2].each do |rule|
within "[data-testid='mr-status-check-issue-#{rule.id}']" do
icon_type = rule.approved?(merge_request, merge_request.source_branch_sha) ? 'success' : 'pending'
expect(page).to have_css(".ci-status-icon-#{icon_type}")
expect(page).to have_content("#{rule.name}, #{rule.external_url}")
end
[check1, check2].each do |rule|
within "[data-testid='mr-status-check-issue-#{rule.id}']" do
icon_type = rule.approved?(merge_request, merge_request.source_branch_sha) ? 'success' : 'pending'
expect(page).to have_css(".ci-status-icon-#{icon_type}")
expect(page).to have_content("#{rule.name}, #{rule.external_url}")
end
end
end
context 'feature flag is disabled' do
before do
stub_feature_flags(ff_external_status_checks: false)
end
it_behaves_like 'no status checks widget'
end
end
context 'user is not logged in' do
......
ee/spec/presenters/merge_request_presenter_spec.rb
View file @ bbfe2b3c
......@@ -186,15 +186,12 @@ RSpec.describe MergeRequestPresenter do
let(:exposed_path) { expose_path("/api/v4/projects/#{merge_request.project.id}/merge_requests/#{merge_request.iid}/status_checks") }
where(:feature_flag_enabled?, :authenticated?, :has_status_checks?, :exposes_path?) do
false | false | false | false
false | false | true | false
false | true | true | false
false | true | false | false
true | false | false | false
true | true | false | false
true | false | true | false
true | true | true | true
where(:authenticated?, :has_status_checks?, :exposes_path?) do
false | false | false
false | true | false
true | true | true
true | false | false
true | true | true
end
with_them do
......@@ -202,7 +199,6 @@ RSpec.describe MergeRequestPresenter do
let(:path) { exposes_path? ? exposed_path : nil }
before do
stub_feature_flags(ff_external_status_checks: feature_flag_enabled?)
allow(project.external_status_checks).to receive(:applicable_to_branch).and_return([{ branch: 'foo' }])
allow(project.external_status_checks.applicable_to_branch).to receive(:any?).and_return(has_status_checks?)
end
......@@ -210,13 +206,9 @@ RSpec.describe MergeRequestPresenter do
it { is_expected.to eq(path) }
end
context 'with the feature flag enabled and user authenticated' do
context 'with the user authenticated' do
let(:presenter) { described_class.new(merge_request, current_user: user) }
before do
stub_feature_flags(ff_external_status_checks: true)
end
context 'without applicable branches' do
before do
create(:external_status_check, project: project, protected_branches: [create(:protected_branch, name: 'testbranch')])
......
ee/spec/requests/api/status_checks_spec.rb
View file @ bbfe2b3c
......@@ -32,18 +32,6 @@ RSpec.describe API::StatusChecks do
describe 'GET :id/merge_requests/:merge_request_iid/status_checks' do
subject { get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_checks", user), params: { external_status_check_id: rule.id, sha: sha } }
context 'feature flag is disabled' do
before do
stub_feature_flags(ff_external_status_checks: false)
end
it 'returns a not found error' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when current_user has access' do
before do
stub_licensed_features(external_status_checks: true)
......@@ -81,18 +69,6 @@ RSpec.describe API::StatusChecks do
describe 'POST :id/:merge_requests/:merge_request_iid/status_check_responses' do
subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/status_check_responses", user), params: { external_status_check_id: rule.id, sha: sha } }
context 'feature flag is disabled' do
before do
stub_feature_flags(ff_external_status_checks: false)
end
it 'returns a not found error' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when user has access' do
before do
stub_licensed_features(external_status_checks: true)
......@@ -153,20 +129,15 @@ RSpec.describe API::StatusChecks do
end
context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do
false | false | false | :not_found
false | false | true | :unauthorized
false | true | true | :unauthorized
false | true | false | :not_found
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :success
where(:licensed, :project_owner, :status) do
false | false | :not_found
false | true | :unauthorized
true | false | :not_found
true | true | :success
end
with_them do
before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed)
end
......@@ -182,7 +153,6 @@ RSpec.describe API::StatusChecks do
describe 'POST projects/:id/external_status_checks' do
context 'successfully creating new external approval rule' do
before do
stub_feature_flags(ff_external_status_checks: true)
stub_licensed_features(external_status_checks: true)
end
......@@ -229,20 +199,15 @@ RSpec.describe API::StatusChecks do
end
context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do
false | false | false | :not_found
false | false | true | :unauthorized
false | true | true | :unauthorized
false | true | false | :not_found
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :created
where(:licensed, :project_owner, :status) do
false | false | :not_found
false | true | :unauthorized
true | false | :not_found
true | true | :created
end
with_them do
before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed)
end
......@@ -280,20 +245,15 @@ RSpec.describe API::StatusChecks do
end
context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do
false | false | false | :not_found
false | false | true | :unauthorized
false | true | true | :unauthorized
false | true | false | :not_found
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :success
where(:licensed, :project_owner, :status) do
false | false | :not_found
false | true | :unauthorized
true | false | :not_found
true | true | :success
end
with_them do
before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed)
end
......@@ -311,7 +271,6 @@ RSpec.describe API::StatusChecks do
context 'successfully updating external approval rule' do
before do
stub_feature_flags(ff_external_status_checks: true)
stub_licensed_features(external_status_checks: true)
end
......@@ -362,20 +321,15 @@ RSpec.describe API::StatusChecks do
end
context 'when feature is disabled, unlicensed or user has permission' do
where(:licensed, :flag, :project_owner, :status) do
false | false | false | :not_found
false | false | true | :unauthorized
false | true | true | :unauthorized
false | true | false | :not_found
true | false | false | :not_found
true | false | true | :unauthorized
true | true | false | :not_found
true | true | true | :success
where(:licensed, :project_owner, :status) do
false | false | :not_found
false | true | :unauthorized
true | false | :not_found
true | true | :success
end
with_them do
before do
stub_feature_flags(ff_external_status_checks: flag)
stub_licensed_features(external_status_checks: licensed)
end
......
ee/spec/views/projects/edit.html.haml_spec.rb
View file @ bbfe2b3c
......@@ -16,9 +16,9 @@ RSpec.describe 'projects/edit' do
end
context 'status checks' do
context 'feature flag is disabled' do
context 'feature is not available' do
before do
stub_feature_flags(ff_external_status_checks: false)
stub_licensed_features(external_status_checks: false)
render
end
......@@ -26,24 +26,6 @@ RSpec.describe 'projects/edit' do
it 'hides the status checks area' do
expect(rendered).not_to have_content('Status check')
end
end
context 'feature flag is enabled' do
before do
stub_feature_flags(ff_external_status_checks: true)
end
context 'feature is not available' do
before do
stub_licensed_features(external_status_checks: false)
render
end
it 'hides the status checks area' do
expect(rendered).not_to have_content('Status check')
end
end
context 'feature is available' do
before do
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7