ProjectsGrade.grades_for returns same grades for all vulnerables
Summary
When it's given more more than one vulnerable,
Vulnerabilities::ProjectsGrade.grades_for returns the grades of all given vulnerable for each vulnerable.
See !172720 (comment 2217560966)
This means that the grades are summed when multiple vulnerables are requested. However, the product currently only uses queries with one vulnerable at the time. This means that the bug is imperceptible unless using the API directly and querying grades for multiple objects at once.
Steps to reproduce
The following spec pass when it's added to ee/spec/models/vulnerabilities/projects_grade_spec.rb.
project_7 belongs to the other_group, and all other projects belong to group.
context 'when the given vulnerables are groups' do
let(:vulnerables) { [group, other_group] }
it 'returns all letter grades for each vulnerable' do
vulnerables.each do |vulnerable|
expected_projects_grades = [
described_class.new(vulnerable, 'a', [project_1.id, project_7.id]),
described_class.new(vulnerable, 'b', [project_2.id, project_3.id]),
described_class.new(vulnerable, 'c', [project_4.id]),
described_class.new(vulnerable, 'f', [project_5.id])
]
expect(projects_grades[vulnerable].map(&compare_key)).to match_array(expected_projects_grades.map(&compare_key))
end
end
end
diff for spec file
diff --git a/ee/spec/models/vulnerabilities/projects_grade_spec.rb b/ee/spec/models/vulnerabilities/projects_grade_spec.rb
index cbebcc8cdf07..466fb2f810fa 100644
--- a/ee/spec/models/vulnerabilities/projects_grade_spec.rb
+++ b/ee/spec/models/vulnerabilities/projects_grade_spec.rb
@@ -5,12 +5,14 @@
RSpec.describe Vulnerabilities::ProjectsGrade, feature_category: :vulnerability_management do
let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
+ let_it_be(:other_group) { create(:group) }
let_it_be(:project_1) { create(:project, group: group) }
let_it_be(:project_2) { create(:project, group: group) }
let_it_be(:project_3) { create(:project, group: group) }
let_it_be(:project_4) { create(:project, group: group) }
let_it_be(:project_5) { create(:project, group: group) }
let_it_be(:project_6) { create(:project, group: subgroup) }
+ let_it_be(:project_7) { create(:project, group: other_group) }
let_it_be(:archived_project) { create(:project, :archived, group: group).tap { |p| create(:vulnerability_statistic, :grade_a, project: p) } }
let_it_be(:unrelated_project) { create(:project).tap { |p| create(:vulnerability_statistic, :grade_a, project: p) } }
@@ -20,13 +22,15 @@
let_it_be(:vulnerability_statistic_4) { create(:vulnerability_statistic, :grade_c, project: project_4) }
let_it_be(:vulnerability_statistic_5) { create(:vulnerability_statistic, :grade_f, project: project_5) }
let_it_be(:vulnerability_statistic_6) { create(:vulnerability_statistic, :grade_d, project: project_6) }
+ let_it_be(:vulnerability_statistic_7) { create(:vulnerability_statistic, :grade_a, project: project_7) }
describe '.grades_for' do
let(:compare_key) { ->(projects_grade) { [projects_grade.grade, projects_grade.project_ids.sort] } }
let(:include_subgroups) { false }
let(:filter) { nil }
+ let(:vulnerables) { [vulnerable] }
- subject(:projects_grades) { described_class.grades_for([vulnerable], filter: filter, include_subgroups: include_subgroups) }
+ subject(:projects_grades) { described_class.grades_for(vulnerables, filter: filter, include_subgroups: include_subgroups) }
context 'when the given vulnerable is a Group' do
let(:vulnerable) { group }
@@ -103,6 +107,27 @@
end
end
+ context 'when the given vulnerables are groups' do
+ let(:vulnerables) { [group, other_group] }
+
+ # The following expectation captures a buggy behavior that is
+ # covered by https://gitlab.com/gitlab-org/gitlab/-/issues/507992.
+ # It ensures that the behavior remains consistent remains unchanged
+ # as we refactor the code, and until the bug is fixed.
+ it 'returns all letter grades for each vulnerable' do
+ vulnerables.each do |vulnerable|
+ expected_projects_grades = [
+ described_class.new(vulnerable, 'a', [project_1.id, project_7.id]),
+ described_class.new(vulnerable, 'b', [project_2.id, project_3.id]),
+ described_class.new(vulnerable, 'c', [project_4.id]),
+ described_class.new(vulnerable, 'f', [project_5.id])
+ ]
+
+ expect(projects_grades[vulnerable].map(&compare_key)).to match_array(expected_projects_grades.map(&compare_key))
+ end
+ end
+ end
+
context 'when the given vulnerable is an InstanceSecurityDashboard' do
let(:user) { create(:user) }
let(:vulnerable) { InstanceSecurityDashboard.new(user) }
What is the current bug behavior?
.grades_for returns a Hash where each given vulnerable has the grades of all given vulnerable.
What is the expected correct behavior?
.grades_for returns a Hash where each given vulnerable has its own grades.