Merge branch '207448_cleanup_vulnerability_findings_controllers' into 'master'

Cleanup findings related controllers

See merge request gitlab-org/gitlab!38193

ee/app/controllers/concerns/project_collection_vulnerability_findings_actions.rb

-# frozen_string_literal: true
-
-module ProjectCollectionVulnerabilityFindingsActions
-  extend ActiveSupport::Concern
-  include ProjectVulnerabilityFindingsActions
-
-  included do
-    def history
-      history_count = Gitlab::Vulnerabilities::History.new(vulnerable, params: filter_params).findings_counter
-
-      respond_to do |format|
-        format.json do
-          render json: history_count
-        end
-      end
-    end
-  end
-end

ee/app/controllers/concerns/project_vulnerability_findings_actions.rb

-# frozen_string_literal: true
-
-# The ProjectVulnerabilityFindingsActions concern contains actions that are used to populate findings
-# on security dashboards.
-#
-# Note: Consumers of this module will need to define a `def vulnerable` method, which must return
-# an object that includes an `#all_pipelines` method
-
-module ProjectVulnerabilityFindingsActions
-  extend ActiveSupport::Concern
-
-  def index
-    pipeline_ids = vulnerable.all_pipelines.with_vulnerabilities.latest_successful_ids_per_project
-    findings = ::Security::VulnerabilityFindingsFinder.new(pipeline_ids, params: filter_params, include_sha: true).execute
-    ordered_findings = findings.ordered.page(params[:page])
-
-    respond_to do |format|
-      format.json do
-        render json: Vulnerabilities::FindingSerializer
-          .new(current_user: current_user)
-          .with_pagination(request, response)
-          .represent(ordered_findings, preload: true)
-      end
-    end
-  end
-
-  def summary
-    summary = Gitlab::Vulnerabilities::Summary.new(vulnerable, filter_params).findings_counter
-
-    respond_to do |format|
-      format.json do
-        render json: summary
-      end
-    end
-  end
-
-  private
-
-  def filter_params
-    params.permit(
-      :scope,
-      report_type: [],
-      confidence: [],
-      project_id: [],
-      severity: []
-    ).tap do |vulnerability_params|
-      if vulnerable.is_a?(Group) && params[:project_id].blank?
-        # This is a temporary change that will go away with Standalone Vulnerabilities
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/13561
-        # For Groups, supply all the project ids to force usage of the SQL index
-        # as when we have no project ids supplied, we want all projects.
-        vulnerability_params.merge!(project_id: Project.for_group_and_its_subgroups(vulnerable).pluck_primary_key)
-      end
-    end
-  end
-end

ee/app/controllers/groups/security/vulnerability_findings_controller.rb

-# frozen_string_literal: true
-
-class Groups::Security::VulnerabilityFindingsController < Groups::ApplicationController
-  include SecurityDashboardsPermissions
-  include ProjectCollectionVulnerabilityFindingsActions
-
-  alias_method :vulnerable, :group
-end

ee/app/controllers/projects/security/vulnerability_findings_controller.rb

-# frozen_string_literal: true
-
-class Projects::Security::VulnerabilityFindingsController < Projects::ApplicationController
-  include SecurityDashboardsPermissions
-  include ProjectVulnerabilityFindingsActions
-
-  alias_method :vulnerable, :project
-end

ee/app/controllers/security/vulnerability_findings_controller.rb

-# frozen_string_literal: true
-
-module Security
-  class VulnerabilityFindingsController < ::Security::ApplicationController
-    include ProjectCollectionVulnerabilityFindingsActions
-  end
-end

ee/app/helpers/ee/projects_helper.rb

@@ -191,8 +191,6 @@ module EE
           has_vulnerabilities: 'true',
           project: { id: project.id, name: project.name },
           project_full_path: project.full_path,
-          vulnerabilities_endpoint: project_security_vulnerability_findings_path(project),
-          vulnerabilities_summary_endpoint: summary_project_security_vulnerability_findings_path(project),
           vulnerabilities_export_endpoint: api_v4_security_projects_vulnerability_exports_path(id: project.id),
           vulnerability_feedback_help_path: help_page_path("user/application_security/index", anchor: "interacting-with-the-vulnerabilities"),
           empty_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),

