From d78037c246d14817937abbc4369e30ae12b022bc Mon Sep 17 00:00:00 2001
From: Jarka Kadlecova <jarka@gitlab.com>
Date: Fri, 11 Aug 2017 18:48:06 +0200
Subject: [PATCH] Use UserNoteEntity instead of UserEntity for notes

---
 app/serializers/note_entity.rb                |  4 +-
 app/serializers/user_entity.rb                |  2 -
 app/serializers/user_note_entity.rb           |  9 ++++
 .../projects/issues_controller_spec.rb        | 16 ++++++
 spec/serializers/note_entity_spec.rb          | 51 +++++++++++++++++++
 5 files changed, 78 insertions(+), 4 deletions(-)
 create mode 100644 app/serializers/user_note_entity.rb
 create mode 100644 spec/serializers/note_entity_spec.rb

diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index f84343aa71a..e5295f5f34d 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -3,7 +3,7 @@ class NoteEntity < API::Entities::Note
 
   expose :type
 
-  expose :author, using: UserEntity
+  expose :author, using: UserNoteEntity
 
   expose :human_access do |note|
     note.project.team.human_max_access(note.author_id)
@@ -15,7 +15,7 @@ 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: UserEntity, if: -> (note, _) { note.is_edited? }
+  expose :last_edited_by, using: UserNoteEntity, if: -> (note, _) { note.is_edited? }
 
   expose :current_user do
     expose :can_edit do |note|
diff --git a/app/serializers/user_entity.rb b/app/serializers/user_entity.rb
index 3bb340065c4..876512b12dc 100644
--- a/app/serializers/user_entity.rb
+++ b/app/serializers/user_entity.rb
@@ -1,8 +1,6 @@
 class UserEntity < API::Entities::UserBasic
   include RequestAwareEntity
 
-  unexpose :web_url
-
   expose :path do |user|
     user_path(user)
   end
diff --git a/app/serializers/user_note_entity.rb b/app/serializers/user_note_entity.rb
new file mode 100644
index 00000000000..fbd87470380
--- /dev/null
+++ b/app/serializers/user_note_entity.rb
@@ -0,0 +1,9 @@
+class UserNoteEntity < API::Entities::UserBasic
+  include RequestAwareEntity
+
+  unexpose :web_url
+
+  expose :path do |user|
+    user_path(user)
+  end
+end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 23601c457b0..c07c035fcda 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -877,4 +877,20 @@ describe Projects::IssuesController do
                                   format: :json
     end
   end
+
+  describe 'GET #discussions' do
+    let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
+
+    before do
+      project.add_developer(user)
+      sign_in(user)
+    end
+
+    it 'returns discussion json' do
+      get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
+
+      expect(JSON.parse(response.body).first.keys).to match_array(
+        ['id', 'reply_id', 'expanded', 'notes', 'individual_note'])
+    end
+  end
 end
diff --git a/spec/serializers/note_entity_spec.rb b/spec/serializers/note_entity_spec.rb
new file mode 100644
index 00000000000..3459cc72063
--- /dev/null
+++ b/spec/serializers/note_entity_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe NoteEntity do
+  include Gitlab::Routing
+
+  let(:request) { double('request', current_user: user, noteable: note.noteable) }
+
+  let(:entity) { described_class.new(note, request: request) }
+  let(:note) { create(:note) }
+  let(:user) { create(:user) }
+  subject { entity.as_json }
+
+  context 'basic note' do
+    it 'exposes correct elements' do
+      expect(subject).to include(:type, :author, :human_access, :note, :note_html, :current_user,
+        :discussion_id, :emoji_awardable, :award_emoji, :toggle_award_path, :report_abuse_path, :path, :attachment)
+    end
+
+    it 'does not expose elements for specific notes cases' do
+      expect(subject).not_to include(:last_edited_by, :last_edited_at, :system_note_icon_name)
+    end
+
+    it 'exposes author correctly' do
+      expect(subject[:author]).to include(:id, :name, :username, :state, :avatar_url, :path)
+    end
+
+    it 'does not expose web_url for author' do
+      expect(subject[:author]).not_to include(:web_url)
+    end
+  end
+
+  context 'when note was edited' do
+    before do
+      note.update(updated_at: 1.minute.from_now, updated_by: user)
+    end
+
+    it 'exposes last_edited_at and last_edited_by elements' do
+      expect(subject).to include(:last_edited_at, :last_edited_by)
+    end
+  end
+
+  context 'when note is a system note' do
+    before do
+      note.update(system: true)
+    end
+
+    it 'exposes system_note_icon_name element' do
+      expect(subject).to include(:system_note_icon_name)
+    end
+  end
+end