diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index a8053b053d90dfc9ed6f726760d0cd641a33fc85..9b492efdcb11421741b50d774754c2c6ad7a94f3 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -216,7 +216,7 @@ export default { ></textarea> <!-- eslint-enable vue/no-mutating-props --> <note-edited-text - v-if="note.last_edited_at" + v-if="note.last_edited_at && note.last_edited_at !== note.created_at" :edited-at="note.last_edited_at" :edited-by="note.last_edited_by" :action-text="__('Edited')" diff --git a/app/assets/javascripts/notes/components/note_preview.vue b/app/assets/javascripts/notes/components/note_preview.vue deleted file mode 100644 index cbcedd7dafad64f257c14b785e11323aaa16f660..0000000000000000000000000000000000000000 --- a/app/assets/javascripts/notes/components/note_preview.vue +++ /dev/null @@ -1,128 +0,0 @@ -<script> -import { renderGFM } from '~/behaviors/markdown/render_gfm'; -import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; -import { TYPENAME_NOTE } from '~/graphql_shared/constants'; -import * as Sentry from '~/sentry/sentry_browser_wrapper'; -import SafeHtml from '~/vue_shared/directives/safe_html'; -import timeagoMixin from '~/vue_shared/mixins/timeago'; -import noteQuery from '../graphql/note.query.graphql'; -import NoteEditedText from './note_edited_text.vue'; -import NoteableNote from './noteable_note.vue'; - -export default { - components: { - NoteEditedText, - NoteableNote, - }, - directives: { - SafeHtml, - }, - mixins: [timeagoMixin], - props: { - noteId: { - type: String, - required: false, - default: '', - }, - }, - data() { - return { - note: null, - hidden: false, - }; - }, - computed: { - showNote() { - return this.note && !this.hidden && !this.isSyntheticNote; - }, - showEdited() { - return this.note && this.note.created_at !== this.note.last_edited_at; - }, - isSyntheticNote() { - return Boolean(this.noteId?.match(/([a-f0-9]{40})/)); - }, - noteHtml() { - return this.note?.body_html; - }, - }, - watch: { - async noteHtml() { - try { - await this.$nextTick(); - renderGFM(this.$refs.noteBody); - } catch { - this.fallback(); - } - }, - }, - mounted() { - if (this.isSyntheticNote) { - this.fallback(); - } - }, - methods: { - fallback() { - this.hidden = true; - }, - }, - apollo: { - note: { - skip() { - return !this.noteId || this.isSyntheticNote; - }, - query: noteQuery, - variables() { - return { - id: convertToGraphQLId(TYPENAME_NOTE, this.noteId), - }; - }, - update(data) { - if (!data?.note) return null; - return { - ...data.note, - author: { - ...data.note.author, - id: getIdFromGraphQLId(data.note.author.id), - }, - last_edited_by: { - ...data.note.last_edited_by, - id: getIdFromGraphQLId(data.note.last_edited_by?.id), - }, - id: getIdFromGraphQLId(data.note.id), - }; - }, - result(result) { - if (result?.errors?.length > 0) { - Sentry.captureException(result.errors[0].message); - this.fallback(); - } - - if (!result?.data?.note) { - this.fallback(); - } - }, - error(error) { - Sentry.captureException(error); - this.fallback(); - }, - }, - }, -}; -</script> - -<template> - <noteable-note v-if="showNote" :id="`note_${noteId}`" :note="note" :show-reply-button="false"> - <template #note-body> - <div ref="noteBody" class="note-body"> - <div v-safe-html:[$options.safeHtmlConfig]="noteHtml" class="note-text md"></div> - <note-edited-text - v-if="showEdited" - :edited-at="note.last_edited_at" - :edited-by="note.last_edited_by" - :action-text="__('Edited')" - class="note_edited_ago" - /> - </div> - </template> - </noteable-note> -</template> diff --git a/app/assets/javascripts/notes/components/notes_app.vue b/app/assets/javascripts/notes/components/notes_app.vue index a5abf1cdedec5784302e17c119165bdccaac7b15..f24414406f1b25dfa47b97606d3a29d48e967c6e 100644 --- a/app/assets/javascripts/notes/components/notes_app.vue +++ b/app/assets/javascripts/notes/components/notes_app.vue @@ -3,6 +3,9 @@ import { mapGetters, mapActions } from 'vuex'; import { v4 as uuidv4 } from 'uuid'; import { InternalEvents } from '~/tracking'; +import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { TYPENAME_NOTE } from '~/graphql_shared/constants'; +import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { getDraft, getAutoSaveKeyFromDiscussion } from '~/lib/utils/autosave'; import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user'; import { scrollToTargetOnResize } from '~/lib/utils/resize_observer'; @@ -10,7 +13,6 @@ import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import DraftNote from '~/batch_comments/components/draft_note.vue'; import { getLocationHash } from '~/lib/utils/url_utility'; -import NotePreview from '~/notes/components/note_preview.vue'; import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue'; import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue'; import SkeletonLoadingContainer from '~/vue_shared/components/notes/skeleton_note.vue'; @@ -20,6 +22,7 @@ import { ISSUABLE_COMMENT_OR_REPLY, keysFor } from '~/behaviors/shortcuts/keybin import { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; import * as constants from '../constants'; import eventHub from '../event_hub'; +import noteQuery from '../graphql/note.query.graphql'; import CommentForm from './comment_form.vue'; import DiscussionFilterNote from './discussion_filter_note.vue'; import NoteableDiscussion from './noteable_discussion.vue'; @@ -31,7 +34,6 @@ import NotesActivityHeader from './notes_activity_header.vue'; export default { name: 'NotesApp', components: { - NotePreview, NotesActivityHeader, NoteableNote, NoteableDiscussion, @@ -88,8 +90,59 @@ export default { renderSkeleton: !this.shouldShow, aiLoading: null, isInitialEventTriggered: false, + previewNote: null, }; }, + apollo: { + previewNote: { + skip() { + const notCommentId = Boolean(this.previewNoteId?.match(/([a-f0-9]{40})/)); + return !this.previewNoteId || notCommentId; + }, + query: noteQuery, + variables() { + return { + id: convertToGraphQLId(TYPENAME_NOTE, this.previewNoteId), + }; + }, + update(data) { + if (!data?.note?.discussion) return null; + return { + id: `${getIdFromGraphQLId(data.note.discussion.id)}`, + expanded: true, + notes: data.note.discussion.notes.nodes.map((note) => ({ + ...note, + id: `${getIdFromGraphQLId(note.id)}`, + author: { + ...note.author, + id: getIdFromGraphQLId(note.author.id), + }, + award_emoji: note.award_emoji.nodes.map((emoji) => ({ + ...emoji, + id: getIdFromGraphQLId(emoji.id), + user: { + ...emoji.user, + id: getIdFromGraphQLId(emoji.user.id), + }, + })), + current_user: { + can_award_emoji: note.userPermissions.awardEmoji, + can_edit: note.userPermissions.adminNote, + can_resolve_discussions: note.userPermissions.resolveNote, + }, + last_edited_by: { + ...note.last_edited_by, + id: getIdFromGraphQLId(note.last_edited_by?.id), + }, + toggle_award_path: '', + })), + }; + }, + error(error) { + Sentry.captureException(error); + }, + }, + }, computed: { ...mapGetters([ 'isNotesFetched', @@ -126,17 +179,12 @@ export default { }); if ( - this.previewNoteId && + this.previewNote && !this.discussions.find((d) => d.notes[0].id === this.previewNoteId) ) { - const previewNote = { - id: this.previewNoteId, - isPreviewNote: true, - }; - skeletonNotes.splice(prerenderedNotesCount / 2, 0, previewNote); + skeletonNotes.splice(prerenderedNotesCount / 2, 0, this.previewNote); } } - if (this.sortDirDesc) { return skeletonNotes.concat(this.discussions); } @@ -190,6 +238,8 @@ export default { }); } + scrollToTargetOnResize(); + eventHub.$on('noteFormAddToReview', this.handleReviewTracking); eventHub.$on('noteFormStartReview', this.handleReviewTracking); @@ -338,13 +388,6 @@ export default { :key="discussion.id" class="note-skeleton" /> - <timeline-entry-item - v-else-if="discussion.isPreviewNote" - :key="discussion.id" - class="target note note-wrapper note-comment" - > - <note-preview :note-id="previewNoteId" /> - </timeline-entry-item> <timeline-entry-item v-else-if="discussion.isDraft" :key="discussion.id"> <draft-note :draft="discussion" /> </timeline-entry-item> diff --git a/app/assets/javascripts/notes/graphql/note.query.graphql b/app/assets/javascripts/notes/graphql/note.query.graphql index ebf6414fd4810c79799ab1e0b4c873da10a4f6e5..c20f4e3292768a7d023c0293141935dc0fe12cf9 100644 --- a/app/assets/javascripts/notes/graphql/note.query.graphql +++ b/app/assets/javascripts/notes/graphql/note.query.graphql @@ -1,26 +1,52 @@ query snakeCaseNote($id: NoteID!) { note(id: $id) { id - author { + discussion { id - avatar_url: avatarUrl - name - username - web_url: webUrl - web_path: webPath + notes { + nodes { + id + author { + id + avatar_url: avatarUrl + name + username + web_url: webUrl + web_path: webPath + } + award_emoji: awardEmoji { + nodes { + emoji + name + user { + id + username + name + } + } + } + note_html: bodyHtml + created_at: createdAt + last_edited_at: lastEditedAt + last_edited_by: lastEditedBy { + id + avatar_url: avatarUrl + name + username + web_url: webUrl + web_path: webPath + } + internal + url + userPermissions { + awardEmoji + adminNote + readNote + createNote + resolveNote + } + } + } } - body_html: bodyHtml - created_at: createdAt - last_edited_at: lastEditedAt - last_edited_by: lastEditedBy { - id - avatar_url: avatarUrl - name - username - web_url: webUrl - web_path: webPath - } - internal - url } } diff --git a/scripts/frontend/quarantined_vue3_specs.txt b/scripts/frontend/quarantined_vue3_specs.txt index 8115d769a73a8e704f53e1008c1992229b219d62..5d1c80c73ba958214f3de13c1a1c4429888d16cb 100644 --- a/scripts/frontend/quarantined_vue3_specs.txt +++ b/scripts/frontend/quarantined_vue3_specs.txt @@ -200,7 +200,6 @@ spec/frontend/ml/model_registry/components/model_version_create_spec.js spec/frontend/notebook/cells/markdown_spec.js spec/frontend/notebook/cells/output/html_spec.js spec/frontend/notes/components/discussion_notes_spec.js -spec/frontend/notes/components/note_preview_spec.js spec/frontend/notes/components/notes_app_spec.js spec/frontend/organizations/groups_and_projects/components/app_spec.js spec/frontend/organizations/shared/components/new_edit_form_spec.js diff --git a/spec/frontend/notes/components/note_preview_spec.js b/spec/frontend/notes/components/note_preview_spec.js deleted file mode 100644 index 89a115a958b39444a788c535867daacab7f0ea52..0000000000000000000000000000000000000000 --- a/spec/frontend/notes/components/note_preview_spec.js +++ /dev/null @@ -1,83 +0,0 @@ -import { shallowMount } from '@vue/test-utils'; -import Vue from 'vue'; -import VueApollo from 'vue-apollo'; -import createMockApollo from 'helpers/mock_apollo_helper'; -import waitForPromises from 'helpers/wait_for_promises'; -import NotePreview from '~/notes/components/note_preview.vue'; -import NoteableNote from '~/notes/components/noteable_note.vue'; -import noteQuery from '~/notes/graphql/note.query.graphql'; - -const noteQueryHandler = jest.fn().mockResolvedValue({ - data: { - note: { - id: 'gid://gitlab/Note/1', - author: { - id: 'gid://gitlab/User/1', - name: 'Administrator', - username: 'root', - avatar_url: '', - web_url: '', - web_path: '', - }, - body_html: 'my quick note', - created_at: '2020-01-01T10:00:00.000Z', - last_edited_at: null, - last_edited_by: null, - internal: false, - url: '/note/1', - }, - }, -}); - -describe('Note preview', () => { - let wrapper; - - Vue.use(VueApollo); - - const createComponent = ({ noteId = '1', queryHandlers = [[noteQuery, noteQueryHandler]] }) => { - wrapper = shallowMount(NotePreview, { - apolloProvider: createMockApollo(queryHandlers), - propsData: { - noteId, - }, - }); - }; - - const findNoteableNote = () => wrapper.findComponent(NoteableNote); - - it('does nothing if URL does not contain a note id', () => { - createComponent({ noteId: null }); - - expect(noteQueryHandler).not.toHaveBeenCalled(); - expect(wrapper.html()).toBe(''); - }); - - it('does nothing if URL links to a system note', () => { - createComponent({ - noteId: '50f036b11addf3c1dc3d4b43a96cfeb799ae2f7c', - }); - - expect(noteQueryHandler).not.toHaveBeenCalled(); - expect(wrapper.html()).toBe(''); - }); - - it('renders a note', async () => { - createComponent({ noteId: '1234' }); - - await waitForPromises(); - - expect(findNoteableNote().exists()).toBe(true); - expect(findNoteableNote().props('showReplyButton')).toBe(false); - }); - - it('renders nothing if note returns null', async () => { - createComponent({ - noteId: '1234', - queryHandlers: [[noteQuery, jest.fn().mockResolvedValue({ data: { note: null } })]], - }); - - await waitForPromises(); - - expect(wrapper.html()).toBe(''); - }); -}); diff --git a/spec/frontend/notes/components/notes_app_spec.js b/spec/frontend/notes/components/notes_app_spec.js index 31056cc2649fe77268cc91e9c84f7adcec5eea76..a10f4d1cefee3dccf289ebc39d2350bdf1885048 100644 --- a/spec/frontend/notes/components/notes_app_spec.js +++ b/spec/frontend/notes/components/notes_app_spec.js @@ -1,7 +1,8 @@ import { mount, shallowMount } from '@vue/test-utils'; import AxiosMockAdapter from 'axios-mock-adapter'; import $ from 'jquery'; -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; import setWindowLocation from 'helpers/set_window_location_helper'; import { mockTracking } from 'helpers/tracking_helper'; import waitForPromises from 'helpers/wait_for_promises'; @@ -15,7 +16,6 @@ import notesEventHub from '~/notes/event_hub'; import CommentForm from '~/notes/components/comment_form.vue'; import NotesApp from '~/notes/components/notes_app.vue'; import NotesActivityHeader from '~/notes/components/notes_activity_header.vue'; -import NotePreview from '~/notes/components/note_preview.vue'; import NoteableDiscussion from '~/notes/components/noteable_discussion.vue'; import * as constants from '~/notes/constants'; import createStore from '~/notes/stores'; @@ -25,6 +25,8 @@ import { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; import { Mousetrap } from '~/lib/mousetrap'; import { ISSUABLE_COMMENT_OR_REPLY, keysFor } from '~/behaviors/shortcuts/keybindings'; import { useFakeRequestAnimationFrame } from 'helpers/fake_request_animation_frame'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import noteQuery from '~/notes/graphql/note.query.graphql'; import * as mockData from '../mock_data'; jest.mock('~/behaviors/markdown/render_gfm'); @@ -74,6 +76,7 @@ describe('note_app', () => { $('body').attr('data-page', 'projects:merge_requests:show'); axiosMock = new AxiosMockAdapter(axios); + Vue.use(VueApollo); store = createStore(); @@ -382,15 +385,16 @@ describe('note_app', () => { }); }); - describe('preview note shown inside skeleton notes', () => { - it.each` - urlHash | exists - ${''} | ${false} - ${'heading_1'} | ${false} - ${'note_123'} | ${true} - `('`$exists` when url hash is `$urlHash`', ({ urlHash, exists }) => { + describe('preview note', () => { + let noteQueryHandler; + + function hashFactory({ urlHash, authorId } = {}) { jest.spyOn(urlUtility, 'getLocationHash').mockReturnValue(urlHash); + noteQueryHandler = jest + .fn() + .mockResolvedValue(mockData.singleNoteResponseFactory({ urlHash, authorId })); + store = createStore(); store.state.isLoading = true; store.state.targetNoteHash = urlHash; @@ -398,12 +402,43 @@ describe('note_app', () => { wrapper = shallowMount(NotesApp, { propsData, store, + apolloProvider: createMockApollo([[noteQuery, noteQueryHandler]]), stubs: { 'ordered-layout': OrderedLayout, }, }); + } + + it('calls query when note id exists', async () => { + hashFactory({ urlHash: 'note_123' }); + + expect(noteQueryHandler).toHaveBeenCalled(); + await waitForPromises(); + + expect(wrapper.findComponent(NoteableDiscussion).exists()).toBe(true); + }); + + it('converts all ids from graphql to numeric', async () => { + hashFactory({ urlHash: 'note_1234', authorId: 5 }); + + await waitForPromises(); + + const note = wrapper.findComponent(NoteableDiscussion).props('discussion').notes[0]; + + expect(note.id).toBe('1234'); + expect(note.author.id).toBe(5); + }); + + it('does not call query when note id does not exist', () => { + hashFactory(); + + expect(noteQueryHandler).not.toHaveBeenCalled(); + }); + + it('does not call query when url hash is not a note', () => { + hashFactory({ urlHash: 'not_123' }); - expect(wrapper.findComponent(NotePreview).exists()).toBe(exists); + expect(noteQueryHandler).not.toHaveBeenCalled(); }); }); diff --git a/spec/frontend/notes/mock_data.js b/spec/frontend/notes/mock_data.js index 08851d0a980c78f80193114fccc8149b80f75a89..c4069958f7d260712604ad36613264033dbb7137 100644 --- a/spec/frontend/notes/mock_data.js +++ b/spec/frontend/notes/mock_data.js @@ -1312,3 +1312,61 @@ export const notesFilters = [ value: 2, }, ]; + +export const singleNoteResponseFactory = ({ urlHash, authorId = 1 } = {}) => { + const id = urlHash?.replace('note_', '') || '5678'; + return { + data: { + note: { + id: `gid://gitlab/Note/${id}`, + discussion: { + id: 'gid://gitlab/Discussion/1', + notes: { + nodes: [ + { + id: `gid://gitlab/Note/${id}`, + author: { + id: `gid://gitlab/User/${authorId}`, + name: 'Administrator', + username: 'root', + avatar_url: '', + web_url: '', + web_path: '', + }, + award_emoji: { + nodes: [ + { + emoji: 'test', + name: 'test', + user: { + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + avatar_url: '', + web_url: '', + web_path: '', + }, + }, + ], + }, + note_html: 'my quick note', + created_at: '2020-01-01T10:00:00.000Z', + last_edited_at: null, + last_edited_by: null, + internal: false, + url: '/note/1', + userPermissions: { + awardEmoji: true, + adminNote: true, + readNote: true, + createNote: true, + resolveNote: true, + }, + }, + ], + }, + }, + }, + }, + }; +};