ee/app/helpers/groups/security_features_helper.rb

@@ -28,8 +28,6 @@ module Groups::SecurityFeaturesHelper
 
   def group_level_security_dashboard_data(group)
     {
-      vulnerabilities_endpoint: group_security_vulnerability_findings_path(group),
-      vulnerabilities_history_endpoint: history_group_security_vulnerability_findings_path(group),
       projects_endpoint: expose_url(api_v4_groups_projects_path(id: group.id)),
       group_full_path: group.full_path,
       vulnerability_feedback_help_path: help_page_path("user/application_security/index", anchor: "interacting-with-the-vulnerabilities"),

ee/app/helpers/security_helper.rb

@@ -10,8 +10,6 @@ module SecurityHelper
       project_add_endpoint: security_projects_path,
       project_list_endpoint: security_projects_path,
       vulnerable_projects_endpoint: security_vulnerable_projects_path,
-      vulnerabilities_endpoint: security_vulnerability_findings_path,
-      vulnerabilities_history_endpoint: history_security_vulnerability_findings_path,
       vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities'),
       vulnerabilities_export_endpoint: expose_path(api_v4_security_vulnerability_exports_path)
     }

ee/config/routes/group.rb

@@ -151,13 +151,6 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
       resources :vulnerable_projects, only: [:index]
       resource :discover, only: [:show], controller: :discover
       resources :credentials, only: [:index]
-
-      resources :vulnerability_findings, only: [:index] do
-        collection do
-          get :summary
-          get :history
-        end
-      end
     end
 
     resource :push_rules, only: [:edit, :update]

ee/config/routes/project.rb

@@ -67,12 +67,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
           resource :discover, only: [:show], controller: :discover
 
-          resources :vulnerability_findings, only: [:index] do
-            collection do
-              get :summary
-            end
-          end
-
           resources :scanned_resources, only: [:index]
 
           resources :vulnerabilities, only: [:show] do

ee/config/routes/security.rb

@@ -5,11 +5,4 @@ namespace :security do
 
   resources :projects, only: [:index, :create, :destroy]
   resources :vulnerable_projects, only: [:index]
-
-  resources :vulnerability_findings, only: [:index] do
-    collection do
-      get :summary
-      get :history
-    end
-  end
 end

