Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2022-02-15 15:15:04 +00:00
parent 524e972622
commit 78cfc7cf4a
94 changed files with 1060 additions and 618 deletions

View File

@ -14,8 +14,6 @@ import { mapActions, mapGetters, mapState } from 'vuex';
import BoardForm from 'ee_else_ce/boards/components/board_form.vue'; import BoardForm from 'ee_else_ce/boards/components/board_form.vue';
import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import axios from '~/lib/utils/axios_utils';
import httpStatusCodes from '~/lib/utils/http_status';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import eventHub from '../eventhub'; import eventHub from '../eventhub';
@ -23,6 +21,8 @@ import groupBoardsQuery from '../graphql/group_boards.query.graphql';
import projectBoardsQuery from '../graphql/project_boards.query.graphql'; import projectBoardsQuery from '../graphql/project_boards.query.graphql';
import groupBoardQuery from '../graphql/group_board.query.graphql'; import groupBoardQuery from '../graphql/group_board.query.graphql';
import projectBoardQuery from '../graphql/project_board.query.graphql'; import projectBoardQuery from '../graphql/project_board.query.graphql';
import groupRecentBoardsQuery from '../graphql/group_recent_boards.query.graphql';
import projectRecentBoardsQuery from '../graphql/project_recent_boards.query.graphql';
const MIN_BOARDS_TO_VIEW_RECENT = 10; const MIN_BOARDS_TO_VIEW_RECENT = 10;
@ -40,7 +40,7 @@ export default {
directives: { directives: {
GlModalDirective, GlModalDirective,
}, },
inject: ['fullPath', 'recentBoardsEndpoint'], inject: ['fullPath'],
props: { props: {
throttleDuration: { throttleDuration: {
type: Number, type: Number,
@ -158,6 +158,10 @@ export default {
this.scrollFadeInitialized = false; this.scrollFadeInitialized = false;
this.$nextTick(this.setScrollFade); this.$nextTick(this.setScrollFade);
}, },
recentBoards() {
this.scrollFadeInitialized = false;
this.$nextTick(this.setScrollFade);
},
}, },
created() { created() {
eventHub.$on('showBoardModal', this.showPage); eventHub.$on('showBoardModal', this.showPage);
@ -173,11 +177,11 @@ export default {
cancel() { cancel() {
this.showPage(''); this.showPage('');
}, },
boardUpdate(data) { boardUpdate(data, boardType) {
if (!data?.[this.parentType]) { if (!data?.[this.parentType]) {
return []; return [];
} }
return data[this.parentType].boards.edges.map(({ node }) => ({ return data[this.parentType][boardType].edges.map(({ node }) => ({
id: getIdFromGraphQLId(node.id), id: getIdFromGraphQLId(node.id),
name: node.name, name: node.name,
})); }));
@ -185,6 +189,9 @@ export default {
boardQuery() { boardQuery() {
return this.isGroupBoard ? groupBoardsQuery : projectBoardsQuery; return this.isGroupBoard ? groupBoardsQuery : projectBoardsQuery;
}, },
recentBoardsQuery() {
return this.isGroupBoard ? groupRecentBoardsQuery : projectRecentBoardsQuery;
},
loadBoards(toggleDropdown = true) { loadBoards(toggleDropdown = true) {
if (toggleDropdown && this.boards.length > 0) { if (toggleDropdown && this.boards.length > 0) {
return; return;
@ -196,39 +203,20 @@ export default {
}, },
query: this.boardQuery, query: this.boardQuery,
loadingKey: 'loadingBoards', loadingKey: 'loadingBoards',
update: this.boardUpdate, update: (data) => this.boardUpdate(data, 'boards'),
}); });
this.loadRecentBoards(); this.loadRecentBoards();
}, },
loadRecentBoards() { loadRecentBoards() {
this.loadingRecentBoards = true; this.$apollo.addSmartQuery('recentBoards', {
// Follow up to fetch recent boards using GraphQL variables() {
// https://gitlab.com/gitlab-org/gitlab/-/issues/300985 return { fullPath: this.fullPath };
axios },
.get(this.recentBoardsEndpoint) query: this.recentBoardsQuery,
.then((res) => { loadingKey: 'loadingRecentBoards',
this.recentBoards = res.data; update: (data) => this.boardUpdate(data, 'recentIssueBoards'),
}) });
.catch((err) => {
/**
* If user is unauthorized we'd still want to resolve the
* request to display all boards.
*/
if (err?.response?.status === httpStatusCodes.UNAUTHORIZED) {
this.recentBoards = []; // recent boards are empty
return;
}
throw err;
})
.then(() => this.$nextTick()) // Wait for boards list in DOM
.then(() => {
this.setScrollFade();
})
.catch(() => {})
.finally(() => {
this.loadingRecentBoards = false;
});
}, },
isScrolledUp() { isScrolledUp() {
const { content } = this.$refs; const { content } = this.$refs;

View File

@ -0,0 +1,14 @@
#import "ee_else_ce/boards/graphql/board.fragment.graphql"
query group_recent_boards($fullPath: ID!) {
group(fullPath: $fullPath) {
id
recentIssueBoards {
edges {
node {
...BoardFragment
}
}
}
}
}

View File

@ -0,0 +1,14 @@
#import "ee_else_ce/boards/graphql/board.fragment.graphql"
query project_recent_boards($fullPath: ID!) {
project(fullPath: $fullPath) {
id
recentIssueBoards {
edges {
node {
...BoardFragment
}
}
}
}
}

View File

@ -144,7 +144,6 @@ export default () => {
mountMultipleBoardsSwitcher({ mountMultipleBoardsSwitcher({
fullPath: $boardApp.dataset.fullPath, fullPath: $boardApp.dataset.fullPath,
rootPath: $boardApp.dataset.boardsEndpoint, rootPath: $boardApp.dataset.boardsEndpoint,
recentBoardsEndpoint: $boardApp.dataset.recentBoardsEndpoint,
allowScopedLabels: $boardApp.dataset.scopedLabels, allowScopedLabels: $boardApp.dataset.scopedLabels,
labelsManagePath: $boardApp.dataset.labelsManagePath, labelsManagePath: $boardApp.dataset.labelsManagePath,
}); });

View File

@ -24,7 +24,6 @@ export default (params = {}) => {
provide: { provide: {
fullPath: params.fullPath, fullPath: params.fullPath,
rootPath: params.rootPath, rootPath: params.rootPath,
recentBoardsEndpoint: params.recentBoardsEndpoint,
allowScopedLabels: params.allowScopedLabels, allowScopedLabels: params.allowScopedLabels,
labelsManagePath: params.labelsManagePath, labelsManagePath: params.labelsManagePath,
allowLabelCreate: parseBoolean(dataset.canAdminBoard), allowLabelCreate: parseBoolean(dataset.canAdminBoard),

View File

@ -53,6 +53,7 @@ import DiffFile from './diff_file.vue';
import HiddenFilesWarning from './hidden_files_warning.vue'; import HiddenFilesWarning from './hidden_files_warning.vue';
import NoChanges from './no_changes.vue'; import NoChanges from './no_changes.vue';
import TreeList from './tree_list.vue'; import TreeList from './tree_list.vue';
import VirtualScrollerScrollSync from './virtual_scroller_scroll_sync';
export default { export default {
name: 'DiffsApp', name: 'DiffsApp',
@ -62,8 +63,7 @@ export default {
DynamicScrollerItem: () => DynamicScrollerItem: () =>
import('vendor/vue-virtual-scroller').then(({ DynamicScrollerItem }) => DynamicScrollerItem), import('vendor/vue-virtual-scroller').then(({ DynamicScrollerItem }) => DynamicScrollerItem),
PreRenderer: () => import('./pre_renderer.vue').then((PreRenderer) => PreRenderer), PreRenderer: () => import('./pre_renderer.vue').then((PreRenderer) => PreRenderer),
VirtualScrollerScrollSync: () => VirtualScrollerScrollSync,
import('./virtual_scroller_scroll_sync').then((VSSSync) => VSSSync),
CompareVersions, CompareVersions,
DiffFile, DiffFile,
NoChanges, NoChanges,
@ -406,10 +406,8 @@ export default {
this.unsubscribeFromEvents(); this.unsubscribeFromEvents();
this.removeEventListeners(); this.removeEventListeners();
if (window.gon?.features?.diffsVirtualScrolling) { diffsEventHub.$off('scrollToFileHash', this.scrollVirtualScrollerToFileHash);
diffsEventHub.$off('scrollToFileHash', this.scrollVirtualScrollerToFileHash); diffsEventHub.$off('scrollToIndex', this.scrollVirtualScrollerToIndex);
diffsEventHub.$off('scrollToIndex', this.scrollVirtualScrollerToIndex);
}
}, },
methods: { methods: {
...mapActions(['startTaskList']), ...mapActions(['startTaskList']),
@ -522,32 +520,27 @@ export default {
); );
} }
if ( let keydownTime;
window.gon?.features?.diffsVirtualScrolling || Mousetrap.bind(['mod+f', 'mod+g'], () => {
window.gon?.features?.usageDataDiffSearches keydownTime = new Date().getTime();
) { });
let keydownTime;
Mousetrap.bind(['mod+f', 'mod+g'], () => {
keydownTime = new Date().getTime();
});
window.addEventListener('blur', () => { window.addEventListener('blur', () => {
if (keydownTime) { if (keydownTime) {
const delta = new Date().getTime() - keydownTime; const delta = new Date().getTime() - keydownTime;
// To make sure the user is using the find function we need to wait for blur // To make sure the user is using the find function we need to wait for blur
// and max 1000ms to be sure it the search box is filtered // and max 1000ms to be sure it the search box is filtered
if (delta >= 0 && delta < 1000) { if (delta >= 0 && delta < 1000) {
this.disableVirtualScroller(); this.disableVirtualScroller();
if (window.gon?.features?.usageDataDiffSearches) { if (window.gon?.features?.usageDataDiffSearches) {
api.trackRedisHllUserEvent('i_code_review_user_searches_diff'); api.trackRedisHllUserEvent('i_code_review_user_searches_diff');
api.trackRedisCounterEvent('diff_searches'); api.trackRedisCounterEvent('diff_searches');
}
} }
} }
}); }
} });
}, },
removeEventListeners() { removeEventListeners() {
Mousetrap.unbind(keysFor(MR_PREVIOUS_FILE_IN_DIFF)); Mousetrap.unbind(keysFor(MR_PREVIOUS_FILE_IN_DIFF));
@ -589,8 +582,6 @@ export default {
this.virtualScrollCurrentIndex = -1; this.virtualScrollCurrentIndex = -1;
}, },
scrollVirtualScrollerToDiffNote() { scrollVirtualScrollerToDiffNote() {
if (!window.gon?.features?.diffsVirtualScrolling) return;
const id = window?.location?.hash; const id = window?.location?.hash;
if (id.startsWith('#note_')) { if (id.startsWith('#note_')) {
@ -605,11 +596,7 @@ export default {
} }
}, },
subscribeToVirtualScrollingEvents() { subscribeToVirtualScrollingEvents() {
if ( if (this.shouldShow && !this.subscribedToVirtualScrollingEvents) {
window.gon?.features?.diffsVirtualScrolling &&
this.shouldShow &&
!this.subscribedToVirtualScrollingEvents
) {
diffsEventHub.$on('scrollToFileHash', this.scrollVirtualScrollerToFileHash); diffsEventHub.$on('scrollToFileHash', this.scrollVirtualScrollerToFileHash);
diffsEventHub.$on('scrollToIndex', this.scrollVirtualScrollerToIndex); diffsEventHub.$on('scrollToIndex', this.scrollVirtualScrollerToIndex);

View File

@ -209,7 +209,7 @@ export default {
handler(val) { handler(val) {
const el = this.$el.closest('.vue-recycle-scroller__item-view'); const el = this.$el.closest('.vue-recycle-scroller__item-view');
if (this.glFeatures.diffsVirtualScrolling && el) { if (el) {
// We can't add a style with Vue because of the way the virtual // We can't add a style with Vue because of the way the virtual
// scroller library renders the diff files // scroller library renders the diff files
el.style.zIndex = val ? '1' : null; el.style.zIndex = val ? '1' : null;

View File

@ -29,8 +29,6 @@ export const UNFOLD_COUNT = 20;
export const COUNT_OF_AVATARS_IN_GUTTER = 3; export const COUNT_OF_AVATARS_IN_GUTTER = 3;
export const LENGTH_OF_AVATAR_TOOLTIP = 17; export const LENGTH_OF_AVATAR_TOOLTIP = 17;
export const LINES_TO_BE_RENDERED_DIRECTLY = 100;
export const DIFF_FILE_SYMLINK_MODE = '120000'; export const DIFF_FILE_SYMLINK_MODE = '120000';
export const DIFF_FILE_DELETED_MODE = '0'; export const DIFF_FILE_DELETED_MODE = '0';

View File

@ -1,8 +1,7 @@
import Vue from 'vue'; import Vue from 'vue';
import { mapActions, mapState, mapGetters } from 'vuex'; import { mapActions, mapState, mapGetters } from 'vuex';
import { getCookie, setCookie, parseBoolean, removeCookie } from '~/lib/utils/common_utils'; import { getCookie, parseBoolean, removeCookie } from '~/lib/utils/common_utils';
import { getParameterValues } from '~/lib/utils/url_utility';
import eventHub from '../notes/event_hub'; import eventHub from '../notes/event_hub';
import diffsApp from './components/app.vue'; import diffsApp from './components/app.vue';
@ -74,11 +73,6 @@ export default function initDiffsApp(store) {
trackClick: false, trackClick: false,
}); });
} }
const vScrollingParam = getParameterValues('virtual_scrolling')[0];
if (vScrollingParam === 'false' || vScrollingParam === 'true') {
setCookie('diffs_virtual_scrolling', vScrollingParam);
}
}, },
methods: { methods: {
...mapActions('diffs', ['setRenderTreeList', 'setShowWhitespace']), ...mapActions('diffs', ['setRenderTreeList', 'setShowWhitespace']),

View File

@ -125,7 +125,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
commit(types.SET_DIFF_DATA_BATCH, { diff_files }); commit(types.SET_DIFF_DATA_BATCH, { diff_files });
commit(types.SET_BATCH_LOADING_STATE, 'loaded'); commit(types.SET_BATCH_LOADING_STATE, 'loaded');
if (window.gon?.features?.diffsVirtualScrolling && !scrolledVirtualScroller) { if (!scrolledVirtualScroller) {
const index = state.diffFiles.findIndex( const index = state.diffFiles.findIndex(
(f) => (f) =>
f.file_hash === hash || f[INLINE_DIFF_LINES_KEY].find((l) => l.line_code === hash), f.file_hash === hash || f[INLINE_DIFF_LINES_KEY].find((l) => l.line_code === hash),
@ -195,9 +195,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
commit(types.SET_BATCH_LOADING_STATE, 'error'); commit(types.SET_BATCH_LOADING_STATE, 'error');
}); });
return getBatch().then( return getBatch();
() => !window.gon?.features?.diffsVirtualScrolling && handleLocationHash(),
);
}; };
export const fetchDiffFilesMeta = ({ commit, state }) => { export const fetchDiffFilesMeta = ({ commit, state }) => {
@ -529,7 +527,7 @@ export const setCurrentFileHash = ({ commit }, hash) => {
commit(types.SET_CURRENT_DIFF_FILE, hash); commit(types.SET_CURRENT_DIFF_FILE, hash);
}; };
export const scrollToFile = ({ state, commit, getters }, { path, setHash = true }) => { export const scrollToFile = ({ state, commit, getters }, { path }) => {
if (!state.treeEntries[path]) return; if (!state.treeEntries[path]) return;
const { fileHash } = state.treeEntries[path]; const { fileHash } = state.treeEntries[path];
@ -539,11 +537,9 @@ export const scrollToFile = ({ state, commit, getters }, { path, setHash = true
if (getters.isVirtualScrollingEnabled) { if (getters.isVirtualScrollingEnabled) {
eventHub.$emit('scrollToFileHash', fileHash); eventHub.$emit('scrollToFileHash', fileHash);
if (setHash) { setTimeout(() => {
setTimeout(() => { window.history.replaceState(null, null, `#${fileHash}`);
window.history.replaceState(null, null, `#${fileHash}`); });
});
}
} else { } else {
document.location.hash = fileHash; document.location.hash = fileHash;

View File

@ -1,6 +1,5 @@
import { getCookie } from '~/lib/utils/common_utils';
import { getParameterValues } from '~/lib/utils/url_utility';
import { __, n__ } from '~/locale'; import { __, n__ } from '~/locale';
import { getParameterValues } from '~/lib/utils/url_utility';
import { import {
PARALLEL_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE,
INLINE_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE,
@ -175,21 +174,11 @@ export function suggestionCommitMessage(state, _, rootState) {
} }
export const isVirtualScrollingEnabled = (state) => { export const isVirtualScrollingEnabled = (state) => {
const vSrollerCookie = getCookie('diffs_virtual_scrolling'); if (state.disableVirtualScroller || getParameterValues('virtual_scrolling')[0] === 'false') {
if (state.disableVirtualScroller) {
return false; return false;
} }
if (vSrollerCookie) { return !state.viewDiffsFileByFile;
return vSrollerCookie === 'true';
}
return (
!state.viewDiffsFileByFile &&
(window.gon?.features?.diffsVirtualScrolling ||
getParameterValues('virtual_scrolling')[0] === 'true')
);
}; };
export const isBatchLoading = (state) => state.batchLoadingState === 'loading'; export const isBatchLoading = (state) => state.batchLoadingState === 'loading';

View File

@ -9,7 +9,6 @@ import {
NEW_LINE_TYPE, NEW_LINE_TYPE,
OLD_LINE_TYPE, OLD_LINE_TYPE,
MATCH_LINE_TYPE, MATCH_LINE_TYPE,
LINES_TO_BE_RENDERED_DIRECTLY,
INLINE_DIFF_LINES_KEY, INLINE_DIFF_LINES_KEY,
CONFLICT_OUR, CONFLICT_OUR,
CONFLICT_THEIR, CONFLICT_THEIR,
@ -380,16 +379,9 @@ function prepareDiffFileLines(file) {
return file; return file;
} }
function finalizeDiffFile(file, index) { function finalizeDiffFile(file) {
let renderIt = Boolean(window.gon?.features?.diffsVirtualScrolling);
if (!window.gon?.features?.diffsVirtualScrolling) {
renderIt =
index < 3 ? file[INLINE_DIFF_LINES_KEY].length < LINES_TO_BE_RENDERED_DIRECTLY : false;
}
Object.assign(file, { Object.assign(file, {
renderIt, renderIt: true,
isShowingFullFile: false, isShowingFullFile: false,
isLoadingFullFile: false, isLoadingFullFile: false,
discussions: [], discussions: [],
@ -417,15 +409,13 @@ export function prepareDiffData({ diff, priorFiles = [], meta = false }) {
.map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles, meta })) .map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles, meta }))
.map(ensureBasicDiffFileLines) .map(ensureBasicDiffFileLines)
.map(prepareDiffFileLines) .map(prepareDiffFileLines)
.map((file, index) => finalizeDiffFile(file, priorFiles.length + index)); .map((file) => finalizeDiffFile(file));
return deduplicateFilesList([...priorFiles, ...cleanedFiles]); return deduplicateFilesList([...priorFiles, ...cleanedFiles]);
} }
export function getDiffPositionByLineCode(diffFiles) { export function getDiffPositionByLineCode(diffFiles) {
let lines = []; const lines = diffFiles.reduce((acc, diffFile) => {
lines = diffFiles.reduce((acc, diffFile) => {
diffFile[INLINE_DIFF_LINES_KEY].forEach((line) => { diffFile[INLINE_DIFF_LINES_KEY].forEach((line) => {
acc.push({ file: diffFile, line }); acc.push({ file: diffFile, line });
}); });

View File

@ -13,6 +13,21 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => {
IssuableTokenKeys.tokenKeys.splice(2, 0, reviewerToken); IssuableTokenKeys.tokenKeys.splice(2, 0, reviewerToken);
IssuableTokenKeys.tokenKeysWithAlternative.splice(2, 0, reviewerToken); IssuableTokenKeys.tokenKeysWithAlternative.splice(2, 0, reviewerToken);
if (window.gon?.features?.mrAttentionRequests) {
const attentionRequestedToken = {
formattedKey: __('Attention'),
key: 'attention',
type: 'string',
param: '',
symbol: '@',
icon: 'user',
tag: '@attention',
hideNotEqual: true,
};
IssuableTokenKeys.tokenKeys.splice(2, 0, attentionRequestedToken);
IssuableTokenKeys.tokenKeysWithAlternative.splice(2, 0, attentionRequestedToken);
}
const draftToken = { const draftToken = {
token: { token: {
formattedKey: __('Draft'), formattedKey: __('Draft'),

View File

@ -77,6 +77,11 @@ export default class AvailableDropdownMappings {
gl: DropdownUser, gl: DropdownUser,
element: this.container.querySelector('#js-dropdown-reviewer'), element: this.container.querySelector('#js-dropdown-reviewer'),
}, },
attention: {
reference: null,
gl: DropdownUser,
element: this.container.getElementById('js-dropdown-attention-requested'),
},
'approved-by': { 'approved-by': {
reference: null, reference: null,
gl: DropdownUser, gl: DropdownUser,

View File

@ -1,4 +1,4 @@
export const USER_TOKEN_TYPES = ['author', 'assignee', 'approved-by', 'reviewer']; export const USER_TOKEN_TYPES = ['author', 'assignee', 'approved-by', 'reviewer', 'attention'];
export const DROPDOWN_TYPE = { export const DROPDOWN_TYPE = {
hint: 'hint', hint: 'hint',

View File

@ -3,8 +3,6 @@ import { scrollToElementWithContext, scrollToElement } from '~/lib/utils/common_
import { updateHistory } from '../../lib/utils/url_utility'; import { updateHistory } from '../../lib/utils/url_utility';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
const isDiffsVirtualScrollingEnabled = () => window.gon?.features?.diffsVirtualScrolling;
/** /**
* @param {string} selector * @param {string} selector
* @returns {boolean} * @returns {boolean}
@ -15,7 +13,7 @@ function scrollTo(selector, { withoutContext = false } = {}) {
if (el) { if (el) {
scrollFunction(el, { scrollFunction(el, {
behavior: isDiffsVirtualScrollingEnabled() ? 'auto' : 'smooth', behavior: 'auto',
}); });
return true; return true;
} }
@ -31,7 +29,7 @@ function updateUrlWithNoteId(noteId) {
replace: true, replace: true,
}; };
if (noteId && isDiffsVirtualScrollingEnabled()) { if (noteId) {
// Temporarily mask the ID to avoid the browser default // Temporarily mask the ID to avoid the browser default
// scrolling taking over which is broken with virtual // scrolling taking over which is broken with virtual
// scrolling enabled. // scrolling enabled.
@ -115,17 +113,13 @@ function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId)
const isDiffView = window.mrTabs.currentAction === 'diffs'; const isDiffView = window.mrTabs.currentAction === 'diffs';
const targetId = fn(discussionId, isDiffView); const targetId = fn(discussionId, isDiffView);
const discussion = self.getDiscussion(targetId); const discussion = self.getDiscussion(targetId);
const setHash = !isDiffView && !isDiffsVirtualScrollingEnabled();
const discussionFilePath = discussion?.diff_file?.file_path; const discussionFilePath = discussion?.diff_file?.file_path;
if (isDiffsVirtualScrollingEnabled()) { window.location.hash = '';
window.location.hash = '';
}
if (discussionFilePath) { if (discussionFilePath) {
self.scrollToFile({ self.scrollToFile({
path: discussionFilePath, path: discussionFilePath,
setHash,
}); });
} }

View File

@ -1,5 +1,6 @@
<script> <script>
import createFlash from '~/flash'; import createFlash from '~/flash';
import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
import { __, s__ } from '~/locale'; import { __, s__ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
@ -79,6 +80,7 @@ export default {
[STOPPING]: { [STOPPING]: {
actionName: STOPPING, actionName: STOPPING,
buttonText: s__('MrDeploymentActions|Stop environment'), buttonText: s__('MrDeploymentActions|Stop environment'),
buttonVariant: 'danger',
busyText: __('This environment is being deployed'), busyText: __('This environment is being deployed'),
confirmMessage: __('Are you sure you want to stop this environment?'), confirmMessage: __('Are you sure you want to stop this environment?'),
errorMessage: __('Something went wrong while stopping this environment. Please try again.'), errorMessage: __('Something went wrong while stopping this environment. Please try again.'),
@ -86,6 +88,7 @@ export default {
[DEPLOYING]: { [DEPLOYING]: {
actionName: DEPLOYING, actionName: DEPLOYING,
buttonText: s__('MrDeploymentActions|Deploy'), buttonText: s__('MrDeploymentActions|Deploy'),
buttonVariant: 'confirm',
busyText: __('This environment is being deployed'), busyText: __('This environment is being deployed'),
confirmMessage: __('Are you sure you want to deploy this environment?'), confirmMessage: __('Are you sure you want to deploy this environment?'),
errorMessage: __('Something went wrong while deploying this environment. Please try again.'), errorMessage: __('Something went wrong while deploying this environment. Please try again.'),
@ -93,14 +96,27 @@ export default {
[REDEPLOYING]: { [REDEPLOYING]: {
actionName: REDEPLOYING, actionName: REDEPLOYING,
buttonText: s__('MrDeploymentActions|Re-deploy'), buttonText: s__('MrDeploymentActions|Re-deploy'),
buttonVariant: 'confirm',
busyText: __('This environment is being re-deployed'), busyText: __('This environment is being re-deployed'),
confirmMessage: __('Are you sure you want to re-deploy this environment?'), confirmMessage: __('Are you sure you want to re-deploy this environment?'),
errorMessage: __('Something went wrong while deploying this environment. Please try again.'), errorMessage: __('Something went wrong while deploying this environment. Please try again.'),
}, },
}, },
methods: { methods: {
executeAction(endpoint, { actionName, confirmMessage, errorMessage }) { async executeAction(
const isConfirmed = confirm(confirmMessage); //eslint-disable-line endpoint,
{
actionName,
buttonText: primaryBtnText,
buttonVariant: primaryBtnVariant,
confirmMessage,
errorMessage,
},
) {
const isConfirmed = await confirmAction(confirmMessage, {
primaryBtnVariant,
primaryBtnText,
});
if (isConfirmed) { if (isConfirmed) {
this.actionInProgress = actionName; this.actionInProgress = actionName;

View File

@ -278,3 +278,33 @@ $gl-line-height-42: px-to-rem(42px);
.gl-pr-10 { .gl-pr-10 {
padding-right: $gl-spacing-scale-10; padding-right: $gl-spacing-scale-10;
} }
/* Will be moved to @gitlab/ui by https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1709 */
.gl-md-grid-template-columns-2 {
@include media-breakpoint-up(md) {
grid-template-columns: 1fr 1fr;
}
}
.gl-gap-6 {
gap: $gl-spacing-scale-6;
}
$gl-spacing-scale-48: 48 * $grid-size;
.gl-max-w-48 {
max-width: $gl-spacing-scale-48;
}
$gl-spacing-scale-75: 75 * $grid-size;
.gl-max-w-75 {
max-width: $gl-spacing-scale-75;
}
.gl-md-pt-11 {
@include media-breakpoint-up(md) {
padding-top: $gl-spacing-scale-11 !important; // only need !important for now so that it overrides styles from @gitlab/ui which currently take precedence
}
}
/* End gitlab-ui#1709 */

View File

@ -20,6 +20,10 @@ class DashboardController < Dashboard::ApplicationController
urgency :low, [:merge_requests] urgency :low, [:merge_requests]
before_action only: [:merge_requests] do
push_frontend_feature_flag(:mr_attention_requests, default_enabled: :yaml)
end
def activity def activity
respond_to do |format| respond_to do |format|
format.html format.html

View File

@ -30,6 +30,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
before_action :set_issuables_index, only: [:index] before_action :set_issuables_index, only: [:index]
before_action :authenticate_user!, only: [:assign_related_issues] before_action :authenticate_user!, only: [:assign_related_issues]
before_action :check_user_can_push_to_source_branch!, only: [:rebase] before_action :check_user_can_push_to_source_branch!, only: [:rebase]
before_action only: [:index, :show] do
push_frontend_feature_flag(:mr_attention_requests, project, default_enabled: :yaml)
end
before_action only: [:show] do before_action only: [:show] do
push_frontend_feature_flag(:file_identifier_hash) push_frontend_feature_flag(:file_identifier_hash)
push_frontend_feature_flag(:merge_request_widget_graphql, @project, default_enabled: :yaml) push_frontend_feature_flag(:merge_request_widget_graphql, @project, default_enabled: :yaml)
@ -38,9 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:paginated_notes, @project, default_enabled: :yaml) push_frontend_feature_flag(:paginated_notes, @project, default_enabled: :yaml)
push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml) push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml)
push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml) push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml)
push_frontend_feature_flag(:diffs_virtual_scrolling, project, default_enabled: :yaml)
push_frontend_feature_flag(:restructured_mr_widget, project, default_enabled: :yaml) push_frontend_feature_flag(:restructured_mr_widget, project, default_enabled: :yaml)
push_frontend_feature_flag(:mr_attention_requests, project, default_enabled: :yaml)
push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml) push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml)
push_frontend_feature_flag(:rebase_without_ci_ui, @project, default_enabled: :yaml) push_frontend_feature_flag(:rebase_without_ci_ui, @project, default_enabled: :yaml)
push_frontend_feature_flag(:rearrange_pipelines_table, project, default_enabled: :yaml) push_frontend_feature_flag(:rearrange_pipelines_table, project, default_enabled: :yaml)
@ -50,6 +52,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:usage_data_diff_searches, @project, default_enabled: :yaml) push_frontend_feature_flag(:usage_data_diff_searches, @project, default_enabled: :yaml)
end end
before_action do
push_frontend_feature_flag(:permit_all_shared_groups_for_approval, @project, default_enabled: :yaml)
end
around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions] around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]
after_action :log_merge_request_show, only: [:show] after_action :log_merge_request_show, only: [:show]

View File

@ -44,7 +44,8 @@ class MergeRequestsFinder < IssuableFinder
:reviewer_id, :reviewer_id,
:reviewer_username, :reviewer_username,
:target_branch, :target_branch,
:wip :wip,
:attention
] ]
end end
@ -69,6 +70,7 @@ class MergeRequestsFinder < IssuableFinder
items = by_approvals(items) items = by_approvals(items)
items = by_deployments(items) items = by_deployments(items)
items = by_reviewer(items) items = by_reviewer(items)
items = by_attention(items)
by_source_project_id(items) by_source_project_id(items)
end end
@ -218,6 +220,12 @@ class MergeRequestsFinder < IssuableFinder
end end
end end
def by_attention(items)
return items unless params.attention?
items.attention(params.attention)
end
def parse_datetime(input) def parse_datetime(input)
# To work around http://www.ruby-lang.org/en/news/2021/11/15/date-parsing-method-regexp-dos-cve-2021-41817/ # To work around http://www.ruby-lang.org/en/news/2021/11/15/date-parsing-method-regexp-dos-cve-2021-41817/
DateTime.parse(input.byteslice(0, 128)) if input DateTime.parse(input.byteslice(0, 128)) if input

View File

@ -21,5 +21,11 @@ class MergeRequestsFinder
end end
end end
end end
def attention
strong_memoize(:attention) do
User.find_by_username(params[:attention])
end
end
end end
end end

View File

@ -40,7 +40,7 @@ module Projects
avenues = [authorizable_project_members] avenues = [authorizable_project_members]
avenues << if project.personal? avenues << if project.personal?
project_owner project_owner_acting_as_maintainer
else else
authorizable_group_members authorizable_group_members
end end
@ -85,15 +85,9 @@ module Projects
Member.from_union(members) Member.from_union(members)
end end
# workaround until we migrate Project#owners to have membership with def project_owner_acting_as_maintainer
# OWNER access level
def project_owner
user_id = project.namespace.owner.id user_id = project.namespace.owner.id
access_level = if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml) access_level = Gitlab::Access::MAINTAINER
Gitlab::Access::OWNER
else
Gitlab::Access::MAINTAINER
end
Member Member
.from(generate_from_statement([[user_id, access_level]])) # rubocop: disable CodeReuse/ActiveRecord .from(generate_from_statement([[user_id, access_level]])) # rubocop: disable CodeReuse/ActiveRecord

View File

@ -17,7 +17,6 @@ module BoardsHelper
can_update: can_update?.to_s, can_update: can_update?.to_s,
can_admin_list: can_admin_list?.to_s, can_admin_list: can_admin_list?.to_s,
time_tracking_limit_to_hours: Gitlab::CurrentSettings.time_tracking_limit_to_hours.to_s, time_tracking_limit_to_hours: Gitlab::CurrentSettings.time_tracking_limit_to_hours.to_s,
recent_boards_endpoint: recent_boards_path,
parent: current_board_parent.model_name.param_key, parent: current_board_parent.model_name.param_key,
group_id: group_id, group_id: group_id,
labels_filter_base_path: build_issue_link_base, labels_filter_base_path: build_issue_link_base,
@ -128,10 +127,6 @@ module BoardsHelper
} }
end end
def recent_boards_path
recent_project_boards_path(@project) if current_board_parent.is_a?(Project)
end
def serializer def serializer
CurrentBoardSerializer.new CurrentBoardSerializer.new
end end

View File

@ -8,14 +8,8 @@ module SelectForProjectAuthorization
select("projects.id AS project_id", "members.access_level") select("projects.id AS project_id", "members.access_level")
end end
# workaround until we migrate Project#owners to have membership with def select_as_maintainer_for_project_authorization
# OWNER access level select(["projects.id AS project_id", "#{Gitlab::Access::MAINTAINER} AS access_level"])
def select_project_owner_for_project_authorization
if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml)
select(["projects.id AS project_id", "#{Gitlab::Access::OWNER} AS access_level"])
else
select(["projects.id AS project_id", "#{Gitlab::Access::MAINTAINER} AS access_level"])
end
end end
end end
end end

View File

@ -274,9 +274,7 @@ class ContainerRepository < ApplicationRecord
def retry_aborted_migration def retry_aborted_migration
return unless migration_state == 'import_aborted' return unless migration_state == 'import_aborted'
import_status = gitlab_api_client.import_status(self.path) case external_import_status
case import_status
when 'native' when 'native'
raise NativeImportError raise NativeImportError
when 'import_in_progress' when 'import_in_progress'
@ -322,6 +320,12 @@ class ContainerRepository < ApplicationRecord
[migration_pre_import_done_at, migration_import_done_at, migration_aborted_at].compact.max [migration_pre_import_done_at, migration_import_done_at, migration_aborted_at].compact.max
end end
def external_import_status
strong_memoize(:import_status) do
gitlab_api_client.import_status(self.path)
end
end
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
def registry def registry
@registry ||= begin @registry ||= begin

View File

@ -406,6 +406,17 @@ class MergeRequest < ApplicationRecord
) )
end end
scope :attention, ->(user) do
# rubocop: disable Gitlab/Union
union = Gitlab::SQL::Union.new([
MergeRequestReviewer.select(:merge_request_id).where(user_id: user.id, state: MergeRequestReviewer.states[:attention_requested]),
MergeRequestAssignee.select(:merge_request_id).where(user_id: user.id, state: MergeRequestAssignee.states[:attention_requested])
])
# rubocop: enable Gitlab/Union
with(Gitlab::SQL::CTE.new(:reviewers_and_assignees, union).to_arel).where('merge_requests.id in (select merge_request_id from reviewers_and_assignees)')
end
def self.total_time_to_merge def self.total_time_to_merge
join_metrics join_metrics
.merge(MergeRequest::Metrics.with_valid_time_to_merge) .merge(MergeRequest::Metrics.with_valid_time_to_merge)

View File

@ -75,6 +75,12 @@ module Namespaces
end end
end end
def self_and_hierarchy
return super unless use_traversal_ids_for_self_and_hierarchy_scopes?
unscoped.from_union([all.self_and_ancestors, all.self_and_descendants(include_self: false)])
end
def order_by_depth(hierarchy_order) def order_by_depth(hierarchy_order)
return all unless hierarchy_order return all unless hierarchy_order
@ -114,6 +120,11 @@ module Namespaces
use_traversal_ids? use_traversal_ids?
end end
def use_traversal_ids_for_self_and_hierarchy_scopes?
Feature.enabled?(:use_traversal_ids_for_self_and_hierarchy_scopes, default_enabled: :yaml) &&
use_traversal_ids?
end
def self_and_descendants_with_comparison_operators(include_self: true) def self_and_descendants_with_comparison_operators(include_self: true)
base = all.select( base = all.select(
:traversal_ids, :traversal_ids,

View File

@ -53,6 +53,11 @@ module Namespaces
self_and_descendants(include_self: include_self).as_ids self_and_descendants(include_self: include_self).as_ids
end end
alias_method :recursive_self_and_descendant_ids, :self_and_descendant_ids alias_method :recursive_self_and_descendant_ids, :self_and_descendant_ids
def self_and_hierarchy
Gitlab::ObjectHierarchy.new(all).all_objects
end
alias_method :recursive_self_and_hierarchy, :self_and_hierarchy
end end
end end
end end

View File

@ -459,7 +459,7 @@ class Project < ApplicationRecord
delegate :name, to: :owner, allow_nil: true, prefix: true delegate :name, to: :owner, allow_nil: true, prefix: true
delegate :members, to: :team, prefix: true delegate :members, to: :team, prefix: true
delegate :add_user, :add_users, to: :team delegate :add_user, :add_users, to: :team
delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_owner, :add_role, to: :team delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team
delegate :group_runners_enabled, :group_runners_enabled=, to: :ci_cd_settings, allow_nil: true delegate :group_runners_enabled, :group_runners_enabled=, to: :ci_cd_settings, allow_nil: true
delegate :root_ancestor, to: :namespace, allow_nil: true delegate :root_ancestor, to: :namespace, allow_nil: true
delegate :last_pipeline, to: :commit, allow_nil: true delegate :last_pipeline, to: :commit, allow_nil: true

View File

@ -23,10 +23,6 @@ class ProjectTeam
add_user(user, :maintainer, current_user: current_user) add_user(user, :maintainer, current_user: current_user)
end end
def add_owner(user, current_user: nil)
add_user(user, :owner, current_user: current_user)
end
def add_role(user, role, current_user: nil) def add_role(user, role, current_user: nil)
public_send(:"add_#{role}", user, current_user: current_user) # rubocop:disable GitlabSecurity/PublicSend public_send(:"add_#{role}", user, current_user: current_user) # rubocop:disable GitlabSecurity/PublicSend
end end
@ -107,9 +103,7 @@ class ProjectTeam
if group if group
group.owners group.owners
else else
# workaround until we migrate Project#owners to have membership with [project.owner]
# OWNER access level
Array.wrap(fetch_members(Gitlab::Access::OWNER)) | Array.wrap(project.owner)
end end
end end

View File

@ -4,7 +4,7 @@ module Members
module Projects module Projects
class CreatorService < Members::CreatorService class CreatorService < Members::CreatorService
def self.access_levels def self.access_levels
Gitlab::Access.sym_options_with_owner Gitlab::Access.sym_options
end end
private private

View File

@ -14,7 +14,6 @@ module NotificationRecipients
return [] unless project return [] unless project
add_recipients(project.team.maintainers, :mention, nil) add_recipients(project.team.maintainers, :mention, nil)
add_recipients(project.team.owners, :mention, nil)
end end
def acting_user def acting_user

View File

@ -147,11 +147,7 @@ module Projects
priority: UserProjectAccessChangedService::LOW_PRIORITY priority: UserProjectAccessChangedService::LOW_PRIORITY
) )
else else
if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml) @project.add_maintainer(@project.namespace.owner, current_user: current_user)
@project.add_owner(@project.namespace.owner, current_user: current_user)
else
@project.add_maintainer(@project.namespace.owner, current_user: current_user)
end
end end
end end

View File

@ -107,7 +107,7 @@
-# We'll eventually migrate to .gl-display-none: https://gitlab.com/gitlab-org/gitlab/-/issues/351792. -# We'll eventually migrate to .gl-display-none: https://gitlab.com/gitlab-org/gitlab/-/issues/351792.
= gl_badge_tag({ size: :sm, variant: :info }, { class: "js-todos-count gl-ml-n2#{(' hidden' if todos_pending_count == 0)}", "aria-label": _("Todos count") }) do = gl_badge_tag({ size: :sm, variant: :info }, { class: "js-todos-count gl-ml-n2#{(' hidden' if todos_pending_count == 0)}", "aria-label": _("Todos count") }) do
= todos_count_format(todos_pending_count) = todos_count_format(todos_pending_count)
%li.nav-item.header-help.dropdown.d-none.d-md-block{ **tracking_attrs('main_navigation', 'click_question_mark_link', 'navigation') } %li.nav-item.header-help.dropdown.d-none.d-md-block{ data: { track_action: 'click_question_mark_link', track_label: 'main_navigation', track_property: 'navigation', track_experiment: 'cross_stage_fdm' } }
= link_to help_path, class: 'header-help-dropdown-toggle gl-relative', data: { toggle: "dropdown" } do = link_to help_path, class: 'header-help-dropdown-toggle gl-relative', data: { toggle: "dropdown" } do
%span.gl-sr-only %span.gl-sr-only
= s_('Nav|Help') = s_('Nav|Help')

View File

@ -1,6 +1,7 @@
%ul %ul
- if current_user_menu?(:help) - if current_user_menu?(:help)
= render 'layouts/header/gitlab_version' = render 'layouts/header/gitlab_version'
= render_if_exists 'layouts/header/help_dropdown/cross_stage_fdm'
= render 'layouts/header/whats_new_dropdown_item' = render 'layouts/header/whats_new_dropdown_item'
%li %li
= link_to _("Help"), help_path = link_to _("Help"), help_path

View File

@ -107,6 +107,16 @@
= render 'shared/issuable/user_dropdown_item', = render 'shared/issuable/user_dropdown_item',
user: User.new(username: '{{username}}', name: '{{name}}'), user: User.new(username: '{{username}}', name: '{{name}}'),
avatar: { lazy: true, url: '{{avatar_url}}' } avatar: { lazy: true, url: '{{avatar_url}}' }
- if Feature.enabled?(:mr_attention_requests, default_enabled: :yaml)
#js-dropdown-attention-requested.filtered-search-input-dropdown-menu.dropdown-menu
- if current_user
%ul{ data: { dropdown: true } }
= render 'shared/issuable/user_dropdown_item',
user: current_user
%ul.filter-dropdown{ data: { dynamic: true, dropdown: true } }
= render 'shared/issuable/user_dropdown_item',
user: User.new(username: '{{username}}', name: '{{name}}'),
avatar: { lazy: true, url: '{{avatar_url}}' }
= render_if_exists 'shared/issuable/approver_dropdown' = render_if_exists 'shared/issuable/approver_dropdown'
= render_if_exists 'shared/issuable/approved_by_dropdown' = render_if_exists 'shared/issuable/approved_by_dropdown'
#js-dropdown-milestone.filtered-search-input-dropdown-menu.dropdown-menu #js-dropdown-milestone.filtered-search-input-dropdown-menu.dropdown-menu

View File

@ -21,18 +21,68 @@ module ContainerRegistry
repositories = ::ContainerRepository.with_stale_migration(step_before_timestamp) repositories = ::ContainerRepository.with_stale_migration(step_before_timestamp)
.limit(max_capacity) .limit(max_capacity)
aborts_count = 0
long_running_migration_ids = []
# the #to_a is safe as the amount of entries is limited. # the #to_a is safe as the amount of entries is limited.
# In addition, we're calling #each in the next line and we don't want two different SQL queries for these two lines # In addition, we're calling #each in the next line and we don't want two different SQL queries for these two lines
log_extra_metadata_on_done(:stale_migrations_count, repositories.to_a.size) log_extra_metadata_on_done(:stale_migrations_count, repositories.to_a.size)
repositories.each do |repository| repositories.each do |repository|
repository.abort_import if abortable?(repository)
repository.abort_import
aborts_count += 1
else
long_running_migration_ids << repository.id if long_running_migration?(repository)
end
end
log_extra_metadata_on_done(:aborted_stale_migrations_count, aborts_count)
if long_running_migration_ids.any?
log_extra_metadata_on_done(:long_running_stale_migration_container_repository_ids, long_running_migration_ids)
end end
end end
private private
# This can ping the Container Registry API.
# We loop on a set of repositories to calls this function (see #perform)
# In the worst case scenario, we have a n+1 API calls situation here.
#
# This is reasonable because the maximum amount of repositories looped
# on is `25`. See ::ContainerRegistry::Migration.capacity.
#
# TODO We can remove this n+1 situation by having a Container Registry API
# endpoint that accepts multiple repository paths at once. This is issue
# https://gitlab.com/gitlab-org/container-registry/-/issues/582
def abortable?(repository)
# early return to save one Container Registry API request
return true unless repository.importing? || repository.pre_importing?
return true unless external_migration_in_progress?(repository)
false
end
def long_running_migration?(repository)
migration_start_timestamp(repository).before?(long_running_migration_threshold)
end
def external_migration_in_progress?(repository)
status = repository.external_import_status
(status == 'pre_import_in_progress' && repository.pre_importing?) ||
(status == 'import_in_progress' && repository.importing?)
end
def migration_start_timestamp(repository)
if repository.pre_importing?
repository.migration_pre_import_started_at
else
repository.migration_import_started_at
end
end
def step_before_timestamp def step_before_timestamp
::ContainerRegistry::Migration.max_step_duration.seconds.ago ::ContainerRegistry::Migration.max_step_duration.seconds.ago
end end
@ -42,6 +92,10 @@ module ContainerRegistry
# is not properly applied # is not properly applied
::ContainerRegistry::Migration.capacity * 2 ::ContainerRegistry::Migration.capacity * 2
end end
def long_running_migration_threshold
@threshold ||= 30.minutes.ago
end
end end
end end
end end

View File

@ -1,8 +0,0 @@
---
name: diffs_virtual_scrolling
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60312
rollout_issue_url:
milestone: '13.12'
type: development
group: group::code review
default_enabled: true

View File

@ -0,0 +1,8 @@
---
name: permit_all_shared_groups_for_approval
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80655
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352766
milestone: '14.8'
type: development
group: group::source code
default_enabled: false

View File

@ -1,7 +1,7 @@
--- ---
name: personal_project_owner_with_owner_access name: use_traversal_ids_for_self_and_hierarchy_scopes
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78193 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80045
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351919 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/352120
milestone: '14.8' milestone: '14.8'
type: development type: development
group: group::workspace group: group::workspace

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
class AddIndexToMergeRequestAssigneesState < Gitlab::Database::Migration[1.0]
disable_ddl_transaction!
INDEX_NAME = 'index_on_merge_request_assignees_state'
def up
add_concurrent_index :merge_request_assignees, :state, where: 'state = 2', name: INDEX_NAME
end
def down
remove_concurrent_index_by_name :merge_request_assignees, INDEX_NAME
end
end

View File

@ -0,0 +1,15 @@
# frozen_string_literal: true
class AddIndexToMergeRequestReviewersState < Gitlab::Database::Migration[1.0]
disable_ddl_transaction!
INDEX_NAME = 'index_on_merge_request_reviewers_state'
def up
add_concurrent_index :merge_request_reviewers, :state, where: 'state = 2', name: INDEX_NAME
end
def down
remove_concurrent_index_by_name :merge_request_reviewers, INDEX_NAME
end
end

View File

@ -0,0 +1 @@
7707b9bcdcd7ec28af31b90355905eb8971c12a90c4334b0ae214e45fac9645a

View File

@ -0,0 +1 @@
350409be3f491b61a1d757dbd839b48d088339883e8e019d00ad90e6adc350e6

View File

@ -27129,6 +27129,10 @@ CREATE UNIQUE INDEX index_on_instance_statistics_recorded_at_and_identifier ON a
CREATE INDEX index_on_label_links_all_columns ON label_links USING btree (target_id, label_id, target_type); CREATE INDEX index_on_label_links_all_columns ON label_links USING btree (target_id, label_id, target_type);
CREATE INDEX index_on_merge_request_assignees_state ON merge_request_assignees USING btree (state) WHERE (state = 2);
CREATE INDEX index_on_merge_request_reviewers_state ON merge_request_reviewers USING btree (state) WHERE (state = 2);
CREATE INDEX index_on_merge_requests_for_latest_diffs ON merge_requests USING btree (target_project_id) INCLUDE (id, latest_merge_request_diff_id); CREATE INDEX index_on_merge_requests_for_latest_diffs ON merge_requests USING btree (target_project_id) INCLUDE (id, latest_merge_request_diff_id);
COMMENT ON INDEX index_on_merge_requests_for_latest_diffs IS 'Index used to efficiently obtain the oldest merge request for a commit SHA'; COMMENT ON INDEX index_on_merge_requests_for_latest_diffs IS 'Index used to efficiently obtain the oldest merge request for a commit SHA';

View File

@ -71,7 +71,7 @@ For example, many projects do releases but don't need to do hotfixes.
## GitHub flow as a simpler alternative ## GitHub flow as a simpler alternative
In reaction to Git flow, GitHub created a simpler alternative. In reaction to Git flow, GitHub created a simpler alternative.
[GitHub flow](https://guides.github.com/introduction/flow/index.html) has only feature branches and a `main` branch: [GitHub flow](https://docs.github.com/en/get-started/quickstart/github-flow) has only feature branches and a `main` branch:
```mermaid ```mermaid
graph TD graph TD
@ -341,7 +341,7 @@ However, as discussed in [the section about rebasing](#squashing-commits-with-re
Rebasing could create more work, as every time you rebase, you may need to resolve the same conflicts. Rebasing could create more work, as every time you rebase, you may need to resolve the same conflicts.
Sometimes you can reuse recorded resolutions (`rerere`), but merging is better, because you only have to resolve conflicts once. Sometimes you can reuse recorded resolutions (`rerere`), but merging is better, because you only have to resolve conflicts once.
Atlassian has a more thorough explanation of the tradeoffs between merging and rebasing [on their blog](https://www.atlassian.com/blog/git/git-team-workflows-merge-or-rebase). Atlassian has [a more thorough explanation of the tradeoffs between merging and rebasing](https://www.atlassian.com/blog/git/git-team-workflows-merge-or-rebase) on their blog.
A good way to prevent creating many merge commits is to not frequently merge `main` into the feature branch. A good way to prevent creating many merge commits is to not frequently merge `main` into the feature branch.
There are three reasons to merge in `main`: utilizing new code, resolving merge conflicts, and updating long-running branches. There are three reasons to merge in `main`: utilizing new code, resolving merge conflicts, and updating long-running branches.
@ -403,7 +403,7 @@ git commit -m 'Properly escape special characters in XML generation'
An example of a good commit message is: "Combine templates to reduce duplicate code in the user views." An example of a good commit message is: "Combine templates to reduce duplicate code in the user views."
The words "change," "improve," "fix," and "refactor" don't add much information to a commit message. The words "change," "improve," "fix," and "refactor" don't add much information to a commit message.
For more information about formatting commit messages, please see this excellent [blog post by Tim Pope](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html). For more information, please see Tim Pope's excellent [note about formatting commit messages](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html).
To add more context to a commit message, consider adding information regarding the To add more context to a commit message, consider adding information regarding the
origin of the change. For example, the URL of a GitLab issue, or a Jira issue number, origin of the change. For example, the URL of a GitLab issue, or a Jira issue number,

View File

@ -351,7 +351,8 @@ or [init scripts](upgrading_from_source.md#configure-sysv-init-script) by [follo
In 14.5 we introduce a set of migrations that wrap up this process by making sure In 14.5 we introduce a set of migrations that wrap up this process by making sure
that all remaining jobs over the `merge_request_diff_commits` table are completed. that all remaining jobs over the `merge_request_diff_commits` table are completed.
These jobs will have already been processed in most cases so that no extra time is necessary during an upgrade to 14.5. These jobs will have already been processed in most cases so that no extra time is necessary during an upgrade to 14.5.
But if there are remaining jobs, the deployment may take a few extra minutes to complete. However, if there are remaining jobs or you haven't already upgraded to 14.1,
the deployment may take multiple hours to complete.
All merge request diff commits will automatically incorporate these changes, and there are no All merge request diff commits will automatically incorporate these changes, and there are no
additional requirements to perform the upgrade. additional requirements to perform the upgrade.
@ -439,6 +440,11 @@ for how to proceed.
with self-managed installations, and ensure 14.0.0-14.0.4 installations do not with self-managed installations, and ensure 14.0.0-14.0.4 installations do not
encounter issues with [batched background migrations](#batched-background-migrations). encounter issues with [batched background migrations](#batched-background-migrations).
- Upgrading to GitLab [14.5](#1450) (or later) may take a lot longer if you do not upgrade to at least 14.1
first. The 14.1 merge request diff commits database migration can take hours to run, but runs in the
background while GitLab is in use. GitLab instances upgraded directly from 14.0 to 14.5 or later must
run the migration in the foreground and therefore take a lot longer to complete.
- See [Maintenance mode issue in GitLab 13.9 to 14.4](#maintenance-mode-issue-in-gitlab-139-to-144). - See [Maintenance mode issue in GitLab 13.9 to 14.4](#maintenance-mode-issue-in-gitlab-139-to-144).
### 14.0.0 ### 14.0.0

View File

@ -11,7 +11,7 @@ When you are using the GitLab Agent for Kubernetes, you might experience issues
You can start by viewing the service logs: You can start by viewing the service logs:
```shell ```shell
kubectl logs -f -l=app=gitlab-kubernetes-agent -n gitlab-kubernetes-agent kubectl logs -f -l=app=gitlab-agent -n gitlab-kubernetes-agent
``` ```
If you are a GitLab administrator, you can also view the [GitLab Agent Server logs](../../../administration/clusters/kas.md#troubleshooting). If you are a GitLab administrator, you can also view the [GitLab Agent Server logs](../../../administration/clusters/kas.md#troubleshooting).

View File

@ -33,27 +33,14 @@ usernames. A GitLab administrator can configure the GitLab instance to
## Project members permissions ## Project members permissions
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8, personal namespace owners appear with Owner role in new projects in their namespace. Introduced [with a flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. Disabled by default.
FLAG:
On self-managed GitLab, personal namespace owners appearing with the Owner role in new projects in their namespace is disabled. To make it available,
ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`.
The feature is not ready for production use.
On GitLab.com, this feature is not available.
A user's role determines what permissions they have on a project. The Owner role provides all permissions but is A user's role determines what permissions they have on a project. The Owner role provides all permissions but is
available only: available only:
- For group owners. The role is inherited for a group's projects. - For group owners. The role is inherited for a group's projects.
- For Administrators. - For Administrators.
Personal [namespace](group/index.md#namespaces) owners: Personal namespace owners have the same permissions as an Owner, but are displayed with the Maintainer role on projects created in their personal namespace.
For more information, see [projects members documentation](project/members/index.md).
- Are displayed as having the Maintainer role on projects in the namespace, but have the same permissions as a user with the Owner role.
- (Disabled by default) In GitLab 14.8 and later, for new projects in the namespace, are displayed as having the Owner role.
For more information about how to manage project members, see
[members of a project](project/members/index.md).
The following table lists project permissions available for each role: The following table lists project permissions available for each role:

View File

@ -32,24 +32,33 @@ in the search field in the upper right corner:
### Filter issue and merge request lists ### Filter issue and merge request lists
> - Filter by Epics was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/195704) in GitLab Ultimate 12.9. > - Filtering by epics was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/195704) in GitLab 12.9.
> - Filter by child Epics was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9029) in GitLab Ultimate 13.0. > - Filtering by child epics was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9029) in GitLab 13.0.
> - Filter by Iterations was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/118742) in GitLab 13.6. Moved to GitLab Premium in 13.9. > - Filtering by iterations was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/118742) in GitLab 13.6. Moved from GitLab Ultimate to Premium in 13.9.
> - Filtering by type was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322755) in GitLab 13.10 [with a flag](../../administration/feature_flags.md) named `vue_issues_list`. Disabled by default.
Follow these steps to filter the **Issues** and **Merge requests** list pages in projects and Follow these steps to filter the **Issues** and **Merge requests** list pages in projects and
groups: groups:
1. Click in the field **Search or filter results...**. 1. Click in the field **Search or filter results...**.
1. In the dropdown menu that appears, select the attribute you wish to filter by: 1. In the dropdown menu that appears, select the attribute you wish to filter by:
- Author
- Assignee - Assignee
- [Milestone](../project/milestones/index.md) - Author
- [Iteration](../group/iterations/index.md)
- Release
- [Label](../project/labels.md)
- My-reaction
- Confidential - Confidential
- [Epic and child Epic](../group/epics/index.md) (available only for the group the Epic was created, not for [higher group levels](https://gitlab.com/gitlab-org/gitlab/-/issues/233729)). - [Epic and child Epic](../group/epics/index.md) (available only for the group the Epic was created, not for [higher group levels](https://gitlab.com/gitlab-org/gitlab/-/issues/233729)).
- [Iteration](../group/iterations/index.md)
- [Label](../project/labels.md)
- [Milestone](../project/milestones/index.md)
- My-reaction
- Release
- Type
FLAG:
On self-managed GitLab, by default filtering by type is not available.
To make it available per group, ask an administrator to [enable the feature flag](../../administration/feature_flags.md) named `vue_issues_list`.
On GitLab.com, this feature is not available.
- Weight
- Search for this text - Search for this text
1. Select or type the operator to use for filtering the attribute. The following operators are 1. Select or type the operator to use for filtering the attribute. The following operators are
available: available:

View File

@ -25,6 +25,7 @@ module ContainerRegistry
end end
end end
# https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#compliance-check
def supports_gitlab_api? def supports_gitlab_api?
strong_memoize(:supports_gitlab_api) do strong_memoize(:supports_gitlab_api) do
registry_features = Gitlab::CurrentSettings.container_registry_features || [] registry_features = Gitlab::CurrentSettings.container_registry_features || []
@ -35,16 +36,19 @@ module ContainerRegistry
end end
end end
# https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository
def pre_import_repository(path) def pre_import_repository(path)
response = start_import_for(path, pre: true) response = start_import_for(path, pre: true)
IMPORT_RESPONSES.fetch(response.status, :error) IMPORT_RESPONSES.fetch(response.status, :error)
end end
# https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository
def import_repository(path) def import_repository(path)
response = start_import_for(path, pre: false) response = start_import_for(path, pre: false)
IMPORT_RESPONSES.fetch(response.status, :error) IMPORT_RESPONSES.fetch(response.status, :error)
end end
# https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#get-repository-import-status
def import_status(path) def import_status(path)
body_hash = response_body(faraday.get(import_url_for(path))) body_hash = response_body(faraday.get(import_url_for(path)))
body_hash['status'] || 'error' body_hash['status'] || 'error'

View File

@ -34,6 +34,11 @@ module ContainerRegistry
end end
def self.capacity def self.capacity
# Increasing capacity numbers will increase the n+1 API calls we can have
# in ContainerRegistry::Migration::GuardWorker#external_migration_in_progress?
#
# TODO: See https://gitlab.com/gitlab-org/container-registry/-/issues/582
#
return 25 if Feature.enabled?(:container_registry_migration_phase2_capacity_25) return 25 if Feature.enabled?(:container_registry_migration_phase2_capacity_25)
return 10 if Feature.enabled?(:container_registry_migration_phase2_capacity_10) return 10 if Feature.enabled?(:container_registry_migration_phase2_capacity_10)
return 1 if Feature.enabled?(:container_registry_migration_phase2_capacity_1) return 1 if Feature.enabled?(:container_registry_migration_phase2_capacity_1)

View File

@ -33,13 +33,7 @@ module Gitlab
MAINTAINER_SUBGROUP_ACCESS = 1 MAINTAINER_SUBGROUP_ACCESS = 1
class << self class << self
def values delegate :values, to: :options
if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml)
options_with_owner.values
else
options.values
end
end
def all_values def all_values
options_with_owner.values options_with_owner.values

View File

@ -22,7 +22,7 @@ module Gitlab
user.projects_with_active_memberships.select_for_project_authorization, user.projects_with_active_memberships.select_for_project_authorization,
# The personal projects of the user. # The personal projects of the user.
user.personal_projects.select_project_owner_for_project_authorization, user.personal_projects.select_as_maintainer_for_project_authorization,
# Projects that belong directly to any of the groups the user has # Projects that belong directly to any of the groups the user has
# access to. # access to.

View File

@ -4999,6 +4999,9 @@ msgstr[1] ""
msgid "Attaching the file failed." msgid "Attaching the file failed."
msgstr "" msgstr ""
msgid "Attention"
msgstr ""
msgid "Audit Events" msgid "Audit Events"
msgstr "" msgstr ""
@ -18605,6 +18608,9 @@ msgstr ""
msgid "InProductMarketing|3 ways to dive into GitLab CI/CD" msgid "InProductMarketing|3 ways to dive into GitLab CI/CD"
msgstr "" msgstr ""
msgid "InProductMarketing|Access advanced features, build more efficiently, strengthen security and compliance."
msgstr ""
msgid "InProductMarketing|Actually, GitLab makes the team work (better)" msgid "InProductMarketing|Actually, GitLab makes the team work (better)"
msgstr "" msgstr ""
@ -18638,6 +18644,9 @@ msgstr ""
msgid "InProductMarketing|Code owners and required merge approvals are part of the paid tiers of GitLab. You can start a free 30-day trial of GitLab Ultimate and enable these features in less than 5 minutes with no credit card required." msgid "InProductMarketing|Code owners and required merge approvals are part of the paid tiers of GitLab. You can start a free 30-day trial of GitLab Ultimate and enable these features in less than 5 minutes with no credit card required."
msgstr "" msgstr ""
msgid "InProductMarketing|Collaboration across stages in GitLab"
msgstr ""
msgid "InProductMarketing|Create a custom CI runner with just a few clicks" msgid "InProductMarketing|Create a custom CI runner with just a few clicks"
msgstr "" msgstr ""
@ -18662,6 +18671,12 @@ msgstr ""
msgid "InProductMarketing|Dig in and create a project and a repo" msgid "InProductMarketing|Dig in and create a project and a repo"
msgstr "" msgstr ""
msgid "InProductMarketing|Discover Premium & Ultimate"
msgstr ""
msgid "InProductMarketing|Discover Premium & Ultimate."
msgstr ""
msgid "InProductMarketing|Do you have a minute?" msgid "InProductMarketing|Do you have a minute?"
msgstr "" msgstr ""
@ -18869,6 +18884,9 @@ msgstr ""
msgid "InProductMarketing|Start a Self-Managed trial" msgid "InProductMarketing|Start a Self-Managed trial"
msgstr "" msgstr ""
msgid "InProductMarketing|Start a free trial"
msgstr ""
msgid "InProductMarketing|Start a free trial of GitLab Ultimate no credit card required" msgid "InProductMarketing|Start a free trial of GitLab Ultimate no credit card required"
msgstr "" msgstr ""
@ -40305,6 +40323,15 @@ msgstr ""
msgid "VulnerabilityManagement|Create Jira issue" msgid "VulnerabilityManagement|Create Jira issue"
msgstr "" msgstr ""
msgid "VulnerabilityManagement|Enter a name"
msgstr ""
msgid "VulnerabilityManagement|Enter the CVE or CWE code"
msgstr ""
msgid "VulnerabilityManagement|Enter the CVE or CWE identifier URL"
msgstr ""
msgid "VulnerabilityManagement|Fetching linked Jira issues" msgid "VulnerabilityManagement|Fetching linked Jira issues"
msgstr "" msgstr ""
@ -40329,6 +40356,12 @@ msgstr ""
msgid "VulnerabilityManagement|Select a method" msgid "VulnerabilityManagement|Select a method"
msgstr "" msgstr ""
msgid "VulnerabilityManagement|Select a severity level"
msgstr ""
msgid "VulnerabilityManagement|Select a status"
msgstr ""
msgid "VulnerabilityManagement|Severity is a required field" msgid "VulnerabilityManagement|Severity is a required field"
msgstr "" msgstr ""

View File

@ -665,7 +665,7 @@ RSpec.describe Projects::ProjectMembersController do
sign_in(user) sign_in(user)
end end
it 'creates a member' do it 'does not create a member' do
expect do expect do
post :create, params: { post :create, params: {
user_ids: stranger.id, user_ids: stranger.id,
@ -673,9 +673,7 @@ RSpec.describe Projects::ProjectMembersController do
access_level: Member::OWNER, access_level: Member::OWNER,
project_id: project project_id: project
} }
end.to change { project.members.count }.by(1) end.to change { project.members.count }.by(0)
expect(project.team_members).to include(user)
end end
end end

View File

@ -32,7 +32,7 @@ RSpec.describe 'Merge request > Batch comments', :js do
expect(page).to have_selector('[data-testid="review_bar_component"]') expect(page).to have_selector('[data-testid="review_bar_component"]')
expect(find('.review-bar-content .btn-confirm')).to have_content('1') expect(find('[data-testid="review_bar_component"] .btn-confirm')).to have_content('1')
end end
it 'publishes review' do it 'publishes review' do
@ -150,10 +150,6 @@ RSpec.describe 'Merge request > Batch comments', :js do
it 'adds draft comments to both sides' do it 'adds draft comments to both sides' do
write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9') write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9')
# make sure line 9 is in the view
execute_script("window.scrollBy(0, -200)")
write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9', button_text: 'Add to review', text: 'Another wrong line') write_parallel_comment('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9', button_text: 'Add to review', text: 'Another wrong line')
expect(find('.new .draft-note-component')).to have_content('Line is wrong') expect(find('.new .draft-note-component')).to have_content('Line is wrong')
@ -255,13 +251,15 @@ RSpec.describe 'Merge request > Batch comments', :js do
end end
def write_diff_comment(**params) def write_diff_comment(**params)
click_diff_line(find("[id='#{sample_compare.changes[0][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[0][:line_code]}']"))
write_comment(**params) write_comment(**params)
end end
def write_parallel_comment(line, **params) def write_parallel_comment(line, **params)
find("div[id='#{line}']").hover line_element = find_by_scrolling("[id='#{line}']")
scroll_to_elements_bottom(line_element)
line_element.hover
find(".js-add-diff-note-button").click find(".js-add-diff-note-button").click
write_comment(selector: "form[data-line-code='#{line}']", **params) write_comment(selector: "form[data-line-code='#{line}']", **params)

View File

@ -25,14 +25,15 @@ RSpec.describe 'User comments on a diff', :js do
context 'when toggling inline comments' do context 'when toggling inline comments' do
context 'in a single file' do context 'in a single file' do
it 'hides a comment' do it 'hides a comment' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) line_element = find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']").find(:xpath, "..")
click_diff_line(line_element)
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'Line is wrong') fill_in('note_note', with: 'Line is wrong')
click_button('Add comment now') click_button('Add comment now')
end end
page.within('.diff-files-holder > div:nth-child(6)') do page.within(line_element.ancestor('[data-path]')) do
expect(page).to have_content('Line is wrong') expect(page).to have_content('Line is wrong')
find('.js-diff-more-actions').click find('.js-diff-more-actions').click
@ -45,7 +46,9 @@ RSpec.describe 'User comments on a diff', :js do
context 'in multiple files' do context 'in multiple files' do
it 'toggles comments' do it 'toggles comments' do
click_diff_line(find("[id='#{sample_compare.changes[0][:line_code]}']")) first_line_element = find_by_scrolling("[id='#{sample_compare.changes[0][:line_code]}']").find(:xpath, "..")
first_root_element = first_line_element.ancestor('[data-path]')
click_diff_line(first_line_element)
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'Line is correct') fill_in('note_note', with: 'Line is correct')
@ -54,11 +57,14 @@ RSpec.describe 'User comments on a diff', :js do
wait_for_requests wait_for_requests
page.within('.diff-files-holder > div:nth-child(5) .note-body > .note-text') do page.within(first_root_element) do
expect(page).to have_content('Line is correct') expect(page).to have_content('Line is correct')
end end
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) second_line_element = find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']")
second_root_element = second_line_element.ancestor('[data-path]')
click_diff_line(second_line_element)
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'Line is wrong') fill_in('note_note', with: 'Line is wrong')
@ -68,7 +74,7 @@ RSpec.describe 'User comments on a diff', :js do
wait_for_requests wait_for_requests
# Hide the comment. # Hide the comment.
page.within('.diff-files-holder > div:nth-child(6)') do page.within(second_root_element) do
find('.js-diff-more-actions').click find('.js-diff-more-actions').click
click_button 'Hide comments on this file' click_button 'Hide comments on this file'
@ -77,37 +83,36 @@ RSpec.describe 'User comments on a diff', :js do
# At this moment a user should see only one comment. # At this moment a user should see only one comment.
# The other one should be hidden. # The other one should be hidden.
page.within('.diff-files-holder > div:nth-child(5) .note-body > .note-text') do page.within(first_root_element) do
expect(page).to have_content('Line is correct') expect(page).to have_content('Line is correct')
end end
# Show the comment. # Show the comment.
page.within('.diff-files-holder > div:nth-child(6)') do page.within(second_root_element) do
find('.js-diff-more-actions').click find('.js-diff-more-actions').click
click_button 'Show comments on this file' click_button 'Show comments on this file'
end end
# Now both the comments should be shown. # Now both the comments should be shown.
page.within('.diff-files-holder > div:nth-child(6) .note-body > .note-text') do page.within(second_root_element) do
expect(page).to have_content('Line is wrong') expect(page).to have_content('Line is wrong')
end end
page.within('.diff-files-holder > div:nth-child(5) .note-body > .note-text') do page.within(first_root_element) do
expect(page).to have_content('Line is correct') expect(page).to have_content('Line is correct')
end end
# Check the same comments in the side-by-side view. # Check the same comments in the side-by-side view.
execute_script("window.scrollTo(0,0);")
find('.js-show-diff-settings').click find('.js-show-diff-settings').click
click_button 'Side-by-side' click_button 'Side-by-side'
wait_for_requests wait_for_requests
page.within('.diff-files-holder > div:nth-child(6) .parallel .note-body > .note-text') do page.within(second_root_element) do
expect(page).to have_content('Line is wrong') expect(page).to have_content('Line is wrong')
end end
page.within('.diff-files-holder > div:nth-child(5) .parallel .note-body > .note-text') do page.within(first_root_element) do
expect(page).to have_content('Line is correct') expect(page).to have_content('Line is correct')
end end
end end
@ -121,7 +126,7 @@ RSpec.describe 'User comments on a diff', :js do
context 'when adding multiline comments' do context 'when adding multiline comments' do
it 'saves a multiline comment' do it 'saves a multiline comment' do
click_diff_line(find("[id='#{sample_commit.line_code}']")) click_diff_line(find_by_scrolling("[id='#{sample_commit.line_code}']").find(:xpath, '..'))
add_comment('-13', '+14') add_comment('-13', '+14')
end end
@ -133,13 +138,13 @@ RSpec.describe 'User comments on a diff', :js do
# In `files/ruby/popen.rb` # In `files/ruby/popen.rb`
it 'allows comments for changes involving both sides' do it 'allows comments for changes involving both sides' do
# click +15, select -13 add and verify comment # click +15, select -13 add and verify comment
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .right-side a[data-linenumber="15"]').find(:xpath, '../../..'), 'right') click_diff_line(find_by_scrolling('div[data-path="files/ruby/popen.rb"] .right-side a[data-linenumber="15"]').find(:xpath, '../../..'), 'right')
add_comment('-13', '+15') add_comment('-13', '+15')
end end
it 'allows comments on previously hidden lines at the top of a file', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/285294' do it 'allows comments on previously hidden lines at the top of a file', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/285294' do
# Click -9, expand up, select 1 add and verify comment # Click -9, expand up, select 1 add and verify comment
page.within('[data-path="files/ruby/popen.rb"]') do page.within find_by_scrolling('[data-path="files/ruby/popen.rb"]') do
all('.js-unfold-all')[0].click all('.js-unfold-all')[0].click
end end
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="9"]').find(:xpath, '../..'), 'left') click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="9"]').find(:xpath, '../..'), 'left')
@ -148,7 +153,7 @@ RSpec.describe 'User comments on a diff', :js do
it 'allows comments on previously hidden lines the middle of a file' do it 'allows comments on previously hidden lines the middle of a file' do
# Click 27, expand up, select 18, add and verify comment # Click 27, expand up, select 18, add and verify comment
page.within('[data-path="files/ruby/popen.rb"]') do page.within find_by_scrolling('[data-path="files/ruby/popen.rb"]') do
all('.js-unfold-all')[1].click all('.js-unfold-all')[1].click
end end
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="21"]').find(:xpath, '../..'), 'left') click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="21"]').find(:xpath, '../..'), 'left')
@ -157,7 +162,7 @@ RSpec.describe 'User comments on a diff', :js do
it 'allows comments on previously hidden lines at the bottom of a file' do it 'allows comments on previously hidden lines at the bottom of a file' do
# Click +28, expand down, select 37 add and verify comment # Click +28, expand down, select 37 add and verify comment
page.within('[data-path="files/ruby/popen.rb"]') do page.within find_by_scrolling('[data-path="files/ruby/popen.rb"]') do
all('.js-unfold-down:not([disabled])')[1].click all('.js-unfold-down:not([disabled])')[1].click
end end
click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="30"]').find(:xpath, '../..'), 'left') click_diff_line(find('div[data-path="files/ruby/popen.rb"] .left-side a[data-linenumber="30"]').find(:xpath, '../..'), 'left')
@ -198,7 +203,7 @@ RSpec.describe 'User comments on a diff', :js do
context 'when editing comments' do context 'when editing comments' do
it 'edits a comment' do it 'edits a comment' do
click_diff_line(find("[id='#{sample_commit.line_code}']")) click_diff_line(find_by_scrolling("[id='#{sample_commit.line_code}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in(:note_note, with: 'Line is wrong') fill_in(:note_note, with: 'Line is wrong')
@ -224,7 +229,7 @@ RSpec.describe 'User comments on a diff', :js do
context 'when deleting comments' do context 'when deleting comments' do
it 'deletes a comment' do it 'deletes a comment' do
click_diff_line(find("[id='#{sample_commit.line_code}']")) click_diff_line(find_by_scrolling("[id='#{sample_commit.line_code}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in(:note_note, with: 'Line is wrong') fill_in(:note_note, with: 'Line is wrong')

View File

@ -7,7 +7,7 @@ RSpec.describe 'User expands diff', :js do
let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) } let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) }
before do before do
allow(Gitlab::CurrentSettings).to receive(:diff_max_patch_bytes).and_return(100.kilobytes) allow(Gitlab::CurrentSettings).to receive(:diff_max_patch_bytes).and_return(100.bytes)
visit(diffs_project_merge_request_path(project, merge_request)) visit(diffs_project_merge_request_path(project, merge_request))
@ -15,7 +15,7 @@ RSpec.describe 'User expands diff', :js do
end end
it 'allows user to expand diff' do it 'allows user to expand diff' do
page.within find('[id="19763941ab80e8c09871c0a425f0560d9053bcb3"]') do page.within find("[id='4c76a1271e41072d7da9fe40bf0f79f7384d472a']") do
find('[data-testid="expand-button"]').click find('[data-testid="expand-button"]').click
wait_for_requests wait_for_requests

View File

@ -15,16 +15,14 @@ RSpec.describe 'Batch diffs', :js do
visit diffs_project_merge_request_path(merge_request.project, merge_request) visit diffs_project_merge_request_path(merge_request.project, merge_request)
wait_for_requests wait_for_requests
# Add discussion to first line of first file click_diff_line(get_first_diff.find('[data-testid="left-side"]', match: :first))
click_diff_line(find('.diff-file.file-holder:first-of-type .line_holder .left-side:first-of-type')) page.within get_first_diff.find('.js-discussion-note-form') do
page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'First Line Comment') fill_in('note_note', with: 'First Line Comment')
click_button('Add comment now') click_button('Add comment now')
end end
# Add discussion to first line of last file click_diff_line(get_second_diff.find('[data-testid="left-side"]', match: :first))
click_diff_line(find('.diff-file.file-holder:last-of-type .line_holder .left-side:first-of-type')) page.within get_second_diff.find('.js-discussion-note-form') do
page.within('.js-discussion-note-form') do
fill_in('note_note', with: 'Last Line Comment') fill_in('note_note', with: 'Last Line Comment')
click_button('Add comment now') click_button('Add comment now')
end end
@ -36,17 +34,14 @@ RSpec.describe 'Batch diffs', :js do
# Reload so we know the discussions are persisting across batch loads # Reload so we know the discussions are persisting across batch loads
visit page.current_url visit page.current_url
# Wait for JS to settle
wait_for_requests wait_for_requests
expect(page).to have_selector('.diff-files-holder .file-holder', count: 39)
# Confirm discussions are applied to appropriate files (should be contained in multiple diff pages) # Confirm discussions are applied to appropriate files (should be contained in multiple diff pages)
page.within('.diff-file.file-holder:first-of-type .notes .timeline-entry .note .note-text') do page.within get_first_diff.find('.notes .timeline-entry .note .note-text') do
expect(page).to have_content('First Line Comment') expect(page).to have_content('First Line Comment')
end end
page.within('.diff-file.file-holder:last-of-type .notes .timeline-entry .note .note-text') do page.within get_second_diff.find('.notes .timeline-entry .note .note-text') do
expect(page).to have_content('Last Line Comment') expect(page).to have_content('Last Line Comment')
end end
end end
@ -54,7 +49,7 @@ RSpec.describe 'Batch diffs', :js do
context 'when user visits a URL with a link directly to to a discussion' do context 'when user visits a URL with a link directly to to a discussion' do
context 'which is in the first batched page of diffs' do context 'which is in the first batched page of diffs' do
it 'scrolls to the correct discussion' do it 'scrolls to the correct discussion' do
page.within('.diff-file.file-holder:first-of-type') do page.within get_first_diff do
click_link('just now') click_link('just now')
end end
@ -63,15 +58,15 @@ RSpec.describe 'Batch diffs', :js do
wait_for_requests wait_for_requests
# Confirm scrolled to correct UI element # Confirm scrolled to correct UI element
expect(page.find('.diff-file.file-holder:first-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_falsey expect(get_first_diff.find('.discussion-notes .timeline-entry li.note[id]').obscured?).to be_falsey
expect(page.find('.diff-file.file-holder:last-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_truthy expect(get_second_diff.find('.discussion-notes .timeline-entry li.note[id]').obscured?).to be_truthy
end end
end end
context 'which is in at least page 2 of the batched pages of diffs' do context 'which is in at least page 2 of the batched pages of diffs' do
it 'scrolls to the correct discussion', it 'scrolls to the correct discussion',
quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/293814' } do quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/293814' } do
page.within('.diff-file.file-holder:last-of-type') do page.within get_first_diff do
click_link('just now') click_link('just now')
end end
@ -80,8 +75,8 @@ RSpec.describe 'Batch diffs', :js do
wait_for_requests wait_for_requests
# Confirm scrolled to correct UI element # Confirm scrolled to correct UI element
expect(page.find('.diff-file.file-holder:first-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_truthy expect(get_first_diff.find('.discussion-notes .timeline-entry li.note[id]').obscured?).to be_truthy
expect(page.find('.diff-file.file-holder:last-of-type .discussion-notes .timeline-entry li.note[id]').obscured?).to be_falsey expect(get_second_diff.find('.discussion-notes .timeline-entry li.note[id]').obscured?).to be_falsey
end end
end end
end end
@ -95,15 +90,21 @@ RSpec.describe 'Batch diffs', :js do
end end
it 'has the correct discussions applied to files across batched pages' do it 'has the correct discussions applied to files across batched pages' do
expect(page).to have_selector('.diff-files-holder .file-holder', count: 39) page.within get_first_diff.find('.notes .timeline-entry .note .note-text') do
page.within('.diff-file.file-holder:first-of-type .notes .timeline-entry .note .note-text') do
expect(page).to have_content('First Line Comment') expect(page).to have_content('First Line Comment')
end end
page.within('.diff-file.file-holder:last-of-type .notes .timeline-entry .note .note-text') do page.within get_second_diff.find('.notes .timeline-entry .note .note-text') do
expect(page).to have_content('Last Line Comment') expect(page).to have_content('Last Line Comment')
end end
end end
end end
def get_first_diff
find('#a9b6f940524f646951cc28d954aa41f814f95d4f')
end
def get_second_diff
find('#b285a86891571c7fdbf1f82e840816079de1cc8b')
end
end end

View File

@ -29,54 +29,54 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'with an old line on the left and no line on the right' do context 'with an old line on the left and no line on the right' do
it 'allows commenting on the left side' do it 'allows commenting on the left side' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]'), 'left') should_allow_commenting(find_by_scrolling('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]'), 'left')
end end
it 'does not allow commenting on the right side' do it 'does not allow commenting on the right side' do
should_not_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right') should_not_allow_commenting(find_by_scrolling('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right')
end end
end end
context 'with no line on the left and a new line on the right' do context 'with no line on the left and a new line on the right' do
it 'does not allow commenting on the left side' do it 'does not allow commenting on the left side' do
should_not_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left') should_not_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left')
end end
it 'allows commenting on the right side' do it 'allows commenting on the right side' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right') should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right')
end end
end end
context 'with an old line on the left and a new line on the right' do context 'with an old line on the left and a new line on the right' do
it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/199050' do it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/199050' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left') should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left')
end end
it 'allows commenting on the right side' do it 'allows commenting on the right side' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right') should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right')
end end
end end
context 'with an unchanged line on the left and an unchanged line on the right' do context 'with an unchanged line on the left and an unchanged line on the right' do
it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/196826' do it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/196826' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'left') should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'left')
end end
it 'allows commenting on the right side' do it 'allows commenting on the right side' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'right') should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'right')
end end
end end
context 'with a match line' do context 'with a match line' do
it 'does not allow commenting' do it 'does not allow commenting' do
line_holder = find('.match', match: :first) line_holder = find_by_scrolling('.match', match: :first)
match_should_not_allow_commenting(line_holder) match_should_not_allow_commenting(line_holder)
end end
end end
context 'with an unfolded line' do context 'with an unfolded line' do
before do before do
page.within('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"]') do page.within find_by_scrolling('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"]') do
find('.js-unfold', match: :first).click find('.js-unfold', match: :first).click
end end
@ -84,12 +84,12 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
end end
it 'allows commenting on the left side' do it 'allows commenting on the left side' do
should_allow_commenting(first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder [data-testid="left-side"]')) should_allow_commenting(find_by_scrolling('#a5cc2925ca8258af241be7e5b0381edf30266302').first('.line_holder [data-testid="left-side"]'))
end end
it 'allows commenting on the right side' do it 'allows commenting on the right side' do
# Automatically shifts comment box to left side. # Automatically shifts comment box to left side.
should_allow_commenting(first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder [data-testid="right-side"]')) should_allow_commenting(find_by_scrolling('#a5cc2925ca8258af241be7e5b0381edf30266302').first('.line_holder [data-testid="right-side"]'))
end end
end end
end end
@ -101,44 +101,44 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'after deleteing a note' do context 'after deleteing a note' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
accept_gl_confirm(button_text: 'Delete Comment') do accept_gl_confirm(button_text: 'Delete Comment') do
first('button.more-actions-toggle').click first('button.more-actions-toggle').click
first('.js-note-delete').click first('.js-note-delete').click
end end
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
end end
end end
context 'with a new line' do context 'with a new line' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
end end
end end
context 'with an old line' do context 'with an old line' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) should_allow_commenting(find_by_scrolling('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
end end
end end
context 'with an unchanged line' do context 'with an unchanged line' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
end end
end end
context 'with a match line' do context 'with a match line' do
it 'does not allow commenting' do it 'does not allow commenting' do
match_should_not_allow_commenting(find('.match', match: :first)) match_should_not_allow_commenting(find_by_scrolling('.match', match: :first))
end end
end end
context 'with an unfolded line' do context 'with an unfolded line' do
before do before do
page.within('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"]') do page.within find_by_scrolling('.file-holder[id="a5cc2925ca8258af241be7e5b0381edf30266302"]') do
find('.js-unfold', match: :first).click find('.js-unfold', match: :first).click
end end
@ -147,7 +147,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
# The first `.js-unfold` unfolds upwards, therefore the first # The first `.js-unfold` unfolds upwards, therefore the first
# `.line_holder` will be an unfolded line. # `.line_holder` will be an unfolded line.
let(:line_holder) { first('[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') } let(:line_holder) { find_by_scrolling('[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') }
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting line_holder should_allow_commenting line_holder
@ -157,7 +157,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'when hovering over a diff discussion' do context 'when hovering over a diff discussion' do
before do before do
visit diffs_project_merge_request_path(project, merge_request, view: 'inline') visit diffs_project_merge_request_path(project, merge_request, view: 'inline')
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
end end
@ -174,7 +174,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'with a new line' do context 'with a new line' do
it 'allows dismissing a comment' do it 'allows dismissing a comment' do
should_allow_dismissing_a_comment(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) should_allow_dismissing_a_comment(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
end end
end end
end end
@ -182,13 +182,13 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
describe 'with multiple note forms' do describe 'with multiple note forms' do
before do before do
visit diffs_project_merge_request_path(project, merge_request, view: 'inline') visit diffs_project_merge_request_path(project, merge_request, view: 'inline')
click_diff_line(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) click_diff_line(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
click_diff_line(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) click_diff_line(find_by_scrolling('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
end end
describe 'posting a note' do describe 'posting a note' do
it 'adds as discussion' do it 'adds as discussion' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false) should_allow_commenting(find_by_scrolling('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'), asset_form_reset: false)
expect(page).to have_css('.notes_holder .note.note-discussion', count: 1) expect(page).to have_css('.notes_holder .note.note-discussion', count: 1)
expect(page).to have_field('Reply…') expect(page).to have_field('Reply…')
end end
@ -203,25 +203,25 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
context 'with a new line' do context 'with a new line' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
end end
end end
context 'with an old line' do context 'with an old line' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]')) should_allow_commenting(find_by_scrolling('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
end end
end end
context 'with an unchanged line' do context 'with an unchanged line' do
it 'allows commenting' do it 'allows commenting' do
should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]')) should_allow_commenting(find_by_scrolling('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
end end
end end
context 'with a match line' do context 'with a match line' do
it 'does not allow commenting' do it 'does not allow commenting' do
match_should_not_allow_commenting(find('.match', match: :first)) match_should_not_allow_commenting(find_by_scrolling('.match', match: :first))
end end
end end
end end

View File

@ -6,6 +6,7 @@ include Spec::Support::Helpers::ModalHelpers # rubocop:disable Style/MixinUsage
RSpec.describe 'Merge request > User sees avatars on diff notes', :js do RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
include NoteInteractionHelpers include NoteInteractionHelpers
include Spec::Support::Helpers::ModalHelpers include Spec::Support::Helpers::ModalHelpers
include MergeRequestDiffHelpers
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator } let(:user) { project.creator }
@ -135,6 +136,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
end end
it 'adds avatar when commenting' do it 'adds avatar when commenting' do
find_by_scrolling('[data-discussion-id]', match: :first)
find_field('Reply…', match: :first).click find_field('Reply…', match: :first).click
page.within '.js-discussion-note-form' do page.within '.js-discussion-note-form' do
@ -154,6 +156,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
it 'adds multiple comments' do it 'adds multiple comments' do
3.times do 3.times do
find_by_scrolling('[data-discussion-id]', match: :first)
find_field('Reply…', match: :first).click find_field('Reply…', match: :first).click
page.within '.js-discussion-note-form' do page.within '.js-discussion-note-form' do
@ -192,7 +195,7 @@ RSpec.describe 'Merge request > User sees avatars on diff notes', :js do
end end
def find_line(line_code) def find_line(line_code)
line = find("[id='#{line_code}']") line = find_by_scrolling("[id='#{line_code}']")
line = line.find(:xpath, 'preceding-sibling::*[1][self::td]/preceding-sibling::*[1][self::td]') if line.tag_name == 'td' line = line.find(:xpath, 'preceding-sibling::*[1][self::td]/preceding-sibling::*[1][self::td]') if line.tag_name == 'td'
line line
end end

View File

@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Merge request > User sees deployment widget', :js do RSpec.describe 'Merge request > User sees deployment widget', :js do
include Spec::Support::Helpers::ModalHelpers
describe 'when merge request has associated environments' do describe 'when merge request has associated environments' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
@ -118,7 +120,9 @@ RSpec.describe 'Merge request > User sees deployment widget', :js do
end end
it 'does start build when stop button clicked' do it 'does start build when stop button clicked' do
accept_confirm { find('.js-stop-env').click } accept_gl_confirm(button_text: 'Stop environment') do
find('.js-stop-env').click
end
expect(page).to have_content('close_app') expect(page).to have_content('close_app')
end end

View File

@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe 'Merge request > User sees diff', :js do RSpec.describe 'Merge request > User sees diff', :js do
include ProjectForksHelper include ProjectForksHelper
include RepoHelpers include RepoHelpers
include MergeRequestDiffHelpers
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
@ -58,12 +59,12 @@ RSpec.describe 'Merge request > User sees diff', :js do
let(:changelog_id) { Digest::SHA1.hexdigest("CHANGELOG") } let(:changelog_id) { Digest::SHA1.hexdigest("CHANGELOG") }
context 'as author' do context 'as author' do
it 'shows direct edit link', :sidekiq_might_not_need_inline do it 'contains direct edit link', :sidekiq_might_not_need_inline do
sign_in(author_user) sign_in(author_user)
visit diffs_project_merge_request_path(project, merge_request) visit diffs_project_merge_request_path(project, merge_request)
# Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax
expect(page).to have_selector("[id=\"#{changelog_id}\"] .js-edit-blob", visible: false) expect(page).to have_selector(".js-edit-blob", visible: false)
end end
end end
@ -72,6 +73,8 @@ RSpec.describe 'Merge request > User sees diff', :js do
sign_in(user) sign_in(user)
visit diffs_project_merge_request_path(project, merge_request) visit diffs_project_merge_request_path(project, merge_request)
find_by_scrolling("[id=\"#{changelog_id}\"]")
# Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax # Throws `Capybara::Poltergeist::InvalidSelector` if we try to use `#hash` syntax
find("[id=\"#{changelog_id}\"] .js-diff-more-actions").click find("[id=\"#{changelog_id}\"] .js-diff-more-actions").click
find("[id=\"#{changelog_id}\"] .js-edit-blob").click find("[id=\"#{changelog_id}\"] .js-edit-blob").click
@ -107,6 +110,7 @@ RSpec.describe 'Merge request > User sees diff', :js do
CONTENT CONTENT
file_name = 'xss_file.rs' file_name = 'xss_file.rs'
file_hash = Digest::SHA1.hexdigest(file_name)
create_file('master', file_name, file_content) create_file('master', file_name, file_content)
merge_request = create(:merge_request, source_project: project) merge_request = create(:merge_request, source_project: project)
@ -116,6 +120,8 @@ RSpec.describe 'Merge request > User sees diff', :js do
visit diffs_project_merge_request_path(project, merge_request) visit diffs_project_merge_request_path(project, merge_request)
find_by_scrolling("[id='#{file_hash}']")
expect(page).to have_text("function foo<input> {") expect(page).to have_text("function foo<input> {")
expect(page).to have_css(".line[lang='rust'] .k") expect(page).to have_css(".line[lang='rust'] .k")
end end
@ -136,7 +142,7 @@ RSpec.describe 'Merge request > User sees diff', :js do
end end
context 'when file is an image', :js do context 'when file is an image', :js do
let(:file_name) { 'files/lfs/image.png' } let(:file_name) { 'a/image.png' }
it 'shows an error message' do it 'shows an error message' do
expect(page).not_to have_content('could not be displayed because it is stored in LFS') expect(page).not_to have_content('could not be displayed because it is stored in LFS')
@ -144,7 +150,7 @@ RSpec.describe 'Merge request > User sees diff', :js do
end end
context 'when file is not an image' do context 'when file is not an image' do
let(:file_name) { 'files/lfs/ruby.rb' } let(:file_name) { 'a/ruby.rb' }
it 'shows an error message' do it 'shows an error message' do
expect(page).to have_content('This source diff could not be displayed because it is stored in LFS') expect(page).to have_content('This source diff could not be displayed because it is stored in LFS')
@ -153,7 +159,14 @@ RSpec.describe 'Merge request > User sees diff', :js do
end end
context 'when LFS is not enabled' do context 'when LFS is not enabled' do
let(:file_name) { 'a/lfs_object.iso' }
before do before do
allow(Gitlab.config.lfs).to receive(:disabled).and_return(true)
project.update_attribute(:lfs_enabled, false)
create_file('master', file_name, project.repository.blob_at('master', 'files/lfs/lfs_object.iso').data)
visit diffs_project_merge_request_path(project, merge_request) visit diffs_project_merge_request_path(project, merge_request)
end end

View File

@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Merge request > User sees versions', :js do RSpec.describe 'Merge request > User sees versions', :js do
include MergeRequestDiffHelpers
let(:merge_request) do let(:merge_request) do
create(:merge_request).tap do |mr| create(:merge_request).tap do |mr|
mr.merge_request_diff.destroy! mr.merge_request_diff.destroy!
@ -27,8 +29,12 @@ RSpec.describe 'Merge request > User sees versions', :js do
diff_file_selector = ".diff-file[id='#{file_id}']" diff_file_selector = ".diff-file[id='#{file_id}']"
line_code = "#{file_id}_#{line_code}" line_code = "#{file_id}_#{line_code}"
page.within(diff_file_selector) do page.within find_by_scrolling(diff_file_selector) do
first("[id='#{line_code}']").hover line_code_element = first("[id='#{line_code}']")
# scrolling to element's bottom is required in order for .hover action to work
# otherwise, the element could be hidden underneath a sticky header
scroll_to_elements_bottom(line_code_element)
line_code_element.hover
first("[id='#{line_code}'] [role='button']").click first("[id='#{line_code}'] [role='button']").click
page.within("form[data-line-code='#{line_code}']") do page.within("form[data-line-code='#{line_code}']") do

View File

@ -34,7 +34,7 @@ RSpec.describe 'User comments on a diff', :js do
context 'single suggestion note' do context 'single suggestion note' do
it 'hides suggestion popover' do it 'hides suggestion popover' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
expect(page).to have_selector('.diff-suggest-popover') expect(page).to have_selector('.diff-suggest-popover')
@ -46,7 +46,7 @@ RSpec.describe 'User comments on a diff', :js do
end end
it 'suggestion is presented' do it 'suggestion is presented' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion\n# change to a comment\n```") fill_in('note_note', with: "```suggestion\n# change to a comment\n```")
@ -74,7 +74,7 @@ RSpec.describe 'User comments on a diff', :js do
end end
it 'allows suggestions in replies' do it 'allows suggestions in replies' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion\n# change to a comment\n```") fill_in('note_note', with: "```suggestion\n# change to a comment\n```")
@ -91,7 +91,7 @@ RSpec.describe 'User comments on a diff', :js do
end end
it 'suggestion is appliable' do it 'suggestion is appliable' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion\n# change to a comment\n```") fill_in('note_note', with: "```suggestion\n# change to a comment\n```")
@ -273,7 +273,7 @@ RSpec.describe 'User comments on a diff', :js do
context 'multiple suggestions in a single note' do context 'multiple suggestions in a single note' do
it 'suggestions are presented', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/258989' do it 'suggestions are presented', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/258989' do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion:-2\n# or that\n# heh\n```") fill_in('note_note', with: "```suggestion\n# change to a comment\n```\n```suggestion:-2\n# or that\n# heh\n```")
@ -316,7 +316,7 @@ RSpec.describe 'User comments on a diff', :js do
context 'multi-line suggestions' do context 'multi-line suggestions' do
before do before do
click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) click_diff_line(find_by_scrolling("[id='#{sample_compare.changes[1][:line_code]}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```") fill_in('note_note', with: "```suggestion:-3+5\n# change to a\n# comment\n# with\n# broken\n# lines\n```")

View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe MergeRequestsFinder::Params do
let(:user) { create(:user) }
subject { described_class.new(params, user, MergeRequest) }
describe 'attention' do
context 'attention param exists' do
let(:params) { { attention: user.username } }
it { expect(subject.attention).to eq(user) }
end
context 'attention param does not exist' do
let(:params) { { attention: nil } }
it { expect(subject.attention).to eq(nil) }
end
end
end

View File

@ -408,6 +408,22 @@ RSpec.describe MergeRequestsFinder do
end end
end end
context 'attention' do
subject { described_class.new(user, params).execute }
before do
reviewer = merge_request1.find_reviewer(user2)
reviewer.update!(state: :reviewed)
end
context 'by username' do
let(:params) { { attention: user2.username } }
let(:expected_mr) { [merge_request2, merge_request3] }
it { is_expected.to contain_exactly(*expected_mr) }
end
end
context 'reviewer filtering' do context 'reviewer filtering' do
subject { described_class.new(user, params).execute } subject { described_class.new(user, params).execute }

View File

@ -11,40 +11,21 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
context 'for a personal project' do context 'for a personal project' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
shared_examples_for 'includes access level of the owner of the project' do shared_examples_for 'includes access level of the owner of the project as Maintainer' do
context 'when personal_project_owner_with_owner_access feature flag is enabled' do it 'includes access level of the owner of the project as Maintainer' do
it 'includes access level of the owner of the project as Owner' do expect(subject).to(
expect(subject).to( contain_exactly(
contain_exactly( hash_including(
hash_including( 'user_id' => project.namespace.owner.id,
'user_id' => project.namespace.owner.id, 'access_level' => Gitlab::Access::MAINTAINER
'access_level' => Gitlab::Access::OWNER
)
) )
) )
end )
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'includes access level of the owner of the project as Maintainer' do
expect(subject).to(
contain_exactly(
hash_including(
'user_id' => project.namespace.owner.id,
'access_level' => Gitlab::Access::MAINTAINER
)
)
)
end
end end
end end
context 'when the project owner is a member of the project' do context 'when the project owner is a member of the project' do
it_behaves_like 'includes access level of the owner of the project' it_behaves_like 'includes access level of the owner of the project as Maintainer'
end end
context 'when the project owner is not explicitly a member of the project' do context 'when the project owner is not explicitly a member of the project' do
@ -52,7 +33,7 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
project.members.find_by(user_id: project.namespace.owner.id).destroy! project.members.find_by(user_id: project.namespace.owner.id).destroy!
end end
it_behaves_like 'includes access level of the owner of the project' it_behaves_like 'includes access level of the owner of the project as Maintainer'
end end
end end
@ -103,32 +84,17 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do
context 'for a project within a group' do context 'for a project within a group' do
context 'project in a root group' do context 'project in a root group' do
context 'includes access levels of users who are direct members of the parent group' do it 'includes access levels of users who are direct members of the parent group' do
it 'when access level is developer' do group_member = create(:group_member, :developer, source: group)
group_member = create(:group_member, :developer, source: group)
expect(subject).to( expect(subject).to(
include( include(
hash_including( hash_including(
'user_id' => group_member.user.id, 'user_id' => group_member.user.id,
'access_level' => Gitlab::Access::DEVELOPER 'access_level' => Gitlab::Access::DEVELOPER
)
) )
) )
end )
it 'when access level is owner' do
group_member = create(:group_member, :owner, source: group)
expect(subject).to(
include(
hash_including(
'user_id' => group_member.user.id,
'access_level' => Gitlab::Access::OWNER
)
)
)
end
end end
end end

View File

@ -1,43 +1,40 @@
import { GlDropdown, GlLoadingIcon, GlDropdownSectionHeader } from '@gitlab/ui'; import { GlDropdown, GlLoadingIcon, GlDropdownSectionHeader } from '@gitlab/ui';
import { mount } from '@vue/test-utils'; import { mount } from '@vue/test-utils';
import MockAdapter from 'axios-mock-adapter';
import Vue, { nextTick } from 'vue'; import Vue, { nextTick } from 'vue';
import VueApollo from 'vue-apollo'; import VueApollo from 'vue-apollo';
import Vuex from 'vuex'; import Vuex from 'vuex';
import { TEST_HOST } from 'spec/test_constants'; import { TEST_HOST } from 'spec/test_constants';
import BoardsSelector from '~/boards/components/boards_selector.vue'; import BoardsSelector from '~/boards/components/boards_selector.vue';
import { BoardType } from '~/boards/constants';
import groupBoardQuery from '~/boards/graphql/group_board.query.graphql'; import groupBoardQuery from '~/boards/graphql/group_board.query.graphql';
import projectBoardQuery from '~/boards/graphql/project_board.query.graphql'; import projectBoardQuery from '~/boards/graphql/project_board.query.graphql';
import groupBoardsQuery from '~/boards/graphql/group_boards.query.graphql';
import projectBoardsQuery from '~/boards/graphql/project_boards.query.graphql';
import groupRecentBoardsQuery from '~/boards/graphql/group_recent_boards.query.graphql';
import projectRecentBoardsQuery from '~/boards/graphql/project_recent_boards.query.graphql';
import defaultStore from '~/boards/stores'; import defaultStore from '~/boards/stores';
import axios from '~/lib/utils/axios_utils';
import createMockApollo from 'helpers/mock_apollo_helper'; import createMockApollo from 'helpers/mock_apollo_helper';
import { mockGroupBoardResponse, mockProjectBoardResponse } from '../mock_data'; import {
mockGroupBoardResponse,
mockProjectBoardResponse,
mockGroupAllBoardsResponse,
mockProjectAllBoardsResponse,
mockGroupRecentBoardsResponse,
mockProjectRecentBoardsResponse,
mockSmallProjectAllBoardsResponse,
mockEmptyProjectRecentBoardsResponse,
boards,
recentIssueBoards,
} from '../mock_data';
const throttleDuration = 1; const throttleDuration = 1;
Vue.use(VueApollo); Vue.use(VueApollo);
function boardGenerator(n) {
return new Array(n).fill().map((board, index) => {
const id = `${index}`;
const name = `board${id}`;
return {
id,
name,
};
});
}
describe('BoardsSelector', () => { describe('BoardsSelector', () => {
let wrapper; let wrapper;
let allBoardsResponse;
let recentBoardsResponse;
let mock;
let fakeApollo; let fakeApollo;
let store; let store;
const boards = boardGenerator(20);
const recentBoards = boardGenerator(5);
const createStore = ({ isGroupBoard = false, isProjectBoard = false } = {}) => { const createStore = ({ isGroupBoard = false, isProjectBoard = false } = {}) => {
store = new Vuex.Store({ store = new Vuex.Store({
@ -63,17 +60,43 @@ describe('BoardsSelector', () => {
}; };
const getDropdownItems = () => wrapper.findAll('.js-dropdown-item'); const getDropdownItems = () => wrapper.findAll('.js-dropdown-item');
const getDropdownHeaders = () => wrapper.findAll(GlDropdownSectionHeader); const getDropdownHeaders = () => wrapper.findAllComponents(GlDropdownSectionHeader);
const getLoadingIcon = () => wrapper.find(GlLoadingIcon); const getLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
const findDropdown = () => wrapper.find(GlDropdown); const findDropdown = () => wrapper.findComponent(GlDropdown);
const projectBoardQueryHandlerSuccess = jest.fn().mockResolvedValue(mockProjectBoardResponse); const projectBoardQueryHandlerSuccess = jest.fn().mockResolvedValue(mockProjectBoardResponse);
const groupBoardQueryHandlerSuccess = jest.fn().mockResolvedValue(mockGroupBoardResponse); const groupBoardQueryHandlerSuccess = jest.fn().mockResolvedValue(mockGroupBoardResponse);
const createComponent = () => { const projectBoardsQueryHandlerSuccess = jest
.fn()
.mockResolvedValue(mockProjectAllBoardsResponse);
const groupBoardsQueryHandlerSuccess = jest.fn().mockResolvedValue(mockGroupAllBoardsResponse);
const projectRecentBoardsQueryHandlerSuccess = jest
.fn()
.mockResolvedValue(mockProjectRecentBoardsResponse);
const groupRecentBoardsQueryHandlerSuccess = jest
.fn()
.mockResolvedValue(mockGroupRecentBoardsResponse);
const smallBoardsQueryHandlerSuccess = jest
.fn()
.mockResolvedValue(mockSmallProjectAllBoardsResponse);
const emptyRecentBoardsQueryHandlerSuccess = jest
.fn()
.mockResolvedValue(mockEmptyProjectRecentBoardsResponse);
const createComponent = ({
projectBoardsQueryHandler = projectBoardsQueryHandlerSuccess,
projectRecentBoardsQueryHandler = projectRecentBoardsQueryHandlerSuccess,
} = {}) => {
fakeApollo = createMockApollo([ fakeApollo = createMockApollo([
[projectBoardQuery, projectBoardQueryHandlerSuccess], [projectBoardQuery, projectBoardQueryHandlerSuccess],
[groupBoardQuery, groupBoardQueryHandlerSuccess], [groupBoardQuery, groupBoardQueryHandlerSuccess],
[projectBoardsQuery, projectBoardsQueryHandler],
[groupBoardsQuery, groupBoardsQueryHandlerSuccess],
[projectRecentBoardsQuery, projectRecentBoardsQueryHandler],
[groupRecentBoardsQuery, groupRecentBoardsQueryHandlerSuccess],
]); ]);
wrapper = mount(BoardsSelector, { wrapper = mount(BoardsSelector, {
@ -91,67 +114,34 @@ describe('BoardsSelector', () => {
attachTo: document.body, attachTo: document.body,
provide: { provide: {
fullPath: '', fullPath: '',
recentBoardsEndpoint: `${TEST_HOST}/recent`,
}, },
}); });
wrapper.vm.$apollo.addSmartQuery = jest.fn((_, options) => {
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
// eslint-disable-next-line no-restricted-syntax
wrapper.setData({
[options.loadingKey]: true,
});
});
}; };
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
wrapper = null; fakeApollo = null;
mock.restore();
}); });
describe('fetching all boards', () => { describe('template', () => {
beforeEach(() => { beforeEach(() => {
mock = new MockAdapter(axios); createStore({ isProjectBoard: true });
allBoardsResponse = Promise.resolve({
data: {
group: {
boards: {
edges: boards.map((board) => ({ node: board })),
},
},
},
});
recentBoardsResponse = Promise.resolve({
data: recentBoards,
});
createStore();
createComponent(); createComponent();
mock.onGet(`${TEST_HOST}/recent`).replyOnce(200, recentBoards);
}); });
describe('loading', () => { describe('loading', () => {
beforeEach(async () => {
// Wait for current board to be loaded
await nextTick();
// Emits gl-dropdown show event to simulate the dropdown is opened at initialization time
findDropdown().vm.$emit('show');
});
// we are testing loading state, so don't resolve responses until after the tests // we are testing loading state, so don't resolve responses until after the tests
afterEach(async () => { afterEach(async () => {
await Promise.all([allBoardsResponse, recentBoardsResponse]);
await nextTick(); await nextTick();
}); });
it('shows loading spinner', () => { it('shows loading spinner', () => {
// Emits gl-dropdown show event to simulate the dropdown is opened at initialization time
findDropdown().vm.$emit('show');
expect(getLoadingIcon().exists()).toBe(true);
expect(getDropdownHeaders()).toHaveLength(0); expect(getDropdownHeaders()).toHaveLength(0);
expect(getDropdownItems()).toHaveLength(0); expect(getDropdownItems()).toHaveLength(0);
expect(getLoadingIcon().exists()).toBe(true);
}); });
}); });
@ -163,16 +153,13 @@ describe('BoardsSelector', () => {
// Emits gl-dropdown show event to simulate the dropdown is opened at initialization time // Emits gl-dropdown show event to simulate the dropdown is opened at initialization time
findDropdown().vm.$emit('show'); findDropdown().vm.$emit('show');
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
// eslint-disable-next-line no-restricted-syntax
await wrapper.setData({
loadingBoards: false,
loadingRecentBoards: false,
});
await Promise.all([allBoardsResponse, recentBoardsResponse]);
await nextTick(); await nextTick();
}); });
it('fetches all issue boards', () => {
expect(projectBoardsQueryHandlerSuccess).toHaveBeenCalled();
});
it('hides loading spinner', async () => { it('hides loading spinner', async () => {
await nextTick(); await nextTick();
expect(getLoadingIcon().exists()).toBe(false); expect(getLoadingIcon().exists()).toBe(false);
@ -180,22 +167,17 @@ describe('BoardsSelector', () => {
describe('filtering', () => { describe('filtering', () => {
beforeEach(async () => { beforeEach(async () => {
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
// eslint-disable-next-line no-restricted-syntax
wrapper.setData({
boards,
});
await nextTick(); await nextTick();
}); });
it('shows all boards without filtering', () => { it('shows all boards without filtering', () => {
expect(getDropdownItems()).toHaveLength(boards.length + recentBoards.length); expect(getDropdownItems()).toHaveLength(boards.length + recentIssueBoards.length);
}); });
it('shows only matching boards when filtering', async () => { it('shows only matching boards when filtering', async () => {
const filterTerm = 'board1'; const filterTerm = 'board1';
const expectedCount = boards.filter((board) => board.name.includes(filterTerm)).length; const expectedCount = boards.filter((board) => board.node.name.includes(filterTerm))
.length;
fillSearchBox(filterTerm); fillSearchBox(filterTerm);
@ -214,32 +196,21 @@ describe('BoardsSelector', () => {
describe('recent boards section', () => { describe('recent boards section', () => {
it('shows only when boards are greater than 10', async () => { it('shows only when boards are greater than 10', async () => {
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details
// eslint-disable-next-line no-restricted-syntax
wrapper.setData({
boards,
});
await nextTick(); await nextTick();
expect(projectRecentBoardsQueryHandlerSuccess).toHaveBeenCalled();
expect(getDropdownHeaders()).toHaveLength(2); expect(getDropdownHeaders()).toHaveLength(2);
}); });
it('does not show when boards are less than 10', async () => { it('does not show when boards are less than 10', async () => {
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details createComponent({ projectBoardsQueryHandler: smallBoardsQueryHandlerSuccess });
// eslint-disable-next-line no-restricted-syntax
wrapper.setData({
boards: boards.slice(0, 5),
});
await nextTick(); await nextTick();
expect(getDropdownHeaders()).toHaveLength(0); expect(getDropdownHeaders()).toHaveLength(0);
}); });
it('does not show when recentBoards api returns empty array', async () => { it('does not show when recentIssueBoards api returns empty array', async () => {
// setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details createComponent({
// eslint-disable-next-line no-restricted-syntax projectRecentBoardsQueryHandler: emptyRecentBoardsQueryHandlerSuccess,
wrapper.setData({
recentBoards: [],
}); });
await nextTick(); await nextTick();
@ -256,15 +227,39 @@ describe('BoardsSelector', () => {
}); });
}); });
describe('fetching all boards', () => {
it.each`
boardType | queryHandler | notCalledHandler
${BoardType.group} | ${groupBoardsQueryHandlerSuccess} | ${projectBoardsQueryHandlerSuccess}
${BoardType.project} | ${projectBoardsQueryHandlerSuccess} | ${groupBoardsQueryHandlerSuccess}
`('fetches $boardType boards', async ({ boardType, queryHandler, notCalledHandler }) => {
createStore({
isProjectBoard: boardType === BoardType.project,
isGroupBoard: boardType === BoardType.group,
});
createComponent();
await nextTick();
// Emits gl-dropdown show event to simulate the dropdown is opened at initialization time
findDropdown().vm.$emit('show');
await nextTick();
expect(queryHandler).toHaveBeenCalled();
expect(notCalledHandler).not.toHaveBeenCalled();
});
});
describe('fetching current board', () => { describe('fetching current board', () => {
it.each` it.each`
boardType | queryHandler | notCalledHandler boardType | queryHandler | notCalledHandler
${'group'} | ${groupBoardQueryHandlerSuccess} | ${projectBoardQueryHandlerSuccess} ${BoardType.group} | ${groupBoardQueryHandlerSuccess} | ${projectBoardQueryHandlerSuccess}
${'project'} | ${projectBoardQueryHandlerSuccess} | ${groupBoardQueryHandlerSuccess} ${BoardType.project} | ${projectBoardQueryHandlerSuccess} | ${groupBoardQueryHandlerSuccess}
`('fetches $boardType board', async ({ boardType, queryHandler, notCalledHandler }) => { `('fetches $boardType board', async ({ boardType, queryHandler, notCalledHandler }) => {
createStore({ createStore({
isProjectBoard: boardType === 'project', isProjectBoard: boardType === BoardType.project,
isGroupBoard: boardType === 'group', isGroupBoard: boardType === BoardType.group,
}); });
createComponent(); createComponent();

View File

@ -29,6 +29,85 @@ export const listObj = {
}, },
}; };
function boardGenerator(n) {
return new Array(n).fill().map((board, index) => {
const id = `${index}`;
const name = `board${id}`;
return {
node: {
id,
name,
weight: 0,
__typename: 'Board',
},
};
});
}
export const boards = boardGenerator(20);
export const recentIssueBoards = boardGenerator(5);
export const mockSmallProjectAllBoardsResponse = {
data: {
project: {
id: 'gid://gitlab/Project/114',
boards: { edges: boardGenerator(3) },
__typename: 'Project',
},
},
};
export const mockEmptyProjectRecentBoardsResponse = {
data: {
project: {
id: 'gid://gitlab/Project/114',
recentIssueBoards: { edges: [] },
__typename: 'Project',
},
},
};
export const mockGroupAllBoardsResponse = {
data: {
group: {
id: 'gid://gitlab/Group/114',
boards: { edges: boards },
__typename: 'Group',
},
},
};
export const mockProjectAllBoardsResponse = {
data: {
project: {
id: 'gid://gitlab/Project/1',
boards: { edges: boards },
__typename: 'Project',
},
},
};
export const mockGroupRecentBoardsResponse = {
data: {
group: {
id: 'gid://gitlab/Group/114',
recentIssueBoards: { edges: recentIssueBoards },
__typename: 'Group',
},
},
};
export const mockProjectRecentBoardsResponse = {
data: {
project: {
id: 'gid://gitlab/Project/1',
recentIssueBoards: { edges: recentIssueBoards },
__typename: 'Project',
},
},
};
export const mockGroupBoardResponse = { export const mockGroupBoardResponse = {
data: { data: {
workspace: { workspace: {

View File

@ -69,6 +69,12 @@ describe('diffs/components/app', () => {
}, },
provide, provide,
store, store,
stubs: {
DynamicScroller: {
template: `<div><slot :item="$store.state.diffs.diffFiles[0]"></slot></div>`,
},
DynamicScrollerItem: true,
},
}); });
} }

View File

@ -202,7 +202,7 @@ describe('DiffsStoreActions', () => {
testAction( testAction(
fetchDiffFilesBatch, fetchDiffFilesBatch,
{}, {},
{ endpointBatch, diffViewType: 'inline' }, { endpointBatch, diffViewType: 'inline', diffFiles: [] },
[ [
{ type: types.SET_BATCH_LOADING_STATE, payload: 'loading' }, { type: types.SET_BATCH_LOADING_STATE, payload: 'loading' },
{ type: types.SET_RETRIEVING_BATCHES, payload: true }, { type: types.SET_RETRIEVING_BATCHES, payload: true },

View File

@ -143,7 +143,7 @@ describe('Discussion navigation mixin', () => {
it('scrolls to element', () => { it('scrolls to element', () => {
expect(utils.scrollToElement).toHaveBeenCalledWith( expect(utils.scrollToElement).toHaveBeenCalledWith(
findDiscussion('div.discussion', expected), findDiscussion('div.discussion', expected),
{ behavior: 'smooth' }, { behavior: 'auto' },
); );
}); });
}); });
@ -171,7 +171,7 @@ describe('Discussion navigation mixin', () => {
expect(utils.scrollToElementWithContext).toHaveBeenCalledWith( expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
findDiscussion('ul.notes', expected), findDiscussion('ul.notes', expected),
{ behavior: 'smooth' }, { behavior: 'auto' },
); );
}); });
}); });
@ -212,21 +212,15 @@ describe('Discussion navigation mixin', () => {
it('scrolls to discussion', () => { it('scrolls to discussion', () => {
expect(utils.scrollToElement).toHaveBeenCalledWith( expect(utils.scrollToElement).toHaveBeenCalledWith(
findDiscussion('div.discussion', expected), findDiscussion('div.discussion', expected),
{ behavior: 'smooth' }, { behavior: 'auto' },
); );
}); });
}); });
}); });
}); });
describe.each` describe('virtual scrolling feature', () => {
diffsVirtualScrolling
${false}
${true}
`('virtual scrolling feature is $diffsVirtualScrolling', ({ diffsVirtualScrolling }) => {
beforeEach(() => { beforeEach(() => {
window.gon = { features: { diffsVirtualScrolling } };
jest.spyOn(store, 'dispatch'); jest.spyOn(store, 'dispatch');
store.state.notes.currentDiscussionId = 'a'; store.state.notes.currentDiscussionId = 'a';
@ -238,22 +232,22 @@ describe('Discussion navigation mixin', () => {
window.location.hash = ''; window.location.hash = '';
}); });
it('resets location hash if diffsVirtualScrolling flag is true', async () => { it('resets location hash', async () => {
wrapper.vm.jumpToNextDiscussion(); wrapper.vm.jumpToNextDiscussion();
await nextTick(); await nextTick();
expect(window.location.hash).toBe(diffsVirtualScrolling ? '' : '#test'); expect(window.location.hash).toBe('');
}); });
it.each` it.each`
tabValue | hashValue tabValue
${'diffs'} | ${false} ${'diffs'}
${'show'} | ${!diffsVirtualScrolling} ${'show'}
${'other'} | ${!diffsVirtualScrolling} ${'other'}
`( `(
'calls scrollToFile with setHash as $hashValue when the tab is $tabValue', 'calls scrollToFile with setHash as $hashValue when the tab is $tabValue',
async ({ hashValue, tabValue }) => { async ({ tabValue }) => {
window.mrTabs.currentAction = tabValue; window.mrTabs.currentAction = tabValue;
wrapper.vm.jumpToNextDiscussion(); wrapper.vm.jumpToNextDiscussion();
@ -262,7 +256,6 @@ describe('Discussion navigation mixin', () => {
expect(store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', { expect(store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', {
path: 'test.js', path: 'test.js',
setHash: hashValue,
}); });
}, },
); );

View File

@ -1,5 +1,7 @@
import { mount } from '@vue/test-utils'; import { mount } from '@vue/test-utils';
import waitForPromises from 'helpers/wait_for_promises';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
import { import {
CREATED, CREATED,
@ -20,6 +22,11 @@ import {
jest.mock('~/flash'); jest.mock('~/flash');
jest.mock('~/lib/utils/url_utility'); jest.mock('~/lib/utils/url_utility');
jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal', () => {
return {
confirmAction: jest.fn(),
};
});
describe('DeploymentAction component', () => { describe('DeploymentAction component', () => {
let wrapper; let wrapper;
@ -51,6 +58,7 @@ describe('DeploymentAction component', () => {
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
confirmAction.mockReset();
}); });
describe('actions do not appear when conditions are unmet', () => { describe('actions do not appear when conditions are unmet', () => {
@ -95,16 +103,6 @@ describe('DeploymentAction component', () => {
'$configConst action', '$configConst action',
({ configConst, computedDeploymentStatus, displayConditionChanges, finderFn, endpoint }) => { ({ configConst, computedDeploymentStatus, displayConditionChanges, finderFn, endpoint }) => {
describe(`${configConst} action`, () => { describe(`${configConst} action`, () => {
const confirmAction = () => {
jest.spyOn(window, 'confirm').mockReturnValueOnce(true);
finderFn().trigger('click');
};
const rejectAction = () => {
jest.spyOn(window, 'confirm').mockReturnValueOnce(false);
finderFn().trigger('click');
};
beforeEach(() => { beforeEach(() => {
factory({ factory({
propsData: { propsData: {
@ -125,13 +123,18 @@ describe('DeploymentAction component', () => {
describe('should show a confirm dialog but not call executeInlineAction when declined', () => { describe('should show a confirm dialog but not call executeInlineAction when declined', () => {
beforeEach(() => { beforeEach(() => {
executeActionSpy.mockResolvedValueOnce(); executeActionSpy.mockResolvedValueOnce();
rejectAction(); confirmAction.mockResolvedValueOnce(false);
finderFn().trigger('click');
}); });
it('should show the confirm dialog', () => { it('should show the confirm dialog', () => {
expect(window.confirm).toHaveBeenCalled(); expect(confirmAction).toHaveBeenCalled();
expect(window.confirm).toHaveBeenCalledWith( expect(confirmAction).toHaveBeenCalledWith(
actionButtonMocks[configConst].confirmMessage, actionButtonMocks[configConst].confirmMessage,
{
primaryBtnVariant: actionButtonMocks[configConst].buttonVariant,
primaryBtnText: actionButtonMocks[configConst].buttonText,
},
); );
}); });
@ -143,13 +146,18 @@ describe('DeploymentAction component', () => {
describe('should show a confirm dialog and call executeInlineAction when accepted', () => { describe('should show a confirm dialog and call executeInlineAction when accepted', () => {
beforeEach(() => { beforeEach(() => {
executeActionSpy.mockResolvedValueOnce(); executeActionSpy.mockResolvedValueOnce();
confirmAction(); confirmAction.mockResolvedValueOnce(true);
finderFn().trigger('click');
}); });
it('should show the confirm dialog', () => { it('should show the confirm dialog', () => {
expect(window.confirm).toHaveBeenCalled(); expect(confirmAction).toHaveBeenCalled();
expect(window.confirm).toHaveBeenCalledWith( expect(confirmAction).toHaveBeenCalledWith(
actionButtonMocks[configConst].confirmMessage, actionButtonMocks[configConst].confirmMessage,
{
primaryBtnVariant: actionButtonMocks[configConst].buttonVariant,
primaryBtnText: actionButtonMocks[configConst].buttonText,
},
); );
}); });
@ -164,11 +172,15 @@ describe('DeploymentAction component', () => {
describe('response includes redirect_url', () => { describe('response includes redirect_url', () => {
const url = '/root/example'; const url = '/root/example';
beforeEach(() => { beforeEach(async () => {
executeActionSpy.mockResolvedValueOnce({ executeActionSpy.mockResolvedValueOnce({
data: { redirect_url: url }, data: { redirect_url: url },
}); });
confirmAction();
await waitForPromises();
confirmAction.mockResolvedValueOnce(true);
finderFn().trigger('click');
}); });
it('calls visit url with the redirect_url', () => { it('calls visit url with the redirect_url', () => {
@ -178,9 +190,13 @@ describe('DeploymentAction component', () => {
}); });
describe('it should call the executeAction method ', () => { describe('it should call the executeAction method ', () => {
beforeEach(() => { beforeEach(async () => {
jest.spyOn(wrapper.vm, 'executeAction').mockImplementation(); jest.spyOn(wrapper.vm, 'executeAction').mockImplementation();
confirmAction();
await waitForPromises();
confirmAction.mockResolvedValueOnce(true);
finderFn().trigger('click');
}); });
it('calls with the expected arguments', () => { it('calls with the expected arguments', () => {
@ -193,9 +209,13 @@ describe('DeploymentAction component', () => {
}); });
describe('when executeInlineAction errors', () => { describe('when executeInlineAction errors', () => {
beforeEach(() => { beforeEach(async () => {
executeActionSpy.mockRejectedValueOnce(); executeActionSpy.mockRejectedValueOnce();
confirmAction();
await waitForPromises();
confirmAction.mockResolvedValueOnce(true);
finderFn().trigger('click');
}); });
it('should call createFlash with error message', () => { it('should call createFlash with error message', () => {

View File

@ -9,6 +9,7 @@ const actionButtonMocks = {
[STOPPING]: { [STOPPING]: {
actionName: STOPPING, actionName: STOPPING,
buttonText: 'Stop environment', buttonText: 'Stop environment',
buttonVariant: 'danger',
busyText: 'This environment is being deployed', busyText: 'This environment is being deployed',
confirmMessage: 'Are you sure you want to stop this environment?', confirmMessage: 'Are you sure you want to stop this environment?',
errorMessage: 'Something went wrong while stopping this environment. Please try again.', errorMessage: 'Something went wrong while stopping this environment. Please try again.',
@ -16,6 +17,7 @@ const actionButtonMocks = {
[DEPLOYING]: { [DEPLOYING]: {
actionName: DEPLOYING, actionName: DEPLOYING,
buttonText: 'Deploy', buttonText: 'Deploy',
buttonVariant: 'confirm',
busyText: 'This environment is being deployed', busyText: 'This environment is being deployed',
confirmMessage: 'Are you sure you want to deploy this environment?', confirmMessage: 'Are you sure you want to deploy this environment?',
errorMessage: 'Something went wrong while deploying this environment. Please try again.', errorMessage: 'Something went wrong while deploying this environment. Please try again.',
@ -23,6 +25,7 @@ const actionButtonMocks = {
[REDEPLOYING]: { [REDEPLOYING]: {
actionName: REDEPLOYING, actionName: REDEPLOYING,
buttonText: 'Re-deploy', buttonText: 'Re-deploy',
buttonVariant: 'confirm',
busyText: 'This environment is being re-deployed', busyText: 'This environment is being re-deployed',
confirmMessage: 'Are you sure you want to re-deploy this environment?', confirmMessage: 'Are you sure you want to re-deploy this environment?',
errorMessage: 'Something went wrong while deploying this environment. Please try again.', errorMessage: 'Something went wrong while deploying this environment. Please try again.',

View File

@ -34,28 +34,12 @@ RSpec.describe Gitlab::ProjectAuthorizations do
.to include(owned_project.id, other_project.id, group_project.id) .to include(owned_project.id, other_project.id, group_project.id)
end end
context 'when personal_project_owner_with_owner_access feature flag is enabled' do it 'includes the correct access levels' do
it 'includes the correct access levels' do mapping = map_access_levels(authorizations)
mapping = map_access_levels(authorizations)
expect(mapping[owned_project.id]).to eq(Gitlab::Access::OWNER) expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER)
expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER) expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER) expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'includes the correct access levels' do
mapping = map_access_levels(authorizations)
expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER)
expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER)
expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER)
end
end end
end end

View File

@ -1179,6 +1179,16 @@ RSpec.describe ContainerRepository, :aggregate_failures do
end end
end end
describe '#external_import_status' do
subject { repository.external_import_status }
it 'returns the response from the client' do
expect(repository.gitlab_api_client).to receive(:import_status).with(repository.path).and_return('test')
expect(subject).to eq('test')
end
end
describe '.with_stale_migration' do describe '.with_stale_migration' do
let_it_be(:repository) { create(:container_repository) } let_it_be(:repository) { create(:container_repository) }
let_it_be(:stale_pre_importing_old_timestamp) { create(:container_repository, :pre_importing, migration_pre_import_started_at: 10.minutes.ago) } let_it_be(:stale_pre_importing_old_timestamp) { create(:container_repository, :pre_importing, migration_pre_import_started_at: 10.minutes.ago) }

View File

@ -144,6 +144,20 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
end end
describe '.attention' do
let_it_be(:merge_request5) { create(:merge_request, :unique_branches, assignees: [user2])}
let_it_be(:merge_request6) { create(:merge_request, :unique_branches, assignees: [user2])}
before do
assignee = merge_request6.find_assignee(user2)
assignee.update!(state: :reviewed)
end
it 'returns MRs that have any attention requests' do
expect(described_class.attention(user2)).to eq([merge_request2, merge_request5])
end
end
describe '.drafts' do describe '.drafts' do
it 'returns MRs where draft == true' do it 'returns MRs where draft == true' do
expect(described_class.drafts).to eq([merge_request4]) expect(described_class.drafts).to eq([merge_request4])

View File

@ -225,7 +225,7 @@ RSpec.describe ProjectTeam do
let_it_be(:maintainer) { create(:user) } let_it_be(:maintainer) { create(:user) }
let_it_be(:developer) { create(:user) } let_it_be(:developer) { create(:user) }
let_it_be(:guest) { create(:user) } let_it_be(:guest) { create(:user) }
let_it_be(:project) { create(:project, group: create(:group)) } let_it_be(:project) { create(:project, namespace: maintainer.namespace) }
let_it_be(:access_levels) { [Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER] } let_it_be(:access_levels) { [Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER] }
subject(:members_with_access_levels) { project.team.members_with_access_levels(access_levels) } subject(:members_with_access_levels) { project.team.members_with_access_levels(access_levels) }

View File

@ -3717,7 +3717,7 @@ RSpec.describe User do
context 'with min_access_level' do context 'with min_access_level' do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:project) { create(:project, :private, group: create(:group)) } let!(:project) { create(:project, :private, namespace: user.namespace) }
before do before do
project.add_developer(user) project.add_developer(user)

View File

@ -675,30 +675,13 @@ RSpec.describe API::Members do
end end
context 'adding owner to project' do context 'adding owner to project' do
context 'when personal_project_owner_with_owner_access feature flag is enabled' do it 'returns 403' do
it 'returns created status' do expect do
expect do post api("/projects/#{project.id}/members", maintainer),
post api("/projects/#{project.id}/members", maintainer), params: { user_id: stranger.id, access_level: Member::OWNER }
params: { user_id: stranger.id, access_level: Member::OWNER }
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:bad_request)
end.to change { project.members.count }.by(1) end.not_to change { project.members.count }
end
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'returns created status' do
expect do
post api("/projects/#{project.id}/members", maintainer),
params: { user_id: stranger.id, access_level: Member::OWNER }
expect(response).to have_gitlab_http_status(:bad_request)
end.not_to change { project.members.count }
end
end end
end end

View File

@ -40,7 +40,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
it 'is called' do it 'is called' do
ProjectAuthorization.delete_all ProjectAuthorization.delete_all
expect(callback).to receive(:call).with(project.id, Gitlab::Access::OWNER).once expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once
service.execute service.execute
end end
@ -60,20 +60,20 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
to_be_removed = [project2.id] to_be_removed = [project2.id]
to_be_added = [ to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
] ]
expect(service.execute).to eq([to_be_removed, to_be_added]) expect(service.execute).to eq([to_be_removed, to_be_added])
end end
it 'finds duplicate entries that has to be removed' do it 'finds duplicate entries that has to be removed' do
[Gitlab::Access::OWNER, Gitlab::Access::REPORTER].each do |access_level| [Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level|
user.project_authorizations.create!(project: project, access_level: access_level) user.project_authorizations.create!(project: project, access_level: access_level)
end end
to_be_removed = [project.id] to_be_removed = [project.id]
to_be_added = [ to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
] ]
expect(service.execute).to eq([to_be_removed, to_be_added]) expect(service.execute).to eq([to_be_removed, to_be_added])
@ -85,7 +85,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
to_be_removed = [project.id] to_be_removed = [project.id]
to_be_added = [ to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
] ]
expect(service.execute).to eq([to_be_removed, to_be_added]) expect(service.execute).to eq([to_be_removed, to_be_added])
@ -143,16 +143,16 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
end end
it 'sets the keys to the project IDs' do it 'sets the keys to the project IDs' do
expect(hash.keys).to match_array([project.id]) expect(hash.keys).to eq([project.id])
end end
it 'sets the values to the access levels' do it 'sets the values to the access levels' do
expect(hash.values).to match_array([Gitlab::Access::OWNER]) expect(hash.values).to eq([Gitlab::Access::MAINTAINER])
end end
context 'personal projects' do context 'personal projects' do
it 'includes the project with the right access level' do it 'includes the project with the right access level' do
expect(hash[project.id]).to eq(Gitlab::Access::OWNER) expect(hash[project.id]).to eq(Gitlab::Access::MAINTAINER)
end end
end end
@ -242,7 +242,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
value = hash.values[0] value = hash.values[0]
expect(value.project_id).to eq(project.id) expect(value.project_id).to eq(project.id)
expect(value.access_level).to eq(Gitlab::Access::OWNER) expect(value.access_level).to eq(Gitlab::Access::MAINTAINER)
end end
end end
@ -267,7 +267,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
end end
it 'includes the access level for every row' do it 'includes the access level for every row' do
expect(row.access_level).to eq(Gitlab::Access::OWNER) expect(row.access_level).to eq(Gitlab::Access::MAINTAINER)
end end
end end
end end
@ -283,7 +283,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
rows = service.fresh_authorizations.to_a rows = service.fresh_authorizations.to_a
expect(rows.length).to eq(1) expect(rows.length).to eq(1)
expect(rows.first.access_level).to eq(Gitlab::Access::OWNER) expect(rows.first.access_level).to eq(Gitlab::Access::MAINTAINER)
end end
context 'every returned row' do context 'every returned row' do
@ -294,7 +294,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do
end end
it 'includes the access level' do it 'includes the access level' do
expect(row.access_level).to eq(Gitlab::Access::OWNER) expect(row.access_level).to eq(Gitlab::Access::MAINTAINER)
end end
end end
end end

View File

@ -9,8 +9,8 @@ RSpec.describe Members::Projects::CreatorService do
end end
describe '.access_levels' do describe '.access_levels' do
it 'returns Gitlab::Access.sym_options_with_owner' do it 'returns Gitlab::Access.sym_options' do
expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) expect(described_class.access_levels).to eq(Gitlab::Access.sym_options)
end end
end end
end end

View File

@ -3312,7 +3312,7 @@ RSpec.describe NotificationService, :mailer do
describe "##{sym}" do describe "##{sym}" do
subject(:notify!) { notification.send(sym, domain) } subject(:notify!) { notification.send(sym, domain) }
it 'emails current watching maintainers and owners' do it 'emails current watching maintainers' do
expect(Notify).to receive(:"#{sym}_email").at_least(:once).and_call_original expect(Notify).to receive(:"#{sym}_email").at_least(:once).and_call_original
notify! notify!
@ -3410,7 +3410,7 @@ RSpec.describe NotificationService, :mailer do
reset_delivered_emails! reset_delivered_emails!
end end
it 'emails current watching maintainers and owners' do it 'emails current watching maintainers' do
notification.remote_mirror_update_failed(remote_mirror) notification.remote_mirror_update_failed(remote_mirror)
should_only_email(u_maintainer1, u_maintainer2, u_owner) should_only_email(u_maintainer1, u_maintainer2, u_owner)

View File

@ -116,34 +116,14 @@ RSpec.describe Projects::CreateService, '#execute' do
end end
context 'user namespace' do context 'user namespace' do
context 'when personal_project_owner_with_owner_access feature flag is enabled' do it 'creates a project in user namespace' do
it 'creates a project in user namespace' do project = create_project(user, opts)
project = create_project(user, opts)
expect(project).to be_valid expect(project).to be_valid
expect(project.first_owner).to eq(user) expect(project.first_owner).to eq(user)
expect(project.team.maintainers).not_to include(user) expect(project.team.maintainers).to include(user)
expect(project.team.owners).to contain_exactly(user) expect(project.namespace).to eq(user.namespace)
expect(project.namespace).to eq(user.namespace) expect(project.project_namespace).to be_in_sync_with_project(project)
expect(project.project_namespace).to be_in_sync_with_project(project)
end
end
context 'when personal_project_owner_with_owner_access feature flag is disabled' do
before do
stub_feature_flags(personal_project_owner_with_owner_access: false)
end
it 'creates a project in user namespace' do
project = create_project(user, opts)
expect(project).to be_valid
expect(project.first_owner).to eq(user)
expect(project.team.maintainers).to contain_exactly(user)
expect(project.team.owners).to contain_exactly(user)
expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project)
end
end end
end end
@ -182,7 +162,7 @@ RSpec.describe Projects::CreateService, '#execute' do
expect(project).to be_persisted expect(project).to be_persisted
expect(project.owner).to eq(user) expect(project.owner).to eq(user)
expect(project.first_owner).to eq(user) expect(project.first_owner).to eq(user)
expect(project.team.owners).to contain_exactly(user) expect(project.team.maintainers).to contain_exactly(user)
expect(project.namespace).to eq(user.namespace) expect(project.namespace).to eq(user.namespace)
expect(project.project_namespace).to be_in_sync_with_project(project) expect(project.project_namespace).to be_in_sync_with_project(project)
end end

View File

@ -52,7 +52,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
it 'is called' do it 'is called' do
ProjectAuthorization.delete_all ProjectAuthorization.delete_all
expect(callback).to receive(:call).with(project.id, Gitlab::Access::OWNER).once expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once
service.execute service.execute
end end
@ -73,7 +73,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
to_be_removed = [project_authorization.project_id] to_be_removed = [project_authorization.project_id]
to_be_added = [ to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
] ]
expect(service).to receive(:update_authorizations) expect(service).to receive(:update_authorizations)
@ -83,14 +83,14 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
end end
it 'removes duplicate entries' do it 'removes duplicate entries' do
[Gitlab::Access::OWNER, Gitlab::Access::REPORTER].each do |access_level| [Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level|
user.project_authorizations.create!(project: project, access_level: access_level) user.project_authorizations.create!(project: project, access_level: access_level)
end end
to_be_removed = [project.id] to_be_removed = [project.id]
to_be_added = [ to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
] ]
expect(service).to( expect(service).to(
receive(:update_authorizations) receive(:update_authorizations)
@ -103,7 +103,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
project_authorization = ProjectAuthorization.where( project_authorization = ProjectAuthorization.where(
project_id: project.id, project_id: project.id,
user_id: user.id, user_id: user.id,
access_level: Gitlab::Access::OWNER) access_level: Gitlab::Access::MAINTAINER)
expect(project_authorization).to exist expect(project_authorization).to exist
end end
@ -116,7 +116,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do
to_be_removed = [project_authorization.project_id] to_be_removed = [project_authorization.project_id]
to_be_added = [ to_be_added = [
{ user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER }
] ]
expect(service).to receive(:update_authorizations) expect(service).to receive(:update_authorizations)

View File

@ -294,8 +294,6 @@ RSpec.configure do |config|
# See https://gitlab.com/gitlab-org/gitlab/-/issues/33867 # See https://gitlab.com/gitlab-org/gitlab/-/issues/33867
stub_feature_flags(file_identifier_hash: false) stub_feature_flags(file_identifier_hash: false)
stub_feature_flags(diffs_virtual_scrolling: false)
# The following `vue_issues_list` stub can be removed # The following `vue_issues_list` stub can be removed
# once the Vue issues page has feature parity with the current Haml page # once the Vue issues page has feature parity with the current Haml page
stub_feature_flags(vue_issues_list: false) stub_feature_flags(vue_issues_list: false)

View File

@ -1,8 +1,11 @@
# frozen_string_literal: true # frozen_string_literal: true
module MergeRequestDiffHelpers module MergeRequestDiffHelpers
PageEndReached = Class.new(StandardError)
def click_diff_line(line_holder, diff_side = nil) def click_diff_line(line_holder, diff_side = nil)
line = get_line_components(line_holder, diff_side) line = get_line_components(line_holder, diff_side)
scroll_to_elements_bottom(line_holder)
line_holder.hover line_holder.hover
line[:num].find('.js-add-diff-note-button').click line[:num].find('.js-add-diff-note-button').click
end end
@ -27,4 +30,55 @@ module MergeRequestDiffHelpers
line_holder.find('.diff-line-num', match: :first) line_holder.find('.diff-line-num', match: :first)
{ content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] } { content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] }
end end
def has_reached_page_end
evaluate_script("(window.innerHeight + window.scrollY) >= document.body.offsetHeight")
end
def scroll_to_elements_bottom(element)
evaluate_script("(function(el) {
window.scrollBy(0, el.getBoundingClientRect().bottom - window.innerHeight);
})(arguments[0]);", element.native)
end
# we're not using Capybara's .obscured here because it also checks if the element is clickable
def within_viewport?(element)
evaluate_script("(function(el) {
var rect = el.getBoundingClientRect();
return (
rect.bottom >= 0 &&
rect.right >= 0 &&
rect.top <= (window.innerHeight || document.documentElement.clientHeight) &&
rect.left <= (window.innerWidth || document.documentElement.clientWidth)
);
})(arguments[0]);", element.native)
end
def find_within_viewport(selector, **options)
begin
element = find(selector, **options, wait: 2)
rescue Capybara::ElementNotFound
return
end
return element if within_viewport?(element)
nil
end
def find_by_scrolling(selector, **options)
element = find_within_viewport(selector, **options)
return element if element
page.execute_script "window.scrollTo(0,0)"
until element
if has_reached_page_end
raise PageEndReached, "Failed to find any elements matching a selector '#{selector}' by scrolling. Page end reached."
end
page.execute_script "window.scrollBy(0,window.innerHeight/1.5)"
element = find_within_viewport(selector, **options)
end
element
end
end end

View File

@ -1,8 +1,10 @@
# frozen_string_literal: true # frozen_string_literal: true
module NoteInteractionHelpers module NoteInteractionHelpers
include MergeRequestDiffHelpers
def open_more_actions_dropdown(note) def open_more_actions_dropdown(note)
note_element = find("#note_#{note.id}") note_element = find_by_scrolling("#note_#{note.id}")
note_element.find('.more-actions-toggle').click note_element.find('.more-actions-toggle').click
note_element.find('.more-actions .dropdown-menu li', match: :first) note_element.find('.more-actions .dropdown-menu li', match: :first)

View File

@ -2,7 +2,7 @@
RSpec.shared_examples 'comment on merge request file' do RSpec.shared_examples 'comment on merge request file' do
it 'adds a comment' do it 'adds a comment' do
click_diff_line(find("[id='#{sample_commit.line_code}']")) click_diff_line(find_by_scrolling("[id='#{sample_commit.line_code}']"))
page.within('.js-discussion-note-form') do page.within('.js-discussion-note-form') do
fill_in(:note_note, with: 'Line is wrong') fill_in(:note_note, with: 'Line is wrong')

View File

@ -299,4 +299,51 @@ RSpec.shared_examples 'namespace traversal scopes' do
include_examples '.self_and_descendant_ids' include_examples '.self_and_descendant_ids'
end end
end end
shared_examples '.self_and_hierarchy' do
let(:base_scope) { Group.where(id: base_groups) }
subject { base_scope.self_and_hierarchy }
context 'with ancestors only' do
let(:base_groups) { [group_1, group_2] }
it { is_expected.to match_array(groups) }
end
context 'with descendants only' do
let(:base_groups) { [deep_nested_group_1, deep_nested_group_2] }
it { is_expected.to match_array(groups) }
end
context 'nodes with both ancestors and descendants' do
let(:base_groups) { [nested_group_1, nested_group_2] }
it { is_expected.to match_array(groups) }
end
context 'with duplicate base groups' do
let(:base_groups) { [nested_group_1, nested_group_1] }
it { is_expected.to contain_exactly(group_1, nested_group_1, deep_nested_group_1) }
end
end
describe '.self_and_hierarchy' do
it_behaves_like '.self_and_hierarchy'
context "use_traversal_ids_for_self_and_hierarchy_scopes feature flag is false" do
before do
stub_feature_flags(use_traversal_ids_for_self_and_hierarchy_scopes: false)
end
it_behaves_like '.self_and_hierarchy'
it 'make recursive queries' do
base_groups = Group.where(id: nested_group_1)
expect { base_groups.self_and_hierarchy.load }.to make_queries_matching(/WITH RECURSIVE/)
end
end
end
end end

View File

@ -26,11 +26,30 @@ RSpec.describe ContainerRegistry::Migration::GuardWorker, :aggregate_failures do
allow(::Gitlab).to receive(:com?).and_return(true) allow(::Gitlab).to receive(:com?).and_return(true)
end end
shared_examples 'not aborting any migration' do
it 'will not abort the migration' do
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1)
expect(worker).to receive(:log_extra_metadata_on_done).with(:aborted_stale_migrations_count, 0)
expect(worker).to receive(:log_extra_metadata_on_done).with(:long_running_stale_migration_container_repository_ids, [stale_migration.id])
expect { subject }
.to not_change(pre_importing_migrations, :count)
.and not_change(pre_import_done_migrations, :count)
.and not_change(importing_migrations, :count)
.and not_change(import_done_migrations, :count)
.and not_change(import_aborted_migrations, :count)
.and not_change { stale_migration.reload.migration_state }
.and not_change { ongoing_migration.migration_state }
end
end
context 'with no stale migrations' do context 'with no stale migrations' do
it_behaves_like 'an idempotent worker' it_behaves_like 'an idempotent worker'
it 'will not update any migration state' do it 'will not update any migration state' do
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 0) expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 0)
expect(worker).to receive(:log_extra_metadata_on_done).with(:aborted_stale_migrations_count, 0)
expect { subject } expect { subject }
.to not_change(pre_importing_migrations, :count) .to not_change(pre_importing_migrations, :count)
.and not_change(pre_import_done_migrations, :count) .and not_change(pre_import_done_migrations, :count)
@ -41,10 +60,19 @@ RSpec.describe ContainerRegistry::Migration::GuardWorker, :aggregate_failures do
context 'with pre_importing stale migrations' do context 'with pre_importing stale migrations' do
let(:ongoing_migration) { create(:container_repository, :pre_importing) } let(:ongoing_migration) { create(:container_repository, :pre_importing) }
let(:stale_migration) { create(:container_repository, :pre_importing, migration_pre_import_started_at: 10.minutes.ago) } let(:stale_migration) { create(:container_repository, :pre_importing, migration_pre_import_started_at: 35.minutes.ago) }
let(:import_status) { 'test' }
before do
allow_next_instance_of(ContainerRegistry::GitlabApiClient) do |client|
allow(client).to receive(:import_status).and_return(import_status)
end
end
it 'will abort the migration' do it 'will abort the migration' do
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1) expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1)
expect(worker).to receive(:log_extra_metadata_on_done).with(:aborted_stale_migrations_count, 1)
expect { subject } expect { subject }
.to change(pre_importing_migrations, :count).by(-1) .to change(pre_importing_migrations, :count).by(-1)
.and not_change(pre_import_done_migrations, :count) .and not_change(pre_import_done_migrations, :count)
@ -54,18 +82,26 @@ RSpec.describe ContainerRegistry::Migration::GuardWorker, :aggregate_failures do
.and change { stale_migration.reload.migration_state }.from('pre_importing').to('import_aborted') .and change { stale_migration.reload.migration_state }.from('pre_importing').to('import_aborted')
.and not_change { ongoing_migration.migration_state } .and not_change { ongoing_migration.migration_state }
end end
context 'the client returns pre_import_in_progress' do
let(:import_status) { 'pre_import_in_progress' }
it_behaves_like 'not aborting any migration'
end
end end
context 'with pre_import_done stale migrations' do context 'with pre_import_done stale migrations' do
let(:ongoing_migration) { create(:container_repository, :pre_import_done) } let(:ongoing_migration) { create(:container_repository, :pre_import_done) }
let(:stale_migration) { create(:container_repository, :pre_import_done, migration_pre_import_done_at: 10.minutes.ago) } let(:stale_migration) { create(:container_repository, :pre_import_done, migration_pre_import_done_at: 35.minutes.ago) }
before do before do
allow(::ContainerRegistry::Migration).to receive(:max_step_duration).and_return(5.minutes) allow(::ContainerRegistry::Migration).to receive(:max_step_duration).and_return(5.minutes)
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1)
end end
it 'will abort the migration' do it 'will abort the migration' do
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1)
expect(worker).to receive(:log_extra_metadata_on_done).with(:aborted_stale_migrations_count, 1)
expect { subject } expect { subject }
.to not_change(pre_importing_migrations, :count) .to not_change(pre_importing_migrations, :count)
.and change(pre_import_done_migrations, :count).by(-1) .and change(pre_import_done_migrations, :count).by(-1)
@ -79,14 +115,19 @@ RSpec.describe ContainerRegistry::Migration::GuardWorker, :aggregate_failures do
context 'with importing stale migrations' do context 'with importing stale migrations' do
let(:ongoing_migration) { create(:container_repository, :importing) } let(:ongoing_migration) { create(:container_repository, :importing) }
let(:stale_migration) { create(:container_repository, :importing, migration_import_started_at: 10.minutes.ago) } let(:stale_migration) { create(:container_repository, :importing, migration_import_started_at: 35.minutes.ago) }
let(:import_status) { 'test' }
before do before do
allow(::ContainerRegistry::Migration).to receive(:max_step_duration).and_return(5.minutes) allow_next_instance_of(ContainerRegistry::GitlabApiClient) do |client|
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1) allow(client).to receive(:import_status).and_return(import_status)
end
end end
it 'will abort the migration' do it 'will abort the migration' do
expect(worker).to receive(:log_extra_metadata_on_done).with(:stale_migrations_count, 1)
expect(worker).to receive(:log_extra_metadata_on_done).with(:aborted_stale_migrations_count, 1)
expect { subject } expect { subject }
.to not_change(pre_importing_migrations, :count) .to not_change(pre_importing_migrations, :count)
.and not_change(pre_import_done_migrations, :count) .and not_change(pre_import_done_migrations, :count)
@ -96,6 +137,12 @@ RSpec.describe ContainerRegistry::Migration::GuardWorker, :aggregate_failures do
.and change { stale_migration.reload.migration_state }.from('importing').to('import_aborted') .and change { stale_migration.reload.migration_state }.from('importing').to('import_aborted')
.and not_change { ongoing_migration.migration_state } .and not_change { ongoing_migration.migration_state }
end end
context 'the client returns import_in_progress' do
let(:import_status) { 'import_in_progress' }
it_behaves_like 'not aborting any migration'
end
end end
end end