diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 726838bb284..18fd8eb114d 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -89,7 +89,7 @@ module NotesActions if note.persisted? attrs[:valid] = true - if noteable.discussions_rendered_on_frontend? + if noteable.nil? || noteable.discussions_rendered_on_frontend? attrs.merge!(note_serializer.represent(note)) else attrs.merge!( diff --git a/app/models/note.rb b/app/models/note.rb index d807d5ad618..1073c115630 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -300,7 +300,7 @@ class Note < ActiveRecord::Base end def expire_etag_cache - return unless noteable.discussions_rendered_on_frontend? + return unless noteable&.discussions_rendered_on_frontend? key = Gitlab::Routing.url_helpers.project_noteable_notes_path( project, diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb index 663a9c06c40..7d50e0ff10d 100644 --- a/app/serializers/note_entity.rb +++ b/app/serializers/note_entity.rb @@ -14,8 +14,8 @@ class NoteEntity < API::Entities::Note expose :redacted_note_html, as: :note_html - expose :last_edited_at, if: -> (note, _) { note.is_edited? } - expose :last_edited_by, using: NoteUserEntity, if: -> (note, _) { note.is_edited? } + expose :last_edited_at, if: -> (note, _) { note.edited? } + expose :last_edited_by, using: NoteUserEntity, if: -> (note, _) { note.edited? } expose :current_user do expose :can_edit do |note| diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index f280c55059c..6ffe41b8608 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -46,10 +46,13 @@ describe Projects::NotesController do end context 'for a discussion note' do - let!(:note) { create(:discussion_note_on_issue, noteable: issue, project: project) } + let(:project) { create(:project, :repository) } + let!(:note) { create(:discussion_note_on_merge_request, project: project) } + + let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id) } it 'responds with the expected attributes' do - get :index, request_params + get :index, params expect(note_json[:id]).to eq(note.id) expect(note_json[:discussion_html]).not_to be_nil @@ -104,10 +107,12 @@ describe Projects::NotesController do end context 'for a regular note' do - let!(:note) { create(:note, noteable: issue, project: project) } + let!(:note) { create(:note_on_merge_request, project: project) } + + let(:params) { request_params.merge(target_type: 'merge_request', target_id: note.noteable_id) } it 'responds with the expected attributes' do - get :index, request_params + get :index, params expect(note_json[:id]).to eq(note.id) expect(note_json[:html]).not_to be_nil @@ -125,7 +130,9 @@ describe Projects::NotesController do note: { note: 'some note', noteable_id: merge_request.id, noteable_type: 'MergeRequest' }, namespace_id: project.namespace, project_id: project, - merge_request_diff_head_sha: 'sha' + merge_request_diff_head_sha: 'sha', + target_type: 'merge_request', + target_id: merge_request.id } end