diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index dc6203d0281e39a0b4f0fbc493437317224d2e79..b78e0efda91f871b7be61547bbc68fc3f33e3bb4 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -259,12 +259,6 @@ def execute_hooks project.execute_integrations(integration_data, :vulnerability_hooks) end - def discussions_for_summary - # Notable#discussions has inc_relations_for_view which - # preloads a bunch of associations that we don't use. - Discussion.build_collection(notes, self) - end - def notes_summary @notes_summary ||= discussions_for_summary.map do |discussion| status = extracted_status(discussion.notes.first.note) @@ -294,6 +288,14 @@ def read_attribute(attribute_name) private + def discussions_for_summary + # Notable#discussions has inc_relations_for_view which + # preloads a bunch of associations that we don't use. + Discussion.build_collection(notes, self) + .each { |discussion| discussion.notes.sort_by!(&:id) } + .sort_by { |discussion| discussion.first_note.id } + end + def note_as_string(time, username, status, text) CSV.generate(col_sep: SUMMARY_DELIMITER) do |csv| csv << [time, username, status, text] diff --git a/ee/spec/models/ee/vulnerability_spec.rb b/ee/spec/models/ee/vulnerability_spec.rb index e2f7dada932123be66c5d31d769687fd21955afe..e46068ffe904d46fc48ce45124f3f86c723ed8b2 100644 --- a/ee/spec/models/ee/vulnerability_spec.rb +++ b/ee/spec/models/ee/vulnerability_spec.rb @@ -997,21 +997,27 @@ end end - describe '#notes_summary', quaratine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/501391' do + describe '#notes_summary' do let(:vulnerability) { create(:vulnerability, project: project) } - let!(:note) { create(:note, project: project, noteable: vulnerability, noteable_type: 'Vulnerability', note: 'changed vulnerabilities from detected to confirmed', author: user) } + let!(:most_recent_user) { create(:user) } + let!(:note) { create(:note, project: project, noteable: vulnerability, noteable_type: 'Vulnerability', note: 'changed vulnerabilities from detected to confirmed', author: most_recent_user) } subject(:notes_summary) { vulnerability.notes_summary } it 'returns summarized version of notes' do - expect(notes_summary).to eq("#{note.last_edited_at}|#{user.username}|confirmed|changed vulnerabilities from detected to confirmed\n") + expect(notes_summary).to eq("#{note.last_edited_at}|#{most_recent_user.username}|confirmed|changed vulnerabilities from detected to confirmed\n") end context 'with multiple notes' do let!(:additional_note) { create(:note, project: project, noteable: vulnerability, noteable_type: 'Vulnerability', note: 'additional comment by the user', author: user, discussion_id: note.discussion_id) } + before do + # This will re-order the tuples on disk based on an index ordered by `author_id`. + Note.connection.execute('CLUSTER notes USING index_notes_on_author_id_and_created_at_and_id') + end + it 'returns summarized version of notes' do - expect(notes_summary).to eq("#{note.last_edited_at}|#{user.username}|confirmed|changed vulnerabilities from detected to confirmed\n; #{additional_note.last_edited_at}|#{user.username}|confirmed|additional comment by the user\n") + expect(notes_summary).to eq("#{note.last_edited_at}|#{most_recent_user.username}|confirmed|changed vulnerabilities from detected to confirmed\n; #{additional_note.last_edited_at}|#{user.username}|confirmed|additional comment by the user\n") end end end