Merge branch 'jnnkl-sast-reports-flag-backend' into 'master'

Remove Backend Implementation of sast_reports_in_inline_diff Flag

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/141909



Merged-by: Allen Cook <acook@gitlab.com>
Approved-by: Allen Cook <acook@gitlab.com>
Reviewed-by: Jannik Lehmann <jlehmann@gitlab.com>
Reviewed-by: Ahmed Hemdan <ahemdan@gitlab.com>
Co-authored-by: Jannik Lehmann <jlehmann@gitlab.com>

app/graphql/resolvers/codequality_reports_comparer_resolver.rb

@@ -9,8 +9,6 @@ class CodequalityReportsComparerResolver < BaseResolver
     authorize :read_build
 
     def resolve
-      return unless Feature.enabled?(:sast_reports_in_inline_diff, object.project)
-
       authorize!(object.actual_head_pipeline)
 
       object.compare_codequality_reports

app/graphql/types/merge_request_type.rb

@@ -237,17 +237,15 @@ class MergeRequestType < BaseObject
       null: true,
       description: 'List of emoji reactions associated with the merge request.'
 
-    field :prepared_at, Types::TimeType, null: true,
-                                         description: 'Timestamp of when the merge request was prepared.'
-
     field :codequality_reports_comparer,
       type: ::Types::Security::CodequalityReportsComparerType,
       null: true,
-      alpha: { milestone: '16.4' },
-      description: 'Code quality reports comparison reported on the merge request. Returns `null` ' \
-                   'if `sast_reports_in_inline_diff` feature flag is disabled.',
+      description: 'Code quality reports comparison reported on the merge request.',
       resolver: ::Resolvers::CodequalityReportsComparerResolver
 
+    field :prepared_at, Types::TimeType, null: true,
+                                         description: 'Timestamp of when the merge request was prepared.'
+
     field :allows_multiple_assignees,
       GraphQL::Types::Boolean,
       method: :allows_multiple_assignees?,

app/serializers/codequality_degradation_entity.rb

@@ -2,9 +2,7 @@
 
 class CodequalityDegradationEntity < Grape::Entity
   expose :description
-  expose :fingerprint, if: ->(_, options) do
-    Feature.enabled?(:sast_reports_in_inline_diff, options[:request]&.project)
-  end
+  expose :fingerprint
   expose :severity do |degradation|
     degradation.dig(:severity)&.downcase
   end

config/feature_flags/development/sast_reports_in_inline_diff.yml

----
-name: sast_reports_in_inline_diff
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/119975
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/410191
-milestone: '16.0'
-type: development
-group: group::static analysis
-default_enabled: true

doc/api/graphql/reference/index.md