ee/spec/controllers/groups/security/vulnerability_findings_controller_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Groups::Security::VulnerabilityFindingsController do
-  let(:group) { create(:group) }
-  let(:sub_group) { create(:group, parent: group) }
-  let(:params) { { group_id: group } }
-  let(:user) { create(:user) }
-
-  it_behaves_like ProjectVulnerabilityFindingsActions do
-    let(:vulnerable) { group }
-    let(:vulnerable_params) { params }
-  end
-
-  it_behaves_like SecurityDashboardsPermissions do
-    let(:vulnerable) { group }
-    let(:security_dashboard_action) { get :index, params: params, format: :json }
-  end
-
-  describe 'GET index.json' do
-    before do
-      sign_in(user)
-      stub_licensed_features(security_dashboard: true)
-      group.add_developer(user)
-    end
-
-    it 'returns vulnerabilities for all projects in the group' do
-      # create projects for the group
-      2.times do
-        project = create(:project, namespace: group)
-        pipeline = create(:ci_pipeline, :success, project: project)
-
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
-      end
-
-      # create a sub group project to ensure we include it
-      sub_group_project = create(:project, namespace: sub_group)
-      pipeline = create(:ci_pipeline, :success, project: sub_group_project)
-      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: sub_group_project, severity: :high)
-
-      # create an ungrouped project to ensure we don't include it
-      project = create(:project)
-      pipeline = create(:ci_pipeline, :success, project: project)
-      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: project, severity: :high)
-
-      get :index, params: { group_id: group }, format: :json
-
-      expect(json_response.count).to be(3)
-    end
-  end
-
-  describe 'GET history.json' do
-    let(:params) { { group_id: group } }
-    let(:project) { create(:project, namespace: group) }
-    let(:pipeline) { create(:ci_pipeline, :success, project: project) }
-
-    before do
-      sign_in(user)
-      stub_licensed_features(security_dashboard: true)
-      group.add_developer(user)
-
-      travel_to(Time.zone.parse('2018-11-10')) do
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :sast,
-                severity: :critical)
-
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :dependency_scanning,
-                severity: :low)
-      end
-
-      travel_to(Time.zone.parse('2018-11-12')) do
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :sast,
-                severity: :critical)
-
-        create(:vulnerabilities_occurrence,
-                pipelines: [pipeline],
-                project: project,
-                report_type: :dependency_scanning,
-                severity: :low)
-      end
-    end
-
-    subject { get :history, params: params, format: :json }
-
-    it 'returns vulnerability history within last 90 days' do
-      travel_to(Time.zone.parse('2019-02-11')) do
-        subject
-      end
-
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(json_response['total']).to eq({ '2018-11-12' => 2 })
-      expect(json_response['critical']).to eq({ '2018-11-12' => 1 })
-      expect(json_response['low']).to eq({ '2018-11-12' => 1 })
-      expect(response).to match_response_schema('vulnerabilities/history', dir: 'ee')
-    end
-
-    it 'returns empty history if there are no vulnerabilities within last 90 days' do
-      travel_to(Time.zone.parse('2019-02-13')) do
-        subject
-      end
-
-      expect(json_response).to eq({
-        "info" => {},
-        "unknown" => {},
-        "low" => {},
-        "medium" => {},
-        "high" => {},
-        "critical" => {},
-        "total" => {}
-      })
-    end
-
-    context 'with a report type filter' do
-      let(:params) { { group_id: group, report_type: %w[sast] } }
-
-      before do
-        travel_to(Time.zone.parse('2019-02-11')) do
-          subject
-        end
-      end
-
-      it 'returns filtered history if filters are enabled' do
-        expect(json_response['total']).to eq({ '2018-11-12' => 1 })
-        expect(json_response['critical']).to eq({ '2018-11-12' => 1 })
-        expect(json_response['low']).to eq({})
-      end
-    end
-  end
-end

ee/spec/controllers/projects/security/dashboard_controller_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe Projects::Security::DashboardController do
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to render_template(:index)
-        expect(response.body).not_to have_css('div#js-security-report-app[data-vulnerabilities-endpoint]')
+        expect(response.body).to have_css('div#js-security-report-app[data-has-vulnerabilities="false"]')
       end
     end
 
@@ -50,7 +50,7 @@ RSpec.describe Projects::Security::DashboardController do
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to render_template(:index)
-        expect(response.body).to have_css('div#js-security-report-app[data-vulnerabilities-endpoint]')
+        expect(response.body).to have_css('div#js-security-report-app[data-has-vulnerabilities="true"]')
       end
     end
   end

ee/spec/controllers/projects/security/vulnerability_findings_controller_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Projects::Security::VulnerabilityFindingsController do
-  let(:project) { create(:project) }
-  let(:params) { { project_id: project, namespace_id: project.creator } }
-
-  it_behaves_like ProjectVulnerabilityFindingsActions do
-    let(:vulnerable) { project }
-    let(:vulnerable_params) { params }
-  end
-
-  it_behaves_like SecurityDashboardsPermissions do
-    let(:vulnerable) { project }
-    let(:security_dashboard_action) { get :index, params: params, format: :json }
-  end
-end

ee/spec/helpers/groups/security_features_helper_spec.rb

