Merge branch '350249-remove-compliance-violations-graphql-type-feature-flag' into 'master'

[Feature flag] Remove compliance_violations_graphql_type

See merge request gitlab-org/gitlab!82603

doc/api/graphql/reference/index.md

@@ -9611,7 +9611,7 @@ Represents a ComplianceFramework associated with a Project.
 
 ### `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
 
@@ -11550,7 +11550,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 
 ##### `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).
 

ee/app/finders/compliance_management/merge_requests/compliance_violations_finder.rb

@@ -26,7 +26,7 @@ module ComplianceManagement
       end
 
       def execute
-        return ::MergeRequests::ComplianceViolation.none unless allowed?
+        return unless allowed?
 
         items = init_collection
 
@@ -42,8 +42,7 @@ module ComplianceManagement
       attr_reader :current_user, :group, :params
 
       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
 
       def init_collection

ee/app/graphql/ee/types/group_type.rb

@@ -105,9 +105,9 @@ module EE
         field :merge_request_violations,
               ::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type,
               null: true,
-              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
+              description: 'Compliance violations reported on merge requests merged within the group.',
+              resolver: ::Resolvers::ComplianceManagement::MergeRequests::ComplianceViolationResolver,
+              authorize: :read_group_compliance_dashboard
 
         def billable_members_count(requested_hosted_plan: nil)
           object.billable_members_count(requested_hosted_plan)

ee/app/graphql/resolvers/compliance_management/merge_requests/compliance_violation_resolver.rb

@@ -4,11 +4,16 @@ module Resolvers
   module ComplianceManagement
     module MergeRequests
       class ComplianceViolationResolver < BaseResolver
+        include Gitlab::Graphql::Authorize::AuthorizeResource
+
         alias_method :group, :object
 
         type ::Types::ComplianceManagement::MergeRequests::ComplianceViolationType.connection_type, null: true
         description 'Compliance violations reported on a merged merge request.'
 
+        authorize :read_group_compliance_dashboard
+        authorizes_object!
+
         argument :filters, Types::ComplianceManagement::MergeRequests::ComplianceViolationInputType,
                  required: false,
                  default_value: {},

ee/app/graphql/types/compliance_management/merge_requests/compliance_violation_type.rb

@@ -5,8 +5,7 @@ module Types
     module MergeRequests
       class ComplianceViolationType < ::Types::BaseObject
         graphql_name 'ComplianceViolation'
-        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.'
+        description 'Compliance violation associated with a merged merge request.'
 
         authorize :read_group_compliance_dashboard
 

ee/app/services/compliance_management/merge_requests/create_compliance_violations_service.rb

@@ -15,8 +15,7 @@ module ComplianceManagement
       private
 
       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

ee/app/services/ee/merge_requests/post_merge_service.rb

@@ -18,8 +18,7 @@ module EE
       private
 
       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

ee/config/feature_flags/development/compliance_violations_graphql_type.yml

----
-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

ee/spec/finders/compliance_management/merge_requests/compliance_violations_finder_spec.rb

@@ -29,83 +29,67 @@ RSpec.describe ComplianceManagement::MergeRequests::ComplianceViolationsFinder d
   describe '#execute' do
     subject { finder.execute }
 
-    context 'when feature is disabled' do
-      before do
-        stub_feature_flags(compliance_violations_graphql_type: false)
-      end
-
-      it 'returns no compliance violations' do
-        expect(subject).to eq(::MergeRequests::ComplianceViolation.none)
+    context 'when the user is unauthorized' do
+      it 'returns nil' do
+        expect(subject).to be_nil
       end
     end
 
-    context 'when feature is enabled' do
+    context 'when the user is authorized' do
       before do
-        stub_feature_flags(compliance_violations_graphql_type: true)
+        group.add_owner(current_user)
       end
 
-      context 'when the user is unauthorized' do
-        it 'returns no compliance violations' do
-          expect(subject).to eq(::MergeRequests::ComplianceViolation.none)
+      context 'without any filters or sorting' do
+        it 'finds all the compliance violations' do
+          expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
         end
       end
 
-      context 'when the user is authorized' do
-        before do
-          group.add_owner(current_user)
-        end
+      context 'filtering the results' do
+        context 'when given an array of project IDs' do
+          let(:params) { { project_ids: [project.id] } }
 
-        context 'without any filters or sorting' do
-          it 'finds all the compliance violations' do
-            expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
+          it 'finds the filtered compliance violations' do
+            expect(subject).to contain_exactly(compliance_violation)
           end
         end
 
