Merge branch '10242-2-rename-vulns-to-findings-in-groups-routes' into 'master'

Rename Vulnerabilities to Findings in Groups security routes

See merge request gitlab-org/gitlab!17355

ee/app/controllers/concerns/vulnerability_findings_history.rb

+# frozen_string_literal: true
+
+module VulnerabilityFindingsHistory
+  extend ActiveSupport::Concern
+
+  included do
+    def history
+      history_count = Gitlab::Vulnerabilities::History.new(group, filter_params).findings_counter
+
+      respond_to do |format|
+        format.json do
+          render json: history_count
+        end
+      end
+    end
+  end
+end

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

 # frozen_string_literal: true
 
 class Groups::Security::VulnerabilitiesController < Groups::ApplicationController
+  include VulnerabilitiesApiFeatureGate # must come first
   include SecurityDashboardsPermissions
   include VulnerabilityFindingsActions
+  include VulnerabilityFindingsHistory
 
   alias_method :vulnerable, :group
 
-  def history
-    history_count = Gitlab::Vulnerabilities::History.new(group, filter_params).findings_counter
+  private
 
-    respond_to do |format|
-      format.json do
-        render json: history_count
-      end
-    end
+  def vulnerabilities_action_enabled?
+    Feature.disabled?(:vulnerability_findings_api)
   end
 end

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

+# frozen_string_literal: true
+
+class Groups::Security::VulnerabilityFindingsController < Groups::ApplicationController
+  include VulnerabilitiesApiFeatureGate # must come first
+  include SecurityDashboardsPermissions
+  include VulnerabilityFindingsActions
+  include VulnerabilityFindingsHistory
+
+  alias_method :vulnerable, :group
+
+  private
+
+  def vulnerabilities_action_enabled?
+    Feature.enabled?(:vulnerability_findings_api)
+  end
+end

ee/app/helpers/ee/groups_helper.rb

@@ -53,6 +53,37 @@ module EE
       @group.dependency_proxy_feature_available?
     end
 
+    def group_path_params(group)
+      { group_id: group }
+    end
+
+    def group_vulnerabilities_endpoint_path(group)
+      params = group_path_params(group)
+      if ::Feature.enabled?(:vulnerability_findings_api)
+        group_security_vulnerability_findings_path(params)
+      else
+        group_security_vulnerabilities_path(params)
+      end
+    end
+
+    def group_vulnerabilities_summary_endpoint_path(group)
+      params = group_path_params(group)
+      if ::Feature.enabled?(:vulnerability_findings_api)
+        summary_group_security_vulnerability_findings_path(params)
+      else
+        summary_group_security_vulnerabilities_path(params)
+      end
+    end
+
+    def group_vulnerabilities_history_endpoint_path(group)
+      params = group_path_params(group)
+      if ::Feature.enabled?(:vulnerability_findings_api)
+        history_group_security_vulnerability_findings_path(params)
+      else
+        history_group_security_vulnerabilities_path(params)
+      end
+    end
+
     private
 
     def get_group_sidebar_links

ee/app/views/groups/security/dashboard/show.html.haml

 - breadcrumb_title _("Security Dashboard")
 - page_title _("Security Dashboard")
 
-#js-group-security-dashboard{ data: { vulnerabilities_endpoint: group_security_vulnerabilities_path(@group),
-  vulnerabilities_summary_endpoint: summary_group_security_vulnerabilities_path(@group),
-  vulnerabilities_history_endpoint: history_group_security_vulnerabilities_path(@group),
+#js-group-security-dashboard{ data: { vulnerabilities_endpoint: group_vulnerabilities_endpoint_path(@group),
+  vulnerabilities_summary_endpoint: group_vulnerabilities_summary_endpoint_path(@group),
+  vulnerabilities_history_endpoint: group_vulnerabilities_history_endpoint_path(@group),
   projects_endpoint: expose_url(api_v4_groups_projects_path(id: @group.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/config/routes/group.rb

@@ -110,7 +110,18 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
 
     namespace :security do
       resource :dashboard, only: [:show], controller: :dashboard
-      resources :vulnerabilities, only: [:index], controller: :vulnerabilities do
+      # We have to define both legacy and new routes for Vulnerability Findings
+      # because they are loaded upon application initialization and preloaded by
+      # web server.
+      # TODO: remove this comment and `resources :vulnerabilities` when feature flag is removed
+      # see https://gitlab.com/gitlab-org/gitlab/issues/33488
+      resources :vulnerabilities, only: [:index] do
+        collection do
+          get :summary
+          get :history
+        end
+      end
+      resources :vulnerability_findings, only: [:index] do
         collection do
           get :summary
           get :history

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

@@ -4,125 +4,41 @@ require 'spec_helper'
 
 describe Groups::Security::VulnerabilitiesController do
   let(:group) { create(:group) }
+  let(:params) { { group_id: group } }
   let(:user) { create(:user) }
 
-  it_behaves_like VulnerabilityFindingsActions do
-    let(:vulnerable) { group }
-    let(:vulnerable_params) { { group_id: group } }
-  end
+  # when new Vulnerability Findings API is enabled this controller is not,
+  # its actions are "moved" Groups::Security::VulnerabilityFindingsController
 
-  it_behaves_like SecurityDashboardsPermissions do
+  it_behaves_like 'VulnerabilityFindingsActions disabled' do
     let(:vulnerable) { group }
-    let(:security_dashboard_action) { get :index, params: { group_id: group }, format: :json }
+    let(:vulnerable_params) { params }
   end
 
-  before do
-    sign_in(user)
-    stub_licensed_features(security_dashboard: true)
-    group.add_developer(user)
-  end
-
-  describe 'GET index.json' do
-    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 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(2)
-    end
+  it_behaves_like 'SecurityDashboardsPermissions disabled' do
+    let(:vulnerable) { group }
+    let(:security_dashboard_action) { get :index, params: params, format: :json }
   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) }
