Commit 4dbd6de1 authored by Mark Chao's avatar Mark Chao

Ensure notes insertion order same as draft note

Notes should be in the same order as user comment order.
parent df4c9b24
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
class DraftNote < ActiveRecord::Base class DraftNote < ActiveRecord::Base
include DiffPositionableNote include DiffPositionableNote
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
include Sortable
PUBLISH_ATTRS = %i(noteable_id noteable_type type note).freeze PUBLISH_ATTRS = %i(noteable_id noteable_type type note).freeze
DIFF_ATTRS = %i(position original_position change_position).freeze DIFF_ATTRS = %i(position original_position change_position).freeze
......
...@@ -11,7 +11,7 @@ module DraftNotes ...@@ -11,7 +11,7 @@ module DraftNotes
private private
def draft_notes def draft_notes
@draft_notes ||= merge_request.draft_notes.authored_by(current_user) @draft_notes ||= merge_request.draft_notes.order_id_asc.authored_by(current_user)
end end
def project def project
......
---
title: Ensure comments from merge request review is displayed in the same order as user commenting order
merge_request: 9684
author:
type: fixed
...@@ -32,7 +32,8 @@ describe DraftNotes::PublishService do ...@@ -32,7 +32,8 @@ describe DraftNotes::PublishService do
context 'multiple draft notes' do context 'multiple draft notes' do
before do before do
create_list(:draft_note, 2, merge_request: merge_request, author: user) create(:draft_note, merge_request: merge_request, author: user, note: 'first note')
create(:draft_note, merge_request: merge_request, author: user, note: 'second note')
end end
context 'when review fails to create' do context 'when review fails to create' do
...@@ -63,6 +64,10 @@ describe DraftNotes::PublishService do ...@@ -63,6 +64,10 @@ describe DraftNotes::PublishService do
it 'publishes all draft notes for a user in a merge request' do it 'publishes all draft notes for a user in a merge request' do
expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2).and change { Review.count }.by(1) expect { publish }.to change { DraftNote.count }.by(-2).and change { Note.count }.by(2).and change { Review.count }.by(1)
expect(DraftNote.count).to eq(0) expect(DraftNote.count).to eq(0)
notes = merge_request.notes.order(id: :asc)
expect(notes.first.note).to eq('first note')
expect(notes.last.note).to eq('second note')
end end
it 'sends batch notification' do it 'sends batch notification' do
......
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