Refactor discussions/notes code

17 files changed, 73 insertions, 54 deletions

diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js
index 6da33a26e58..0e1ca7fe883 100644
--- a/app/assets/javascripts/awards_handler.js
+++ b/app/assets/javascripts/awards_handler.js
@@ -4,7 +4,7 @@ import $ from 'jquery';
 import _ from 'underscore';
 import Cookies from 'js-cookie';
 import { __ } from './locale';
-import { isInIssuePage, isInMRPage, hasVueMRDiscussionsCookie, updateTooltipTitle } from './lib/utils/common_utils';
+import { isInIssuePage, isInMRPage, isInEpicPage, hasVueMRDiscussionsCookie, updateTooltipTitle } from './lib/utils/common_utils';
 import flash from './flash';
 import axios from './lib/utils/axios_utils';
 
@@ -300,7 +300,7 @@ class AwardsHandler {
   }
 
   isInVueNoteablePage() {
-    return isInIssuePage() || this.isVueMRDiscussions();
+    return isInIssuePage() || isInEpicPage() || this.isVueMRDiscussions();
   }
 
   getVotesBlock() {
diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js
index 0830ebe9e4e..9ff2042475b 100644
--- a/app/assets/javascripts/lib/utils/common_utils.js
+++ b/app/assets/javascripts/lib/utils/common_utils.js
@@ -33,6 +33,7 @@ export const checkPageAndAction = (page, action) => {
 
 export const isInIssuePage = () => checkPageAndAction('issues', 'show');
 export const isInMRPage = () => checkPageAndAction('merge_requests', 'show');
+export const isInEpicPage = () => checkPageAndAction('epics', 'show');
 export const isInNoteablePage = () => isInIssuePage() || isInMRPage();
 export const hasVueMRDiscussionsCookie = () => Cookies.get('vue_mr_discussions');
 
diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue
index 90dcafd75b7..648fa6ff804 100644
--- a/app/assets/javascripts/notes/components/comment_form.vue
+++ b/app/assets/javascripts/notes/components/comment_form.vue
@@ -99,6 +99,10 @@ export default {
         'js-note-target-reopen': !this.isOpen,
       };
     },
+    supportQuickActions() {
+      // Disable quick actions support for Epics
+      return this.noteableType !== constants.EPIC_NOTEABLE_TYPE;
+    },
     markdownDocsPath() {
       return this.getNotesData.markdownDocsPath;
     },
@@ -355,7 +359,7 @@ Please check your network connection and try again.`;
                   name="note[note]"
                   class="note-textarea js-vue-comment-form
 js-gfm-input js-autosize markdown-area js-vue-textarea"
-                  data-supports-quick-actions="true"
+                  :data-supports-quick-actions="supportQuickActions"
                   aria-label="Description"
                   v-model="note"
                   ref="textarea"
diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue
index a90c6d6381d..5bd81c7cad6 100644
--- a/app/assets/javascripts/notes/components/notes_app.vue
+++ b/app/assets/javascripts/notes/components/notes_app.vue
@@ -50,7 +50,11 @@ export default {
     ...mapGetters(['notes', 'getNotesDataByProp', 'discussionCount']),
     noteableType() {
       // FIXME -- @fatihacet Get this from JSON data.
-      const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE } = constants;
+      const { ISSUE_NOTEABLE_TYPE, MERGE_REQUEST_NOTEABLE_TYPE, EPIC_NOTEABLE_TYPE } = constants;
+
+      if (this.noteableData.noteableType === EPIC_NOTEABLE_TYPE) {
+        return EPIC_NOTEABLE_TYPE;
+      }
 
       return this.noteableData.merge_params
         ? MERGE_REQUEST_NOTEABLE_TYPE
diff --git a/app/assets/javascripts/notes/constants.js b/app/assets/javascripts/notes/constants.js
index f4f407ffd8a..68f8cb1cf1e 100644
--- a/app/assets/javascripts/notes/constants.js
+++ b/app/assets/javascripts/notes/constants.js
@@ -10,6 +10,7 @@ export const CLOSED = 'closed';
 export const EMOJI_THUMBSUP = 'thumbsup';
 export const EMOJI_THUMBSDOWN = 'thumbsdown';
 export const ISSUE_NOTEABLE_TYPE = 'issue';
+export const EPIC_NOTEABLE_TYPE = 'epic';
 export const MERGE_REQUEST_NOTEABLE_TYPE = 'merge_request';
 export const UNRESOLVE_NOTE_METHOD_NAME = 'delete';
 export const RESOLVE_NOTE_METHOD_NAME = 'post';
diff --git a/app/assets/javascripts/notes/index.js b/app/assets/javascripts/notes/index.js
index f90775d0157..e4121f151db 100644
--- a/app/assets/javascripts/notes/index.js
+++ b/app/assets/javascripts/notes/index.js
@@ -12,8 +12,11 @@ document.addEventListener(
       data() {
         const notesDataset = document.getElementById('js-vue-notes').dataset;
         const parsedUserData = JSON.parse(notesDataset.currentUserData);
+        const noteableData = JSON.parse(notesDataset.noteableData);
         let currentUserData = {};
 
+        noteableData.noteableType = notesDataset.noteableType;
+
         if (parsedUserData) {
           currentUserData = {
             id: parsedUserData.id,
@@ -25,7 +28,7 @@ document.addEventListener(
         }
 
         return {
-          noteableData: JSON.parse(notesDataset.noteableData),
+          noteableData,
           currentUserData,
           notesData: JSON.parse(notesDataset.notesData),
         };
diff --git a/app/assets/javascripts/notes/mixins/noteable.js b/app/assets/javascripts/notes/mixins/noteable.js
index 0da4ff49f08..5bf8216a1f3 100644
--- a/app/assets/javascripts/notes/mixins/noteable.js
+++ b/app/assets/javascripts/notes/mixins/noteable.js
@@ -14,6 +14,8 @@ export default {
           return constants.MERGE_REQUEST_NOTEABLE_TYPE;
         case 'Issue':
           return constants.ISSUE_NOTEABLE_TYPE;
+        case 'Epic':
+          return constants.EPIC_NOTEABLE_TYPE;
         default:
           return '';
       }
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index a21e658fda1..0379f76fc3d 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -88,11 +88,15 @@ module IssuableActions
 
     discussions = Discussion.build_collection(notes, issuable)
 
-    render json: DiscussionSerializer.new(project: project, noteable: issuable, current_user: current_user).represent(discussions, context: self)
+    render json: discussion_serializer.represent(discussions, context: self)
   end
 
   private
 
+  def discussion_serializer
+    DiscussionSerializer.new(project: project, noteable: issuable, current_user: current_user, note_entity: ProjectNoteEntity)
+  end
+
   def recaptcha_check_if_spammable(should_redirect = true, &block)
     return yield unless issuable.is_a? Spammable
 
diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb
index 03ed5b5310b..839cac3687c 100644
--- a/app/controllers/concerns/notes_actions.rb
+++ b/app/controllers/concerns/notes_actions.rb
@@ -212,7 +212,7 @@ module NotesActions
   end
 
   def note_serializer
-    NoteSerializer.new(project: project, noteable: noteable, current_user: current_user)
+    ProjectNoteSerializer.new(project: project, noteable: noteable, current_user: current_user)
   end
 
   def note_project
diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb
index cba9a53dc4b..7bc16214010 100644
--- a/app/controllers/projects/discussions_controller.rb
+++ b/app/controllers/projects/discussions_controller.rb
@@ -43,7 +43,7 @@ class Projects::DiscussionsController < Projects::ApplicationController
 
   def render_json_with_discussions_serializer
     render json:
-      DiscussionSerializer.new(project: project, noteable: discussion.noteable, current_user: current_user)
+      DiscussionSerializer.new(project: project, noteable: discussion.noteable, current_user: current_user, note_entity:  ProjectNoteEntity)
       .represent(discussion, context: self)
   end
 
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 20aed60cb7a..27ed48fdbc7 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -151,16 +151,17 @@ module NotesHelper
     }
   end
 
-  def notes_data(issuable)
-    discussions_path =
-      if issuable.is_a?(Issue)
-        discussions_project_issue_path(@project, issuable, format: :json)
-      else
-        discussions_project_merge_request_path(@project, issuable, format: :json)
-      end
+  def discussions_path(issuable)
+    if issuable.is_a?(Issue)
+      discussions_project_issue_path(@project, issuable, format: :json)
+    else
+      discussions_project_merge_request_path(@project, issuable, format: :json)
+    end
+  end
 
+  def notes_data(issuable)
     {
-      discussionsPath: discussions_path,
+      discussionsPath: discussions_path(issuable),
       registerPath: new_session_path(:user, redirect_to_referer: 'yes', anchor: 'register-pane'),
       newSessionPath: new_session_path(:user, redirect_to_referer: 'yes'),
       markdownDocsPath: help_page_path('user/markdown'),
@@ -170,7 +171,6 @@ module NotesHelper
       notesPath: notes_url,
       totalNotes: issuable.discussions.length,
       lastFetchedAt: Time.now.to_i
-
     }.to_json
   end
 
diff --git a/app/models/note.rb b/app/models/note.rb
index 787a80f0196..0f5fb529a87 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -379,12 +379,15 @@ class Note < ActiveRecord::Base
   def expire_etag_cache
     return unless noteable&.discussions_rendered_on_frontend?
 
-    key = Gitlab::Routing.url_helpers.project_noteable_notes_path(
+    Gitlab::EtagCaching::Store.new.touch(etag_key)
+  end
+
+  def etag_key
+    Gitlab::Routing.url_helpers.project_noteable_notes_path(
       project,
       target_type: noteable_type.underscore,
       target_id: noteable_id
     )
-    Gitlab::EtagCaching::Store.new.touch(key)
   end
 
   def touch(*args)
diff --git a/app/serializers/discussion_entity.rb b/app/serializers/discussion_entity.rb
index bbbcf6a97c1..718fb35e62d 100644
--- a/app/serializers/discussion_entity.rb
+++ b/app/serializers/discussion_entity.rb
@@ -4,7 +4,9 @@ class DiscussionEntity < Grape::Entity
   expose :id, :reply_id
   expose :expanded?, as: :expanded
 
-  expose :notes, using: NoteEntity
+  expose :notes do |discussion, opts|
+    request.note_entity.represent(discussion.notes, opts)
+  end
 
   expose :individual_note?, as: :individual_note
   expose :resolvable?, as: :resolvable
@@ -12,7 +14,7 @@ class DiscussionEntity < Grape::Entity
   expose :resolve_path, if: -> (d, _) { d.resolvable? } do |discussion|
     resolve_project_merge_request_discussion_path(discussion.project, discussion.noteable, discussion.id)
   end
-  expose :resolve_with_issue_path do |discussion|
+  expose :resolve_with_issue_path, if: -> (d, _) { d.resolvable? } do |discussion|
     new_project_issue_path(discussion.project, merge_request_to_resolve_discussions_of: discussion.noteable.iid, discussion_to_resolve: discussion.id)
   end
 
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index 4ccf0bca476..c964aa9c99b 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -5,10 +5,6 @@ class NoteEntity < API::Entities::Note
 
   expose :author, using: NoteUserEntity
 
-  expose :human_access do |note|
-    note.project.team.human_max_access(note.author_id)
-  end
-
   unexpose :note, as: :body
   expose :note
 
@@ -37,36 +33,10 @@ class NoteEntity < API::Entities::Note
 
   expose :emoji_awardable?, as: :emoji_awardable
   expose :award_emoji, if: -> (note, _) { note.emoji_awardable? }, using: AwardEmojiEntity
-  expose :toggle_award_path, if: -> (note, _) { note.emoji_awardable? } do |note|
-    if note.for_personal_snippet?
-      toggle_award_emoji_snippet_note_path(note.noteable, note)
-    else
-      toggle_award_emoji_project_note_path(note.project, note.id)
-    end
-  end
 
   expose :report_abuse_path do |note|
     new_abuse_report_path(user_id: note.author.id, ref_url: Gitlab::UrlBuilder.build(note))
   end
 
-  expose :path do |note|
-    if note.for_personal_snippet?
-      snippet_note_path(note.noteable, note)
-    else
-      project_note_path(note.project, note)
-    end
-  end
-
-  expose :resolve_path, if: -> (note, _) { note.part_of_discussion? && note.resolvable? } do |note|
-    resolve_project_merge_request_discussion_path(note.project, note.noteable, note.discussion_id)
-  end
-
-  expose :resolve_with_issue_path, if: -> (note, _) { note.part_of_discussion? && note.resolvable? } do |note|
-    new_project_issue_path(note.project, merge_request_to_resolve_discussions_of: note.noteable.iid, discussion_to_resolve: note.discussion_id)
-  end
-
   expose :attachment, using: NoteAttachmentEntity, if: -> (note, _) { note.attachment? }
-  expose :delete_attachment_path, if: -> (note, _) { note.attachment? } do |note|
-    delete_attachment_project_note_path(note.project, note)
-  end
 end
diff --git a/app/serializers/note_serializer.rb b/app/serializers/note_serializer.rb
deleted file mode 100644
index 2afe40d7a34..00000000000
--- a/app/serializers/note_serializer.rb
+++ /dev/null
@@ -1,3 +0,0 @@
-class NoteSerializer < BaseSerializer
-  entity NoteEntity
-end
diff --git a/app/serializers/project_note_entity.rb b/app/serializers/project_note_entity.rb
new file mode 100644
index 00000000000..e541bfbee8d
--- /dev/null
+++ b/app/serializers/project_note_entity.rb
@@ -0,0 +1,25 @@
+class ProjectNoteEntity < NoteEntity
+  expose :human_access do |note|
+    note.project.team.human_max_access(note.author_id)
+  end
+
+  expose :toggle_award_path, if: -> (note, _) { note.emoji_awardable? } do |note|
+    toggle_award_emoji_project_note_path(note.project, note.id)
+  end
+
+  expose :path do |note|
+    project_note_path(note.project, note)
+  end
+
+  expose :resolve_path, if: -> (note, _) { note.part_of_discussion? && note.resolvable? } do |note|
+    resolve_project_merge_request_discussion_path(note.project, note.noteable, note.discussion_id)
+  end
+
+  expose :resolve_with_issue_path, if: -> (note, _) { note.part_of_discussion? && note.resolvable? } do |note|
+    new_project_issue_path(note.project, merge_request_to_resolve_discussions_of: note.noteable.iid, discussion_to_resolve: note.discussion_id)
+  end
+
+  expose :delete_attachment_path, if: -> (note, _) { note.attachment? } do |note|
+    delete_attachment_project_note_path(note.project, note)
+  end
+end
diff --git a/app/serializers/project_note_serializer.rb b/app/serializers/project_note_serializer.rb
new file mode 100644
index 00000000000..763ad0bdb3f
--- /dev/null
+++ b/app/serializers/project_note_serializer.rb
@@ -0,0 +1,3 @@
+class ProjectNoteSerializer < BaseSerializer
+  entity ProjectNoteEntity
+end