root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Fixed test specs 

- added suggestions to mock data
- fixed props to be not required
This commit is contained in:
Oswaldo Ferreira 2019-04-04 13:08:34 +00:00 committed by Douwe Maan
parent 30988aecd9
commit e540c0d71e
36 changed files with 755 additions and 267 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/assets/javascripts/diffs/components/diff_line_note_form.vue
View File

@ -48,10 +48,13 @@ export default {
noteableType: this.noteableType,
noteTargetLine: this.noteTargetLine,
diffViewType: this.diffViewType,
diffFile: this.getDiffFileByHash(this.diffFileHash),
diffFile: this.diffFile,
linePosition: this.linePosition,
};
},
diffFile() {
return this.getDiffFileByHash(this.diffFileHash);
},
},
mounted() {
if (this.isLoggedIn) {
@ -102,6 +105,7 @@ export default {
:line-code="line.line_code"
:line="line"
:help-page-path="helpPagePath"
:diff-file="diffFile"
save-button-title="Comment"
class="diff-comment-form"
@handleFormUpdateAddToReview="addToReview"

44
app/assets/javascripts/notes/components/note_form.vue
View File

@ -61,6 +61,11 @@ export default {
required: false,
default: null,
},
diffFile: {
type: Object,
required: false,
default: null,
},
helpPagePath: {
type: String,
required: false,
@ -102,9 +107,42 @@ export default {
}
return '#';
},
diffParams() {
if (this.diffFile) {
return {
filePath: this.diffFile.file_path,
refs: this.diffFile.diff_refs,
};
} else if (this.note && this.note.position) {
return {
filePath: this.note.position.new_path,
refs: this.note.position,
};
} else if (this.discussion && this.discussion.diff_file) {
return {
filePath: this.discussion.diff_file.file_path,
refs: this.discussion.diff_file.diff_refs,
};
}
return null;
},
markdownPreviewPath() {
const notable = this.getNoteableDataByProp('preview_note_path');
return mergeUrlParams({ preview_suggestions: true }, notable);
const previewSuggestions = this.line && this.diffParams;
const params = previewSuggestions
? {
preview_suggestions: previewSuggestions,
line: this.line.new_line,
file_path: this.diffParams.filePath,
base_sha: this.diffParams.refs.base_sha,
start_sha: this.diffParams.refs.start_sha,
head_sha: this.diffParams.refs.head_sha,
}
: {};
return mergeUrlParams(params, notable);
},
markdownDocsPath() {
return this.getNotesDataByProp('markdownDocsPath');
@ -234,8 +272,8 @@ export default {
placeholder="Write a comment or drag your files here…"
@keydown.meta.enter="handleKeySubmit()"
@keydown.ctrl.enter="handleKeySubmit()"
@keydown.up="editMyLastNote()"
@keydown.esc="cancelHandler(true)"
@keydown.exact.up="editMyLastNote()"
@keydown.exact.esc="cancelHandler(true)"
@input="onInput"
></textarea>
</markdown-field>

20
app/assets/javascripts/vue_shared/components/lib/utils/diff_utils.js Normal file
View File

@ -0,0 +1,20 @@
/* eslint-disable import/prefer-default-export */
function trimFirstCharOfLineContent(text) {
if (!text) {
return text;
}
return text.replace(/^( |\+|-)/, '');
}
function cleanSuggestionLine(line = {}) {
return {
...line,
text: trimFirstCharOfLineContent(line.text),
};
}
export function selectDiffLines(lines) {
return lines.filter(line => line.type !== 'match').map(line => cleanSuggestionLine(line));
}

5
app/assets/javascripts/vue_shared/components/markdown/field.vue
View File

@ -76,6 +76,7 @@ export default {
hasSuggestion: false,
markdownPreviewLoading: false,
previewMarkdown: false,
suggestions: this.note.suggestions || [],
};
},
computed: {
@ -109,9 +110,6 @@ export default {
}
return lineNumber;
},
suggestions() {
return this.note.suggestions || [];
},
lineType() {
return this.line ? this.line.type : '';
},
@ -175,6 +173,7 @@ export default {
this.referencedCommands = data.references.commands;
this.referencedUsers = data.references.users;
this.hasSuggestion = data.references.suggestions && data.references.suggestions.length;
this.suggestions = data.references.suggestions;
}
this.$nextTick()

2
app/assets/javascripts/vue_shared/components/markdown/header.vue
View File

@ -38,7 +38,7 @@ export default {
].join('\n');
},
mdSuggestion() {
return ['```suggestion', `{text}`, '```'].join('\n');
return ['```suggestion:-0+0', `{text}`, '```'].join('\n');
},
},
mounted() {

42
app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue
View File

@ -1,24 +1,14 @@
<script>
import SuggestionDiffHeader from './suggestion_diff_header.vue';
import SuggestionDiffRow from './suggestion_diff_row.vue';
import { selectDiffLines } from '../lib/utils/diff_utils';
export default {
components: {
SuggestionDiffHeader,
SuggestionDiffRow,
},
props: {
newLines: {
type: Array,
required: true,
},
fromContent: {
type: String,
required: false,
default: '',
},
fromLine: {
type: Number,
required: true,
},
suggestion: {
type: Object,
required: true,
@ -33,6 +23,11 @@ export default {
required: true,
},
},
computed: {
lines() {
return selectDiffLines(this.suggestion.diff_lines);
},
},
methods: {
applySuggestion(callback) {
this.$emit('apply', { suggestionId: this.suggestion.id, callback });
@ -52,22 +47,11 @@ export default {
/>
<table class="mb-3 md-suggestion-diff js-syntax-highlight code">
<tbody>
<!-- Old Line -->
<tr class="line_holder old">
<td class="diff-line-num old_line qa-old-diff-line-number old">{{ fromLine }}</td>
<td class="diff-line-num new_line old"></td>
<td class="line_content old">
<span>{{ fromContent }}</span>
</td>
</tr>
<!-- New Line(s) -->
<tr v-for="(line, key) of newLines" :key="key" class="line_holder new">
<td class="diff-line-num old_line new"></td>
<td class="diff-line-num new_line qa-new-diff-line-number new">{{ line.lineNumber }}</td>
<td class="line_content new">
<span>{{ line.content }}</span>
</td>
</tr>
<suggestion-diff-row
v-for="(line, index) of lines"
:key="`${index}-${line.text}`"
:line="line"
/>
</tbody>
</table>
</div>

32
app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_row.vue Normal file
View File

@ -0,0 +1,32 @@
<script>
export default {
name: 'SuggestionDiffRow',
props: {
line: {
type: Object,
required: true,
},
},
computed: {
lineType() {
return this.line.type;
},
},
};
</script>
<template>
<tr class="line_holder" :class="lineType">
<td class="diff-line-num old_line" :class="lineType">
{{ line.old_line }}
</td>
<td class="diff-line-num new_line" :class="lineType">
{{ line.new_line }}
</td>
<td class="line_content" :class="lineType">
<span v-if="line.text">{{ line.text }}</span>
<!-- TODO: replace this hack with zero-width whitespace when we have rich_text from BE -->
<span v-else>&#8203;</span>
</td>
</tr>
</template>

38
app/assets/javascripts/vue_shared/components/markdown/suggestions.vue
View File

@ -6,16 +6,6 @@ import Flash from '~/flash';
export default {
components: { SuggestionDiff },
props: {
fromLine: {
type: Number,
required: false,
default: 0,
},
fromContent: {
type: String,
required: false,
default: '',
},
lineType: {
type: String,
required: false,
@ -71,41 +61,19 @@ export default {
suggestionElements.forEach((suggestionEl, i) => {
const suggestionParentEl = suggestionEl.parentElement;
const newLines = this.extractNewLines(suggestionParentEl);
const diffComponent = this.generateDiff(newLines, i);
const diffComponent = this.generateDiff(i);
diffComponent.$mount(suggestionParentEl);
});
this.isRendered = true;
},
extractNewLines(suggestionEl) {
// extracts the suggested lines from the markdown
// calculates a line number for each line
const newLines = suggestionEl.querySelectorAll('.line');
const fromLine = this.suggestions.length ? this.suggestions[0].from_line : this.fromLine;
const lines = [];
newLines.forEach((line, i) => {
const content = `${line.innerText}\n`;
const lineNumber = fromLine + i;
lines.push({ content, lineNumber });
});
return lines;
},
generateDiff(newLines, suggestionIndex) {
// generates the diff <suggestion-diff /> component
// all `suggestion` markdown will be swapped out by this component
generateDiff(suggestionIndex) {
const { suggestions, disabled, helpPagePath } = this;
const suggestion =
suggestions && suggestions[suggestionIndex] ? suggestions[suggestionIndex] : {};
const fromContent = suggestion.from_content || this.fromContent;
const fromLine = suggestion.from_line || this.fromLine;
const SuggestionDiffComponent = Vue.extend(SuggestionDiff);
const suggestionDiff = new SuggestionDiffComponent({
propsData: { newLines, fromLine, fromContent, disabled, suggestion, helpPagePath },
propsData: { disabled, suggestion, helpPagePath },
});
suggestionDiff.$on('apply', ({ suggestionId, callback }) => {

2
app/controllers/concerns/preview_markdown.rb
View File

@ -20,7 +20,7 @@ module PreviewMarkdown
body: view_context.markdown(result[:text], markdown_params),
references: {
users: result[:users],
suggestions: result[:suggestions],
suggestions: SuggestionSerializer.new.represent_diff(result[:suggestions]),
commands: view_context.markdown(result[:commands])
}
}

2
app/serializers/issue_entity.rb
View File

@ -42,6 +42,6 @@ class IssueEntity < IssuableEntity
end
expose :preview_note_path do |issue|
preview_markdown_path(issue.project, quick_actions_target_type: 'Issue', quick_actions_target_id: issue.iid)
preview_markdown_path(issue.project, target_type: 'Issue', target_id: issue.iid)
end
end

2
app/serializers/merge_request_widget_entity.rb
View File

@ -235,7 +235,7 @@ class MergeRequestWidgetEntity < IssuableEntity
end
expose :preview_note_path do |merge_request|
preview_markdown_path(merge_request.project, quick_actions_target_type: 'MergeRequest', quick_actions_target_id: merge_request.iid)
preview_markdown_path(merge_request.project, target_type: 'MergeRequest', target_id: merge_request.iid)
end
expose :merge_commit_path do |merge_request|

2
app/serializers/suggestion_entity.rb
View File

@ -3,6 +3,8 @@
class SuggestionEntity < API::Entities::Suggestion
include RequestAwareEntity
unexpose :from_line, :to_line, :from_content, :to_content
expose :diff_lines, using: DiffLineEntity
expose :current_user do
expose :can_apply do |suggestion|
Ability.allowed?(current_user, :apply_suggestion, suggestion)

9
app/serializers/suggestion_serializer.rb Normal file
View File

@ -0,0 +1,9 @@
# frozen_string_literal: true
class SuggestionSerializer < BaseSerializer
entity SuggestionEntity
def represent_diff(resource)
represent(resource, { only: [:diff_lines] })
end
end

7
app/services/concerns/suggestible.rb
View File

@ -2,10 +2,17 @@
module Suggestible
extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize
# This translates into limiting suggestion changes to `suggestion:-100+100`.
MAX_LINES_CONTEXT = 100.freeze
def diff_lines
strong_memoize(:diff_lines) do
Gitlab::Diff::SuggestionDiff.new(self).diff_lines
end
end
def fetch_from_content
diff_file.new_blob_lines_between(from_line, to_line).join
end

28
app/services/preview_markdown_service.rb
View File

@ -17,7 +17,7 @@ class PreviewMarkdownService < BaseService
private
def explain_quick_actions(text)
return text, [] unless %w(Issue MergeRequest Commit).include?(commands_target_type)
return text, [] unless %w(Issue MergeRequest Commit).include?(target_type)
quick_actions_service = QuickActions::InterpretService.new(project, current_user)
quick_actions_service.explain(text, find_commands_target)
@ -30,22 +30,34 @@ class PreviewMarkdownService < BaseService
end
def find_suggestions(text)
return [] unless params[:preview_suggestions]
return [] unless preview_sugestions?
Banzai::SuggestionsParser.parse(text)
position = Gitlab::Diff::Position.new(new_path: params[:file_path],
new_line: params[:line].to_i,
base_sha: params[:base_sha],
head_sha: params[:head_sha],
start_sha: params[:start_sha])
Gitlab::Diff::SuggestionsParser.parse(text, position: position, project: project)
end
def preview_sugestions?
params[:preview_suggestions] &&
target_type == 'MergeRequest' &&
Ability.allowed?(current_user, :download_code, project)
end
def find_commands_target
QuickActions::TargetService
.new(project, current_user)
.execute(commands_target_type, commands_target_id)
.execute(target_type, target_id)
end
def commands_target_type
params[:quick_actions_target_type]
def target_type
params[:target_type]
end
def commands_target_id
params[:quick_actions_target_id]
def target_id
params[:target_id]
end
end

2
app/views/shared/form_elements/_description.html.haml
View File

@ -5,7 +5,7 @@
- supports_quick_actions = model.new_record?
- if supports_quick_actions
- preview_url = preview_markdown_path(project, quick_actions_target_type: model.class.name)
- preview_url = preview_markdown_path(project, target_type: model.class.name)
- else
- preview_url = preview_markdown_path(project)

2
app/views/shared/notes/_form.html.haml
View File

@ -1,7 +1,7 @@
- supports_autocomplete = local_assigns.fetch(:supports_autocomplete, true)
- supports_quick_actions = note_supports_quick_actions?(@note)
- if supports_quick_actions
- preview_url = preview_markdown_path(@project, quick_actions_target_type: @note.noteable_type, quick_actions_target_id: @note.noteable_id)
- preview_url = preview_markdown_path(@project, target_type: @note.noteable_type, target_id: @note.noteable_id)
- else
- preview_url = preview_markdown_path(@project)

5
changelogs/unreleased/osw-support-multi-line-suggestions.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Support multi-line suggestions
merge_request: 25211
author:
type: added

BIN
doc/user/discussions/img/multi-line-suggestion-preview.png Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 60 KiB

BIN
doc/user/discussions/img/multi-line-suggestion-syntax.png Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 29 KiB

18
doc/user/discussions/index.md
View File

@ -344,6 +344,24 @@ and push the suggested change directly into the codebase in the merge request's
Custom commit messages will be introduced by
[#54404](https://gitlab.com/gitlab-org/gitlab-ce/issues/54404).
### Multi-line suggestions
> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/53310) in GitLab 11.10.
Reviewers can also suggest changes to
multiple lines with a single suggestion within Merge Request diff discussions.
![Multi-line suggestion syntax](img/multi-line-suggestion-syntax.png)
In the example above, the suggestion covers three lines above and four lines below the commented diff line.
It'd change from 3 lines _above_ to 4 lines _below_ the commented Diff line.
![Multi-line suggestion preview](img/multi-line-suggestion-preview.png)
NOTE: **Note:**
Suggestions covering multiple lines are limited to 100 lines _above_ and 100 lines _below_
the commented diff line, allowing up to 200 changed lines per suggestion.
## Start a discussion by replying to a standard comment
> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/30299) in GitLab 11.9

16
lib/banzai/suggestions_parser.rb
View File

@ -1,16 +0,0 @@
# frozen_string_literal: true
# TODO: Delete when https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/26107
# exchange this parser by `Gitlab::Diff::SuggestionsParser`.
module Banzai
module SuggestionsParser
# Returns the content of each suggestion code block.
#
def self.parse(text)
html = Banzai.render(text, project: nil, no_original_data: true)
doc = Nokogiri::HTML(html)
doc.search('pre.suggestion').map { |node| node.text }
end
end
end

10
spec/controllers/projects_controller_spec.rb
View File

@ -703,6 +703,16 @@ describe ProjectsController do
expect(JSON.parse(response.body).keys).to match_array(%w(body references))
end
context 'when not authorized' do
let(:private_project) { create(:project, :private) }
it 'returns 404' do
post :preview_markdown, params: { namespace_id: private_project.namespace, id: private_project, text: '*Markdown* text' }
expect(response).to have_gitlab_http_status(404)
end
end
context 'state filter on references' do
let(:issue) { create(:issue, :closed, project: public_project) }
let(:merge_request) { create(:merge_request, :closed, target_project: public_project) }

111
spec/features/merge_request/user_suggests_changes_on_diff_spec.rb
View File

@ -6,6 +6,14 @@ describe 'User comments on a diff', :js do
include MergeRequestDiffHelpers
include RepoHelpers
def expect_suggestion_has_content(element, expected_changing_content, expected_suggested_content)
changing_content = element.all(:css, '.line_holder.old').map(&:text)
suggested_content = element.all(:css, '.line_holder.new').map(&:text)
expect(changing_content).to eq(expected_changing_content)
expect(suggested_content).to eq(expected_suggested_content)
end
let(:project) { create(:project, :repository) }
let(:merge_request) do
create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
@ -33,8 +41,18 @@ describe 'User comments on a diff', :js do
page.within('.diff-discussions') do
expect(page).to have_button('Apply suggestion')
expect(page).to have_content('Suggested change')
expect(page).to have_content(' url = https://github.com/gitlabhq/gitlab-shell.git')
expect(page).to have_content('# change to a comment')
end
page.within('.md-suggestion-diff') do
expected_changing_content = [
"6 url = https://github.com/gitlabhq/gitlab-shell.git"
]
expected_suggested_content = [
"6 # change to a comment"
]
expect_suggestion_has_content(page, expected_changing_content, expected_suggested_content)
end
end
@ -64,7 +82,7 @@ describe 'User comments on a diff', :js do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion\n# or that\n```")
fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion:-2\n# or that\n# heh\n```")
click_button('Comment')
end
@ -74,11 +92,90 @@ describe 'User comments on a diff', :js do
suggestion_1 = page.all(:css, '.md-suggestion-diff')[0]
suggestion_2 = page.all(:css, '.md-suggestion-diff')[1]
expect(suggestion_1).to have_content(' url = https://github.com/gitlabhq/gitlab-shell.git')
expect(suggestion_1).to have_content('# change to a comment')
suggestion_1_expected_changing_content = [
"6 url = https://github.com/gitlabhq/gitlab-shell.git"
]
suggestion_1_expected_suggested_content = [
"6 # change to a comment"
]
expect(suggestion_2).to have_content(' url = https://github.com/gitlabhq/gitlab-shell.git')
expect(suggestion_2).to have_content('# or that')
suggestion_2_expected_changing_content = [
"4 [submodule \"gitlab-shell\"]",
"5 path = gitlab-shell",
"6 url = https://github.com/gitlabhq/gitlab-shell.git"
]
suggestion_2_expected_suggested_content = [
"4 # or that",
"5 # heh"
]
expect_suggestion_has_content(suggestion_1,
suggestion_1_expected_changing_content,
suggestion_1_expected_suggested_content)
expect_suggestion_has_content(suggestion_2,
suggestion_2_expected_changing_content,
suggestion_2_expected_suggested_content)
end
end
end
context 'multi-line suggestions' do
it 'suggestion is presented' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```")
click_button('Comment')
end
wait_for_requests
page.within('.diff-discussions') do
expect(page).to have_button('Apply suggestion')
expect(page).to have_content('Suggested change')
end
page.within('.md-suggestion-diff') do
expected_changing_content = [
"3 url = git://github.com/randx/six.git",
"4 [submodule \"gitlab-shell\"]",
"5 path = gitlab-shell",
"6 url = https://github.com/gitlabhq/gitlab-shell.git",
"7 [submodule \"gitlab-grack\"]",
"8 path = gitlab-grack",
"9 url = https://gitlab.com/gitlab-org/gitlab-grack.git"
]
expected_suggested_content = [
"3 # change to a",
"4 # comment",
"5 # with",
"6 # broken",
"7 # lines"
]
expect_suggestion_has_content(page, expected_changing_content, expected_suggested_content)
end
end
it 'suggestion is appliable' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```")
click_button('Comment')
end
wait_for_requests
page.within('.diff-discussions') do
expect(page).not_to have_content('Applied')
click_button('Apply suggestion')
wait_for_requests
expect(page).to have_content('Applied')
end
end
end

98
spec/frontend/vue_shared/components/markdown/suggestion_diff_row_spec.js Normal file
View File

@ -0,0 +1,98 @@
import { shallowMount, createLocalVue } from '@vue/test-utils';
import SuggestionDiffRow from '~/vue_shared/components/markdown/suggestion_diff_row.vue';
const oldLine = {
can_receive_suggestion: false,
line_code: null,
meta_data: null,
new_line: null,
old_line: 5,
rich_text: '-oldtext',
text: '-oldtext',
type: 'old',
};
const newLine = {
can_receive_suggestion: false,
line_code: null,
meta_data: null,
new_line: 6,
old_line: null,
rich_text: '-newtext',
text: '-newtext',
type: 'new',
};
describe(SuggestionDiffRow.name, () => {
let wrapper;
const factory = (options = {}) => {
const localVue = createLocalVue();
wrapper = shallowMount(SuggestionDiffRow, {
localVue,
...options,
});
};
const findOldLineWrapper = () => wrapper.find('.old_line');
const findNewLineWrapper = () => wrapper.find('.new_line');
afterEach(() => {
wrapper.destroy();
});
it('renders correctly', () => {
factory({
propsData: {
line: oldLine,
},
});
expect(wrapper.is('.line_holder')).toBe(true);
});
describe('when passed line has type old', () => {
beforeEach(() => {
factory({
propsData: {
line: oldLine,
},
});
});
it('has old class when line has type old', () => {
expect(wrapper.find('td').classes()).toContain('old');
});
it('has old line number rendered', () => {
expect(findOldLineWrapper().text()).toBe('5');
});
it('has no new line number rendered', () => {
expect(findNewLineWrapper().text()).toBe('');
});
});
describe('when passed line has type new', () => {
beforeEach(() => {
factory({
propsData: {
line: newLine,
},
});
});
it('has new class when line has type new', () => {
expect(wrapper.find('td').classes()).toContain('new');
});
it('has no old line number rendered', () => {
expect(findOldLineWrapper().text()).toBe('');
});
it('has no new line number rendered', () => {
expect(findNewLineWrapper().text()).toBe('6');
});
});
});

6
spec/javascripts/notes/mock_data.js
View File

@ -44,8 +44,7 @@ export const noteableDataMock = {
milestone: null,
milestone_id: null,
moved_to_id: null,
preview_note_path:
'/gitlab-org/gitlab-ce/preview_markdown?quick_actions_target_id=98&quick_actions_target_type=Issue',
preview_note_path: '/gitlab-org/gitlab-ce/preview_markdown?target_id=98&target_type=Issue',
project_id: 2,
state: 'opened',
time_estimate: 0,
@ -347,8 +346,7 @@ export const loggedOutnoteableData = {
},
noteable_note_url: '/group/project/merge_requests/1#note_1',
create_note_path: '/gitlab-org/gitlab-ce/notes?target_id=98&target_type=issue',
preview_note_path:
'/gitlab-org/gitlab-ce/preview_markdown?quick_actions_target_id=98&quick_actions_target_type=Issue',
preview_note_path: '/gitlab-org/gitlab-ce/preview_markdown?target_id=98&target_type=Issue',
};
export const collapseNotesMock = [

2
spec/javascripts/vue_shared/components/markdown/header_spec.js
View File

@ -98,7 +98,7 @@ describe('Markdown field header component', () => {
it('renders suggestion template', () => {
vm.lineContent = 'Some content';
expect(vm.mdSuggestion).toEqual('```suggestion\n{text}\n```');
expect(vm.mdSuggestion).toEqual('```suggestion:-0+0\n{text}\n```');
});
it('does not render suggestion button if `canSuggest` is set to false', () => {

66
spec/javascripts/vue_shared/components/markdown/suggestion_diff_spec.js
View File

@ -1,21 +1,50 @@
import Vue from 'vue';
import SuggestionDiffComponent from '~/vue_shared/components/markdown/suggestion_diff.vue';
import { selectDiffLines } from '~/vue_shared/components/lib/utils/diff_utils';
const MOCK_DATA = {
canApply: true,
newLines: [
{ content: 'Line 1\n', lineNumber: 1 },
{ content: 'Line 2\n', lineNumber: 2 },
{ content: 'Line 3\n', lineNumber: 3 },
],
fromLine: 1,
fromContent: 'Old content',
suggestion: {
id: 1,
diff_lines: [
{
can_receive_suggestion: false,
line_code: null,
meta_data: null,
new_line: null,
old_line: 5,
rich_text: '-test',
text: '-test',
type: 'old',
},
{
can_receive_suggestion: true,
line_code: null,
meta_data: null,
new_line: 5,
old_line: null,
rich_text: '+new test',
text: '+new test',
type: 'new',
},
{
can_receive_suggestion: true,
line_code: null,
meta_data: null,
new_line: 5,
old_line: null,
rich_text: '+new test2',
text: '+new test2',
type: 'new',
},
],
},
helpPagePath: 'path_to_docs',
};
const lines = selectDiffLines(MOCK_DATA.suggestion.diff_lines);
const newLines = lines.filter(line => line.type === 'new');
describe('Suggestion Diff component', () => {
let vm;
@ -39,30 +68,23 @@ describe('Suggestion Diff component', () => {
});
it('renders the oldLineNumber', () => {
const fromLine = vm.$el.querySelector('.qa-old-diff-line-number').innerHTML;
const fromLine = vm.$el.querySelector('.old_line').innerHTML;
expect(parseInt(fromLine, 10)).toBe(vm.fromLine);
expect(parseInt(fromLine, 10)).toBe(lines[0].old_line);
});
it('renders the oldLineContent', () => {
const fromContent = vm.$el.querySelector('.line_content.old').innerHTML;
expect(fromContent.includes(vm.fromContent)).toBe(true);
expect(fromContent.includes(lines[0].text)).toBe(true);
});
it('renders the contents of newLines', () => {
const newLines = vm.$el.querySelectorAll('.line_holder.new');
it('renders new lines', () => {
const newLinesElements = vm.$el.querySelectorAll('.line_holder.new');
newLines.forEach((line, i) => {
expect(newLines[i].innerHTML.includes(vm.newLines[i].content)).toBe(true);
});
});
it('renders a line number for each line', () => {
const newLineNumbers = vm.$el.querySelectorAll('.qa-new-diff-line-number');
newLineNumbers.forEach((line, i) => {
expect(newLineNumbers[i].innerHTML.includes(vm.newLines[i].lineNumber)).toBe(true);
newLinesElements.forEach((line, i) => {
expect(newLinesElements[i].innerHTML.includes(newLines[i].new_line)).toBe(true);
expect(newLinesElements[i].innerHTML.includes(newLines[i].text)).toBe(true);
});
});
});

109
spec/javascripts/vue_shared/components/markdown/suggestions_spec.js
View File

@ -2,46 +2,52 @@ import Vue from 'vue';
import SuggestionsComponent from '~/vue_shared/components/markdown/suggestions.vue';
const MOCK_DATA = {
fromLine: 1,
fromContent: 'Old content',
suggestions: [],
suggestions: [
{
id: 1,
appliable: true,
applied: false,
current_user: {
can_apply: true,
},
diff_lines: [
{
can_receive_suggestion: false,
line_code: null,
meta_data: null,
new_line: null,
old_line: 5,
rich_text: '-test',
text: '-test',
type: 'old',
},
{
can_receive_suggestion: true,
line_code: null,
meta_data: null,
new_line: 5,
old_line: null,
rich_text: '+new test',
text: '+new test',
type: 'new',
},
],
},
],
noteHtml: `
<div class="suggestion">
<div class="line">-oldtest</div>
</div>
<div class="suggestion">
<div class="line">Suggestion 1</div>
<div class="line">+newtest</div>
</div>
<div class="suggestion">
<div class="line">Suggestion 2</div>
</div>
`,
isApplied: false,
helpPagePath: 'path_to_docs',
};
const generateLine = content => {
const line = document.createElement('div');
line.className = 'line';
line.innerHTML = content;
return line;
};
const generateMockLines = () => {
const line1 = generateLine('Line 1');
const line2 = generateLine('Line 2');
const line3 = generateLine('- Line 3');
const container = document.createElement('div');
container.appendChild(line1);
container.appendChild(line2);
container.appendChild(line3);
return container;
};
describe('Suggestion component', () => {
let vm;
let extractedLines;
let diffTable;
beforeEach(done => {
@ -51,8 +57,7 @@ describe('Suggestion component', () => {
propsData: MOCK_DATA,
}).$mount();
extractedLines = vm.extractNewLines(generateMockLines());
diffTable = vm.generateDiff(extractedLines).$mount().$el;
diffTable = vm.generateDiff(0).$mount().$el;
spyOn(vm, 'renderSuggestions');
vm.renderSuggestions();
@ -70,32 +75,8 @@ describe('Suggestion component', () => {
it('renders suggestions', () => {
expect(vm.renderSuggestions).toHaveBeenCalled();
expect(vm.$el.innerHTML.includes('Suggestion 1')).toBe(true);
expect(vm.$el.innerHTML.includes('Suggestion 2')).toBe(true);
});
});
describe('extractNewLines', () => {
it('extracts suggested lines', () => {
const expectedReturn = [
{ content: 'Line 1\n', lineNumber: 1 },
{ content: 'Line 2\n', lineNumber: 2 },
{ content: '- Line 3\n', lineNumber: 3 },
];
expect(vm.extractNewLines(generateMockLines())).toEqual(expectedReturn);
});
it('increments line number for each extracted line', () => {
expect(extractedLines[0].lineNumber).toEqual(1);
expect(extractedLines[1].lineNumber).toEqual(2);
expect(extractedLines[2].lineNumber).toEqual(3);
});
it('returns empty array if no lines are found', () => {
const el = document.createElement('div');
expect(vm.extractNewLines(el)).toEqual([]);
expect(vm.$el.innerHTML.includes('oldtest')).toBe(true);
expect(vm.$el.innerHTML.includes('newtest')).toBe(true);
});
});
@ -109,17 +90,17 @@ describe('Suggestion component', () => {
});
it('generates a diff table that contains contents the suggested lines', () => {
extractedLines.forEach((line, i) => {
expect(diffTable.innerHTML.includes(extractedLines[i].content)).toBe(true);
MOCK_DATA.suggestions[0].diff_lines.forEach(line => {
const text = line.text.substring(1);
expect(diffTable.innerHTML.includes(text)).toBe(true);
});
});
it('generates a diff table with the correct line number for each suggested line', () => {
const lines = diffTable.getElementsByClassName('qa-new-diff-line-number');
const lines = diffTable.querySelectorAll('.old_line');
expect([...lines][0].innerHTML).toBe('1');
expect([...lines][1].innerHTML).toBe('2');
expect([...lines][2].innerHTML).toBe('3');
expect(parseInt([...lines][0].innerHTML, 10)).toBe(5);
});
});
});

32
spec/lib/banzai/suggestions_parser_spec.rb
View File

@ -1,32 +0,0 @@
# frozen_string_literal: true
require 'spec_helper'
describe Banzai::SuggestionsParser do
describe '.parse' do
it 'returns a list of suggestion contents' do
markdown = <<-MARKDOWN.strip_heredoc
```suggestion
foo
bar
```
```
nothing
```
```suggestion
xpto
baz
```
```thing
this is not a suggestion, it's a thing
```
MARKDOWN
expect(described_class.parse(markdown)).to eq([" foo\n bar",
" xpto\n baz"])
end
end
end

87
spec/lib/gitlab/diff/suggestion_spec.rb
View File

@ -10,6 +10,16 @@ describe Gitlab::Diff::Suggestion do
lines_above: above,
lines_below: below)
end
it 'returns diff lines with correct line numbers' do
diff_lines = suggestion.diff_lines
expect(diff_lines).to all(be_a(Gitlab::Diff::Line))
expected_diff_lines.each_with_index do |expected_line, index|
expect(diff_lines[index].to_hash).to include(expected_line)
end
end
end
let(:merge_request) { create(:merge_request) }
@ -48,6 +58,18 @@ describe Gitlab::Diff::Suggestion do
let(:expected_above) { line - 1 }
let(:expected_below) { below }
let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
let(:expected_diff_lines) do
[
{ old_pos: 1, new_pos: 1, type: 'old', text: "-require 'fileutils'" },
{ old_pos: 2, new_pos: 1, type: 'old', text: "-require 'open3'" },
{ old_pos: 3, new_pos: 1, type: 'old', text: "-" },
{ old_pos: 4, new_pos: 1, type: 'old', text: "-module Popen" },
{ old_pos: 5, new_pos: 1, type: 'old', text: "- extend self" },
{ old_pos: 6, new_pos: 1, type: 'old', text: "-" },
{ old_pos: 7, new_pos: 1, type: 'new', text: "+# parsed suggestion content" },
{ old_pos: 7, new_pos: 2, type: 'new', text: "+# with comments" }
]
end
it_behaves_like 'correct suggestion raw content'
end
@ -59,6 +81,47 @@ describe Gitlab::Diff::Suggestion do
let(:expected_below) { below }
let(:expected_above) { above }
let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
let(:expected_diff_lines) do
[
{ old_pos: 4, new_pos: 4, type: "match", text: "@@ -4 +4" },
{ old_pos: 4, new_pos: 4, type: "old", text: "-module Popen" },
{ old_pos: 5, new_pos: 4, type: "old", text: "- extend self" },
{ old_pos: 6, new_pos: 4, type: "old", text: "-" },
{ old_pos: 7, new_pos: 4, type: "old", text: "- def popen(cmd, path=nil)" },
{ old_pos: 8, new_pos: 4, type: "old", text: "- unless cmd.is_a?(Array)" },
{ old_pos: 9, new_pos: 4, type: "old", text: "- raise RuntimeError, \"System commands must be given as an array of strings\"" },
{ old_pos: 10, new_pos: 4, type: "old", text: "- end" },
{ old_pos: 11, new_pos: 4, type: "old", text: "-" },
{ old_pos: 12, new_pos: 4, type: "old", text: "- path ||= Dir.pwd" },
{ old_pos: 13, new_pos: 4, type: "old", text: "-" },
{ old_pos: 14, new_pos: 4, type: "old", text: "- vars = {" },
{ old_pos: 15, new_pos: 4, type: "old", text: "- \"PWD\" => path" },
{ old_pos: 16, new_pos: 4, type: "old", text: "- }" },
{ old_pos: 17, new_pos: 4, type: "old", text: "-" },
{ old_pos: 18, new_pos: 4, type: "old", text: "- options = {" },
{ old_pos: 19, new_pos: 4, type: "old", text: "- chdir: path" },
{ old_pos: 20, new_pos: 4, type: "old", text: "- }" },
{ old_pos: 21, new_pos: 4, type: "old", text: "-" },
{ old_pos: 22, new_pos: 4, type: "old", text: "- unless File.directory?(path)" },
{ old_pos: 23, new_pos: 4, type: "old", text: "- FileUtils.mkdir_p(path)" },
{ old_pos: 24, new_pos: 4, type: "old", text: "- end" },
{ old_pos: 25, new_pos: 4, type: "old", text: "-" },
{ old_pos: 26, new_pos: 4, type: "old", text: "- @cmd_output = \"\"" },
{ old_pos: 27, new_pos: 4, type: "old", text: "- @cmd_status = 0" },
{ old_pos: 28, new_pos: 4, type: "old", text: "-" },
{ old_pos: 29, new_pos: 4, type: "old", text: "- Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|" },
{ old_pos: 30, new_pos: 4, type: "old", text: "- @cmd_output << stdout.read" },
{ old_pos: 31, new_pos: 4, type: "old", text: "- @cmd_output << stderr.read" },
{ old_pos: 32, new_pos: 4, type: "old", text: "- @cmd_status = wait_thr.value.exitstatus" },
{ old_pos: 33, new_pos: 4, type: "old", text: "- end" },
{ old_pos: 34, new_pos: 4, type: "old", text: "-" },
{ old_pos: 35, new_pos: 4, type: "old", text: "- return @cmd_output, @cmd_status" },
{ old_pos: 36, new_pos: 4, type: "old", text: "- end" },
{ old_pos: 37, new_pos: 4, type: "old", text: "-end" },
{ old_pos: 38, new_pos: 4, type: "new", text: "+# parsed suggestion content" },
{ old_pos: 38, new_pos: 5, type: "new", text: "+# with comments" }
]
end
it_behaves_like 'correct suggestion raw content'
end
@ -70,17 +133,19 @@ describe Gitlab::Diff::Suggestion do
let(:expected_below) { below }
let(:expected_above) { above }
let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
it_behaves_like 'correct suggestion raw content'
end
context 'when no extra lines (single-line suggestion)' do
let(:line) { 5 }
let(:above) { 0 }
let(:below) { 0 }
let(:expected_below) { below }
let(:expected_above) { above }
let(:expected_lines) { blob_lines_data(line - expected_above, line + expected_below) }
let(:expected_diff_lines) do
[
{ old_pos: 3, new_pos: 3, type: "match", text: "@@ -3 +3" },
{ old_pos: 3, new_pos: 3, type: "old", text: "-" },
{ old_pos: 4, new_pos: 3, type: "old", text: "-module Popen" },
{ old_pos: 5, new_pos: 3, type: "old", text: "- extend self" },
{ old_pos: 6, new_pos: 3, type: "old", text: "-" },
{ old_pos: 7, new_pos: 3, type: "old", text: "- def popen(cmd, path=nil)" },
{ old_pos: 8, new_pos: 3, type: "old", text: "- unless cmd.is_a?(Array)" },
{ old_pos: 9, new_pos: 3, type: "new", text: "+# parsed suggestion content" },
{ old_pos: 9, new_pos: 4, type: "new", text: "+# with comments" }
]
end
it_behaves_like 'correct suggestion raw content'
end

61
spec/lib/gitlab/diff/suggestions_parser_spec.rb
View File

@ -69,5 +69,66 @@ describe Gitlab::Diff::SuggestionsParser do
lines_below: 0)
end
end
context 'multi-line suggestions' do
let(:markdown) do
<<-MARKDOWN.strip_heredoc
```suggestion:-2+1
# above and below
```
```
nothing
```
```suggestion:-3
# only above
```
```suggestion:+3
# only below
```
```thing
this is not a suggestion, it's a thing
```
MARKDOWN
end
it 'returns a list of Gitlab::Diff::Suggestion' do
expect(subject).to all(be_a(Gitlab::Diff::Suggestion))
expect(subject.size).to eq(3)
end
it 'suggestion with above and below param has correct data' do
from_line = position.new_line - 2
to_line = position.new_line + 1
expect(subject.first.to_hash).to include(from_content: blob_lines_data(from_line, to_line),
to_content: " # above and below\n",
lines_above: 2,
lines_below: 1)
end
it 'suggestion with above param has correct data' do
from_line = position.new_line - 3
to_line = position.new_line
expect(subject.second.to_hash).to eq(from_content: blob_lines_data(from_line, to_line),
to_content: " # only above\n",
lines_above: 3,
lines_below: 0)
end
it 'suggestion with below param has correct data' do
from_line = position.new_line
to_line = position.new_line + 3
expect(subject.third.to_hash).to eq(from_content: blob_lines_data(from_line, to_line),
to_content: " # only below\n",
lines_above: 0,
lines_below: 3)
end
end
end
end

16
spec/models/suggestion_spec.rb
View File

@ -21,6 +21,22 @@ describe Suggestion do
end
end
describe '#diff_lines' do
let(:suggestion) { create(:suggestion, :content_from_repo) }
it 'returns parsed diff lines' do
expected_diff_lines = Gitlab::Diff::SuggestionDiff.new(suggestion).diff_lines
diff_lines = suggestion.diff_lines
expect(diff_lines.size).to eq(expected_diff_lines.size)
expect(diff_lines).to all(be_a(Gitlab::Diff::Line))
expected_diff_lines.each_with_index do |expected_line, index|
expect(diff_lines[index].to_hash).to eq(expected_line.to_hash)
end
end
end
describe '#appliable?' do
context 'when note does not support suggestions' do
it 'returns false' do

3
spec/serializers/suggestion_entity_spec.rb
View File

@ -13,8 +13,7 @@ describe SuggestionEntity do
subject { entity.as_json }
it 'exposes correct attributes' do
expect(subject).to include(:id, :from_line, :to_line, :appliable,
:applied, :from_content, :to_content)
expect(subject.keys).to match_array([:id, :appliable, :applied, :diff_lines, :current_user])
end
it 'exposes current user abilities' do

73
spec/services/preview_markdown_service_spec.rb
View File

@ -2,7 +2,7 @@ require 'spec_helper'
describe PreviewMarkdownService do
let(:user) { create(:user) }
let(:project) { create(:project) }
let(:project) { create(:project, :repository) }
before do
project.add_developer(user)
@ -20,23 +20,72 @@ describe PreviewMarkdownService do
end
describe 'suggestions' do
let(:params) { { text: "```suggestion\nfoo\n```", preview_suggestions: preview_suggestions } }
let(:merge_request) do
create(:merge_request, target_project: project, source_project: project)
end
let(:text) { "```suggestion\nfoo\n```" }
let(:params) do
suggestion_params.merge(text: text,
target_type: 'MergeRequest',
target_id: merge_request.iid)
end
let(:service) { described_class.new(project, user, params) }
context 'when preview markdown param is present' do
let(:preview_suggestions) { true }
let(:path) { "files/ruby/popen.rb" }
let(:line) { 10 }
let(:diff_refs) { merge_request.diff_refs }
let(:suggestion_params) do
{
preview_suggestions: true,
file_path: path,
line: line,
base_sha: diff_refs.base_sha,
start_sha: diff_refs.start_sha,
head_sha: diff_refs.head_sha
}
end
it 'returns suggestions referenced in text' do
position = Gitlab::Diff::Position.new(new_path: path,
new_line: line,
diff_refs: diff_refs)
expect(Gitlab::Diff::SuggestionsParser)
.to receive(:parse)
.with(text, position: position, project: merge_request.project)
.and_call_original
it 'returns users referenced in text' do
result = service.execute
expect(result[:suggestions]).to eq(['foo'])
expect(result[:suggestions]).to all(be_a(Gitlab::Diff::Suggestion))
end
context 'when user is not authorized' do
let(:another_user) { create(:user) }
let(:service) { described_class.new(project, another_user, params) }
before do
project.add_guest(another_user)
end
it 'returns no suggestions' do
result = service.execute
expect(result[:suggestions]).to be_empty
end
end
end
context 'when preview markdown param is not present' do
let(:preview_suggestions) { false }
let(:suggestion_params) do
{
preview_suggestions: false
}
end
it 'returns users referenced in text' do
it 'returns suggestions referenced in text' do
result = service.execute
expect(result[:suggestions]).to eq([])
@ -49,8 +98,8 @@ describe PreviewMarkdownService do
let(:params) do
{
text: "Please do it\n/assign #{user.to_reference}",
quick_actions_target_type: 'Issue',
quick_actions_target_id: issue.id
target_type: 'Issue',
target_id: issue.id
}
end
let(:service) { described_class.new(project, user, params) }
@ -72,7 +121,7 @@ describe PreviewMarkdownService do
let(:params) do
{
text: "My work\n/estimate 2y",
quick_actions_target_type: 'MergeRequest'
target_type: 'MergeRequest'
}
end
let(:service) { described_class.new(project, user, params) }
@ -96,8 +145,8 @@ describe PreviewMarkdownService do
let(:params) do
{
text: "My work\n/tag v1.2.3 Stable release",
quick_actions_target_type: 'Commit',
quick_actions_target_id: commit.id
target_type: 'Commit',
target_id: commit.id
}
end
let(:service) { described_class.new(project, user, params) }

64
spec/services/suggestions/apply_service_spec.rb
View File

@ -51,6 +51,10 @@ describe Suggestions::ApplyService do
diff_refs: merge_request.diff_refs)
end
let(:diff_note) do
create(:diff_note_on_merge_request, noteable: merge_request, position: position, project: project)
end
let(:suggestion) do
create(:suggestion, :content_from_repo, note: diff_note,
to_content: " raise RuntimeError, 'Explosion'\n # explosion?\n")
@ -108,12 +112,6 @@ describe Suggestions::ApplyService do
target_project: project)
end
let!(:diff_note) do
create(:diff_note_on_merge_request, noteable: merge_request,
position: position,
project: project)
end
before do
project.add_maintainer(user)
end
@ -192,11 +190,6 @@ describe Suggestions::ApplyService do
CONTENT
end
let(:merge_request) do
create(:merge_request, source_project: project,
target_project: project)
end
def create_suggestion(diff, old_line: nil, new_line: nil, from_content:, to_content:, path:)
position = Gitlab::Diff::Position.new(old_path: path,
new_path: path,
@ -291,6 +284,55 @@ describe Suggestions::ApplyService do
expect(suggestion_2_diff.strip).to eq(expected_suggestion_2_diff.strip)
end
end
context 'multi-line suggestion' do
let(:expected_content) do
<<~CONTENT
require 'fileutils'
require 'open3'
module Popen
extend self
# multi
# line
vars = {
"PWD" => path
}
options = {
chdir: path
}
unless File.directory?(path)
FileUtils.mkdir_p(path)
end
@cmd_output = ""
@cmd_status = 0
Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
@cmd_output << stdout.read
@cmd_output << stderr.read
@cmd_status = wait_thr.value.exitstatus
end
return @cmd_output, @cmd_status
end
end
CONTENT
end
let(:suggestion) do
create(:suggestion, :content_from_repo, note: diff_note,
lines_above: 2,
lines_below: 3,
to_content: "# multi\n# line\n")
end
it_behaves_like 'successfully creates commit and updates suggestion'
end
end
context 'fork-project' do