kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add latest changes from gitlab-org/gitlab@master

Browse source
This commit is contained in:
GitLab Bot 2020-07-18 03:09:24 +00:00
parent ccefff8087
commit 95f5aad5aa
9 changed files with 169 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
app/assets/javascripts/batch_comments/components/draft_note.vue
View file

@ -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

18
app/assets/javascripts/notes/components/discussion_notes.vue
View file

@ -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

7
app/assets/javascripts/notes/components/noteable_note.vue
View file

@ -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"

19
app/models/merge_request.rb
View file

@ -251,17 +251,12 @@ class MergeRequest < ApplicationRecord
end
scope :join_project, -> { joins(:target_project) }
scope :references_project, -> { references(:target_project) }
PROJECT_ROUTE_AND_NAMESPACE_ROUTE = [
target_project: [:route, { namespace: :route }],
source_project: [:route, { namespace: :route }]
].freeze
scope :with_api_entity_associations, -> {
preload(:assignees, :author, :unresolved_notes, :labels, :milestone,
:timelogs, :latest_merge_request_diff,
*PROJECT_ROUTE_AND_NAMESPACE_ROUTE,
metrics: [:latest_closed_by, :merged_by])
preload_routables
.preload(:assignees, :author, :unresolved_notes, :labels, :milestone,
:timelogs, :latest_merge_request_diff,
target_project: :project_feature,
metrics: [:latest_closed_by, :merged_by])
}
scope :by_target_branch_wildcard, ->(wildcard_branch_name) do
@ -269,6 +264,10 @@ class MergeRequest < ApplicationRecord
end
scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) }
scope :preload_source_project, -> { preload(:source_project) }
scope :preload_routables, -> do
preload(target_project: [:route, { namespace: :route }],
source_project: [:route, { namespace: :route }])
end
scope :with_auto_merge_enabled, -> do
with_state(:opened).where(auto_merge_enabled: true)

5
changelogs/unreleased/jdb-fix-js-error-multilinecomment-overview.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Fix editing note throws js error
merge_request: 37216
author:
type: fixed

52
spec/frontend/batch_comments/components/draft_note_spec.js
View file

@ -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);
});
});
});
});

47
spec/frontend/notes/components/discussion_notes_spec.js
View file

@ -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();

4
spec/frontend/notes/components/noteable_note_spec.js
View file

@ -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(() => {

27
spec/services/search_service_spec.rb
View file

@ -374,6 +374,19 @@ RSpec.describe SearchService do
subject(:result) { search_service.search_objects }
shared_examples "redaction limits N+1 queries" do |limit:|
it 'does not exceed the query limit' do
# issuing the query to remove the data loading call
unredacted_results.to_a
# only the calls from the redaction are left
query = ActiveRecord::QueryRecorder.new { result }
# these are the project authorization calls, which are not preloaded
expect(query.count).to be <= limit
end
end
def found_blob(project)
Gitlab::Search::FoundBlob.new(project: project)
end
@ -427,6 +440,12 @@ RSpec.describe SearchService do
it 'redacts the inaccessible merge request' do
expect(result).to contain_exactly(readable)
end
context 'with :with_api_entity_associations' do
let(:unredacted_results) { ar_relation(MergeRequest.with_api_entity_associations, readable, unreadable) }
it_behaves_like "redaction limits N+1 queries", limit: 7
end
end
context 'project repository blobs' do
@ -460,6 +479,10 @@ RSpec.describe SearchService do
it 'redacts the inaccessible snippet' do
expect(result).to contain_exactly(readable)
end
context 'with :with_api_entity_associations' do
it_behaves_like "redaction limits N+1 queries", limit: 12
end
end
context 'personal snippets' do
@ -471,6 +494,10 @@ RSpec.describe SearchService do
it 'redacts the inaccessible snippet' do
expect(result).to contain_exactly(readable)
end
context 'with :with_api_entity_associations' do
it_behaves_like "redaction limits N+1 queries", limit: 3
end
end
context 'commits' do