Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2023-06-07 21:08:30 +00:00
parent 9ee2305f46
commit e0d38e233d
63 changed files with 1402 additions and 225 deletions

View File

@ -15,11 +15,23 @@ export default {
type: String,
required: true,
},
showPin: {
type: Boolean,
required: false,
default: true,
},
positionType: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapGetters('batchComments', ['draftsForFile']),
drafts() {
return this.draftsForFile(this.fileHash);
return this.draftsForFile(this.fileHash).filter(
(f) => f.position?.position_type === this.positionType,
);
},
},
};
@ -34,6 +46,7 @@ export default {
>
<div class="notes">
<design-note-pin
v-if="showPin"
:label="toggleText(draft, index)"
is-draft
class="js-diff-notes-index gl-translate-x-n50"

View File

@ -2,6 +2,7 @@ import { isEmpty } from 'lodash';
import { createAlert } from '~/alert';
import { scrollToElement } from '~/lib/utils/common_utils';
import { __ } from '~/locale';
import { FILE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import { CHANGES_TAB, DISCUSSION_TAB, SHOW_TAB } from '../../../constants';
import service from '../../../services/drafts_service';
import * as types from './mutation_types';
@ -56,7 +57,9 @@ export const fetchDrafts = ({ commit, getters, state, dispatch }) =>
.then((data) => commit(types.SET_BATCH_COMMENTS_DRAFTS, data))
.then(() => {
state.drafts.forEach((draft) => {
if (!draft.line_code) {
if (draft.position?.position_type === FILE_DIFF_POSITION_TYPE) {
dispatch('diffs/addDraftToFile', { filePath: draft.file_path, draft }, { root: true });
} else if (!draft.line_code) {
dispatch('convertToDiscussion', draft.discussion_id, { root: true });
}
});

View File

@ -75,7 +75,10 @@ export default {
return this.getCommentFormForDiffFile(this.diffFileHash);
},
showNotesContainer() {
return this.imageDiscussions.length || this.diffFileCommentForm;
return (
this.diffViewerMode === diffViewerModes.image &&
(this.imageDiscussionsWithDrafts.length || this.diffFileCommentForm)
);
},
diffFileHash() {
return this.diffFile.file_hash;
@ -87,6 +90,11 @@ export default {
// TODO: Do this data generation when we receive a response to save a computed property being created
return this.diffLines(this.diffFile).map(mapParallel(this)) || [];
},
imageDiscussions() {
return this.diffFile.discussions.filter(
(f) => f.position?.position_type === IMAGE_DIFF_POSITION_TYPE,
);
},
},
updated() {
this.$nextTick(() => {
@ -111,6 +119,7 @@ export default {
});
},
},
IMAGE_DIFF_POSITION_TYPE,
};
</script>
@ -181,13 +190,17 @@ export default {
class="d-none d-sm-block new-comment"
/>
<diff-discussions
v-if="diffFile.discussions.length"
v-if="imageDiscussions.length"
class="diff-file-discussions"
:discussions="diffFile.discussions"
:discussions="imageDiscussions"
should-collapse-discussions
render-avatar-badge
/>
<diff-file-drafts :file-hash="diffFileHash" class="diff-file-discussions" />
<diff-file-drafts
:file-hash="diffFileHash"
:position-type="$options.IMAGE_DIFF_POSITION_TYPE"
class="diff-file-discussions"
/>
<note-form
v-if="diffFileCommentForm"
ref="noteForm"

View File

@ -12,6 +12,9 @@ import { scrollToElement } from '~/lib/utils/common_utils';
import { sprintf } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import notesEventHub from '~/notes/event_hub';
import DiffFileDrafts from '~/batch_comments/components/diff_file_drafts.vue';
import NoteForm from '~/notes/components/note_form.vue';
import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form';
import {
DIFF_FILE_AUTOMATIC_COLLAPSE,
@ -19,10 +22,12 @@ import {
EVT_EXPAND_ALL_FILES,
EVT_PERF_MARK_DIFF_FILES_END,
EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN,
FILE_DIFF_POSITION_TYPE,
} from '../constants';
import eventHub from '../event_hub';
import { DIFF_FILE, GENERIC_ERROR, CONFLICT_TEXT } from '../i18n';
import { collapsedType, getShortShaFromFile } from '../utils/diff_file';
import DiffDiscussions from './diff_discussions.vue';
import DiffFileHeader from './diff_file_header.vue';
export default {
@ -33,11 +38,18 @@ export default {
GlLoadingIcon,
GlSprintf,
GlAlert,
DiffFileDrafts,
NoteForm,
DiffDiscussions,
},
directives: {
SafeHtml,
},
mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.file.file_hash })],
mixins: [
glFeatureFlagsMixin(),
IdState({ idProp: (vm) => vm.file.file_hash }),
diffLineNoteFormMixin,
],
props: {
file: {
type: Object,
@ -101,7 +113,7 @@ export default {
'conflictResolutionPath',
'canMerge',
]),
...mapGetters(['isNotesFetched']),
...mapGetters(['isNotesFetched', 'getNoteableData', 'noteableType']),
...mapGetters('diffs', ['getDiffFileDiscussions', 'isVirtualScrollingEnabled']),
viewBlobHref() {
return escape(this.file.view_path);
@ -175,6 +187,18 @@ export default {
return this.file.viewer?.manuallyCollapsed;
},
fileDiscussions() {
return this.file.discussions.filter(
(f) => f.position?.position_type === FILE_DIFF_POSITION_TYPE,
);
},
showFileDiscussions() {
return (
this.glFeatures.commentOnFiles &&
!this.file.viewer?.manuallyCollapsed &&
(this.fileDiscussions.length || this.file.drafts.length || this.file.hasCommentForm)
);
},
},
watch: {
'file.id': {
@ -227,6 +251,8 @@ export default {
'assignDiscussionsToDiff',
'setRenderIt',
'setFileCollapsedByUser',
'saveDiffDiscussion',
'toggleFileCommentForm',
]),
manageViewedEffects() {
if (
@ -316,8 +342,20 @@ export default {
hideForkMessage() {
this.idState.forkMessageVisible = false;
},
handleSaveNote(note) {
this.saveDiffDiscussion({
note,
formData: {
noteableData: this.getNoteableData,
noteableType: this.noteableType,
diffFile: this.file,
positionType: FILE_DIFF_POSITION_TYPE,
},
});
},
},
CONFLICT_TEXT,
FILE_DIFF_POSITION_TYPE,
};
</script>
@ -422,6 +460,35 @@ export default {
</template>
</gl-sprintf>
</gl-alert>
<div v-if="showFileDiscussions" class="gl-border-b" data-testid="file-discussions">
<div class="diff-file-discussions-wrapper">
<diff-file-drafts
:file-hash="file.file_hash"
:show-pin="false"
:position-type="$options.FILE_DIFF_POSITION_TYPE"
class="diff-file-discussions"
/>
<diff-discussions
v-if="fileDiscussions.length"
class="diff-file-discussions"
data-testid="diff-file-discussions"
:discussions="fileDiscussions"
/>
<note-form
v-if="file.hasCommentForm"
:save-button-title="__('Comment')"
:diff-file="file"
autofocus
class="gl-py-3 gl-px-5"
data-testid="file-note-form"
@handleFormUpdate="handleSaveNote"
@handleFormUpdateAddToReview="
(note) => addToReview(note, $options.FILE_DIFF_POSITION_TYPE)
"
@cancelForm="toggleFileCommentForm(file.file_path)"
/>
</div>
</div>
<gl-loading-icon
v-if="showLoadingIcon"
size="sm"

View File

@ -115,6 +115,7 @@ export default {
computed: {
...mapState('diffs', ['latestDiff']),
...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']),
...mapGetters(['getNoteableData']),
diffContentIDSelector() {
return `#diff-content-${this.diffFile.file_hash}`;
},
@ -210,6 +211,9 @@ export default {
labelToggleFile() {
return this.expanded ? __('Hide file contents') : __('Show file contents');
},
showCommentButton() {
return this.getNoteableData.current_user.can_create_note && this.glFeatures.commentOnFiles;
},
},
watch: {
'idState.moreActionsShown': {
@ -233,6 +237,7 @@ export default {
'reviewFile',
'setFileCollapsedByUser',
'setGenerateTestFilePath',
'toggleFileCommentForm',
]),
handleToggleFile() {
this.$emit('toggleFile');
@ -389,6 +394,18 @@ export default {
>
{{ $options.i18n.fileReviewLabel }}
</gl-form-checkbox>
<gl-button
v-if="showCommentButton"
v-gl-tooltip.hover
:title="__('Comment on this file')"
:aria-label="__('Comment on this file')"
icon="comment"
category="tertiary"
size="small"
class="gl-mr-3 btn-icon"
data-testid="comment-files-button"
@click="toggleFileCommentForm(diffFile.file_path)"
/>
<gl-button-group class="gl-pt-0!">
<gl-button
v-if="diffFile.external_url"

View File

@ -12,6 +12,7 @@ export const NEW_LINE_TYPE = 'new';
export const OLD_LINE_TYPE = 'old';
export const TEXT_DIFF_POSITION_TYPE = 'text';
export const IMAGE_DIFF_POSITION_TYPE = 'image';
export const FILE_DIFF_POSITION_TYPE = 'file';
export const LINE_POSITION_LEFT = 'left';
export const LINE_POSITION_RIGHT = 'right';

View File

@ -1,4 +1,5 @@
import { mapGetters } from 'vuex';
import { IMAGE_DIFF_POSITION_TYPE } from '../constants';
export default {
computed: {
@ -10,8 +11,10 @@ export default {
'hasParallelDraftLeft',
'hasParallelDraftRight',
]),
imageDiscussions() {
return this.diffFile.discussions.concat(this.draftsForFile(this.diffFile.file_hash));
imageDiscussionsWithDrafts() {
return this.diffFile.discussions
.filter((f) => f.position?.position_type === IMAGE_DIFF_POSITION_TYPE)
.concat(this.draftsForFile(this.diffFile.file_hash));
},
},
};

View File

@ -50,6 +50,7 @@ import {
TRACKING_SINGLE_FILE_MODE,
TRACKING_MULTIPLE_FILES_MODE,
EVT_MR_PREPARED,
FILE_DIFF_POSITION_TYPE,
} from '../constants';
import { DISCUSSION_SINGLE_DIFF_FAILED, LOAD_SINGLE_DIFF_FAILED } from '../i18n';
import eventHub from '../event_hub';
@ -614,6 +615,11 @@ export const saveDiffDiscussion = async ({ state, dispatch }, { note, formData }
.then((discussion) => dispatch('assignDiscussionsToDiff', [discussion]))
.then(() => dispatch('updateResolvableDiscussionsCounts', null, { root: true }))
.then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.file_hash))
.then(() => {
if (formData.positionType === FILE_DIFF_POSITION_TYPE) {
dispatch('toggleFileCommentForm', formData.diffFile.file_path);
}
})
.catch(() =>
createAlert({
message: s__('MergeRequests|Saving the comment failed'),
@ -1012,3 +1018,9 @@ export function reviewFile({ commit, state }, { file, reviewed = true }) {
}
export const disableVirtualScroller = ({ commit }) => commit(types.DISABLE_VIRTUAL_SCROLLING);
export const toggleFileCommentForm = ({ commit }, filePath) =>
commit(types.TOGGLE_FILE_COMMENT_FORM, filePath);
export const addDraftToFile = ({ commit }, { filePath, draft }) =>
commit(types.ADD_DRAFT_TO_FILE, { filePath, draft });

View File

@ -63,9 +63,12 @@ export const diffHasAllCollapsedDiscussions = (state, getters) => (diff) => {
* @returns {Boolean}
*/
export const diffHasExpandedDiscussions = () => (diff) => {
return diff[INLINE_DIFF_LINES_KEY].filter((l) => l.discussions.length >= 1).some(
(l) => l.discussionsExpanded,
);
const diffLineDiscussionsExpanded = diff[INLINE_DIFF_LINES_KEY].filter(
(l) => l.discussions.length >= 1,
).some((l) => l.discussionsExpanded);
const diffFileDiscussionsExpanded = diff.discussions?.some((d) => d.expanded);
return diffFileDiscussionsExpanded || diffLineDiscussionsExpanded;
};
/**
@ -74,7 +77,10 @@ export const diffHasExpandedDiscussions = () => (diff) => {
* @returns {Boolean}
*/
export const diffHasDiscussions = () => (diff) => {
return diff[INLINE_DIFF_LINES_KEY].some((l) => l.discussions.length >= 1);
return (
diff.discussions?.length >= 1 ||
diff[INLINE_DIFF_LINES_KEY].some((l) => l.discussions.length >= 1)
);
};
/**

View File

@ -49,3 +49,6 @@ export const SET_SHOW_SUGGEST_POPOVER = 'SET_SHOW_SUGGEST_POPOVER';
export const TOGGLE_LINE_DISCUSSIONS = 'TOGGLE_LINE_DISCUSSIONS';
export const DISABLE_VIRTUAL_SCROLLING = 'DISABLE_VIRTUAL_SCROLLING';
export const TOGGLE_FILE_COMMENT_FORM = 'TOGGLE_FILE_COMMENT_FORM';
export const ADD_DRAFT_TO_FILE = 'ADD_DRAFT_TO_FILE';

View File

@ -5,6 +5,7 @@ import {
DIFF_FILE_AUTOMATIC_COLLAPSE,
INLINE_DIFF_LINES_KEY,
EXPANDED_LINE_TYPE,
FILE_DIFF_POSITION_TYPE,
} from '../constants';
import * as types from './mutation_types';
import {
@ -168,6 +169,7 @@ export default {
const { latestDiff } = state;
const originalStartLineCode = discussion.original_position?.line_range?.start?.line_code;
const positionType = discussion.position?.position_type;
const discussionLineCodes = [
discussion.line_code,
originalStartLineCode,
@ -212,16 +214,7 @@ export default {
state.diffFiles.forEach((file) => {
if (file.file_hash === fileHash) {
if (file[INLINE_DIFF_LINES_KEY].length) {
file[INLINE_DIFF_LINES_KEY].forEach((line) => {
Object.assign(
line,
setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line),
);
});
}
if (!file[INLINE_DIFF_LINES_KEY].length) {
if (positionType === FILE_DIFF_POSITION_TYPE) {
const newDiscussions = (file.discussions || [])
.filter((d) => d.id !== discussion.id)
.concat(discussion);
@ -229,6 +222,25 @@ export default {
Object.assign(file, {
discussions: newDiscussions,
});
} else {
if (file[INLINE_DIFF_LINES_KEY].length) {
file[INLINE_DIFF_LINES_KEY].forEach((line) => {
Object.assign(
line,
setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line),
);
});
}
if (!file[INLINE_DIFF_LINES_KEY].length) {
const newDiscussions = (file.discussions || [])
.filter((d) => d.id !== discussion.id)
.concat(discussion);
Object.assign(file, {
discussions: newDiscussions,
});
}
}
}
});
@ -378,4 +390,14 @@ export default {
[types.DISABLE_VIRTUAL_SCROLLING](state) {
state.disableVirtualScroller = true;
},
[types.TOGGLE_FILE_COMMENT_FORM](state, filePath) {
const file = findDiffFile(state.diffFiles, filePath, 'file_path');
file.hasCommentForm = !file.hasCommentForm;
},
[types.ADD_DRAFT_TO_FILE](state, { filePath, draft }) {
const file = findDiffFile(state.diffFiles, filePath, 'file_path');
file?.drafts.push(draft);
},
};

View File

@ -53,6 +53,9 @@ export const isNotDiffable = (file) => file?.viewer?.name === viewerModes.not_di
export function prepareRawDiffFile({ file, allFiles, meta = false, index = -1 }) {
const additionalProperties = {
brokenSymlink: fileSymlinkInformation(file, allFiles),
hasCommentForm: false,
discussions: file.discussions || [],
drafts: [],
viewer: {
...file.viewer,
...collapsed(file),

View File

@ -5,6 +5,7 @@ import { mapActions } from 'vuex';
import SafeHtml from '~/vue_shared/directives/safe_html';
import { truncateSha } from '~/lib/utils/text_utility';
import { s__, __, sprintf } from '~/locale';
import { FILE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import NoteEditedText from './note_edited_text.vue';
import NoteHeader from './note_header.vue';
@ -62,6 +63,7 @@ export default {
for_commit: isForCommit,
diff_discussion: isDiffDiscussion,
active: isActive,
position,
} = this.discussion;
let text = s__('MergeRequests|started a thread');
@ -75,6 +77,10 @@ export default {
: s__(
'MergeRequests|started a thread on an outdated change in commit %{linkStart}%{commitDisplay}%{linkEnd}',
);
} else if (isDiffDiscussion && position?.position_type === FILE_DIFF_POSITION_TYPE) {
text = isActive
? s__('MergeRequests|started a thread on %{linkStart}a file%{linkEnd}')
: s__('MergeRequests|started a thread on %{linkStart}an old version of a file%{linkEnd}');
} else if (isDiffDiscussion) {
text = isActive
? s__('MergeRequests|started a thread on %{linkStart}the diff%{linkEnd}')

View File

@ -8,6 +8,7 @@ import { getDiffMode } from '~/diffs/store/utils';
import { diffViewerModes } from '~/ide/constants';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
import { isCollapsed } from '~/diffs/utils/diff_file';
import { FILE_DIFF_POSITION_TYPE } from '~/diffs/constants';
const FIRST_CHAR_REGEX = /^(\+|-| )/;
@ -53,6 +54,12 @@ export default {
isCollapsed() {
return isCollapsed(this.discussion.diff_file);
},
positionType() {
return this.discussion.position?.position_type;
},
isFileDiscussion() {
return this.positionType === FILE_DIFF_POSITION_TYPE;
},
},
mounted() {
if (this.isTextFile && !this.hasTruncatedDiffLines) {
@ -87,43 +94,52 @@ export default {
/>
<div v-if="isTextFile" class="diff-content">
<table class="code js-syntax-highlight" :class="$options.userColorSchemeClass">
<template v-if="hasTruncatedDiffLines">
<tr
v-for="line in discussion.truncated_diff_lines"
v-once
:key="line.line_code"
class="line_holder"
>
<td :class="line.type" class="diff-line-num old_line">{{ line.old_line }}</td>
<td :class="line.type" class="diff-line-num new_line">{{ line.new_line }}</td>
<td v-safe-html="trimChar(line.rich_text)" :class="line.type" class="line_content"></td>
<template v-if="!isFileDiscussion">
<template v-if="hasTruncatedDiffLines">
<tr
v-for="line in discussion.truncated_diff_lines"
v-once
:key="line.line_code"
class="line_holder"
>
<td :class="line.type" class="diff-line-num old_line">{{ line.old_line }}</td>
<td :class="line.type" class="diff-line-num new_line">{{ line.new_line }}</td>
<td
v-safe-html="trimChar(line.rich_text)"
:class="line.type"
class="line_content"
></td>
</tr>
</template>
<tr v-if="!hasTruncatedDiffLines" class="line_holder line-holder-placeholder">
<td class="old_line diff-line-num"></td>
<td class="new_line diff-line-num"></td>
<td v-if="error" class="js-error-lazy-load-diff diff-loading-error-block">
{{ __('Unable to load the diff') }}
<button
class="gl-button btn-link btn-link-retry gl-p-0 js-toggle-lazy-diff-retry-button gl-reset-font-size!"
@click="fetchDiff"
>
{{ __('Try again') }}
</button>
</td>
<td v-else class="line_content js-success-lazy-load">
<span></span>
<gl-skeleton-loader />
<span></span>
</td>
</tr>
</template>
<tr v-if="!hasTruncatedDiffLines" class="line_holder line-holder-placeholder">
<td class="old_line diff-line-num"></td>
<td class="new_line diff-line-num"></td>
<td v-if="error" class="js-error-lazy-load-diff diff-loading-error-block">
{{ __('Unable to load the diff') }}
<button
class="gl-button btn-link btn-link-retry gl-p-0 js-toggle-lazy-diff-retry-button gl-reset-font-size!"
@click="fetchDiff"
>
{{ __('Try again') }}
</button>
</td>
<td v-else class="line_content js-success-lazy-load">
<span></span>
<gl-skeleton-loader />
<span></span>
</td>
</tr>
<tr class="notes_holder">
<td class="notes-content" colspan="3"><slot></slot></td>
<td :class="{ 'gl-border-top-0!': isFileDiscussion }" class="notes-content" colspan="3">
<slot></slot>
</td>
</tr>
</table>
</div>
<div v-else>
<div v-else class="diff-content">
<diff-viewer
v-if="!isFileDiscussion"
:diff-file="discussion.diff_file"
:diff-mode="diffMode"
:diff-viewer-mode="diffViewerMode"

View File

@ -4,6 +4,7 @@ import { __ } from '~/locale';
import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue';
import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue';
import SystemNote from '~/vue_shared/components/notes/system_note.vue';
import { FILE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import { SYSTEM_NOTE } from '../constants';
import DiscussionNotesRepliesWrapper from './discussion_notes_replies_wrapper.vue';
import NoteEditedText from './note_edited_text.vue';
@ -85,6 +86,9 @@ export default {
isDiscussionInternal() {
return this.discussion.notes[0]?.internal;
},
isFileDiscussion() {
return this.discussion.position?.position_type === FILE_DIFF_POSITION_TYPE;
},
},
methods: {
...mapActions(['toggleDiscussion', 'setSelectedCommentPositionHover']),
@ -143,6 +147,7 @@ export default {
:is-overview-tab="isOverviewTab"
:should-scroll-to-note="shouldScrollToNote"
:internal-note="isDiscussionInternal"
:class="{ 'gl-border-top-0!': isFileDiscussion }"
@handleDeleteNote="$emit('deleteNote')"
@startReplying="$emit('startReplying')"
>

View File

@ -170,6 +170,7 @@ export default {
return {
'is-replying gl-pt-0!': this.isReplying,
'internal-note': this.isDiscussionInternal,
'gl-pt-0!': !this.discussion.diff_discussion && this.isReplying,
};
},
},

View File

@ -1,6 +1,10 @@
import { mapActions, mapGetters, mapState } from 'vuex';
import { getDraftReplyFormData, getDraftFormData } from '~/batch_comments/utils';
import { TEXT_DIFF_POSITION_TYPE, IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import {
TEXT_DIFF_POSITION_TYPE,
IMAGE_DIFF_POSITION_TYPE,
FILE_DIFF_POSITION_TYPE,
} from '~/diffs/constants';
import { createAlert } from '~/alert';
import { clearDraft } from '~/lib/utils/autosave';
import { s__ } from '~/locale';
@ -18,7 +22,7 @@ export default {
...mapState('diffs', ['commit', 'showWhitespace']),
},
methods: {
...mapActions('diffs', ['cancelCommentForm']),
...mapActions('diffs', ['cancelCommentForm', 'toggleFileCommentForm']),
...mapActions('batchComments', ['addDraftToReview', 'saveDraft', 'insertDraftIntoDrafts']),
addReplyToReview(noteText, isResolving) {
const postData = getDraftReplyFormData({
@ -47,13 +51,13 @@ export default {
});
});
},
addToReview(note) {
addToReview(note, positionType = null) {
const lineRange =
(this.line && this.commentLineStart && formatLineRange(this.commentLineStart, this.line)) ||
{};
const positionType = this.diffFileCommentForm
? IMAGE_DIFF_POSITION_TYPE
: TEXT_DIFF_POSITION_TYPE;
const position =
positionType ||
(this.diffFileCommentForm ? IMAGE_DIFF_POSITION_TYPE : TEXT_DIFF_POSITION_TYPE);
const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash);
const postData = getDraftFormData({
note,
@ -64,7 +68,7 @@ export default {
diffViewType: this.diffViewType,
diffFile: selectedDiffFile,
linePosition: this.position,
positionType,
positionType: position,
...this.diffFileCommentForm,
lineRange,
showWhitespace: this.showWhitespace,
@ -76,10 +80,12 @@ export default {
return this.saveDraft(postData)
.then(() => {
if (positionType === IMAGE_DIFF_POSITION_TYPE) {
if (position === IMAGE_DIFF_POSITION_TYPE) {
this.closeDiffFileCommentForm(this.diffFileHash);
} else {
} else if (this.line?.line_code) {
this.handleClearForm(this.line.line_code);
} else if (position === FILE_DIFF_POSITION_TYPE) {
this.toggleFileCommentForm(this.diffFile.file_path);
}
})
.catch(() => {

View File

@ -1271,3 +1271,32 @@ $tabs-holder-z-index: 250;
margin-right: 8px;
border: 2px solid var(--gray-50, $gray-50);
}
.diff-file-discussions-wrapper {
@include gl-w-full;
max-width: 800px;
.diff-discussions > .notes {
@include gl-p-5;
}
.diff-discussions:not(:first-child) >.notes {
@include gl-pt-0;
}
.note-discussion {
@include gl-rounded-base;
border: 1px solid var(--gray-100, $gray-100) !important;
}
.discussion-collapsible {
@include gl-m-0;
@include gl-border-l-0;
@include gl-border-r-0;
@include gl-border-b-0;
@include gl-rounded-top-left-none;
@include gl-rounded-top-right-none;
}
}

View File

@ -667,7 +667,7 @@ $system-note-icon-m-left: $avatar-m-left + $icon-size-diff / $avatar-m-ratio;
.discussion-reply-holder {
border-top: 0;
border-radius: 0 0 $gl-border-radius-base $gl-border-radius-base;
border-radius: $gl-border-radius-base $gl-border-radius-base;
position: relative;
.discussion-form {

View File

@ -53,6 +53,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_force_frontend_feature_flag(:summarize_my_code_review, summarize_my_code_review_enabled?)
push_frontend_feature_flag(:mr_activity_filters, current_user)
push_frontend_feature_flag(:review_apps_redeploy_mr_widget, project)
push_frontend_feature_flag(:comment_on_files, current_user)
end
around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :diffs, :discussions]

View File

@ -3,6 +3,7 @@ module Ci
module SecureFilesHelper
def show_secure_files_setting(project, user)
return false if user.nil?
return false unless Gitlab.config.ci_secure_files.enabled
user.can?(:read_secure_files, project)
end

View File

@ -4,7 +4,7 @@ module DiffPositionableNote
included do
before_validation :set_original_position, on: :create
before_validation :update_position, on: :create, if: :on_text?, unless: :importing?
before_validation :update_position, on: :create, if: :should_update_position?, unless: :importing?
serialize :original_position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
serialize :position, Gitlab::Diff::Position # rubocop:disable Cop/ActiveRecordSerialize
@ -37,10 +37,18 @@ module DiffPositionableNote
end
end
def should_update_position?
on_text? || on_file?
end
def on_text?
!!position&.on_text?
end
def on_file?
!!position&.on_file?
end
def on_image?
!!position&.on_image?
end

View File

@ -689,6 +689,7 @@ class Note < ApplicationRecord
def show_outdated_changes?
return false unless for_merge_request?
return false unless system?
return false if change_position&.on_file?
return false unless change_position&.line_range
change_position.line_range["end"] || change_position.line_range["start"]

View File

@ -0,0 +1,8 @@
---
name: comment_on_files
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121429
rollout_issue_url:
milestone: '16.1'
type: development
group: group::code review
default_enabled: false

View File

@ -2,7 +2,7 @@
# Set default values for object_store settings
class ObjectStoreSettings
SUPPORTED_TYPES = %w(artifacts external_diffs lfs uploads packages dependency_proxy terraform_state pages ci_secure_files).freeze
SUPPORTED_TYPES = %w(artifacts external_diffs lfs uploads packages dependency_proxy terraform_state pages).freeze
ALLOWED_OBJECT_STORE_OVERRIDES = %w(bucket enabled proxy_download cdn).freeze
# To ensure the one Workhorse credential matches the Rails config, we

View File

@ -58,7 +58,7 @@ class Gitlab::Seeder::Achievements
end
Gitlab::Seeder.quiet do
puts "\nGenerating ahievements"
puts "\nGenerating achievements"
group = Group.first
users = User.first(4).pluck(:id)

View File

@ -0,0 +1,21 @@
# frozen_string_literal: true
class AddIdColumnToPmCheckpoints < Gitlab::Database::Migration[2.1]
enable_lock_retries!
def up
add_column(:pm_checkpoints, :id, :bigserial)
add_index(:pm_checkpoints, :id, unique: true, name: :pm_checkpoints_unique_index) # rubocop:disable Migration/AddIndex
add_index(:pm_checkpoints, [:purl_type, :data_type, :version_format], unique: true, # rubocop:disable Migration/AddIndex
name: :pm_checkpoints_path_components)
swap_primary_key(:pm_checkpoints, :pm_checkpoints_pkey, :pm_checkpoints_unique_index)
end
def down
add_index(:pm_checkpoints, [:purl_type, :data_type, :version_format], unique: true, # rubocop:disable Migration/AddIndex
name: :pm_checkpoints_unique_index)
remove_index(:pm_checkpoints, name: :pm_checkpoints_path_components) # rubocop:disable Migration/RemoveIndex
unswap_primary_key(:pm_checkpoints, :pm_checkpoints_pkey, :pm_checkpoints_unique_index)
remove_column(:pm_checkpoints, :id)
end
end

View File

@ -0,0 +1 @@
5b2b9a7c58ed2153a70e4f32dbd8a4c4c9976afce478d761ced2ae9de43ea377

View File

@ -20311,9 +20311,19 @@ CREATE TABLE pm_checkpoints (
purl_type smallint NOT NULL,
chunk smallint NOT NULL,
data_type smallint DEFAULT 1 NOT NULL,
version_format smallint DEFAULT 1 NOT NULL
version_format smallint DEFAULT 1 NOT NULL,
id bigint NOT NULL
);
CREATE SEQUENCE pm_checkpoints_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE pm_checkpoints_id_seq OWNED BY pm_checkpoints.id;
CREATE TABLE pm_licenses (
id bigint NOT NULL,
spdx_identifier text NOT NULL,
@ -25668,6 +25678,8 @@ ALTER TABLE ONLY pm_advisories ALTER COLUMN id SET DEFAULT nextval('pm_advisorie
ALTER TABLE ONLY pm_affected_packages ALTER COLUMN id SET DEFAULT nextval('pm_affected_packages_id_seq'::regclass);
ALTER TABLE ONLY pm_checkpoints ALTER COLUMN id SET DEFAULT nextval('pm_checkpoints_id_seq'::regclass);
ALTER TABLE ONLY pm_licenses ALTER COLUMN id SET DEFAULT nextval('pm_licenses_id_seq'::regclass);
ALTER TABLE ONLY pm_package_version_licenses ALTER COLUMN id SET DEFAULT nextval('pm_package_version_licenses_id_seq'::regclass);
@ -27977,7 +27989,7 @@ ALTER TABLE ONLY pm_affected_packages
ADD CONSTRAINT pm_affected_packages_pkey PRIMARY KEY (id);
ALTER TABLE ONLY pm_checkpoints
ADD CONSTRAINT pm_checkpoints_pkey PRIMARY KEY (purl_type, data_type, version_format);
ADD CONSTRAINT pm_checkpoints_pkey PRIMARY KEY (id);
ALTER TABLE ONLY pm_licenses
ADD CONSTRAINT pm_licenses_pkey PRIMARY KEY (id);
@ -33506,6 +33518,8 @@ CREATE UNIQUE INDEX partial_index_sop_configs_on_project_id ON security_orchestr
CREATE INDEX partial_index_user_id_app_id_created_at_token_not_revoked ON oauth_access_tokens USING btree (resource_owner_id, application_id, created_at) WHERE (revoked_at IS NULL);
CREATE UNIQUE INDEX pm_checkpoints_path_components ON pm_checkpoints USING btree (purl_type, data_type, version_format);
CREATE INDEX scan_finding_approval_mr_rule_index_id ON approval_merge_request_rules USING btree (id) WHERE (report_type = 4);
CREATE INDEX scan_finding_approval_mr_rule_index_merge_request_id ON approval_merge_request_rules USING btree (merge_request_id) WHERE (report_type = 4);

View File

@ -0,0 +1,58 @@
---
stage: Deploy
group: Environments
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
type: reference
---
# Configure Kubernetes deployments (deprecated)
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27630) in GitLab 12.6.
> - [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5.
WARNING:
This feature was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5.
If you are deploying to a [Kubernetes cluster](../../user/infrastructure/clusters/index.md)
associated with your project, you can configure these deployments from your
`.gitlab-ci.yml` file.
NOTE:
Kubernetes configuration isn't supported for Kubernetes clusters
[managed by GitLab](../../user/project/clusters/gitlab_managed_clusters.md).
The following configuration options are supported:
- [`namespace`](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/)
In the following example, the job deploys your application to the
`production` Kubernetes namespace.
```yaml
deploy:
stage: deploy
script:
- echo "Deploy to production server"
environment:
name: production
url: https://example.com
kubernetes:
namespace: production
rules:
- if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH
```
When you use the GitLab Kubernetes integration to deploy to a Kubernetes cluster,
you can view cluster and namespace information. On the deployment
job page, it's displayed above the job trace:
![Deployment cluster information](../img/environments_deployment_cluster_v12_8.png)
## Configure incremental rollouts
Learn how to release production changes to only a portion of your Kubernetes pods with
[incremental rollouts](../environments/incremental_rollouts.md).
## Related topics
- [Deploy boards (deprecated)](../../user/project/deploy_boards.md)

View File

@ -300,54 +300,6 @@ The `when: manual` action:
You can find the play button in the pipelines, environments, deployments, and jobs views.
## Configure Kubernetes deployments (deprecated)
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27630) in GitLab 12.6.
> - [Deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5.
WARNING:
This feature was [deprecated](https://gitlab.com/groups/gitlab-org/configure/-/epics/8) in GitLab 14.5.
If you are deploying to a [Kubernetes cluster](../../user/infrastructure/clusters/index.md)
associated with your project, you can configure these deployments from your
`.gitlab-ci.yml` file.
NOTE:
Kubernetes configuration isn't supported for Kubernetes clusters
[managed by GitLab](../../user/project/clusters/gitlab_managed_clusters.md).
The following configuration options are supported:
- [`namespace`](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/)
In the following example, the job deploys your application to the
`production` Kubernetes namespace.
```yaml
deploy:
stage: deploy
script:
- echo "Deploy to production server"
environment:
name: production
url: https://example.com
kubernetes:
namespace: production
rules:
- if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH
```
When you use the GitLab Kubernetes integration to deploy to a Kubernetes cluster,
you can view cluster and namespace information. On the deployment
job page, it's displayed above the job trace:
![Deployment cluster information](../img/environments_deployment_cluster_v12_8.png)
### Configure incremental rollouts
Learn how to release production changes to only a portion of your Kubernetes pods with
[incremental rollouts](../environments/incremental_rollouts.md).
## Track newly included merge requests per deployment
GitLab can track newly included merge requests per deployment.
@ -971,14 +923,14 @@ the `review/feature-1` spec takes precedence over `review/*` and `*` specs.
## Related topics
- [Use GitLab CI to deploy to multiple environments (blog post)](https://about.gitlab.com/blog/2021/02/05/ci-deployment-and-environments/)
- [Review Apps](../review_apps/index.md): Use dynamic environments to deploy your code for every branch.
- [Deploy boards](../../user/project/deploy_boards.md): View the status of your applications running on Kubernetes.
- [Protected environments](protected_environments.md): Determine who can deploy code to your environments.
- [Environments Dashboard](../environments/environments_dashboard.md): View a summary of each
environment's operational health. **(PREMIUM)**
- [Deployment safety](deployment_safety.md#restrict-write-access-to-a-critical-environment): Secure your deployments.
- [Track deployments of an external deployment tool](external_deployment_tools.md): Use an external deployment tool instead of built-in deployment solution.
- [Kubernetes dashboard](kubernetes_dashboard.md)
- [Deploy to multiple environments with GitLab CI/CD (blog post)](https://about.gitlab.com/blog/2021/02/05/ci-deployment-and-environments/)
- [Review Apps](../review_apps/index.md)
- [Protected environments](protected_environments.md)
- [Environments Dashboard](../environments/environments_dashboard.md)
- [Deployment safety](deployment_safety.md#restrict-write-access-to-a-critical-environment)
- [Track deployments of an external deployment tool](external_deployment_tools.md)
- [Configure Kubernetes deployments (deprecated)](configure_kubernetes_deployments.md)
## Troubleshooting

View File

@ -0,0 +1,69 @@
---
stage: Deploy
group: Environments
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
type: reference
---
# Kubernetes Dashboard **(FREE)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/390769) in GitLab 16.1, with [flags](../../administration/feature_flags.md) named `environment_settings_to_graphql`, `kas_user_access`, `kas_user_access_project`, and `expose_authorized_cluster_agents`. Disabled by default.
FLAG:
On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flags](../../administration/feature_flags.md) named `environment_settings_to_graphql`, `kas_user_access`, `kas_user_access_project`, and `expose_authorized_cluster_agents`.
Use the Kubernetes Dashboard to understand the status of your clusters with an intuitive visual interface.
The Kubernetes Dashboard works with every connected Kubernetes cluster, whether you deployed them
with CI/CD or GitOps.
For Flux users, the synchronization status of a given environment is not displayed in the dashboard.
[Issue 391581](https://gitlab.com/gitlab-org/gitlab/-/issues/391581) proposes to add this functionality.
## Configure the Kubernetes Dashboard
Configure a Kubernetes Dashboard to use it for a given environment.
You can configure dashboard for an environment that already exists, or
add one when you create an environment.
Prerequisite:
- The agent for Kubernetes must be shared with the environment's project, or its parent group, using the [`user_access`](../../user/clusters/agent/user_access.md) keyword.
### The environment already exists
1. On the top bar, select **Main menu > Projects** and find your project.
1. On the left sidebar, select **Deployments > Environments**.
1. Select the environment to be associated with the Kubernetes.
1. Select **Edit**.
1. Select a GitLab agent for Kubernetes.
1. Select **Save**.
### The environment doesn't exist
1. On the top bar, select **Main menu > Projects** and find your project.
1. On the left sidebar, select **Deployments > Environments**.
1. Select **New environment**.
1. Complete the **Name** field.
1. Select a GitLab agent for Kubernetes.
1. Select **Save**.
## View a dashboard
To view a configured Kubernetes Dashboard:
1. On the top bar, select **Main menu > Projects** and find your project.
1. On the left sidebar, select **Deployments > Environments**.
1. Expand the environment associated with GitLab agent for Kubernetes.
1. Expand **Kubernetes overview**.
## Troubleshooting
When working with the Kubernetes Dashboard, you might encounter the following issues.
### User cannot list resource in API group
You might get an error that states `Error: services is forbidden: User "gitlab:user:<user-name>" cannot list resource "<resource-name>" in API group "" at the cluster scope`.
This error happens when a user is not allowed to do the specified operation in the [Kubernetes RBAC](https://kubernetes.io/docs/reference/access-authn-authz/rbac/).
To resolve, check your [RBAC configuration](../../user/clusters/agent/user_access.md#configure-kubernetes-access). If the RBAC is properly configured, contact your Kubernetes administrator.

View File

@ -1850,7 +1850,7 @@ environment, using the `production`
**Related topics**:
- [Available settings for `kubernetes`](../environments/index.md#configure-kubernetes-deployments-deprecated).
- [Available settings for `kubernetes`](../environments/configure_kubernetes_deployments.md).
#### `environment:deployment_tier`

View File

@ -13,62 +13,48 @@ eventually.
## What are the potential cause for a test to be flaky?
### Unclean environment
### State leak
**Label:** `flaky-test::unclean environment`
**Label:** `flaky-test::state leak`
**Description:** The environment got dirtied by a previous test. The actual cause is probably not the flaky test here.
**Description:** Data state has leaked from a previous test. The actual cause is probably not the flaky test here.
**Difficulty to reproduce:** Moderate. Usually, running the same spec files until the one that's failing reproduces the problem.
**Resolution:** Fix the previous tests and/or places where the environment is modified, so that
**Resolution:** Fix the previous tests and/or places where the test data or environment is modified, so that
it's reset to a pristine test after each test.
**Examples:**
- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/378414#note_1142026988): A migration
- [Example 1](https://gitlab.com/gitlab-org/gitlab/-/issues/402915): State leakage can result from
data records created with `let_it_be` shared between test examples, while some test modifies the model
either deliberately or unwillingly causing out-of-sync data in test examples. This can result in `PG::QueryCanceled: ERROR` in the subsequent test examples or retries.
For more information about state leakages and resolution options, see [GitLab testing best practices](best_practices.md#lets-talk-about-let).
- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/issues/378414#note_1142026988): A migration
test might roll-back the database, perform its testing, and then roll-up the database in an
inconsistent state, so that following tests might not know about certain columns.
- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/issues/368500): A test modifies data that is
- [Example 3](https://gitlab.com/gitlab-org/gitlab/-/issues/368500): A test modifies data that is
used by a following test.
- [Example 3](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103434#note_1172316521): A test for a database query passes in a fresh database, but in a
- [Example 4](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/103434#note_1172316521): A test for a database query passes in a fresh database, but in a
CI/CD pipeline where the database is used to process previous test sequences, the test fails. This likely
means that the query itself needs to be updated to work in a non-clean database.
### Ordering assertion
**Label:** `flaky-test::ordering assertion`
**Description:** The test is expecting a specific order in the data under test yet the data is in
a non-deterministic order.
**Difficulty to reproduce:** Easy. Usually, running the test locally several times would reproduce
the problem.
**Resolution:** Depending on the problem, you might want to:
- loosen the assertion if the test shouldn't care about ordering but only on the elements
- fix the test by specifying a deterministic ordering
- fix the app code by specifying a deterministic ordering
**Examples:**
- [Example 1](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10148/diffs): Without
specifying `ORDER BY`, database will not give deterministic ordering, or data race happening
in the tests.
- [Example 2](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106936/diffs).
means that the query itself needs to be updated to work in a non-clean database.
### Dataset-specific
**Label:** `flaky-test::dataset-specific`
**Description:** The test assumes the dataset is in a particular (usually limited) state, which
**Description:** The test assumes the dataset is in a particular (usually limited) state or order, which
might not be true depending on when the test run during the test suite.
**Difficulty to reproduce:** Moderate, as the amount of data needed to reproduce the issue might be
difficult to achieve locally.
difficult to achieve locally. Ordering issues are easier to reproduce by repeatedly running the tests several times.
**Resolution:** Fix the test to not assume that the dataset is in a particular state, don't hardcode IDs.
**Resolution:**
- Fix the test to not assume that the dataset is in a particular state, don't hardcode IDs.
- Loosen the assertion if the test shouldn't care about ordering but only on the elements.
- Fix the test by specifying a deterministic ordering.
- Fix the app code by specifying a deterministic ordering.
**Examples:**
@ -81,11 +67,10 @@ difficult to achieve locally.
suite, it might pass as not enough records were created before it, but as soon as it would run
later in the suite, there could be a record that actually has the ID `42`, hence the test would
start to fail.
- [Example 3](https://gitlab.com/gitlab-org/gitlab/-/issues/402915): State leakage can result from
data records created with `let_it_be` shared between test examples, while some test modifies the model
either deliberately or unwillingly causing out-of-sync data in test examples. This can result in `PG::QueryCanceled: ERROR` in the subsequent test examples or retries.
For more information about state leakages and resolution options,
see [GitLab testing best practices](best_practices.md#lets-talk-about-let).
- [Example 3](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10148/diffs): Without
specifying `ORDER BY`, database is not given deterministic ordering, or data race can happen
in the tests.
- [Example 4](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106936/diffs).
### Random input

View File

@ -29,7 +29,7 @@ Before you can install the agent in your cluster, you need:
To install the agent in your cluster:
1. Optional. [Create an agent configuration file](#create-an-agent-configuration-file).
1. [Create an agent configuration file](#create-an-agent-configuration-file).
1. [Register the agent with GitLab](#register-the-agent-with-gitlab).
1. [Install the agent in your cluster](#install-the-agent-in-the-cluster).
@ -44,8 +44,7 @@ For configuration settings, the agent uses a YAML file in the GitLab project. Yo
- You use [a GitOps workflow](../gitops.md#gitops-workflow-steps).
- You use [a GitLab CI/CD workflow](../ci_cd_workflow.md#gitlab-cicd-workflow-steps) and want to authorize a different project to use the agent.
Otherwise it is optional.
- You [allow specific project or group members to access Kubernetes](../user_access.md).
To create an agent configuration file:
@ -58,14 +57,12 @@ To create an agent configuration file:
- Start with an alphanumeric character.
- End with an alphanumeric character.
1. In the repository, in the default branch, create this directory at the root:
1. In the repository, in the default branch, create an agent configuration file at the root:
```plaintext
.gitlab/agents/<agent-name>
.gitlab/agents/<agent-name>/config.yaml
```
1. In the directory, create a `config.yaml` file. Ensure the filename ends in `.yaml`, not `.yml`.
You can leave the file blank for now, and [configure it](#configure-your-agent) later.
### Register the agent with GitLab

View File

@ -0,0 +1,155 @@
---
stage: Deploy
group: Environments
info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments
---
# Grant users Kubernetes access **(FREE)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/390769) in GitLab 16.1, with [flags](../../../administration/feature_flags.md) named `environment_settings_to_graphql`, `kas_user_access`, `kas_user_access_project`, and `expose_authorized_cluster_agents`. Disabled by default.
FLAG:
On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the feature flags](../../../administration/feature_flags.md) named `environment_settings_to_graphql`, `kas_user_access`, `kas_user_access_project`, and `expose_authorized_cluster_agents`.
As an administrator of Kubernetes clusters in an organization, you can grant Kubernetes access to members
of a specific project or group.
Granting access also activates the Kubernetes Dashboard for a project or group.
## Configure Kubernetes access
Configure access when you want to grant users access
to a Kubernetes cluster.
Prerequisites:
- The agent for Kubernetes is installed in the Kubernetes cluster.
- You must have the Developer role or higher.
To configure access:
- In the agent configuration file, define a `user_access` keyword with the following parameters:
- `projects`: A list of projects whose members should have access.
- `groups`: A list of groups whose members should have access.
- `access_as`: Required. For plain access, the value is `{ agent: {...} }`.
After you configure access, requests are forwarded to the API server using
the agent service account.
For example:
```yaml
# .gitlab/agents/my-agent/config.yaml
user_access:
access_as:
agent: {}
projects:
- id: group-1/project-1
- id: group-2/project-2
groups:
- id: group-2
- id: group-3/subgroup
```
## Configure access with user impersonation **(PREMIUM)**
You can grant access to a Kubernetes cluster and transform
requests into impersonation requests for authenticated users.
Prerequisites:
- The agent for Kubernetes is installed in the Kubernetes cluster.
- You must have the Developer role or higher.
To configure access with user impersonation:
- In the agent configuration file, define a `user_access` keyword with the following parameters:
- `projects`: A list of projects whose members should have access.
- `groups`: A list of groups whose members should have access.
- `access_as`: Required. For user impersonation, the value is `{ user: {...} }`.
After you configure access, requests are transformed into impersonation requests for
authenticated users.
### User impersonation workflow
The installed `agentk` impersonates the given users as follows:
- `UserName` is `gitlab:user:<username>`
- `Groups` is:
- `gitlab:user`: Common to all requests coming from GitLab users.
- `gitlab:project_role:<project_id>:<role>` for each role in each authorized project.
- `gitlab:group_role:<group_id>:<role>` for each role in each authorized group.
- `Extra` carries additional information about the request:
- `agent.gitlab.com/id`: The agent ID.
- `agent.gitlab.com/username`: The username of the GitLab user.
- `agent.gitlab.com/config_project_id`: The agent configuration project ID.
- `agent.gitlab.com/access_type`: One of `personal_access_token`,
`oidc_id_token`, or `session_cookie`.
Only projects and groups directly listed in the under `user_access` in the configuration
file are impersonated. For example:
```yaml
# .gitlab/agents/my-agent/config.yaml
user_access:
access_as:
user: {}
projects:
- id: group-1/project-1 # group_id=1, project_id=1
- id: group-2/project-2 # group_id=2, project_id=2
groups:
- id: group-2 # group_id=2
- id: group-3/subgroup # group_id=3, group_id=4
```
In this configuration:
- If a user is a member of only `group-1`, they receive
only the Kubernetes RBAC groups `gitlab:project_role:1:<role>`.
- If a user is a member of `group-2`, they receive both Kubernetes RBAC groups:
- `gitlab:project_role:2:<role>`,
- `gitlab:group_role:2:<role>`.
### RBAC authorization
Impersonated requests require `ClusterRoleBinding` or `RoleBinding` to identify the resource permissions
inside Kubernetes. See [RBAC authorization](https://kubernetes.io/docs/reference/access-authn-authz/rbac/)
for the appropriate configuration.
For example, if you allow maintainers in `awesome-org/deployment` project (ID: 123) to read the Kubernetes workloads,
you must add a `ClusterRoleBinding` resource to your Kubernetes configuration:
```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: my-cluster-role-binding
roleRef:
name: view
kind: ClusterRole
apiGroup: rbac.authorization.k8s.io
subjects:
- name: gitlab:project_role:123:maintainer
kind: Group
```
## Related topics
- [Architectural blueprint](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/doc/kubernetes_user_access.md)
- [Kubernetes Dashboard](https://gitlab.com/groups/gitlab-org/-/epics/2493)
<!-- ## Troubleshooting
Include any troubleshooting steps that you can foresee. If you know beforehand what issues
one might have when setting this up, or when something is changed, or on upgrading, it's
important to describe those, too. Think of things that may go wrong and include them here.
This is important to minimize requests for support, and to avoid doc comments with
questions that you know someone might ask.
Each scenario can be a third-level heading, for example `### Getting error message X`.
If you have none to add when creating a doc, leave this section in place
but commented out to help encourage others to add to it in the future. -->

View File

@ -30,6 +30,22 @@ On this page, you can view:
- The version of `agentk` installed on your cluster.
- The path to each agent configuration file.
## View shared agents
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/395498) in GitLab 16.1.
In addition to the agents owned by your project, you can also view agents shared with the
[`ci_access`](ci_cd_workflow.md) and [`user_access`](user_access.md) keywords. Once an agent
is shared with a project, it automatically appears in the project agent tab.
To view the list of shared agents:
1. On the top bar, select **Main menu > Projects** and find the project.
1. On the left sidebar, select **Infrastructure > Kubernetes clusters**.
1. Select the **Agent** tab.
The list of shared agents and their clusters are displayed.
## View an agent's activity information
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/277323) in GitLab 14.6.

View File

@ -78,7 +78,7 @@ You can customize the deployment namespace in a few ways:
- For **non-managed** clusters, the auto-generated namespace is set in the `KUBECONFIG`,
but the user is responsible for ensuring its existence. You can fully customize
this value using
[`environment:kubernetes:namespace`](../../../ci/environments/index.md#configure-kubernetes-deployments-deprecated)
[`environment:kubernetes:namespace`](../../../ci/environments/configure_kubernetes_deployments.md)
in `.gitlab-ci.yml`.
When you customize the namespace, existing environments remain linked to their current

View File

@ -6,6 +6,7 @@ module API
include PaginationParams
before do
check_api_enabled!
authenticate!
authorize! :read_secure_files, user_project
end
@ -64,7 +65,7 @@ module API
resource do
before do
read_only_feature_flag_enabled?
check_read_only_feature_flag_enabled!
authorize! :admin_secure_files, user_project
end
@ -112,7 +113,11 @@ module API
end
helpers do
def read_only_feature_flag_enabled?
def check_api_enabled!
forbidden! unless Gitlab.config.ci_secure_files.enabled
end
def check_read_only_feature_flag_enabled!
service_unavailable! if Feature.enabled?(:ci_secure_files_read_only, user_project, type: :ops)
end
end

View File

@ -0,0 +1,33 @@
# frozen_string_literal: true
module Gitlab
module Diff
module Formatters
class FileFormatter < BaseFormatter
def initialize(attrs)
@ignore_whitespace_change = false
super(attrs)
end
def key
@key ||= super.push(new_path, old_path)
end
def position_type
"file"
end
def complete?
[new_path, old_path].all?(&:present?)
end
def ==(other)
other.is_a?(self.class) &&
old_path == other.old_path &&
new_path == other.new_path
end
end
end
end
end

View File

@ -150,6 +150,10 @@ module Gitlab
@file_hash ||= Digest::SHA1.hexdigest(file_path)
end
def on_file?
position_type == 'file'
end
def on_image?
position_type == 'image'
end
@ -185,6 +189,8 @@ module Gitlab
case type
when 'image'
Gitlab::Diff::Formatters::ImageFormatter
when 'file'
Gitlab::Diff::Formatters::FileFormatter
else
Gitlab::Diff::Formatters::TextFormatter
end

View File

@ -22,9 +22,8 @@ module Gitlab
return unless old_position.diff_refs == old_diff_refs
@ignore_whitespace_change = old_position.ignore_whitespace_change
strategy = old_position.on_text? ? LineStrategy : ImageStrategy
strategy.new(self).trace(old_position)
strategy(old_position).new(self).trace(old_position)
end
def ac_diffs
@ -49,6 +48,16 @@ module Gitlab
private
def strategy(old_position)
if old_position.on_text?
LineStrategy
elsif old_position.on_file?
FileStrategy
else
ImageStrategy
end
end
def compare(start_sha, head_sha, straight: false)
compare = CompareService.new(project, head_sha).execute(project, start_sha, straight: straight)
compare.diffs(paths: paths, expanded: true, ignore_whitespace_change: @ignore_whitespace_change)

View File

@ -0,0 +1,47 @@
# frozen_string_literal: true
module Gitlab
module Diff
class PositionTracer
class FileStrategy < BaseStrategy
def trace(position)
a_path = position.old_path
b_path = position.new_path
# If file exists in B->D (e.g. updated, renamed, removed), let the
# note become outdated.
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
return { position: new_position(position, bd_diff), outdated: true } if bd_diff
# If file still exists in the new diff, update the position.
cd_diff = cd_diffs.diff_file_with_new_path(b_path)
return { position: new_position(position, cd_diff), outdated: false } if cd_diff
# If file exists in A->C (e.g. rebased and same changes were present
# in target branch), let the note become outdated.
ac_diff = ac_diffs.diff_file_with_old_path(a_path)
return { position: new_position(position, ac_diff), outdated: true } if ac_diff
# If ever there's a case that the file no longer exists in any diff,
# don't set a change position and let the note become outdated.
#
# This should never happen given the file should exist in one of the
# diffs above.
{ outdated: true }
end
private
def new_position(position, diff_file)
Position.new(
diff_file: diff_file,
position_type: position.position_type
)
end
end
end
end
end

View File

@ -3,36 +3,7 @@
module Gitlab
module Diff
class PositionTracer
class ImageStrategy < BaseStrategy
def trace(position)
a_path = position.old_path
b_path = position.new_path
# If file exists in B->D (e.g. updated, renamed, removed), let the
# note become outdated.
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
return { position: new_position(position, bd_diff), outdated: true } if bd_diff
# If file still exists in the new diff, update the position.
cd_diff = cd_diffs.diff_file_with_new_path(b_path)
return { position: new_position(position, cd_diff), outdated: false } if cd_diff
# If file exists in A->C (e.g. rebased and same changes were present
# in target branch), let the note become outdated.
ac_diff = ac_diffs.diff_file_with_old_path(a_path)
return { position: new_position(position, ac_diff), outdated: true } if ac_diff
# If ever there's a case that the file no longer exists in any diff,
# don't set a change position and let the note become outdated.
#
# This should never happen given the file should exist in one of the
# diffs above.
{ outdated: true }
end
class ImageStrategy < FileStrategy
private
def new_position(position, diff_file)

View File

@ -11207,6 +11207,9 @@ msgstr ""
msgid "Comment on lines %{startLine} to %{endLine}"
msgstr ""
msgid "Comment on this file"
msgstr ""
msgid "Comment template actions"
msgstr ""
@ -28503,6 +28506,12 @@ msgstr ""
msgid "MergeRequests|started a thread"
msgstr ""
msgid "MergeRequests|started a thread on %{linkStart}a file%{linkEnd}"
msgstr ""
msgid "MergeRequests|started a thread on %{linkStart}an old version of a file%{linkEnd}"
msgstr ""
msgid "MergeRequests|started a thread on %{linkStart}an old version of the diff%{linkEnd}"
msgstr ""

View File

@ -109,6 +109,7 @@ module QA
# Happens on clean GDK installations when seeded root admin password is expired
#
def set_up_new_password_if_required(user:, skip_page_validation:)
Support::WaitForRequests.wait_for_requests
return unless has_content?('Set up new password', wait: 1)
Profile::Password.perform do |new_password_page|

View File

@ -25,7 +25,6 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do
'artifacts' => { 'enabled' => true },
'external_diffs' => { 'enabled' => false },
'pages' => { 'enabled' => true },
'ci_secure_files' => { 'enabled' => true },
'object_store' => {
'enabled' => true,
'connection' => connection,
@ -101,14 +100,6 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do
expect(settings.external_diffs['enabled']).to be false
expect(settings.external_diffs['object_store']).to be_nil
expect(settings.external_diffs).to eq(settings['external_diffs'])
expect(settings.ci_secure_files['enabled']).to be true
expect(settings.ci_secure_files['object_store']['enabled']).to be true
expect(settings.ci_secure_files['object_store']['connection'].to_hash).to eq(connection)
expect(settings.ci_secure_files['object_store']['remote_directory']).to eq('ci_secure_files')
expect(settings.ci_secure_files['object_store']['bucket_prefix']).to eq(nil)
expect(settings.ci_secure_files['object_store']['consolidated_settings']).to be true
expect(settings.ci_secure_files).to eq(settings['ci_secure_files'])
end
it 'supports bucket prefixes' do

View File

@ -0,0 +1,28 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'User creates discussion on diff file', :js, feature_category: :code_review_workflow do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public, :repository) }
let_it_be(:merge_request) do
create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
end
before do
project.add_maintainer(user)
sign_in(user)
visit(diffs_project_merge_request_path(project, merge_request))
end
it 'creates discussion on diff file' do
first('.diff-file [data-testid="comment-files-button"]').click
send_keys "Test comment"
click_button "Add comment now"
expect(first('.diff-file')).to have_selector('.note-text', text: 'Test comment')
end
end

View File

@ -12,6 +12,17 @@ RSpec.describe 'Secure Files', :js, feature_category: :groups_and_projects do
sign_in(user)
end
context 'when disabled at the instance level' do
before do
stub_config(ci_secure_files: { enabled: false })
end
it 'does not show the secure files settings' do
visit project_settings_ci_cd_path(project)
expect(page).not_to have_content('Secure Files')
end
end
context 'authenticated user with admin permissions' do
it 'shows the secure files settings' do
visit project_settings_ci_cd_path(project)

View File

@ -16,7 +16,10 @@ describe('Batch comments diff file drafts component', () => {
batchComments: {
namespaced: true,
getters: {
draftsForFile: () => () => [{ id: 1 }, { id: 2 }],
draftsForFile: () => () => [
{ id: 1, position: { position_type: 'file' } },
{ id: 2, position: { position_type: 'file' } },
],
},
},
},
@ -24,7 +27,7 @@ describe('Batch comments diff file drafts component', () => {
vm = shallowMount(DiffFileDrafts, {
store,
propsData: { fileHash: 'filehash' },
propsData: { fileHash: 'filehash', positionType: 'file' },
});
}

View File

@ -176,7 +176,12 @@ describe('DiffContent', () => {
getCommentFormForDiffFileGetterMock.mockReturnValue(() => true);
createComponent({
props: {
diffFile: { ...imageDiffFile, discussions: [{ name: 'discussion-stub ' }] },
diffFile: {
...imageDiffFile,
discussions: [
{ name: 'discussion-stub', position: { position_type: IMAGE_DIFF_POSITION_TYPE } },
],
},
},
});
@ -186,7 +191,12 @@ describe('DiffContent', () => {
it('emits saveDiffDiscussion when note-form emits `handleFormUpdate`', () => {
const noteStub = {};
getCommentFormForDiffFileGetterMock.mockReturnValue(() => true);
const currentDiffFile = { ...imageDiffFile, discussions: [{ name: 'discussion-stub ' }] };
const currentDiffFile = {
...imageDiffFile,
discussions: [
{ name: 'discussion-stub', position: { position_type: IMAGE_DIFF_POSITION_TYPE } },
],
};
createComponent({
props: {
diffFile: currentDiffFile,

View File

@ -18,7 +18,10 @@ import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import testAction from '../../__helpers__/vuex_action_helper';
import diffDiscussionsMockData from '../mock_data/diff_discussions';
jest.mock('~/lib/utils/common_utils');
jest.mock('~/lib/utils/common_utils', () => ({
scrollToElement: jest.fn(),
isLoggedIn: () => true,
}));
const diffFile = Object.freeze(
Object.assign(diffDiscussionsMockData.diff_file, {
@ -47,6 +50,9 @@ describe('DiffFileHeader component', () => {
const diffHasDiscussionsResultMock = jest.fn();
const defaultMockStoreConfig = {
state: {},
getters: {
getNoteableData: () => ({ current_user: { can_create_note: true } }),
},
modules: {
diffs: {
namespaced: true,
@ -637,4 +643,23 @@ describe('DiffFileHeader component', () => {
},
);
});
it.each`
commentOnFiles | exists | existsText
${false} | ${false} | ${'does not'}
${true} | ${true} | ${'does'}
`(
'$existsText render comment on files button when commentOnFiles is $commentOnFiles',
({ commentOnFiles, exists }) => {
window.gon = { current_user_id: 1 };
createComponent({
props: {
addMergeRequestButtons: true,
},
options: { provide: { glFeatures: { commentOnFiles } } },
});
expect(wrapper.find('[data-testid="comment-files-button"]').exists()).toEqual(exists);
},
);
});

View File

@ -553,4 +553,69 @@ describe('DiffFile', () => {
expect(wrapper.find('[data-testid="conflictsAlert"]').exists()).toBe(true);
});
});
describe('file discussions', () => {
it.each`
extraProps | exists | existsText
${{}} | ${false} | ${'does not'}
${{ hasCommentForm: false }} | ${false} | ${'does not'}
${{ hasCommentForm: true }} | ${true} | ${'does'}
${{ discussions: [{ id: 1, position: { position_type: 'file' } }] }} | ${true} | ${'does'}
${{ drafts: [{ id: 1 }] }} | ${true} | ${'does'}
`(
'discussions wrapper $existsText exist for file with $extraProps',
({ extraProps, exists }) => {
const file = {
...getReadableFile(),
...extraProps,
};
({ wrapper, store } = createComponent({
file,
options: { provide: { glFeatures: { commentOnFiles: true } } },
}));
expect(wrapper.find('[data-testid="file-discussions"]').exists()).toEqual(exists);
},
);
it.each`
hasCommentForm | exists | existsText
${false} | ${false} | ${'does not'}
${true} | ${true} | ${'does'}
`(
'comment form $existsText exist for hasCommentForm with $hasCommentForm',
({ hasCommentForm, exists }) => {
const file = {
...getReadableFile(),
hasCommentForm,
};
({ wrapper, store } = createComponent({
file,
options: { provide: { glFeatures: { commentOnFiles: true } } },
}));
expect(wrapper.find('[data-testid="file-note-form"]').exists()).toEqual(exists);
},
);
it.each`
discussions | exists | existsText
${[]} | ${false} | ${'does not'}
${[{ id: 1, position: { position_type: 'file' } }]} | ${true} | ${'does'}
`('discussions $existsText exist for $discussions', ({ discussions, exists }) => {
const file = {
...getReadableFile(),
discussions,
};
({ wrapper, store } = createComponent({
file,
options: { provide: { glFeatures: { commentOnFiles: true } } },
}));
expect(wrapper.find('[data-testid="diff-file-discussions"]').exists()).toEqual(exists);
});
});
});

View File

@ -334,5 +334,6 @@ export const getDiffFileMock = () => ({
},
],
discussions: [],
drafts: [],
renderingLines: false,
});

View File

@ -1889,4 +1889,28 @@ describe('DiffsStoreActions', () => {
},
);
});
describe('toggleFileCommentForm', () => {
it('commits TOGGLE_FILE_COMMENT_FORM', () => {
return testAction(
diffActions.toggleFileCommentForm,
'path',
{},
[{ type: types.TOGGLE_FILE_COMMENT_FORM, payload: 'path' }],
[],
);
});
});
describe('addDraftToFile', () => {
it('commits ADD_DRAFT_TO_FILE', () => {
return testAction(
diffActions.addDraftToFile,
{ filePath: 'path', draft: 'draft' },
{},
[{ type: types.ADD_DRAFT_TO_FILE, payload: { filePath: 'path', draft: 'draft' } }],
[],
);
});
});
});

View File

@ -188,6 +188,24 @@ describe('Diffs Module Getters', () => {
expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(true);
});
it('returns true when file discussion is expanded', () => {
const diffFile = {
discussions: [{ ...discussionMock, expanded: true }],
highlighted_diff_lines: [],
};
expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(true);
});
it('returns false when file discussion is expanded', () => {
const diffFile = {
discussions: [{ ...discussionMock, expanded: false }],
highlighted_diff_lines: [],
};
expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(false);
});
it('returns false when there are no discussions', () => {
const diffFile = {
parallel_diff_lines: [],
@ -231,6 +249,15 @@ describe('Diffs Module Getters', () => {
expect(getters.diffHasDiscussions(localState)(diffFile)).toEqual(true);
});
it('returns true when file has discussions', () => {
const diffFile = {
discussions: [discussionMock, discussionMock],
highlighted_diff_lines: [],
};
expect(getters.diffHasDiscussions(localState)(diffFile)).toEqual(true);
});
it('returns false when getDiffFileDiscussions returns no discussions', () => {
const diffFile = {
parallel_diff_lines: [],

View File

@ -269,6 +269,53 @@ describe('DiffsStoreMutations', () => {
expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1);
});
it('should add discussions to the given file', () => {
const diffPosition = {
base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
new_line: null,
new_path: '500-lines-4.txt',
old_line: 5,
old_path: '500-lines-4.txt',
start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
type: 'file',
};
const state = {
latestDiff: true,
diffFiles: [
{
file_hash: 'ABC',
[INLINE_DIFF_LINES_KEY]: [],
discussions: [],
},
],
};
const discussion = {
id: 1,
line_code: 'ABC_1',
diff_discussion: true,
resolvable: true,
original_position: diffPosition,
position: diffPosition,
diff_file: {
file_hash: state.diffFiles[0].file_hash,
},
};
const diffPositionByLineCode = {
ABC_1: diffPosition,
};
mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
discussion,
diffPositionByLineCode,
});
expect(state.diffFiles[0].discussions.length).toEqual(1);
expect(state.diffFiles[0].discussions[0].id).toEqual(1);
});
it('should not duplicate discussions on line', () => {
const diffPosition = {
base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
@ -957,4 +1004,25 @@ describe('DiffsStoreMutations', () => {
expect(state.mrReviews).toStrictEqual(newReviews);
});
});
describe('TOGGLE_FILE_COMMENT_FORM', () => {
it('toggles diff files hasCommentForm', () => {
const state = { diffFiles: [{ file_path: 'path', hasCommentForm: false }] };
mutations[types.TOGGLE_FILE_COMMENT_FORM](state, 'path');
expect(state.diffFiles[0].hasCommentForm).toEqual(true);
});
});
describe('ADD_DRAFT_TO_FILE', () => {
it('adds draft to diff file', () => {
const state = { diffFiles: [{ file_path: 'path', drafts: [] }] };
mutations[types.ADD_DRAFT_TO_FILE](state, { filePath: 'path', draft: 'test' });
expect(state.diffFiles[0].drafts.length).toEqual(1);
expect(state.diffFiles[0].drafts[0]).toEqual('test');
});
});
});

View File

@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Ci::SecureFilesHelper do
RSpec.describe Ci::SecureFilesHelper, feature_category: :mobile_devops do
let_it_be(:maintainer) { create(:user) }
let_it_be(:developer) { create(:user) }
let_it_be(:guest) { create(:user) }
@ -19,6 +19,16 @@ RSpec.describe Ci::SecureFilesHelper do
subject { helper.show_secure_files_setting(project, user) }
describe '#show_secure_files_setting' do
context 'when disabled at the instance level' do
before do
stub_config(ci_secure_files: { enabled: false })
end
let(:user) { maintainer }
it { is_expected.to be false }
end
context 'authenticated user with admin permissions' do
let(:user) { maintainer }

View File

@ -0,0 +1,44 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Diff::Formatters::FileFormatter, feature_category: :code_review_workflow do
let(:base_attrs) do
{
base_sha: 123,
start_sha: 456,
head_sha: 789,
old_path: nil,
new_path: nil,
position_type: 'file'
}
end
let(:attrs) { base_attrs.merge(old_path: 'path.rb', new_path: 'path.rb') }
it_behaves_like 'position formatter' do
# rubocop:disable Fips/SHA1 (This is used to match the existing class method)
let(:key) do
[123, 456, 789,
Digest::SHA1.hexdigest(formatter.old_path), Digest::SHA1.hexdigest(formatter.new_path),
'path.rb', 'path.rb']
end
# rubocop:enable Fips/SHA1
end
describe '#==' do
subject { described_class.new(attrs) }
it { is_expected.to eq(subject) }
[:old_path, :new_path].each do |attr|
context "with attribute:#{attr}" do
let(:other_formatter) do
described_class.new(attrs.merge(attr => 9))
end
it { is_expected.not_to eq(other_formatter) }
end
end
end
end

View File

@ -0,0 +1,238 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Diff::PositionTracer::FileStrategy, feature_category: :code_review_workflow do
include PositionTracerHelpers
let_it_be(:project) { create(:project, :repository) }
let(:current_user) { project.first_owner }
let(:file_name) { 'test-file' }
let(:new_file_name) { "#{file_name}-new" }
let(:second_file_name) { "#{file_name}-2" }
let(:branch_name) { 'position-tracer-test' }
let(:old_position) { position(old_path: file_name, new_path: file_name, position_type: 'file') }
let(:tracer) do
Gitlab::Diff::PositionTracer.new(
project: project,
old_diff_refs: old_diff_refs,
new_diff_refs: new_diff_refs
)
end
let(:strategy) { described_class.new(tracer) }
let(:initial_commit) do
project.commit(create_branch(branch_name, 'master')[:branch]&.name || 'master')
end
subject { strategy.trace(old_position) }
describe '#trace' do
describe 'diff scenarios' do
let(:create_file_commit) do
initial_commit
create_file(
branch_name,
file_name,
Base64.encode64('content')
)
end
let(:update_file_commit) do
create_file_commit
update_file(
branch_name,
file_name,
Base64.encode64('updatedcontent')
)
end
let(:update_file_again_commit) do
update_file_commit
update_file(
branch_name,
file_name,
Base64.encode64('updatedcontentagain')
)
end
let(:delete_file_commit) do
create_file_commit
delete_file(branch_name, file_name)
end
let(:rename_file_commit) do
delete_file_commit
create_file(
branch_name,
new_file_name,
Base64.encode64('renamedcontent')
)
end
let(:create_second_file_commit) do
create_file_commit
create_file(
branch_name,
second_file_name,
Base64.encode64('morecontent')
)
end
let(:create_another_file_commit) do
create_file(
branch_name,
second_file_name,
Base64.encode64('morecontent')
)
end
let(:update_another_file_commit) do
update_file(
branch_name,
second_file_name,
Base64.encode64('updatedmorecontent')
)
end
context 'when the file was created in the old diff' do
context 'when the file is unchanged between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, create_second_file_commit) }
it 'returns the new position' do
expect_new_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was updated between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, update_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was renamed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, rename_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, rename_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was removed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, delete_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, delete_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file is unchanged in the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) }
let(:change_diff_refs) { diff_refs(initial_commit, create_another_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
end
context 'when the file was changed in the old diff' do
context 'when the file is unchanged in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, create_second_file_commit) }
it 'returns the new position' do
expect_new_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was updated in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
let(:change_diff_refs) { diff_refs(update_file_commit, update_file_again_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was renamed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, rename_file_commit) }
let(:change_diff_refs) { diff_refs(update_file_commit, rename_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was removed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, delete_file_commit) }
let(:change_diff_refs) { diff_refs(update_file_commit, delete_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file is unchanged in the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, create_another_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
end
end
end
end

View File

@ -18,8 +18,13 @@ RSpec.describe Gitlab::Diff::PositionTracer do
let(:project) { double }
let(:old_diff_refs) { diff_refs }
let(:new_diff_refs) { diff_refs }
let(:position) { double(on_text?: on_text?, diff_refs: diff_refs, ignore_whitespace_change: false) }
let(:on_file?) { false }
let(:on_text?) { false }
let(:tracer) { double }
let(:position) do
double(on_text?: on_text?, on_image?: false, on_file?: on_file?, diff_refs: diff_refs,
ignore_whitespace_change: false)
end
context 'position is on text' do
let(:on_text?) { true }
@ -48,6 +53,20 @@ RSpec.describe Gitlab::Diff::PositionTracer do
subject.trace(position)
end
end
context 'position on file' do
let(:on_file?) { true }
it 'calls ImageStrategy#trace' do
expect(Gitlab::Diff::PositionTracer::FileStrategy)
.to receive(:new)
.with(subject)
.and_return(tracer)
expect(tracer).to receive(:trace).with(position)
subject.trace(position)
end
end
end
describe 'diffs methods' do

View File

@ -56,6 +56,26 @@ RSpec.describe API::Ci::SecureFiles, feature_category: :mobile_devops do
end
end
context 'when the feature is disabled at the instance level' do
before do
stub_config(ci_secure_files: { enabled: false })
end
it 'returns a 403 when attempting to upload a file' do
expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:forbidden)
end
it 'returns a 403 when downloading a file' do
get api("/projects/#{project.id}/secure_files", developer)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'when the flag is disabled' do
it 'returns a 201 when uploading a file when the ci_secure_files_read_only feature flag is disabled' do
expect do

View File

@ -2,10 +2,9 @@
RSpec.shared_examples "position formatter" do
let(:formatter) { described_class.new(attrs) }
let(:key) { [123, 456, 789, Digest::SHA1.hexdigest(formatter.old_path), Digest::SHA1.hexdigest(formatter.new_path), 1, 2] }
describe '#key' do
let(:key) { [123, 456, 789, Digest::SHA1.hexdigest(formatter.old_path), Digest::SHA1.hexdigest(formatter.new_path), 1, 2] }
subject { formatter.key }
it { is_expected.to eq(key) }