@@ -123,8 +123,6 @@ RSpec.describe Groups::SecurityFeaturesHelper do
   describe '#group_level_security_dashboard_data' do
     let(:expected_data) do
       {
-        vulnerabilities_endpoint: "/groups/#{group.full_path}/-/security/vulnerability_findings",
-        vulnerabilities_history_endpoint: "/groups/#{group.full_path}/-/security/vulnerability_findings/history",
         projects_endpoint: "http://localhost/api/v4/groups/#{group.id}/projects",
         group_full_path: group.full_path,
         no_vulnerabilities_svg_path: '/images/illustrations/issues.svg',

ee/spec/helpers/projects_helper_spec.rb

@@ -151,8 +151,6 @@ RSpec.describe ProjectsHelper do
           has_vulnerabilities: 'true',
           project: { id: project.id, name: project.name },
           project_full_path: project.full_path,
-          vulnerabilities_endpoint: "/#{project.full_path}/-/security/vulnerability_findings",
-          vulnerabilities_summary_endpoint: "/#{project.full_path}/-/security/vulnerability_findings/summary",
           vulnerabilities_export_endpoint: "/api/v4/security/projects/#{project.id}/vulnerability_exports",
           vulnerability_feedback_help_path: '/help/user/application_security/index#interacting-with-the-vulnerabilities',
           no_vulnerabilities_svg_path: start_with('/assets/illustrations/issues-'),

ee/spec/helpers/security_helper_spec.rb

@@ -15,8 +15,6 @@ RSpec.describe SecurityHelper do
         project_add_endpoint: security_projects_path,
         project_list_endpoint: security_projects_path,
         vulnerable_projects_endpoint: security_vulnerable_projects_path,
-        vulnerabilities_endpoint: security_vulnerability_findings_path,
-        vulnerabilities_history_endpoint: history_security_vulnerability_findings_path,
         vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities'),
         vulnerabilities_export_endpoint: api_v4_security_vulnerability_exports_path
       })

ee/spec/routing/group_routing_spec.rb

@@ -25,18 +25,6 @@ RSpec.describe 'Group routing', "routing" do
     it 'shows group dashboard' do
       expect(get('/groups/gitlabhq/-/security/dashboard')).to route_to('groups/security/dashboard#show', group_id: 'gitlabhq')
     end
-
-    it 'lists vulnerabilities' do
-      expect(get('/groups/gitlabhq/-/security/vulnerability_findings')).to route_to('groups/security/vulnerability_findings#index', group_id: 'gitlabhq')
-    end
-
-    it 'shows vulnerability summary' do
-      expect(get('/groups/gitlabhq/-/security/vulnerability_findings/summary')).to route_to('groups/security/vulnerability_findings#summary', group_id: 'gitlabhq')
-    end
-
-    it 'shows vulnerability history' do
-      expect(get('/groups/gitlabhq/-/security/vulnerability_findings/history')).to route_to('groups/security/vulnerability_findings#history', group_id: 'gitlabhq')
-    end
   end
 
   describe 'dependency proxy for containers' do

ee/spec/support/shared_examples/controllers/concerns/project_vulnerability_findings_actions_shared_examples.rb

