diff --git a/app/assets/javascripts/batch_comments/components/draft_note.vue b/app/assets/javascripts/batch_comments/components/draft_note.vue
index 4c100ec7335aebba4584be844576dade206e1d52..922033811546cf9dbe4622d867ad3bb55d5be081 100644
--- a/app/assets/javascripts/batch_comments/components/draft_note.vue
+++ b/app/assets/javascripts/batch_comments/components/draft_note.vue
@@ -3,6 +3,7 @@ import { mapActions, mapGetters, mapState } from 'vuex';
import NoteableNote from '~/notes/components/noteable_note.vue';
import LoadingButton from '~/vue_shared/components/loading_button.vue';
import PublishButton from './publish_button.vue';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default {
components: {
@@ -10,6 +11,7 @@ export default {
PublishButton,
LoadingButton,
},
+ mixins: [glFeatureFlagsMixin()],
props: {
draft: {
type: Object,
@@ -64,14 +66,27 @@ export default {
handleNotEditing() {
this.isEditingDraft = false;
},
+ handleMouseEnter(draft) {
+ if (this.glFeatures.multilineComments && draft.position) {
+ this.setSelectedCommentPositionHover(draft.position.line_range);
+ }
+ },
+ handleMouseLeave(draft) {
+ // Even though position isn't used here we still don't want to unecessarily call a mutation
+ // The lack of position tells us that highlighting is irrelevant in this context
+ if (this.glFeatures.multilineComments && draft.position) {
+ this.setSelectedCommentPositionHover();
+ }
+ },
},
};
</script>
<template>
<article
+ role="article"
class="draft-note-component note-wrapper"
- @mouseenter="setSelectedCommentPositionHover(draft.position.line_range)"
- @mouseleave="setSelectedCommentPositionHover()"
+ @mouseenter="handleMouseEnter(draft)"
+ @mouseleave="handleMouseLeave(draft)"
>
<ul class="notes draft-notes">
<noteable-note
diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue
index 458da5cf67f2b8ee2d2548058e76042d78b398f2..a1e887c47d01ffd23eb10d2ce83b732bcead7b43 100644
--- a/app/assets/javascripts/notes/components/discussion_notes.vue
+++ b/app/assets/javascripts/notes/components/discussion_notes.vue
@@ -9,6 +9,7 @@ import NoteableNote from './noteable_note.vue';
import ToggleRepliesWidget from './toggle_replies_widget.vue';
import NoteEditedText from './note_edited_text.vue';
import DiscussionNotesRepliesWrapper from './discussion_notes_replies_wrapper.vue';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default {
name: 'DiscussionNotes',
@@ -17,6 +18,7 @@ export default {
NoteEditedText,
DiscussionNotesRepliesWrapper,
},
+ mixins: [glFeatureFlagsMixin()],
props: {
discussion: {
type: Object,
@@ -93,6 +95,18 @@ export default {
componentData(note) {
return note.isPlaceholderNote ? note.notes[0] : note;
},
+ handleMouseEnter(discussion) {
+ if (this.glFeatures.multilineComments && discussion.position) {
+ this.setSelectedCommentPositionHover(discussion.position.line_range);
+ }
+ },
+ handleMouseLeave(discussion) {
+ // Even though position isn't used here we still don't want to unecessarily call a mutation
+ // The lack of position tells us that highlighting is irrelevant in this context
+ if (this.glFeatures.multilineComments && discussion.position) {
+ this.setSelectedCommentPositionHover();
+ }
+ },
},
};
</script>
@@ -101,8 +115,8 @@ export default {
<div class="discussion-notes">
<ul
class="notes"
- @mouseenter="setSelectedCommentPositionHover(discussion.position.line_range)"
- @mouseleave="setSelectedCommentPositionHover()"
+ @mouseenter="handleMouseEnter(discussion)"
+ @mouseleave="handleMouseLeave(discussion)"
>
<template v-if="shouldGroupReplies">
<component
diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue
index 9bf8cffe940a0bff5b8671992285afd518578b36..5999ded87210bb9d7bcbbcd4f909fde081e5304f 100644
--- a/app/assets/javascripts/notes/components/noteable_note.vue
+++ b/app/assets/javascripts/notes/components/noteable_note.vue
@@ -152,9 +152,10 @@ export default {
return this.line && this.startLineNumber !== this.endLineNumber;
},
+ showMultilineCommentForm() {
+ return Boolean(this.isEditing && this.note.position && this.diffFile && this.line);
+ },
commentLineOptions() {
- if (!this.diffFile || !this.line) return [];
-
const sideA = this.line.type === 'new' ? 'right' : 'left';
const sideB = sideA === 'left' ? 'right' : 'left';
const lines = this.diffFile.highlighted_diff_lines.length
@@ -339,7 +340,7 @@ export default {
>
<div v-if="showMultiLineComment" data-testid="multiline-comment">
<multiline-comment-form
- v-if="isEditing && note.position"
+ v-if="showMultilineCommentForm"
v-model="commentLineStart"
:line="line"
:comment-line-options="commentLineOptions"
diff --git a/changelogs/unreleased/jdb-fix-js-error-multilinecomment-overview.yml b/changelogs/unreleased/jdb-fix-js-error-multilinecomment-overview.yml
new file mode 100644
index 0000000000000000000000000000000000000000..4909867e25a539dbd1ecfb65caff3c2a8b20cbdd
--- /dev/null
+++ b/changelogs/unreleased/jdb-fix-js-error-multilinecomment-overview.yml
@@ -0,0 +1,5 @@
+---
+title: Fix editing note throws js error
+merge_request: 37216
+author:
+type: fixed
diff --git a/spec/frontend/batch_comments/components/draft_note_spec.js b/spec/frontend/batch_comments/components/draft_note_spec.js
index eea7f25dbc173f69051bf3d9ccce69ddcb7cf1bb..99980c98f8bb402f8ace9a83a377e3f7061038bc 100644
--- a/spec/frontend/batch_comments/components/draft_note_spec.js
+++ b/spec/frontend/batch_comments/components/draft_note_spec.js
@@ -1,4 +1,5 @@
import { shallowMount, createLocalVue } from '@vue/test-utils';
+import { getByRole } from '@testing-library/dom';
import DraftNote from '~/batch_comments/components/draft_note.vue';
import { createStore } from '~/batch_comments/stores';
import NoteableNote from '~/notes/components/noteable_note.vue';
@@ -8,21 +9,34 @@ import { createDraft } from '../mock_data';
const localVue = createLocalVue();
describe('Batch comments draft note component', () => {
+ let store;
let wrapper;
let draft;
+ const LINE_RANGE = {};
+ const draftWithLineRange = {
+ position: {
+ line_range: LINE_RANGE,
+ },
+ };
- beforeEach(() => {
- const store = createStore();
-
- draft = createDraft();
+ const getList = () => getByRole(wrapper.element, 'list');
+ const createComponent = (propsData = { draft }, features = {}) => {
wrapper = shallowMount(localVue.extend(DraftNote), {
store,
- propsData: { draft },
+ propsData,
localVue,
+ provide: {
+ glFeatures: { multilineComments: true, ...features },
+ },
});
jest.spyOn(wrapper.vm.$store, 'dispatch').mockImplementation();
+ };
+
+ beforeEach(() => {
+ store = createStore();
+ draft = createDraft();
});
afterEach(() => {
@@ -30,6 +44,7 @@ describe('Batch comments draft note component', () => {
});
it('renders template', () => {
+ createComponent();
expect(wrapper.find('.draft-pending-label').exists()).toBe(true);
const note = wrapper.find(NoteableNote);
@@ -40,6 +55,7 @@ describe('Batch comments draft note component', () => {
describe('add comment now', () => {
it('dispatches publishSingleDraft when clicking', () => {
+ createComponent();
const publishNowButton = wrapper.find({ ref: 'publishNowButton' });
publishNowButton.vm.$emit('click');
@@ -50,6 +66,7 @@ describe('Batch comments draft note component', () => {
});
it('sets as loading when draft is publishing', done => {
+ createComponent();
wrapper.vm.$store.state.batchComments.currentlyPublishingDrafts.push(1);
wrapper.vm.$nextTick(() => {
@@ -64,6 +81,7 @@ describe('Batch comments draft note component', () => {
describe('update', () => {
it('dispatches updateDraft', done => {
+ createComponent();
const note = wrapper.find(NoteableNote);
note.vm.$emit('handleEdit');
@@ -91,6 +109,7 @@ describe('Batch comments draft note component', () => {
describe('deleteDraft', () => {
it('dispatches deleteDraft', () => {
+ createComponent();
jest.spyOn(window, 'confirm').mockImplementation(() => true);
const note = wrapper.find(NoteableNote);
@@ -103,6 +122,7 @@ describe('Batch comments draft note component', () => {
describe('quick actions', () => {
it('renders referenced commands', done => {
+ createComponent();
wrapper.setProps({
draft: {
...draft,
@@ -122,4 +142,26 @@ describe('Batch comments draft note component', () => {
});
});
});
+
+ describe('multiline comments', () => {
+ describe.each`
+ desc | props | features | event | expectedCalls
+ ${'with `draft.position`'} | ${draftWithLineRange} | ${{}} | ${'mouseenter'} | ${[['setSelectedCommentPositionHover', LINE_RANGE]]}
+ ${'with `draft.position`'} | ${draftWithLineRange} | ${{}} | ${'mouseleave'} | ${[['setSelectedCommentPositionHover']]}
+ ${'with `draft.position`'} | ${draftWithLineRange} | ${{ multilineComments: false }} | ${'mouseenter'} | ${[]}
+ ${'with `draft.position`'} | ${draftWithLineRange} | ${{ multilineComments: false }} | ${'mouseleave'} | ${[]}
+ ${'without `draft.position`'} | ${{}} | ${{}} | ${'mouseenter'} | ${[]}
+ ${'without `draft.position`'} | ${{}} | ${{}} | ${'mouseleave'} | ${[]}
+ `('$desc and features $features', ({ props, event, features, expectedCalls }) => {
+ beforeEach(() => {
+ createComponent({ draft: { ...draft, ...props } }, features);
+ jest.spyOn(store, 'dispatch');
+ });
+
+ it(`calls store ${expectedCalls.length} times on ${event}`, () => {
+ getList().dispatchEvent(new MouseEvent(event, { bubbles: true }));
+ expect(store.dispatch.mock.calls).toEqual(expectedCalls);
+ });
+ });
+ });
});
diff --git a/spec/frontend/notes/components/discussion_notes_spec.js b/spec/frontend/notes/components/discussion_notes_spec.js
index 5a10deefd0943c9e8932c4a85e7f785707381ab2..8cc98f978c2a09afc49d8c5399b1ec7f81a973cb 100644
--- a/spec/frontend/notes/components/discussion_notes_spec.js
+++ b/spec/frontend/notes/components/discussion_notes_spec.js
@@ -1,4 +1,5 @@
import { shallowMount } from '@vue/test-utils';
+import { getByRole } from '@testing-library/dom';
import '~/behaviors/markdown/render_gfm';
import { SYSTEM_NOTE } from '~/notes/constants';
import DiscussionNotes from '~/notes/components/discussion_notes.vue';
@@ -9,14 +10,20 @@ import SystemNote from '~/vue_shared/components/notes/system_note.vue';
import createStore from '~/notes/stores';
import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data';
+const LINE_RANGE = {};
+const DISCUSSION_WITH_LINE_RANGE = {
+ ...discussionMock,
+ position: {
+ line_range: LINE_RANGE,
+ },
+};
+
describe('DiscussionNotes', () => {
+ let store;
let wrapper;
- const createComponent = props => {
- const store = createStore();
- store.dispatch('setNoteableData', noteableDataMock);
- store.dispatch('setNotesData', notesDataMock);
-
+ const getList = () => getByRole(wrapper.element, 'list');
+ const createComponent = (props, features = {}) => {
wrapper = shallowMount(DiscussionNotes, {
store,
propsData: {
@@ -31,11 +38,21 @@ describe('DiscussionNotes', () => {
slots: {
'avatar-badge': '<span class="avatar-badge-slot-content" />',
},
+ provide: {
+ glFeatures: { multilineComments: true, ...features },
+ },
});
};
+ beforeEach(() => {
+ store = createStore();
+ store.dispatch('setNoteableData', noteableDataMock);
+ store.dispatch('setNotesData', notesDataMock);
+ });
+
afterEach(() => {
wrapper.destroy();
+ wrapper = null;
});
describe('rendering', () => {
@@ -160,6 +177,26 @@ describe('DiscussionNotes', () => {
});
});
+ describe.each`
+ desc | props | features | event | expectedCalls
+ ${'with `discussion.position`'} | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{}} | ${'mouseenter'} | ${[['setSelectedCommentPositionHover', LINE_RANGE]]}
+ ${'with `discussion.position`'} | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{}} | ${'mouseleave'} | ${[['setSelectedCommentPositionHover']]}
+ ${'with `discussion.position`'} | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{ multilineComments: false }} | ${'mouseenter'} | ${[]}
+ ${'with `discussion.position`'} | ${{ discussion: DISCUSSION_WITH_LINE_RANGE }} | ${{ multilineComments: false }} | ${'mouseleave'} | ${[]}
+ ${'without `discussion.position`'} | ${{}} | ${{}} | ${'mouseenter'} | ${[]}
+ ${'without `discussion.position`'} | ${{}} | ${{}} | ${'mouseleave'} | ${[]}
+ `('$desc and features $features', ({ props, event, features, expectedCalls }) => {
+ beforeEach(() => {
+ createComponent(props, features);
+ jest.spyOn(store, 'dispatch');
+ });
+
+ it(`calls store ${expectedCalls.length} times on ${event}`, () => {
+ getList().dispatchEvent(new MouseEvent(event));
+ expect(store.dispatch.mock.calls).toEqual(expectedCalls);
+ });
+ });
+
describe('componentData', () => {
beforeEach(() => {
createComponent();
diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js
index fc238feb974175d58986137a686637dfde5e90e9..b0b43075c3d22cb0816a01b22e2ca4c1cf6717fa 100644
--- a/spec/frontend/notes/components/noteable_note_spec.js
+++ b/spec/frontend/notes/components/noteable_note_spec.js
@@ -92,8 +92,8 @@ describe('issue_note', () => {
});
});
- it('should not render multiline comment form unless it is the discussion root', () => {
- wrapper.setProps({ discussionRoot: false });
+ it('should only render multiline comment form if it has everything it needs', () => {
+ wrapper.setProps({ line: { line_code: '' } });
wrapper.vm.isEditing = true;
return wrapper.vm.$nextTick().then(() => {