-
-    subject { get :history, params: params, format: :json }
+  it_behaves_like 'disabled group vulnerability findings controller'
 
+  context 'when new Vulnerability Findings API is disabled' do
     before do
-      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
+      stub_feature_flags(vulnerability_findings_api: false)
     end
 
-    it 'returns vulnerability history within last 90 days' do
-      travel_to(Time.zone.parse('2019-02-11')) do
-        subject
-      end
+    # when new Vulnerability Findings API is disabled, we fall back to this controller
 
-      expect(response).to have_gitlab_http_status(200)
-      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')
+    it_behaves_like VulnerabilityFindingsActions do
+      let(:vulnerable) { group }
+      let(:vulnerable_params) { params }
     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({
-        "undefined" => {},
-        "info" => {},
-        "unknown" => {},
-        "low" => {},
-        "medium" => {},
-        "high" => {},
-        "critical" => {},
-        "total" => {}
-      })
+    it_behaves_like SecurityDashboardsPermissions do
+      let(:vulnerable) { group }
+      let(:security_dashboard_action) { get :index, params: params, format: :json }
     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
+    it_behaves_like 'group vulnerability findings controller'
   end
 end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Groups::Security::VulnerabilityFindingsController do
+  let(:group) { create(:group) }
+  let(:params) { { group_id: group } }
+  let(:user) { create(:user) }
+
+  # when new Vulnerability Findings API is enabled, this controller is enabled as well
+
+  it_behaves_like VulnerabilityFindingsActions 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
+
+  it_behaves_like 'group vulnerability findings controller'
+
+  context 'when new Vulnerability Findings API is disabled' do
+    before do
+      stub_feature_flags(vulnerability_findings_api: false)
+    end
+
+    # when new Vulnerability Findings API is disabled, this controller is disabled as well
+    # and its actions are "moved" to Groups::Security::VulnerabilitiesController
+
+    it_behaves_like 'VulnerabilityFindingsActions disabled' do
+      let(:vulnerable) { group }
+      let(:vulnerable_params) { params }
+    end
+
+    it_behaves_like 'SecurityDashboardsPermissions disabled' do
+      let(:vulnerable) { group }
+      let(:security_dashboard_action) { get :index, params: params, format: :json }
+    end
+
+    it_behaves_like 'disabled group vulnerability findings controller'
+  end
+end

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -50,4 +50,24 @@ describe GroupsHelper do
       expect(helper.group_sidebar_links).not_to include(:contribution_analytics, :epics)
     end
   end
+
+  context 'when new Vulnerability Findings API enabled' do
+    it 'returns new "vulnerability findings" endpoint paths' do
+      expect(helper.group_vulnerabilities_endpoint_path(group)).to eq group_security_vulnerability_findings_path(group)
+      expect(helper.group_vulnerabilities_summary_endpoint_path(group)).to eq summary_group_security_vulnerability_findings_path(group)
+      expect(helper.group_vulnerabilities_history_endpoint_path(group)).to eq history_group_security_vulnerability_findings_path(group)
+    end
+  end
+
+  context 'when new Vulnerability Findings API disabled' do
+    before do
+      stub_feature_flags(vulnerability_findings_api: false)
+    end
+
+    it 'returns legacy "vulnerabilities" endpoint paths' do
+      expect(helper.group_vulnerabilities_endpoint_path(group)).to eq group_security_vulnerabilities_path(group)
+      expect(helper.group_vulnerabilities_summary_endpoint_path(group)).to eq summary_group_security_vulnerabilities_path(group)
+      expect(helper.group_vulnerabilities_history_endpoint_path(group)).to eq history_group_security_vulnerabilities_path(group)
+    end
+  end
 end

ee/spec/support/shared_examples/controllers/groups/security/vulnerabilities_controller_disabled_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'disabled group vulnerability findings controller' do
+  describe 'GET index.json' do
+    it 'is disabled and returns "not found" response' do
+      get :index, params: { group_id: group }, format: :json
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+
+  describe 'GET history.json' do
+    it 'is disabled and returns "not found" response' do
+      get :history, params: { group_id: group }, format: :json
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+  end
+end

ee/spec/support/shared_examples/controllers/groups/security/vulnerabilities_controller_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'group vulnerability findings controller' do
+  before do
+    sign_in(user)
+    stub_licensed_features(security_dashboard: true)
+    group.add_developer(user)
+  end
+
+  describe 'GET index.json' do
+    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 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(2)
+    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) }
+
+    subject { get :history, params: params, format: :json }
+
+    before do
+      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
+
+    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(200)
+      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({
+        "undefined" => {},
+        "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