From 37ca3ffc0f8bbc9c032e3670f71438c83d967236 Mon Sep 17 00:00:00 2001
From: Lukas Eipert <leipert@gitlab.com>
Date: Thu, 26 Sep 2024 15:44:58 +0000
Subject: [PATCH] Make a Vulnerability a proper todo target
Currently we cannot search for todos on vulnerabilities and the GraphQL
endpoint errors out if it encounters a Todo on a vulnerability. Now both
should work just fine.
Changelog: fixed
EE: true
---
.../graphql_shared/possible_types.json | 1 +
doc/api/graphql/reference/index.md | 3 ++
doc/api/todos.md | 2 +-
ee/app/finders/ee/todos_finder.rb | 2 +-
ee/app/graphql/ee/types/todo_target_enum.rb | 1 +
ee/app/graphql/ee/types/todoable_interface.rb | 11 +++++--
ee/app/graphql/types/vulnerability_type.rb | 1 +
ee/app/models/ee/todo.rb | 2 +-
ee/app/models/ee/vulnerability.rb | 1 +
.../ee/types/todoable_interface_spec.rb | 3 +-
.../graphql/types/vulnerability_type_spec.rb | 2 ++
ee/spec/models/todo_spec.rb | 33 +++++++++++++++++--
spec/finders/todos_finder_spec.rb | 5 +--
13 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/app/assets/javascripts/graphql_shared/possible_types.json b/app/assets/javascripts/graphql_shared/possible_types.json
index 81b0fad47a040..d72ad66943409 100644
--- a/app/assets/javascripts/graphql_shared/possible_types.json
+++ b/app/assets/javascripts/graphql_shared/possible_types.json
@@ -179,6 +179,7 @@
"MergeRequest",
"Namespace",
"Project",
+ "Vulnerability",
"WorkItem"
],
"User": [
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 933910880c551..9d39ad712e792 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -34399,6 +34399,7 @@ Represents a vulnerability.
| <a id="vulnerabilitylinks"></a>`links` | [`[VulnerabilityLink!]!`](#vulnerabilitylink) | List of links associated with the vulnerability. |
| <a id="vulnerabilitylocation"></a>`location` | [`VulnerabilityLocation`](#vulnerabilitylocation) | Location metadata for the vulnerability. Its fields depend on the type of security scan that found the vulnerability. |
| <a id="vulnerabilitymergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request that fixes the vulnerability. |
+| <a id="vulnerabilityname"></a>`name` | [`String`](#string) | Name or title of this object. |
| <a id="vulnerabilitypresentondefaultbranch"></a>`presentOnDefaultBranch` | [`Boolean!`](#boolean) | Indicates whether the vulnerability is present on the default branch or not. |
| <a id="vulnerabilityprimaryidentifier"></a>`primaryIdentifier` | [`VulnerabilityIdentifier`](#vulnerabilityidentifier) | Primary identifier of the vulnerability. |
| <a id="vulnerabilityproject"></a>`project` | [`Project`](#project) | Project on which the vulnerability was found. |
@@ -38672,6 +38673,7 @@ Sort options for todos.
| <a id="todotargetenummergerequest"></a>`MERGEREQUEST` | Merge request. |
| <a id="todotargetenumnamespace"></a>`NAMESPACE` | Namespace. |
| <a id="todotargetenumproject"></a>`PROJECT` | Project. |
+| <a id="todotargetenumvulnerability"></a>`VULNERABILITY` | Vulnerability. |
| <a id="todotargetenumworkitem"></a>`WORKITEM` | Work item. |
### `TrainingUrlRequestStatus`
@@ -40855,6 +40857,7 @@ Implementations:
- [`MergeRequest`](#mergerequest)
- [`Namespace`](#namespace)
- [`Project`](#project)
+- [`Vulnerability`](#vulnerability)
- [`WorkItem`](#workitem)
##### Fields
diff --git a/doc/api/todos.md b/doc/api/todos.md
index bd49128f1873e..f84084c8a986f 100644
--- a/doc/api/todos.md
+++ b/doc/api/todos.md
@@ -31,7 +31,7 @@ Parameters:
| `project_id` | integer | no | The ID of a project |
| `group_id` | integer | no | The ID of a group |
| `state` | string | no | The state of the to-do item. Can be either `pending` or `done` |
-| `type` | string | no | The type of to-do item. Can be either `Issue`, `MergeRequest`, `Commit`, `Epic`, `DesignManagement::Design` or `AlertManagement::Alert` |
+| `type` | string | no | The type of to-do item. Can be either `Issue`, `MergeRequest`, `Commit`, `Epic`, `DesignManagement::Design`, `AlertManagement::Alert`, `Project`, `Namespace` or `Vulnerability` |
```shell
curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/todos"
diff --git a/ee/app/finders/ee/todos_finder.rb b/ee/app/finders/ee/todos_finder.rb
index eece83c6e9796..25f0af4a4b5bf 100644
--- a/ee/app/finders/ee/todos_finder.rb
+++ b/ee/app/finders/ee/todos_finder.rb
@@ -4,7 +4,7 @@ module EE
module TodosFinder
extend ActiveSupport::Concern
- EE_TODO_TYPES = (::TodosFinder::TODO_TYPES + %w[Epic]).freeze
+ EE_TODO_TYPES = (::TodosFinder::TODO_TYPES + %w[Epic Vulnerability]).freeze
class_methods do
extend ::Gitlab::Utils::Override
diff --git a/ee/app/graphql/ee/types/todo_target_enum.rb b/ee/app/graphql/ee/types/todo_target_enum.rb
index 6f3ff5926ebce..52422d2e448ad 100644
--- a/ee/app/graphql/ee/types/todo_target_enum.rb
+++ b/ee/app/graphql/ee/types/todo_target_enum.rb
@@ -7,6 +7,7 @@ module TodoTargetEnum
prepended do
value 'EPIC', value: 'Epic', description: 'An Epic.'
+ value 'VULNERABILITY', value: 'Vulnerability', description: 'Vulnerability.'
end
end
end
diff --git a/ee/app/graphql/ee/types/todoable_interface.rb b/ee/app/graphql/ee/types/todoable_interface.rb
index dcf3480c61246..ea46754e16c15 100644
--- a/ee/app/graphql/ee/types/todoable_interface.rb
+++ b/ee/app/graphql/ee/types/todoable_interface.rb
@@ -10,9 +10,14 @@ module TodoableInterface
override :resolve_type
def resolve_type(object, *)
- return ::Types::EpicType if Epic === object
-
- super
+ case object
+ when Epic
+ ::Types::EpicType
+ when Vulnerability
+ ::Types::VulnerabilityType
+ else
+ super
+ end
end
end
end
diff --git a/ee/app/graphql/types/vulnerability_type.rb b/ee/app/graphql/types/vulnerability_type.rb
index 12c4e0d77c531..286017e161705 100644
--- a/ee/app/graphql/types/vulnerability_type.rb
+++ b/ee/app/graphql/types/vulnerability_type.rb
@@ -6,6 +6,7 @@ class VulnerabilityType < BaseObject
description 'Represents a vulnerability'
implements Types::Notes::NoteableInterface
+ implements Types::TodoableInterface
authorize :read_vulnerability
diff --git a/ee/app/models/ee/todo.rb b/ee/app/models/ee/todo.rb
index 85655f318fb79..3cc49b1fb4b26 100644
--- a/ee/app/models/ee/todo.rb
+++ b/ee/app/models/ee/todo.rb
@@ -14,7 +14,7 @@ def target_url
return if target.nil?
case target
- when Epic
+ when Vulnerability, Epic
::Gitlab::UrlBuilder.build(
target,
anchor: note.present? ? ActionView::RecordIdentifier.dom_id(note) : nil
diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb
index a0142ce56840e..61e552840b0e5 100644
--- a/ee/app/models/ee/vulnerability.rb
+++ b/ee/app/models/ee/vulnerability.rb
@@ -3,6 +3,7 @@
module EE
module Vulnerability
include ::Gitlab::Utils::StrongMemoize
+ include Todoable
extend ActiveSupport::Concern
prepended do
diff --git a/ee/spec/graphql/ee/types/todoable_interface_spec.rb b/ee/spec/graphql/ee/types/todoable_interface_spec.rb
index 2ec1f7ba09574..d4d64e2c107c2 100644
--- a/ee/spec/graphql/ee/types/todoable_interface_spec.rb
+++ b/ee/spec/graphql/ee/types/todoable_interface_spec.rb
@@ -2,12 +2,13 @@
require 'spec_helper'
-RSpec.describe Types::TodoableInterface do
+RSpec.describe Types::TodoableInterface, feature_category: :notifications do
let(:extended_class) { described_class }
describe ".resolve_type" do
it 'knows the correct type for EE-only objects' do
expect(extended_class.resolve_type(build(:epic), {})).to eq(Types::EpicType)
+ expect(extended_class.resolve_type(build(:vulnerability), {})).to eq(Types::VulnerabilityType)
end
end
end
diff --git a/ee/spec/graphql/types/vulnerability_type_spec.rb b/ee/spec/graphql/types/vulnerability_type_spec.rb
index bd0ec90a32a5b..7165a37f0652e 100644
--- a/ee/spec/graphql/types/vulnerability_type_spec.rb
+++ b/ee/spec/graphql/types/vulnerability_type_spec.rb
@@ -37,6 +37,7 @@
report_type
resolved_on_default_branch
vulnerability_path
+ name
web_url
location
scanner
@@ -93,6 +94,7 @@
subject(:graphql_response) { GitlabSchema.execute(query, context: { current_user: user }).as_json }
+ it { expect(described_class.interfaces).to include(Types::TodoableInterface) }
it { expect(described_class).to have_graphql_fields(fields) }
it { expect(described_class).to require_graphql_authorizations(:read_vulnerability) }
diff --git a/ee/spec/models/todo_spec.rb b/ee/spec/models/todo_spec.rb
index 9c10bb4e3849e..151bc175c3c4a 100644
--- a/ee/spec/models/todo_spec.rb
+++ b/ee/spec/models/todo_spec.rb
@@ -2,15 +2,16 @@
require 'spec_helper'
-RSpec.describe Todo, feature_category: :team_planning do
+RSpec.describe Todo, feature_category: :notifications do
let_it_be(:current_user) { create(:user) }
- let_it_be(:group) { create(:group, developers: current_user) }
- let_it_be(:epic) { create(:epic, group: group) }
describe '#target_url' do
subject { todo.target_url }
context 'when the todo is coming from an epic' do
+ let_it_be(:group) { create(:group, developers: current_user) }
+ let_it_be(:epic) { create(:epic, group: group) }
+
context 'when coming from the epic itself' do
let_it_be(:todo) { create(:todo, project: nil, group: group, user: current_user, target: epic) }
@@ -28,5 +29,31 @@
end
end
end
+
+ context 'when the todo is coming from a vulnerability' do
+ let_it_be(:project) { create(:project) }
+ let_it_be(:vulnerability) { create(:vulnerability, project: project) }
+
+ context 'when coming from the vulnerability itself' do
+ let_it_be(:todo) do
+ create(:todo, project: project, group: project.group, user: current_user, target: vulnerability)
+ end
+
+ it 'returns the work item web path' do
+ is_expected.to eq("http://localhost/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}")
+ end
+ end
+
+ context 'when coming from a note on the vulnerability' do
+ let_it_be(:note) { create(:note, noteable: vulnerability, project: project) }
+ let_it_be(:todo) do
+ create(:todo, project: project, group: project.group, user: current_user, note: note, target: vulnerability)
+ end
+
+ it 'returns the work item web path with an anchor to the note' do
+ is_expected.to eq("http://localhost/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}#note_#{note.id}")
+ end
+ end
+ end
end
end
diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb
index ef47c66e8b251..1284dbe339448 100644
--- a/spec/finders/todos_finder_spec.rb
+++ b/spec/finders/todos_finder_spec.rb
@@ -340,11 +340,12 @@
describe '.todo_types' do
it 'returns the expected types' do
+ shared_types = %w[Commit Issue WorkItem MergeRequest DesignManagement::Design AlertManagement::Alert Namespace Project]
expected_result =
if Gitlab.ee?
- %w[Epic Commit Issue WorkItem MergeRequest DesignManagement::Design AlertManagement::Alert Namespace Project]
+ %w[Epic Vulnerability] + shared_types
else
- %w[Commit Issue WorkItem MergeRequest DesignManagement::Design AlertManagement::Alert Namespace Project]
+ shared_types
end
expect(described_class.todo_types).to contain_exactly(*expected_result)
--
GitLab