Merge branch '213743_extend_graphql_api_to_return_discussions_for_vulnerabilities' into 'master'

Extend GraphQL api to return user notes count for vulnerabilities

See merge request gitlab-org/gitlab!33058

doc/api/graphql/reference/gitlab_schema.graphql

@@ -12008,6 +12008,11 @@ type Vulnerability {
   """
   title: String
 
+  """
+  Number of user notes attached to the vulnerability
+  """
+  userNotesCount: Int!
+
   """
   Permissions for the current user on the resource
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -35537,6 +35537,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "userNotesCount",
+              "description": "Number of user notes attached to the vulnerability",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "Int",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "userPermissions",
               "description": "Permissions for the current user on the resource",

doc/api/graphql/reference/index.md

@@ -1816,6 +1816,7 @@ Represents a vulnerability.
 | `severity` | VulnerabilitySeverity | Severity of the vulnerability (INFO, UNKNOWN, LOW, MEDIUM, HIGH, CRITICAL) |
 | `state` | VulnerabilityState | State of the vulnerability (DETECTED, DISMISSED, RESOLVED, CONFIRMED) |
 | `title` | String | Title of the vulnerability |
+| `userNotesCount` | Int! | Number of user notes attached to the vulnerability |
 | `userPermissions` | VulnerabilityPermissions! | Permissions for the current user on the resource |
 | `vulnerabilityPath` | String | URL to the vulnerability's details page |
 

ee/app/graphql/types/vulnerability_type.rb

@@ -27,6 +27,9 @@ module Types
     field :report_type, VulnerabilityReportTypeEnum, null: true,
           description: "Type of the security report that found the vulnerability (#{::Vulnerabilities::Occurrence::REPORT_TYPES.keys.join(', ').upcase})"
 
+    field :user_notes_count, GraphQL::INT_TYPE, null: false,
+          description: 'Number of user notes attached to the vulnerability'
+
     field :vulnerability_path, GraphQL::STRING_TYPE, null: true,
           description: "URL to the vulnerability's details page",
           resolve: -> (obj, _args, _ctx) { ::Gitlab::Routing.url_helpers.project_security_vulnerability_path(obj.project, obj) }

ee/app/models/vulnerability.rb

@@ -67,28 +67,38 @@ class Vulnerability < ApplicationRecord
   scope :with_states, -> (states) { where(state: states) }
   scope :counts_by_severity, -> { group(:severity).count }
 
-  def self.counts_by_day_and_severity(start_date, end_date)
-    return [] unless Feature.enabled?(:vulnerability_history, default_enabled: true)
-
-    num_days_of_history = end_date - start_date + 1
-
-    # this clause guards against query timeouts
-    raise TooManyDaysError, "Cannot fetch counts for more than #{MAX_DAYS_OF_HISTORY} days" if num_days_of_history > MAX_DAYS_OF_HISTORY
-
-    quoted_start_date = connection.quote(start_date)
-    quoted_end_date = connection.quote(end_date)
-
-    select(
-      'DATE(calendar.entry) AS day, severity, COUNT(*)'
-    ).from(
-      "generate_series(DATE #{quoted_start_date}, DATE #{quoted_end_date}, INTERVAL '1 day') as calendar(entry)"
-    ).joins(
-      'INNER JOIN vulnerabilities ON vulnerabilities.created_at <= calendar.entry'
-    ).where(
-      '(vulnerabilities.dismissed_at IS NULL OR vulnerabilities.dismissed_at > calendar.entry) AND (vulnerabilities.resolved_at IS NULL OR vulnerabilities.resolved_at > calendar.entry)'
-    ).group(
-      :day, :severity
-    )
+  class << self
+    def parent_class
+      ::Project
+    end
+
+    def to_ability_name
+      model_name.singular
+    end
+
+    def counts_by_day_and_severity(start_date, end_date)
+      return [] unless Feature.enabled?(:vulnerability_history, default_enabled: true)
+
+      num_days_of_history = end_date - start_date + 1
+
+      # this clause guards against query timeouts
+      raise TooManyDaysError, "Cannot fetch counts for more than #{MAX_DAYS_OF_HISTORY} days" if num_days_of_history > MAX_DAYS_OF_HISTORY
+
+      quoted_start_date = connection.quote(start_date)
+      quoted_end_date = connection.quote(end_date)
+
+      select(
+        'DATE(calendar.entry) AS day, severity, COUNT(*)'
+      ).from(
+        "generate_series(DATE #{quoted_start_date}, DATE #{quoted_end_date}, INTERVAL '1 day') as calendar(entry)"
+      ).joins(
+        'INNER JOIN vulnerabilities ON vulnerabilities.created_at <= calendar.entry'
+      ).where(
+        '(vulnerabilities.dismissed_at IS NULL OR vulnerabilities.dismissed_at > calendar.entry) AND (vulnerabilities.resolved_at IS NULL OR vulnerabilities.resolved_at > calendar.entry)'
+      ).group(
+        :day, :severity
+      )
+    end
   end
 
   # There will only be one finding associated with a vulnerability for the foreseeable future
@@ -122,11 +132,19 @@ class Vulnerability < ApplicationRecord
     latest_pipeline_with_vulnerability != latest_successful_pipeline_for_default_branch
   end
 
-  def self.parent_class
-    ::Project
+  def user_notes_count
+    user_notes_count_service.count
   end
 
-  def self.to_ability_name
-    model_name.singular
+  def after_note_changed(note)
+    user_notes_count_service.delete_cache unless note.system?
+  end
+  alias_method :after_note_created,   :after_note_changed
+  alias_method :after_note_destroyed, :after_note_changed
+
+  private
+
+  def user_notes_count_service
+    @user_notes_count_service ||= Vulnerabilities::UserNotesCountService.new(self) # rubocop: disable CodeReuse/ServiceClass
   end
 end

ee/app/services/vulnerabilities/user_notes_count_service.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  class UserNotesCountService < ::BaseCountService
+    VERSION = 1
+
+    def initialize(vulnerability)
+      self.vulnerability = vulnerability
+    end
+
+    def relation_for_count
+      vulnerability.notes.user
+    end
+
+    # Overrides super class' #raw method as we are just
+    # storing primitive value in cache.
+    def raw?
+      true
+    end
+
+    def cache_key
+      ['vulnerabilities', 'user_notes_count', VERSION, vulnerability.id]
+    end
+
+    private
+
+    attr_accessor :vulnerability
+  end
+end

ee/changelogs/unreleased/213743_extend_graphql_api_to_return_discussions_for_vulnerabilities.yml

+---
+title: Introduce `userNotesCount` field for VulnerabilityType on GraphQL API
+merge_request: 33058
+author:
+type: added

ee/spec/graphql/types/vulnerability_type_spec.rb

@@ -8,7 +8,7 @@ describe GitlabSchema.types['Vulnerability'] do
   let_it_be(:vulnerability) { create(:vulnerability, project: project) }
 
   let(:fields) do
-    %i[userPermissions id title description state severity report_type vulnerability_path location project]
+    %i[userPermissions id title description user_notes_count state severity report_type vulnerability_path location project]
   end
 
   before do

ee/spec/models/vulnerability_spec.rb

@@ -342,4 +342,71 @@ describe Vulnerability do
       it { is_expected.to eq(user_1.id) }
     end
   end
+
+  describe '#user_notes_count' do
+    let_it_be(:vulnerability) { create(:vulnerability) }
+
+    let(:expected_count) { 10 }
+    let(:mock_service_class) { instance_double(Vulnerabilities::UserNotesCountService, count: expected_count) }
+
+    subject(:user_notes_count) { vulnerability.user_notes_count }
+
+    before do
+      allow(Vulnerabilities::UserNotesCountService).to receive(:new).with(vulnerability).and_return(mock_service_class)
+    end
+
+    it 'delegates the call to Vulnerabilities::UserNotesCountService' do
+      expect(user_notes_count).to eq(expected_count)
+      expect(mock_service_class).to have_received(:count)
+    end
+  end
+
+  describe '#after_note_changed' do
+    let_it_be(:vulnerability) { create(:vulnerability) }
+
+    let(:note) { instance_double(Note, system?: is_system_note?) }
+    let(:mock_service_class) { instance_double(Vulnerabilities::UserNotesCountService, delete_cache: true) }
+
+    subject(:after_note_changed) { vulnerability.after_note_changed(note) }
+
+    before do
+      allow(Vulnerabilities::UserNotesCountService).to receive(:new).with(vulnerability).and_return(mock_service_class)
+    end
+
+    context 'when the changed note is a system note' do
+      let(:is_system_note?) { true }
+
+      it 'does not send #delete_cache message to Vulnerabilities::UserNotesCountService' do
+        after_note_changed
+
+        expect(mock_service_class).not_to have_received(:delete_cache)
+      end
+    end
+
+    context 'when the changed note is not a system note' do
+      let(:is_system_note?) { false }
+
+      it 'sends #delete_cache message to Vulnerabilities::UserNotesCountService' do
+        after_note_changed
+
+        expect(mock_service_class).to have_received(:delete_cache)
+      end
+    end
+  end
+
+  describe '#after_note_created' do
+    let(:after_note_changed_method) { described_class.instance_method(:after_note_changed) }
+
+    subject { described_class.instance_method(:after_note_created) }
+
+    it { is_expected.to eq(after_note_changed_method) }
+  end
+
+  describe '#after_note_destroyed' do
+    let(:after_note_changed_method) { described_class.instance_method(:after_note_changed) }
+
+    subject { described_class.instance_method(:after_note_destroyed) }
+
+    it { is_expected.to eq(after_note_changed_method) }
+  end
 end

ee/spec/services/vulnerabilities/user_notes_count_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Vulnerabilities::UserNotesCountService, :use_clean_rails_memory_store_caching do
+  let_it_be(:vulnerability) { create(:vulnerability) }
+
+  subject { described_class.new(vulnerability) }
+
+  it_behaves_like 'a counter caching service'
+end