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

Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2021-06-21 12:07:45 +00:00
parent 3ab7e70965
commit d4c968c95c
96 changed files with 743 additions and 1020 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.gitlab/ci/rules.gitlab-ci.yml
View File

@ -1057,6 +1057,8 @@
.reports:rules:package_hunter: .reports:rules:package_hunter:
rules: rules:
- if: "$PACKAGE_HUNTER_USER == null || $PACKAGE_HUNTER_USER == ''"
when: never
- <<: *if-default-branch-schedule-2-hourly - <<: *if-default-branch-schedule-2-hourly
- <<: *if-merge-request - <<: *if-merge-request
changes: ["yarn.lock"] changes: ["yarn.lock"]

2
app/assets/javascripts/environments/components/environments_app.vue
View File

@ -135,7 +135,7 @@ export default {
>{{ $options.i18n.newEnvironmentButtonLabel }}</gl-button >{{ $options.i18n.newEnvironmentButtonLabel }}</gl-button
> >
</div> </div>
<gl-tabs content-class="gl-display-none"> <gl-tabs :value="activeTab" content-class="gl-display-none">
<gl-tab <gl-tab
v-for="(tab, idx) in tabs" v-for="(tab, idx) in tabs"
:key="idx" :key="idx"

3
app/assets/javascripts/environments/mixins/environments_mixin.js
View File

@ -208,6 +208,9 @@ export default {
}, },
]; ];
}, },
activeTab() {
return this.tabs.findIndex(({ isActive }) => isActive) ?? 0;
},
}, },
/** /**

61
app/assets/javascripts/issues_list/components/issues_list_app.vue
View File

@ -16,7 +16,6 @@ import IssuableByEmail from '~/issuable/components/issuable_by_email.vue';
import IssuableList from '~/issuable_list/components/issuable_list_root.vue'; import IssuableList from '~/issuable_list/components/issuable_list_root.vue';
import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants'; import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants';
import { import {
API_PARAM,
CREATED_DESC, CREATED_DESC,
i18n, i18n,
initialPageParams, initialPageParams,
@ -25,7 +24,7 @@ import {
PARAM_DUE_DATE, PARAM_DUE_DATE,
PARAM_SORT, PARAM_SORT,
PARAM_STATE, PARAM_STATE,
RELATIVE_POSITION_DESC, RELATIVE_POSITION_ASC,
TOKEN_TYPE_ASSIGNEE, TOKEN_TYPE_ASSIGNEE,
TOKEN_TYPE_AUTHOR, TOKEN_TYPE_AUTHOR,
TOKEN_TYPE_CONFIDENTIAL, TOKEN_TYPE_CONFIDENTIAL,
@ -36,12 +35,12 @@ import {
TOKEN_TYPE_MILESTONE, TOKEN_TYPE_MILESTONE,
TOKEN_TYPE_WEIGHT, TOKEN_TYPE_WEIGHT,
UPDATED_DESC, UPDATED_DESC,
URL_PARAM,
urlSortParams, urlSortParams,
} from '~/issues_list/constants'; } from '~/issues_list/constants';
import { import {
convertToParams, convertToApiParams,
convertToSearchQuery, convertToSearchQuery,
convertToUrlParams,
getDueDateValue, getDueDateValue,
getFilterTokens, getFilterTokens,
getSortKey, getSortKey,
@ -192,10 +191,10 @@ export default {
...this.apiFilterParams, ...this.apiFilterParams,
}; };
}, },
update: ({ project }) => project.issues.nodes, update: ({ project }) => project?.issues.nodes ?? [],
result({ data }) { result({ data }) {
this.pageInfo = data.project.issues.pageInfo; this.pageInfo = data.project?.issues.pageInfo ?? {};
this.totalIssues = data.project.issues.count; this.totalIssues = data.project?.issues.count ?? 0;
this.exportCsvPathWithQuery = this.getExportCsvPathWithQuery(); this.exportCsvPathWithQuery = this.getExportCsvPathWithQuery();
}, },
error(error) { error(error) {
@ -215,32 +214,30 @@ export default {
return this.showBulkEditSidebar || !this.issues.length; return this.showBulkEditSidebar || !this.issues.length;
}, },
isManualOrdering() { isManualOrdering() {
return this.sortKey === RELATIVE_POSITION_DESC; return this.sortKey === RELATIVE_POSITION_ASC;
}, },
isOpenTab() { isOpenTab() {
return this.state === IssuableStates.Opened; return this.state === IssuableStates.Opened;
}, },
apiFilterParams() { apiFilterParams() {
return convertToParams(this.filterTokens, API_PARAM); return convertToApiParams(this.filterTokens);
}, },
urlFilterParams() { urlFilterParams() {
return convertToParams(this.filterTokens, URL_PARAM); return convertToUrlParams(this.filterTokens);
}, },
searchQuery() { searchQuery() {
return convertToSearchQuery(this.filterTokens) || undefined; return convertToSearchQuery(this.filterTokens) || undefined;
}, },
searchTokens() { searchTokens() {
let preloadedAuthors = []; const preloadedAuthors = [];
if (gon.current_user_id) { if (gon.current_user_id) {
preloadedAuthors = [ preloadedAuthors.push({
{ id: gon.current_user_id,
id: gon.current_user_id, name: gon.current_user_fullname,
name: gon.current_user_fullname, username: gon.current_username,
username: gon.current_username, avatar_url: gon.current_user_avatar_url,
avatar_url: gon.current_user_avatar_url, });
},
];
} }
const tokens = [ const tokens = [
@ -252,6 +249,7 @@ export default {
dataType: 'user', dataType: 'user',
unique: true, unique: true,
defaultAuthors: [], defaultAuthors: [],
operators: OPERATOR_IS_ONLY,
fetchAuthors: this.fetchUsers, fetchAuthors: this.fetchUsers,
preloadedAuthors, preloadedAuthors,
}, },
@ -280,7 +278,7 @@ export default {
title: TOKEN_TITLE_LABEL, title: TOKEN_TITLE_LABEL,
icon: 'labels', icon: 'labels',
token: LabelToken, token: LabelToken,
defaultLabels: [], defaultLabels: DEFAULT_NONE_ANY,
fetchLabels: this.fetchLabels, fetchLabels: this.fetchLabels,
}, },
]; ];
@ -329,6 +327,7 @@ export default {
token: EpicToken, token: EpicToken,
unique: true, unique: true,
idProperty: 'id', idProperty: 'id',
useIdValue: true,
fetchEpics: this.fetchEpics, fetchEpics: this.fetchEpics,
}); });
} }
@ -346,7 +345,7 @@ export default {
return tokens; return tokens;
}, },
showPaginationControls() { showPaginationControls() {
return this.issues.length > 0; return this.issues.length > 0 && (this.pageInfo.hasNextPage || this.pageInfo.hasPreviousPage);
}, },
sortOptions() { sortOptions() {
return getSortOptions(this.hasIssueWeightsFeature, this.hasBlockedIssuesFeature); return getSortOptions(this.hasIssueWeightsFeature, this.hasBlockedIssuesFeature);
@ -361,22 +360,12 @@ export default {
); );
}, },
urlParams() { urlParams() {
const filterParams = {
...this.urlFilterParams,
};
if (filterParams.epic_id) {
filterParams.epic_id = encodeURIComponent(filterParams.epic_id);
} else if (filterParams['not[epic_id]']) {
filterParams['not[epic_id]'] = encodeURIComponent(filterParams['not[epic_id]']);
}
return { return {
due_date: this.dueDateFilter, due_date: this.dueDateFilter,
search: this.searchQuery, search: this.searchQuery,
sort: urlSortParams[this.sortKey],
state: this.state, state: this.state,
...urlSortParams[this.sortKey], ...this.urlFilterParams,
...filterParams,
}; };
}, },
}, },
@ -424,7 +413,10 @@ export default {
return this.fetchWithCache(this.projectMilestonesPath, 'milestones', 'title', search, true); return this.fetchWithCache(this.projectMilestonesPath, 'milestones', 'title', search, true);
}, },
fetchIterations(search) { fetchIterations(search) {
return axios.get(this.projectIterationsPath, { params: { search } }); const id = Number(search);
return !search || Number.isNaN(id)
? axios.get(this.projectIterationsPath, { params: { search } })
: axios.get(this.projectIterationsPath, { params: { id } });
}, },
fetchUsers(search) { fetchUsers(search) {
return axios.get(this.autocompleteUsersPath, { params: { search } }); return axios.get(this.autocompleteUsersPath, { params: { search } });
@ -471,6 +463,7 @@ export default {
this.state = state; this.state = state;
}, },
handleFilter(filter) { handleFilter(filter) {
this.pageParams = initialPageParams;
this.filterTokens = filter; this.filterTokens = filter;
}, },
handleNextPage() { handleNextPage() {

218
app/assets/javascripts/issues_list/constants.js
View File

@ -128,21 +128,21 @@ export const CREATED_ASC = 'CREATED_ASC';
export const CREATED_DESC = 'CREATED_DESC'; export const CREATED_DESC = 'CREATED_DESC';
export const DUE_DATE_ASC = 'DUE_DATE_ASC'; export const DUE_DATE_ASC = 'DUE_DATE_ASC';
export const DUE_DATE_DESC = 'DUE_DATE_DESC'; export const DUE_DATE_DESC = 'DUE_DATE_DESC';
export const LABEL_PRIORITY_ASC = 'LABEL_PRIORITY_ASC';
export const LABEL_PRIORITY_DESC = 'LABEL_PRIORITY_DESC'; export const LABEL_PRIORITY_DESC = 'LABEL_PRIORITY_DESC';
export const MILESTONE_DUE_ASC = 'MILESTONE_DUE_ASC'; export const MILESTONE_DUE_ASC = 'MILESTONE_DUE_ASC';
export const MILESTONE_DUE_DESC = 'MILESTONE_DUE_DESC'; export const MILESTONE_DUE_DESC = 'MILESTONE_DUE_DESC';
export const POPULARITY_ASC = 'POPULARITY_ASC'; export const POPULARITY_ASC = 'POPULARITY_ASC';
export const POPULARITY_DESC = 'POPULARITY_DESC'; export const POPULARITY_DESC = 'POPULARITY_DESC';
export const PRIORITY_ASC = 'PRIORITY_ASC';
export const PRIORITY_DESC = 'PRIORITY_DESC'; export const PRIORITY_DESC = 'PRIORITY_DESC';
export const RELATIVE_POSITION_DESC = 'RELATIVE_POSITION_DESC'; export const RELATIVE_POSITION_ASC = 'RELATIVE_POSITION_ASC';
export const UPDATED_ASC = 'UPDATED_ASC'; export const UPDATED_ASC = 'UPDATED_ASC';
export const UPDATED_DESC = 'UPDATED_DESC'; export const UPDATED_DESC = 'UPDATED_DESC';
export const WEIGHT_ASC = 'WEIGHT_ASC'; export const WEIGHT_ASC = 'WEIGHT_ASC';
export const WEIGHT_DESC = 'WEIGHT_DESC'; export const WEIGHT_DESC = 'WEIGHT_DESC';
const SORT_ASC = 'asc'; const PRIORITY_ASC_SORT = 'priority_asc';
const SORT_DESC = 'desc';
const CREATED_DATE_SORT = 'created_date'; const CREATED_DATE_SORT = 'created_date';
const CREATED_ASC_SORT = 'created_asc'; const CREATED_ASC_SORT = 'created_asc';
const UPDATED_DESC_SORT = 'updated_desc'; const UPDATED_DESC_SORT = 'updated_desc';
@ -150,129 +150,30 @@ const UPDATED_ASC_SORT = 'updated_asc';
const MILESTONE_SORT = 'milestone'; const MILESTONE_SORT = 'milestone';
const MILESTONE_DUE_DESC_SORT = 'milestone_due_desc'; const MILESTONE_DUE_DESC_SORT = 'milestone_due_desc';
const DUE_DATE_DESC_SORT = 'due_date_desc'; const DUE_DATE_DESC_SORT = 'due_date_desc';
const LABEL_PRIORITY_ASC_SORT = 'label_priority_asc';
const POPULARITY_ASC_SORT = 'popularity_asc'; const POPULARITY_ASC_SORT = 'popularity_asc';
const WEIGHT_DESC_SORT = 'weight_desc'; const WEIGHT_DESC_SORT = 'weight_desc';
const BLOCKING_ISSUES_DESC_SORT = 'blocking_issues_desc'; const BLOCKING_ISSUES_DESC_SORT = 'blocking_issues_desc';
const BLOCKING_ISSUES = 'blocking_issues';
export const apiSortParams = {
[PRIORITY_DESC]: {
order_by: PRIORITY,
sort: SORT_DESC,
},
[CREATED_ASC]: {
order_by: CREATED_AT,
sort: SORT_ASC,
},
[CREATED_DESC]: {
order_by: CREATED_AT,
sort: SORT_DESC,
},
[UPDATED_ASC]: {
order_by: UPDATED_AT,
sort: SORT_ASC,
},
[UPDATED_DESC]: {
order_by: UPDATED_AT,
sort: SORT_DESC,
},
[MILESTONE_DUE_ASC]: {
order_by: MILESTONE_DUE,
sort: SORT_ASC,
},
[MILESTONE_DUE_DESC]: {
order_by: MILESTONE_DUE,
sort: SORT_DESC,
},
[DUE_DATE_ASC]: {
order_by: DUE_DATE,
sort: SORT_ASC,
},
[DUE_DATE_DESC]: {
order_by: DUE_DATE,
sort: SORT_DESC,
},
[POPULARITY_ASC]: {
order_by: POPULARITY,
sort: SORT_ASC,
},
[POPULARITY_DESC]: {
order_by: POPULARITY,
sort: SORT_DESC,
},
[LABEL_PRIORITY_DESC]: {
order_by: LABEL_PRIORITY,
sort: SORT_DESC,
},
[RELATIVE_POSITION_DESC]: {
order_by: RELATIVE_POSITION,
per_page: 100,
sort: SORT_ASC,
},
[WEIGHT_ASC]: {
order_by: WEIGHT,
sort: SORT_ASC,
},
[WEIGHT_DESC]: {
order_by: WEIGHT,
sort: SORT_DESC,
},
[BLOCKING_ISSUES_DESC]: {
order_by: BLOCKING_ISSUES,
sort: SORT_DESC,
},
};
export const urlSortParams = { export const urlSortParams = {
[PRIORITY_DESC]: { [PRIORITY_ASC]: PRIORITY_ASC_SORT,
sort: PRIORITY, [PRIORITY_DESC]: PRIORITY,
}, [CREATED_ASC]: CREATED_ASC_SORT,
[CREATED_ASC]: { [CREATED_DESC]: CREATED_DATE_SORT,
sort: CREATED_ASC_SORT, [UPDATED_ASC]: UPDATED_ASC_SORT,
}, [UPDATED_DESC]: UPDATED_DESC_SORT,
[CREATED_DESC]: { [MILESTONE_DUE_ASC]: MILESTONE_SORT,
sort: CREATED_DATE_SORT, [MILESTONE_DUE_DESC]: MILESTONE_DUE_DESC_SORT,
}, [DUE_DATE_ASC]: DUE_DATE,
[UPDATED_ASC]: { [DUE_DATE_DESC]: DUE_DATE_DESC_SORT,
sort: UPDATED_ASC_SORT, [POPULARITY_ASC]: POPULARITY_ASC_SORT,
}, [POPULARITY_DESC]: POPULARITY,
[UPDATED_DESC]: { [LABEL_PRIORITY_ASC]: LABEL_PRIORITY_ASC_SORT,
sort: UPDATED_DESC_SORT, [LABEL_PRIORITY_DESC]: LABEL_PRIORITY,
}, [RELATIVE_POSITION_ASC]: RELATIVE_POSITION,
[MILESTONE_DUE_ASC]: { [WEIGHT_ASC]: WEIGHT,
sort: MILESTONE_SORT, [WEIGHT_DESC]: WEIGHT_DESC_SORT,
}, [BLOCKING_ISSUES_DESC]: BLOCKING_ISSUES_DESC_SORT,
[MILESTONE_DUE_DESC]: {
sort: MILESTONE_DUE_DESC_SORT,
},
[DUE_DATE_ASC]: {
sort: DUE_DATE,
},
[DUE_DATE_DESC]: {
sort: DUE_DATE_DESC_SORT,
},
[POPULARITY_ASC]: {
sort: POPULARITY_ASC_SORT,
},
[POPULARITY_DESC]: {
sort: POPULARITY,
},
[LABEL_PRIORITY_DESC]: {
sort: LABEL_PRIORITY,
},
[RELATIVE_POSITION_DESC]: {
sort: RELATIVE_POSITION,
per_page: 100,
},
[WEIGHT_ASC]: {
sort: WEIGHT,
},
[WEIGHT_DESC]: {
sort: WEIGHT_DESC_SORT,
},
[BLOCKING_ISSUES_DESC]: {
sort: BLOCKING_ISSUES_DESC_SORT,
},
}; };
export const MAX_LIST_SIZE = 10; export const MAX_LIST_SIZE = 10;
@ -297,12 +198,7 @@ export const TOKEN_TYPE_WEIGHT = 'weight';
export const filters = { export const filters = {
[TOKEN_TYPE_AUTHOR]: { [TOKEN_TYPE_AUTHOR]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'authorUsername',
[NORMAL_FILTER]: 'author_username',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[author_username]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
@ -315,13 +211,8 @@ export const filters = {
}, },
[TOKEN_TYPE_ASSIGNEE]: { [TOKEN_TYPE_ASSIGNEE]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'assigneeUsernames',
[NORMAL_FILTER]: 'assignee_username', [SPECIAL_FILTER]: 'assigneeId',
[SPECIAL_FILTER]: 'assignee_id',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[assignee_username]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
@ -336,12 +227,7 @@ export const filters = {
}, },
[TOKEN_TYPE_MILESTONE]: { [TOKEN_TYPE_MILESTONE]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'milestoneTitle',
[NORMAL_FILTER]: 'milestone',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[milestone]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
@ -354,16 +240,13 @@ export const filters = {
}, },
[TOKEN_TYPE_LABEL]: { [TOKEN_TYPE_LABEL]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'labelName',
[NORMAL_FILTER]: 'labels', [SPECIAL_FILTER]: 'labelName',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[labels]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
[NORMAL_FILTER]: 'label_name[]', [NORMAL_FILTER]: 'label_name[]',
[SPECIAL_FILTER]: 'label_name[]',
}, },
[OPERATOR_IS_NOT]: { [OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[label_name][]', [NORMAL_FILTER]: 'not[label_name][]',
@ -372,10 +255,8 @@ export const filters = {
}, },
[TOKEN_TYPE_MY_REACTION]: { [TOKEN_TYPE_MY_REACTION]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'myReactionEmoji',
[NORMAL_FILTER]: 'my_reaction_emoji', [SPECIAL_FILTER]: 'myReactionEmoji',
[SPECIAL_FILTER]: 'my_reaction_emoji',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
@ -386,9 +267,7 @@ export const filters = {
}, },
[TOKEN_TYPE_CONFIDENTIAL]: { [TOKEN_TYPE_CONFIDENTIAL]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'confidential',
[NORMAL_FILTER]: 'confidential',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
@ -398,33 +277,23 @@ export const filters = {
}, },
[TOKEN_TYPE_ITERATION]: { [TOKEN_TYPE_ITERATION]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'iterationId',
[NORMAL_FILTER]: 'iteration_title', [SPECIAL_FILTER]: 'iterationWildcardId',
[SPECIAL_FILTER]: 'iteration_id',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[iteration_title]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
[NORMAL_FILTER]: 'iteration_title', [NORMAL_FILTER]: 'iteration_id',
[SPECIAL_FILTER]: 'iteration_id', [SPECIAL_FILTER]: 'iteration_id',
}, },
[OPERATOR_IS_NOT]: { [OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[iteration_title]', [NORMAL_FILTER]: 'not[iteration_id]',
}, },
}, },
}, },
[TOKEN_TYPE_EPIC]: { [TOKEN_TYPE_EPIC]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'epicId',
[NORMAL_FILTER]: 'epic_id', [SPECIAL_FILTER]: 'epicId',
[SPECIAL_FILTER]: 'epic_id',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[epic_id]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {
@ -438,13 +307,8 @@ export const filters = {
}, },
[TOKEN_TYPE_WEIGHT]: { [TOKEN_TYPE_WEIGHT]: {
[API_PARAM]: { [API_PARAM]: {
[OPERATOR_IS]: { [NORMAL_FILTER]: 'weight',
[NORMAL_FILTER]: 'weight', [SPECIAL_FILTER]: 'weight',
[SPECIAL_FILTER]: 'weight',
},
[OPERATOR_IS_NOT]: {
[NORMAL_FILTER]: 'not[weight]',
},
}, },
[URL_PARAM]: { [URL_PARAM]: {
[OPERATOR_IS]: { [OPERATOR_IS]: {

54
app/assets/javascripts/issues_list/utils.js
View File

@ -1,4 +1,5 @@
import { import {
API_PARAM,
BLOCKING_ISSUES_DESC, BLOCKING_ISSUES_DESC,
CREATED_ASC, CREATED_ASC,
CREATED_DESC, CREATED_DESC,
@ -6,29 +7,36 @@ import {
DUE_DATE_DESC, DUE_DATE_DESC,
DUE_DATE_VALUES, DUE_DATE_VALUES,
filters, filters,
LABEL_PRIORITY_ASC,
LABEL_PRIORITY_DESC, LABEL_PRIORITY_DESC,
MILESTONE_DUE_ASC, MILESTONE_DUE_ASC,
MILESTONE_DUE_DESC, MILESTONE_DUE_DESC,
NORMAL_FILTER, NORMAL_FILTER,
POPULARITY_ASC, POPULARITY_ASC,
POPULARITY_DESC, POPULARITY_DESC,
PRIORITY_ASC,
PRIORITY_DESC, PRIORITY_DESC,
RELATIVE_POSITION_DESC, RELATIVE_POSITION_ASC,
SPECIAL_FILTER, SPECIAL_FILTER,
SPECIAL_FILTER_VALUES, SPECIAL_FILTER_VALUES,
TOKEN_TYPE_ASSIGNEE, TOKEN_TYPE_ASSIGNEE,
TOKEN_TYPE_ITERATION,
UPDATED_ASC, UPDATED_ASC,
UPDATED_DESC, UPDATED_DESC,
URL_PARAM,
urlSortParams, urlSortParams,
WEIGHT_ASC, WEIGHT_ASC,
WEIGHT_DESC, WEIGHT_DESC,
} from '~/issues_list/constants'; } from '~/issues_list/constants';
import { isPositiveInteger } from '~/lib/utils/number_utils'; import { isPositiveInteger } from '~/lib/utils/number_utils';
import { __ } from '~/locale'; import { __ } from '~/locale';
import { FILTERED_SEARCH_TERM } from '~/vue_shared/components/filtered_search_bar/constants'; import {
FILTERED_SEARCH_TERM,
OPERATOR_IS_NOT,
} from '~/vue_shared/components/filtered_search_bar/constants';
export const getSortKey = (sort) => export const getSortKey = (sort) =>
Object.keys(urlSortParams).find((key) => urlSortParams[key].sort === sort); Object.keys(urlSortParams).find((key) => urlSortParams[key] === sort);
export const getDueDateValue = (value) => (DUE_DATE_VALUES.includes(value) ? value : undefined); export const getDueDateValue = (value) => (DUE_DATE_VALUES.includes(value) ? value : undefined);
@ -38,7 +46,7 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 1, id: 1,
title: __('Priority'), title: __('Priority'),
sortDirection: { sortDirection: {
ascending: PRIORITY_DESC, ascending: PRIORITY_ASC,
descending: PRIORITY_DESC, descending: PRIORITY_DESC,
}, },
}, },
@ -86,7 +94,7 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 7, id: 7,
title: __('Label priority'), title: __('Label priority'),
sortDirection: { sortDirection: {
ascending: LABEL_PRIORITY_DESC, ascending: LABEL_PRIORITY_ASC,
descending: LABEL_PRIORITY_DESC, descending: LABEL_PRIORITY_DESC,
}, },
}, },
@ -94,8 +102,8 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
id: 8, id: 8,
title: __('Manual'), title: __('Manual'),
sortDirection: { sortDirection: {
ascending: RELATIVE_POSITION_DESC, ascending: RELATIVE_POSITION_ASC,
descending: RELATIVE_POSITION_DESC, descending: RELATIVE_POSITION_ASC,
}, },
}, },
]; ];
@ -128,7 +136,7 @@ export const getSortOptions = (hasIssueWeightsFeature, hasBlockedIssuesFeature)
const tokenTypes = Object.keys(filters); const tokenTypes = Object.keys(filters);
const getUrlParams = (tokenType) => const getUrlParams = (tokenType) =>
Object.values(filters[tokenType].urlParam).flatMap((filterObj) => Object.values(filterObj)); Object.values(filters[tokenType][URL_PARAM]).flatMap((filterObj) => Object.values(filterObj));
const urlParamKeys = tokenTypes.flatMap(getUrlParams); const urlParamKeys = tokenTypes.flatMap(getUrlParams);
@ -136,7 +144,7 @@ const getTokenTypeFromUrlParamKey = (urlParamKey) =>
tokenTypes.find((tokenType) => getUrlParams(tokenType).includes(urlParamKey)); tokenTypes.find((tokenType) => getUrlParams(tokenType).includes(urlParamKey));
const getOperatorFromUrlParamKey = (tokenType, urlParamKey) => const getOperatorFromUrlParamKey = (tokenType, urlParamKey) =>
Object.entries(filters[tokenType].urlParam).find(([, filterObj]) => Object.entries(filters[tokenType][URL_PARAM]).find(([, filterObj]) =>
Object.values(filterObj).includes(urlParamKey), Object.values(filterObj).includes(urlParamKey),
)[0]; )[0];
@ -178,12 +186,36 @@ const getFilterType = (data, tokenType = '') =>
? SPECIAL_FILTER ? SPECIAL_FILTER
: NORMAL_FILTER; : NORMAL_FILTER;
export const convertToParams = (filterTokens, paramType) => const isIterationSpecialValue = (tokenType, value) =>
tokenType === TOKEN_TYPE_ITERATION && SPECIAL_FILTER_VALUES.includes(value);
export const convertToApiParams = (filterTokens) => {
const params = {};
const not = {};
filterTokens
.filter((token) => token.type !== FILTERED_SEARCH_TERM)
.forEach((token) => {
const filterType = getFilterType(token.value.data, token.type);
const field = filters[token.type][API_PARAM][filterType];
const obj = token.value.operator === OPERATOR_IS_NOT ? not : params;
const data = isIterationSpecialValue(token.type, token.value.data)
? token.value.data.toUpperCase()
: token.value.data;
Object.assign(obj, {
[field]: obj[field] ? [obj[field], data].flat() : data,
});
});
return Object.keys(not).length ? Object.assign(params, { not }) : params;
};
export const convertToUrlParams = (filterTokens) =>
filterTokens filterTokens
.filter((token) => token.type !== FILTERED_SEARCH_TERM) .filter((token) => token.type !== FILTERED_SEARCH_TERM)
.reduce((acc, token) => { .reduce((acc, token) => {
const filterType = getFilterType(token.value.data, token.type); const filterType = getFilterType(token.value.data, token.type);
const param = filters[token.type][paramType][token.value.operator]?.[filterType]; const param = filters[token.type][URL_PARAM][token.value.operator]?.[filterType];
return Object.assign(acc, { return Object.assign(acc, {
[param]: acc[param] ? [acc[param], token.value.data].flat() : token.value.data, [param]: acc[param] ? [acc[param], token.value.data].flat() : token.value.data,
}); });

4
app/assets/javascripts/nav/components/top_nav_dropdown_menu.vue
View File

@ -72,7 +72,7 @@ export default {
<template> <template>
<div class="gl-display-flex gl-align-items-stretch"> <div class="gl-display-flex gl-align-items-stretch">
<div <div
class="gl-w-grid-size-30 gl-flex-shrink-0 gl-bg-gray-10 gl-py-3 gl-px-5" class="gl-w-grid-size-30 gl-flex-shrink-0 gl-bg-gray-10 gl-p-3"
:class="menuClass" :class="menuClass"
data-testid="menu-sidebar" data-testid="menu-sidebar"
> >
@ -81,7 +81,7 @@ export default {
<keep-alive-slots <keep-alive-slots
v-show="activeView" v-show="activeView"
:slot-key="activeView" :slot-key="activeView"
class="gl-w-grid-size-40 gl-overflow-hidden gl-py-3 gl-px-5" class="gl-w-grid-size-40 gl-overflow-hidden gl-p-3"
data-testid="menu-subview" data-testid="menu-subview"
data-qa-selector="menu_subview_container" data-qa-selector="menu_subview_container"
> >

19
app/assets/javascripts/vue_shared/components/filtered_search_bar/tokens/epic_token.vue
View File

@ -56,7 +56,7 @@ export default {
} }
// Current value is a string. // Current value is a string.
const [groupPath, idProperty] = this.currentValue?.split('::&'); const [groupPath, idProperty] = this.currentValue?.split(this.$options.separator);
return this.epics.find( return this.epics.find(
(epic) => (epic) =>
epic.group_full_path === groupPath && epic.group_full_path === groupPath &&
@ -65,6 +65,9 @@ export default {
} }
return null; return null;
}, },
displayText() {
return `${this.activeEpic?.title}${this.$options.separator}${this.activeEpic?.iid}`;
},
}, },
watch: { watch: {
active: { active: {
@ -103,8 +106,10 @@ export default {
this.fetchEpicsBySearchTerm({ epicPath, search: data }); this.fetchEpicsBySearchTerm({ epicPath, search: data });
}, DEBOUNCE_DELAY), }, DEBOUNCE_DELAY),
getEpicDisplayText(epic) { getValue(epic) {
return `${epic.title}${this.$options.separator}${epic.iid}`; return this.config.useIdValue
? String(epic[this.idProperty])
: `${epic.group_full_path}${this.$options.separator}${epic[this.idProperty]}`;
}, },
}, },
}; };
@ -118,7 +123,7 @@ export default {
@input="searchEpics" @input="searchEpics"
> >
<template #view="{ inputValue }"> <template #view="{ inputValue }">
{{ activeEpic ? getEpicDisplayText(activeEpic) : inputValue }} {{ activeEpic ? displayText : inputValue }}
</template> </template>
<template #suggestions> <template #suggestions>
<gl-filtered-search-suggestion <gl-filtered-search-suggestion
@ -131,11 +136,7 @@ export default {
<gl-dropdown-divider v-if="defaultEpics.length" /> <gl-dropdown-divider v-if="defaultEpics.length" />
<gl-loading-icon v-if="loading" /> <gl-loading-icon v-if="loading" />
<template v-else> <template v-else>
<gl-filtered-search-suggestion <gl-filtered-search-suggestion v-for="epic in epics" :key="epic.id" :value="getValue(epic)">
v-for="epic in epics"
:key="epic.id"
:value="`${epic.group_full_path}::&${epic[idProperty]}`"
>
{{ epic.title }} {{ epic.title }}
</gl-filtered-search-suggestion> </gl-filtered-search-suggestion>
</template> </template>

10
app/assets/javascripts/vue_shared/components/filtered_search_bar/tokens/iteration_token.vue
View File

@ -30,7 +30,6 @@ export default {
data() { data() {
return { return {
iterations: this.config.initialIterations || [], iterations: this.config.initialIterations || [],
defaultIterations: this.config.defaultIterations || DEFAULT_ITERATIONS,
loading: true, loading: true,
}; };
}, },
@ -39,7 +38,10 @@ export default {
return this.value.data; return this.value.data;
}, },
activeIteration() { activeIteration() {
return this.iterations.find((iteration) => iteration.title === this.currentValue); return this.iterations.find((iteration) => iteration.id === Number(this.currentValue));
},
defaultIterations() {
return this.config.defaultIterations || DEFAULT_ITERATIONS;
}, },
}, },
watch: { watch: {
@ -99,8 +101,8 @@ export default {
<template v-else> <template v-else>
<gl-filtered-search-suggestion <gl-filtered-search-suggestion
v-for="iteration in iterations" v-for="iteration in iterations"
:key="iteration.title" :key="iteration.id"
:value="iteration.title" :value="String(iteration.id)"
> >
{{ iteration.title }} {{ iteration.title }}
</gl-filtered-search-suggestion> </gl-filtered-search-suggestion>

23
app/assets/javascripts/vue_shared/components/user_select/user_select.vue
View File

@ -96,9 +96,6 @@ export default {
}, },
}, },
searchUsers: { searchUsers: {
// TODO Remove error policy
// https://gitlab.com/gitlab-org/gitlab/-/issues/329750
errorPolicy: 'all',
query: searchUsers, query: searchUsers,
variables() { variables() {
return { return {
@ -111,28 +108,10 @@ export default {
return !this.isEditing; return !this.isEditing;
}, },
update(data) { update(data) {
// TODO Remove null filter (BE fix required)
// https://gitlab.com/gitlab-org/gitlab/-/issues/329750
return data.workspace?.users?.nodes.filter((x) => x?.user).map(({ user }) => user) || []; return data.workspace?.users?.nodes.filter((x) => x?.user).map(({ user }) => user) || [];
}, },
debounce: ASSIGNEES_DEBOUNCE_DELAY, debounce: ASSIGNEES_DEBOUNCE_DELAY,
error({ graphQLErrors }) { error() {
// TODO This error suppression is temporary (BE fix required)
// https://gitlab.com/gitlab-org/gitlab/-/issues/329750
const isNullError = ({ message }) => {
return message === 'Cannot return null for non-nullable field GroupMember.user';
};
if (graphQLErrors?.length > 0 && graphQLErrors.every(isNullError)) {
// only null-related errors exist, suppress them.
// eslint-disable-next-line no-console
console.error(
"Suppressing the error 'Cannot return null for non-nullable field GroupMember.user'. Please see https://gitlab.com/gitlab-org/gitlab/-/issues/329750",
);
this.isSearching = false;
return;
}
this.$emit('error'); this.$emit('error');
this.isSearching = false; this.isSearching = false;
}, },

2
app/controllers/boards/issues_controller.rb
View File

@ -136,7 +136,7 @@ module Boards
def issue_params def issue_params
params.require(:issue) params.require(:issue)
.permit(:title, :milestone_id, :project_id) .permit(:title, :milestone_id, :project_id)
.merge(board_id: params[:board_id], list_id: params[:list_id], request: request) .merge(board_id: params[:board_id], list_id: params[:list_id])
end end
def serializer def serializer

39
app/controllers/concerns/spammable_actions.rb
View File

@ -47,31 +47,20 @@ module SpammableActions
end end
end end
def spammable_params # TODO: This method is currently only needed for issue create and update. It can be removed when:
# NOTE: For the legacy reCAPTCHA implementation based on the HTML/HAML form, the #
# 'g-recaptcha-response' field name comes from `Recaptcha::ClientHelper#recaptcha_tags` in the # 1. Issue create is is converted to a client/JS based approach instead of the legacy HAML
# recaptcha gem, which is called from the HAML `_recaptcha_form.html.haml` form. # `_recaptcha_form.html.haml` which is rendered via the `projects/issues/verify` template.
# # In this case, which is based on the legacy reCAPTCHA implementation using the HTML/HAML form,
# It is used in the `Recaptcha::Verify#verify_recaptcha` to extract the value from `params`, # the 'g-recaptcha-response' field name comes from `Recaptcha::ClientHelper#recaptcha_tags` in the
# if the `response` option is not passed explicitly. # recaptcha gem, which is called from the HAML `_recaptcha_form.html.haml` form.
# # 2. Issue update is converted to use the headers-based approach, which will require adding
# Instead of relying on this behavior, we are extracting and passing it explicitly. This will # support to captcha_modal_axios_interceptor.js like we have already added to
# make it consistent with the newer, modern reCAPTCHA verification process as it will be # apollo_captcha_link.js.
# implemented via the GraphQL API and in Vue components via the native reCAPTCHA Javascript API, # In this case, the `captcha_response` field name comes from our captcha_modal_axios_interceptor.js.
# which requires that the recaptcha response param be obtained and passed explicitly. def extract_legacy_spam_params_to_headers
# request.headers['X-GitLab-Captcha-Response'] = params['g-recaptcha-response'] || params[:captcha_response]
# It can also be expanded to multiple fields when we move to future alternative captcha request.headers['X-GitLab-Spam-Log-Id'] = params[:spam_log_id]
# implementations such as FriendlyCaptcha. See https://gitlab.com/gitlab-org/gitlab/-/issues/273480
# After this newer GraphQL/JS API process is fully supported by the backend, we can remove the
# check for the 'g-recaptcha-response' field and other HTML/HAML form-specific support.
captcha_response = params['g-recaptcha-response'] || params[:captcha_response]
{
request: request,
spam_log_id: params[:spam_log_id],
captcha_response: captcha_response
}
end end
def spammable def spammable

11
app/controllers/projects/issues_controller.rb
View File

@ -129,12 +129,14 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def create def create
create_params = issue_params.merge(spammable_params).merge( extract_legacy_spam_params_to_headers
create_params = issue_params.merge(
merge_request_to_resolve_discussions_of: params[:merge_request_to_resolve_discussions_of], merge_request_to_resolve_discussions_of: params[:merge_request_to_resolve_discussions_of],
discussion_to_resolve: params[:discussion_to_resolve] discussion_to_resolve: params[:discussion_to_resolve]
) )
service = ::Issues::CreateService.new(project: project, current_user: current_user, params: create_params) spam_params = ::Spam::SpamParams.new_from_request(request: request)
service = ::Issues::CreateService.new(project: project, current_user: current_user, params: create_params, spam_params: spam_params)
@issue = service.execute @issue = service.execute
create_vulnerability_issue_feedback(issue) create_vulnerability_issue_feedback(issue)
@ -334,8 +336,9 @@ class Projects::IssuesController < Projects::ApplicationController
end end
def update_service def update_service
update_params = issue_params.merge(spammable_params) extract_legacy_spam_params_to_headers
::Issues::UpdateService.new(project: project, current_user: current_user, params: update_params) spam_params = ::Spam::SpamParams.new_from_request(request: request)
::Issues::UpdateService.new(project: project, current_user: current_user, params: issue_params, spam_params: spam_params)
end end
def finder_type def finder_type

2
app/controllers/projects/usage_ping_controller.rb → app/controllers/projects/service_ping_controller.rb
View File

@ -1,6 +1,6 @@
# frozen_string_literal: true # frozen_string_literal: true
class Projects::UsagePingController < Projects::ApplicationController class Projects::ServicePingController < Projects::ApplicationController
before_action :authenticate_user! before_action :authenticate_user!
feature_category :usage_ping feature_category :usage_ping

19
app/graphql/mutations/concerns/mutations/spam_protection.rb
View File

@ -16,25 +16,6 @@ module Mutations
private private
# additional_spam_params -> hash
#
# Used from a spammable mutation's #resolve method to generate
# the required additional spam/CAPTCHA params which must be merged into the params
# passed to the constructor of a service, where they can then be used in the service
# to perform spam checking via SpamActionService.
#
# Also accesses the #context of the mutation's Resolver superclass to obtain the request.
#
# Example:
#
# existing_args.merge!(additional_spam_params)
def additional_spam_params
{
api: true,
request: context[:request]
}
end
def spam_action_response(object) def spam_action_response(object)
fields = spam_action_response_fields(object) fields = spam_action_response_fields(object)

3
app/graphql/mutations/issues/create.rb
View File

@ -73,7 +73,8 @@ module Mutations
project = authorized_find!(project_path) project = authorized_find!(project_path)
params = build_create_issue_params(attributes.merge(author_id: current_user.id)) params = build_create_issue_params(attributes.merge(author_id: current_user.id))
issue = ::Issues::CreateService.new(project: project, current_user: current_user, params: params).execute spam_params = ::Spam::SpamParams.new_from_request(request: context[:request])
issue = ::Issues::CreateService.new(project: project, current_user: current_user, params: params, spam_params: spam_params).execute
if issue.spam? if issue.spam?
issue.errors.add(:base, 'Spam detected.') issue.errors.add(:base, 'Spam detected.')

5
app/graphql/mutations/issues/set_confidential.rb
View File

@ -13,8 +13,11 @@ module Mutations
def resolve(project_path:, iid:, confidential:) def resolve(project_path:, iid:, confidential:)
issue = authorized_find!(project_path: project_path, iid: iid) issue = authorized_find!(project_path: project_path, iid: iid)
project = issue.project project = issue.project
# Changing confidentiality affects spam checking rules, therefore we need to provide
# spam_params so a check can be performed.
spam_params = ::Spam::SpamParams.new_from_request(request: context[:request])
::Issues::UpdateService.new(project: project, current_user: current_user, params: { confidential: confidential }) ::Issues::UpdateService.new(project: project, current_user: current_user, params: { confidential: confidential }, spam_params: spam_params)
.execute(issue) .execute(issue)
{ {

3
app/graphql/mutations/issues/update.rb
View File

@ -31,7 +31,8 @@ module Mutations
issue = authorized_find!(project_path: project_path, iid: iid) issue = authorized_find!(project_path: project_path, iid: iid)
project = issue.project project = issue.project
::Issues::UpdateService.new(project: project, current_user: current_user, params: args).execute(issue) spam_params = ::Spam::SpamParams.new_from_request(request: context[:request])
::Issues::UpdateService.new(project: project, current_user: current_user, params: args, spam_params: spam_params).execute(issue)
{ {
issue: issue, issue: issue,

10
app/graphql/mutations/snippets/create.rb
View File

@ -49,7 +49,9 @@ module Mutations
process_args_for_params!(args) process_args_for_params!(args)
service_response = ::Snippets::CreateService.new(project: project, current_user: current_user, params: args).execute spam_params = ::Spam::SpamParams.new_from_request(request: context[:request])
service = ::Snippets::CreateService.new(project: project, current_user: current_user, params: args, spam_params: spam_params)
service_response = service.execute
# Only when the user is not an api user and the operation was successful # Only when the user is not an api user and the operation was successful
if !api_user? && service_response.success? if !api_user? && service_response.success?
@ -81,12 +83,6 @@ module Mutations
# it's the expected key param # it's the expected key param
args[:files] = args.delete(:uploaded_files) args[:files] = args.delete(:uploaded_files)
if Feature.enabled?(:snippet_spam)
args.merge!(additional_spam_params)
else
args[:disable_spam_action_service] = true
end
# Return nil to make it explicit that this method is mutating the args parameter, and that # Return nil to make it explicit that this method is mutating the args parameter, and that
# the return value is not relevant and is not to be used. # the return value is not relevant and is not to be used.
nil nil

10
app/graphql/mutations/snippets/update.rb
View File

@ -34,7 +34,9 @@ module Mutations
process_args_for_params!(args) process_args_for_params!(args)
service_response = ::Snippets::UpdateService.new(project: snippet.project, current_user: current_user, params: args).execute(snippet) spam_params = ::Spam::SpamParams.new_from_request(request: context[:request])
service = ::Snippets::UpdateService.new(project: snippet.project, current_user: current_user, params: args, spam_params: spam_params)
service_response = service.execute(snippet)
# TODO: DRY this up - From here down, this is all duplicated with Mutations::Snippets::Create#resolve, except for # TODO: DRY this up - From here down, this is all duplicated with Mutations::Snippets::Create#resolve, except for
# `snippet.reset`, which is required in order to return the object in its non-dirty, unmodified, database state # `snippet.reset`, which is required in order to return the object in its non-dirty, unmodified, database state
@ -62,12 +64,6 @@ module Mutations
def process_args_for_params!(args) def process_args_for_params!(args)
convert_blob_actions_to_snippet_actions!(args) convert_blob_actions_to_snippet_actions!(args)
if Feature.enabled?(:snippet_spam)
args.merge!(additional_spam_params)
else
args[:disable_spam_action_service] = true
end
# Return nil to make it explicit that this method is mutating the args parameter, and that # Return nil to make it explicit that this method is mutating the args parameter, and that
# the return value is not relevant and is not to be used. # the return value is not relevant and is not to be used.
nil nil

8
app/mailers/emails/service_desk.rb
View File

@ -20,9 +20,7 @@ module Emails
options = service_desk_options(email_sender, 'thank_you', @issue.external_author) options = service_desk_options(email_sender, 'thank_you', @issue.external_author)
.merge(subject: "Re: #{subject_base}") .merge(subject: "Re: #{subject_base}")
mail_new_thread(@issue, options).tap do mail_new_thread(@issue, options)
Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email)
end
end end
def service_desk_new_note_email(issue_id, note_id, recipient) def service_desk_new_note_email(issue_id, note_id, recipient)
@ -33,9 +31,7 @@ module Emails
options = service_desk_options(email_sender, 'new_note', recipient) options = service_desk_options(email_sender, 'new_note', recipient)
.merge(subject: subject_base) .merge(subject: subject_base)
mail_answer_thread(@issue, options).tap do mail_answer_thread(@issue, options)
Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email)
end
end end
private private

7
app/models/integration.rb
View File

@ -14,14 +14,14 @@ class Integration < ApplicationRecord
self.table_name = 'services' self.table_name = 'services'
INTEGRATION_NAMES = %w[ INTEGRATION_NAMES = %w[
asana assembla bamboo bugzilla buildkite campfire confluence custom_issue_tracker discord asana assembla bamboo bugzilla buildkite campfire confluence custom_issue_tracker datadog discord
drone_ci emails_on_push ewm external_wiki flowdock hangouts_chat irker jira drone_ci emails_on_push ewm external_wiki flowdock hangouts_chat irker jira
mattermost mattermost_slash_commands microsoft_teams packagist pipelines_email mattermost mattermost_slash_commands microsoft_teams packagist pipelines_email
pivotaltracker prometheus pushover redmine slack slack_slash_commands teamcity unify_circuit webex_teams youtrack pivotaltracker prometheus pushover redmine slack slack_slash_commands teamcity unify_circuit webex_teams youtrack
].freeze ].freeze
PROJECT_SPECIFIC_INTEGRATION_NAMES = %w[ PROJECT_SPECIFIC_INTEGRATION_NAMES = %w[
datadog jenkins jenkins
].freeze ].freeze
# Fake integrations to help with local development. # Fake integrations to help with local development.
@ -54,6 +54,9 @@ class Integration < ApplicationRecord
redmine redmine
slack slack_slash_commands slack slack_slash_commands
teamcity teamcity
unify_circuit
webex_teams
youtrack
].to_set.freeze ].to_set.freeze
def self.renamed?(name) def self.renamed?(name)

8
app/models/project.rb
View File

@ -188,9 +188,9 @@ class Project < ApplicationRecord
has_one :slack_integration, class_name: 'Integrations::Slack' has_one :slack_integration, class_name: 'Integrations::Slack'
has_one :slack_slash_commands_integration, class_name: 'Integrations::SlackSlashCommands' has_one :slack_slash_commands_integration, class_name: 'Integrations::SlackSlashCommands'
has_one :teamcity_integration, class_name: 'Integrations::Teamcity' has_one :teamcity_integration, class_name: 'Integrations::Teamcity'
has_one :unify_circuit_service, class_name: 'Integrations::UnifyCircuit' has_one :unify_circuit_integration, class_name: 'Integrations::UnifyCircuit'
has_one :webex_teams_service, class_name: 'Integrations::WebexTeams' has_one :webex_teams_integration, class_name: 'Integrations::WebexTeams'
has_one :youtrack_service, class_name: 'Integrations::Youtrack' has_one :youtrack_integration, class_name: 'Integrations::Youtrack'
has_one :root_of_fork_network, has_one :root_of_fork_network,
foreign_key: 'root_project_id', foreign_key: 'root_project_id',
@ -1407,8 +1407,6 @@ class Project < ApplicationRecord
end end
def disabled_services def disabled_services
return %w[datadog] unless Feature.enabled?(:datadog_ci_integration, self)
[] []
end end

4
app/services/boards/issues/create_service.rb
View File

@ -30,7 +30,9 @@ module Boards
end end
def create_issue(params) def create_issue(params)
::Issues::CreateService.new(project: project, current_user: current_user, params: params).execute # NOTE: We are intentionally not doing a spam/CAPTCHA check for issues created via boards.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/29400#note_598479184 for more context.
::Issues::CreateService.new(project: project, current_user: current_user, params: params, spam_params: nil).execute
end end
end end
end end

25
app/services/captcha/captcha_verification_service.rb
View File

@ -7,20 +7,27 @@ module Captcha
class CaptchaVerificationService class CaptchaVerificationService
include Recaptcha::Verify include Recaptcha::Verify
# Currently the only value that is used out of the request by the reCAPTCHA library
# is 'remote_ip'. Therefore, we just create a struct to avoid passing the full request
# object through all the service layer objects, and instead just rely on passing only
# the required remote_ip value. This eliminates the need to couple the service layer
# to the HTTP request (for the purpose of this service, at least).
RequestStruct = Struct.new(:remote_ip)
def initialize(spam_params:)
@spam_params = spam_params
end
## ##
# Performs verification of a captcha response. # Performs verification of a captcha response.
# #
# 'captcha_response' parameter is the response from the user solving a client-side captcha.
#
# 'request' parameter is the request which submitted the captcha.
#
# NOTE: Currently only supports reCAPTCHA, and is not yet used in all places of the app in which # NOTE: Currently only supports reCAPTCHA, and is not yet used in all places of the app in which
# captchas are verified, but these can be addressed in future MRs. See: # captchas are verified, but these can be addressed in future MRs. See:
# https://gitlab.com/gitlab-org/gitlab/-/issues/273480 # https://gitlab.com/gitlab-org/gitlab/-/issues/273480
def execute(captcha_response: nil, request:) def execute
return false unless captcha_response return false unless spam_params.captcha_response
@request = request @request = RequestStruct.new(spam_params.ip_address)
Gitlab::Recaptcha.load_configurations! Gitlab::Recaptcha.load_configurations!
@ -31,11 +38,13 @@ module Captcha
# 2. We want control over the wording and i18n of the message # 2. We want control over the wording and i18n of the message
# 3. We want a consistent interface and behavior when adding support for other captcha # 3. We want a consistent interface and behavior when adding support for other captcha
# libraries which may not support automatically adding errors to the model. # libraries which may not support automatically adding errors to the model.
verify_recaptcha(response: captcha_response) verify_recaptcha(response: spam_params.captcha_response)
end end
private private
attr_reader :spam_params
# The recaptcha library's Recaptcha::Verify#verify_recaptcha method requires that # The recaptcha library's Recaptcha::Verify#verify_recaptcha method requires that
# 'request' be a readable attribute - it doesn't support passing it as an options argument. # 'request' be a readable attribute - it doesn't support passing it as an options argument.
attr_reader :request attr_reader :request

3
app/services/incident_management/incidents/create_service.rb
View File

@ -21,7 +21,8 @@ module IncidentManagement
title: title, title: title,
description: description, description: description,
issue_type: ISSUE_TYPE issue_type: ISSUE_TYPE
} },
spam_params: nil
).execute ).execute
return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid?

5
app/services/issuable/import_csv/base_service.rb
View File

@ -68,7 +68,10 @@ module Issuable
end end
def create_issuable(attributes) def create_issuable(attributes)
create_issuable_class.new(project: @project, current_user: @user, params: attributes).execute # NOTE: CSV imports are performed by workers, so we do not have a request context in order
# to create a SpamParams object to pass to the issuable create service.
spam_params = nil
create_issuable_class.new(project: @project, current_user: @user, params: attributes, spam_params: spam_params).execute
end end
def email_results_to_user def email_results_to_user

6
app/services/issues/clone_service.rb
View File

@ -55,9 +55,13 @@ module Issues
new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params) new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params)
# spam checking is not necessary, as no new content is being created. Passing nil for
# spam_params will cause SpamActionService to skip checking and return a success response.
spam_params = nil
# Skip creation of system notes for existing attributes of the issue. The system notes of the old # Skip creation of system notes for existing attributes of the issue. The system notes of the old
# issue are copied over so we don't want to end up with duplicate notes. # issue are copied over so we don't want to end up with duplicate notes.
CreateService.new(project: target_project, current_user: current_user, params: new_params).execute(skip_system_notes: true) CreateService.new(project: target_project, current_user: current_user, params: new_params, spam_params: spam_params).execute(skip_system_notes: true)
end end
def queue_copy_designs def queue_copy_designs

25
app/services/issues/create_service.rb
View File

@ -4,10 +4,21 @@ module Issues
class CreateService < Issues::BaseService class CreateService < Issues::BaseService
include ResolveDiscussions include ResolveDiscussions
def execute(skip_system_notes: false) # NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because
@request = params.delete(:request) # spam_checking is likely to be necessary. However, if there is not a request available in scope
@spam_params = Spam::SpamActionService.filter_spam_params!(params, @request) # in the caller (for example, an issue created via email) and the required arguments to the
# SpamParams constructor are not otherwise available, spam_params: must be explicitly passed as nil.
def initialize(project:, current_user: nil, params: {}, spam_params:)
# Temporary check to ensure we are no longer passing request in params now that we have
# introduced spam_params. Raise an exception if it is present.
# Remove after https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58603 is complete.
raise if params[:request]
super(project: project, current_user: current_user, params: params)
@spam_params = spam_params
end
def execute(skip_system_notes: false)
@issue = BuildService.new(project: project, current_user: current_user, params: params).execute @issue = BuildService.new(project: project, current_user: current_user, params: params).execute
filter_resolve_discussion_params filter_resolve_discussion_params
@ -18,10 +29,10 @@ module Issues
def before_create(issue) def before_create(issue)
Spam::SpamActionService.new( Spam::SpamActionService.new(
spammable: issue, spammable: issue,
request: request, spam_params: spam_params,
user: current_user, user: current_user,
action: :create action: :create
).execute(spam_params: spam_params) ).execute
# current_user (defined in BaseService) is not available within run_after_commit block # current_user (defined in BaseService) is not available within run_after_commit block
user = current_user user = current_user
@ -64,10 +75,10 @@ module Issues
private private
attr_reader :request, :spam_params attr_reader :spam_params
def user_agent_detail_service def user_agent_detail_service
UserAgentDetailService.new(@issue, request) UserAgentDetailService.new(spammable: @issue, spam_params: spam_params)
end end
end end
end end

1
app/services/issues/duplicate_service.rb
View File

@ -28,6 +28,7 @@ module Issues
def relate_two_issues(duplicate_issue, canonical_issue) def relate_two_issues(duplicate_issue, canonical_issue)
params = { target_issuable: canonical_issue } params = { target_issuable: canonical_issue }
IssueLinks::CreateService.new(duplicate_issue, current_user, params).execute IssueLinks::CreateService.new(duplicate_issue, current_user, params).execute
end end
end end

5
app/services/issues/move_service.rb
View File

@ -58,10 +58,13 @@ module Issues
} }
new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params) new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params)
# spam checking is not necessary, as no new content is being created. Passing nil for
# spam_params will cause SpamActionService to skip checking and return a success response.
spam_params = nil
# Skip creation of system notes for existing attributes of the issue. The system notes of the old # Skip creation of system notes for existing attributes of the issue. The system notes of the old
# issue are copied over so we don't want to end up with duplicate notes. # issue are copied over so we don't want to end up with duplicate notes.
CreateService.new(project: @target_project, current_user: @current_user, params: new_params).execute(skip_system_notes: true) CreateService.new(project: @target_project, current_user: @current_user, params: new_params, spam_params: spam_params).execute(skip_system_notes: true)
end end
def queue_copy_designs def queue_copy_designs

17
app/services/issues/update_service.rb
View File

@ -4,12 +4,17 @@ module Issues
class UpdateService < Issues::BaseService class UpdateService < Issues::BaseService
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
# NOTE: For Issues::UpdateService, we default the spam_params to nil, because spam_checking is not
# necessary in many cases, and we don't want to require every caller to explicitly pass it as nil
# to disable spam checking.
def initialize(project:, current_user: nil, params: {}, spam_params: nil)
super(project: project, current_user: current_user, params: params)
@spam_params = spam_params
end
def execute(issue) def execute(issue)
handle_move_between_ids(issue) handle_move_between_ids(issue)
@request = params.delete(:request)
@spam_params = Spam::SpamActionService.filter_spam_params!(params, @request)
change_issue_duplicate(issue) change_issue_duplicate(issue)
move_issue_to_new_project(issue) || clone_issue(issue) || update_task_event(issue) || update(issue) move_issue_to_new_project(issue) || clone_issue(issue) || update_task_event(issue) || update(issue)
end end
@ -25,10 +30,10 @@ module Issues
Spam::SpamActionService.new( Spam::SpamActionService.new(
spammable: issue, spammable: issue,
request: request, spam_params: spam_params,
user: current_user, user: current_user,
action: :update action: :update
).execute(spam_params: spam_params) ).execute
end end
def handle_changes(issue, options) def handle_changes(issue, options)
@ -127,7 +132,7 @@ module Issues
private private
attr_reader :request, :spam_params attr_reader :spam_params
def clone_issue(issue) def clone_issue(issue)
target_project = params.delete(:target_clone_project) target_project = params.delete(:target_clone_project)

1
app/services/notification_service.rb
View File

@ -396,6 +396,7 @@ class NotificationService
recipients.each do |recipient| recipients.each do |recipient|
mailer.service_desk_new_note_email(issue.id, note.id, recipient).deliver_later mailer.service_desk_new_note_email(issue.id, note.id, recipient).deliver_later
Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email)
end end
end end

22
app/services/snippets/create_service.rb
View File

@ -2,12 +2,14 @@
module Snippets module Snippets
class CreateService < Snippets::BaseService class CreateService < Snippets::BaseService
def execute # NOTE: For Issues::CreateService, we require the spam_params and do not default it to nil, because
# NOTE: disable_spam_action_service can be removed when the ':snippet_spam' feature flag is removed. # spam_checking is likely to be necessary.
disable_spam_action_service = params.delete(:disable_spam_action_service) == true def initialize(project:, current_user: nil, params: {}, spam_params:)
@request = params.delete(:request) super(project: project, current_user: current_user, params: params)
@spam_params = Spam::SpamActionService.filter_spam_params!(params, @request) @spam_params = spam_params
end
def execute
@snippet = build_from_params @snippet = build_from_params
return invalid_params_error(@snippet) unless valid_params? return invalid_params_error(@snippet) unless valid_params?
@ -18,17 +20,17 @@ module Snippets
@snippet.author = current_user @snippet.author = current_user
unless disable_spam_action_service if Feature.enabled?(:snippet_spam)
Spam::SpamActionService.new( Spam::SpamActionService.new(
spammable: @snippet, spammable: @snippet,
request: request, spam_params: spam_params,
user: current_user, user: current_user,
action: :create action: :create
).execute(spam_params: spam_params) ).execute
end end
if save_and_commit if save_and_commit
UserAgentDetailService.new(@snippet, request).create UserAgentDetailService.new(spammable: @snippet, spam_params: spam_params).create
Gitlab::UsageDataCounters::SnippetCounter.count(:create) Gitlab::UsageDataCounters::SnippetCounter.count(:create)
move_temporary_files move_temporary_files
@ -41,7 +43,7 @@ module Snippets
private private
attr_reader :snippet, :request, :spam_params attr_reader :snippet, :spam_params
def build_from_params def build_from_params
if project if project

21
app/services/snippets/update_service.rb
View File

@ -6,12 +6,15 @@ module Snippets
UpdateError = Class.new(StandardError) UpdateError = Class.new(StandardError)
def execute(snippet) # NOTE: For Snippets::UpdateService, we default the spam_params to nil, because spam_checking is not
# NOTE: disable_spam_action_service can be removed when the ':snippet_spam' feature flag is removed. # necessary in many cases, and we don't want every caller to have to explicitly pass it as nil
disable_spam_action_service = params.delete(:disable_spam_action_service) == true # to disable spam checking.
@request = params.delete(:request) def initialize(project:, current_user: nil, params: {}, spam_params: nil)
@spam_params = Spam::SpamActionService.filter_spam_params!(params, @request) super(project: project, current_user: current_user, params: params)
@spam_params = spam_params
end
def execute(snippet)
return invalid_params_error(snippet) unless valid_params? return invalid_params_error(snippet) unless valid_params?
if visibility_changed?(snippet) && !visibility_allowed?(visibility_level) if visibility_changed?(snippet) && !visibility_allowed?(visibility_level)
@ -20,13 +23,13 @@ module Snippets
update_snippet_attributes(snippet) update_snippet_attributes(snippet)
unless disable_spam_action_service if Feature.enabled?(:snippet_spam)
Spam::SpamActionService.new( Spam::SpamActionService.new(
spammable: snippet, spammable: snippet,
request: request, spam_params: spam_params,
user: current_user, user: current_user,
action: :update action: :update
).execute(spam_params: spam_params) ).execute
end end
if save_and_commit(snippet) if save_and_commit(snippet)
@ -40,7 +43,7 @@ module Snippets
private private
attr_reader :request, :spam_params attr_reader :spam_params
def visibility_changed?(snippet) def visibility_changed?(snippet)
visibility_level && visibility_level.to_i != snippet.visibility_level visibility_level && visibility_level.to_i != snippet.visibility_level

1
app/services/spam/akismet_service.rb
View File

@ -20,6 +20,7 @@ module Spam
created_at: DateTime.current, created_at: DateTime.current,
author: owner_name, author: owner_name,
author_email: owner_email, author_email: owner_email,
# NOTE: The akismet_client needs the option to be named `:referrer`, not `:referer`
referrer: options[:referer] referrer: options[:referer]
} }

96
app/services/spam/spam_action_service.rb
View File

@ -4,67 +4,22 @@ module Spam
class SpamActionService class SpamActionService
include SpamConstants include SpamConstants
## def initialize(spammable:, spam_params:, user:, action:)
# Utility method to filter SpamParams from parameters, which will later be passed to #execute
# after the spammable is created/updated based on the remaining parameters.
#
# Takes a hash of parameters from an incoming request to modify a model (via a controller,
# service, or GraphQL mutation). The parameters will either be camelCase (if they are
# received directly via controller params) or underscore_case (if they have come from
# a GraphQL mutation which has converted them to underscore), or in the
# headers when using the header based flow.
#
# Deletes the parameters which are related to spam and captcha processing, and returns
# them in a SpamParams parameters object. See:
# https://refactoring.com/catalog/introduceParameterObject.html
def self.filter_spam_params!(params, request)
# NOTE: The 'captcha_response' field can be expanded to multiple fields when we move to future
# alternative captcha implementations such as FriendlyCaptcha. See
# https://gitlab.com/gitlab-org/gitlab/-/issues/273480
headers = request&.headers || {}
api = params.delete(:api)
captcha_response = read_parameter(:captcha_response, params, headers)
spam_log_id = read_parameter(:spam_log_id, params, headers)&.to_i
SpamParams.new(api: api, captcha_response: captcha_response, spam_log_id: spam_log_id)
end
def self.read_parameter(name, params, headers)
[
params.delete(name),
params.delete(name.to_s.camelize(:lower).to_sym),
headers["X-GitLab-#{name.to_s.titlecase(keep_id_suffix: true).tr(' ', '-')}"]
].compact.first
end
attr_accessor :target, :request, :options
attr_reader :spam_log
def initialize(spammable:, request:, user:, action:)
@target = spammable @target = spammable
@request = request @spam_params = spam_params
@user = user @user = user
@action = action @action = action
@options = {}
end end
# rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/AbcSize
def execute(spam_params:) def execute
if request # If spam_params is passed as `nil`, no check will be performed. This is the easiest way to allow
options[:ip_address] = request.env['action_dispatch.remote_ip'].to_s # composed services which may not need to do spam checking to "opt out". For example, when
options[:user_agent] = request.env['HTTP_USER_AGENT'] # MoveService is calling CreateService, spam checking is not necessary, as no new content is
options[:referer] = request.env['HTTP_REFERER'] # being created.
else return ServiceResponse.success(message: 'Skipped spam check because spam_params was not present') unless spam_params
# TODO: This code is never used, because we do not perform a verification if there is not a
# request. Why? Should it be deleted? Or should we check even if there is no request?
options[:ip_address] = target.ip_address
options[:user_agent] = target.user_agent
end
recaptcha_verified = Captcha::CaptchaVerificationService.new.execute( recaptcha_verified = Captcha::CaptchaVerificationService.new(spam_params: spam_params).execute
captcha_response: spam_params.captcha_response,
request: request
)
if recaptcha_verified if recaptcha_verified
# If it's a request which is already verified through CAPTCHA, # If it's a request which is already verified through CAPTCHA,
@ -73,10 +28,9 @@ module Spam
ServiceResponse.success(message: "CAPTCHA successfully verified") ServiceResponse.success(message: "CAPTCHA successfully verified")
else else
return ServiceResponse.success(message: 'Skipped spam check because user was allowlisted') if allowlisted?(user) return ServiceResponse.success(message: 'Skipped spam check because user was allowlisted') if allowlisted?(user)
return ServiceResponse.success(message: 'Skipped spam check because request was not present') unless request
return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam? return ServiceResponse.success(message: 'Skipped spam check because it was not required') unless check_for_spam?
perform_spam_service_check(spam_params.api) perform_spam_service_check
ServiceResponse.success(message: "Spam check performed. Check #{target.class.name} spammable model for any errors or CAPTCHA requirement") ServiceResponse.success(message: "Spam check performed. Check #{target.class.name} spammable model for any errors or CAPTCHA requirement")
end end
end end
@ -86,7 +40,7 @@ module Spam
private private
attr_reader :user, :action attr_reader :user, :action, :target, :spam_params, :spam_log
## ##
# In order to be proceed to the spam check process, the target must be # In order to be proceed to the spam check process, the target must be
@ -104,7 +58,7 @@ module Spam
## ##
# Performs the spam check using the spam verdict service, and modifies the target model # Performs the spam check using the spam verdict service, and modifies the target model
# accordingly based on the result. # accordingly based on the result.
def perform_spam_service_check(api) def perform_spam_service_check
ensure_target_is_dirty ensure_target_is_dirty
# since we can check for spam, and recaptcha is not verified, # since we can check for spam, and recaptcha is not verified,
@ -113,7 +67,7 @@ module Spam
case result case result
when CONDITIONAL_ALLOW when CONDITIONAL_ALLOW
# at the moment, this means "ask for reCAPTCHA" # at the moment, this means "ask for reCAPTCHA"
create_spam_log(api) create_spam_log
break if target.allow_possible_spam? break if target.allow_possible_spam?
@ -122,12 +76,12 @@ module Spam
# TODO: remove `unless target.allow_possible_spam?` once this flag has been passed to `SpamVerdictService` # TODO: remove `unless target.allow_possible_spam?` once this flag has been passed to `SpamVerdictService`
# https://gitlab.com/gitlab-org/gitlab/-/issues/214739 # https://gitlab.com/gitlab-org/gitlab/-/issues/214739
target.spam! unless target.allow_possible_spam? target.spam! unless target.allow_possible_spam?
create_spam_log(api) create_spam_log
when BLOCK_USER when BLOCK_USER
# TODO: improve BLOCK_USER handling, non-existent until now # TODO: improve BLOCK_USER handling, non-existent until now
# https://gitlab.com/gitlab-org/gitlab/-/issues/329666 # https://gitlab.com/gitlab-org/gitlab/-/issues/329666
target.spam! unless target.allow_possible_spam? target.spam! unless target.allow_possible_spam?
create_spam_log(api) create_spam_log
when ALLOW when ALLOW
target.clear_spam_flags! target.clear_spam_flags!
when NOOP when NOOP
@ -137,16 +91,21 @@ module Spam
end end
end end
def create_spam_log(api) def create_spam_log
@spam_log = SpamLog.create!( @spam_log = SpamLog.create!(
{ {
user_id: target.author_id, user_id: target.author_id,
title: target.spam_title, title: target.spam_title,
description: target.spam_description, description: target.spam_description,
source_ip: options[:ip_address], source_ip: spam_params.ip_address,
user_agent: options[:user_agent], user_agent: spam_params.user_agent,
noteable_type: noteable_type, noteable_type: noteable_type,
via_api: api # Now, all requests are via the API, so hardcode it to true to simplify the logic and API
# of this service. See https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/2266
# for original introduction of `via_api` field.
# See discussion here about possibly deprecating this field:
# https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/2266#note_542527450
via_api: true
} }
) )
@ -159,9 +118,14 @@ module Spam
target_type: noteable_type target_type: noteable_type
} }
options = {
ip_address: spam_params.ip_address,
user_agent: spam_params.user_agent,
referer: spam_params.referer
}
SpamVerdictService.new(target: target, SpamVerdictService.new(target: target,
user: user, user: user,
request: request,
options: options, options: options,
context: context) context: context)
end end

44
app/services/spam/spam_params.rb
View File

@ -3,30 +3,54 @@
module Spam module Spam
## ##
# This class is a Parameter Object (https://refactoring.com/catalog/introduceParameterObject.html) # This class is a Parameter Object (https://refactoring.com/catalog/introduceParameterObject.html)
# which acts as an container abstraction for multiple parameter values related to spam and # which acts as an container abstraction for multiple values related to spam and
# captcha processing for a request. # captcha processing for a provided HTTP request object.
#
# It is used to encapsulate these values and allow them to be passed from the Controller/GraphQL
# layers down into to the Service layer, without needing to pass the entire request and therefore
# unnecessarily couple the Service layer to the HTTP request.
# #
# Values contained are: # Values contained are:
# #
# api: A boolean flag indicating if the request was submitted via the REST or GraphQL API
# captcha_response: The response resulting from the user solving a captcha. Currently it is # captcha_response: The response resulting from the user solving a captcha. Currently it is
# a scalar reCAPTCHA response string, but it can be expanded to an object in the future to # a scalar reCAPTCHA response string, but it can be expanded to an object in the future to
# support other captcha implementations such as FriendlyCaptcha. # support other captcha implementations such as FriendlyCaptcha. Obtained from
# spam_log_id: The id of a SpamLog record. # request.headers['X-GitLab-Captcha-Response']
# spam_log_id: The id of a SpamLog record. Obtained from request.headers['X-GitLab-Spam-Log-Id']
# ip_address = The remote IP. Obtained from request.env['action_dispatch.remote_ip']
# user_agent = The user agent. Obtained from request.env['HTTP_USER_AGENT']
# referer = The HTTP referer. Obtained from request.env['HTTP_REFERER']
#
# NOTE: The presence of these values in the request is not currently enforced. If they are missing,
# then the spam check may fail, or the SpamLog or UserAgentDetail may have missing fields.
class SpamParams class SpamParams
attr_reader :api, :captcha_response, :spam_log_id def self.new_from_request(request:)
self.new(
captcha_response: request.headers['X-GitLab-Captcha-Response'],
spam_log_id: request.headers['X-GitLab-Spam-Log-Id'],
ip_address: request.env['action_dispatch.remote_ip'].to_s,
user_agent: request.env['HTTP_USER_AGENT'],
referer: request.env['HTTP_REFERER']
)
end
def initialize(api:, captcha_response:, spam_log_id:) attr_reader :captcha_response, :spam_log_id, :ip_address, :user_agent, :referer
@api = api.present?
def initialize(captcha_response:, spam_log_id:, ip_address:, user_agent:, referer:)
@captcha_response = captcha_response @captcha_response = captcha_response
@spam_log_id = spam_log_id @spam_log_id = spam_log_id
@ip_address = ip_address
@user_agent = user_agent
@referer = referer
end end
def ==(other) def ==(other)
other.class <= self.class && other.class <= self.class &&
other.api == api &&
other.captcha_response == captcha_response && other.captcha_response == captcha_response &&
other.spam_log_id == spam_log_id other.spam_log_id == spam_log_id &&
other.ip_address == ip_address &&
other.user_agent == user_agent &&
other.referer == referer
end end
end end
end end

5
app/services/spam/spam_verdict_service.rb
View File

@ -5,9 +5,8 @@ module Spam
include AkismetMethods include AkismetMethods
include SpamConstants include SpamConstants
def initialize(user:, target:, request:, options:, context: {}) def initialize(user:, target:, options:, context: {})
@target = target @target = target
@request = request
@user = user @user = user
@options = options @options = options
@context = context @context = context
@ -59,7 +58,7 @@ module Spam
private private
attr_reader :user, :target, :request, :options, :context attr_reader :user, :target, :options, :context
def akismet_verdict def akismet_verdict
if akismet.spam? if akismet.spam?

17
app/services/user_agent_detail_service.rb
View File

@ -1,16 +1,21 @@
# frozen_string_literal: true # frozen_string_literal: true
class UserAgentDetailService class UserAgentDetailService
attr_accessor :spammable, :request def initialize(spammable:, spam_params:)
def initialize(spammable, request)
@spammable = spammable @spammable = spammable
@request = request @spam_params = spam_params
end end
def create def create
return unless request unless spam_params&.user_agent && spam_params&.ip_address
messasge = 'Skipped UserAgentDetail creation because necessary spam_params were not provided'
return ServiceResponse.success(message: messasge)
end
spammable.create_user_agent_detail(user_agent: request.env['HTTP_USER_AGENT'], ip_address: request.env['action_dispatch.remote_ip'].to_s) spammable.create_user_agent_detail(user_agent: spam_params.user_agent, ip_address: spam_params.ip_address)
end end
private
attr_reader :spammable, :spam_params
end end

8
config/feature_flags/development/datadog_ci_integration.yml
View File

@ -1,8 +0,0 @@
---
name: datadog_ci_integration
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46564
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284088
type: development
group: group::ecosystem
default_enabled: false
milestone: '13.7'

8
config/routes/project.rb
View File

@ -547,7 +547,13 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
end end
scope :usage_ping, controller: :usage_ping do # TODO: Deprecated. This should be removed in 14.2.
scope :usage_ping, controller: :service_ping do
post :web_ide_clientside_preview # rubocop:todo Cop/PutProjectRoutesUnderScope
post :web_ide_pipelines_count # rubocop:todo Cop/PutProjectRoutesUnderScope
end
scope :service_ping, controller: :service_ping do
post :web_ide_clientside_preview # rubocop:todo Cop/PutProjectRoutesUnderScope post :web_ide_clientside_preview # rubocop:todo Cop/PutProjectRoutesUnderScope
post :web_ide_pipelines_count # rubocop:todo Cop/PutProjectRoutesUnderScope post :web_ide_pipelines_count # rubocop:todo Cop/PutProjectRoutesUnderScope
end end

24
danger/feature_flag/Dangerfile
View File

@ -68,27 +68,3 @@ end
if helper.security_mr? && feature_flag_file_added? if helper.security_mr? && feature_flag_file_added?
fail "Feature flags are discouraged from security merge requests. Read the [security documentation](https://gitlab.com/gitlab-org/release/docs/-/blob/master/general/security/utilities/feature_flags.md) for details." fail "Feature flags are discouraged from security merge requests. Read the [security documentation](https://gitlab.com/gitlab-org/release/docs/-/blob/master/general/security/utilities/feature_flags.md) for details."
end end
if feature_flag_file_added_or_removed?
new_mr_title = helper.mr_title.dup
new_mr_title << ' [RUN ALL RSPEC]' unless helper.run_all_rspec_mr?
new_mr_title << ' [RUN AS-IF-FOSS]' unless helper.run_as_if_foss_mr?
changes = {}
changes[:add_labels] = FEATURE_FLAG_LABEL unless helper.mr_has_labels?(FEATURE_FLAG_LABEL)
if new_mr_title != helper.mr_title
changes[:title] = new_mr_title
else
message "You're adding or removing a feature flag, your MR title needs to include `[RUN ALL RSPEC] [RUN AS-IF-FOSS]` (we may have updated it automatically for you and started a new MR pipeline) to ensure everything is covered."
end
if changes.any?
gitlab.api.update_merge_request(
gitlab.mr_json['project_id'],
gitlab.mr_json['iid'],
**changes
)
gitlab.api.post("/projects/#{gitlab.mr_json['project_id']}/merge_requests/#{gitlab.mr_json['iid']}/pipelines")
end
end

3
danger/specialization_labels/Dangerfile
View File

@ -9,7 +9,8 @@ SPECIALIZATIONS = {
docs: 'documentation', docs: 'documentation',
qa: 'QA', qa: 'QA',
engineering_productivity: 'Engineering Productivity', engineering_productivity: 'Engineering Productivity',
ci_template: 'ci::templates' ci_template: 'ci::templates',
feature_flag: 'feature flag'
}.freeze }.freeze
labels_to_add = project_helper.changes_by_category.each_with_object([]) do |(category, _changes), memo| labels_to_add = project_helper.changes_by_category.each_with_object([]) do |(category, _changes), memo|

10
doc/user/profile/notifications.md
View File

@ -176,14 +176,14 @@ Users are notified of the following events:
| Event | Sent to | Settings level | | Event | Sent to | Settings level |
|------------------------------|---------------------|------------------------------| |------------------------------|---------------------|------------------------------|
| New SSH key added | User | Security email, always sent. | | New SSH key added | User | Security email, always sent. |
| SSH key has expired | User | Security email, always sent. | | SSH key has expired | User | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322637) in GitLab 13.12 |
| New email added | User | Security email, always sent. | | New email added | User | Security email, always sent. |
| Email changed | User | Security email, always sent. | | Email changed | User | Security email, always sent. |
| Password changed | User | Security email, always sent when user changes their own password | | Password changed | User | Security email, always sent when user changes their own password |
| Password changed by administrator | User | Security email, always sent when an administrator changes the password of another user | | Password changed by administrator | User | Security email, always sent when an administrator changes the password of another user |
| Two-factor authentication disabled | User | Security email, always sent. | | Two-factor authentication disabled | User | Security email, always sent. |
| New user created | User | Sent on user creation, except for OmniAuth (LDAP)| | New user created | User | Sent on user creation, except for OmniAuth (LDAP)|
| New SAML/SCIM user provisioned. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276018) in GitLab 13.8 | User | Sent when a user is provisioned through SAML/SCIM | | New SAML/SCIM user provisioned | User | Sent when a user is provisioned through SAML/SCIM. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276018) in GitLab 13.8 |
| User added to project | User | Sent when user is added to project | | User added to project | User | Sent when user is added to project |
| Project access level changed | User | Sent when user project access level is changed | | Project access level changed | User | Sent when user project access level is changed |
| User added to group | User | Sent when user is added to group | | User added to group | User | Sent when user is added to group |
@ -237,10 +237,10 @@ epics:
| Close merge request | | | Close merge request | |
| Due issue | Participants and Custom notification level with this event selected | | Due issue | Participants and Custom notification level with this event selected |
| Failed pipeline | The author of the pipeline | | Failed pipeline | The author of the pipeline |
| Fixed pipeline ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24309) in GitLab 13.1.) | The author of the pipeline. Enabled by default. | | Fixed pipeline | The author of the pipeline. Enabled by default. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/24309) in GitLab 13.1 |
| Merge merge request | | | Merge merge request | |
| Merge when pipeline succeeds ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211961) in GitLab 13.4) | Author, Participants, Watchers, Subscribers, and Custom notification level with this event selected. `Note:` Custom notification level is ignored for Author, Watchers and Subscribers | | Merge when pipeline succeeds | Author, Participants, Watchers, Subscribers, and Custom notification level with this event selected. Custom notification level is ignored for Author, Watchers and Subscribers. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211961) in GitLab 13.4 |
| Merge request [marked as ready](../project/merge_requests/drafts.md) (introduced in [GitLab 13.10](https://gitlab.com/gitlab-org/gitlab/-/issues/15332)) | Watchers and participants | | Merge request [marked as ready](../project/merge_requests/drafts.md) | Watchers and participants. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/15332) in GitLab 13.10 |
| New comment | Participants, Watchers, Subscribers, and Custom notification level with this event selected, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher | | New comment | Participants, Watchers, Subscribers, and Custom notification level with this event selected, plus anyone mentioned by `@username` in the comment, with notification level "Mention" or higher |
| New epic | | | New epic | |
| New issue | | | New issue | |

1
doc/user/project/integrations/overview.md
View File

@ -32,6 +32,7 @@ Click on the service links to see further configuration instructions and details
| Campfire | Connect to chat. | **{dotted-circle}** No | | Campfire | Connect to chat. | **{dotted-circle}** No |
| [Confluence Workspace](../../../api/services.md#confluence-service) | Replace the link to the internal wiki with a link to a Confluence Cloud Workspace. | **{dotted-circle}** No | | [Confluence Workspace](../../../api/services.md#confluence-service) | Replace the link to the internal wiki with a link to a Confluence Cloud Workspace. | **{dotted-circle}** No |
| [Custom issue tracker](custom_issue_tracker.md) | Use a custom issue tracker. | **{dotted-circle}** No | | [Custom issue tracker](custom_issue_tracker.md) | Use a custom issue tracker. | **{dotted-circle}** No |
| Datadog | Trace your GitLab pipelines with Datadog. | **{check-circle}** Yes |
| [Discord Notifications](discord_notifications.md) | Send notifications about project events to a Discord channel. | **{dotted-circle}** No | | [Discord Notifications](discord_notifications.md) | Send notifications about project events to a Discord channel. | **{dotted-circle}** No |
| Drone CI | Run CI/CD pipelines with Drone. | **{check-circle}** Yes | | Drone CI | Run CI/CD pipelines with Drone. | **{check-circle}** Yes |
| [Emails on push](emails_on_push.md) | Send commits and diff of each push by email. | **{dotted-circle}** No | | [Emails on push](emails_on_push.md) | Send commits and diff of each push by email. | **{dotted-circle}** No |

4
lib/api/helpers.rb
View File

@ -577,10 +577,6 @@ module API
Gitlab::AppLogger.warn("Redis tracking event failed for event: #{event_name}, message: #{error.message}") Gitlab::AppLogger.warn("Redis tracking event failed for event: #{event_name}, message: #{error.message}")
end end
def with_api_params(&block)
yield({ api: true, request: request })
end
protected protected
def project_finder_params_visibility_ce def project_finder_params_visibility_ce

18
lib/api/helpers/snippets_helpers.rb
View File

@ -72,22 +72,18 @@ module API
end end
def process_create_params(args) def process_create_params(args)
with_api_params do |api_params| args[:snippet_actions] = args.delete(:files)&.map do |file|
args[:snippet_actions] = args.delete(:files)&.map do |file| file[:action] = :create
file[:action] = :create file.symbolize_keys
file.symbolize_keys
end
args.merge(api_params)
end end
args
end end
def process_update_params(args) def process_update_params(args)
with_api_params do |api_params| args[:snippet_actions] = args.delete(:files)&.map(&:symbolize_keys)
args[:snippet_actions] = args.delete(:files)&.map(&:symbolize_keys)
args.merge(api_params) args
end
end end
def validate_params_for_multiple_files(snippet) def validate_params_for_multiple_files(snippet)

10
lib/api/issues.rb
View File

@ -255,9 +255,11 @@ module API
issue_params = convert_parameters_from_legacy_format(issue_params) issue_params = convert_parameters_from_legacy_format(issue_params)
begin begin
spam_params = ::Spam::SpamParams.new_from_request(request: request)
issue = ::Issues::CreateService.new(project: user_project, issue = ::Issues::CreateService.new(project: user_project,
current_user: current_user, current_user: current_user,
params: issue_params.merge(request: request, api: true)).execute params: issue_params,
spam_params: spam_params).execute
if issue.spam? if issue.spam?
render_api_error!({ error: 'Spam detected' }, 400) render_api_error!({ error: 'Spam detected' }, 400)
@ -294,13 +296,15 @@ module API
issue = user_project.issues.find_by!(iid: params.delete(:issue_iid)) issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
authorize! :update_issue, issue authorize! :update_issue, issue
update_params = declared_params(include_missing: false).merge(request: request, api: true) update_params = declared_params(include_missing: false)
update_params = convert_parameters_from_legacy_format(update_params) update_params = convert_parameters_from_legacy_format(update_params)
spam_params = ::Spam::SpamParams.new_from_request(request: request)
issue = ::Issues::UpdateService.new(project: user_project, issue = ::Issues::UpdateService.new(project: user_project,
current_user: current_user, current_user: current_user,
params: update_params).execute(issue) params: update_params,
spam_params: spam_params).execute(issue)
render_spam_error! if issue.spam? render_spam_error! if issue.spam?

6
lib/api/project_snippets.rb
View File

@ -75,7 +75,8 @@ module API
snippet_params = process_create_params(declared_params(include_missing: false)) snippet_params = process_create_params(declared_params(include_missing: false))
service_response = ::Snippets::CreateService.new(project: user_project, current_user: current_user, params: snippet_params).execute spam_params = ::Spam::SpamParams.new_from_request(request: request)
service_response = ::Snippets::CreateService.new(project: user_project, current_user: current_user, params: snippet_params, spam_params: spam_params).execute
snippet = service_response.payload[:snippet] snippet = service_response.payload[:snippet]
if service_response.success? if service_response.success?
@ -116,7 +117,8 @@ module API
snippet_params = process_update_params(declared_params(include_missing: false)) snippet_params = process_update_params(declared_params(include_missing: false))
service_response = ::Snippets::UpdateService.new(project: user_project, current_user: current_user, params: snippet_params).execute(snippet) spam_params = ::Spam::SpamParams.new_from_request(request: request)
service_response = ::Snippets::UpdateService.new(project: user_project, current_user: current_user, params: snippet_params, spam_params: spam_params).execute(snippet)
snippet = service_response.payload[:snippet] snippet = service_response.payload[:snippet]
if service_response.success? if service_response.success?

6
lib/api/snippets.rb
View File

@ -84,7 +84,8 @@ module API
attrs = process_create_params(declared_params(include_missing: false)) attrs = process_create_params(declared_params(include_missing: false))
service_response = ::Snippets::CreateService.new(project: nil, current_user: current_user, params: attrs).execute spam_params = ::Spam::SpamParams.new_from_request(request: request)
service_response = ::Snippets::CreateService.new(project: nil, current_user: current_user, params: attrs, spam_params: spam_params).execute
snippet = service_response.payload[:snippet] snippet = service_response.payload[:snippet]
if service_response.success? if service_response.success?
@ -126,7 +127,8 @@ module API
attrs = process_update_params(declared_params(include_missing: false)) attrs = process_update_params(declared_params(include_missing: false))
service_response = ::Snippets::UpdateService.new(project: nil, current_user: current_user, params: attrs).execute(snippet) spam_params = ::Spam::SpamParams.new_from_request(request: request)
service_response = ::Snippets::UpdateService.new(project: nil, current_user: current_user, params: attrs, spam_params: spam_params).execute(snippet)
snippet = service_response.payload[:snippet] snippet = service_response.payload[:snippet]

3
lib/gitlab/email/handler/create_issue_handler.rb
View File

@ -61,7 +61,8 @@ module Gitlab
params: { params: {
title: mail.subject, title: mail.subject,
description: message_including_reply description: message_including_reply
} },
spam_params: nil
).execute ).execute
end end

4
lib/gitlab/email/handler/service_desk_handler.rb
View File

@ -83,7 +83,8 @@ module Gitlab
description: message_including_template, description: message_including_template,
confidential: true, confidential: true,
external_author: from_address external_author: from_address
} },
spam_params: nil
).execute ).execute
raise InvalidIssueError unless @issue.persisted? raise InvalidIssueError unless @issue.persisted?
@ -95,6 +96,7 @@ module Gitlab
def send_thank_you_email def send_thank_you_email
Notify.service_desk_thank_you_email(@issue.id).deliver_later Notify.service_desk_thank_you_email(@issue.id).deliver_later
Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email)
end end
def message_including_template def message_including_template

2
lib/gitlab/slash_commands/issue_new.rb
View File

@ -33,7 +33,7 @@ module Gitlab
private private
def create_issue(title:, description:) def create_issue(title:, description:)
Issues::CreateService.new(project: project, current_user: current_user, params: { title: title, description: description }).execute Issues::CreateService.new(project: project, current_user: current_user, params: { title: title, description: description }, spam_params: nil).execute
end end
def presenter(issue) def presenter(issue)

2
lib/quality/seeders/issues.rb
View File

@ -30,7 +30,7 @@ module Quality
labels: labels.join(',') labels: labels.join(',')
} }
params[:closed_at] = params[:created_at] + rand(35).days if params[:state] == 'closed' params[:closed_at] = params[:created_at] + rand(35).days if params[:state] == 'closed'
issue = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params).execute issue = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params, spam_params: nil).execute
if issue.persisted? if issue.persisted?
created_issues_count += 1 created_issues_count += 1

2
spec/controllers/projects/usage_ping_controller_spec.rb → spec/controllers/projects/service_ping_controller_spec.rb
View File

@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Projects::UsagePingController do RSpec.describe Projects::ServicePingController do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }

2
spec/factories/integrations.rb
View File

@ -97,7 +97,7 @@ FactoryBot.define do
issue_tracker issue_tracker
end end
factory :youtrack_service, class: 'Integrations::Youtrack' do factory :youtrack_integration, class: 'Integrations::Youtrack' do
project project
active { true } active { true }
issue_tracker issue_tracker

2
spec/factories/projects.rb
View File

@ -402,7 +402,7 @@ FactoryBot.define do
factory :youtrack_project, parent: :project do factory :youtrack_project, parent: :project do
has_external_issue_tracker { true } has_external_issue_tracker { true }
youtrack_service youtrack_integration
end end
factory :jira_project, parent: :project do factory :jira_project, parent: :project do

4
spec/features/calendar_spec.rb
View File

@ -145,7 +145,7 @@ RSpec.describe 'Contributions Calendar', :js do
describe '1 issue creation calendar activity' do describe '1 issue creation calendar activity' do
before do before do
Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params).execute Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params, spam_params: nil).execute
end end
it_behaves_like 'a day with activity', contribution_count: 1 it_behaves_like 'a day with activity', contribution_count: 1
@ -180,7 +180,7 @@ RSpec.describe 'Contributions Calendar', :js do
push_code_contribution push_code_contribution
travel_to(Date.yesterday) do travel_to(Date.yesterday) do
Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params).execute Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params, spam_params: nil).execute
end end
end end
include_context 'visit user page' include_context 'visit user page'

2
spec/features/unsubscribe_links_spec.rb
View File

@ -9,7 +9,7 @@ RSpec.describe 'Unsubscribe links', :sidekiq_might_not_need_inline do
let(:author) { create(:user) } let(:author) { create(:user) }
let(:project) { create(:project, :public) } let(:project) { create(:project, :public) }
let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } } let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } }
let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params).execute } let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params, spam_params: nil).execute }
let(:mail) { ActionMailer::Base.deliveries.last } let(:mail) { ActionMailer::Base.deliveries.last }
let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) } let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) }

2
spec/features/users/user_browses_projects_on_user_page_spec.rb
View File

@ -125,7 +125,7 @@ RSpec.describe 'Users > User browses projects on user page', :js do
end end
before do before do
Issues::CreateService.new(project: contributed_project, current_user: user, params: { title: 'Bug in old browser' }).execute Issues::CreateService.new(project: contributed_project, current_user: user, params: { title: 'Bug in old browser' }, spam_params: nil).execute
event = create(:push_event, project: contributed_project, author: user) event = create(:push_event, project: contributed_project, author: user)
create(:push_event_payload, event: event, commit_count: 3) create(:push_event_payload, event: event, commit_count: 3)
end end

16
spec/frontend/environments/environments_app_spec.js
View File

@ -1,3 +1,4 @@
import { GlTabs } from '@gitlab/ui';
import { mount, shallowMount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
import MockAdapter from 'axios-mock-adapter'; import MockAdapter from 'axios-mock-adapter';
import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { extendedWrapper } from 'helpers/vue_test_utils_helper';
@ -7,6 +8,7 @@ import EmptyState from '~/environments/components/empty_state.vue';
import EnableReviewAppModal from '~/environments/components/enable_review_app_modal.vue'; import EnableReviewAppModal from '~/environments/components/enable_review_app_modal.vue';
import EnvironmentsApp from '~/environments/components/environments_app.vue'; import EnvironmentsApp from '~/environments/components/environments_app.vue';
import axios from '~/lib/utils/axios_utils'; import axios from '~/lib/utils/axios_utils';
import * as utils from '~/lib/utils/common_utils';
import { environment, folder } from './mock_data'; import { environment, folder } from './mock_data';
describe('Environment', () => { describe('Environment', () => {
@ -264,4 +266,18 @@ describe('Environment', () => {
}); });
}); });
}); });
describe('tabs', () => {
beforeEach(() => {
mockRequest(200, { environments: [] });
jest
.spyOn(utils, 'getParameterByName')
.mockImplementation((param) => (param === 'scope' ? 'stopped' : null));
return createWrapper(true);
});
it('selects the tab for the parameter', () => {
expect(wrapper.findComponent(GlTabs).attributes('value')).toBe('1');
});
});
}); });

19
spec/frontend/issues_list/components/issues_list_app_spec.js
View File

@ -21,7 +21,6 @@ import IssuableList from '~/issuable_list/components/issuable_list_root.vue';
import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants'; import { IssuableListTabs, IssuableStates } from '~/issuable_list/constants';
import IssuesListApp from '~/issues_list/components/issues_list_app.vue'; import IssuesListApp from '~/issues_list/components/issues_list_app.vue';
import { import {
apiSortParams,
CREATED_DESC, CREATED_DESC,
DUE_DATE_OVERDUE, DUE_DATE_OVERDUE,
PARAM_DUE_DATE, PARAM_DUE_DATE,
@ -148,8 +147,8 @@ describe('IssuesListApp component', () => {
hasPreviousPage: getIssuesQueryResponse.data.project.issues.pageInfo.hasPreviousPage, hasPreviousPage: getIssuesQueryResponse.data.project.issues.pageInfo.hasPreviousPage,
hasNextPage: getIssuesQueryResponse.data.project.issues.pageInfo.hasNextPage, hasNextPage: getIssuesQueryResponse.data.project.issues.pageInfo.hasNextPage,
urlParams: { urlParams: {
sort: urlSortParams[CREATED_DESC],
state: IssuableStates.Opened, state: IssuableStates.Opened,
...urlSortParams[CREATED_DESC],
}, },
}); });
}); });
@ -178,7 +177,7 @@ describe('IssuesListApp component', () => {
describe('csv import/export component', () => { describe('csv import/export component', () => {
describe('when user is signed in', () => { describe('when user is signed in', () => {
const search = '?search=refactor&state=opened&sort=created_date'; const search = '?search=refactor&sort=created_date&state=opened';
beforeEach(() => { beforeEach(() => {
global.jsdom.reconfigure({ url: `${TEST_HOST}${search}` }); global.jsdom.reconfigure({ url: `${TEST_HOST}${search}` });
@ -273,13 +272,17 @@ describe('IssuesListApp component', () => {
describe('sort', () => { describe('sort', () => {
it.each(Object.keys(urlSortParams))('is set as %s from the url params', (sortKey) => { it.each(Object.keys(urlSortParams))('is set as %s from the url params', (sortKey) => {
global.jsdom.reconfigure({ url: setUrlParams(urlSortParams[sortKey], TEST_HOST) }); global.jsdom.reconfigure({
url: setUrlParams({ sort: urlSortParams[sortKey] }, TEST_HOST),
});
wrapper = mountComponent(); wrapper = mountComponent();
expect(findIssuableList().props()).toMatchObject({ expect(findIssuableList().props()).toMatchObject({
initialSortBy: sortKey, initialSortBy: sortKey,
urlParams: urlSortParams[sortKey], urlParams: {
sort: urlSortParams[sortKey],
},
}); });
}); });
}); });
@ -640,7 +643,7 @@ describe('IssuesListApp component', () => {
}); });
describe('when "sort" event is emitted by IssuableList', () => { describe('when "sort" event is emitted by IssuableList', () => {
it.each(Object.keys(apiSortParams))( it.each(Object.keys(urlSortParams))(
'updates to the new sort when payload is `%s`', 'updates to the new sort when payload is `%s`',
async (sortKey) => { async (sortKey) => {
wrapper = mountComponent(); wrapper = mountComponent();
@ -650,7 +653,9 @@ describe('IssuesListApp component', () => {
jest.runOnlyPendingTimers(); jest.runOnlyPendingTimers();
await nextTick(); await nextTick();
expect(findIssuableList().props('urlParams')).toMatchObject(urlSortParams[sortKey]); expect(findIssuableList().props('urlParams')).toMatchObject({
sort: urlSortParams[sortKey],
});
}, },
); );
}); });

66
spec/frontend/issues_list/mock_data.js
View File

@ -9,7 +9,7 @@ export const getIssuesQueryResponse = {
issues: { issues: {
count: 1, count: 1,
pageInfo: { pageInfo: {
hasNextPage: false, hasNextPage: true,
hasPreviousPage: false, hasPreviousPage: false,
startCursor: 'startcursor', startCursor: 'startcursor',
endCursor: 'endcursor', endCursor: 'endcursor',
@ -86,10 +86,10 @@ export const locationSearch = [
'not[label_name][]=drama', 'not[label_name][]=drama',
'my_reaction_emoji=thumbsup', 'my_reaction_emoji=thumbsup',
'confidential=no', 'confidential=no',
'iteration_title=season:+%234', 'iteration_id=4',
'not[iteration_title]=season:+%2320', 'not[iteration_id]=20',
'epic_id=gitlab-org%3A%3A%2612', 'epic_id=12',
'not[epic_id]=gitlab-org%3A%3A%2634', 'not[epic_id]=34',
'weight=1', 'weight=1',
'not[weight]=3', 'not[weight]=3',
].join('&'); ].join('&');
@ -118,10 +118,10 @@ export const filteredTokens = [
{ type: 'labels', value: { data: 'drama', operator: OPERATOR_IS_NOT } }, { type: 'labels', value: { data: 'drama', operator: OPERATOR_IS_NOT } },
{ type: 'my_reaction_emoji', value: { data: 'thumbsup', operator: OPERATOR_IS } }, { type: 'my_reaction_emoji', value: { data: 'thumbsup', operator: OPERATOR_IS } },
{ type: 'confidential', value: { data: 'no', operator: OPERATOR_IS } }, { type: 'confidential', value: { data: 'no', operator: OPERATOR_IS } },
{ type: 'iteration', value: { data: 'season: #4', operator: OPERATOR_IS } }, { type: 'iteration', value: { data: '4', operator: OPERATOR_IS } },
{ type: 'iteration', value: { data: 'season: #20', operator: OPERATOR_IS_NOT } }, { type: 'iteration', value: { data: '20', operator: OPERATOR_IS_NOT } },
{ type: 'epic_id', value: { data: 'gitlab-org::&12', operator: OPERATOR_IS } }, { type: 'epic_id', value: { data: '12', operator: OPERATOR_IS } },
{ type: 'epic_id', value: { data: 'gitlab-org::&34', operator: OPERATOR_IS_NOT } }, { type: 'epic_id', value: { data: '34', operator: OPERATOR_IS_NOT } },
{ type: 'weight', value: { data: '1', operator: OPERATOR_IS } }, { type: 'weight', value: { data: '1', operator: OPERATOR_IS } },
{ type: 'weight', value: { data: '3', operator: OPERATOR_IS_NOT } }, { type: 'weight', value: { data: '3', operator: OPERATOR_IS_NOT } },
{ type: 'filtered-search-term', value: { data: 'find' } }, { type: 'filtered-search-term', value: { data: 'find' } },
@ -138,30 +138,32 @@ export const filteredTokensWithSpecialValues = [
]; ];
export const apiParams = { export const apiParams = {
author_username: 'homer', authorUsername: 'homer',
'not[author_username]': 'marge', assigneeUsernames: ['bart', 'lisa'],
assignee_username: ['bart', 'lisa'], milestoneTitle: 'season 4',
'not[assignee_username]': ['patty', 'selma'], labelName: ['cartoon', 'tv'],
milestone: 'season 4', myReactionEmoji: 'thumbsup',
'not[milestone]': 'season 20',
labels: ['cartoon', 'tv'],
'not[labels]': ['live action', 'drama'],
my_reaction_emoji: 'thumbsup',
confidential: 'no', confidential: 'no',
iteration_title: 'season: #4', iterationId: '4',
'not[iteration_title]': 'season: #20', epicId: '12',
epic_id: '12',
'not[epic_id]': 'gitlab-org::&34',
weight: '1', weight: '1',
'not[weight]': '3', not: {
authorUsername: 'marge',
assigneeUsernames: ['patty', 'selma'],
milestoneTitle: 'season 20',
labelName: ['live action', 'drama'],
iterationId: '20',
epicId: '34',
weight: '3',
},
}; };
export const apiParamsWithSpecialValues = { export const apiParamsWithSpecialValues = {
assignee_id: '123', assigneeId: '123',
assignee_username: 'bart', assigneeUsernames: 'bart',
my_reaction_emoji: 'None', myReactionEmoji: 'None',
iteration_id: 'Current', iterationWildcardId: 'CURRENT',
epic_id: 'None', epicId: 'None',
weight: 'None', weight: 'None',
}; };
@ -176,10 +178,10 @@ export const urlParams = {
'not[label_name][]': ['live action', 'drama'], 'not[label_name][]': ['live action', 'drama'],
my_reaction_emoji: 'thumbsup', my_reaction_emoji: 'thumbsup',
confidential: 'no', confidential: 'no',
iteration_title: 'season: #4', iteration_id: '4',
'not[iteration_title]': 'season: #20', 'not[iteration_id]': '20',
epic_id: 'gitlab-org%3A%3A%2612', epic_id: '12',
'not[epic_id]': 'gitlab-org::&34', 'not[epic_id]': '34',
weight: '1', weight: '1',
'not[weight]': '3', 'not[weight]': '3',
}; };

29
spec/frontend/issues_list/utils_spec.js
View File

@ -8,10 +8,11 @@ import {
urlParams, urlParams,
urlParamsWithSpecialValues, urlParamsWithSpecialValues,
} from 'jest/issues_list/mock_data'; } from 'jest/issues_list/mock_data';
import { API_PARAM, DUE_DATE_VALUES, URL_PARAM, urlSortParams } from '~/issues_list/constants'; import { DUE_DATE_VALUES, urlSortParams } from '~/issues_list/constants';
import { import {
convertToParams, convertToApiParams,
convertToSearchQuery, convertToSearchQuery,
convertToUrlParams,
getDueDateValue, getDueDateValue,
getFilterTokens, getFilterTokens,
getSortKey, getSortKey,
@ -20,7 +21,7 @@ import {
describe('getSortKey', () => { describe('getSortKey', () => {
it.each(Object.keys(urlSortParams))('returns %s given the correct inputs', (sortKey) => { it.each(Object.keys(urlSortParams))('returns %s given the correct inputs', (sortKey) => {
const { sort } = urlSortParams[sortKey]; const sort = urlSortParams[sortKey];
expect(getSortKey(sort)).toBe(sortKey); expect(getSortKey(sort)).toBe(sortKey);
}); });
}); });
@ -80,31 +81,23 @@ describe('getFilterTokens', () => {
}); });
}); });
describe('convertToParams', () => { describe('convertToApiParams', () => {
it('returns api params given filtered tokens', () => { it('returns api params given filtered tokens', () => {
expect(convertToParams(filteredTokens, API_PARAM)).toEqual({ expect(convertToApiParams(filteredTokens)).toEqual(apiParams);
...apiParams,
epic_id: 'gitlab-org::&12',
});
}); });
it('returns api params given filtered tokens with special values', () => { it('returns api params given filtered tokens with special values', () => {
expect(convertToParams(filteredTokensWithSpecialValues, API_PARAM)).toEqual( expect(convertToApiParams(filteredTokensWithSpecialValues)).toEqual(apiParamsWithSpecialValues);
apiParamsWithSpecialValues,
);
}); });
});
describe('convertToUrlParams', () => {
it('returns url params given filtered tokens', () => { it('returns url params given filtered tokens', () => {
expect(convertToParams(filteredTokens, URL_PARAM)).toEqual({ expect(convertToUrlParams(filteredTokens)).toEqual(urlParams);
...urlParams,
epic_id: 'gitlab-org::&12',
});
}); });
it('returns url params given filtered tokens with special values', () => { it('returns url params given filtered tokens with special values', () => {
expect(convertToParams(filteredTokensWithSpecialValues, URL_PARAM)).toEqual( expect(convertToUrlParams(filteredTokensWithSpecialValues)).toEqual(urlParamsWithSpecialValues);
urlParamsWithSpecialValues,
);
}); });
}); });

10
spec/frontend/vue_shared/components/filtered_search_bar/tokens/iteration_token_spec.js
View File

@ -7,7 +7,7 @@ import { mockIterationToken } from '../mock_data';
jest.mock('~/flash'); jest.mock('~/flash');
describe('IterationToken', () => { describe('IterationToken', () => {
const title = 'gitlab-org: #1'; const id = 123;
let wrapper; let wrapper;
const createComponent = ({ config = mockIterationToken, value = { data: '' } } = {}) => const createComponent = ({ config = mockIterationToken, value = { data: '' } } = {}) =>
@ -28,14 +28,14 @@ describe('IterationToken', () => {
}); });
it('renders iteration value', async () => { it('renders iteration value', async () => {
wrapper = createComponent({ value: { data: title } }); wrapper = createComponent({ value: { data: id } });
await wrapper.vm.$nextTick(); await wrapper.vm.$nextTick();
const tokenSegments = wrapper.findAllComponents(GlFilteredSearchTokenSegment); const tokenSegments = wrapper.findAllComponents(GlFilteredSearchTokenSegment);
expect(tokenSegments).toHaveLength(3); // `Iteration` `=` `gitlab-org: #1` expect(tokenSegments).toHaveLength(3); // `Iteration` `=` `gitlab-org: #1`
expect(tokenSegments.at(2).text()).toBe(title); expect(tokenSegments.at(2).text()).toBe(id.toString());
}); });
it('fetches initial values', () => { it('fetches initial values', () => {
@ -43,10 +43,10 @@ describe('IterationToken', () => {
wrapper = createComponent({ wrapper = createComponent({
config: { ...mockIterationToken, fetchIterations: fetchIterationsSpy }, config: { ...mockIterationToken, fetchIterations: fetchIterationsSpy },
value: { data: title }, value: { data: id },
}); });
expect(fetchIterationsSpy).toHaveBeenCalledWith(title); expect(fetchIterationsSpy).toHaveBeenCalledWith(id);
}); });
it('fetches iterations on user input', () => { it('fetches iterations on user input', () => {

44
spec/frontend/vue_shared/components/user_select_spec.js
View File

@ -275,48 +275,4 @@ describe('User select dropdown', () => {
expect(findEmptySearchResults().exists()).toBe(true); expect(findEmptySearchResults().exists()).toBe(true);
}); });
}); });
// TODO Remove this test after the following issue is resolved in the backend
// https://gitlab.com/gitlab-org/gitlab/-/issues/329750
describe('temporary error suppression', () => {
beforeEach(() => {
jest.spyOn(console, 'error').mockImplementation();
});
const nullError = { message: 'Cannot return null for non-nullable field GroupMember.user' };
it.each`
mockErrors
${[nullError]}
${[nullError, nullError]}
`('does not emit errors', async ({ mockErrors }) => {
createComponent({
searchQueryHandler: jest.fn().mockResolvedValue({
errors: mockErrors,
}),
});
await waitForSearch();
expect(wrapper.emitted()).toEqual({});
// eslint-disable-next-line no-console
expect(console.error).toHaveBeenCalled();
});
it.each`
mockErrors
${[{ message: 'serious error' }]}
${[nullError, { message: 'serious error' }]}
`('emits error when non-null related errors are included', async ({ mockErrors }) => {
createComponent({
searchQueryHandler: jest.fn().mockResolvedValue({
errors: mockErrors,
}),
});
await waitForSearch();
expect(wrapper.emitted('error')).toEqual([[]]);
// eslint-disable-next-line no-console
expect(console.error).not.toHaveBeenCalled();
});
});
}); });

1
spec/graphql/mutations/issues/create_spec.rb
View File

@ -50,6 +50,7 @@ RSpec.describe Mutations::Issues::Create do
stub_licensed_features(multiple_issue_assignees: false, issue_weights: false) stub_licensed_features(multiple_issue_assignees: false, issue_weights: false)
project.add_guest(assignee1) project.add_guest(assignee1)
project.add_guest(assignee2) project.add_guest(assignee2)
stub_spam_services
end end
subject { mutation.resolve(**mutation_params) } subject { mutation.resolve(**mutation_params) }

4
spec/graphql/mutations/issues/set_confidential_spec.rb
View File

@ -17,6 +17,10 @@ RSpec.describe Mutations::Issues::SetConfidential do
subject { mutation.resolve(project_path: project.full_path, iid: issue.iid, confidential: confidential) } subject { mutation.resolve(project_path: project.full_path, iid: issue.iid, confidential: confidential) }
before do
stub_spam_services
end
it_behaves_like 'permission level for issue mutation is correctly verified' it_behaves_like 'permission level for issue mutation is correctly verified'
context 'when the user can update the issue' do context 'when the user can update the issue' do

4
spec/graphql/mutations/issues/update_spec.rb
View File

@ -35,6 +35,10 @@ RSpec.describe Mutations::Issues::Update do
subject { mutation.resolve(**mutation_params) } subject { mutation.resolve(**mutation_params) }
before do
stub_spam_services
end
it_behaves_like 'permission level for issue mutation is correctly verified' it_behaves_like 'permission level for issue mutation is correctly verified'
context 'when the user can update the issue' do context 'when the user can update the issue' do

4
spec/lib/banzai/filter/references/external_issue_reference_filter_spec.rb
View File

@ -140,7 +140,9 @@ RSpec.describe Banzai::Filter::References::ExternalIssueReferenceFilter do
end end
context "youtrack project" do context "youtrack project" do
let_it_be(:service) { create(:youtrack_service, project: project) } before_all do
create(:youtrack_integration, project: project)
end
before do before do
project.update!(issues_enabled: false) project.update!(issues_enabled: false)

9
spec/lib/gitlab/email/handler/service_desk_handler_spec.rb
View File

@ -50,6 +50,15 @@ RSpec.describe Gitlab::Email::Handler::ServiceDeskHandler do
it 'sends thank you email' do it 'sends thank you email' do
expect { receiver.execute }.to have_enqueued_job.on_queue('mailers') expect { receiver.execute }.to have_enqueued_job.on_queue('mailers')
end end
it 'adds metric events for incoming and reply emails' do
metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
expect(metric_transaction).to receive(:add_event).with(:receive_email_service_desk, anything)
expect(metric_transaction).to receive(:add_event).with(:service_desk_thank_you_email)
receiver.execute
end
end end
context 'when everything is fine' do context 'when everything is fine' do

6
spec/lib/gitlab/import_export/all_models.yml
View File

@ -381,14 +381,14 @@ project:
- microsoft_teams_integration - microsoft_teams_integration
- mattermost_integration - mattermost_integration
- hangouts_chat_integration - hangouts_chat_integration
- unify_circuit_service - unify_circuit_integration
- buildkite_integration - buildkite_integration
- bamboo_integration - bamboo_integration
- teamcity_integration - teamcity_integration
- pushover_integration - pushover_integration
- jira_integration - jira_integration
- redmine_integration - redmine_integration
- youtrack_service - youtrack_integration
- custom_issue_tracker_integration - custom_issue_tracker_integration
- bugzilla_integration - bugzilla_integration
- ewm_integration - ewm_integration
@ -557,7 +557,7 @@ project:
- alert_management_alerts - alert_management_alerts
- repository_storage_moves - repository_storage_moves
- freeze_periods - freeze_periods
- webex_teams_service - webex_teams_integration
- build_report_results - build_report_results
- vulnerability_statistic - vulnerability_statistic
- vulnerability_historical_statistics - vulnerability_historical_statistics

12
spec/mailers/emails/service_desk_spec.rb
View File

@ -115,16 +115,6 @@ RSpec.describe Emails::ServiceDesk do
end end
end end
shared_examples 'notification with metric event' do |event_type|
it 'adds metric event' do
metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
expect(metric_transaction).to receive(:add_event).with(event_type)
subject.content_type
end
end
describe '.service_desk_thank_you_email' do describe '.service_desk_thank_you_email' do
let_it_be(:reply_in_subject) { true } let_it_be(:reply_in_subject) { true }
let_it_be(:default_text) do let_it_be(:default_text) do
@ -134,7 +124,6 @@ RSpec.describe Emails::ServiceDesk do
subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) } subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) }
it_behaves_like 'read template from repository', 'thank_you' it_behaves_like 'read template from repository', 'thank_you'
it_behaves_like 'notification with metric event', :service_desk_thank_you_email
context 'handling template markdown' do context 'handling template markdown' do
context 'with a simple text' do context 'with a simple text' do
@ -175,7 +164,6 @@ RSpec.describe Emails::ServiceDesk do
subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) } subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) }
it_behaves_like 'read template from repository', 'new_note' it_behaves_like 'read template from repository', 'new_note'
it_behaves_like 'notification with metric event', :service_desk_new_note_email
context 'handling template markdown' do context 'handling template markdown' do
context 'with a simple text' do context 'with a simple text' do

6
spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb
View File

@ -26,11 +26,11 @@ RSpec.describe MigrateIssueTrackersData do
services.create!(type: 'BugzillaService', properties: properties, category: 'issue_tracker') services.create!(type: 'BugzillaService', properties: properties, category: 'issue_tracker')
end end
let!(:youtrack_service) do let!(:youtrack_integration) do
services.create!(type: 'YoutrackService', properties: properties, category: 'issue_tracker') services.create!(type: 'YoutrackService', properties: properties, category: 'issue_tracker')
end end
let!(:youtrack_service_empty) do let!(:youtrack_integration_empty) do
services.create!(type: 'YoutrackService', properties: '', category: 'issue_tracker') services.create!(type: 'YoutrackService', properties: '', category: 'issue_tracker')
end end
@ -56,7 +56,7 @@ RSpec.describe MigrateIssueTrackersData do
migrate! migrate!
expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_integration.id, bugzilla_integration.id) expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_integration.id, bugzilla_integration.id)
expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_service.id, gitlab_service.id) expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_integration.id, gitlab_service.id)
expect(BackgroundMigrationWorker.jobs.size).to eq(2) expect(BackgroundMigrationWorker.jobs.size).to eq(2)
end end
end end

6
spec/migrations/20200130145430_reschedule_migrate_issue_trackers_data_spec.rb
View File

@ -26,11 +26,11 @@ RSpec.describe RescheduleMigrateIssueTrackersData do
services.create!(id: 12, type: 'BugzillaService', properties: properties, category: 'issue_tracker') services.create!(id: 12, type: 'BugzillaService', properties: properties, category: 'issue_tracker')
end end
let!(:youtrack_service) do let!(:youtrack_integration) do
services.create!(id: 13, type: 'YoutrackService', properties: properties, category: 'issue_tracker') services.create!(id: 13, type: 'YoutrackService', properties: properties, category: 'issue_tracker')
end end
let!(:youtrack_service_empty) do let!(:youtrack_integration_empty) do
services.create!(id: 14, type: 'YoutrackService', properties: '', category: 'issue_tracker') services.create!(id: 14, type: 'YoutrackService', properties: '', category: 'issue_tracker')
end end
@ -57,7 +57,7 @@ RSpec.describe RescheduleMigrateIssueTrackersData do
migrate! migrate!
expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_integration.id, bugzilla_integration.id) expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_integration.id, bugzilla_integration.id)
expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_service.id, gitlab_service.id) expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_integration.id, gitlab_service.id)
expect(BackgroundMigrationWorker.jobs.size).to eq(2) expect(BackgroundMigrationWorker.jobs.size).to eq(2)
end end
end end

2
spec/models/integrations/microsoft_teams_spec.rb
View File

@ -74,7 +74,7 @@ RSpec.describe Integrations::MicrosoftTeams do
context 'with issue events' do context 'with issue events' do
let(:opts) { { title: 'Awesome issue', description: 'please fix' } } let(:opts) { { title: 'Awesome issue', description: 'please fix' } }
let(:issues_sample_data) do let(:issues_sample_data) do
service = Issues::CreateService.new(project: project, current_user: user, params: opts) service = Issues::CreateService.new(project: project, current_user: user, params: opts, spam_params: nil)
issue = service.execute issue = service.execute
service.hook_data(issue, 'open') service.hook_data(issue, 'open')
end end

26
spec/models/project_spec.rb
View File

@ -39,8 +39,8 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to have_one(:microsoft_teams_integration) } it { is_expected.to have_one(:microsoft_teams_integration) }
it { is_expected.to have_one(:mattermost_integration) } it { is_expected.to have_one(:mattermost_integration) }
it { is_expected.to have_one(:hangouts_chat_integration) } it { is_expected.to have_one(:hangouts_chat_integration) }
it { is_expected.to have_one(:unify_circuit_service) } it { is_expected.to have_one(:unify_circuit_integration) }
it { is_expected.to have_one(:webex_teams_service) } it { is_expected.to have_one(:webex_teams_integration) }
it { is_expected.to have_one(:packagist_integration) } it { is_expected.to have_one(:packagist_integration) }
it { is_expected.to have_one(:pushover_integration) } it { is_expected.to have_one(:pushover_integration) }
it { is_expected.to have_one(:asana_integration) } it { is_expected.to have_one(:asana_integration) }
@ -62,7 +62,7 @@ RSpec.describe Project, factory_default: :keep do
it { is_expected.to have_one(:teamcity_integration) } it { is_expected.to have_one(:teamcity_integration) }
it { is_expected.to have_one(:jira_integration) } it { is_expected.to have_one(:jira_integration) }
it { is_expected.to have_one(:redmine_integration) } it { is_expected.to have_one(:redmine_integration) }
it { is_expected.to have_one(:youtrack_service) } it { is_expected.to have_one(:youtrack_integration) }
it { is_expected.to have_one(:custom_issue_tracker_integration) } it { is_expected.to have_one(:custom_issue_tracker_integration) }
it { is_expected.to have_one(:bugzilla_integration) } it { is_expected.to have_one(:bugzilla_integration) }
it { is_expected.to have_one(:ewm_integration) } it { is_expected.to have_one(:ewm_integration) }
@ -5870,26 +5870,6 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
describe '#disabled_services' do
subject { build(:project).disabled_services }
context 'without datadog_ci_integration' do
before do
stub_feature_flags(datadog_ci_integration: false)
end
it { is_expected.to include('datadog') }
end
context 'with datadog_ci_integration' do
before do
stub_feature_flags(datadog_ci_integration: true)
end
it { is_expected.not_to include('datadog') }
end
end
describe '#find_or_initialize_service' do describe '#find_or_initialize_service' do
it 'avoids N+1 database queries' do it 'avoids N+1 database queries' do
allow(Integration).to receive(:available_services_names).and_return(%w[prometheus pushover]) allow(Integration).to receive(:available_services_names).and_return(%w[prometheus pushover])

29
spec/requests/api/graphql/mutations/snippets/create_spec.rb
View File

@ -17,7 +17,6 @@ RSpec.describe 'Creating a Snippet' do
let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] } let(:actions) { [{ action: action }.merge(file_1), { action: action }.merge(file_2)] }
let(:project_path) { nil } let(:project_path) { nil }
let(:uploaded_files) { nil } let(:uploaded_files) { nil }
let(:spam_mutation_vars) { {} }
let(:mutation_vars) do let(:mutation_vars) do
{ {
description: description, description: description,
@ -26,7 +25,7 @@ RSpec.describe 'Creating a Snippet' do
project_path: project_path, project_path: project_path,
uploaded_files: uploaded_files, uploaded_files: uploaded_files,
blob_actions: actions blob_actions: actions
}.merge(spam_mutation_vars) }
end end
let(:mutation) do let(:mutation) do
@ -77,21 +76,6 @@ RSpec.describe 'Creating a Snippet' do
expect(mutation_response['snippet']).to be_nil expect(mutation_response['snippet']).to be_nil
end end
context 'when snippet_spam flag is disabled' do
before do
stub_feature_flags(snippet_spam: false)
end
it 'passes disable_spam_action_service param to service' do
expect(::Snippets::CreateService)
.to receive(:new)
.with(project: anything, current_user: anything, params: hash_including(disable_spam_action_service: true))
.and_call_original
subject
end
end
end end
shared_examples 'creates snippet' do shared_examples 'creates snippet' do
@ -121,15 +105,6 @@ RSpec.describe 'Creating a Snippet' do
it_behaves_like 'snippet edit usage data counters' it_behaves_like 'snippet edit usage data counters'
it_behaves_like 'a mutation which can mutate a spammable' do it_behaves_like 'a mutation which can mutate a spammable' do
let(:captcha_response) { 'abc123' }
let(:spam_log_id) { 1234 }
let(:spam_mutation_vars) do
{
captcha_response: captcha_response,
spam_log_id: spam_log_id
}
end
let(:service) { Snippets::CreateService } let(:service) { Snippets::CreateService }
end end
end end
@ -190,7 +165,7 @@ RSpec.describe 'Creating a Snippet' do
it do it do
expect(::Snippets::CreateService).to receive(:new) expect(::Snippets::CreateService).to receive(:new)
.with(project: nil, current_user: user, params: hash_including(files: expected_value)) .with(project: nil, current_user: user, params: hash_including(files: expected_value), spam_params: instance_of(::Spam::SpamParams))
.and_return(double(execute: creation_response)) .and_return(double(execute: creation_response))
subject subject

27
spec/requests/api/graphql/mutations/snippets/update_spec.rb
View File

@ -16,7 +16,6 @@ RSpec.describe 'Updating a Snippet' do
let(:updated_file) { 'CHANGELOG' } let(:updated_file) { 'CHANGELOG' }
let(:deleted_file) { 'README' } let(:deleted_file) { 'README' }
let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s } let(:snippet_gid) { GitlabSchema.id_from_object(snippet).to_s }
let(:spam_mutation_vars) { {} }
let(:mutation_vars) do let(:mutation_vars) do
{ {
id: snippet_gid, id: snippet_gid,
@ -27,7 +26,7 @@ RSpec.describe 'Updating a Snippet' do
{ action: :update, filePath: updated_file, content: updated_content }, { action: :update, filePath: updated_file, content: updated_content },
{ action: :delete, filePath: deleted_file } { action: :delete, filePath: deleted_file }
] ]
}.merge(spam_mutation_vars) }
end end
let(:mutation) do let(:mutation) do
@ -82,21 +81,6 @@ RSpec.describe 'Updating a Snippet' do
end end
end end
context 'when snippet_spam flag is disabled' do
before do
stub_feature_flags(snippet_spam: false)
end
it 'passes disable_spam_action_service param to service' do
expect(::Snippets::UpdateService)
.to receive(:new)
.with(project: anything, current_user: anything, params: hash_including(disable_spam_action_service: true))
.and_call_original
subject
end
end
context 'when there are ActiveRecord validation errors' do context 'when there are ActiveRecord validation errors' do
let(:updated_title) { '' } let(:updated_title) { '' }
@ -125,15 +109,6 @@ RSpec.describe 'Updating a Snippet' do
end end
it_behaves_like 'a mutation which can mutate a spammable' do it_behaves_like 'a mutation which can mutate a spammable' do
let(:captcha_response) { 'abc123' }
let(:spam_log_id) { 1234 }
let(:spam_mutation_vars) do
{
captcha_response: captcha_response,
spam_log_id: spam_log_id
}
end
let(:service) { Snippets::UpdateService } let(:service) { Snippets::UpdateService }
end end

20
spec/routing/project_routing_spec.rb
View File

@ -770,13 +770,23 @@ RSpec.describe 'project routing' do
end end
end end
describe Projects::UsagePingController, 'routing' do describe Projects::ServicePingController, 'routing' do
it 'routes to usage_ping#web_ide_clientside_preview' do describe 'deprecated routing' do
expect(post('/gitlab/gitlabhq/usage_ping/web_ide_clientside_preview')).to route_to('projects/usage_ping#web_ide_clientside_preview', namespace_id: 'gitlab', project_id: 'gitlabhq') it 'routes to service_ping#web_ide_clientside_preview' do
expect(post('/gitlab/gitlabhq/usage_ping/web_ide_clientside_preview')).to route_to('projects/service_ping#web_ide_clientside_preview', namespace_id: 'gitlab', project_id: 'gitlabhq')
end
it 'routes to service_ping#web_ide_pipelines_count' do
expect(post('/gitlab/gitlabhq/usage_ping/web_ide_pipelines_count')).to route_to('projects/service_ping#web_ide_pipelines_count', namespace_id: 'gitlab', project_id: 'gitlabhq')
end
end end
it 'routes to usage_ping#web_ide_pipelines_count' do it 'routes to service_ping#web_ide_clientside_preview' do
expect(post('/gitlab/gitlabhq/usage_ping/web_ide_pipelines_count')).to route_to('projects/usage_ping#web_ide_pipelines_count', namespace_id: 'gitlab', project_id: 'gitlabhq') expect(post('/gitlab/gitlabhq/service_ping/web_ide_clientside_preview')).to route_to('projects/service_ping#web_ide_clientside_preview', namespace_id: 'gitlab', project_id: 'gitlabhq')
end
it 'routes to service_ping#web_ide_pipelines_count' do
expect(post('/gitlab/gitlabhq/service_ping/web_ide_pipelines_count')).to route_to('projects/service_ping#web_ide_pipelines_count', namespace_id: 'gitlab', project_id: 'gitlabhq')
end end
end end

28
spec/services/captcha/captcha_verification_service_spec.rb
View File

@ -4,21 +4,31 @@ require 'spec_helper'
RSpec.describe Captcha::CaptchaVerificationService do RSpec.describe Captcha::CaptchaVerificationService do
describe '#execute' do describe '#execute' do
let(:captcha_response) { nil } let(:captcha_response) { 'abc123' }
let(:request) { double(:request) } let(:fake_ip) { '1.2.3.4' }
let(:service) { described_class.new } let(:spam_params) do
::Spam::SpamParams.new(
captcha_response: captcha_response,
spam_log_id: double,
ip_address: fake_ip,
user_agent: double,
referer: double
)
end
subject { service.execute(captcha_response: captcha_response, request: request) } let(:service) { described_class.new(spam_params: spam_params) }
subject { service.execute }
context 'when there is no captcha_response' do context 'when there is no captcha_response' do
let(:captcha_response) { nil }
it 'returns false' do it 'returns false' do
expect(subject).to eq(false) expect(subject).to eq(false)
end end
end end
context 'when there is a captcha_response' do context 'when there is a captcha_response' do
let(:captcha_response) { 'abc123' }
before do before do
expect(Gitlab::Recaptcha).to receive(:load_configurations!) expect(Gitlab::Recaptcha).to receive(:load_configurations!)
end end
@ -29,10 +39,12 @@ RSpec.describe Captcha::CaptchaVerificationService do
expect(subject).to eq(true) expect(subject).to eq(true)
end end
it 'has a request method which returns the request' do it 'has a request method which returns an object with the ip address #remote_ip' do
subject subject
expect(service.send(:request)).to eq(request) request_struct = service.send(:request)
expect(request_struct).to respond_to(:remote_ip)
expect(request_struct.remote_ip).to eq(fake_ip)
end end
end end
end end

82
spec/services/issues/create_service_spec.rb
View File

@ -8,11 +8,17 @@ RSpec.describe Issues::CreateService do
let_it_be_with_reload(:project) { create(:project) } let_it_be_with_reload(:project) { create(:project) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:spam_params) { double }
describe '#execute' do describe '#execute' do
let_it_be(:assignee) { create(:user) } let_it_be(:assignee) { create(:user) }
let_it_be(:milestone) { create(:milestone, project: project) } let_it_be(:milestone) { create(:milestone, project: project) }
let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute } let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute }
before do
stub_spam_services
end
context 'when params are valid' do context 'when params are valid' do
let_it_be(:labels) { create_pair(:label, project: project) } let_it_be(:labels) { create_pair(:label, project: project) }
@ -44,7 +50,7 @@ RSpec.describe Issues::CreateService do
end end
context 'when skip_system_notes is true' do context 'when skip_system_notes is true' do
let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute(skip_system_notes: true) } let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute(skip_system_notes: true) }
it 'does not call Issuable::CommonSystemNotesService' do it 'does not call Issuable::CommonSystemNotesService' do
expect(Issuable::CommonSystemNotesService).not_to receive(:new) expect(Issuable::CommonSystemNotesService).not_to receive(:new)
@ -92,7 +98,7 @@ RSpec.describe Issues::CreateService do
let_it_be(:non_member) { create(:user) } let_it_be(:non_member) { create(:user) }
it 'filters out params that cannot be set without the :set_issue_metadata permission' do it 'filters out params that cannot be set without the :set_issue_metadata permission' do
issue = described_class.new(project: project, current_user: non_member, params: opts).execute issue = described_class.new(project: project, current_user: non_member, params: opts, spam_params: spam_params).execute
expect(issue).to be_persisted expect(issue).to be_persisted
expect(issue.title).to eq('Awesome issue') expect(issue.title).to eq('Awesome issue')
@ -104,7 +110,7 @@ RSpec.describe Issues::CreateService do
end end
it 'can create confidential issues' do it 'can create confidential issues' do
issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }).execute issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }, spam_params: spam_params).execute
expect(issue.confidential).to be_truthy expect(issue.confidential).to be_truthy
end end
@ -113,7 +119,7 @@ RSpec.describe Issues::CreateService do
it 'moves the issue to the end, in an asynchronous worker' do it 'moves the issue to the end, in an asynchronous worker' do
expect(IssuePlacementWorker).to receive(:perform_async).with(be_nil, Integer) expect(IssuePlacementWorker).to receive(:perform_async).with(be_nil, Integer)
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
end end
context 'when label belongs to project group' do context 'when label belongs to project group' do
@ -200,7 +206,7 @@ RSpec.describe Issues::CreateService do
it 'invalidates open issues counter for assignees when issue is assigned' do it 'invalidates open issues counter for assignees when issue is assigned' do
project.add_maintainer(assignee) project.add_maintainer(assignee)
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(assignee.assigned_open_issues_count).to eq 1 expect(assignee.assigned_open_issues_count).to eq 1
end end
@ -226,7 +232,7 @@ RSpec.describe Issues::CreateService do
expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks)
expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks)
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
end end
it 'executes confidential issue hooks when issue is confidential' do it 'executes confidential issue hooks when issue is confidential' do
@ -235,7 +241,7 @@ RSpec.describe Issues::CreateService do
expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks)
expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks)
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
end end
context 'after_save callback to store_mentions' do context 'after_save callback to store_mentions' do
@ -279,7 +285,7 @@ RSpec.describe Issues::CreateService do
it 'removes assignee when user id is invalid' do it 'removes assignee when user id is invalid' do
opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } opts = { title: 'Title', description: 'Description', assignee_ids: [-1] }
issue = described_class.new(project: project, current_user: user, params: opts).execute issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(issue.assignees).to be_empty expect(issue.assignees).to be_empty
end end
@ -287,7 +293,7 @@ RSpec.describe Issues::CreateService do
it 'removes assignee when user id is 0' do it 'removes assignee when user id is 0' do
opts = { title: 'Title', description: 'Description', assignee_ids: [0] } opts = { title: 'Title', description: 'Description', assignee_ids: [0] }
issue = described_class.new(project: project, current_user: user, params: opts).execute issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(issue.assignees).to be_empty expect(issue.assignees).to be_empty
end end
@ -296,7 +302,7 @@ RSpec.describe Issues::CreateService do
project.add_maintainer(assignee) project.add_maintainer(assignee)
opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] }
issue = described_class.new(project: project, current_user: user, params: opts).execute issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(issue.assignees).to eq([assignee]) expect(issue.assignees).to eq([assignee])
end end
@ -314,7 +320,7 @@ RSpec.describe Issues::CreateService do
project.update!(visibility_level: level) project.update!(visibility_level: level)
opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] }
issue = described_class.new(project: project, current_user: user, params: opts).execute issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(issue.assignees).to be_empty expect(issue.assignees).to be_empty
end end
@ -324,7 +330,7 @@ RSpec.describe Issues::CreateService do
end end
it_behaves_like 'issuable record that supports quick actions' do it_behaves_like 'issuable record that supports quick actions' do
let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute } let(:issuable) { described_class.new(project: project, current_user: user, params: params, spam_params: spam_params).execute }
end end
context 'Quick actions' do context 'Quick actions' do
@ -364,14 +370,14 @@ RSpec.describe Issues::CreateService do
let(:opts) { { discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid } } let(:opts) { { discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid } }
it 'resolves the discussion' do it 'resolves the discussion' do
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
discussion.first_note.reload discussion.first_note.reload
expect(discussion.resolved?).to be(true) expect(discussion.resolved?).to be(true)
end end
it 'added a system note to the discussion' do it 'added a system note to the discussion' do
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first
@ -379,7 +385,7 @@ RSpec.describe Issues::CreateService do
end end
it 'assigns the title and description for the issue' do it 'assigns the title and description for the issue' do
issue = described_class.new(project: project, current_user: user, params: opts).execute issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(issue.title).not_to be_nil expect(issue.title).not_to be_nil
expect(issue.description).not_to be_nil expect(issue.description).not_to be_nil
@ -391,7 +397,8 @@ RSpec.describe Issues::CreateService do
merge_request_to_resolve_discussions_of: merge_request, merge_request_to_resolve_discussions_of: merge_request,
description: nil, description: nil,
title: nil title: nil
}).execute },
spam_params: spam_params).execute
expect(issue.description).to be_nil expect(issue.description).to be_nil
expect(issue.title).to be_nil expect(issue.title).to be_nil
@ -402,14 +409,14 @@ RSpec.describe Issues::CreateService do
let(:opts) { { merge_request_to_resolve_discussions_of: merge_request.iid } } let(:opts) { { merge_request_to_resolve_discussions_of: merge_request.iid } }
it 'resolves the discussion' do it 'resolves the discussion' do
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
discussion.first_note.reload discussion.first_note.reload
expect(discussion.resolved?).to be(true) expect(discussion.resolved?).to be(true)
end end
it 'added a system note to the discussion' do it 'added a system note to the discussion' do
described_class.new(project: project, current_user: user, params: opts).execute described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first
@ -417,7 +424,7 @@ RSpec.describe Issues::CreateService do
end end
it 'assigns the title and description for the issue' do it 'assigns the title and description for the issue' do
issue = described_class.new(project: project, current_user: user, params: opts).execute issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute
expect(issue.title).not_to be_nil expect(issue.title).not_to be_nil
expect(issue.description).not_to be_nil expect(issue.description).not_to be_nil
@ -429,7 +436,8 @@ RSpec.describe Issues::CreateService do
merge_request_to_resolve_discussions_of: merge_request, merge_request_to_resolve_discussions_of: merge_request,
description: nil, description: nil,
title: nil title: nil
}).execute },
spam_params: spam_params).execute
expect(issue.description).to be_nil expect(issue.description).to be_nil
expect(issue.title).to be_nil expect(issue.title).to be_nil
@ -438,47 +446,27 @@ RSpec.describe Issues::CreateService do
end end
context 'checking spam' do context 'checking spam' do
let(:request) { double(:request, headers: nil) }
let(:api) { true }
let(:captcha_response) { 'abc123' }
let(:spam_log_id) { 1 }
let(:params) do let(:params) do
{ {
title: 'Spam issue', title: 'Spam issue'
request: request,
api: api,
captcha_response: captcha_response,
spam_log_id: spam_log_id
} }
end end
subject do subject do
described_class.new(project: project, current_user: user, params: params) described_class.new(project: project, current_user: user, params: params, spam_params: spam_params)
end
before do
allow_next_instance_of(UserAgentDetailService) do |instance|
allow(instance).to receive(:create)
end
end end
it 'executes SpamActionService' do it 'executes SpamActionService' do
spam_params = Spam::SpamParams.new(
api: api,
captcha_response: captcha_response,
spam_log_id: spam_log_id
)
expect_next_instance_of( expect_next_instance_of(
Spam::SpamActionService, Spam::SpamActionService,
{ {
spammable: an_instance_of(Issue), spammable: kind_of(Issue),
request: request, spam_params: spam_params,
user: user, user: an_instance_of(User),
action: :create action: :create
} }
) do |instance| ) do |instance|
expect(instance).to receive(:execute).with(spam_params: spam_params) expect(instance).to receive(:execute)
end end
subject.execute subject.execute

61
spec/services/notification_service_spec.rb
View File

@ -376,42 +376,32 @@ RSpec.describe NotificationService, :mailer do
let(:subject) { NotificationService.new } let(:subject) { NotificationService.new }
let(:mailer) { double(deliver_later: true) } let(:mailer) { double(deliver_later: true) }
def should_email!
expect(Notify).to receive(:service_desk_new_note_email)
.with(issue.id, note.id, issue.external_author)
end
def should_not_email!
expect(Notify).not_to receive(:service_desk_new_note_email)
end
def execute!
subject.new_note(note)
end
def self.it_should_email!
it 'sends the email' do
should_email!
execute!
end
end
def self.it_should_not_email!
it 'doesn\'t send the email' do
should_not_email!
execute!
end
end
let(:issue) { create(:issue, author: User.support_bot) } let(:issue) { create(:issue, author: User.support_bot) }
let(:project) { issue.project } let(:project) { issue.project }
let(:note) { create(:note, noteable: issue, project: project) } let(:note) { create(:note, noteable: issue, project: project) }
context 'do not exist' do shared_examples 'notification with exact metric events' do |number_of_events|
it_should_not_email! it 'adds metric event' do
metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true)
allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
expect(metric_transaction).to receive(:add_event).with(:service_desk_new_note_email).exactly(number_of_events).times
subject.new_note(note)
end
end end
shared_examples 'no participants are notified' do
it 'does not send the email' do
expect(Notify).not_to receive(:service_desk_new_note_email)
subject.new_note(note)
end
it_behaves_like 'notification with exact metric events', 0
end
it_behaves_like 'no participants are notified'
context 'do exist and note not confidential' do context 'do exist and note not confidential' do
let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') } let!(:issue_email_participant) { issue.issue_email_participants.create!(email: 'service.desk@example.com') }
@ -420,7 +410,14 @@ RSpec.describe NotificationService, :mailer do
project.update!(service_desk_enabled: true) project.update!(service_desk_enabled: true)
end end
it_should_email! it 'sends the email' do
expect(Notify).to receive(:service_desk_new_note_email)
.with(issue.id, note.id, issue.external_author)
subject.new_note(note)
end
it_behaves_like 'notification with exact metric events', 1
end end
context 'do exist and note is confidential' do context 'do exist and note is confidential' do
@ -432,7 +429,7 @@ RSpec.describe NotificationService, :mailer do
project.update!(service_desk_enabled: true) project.update!(service_desk_enabled: true)
end end
it_should_not_email! it_behaves_like 'no participants are notified'
end end
end end

7
spec/services/snippets/create_service_spec.rb
View File

@ -19,8 +19,9 @@ RSpec.describe Snippets::CreateService do
let(:extra_opts) { {} } let(:extra_opts) { {} }
let(:creator) { admin } let(:creator) { admin }
let(:spam_params) { double }
subject { described_class.new(project: project, current_user: creator, params: opts).execute } subject { described_class.new(project: project, current_user: creator, params: opts, spam_params: spam_params).execute }
let(:snippet) { subject.payload[:snippet] } let(:snippet) { subject.payload[:snippet] }
@ -301,6 +302,10 @@ RSpec.describe Snippets::CreateService do
end end
end end
before do
stub_spam_services
end
context 'when ProjectSnippet' do context 'when ProjectSnippet' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }

8
spec/services/snippets/update_service_spec.rb
View File

@ -20,7 +20,9 @@ RSpec.describe Snippets::UpdateService do
let(:extra_opts) { {} } let(:extra_opts) { {} }
let(:options) { base_opts.merge(extra_opts) } let(:options) { base_opts.merge(extra_opts) }
let(:updater) { user } let(:updater) { user }
let(:service) { Snippets::UpdateService.new(project: project, current_user: updater, params: options) } let(:spam_params) { double }
let(:service) { Snippets::UpdateService.new(project: project, current_user: updater, params: options, spam_params: spam_params) }
subject { service.execute(snippet) } subject { service.execute(snippet) }
@ -721,6 +723,10 @@ RSpec.describe Snippets::UpdateService do
end end
end end
before do
stub_spam_services
end
context 'when Project Snippet' do context 'when Project Snippet' do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) } let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) }

2
spec/services/spam/akismet_service_spec.rb
View File

@ -59,7 +59,7 @@ RSpec.describe Spam::AkismetService do
it_behaves_like 'no activity if Akismet is not enabled', :spam?, :check it_behaves_like 'no activity if Akismet is not enabled', :spam?, :check
context 'if Akismet is enabled' do context 'if Akismet is enabled' do
it 'correctly transforms options for the akismet client' do it 'correctly transforms options for the akismet client, including spelling of referrer key' do
expected_check_params = { expected_check_params = {
type: 'comment', type: 'comment',
text: text, text: text,

139
spec/services/spam/spam_action_service_spec.rb
View File

@ -5,15 +5,20 @@ require 'spec_helper'
RSpec.describe Spam::SpamActionService do RSpec.describe Spam::SpamActionService do
include_context 'includes Spam constants' include_context 'includes Spam constants'
let(:request) { double(:request, env: env, headers: {}) }
let(:issue) { create(:issue, project: project, author: user) } let(:issue) { create(:issue, project: project, author: user) }
let(:fake_ip) { '1.2.3.4' } let(:fake_ip) { '1.2.3.4' }
let(:fake_user_agent) { 'fake-user-agent' } let(:fake_user_agent) { 'fake-user-agent' }
let(:fake_referer) { 'fake-http-referer' } let(:fake_referer) { 'fake-http-referer' }
let(:env) do let(:captcha_response) { 'abc123' }
{ 'action_dispatch.remote_ip' => fake_ip, let(:spam_log_id) { existing_spam_log.id }
'HTTP_USER_AGENT' => fake_user_agent, let(:spam_params) do
'HTTP_REFERER' => fake_referer } ::Spam::SpamParams.new(
captcha_response: captcha_response,
spam_log_id: spam_log_id,
ip_address: fake_ip,
user_agent: fake_user_agent,
referer: fake_referer
)
end end
let_it_be(:project) { create(:project, :public) } let_it_be(:project) { create(:project, :public) }
@ -23,32 +28,33 @@ RSpec.describe Spam::SpamActionService do
issue.spam = false issue.spam = false
end end
shared_examples 'only checks for spam if a request is provided' do describe 'constructor argument validation' do
context 'when request is missing' do subject do
let(:request) { nil } described_service = described_class.new(spammable: issue, spam_params: spam_params, user: user, action: :create)
described_service.execute
it "doesn't check as spam" do
expect(fake_verdict_service).not_to receive(:execute)
response = subject
expect(response.message).to match(/request was not present/)
expect(issue).not_to be_spam
end
end end
context 'when request exists' do context 'when spam_params is nil' do
it 'creates a spam log' do let(:spam_params) { nil }
expect { subject } let(:expected_service_params_not_present_message) do
.to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue') /Skipped spam check because spam_params was not present/
end
it "returns success with a messaage" do
response = subject
expect(response.message).to match(expected_service_params_not_present_message)
expect(issue).not_to be_spam
end end
end end
end end
shared_examples 'creates a spam log' do shared_examples 'creates a spam log' do
it do it do
expect { subject }.to change(SpamLog, :count).by(1) expect { subject }
.to log_spam(title: issue.title, description: issue.description, noteable_type: 'Issue')
# TODO: These checks should be incorporated into the `log_spam` RSpec matcher above
new_spam_log = SpamLog.last new_spam_log = SpamLog.last
expect(new_spam_log.user_id).to eq(user.id) expect(new_spam_log.user_id).to eq(user.id)
expect(new_spam_log.title).to eq(issue.title) expect(new_spam_log.title).to eq(issue.title)
@ -56,25 +62,14 @@ RSpec.describe Spam::SpamActionService do
expect(new_spam_log.source_ip).to eq(fake_ip) expect(new_spam_log.source_ip).to eq(fake_ip)
expect(new_spam_log.user_agent).to eq(fake_user_agent) expect(new_spam_log.user_agent).to eq(fake_user_agent)
expect(new_spam_log.noteable_type).to eq('Issue') expect(new_spam_log.noteable_type).to eq('Issue')
expect(new_spam_log.via_api).to eq(false) expect(new_spam_log.via_api).to eq(true)
end end
end end
describe '#execute' do describe '#execute' do
let(:request) { double(:request, env: env, headers: nil) }
let(:fake_captcha_verification_service) { double(:captcha_verification_service) } let(:fake_captcha_verification_service) { double(:captcha_verification_service) }
let(:fake_verdict_service) { double(:spam_verdict_service) } let(:fake_verdict_service) { double(:spam_verdict_service) }
let(:allowlisted) { false } let(:allowlisted) { false }
let(:api) { nil }
let(:captcha_response) { 'abc123' }
let(:spam_log_id) { existing_spam_log.id }
let(:spam_params) do
::Spam::SpamParams.new(
api: api,
captcha_response: captcha_response,
spam_log_id: spam_log_id
)
end
let(:verdict_service_opts) do let(:verdict_service_opts) do
{ {
@ -88,7 +83,6 @@ RSpec.describe Spam::SpamActionService do
{ {
target: issue, target: issue,
user: user, user: user,
request: request,
options: verdict_service_opts, options: verdict_service_opts,
context: { context: {
action: :create, action: :create,
@ -100,40 +94,20 @@ RSpec.describe Spam::SpamActionService do
let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) } let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) }
subject do subject do
described_service = described_class.new(spammable: issue, request: request, user: user, action: :create) described_service = described_class.new(spammable: issue, spam_params: spam_params, user: user, action: :create)
allow(described_service).to receive(:allowlisted?).and_return(allowlisted) allow(described_service).to receive(:allowlisted?).and_return(allowlisted)
described_service.execute(spam_params: spam_params) described_service.execute
end end
before do before do
allow(Captcha::CaptchaVerificationService).to receive(:new) { fake_captcha_verification_service } allow(Captcha::CaptchaVerificationService).to receive(:new).with(spam_params: spam_params) { fake_captcha_verification_service }
allow(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args).and_return(fake_verdict_service) allow(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args).and_return(fake_verdict_service)
end end
context 'when the captcha params are passed in the headers' do
let(:request) { double(:request, env: env, headers: headers) }
let(:spam_params) { Spam::SpamActionService.filter_spam_params!({ api: api }, request) }
let(:headers) do
{
'X-GitLab-Captcha-Response' => captcha_response,
'X-GitLab-Spam-Log-Id' => spam_log_id
}
end
it 'extracts the headers correctly' do
expect(fake_captcha_verification_service)
.to receive(:execute).with(captcha_response: captcha_response, request: request).and_return(true)
expect(SpamLog)
.to receive(:verify_recaptcha!).with(user_id: user.id, id: spam_log_id)
subject
end
end
context 'when captcha response verification returns true' do context 'when captcha response verification returns true' do
before do before do
allow(fake_captcha_verification_service) allow(fake_captcha_verification_service)
.to receive(:execute).with(captcha_response: captcha_response, request: request).and_return(true) .to receive(:execute).and_return(true)
end end
it "doesn't check with the SpamVerdictService" do it "doesn't check with the SpamVerdictService" do
@ -156,7 +130,7 @@ RSpec.describe Spam::SpamActionService do
context 'when captcha response verification returns false' do context 'when captcha response verification returns false' do
before do before do
allow(fake_captcha_verification_service) allow(fake_captcha_verification_service)
.to receive(:execute).with(captcha_response: captcha_response, request: request).and_return(false) .to receive(:execute).and_return(false)
end end
context 'when spammable attributes have not changed' do context 'when spammable attributes have not changed' do
@ -200,8 +174,6 @@ RSpec.describe Spam::SpamActionService do
stub_feature_flags(allow_possible_spam: false) stub_feature_flags(allow_possible_spam: false)
end end
it_behaves_like 'only checks for spam if a request is provided'
it 'marks as spam' do it 'marks as spam' do
response = subject response = subject
@ -211,8 +183,6 @@ RSpec.describe Spam::SpamActionService do
end end
context 'when allow_possible_spam feature flag is true' do context 'when allow_possible_spam feature flag is true' do
it_behaves_like 'only checks for spam if a request is provided'
it 'does not mark as spam' do it 'does not mark as spam' do
response = subject response = subject
@ -232,8 +202,6 @@ RSpec.describe Spam::SpamActionService do
stub_feature_flags(allow_possible_spam: false) stub_feature_flags(allow_possible_spam: false)
end end
it_behaves_like 'only checks for spam if a request is provided'
it 'marks as spam' do it 'marks as spam' do
response = subject response = subject
@ -243,8 +211,6 @@ RSpec.describe Spam::SpamActionService do
end end
context 'when allow_possible_spam feature flag is true' do context 'when allow_possible_spam feature flag is true' do
it_behaves_like 'only checks for spam if a request is provided'
it 'does not mark as spam' do it 'does not mark as spam' do
response = subject response = subject
@ -264,8 +230,6 @@ RSpec.describe Spam::SpamActionService do
stub_feature_flags(allow_possible_spam: false) stub_feature_flags(allow_possible_spam: false)
end end
it_behaves_like 'only checks for spam if a request is provided'
it_behaves_like 'creates a spam log' it_behaves_like 'creates a spam log'
it 'does not mark as spam' do it 'does not mark as spam' do
@ -284,8 +248,6 @@ RSpec.describe Spam::SpamActionService do
end end
context 'when allow_possible_spam feature flag is true' do context 'when allow_possible_spam feature flag is true' do
it_behaves_like 'only checks for spam if a request is provided'
it_behaves_like 'creates a spam log' it_behaves_like 'creates a spam log'
it 'does not mark as needing reCAPTCHA' do it 'does not mark as needing reCAPTCHA' do
@ -334,37 +296,10 @@ RSpec.describe Spam::SpamActionService do
allow(fake_verdict_service).to receive(:execute).and_return(ALLOW) allow(fake_verdict_service).to receive(:execute).and_return(ALLOW)
end end
context 'when the request is nil' do it 'assembles the options with information from the request' do
let(:request) { nil } expect(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args)
let(:issue_ip_address) { '1.2.3.4' }
let(:issue_user_agent) { 'lynx' }
let(:verdict_service_opts) do
{
ip_address: issue_ip_address,
user_agent: issue_user_agent
}
end
before do subject
allow(issue).to receive(:ip_address) { issue_ip_address }
allow(issue).to receive(:user_agent) { issue_user_agent }
end
it 'assembles the options with information from the spammable' do
# TODO: This code untestable, because we do not perform a verification if there is not a
# request. See corresponding comment in code
# expect(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args)
subject
end
end
context 'when the request is present' do
it 'assembles the options with information from the request' do
expect(Spam::SpamVerdictService).to receive(:new).with(verdict_service_args)
subject
end
end end
end end
end end

40
spec/services/spam/spam_params_spec.rb Normal file
View File

@ -0,0 +1,40 @@
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Spam::SpamParams do
describe '.new_from_request' do
let(:captcha_response) { 'abc123' }
let(:spam_log_id) { 42 }
let(:ip_address) { '0.0.0.0' }
let(:user_agent) { 'Lynx' }
let(:referer) { 'http://localhost' }
let(:headers) do
{
'X-GitLab-Captcha-Response' => captcha_response,
'X-GitLab-Spam-Log-Id' => spam_log_id
}
end
let(:env) do
{
'action_dispatch.remote_ip' => ip_address,
'HTTP_USER_AGENT' => user_agent,
'HTTP_REFERER' => referer
}
end
let(:request) {double(:request, headers: headers, env: env)}
it 'constructs from a request' do
expected = ::Spam::SpamParams.new(
captcha_response: captcha_response,
spam_log_id: spam_log_id,
ip_address: ip_address,
user_agent: user_agent,
referer: referer
)
expect(described_class.new_from_request(request: request)).to eq(expected)
end
end
end

4
spec/services/spam/spam_verdict_service_spec.rb
View File

@ -14,13 +14,11 @@ RSpec.describe Spam::SpamVerdictService do
'HTTP_REFERER' => fake_referer } 'HTTP_REFERER' => fake_referer }
end end
let(:request) { double(:request, env: env) }
let(:check_for_spam) { true } let(:check_for_spam) { true }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:issue) { create(:issue, author: user) } let_it_be(:issue) { create(:issue, author: user) }
let(:service) do let(:service) do
described_class.new(user: user, target: issue, request: request, options: {}) described_class.new(user: user, target: issue, options: {})
end end
let(:attribs) do let(:attribs) do

1
spec/spec_helper.rb
View File

@ -190,6 +190,7 @@ RSpec.configure do |config|
config.include RailsHelpers config.include RailsHelpers
config.include SidekiqMiddleware config.include SidekiqMiddleware
config.include StubActionCableConnection, type: :channel config.include StubActionCableConnection, type: :channel
config.include StubSpamServices
include StubFeatureFlags include StubFeatureFlags

23
spec/support/helpers/stub_spam_services.rb Normal file
View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
module StubSpamServices
def stub_spam_services
allow(::Spam::SpamParams).to receive(:new_from_request) do
::Spam::SpamParams.new(
captcha_response: double(:captcha_response),
spam_log_id: double(:spam_log_id),
ip_address: double(:ip_address),
user_agent: double(:user_agent),
referer: double(:referer)
)
end
allow_next_instance_of(::Spam::SpamActionService) do |service|
allow(service).to receive(:execute)
end
allow_next_instance_of(::UserAgentDetailService) do |service|
allow(service).to receive(:create)
end
end
end

12
spec/support/shared_examples/graphql/mutations/can_mutate_spammable_examples.rb
View File

@ -3,17 +3,13 @@
require 'spec_helper' require 'spec_helper'
RSpec.shared_examples 'a mutation which can mutate a spammable' do RSpec.shared_examples 'a mutation which can mutate a spammable' do
describe "#additional_spam_params" do describe "#spam_params" do
it 'passes additional spam params to the service' do it 'passes spam params to the service constructor' do
args = [ args = [
project: anything, project: anything,
current_user: anything, current_user: anything,
params: hash_including( params: anything,
api: true, spam_params: instance_of(::Spam::SpamParams)
request: instance_of(ActionDispatch::Request),
captcha_response: captcha_response,
spam_log_id: spam_log_id
)
] ]
expect(service).to receive(:new).with(*args).and_call_original expect(service).to receive(:new).with(*args).and_call_original

2
spec/support/shared_examples/graphql/spam_protection_shared_examples.rb
View File

@ -57,7 +57,7 @@ RSpec.shared_examples 'has spam protection' do
context 'and no CAPTCHA is required' do context 'and no CAPTCHA is required' do
let(:render_captcha) { false } let(:render_captcha) { false }
it 'does not return a to-level error' do it 'does not return a top-level error' do
send_request send_request
expect(graphql_errors).to be_blank expect(graphql_errors).to be_blank

2
spec/support/shared_examples/models/chat_integration_shared_examples.rb
View File

@ -163,7 +163,7 @@ RSpec.shared_examples "chat integration" do |integration_name|
context "with issue events" do context "with issue events" do
let(:opts) { { title: "Awesome issue", description: "please fix" } } let(:opts) { { title: "Awesome issue", description: "please fix" } }
let(:sample_data) do let(:sample_data) do
service = Issues::CreateService.new(project: project, current_user: user, params: opts) service = Issues::CreateService.new(project: project, current_user: user, params: opts, spam_params: nil)
issue = service.execute issue = service.execute
service.hook_data(issue, "open") service.hook_data(issue, "open")
end end

68
spec/support/shared_examples/services/snippets_shared_examples.rb
View File

@ -1,23 +1,6 @@
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'checking spam' do RSpec.shared_examples 'checking spam' do
let(:request) { double(:request, headers: headers) }
let(:headers) { nil }
let(:api) { true }
let(:captcha_response) { 'abc123' }
let(:spam_log_id) { 1 }
let(:disable_spam_action_service) { false }
let(:extra_opts) do
{
request: request,
api: api,
captcha_response: captcha_response,
spam_log_id: spam_log_id,
disable_spam_action_service: disable_spam_action_service
}
end
before do before do
allow_next_instance_of(UserAgentDetailService) do |instance| allow_next_instance_of(UserAgentDetailService) do |instance|
allow(instance).to receive(:create) allow(instance).to receive(:create)
@ -25,67 +8,26 @@ RSpec.shared_examples 'checking spam' do
end end
it 'executes SpamActionService' do it 'executes SpamActionService' do
spam_params = Spam::SpamParams.new(
api: api,
captcha_response: captcha_response,
spam_log_id: spam_log_id
)
expect_next_instance_of( expect_next_instance_of(
Spam::SpamActionService, Spam::SpamActionService,
{ {
spammable: kind_of(Snippet), spammable: kind_of(Snippet),
request: request, spam_params: spam_params,
user: an_instance_of(User), user: an_instance_of(User),
action: action action: action
} }
) do |instance| ) do |instance|
expect(instance).to receive(:execute).with(spam_params: spam_params) expect(instance).to receive(:execute)
end end
subject subject
end end
context 'when CAPTCHA arguments are passed in the headers' do context 'when snippet_spam flag is disabled' do
let(:headers) do before do
{ stub_feature_flags(snippet_spam: false)
'X-GitLab-Spam-Log-Id' => spam_log_id,
'X-GitLab-Captcha-Response' => captcha_response
}
end end
let(:extra_opts) do
{
request: request,
api: api,
disable_spam_action_service: disable_spam_action_service
}
end
it 'executes the SpamActionService correctly' do
spam_params = Spam::SpamParams.new(
api: api,
captcha_response: captcha_response,
spam_log_id: spam_log_id
)
expect_next_instance_of(
Spam::SpamActionService,
{
spammable: kind_of(Snippet),
request: request,
user: an_instance_of(User),
action: action
}
) do |instance|
expect(instance).to receive(:execute).with(spam_params: spam_params)
end
subject
end
end
context 'when spam action service is disabled' do
let(:disable_spam_action_service) { true }
it 'request parameter is not passed to the service' do it 'request parameter is not passed to the service' do
expect(Spam::SpamActionService).not_to receive(:new) expect(Spam::SpamActionService).not_to receive(:new)