@@ -16550,8 +16550,8 @@ Represents finding.
 | ---- | ---- | ----------- |
 | <a id="comparedsecurityreportfindingdescription"></a>`description` | [`String`](#string) | Description of the vulnerability finding. |
 | <a id="comparedsecurityreportfindingfoundbypipelineiid"></a>`foundByPipelineIid` | [`String`](#string) | IID of the pipeline. |
-| <a id="comparedsecurityreportfindingidentifiers"></a>`identifiers` **{warning-solid}** | [`[VulnerabilityIdentifier!]`](#vulnerabilityidentifier) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Identifiers of the vulnerability finding. Returns `null` if `sast_reports_in_inline_diff` feature flag is disabled. |
-| <a id="comparedsecurityreportfindinglocation"></a>`location` **{warning-solid}** | [`VulnerabilityLocation`](#vulnerabilitylocation) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Location of the vulnerability finding. Returns `null` if `sast_reports_in_inline_diff` feature flag is disabled. |
+| <a id="comparedsecurityreportfindingidentifiers"></a>`identifiers` **{warning-solid}** | [`[VulnerabilityIdentifier!]`](#vulnerabilityidentifier) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Identifiers of the vulnerability finding. |
+| <a id="comparedsecurityreportfindinglocation"></a>`location` **{warning-solid}** | [`VulnerabilityLocation`](#vulnerabilitylocation) | **Introduced** in 16.3. This feature is an Experiment. It can be changed or removed at any time. Location of the vulnerability finding. |
 | <a id="comparedsecurityreportfindingscanner"></a>`scanner` | [`ComparedSecurityReportScanner`](#comparedsecurityreportscanner) | Compared report vulnerability scanner. |
 | <a id="comparedsecurityreportfindingseverity"></a>`severity` | [`VulnerabilitySeverity`](#vulnerabilityseverity) | Severity of the vulnerability finding. |
 | <a id="comparedsecurityreportfindingstate"></a>`state` | [`VulnerabilityState`](#vulnerabilitystate) | Finding status. |
@@ -21598,7 +21598,7 @@ Defines which user roles, users, or groups can merge into a protected branch.
 | <a id="mergerequestavailableautomergestrategies"></a>`availableAutoMergeStrategies` | [`[String!]`](#string) | Array of available auto merge strategies. |
 | <a id="mergerequestawardemoji"></a>`awardEmoji` | [`AwardEmojiConnection`](#awardemojiconnection) | List of emoji reactions associated with the merge request. (see [Connections](#connections)) |
 | <a id="mergerequestblockingmergerequests"></a>`blockingMergeRequests` **{warning-solid}** | [`BlockingMergeRequests`](#blockingmergerequests) | **Introduced** in 16.5. This feature is an Experiment. It can be changed or removed at any time. Merge requests that block another merge request from merging. |
-| <a id="mergerequestcodequalityreportscomparer"></a>`codequalityReportsComparer` **{warning-solid}** | [`CodequalityReportsComparer`](#codequalityreportscomparer) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Code quality reports comparison reported on the merge request. Returns `null` if `sast_reports_in_inline_diff` feature flag is disabled. |
+| <a id="mergerequestcodequalityreportscomparer"></a>`codequalityReportsComparer` | [`CodequalityReportsComparer`](#codequalityreportscomparer) | Code quality reports comparison reported on the merge request. |
 | <a id="mergerequestcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) |
 | <a id="mergerequestcommitcount"></a>`commitCount` | [`Int`](#int) | Number of commits in the merge request. |
 | <a id="mergerequestcommits"></a>`commits` | [`CommitConnection`](#commitconnection) | Merge request commits. (see [Connections](#connections)) |

doc/user/application_security/sast/index.md

@@ -265,6 +265,7 @@ were introduced by the changes made in the merge request.
 
 > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/10959) in GitLab 16.6 with a [flag](../../../administration/feature_flags.md) named `sast_reports_in_inline_diff`. Disabled by default.
 > - Enabled by default in GitLab 16.8.
+> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/410191) in GitLab 16.9.
 
 SAST results display in the merge request **Changes** view. Lines containing SAST
 issues are marked by a symbol beside the gutter. Select the symbol to see the list of issues, then select an issue to see its details.

ee/app/graphql/types/security/finding_reports_comparer/finding_type.rb

@@ -39,7 +39,7 @@ class FindingType < BaseObject
         field :scanner,
           type: FindingReportsComparer::ScannerType,
           null: true,
-          description: 'Compared report vulnerability scanner.'
+          description: 'Compared report vulnerability scanner.', hash_key: "scanner"
 
         field :found_by_pipeline_iid,
           type: GraphQL::Types::String,
@@ -50,46 +50,24 @@ class FindingType < BaseObject
           type: VulnerabilityLocationType,
           null: true,
           alpha: { milestone: '16.3' },
-          description: 'Location of the vulnerability finding. Returns `null` ' \
-                       'if `sast_reports_in_inline_diff` feature flag is disabled.'
+          description: 'Location of the vulnerability finding.'
 
         field :identifiers,
           type: [VulnerabilityIdentifierType],
           null: true,
           alpha: { milestone: '16.3' },
-          description: 'Identifiers of the vulnerability finding. Returns `null` ' \
-                       'if `sast_reports_in_inline_diff` feature flag is disabled.'
+          description: 'Identifiers of the vulnerability finding.', hash_key: "identifiers"
 
         def found_by_pipeline_iid
           object.dig('found_by_pipeline', 'iid')
         end
 
         def location
-          return unless sast_reports_enabled?
-
           object['location'].merge(
             report_type: object['report_type'],
             blob_path: object['blob_path']
           )
         end
-
-        def identifiers
-          return unless sast_reports_enabled?
-
-          object['identifiers']
-        end
-
-        def scanner
-          return unless sast_reports_enabled?
-
-          object['scanner']
-        end
-
-        private
-
-        def sast_reports_enabled?
-          Feature.enabled?(:sast_reports_in_inline_diff, context[:project])
-        end
       end
       # rubocop: enable Graphql/AuthorizeTypes
     end

ee/app/helpers/ee/merge_requests_helper.rb

@@ -21,13 +21,11 @@ def render_items_list(items, separator = "and")
     override :diffs_tab_pane_data
     def diffs_tab_pane_data(project, merge_request, params)
       data = {
-        endpoint_codequality: (codequality_mr_diff_reports_project_merge_request_path(project, merge_request, 'json') if project.licensed_feature_available?(:inline_codequality) && merge_request.has_codequality_mr_diff_report?)
+        endpoint_codequality: (codequality_mr_diff_reports_project_merge_request_path(project, merge_request, 'json') if project.licensed_feature_available?(:inline_codequality) && merge_request.has_codequality_mr_diff_report?),
+        sast_report_available: merge_request.has_sast_reports?.to_s
       }
 
-      if ::Feature.enabled?(:sast_reports_in_inline_diff, project)
-        data[:codequality_report_available] = merge_request.has_codequality_reports?.to_s if project.licensed_feature_available?(:inline_codequality)
-        data[:sast_report_available] = merge_request.has_sast_reports?.to_s
-      end
+      data[:codequality_report_available] = merge_request.has_codequality_reports?.to_s if project.licensed_feature_available?(:inline_codequality)
 
       super.merge(data)
     end

ee/spec/helpers/merge_requests_helper_spec.rb

@@ -70,16 +70,6 @@
             expect(subject[:codequality_report_available]).to eq('false')
           end
         end
-
-        context 'when feature flag is disabled' do
-          before do
-            stub_feature_flags(sast_reports_in_inline_diff: false)
-          end
-
-          it 'does not return the variable' do
-            expect(subject).not_to have_key(:codequality_report_available)
-          end
-        end
       end
 
       context 'when feature is not licensed' do
@@ -107,16 +97,6 @@
           expect(subject[:sast_report_available]).to eq('false')
         end
       end
-
-      context 'when feature flag is disabled' do
-        before do
-          stub_feature_flags(sast_reports_in_inline_diff: false)
-        end
-
-        it 'does not return the variable' do
-          expect(subject).not_to have_key(:sast_report_available)
-        end
-      end
     end
   end
 

ee/spec/requests/api/graphql/merge_requests/finding_reports_comparer_spec.rb

@@ -154,24 +154,6 @@
       post_graphql(query, current_user: user)
     end
 
-    context 'when sast_reports_in_inline_diff FF is disabled' do
-      before_all do
-        stub_feature_flags(sast_reports_in_inline_diff: false)
-      end
-
-      it 'returns null for scanner, identifiers, and location fields' do
-        expect(result['report']['added']).to include(
-          a_hash_including(
-            {
-              location: nil,
-              identifiers: nil,
-              scanner: nil
-            }.deep_stringify_keys
-          )
-        )
-      end
-    end
-
     it 'returns expected data' do
       expect(result).to match(a_hash_including(
         {

spec/requests/api/graphql/merge_requests/codequality_reports_comparer_spec.rb

@@ -125,16 +125,6 @@
       post_graphql(query, current_user: user)
     end
 
-    context 'when when sast_reports_in_inline_diff FF is disabled' do
-      before_all do
-        stub_feature_flags(sast_reports_in_inline_diff: false)
-      end
-
-      it 'returns null for codequality_reports_comparer field' do
-        expect(result).to be_nil
-      end
-    end
-
     it 'returns expected data' do
       expect(result).to match(
         a_hash_including(

spec/serializers/codequality_degradation_entity_spec.rb

@@ -8,18 +8,6 @@
   describe '#as_json' do
     subject { entity.as_json }
 
-    context 'when sast_reports_in_inline_diff is disabled' do
-      before do
-        stub_feature_flags(sast_reports_in_inline_diff: false)
-      end
-
-      let(:codequality_degradation) { build(:codequality_degradation_1) }
-
-      it 'does not contain fingerprint' do
-        expect(subject[:fingerprint]).to be_nil
-      end
-    end
-
     context 'when codequality contains an error' do
       context 'when line is included in location' do
         let(:codequality_degradation) { build(:codequality_degradation_2) }