-# frozen_string_literal: true
-
-RSpec.shared_examples ProjectVulnerabilityFindingsActions do
-  include ApiHelpers
-  include VulnerableHelpers
-
-  let(:action_params) { vulnerable_params }
-  let(:user) { create(:user) }
-  let(:pipeline) { create(:ci_pipeline, :success, project: vulnerable_project) }
-  let(:vulnerable_project) { as_vulnerable_project(vulnerable) }
-
-  before do
-    vulnerable.add_developer(user)
-
-    sign_in(user)
-    stub_licensed_features(security_dashboard: true)
-  end
-
-  describe 'GET index.json' do
-    subject { get :index, params: action_params, format: :json }
-
-    it 'returns an ordered list of vulnerability findings' do
-      critical_vulnerability = create(
-        :vulnerabilities_occurrence,
-        pipelines: [pipeline],
-        project: vulnerable_project,
-        severity: :critical
-      )
-      create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, severity: :high)
-
-      subject
-
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(json_response.length).to eq 2
-      expect(json_response.first['id']).to be(critical_vulnerability.id)
-
-      blob_path = project_blob_path(vulnerable_project, File.join(pipeline.sha, critical_vulnerability.location['file'])).tap do |path|
-        path << "#L#{critical_vulnerability.location['start_line']}"
-        path << "-#{critical_vulnerability.location['end_line']}"
-      end
-
-      expect(json_response.first['blob_path']).to eq(blob_path)
-      expect(response).to match_response_schema('vulnerabilities/finding_list', dir: 'ee')
-    end
-
-    context 'when a specific page is requested' do
-      let(:action_params) { vulnerable_params.merge(page: 2) }
-
-      before do
-        Vulnerabilities::Finding.paginates_per 2
-
-        create_list(:vulnerabilities_occurrence, 3, pipelines: [pipeline], project: vulnerable_project)
-
-        subject
-      end
-
-      after do
-        Vulnerabilities::Finding.paginates_per Vulnerabilities::Finding::FINDINGS_PER_PAGE
-      end
-
-      it 'returns the list of vulnerability findings that are on the requested page' do
-        expect(json_response.length).to eq 1
-      end
-    end
-
-    context 'when the vulnerability findings have feedback' do
-      before do
-        vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
-        create(:vulnerability_feedback,
-                :sast,
-                :issue,
-                pipeline: pipeline,
-                issue: create(:issue, project: vulnerable_project),
-                project: vulnerable_project,
-                project_fingerprint: vulnerability.project_fingerprint)
-      end
-
-      it 'avoids N+1 queries', :request_store do
-        control_count = ActiveRecord::QueryRecorder.new { subject }
-
-        vulnerability = create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
-        create(:vulnerability_feedback,
-                :sast,
-                :issue,
-                pipeline: pipeline,
-                issue: create(:issue, project: vulnerable_project),
-                project: vulnerable_project,
-                project_fingerprint: vulnerability.project_fingerprint)
-
-        expect { subject }.not_to exceed_all_query_limit(control_count)
-      end
-    end
-
-    context 'with multiple report types' do
-      before do
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :sast)
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :dast)
-        create(:vulnerabilities_occurrence, pipelines: [pipeline], project: vulnerable_project, report_type: :dependency_scanning)
-
-        subject
-      end
-
-      context 'with a single report filter' do
-        let(:action_params) { vulnerable_params.merge(report_type: ['sast']) }
-
-        it 'returns a list of vulnerability findings for that report type only' do
-          expect(json_response.length).to eq 1
-          expect(json_response.map { |v| v['report_type'] }.uniq).to contain_exactly('sast')
-        end
-      end
-
-      context 'with multiple report filters' do
-        let(:action_params) { vulnerable_params.merge(report_type: %w[sast dependency_scanning]) }
-
-        it 'returns a list of vulnerability findings for all filtered upon types' do
-          expect(json_response.length).to eq 2
-          expect(json_response.map { |v| v['report_type'] }.uniq).to contain_exactly('sast', 'dependency_scanning')
-        end
-      end
-    end
-  end
-
-  describe 'GET summary.json' do
-    subject { get :summary, params: action_params, format: :json }
-
-    before do
-      create_list(:vulnerabilities_occurrence, 3,
-        pipelines: [pipeline], project: vulnerable_project, report_type: :sast, severity: :high)
-      create_list(:vulnerabilities_occurrence, 2,
-        pipelines: [pipeline], project: vulnerable_project, report_type: :dependency_scanning, severity: :low)
-      create_list(:vulnerabilities_occurrence, 1,
-        pipelines: [pipeline], project: vulnerable_project, report_type: :dast, severity: :medium)
-      create_list(:vulnerabilities_occurrence, 1,
-        pipelines: [pipeline], project: vulnerable_project, report_type: :sast, severity: :medium)
-
-      subject
-    end
-
-    it 'returns vulnerability findings counts for all report types' do
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(json_response['high']).to eq(3)
-      expect(json_response['low']).to eq(2)
-      expect(json_response['medium']).to eq(2)
-      expect(response).to match_response_schema('vulnerabilities/summary', dir: 'ee')
-    end
-
-    context 'with enabled filters' do
-      let(:action_params) { vulnerable_params.merge(report_type: %w[sast dast], severity: %w[high low]) }
-
-      it 'returns counts for filtered vulnerability findings' do
-        expect(json_response['high']).to eq(3)
-        expect(json_response['low']).to eq(0)
-        expect(json_response['medium']).to eq(0)
-      end
-    end
-  end
-end