Commit 085a7f27 authored by Robert Hunt's avatar Robert Hunt

Removed compliance_violations_graphql_type feature flag

Removing this feature flag fully enables the compliance violations
GraphQL endpoint for all Ultimate users

Changelog: changed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82603
EE: true
parent 5e223920
...@@ -9584,7 +9584,7 @@ Represents a ComplianceFramework associated with a Project. ...@@ -9584,7 +9584,7 @@ Represents a ComplianceFramework associated with a Project.
### `ComplianceViolation` ### `ComplianceViolation`
Compliance violation associated with a merged merge request. Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice. Compliance violation associated with a merged merge request.
#### Fields #### Fields
...@@ -11523,7 +11523,7 @@ four standard [pagination arguments](#connection-pagination-arguments): ...@@ -11523,7 +11523,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
##### `Group.mergeRequestViolations` ##### `Group.mergeRequestViolations`
Compliance violations reported on merge requests merged within the group. Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice. Compliance violations reported on merge requests merged within the group.
Returns [`ComplianceViolationConnection`](#complianceviolationconnection). Returns [`ComplianceViolationConnection`](#complianceviolationconnection).
...@@ -26,7 +26,7 @@ module ComplianceManagement ...@@ -26,7 +26,7 @@ module ComplianceManagement
end end
def execute def execute
return ::MergeRequests::ComplianceViolation.none unless allowed? return unless allowed?
items = init_collection items = init_collection
...@@ -42,8 +42,7 @@ module ComplianceManagement ...@@ -42,8 +42,7 @@ module ComplianceManagement
attr_reader :current_user, :group, :params attr_reader :current_user, :group, :params
def allowed? def allowed?
::Feature.enabled?(:compliance_violations_graphql_type, group, default_enabled: :yaml) && Ability.allowed?(current_user, :read_group_compliance_dashboard, group)
Ability.allowed?(current_user, :read_group_compliance_dashboard, group)
end end
def init_collection def init_collection
......
...@@ -104,9 +104,9 @@ module EE ...@@ -104,9 +104,9 @@ module EE
field :merge_request_violations, field :merge_request_violations,
::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type, ::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type,
null: true, null: true,
description: 'Compliance violations reported on merge requests merged within the group.' \ description: 'Compliance violations reported on merge requests merged within the group.',
' Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.', resolver: ::Resolvers::ComplianceManagement::MergeRequests::ComplianceViolationResolver,
resolver: ::Resolvers::ComplianceManagement::MergeRequests::ComplianceViolationResolver authorize: :read_group_compliance_dashboard
def billable_members_count(requested_hosted_plan: nil) def billable_members_count(requested_hosted_plan: nil)
object.billable_members_count(requested_hosted_plan) object.billable_members_count(requested_hosted_plan)
......
...@@ -4,11 +4,16 @@ module Resolvers ...@@ -4,11 +4,16 @@ module Resolvers
module ComplianceManagement module ComplianceManagement
module MergeRequests module MergeRequests
class ComplianceViolationResolver < BaseResolver class ComplianceViolationResolver < BaseResolver
include Gitlab::Graphql::Authorize::AuthorizeResource
alias_method :group, :object alias_method :group, :object
type ::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type, null: true type ::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type, null: true
description 'Compliance violations reported on a merged merge request.' description 'Compliance violations reported on a merged merge request.'
authorize :read_group_compliance_dashboard
authorizes_object!
argument :filters, Types::ComplianceManagement::MergeRequests::ComplianceViolationInputType, argument :filters, Types::ComplianceManagement::MergeRequests::ComplianceViolationInputType,
required: false, required: false,
default_value: {}, default_value: {},
......
...@@ -5,8 +5,7 @@ module Types ...@@ -5,8 +5,7 @@ module Types
module MergeRequests module MergeRequests
class ComplianceViolationType < ::Types::BaseObject class ComplianceViolationType < ::Types::BaseObject
graphql_name 'ComplianceViolation' graphql_name 'ComplianceViolation'
description 'Compliance violation associated with a merged merge request.' \ description 'Compliance violation associated with a merged merge request.'
' Available only when feature flag `compliance_violations_graphql_type` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.'
authorize :read_group_compliance_dashboard authorize :read_group_compliance_dashboard
......
...@@ -15,8 +15,7 @@ module ComplianceManagement ...@@ -15,8 +15,7 @@ module ComplianceManagement
private private
def permitted?(group) def permitted?(group)
::Feature.enabled?(:compliance_violations_graphql_type, group, default_enabled: :yaml) && group.licensed_feature_available?(:group_level_compliance_dashboard)
group.licensed_feature_available?(:group_level_compliance_dashboard)
end end
end end
end end
......
...@@ -18,8 +18,7 @@ module EE ...@@ -18,8 +18,7 @@ module EE
private private
def compliance_violations_enabled?(group) def compliance_violations_enabled?(group)
::Feature.enabled?(:compliance_violations_graphql_type, group, default_enabled: :yaml) && group.licensed_feature_available?(:group_level_compliance_dashboard)
group.licensed_feature_available?(:group_level_compliance_dashboard)
end end
end end
end end
......
---
name: compliance_violations_graphql_type
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77954
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350249
milestone: '14.8'
type: development
group: group::compliance
default_enabled: false
...@@ -29,83 +29,67 @@ RSpec.describe ComplianceManagement::MergeRequests::ComplianceViolationsFinder d ...@@ -29,83 +29,67 @@ RSpec.describe ComplianceManagement::MergeRequests::ComplianceViolationsFinder d
describe '#execute' do describe '#execute' do
subject { finder.execute } subject { finder.execute }
context 'when feature is disabled' do context 'when the user is unauthorized' do
before do it 'returns nil' do
stub_feature_flags(compliance_violations_graphql_type: false) expect(subject).to be_nil
end
it 'returns no compliance violations' do
expect(subject).to eq(::MergeRequests::ComplianceViolation.none)
end end
end end
context 'when feature is enabled' do context 'when the user is authorized' do
before do before do
stub_feature_flags(compliance_violations_graphql_type: true) group.add_owner(current_user)
end end
context 'when the user is unauthorized' do context 'without any filters or sorting' do
it 'returns no compliance violations' do it 'finds all the compliance violations' do
expect(subject).to eq(::MergeRequests::ComplianceViolation.none) expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
end end
end end
context 'when the user is authorized' do context 'filtering the results' do
before do context 'when given an array of project IDs' do
group.add_owner(current_user) let(:params) { { project_ids: [project.id] } }
end
context 'without any filters or sorting' do it 'finds the filtered compliance violations' do
it 'finds all the compliance violations' do expect(subject).to contain_exactly(compliance_violation)
expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
end end
end end
context 'filtering the results' do context 'when given merged at dates' do
context 'when given an array of project IDs' do where(:merged_params, :result) do
let(:params) { { project_ids: [project.id] } } { merged_before: 2.days.ago } | lazy { compliance_violation }
{ merged_after: 2.days.ago } | lazy { compliance_violation2 }
it 'finds the filtered compliance violations' do { merged_before: Date.current, merged_after: 2.days.ago } | lazy { compliance_violation2 }
expect(subject).to contain_exactly(compliance_violation)
end
end end
context 'when given merged at dates' do with_them do
where(:merged_params, :result) do let(:params) { merged_params }
{ merged_before: 2.days.ago } | lazy { compliance_violation }
{ merged_after: 2.days.ago } | lazy { compliance_violation2 }
{ merged_before: Date.current, merged_after: 2.days.ago } | lazy { compliance_violation2 }
end
with_them do
let(:params) { merged_params }
it 'finds the filtered compliance violations' do it 'finds the filtered compliance violations' do
expect(subject).to contain_exactly(result) expect(subject).to contain_exactly(result)
end
end end
end end
end end
end
context 'sorting the results' do context 'sorting the results' do
where(:direction, :result) do where(:direction, :result) do
'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] } 'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] }
'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] } 'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] }
'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] } 'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] }
'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] } 'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] }
'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] } 'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] }
'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] } 'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] }
'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] } 'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] }
'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] } 'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] }
'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] } 'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] }
end end
with_them do with_them do
let(:params) { { sort: direction } } let(:params) { { sort: direction } }
it 'finds the filtered compliance violations' do it 'finds the filtered compliance violations' do
expect(subject).to match_array(result) expect(subject).to match_array(result)
end
end end
end end
end end
......
...@@ -29,79 +29,67 @@ RSpec.describe Resolvers::ComplianceManagement::MergeRequests::ComplianceViolati ...@@ -29,79 +29,67 @@ RSpec.describe Resolvers::ComplianceManagement::MergeRequests::ComplianceViolati
subject(:resolve_compliance_violations) { resolve(described_class, obj: group, args: args, ctx: { current_user: current_user }) } subject(:resolve_compliance_violations) { resolve(described_class, obj: group, args: args, ctx: { current_user: current_user }) }
context 'feature flag is disabled' do context 'user is unauthorized' do
before do it 'returns nil' do
stub_feature_flags(compliance_violations_graphql_type: false) expect(subject).to be_nil
end end
end
it 'returns an empty collection' do context 'user is authorized' do
expect(subject).to be_empty before do
group.add_owner(current_user)
end end
end
context 'feature flag is enabled' do context 'without any filters or sorting' do
context 'user is unauthorized' do it 'finds all the compliance violations' do
it 'returns an empty collection' do expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
expect(subject).to be_empty
end end
end end
context 'user is authorized' do context 'filtering the results' do
before do context 'when given an array of project IDs' do
group.add_owner(current_user) let(:args) { { filters: { project_ids: [::Gitlab::GlobalId.as_global_id(project.id, model_name: 'Project')] } } }
end
context 'without any filters or sorting' do it 'finds the filtered compliance violations' do
it 'finds all the compliance violations' do expect(subject).to contain_exactly(compliance_violation)
expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
end end
end end
context 'filtering the results' do context 'when given merged at dates' do
context 'when given an array of project IDs' do where(:merged_params, :result) do
let(:args) { { filters: { project_ids: [::Gitlab::GlobalId.as_global_id(project.id, model_name: 'Project')] } } } { merged_before: 2.days.ago } | lazy { compliance_violation }
{ merged_after: 2.days.ago } | lazy { compliance_violation2 }
it 'finds the filtered compliance violations' do { merged_before: Date.current, merged_after: 2.days.ago } | lazy { compliance_violation2 }
expect(subject).to contain_exactly(compliance_violation)
end
end end
context 'when given merged at dates' do with_them do
where(:merged_params, :result) do let(:args) { { filters: merged_params } }
{ merged_before: 2.days.ago } | lazy { compliance_violation }
{ merged_after: 2.days.ago } | lazy { compliance_violation2 }
{ merged_before: Date.current, merged_after: 2.days.ago } | lazy { compliance_violation2 }
end
with_them do
let(:args) { { filters: merged_params } }
it 'finds the filtered compliance violations' do it 'finds the filtered compliance violations' do
expect(subject).to contain_exactly(result) expect(subject).to contain_exactly(result)
end
end end
end end
end end
end
context 'sorting the results' do context 'sorting the results' do
where(:direction, :result) do where(:direction, :result) do
'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] } 'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] }
'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] } 'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] }
'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] } 'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] }
'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] } 'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] }
'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] } 'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] }
'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] } 'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] }
'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] } 'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] }
'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] } 'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] }
'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] } 'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] }
end end
with_them do with_them do
let(:args) { { sort: direction } } let(:args) { { sort: direction } }
it 'finds the filtered compliance violations' do it 'finds the filtered compliance violations' do
expect(subject).to match_array(result) expect(subject).to match_array(result)
end
end end
end end
end end
......
...@@ -70,88 +70,70 @@ RSpec.describe 'getting the compliance violations for a group' do ...@@ -70,88 +70,70 @@ RSpec.describe 'getting the compliance violations for a group' do
merge_request2.metrics.update!(merged_at: 1.day.ago) merge_request2.metrics.update!(merged_at: 1.day.ago)
end end
context 'when feature is disabled' do context 'when the user is unauthorized' do
before do it 'returns nil' do
stub_feature_flags(compliance_violations_graphql_type: false)
end
it 'returns empty' do
post_graphql(query, current_user: current_user) post_graphql(query, current_user: current_user)
expect(compliance_violations).to be_empty expect(compliance_violations).to be_nil
end end
end end
context 'when feature is enabled' do context 'when the user is authorized' do
before do before do
stub_feature_flags(compliance_violations_graphql_type: true) group.add_owner(current_user)
end end
context 'when the user is unauthorized' do context 'without any filters or sorting' do
it 'returns empty' do it 'finds all the compliance violations' do
post_graphql(query, current_user: current_user) post_graphql(query, current_user: current_user)
expect(compliance_violations).to be_empty expect(compliance_violations).to contain_exactly(violation_output, violation2_output)
end end
end end
context 'when the user is authorized' do context 'filtering the results' do
before do context 'when given an array of project IDs' do
group.add_owner(current_user)
end
context 'without any filters or sorting' do
it 'finds all the compliance violations' do it 'finds all the compliance violations' do
post_graphql(query, current_user: current_user) post_graphql(query({ filters: { projectIds: [project.to_global_id.to_s] } }), current_user: current_user)
expect(compliance_violations).to contain_exactly(violation_output, violation2_output) expect(compliance_violations).to contain_exactly(violation_output)
end end
end end
context 'filtering the results' do context 'when given merged at dates' do
context 'when given an array of project IDs' do where(:merged_params, :result) do
it 'finds all the compliance violations' do { 'mergedBefore' => 2.days.ago.to_date.iso8601 } | lazy { violation_output }
post_graphql(query({ filters: { projectIds: [project.to_global_id.to_s] } }), current_user: current_user) { 'mergedAfter' => 2.days.ago.to_date.iso8601 } | lazy { violation2_output }
{ 'mergedBefore' => Date.current.iso8601, 'mergedAfter' => 2.days.ago.to_date.iso8601 } | lazy { violation2_output }
expect(compliance_violations).to contain_exactly(violation_output)
end
end end
context 'when given merged at dates' do with_them do
where(:merged_params, :result) do it 'finds all the compliance violations' do
{ 'mergedBefore' => 2.days.ago.to_date.iso8601 } | lazy { violation_output } post_graphql(query({ filters: merged_params }), current_user: current_user)
{ 'mergedAfter' => 2.days.ago.to_date.iso8601 } | lazy { violation2_output }
{ 'mergedBefore' => Date.current.iso8601, 'mergedAfter' => 2.days.ago.to_date.iso8601 } | lazy { violation2_output }
end
with_them do
it 'finds all the compliance violations' do
post_graphql(query({ filters: merged_params }), current_user: current_user)
expect(compliance_violations).to contain_exactly(result) expect(compliance_violations).to contain_exactly(result)
end
end end
end end
end end
end
context 'sorting the results' do context 'sorting the results' do
where(:direction, :result) do where(:direction, :result) do
:SEVERITY_LEVEL_ASC | lazy { [violation_output, violation2_output] } :SEVERITY_LEVEL_ASC | lazy { [violation_output, violation2_output] }
:SEVERITY_LEVEL_DESC | lazy { [violation2_output, violation_output] } :SEVERITY_LEVEL_DESC | lazy { [violation2_output, violation_output] }
:VIOLATION_REASON_ASC | lazy { [violation_output, violation2_output] } :VIOLATION_REASON_ASC | lazy { [violation_output, violation2_output] }
:VIOLATION_REASON_DESC | lazy { [violation2_output, violation_output] } :VIOLATION_REASON_DESC | lazy { [violation2_output, violation_output] }
:MERGE_REQUEST_TITLE_ASC | lazy { [violation_output, violation2_output] } :MERGE_REQUEST_TITLE_ASC | lazy { [violation_output, violation2_output] }
:MERGE_REQUEST_TITLE_DESC | lazy { [violation2_output, violation_output] } :MERGE_REQUEST_TITLE_DESC | lazy { [violation2_output, violation_output] }
:MERGED_AT_ASC | lazy { [violation_output, violation2_output] } :MERGED_AT_ASC | lazy { [violation_output, violation2_output] }
:MERGED_AT_DESC | lazy { [violation2_output, violation_output] } :MERGED_AT_DESC | lazy { [violation2_output, violation_output] }
end end
with_them do with_them do
it 'finds all the compliance violations' do it 'finds all the compliance violations' do
post_graphql(query({ sort: direction }), current_user: current_user) post_graphql(query({ sort: direction }), current_user: current_user)
expect(compliance_violations).to match_array(result) expect(compliance_violations).to match_array(result)
end
end end
end end
end end
......
...@@ -26,17 +26,8 @@ RSpec.describe ComplianceManagement::MergeRequests::CreateComplianceViolationsSe ...@@ -26,17 +26,8 @@ RSpec.describe ComplianceManagement::MergeRequests::CreateComplianceViolationsSe
it_behaves_like 'does not call process_merge_request' it_behaves_like 'does not call process_merge_request'
end end
context 'when the compliance violations graphql type is disabled' do context 'when the compliance report feature is enabled' do
before do before do
stub_feature_flags(compliance_violations_graphql_type: false)
end
it_behaves_like 'does not call process_merge_request'
end
context 'when the compliance violations graphql type is enabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: true)
stub_licensed_features(group_level_compliance_dashboard: true) stub_licensed_features(group_level_compliance_dashboard: true)
end end
......
...@@ -41,17 +41,8 @@ RSpec.describe MergeRequests::PostMergeService do ...@@ -41,17 +41,8 @@ RSpec.describe MergeRequests::PostMergeService do
it_behaves_like 'does not call the compliance violations worker' it_behaves_like 'does not call the compliance violations worker'
end end
context 'when the compliance violations graphql type is disabled' do context 'when the compliance report feature is licensed' do
before do before do
stub_feature_flags(compliance_violations_graphql_type: false)
end
it_behaves_like 'does not call the compliance violations worker'
end
context 'when the compliance violations graphql type is enabled' do
before do
stub_feature_flags(compliance_violations_graphql_type: true)
stub_licensed_features(group_level_compliance_dashboard: true) stub_licensed_features(group_level_compliance_dashboard: true)
end end
......
...@@ -20,8 +20,6 @@ RSpec.describe 'getting group information' do ...@@ -20,8 +20,6 @@ RSpec.describe 'getting group information' do
fields = all_graphql_fields_for('Group') fields = all_graphql_fields_for('Group')
# TODO: Set required timelogs args elsewhere https://gitlab.com/gitlab-org/gitlab/-/issues/325499 # TODO: Set required timelogs args elsewhere https://gitlab.com/gitlab-org/gitlab/-/issues/325499
fields.selection['timelogs(startDate: "2021-03-01" endDate: "2021-03-30")'] = fields.selection.delete('timelogs') fields.selection['timelogs(startDate: "2021-03-01" endDate: "2021-03-30")'] = fields.selection.delete('timelogs')
# TODO: Remove when `compliance_violations_graphql_type` feature flag is removed in https://gitlab.com/gitlab-org/gitlab/-/issues/350249
fields.selection.delete('mergeRequestViolations')
graphql_query_for( graphql_query_for(
'group', 'group',
......
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