diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 6da33a26e58db175e686050c94f78536b33d4eb5..0e1ca7fe883365f15ca2053a35d9b4db98a3eb64 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 0830ebe9e4e087a5b719feeb99ca09b5d6ebb899..9ff2042475bc0f4485c8326fe0d8aca53dd3858f 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 90dcafd75b7602dbe1909abe0f651bb034e42661..648fa6ff804c822a99d79bbedf78cb5735b74fac 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 a90c6d6381dfd3e36fc532f5632f1501871881c3..5bd81c7cad67ad0eb14ee44cec5f0ab8dbd8464f 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 f4f407ffd8adbbdcb92cd46dd3a08f701836a76a..68f8cb1cf1e9859d1783a8e3ee95b4fc70fe4e33 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 f90775d01573bccb9d3a8d97563352dd6551fda9..e4121f151dbb1e5007a995d745c09a4d04b75c36 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 0da4ff49f0862136687a9209957d45c7f01e64ad..5bf8216a1f331aaf01f2199c8796732655006bed 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 a21e658fda12cbc05d622318ec276079dfc95036..0379f76fc3d9f6524633fcfe18812ba10ca9db72 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 03ed5b5310bf58200765ee9c7ea0b8b9630e080a..839cac3687ceed9178a72cb35b429b045cee3ff8 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 cba9a53dc4b8b3bc8d0383c395ddec5078d75d27..7bc162140100e09d04e36885db0f88ccd1866b18 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 20aed60cb7a06b88741f203612458fea106785d4..27ed48fdbc7e38132a42c0e389aa98c816bfc329 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 787a80f0196485dcc177cda2d40360f0f1010f02..0f5fb529a87c6d4d5045f48f0981467f26fa371a 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 bbbcf6a97c189e8ca70a92f3126d7989d303c46b..718fb35e62d0cfa21c549fade0495b9aa1e55227 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 4ccf0bca476084a938161bd4a053b851634025aa..c964aa9c99be6ea51dad576b1aeb271a793d3ef9 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 2afe40d7a34bf67e3de870a6f16f30ff64b2d5d4..0000000000000000000000000000000000000000 --- 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 0000000000000000000000000000000000000000..e541bfbee8d11bc6fb4c5af678ad79c06d3702d6 --- /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 0000000000000000000000000000000000000000..763ad0bdb3fab85f167044f907e2b9b267d8895e --- /dev/null +++ b/app/serializers/project_note_serializer.rb @@ -0,0 +1,3 @@ +class ProjectNoteSerializer < BaseSerializer + entity ProjectNoteEntity +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 9918d52e4028a036eef3ee6343e5c41d396e75eb..01b5506b64bba917e47cd1dcb92b2c23feafe86e 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -974,7 +974,7 @@ describe Projects::IssuesController do it 'returns discussion json' do get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid - expect(json_response.first.keys).to match_array(%w[id reply_id expanded notes diff_discussion individual_note resolvable resolve_with_issue_path resolved]) + expect(json_response.first.keys).to match_array(%w[id reply_id expanded notes diff_discussion individual_note resolvable resolved]) end context 'with cross-reference system note', :request_store do diff --git a/spec/serializers/discussion_entity_spec.rb b/spec/serializers/discussion_entity_spec.rb index 7ee8e38af1c81a69c1615959df7388e3ed30e94b..7e19e74ca00dcc03a36c940d64e706c62883b1fd 100644 --- a/spec/serializers/discussion_entity_spec.rb +++ b/spec/serializers/discussion_entity_spec.rb @@ -6,7 +6,7 @@ describe DiscussionEntity do let(:user) { create(:user) } let(:note) { create(:discussion_note_on_merge_request) } let(:discussion) { note.discussion } - let(:request) { double('request') } + let(:request) { double('request', note_entity: ProjectNoteEntity) } let(:controller) { double('controller') } let(:entity) { described_class.new(discussion, request: request, context: controller) } diff --git a/spec/serializers/note_entity_spec.rb b/spec/serializers/note_entity_spec.rb index 51a8587ace9615883059bf862507a6cd12743274..13cda781cda82ff2cf97af7497f1efc7cb33a57f 100644 --- a/spec/serializers/note_entity_spec.rb +++ b/spec/serializers/note_entity_spec.rb @@ -10,53 +10,5 @@ describe NoteEntity do 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 - - context 'when note is part of resolvable discussion' do - before do - allow(note).to receive(:part_of_discussion?).and_return(true) - allow(note).to receive(:resolvable?).and_return(true) - end - - it 'exposes paths to resolve note' do - expect(subject).to include(:resolve_path, :resolve_with_issue_path) - end - end + it_behaves_like 'note entity' end diff --git a/spec/serializers/project_note_entity_spec.rb b/spec/serializers/project_note_entity_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..dafd1cf603e417383ef23b80ed0b7bab9e5af624 --- /dev/null +++ b/spec/serializers/project_note_entity_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe ProjectNoteEntity 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 } + + it_behaves_like 'note entity' + + it 'exposes project-specific elements' do + expect(subject).to include(:human_access, :toggle_award_path, :path) + end + + context 'when note is part of resolvable discussion' do + before do + allow(note).to receive(:part_of_discussion?).and_return(true) + allow(note).to receive(:resolvable?).and_return(true) + end + + it 'exposes paths to resolve note' do + expect(subject).to include(:resolve_path, :resolve_with_issue_path) + end + end +end diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index c8662d417696c2ffefcb88ce8a6616441436e791..80604395adfea7b9285ff5cbb90aa1eb12322664 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -81,7 +81,10 @@ shared_examples 'discussion comments' do |resource_name| # on issues page, the menu closes when clicking anywhere, on other pages it will # remain open if clicking divider or menu padding, but should not change button action - if resource_name == 'issue' + # + # if dropdown menu is not toggled (and also not present), + # it's "issue-type" dropdown + if first(menu_selector).nil? expect(find(dropdown_selector)).to have_content 'Comment' find(toggle_selector).click @@ -107,8 +110,10 @@ shared_examples 'discussion comments' do |resource_name| end it 'updates the submit button text and closes the dropdown' do + button = find(submit_selector) + # on issues page, the submit input is a