Skip to content

G
gitlab-ce
Commit ed3034bb authored by Oswaldo Ferreira's avatar Oswaldo Ferreira Committed by Phil Hughes
Browse files
Options

Allow suggesting single line changes in diffs

parent eb81c123
Hide whitespace changes
Inline Side-by-side
Showing with 2150 additions and 25 deletions
app/assets/javascripts/api.js
View file @ ed3034bb
......@@ -25,6 +25,7 @@ const Api = {
userStatusPath: '/api/:version/users/:id/status',
userPostStatusPath: '/api/:version/user/status',
commitPath: '/api/:version/projects/:id/repository/commits',
applySuggestionPath: '/api/:version/suggestions/:id/apply',
commitPipelinesPath: '/:project_id/commit/:sha/pipelines',
branchSinglePath: '/api/:version/projects/:id/repository/branches/:branch',
createBranchPath: '/api/:version/projects/:id/repository/branches',
......@@ -185,6 +186,12 @@ const Api = {
});
},
applySuggestion(id) {
const url = Api.buildUrl(Api.applySuggestionPath).replace(':id', encodeURIComponent(id));
return axios.put(url);
},
commitPipelines(projectId, sha) {
const encodedProjectId = projectId
.split('/')
......
app/assets/javascripts/diffs/components/app.vue
View file @ ed3034bb
......@@ -42,6 +42,11 @@ export default {
type: Object,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
changesEmptyStateIllustration: {
type: String,
required: false,
......@@ -208,6 +213,7 @@ export default {
v-for="file in diffFiles"
:key="file.newPath"
:file="file"
:help-page-path="helpPagePath"
:can-current-user-fork="canCurrentUserFork"
/>
</template>
......
app/assets/javascripts/diffs/components/diff_content.vue
View file @ ed3034bb
......@@ -23,6 +23,11 @@ export default {
type: Object,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapState({
......@@ -74,11 +79,13 @@ export default {
v-if="isInlineView"
:diff-file="diffFile"
:diff-lines="diffFile.highlighted_diff_lines || []"
:help-page-path="helpPagePath"
/>
<parallel-diff-view
v-if="isParallelView"
:diff-file="diffFile"
:diff-lines="diffFile.parallel_diff_lines || []"
:help-page-path="helpPagePath"
/>
</template>
<diff-viewer
......
app/assets/javascripts/diffs/components/diff_discussions.vue
View file @ ed3034bb
......@@ -13,6 +13,11 @@ export default {
type: Array,
required: true,
},
line: {
type: Object,
required: false,
default: null,
},
shouldCollapseDiscussions: {
type: Boolean,
required: false,
......@@ -23,6 +28,11 @@ export default {
required: false,
default: false,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
methods: {
...mapActions(['toggleDiscussion']),
......@@ -72,6 +82,8 @@ export default {
:render-diff-file="false"
:always-expanded="true"
:discussions-by-diff-order="true"
:line="line"
:help-page-path="helpPagePath"
@noteDeleted="deleteNoteHandler"
>
<span v-if="renderAvatarBadge" slot="avatar-badge" class="badge badge-pill">
......
app/assets/javascripts/diffs/components/diff_file.vue
View file @ ed3034bb
......@@ -23,6 +23,11 @@ export default {
type: Boolean,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
data() {
return {
......@@ -164,6 +169,7 @@ export default {
v-if="!isCollapsed && file.renderIt"
:class="{ hidden: isCollapsed || file.too_large }"
:diff-file="file"
:help-page-path="helpPagePath"
/>
<gl-loading-icon v-if="showLoadingIcon" class="diff-content loading" />
<div v-else-if="showExpandMessage" class="nothing-here-block diff-collapsed">
......
app/assets/javascripts/diffs/components/diff_line_note_form.vue
View file @ ed3034bb
......@@ -94,6 +94,7 @@ export default {
ref="noteForm"
:is-editing="true"
:line-code="line.line_code"
:line="line"
save-button-title="Comment"
class="diff-comment-form"
@cancelForm="handleCancelCommentForm"
......
app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
View file @ ed3034bb
......@@ -16,6 +16,11 @@ export default {
type: String,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
className() {
......@@ -38,7 +43,12 @@ export default {
<tr v-if="shouldRender" :class="className" class="notes_holder">
<td class="notes_content" colspan="3">
<div class="content">
<diff-discussions v-if="line.discussions.length" :discussions="line.discussions" />
<diff-discussions
v-if="line.discussions.length"
:line="line"
:discussions="line.discussions"
:help-page-path="helpPagePath"
/>
<diff-line-note-form
v-if="line.hasForm"
:diff-file-hash="diffFileHash"
......
app/assets/javascripts/diffs/components/inline_diff_view.vue
View file @ ed3034bb
......@@ -17,6 +17,11 @@ export default {
type: Array,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapGetters('diffs', ['commitId']),
......@@ -47,6 +52,7 @@ export default {
:key="`icr-${index}`"
:diff-file-hash="diffFile.file_hash"
:line="line"
:help-page-path="helpPagePath"
/>
</template>
</tbody>
......
app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
View file @ ed3034bb
......@@ -20,6 +20,11 @@ export default {
type: Number,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
hasExpandedDiscussionOnLeft() {
......@@ -87,6 +92,8 @@ export default {
<diff-discussions
v-if="line.left.discussions.length"
:discussions="line.left.discussions"
:line="line.left"
:help-page-path="helpPagePath"
/>
</div>
<diff-line-note-form
......@@ -102,6 +109,8 @@ export default {
<diff-discussions
v-if="line.right.discussions.length"
:discussions="line.right.discussions"
:line="line.right"
:help-page-path="helpPagePath"
/>
</div>
<diff-line-note-form
......
app/assets/javascripts/diffs/components/parallel_diff_view.vue
View file @ ed3034bb
......@@ -17,6 +17,11 @@ export default {
type: Array,
required: true,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapGetters('diffs', ['commitId']),
......@@ -49,6 +54,7 @@ export default {
:line="line"
:diff-file-hash="diffFile.file_hash"
:line-index="index"
:help-page-path="helpPagePath"
/>
</template>
</tbody>
......
app/assets/javascripts/diffs/index.js
View file @ ed3034bb
......@@ -16,6 +16,7 @@ export default function initDiffsApp(store) {
return {
endpoint: dataset.endpoint,
projectPath: dataset.projectPath,
helpPagePath: dataset.helpPagePath,
currentUser: JSON.parse(dataset.currentUserData) || {},
changesEmptyStateIllustration: dataset.changesEmptyStateIllustration,
};
......@@ -31,6 +32,7 @@ export default function initDiffsApp(store) {
endpoint: this.endpoint,
currentUser: this.currentUser,
projectPath: this.projectPath,
helpPagePath: this.helpPagePath,
shouldShow: this.activeTab === 'diffs',
changesEmptyStateIllustration: this.changesEmptyStateIllustration,
},
......
app/assets/javascripts/lib/utils/text_markdown.js
View file @ ed3034bb
......@@ -39,7 +39,14 @@ function blockTagText(text, textArea, blockTag, selected) {
}
}
function moveCursor({ textArea, tag, positionBetweenTags, removedLastNewLine, select }) {
function moveCursor({
textArea,
tag,
cursorOffset,
positionBetweenTags,
removedLastNewLine,
select,
}) {
var pos;
if (!textArea.setSelectionRange) {
return;
......@@ -61,11 +68,24 @@ function moveCursor({ textArea, tag, positionBetweenTags, removedLastNewLine, se
pos -= 1;
}
if (cursorOffset) {
pos -= cursorOffset;
}
return textArea.setSelectionRange(pos, pos);
}
}
export function insertMarkdownText({ textArea, text, tag, blockTag, selected, wrap, select }) {
export function insertMarkdownText({
textArea,
text,
tag,
cursorOffset,
blockTag,
selected,
wrap,
select,
}) {
var textToInsert,
selectedSplit,
startChar,
......@@ -154,20 +174,30 @@ export function insertMarkdownText({ textArea, text, tag, blockTag, selected, wr
return moveCursor({
textArea,
tag: tag.replace(textPlaceholder, selected),
cursorOffset,
positionBetweenTags: wrap && selected.length === 0,
removedLastNewLine,
select,
});
}
function updateText({ textArea, tag, blockTag, wrap, select }) {
function updateText({ textArea, tag, cursorOffset, blockTag, wrap, select, tagContent }) {
var $textArea, selected, text;
$textArea = $(textArea);
textArea = $textArea.get(0);
text = $textArea.val();
selected = selectedText(text, textArea);
selected = selectedText(text, textArea) || tagContent;
$textArea.focus();
return insertMarkdownText({ textArea, text, tag, blockTag, selected, wrap, select });
return insertMarkdownText({
textArea,
text,
tag,
cursorOffset,
blockTag,
selected,
wrap,
select,
});
}
export function addMarkdownListeners(form) {
......@@ -178,9 +208,11 @@ export function addMarkdownListeners(form) {
return updateText({
textArea: $this.closest('.md-area').find('textarea'),
tag: $this.data('mdTag'),
cursorOffset: $this.data('mdCursorOffset'),
blockTag: $this.data('mdBlock'),
wrap: !$this.data('mdPrepend'),
select: $this.data('mdSelect'),
tagContent: $this.data('mdTagContent').toString(),
});
});
}
......
app/assets/javascripts/mr_notes/index.js
View file @ ed3034bb
......@@ -33,6 +33,7 @@ export default function initMrNotes() {
noteableData,
currentUserData: JSON.parse(notesDataset.currentUserData),
notesData: JSON.parse(notesDataset.notesData),
helpPagePath: notesDataset.helpPagePath,
};
},
computed: {
......@@ -71,6 +72,7 @@ export default function initMrNotes() {
notesData: this.notesData,
userData: this.currentUserData,
shouldShow: this.activeTab === 'show',
helpPagePath: this.helpPagePath,
},
});
},
......
app/assets/javascripts/notes/components/note_body.vue
View file @ ed3034bb
<script>
import { mapActions } from 'vuex';
import $ from 'jquery';
import noteEditedText from './note_edited_text.vue';
import noteAwardsList from './note_awards_list.vue';
import noteAttachment from './note_attachment.vue';
import noteForm from './note_form.vue';
import autosave from '../mixins/autosave';
import Suggestions from '~/vue_shared/components/markdown/suggestions.vue';
export default {
components: {
......@@ -12,6 +14,7 @@ export default {
noteAwardsList,
noteAttachment,
noteForm,
Suggestions,
},
mixins: [autosave],
props: {
......@@ -19,6 +22,11 @@ export default {
type: Object,
required: true,
},
line: {
type: Object,
required: false,
default: null,
},
canEdit: {
type: Boolean,
required: true,
......@@ -28,11 +36,22 @@ export default {
required: false,
default: false,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
noteBody() {
return this.note.note;
},
hasSuggestion() {
return this.note.suggestions && this.note.suggestions.length;
},
lineType() {
return this.line ? this.line.type : null;
},
},
mounted() {
this.renderGFM();
......@@ -53,6 +72,7 @@ export default {
}
},
methods: {
...mapActions(['submitSuggestion']),
renderGFM() {
$(this.$refs['note-body']).renderGFM();
},
......@@ -62,19 +82,35 @@ export default {
formCancelHandler(shouldConfirm, isDirty) {
this.$emit('cancelForm', shouldConfirm, isDirty);
},
applySuggestion({ suggestionId, flashContainer, callback }) {
const { discussion_id: discussionId, id: noteId } = this.note;
this.submitSuggestion({ discussionId, noteId, suggestionId, flashContainer, callback });
},
},
};
</script>
<template>
<div ref="note-body" :class="{ 'js-task-list-container': canEdit }" class="note-body">
<div class="note-text md" v-html="note.note_html"></div>
<suggestions
v-if="hasSuggestion && !isEditing"
:suggestions="note.suggestions"
:note-html="note.note_html"
:line-type="lineType"
:help-page-path="helpPagePath"
@apply="applySuggestion"
/>
<div v-else class="note-text md" v-html="note.note_html"></div>
<note-form
v-if="isEditing"
ref="noteForm"
:is-editing="isEditing"
:note-body="noteBody"
:note-id="note.id"
:line="line"
:note="note"
:help-page-path="helpPagePath"
:markdown-version="note.cached_markdown_version"
@handleFormUpdate="handleFormUpdate"
@cancelForm="formCancelHandler"
......
app/assets/javascripts/notes/components/note_form.vue
View file @ ed3034bb
<script>
import { mergeUrlParams } from '~/lib/utils/url_utility';
import { mapGetters, mapActions } from 'vuex';
import eventHub from '../event_hub';
import issueWarning from '../../vue_shared/components/issue/issue_warning.vue';
......@@ -53,6 +54,21 @@ export default {
required: false,
default: false,
},
line: {
type: Object,
required: false,
default: null,
},
note: {
type: Object,
required: false,
default: null,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
data() {
return {
......@@ -79,7 +95,8 @@ export default {
return '#';
},
markdownPreviewPath() {
return this.getNoteableDataByProp('preview_note_path');
const notable = this.getNoteableDataByProp('preview_note_path');
return mergeUrlParams({ preview_suggestions: true }, notable);
},
markdownDocsPath() {
return this.getNotesDataByProp('markdownDocsPath');
......@@ -93,6 +110,18 @@ export default {
isDisabled() {
return !this.updatedNoteBody.length || this.isSubmitting;
},
discussionNote() {
const discussionNote = this.discussion.id
? this.getDiscussionLastNote(this.discussion)
: this.note;
return discussionNote || {};
},
canSuggest() {
return (
this.getNoteableData.can_receive_suggestion &&
(this.line && this.line.can_receive_suggestion)
);
},
},
watch: {
noteBody() {
......@@ -171,7 +200,11 @@ export default {
:markdown-docs-path="markdownDocsPath"
:markdown-version="markdownVersion"
:quick-actions-docs-path="quickActionsDocsPath"
:line="line"
:note="discussionNote"
:can-suggest="canSuggest"
:add-spacing-classes="false"
:help-page-path="helpPagePath"
>
<textarea
id="note_note"
......
app/assets/javascripts/notes/components/noteable_discussion.vue
View file @ ed3034bb
......@@ -49,6 +49,11 @@ export default {
type: Object,
required: true,
},
line: {
type: Object,
required: false,
default: null,
},
renderDiffFile: {
type: Boolean,
required: false,
......@@ -64,6 +69,11 @@ export default {
required: false,
default: false,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
data() {
const { diff_discussion: isDiffDiscussion, resolved } = this.discussion;
......@@ -194,6 +204,13 @@ export default {
false,
);
},
diffLine() {
if (this.discussion.diff_discussion && this.discussion.truncated_diff_lines) {
return this.discussion.truncated_diff_lines.slice(-1)[0];
}
return this.line;
},
},
watch: {
isReplying() {
......@@ -357,6 +374,8 @@ Please check your network connection and try again.`;
<component
:is="componentName(initialDiscussion)"
:note="componentData(initialDiscussion)"
:line="line"
:help-page-path="helpPagePath"
@handleDeleteNote="deleteNoteHandler"
>
<slot slot="avatar-badge" name="avatar-badge"></slot>
......@@ -373,6 +392,8 @@ Please check your network connection and try again.`;
v-for="note in replies"
:key="note.id"
:note="componentData(note)"
:help-page-path="helpPagePath"
:line="line"
@handleDeleteNote="deleteNoteHandler"
/>
</template>
......@@ -383,6 +404,8 @@ Please check your network connection and try again.`;
v-for="(note, index) in discussion.notes"
:key="note.id"
:note="componentData(note)"
:help-page-path="helpPagePath"
:line="diffLine"
@handleDeleteNote="deleteNoteHandler"
>
<slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot>
......@@ -447,6 +470,7 @@ Please check your network connection and try again.`;
ref="noteForm"
:discussion="discussion"
:is-editing="false"
:line="diffLine"
save-button-title="Comment"
@handleFormUpdate="saveReply"
@cancelForm="cancelReplyForm"
......
app/assets/javascripts/notes/components/noteable_note.vue
View file @ ed3034bb
......@@ -27,6 +27,16 @@ export default {
type: Object,
required: true,
},
line: {
type: Object,
required: false,
default: null,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
data() {
return {
......@@ -220,8 +230,10 @@ export default {
<note-body
ref="noteBody"
:note="note"
:line="line"
:can-edit="note.current_user.can_edit"
:is-editing="isEditing"
:help-page-path="helpPagePath"
@handleFormUpdate="formUpdateHandler"
@cancelForm="formCancelHandler"
/>
......
app/assets/javascripts/notes/components/notes_app.vue
View file @ ed3034bb
......@@ -49,6 +49,11 @@ export default {
required: false,
default: 0,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
data() {
return {
......@@ -206,6 +211,7 @@ export default {
:key="discussion.id"
:discussion="discussion"
:render-diff-file="true"
:help-page-path="helpPagePath"
/>
</template>
</ul>
......
app/assets/javascripts/notes/services/notes_service.js
View file @ ed3034bb
import Vue from 'vue';
import Api from '~/api';
import VueResource from 'vue-resource';
import * as constants from '../constants';
......@@ -44,4 +45,7 @@ export default {
toggleIssueState(endpoint, data) {
return Vue.http.put(endpoint, data);
},
applySuggestion(id) {
return Api.applySuggestion(id);
},
};
app/assets/javascripts/notes/stores/actions.js
View file @ ed3034bb
......@@ -405,5 +405,25 @@ export const startTaskList = ({ dispatch }) =>
export const updateResolvableDiscussonsCounts = ({ commit }) =>
commit(types.UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS);
export const submitSuggestion = (
{ commit },
{ discussionId, noteId, suggestionId, flashContainer, callback },
) => {
service
.applySuggestion(suggestionId)
.then(() => {
commit(types.APPLY_SUGGESTION, { discussionId, noteId, suggestionId });
callback();
})
.catch(() => {
Flash(
__('Something went wrong while applying the suggestion. Please try again.'),
'alert',
flashContainer,
);
callback();
});
};
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
app/assets/javascripts/notes/stores/modules/index.js
View file @ ed3034bb
......@@ -20,6 +20,7 @@ export default () => ({
userData: {},
noteableData: {
current_user: {},
preview_note_path: 'path/to/preview',
},
commentsDisabled: false,
resolvableDiscussionsCount: 0,
......
app/assets/javascripts/notes/stores/mutation_types.js
View file @ ed3034bb
......@@ -16,6 +16,7 @@ export const SET_DISCUSSION_DIFF_LINES = 'SET_DISCUSSION_DIFF_LINES';
export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE';
export const SET_NOTES_LOADING_STATE = 'SET_NOTES_LOADING_STATE';
export const DISABLE_COMMENTS = 'DISABLE_COMMENTS';
export const APPLY_SUGGESTION = 'APPLY_SUGGESTION';
// DISCUSSION
export const COLLAPSE_DISCUSSION = 'COLLAPSE_DISCUSSION';
......
app/assets/javascripts/notes/stores/mutations.js
View file @ ed3034bb
......@@ -197,6 +197,17 @@ export default {
}
},
[types.APPLY_SUGGESTION](state, { noteId, discussionId, suggestionId }) {
const noteObj = utils.findNoteObjectById(state.discussions, discussionId);
const comment = utils.findNoteObjectById(noteObj.notes, noteId);
comment.suggestions = comment.suggestions.map(suggestion => ({
...suggestion,
applied: suggestion.applied || suggestion.id === suggestionId,
appliable: false,
}));
},
[types.UPDATE_DISCUSSION](state, noteData) {
const note = noteData;
const selectedDiscussion = state.discussions.find(disc => disc.id === note.id);
......
app/assets/javascripts/vue_shared/components/markdown/field.vue
View file @ ed3034bb
<script>
import $ from 'jquery';
import _ from 'underscore';
import { __ } from '~/locale';
import { stripHtml } from '~/lib/utils/text_utility';
import Flash from '../../../flash';
import GLForm from '../../../gl_form';
import markdownHeader from './header.vue';
import markdownToolbar from './toolbar.vue';
import icon from '../icon.vue';
import Suggestions from '~/vue_shared/components/markdown/suggestions.vue';
export default {
components: {
markdownHeader,
markdownToolbar,
icon,
Suggestions,
},
props: {
markdownPreviewPath: {
......@@ -48,12 +52,33 @@ export default {
required: false,
default: true,
},
line: {
type: Object,
required: false,
default: null,
},
note: {
type: Object,
required: false,
default: () => ({}),
},
canSuggest: {
type: Boolean,
required: false,
default: false,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
data() {
return {
markdownPreview: '',
referencedCommands: '',
referencedUsers: '',
hasSuggestion: false,
markdownPreviewLoading: false,
previewMarkdown: false,
};
......@@ -63,6 +88,39 @@ export default {
const referencedUsersThreshold = 10;
return this.referencedUsers.length >= referencedUsersThreshold;
},
lineContent() {
const FIRST_CHAR_REGEX = /^(\+|-)/;
const [firstSuggestion] = this.suggestions;
if (firstSuggestion) {
return firstSuggestion.from_content;
}
if (this.line) {
const { rich_text: richText, text } = this.line;
if (text) {
return text.replace(FIRST_CHAR_REGEX, '');
}
return _.unescape(stripHtml(richText).replace(/\n/g, ''));
}
return '';
},
lineNumber() {
let lineNumber;
if (this.line) {
const { new_line: newLine, old_line: oldLine } = this.line;
lineNumber = newLine || oldLine;
}
return lineNumber;
},
suggestions() {
return this.note.suggestions || [];
},
lineType() {
return this.line ? this.line.type : '';
},
},
mounted() {
/*
......@@ -122,6 +180,7 @@ export default {
if (data.references) {
this.referencedCommands = data.references.commands;
this.referencedUsers = data.references.users;
this.hasSuggestion = data.references.suggestions && data.references.suggestions.length;
}
this.$nextTick(() => {
......@@ -147,6 +206,8 @@ export default {
>
<markdown-header
:preview-markdown="previewMarkdown"
:line-content="lineContent"
:can-suggest="canSuggest"
@preview-markdown="showPreviewTab"
@write-markdown="showWriteTab"
/>
......@@ -163,19 +224,39 @@ export default {
/>
</div>
</div>
<div
v-show="previewMarkdown"
ref="markdown-preview"
class="md-preview js-vue-md-preview md md-preview-holder"
v-html="markdownPreview"
></div>
<template v-if="hasSuggestion">
<div
v-show="previewMarkdown"
ref="markdown-preview"
class="md-preview js-vue-md-preview md md-preview-holder"
>
<suggestions
v-if="hasSuggestion"
:note-html="markdownPreview"
:from-line="lineNumber"
:from-content="lineContent"
:line-type="lineType"
:disabled="true"
:suggestions="suggestions"
:help-page-path="helpPagePath"
/>
</div>
</template>
<template v-else>
<div
v-show="previewMarkdown"
ref="markdown-preview"
class="md-preview js-vue-md-preview md md-preview-holder"
v-html="markdownPreview"
></div>
</template>
<template v-if="previewMarkdown && !markdownPreviewLoading">
<div v-if="referencedCommands" class="referenced-commands" v-html="referencedCommands"></div>
<div v-if="shouldShowReferencedUsers" class="referenced-users">
<span>
<i class="fa fa-exclamation-triangle" aria-hidden="true"> </i> You are about to add
<i class="fa fa-exclamation-triangle" aria-hidden="true"></i> You are about to add
<strong>
<span class="js-referenced-users-count"> {{ referencedUsers.length }} </span>
<span class="js-referenced-users-count">{{ referencedUsers.length }}</span>
</strong>
people to the discussion. Proceed with caution.
</span>
......
app/assets/javascripts/vue_shared/components/markdown/header.vue
View file @ ed3034bb
......@@ -17,6 +17,16 @@ export default {
type: Boolean,
required: true,
},
lineContent: {
type: String,
required: false,
default: '',
},
canSuggest: {
type: Boolean,
required: false,
default: true,
},
},
computed: {
mdTable() {
......@@ -27,6 +37,9 @@ export default {
'| cell | cell |',
].join('\n');
},
mdSuggestion() {
return ['```suggestion', `{text}`, '```'].join('\n');
},
},
mounted() {
$(document).on('markdown-preview:show.vue', this.previewMarkdownTab);
......@@ -119,6 +132,16 @@ export default {
:button-title="__('Add a table')"
icon="table"
/>
<toolbar-button
v-if="canSuggest"
:tag="mdSuggestion"
:prepend="true"
:button-title="__('Insert suggestion')"
:cursor-offset="4"
:tag-content="lineContent"
icon="doc-code"
class="qa-suggestion-btn"
/>
<button
v-gl-tooltip
aria-label="Go full screen"
......
app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue 0 → 100644
View file @ ed3034bb
<script>
import SuggestionDiffHeader from './suggestion_diff_header.vue';
export default {
components: {
SuggestionDiffHeader,
},
props: {
newLines: {
type: Array,
required: true,
},
fromContent: {
type: String,
required: false,
default: '',
},
fromLine: {
type: Number,
required: true,
},
suggestion: {
type: Object,
required: true,
},
disabled: {
type: Boolean,
required: false,
default: false,
},
helpPagePath: {
type: String,
required: true,
},
},
methods: {
applySuggestion(callback) {
this.$emit('apply', { suggestionId: this.suggestion.id, callback });
},
},
};
</script>
<template>
<div>
<suggestion-diff-header
class="qa-suggestion-diff-header"
:can-apply="suggestion.appliable && suggestion.current_user.can_apply && !disabled"
:is-applied="suggestion.applied"
:help-page-path="helpPagePath"
@apply="applySuggestion"
/>
<table class="mb-3 md-suggestion-diff">
<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>
</tbody>
</table>
</div>
</template>
app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue 0 → 100644
View file @ ed3034bb
<script>
import Icon from '~/vue_shared/components/icon.vue';
export default {
components: { Icon },
props: {
canApply: {
type: Boolean,
required: false,
default: false,
},
isApplied: {
type: Boolean,
required: true,
default: false,
},
helpPagePath: {
type: String,
required: true,
},
},
data() {
return {
isAppliedSuccessfully: false,
isApplying: false,
};
},
methods: {
applySuggestion() {
if (!this.canApply) return;
this.isApplying = true;
this.$emit('apply', this.applySuggestionCallback);
},
applySuggestionCallback() {
this.isApplying = false;
},
},
};
</script>
<template>
<div class="md-suggestion-header border-bottom-0 mt-2">
<div class="qa-suggestion-diff-header font-weight-bold">
{{ __('Suggested change') }}
<a v-if="helpPagePath" :href="helpPagePath" :aria-label="__('Help')">
<icon name="question-o" css-classes="link-highlight" />
</a>
</div>
<span v-if="isApplied" class="badge badge-success">{{ __('Applied') }}</span>
<button
v-if="canApply"
type="button"
class="btn qa-apply-btn"
:disabled="isApplying"
@click="applySuggestion"
>
{{ __('Apply suggestion') }}
</button>
</div>
</template>
app/assets/javascripts/vue_shared/components/markdown/suggestions.vue 0 → 100644
View file @ ed3034bb
<script>
import Vue from 'vue';
import SuggestionDiff from './suggestion_diff.vue';
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,
default: '',
},
suggestions: {
type: Array,
required: false,
default: () => [],
},
noteHtml: {
type: String,
required: true,
},
disabled: {
type: Boolean,
required: false,
default: false,
},
helpPagePath: {
type: String,
required: true,
},
},
data() {
return {
isRendered: false,
};
},
watch: {
suggestions() {
this.reset();
},
noteHtml() {
this.reset();
},
},
mounted() {
this.renderSuggestions();
},
methods: {
renderSuggestions() {
// swaps out suggestion(s) markdown with rich diff components
// (while still keeping non-suggestion markdown in place)
if (!this.noteHtml) return;
const { container } = this.$refs;
const suggestionElements = container.querySelectorAll('.js-render-suggestion');
if (this.lineType === 'old') {
Flash('Unable to apply suggestions to a deleted line.', 'alert', this.$el);
}
suggestionElements.forEach((suggestionEl, i) => {
const suggestionParentEl = suggestionEl.parentElement;
const newLines = this.extractNewLines(suggestionParentEl);
const diffComponent = this.generateDiff(newLines, i);
diffComponent.$mount(suggestionParentEl);
});
this.isRendered = true;
},
extractNewLines(suggestionEl) {
// extracts the suggested lines from the markdown
// calculates a line number for each line
const FIRST_CHAR_REGEX = /^(\+|-)/;
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.replace(FIRST_CHAR_REGEX, '')}\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
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 },
});
suggestionDiff.$on('apply', ({ suggestionId, callback }) => {
this.$emit('apply', { suggestionId, callback, flashContainer: this.$el });
});
return suggestionDiff;
},
reset() {
// resets the container HTML (replaces it with the updated noteHTML)
// calls `renderSuggestions` once the updated noteHTML is added to the DOM
this.$refs.container.innerHTML = this.noteHtml;
this.isRendered = false;
this.renderSuggestions();
this.$nextTick(() => this.renderSuggestions());
},
},
};
</script>
<template>
<div>
<div class="flash-container mt-3"></div>
<div v-show="isRendered" ref="container" v-html="noteHtml"></div>
</div>
</template>
app/assets/javascripts/vue_shared/components/markdown/toolbar_button.vue
View file @ ed3034bb
......@@ -37,6 +37,16 @@ export default {
required: false,
default: false,
},
tagContent: {
type: String,
required: false,
default: '',
},
cursorOffset: {
type: Number,
required: false,
default: 0,
},
},
};
</script>
......@@ -45,8 +55,10 @@ export default {
<button
v-gl-tooltip
:data-md-tag="tag"
:data-md-cursor-offset="cursorOffset"
:data-md-select="tagSelect"
:data-md-block="tagBlock"
:data-md-tag-content="tagContent"
:data-md-prepend="prepend"
:title="buttonTitle"
:aria-label="buttonTitle"
......
app/assets/stylesheets/framework/markdown_area.scss
View file @ ed3034bb
......@@ -277,6 +277,27 @@
}
}
.md-suggestion-diff {
display: table !important;
border: 1px solid $border-color !important;
}
.md-suggestion-header {
height: $suggestion-header-height;
display: flex;
align-items: center;
justify-content: space-between;
background-color: $gray-light;
border: 1px solid $border-color;
padding: $gl-padding;
border-radius: $border-radius-default $border-radius-default 0 0;
svg {
vertical-align: middle;
margin-bottom: 3px;
}
}
@include media-breakpoint-down(xs) {
.atwho-view-ul {
width: 350px;
......
app/assets/stylesheets/framework/variables.scss
View file @ ed3034bb
......@@ -252,6 +252,7 @@ $browserScrollbarSize: 10px;
* Misc
*/
$header-height: 40px;
$suggestion-header-height: 46px;
$ide-statusbar-height: 25px;
$fixed-layout-width: 1280px;
$limited-layout-width: 990px;
......
app/controllers/concerns/preview_markdown.rb
View file @ ed3034bb
......@@ -12,7 +12,7 @@ module PreviewMarkdown
when 'wikis' then { pipeline: :wiki, project_wiki: @project_wiki, page_slug: params[:id] }
when 'snippets' then { skip_project_check: true }
when 'groups' then { group: group }
when 'projects' then { issuable_state_filter_enabled: true }
when 'projects' then projects_filter_params
else {}
end
......@@ -22,9 +22,17 @@ module PreviewMarkdown
body: view_context.markdown(result[:text], markdown_params),
references: {
users: result[:users],
suggestions: result[:suggestions],
commands: view_context.markdown(result[:commands])
}
}
end
def projects_filter_params
{
issuable_state_filter_enabled: true,
suggestions_filter_enabled: params[:preview_suggestions].present?
}
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end
app/models/concerns/noteable.rb
View file @ ed3034bb
......@@ -26,6 +26,10 @@ module Noteable
DiscussionNote.noteable_types.include?(base_class_name)
end
def supports_suggestion?
false
end
def discussions_rendered_on_frontend?
false
end
......
app/models/diff_note.rb
View file @ ed3034bb
......@@ -66,10 +66,23 @@ class DiffNote < Note
self.original_position.diff_refs == diff_refs
end
def supports_suggestion?
return false unless noteable.supports_suggestion? && on_text?
# We don't want to trigger side-effects of `diff_file` call.
return false unless file = fetch_diff_file
return false unless line = file.line_for_position(self.original_position)
line&.suggestible?
end
def discussion_first_note?
self == discussion.first_note
end
def banzai_render_context(field)
super.merge(suggestions_filter_enabled: supports_suggestion?)
end
private
def enqueue_diff_file_creation_job
......
app/models/merge_request.rb
View file @ ed3034bb
......@@ -363,6 +363,11 @@ class MergeRequest < ActiveRecord::Base
end
end
def supports_suggestion?
# Should be `true` when removing the FF.
Suggestion.feature_enabled?
end
# Calls `MergeWorker` to proceed with the merge process and
# updates `merge_jid` with the MergeWorker#jid.
# This helps tracking enqueued and ongoing merge jobs.
......
app/models/note.rb
View file @ ed3034bb
......@@ -69,6 +69,12 @@ class Note < ActiveRecord::Base
belongs_to :last_edited_by, class_name: 'User'
has_many :todos
# The delete_all definition is required here in order
# to generate the correct DELETE sql for
# suggestions.delete_all calls
has_many :suggestions, -> { order(:relative_order) },
inverse_of: :note, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :events, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_one :system_note_metadata
has_one :note_diff_file, inverse_of: :diff_note, foreign_key: :diff_note_id
......@@ -110,7 +116,7 @@ class Note < ActiveRecord::Base
scope :inc_author, -> { includes(:author) }
scope :inc_relations_for_view, -> do
includes(:project, { author: :status }, :updated_by, :resolved_by, :award_emoji,
:system_note_metadata, :note_diff_file)
:system_note_metadata, :note_diff_file, :suggestions)
end
scope :with_notes_filter, -> (notes_filter) do
......@@ -226,6 +232,10 @@ class Note < ActiveRecord::Base
Gitlab::HookData::NoteBuilder.new(self).build
end
def supports_suggestion?
false
end
def for_commit?
noteable_type == "Commit"
end
......
app/models/suggestion.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
class Suggestion < ApplicationRecord
FEATURE_FLAG = :diff_suggestions
belongs_to :note, inverse_of: :suggestions
validates :note, presence: true
validates :commit_id, presence: true, if: :applied?
delegate :original_position, :position, :diff_file,
:noteable, to: :note
def self.feature_enabled?
Feature.enabled?(FEATURE_FLAG)
end
def project
noteable.source_project
end
def branch
noteable.source_branch
end
# For now, suggestions only serve as a way to send patches that
# will change a single line (being able to apply multiple in the same place),
# which explains `from_line` and `to_line` being the same line.
# We'll iterate on that in https://gitlab.com/gitlab-org/gitlab-ce/issues/53310
# when allowing multi-line suggestions.
def from_line
position.new_line
end
alias_method :to_line, :from_line
def from_original_line
original_position.new_line
end
alias_method :to_original_line, :from_original_line
# `from_line_index` and `to_line_index` represents diff/blob line numbers in
# index-like way (N-1).
def from_line_index
from_line - 1
end
alias_method :to_line_index, :from_line_index
def appliable?
return false unless note.supports_suggestion?
!applied? &&
noteable.opened? &&
different_content? &&
note.active?
end
private
def different_content?
from_content != to_content
end
end
app/policies/suggestion_policy.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
class SuggestionPolicy < BasePolicy
delegate { @subject.project }
condition(:can_push_to_branch) do
Gitlab::UserAccess.new(@user, project: @subject.project).can_push_to_branch?(@subject.branch)
end
rule { can_push_to_branch }.enable :apply_suggestion
end
app/serializers/diff_line_entity.rb
View file @ ed3034bb
......@@ -11,4 +11,6 @@ class DiffLineEntity < Grape::Entity
expose :rich_text do |line|
ERB::Util.html_escape(line.rich_text || line.text)
end
expose :suggestible?, as: :can_receive_suggestion
end
app/serializers/merge_request_widget_entity.rb
View file @ ed3034bb
......@@ -238,6 +238,8 @@ class MergeRequestWidgetEntity < IssuableEntity
end
end
expose :supports_suggestion?, as: :can_receive_suggestion
private
delegate :current_user, to: :request
......
app/serializers/note_entity.rb
View file @ ed3034bb
......@@ -36,6 +36,7 @@ class NoteEntity < API::Entities::Note
end
end
expose :suggestions, using: SuggestionEntity
expose :resolved?, as: :resolved
expose :resolvable?, as: :resolvable
......
app/serializers/suggestion_entity.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
class SuggestionEntity < API::Entities::Suggestion
include RequestAwareEntity
expose :current_user do
expose :can_apply do |suggestion|
Ability.allowed?(current_user, :apply_suggestion, suggestion)
end
end
private
def current_user
request.current_user
end
end
app/services/notes/create_service.rb
View file @ ed3034bb
......@@ -36,6 +36,7 @@ module Notes
if !only_commands && note.save
todo_service.new_note(note, current_user)
clear_noteable_diffs_cache(note)
Suggestions::CreateService.new(note).execute
end
if command_params.present?
......
app/services/notes/update_service.rb
View file @ ed3034bb
......@@ -14,6 +14,17 @@ module Notes
TodoService.new.update_note(note, current_user, old_mentioned_users)
end
if note.supports_suggestion?
Suggestion.transaction do
note.suggestions.delete_all
Suggestions::CreateService.new(note).execute
end
# We need to refresh the previous suggestions call cache
# in order to get the new records.
note.reload
end
note
end
end
......
app/services/preview_markdown_service.rb
View file @ ed3034bb
......@@ -4,10 +4,12 @@ class PreviewMarkdownService < BaseService
def execute
text, commands = explain_quick_actions(params[:text])
users = find_user_references(text)
suggestions = find_suggestions(text)
success(
text: text,
users: users,
suggestions: suggestions,
commands: commands.join(' '),
markdown_engine: markdown_engine
)
......@@ -28,6 +30,12 @@ class PreviewMarkdownService < BaseService
extractor.users.map(&:username)
end
def find_suggestions(text)
return [] unless params[:preview_suggestions]
Banzai::SuggestionsParser.parse(text)
end
def find_commands_target
QuickActions::TargetService
.new(project, current_user)
......
app/services/suggestions/apply_service.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
module Suggestions
class ApplyService < ::BaseService
def initialize(current_user)
@current_user = current_user
end
def execute(suggestion)
unless suggestion.appliable?
return error('Suggestion is not appliable')
end
params = file_update_params(suggestion)
result = ::Files::UpdateService.new(suggestion.project, @current_user, params).execute
if result[:status] == :success
suggestion.update(commit_id: result[:result], applied: true)
end
result
end
private
def file_update_params(suggestion)
diff_file = suggestion.diff_file
file_path = diff_file.file_path
branch_name = suggestion.noteable.source_branch
file_content = new_file_content(suggestion)
commit_message = "Apply suggestion to #{file_path}"
{
file_path: file_path,
branch_name: branch_name,
start_branch: branch_name,
commit_message: commit_message,
file_content: file_content
}
end
def new_file_content(suggestion)
range = suggestion.from_line_index..suggestion.to_line_index
blob = suggestion.diff_file.new_blob
blob.load_all_data!
content = blob.data.lines
content[range] = suggestion.to_content
content.join
end
end
end
app/services/suggestions/create_service.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
module Suggestions
class CreateService
def initialize(note)
@note = note
end
def execute
return unless @note.supports_suggestion?
suggestions = Banzai::SuggestionsParser.parse(@note.note)
# For single line suggestion we're only looking forward to
# change the line receiving the comment. Though, in
# https://gitlab.com/gitlab-org/gitlab-ce/issues/53310
# we'll introduce a ```suggestion:L<x>-<y>, so this will
# slightly change.
comment_line = @note.position.new_line
rows =
suggestions.map.with_index do |suggestion, index|
from_content = changing_lines(comment_line, comment_line)
# The parsed suggestion doesn't have information about the correct
# ending characters (we may have a line break, or not), so we take
# this information from the last line being changed (last
# characters).
endline_chars = line_break_chars(from_content.lines.last)
to_content = "#{suggestion}#{endline_chars}"
{
note_id: @note.id,
from_content: from_content,
to_content: to_content,
relative_order: index
}
end
rows.in_groups_of(100, false) do |rows|
Gitlab::Database.bulk_insert('suggestions', rows)
end
end
private
def changing_lines(from_line, to_line)
@note.diff_file.new_blob_lines_between(from_line, to_line).join
end
def line_break_chars(line)
match = /\r\n|\r|\n/.match(line)
match[0] if match
end
end
end
app/views/projects/merge_requests/show.html.haml
View file @ ed3034bb
......@@ -67,6 +67,7 @@
noteable_data: serialize_issuable(@merge_request),
noteable_type: 'MergeRequest',
target_type: 'merge_request',
help_page_path: nil,
current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json} }
#commits.commits.tab-pane
......@@ -76,6 +77,7 @@
= render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request)
#js-diffs-app.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked?,
endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', request.query_parameters),
help_page_path: nil,
current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json,
project_path: project_path(@merge_request.project),
changes_empty_state_illustration: image_path('illustrations/merge_request_changes_empty.svg') } }
......
changelogs/unreleased/suggest-change-to-diff-line.yml 0 → 100644
View file @ ed3034bb
---
title: Add ability to render suggestions
merge_request: 23147
author:
type: added
db/migrate/20181123144235_create_suggestions.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
class CreateSuggestions < ActiveRecord::Migration
DOWNTIME = false
def change
create_table :suggestions, id: :bigserial do |t|
t.references :note, foreign_key: { on_delete: :cascade }, null: false
t.integer :relative_order, null: false, limit: 2
t.boolean :applied, null: false, default: false
t.string :commit_id
t.text :from_content, null: false
t.text :to_content, null: false
t.index [:note_id, :relative_order],
name: 'index_suggestions_on_note_id_and_relative_order',
unique: true
end
end
end
db/schema.rb
View file @ ed3034bb
......@@ -1956,6 +1956,16 @@ ActiveRecord::Schema.define(version: 20181204154019) do
t.index ["subscribable_id", "subscribable_type", "user_id", "project_id"], name: "index_subscriptions_on_subscribable_and_user_id_and_project_id", unique: true, using: :btree
end
create_table "suggestions", id: :bigserial, force: :cascade do |t|
t.integer "note_id", null: false
t.integer "relative_order", limit: 2, null: false
t.boolean "applied", default: false, null: false
t.string "commit_id"
t.text "from_content", null: false
t.text "to_content", null: false
t.index ["note_id", "relative_order"], name: "index_suggestions_on_note_id_and_relative_order", unique: true, using: :btree
end
create_table "system_note_metadata", force: :cascade do |t|
t.integer "note_id", null: false
t.integer "commit_count"
......@@ -2432,6 +2442,7 @@ ActiveRecord::Schema.define(version: 20181204154019) do
add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade
add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade
add_foreign_key "subscriptions", "projects", on_delete: :cascade
add_foreign_key "suggestions", "notes", on_delete: :cascade
add_foreign_key "system_note_metadata", "notes", name: "fk_d83a918cb1", on_delete: :cascade
add_foreign_key "term_agreements", "application_setting_terms", column: "term_id"
add_foreign_key "term_agreements", "users", on_delete: :cascade
......
lib/api/api.rb
View file @ ed3034bb
......@@ -149,6 +149,7 @@ module API
mount ::API::Snippets
mount ::API::Submodules
mount ::API::Subscriptions
mount ::API::Suggestions
mount ::API::SystemHooks
mount ::API::Tags
mount ::API::Templates
......
lib/api/entities.rb
View file @ ed3034bb
......@@ -1495,5 +1495,17 @@ module API
expose :label, using: Entities::LabelBasic
expose :action
end
class Suggestion < Grape::Entity
expose :id
expose :from_original_line
expose :to_original_line
expose :from_line
expose :to_line
expose :appliable?, as: :appliable
expose :applied
expose :from_content
expose :to_content
end
end
end
lib/api/suggestions.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
module API
class Suggestions < Grape::API
before { authenticate! }
resource :suggestions do
desc 'Apply suggestion patch in the Merge Request it was created' do
success Entities::Suggestion
end
params do
requires :id, type: String, desc: 'The suggestion ID'
end
put ':id/apply' do
suggestion = Suggestion.find_by_id(params[:id])
not_found! unless suggestion
authorize! :apply_suggestion, suggestion
result = ::Suggestions::ApplyService.new(current_user).execute(suggestion)
if result[:status] == :success
present suggestion, with: Entities::Suggestion, current_user: current_user
else
http_status = result[:http_status] || 400
render_api_error!(result[:message], http_status)
end
end
end
end
end
lib/banzai/filter/suggestion_filter.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
module Banzai
module Filter
class SuggestionFilter < HTML::Pipeline::Filter
# Class used for tagging elements that should be rendered
TAG_CLASS = 'js-render-suggestion'.freeze
def call
return doc unless Suggestion.feature_enabled?
return doc unless suggestions_filter_enabled?
doc.search('pre.suggestion > code').each do |node|
node.add_class(TAG_CLASS)
end
doc
end
def suggestions_filter_enabled?
context[:suggestions_filter_enabled]
end
end
end
end
lib/banzai/filter/syntax_highlight_filter.rb
View file @ ed3034bb
......@@ -69,7 +69,7 @@ module Banzai
end
def use_rouge?(language)
%w(math mermaid plantuml).exclude?(language)
%w(math mermaid plantuml suggestion).exclude?(language)
end
end
end
......
lib/banzai/pipeline/gfm_pipeline.rb
View file @ ed3034bb
......@@ -29,6 +29,7 @@ module Banzai
Filter::TableOfContentsFilter,
Filter::AutolinkFilter,
Filter::ExternalLinkFilter,
Filter::SuggestionFilter,
*reference_filters,
......
lib/banzai/pipeline/post_process_pipeline.rb
View file @ ed3034bb
......@@ -14,7 +14,8 @@ module Banzai
[
Filter::RedactorFilter,
Filter::RelativeLinkFilter,
Filter::IssuableStateFilter
Filter::IssuableStateFilter,
Filter::SuggestionFilter
]
end
......
lib/banzai/suggestions_parser.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
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
lib/gitlab/diff/file.rb
View file @ ed3034bb
......@@ -122,6 +122,16 @@ module Gitlab
old_blob_lazy&.itself
end
def new_blob_lines_between(from_line, to_line)
return [] unless new_blob
from_index = from_line - 1
to_index = to_line - 1
new_blob.load_all_data!
new_blob.data.lines[from_index..to_index]
end
def content_sha
new_content_sha || old_content_sha
end
......
lib/gitlab/diff/line.rb
View file @ ed3034bb
......@@ -73,6 +73,10 @@ module Gitlab
!meta?
end
def suggestible?
!removed?
end
def rich_text
@parent_file.try(:highlight_lines!) if @parent_file && !@rich_text
......
lib/gitlab/usage_data.rb
View file @ ed3034bb
......@@ -85,6 +85,8 @@ module Gitlab
releases: count(Release),
remote_mirrors: count(RemoteMirror),
snippets: count(Snippet),
suggestions: count(Suggestion),
todos: count(Todo),
uploads: count(Upload),
web_hooks: count(WebHook)
}.merge(services_usage).merge(approximate_counts)
......
locale/gitlab.pot
View file @ ed3034bb
......@@ -657,6 +657,12 @@ msgstr ""
msgid "Applications"
msgstr ""
msgid "Applied"
msgstr ""
msgid "Apply suggestion"
msgstr ""
msgid "Apr"
msgstr ""
......@@ -3597,6 +3603,9 @@ msgstr ""
msgid "Input your repository URL"
msgstr ""
msgid "Insert suggestion"
msgstr ""
msgid "Install GitLab Runner"
msgstr ""
......@@ -6080,6 +6089,9 @@ msgstr ""
msgid "Something went wrong when toggling the button"
msgstr ""
msgid "Something went wrong while applying the suggestion. Please try again."
msgstr ""
msgid "Something went wrong while closing the %{issuable}. Please try again later"
msgstr ""
......@@ -6347,6 +6359,9 @@ msgstr ""
msgid "Subscribed"
msgstr ""
msgid "Suggested change"
msgstr ""
msgid "Switch branch/tag"
msgstr ""
......
spec/db/schema_spec.rb
View file @ ed3034bb
......@@ -54,7 +54,8 @@ describe 'Database schema' do
user_agent_details: %w[subject_id],
users: %w[color_scheme_id created_by_id theme_id],
users_star_projects: %w[user_id],
web_hooks: %w[service_id]
web_hooks: %w[service_id],
suggestions: %w[commit_id]
}.with_indifferent_access.freeze
context 'for table' do
......
spec/factories/suggestions.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
FactoryBot.define do
factory :suggestion do
relative_order 0
association :note, factory: :diff_note_on_merge_request
from_content " vars = {\n"
to_content " vars = [\n"
trait :unappliable do
from_content "foo"
to_content "foo"
end
trait :applied do
applied true
commit_id { RepoHelpers.sample_commit.id }
end
end
end
spec/features/merge_request/user_suggests_changes_on_diff_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe 'User comments on a diff', :js do
include MergeRequestDiffHelpers
include RepoHelpers
let(:project) { create(:project, :repository) }
let(:merge_request) do
create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
end
let(:user) { create(:user) }
before do
project.add_maintainer(user)
sign_in(user)
visit(diffs_project_merge_request_path(project, merge_request))
end
context 'single suggestion note' 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\n# change to a comment\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')
expect(page).to have_content(' url = https://github.com/gitlabhq/gitlab-shell.git')
expect(page).to have_content('# change to a comment')
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\n# change to a comment\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
context 'multiple suggestions in a single note' do
it 'suggestions are 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\n# change to a comment\n```\n```suggestion\n# or that\n```")
click_button('Comment')
end
wait_for_requests
page.within('.diff-discussions') 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')
expect(suggestion_2).to have_content(' url = https://github.com/gitlabhq/gitlab-shell.git')
expect(suggestion_2).to have_content('# or that')
end
end
end
end
spec/fixtures/api/schemas/entities/diff_line.json
View file @ ed3034bb
......@@ -8,7 +8,8 @@
"new_line": { "type": ["integer", "null"] },
"text": { "type": ["string"] },
"rich_text": { "type": ["string"] },
"meta_data": { "type": ["object", "null"] }
"meta_data": { "type": ["object", "null"] },
"can_receive_suggestion": { "type": "boolean" }
},
"additionalProperties": false
}
spec/fixtures/api/schemas/entities/merge_request_widget.json
View file @ ed3034bb
......@@ -119,7 +119,8 @@
"can_push_to_source_branch": { "type": "boolean" },
"rebase_path": { "type": ["string", "null"] },
"squash": { "type": "boolean" },
"test_reports_path": { "type": ["string", "null"] }
"test_reports_path": { "type": ["string", "null"] },
"can_receive_suggestion": { "type": "boolean" }
},
"additionalProperties": false
}
spec/javascripts/diffs/components/diff_content_spec.js
View file @ ed3034bb
......@@ -17,6 +17,7 @@ describe('DiffContent', () => {
current_user: {
can_create_note: false,
},
preview_note_path: 'path/to/preview',
};
vm = mountComponentWithStore(Component, {
......
spec/javascripts/diffs/mock_data/diff_discussions.js
View file @ ed3034bb
......@@ -487,8 +487,19 @@ export default {
],
},
diff_discussion: true,
truncated_diff_lines:
'<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="1">\n1\n</td>\n<td class="line_content new noteable_line"><span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n</td>\n</tr>\n<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="2">\n2\n</td>\n<td class="line_content new noteable_line"><span id="LC2" class="line" lang="plaintext"></span>\n</td>\n</tr>\n',
truncated_diff_lines: [
{
text: 'line',
rich_text:
'<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="1">\n1\n</td>\n<td class="line_content new noteable_line"><span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n</td>\n</tr>\n<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="2">\n2\n</td>\n<td class="line_content new noteable_line"><span id="LC2" class="line" lang="plaintext"></span>\n</td>\n</tr>\n',
can_receive_suggestion: true,
line_code: '6f209374f7e565f771b95720abf46024c41d1885_1_1',
type: 'new',
old_line: null,
new_line: 1,
meta_data: null,
},
],
};
export const imageDiffDiscussions = [
......
spec/javascripts/vue_shared/components/markdown/header_spec.js
View file @ ed3034bb
......@@ -28,6 +28,7 @@ describe('Markdown field header component', () => {
'Add a numbered list',
'Add a task list',
'Add a table',
'Insert suggestion',
'Go full screen',
];
const elements = vm.$el.querySelectorAll('.toolbar-btn');
......@@ -93,4 +94,18 @@ describe('Markdown field header component', () => {
'| header | header |\n| ------ | ------ |\n| cell | cell |\n| cell | cell |',
);
});
it('renders suggestion template', () => {
vm.lineContent = 'Some content';
expect(vm.mdSuggestion).toEqual('```suggestion\n{text}\n```');
});
it('does not render suggestion button if `canSuggest` is set to false', () => {
vm.canSuggest = false;
Vue.nextTick(() => {
expect(vm.$el.querySelector('.qa-suggestion-btn')).toBe(null);
});
});
});
spec/javascripts/vue_shared/components/markdown/suggestion_diff_header_spec.js 0 → 100644
View file @ ed3034bb
import Vue from 'vue';
import SuggestionDiffHeaderComponent from '~/vue_shared/components/markdown/suggestion_diff_header.vue';
const MOCK_DATA = {
canApply: true,
isApplied: false,
helpPagePath: 'path_to_docs',
};
describe('Suggestion Diff component', () => {
let vm;
function createComponent(propsData) {
const Component = Vue.extend(SuggestionDiffHeaderComponent);
return new Component({
propsData,
}).$mount();
}
beforeEach(done => {
vm = createComponent(MOCK_DATA);
Vue.nextTick(done);
});
describe('init', () => {
it('renders a suggestion header', () => {
const header = vm.$el.querySelector('.qa-suggestion-diff-header');
expect(header).not.toBeNull();
expect(header.innerHTML.includes('Suggested change')).toBe(true);
});
it('renders an apply button', () => {
const applyBtn = vm.$el.querySelector('.qa-apply-btn');
expect(applyBtn).not.toBeNull();
expect(applyBtn.innerHTML.includes('Apply suggestion')).toBe(true);
});
it('does not render an apply button if `canApply` is set to false', () => {
const props = Object.assign(MOCK_DATA, { canApply: false });
vm = createComponent(props);
expect(vm.$el.querySelector('.qa-apply-btn')).toBeNull();
});
});
describe('applySuggestion', () => {
it('emits when the apply button is clicked', () => {
const props = Object.assign(MOCK_DATA, { canApply: true });
vm = createComponent(props);
spyOn(vm, '$emit');
vm.applySuggestion();
expect(vm.$emit).toHaveBeenCalled();
});
it('does not emit when the canApply is set to false', () => {
spyOn(vm, '$emit');
vm.canApply = false;
vm.applySuggestion();
expect(vm.$emit).not.toHaveBeenCalled();
});
});
});
spec/javascripts/vue_shared/components/markdown/suggestion_diff_spec.js 0 → 100644
View file @ ed3034bb
import Vue from 'vue';
import SuggestionDiffComponent from '~/vue_shared/components/markdown/suggestion_diff.vue';
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,
},
helpPagePath: 'path_to_docs',
};
describe('Suggestion Diff component', () => {
let vm;
beforeEach(done => {
const Component = Vue.extend(SuggestionDiffComponent);
vm = new Component({
propsData: MOCK_DATA,
}).$mount();
Vue.nextTick(done);
});
describe('init', () => {
it('renders a suggestion header', () => {
expect(vm.$el.querySelector('.qa-suggestion-diff-header')).not.toBeNull();
});
it('renders a diff table', () => {
expect(vm.$el.querySelector('table.md-suggestion-diff')).not.toBeNull();
});
it('renders the oldLineNumber', () => {
const fromLine = vm.$el.querySelector('.qa-old-diff-line-number').innerHTML;
expect(parseInt(fromLine, 10)).toBe(vm.fromLine);
});
it('renders the oldLineContent', () => {
const fromContent = vm.$el.querySelector('.line_content.old').innerHTML;
expect(fromContent.includes(vm.fromContent)).toBe(true);
});
it('renders the contents of newLines', () => {
const newLines = 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);
});
});
});
describe('applySuggestion', () => {
it('emits apply event when applySuggestion is called', () => {
const callback = () => {};
spyOn(vm, '$emit');
vm.applySuggestion(callback);
expect(vm.$emit).toHaveBeenCalledWith('apply', { suggestionId: vm.suggestion.id, callback });
});
});
});
spec/javascripts/vue_shared/components/markdown/suggestions_spec.js 0 → 100644
View file @ ed3034bb
import Vue from 'vue';
import SuggestionsComponent from '~/vue_shared/components/markdown/suggestions.vue';
const MOCK_DATA = {
fromLine: 1,
fromContent: 'Old content',
suggestions: [],
noteHtml: `
<div class="suggestion">
<div class="line">Suggestion 1</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 => {
const Component = Vue.extend(SuggestionsComponent);
vm = new Component({
propsData: MOCK_DATA,
}).$mount();
extractedLines = vm.extractNewLines(generateMockLines());
diffTable = vm.generateDiff(extractedLines).$mount().$el;
spyOn(vm, 'renderSuggestions');
vm.renderSuggestions();
Vue.nextTick(done);
});
describe('mounted', () => {
it('renders a flash container', () => {
expect(vm.$el.querySelector('.flash-container')).not.toBeNull();
});
it('renders a container for suggestions', () => {
expect(vm.$refs.container).not.toBeNull();
});
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([]);
});
});
describe('generateDiff', () => {
it('generates a diff table', () => {
expect(diffTable.querySelector('.md-suggestion-diff')).not.toBeNull();
});
it('generates a diff table that contains contents of `oldLineContent`', () => {
expect(diffTable.innerHTML.includes(vm.fromContent)).toBe(true);
});
it('generates a diff table that contains contents the suggested lines', () => {
extractedLines.forEach((line, i) => {
expect(diffTable.innerHTML.includes(extractedLines[i].content)).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');
expect([...lines][0].innerHTML).toBe('1');
expect([...lines][1].innerHTML).toBe('2');
expect([...lines][2].innerHTML).toBe('3');
});
});
});
spec/lib/banzai/filter/suggestion_filter_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe Banzai::Filter::SuggestionFilter do
include FilterSpecHelper
let(:input) { "<pre class='code highlight js-syntax-highlight suggestion'><code>foo\n</code></pre>" }
let(:default_context) do
{ suggestions_filter_enabled: true }
end
it 'includes `js-render-suggestion` class' do
doc = filter(input, default_context)
result = doc.css('code').first
expect(result[:class]).to include('js-render-suggestion')
end
it 'includes no `js-render-suggestion` when feature disabled' do
stub_feature_flags(diff_suggestions: false)
doc = filter(input, default_context)
result = doc.css('code').first
expect(result[:class]).to be_nil
end
it 'includes no `js-render-suggestion` when filter is disabled' do
doc = filter(input)
result = doc.css('code').first
expect(result[:class]).to be_nil
end
end
spec/lib/banzai/suggestions_parser_spec.rb 0 → 100644
View file @ ed3034bb
# 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
spec/lib/gitlab/import_export/all_models.yml
View file @ ed3034bb
......@@ -37,6 +37,7 @@ notes:
- events
- system_note_metadata
- note_diff_file
- suggestions
label_links:
- target
- label
......
spec/lib/gitlab/usage_data_spec.rb
View file @ ed3034bb
......@@ -117,6 +117,7 @@ describe Gitlab::UsageData do
releases
remote_mirrors
snippets
suggestions
todos
uploads
web_hooks
......
spec/models/diff_note_spec.rb
View file @ ed3034bb
......@@ -318,6 +318,24 @@ describe DiffNote do
end
end
describe '#supports_suggestion?' do
context 'when noteable does not support suggestions' do
it 'returns false' do
allow(subject.noteable).to receive(:supports_suggestion?) { false }
expect(subject.supports_suggestion?).to be(false)
end
end
context 'when line is not suggestible' do
it 'returns false' do
allow_any_instance_of(Gitlab::Diff::Line).to receive(:suggestible?) { false }
expect(subject.supports_suggestion?).to be(false)
end
end
end
describe "image diff notes" do
let(:path) { "files/images/any_image.png" }
......
spec/models/suggestion_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe Suggestion do
let(:suggestion) { create(:suggestion) }
describe 'associations' do
it { is_expected.to belong_to(:note) }
end
describe 'validations' do
it { is_expected.to validate_presence_of(:note) }
context 'when suggestion is applied' do
before do
allow(subject).to receive(:applied?).and_return(true)
end
it { is_expected.to validate_presence_of(:commit_id) }
end
end
describe '#appliable?' do
context 'when note does not support suggestions' do
it 'returns false' do
expect_next_instance_of(DiffNote) do |note|
allow(note).to receive(:supports_suggestion?) { false }
end
expect(suggestion).not_to be_appliable
end
end
context 'when patch is already applied' do
let(:suggestion) { create(:suggestion, :applied) }
it 'returns false' do
expect(suggestion).not_to be_appliable
end
end
context 'when merge request is not opened' do
let(:merge_request) { create(:merge_request, :merged) }
let(:note) do
create(:diff_note_on_merge_request, project: merge_request.project,
noteable: merge_request)
end
let(:suggestion) { create(:suggestion, note: note) }
it 'returns false' do
expect(suggestion).not_to be_appliable
end
end
end
end
spec/requests/api/suggestions_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe API::Suggestions do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:merge_request) do
create(:merge_request, source_project: project,
target_project: project)
end
let(:position) do
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 9,
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
describe "PUT /suggestions/:id/apply" do
let(:url) { "/suggestions/#{suggestion.id}/apply" }
context 'when successfully applies patch' do
let(:suggestion) do
create(:suggestion, note: diff_note,
from_content: " raise RuntimeError, \"System commands must be given as an array of strings\"\n",
to_content: " raise RuntimeError, 'Explosion'\n # explosion?")
end
it 'returns 200 with json content' do
project.add_maintainer(user)
put api(url, user), id: suggestion.id
expect(response).to have_gitlab_http_status(200)
expect(json_response)
.to include('id', 'from_original_line', 'to_original_line',
'from_line', 'to_line', 'appliable', 'applied',
'from_content', 'to_content')
end
end
context 'when not able to apply patch' do
let(:suggestion) do
create(:suggestion, :unappliable, note: diff_note)
end
it 'returns 400 with json content' do
project.add_maintainer(user)
put api(url, user), id: suggestion.id
expect(response).to have_gitlab_http_status(400)
expect(json_response).to eq({ 'message' => 'Suggestion is not appliable' })
end
end
context 'when unauthorized' do
let(:suggestion) do
create(:suggestion, note: diff_note,
from_content: " raise RuntimeError, \"System commands must be given as an array of strings\"\n",
to_content: " raise RuntimeError, 'Explosion'\n # explosion?")
end
it 'returns 403 with json content' do
project.add_reporter(user)
put api(url, user), id: suggestion.id
expect(response).to have_gitlab_http_status(403)
expect(json_response).to eq({ 'message' => '403 Forbidden' })
end
end
end
end
spec/serializers/suggestion_entity_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe SuggestionEntity do
include RepoHelpers
let(:user) { create(:user) }
let(:request) { double('request', current_user: user) }
let(:suggestion) { create(:suggestion) }
let(:entity) { described_class.new(suggestion, request: request) }
subject { entity.as_json }
it 'exposes correct attributes' do
expect(subject).to include(:id, :from_original_line, :to_original_line, :from_line,
:to_line, :appliable, :applied, :from_content, :to_content)
end
it 'exposes current user abilities' do
expect(subject[:current_user]).to include(:can_apply)
end
end
spec/services/notes/update_service_spec.rb
View file @ ed3034bb
......@@ -20,6 +20,29 @@ describe Notes::UpdateService do
@note.reload
end
context 'suggestions' do
it 'refreshes note suggestions' do
markdown = <<-MARKDOWN.strip_heredoc
```suggestion
foo
```
```suggestion
bar
```
MARKDOWN
suggestion = create(:suggestion)
note = suggestion.note
expect { described_class.new(project, user, note: markdown).execute(note) }
.to change { note.suggestions.count }.from(1).to(2)
expect(note.suggestions.order(:relative_order).map(&:to_content))
.to eq([" foo\n", " bar\n"])
end
end
context 'todos' do
let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) }
......
spec/services/preview_markdown_service_spec.rb
View file @ ed3034bb
......@@ -19,6 +19,31 @@ describe PreviewMarkdownService do
end
end
describe 'suggestions' do
let(:params) { { text: "```suggestion\nfoo\n```", preview_suggestions: preview_suggestions } }
let(:service) { described_class.new(project, user, params) }
context 'when preview markdown param is present' do
let(:preview_suggestions) { true }
it 'returns users referenced in text' do
result = service.execute
expect(result[:suggestions]).to eq(['foo'])
end
end
context 'when preview markdown param is not present' do
let(:preview_suggestions) { false }
it 'returns users referenced in text' do
result = service.execute
expect(result[:suggestions]).to eq([])
end
end
end
context 'new note with quick actions' do
let(:issue) { create(:issue, project: project) }
let(:params) do
......
spec/services/suggestions/apply_service_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe Suggestions::ApplyService do
include ProjectForksHelper
let(:project) { create(:project, :repository) }
let(:user) { create(:user, :commit_email) }
let(:position) do
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 9,
diff_refs: merge_request.diff_refs)
end
let(:suggestion) do
create(:suggestion, note: diff_note,
from_content: " raise RuntimeError, \"System commands must be given as an array of strings\"\n",
to_content: " raise RuntimeError, 'Explosion'\n # explosion?\n")
end
subject { described_class.new(user) }
context 'patch is appliable' do
let(:expected_content) do
<<-CONTENT.strip_heredoc
require 'fileutils'
require 'open3'
module Popen
extend self
def popen(cmd, path=nil)
unless cmd.is_a?(Array)
raise RuntimeError, 'Explosion'
# explosion?
end
path ||= Dir.pwd
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
context 'non-fork project' do
let(:merge_request) do
create(:merge_request, source_project: project,
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
it 'updates the file with the new contents' do
subject.execute(suggestion)
blob = project.repository.blob_at_branch(merge_request.source_branch,
position.new_path)
expect(blob.data).to eq(expected_content)
end
it 'returns success status' do
result = subject.execute(suggestion)
expect(result[:status]).to eq(:success)
end
it 'updates suggestion applied and commit_id columns' do
expect { subject.execute(suggestion) }
.to change(suggestion, :applied)
.from(false).to(true)
.and change(suggestion, :commit_id)
.from(nil)
end
it 'created commit has users email and name' do
subject.execute(suggestion)
commit = project.repository.commit
expect(user.commit_email).not_to eq(user.email)
expect(commit.author_email).to eq(user.commit_email)
expect(commit.committer_email).to eq(user.commit_email)
expect(commit.author_name).to eq(user.name)
end
end
context 'fork-project' do
let(:project) { create(:project, :public, :repository) }
let(:forked_project) do
fork_project_with_submodules(project, user)
end
let(:merge_request) do
create(:merge_request,
source_branch: 'conflict-resolvable-fork', source_project: forked_project,
target_branch: 'conflict-start', 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
it 'updates file in the source project' do
expect(Files::UpdateService).to receive(:new)
.with(merge_request.source_project, user, anything)
.and_call_original
subject.execute(suggestion)
end
end
end
context 'no permission' do
let(:merge_request) do
create(:merge_request, source_project: project,
target_project: project)
end
let(:diff_note) do
create(:diff_note_on_merge_request, noteable: merge_request,
position: position,
project: project)
end
context 'user cannot write in project repo' do
before do
project.add_reporter(user)
end
it 'returns error' do
result = subject.execute(suggestion)
expect(result).to eq(message: "You are not allowed to push into this branch",
status: :error)
end
end
end
context 'patch is not appliable' do
let(:merge_request) do
create(:merge_request, source_project: project,
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
context 'suggestion was already applied' do
it 'returns success status' do
result = subject.execute(suggestion)
expect(result[:status]).to eq(:success)
end
end
context 'note is outdated' do
before do
allow(diff_note).to receive(:active?) { false }
end
it 'returns error message' do
result = subject.execute(suggestion)
expect(result).to eq(message: 'Suggestion is not appliable',
status: :error)
end
end
context 'suggestion was already applied' do
before do
suggestion.update!(applied: true, commit_id: 'sha')
end
it 'returns error message' do
result = subject.execute(suggestion)
expect(result).to eq(message: 'Suggestion is not appliable',
status: :error)
end
end
end
end
spec/services/suggestions/create_service_spec.rb 0 → 100644
View file @ ed3034bb
# frozen_string_literal: true
require 'spec_helper'
describe Suggestions::CreateService do
let(:project_with_repo) { create(:project, :repository) }
let(:merge_request) do
create(:merge_request, source_project: project_with_repo,
target_project: project_with_repo)
end
let(:position) do
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 14,
diff_refs: merge_request.diff_refs)
end
let(:markdown) do
<<-MARKDOWN.strip_heredoc
```suggestion
foo
bar
```
```
nothing
```
```suggestion
xpto
baz
```
```thing
this is not a suggestion, it's a thing
```
MARKDOWN
end
subject { described_class.new(note) }
describe '#execute' do
context 'should not try to parse suggestions' do
context 'when not a diff note for merge requests' do
let(:note) do
create(:diff_note_on_commit, project: project_with_repo,
note: markdown)
end
it 'does not try to parse suggestions' do
expect(Banzai::SuggestionsParser).not_to receive(:parse)
subject.execute
end
end
context 'when diff note is not for text' do
let(:note) do
create(:diff_note_on_merge_request, project: project_with_repo,
noteable: merge_request,
position: position,
note: markdown)
end
it 'does not try to parse suggestions' do
allow(note).to receive(:on_text?) { false }
expect(Banzai::SuggestionsParser).not_to receive(:parse)
subject.execute
end
end
end
context 'should create suggestions' do
let(:note) do
create(:diff_note_on_merge_request, project: project_with_repo,
noteable: merge_request,
position: position,
note: markdown)
end
context 'single line suggestions' do
it 'persists suggestion records' do
expect { subject.execute }
.to change { note.suggestions.count }
.from(0)
.to(2)
end
it 'persists original from_content lines and suggested lines' do
subject.execute
suggestions = note.suggestions.order(:relative_order)
suggestion_1 = suggestions.first
suggestion_2 = suggestions.last
expect(suggestion_1).to have_attributes(from_content: " vars = {\n",
to_content: " foo\n bar\n")
expect(suggestion_2).to have_attributes(from_content: " vars = {\n",
to_content: " xpto\n baz\n")
end
end
end
end
end
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7