Commit c3468692 authored by Paul Slaughter's avatar Paul Slaughter

Merge branch '8225-allow-add-discussion-to-review' into 'master'

Allow Add Comment To Review

See merge request gitlab-org/gitlab!51718
parents 996f03eb 08721092
...@@ -41,13 +41,17 @@ export default { ...@@ -41,13 +41,17 @@ export default {
titleText() { titleText() {
const file = this.discussion ? this.discussion.diff_file : this.draft; const file = this.discussion ? this.discussion.diff_file : this.draft;
if (file) { if (file?.file_path) {
return file.file_path; return file.file_path;
} }
return sprintf(__("%{authorsName}'s thread"), { if (this.discussion) {
authorsName: this.discussion.notes.find((note) => !note.system).author.name, return sprintf(__("%{authorsName}'s thread"), {
}); authorsName: this.discussion.notes.find((note) => !note.system).author.name,
});
}
return __('Your new comment');
}, },
linePosition() { linePosition() {
if (this.position?.position_type === IMAGE_DIFF_POSITION_TYPE) { if (this.position?.position_type === IMAGE_DIFF_POSITION_TYPE) {
...@@ -94,7 +98,7 @@ export default { ...@@ -94,7 +98,7 @@ export default {
<span class="review-preview-item-header"> <span class="review-preview-item-header">
<gl-icon class="flex-shrink-0" :name="iconName" /> <gl-icon class="flex-shrink-0" :name="iconName" />
<span class="bold text-nowrap gl-align-items-center"> <span class="bold text-nowrap gl-align-items-center">
<span class="review-preview-item-header-text block-truncated"> <span class="review-preview-item-header-text block-truncated gl-ml-2">
{{ titleText }} {{ titleText }}
</span> </span>
<template v-if="showLinePosition"> <template v-if="showLinePosition">
......
...@@ -84,6 +84,7 @@ export default { ...@@ -84,6 +84,7 @@ export default {
'getNoteableDataByProp', 'getNoteableDataByProp',
'getNotesData', 'getNotesData',
'openState', 'openState',
'hasDrafts',
]), ]),
...mapState(['isToggleStateButtonLoading']), ...mapState(['isToggleStateButtonLoading']),
isNoteTypeComment() { isNoteTypeComment() {
...@@ -171,6 +172,9 @@ export default { ...@@ -171,6 +172,9 @@ export default {
endpoint() { endpoint() {
return this.getNoteableData.create_note_path; return this.getNoteableData.create_note_path;
}, },
draftEndpoint() {
return this.getNotesData.draftsPath;
},
issuableTypeTitle() { issuableTypeTitle() {
return this.noteableType === constants.MERGE_REQUEST_NOTEABLE_TYPE return this.noteableType === constants.MERGE_REQUEST_NOTEABLE_TYPE
? this.$options.i18n.mergeRequest ? this.$options.i18n.mergeRequest
...@@ -214,12 +218,15 @@ export default { ...@@ -214,12 +218,15 @@ export default {
this.errors = [this.$options.i18n.GENERIC_UNSUBMITTABLE_NETWORK]; this.errors = [this.$options.i18n.GENERIC_UNSUBMITTABLE_NETWORK];
} }
}, },
handleSave(withIssueAction) { handleSaveDraft() {
this.handleSave({ isDraft: true });
},
handleSave({ withIssueAction = false, isDraft = false } = {}) {
this.errors = []; this.errors = [];
if (this.note.length) { if (this.note.length) {
const noteData = { const noteData = {
endpoint: this.endpoint, endpoint: isDraft ? this.draftEndpoint : this.endpoint,
data: { data: {
note: { note: {
noteable_type: this.noteableType, noteable_type: this.noteableType,
...@@ -229,6 +236,7 @@ export default { ...@@ -229,6 +236,7 @@ export default {
}, },
merge_request_diff_head_sha: this.getNoteableData.diff_head_sha, merge_request_diff_head_sha: this.getNoteableData.diff_head_sha,
}, },
isDraft,
}; };
if (this.noteType === constants.DISCUSSION) { if (this.noteType === constants.DISCUSSION) {
...@@ -392,62 +400,82 @@ export default { ...@@ -392,62 +400,82 @@ export default {
</markdown-field> </markdown-field>
</comment-field-layout> </comment-field-layout>
<div class="note-form-actions"> <div class="note-form-actions">
<gl-form-checkbox <template v-if="hasDrafts">
v-if="confidentialNotesEnabled && canSetConfidential" <gl-button
v-model="noteIsConfidential" :disabled="disableSubmitButton"
class="gl-mb-6" data-testid="add-to-review-button"
data-testid="confidential-note-checkbox" type="submit"
> category="primary"
{{ $options.i18n.confidential }} variant="success"
<gl-icon @click.prevent="handleSaveDraft()"
v-gl-tooltip:tooltipcontainer.bottom >{{ __('Add to review') }}</gl-button
name="question" >
:size="16" <gl-button
:title="$options.i18n.confidentialVisibility" :disabled="disableSubmitButton"
class="gl-text-gray-500" data-testid="add-comment-now-button"
/> category="secondary"
</gl-form-checkbox> @click.prevent="handleSave()"
<gl-dropdown >{{ __('Add comment now') }}</gl-button
split >
:text="commentButtonTitle" </template>
class="gl-mr-3 js-comment-button js-comment-submit-button comment-type-dropdown" <template v-else>
category="primary" <gl-form-checkbox
variant="confirm" v-if="confidentialNotesEnabled && canSetConfidential"
:disabled="disableSubmitButton" v-model="noteIsConfidential"
data-testid="comment-button" class="gl-mb-6"
data-qa-selector="comment_button" data-testid="confidential-note-checkbox"
:data-track-label="trackingLabel"
data-track-event="click_button"
@click="handleSave()"
>
<gl-dropdown-item
is-check-item
:is-checked="isNoteTypeComment"
:selected="isNoteTypeComment"
@click="setNoteTypeToComment"
> >
<strong>{{ $options.i18n.submitButton.comment }}</strong> {{ $options.i18n.confidential }}
<p class="gl-m-0">{{ commentDescription }}</p> <gl-icon
</gl-dropdown-item> v-gl-tooltip:tooltipcontainer.bottom
<gl-dropdown-divider /> name="question"
<gl-dropdown-item :size="16"
is-check-item :title="$options.i18n.confidentialVisibility"
:is-checked="isNoteTypeDiscussion" class="gl-text-gray-500"
:selected="isNoteTypeDiscussion" />
data-qa-selector="discussion_menu_item" </gl-form-checkbox>
@click="setNoteTypeToDiscussion" <gl-dropdown
split
:text="commentButtonTitle"
class="gl-mr-3 js-comment-button js-comment-submit-button comment-type-dropdown"
category="primary"
variant="confirm"
:disabled="disableSubmitButton"
data-testid="comment-button"
data-qa-selector="comment_button"
:data-track-label="trackingLabel"
data-track-event="click_button"
@click="handleSave()"
> >
<strong>{{ $options.i18n.submitButton.startThread }}</strong> <gl-dropdown-item
<p class="gl-m-0">{{ startDiscussionDescription }}</p> is-check-item
</gl-dropdown-item> :is-checked="isNoteTypeComment"
</gl-dropdown> :selected="isNoteTypeComment"
@click="setNoteTypeToComment"
>
<strong>{{ $options.i18n.submitButton.comment }}</strong>
<p class="gl-m-0">{{ commentDescription }}</p>
</gl-dropdown-item>
<gl-dropdown-divider />
<gl-dropdown-item
is-check-item
:is-checked="isNoteTypeDiscussion"
:selected="isNoteTypeDiscussion"
data-qa-selector="discussion_menu_item"
@click="setNoteTypeToDiscussion"
>
<strong>{{ $options.i18n.submitButton.startThread }}</strong>
<p class="gl-m-0">{{ startDiscussionDescription }}</p>
</gl-dropdown-item>
</gl-dropdown>
</template>
<gl-button <gl-button
v-if="canToggleIssueState" v-if="canToggleIssueState"
:loading="isToggleStateButtonLoading" :loading="isToggleStateButtonLoading"
:class="[actionButtonClassNames, 'btn-comment btn-comment-and-close']" :class="[actionButtonClassNames, 'btn-comment btn-comment-and-close']"
:disabled="isSubmitting" :disabled="isSubmitting"
data-testid="close-reopen-button" data-testid="close-reopen-button"
@click="handleSave(true)" @click="handleSave({ withIssueAction: true })"
>{{ issueActionButtonTitle }}</gl-button >{{ issueActionButtonTitle }}</gl-button
> >
</div> </div>
......
...@@ -3,8 +3,10 @@ import { mapGetters, mapActions } from 'vuex'; ...@@ -3,8 +3,10 @@ import { mapGetters, mapActions } from 'vuex';
import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user'; import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user';
import { __ } from '~/locale'; import { __ } from '~/locale';
import initUserPopovers from '~/user_popovers'; import initUserPopovers from '~/user_popovers';
import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
import OrderedLayout from '~/vue_shared/components/ordered_layout.vue'; import OrderedLayout from '~/vue_shared/components/ordered_layout.vue';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import draftNote from '../../batch_comments/components/draft_note.vue';
import { deprecatedCreateFlash as Flash } from '../../flash'; import { deprecatedCreateFlash as Flash } from '../../flash';
import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility'; import { getLocationHash, doesHashExistInUrl } from '../../lib/utils/url_utility';
import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue'; import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
...@@ -32,6 +34,8 @@ export default { ...@@ -32,6 +34,8 @@ export default {
discussionFilterNote, discussionFilterNote,
OrderedLayout, OrderedLayout,
SidebarSubscription, SidebarSubscription,
draftNote,
TimelineEntryItem,
}, },
mixins: [glFeatureFlagsMixin()], mixins: [glFeatureFlagsMixin()],
props: { props: {
...@@ -276,6 +280,9 @@ export default { ...@@ -276,6 +280,9 @@ export default {
<ul id="notes-list" class="notes main-notes-list timeline"> <ul id="notes-list" class="notes main-notes-list timeline">
<template v-for="discussion in allDiscussions"> <template v-for="discussion in allDiscussions">
<skeleton-loading-container v-if="discussion.isSkeletonNote" :key="discussion.id" /> <skeleton-loading-container v-if="discussion.isSkeletonNote" :key="discussion.id" />
<timeline-entry-item v-else-if="discussion.isDraft" :key="discussion.id">
<draft-note :draft="discussion" />
</timeline-entry-item>
<template v-else-if="discussion.isPlaceholderNote"> <template v-else-if="discussion.isPlaceholderNote">
<placeholder-system-note <placeholder-system-note
v-if="discussion.placeholderType === $options.systemNote" v-if="discussion.placeholderType === $options.systemNote"
......
...@@ -2,7 +2,23 @@ import { flattenDeep, clone } from 'lodash'; ...@@ -2,7 +2,23 @@ import { flattenDeep, clone } from 'lodash';
import * as constants from '../constants'; import * as constants from '../constants';
import { collapseSystemNotes } from './collapse_utils'; import { collapseSystemNotes } from './collapse_utils';
export const discussions = (state) => { const getDraftComments = (state) => {
if (!state.batchComments) {
return [];
}
return state.batchComments.drafts
.filter((draft) => !draft.line_code && !draft.discussion_id)
.map((x) => ({
...x,
// Treat a top-level draft note as individual_note so it's not included in
// expand/collapse threads
individual_note: true,
}))
.sort((a, b) => a.id - b.id);
};
export const discussions = (state, getters, rootState) => {
let discussionsInState = clone(state.discussions); let discussionsInState = clone(state.discussions);
// NOTE: not testing bc will be removed when backend is finished. // NOTE: not testing bc will be removed when backend is finished.
...@@ -22,11 +38,15 @@ export const discussions = (state) => { ...@@ -22,11 +38,15 @@ export const discussions = (state) => {
.sort((a, b) => new Date(a.created_at) - new Date(b.created_at)); .sort((a, b) => new Date(a.created_at) - new Date(b.created_at));
} }
discussionsInState = collapseSystemNotes(discussionsInState);
discussionsInState = discussionsInState.concat(getDraftComments(rootState));
if (state.discussionSortOrder === constants.DESC) { if (state.discussionSortOrder === constants.DESC) {
discussionsInState = discussionsInState.reverse(); discussionsInState = discussionsInState.reverse();
} }
return collapseSystemNotes(discussionsInState); return discussionsInState;
}; };
export const convertedDisscussionIds = (state) => state.convertedDisscussionIds; export const convertedDisscussionIds = (state) => state.convertedDisscussionIds;
...@@ -257,3 +277,6 @@ export const commentsDisabled = (state) => state.commentsDisabled; ...@@ -257,3 +277,6 @@ export const commentsDisabled = (state) => state.commentsDisabled;
export const suggestionsCount = (state, getters) => export const suggestionsCount = (state, getters) =>
Object.values(getters.notesById).filter((n) => n.suggestions.length).length; Object.values(getters.notesById).filter((n) => n.suggestions.length).length;
export const hasDrafts = (state, getters, rootState, rootGetters) =>
Boolean(rootGetters['batchComments/hasDrafts']);
---
title: Allow Add Comment To Review
merge_request: 51718
author: Lee Tickett @leetickett
type: added
...@@ -334,22 +334,28 @@ comment itself. ...@@ -334,22 +334,28 @@ comment itself.
![Unresolve status](img/mr_review_unresolve.png) ![Unresolve status](img/mr_review_unresolve.png)
### Adding a new comment
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/8225) in GitLab 13.10.
If you have a review in progress, you will be presented with the option to **Add to review**:
![New thread](img/mr_review_new_comment_v13_11.png)
### Submitting a review ### Submitting a review
If you have any comments that have not been submitted, a bar displays at the If you have any comments that have not been submitted, a bar displays at the
bottom of the screen with two buttons: bottom of the screen with two buttons:
- **Discard**: Discards all comments that have not been submitted. - **Pending comments**: Opens a list of comments ready to be submitted for review.
- **Finish review**: Opens a list of comments ready to be submitted for review. - **Submit review**: Publishes all comments. Any quick actions submitted are performed at this time.
Clicking **Submit review** publishes all comments. Any quick actions
submitted are performed at this time.
Alternatively, to finish the entire review from a pending comment: Alternatively, to finish the entire review from a pending comment:
- Click the **Finish review** button on the comment. - Click the **Submit review** button on the comment.
- Use the `/submit_review` [quick action](../project/quick_actions.md) in the text of non-review comment. - Use the `/submit_review` [quick action](../project/quick_actions.md) in the text of non-review comment.
![Review submission](img/review_preview.png) ![Review submission](img/review_preview_v13_11.png)
Submitting the review sends a single email to every notifiable user of the Submitting the review sends a single email to every notifiable user of the
merge request with all the comments associated to it. merge request with all the comments associated to it.
......
...@@ -40,20 +40,18 @@ button[disabled] { ...@@ -40,20 +40,18 @@ button[disabled] {
vertical-align: text-top; vertical-align: text-top;
} }
.discussion-body, .note.draft-note {
.diff-file { margin: 0 0 $gl-padding;
.notes .note { padding: 0;
&.draft-note { border: 0;
margin: 0 0 $gl-padding;
padding: 0;
border: 0;
&.is-editing { &.is-editing {
margin-bottom: 0; margin-bottom: 0;
}
}
} }
}
.discussion-body,
.diff-file {
.notes_holder { .notes_holder {
.notes-content { .notes-content {
.notes { .notes {
......
...@@ -36303,6 +36303,9 @@ msgstr "" ...@@ -36303,6 +36303,9 @@ msgstr ""
msgid "Your new SCIM token" msgid "Your new SCIM token"
msgstr "" msgstr ""
msgid "Your new comment"
msgstr ""
msgid "Your new personal access token has been created." msgid "Your new personal access token has been created."
msgstr "" msgstr ""
......
...@@ -28,7 +28,7 @@ RSpec.describe 'Merge request > Batch comments', :js do ...@@ -28,7 +28,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
end end
it 'adds draft note' do it 'adds draft note' do
write_comment write_diff_comment
expect(find('.draft-note-component')).to have_content('Line is wrong') expect(find('.draft-note-component')).to have_content('Line is wrong')
...@@ -38,7 +38,7 @@ RSpec.describe 'Merge request > Batch comments', :js do ...@@ -38,7 +38,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
end end
it 'publishes review' do it 'publishes review' do
write_comment write_diff_comment
page.within('.review-bar-content') do page.within('.review-bar-content') do
click_button 'Submit review' click_button 'Submit review'
...@@ -52,7 +52,7 @@ RSpec.describe 'Merge request > Batch comments', :js do ...@@ -52,7 +52,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
end end
it 'publishes single comment' do it 'publishes single comment' do
write_comment write_diff_comment
click_button 'Add comment now' click_button 'Add comment now'
...@@ -64,7 +64,7 @@ RSpec.describe 'Merge request > Batch comments', :js do ...@@ -64,7 +64,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
end end
it 'deletes draft note' do it 'deletes draft note' do
write_comment write_diff_comment
accept_alert { find('.js-note-delete').click } accept_alert { find('.js-note-delete').click }
...@@ -74,21 +74,57 @@ RSpec.describe 'Merge request > Batch comments', :js do ...@@ -74,21 +74,57 @@ RSpec.describe 'Merge request > Batch comments', :js do
end end
it 'edits draft note' do it 'edits draft note' do
write_comment write_diff_comment
find('.js-note-edit').click find('.js-note-edit').click
# make sure comment form is in view # make sure comment form is in view
execute_script("window.scrollBy(0, 200)") execute_script("window.scrollBy(0, 200)")
page.within('.js-discussion-note-form') do write_comment(text: 'Testing update', button_text: 'Save comment')
fill_in('note_note', with: 'Testing update')
click_button('Save comment') expect(page).to have_selector('.draft-note-component', text: 'Testing update')
end
context 'adding single comment to review' do
before do
visit_overview
end end
wait_for_requests it 'at first does not show `Add to review` and `Add comment now` buttons' do
expect(page).to have_no_button('Add to review')
expect(page).to have_no_button('Add comment now')
end
expect(page).to have_selector('.draft-note-component', text: 'Testing update') context 'when review has started' do
before do
visit_diffs
write_diff_comment
visit_overview
end
it 'can add comment to review' do
write_comment(selector: '.js-main-target-form', field: 'note-body', text: 'Its a draft comment', button_text: 'Add to review')
expect(page).to have_selector('.draft-note-component', text: 'Its a draft comment')
click_button('Pending comments')
expect(page).to have_text('2 pending comments')
end
it 'can add comment right away' do
write_comment(selector: '.js-main-target-form', field: 'note-body', text: 'Its a regular comment', button_text: 'Add comment now')
expect(page).to have_selector('.note:not(.draft-note)', text: 'Its a regular comment')
click_button('Pending comments')
expect(page).to have_text('1 pending comment')
end
end
end end
context 'in parallel diff' do context 'in parallel diff' do
...@@ -197,46 +233,51 @@ RSpec.describe 'Merge request > Batch comments', :js do ...@@ -197,46 +233,51 @@ RSpec.describe 'Merge request > Batch comments', :js do
wait_for_requests wait_for_requests
end end
def write_comment(button_text: 'Start a review', text: 'Line is wrong') def visit_overview
click_diff_line(find("[id='#{sample_compare.changes[0][:line_code]}']")) visit project_merge_request_path(merge_request.project, merge_request)
page.within('.js-discussion-note-form') do
fill_in('note_note', with: text)
click_button(button_text)
end
wait_for_requests wait_for_requests
end end
def write_parallel_comment(line, button_text: 'Start a review', text: 'Line is wrong') def write_diff_comment(**params)
click_diff_line(find("[id='#{sample_compare.changes[0][:line_code]}']"))
write_comment(**params)
end
def write_parallel_comment(line, **params)
find("td[id='#{line}']").hover find("td[id='#{line}']").hover
find(".is-over button").click find(".is-over button").click
page.within("form[data-line-code='#{line}']") do write_comment(selector: "form[data-line-code='#{line}']", **params)
fill_in('note_note', with: text) end
def write_comment(selector: '.js-discussion-note-form', field: 'note_note', button_text: 'Start a review', text: 'Line is wrong')
page.within(selector) do
fill_in(field, with: text)
click_button(button_text) click_button(button_text)
end end
wait_for_requests wait_for_requests
end end
end
def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false) def write_reply_to_discussion(button_text: 'Start a review', text: 'Line is wrong', resolve: false, unresolve: false)
page.within(first('.diff-files-holder .discussion-reply-holder')) do page.within(first('.diff-files-holder .discussion-reply-holder')) do
find_field('Reply…', match: :first).click find_field('Reply…', match: :first).click
fill_in('note_note', with: text) fill_in('note_note', with: text)
if resolve if resolve
page.check('Resolve thread') page.check('Resolve thread')
end end
if unresolve if unresolve
page.check('Unresolve thread') page.check('Unresolve thread')
end
click_button(button_text)
end end
click_button(button_text) wait_for_requests
end end
wait_for_requests
end end
...@@ -124,4 +124,16 @@ describe('Batch comments draft preview item component', () => { ...@@ -124,4 +124,16 @@ describe('Batch comments draft preview item component', () => {
); );
}); });
}); });
describe('for new comment', () => {
it('renders title', () => {
createComponent(false, {}, (store) => {
store.state.notes.discussions.push({});
});
expect(vm.$el.querySelector('.review-preview-item-header-text').textContent).toContain(
'Your new comment',
);
});
});
}); });
import { GlDropdown, GlAlert } from '@gitlab/ui'; import { GlAlert } from '@gitlab/ui';
import { mount, shallowMount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
import Autosize from 'autosize'; import Autosize from 'autosize';
import MockAdapter from 'axios-mock-adapter'; import MockAdapter from 'axios-mock-adapter';
import Vue, { nextTick } from 'vue'; import Vue, { nextTick } from 'vue';
import Vuex from 'vuex'; import Vuex from 'vuex';
import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import batchComments from '~/batch_comments/stores/modules/batch_comments';
import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests'; import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests';
import { deprecatedCreateFlash as flash } from '~/flash'; import { deprecatedCreateFlash as flash } from '~/flash';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
...@@ -29,8 +30,10 @@ describe('issue_comment_form component', () => { ...@@ -29,8 +30,10 @@ describe('issue_comment_form component', () => {
const findCloseReopenButton = () => wrapper.findByTestId('close-reopen-button'); const findCloseReopenButton = () => wrapper.findByTestId('close-reopen-button');
const findTextArea = () => wrapper.findByTestId('comment-field'); const findTextArea = () => wrapper.findByTestId('comment-field');
const findAddToReviewButton = () => wrapper.findByTestId('add-to-review-button');
const findAddCommentNowButton = () => wrapper.findByTestId('add-comment-now-button');
const findConfidentialNoteCheckbox = () => wrapper.findByTestId('confidential-note-checkbox'); const findConfidentialNoteCheckbox = () => wrapper.findByTestId('confidential-note-checkbox');
const findCommentGlDropdown = () => wrapper.find(GlDropdown); const findCommentGlDropdown = () => wrapper.findByTestId('comment-button');
const findCommentButton = () => findCommentGlDropdown().find('button'); const findCommentButton = () => findCommentGlDropdown().find('button');
const findErrorAlerts = () => wrapper.findAllComponents(GlAlert).wrappers; const findErrorAlerts = () => wrapper.findAllComponents(GlAlert).wrappers;
...@@ -582,4 +585,64 @@ describe('issue_comment_form component', () => { ...@@ -582,4 +585,64 @@ describe('issue_comment_form component', () => {
expect(findTextArea().exists()).toBe(false); expect(findTextArea().exists()).toBe(false);
}); });
}); });
describe('with batchComments in store', () => {
beforeEach(() => {
store.registerModule('batchComments', batchComments());
});
describe('add to review and comment now buttons', () => {
it('when no drafts exist, should not render', () => {
mountComponent();
expect(findCommentGlDropdown().exists()).toBe(true);
expect(findAddToReviewButton().exists()).toBe(false);
expect(findAddCommentNowButton().exists()).toBe(false);
});
describe('when drafts exist', () => {
beforeEach(() => {
store.state.batchComments.drafts = [{ note: 'A' }];
});
it('should render', () => {
mountComponent();
expect(findCommentGlDropdown().exists()).toBe(false);
expect(findAddToReviewButton().exists()).toBe(true);
expect(findAddCommentNowButton().exists()).toBe(true);
});
it('clicking `add to review`, should call draft endpoint, set `isDraft` true', () => {
mountComponent({ mountFunction: mount, initialData: { note: 'a draft note' } });
jest.spyOn(store, 'dispatch').mockResolvedValue();
findAddToReviewButton().trigger('click');
expect(store.dispatch).toHaveBeenCalledWith(
'saveNote',
expect.objectContaining({
endpoint: notesDataMock.draftsPath,
isDraft: true,
}),
);
});
it('clicking `add comment now`, should call note endpoint, set `isDraft` false ', () => {
mountComponent({ mountFunction: mount, initialData: { note: 'a comment' } });
jest.spyOn(store, 'dispatch').mockResolvedValue();
findAddCommentNowButton().trigger('click');
expect(store.dispatch).toHaveBeenCalledWith(
'saveNote',
expect.objectContaining({
endpoint: noteableDataMock.create_note_path,
isDraft: false,
}),
);
});
});
});
});
}); });
...@@ -3,6 +3,8 @@ import AxiosMockAdapter from 'axios-mock-adapter'; ...@@ -3,6 +3,8 @@ import AxiosMockAdapter from 'axios-mock-adapter';
import $ from 'jquery'; import $ from 'jquery';
import Vue from 'vue'; import Vue from 'vue';
import { setTestTimeout } from 'helpers/timeout'; import { setTestTimeout } from 'helpers/timeout';
import DraftNote from '~/batch_comments/components/draft_note.vue';
import batchComments from '~/batch_comments/stores/modules/batch_comments';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
import * as urlUtility from '~/lib/utils/url_utility'; import * as urlUtility from '~/lib/utils/url_utility';
import CommentForm from '~/notes/components/comment_form.vue'; import CommentForm from '~/notes/components/comment_form.vue';
...@@ -400,4 +402,34 @@ describe('note_app', () => { ...@@ -400,4 +402,34 @@ describe('note_app', () => {
expect(getComponentOrder()).toStrictEqual([TYPE_NOTES_LIST, TYPE_COMMENT_FORM]); expect(getComponentOrder()).toStrictEqual([TYPE_NOTES_LIST, TYPE_COMMENT_FORM]);
}); });
}); });
describe('when multiple draft types are present', () => {
beforeEach(() => {
store = createStore();
store.registerModule('batchComments', batchComments());
store.state.batchComments.drafts = [
{ line_code: 1, isDraft: true },
{ discussion_id: 1, isDraft: true },
{ note: 'A', isDraft: true },
{ note: 'B', isDraft: true },
];
store.state.isLoading = false;
wrapper = shallowMount(NotesApp, {
propsData,
store,
stubs: {
OrderedLayout,
},
});
});
it('correctly finds only draft comments', () => {
const drafts = wrapper.findAll(DraftNote).wrappers;
expect(drafts.map((x) => x.props('draft'))).toEqual([
expect.objectContaining({ note: 'A' }),
expect.objectContaining({ note: 'B' }),
]);
});
});
}); });
...@@ -6,6 +6,7 @@ export const notesDataMock = { ...@@ -6,6 +6,7 @@ export const notesDataMock = {
markdownDocsPath: '/help/user/markdown', markdownDocsPath: '/help/user/markdown',
newSessionPath: '/users/sign_in?redirect_to_referer=yes', newSessionPath: '/users/sign_in?redirect_to_referer=yes',
notesPath: '/gitlab-org/gitlab-foss/noteable/issue/98/notes', notesPath: '/gitlab-org/gitlab-foss/noteable/issue/98/notes',
draftsPath: '/flightjs/flight/-/merge_requests/4/drafts',
quickActionsDocsPath: '/help/user/project/quick_actions', quickActionsDocsPath: '/help/user/project/quick_actions',
registerPath: '/users/sign_up?redirect_to_referer=yes', registerPath: '/users/sign_up?redirect_to_referer=yes',
prerenderedNotesCount: 1, prerenderedNotesCount: 1,
...@@ -1270,3 +1271,12 @@ export const batchSuggestionsInfoMock = [ ...@@ -1270,3 +1271,12 @@ export const batchSuggestionsInfoMock = [
discussionId: 'c003', discussionId: 'c003',
}, },
]; ];
export const draftComments = [
{ id: 7, note: 'test draft note' },
{ id: 9, note: 'draft note 2' },
];
export const draftReply = { id: 8, note: 'draft reply', discussion_id: 1 };
export const draftDiffDiscussion = { id: 6, note: 'draft diff discussion', line_code: 1 };
import { DESC } from '~/notes/constants'; import { DESC, ASC } from '~/notes/constants';
import * as getters from '~/notes/stores/getters'; import * as getters from '~/notes/stores/getters';
import { import {
notesDataMock, notesDataMock,
...@@ -12,6 +12,9 @@ import { ...@@ -12,6 +12,9 @@ import {
discussion3, discussion3,
resolvedDiscussion1, resolvedDiscussion1,
unresolvableDiscussion, unresolvableDiscussion,
draftComments,
draftReply,
draftDiffDiscussion,
} from '../mock_data'; } from '../mock_data';
const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
...@@ -23,6 +26,8 @@ const createDiscussionNeighborParams = (discussionId, diffOrder, step) => ({ ...@@ -23,6 +26,8 @@ const createDiscussionNeighborParams = (discussionId, diffOrder, step) => ({
step, step,
}); });
const asDraftDiscussion = (x) => ({ ...x, individual_note: true });
describe('Getters Notes Store', () => { describe('Getters Notes Store', () => {
let state; let state;
...@@ -61,20 +66,58 @@ describe('Getters Notes Store', () => { ...@@ -61,20 +66,58 @@ describe('Getters Notes Store', () => {
}); });
describe('discussions', () => { describe('discussions', () => {
it('should return all discussions in the store', () => { let batchComments = null;
expect(getters.discussions(state)).toEqual([individualNote]);
}); const getDiscussions = () => getters.discussions(state, {}, { batchComments });
describe('without batchComments module', () => {
it('should return all discussions in the store', () => {
expect(getDiscussions()).toEqual([individualNote]);
});
it('should transform discussion to individual notes in timeline view', () => {
state.discussions = [discussionMock];
state.isTimelineEnabled = true;
it('should transform discussion to individual notes in timeline view', () => { const discussions = getDiscussions();
state.discussions = [discussionMock];
state.isTimelineEnabled = true; expect(discussions.length).toEqual(discussionMock.notes.length);
discussions.forEach((discussion) => {
expect(discussion.individual_note).toBe(true);
expect(discussion.id).toBe(discussion.notes[0].id);
expect(discussion.created_at).toBe(discussion.notes[0].created_at);
});
});
});
expect(getters.discussions(state).length).toEqual(discussionMock.notes.length); describe('with batchComments', () => {
getters.discussions(state).forEach((discussion) => { beforeEach(() => {
expect(discussion.individual_note).toBe(true); batchComments = { drafts: [...draftComments, draftReply, draftDiffDiscussion] };
expect(discussion.id).toBe(discussion.notes[0].id);
expect(discussion.created_at).toBe(discussion.notes[0].created_at);
}); });
it.each`
discussionSortOrder | expectation
${ASC} | ${[individualNote, ...draftComments.map(asDraftDiscussion)]}
${DESC} | ${[...draftComments.reverse().map(asDraftDiscussion), individualNote]}
`(
'only appends draft comments (discussionSortOrder=$discussionSortOrder)',
({ discussionSortOrder, expectation }) => {
state.discussionSortOrder = discussionSortOrder;
expect(getDiscussions()).toEqual(expectation);
},
);
});
});
describe('hasDrafts', () => {
it.each`
rootGetters | expected
${{}} | ${false}
${{ 'batchComments/hasDrafts': true }} | ${true}
${{ 'batchComments/hasDrafts': false }} | ${false}
`('with rootGetters=$rootGetters, returns $expected', ({ rootGetters, expected }) => {
expect(getters.hasDrafts({}, {}, {}, rootGetters)).toBe(expected);
}); });
}); });
...@@ -103,7 +146,7 @@ describe('Getters Notes Store', () => { ...@@ -103,7 +146,7 @@ describe('Getters Notes Store', () => {
}; };
it('should return a single system note when a description was updated multiple times', () => { it('should return a single system note when a description was updated multiple times', () => {
expect(getters.discussions(stateCollapsedNotes).length).toEqual(1); expect(getters.discussions(stateCollapsedNotes, {}, {}).length).toEqual(1);
}); });
}); });
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment