Commit 603f057a authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'truncate-vulnerability-title' into 'master'

Truncate vulnerability title if longer than 255 characters

See merge request gitlab-org/gitlab!48327
parents eb33b6c0 38ba4223
......@@ -34,7 +34,7 @@ module Vulnerabilities
vulnerability.assign_attributes(
author: @author,
project: @project,
title: finding.name,
title: finding.name.truncate(::Issuable::TITLE_LENGTH_MAX),
state: finding.state,
severity: finding.severity,
severity_overridden: false,
......
......@@ -29,7 +29,7 @@ module Vulnerabilities
def vulnerability_params
{
title: finding.name,
title: finding.name.truncate(::Issuable::TITLE_LENGTH_MAX),
severity: vulnerability.severity_overridden? ? vulnerability.severity : finding.severity,
confidence: vulnerability.confidence_overridden? ? vulnerability.confidence : finding.confidence,
resolved_on_default_branch: resolved_on_default_branch.nil? ? vulnerability.resolved_on_default_branch : resolved_on_default_branch
......
---
title: Truncate vulnerability title if longer than 255 characters
merge_request: 48327
author:
type: fixed
......@@ -9,9 +9,11 @@ RSpec.describe Vulnerabilities::CreateService do
let_it_be(:user) { create(:user) }
let(:project) { create(:project) } # cannot use let_it_be here: caching causes problems with permission-related tests
let(:finding) { create(:vulnerabilities_finding, project: project) }
let(:finding) { create(:vulnerabilities_finding, name: finding_name, project: project) }
let(:finding_id) { finding.id }
let(:expected_error_messages) { { base: ['finding is not found or is already attached to a vulnerability'] } }
let(:finding_name) { 'New title' }
let(:vulnerability) { project.vulnerabilities.last }
subject { described_class.new(project, user, finding_id: finding_id).execute }
......@@ -40,7 +42,6 @@ RSpec.describe Vulnerabilities::CreateService do
context 'and finding is dismissed' do
let(:finding) { create(:vulnerabilities_finding, :with_dismissal_feedback, project: project) }
let(:vulnerability) { project.vulnerabilities.last }
it 'creates a vulnerability in a dismissed state and sets dismissal information' do
expect { subject }.to change { project.vulnerabilities.count }.by(1)
......@@ -51,6 +52,15 @@ RSpec.describe Vulnerabilities::CreateService do
end
end
context 'when finding name is longer than 255 characters' do
let(:finding_name) { 'a' * 256 }
it 'truncates vulnerability title to have 255 characters' do
expect { subject }.to change { project.vulnerabilities.count }.by(1)
expect(vulnerability.title).to have_attributes(size: 255)
end
end
it 'starts a new transaction for the create sequence' do
allow(Vulnerabilities::Finding).to receive(:transaction).and_call_original
......
......@@ -9,9 +9,10 @@ RSpec.describe Vulnerabilities::UpdateService do
let_it_be(:user) { create(:user) }
let!(:project) { create(:project) } # cannot use let_it_be here: caching causes problems with permission-related tests
let!(:updated_finding) { create(:vulnerabilities_finding, project: project, name: 'New title', severity: :critical, confidence: :confirmed, vulnerability: vulnerability) }
let!(:updated_finding) { create(:vulnerabilities_finding, project: project, name: finding_name, severity: :critical, confidence: :confirmed, vulnerability: vulnerability) }
let!(:vulnerability) { create(:vulnerability, project: project, severity: :low, severity_overridden: severity_overridden, confidence: :ignore, confidence_overridden: confidence_overridden) }
let(:finding_name) { 'New title' }
let(:severity_overridden) { false }
let(:confidence_overridden) { false }
let(:resolved_on_default_branch) { nil }
......@@ -26,6 +27,15 @@ RSpec.describe Vulnerabilities::UpdateService do
it_behaves_like 'calls Vulnerabilities::Statistics::UpdateService'
it_behaves_like 'calls Vulnerabilities::HistoricalStatistics::UpdateService'
context 'when finding name is longer than 255 characters' do
let(:finding_name) { 'a' * 256 }
it 'truncates vulnerability title to have 255 characters' do
expect { subject }.not_to change { project.vulnerabilities.count }
expect(vulnerability.title).to have_attributes(size: 255)
end
end
context 'when neither severity nor confidence are overridden' do
it 'updates the vulnerability from updated finding (title, severity and confidence only)', :aggregate_failures do
expect { subject }.not_to change { project.vulnerabilities.count }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment