Commit 9cc333b2 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Merge branch '9137-notes-order' into 'master'

Ensure notes insertion order same as draft note

Closes #9137

See merge request gitlab-org/gitlab-ee!9684
parents 96962701 4dbd6de1
...@@ -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