-        context 'filtering the results' do
-          context 'when given an array of project IDs' do
-            let(:params) { { project_ids: [project.id] } }
-
-            it 'finds the filtered compliance violations' do
-              expect(subject).to contain_exactly(compliance_violation)
-            end
+        context 'when given merged at dates' do
+          where(:merged_params, :result) do
+            { 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
 
-          context 'when given merged at dates' do
-            where(:merged_params, :result) do
-              { 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 }
+          with_them do
+            let(:params) { merged_params }
 
-              it 'finds the filtered compliance violations' do
-                expect(subject).to contain_exactly(result)
-              end
+            it 'finds the filtered compliance violations' do
+              expect(subject).to contain_exactly(result)
             end
           end
         end
+      end
 
-        context 'sorting the results' do
-          where(:direction, :result) do
-            'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] }
-          end
+      context 'sorting the results' do
+        where(:direction, :result) do
+          'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] }
+        end
 
-          with_them do
-            let(:params) { { sort: direction } }
+        with_them do
+          let(:params) { { sort: direction } }
 
-            it 'finds the filtered compliance violations' do
-              expect(subject).to match_array(result)
-            end
+          it 'finds the filtered compliance violations' do
+            expect(subject).to match_array(result)
           end
         end
       end

ee/spec/graphql/resolvers/compliance_management/merge_requests/compliance_violation_resolver_spec.rb

@@ -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 }) }
 
-    context 'feature flag is disabled' do
-      before do
-        stub_feature_flags(compliance_violations_graphql_type: false)
+    context 'user is unauthorized' do
+      it 'returns nil' do
+        expect(subject).to be_nil
       end
+    end
 
-      it 'returns an empty collection' do
-        expect(subject).to be_empty
+    context 'user is authorized' do
+      before do
+        group.add_owner(current_user)
       end
-    end
 
-    context 'feature flag is enabled' do
-      context 'user is unauthorized' do
-        it 'returns an empty collection' do
-          expect(subject).to be_empty
+      context 'without any filters or sorting' do
+        it 'finds all the compliance violations' do
+          expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
         end
       end
 
-      context 'user is authorized' do
-        before do
-          group.add_owner(current_user)
-        end
+      context 'filtering the results' do
+        context 'when given an array of project IDs' do
+          let(:args) { { filters: { project_ids: [::Gitlab::GlobalId.as_global_id(project.id, model_name: 'Project')] } } }
 
-        context 'without any filters or sorting' do
-          it 'finds all the compliance violations' do
-            expect(subject).to contain_exactly(compliance_violation, compliance_violation2)
+          it 'finds the filtered compliance violations' do
+            expect(subject).to contain_exactly(compliance_violation)
           end
         end
 
-        context 'filtering the results' do
-          context 'when given an array of project IDs' do
-            let(:args) { { filters: { project_ids: [::Gitlab::GlobalId.as_global_id(project.id, model_name: 'Project')] } } }
-
-            it 'finds the filtered compliance violations' do
-              expect(subject).to contain_exactly(compliance_violation)
-            end
+        context 'when given merged at dates' do
+          where(:merged_params, :result) do
+            { 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
 
-          context 'when given merged at dates' do
-            where(:merged_params, :result) do
-              { 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 } }
+          with_them do
+            let(:args) { { filters: merged_params } }
 
-              it 'finds the filtered compliance violations' do
-                expect(subject).to contain_exactly(result)
-              end
+            it 'finds the filtered compliance violations' do
+              expect(subject).to contain_exactly(result)
             end
           end
         end
+      end
 
-        context 'sorting the results' do
-          where(:direction, :result) do
-            'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] }
-            'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] }
-            'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] }
-          end
+      context 'sorting the results' do
+        where(:direction, :result) do
+          'SEVERITY_LEVEL_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'SEVERITY_LEVEL_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'VIOLATION_REASON_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'VIOLATION_REASON_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'MERGE_REQUEST_TITLE_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'MERGE_REQUEST_TITLE_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'MERGED_AT_ASC' | lazy { [compliance_violation, compliance_violation2] }
+          'MERGED_AT_DESC' | lazy { [compliance_violation2, compliance_violation] }
+          'UNKNOWN_SORT' | lazy { [compliance_violation, compliance_violation2] }
+        end
 
-          with_them do
-            let(:args) { { sort: direction } }
+        with_them do
+          let(:args) { { sort: direction } }
 
-            it 'finds the filtered compliance violations' do
-              expect(subject).to match_array(result)
-            end
+          it 'finds the filtered compliance violations' do
+            expect(subject).to match_array(result)
           end
         end
       end

ee/spec/requests/api/graphql/compliance_management/merge_requests/compliance_violations_spec.rb

@@ -70,88 +70,70 @@ RSpec.describe 'getting the compliance violations for a group' do
     merge_request2.metrics.update!(merged_at: 1.day.ago)
   end
 
-  context 'when feature is disabled' do
-    before do
-      stub_feature_flags(compliance_violations_graphql_type: false)
-    end
-
-    it 'returns empty' do
+  context 'when the user is unauthorized' do
+    it 'returns nil' do
       post_graphql(query, current_user: current_user)
 
-      expect(compliance_violations).to be_empty
+      expect(compliance_violations).to be_nil
     end
   end
 
-  context 'when feature is enabled' do
+  context 'when the user is authorized' do
     before do
-      stub_feature_flags(compliance_violations_graphql_type: true)
+      group.add_owner(current_user)
     end
 
-    context 'when the user is unauthorized' do
-      it 'returns empty' do
+    context 'without any filters or sorting' do
+      it 'finds all the compliance violations' do
         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
 
-    context 'when the user is authorized' do
-      before do
-        group.add_owner(current_user)
-      end
-
-      context 'without any filters or sorting' do
+    context 'filtering the results' do
+      context 'when given an array of project IDs' 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
 
-      context 'filtering the results' do
-        context 'when given an array of project IDs' do
-          it 'finds all the compliance violations' do
-            post_graphql(query({ filters: { projectIds: [project.to_global_id.to_s] } }), current_user: current_user)
-
-            expect(compliance_violations).to contain_exactly(violation_output)
-          end
+      context 'when given merged at dates' do
+        where(:merged_params, :result) do
+          { 'mergedBefore' => 2.days.ago.to_date.iso8601 } | lazy { violation_output }
+          { '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
 
-        context 'when given merged at dates' do
-          where(:merged_params, :result) do
-            { 'mergedBefore' => 2.days.ago.to_date.iso8601 } | lazy { violation_output }
-            { '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)
+        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)
-            end
+            expect(compliance_violations).to contain_exactly(result)
           end
         end
       end
+    end
 
-      context 'sorting the results' do
-        where(:direction, :result) do
-          :SEVERITY_LEVEL_ASC | lazy { [violation_output, violation2_output] }
-          :SEVERITY_LEVEL_DESC | lazy { [violation2_output, violation_output] }
-          :VIOLATION_REASON_ASC | lazy { [violation_output, violation2_output] }
-          :VIOLATION_REASON_DESC | lazy { [violation2_output, violation_output] }
-          :MERGE_REQUEST_TITLE_ASC | lazy { [violation_output, violation2_output] }
-          :MERGE_REQUEST_TITLE_DESC | lazy { [violation2_output, violation_output] }
-          :MERGED_AT_ASC | lazy { [violation_output, violation2_output] }
-          :MERGED_AT_DESC | lazy { [violation2_output, violation_output] }
-        end
+    context 'sorting the results' do
+      where(:direction, :result) do
+        :SEVERITY_LEVEL_ASC | lazy { [violation_output, violation2_output] }
+        :SEVERITY_LEVEL_DESC | lazy { [violation2_output, violation_output] }
+        :VIOLATION_REASON_ASC | lazy { [violation_output, violation2_output] }
+        :VIOLATION_REASON_DESC | lazy { [violation2_output, violation_output] }
+        :MERGE_REQUEST_TITLE_ASC | lazy { [violation_output, violation2_output] }
+        :MERGE_REQUEST_TITLE_DESC | lazy { [violation2_output, violation_output] }
+        :MERGED_AT_ASC | lazy { [violation_output, violation2_output] }
+        :MERGED_AT_DESC | lazy { [violation2_output, violation_output] }
+      end
 
-        with_them do
-          it 'finds all the compliance violations' do
-            post_graphql(query({ sort: direction }), current_user: current_user)
+      with_them do
+        it 'finds all the compliance violations' do
+          post_graphql(query({ sort: direction }), current_user: current_user)
 
-            expect(compliance_violations).to match_array(result)
-          end
+          expect(compliance_violations).to match_array(result)
         end
       end
     end

ee/spec/services/compliance_management/merge_requests/create_compliance_violations_service_spec.rb

@@ -26,17 +26,8 @@ RSpec.describe ComplianceManagement::MergeRequests::CreateComplianceViolationsSe
     it_behaves_like 'does not call process_merge_request'
   end
 
-  context 'when the compliance violations graphql type is disabled' do
+  context 'when the compliance report feature is enabled' 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)
     end
 

ee/spec/services/ee/merge_requests/post_merge_service_spec.rb

@@ -41,17 +41,8 @@ RSpec.describe MergeRequests::PostMergeService do
         it_behaves_like 'does not call the compliance violations worker'
       end
 
-      context 'when the compliance violations graphql type is disabled' do
+      context 'when the compliance report feature is licensed' 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)
         end
 

spec/requests/api/graphql/group_query_spec.rb

@@ -20,8 +20,6 @@ RSpec.describe 'getting group information' do
       fields = all_graphql_fields_for('Group')
       # 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')
-      # 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(
         'group',