diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-28 15:09:17 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-28 15:09:17 +0000 |
| commit | 1bb7f81e238569fd0fe2b0c4385f1015407a2d59 (patch) | |
| tree | 768c7d44fa3ed641a7e26fdf9db61422902e8294 | |
| parent | eb3a23aaaa99ef8ae08c7b440fad676e3c71a1af (diff) | |
| download | gitlab-ce-1bb7f81e238569fd0fe2b0c4385f1015407a2d59.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
123 files changed, 2285 insertions, 1353 deletions
diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md index b9fed3745d1..1266ee7efe7 100644 --- a/.gitlab/issue_templates/Bug.md +++ b/.gitlab/issue_templates/Bug.md @@ -39,7 +39,10 @@ will also determine whether the bug is fixed in a more recent version. --> ### Output of checks -<!-- If you are reporting a bug on GitLab.com, write: This bug happens on GitLab.com --> +<!-- If you are reporting a bug on GitLab.com, uncomment below --> + +<!-- This bug happens on GitLab.com --> +<!-- /label ~"reproduced on GitLab.com" --> #### Results of GitLab environment info diff --git a/.gitlab/issue_templates/Feature Flag Roll Out.md b/.gitlab/issue_templates/Feature Flag Roll Out.md index f3830a5cc10..4b628126fc9 100644 --- a/.gitlab/issue_templates/Feature Flag Roll Out.md +++ b/.gitlab/issue_templates/Feature Flag Roll Out.md @@ -168,3 +168,4 @@ You can either [create a follow-up issue for Feature Flag Cleanup](https://gitla ``` /label ~"feature flag" ~"type::feature" ~"feature::addition" +/label ~group:: diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index 833fbb8789e..23eb470503e 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -1,6 +1,6 @@ import $ from 'jquery'; import { merge } from 'lodash'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { __ } from '~/locale'; import FilesCommentButton from './files_comment_button'; @@ -82,7 +82,7 @@ export default class Diff { .get(link, { params }) .then(({ data }) => $target.parent().replaceWith(data)) .catch(() => - createFlash({ + createAlert({ message: __('An error occurred while loading diff'), }), ); diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index f5c0776ca35..380aa137a7c 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -11,7 +11,7 @@ import { MR_COMMITS_NEXT_COMMIT, MR_COMMITS_PREVIOUS_COMMIT, } from '~/behaviors/shortcuts/keybindings'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { isSingleViewStyle } from '~/helpers/diffs_helper'; import { helpPagePath } from '~/helpers/help_page_helper'; import { parseBoolean } from '~/lib/utils/common_utils'; @@ -480,7 +480,7 @@ export default { this.updateChangesTabCount(); }) .catch(() => { - createFlash({ + createAlert({ message: __('Something went wrong on our end. Please try again!'), }); }); @@ -495,7 +495,7 @@ export default { this.setDiscussions(); }) .catch(() => { - createFlash({ + createAlert({ message: __('Something went wrong on our end. Please try again!'), }); }); diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index c5f263eb5f1..b2098b9e82d 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -1,7 +1,7 @@ <script> import { GlTooltipDirective, GlSafeHtmlDirective, GlIcon, GlLoadingIcon } from '@gitlab/ui'; import { mapActions } from 'vuex'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { s__, sprintf } from '~/locale'; import { UNFOLD_COUNT, INLINE_DIFF_LINES_KEY } from '../constants'; import * as utils from '../store/utils'; @@ -92,7 +92,7 @@ export default { ) { this.loadMoreLines({ endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers }) .catch(() => { - createFlash({ + createAlert({ message: s__('Diffs|Something went wrong while fetching diff lines.'), }); }) diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index aec608007d5..422bf52a1fa 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -10,7 +10,7 @@ import { escape } from 'lodash'; import { mapActions, mapGetters, mapState } from 'vuex'; import { IdState } from 'vendor/vue-virtual-scroller'; import DiffContent from 'jh_else_ce/diffs/components/diff_content.vue'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { hasDiff } from '~/helpers/diffs_helper'; import { diffViewerErrors } from '~/ide/constants'; import { scrollToElement } from '~/lib/utils/common_utils'; @@ -309,7 +309,7 @@ export default { }) .catch(() => { idState.isLoadingCollapsedDiff = false; - createFlash({ + createAlert({ message: this.$options.i18n.genericError, }); }); diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 5e74a7206b3..7b523c95c74 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -5,7 +5,7 @@ import { historyPushState, scrollToElement, } from '~/lib/utils/common_utils'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { diffViewerModes } from '~/ide/constants'; import axios from '~/lib/utils/axios_utils'; @@ -246,7 +246,7 @@ export const fetchCoverageFiles = ({ commit, state }) => { } }, errorCallback: () => - createFlash({ + createAlert({ message: __('Something went wrong on our end. Please try again!'), }), }); @@ -509,7 +509,7 @@ export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => { .then(() => dispatch('updateResolvableDiscussionsCounts', null, { root: true })) .then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.file_hash)) .catch(() => - createFlash({ + createAlert({ message: s__('MergeRequests|Saving the comment failed'), }), ); @@ -619,7 +619,7 @@ export const cacheTreeListWidth = (_, size) => { export const receiveFullDiffError = ({ commit }, filePath) => { commit(types.RECEIVE_FULL_DIFF_ERROR, filePath); - createFlash({ + createAlert({ message: s__('MergeRequest|Error loading full diff. Please try again.'), }); }; @@ -757,7 +757,7 @@ export const setSuggestPopoverDismissed = ({ commit, state }) => commit(types.SET_SHOW_SUGGEST_POPOVER); }) .catch(() => { - createFlash({ + createAlert({ message: s__('MergeRequest|Error dismissing suggestion popover. Please try again.'), }); }); diff --git a/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js b/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js index bc3cb163c39..999e91eed19 100644 --- a/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js +++ b/app/assets/javascripts/editor/extensions/source_editor_markdown_livepreview_ext.js @@ -1,7 +1,7 @@ import { KeyMod, KeyCode } from 'monaco-editor'; import { debounce } from 'lodash'; import { BLOB_PREVIEW_ERROR } from '~/blob_edit/constants'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { sanitize } from '~/lib/dompurify'; import axios from '~/lib/utils/axios_utils'; import syntaxHighlight from '~/syntax_highlight'; @@ -152,7 +152,7 @@ export class EditorMarkdownPreviewExtension { syntaxHighlight(previewEl.querySelectorAll('.js-syntax-highlight')); previewEl.style.display = 'block'; }) - .catch(() => createFlash(BLOB_PREVIEW_ERROR)); + .catch(() => createAlert(BLOB_PREVIEW_ERROR)); } setupPreviewAction(instance) { diff --git a/app/assets/javascripts/environments/components/delete_environment_modal.vue b/app/assets/javascripts/environments/components/delete_environment_modal.vue index 3173c2bd644..78e1b8d5cb2 100644 --- a/app/assets/javascripts/environments/components/delete_environment_modal.vue +++ b/app/assets/javascripts/environments/components/delete_environment_modal.vue @@ -1,6 +1,6 @@ <script> import { GlTooltipDirective, GlModal } from '@gitlab/ui'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { __, s__, sprintf } from '~/locale'; import eventHub from '../event_hub'; import deleteEnvironmentMutation from '../graphql/mutations/delete_environment.mutation.graphql'; @@ -65,11 +65,11 @@ export default { .then(({ data }) => { const [message] = data?.deleteEvironment?.errors ?? []; if (message) { - createFlash({ message }); + createAlert({ message }); } }) .catch((error) => - createFlash({ + createAlert({ message: s__( 'Environments|An error occurred while deleting the environment. Check if the environment stopped; if not, stop it and try again.', ), diff --git a/app/assets/javascripts/environments/components/deployment.vue b/app/assets/javascripts/environments/components/deployment.vue index 3475b38c8c9..b00a0777a03 100644 --- a/app/assets/javascripts/environments/components/deployment.vue +++ b/app/assets/javascripts/environments/components/deployment.vue @@ -10,7 +10,7 @@ import { import { __, s__ } from '~/locale'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import deploymentDetails from '../graphql/queries/deployment_details.query.graphql'; import DeploymentStatusBadge from './deployment_status_badge.vue'; import Commit from './commit.vue'; @@ -119,7 +119,7 @@ export default { return data?.project?.deployment?.tags; }, error(error) { - createFlash({ + createAlert({ message: this.$options.i18n.LOAD_ERROR_MESSAGE, captureError: true, error, diff --git a/app/assets/javascripts/environments/components/edit_environment.vue b/app/assets/javascripts/environments/components/edit_environment.vue index 96742a11ebb..901d0f5b34d 100644 --- a/app/assets/javascripts/environments/components/edit_environment.vue +++ b/app/assets/javascripts/environments/components/edit_environment.vue @@ -1,5 +1,5 @@ <script> -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { visitUrl } from '~/lib/utils/url_utility'; import EnvironmentForm from './environment_form.vue'; @@ -39,7 +39,7 @@ export default { .then(({ data: { path } }) => visitUrl(path)) .catch((error) => { const message = error.response.data.message[0]; - createFlash({ message }); + createAlert({ message }); this.loading = false; }); }, diff --git a/app/assets/javascripts/environments/components/new_environment.vue b/app/assets/javascripts/environments/components/new_environment.vue index 14da2668417..bb4d6ab3428 100644 --- a/app/assets/javascripts/environments/components/new_environment.vue +++ b/app/assets/javascripts/environments/components/new_environment.vue @@ -1,5 +1,5 @@ <script> -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { visitUrl } from '~/lib/utils/url_utility'; import EnvironmentForm from './environment_form.vue'; @@ -32,7 +32,7 @@ export default { .then(({ data: { path } }) => visitUrl(path)) .catch((error) => { const message = error.response.data.message[0]; - createFlash({ message }); + createAlert({ message }); this.loading = false; }); }, diff --git a/app/assets/javascripts/environments/mixins/environments_mixin.js b/app/assets/javascripts/environments/mixins/environments_mixin.js index 8957a3074ed..5e936ad8c96 100644 --- a/app/assets/javascripts/environments/mixins/environments_mixin.js +++ b/app/assets/javascripts/environments/mixins/environments_mixin.js @@ -3,7 +3,7 @@ */ import { isEqual, isFunction, omitBy } from 'lodash'; import Visibility from 'visibilityjs'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import Poll from '~/lib/utils/poll'; import { getParameterByName } from '~/lib/utils/url_utility'; import { s__, __ } from '~/locale'; @@ -94,7 +94,7 @@ export default { errorCallback() { this.isLoading = false; - createFlash({ + createAlert({ message: s__('Environments|An error occurred while fetching the environments.'), }); }, @@ -123,7 +123,7 @@ export default { }) .catch((err) => { this.isLoading = false; - createFlash({ + createAlert({ message: isFunction(errorMessage) ? errorMessage(err.response.data) : errorMessage, }); }); @@ -179,7 +179,7 @@ export default { window.location.href = url.join('/'); }) .catch(() => { - createFlash({ + createAlert({ message: errorMessage, }); }); diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 00ed7f0291b..2dbc9b10836 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -142,7 +142,7 @@ export default { :edited-at="discussion.resolved_at" :edited-by="discussion.resolved_by" :action-text="resolvedText" - class-name="discussion-headline-light js-discussion-headline discussion-resolved-text gl-mb-2" + class-name="discussion-headline-light js-discussion-headline discussion-resolved-text gl-mb-2 gl-ml-3" /> </template> <template #avatar-badge> diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index 6fdafe34d67..930876e90b1 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -281,6 +281,7 @@ export default { > {{ __('Contributor') }} </user-access-role-badge> + <span class="note-actions__mobile-spacer"></span> <gl-button v-if="canResolve" ref="resolveButton" diff --git a/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue new file mode 100644 index 00000000000..ec70ab88870 --- /dev/null +++ b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection.vue @@ -0,0 +1,84 @@ +<script> +import { GlCard, GlLink } from '@gitlab/ui'; +import { s__ } from '~/locale'; +import ProtectionRow from './protection_row.vue'; + +export const i18n = { + rolesTitle: s__('BranchRules|Roles'), + usersTitle: s__('BranchRules|Users'), + groupsTitle: s__('BranchRules|Groups'), +}; + +export default { + name: 'ProtectionDetail', + i18n, + components: { GlCard, GlLink, ProtectionRow }, + props: { + header: { + type: String, + required: true, + }, + headerLinkTitle: { + type: String, + required: true, + }, + headerLinkHref: { + type: String, + required: true, + }, + roles: { + type: Array, + required: false, + default: () => [], + }, + users: { + type: Array, + required: false, + default: () => [], + }, + groups: { + type: Array, + required: false, + default: () => [], + }, + }, + computed: { + showUsersDivider() { + return Boolean(this.roles.length); + }, + showGroupsDivider() { + return Boolean(this.roles.length || this.users.length); + }, + }, +}; +</script> + +<template> + <gl-card class="gl-mb-5" body-class="gl-py-0"> + <template #header> + <div class="gl-display-flex gl-justify-content-space-between"> + <strong>{{ header }}</strong> + <gl-link :href="headerLinkHref" target="_blank">{{ headerLinkTitle }}</gl-link> + </div> + </template> + + <!-- Roles --> + <protection-row v-if="roles.length" :title="$options.i18n.rolesTitle" :access-levels="roles" /> + + <!-- Users --> + <protection-row + v-if="users.length" + :show-divider="showUsersDivider" + :users="users" + :title="$options.i18n.usersTitle" + /> + + <!-- Groups --> + <protection-row + v-if="groups.length" + :show-divider="showGroupsDivider" + :title="$options.i18n.groupsTitle" + :access-levels="groups" + /> + </gl-card> +</template> diff --git a/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue new file mode 100644 index 00000000000..56be0198574 --- /dev/null +++ b/app/assets/javascripts/projects/settings/branch_rules/components/view/protection_row.vue @@ -0,0 +1,86 @@ +<script> +import { GlAvatarsInline, GlAvatar, GlAvatarLink, GlTooltipDirective } from '@gitlab/ui'; +import { n__ } from '~/locale'; + +const AVATAR_TOOLTIP_MAX_CHARS = 100; +export const MAX_VISIBLE_AVATARS = 4; +export const AVATAR_SIZE = 32; + +export default { + name: 'ProtectionRow', + AVATAR_TOOLTIP_MAX_CHARS, + MAX_VISIBLE_AVATARS, + AVATAR_SIZE, + components: { GlAvatarsInline, GlAvatar, GlAvatarLink }, + directives: { + GlTooltip: GlTooltipDirective, + }, + props: { + title: { + type: String, + required: false, + default: null, + }, + accessLevels: { + type: Array, + required: false, + default: () => [], + }, + showDivider: { + type: Boolean, + required: false, + default: false, + }, + users: { + type: Array, + required: false, + default: () => [], + }, + }, + computed: { + avatarBadgeSrOnlyText() { + return n__( + '%d additional user', + '%d additional users', + this.users.length - this.$options.MAX_VISIBLE_AVATARS, + ); + }, + }, +}; +</script> + +<template> + <div + class="gl-display-flex gl-align-items-center gl-border-gray-100 gl-mb-4 gl-pt-4" + :class="{ 'gl-border-t-solid': showDivider }" + > + <div class="gl-mr-7">{{ title }}</div> + + <gl-avatars-inline + v-if="users.length" + :avatars="users" + :collapsed="true" + :max-visible="$options.MAX_VISIBLE_AVATARS" + :avatar-size="$options.AVATAR_SIZE" + badge-tooltip-prop="name" + :badge-tooltip-max-chars="$options.AVATAR_TOOLTIP_MAX_CHARS" + :badge-sr-only-text="avatarBadgeSrOnlyText" + > + <template #avatar="{ avatar }"> + <gl-avatar-link + :key="avatar.username" + v-gl-tooltip + target="_blank" + :href="avatar.webUrl" + :title="avatar.name" + > + <gl-avatar :src="avatar.avatarUrl" :label="avatar.name" :size="$options.AVATAR_SIZE" /> + </gl-avatar-link> + </template> + </gl-avatars-inline> + + <div v-for="(item, index) in accessLevels" :key="index" data-testid="access-level"> + {{ item.accessLevelDescription }} + </div> + </div> +</template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/action_buttons.vue b/app/assets/javascripts/vue_merge_request_widget/components/action_buttons.vue index 30a0e7c383c..f3186723a49 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/action_buttons.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/action_buttons.vue @@ -74,7 +74,7 @@ export default { </script> <template> - <div> + <div class="gl-display-flex"> <gl-dropdown v-if="tertiaryButtons.length" :text="dropdownLabel" diff --git a/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_actions.vue b/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_actions.vue index 7ba387c79b1..fbd0f6235ba 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_actions.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/deployment/deployment_actions.vue @@ -4,6 +4,7 @@ import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_m import { visitUrl } from '~/lib/utils/url_utility'; import { __, s__ } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import eventHub from '../../event_hub'; import MRWidgetService from '../../services/mr_widget_service'; import { MANUAL_DEPLOY, @@ -134,6 +135,7 @@ export default { }); }) .finally(() => { + eventHub.$emit('FetchDeployments'); this.actionInProgress = null; }); } diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue index 300e2a672cb..7d69a2d78b1 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue @@ -324,7 +324,7 @@ export default { @mouseup="onRowMouseUp" > <div - class="media-body gl-display-flex gl-flex-direction-row! gl-align-self-center" + class="media-body gl-display-flex gl-flex-direction-row! gl-w-full" data-testid="widget-extension-top-level" > <div class="gl-flex-grow-1" data-testid="widget-extension-top-level-summary"> diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index c8a2a8d119b..4ff2643057f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -506,6 +506,13 @@ export default { eventHub.$on('DisablePolling', () => { this.stopPolling(); }); + + eventHub.$on('FetchDeployments', () => { + this.fetchPreMergeDeployments(); + if (this.shouldRenderMergedPipeline) { + this.fetchPostMergeDeployments(); + } + }); }, dismissSuggestPipelines() { this.mr.isDismissedSuggestPipeline = true; diff --git a/app/assets/javascripts/vue_shared/components/markdown/field.vue b/app/assets/javascripts/vue_shared/components/markdown/field.vue index 32b3a0e22c2..657e4498b53 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/field.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/field.vue @@ -3,7 +3,7 @@ import { GlIcon, GlSafeHtmlDirective } from '@gitlab/ui'; import $ from 'jquery'; import '~/behaviors/markdown/render_gfm'; import { debounce, unescape } from 'lodash'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import GLForm from '~/gl_form'; import axios from '~/lib/utils/axios_utils'; import { stripHtml } from '~/lib/utils/text_utility'; @@ -272,7 +272,7 @@ export default { this.fetchMarkdown() .then((data) => this.renderMarkdown(data)) .catch(() => - createFlash({ + createAlert({ message: __('Error loading markdown preview'), }), ); @@ -315,7 +315,7 @@ export default { this.$nextTick() .then(() => $(this.$refs['markdown-preview']).renderGFM()) .catch(() => - createFlash({ + createAlert({ message: __('Error rendering Markdown preview'), }), ); diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue index 9b81444fc04..30d72332c90 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/suggestions.vue @@ -1,7 +1,7 @@ <script> import { GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import Vue from 'vue'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { __ } from '~/locale'; import SuggestionDiff from './suggestion_diff.vue'; @@ -91,7 +91,7 @@ export default { const suggestionElements = container.querySelectorAll('.js-render-suggestion'); if (this.lineType === 'old') { - createFlash({ + createAlert({ message: __('Unable to apply suggestions to a deleted line.'), parent: this.$el, }); diff --git a/app/assets/javascripts/vue_shared/components/metric_images/store/actions.js b/app/assets/javascripts/vue_shared/components/metric_images/store/actions.js index 832fb891838..1c4e8d332a9 100644 --- a/app/assets/javascripts/vue_shared/components/metric_images/store/actions.js +++ b/app/assets/javascripts/vue_shared/components/metric_images/store/actions.js @@ -1,4 +1,4 @@ -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { s__ } from '~/locale'; import * as types from './mutation_types'; @@ -11,7 +11,7 @@ export const fetchImagesFactory = (service) => async ({ state, commit }) => { commit(types.RECEIVE_METRIC_IMAGES_SUCCESS, response); } catch (error) { commit(types.RECEIVE_METRIC_IMAGES_ERROR); - createFlash({ message: s__('MetricImages|There was an issue loading metric images.') }); + createAlert({ message: s__('MetricImages|There was an issue loading metric images.') }); } }; @@ -34,7 +34,7 @@ export const uploadImageFactory = (service) => async ( commit(types.RECEIVE_METRIC_UPLOAD_SUCCESS, response); } catch (error) { commit(types.RECEIVE_METRIC_UPLOAD_ERROR); - createFlash({ message: s__('MetricImages|There was an issue uploading your image.') }); + createAlert({ message: s__('MetricImages|There was an issue uploading your image.') }); } }; @@ -57,7 +57,7 @@ export const updateImageFactory = (service) => async ( commit(types.RECEIVE_METRIC_UPDATE_SUCCESS, response); } catch (error) { commit(types.RECEIVE_METRIC_UPLOAD_ERROR); - createFlash({ message: s__('MetricImages|There was an issue updating your image.') }); + createAlert({ message: s__('MetricImages|There was an issue updating your image.') }); } }; @@ -68,7 +68,7 @@ export const deleteImageFactory = (service) => async ({ state, commit }, imageId await service.deleteMetricImage({ imageId, id: projectId, modelIid }); commit(types.RECEIVE_METRIC_DELETE_SUCCESS, imageId); } catch (error) { - createFlash({ message: s__('MetricImages|There was an issue deleting the image.') }); + createAlert({ message: s__('MetricImages|There was an issue deleting the image.') }); } }; diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/store/actions.js b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/store/actions.js index 0c697e624ab..2dab97826b9 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/store/actions.js +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_vue/store/actions.js @@ -1,4 +1,4 @@ -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { __ } from '~/locale'; import * as types from './mutation_types'; @@ -16,7 +16,7 @@ export const receiveLabelsSuccess = ({ commit }, labels) => commit(types.RECEIVE_SET_LABELS_SUCCESS, labels); export const receiveLabelsFailure = ({ commit }) => { commit(types.RECEIVE_SET_LABELS_FAILURE); - createFlash({ + createAlert({ message: __('Error fetching labels.'), }); }; @@ -38,7 +38,7 @@ export const requestCreateLabel = ({ commit }) => commit(types.REQUEST_CREATE_LA export const receiveCreateLabelSuccess = ({ commit }) => commit(types.RECEIVE_CREATE_LABEL_SUCCESS); export const receiveCreateLabelFailure = ({ commit }) => { commit(types.RECEIVE_CREATE_LABEL_FAILURE); - createFlash({ + createAlert({ message: __('Error creating label.'), }); }; diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view.vue index 5f344ae4214..ce93ad216ec 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view.vue @@ -8,7 +8,7 @@ import { GlLoadingIcon, } from '@gitlab/ui'; import produce from 'immer'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { __ } from '~/locale'; import { workspaceLabelsQueries } from '~/sidebar/constants'; import createLabelMutation from './graphql/create_label.mutation.graphql'; @@ -129,7 +129,7 @@ export default { this.$emit('hideCreateView'); } } catch { - createFlash({ message: errorMessage }); + createAlert({ message: errorMessage }); } this.labelCreateInProgress = false; }, diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view.vue index 8d3d4d5f86a..1d854505d11 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view.vue @@ -1,7 +1,7 @@ <script> import { GlDropdownForm, GlDropdownItem, GlLoadingIcon, GlIntersectionObserver } from '@gitlab/ui'; import fuzzaldrinPlus from 'fuzzaldrin-plus'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { __ } from '~/locale'; import { workspaceLabelsQueries } from '~/sidebar/constants'; @@ -62,7 +62,7 @@ export default { }, update: (data) => data.workspace?.labels?.nodes || [], error() { - createFlash({ message: __('Error fetching labels.') }); + createAlert({ message: __('Error fetching labels.') }); }, }, }, diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue index 522fbc07f5e..0e8da7281d8 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue @@ -2,7 +2,7 @@ import { debounce } from 'lodash'; import issuableLabelsSubscription from 'ee_else_ce/sidebar/queries/issuable_labels.subscription.graphql'; import { MutationOperationMode, getIdFromGraphQLId } from '~/graphql_shared/utils'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { IssuableType } from '~/issues/constants'; @@ -151,7 +151,7 @@ export default { return data.workspace?.issuable; }, error() { - createFlash({ message: __('Error fetching labels.') }); + createAlert({ message: __('Error fetching labels.') }); }, subscribeToMore: { document() { @@ -275,7 +275,7 @@ export default { }); }) .catch((error) => - createFlash({ + createAlert({ message: __('An error occurred while updating labels.'), captureError: true, error, diff --git a/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue b/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue index 9c6c12eac7d..536b2c8a281 100644 --- a/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer.vue @@ -53,7 +53,7 @@ export default { }, computed: { splitContent() { - return this.content.split('\n'); + return this.content.split(/\r?\n/); }, lineNumbers() { return this.splitContent.length; diff --git a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue index 79021161a65..6151d2ff85c 100644 --- a/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue +++ b/app/assets/javascripts/vue_shared/components/user_popover/user_popover.vue @@ -10,7 +10,7 @@ import { GlAvatarLabeled, } from '@gitlab/ui'; import { glEmojiTag } from '~/emoji'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { followUser, unfollowUser } from '~/rest_api'; import { isUserBusy } from '~/set_status_modal/utils'; import Tracking from '~/tracking'; @@ -141,7 +141,7 @@ export default { await followUser(this.user.id); this.$emit('follow'); } catch (error) { - createFlash({ + createAlert({ message: I18N_ERROR_FOLLOW, error, captureError: true, @@ -161,7 +161,7 @@ export default { await unfollowUser(this.user.id); this.$emit('unfollow'); } catch (error) { - createFlash({ + createAlert({ message: I18N_ERROR_UNFOLLOW, error, captureError: true, diff --git a/app/assets/javascripts/vue_shared/security_reports/components/artifact_downloads/merge_request_artifact_download.vue b/app/assets/javascripts/vue_shared/security_reports/components/artifact_downloads/merge_request_artifact_download.vue index e0669b3ed27..a4fb30a03a1 100644 --- a/app/assets/javascripts/vue_shared/security_reports/components/artifact_downloads/merge_request_artifact_download.vue +++ b/app/assets/javascripts/vue_shared/security_reports/components/artifact_downloads/merge_request_artifact_download.vue @@ -1,6 +1,6 @@ <script> import { reportTypeToSecurityReportTypeEnum } from 'ee_else_ce/vue_shared/security_reports/constants'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { s__ } from '~/locale'; import SecurityReportDownloadDropdown from '~/vue_shared/security_reports/components/security_report_download_dropdown.vue'; import securityReportMergeRequestDownloadPathsQuery from '~/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql'; @@ -67,7 +67,7 @@ export default { }, methods: { showError(error) { - createFlash({ + createAlert({ message: this.$options.i18n.apiError, captureError: true, error, diff --git a/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue b/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue index f6d85599dba..0e1975e1c09 100644 --- a/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue +++ b/app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue @@ -1,6 +1,6 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { s__ } from '~/locale'; import ReportSection from '~/reports/components/report_section.vue'; import { ERROR, SLOT_SUCCESS, SLOT_LOADING, SLOT_ERROR } from '~/reports/constants'; @@ -160,7 +160,7 @@ export default { this.fetchCounts(); }, showError(error) { - createFlash({ + createAlert({ message: this.$options.i18n.apiError, captureError: true, error, diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 84095819db9..82fe307bde9 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -98,7 +98,14 @@ $system-note-svg-size: 1rem; border-left: 1px solid $border-color; border-right: 1px solid $border-color; background-color: $white; - padding: $gl-padding-8 $gl-padding-8 $gl-padding-8 $gl-padding; + + .timeline-content { + padding: $gl-padding-8 $gl-padding-8 $gl-padding-8 $gl-padding; + } + + .timeline-avatar { + margin: $gl-padding-8 0 0 $gl-padding; + } .timeline-discussion-body { margin-left: 2rem; @@ -252,7 +259,7 @@ $system-note-svg-size: 1rem; } .note-body { - padding: $gl-padding-8; + padding: 0 $gl-padding-8 $gl-padding-8; overflow-x: auto; overflow-y: hidden; @@ -281,7 +288,7 @@ $system-note-svg-size: 1rem; padding: $gl-padding-8 0; margin: $gl-padding 0; background-color: transparent; - font-size: $gl-font-size-sm; + font-size: $gl-font-size; .note-header-info { padding-bottom: 0; @@ -815,17 +822,20 @@ $system-note-svg-size: 1rem; } .note-actions { - align-self: flex-start; justify-content: flex-end; flex-shrink: 1; display: inline-flex; align-items: center; - margin-left: 10px; + margin-left: $gl-padding-8; color: $gray-400; @include notes-media('max', map-get($grid-breakpoints, sm) - 1) { + justify-content: flex-start; float: none; - margin-left: 0; + + .note-actions__mobile-spacer { + flex-grow: 1; + } } } diff --git a/app/controllers/concerns/boards_actions.rb b/app/controllers/concerns/boards_actions.rb index 2f9edfad12d..7b056c228e6 100644 --- a/app/controllers/concerns/boards_actions.rb +++ b/app/controllers/concerns/boards_actions.rb @@ -8,38 +8,37 @@ module BoardsActions include BoardsResponses before_action :authorize_read_board!, only: [:index, :show] - before_action :boards, only: :index - before_action :board, only: :show + before_action :redirect_to_recent_board, only: [:index] + before_action :board, only: [:index, :show] before_action :push_licensed_features, only: [:index, :show] end def index - respond_with_boards + # if no board exists, create one + @board = board_create_service.execute.payload unless board # rubocop:disable Gitlab/ModuleWithInstanceVariables end def show - # Add / update the board in the recent visits table - board_visit_service.new(parent, current_user).execute(board) if request.format.html? + return render_404 unless board - respond_with_board + # Add / update the board in the recent visits table + board_visit_service.new(parent, current_user).execute(board) end private - # Noop on FOSS - def push_licensed_features + def redirect_to_recent_board + return if !parent.multiple_issue_boards_available? || !latest_visited_board + + redirect_to board_path(latest_visited_board.board) end - def boards - strong_memoize(:boards) do - existing_boards = boards_finder.execute - if existing_boards.any? - existing_boards - else - # if no board exists, create one - [board_create_service.execute.payload] - end - end + def latest_visited_board + @latest_visited_board ||= Boards::VisitsFinder.new(parent, current_user).latest + end + + # Noop on FOSS + def push_licensed_features end def board @@ -48,21 +47,9 @@ module BoardsActions end end - def board_type - board_klass.to_type - end - def board_visit_service Boards::Visits::CreateService end - - def serializer - BoardSerializer.new(current_user: current_user) - end - - def serialize_as_json(resource) - serializer.represent(resource, serializer: 'board', include_full_project_path: board.group_board?) - end end BoardsActions.prepend_mod_with('BoardsActions') diff --git a/app/controllers/concerns/boards_responses.rb b/app/controllers/concerns/boards_responses.rb index eb7392648a1..a1e7eaa7f60 100644 --- a/app/controllers/concerns/boards_responses.rb +++ b/app/controllers/concerns/boards_responses.rb @@ -60,35 +60,6 @@ module BoardsResponses def authorize_action_for!(resource, ability) return render_403 unless can?(current_user, ability, resource) end - - def respond_with_boards - respond_with(@boards) # rubocop:disable Gitlab/ModuleWithInstanceVariables - end - - def respond_with_board - # rubocop:disable Gitlab/ModuleWithInstanceVariables - return render_404 unless @board - - respond_with(@board) - # rubocop:enable Gitlab/ModuleWithInstanceVariables - end - - def serialize_as_json(resource) - serializer.represent(resource).as_json - end - - def respond_with(resource) - respond_to do |format| - format.html - format.json do - render json: serialize_as_json(resource) - end - end - end - - def serializer - BoardSerializer.new - end end BoardsResponses.prepend_mod_with('BoardsResponses') diff --git a/app/controllers/concerns/multiple_boards_actions.rb b/app/controllers/concerns/multiple_boards_actions.rb deleted file mode 100644 index 685c93fc2a2..00000000000 --- a/app/controllers/concerns/multiple_boards_actions.rb +++ /dev/null @@ -1,93 +0,0 @@ -# frozen_string_literal: true - -module MultipleBoardsActions - include Gitlab::Utils::StrongMemoize - extend ActiveSupport::Concern - - included do - include BoardsActions - - before_action :redirect_to_recent_board, only: [:index] - before_action :authenticate_user!, only: [:recent] - before_action :authorize_create_board!, only: [:create] - before_action :authorize_admin_board!, only: [:create, :update, :destroy] - end - - def recent - recent_visits = ::Boards::VisitsFinder.new(parent, current_user).latest(Board::RECENT_BOARDS_SIZE) - recent_boards = recent_visits.map(&:board) - - render json: serialize_as_json(recent_boards) - end - - def create - response = Boards::CreateService.new(parent, current_user, board_params).execute - - respond_to do |format| - format.json do - board = response.payload - - if response.success? - extra_json = { board_path: board_path(board) } - render json: serialize_as_json(board).merge(extra_json) - else - render json: board.errors, status: :unprocessable_entity - end - end - end - end - - def update - service = Boards::UpdateService.new(parent, current_user, board_params) - - respond_to do |format| - format.json do - if service.execute(board) - extra_json = { board_path: board_path(board) } - render json: serialize_as_json(board).merge(extra_json) - else - render json: board.errors, status: :unprocessable_entity - end - end - end - end - - def destroy - service = Boards::DestroyService.new(parent, current_user) - service.execute(board) - - respond_to do |format| - format.json { head :ok } - format.html { redirect_to boards_path, status: :found } - end - end - - private - - def redirect_to_recent_board - return unless board_type == Board.to_type - return if request.format.json? || !parent.multiple_issue_boards_available? || !latest_visited_board - - redirect_to board_path(latest_visited_board.board) - end - - def latest_visited_board - @latest_visited_board ||= Boards::VisitsFinder.new(parent, current_user).latest - end - - def authorize_create_board! - check_multiple_group_issue_boards_available! if group? - end - - def authorize_admin_board! - return render_404 unless can?(current_user, :admin_issue_board, parent) - end - - def serializer - BoardSerializer.new(current_user: current_user) - end - - def serialize_as_json(resource) - serializer.represent(resource, serializer: 'board', include_full_project_path: board.group_board?) - end -end diff --git a/app/controllers/groups/boards_controller.rb b/app/controllers/groups/boards_controller.rb index e64d838b7d1..4fa642f7446 100644 --- a/app/controllers/groups/boards_controller.rb +++ b/app/controllers/groups/boards_controller.rb @@ -20,16 +20,6 @@ class Groups::BoardsController < Groups::ApplicationController private - def board_klass - Board - end - - def boards_finder - strong_memoize :boards_finder do - Boards::BoardsFinder.new(parent, current_user) - end - end - def board_finder strong_memoize :board_finder do Boards::BoardsFinder.new(parent, current_user, board_id: params[:id]) diff --git a/app/controllers/jira_connect/public_keys_controller.rb b/app/controllers/jira_connect/public_keys_controller.rb new file mode 100644 index 00000000000..b3144993edb --- /dev/null +++ b/app/controllers/jira_connect/public_keys_controller.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module JiraConnect + class PublicKeysController < ::ApplicationController + # This is not inheriting from JiraConnect::Application controller because + # it doesn't need to handle JWT authentication. + + feature_category :integrations + + skip_before_action :authenticate_user! + + def show + return render_404 if Feature.disabled?(:jira_connect_oauth_self_managed) || !Gitlab.com? + + render plain: public_key.key + end + + private + + def public_key + JiraConnect::PublicKey.find(params[:id]) + end + end +end diff --git a/app/controllers/projects/boards_controller.rb b/app/controllers/projects/boards_controller.rb index 82b35a22669..082ebca40a3 100644 --- a/app/controllers/projects/boards_controller.rb +++ b/app/controllers/projects/boards_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Projects::BoardsController < Projects::ApplicationController - include MultipleBoardsActions + include BoardsActions include IssuableCollections before_action :check_issues_available! @@ -20,16 +20,6 @@ class Projects::BoardsController < Projects::ApplicationController private - def board_klass - Board - end - - def boards_finder - strong_memoize :boards_finder do - Boards::BoardsFinder.new(parent, current_user) - end - end - def board_finder strong_memoize :board_finder do Boards::BoardsFinder.new(parent, current_user, board_id: params[:id]) diff --git a/app/controllers/projects/pages_domains_controller.rb b/app/controllers/projects/pages_domains_controller.rb index 896c88cf8c3..43952a2efe4 100644 --- a/app/controllers/projects/pages_domains_controller.rb +++ b/app/controllers/projects/pages_domains_controller.rb @@ -41,9 +41,9 @@ class Projects::PagesDomainsController < Projects::ApplicationController end def create - @domain = @project.pages_domains.create(create_params) + @domain = PagesDomains::CreateService.new(@project, current_user, create_params).execute - if @domain.valid? + if @domain&.persisted? redirect_to project_pages_domain_path(@project, @domain) else render 'new' @@ -63,7 +63,9 @@ class Projects::PagesDomainsController < Projects::ApplicationController end def destroy - @domain.destroy + PagesDomains::DeleteService + .new(@project, current_user) + .execute(@domain) respond_to do |format| format.html do diff --git a/app/events/pages_domains/pages_domain_created_event.rb b/app/events/pages_domains/pages_domain_created_event.rb new file mode 100644 index 00000000000..a86718f4681 --- /dev/null +++ b/app/events/pages_domains/pages_domain_created_event.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module PagesDomains + class PagesDomainCreatedEvent < ::Gitlab::EventStore::Event + def schema + { + 'type' => 'object', + 'properties' => { + 'project_id' => { 'type' => 'integer' }, + 'namespace_id' => { 'type' => 'integer' }, + 'root_namespace_id' => { 'type' => 'integer' }, + 'domain' => { 'type' => 'string' } + }, + 'required' => %w[project_id namespace_id root_namespace_id] + } + end + end +end diff --git a/app/events/pages_domains/pages_domain_deleted_event.rb b/app/events/pages_domains/pages_domain_deleted_event.rb new file mode 100644 index 00000000000..7fe165a7249 --- /dev/null +++ b/app/events/pages_domains/pages_domain_deleted_event.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module PagesDomains + class PagesDomainDeletedEvent < ::Gitlab::EventStore::Event + def schema + { + 'type' => 'object', + 'properties' => { + 'project_id' => { 'type' => 'integer' }, + 'namespace_id' => { 'type' => 'integer' }, + 'root_namespace_id' => { 'type' => 'integer' }, + 'domain' => { 'type' => 'string' } + }, + 'required' => %w[project_id namespace_id root_namespace_id] + } + end + end +end diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index f98e70e41d8..a14b51d42bf 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -2,7 +2,7 @@ module BoardsHelper def board - @board ||= @board || @boards.first + @board end def board_data @@ -125,14 +125,6 @@ module BoardsHelper def can_admin_issue? can?(current_user, :admin_issue, current_board_parent) end - - def serializer - CurrentBoardSerializer.new - end - - def current_board_json - serializer.represent(board).as_json - end end BoardsHelper.prepend_mod_with('BoardsHelper') diff --git a/app/models/jira_connect/public_key.rb b/app/models/jira_connect/public_key.rb new file mode 100644 index 00000000000..8959884861b --- /dev/null +++ b/app/models/jira_connect/public_key.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module JiraConnect + class PublicKey + # Public keys are created with JWT tokens via JiraConnect::CreateAsymmetricJwtService + # They need to be available for third party applications to verify the token. + # This should happen right after the application received the token so public keys + # only need to exist for a few minutes. + REDIS_EXPIRY_TIME = 5.minutes.to_i.freeze + + attr_reader :key, :uuid + + def self.create!(key:) + new(key: key, uuid: Gitlab::UUID.v5(SecureRandom.hex)).save! + end + + def self.find(uuid) + Gitlab::Redis::SharedState.with do |redis| + key = redis.get(redis_key(uuid)) + + raise ActiveRecord::RecordNotFound if key.nil? + + new(key: key, uuid: uuid) + end + end + + def initialize(key:, uuid:) + key = OpenSSL::PKey.read(key) unless key.is_a?(OpenSSL::PKey::RSA) + + @key = key.to_s + @uuid = uuid + rescue OpenSSL::PKey::PKeyError + raise ArgumentError, 'Invalid public key' + end + + def save! + Gitlab::Redis::SharedState.with do |redis| + redis.set(self.class.redis_key(uuid), key, ex: REDIS_EXPIRY_TIME) + end + + self + end + + def self.redis_key(uuid) + "JiraConnect:public_key:uuid=#{uuid}" + end + end +end diff --git a/app/models/jira_connect_installation.rb b/app/models/jira_connect_installation.rb index 4496d5f2507..23813fa138f 100644 --- a/app/models/jira_connect_installation.rb +++ b/app/models/jira_connect_installation.rb @@ -33,4 +33,20 @@ class JiraConnectInstallation < ApplicationRecord instance_url end + + def audience_url + return unless proxy? + + Gitlab::Utils.append_path(instance_url, '/-/jira_connect') + end + + def audience_installed_event_url + return unless proxy? + + Gitlab::Utils.append_path(instance_url, '/-/jira_connect/events/installed') + end + + def proxy? + instance_url.present? + end end diff --git a/app/serializers/board_serializer.rb b/app/serializers/board_serializer.rb deleted file mode 100644 index 70a4c9ae282..00000000000 --- a/app/serializers/board_serializer.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -class BoardSerializer < BaseSerializer - entity BoardSimpleEntity -end diff --git a/app/serializers/board_simple_entity.rb b/app/serializers/board_simple_entity.rb deleted file mode 100644 index ab625490966..00000000000 --- a/app/serializers/board_simple_entity.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -class BoardSimpleEntity < Grape::Entity - expose :id - expose :name -end - -BoardSimpleEntity.prepend_mod_with('BoardSimpleEntity') diff --git a/app/serializers/current_board_entity.rb b/app/serializers/current_board_entity.rb deleted file mode 100644 index 530f7f5dea3..00000000000 --- a/app/serializers/current_board_entity.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -class CurrentBoardEntity < Grape::Entity - expose :id - expose :name - expose :hide_backlog_list - expose :hide_closed_list -end - -CurrentBoardEntity.prepend_mod_with('CurrentBoardEntity') diff --git a/app/serializers/current_board_serializer.rb b/app/serializers/current_board_serializer.rb deleted file mode 100644 index c58c77194f2..00000000000 --- a/app/serializers/current_board_serializer.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -class CurrentBoardSerializer < BaseSerializer - entity CurrentBoardEntity -end diff --git a/app/services/jira_connect/create_asymmetric_jwt_service.rb b/app/services/jira_connect/create_asymmetric_jwt_service.rb new file mode 100644 index 00000000000..71aba6feddd --- /dev/null +++ b/app/services/jira_connect/create_asymmetric_jwt_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module JiraConnect + class CreateAsymmetricJwtService + ARGUMENT_ERROR_MESSAGE = 'jira_connect_installation is not a proxy installation' + + def initialize(jira_connect_installation) + raise ArgumentError, ARGUMENT_ERROR_MESSAGE unless jira_connect_installation.proxy? + + @jira_connect_installation = jira_connect_installation + end + + def execute + JWT.encode(jwt_claims, private_key, 'RS256', jwt_headers) + end + + private + + def jwt_claims + { aud: aud_claim, iss: iss_claim, qsh: qsh_claim } + end + + def aud_claim + @jira_connect_installation.audience_url + end + + def iss_claim + @jira_connect_installation.client_key + end + + def qsh_claim + Atlassian::Jwt.create_query_string_hash( + @jira_connect_installation.audience_installed_event_url, + 'POST', + @jira_connect_installation.audience_url + ) + end + + def private_key + @private_key ||= OpenSSL::PKey::RSA.generate(3072) + end + + def public_key_storage + @public_key_storage ||= JiraConnect::PublicKey.create!(key: private_key.public_key) + end + + def jwt_headers + { kid: public_key_storage.uuid } + end + end +end diff --git a/app/services/pages_domains/create_service.rb b/app/services/pages_domains/create_service.rb new file mode 100644 index 00000000000..1f771ca3a05 --- /dev/null +++ b/app/services/pages_domains/create_service.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module PagesDomains + class CreateService < BaseService + def execute + return unless authorized? + + domain = project.pages_domains.create(params) + + publish_event(domain) if domain.persisted? + + domain + end + + private + + def authorized? + current_user.can?(:update_pages, project) + end + + def publish_event(domain) + event = PagesDomainCreatedEvent.new( + data: { + project_id: project.id, + namespace_id: project.namespace_id, + root_namespace_id: project.root_namespace.id, + domain: domain.domain + } + ) + + Gitlab::EventStore.publish(event) + end + end +end diff --git a/app/services/pages_domains/delete_service.rb b/app/services/pages_domains/delete_service.rb new file mode 100644 index 00000000000..af69e1845a9 --- /dev/null +++ b/app/services/pages_domains/delete_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module PagesDomains + class DeleteService < BaseService + def execute(domain) + return unless authorized? + + domain.destroy + + publish_event(domain) + end + + private + + def authorized? + current_user.can?(:update_pages, project) + end + + def publish_event(domain) + event = PagesDomainDeletedEvent.new( + data: { + project_id: project.id, + namespace_id: project.namespace_id, + root_namespace_id: project.root_namespace.id, + domain: domain.domain + } + ) + + Gitlab::EventStore.publish(event) + end + end +end diff --git a/app/services/projects/container_repository/cleanup_tags_base_service.rb b/app/services/projects/container_repository/cleanup_tags_base_service.rb index 8ea4ae4830a..5393c2c080d 100644 --- a/app/services/projects/container_repository/cleanup_tags_base_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_base_service.rb @@ -60,23 +60,6 @@ module Projects service.execute(container_repository) end - def can_destroy? - return true if container_expiration_policy - - can?(current_user, :destroy_container_image, project) - end - - def valid_regex? - %w[name_regex_delete name_regex name_regex_keep].each do |param_name| - regex = params[param_name] - ::Gitlab::UntrustedRegexp.new(regex) unless regex.blank? - end - true - rescue RegexpError => e - ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id) - false - end - def older_than params['older_than'] end diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index 285c3e252ef..0b31ac0c877 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -2,101 +2,58 @@ module Projects module ContainerRepository - class CleanupTagsService < CleanupTagsBaseService - def initialize(container_repository:, current_user: nil, params: {}) - super - - @params = params.dup - @counts = { cached_tags_count: 0 } - end - + class CleanupTagsService < BaseContainerRepositoryService def execute return error('access denied') unless can_destroy? return error('invalid regex') unless valid_regex? - tags = container_repository.tags - @counts[:original_size] = tags.size - - filter_out_latest!(tags) - filter_by_name!(tags) - - tags = truncate(tags) - populate_from_cache(tags) - - tags = filter_keep_n(tags) - tags = filter_by_older_than(tags) - - @counts[:before_delete_size] = tags.size - - delete_tags(tags).merge(@counts).tap do |result| - result[:deleted_size] = result[:deleted]&.size - - result[:status] = :error if @counts[:before_truncate_size] != @counts[:after_truncate_size] - end + cleanup_tags_service_class.new(container_repository: container_repository, current_user: current_user, params: params) + .execute end private - def filter_keep_n(tags) - tags, tags_to_keep = partition_by_keep_n(tags) - - cache_tags(tags_to_keep) - - tags - end - - def filter_by_older_than(tags) - tags, tags_to_keep = partition_by_older_than(tags) - - cache_tags(tags_to_keep) - - tags + def cleanup_tags_service_class + log_data = { + container_repository_id: container_repository.id, + container_repository_path: container_repository.path, + project_id: project.id + } + + if use_gitlab_service? + log_info(log_data.merge(gitlab_cleanup_tags_service: true)) + ::Projects::ContainerRepository::Gitlab::CleanupTagsService + else + log_info(log_data.merge(third_party_cleanup_tags_service: true)) + ::Projects::ContainerRepository::ThirdParty::CleanupTagsService + end end - def pushed_at(tag) - tag.created_at + def use_gitlab_service? + Feature.enabled?(:container_registry_new_cleanup_service, project) && + container_repository.migrated? && + container_repository.gitlab_api_client.supports_gitlab_api? end - def truncate(tags) - @counts[:before_truncate_size] = tags.size - @counts[:after_truncate_size] = tags.size - - return tags if max_list_size == 0 - - # truncate the list to make sure that after the #filter_keep_n - # execution, the resulting list will be max_list_size - truncated_size = max_list_size + keep_n_as_integer - - return tags if tags.size <= truncated_size + def can_destroy? + return true if container_expiration_policy - tags = tags.sample(truncated_size) - @counts[:after_truncate_size] = tags.size - tags + can?(current_user, :destroy_container_image, project) end - def populate_from_cache(tags) - @counts[:cached_tags_count] = cache.populate(tags) if caching_enabled? - end - - def cache_tags(tags) - cache.insert(tags, older_than_in_seconds) if caching_enabled? - end - - def cache - strong_memoize(:cache) do - ::Gitlab::ContainerRepository::Tags::Cache.new(container_repository) + def valid_regex? + %w[name_regex_delete name_regex name_regex_keep].each do |param_name| + regex = params[param_name] + ::Gitlab::UntrustedRegexp.new(regex) unless regex.blank? end + true + rescue RegexpError => e + ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id) + false end - def caching_enabled? - result = ::Gitlab::CurrentSettings.current_application_settings.container_registry_expiration_policies_caching && - container_expiration_policy && - older_than.present? - !!result - end - - def max_list_size - ::Gitlab::CurrentSettings.current_application_settings.container_registry_cleanup_tags_service_max_list_size.to_i + def container_expiration_policy + params['container_expiration_policy'] end end end diff --git a/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb b/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb index 81bb94c867a..e947e9575e2 100644 --- a/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/gitlab/cleanup_tags_service.rb @@ -14,9 +14,6 @@ module Projects end def execute - return error('access denied') unless can_destroy? - return error('invalid regex') unless valid_regex? - with_timeout do |start_time, result| container_repository.each_tags_page(page_size: TAGS_PAGE_SIZE) do |tags| execute_for_tags(tags, result) diff --git a/app/services/projects/container_repository/third_party/cleanup_tags_service.rb b/app/services/projects/container_repository/third_party/cleanup_tags_service.rb new file mode 100644 index 00000000000..c6335629b52 --- /dev/null +++ b/app/services/projects/container_repository/third_party/cleanup_tags_service.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +module Projects + module ContainerRepository + module ThirdParty + class CleanupTagsService < CleanupTagsBaseService + def initialize(container_repository:, current_user: nil, params: {}) + super + + @params = params.dup + @counts = { cached_tags_count: 0 } + end + + def execute + tags = container_repository.tags + @counts[:original_size] = tags.size + + filter_out_latest!(tags) + filter_by_name!(tags) + + tags = truncate(tags) + populate_from_cache(tags) + + tags = filter_keep_n(tags) + tags = filter_by_older_than(tags) + + @counts[:before_delete_size] = tags.size + + delete_tags(tags).merge(@counts).tap do |result| + result[:deleted_size] = result[:deleted]&.size + + result[:status] = :error if @counts[:before_truncate_size] != @counts[:after_truncate_size] + end + end + + private + + def filter_keep_n(tags) + tags, tags_to_keep = partition_by_keep_n(tags) + + cache_tags(tags_to_keep) + + tags + end + + def filter_by_older_than(tags) + tags, tags_to_keep = partition_by_older_than(tags) + + cache_tags(tags_to_keep) + + tags + end + + def pushed_at(tag) + tag.created_at + end + + def truncate(tags) + @counts[:before_truncate_size] = tags.size + @counts[:after_truncate_size] = tags.size + + return tags if max_list_size == 0 + + # truncate the list to make sure that after the #filter_keep_n + # execution, the resulting list will be max_list_size + truncated_size = max_list_size + keep_n_as_integer + + return tags if tags.size <= truncated_size + + tags = tags.sample(truncated_size) + @counts[:after_truncate_size] = tags.size + tags + end + + def populate_from_cache(tags) + @counts[:cached_tags_count] = cache.populate(tags) if caching_enabled? + end + + def cache_tags(tags) + cache.insert(tags, older_than_in_seconds) if caching_enabled? + end + + def cache + strong_memoize(:cache) do + ::Gitlab::ContainerRepository::Tags::Cache.new(container_repository) + end + end + + def caching_enabled? + result = current_application_settings.container_registry_expiration_policies_caching && + container_expiration_policy && + older_than.present? + !!result + end + + def max_list_size + current_application_settings.container_registry_cleanup_tags_service_max_list_size.to_i + end + + def current_application_settings + ::Gitlab::CurrentSettings.current_application_settings + end + end + end + end +end diff --git a/app/views/admin/users/_projects.html.haml b/app/views/admin/users/_projects.html.haml index a9f5c560b41..3ccf3ef4f2a 100644 --- a/app/views/admin/users/_projects.html.haml +++ b/app/views/admin/users/_projects.html.haml @@ -1,13 +1,17 @@ - if local_assigns.has_key?(:contributed_projects) && contributed_projects.present? - .card.contributed-projects - .card-header= _('Projects contributed to') - = render 'shared/projects/list', - projects: contributed_projects.sort_by(&:star_count).reverse, - projects_limit: 5, stars: true, avatar: false + = render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5' }, body_options: { class: 'gl-py-0' }) do |c| + - c.header do + = _('Projects contributed to') + - c.body do + = render 'shared/projects/list', + projects: contributed_projects.sort_by(&:star_count).reverse, + projects_limit: 5, stars: true, avatar: false - if local_assigns.has_key?(:projects) && projects.present? - .card - .card-header= _('Personal projects') - = render 'shared/projects/list', - projects: projects.sort_by(&:star_count).reverse, - projects_limit: 10, stars: true, avatar: false + = render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5' }, body_options: { class: 'gl-py-0' }) do |c| + - c.header do + = _('Personal projects') + - c.body do + = render 'shared/projects/list', + projects: projects.sort_by(&:star_count).reverse, + projects_limit: 10, stars: true, avatar: false diff --git a/app/views/groups/boards/index.html.haml b/app/views/groups/boards/index.html.haml index bb56769bd3f..e5b5f6404bb 100644 --- a/app/views/groups/boards/index.html.haml +++ b/app/views/groups/boards/index.html.haml @@ -1 +1 @@ -= render "shared/boards/show", board: @boards.first += render "shared/boards/show", board: @board diff --git a/app/views/groups/projects.html.haml b/app/views/groups/projects.html.haml index 91a66edd3b4..1ca85812f68 100644 --- a/app/views/groups/projects.html.haml +++ b/app/views/groups/projects.html.haml @@ -2,45 +2,49 @@ - page_title _("Projects") - @content_class = "limit-container-width" unless fluid_layout -.card.gl-mt-3.js-search-settings-section - .card-header - %strong= @group.name - projects: += render Pajamas::CardComponent.new(card_options: { class: 'gl-mt-3 js-search-settings-section' }, header_options: { class: 'gl-display-flex' }, body_options: { class: 'gl-py-0' }) do |c| + - c.header do + .gl-flex-grow-1 + %strong= @group.name + projects: - if can? current_user, :admin_group, @group .controls = link_to new_project_path(namespace_id: @group.id), class: "btn gl-button btn-sm btn-confirm" do New project - %ul.projects-list.content-list.group-settings-projects - - @projects.each do |project| - %li.project-row{ class: ('no-description' if project.description.blank?) } - .controls - = link_to _('Members'), project_project_members_path(project), id: "edit_#{dom_id(project)}", class: "btn gl-button" - = link_to _('Edit'), edit_project_path(project), id: "edit_#{dom_id(project)}", class: "btn gl-button" - = render 'delete_project_button', project: project + - c.body do + %ul.projects-list.content-list.group-settings-projects + - @projects.each do |project| + %li.project-row{ class: ('no-description' if project.description.blank?) } + .controls + = render Pajamas::ButtonComponent.new(href: project_project_members_path(project), button_options: { id: "edit_#{dom_id(project)}" }) do + = _('Members') + = render Pajamas::ButtonComponent.new(href: edit_project_path(project), button_options: { id: "edit_#{dom_id(project)}" }) do + = _('Edit') + = render 'delete_project_button', project: project - .stats - = gl_badge_tag storage_counter(project.statistics&.storage_size) - = render 'project_badges', project: project + .stats + = gl_badge_tag storage_counter(project.statistics&.storage_size) + = render 'project_badges', project: project - .title - = link_to project_path(project), class: 'js-prefetch-document' do - .dash-project-avatar - .avatar-container.rect-avatar.s40 - = project_icon(project, alt: '', class: 'avatar project-avatar s40', width: 40, height: 40) - %span.project-full-name - %span.namespace-name - - if project.namespace - = project.namespace.human_name - \/ - %span.project-name - = project.name - %span{ class: visibility_level_color(project.visibility_level) } - = visibility_level_icon(project.visibility_level) + .title + = link_to project_path(project), class: 'js-prefetch-document' do + .dash-project-avatar + .avatar-container.rect-avatar.s40 + = project_icon(project, alt: '', class: 'avatar project-avatar s40', width: 40, height: 40) + %span.project-full-name + %span.namespace-name + - if project.namespace + = project.namespace.human_name + \/ + %span.project-name + = project.name + %span{ class: visibility_level_color(project.visibility_level) } + = visibility_level_icon(project.visibility_level) - - if project.description.present? - .description - = markdown_field(project, :description) - - if @projects.blank? - .nothing-here-block This group has no projects yet + - if project.description.present? + .description + = markdown_field(project, :description) + - if @projects.blank? + .nothing-here-block This group has no projects yet = paginate @projects, theme: "gitlab" diff --git a/app/views/projects/boards/index.html.haml b/app/views/projects/boards/index.html.haml index bb56769bd3f..e5b5f6404bb 100644 --- a/app/views/projects/boards/index.html.haml +++ b/app/views/projects/boards/index.html.haml @@ -1 +1 @@ -= render "shared/boards/show", board: @boards.first += render "shared/boards/show", board: @board diff --git a/app/views/projects/jobs/_user.html.haml b/app/views/projects/jobs/_user.html.haml index 90ce581a903..03cbabb0c2a 100644 --- a/app/views/projects/jobs/_user.html.haml +++ b/app/views/projects/jobs/_user.html.haml @@ -1,7 +1,7 @@ by %a{ href: user_path(@build.user) } %span.d-none.d-sm-inline - = image_tag avatar_icon_for_user(@build.user, 24), class: "avatar s24" + = render Pajamas::AvatarComponent.new(@build.user, size: 24, alt: "") %strong{ data: { toggle: 'tooltip', placement: 'top', title: @build.user.to_reference } } = @build.user.name %strong.d-inline.d-sm-none= @build.user.to_reference diff --git a/config/feature_flags/development/container_registry_new_cleanup_service.yml b/config/feature_flags/development/container_registry_new_cleanup_service.yml new file mode 100644 index 00000000000..0826abbf994 --- /dev/null +++ b/config/feature_flags/development/container_registry_new_cleanup_service.yml @@ -0,0 +1,8 @@ +--- +name: container_registry_new_cleanup_service +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98651 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/375037 +milestone: '15.5' +type: development +group: group::package +default_enabled: false diff --git a/config/routes/jira_connect.rb b/config/routes/jira_connect.rb index 202d2574ad0..f45f524935a 100644 --- a/config/routes/jira_connect.rb +++ b/config/routes/jira_connect.rb @@ -14,6 +14,7 @@ namespace :jira_connect do resources :subscriptions, only: [:index, :create, :destroy] resources :branches, only: [:new] + resources :public_keys, only: :show resources :installations, only: [:index] do collection do diff --git a/config/routes/project.rb b/config/routes/project.rb index db121794f57..cd9315ba2aa 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -223,11 +223,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end - resources :boards, only: [:index, :show, :create, :update, :destroy], constraints: { id: /\d+/ } do - collection do - get :recent - end - end + resources :boards, only: [:index, :show], constraints: { id: /\d+/ } get 'releases/permalink/latest(/)(*suffix_path)', to: 'releases#latest_permalink', as: :latest_release_permalink, format: false diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index a59549cca66..2f997ec751c 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -165,7 +165,7 @@ This setting limits global search requests as follows: | Authenticated user | 30 | | Unauthenticated user | 10 | -Depending on the number of enabled [scopes](../user/search/advanced_search.md#global-search-scopes), a global search request can consume two to seven requests per minute. You may want to disable one or more scopes to use fewer requests. Global search requests that exceed the search rate limit per minute return the following error: +Depending on the number of enabled [scopes](../user/search/index.md#global-search-scopes), a global search request can consume two to seven requests per minute. You may want to disable one or more scopes to use fewer requests. Global search requests that exceed the search rate limit per minute return the following error: ```plaintext This endpoint has been requested too many times. Try again later. diff --git a/doc/ci/large_repositories/index.md b/doc/ci/large_repositories/index.md index 571b8eee1f9..c7ecc25dc44 100644 --- a/doc/ci/large_repositories/index.md +++ b/doc/ci/large_repositories/index.md @@ -260,3 +260,8 @@ For very active repositories with a large number of references and files, you ca - Optimize your CI/CD jobs by seeding repository data in a pre-clone step with the [`pre_clone_script`](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-section) of GitLab Runner. See [SaaS runners on Linux](../runners/saas/linux_saas_runner.md#pre-clone-script) for details. + Besides speeding up pipelines in large and active projects, + seeding the repository data also helps avoid + `429 Too many requests` errors from Cloudflare. + This error can occur if you have many runners behind a single, + NAT'ed IP address that pulls from GitLab.com. diff --git a/doc/development/features_inside_dot_gitlab.md b/doc/development/features_inside_dot_gitlab.md index da823121154..7a46cd40da1 100644 --- a/doc/development/features_inside_dot_gitlab.md +++ b/doc/development/features_inside_dot_gitlab.md @@ -16,6 +16,6 @@ When implementing new features, please refer to these existing features to avoid - [CODEOWNERS](../user/project/code_owners.md#set-up-code-owners): `.gitlab/CODEOWNERS`. - [Route Maps](../ci/review_apps/index.md#route-maps): `.gitlab/route-map.yml`. - [Customize Auto DevOps Helm Values](../topics/autodevops/customize.md#customize-values-for-helm-chart): `.gitlab/auto-deploy-values.yaml`. -- [Insights](../user/project/insights/index.md#configure-your-insights): `.gitlab/insights.yml`. +- [Insights](../user/project/insights/index.md#configure-project-insights): `.gitlab/insights.yml`. - [Service Desk Templates](../user/project/service_desk.md#using-customized-email-templates): `.gitlab/service_desk_templates/`. - [Web IDE](../user/project/web_ide/index.md#web-ide-configuration-file): `.gitlab/.gitlab-webide.yml`. diff --git a/doc/user/group/insights/index.md b/doc/user/group/insights/index.md index 79034264787..c9911fc5efa 100644 --- a/doc/user/group/insights/index.md +++ b/doc/user/group/insights/index.md @@ -35,7 +35,7 @@ You can also create custom Insights charts that are more relevant for your group To customize your Insights: -1. Create a new file [`.gitlab/insights.yml`](../../project/insights/index.md#writing-your-gitlabinsightsyml) +1. Create a new file [`.gitlab/insights.yml`](../../project/insights/index.md#configure-project-insights) in a project that belongs to your group. 1. On the top bar, select **Main menu > Groups** and find your group. 1. On the left sidebar, select **Settings > General**. diff --git a/doc/user/project/insights/img/project_insights.png b/doc/user/project/insights/img/project_insights.png Binary files differdeleted file mode 100644 index 83674c94110..00000000000 --- a/doc/user/project/insights/img/project_insights.png +++ /dev/null diff --git a/doc/user/project/insights/index.md b/doc/user/project/insights/index.md index 133f2ad55e1..1107e91c33e 100644 --- a/doc/user/project/insights/index.md +++ b/doc/user/project/insights/index.md @@ -4,61 +4,65 @@ group: Optimize info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Insights **(ULTIMATE)** +# Insights for projects **(ULTIMATE)** -> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/725) in GitLab 12.0. +Configure project insights to explore data such as: -Configure the Insights that matter for your projects to explore data such as -triage hygiene, issues created/closed per a given period, average time for merge -requests to be merged and much more. +- Issues created and closed during a specified period. +- Average time for merge requests to be merged. +- Triage hygiene. - +Insights are also available for [groups](../../group/insights/index.md). -NOTE: -This feature is [also available at the group level](../../group/insights/index.md). +## View project insights -## View your project's Insights +Prerequisites: -You can access your project's Insights by clicking the **Analytics > Insights** -link in the left sidebar. +- You must have: + - Access to a project to view information about its merge requests and issues. + - Permission to view confidential merge requests and issues in the project. -## Configure your Insights +To view project insights: -Insights are configured using a YAML file called `.gitlab/insights.yml` within -a project. That file is used in the project's Insights page. +1. On the top bar, select **Main menu > Projects** and find your project. +1. On the left sidebar, select **Analytics > Insights**. +1. To view a report, select the **Select page** dropdown list. -See [Writing your `.gitlab/insights.yml`](#writing-your-gitlabinsightsyml) below -for details about the content of this file. +## Configure project insights -NOTE: -After the configuration file is created, you can also -[use it for your project's group](../../group/insights/index.md#configure-your-insights). +Prerequisites: -NOTE: -If the project doesn't have any configuration file, it attempts to use -the group configuration if possible. If the group doesn't have any -configuration, the default configuration is used. +- Depending on your project configuration, you must have at least the Developer role. -## Permissions +Project insights are configured with the [`.gitlab/insights.yml`](#insights-configuration-file) file in the project. If a project doesn't have a configuration file, it uses the [group configuration](../../group/insights/index.md#configure-your-insights). -If you have access to view a project, then you have access to view their -Insights. +The `.gitlab/insights.yml` file is a YAML file where you define: -NOTE: -Issues or merge requests that you don't have access to (because you don't have -access to the project they belong to, or because they are confidential) are -filtered out of the Insights charts. +- The structure and order of charts in a report. +- The style of charts displayed in the report of your project or group. + +To configure project insights, either: + +- Create a `.gitlab/insights.yml` file locally in the root directory of your project, and push your changes. +- Create a `.gitlab/insights.yml` file in the UI: + 1. On the top bar, select **Main menu > Projects** and find your project. + 1. Above the file list, select the branch you want to commit to, select the plus icon, then select **New file**. + 1. In the **File name** text box, enter `.gitlab/insights.yml`. + 1. In the large text box, update the file contents. + 1. Select **Commit changes**. -You may also consult the [group permissions table](../../permissions.md#group-members-permissions). +After you create the configuration file, you can also +[use it for the project's group](../../group/insights/index.md#configure-your-insights). -## Writing your `.gitlab/insights.yml` +## Insights configuration file -The `.gitlab/insights.yml` file defines the structure and order of the Insights -charts displayed in each Insights page of your project or group. +In the `.gitlab/insights.yml` file: -Each page has a unique key and a collection of charts to fetch and display. +- [Configuration parameters](#insights-configuration-parameters) define the chart behavior. +- Each report has a unique key and a collection of charts to fetch and display. +- Each chart definition is made up of a hash composed of key-value pairs. -For example, here's a single definition for Insights that displays one page with one chart: +The following example shows a single definition that displays one report with one chart. ```yaml bugsCharts: @@ -78,30 +82,9 @@ bugsCharts: period_limit: 24 ``` -Each chart definition is made up of a hash composed of key-value pairs. - -For example, here's single chart definition: - -```yaml -- title: "Monthly bugs created" - description: "Open bugs created per month" - type: bar - query: - data_source: issuables - params: - issuable_type: issue - issuable_state: opened - filter_labels: - - bug - group_by: month - period_limit: 24 -``` - -## Configuration parameters - -A chart is defined as a list of parameters that define the chart's behavior. +## Insights configuration parameters -The following table lists available parameters for charts: +The following table lists the chart parameters: | Keyword | Description | |:---------------------------------------------------|:------------| @@ -110,11 +93,6 @@ The following table lists available parameters for charts: | [`type`](#type) | The type of chart: `bar`, `line` or `stacked-bar`. | | [`query`](#query) | A hash that defines the data source and filtering conditions for the chart. | -## Parameter details - -The following are detailed explanations for parameters used to configure -Insights charts. - ### `title` `title` is the title of the chart as it displays on the Insights page. @@ -405,8 +383,6 @@ An array of environments to include into the calculation (default: production). ### `projects` -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/10904) in GitLab 12.4. - You can limit where the "issuables" can be queried from: - If `.gitlab/insights.yml` is used for a [group's insights](../../group/insights/index.md#configure-your-insights), with `projects`, you can limit the projects to be queried. By default, all projects currently under the group are used. diff --git a/doc/user/search/advanced_search.md b/doc/user/search/advanced_search.md index 1b41a149029..86b59572e77 100644 --- a/doc/user/search/advanced_search.md +++ b/doc/user/search/advanced_search.md @@ -43,42 +43,3 @@ See [Advanced Search syntax](global_search/advanced_search_syntax.md) for more i - To search by issue ID, use the `#` prefix followed by the issue ID (for example, [`#23456`](https://gitlab.com/search?snippets=&scope=issues&repository_ref=&search=%2323456&group_id=9970&project_id=278964)). - To search by merge request ID, use the `!` prefix followed by the merge request ID (for example, [`!23456`](https://gitlab.com/search?snippets=&scope=merge_requests&repository_ref=&search=%2123456&group_id=9970&project_id=278964)). - -## Global search scopes **(FREE SELF)** - -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68640) in GitLab 14.3. - -To improve the performance of your instance's global search, you can limit -the scope of the search. To do so, you can exclude global search scopes by disabling -[`ops` feature flags](../../development/feature_flags/index.md#ops-type). - -Global search has all its scopes **enabled** by default in GitLab SaaS and -self-managed instances. A GitLab administrator can disable the following `ops` -feature flags to limit the scope of your instance's global search and optimize -its performance: - -| Scope | Feature flag | Description | -|--|--|--| -| Code | `global_search_code_tab` | When enabled, the global search includes code as part of the search. | -| Commits | `global_search_commits_tab` | When enabled, the global search includes commits as part of the search. | -| Issues | `global_search_issues_tab` | When enabled, the global search includes issues as part of the search. | -| Merge Requests | `global_search_merge_requests_tab` | When enabled, the global search includes merge requests as part of the search. | -| Users | `global_search_users_tab` | When enabled, the global search includes users as part of the search. | -| Wiki | `global_search_wiki_tab` | When enabled, the global search includes wiki as part of the search. [Group wikis](../project/wiki/group.md) are not included. | - -## Global Search validation - -To prevent abusive searches, such as searches that may result in a Distributed Denial of Service (DDoS), Global Search ignores, logs, and -doesn't return any results for searches considered abusive according to the following criteria: - -- Searches with less than 2 characters. -- Searches with any term greater than 100 characters. URL search terms have a maximum of 200 characters. -- Searches with a stop word as the only term (for example, "the", "and", "if", etc.). -- Searches with a `group_id` or `project_id` parameter that is not completely numeric. -- Searches with a `repository_ref` or `project_ref` parameter that has special characters not allowed by [Git refname](https://git-scm.com/docs/git-check-ref-format). -- Searches with a `scope` that is unknown. - -Searches that don't comply with the criteria described below aren't logged as abusive but are flagged with an error: - -- Searches with more than 4096 characters. -- Searches with more than 64 terms. diff --git a/doc/user/search/index.md b/doc/user/search/index.md index decac561f52..aab13527428 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -13,20 +13,40 @@ Both types of search are the same, except when you are searching through code. - When you use basic search to search code, your search includes one project at a time. - When you use [advanced search](advanced_search.md) to search code, your search includes all projects at once. -## Basic search - -Use basic search to find: - -- Projects -- Issues -- Merge requests -- Milestones -- Users -- Epics (when searching in a group only) -- Code -- Comments -- Commits -- Wiki +## Global search scopes **(FREE SELF)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68640) in GitLab 14.3. + +To improve the performance of your instance's global search, a GitLab administrator +can limit the search scope by disabling the following [`ops` feature flags](../../development/feature_flags/index.md#ops-type). + +| Scope | Feature flag | Description | +|--|--|--| +| Code | `global_search_code_tab` | When enabled, global search includes code. | +| Commits | `global_search_commits_tab` | When enabled, global search includes commits. | +| Issues | `global_search_issues_tab` | When enabled, global search includes issues. | +| Merge requests | `global_search_merge_requests_tab` | When enabled, global search includes merge requests. | +| Users | `global_search_users_tab` | When enabled, global search includes users. | +| Wiki | `global_search_wiki_tab` | When enabled, global search includes project wikis (not [group wikis](../project/wiki/group.md)). | + +All global search scopes are enabled by default on GitLab.com +and self-managed instances. + +## Global search validation + +Global search ignores and logs as abusive any search with: + +- Fewer than 2 characters +- A term longer than 100 characters (URL search terms must not exceed 200 characters) +- A stop word only (for example, `the`, `and`, or `if`) +- An unknown `scope` +- `group_id` or `project_id` that is not completely numeric +- `repository_ref` or `project_ref` with special characters not allowed by [Git refname](https://git-scm.com/docs/git-check-ref-format) + +Global search only flags with an error any search that includes more than: + +- 4096 characters +- 64 terms ## Perform a search diff --git a/lib/api/pages_domains.rb b/lib/api/pages_domains.rb index b417badb9c9..9cf61967ba4 100644 --- a/lib/api/pages_domains.rb +++ b/lib/api/pages_domains.rb @@ -106,7 +106,9 @@ module API pages_domain_params = declared(params, include_parent_namespaces: false) - pages_domain = user_project.pages_domains.create(pages_domain_params) + pages_domain = ::PagesDomains::CreateService + .new(user_project, current_user, pages_domain_params) + .execute if pages_domain.persisted? present pages_domain, with: Entities::PagesDomain @@ -152,7 +154,9 @@ module API delete ":id/pages/domains/:domain", requirements: PAGES_DOMAINS_ENDPOINT_REQUIREMENTS do authorize! :update_pages, user_project - pages_domain.destroy + ::PagesDomains::DeleteService + .new(user_project, current_user) + .execute(pages_domain) no_content! end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6b4bd89615d..e69e89e62ea 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -138,6 +138,11 @@ msgid_plural "%d additional committers" msgstr[0] "" msgstr[1] "" +msgid "%d additional user" +msgid_plural "%d additional users" +msgstr[0] "" +msgstr[1] "" + msgid "%d approver" msgid_plural "%d approvers" msgstr[0] "" @@ -6785,6 +6790,9 @@ msgstr "" msgid "BranchRules|Create wildcard: %{searchTerm}" msgstr "" +msgid "BranchRules|Groups" +msgstr "" + msgid "BranchRules|Keep stable branches secure and force developers to use merge requests. %{linkStart}What are protected branches?%{linkEnd}" msgstr "" @@ -6800,9 +6808,15 @@ msgstr "" msgid "BranchRules|Require approval from code owners." msgstr "" +msgid "BranchRules|Roles" +msgstr "" + msgid "BranchRules|Target Branch" msgstr "" +msgid "BranchRules|Users" +msgstr "" + msgid "BranchRules|default" msgstr "" diff --git a/qa/Gemfile b/qa/Gemfile index cf939f8e301..53d05b1ce89 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -22,13 +22,15 @@ gem 'timecop', '~> 0.9.1' gem 'parallel', '~> 1.19' gem 'rainbow', '~> 3.0.0' gem 'rspec-parameterized', '~> 0.4.2' -gem 'octokit', '~> 4.21' +gem 'octokit', '~> 5.6.1' +gem "faraday-retry", "~> 2.0" gem 'webdrivers', '~> 5.0' gem 'zeitwerk', '~> 2.4' gem 'influxdb-client', '~> 1.17' gem 'terminal-table', '~> 3.0.0', require: false gem 'slack-notifier', '~> 2.4', require: false -gem 'fog-google', '~> 1.17', require: false +gem 'fog-google', '~> 1.19', require: false +gem 'fog-core', '2.1.0', require: false # fog-google generates a ton of warnings with latest core gem "warning", "~> 1.3" gem 'confiner', '~> 0.3' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index dd14b675769..0773aae706a 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -67,26 +67,15 @@ GEM domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) equalizer (0.0.11) - excon (0.88.0) + excon (0.92.4) faker (2.19.0) i18n (>= 1.6, < 2) - faraday (1.5.1) - faraday-em_http (~> 1.0) - faraday-em_synchrony (~> 1.0) - faraday-excon (~> 1.1) - faraday-httpclient (~> 1.0.1) - faraday-net_http (~> 1.0) - faraday-net_http_persistent (~> 1.1) - faraday-patron (~> 1.0) - multipart-post (>= 1.2, < 3) + faraday (2.5.2) + faraday-net_http (>= 2.0, < 3.1) ruby2_keywords (>= 0.0.4) - faraday-em_http (1.0.0) - faraday-em_synchrony (1.0.0) - faraday-excon (1.1.0) - faraday-httpclient (1.0.1) - faraday-net_http (1.0.1) - faraday-net_http_persistent (1.2.0) - faraday-patron (1.0.0) + faraday-net_http (3.0.0) + faraday-retry (2.0.0) + faraday (~> 2.0) ffi (1.15.5) ffi-compiler (1.0.1) ffi (>= 1.0.0) @@ -96,8 +85,8 @@ GEM excon (~> 0.58) formatador (~> 0.2) mime-types - fog-google (1.17.0) - fog-core (<= 2.1.0) + fog-google (1.19.0) + fog-core (< 2.3) fog-json (~> 1.2) fog-xml (~> 0.1.0) google-apis-compute_v1 (~> 0.14) @@ -118,7 +107,7 @@ GEM gitlab (4.18.0) httparty (~> 0.18) terminal-table (>= 1.5.1) - gitlab-qa (8.4.2) + gitlab-qa (8.5.0) activesupport (~> 6.1) gitlab (~> 4.18.0) http (~> 5.0) @@ -126,9 +115,9 @@ GEM rainbow (~> 3.0.0) table_print (= 1.5.7) zeitwerk (~> 2.4) - google-apis-compute_v1 (0.21.0) - google-apis-core (>= 0.4, < 2.a) - google-apis-core (0.4.1) + google-apis-compute_v1 (0.51.0) + google-apis-core (>= 0.7.2, < 2.a) + google-apis-core (0.9.0) addressable (~> 2.5, >= 2.5.1) googleauth (>= 0.16.2, < 2.a) httpclient (>= 2.8.1, < 3.a) @@ -137,22 +126,22 @@ GEM retriable (>= 2.0, < 4.a) rexml webrick - google-apis-dns_v1 (0.16.0) - google-apis-core (>= 0.4, < 2.a) - google-apis-iamcredentials_v1 (0.8.0) - google-apis-core (>= 0.4, < 2.a) - google-apis-monitoring_v3 (0.18.0) - google-apis-core (>= 0.4, < 2.a) - google-apis-pubsub_v1 (0.10.0) - google-apis-core (>= 0.4, < 2.a) - google-apis-sqladmin_v1beta4 (0.21.0) - google-apis-core (>= 0.4, < 2.a) - google-apis-storage_v1 (0.9.0) - google-apis-core (>= 0.4, < 2.a) + google-apis-dns_v1 (0.27.0) + google-apis-core (>= 0.7.2, < 2.a) + google-apis-iamcredentials_v1 (0.14.0) + google-apis-core (>= 0.7.2, < 2.a) + google-apis-monitoring_v3 (0.33.0) + google-apis-core (>= 0.7, < 2.a) + google-apis-pubsub_v1 (0.28.0) + google-apis-core (>= 0.7.2, < 2.a) + google-apis-sqladmin_v1beta4 (0.36.0) + google-apis-core (>= 0.7.2, < 2.a) + google-apis-storage_v1 (0.18.0) + google-apis-core (>= 0.7, < 2.a) google-cloud-env (1.6.0) faraday (>= 0.17.3, < 3.0) - googleauth (1.1.0) - faraday (>= 0.17.3, < 2.0) + googleauth (1.2.0) + faraday (>= 0.17.3, < 3.a) jwt (>= 1.4, < 3.0) memoist (~> 0.16) multi_json (~> 1.11) @@ -175,7 +164,7 @@ GEM concurrent-ruby (~> 1.0) ice_nine (0.11.2) influxdb-client (1.17.0) - jwt (2.3.0) + jwt (2.5.0) knapsack (4.0.0) rake launchy (2.4.3) @@ -197,12 +186,11 @@ GEM minitest (5.16.3) multi_json (1.15.0) multi_xml (0.6.0) - multipart-post (2.1.1) netrc (0.11.0) nokogiri (1.13.8) mini_portile2 (~> 2.8.0) racc (~> 1.4) - octokit (4.25.1) + octokit (5.6.1) faraday (>= 1, < 3) sawyer (~> 0.9) oj (3.13.11) @@ -231,7 +219,7 @@ GEM rainbow (3.0.0) rake (13.0.6) regexp_parser (2.1.1) - representable (3.1.1) + representable (3.2.0) declarative (< 0.1.0) trailblazer-option (>= 0.1.1, < 0.2.0) uber (< 0.2.0) @@ -269,7 +257,7 @@ GEM rspec-core (>= 2, < 4, != 2.12.0) ruby-debug-ide (0.7.2) rake (>= 0.8.1) - ruby2_keywords (0.0.4) + ruby2_keywords (0.0.5) rubyzip (2.3.2) sawyer (0.9.2) addressable (>= 2.3.5) @@ -278,9 +266,9 @@ GEM childprocess (>= 0.5, < 5.0) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2) - signet (0.16.0) + signet (0.17.0) addressable (~> 2.8) - faraday (>= 0.17.3, < 2.0) + faraday (>= 0.17.5, < 3.a) jwt (>= 1.5, < 3.0) multi_json (~> 1.10) slack-notifier (2.4.0) @@ -297,7 +285,7 @@ GEM unf (0.1.4) unf_ext unf_ext (0.0.8.2) - unicode-display_width (2.2.0) + unicode-display_width (2.3.0) unparser (0.4.7) abstract_type (~> 0.0.7) adamantium (~> 0.2.0) @@ -335,12 +323,14 @@ DEPENDENCIES confiner (~> 0.3) deprecation_toolkit (~> 1.5.1) faker (~> 2.19, >= 2.19.0) - fog-google (~> 1.17) + faraday-retry (~> 2.0) + fog-core (= 2.1.0) + fog-google (~> 1.19) gitlab-qa (~> 8) influxdb-client (~> 1.17) knapsack (~> 4.0) nokogiri (~> 1.12) - octokit (~> 4.21) + octokit (~> 5.6.1) parallel (~> 1.19) parallel_tests (~> 2.29) pry-byebug (~> 3.5.1) diff --git a/qa/lib/gitlab/page/group/settings/usage_quotas.rb b/qa/lib/gitlab/page/group/settings/usage_quotas.rb index 2b491188595..9f34f48fee0 100644 --- a/qa/lib/gitlab/page/group/settings/usage_quotas.rb +++ b/qa/lib/gitlab/page/group/settings/usage_quotas.rb @@ -9,7 +9,7 @@ module Gitlab link :pipelines_tab link :storage_tab link :buy_ci_minutes - link :buy_storage + link :purchase_more_storage div :plan_ci_minutes div :additional_ci_minutes span :purchased_usage_total diff --git a/qa/qa/resource/issue.rb b/qa/qa/resource/issue.rb index 1e38de97c1e..40e2e819c99 100644 --- a/qa/qa/resource/issue.rb +++ b/qa/qa/resource/issue.rb @@ -104,6 +104,33 @@ module QA api_post_to(api_comments_path, body: body, confidential: confidential) end + # Issue label events + # + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def label_events(auto_paginate: false, attempts: 0) + events("label", auto_paginate: auto_paginate, attempts: attempts) + end + + # Issue state events + # + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def state_events(auto_paginate: false, attempts: 0) + events("state", auto_paginate: auto_paginate, attempts: attempts) + end + + # Issue milestone events + # + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def milestone_events(auto_paginate: false, attempts: 0) + events("milestone", auto_paginate: auto_paginate, attempts: attempts) + end + protected # Return subset of fields for comparing issues @@ -134,6 +161,23 @@ module QA :created_at ) end + + private + + # Issue events + # + # @param [String] name event name + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def events(name, auto_paginate:, attempts:) + return parse_body(api_get_from("#{api_get_path}/resource_#{name}_events")) unless auto_paginate + + auto_paginated_response( + Runtime::API::Request.new(api_client, "#{api_get_path}/resource_#{name}_events", per_page: '100').url, + attempts: attempts + ) + end end end end diff --git a/qa/qa/resource/merge_request.rb b/qa/qa/resource/merge_request.rb index 0a92553690f..e4314c4dae7 100644 --- a/qa/qa/resource/merge_request.rb +++ b/qa/qa/resource/merge_request.rb @@ -186,6 +186,33 @@ module QA api_post_to(api_comments_path, body: body) end + # Merge request label events + # + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def label_events(auto_paginate: false, attempts: 0) + events("label", auto_paginate: auto_paginate, attempts: attempts) + end + + # Merge request state events + # + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def state_events(auto_paginate: false, attempts: 0) + events("state", auto_paginate: auto_paginate, attempts: attempts) + end + + # Merge request milestone events + # + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def milestone_events(auto_paginate: false, attempts: 0) + events("milestone", auto_paginate: auto_paginate, attempts: attempts) + end + # Return subset of fields for comparing merge requests # # @return [Hash] @@ -239,6 +266,21 @@ module QA def create_target? !(project.initialize_with_readme && target_branch == project.default_branch) && target_new_branch end + + # Merge request events + # + # @param [String] name event name + # @param [Boolean] auto_paginate + # @param [Integer] attempts + # @return [Array<Hash>] + def events(name, auto_paginate:, attempts:) + return parse_body(api_get_from("#{api_get_path}/resource_#{name}_events")) unless auto_paginate + + auto_paginated_response( + Runtime::API::Request.new(api_client, "#{api_get_path}/resource_#{name}_events", per_page: '100').url, + attempts: attempts + ) + end end end end diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 13c6f285259..b188e432679 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -324,8 +324,11 @@ module QA result = parse_body(response) if result[:import_status] == "failed" - Runtime::Logger.error("Import failed: #{result[:import_error]}") - Runtime::Logger.error("Failed relations: #{result[:failed_relations]}") + Runtime::Logger.error(<<~ERR) + Import of project '#{full_path}' failed! + error: '#{result[:import_error]}' + failed relations: '#{result[:failed_relations]}' + ERR end result diff --git a/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb b/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb index e6b60a5b090..58820a7e477 100644 --- a/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb +++ b/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb @@ -1,21 +1,76 @@ # frozen_string_literal: true +require "etc" + # Lifesize project import test executed from https://gitlab.com/gitlab-org/manage/import/import-metrics # rubocop:disable Rails/Pluck module QA RSpec.describe 'Manage', :github, requires_admin: 'creates users', only: { job: 'large-github-import' } do - describe 'Project import' do + describe 'Project import' do # rubocop:disable RSpec/MultipleMemoizedHelpers + let(:github_repo) { ENV['QA_LARGE_IMPORT_REPO'] || 'rspec/rspec-core' } + let(:import_max_duration) { ENV['QA_LARGE_IMPORT_DURATION']&.to_i || 7200 } let(:logger) { Runtime::Logger.logger } let(:differ) { RSpec::Support::Differ.new(color: true) } let(:gitlab_address) { QA::Runtime::Scenario.gitlab_address } let(:dummy_url) { "https://example.com" } + let(:api_request_params) { { auto_paginate: true, attempts: 2 } } let(:created_by_pattern) { /\*Created by: \S+\*\n\n/ } let(:suggestion_pattern) { /suggestion:-\d+\+\d+/ } let(:gh_link_pattern) { %r{https://github.com/#{github_repo}/(issues|pull)} } let(:gl_link_pattern) { %r{#{gitlab_address}/#{imported_project.path_with_namespace}/-/(issues|merge_requests)} } - let(:event_pattern) { %r{(un)?assigned( to)? @\S+|mentioned in (issue|merge request) [!#]\d+|changed title from \*\*.*\*\* to \*\*.*\*\*} } # rubocop:disable Layout/LineLength + # rubocop:disable Lint/MixedRegexpCaptureTypes + let(:event_pattern) do + Regexp.union( + [ + /(?<event>(un)?assigned)( to)? @\S+/, + /(?<event>mentioned) in (issue|merge request) [!#]\d+/, + /(?<event>changed title) from \*\*.*\*\* to \*\*.*\*\*/, + /(?<event>requested review) from @\w+/, + /\*(?<event>Merged) by:/, + /\*\*(Review):\*\*/ + ] + ) + end + # rubocop:enable Lint/MixedRegexpCaptureTypes + + # mapping from gitlab to github names + let(:event_mapping) do + { + "label_add" => "labeled", + "label_remove" => "unlabeled", + "milestone_add" => "milestoned", + "milestone_remove" => "demilestoned", + "assigned" => "assigned", + "unassigned" => "unassigned", + "changed title" => "renamed", + "requested review" => "review_requested", + "Merged" => "merged" + } + end + + # github events that are not migrated or are not correctly mapable in gitlab + let(:unsupported_events) do + [ + "head_ref_deleted", + "head_ref_force_pushed", + "head_ref_restored", + "auto_squash_enabled", + "auto_merge_disabled", + "comment_deleted", + "convert_to_draft", + "ready_for_review", + "subscribed", + "unsubscribed", + "transferred", + # mentions are supported but they can be reported differently on gitlab's side + # for example mention of issue creation in pr will be reported in the issue on gitlab side + # or referenced in github will still create a 'mentioned in' comment in gitlab + "referenced", + "mentioned" + ] + end let(:api_client) { Runtime::API::Client.as_admin } @@ -25,79 +80,105 @@ module QA end end - let(:github_repo) { ENV['QA_LARGE_IMPORT_REPO'] || 'rspec/rspec-core' } - let(:import_max_duration) { ENV['QA_LARGE_IMPORT_DURATION'] ? ENV['QA_LARGE_IMPORT_DURATION'].to_i : 7200 } let(:github_client) do Octokit::Client.new( access_token: ENV['QA_LARGE_IMPORT_GH_TOKEN'] || Runtime::Env.github_access_token, - auto_paginate: true + auto_paginate: true, + middleware: Faraday::RackBuilder.new do |builder| + builder.use(Faraday::Retry::Middleware, exceptions: [Octokit::InternalServerError, Octokit::ServerError]) + end ) end let(:gh_repo) { github_client.repository(github_repo) } let(:gh_branches) do - logger.debug("= Fetching branches =") + logger.info("= Fetching branches =") github_client.branches(github_repo).map(&:name) end let(:gh_commits) do - logger.debug("= Fetching commits =") + logger.info("= Fetching commits =") github_client.commits(github_repo).map(&:sha) end let(:gh_labels) do - logger.debug("= Fetching labels =") + logger.info("= Fetching labels =") github_client.labels(github_repo).map { |label| { name: label.name, color: "##{label.color}" } } end let(:gh_milestones) do - logger.debug("= Fetching milestones =") + logger.info("= Fetching milestones =") github_client .list_milestones(github_repo, state: 'all') .map { |ms| { title: ms.title, description: ms.description } } end - let(:gh_all_issues) do - logger.debug("= Fetching issues and prs =") - github_client.list_issues(github_repo, state: 'all') - end - let(:gh_prs) do gh_all_issues.select(&:pull_request).each_with_object({}) do |pr, hash| - hash[pr.number] = { + id = pr.number + hash[id] = { url: pr.html_url, title: pr.title, body: pr.body || '', - comments: [*gh_pr_comments[pr.html_url], *gh_issue_comments[pr.html_url]].compact + comments: [*gh_pr_comments[id], *gh_issue_comments[id]].compact, + events: gh_pr_events[id].reject { |event| unsupported_events.include?(event) } } end end let(:gh_issues) do gh_all_issues.reject(&:pull_request).each_with_object({}) do |issue, hash| - hash[issue.number] = { + id = issue.number + hash[id] = { url: issue.html_url, title: issue.title, body: issue.body || '', - comments: gh_issue_comments[issue.html_url] + comments: gh_issue_comments[id], + events: gh_issue_events[id].reject { |event| unsupported_events.include?(event) } } end end + let(:gh_all_issues) do + logger.info("= Fetching issues and prs =") + github_client.list_issues(github_repo, state: 'all') + end + + let(:gh_all_events) do + logger.info("- Fetching issue and pr events -") + github_client.repository_issue_events(github_repo).map do |event| + { name: event[:event], **(event[:issue] || {}) } # some events don't have issue object at all + end + end + + let(:gh_issue_events) do + gh_all_events.each_with_object(Hash.new { |h, k| h[k] = [] }) do |event, hash| + next if event[:pull_request] || !event[:number] + + hash[event[:number]] << event[:name] + end + end + + let(:gh_pr_events) do + gh_all_events.each_with_object(Hash.new { |h, k| h[k] = [] }) do |event, hash| + next unless event[:pull_request] + + hash[event[:number]] << event[:name] + end + end + let(:gh_issue_comments) do - logger.debug("= Fetching issue comments =") + logger.info("- Fetching issue comments -") github_client.issues_comments(github_repo).each_with_object(Hash.new { |h, k| h[k] = [] }) do |c, hash| - # use base html url as key - hash[c.html_url.gsub(/\#\S+/, "")] << c.body&.gsub(gh_link_pattern, dummy_url) + hash[id_from_url(c.html_url)] << c.body&.gsub(gh_link_pattern, dummy_url) end end let(:gh_pr_comments) do - logger.debug("= Fetching pr comments =") + logger.info("- Fetching pr comments -") github_client.pull_requests_comments(github_repo).each_with_object(Hash.new { |h, k| h[k] = [] }) do |c, hash| - # use base html url as key - hash[c.html_url.gsub(/\#\S+/, "")] << c.body + hash[id_from_url(c.html_url)] << c.body # some suggestions can contain extra whitespaces which gitlab will remove &.gsub(/suggestion\s+\r/, "suggestion\r") &.gsub(gh_link_pattern, dummy_url) @@ -115,7 +196,6 @@ module QA end end - # rubocop:disable RSpec/InstanceVariable after do |example| next unless defined?(@import_time) @@ -164,7 +244,6 @@ module QA } ) end - # rubocop:enable RSpec/InstanceVariable it( 'imports large Github repo via api', @@ -172,17 +251,18 @@ module QA ) do start = Time.now - # import the project and log gitlab path - logger.info("== Importing project '#{github_repo}' in to '#{imported_project.reload!.full_path}' ==") # fetch all objects right after import has started fetch_github_objects + # import the project and log gitlab path + logger.info("== Importing project '#{github_repo}' in to '#{imported_project.reload!.full_path}' ==") + import_status = lambda do imported_project.project_import_status.yield_self do |status| @stats = status.dig(:stats, :imported) # fail fast if import explicitly failed - raise "Import of '#{imported_project.name}' failed!" if status[:import_status] == 'failed' + raise "Import of '#{imported_project.full_path}' failed!" if status[:import_status] == 'failed' status[:import_status] end @@ -294,7 +374,7 @@ module QA actual.each_with_object([]) do |(key, actual_item), missing_comments| expected_item = expected[key] title = actual_item[:title] - msg = "expected #{type} with title '#{title}' to have" + msg = "expected #{type} with iid '#{key}' to have" # Print title in the error message to see which object is missing # @@ -320,6 +400,14 @@ module QA expect(expected_comments.length).to eq(actual_comments.length), comment_count_msg expect(expected_comments).to match_array(actual_comments) + expected_events = expected_item[:events] + actual_events = actual_item[:events] + event_count_msg = <<~MSG + #{msg} same amount of events. Gitlab: #{expected_events.length}, Github: #{actual_events.length} + MSG + expect(expected_events.length).to eq(actual_events.length), event_count_msg + expect(expected_events).to match_array(actual_events) + # Save missing comments # comment_diff = actual_comments - expected_comments @@ -380,26 +468,27 @@ module QA def mrs @mrs ||= begin logger.debug("= Fetching merge requests =") - imported_mrs = imported_project.merge_requests(auto_paginate: true, attempts: 2) + imported_mrs = imported_project.merge_requests(**api_request_params) logger.debug("= Fetching merge request comments =") - Parallel.map(imported_mrs, in_threads: 4) do |mr| + Parallel.map(imported_mrs, in_threads: Etc.nprocessors) do |mr| resource = Resource::MergeRequest.init do |resource| resource.project = imported_project resource.iid = mr[:iid] resource.api_client = api_client end - logger.debug("Fetching comments for mr '#{mr[:title]}'") - comments = resource - .comments(auto_paginate: true, attempts: 2) - .reject { |c| c[:system] || c[:body].match?(/^(\*\*Review:\*\*)|(\*Merged by:).*/) } + logger.debug("Fetching events and comments for mr '!#{mr[:iid]}'") + comments = resource.comments(**api_request_params) + label_events = resource.label_events(**api_request_params) + state_events = resource.state_events(**api_request_params) + milestone_events = resource.milestone_events(**api_request_params) [mr[:iid], { url: mr[:web_url], title: mr[:title], body: sanitize_description(mr[:description]) || '', - events: events(comments), + events: events(comments, label_events, state_events, milestone_events), comments: non_event_comments(comments) }] end.to_h @@ -412,48 +501,59 @@ module QA def gl_issues @gl_issues ||= begin logger.debug("= Fetching issues =") - imported_issues = imported_project.issues(auto_paginate: true, attempts: 2) + imported_issues = imported_project.issues(**api_request_params) logger.debug("= Fetching issue comments =") - Parallel.map(imported_issues, in_threads: 4) do |issue| + Parallel.map(imported_issues, in_threads: Etc.nprocessors) do |issue| resource = Resource::Issue.init do |issue_resource| issue_resource.project = imported_project issue_resource.iid = issue[:iid] issue_resource.api_client = api_client end - logger.debug("Fetching comments for issue '#{issue[:title]}'") - comments = resource.comments(auto_paginate: true, attempts: 2) + logger.debug("Fetching events and comments for issue '!#{issue[:iid]}'") + comments = resource.comments(**api_request_params) + label_events = resource.label_events(**api_request_params) + state_events = resource.state_events(**api_request_params) + milestone_events = resource.milestone_events(**api_request_params) [issue[:iid], { url: issue[:web_url], title: issue[:title], body: sanitize_description(issue[:description]) || '', - events: events(comments), + events: events(comments, label_events, state_events, milestone_events), comments: non_event_comments(comments) }] end.to_h end end - # Fetch comments without events + # Filter out event comments # # @param [Array] comments # @return [Array] def non_event_comments(comments) comments - .reject { |c| c[:body].match?(event_pattern) } + .reject { |c| c[:system] || c[:body].match?(event_pattern) } .map { |c| sanitize_comment(c[:body]) } end # Events # # @param [Array] comments + # @param [Array] label_events + # @param [Array] state_events + # @param [Array] milestone_events # @return [Array] - def events(comments) - comments - .select { |c| c[:body].match?(event_pattern) } - .map { |c| c[:body] } + def events(comments, label_events, state_events, milestone_events) + mapped_label_events = label_events.map { |event| event_mapping["label_#{event[:action]}"] } + mapped_milestone_events = milestone_events.map { |event| event_mapping["milestone_#{event[:action]}"] } + mapped_state_event = state_events.map { |event| event[:state] } + mapped_comment_events = comments.map do |c| + event_mapping[c[:body].match(event_pattern)&.named_captures&.fetch("event", nil)] + end + + [*mapped_label_events, *mapped_milestone_events, *mapped_state_event, *mapped_comment_events].compact end # Normalize comments and make them directly comparable @@ -489,6 +589,16 @@ module QA def save_json(name, json) File.open("tmp/#{name}.json", "w") { |file| file.write(JSON.pretty_generate(json)) } end + + # Extract id number from web url of issue or pull request + # + # Some endpoints don't return object id as separate parameter so web url can be used as a workaround + # + # @param [String] url + # @return [Integer] + def id_from_url(url) + url.match(%r{(?<type>issues|pull)/(?<id>\d+)})&.named_captures&.fetch(:id, nil).to_i + end end end end diff --git a/qa/qa/specs/features/sanity/framework_spec.rb b/qa/qa/specs/features/sanity/framework_spec.rb index feec56478c0..fa34f525a85 100644 --- a/qa/qa/specs/features/sanity/framework_spec.rb +++ b/qa/qa/specs/features/sanity/framework_spec.rb @@ -6,7 +6,7 @@ module QA it 'succeeds' do Runtime::Browser.visit(:gitlab, Page::Main::Login) - expect(page).to have_text('A complete DevOps platform') + expect(page).to have_text('GitLab') end end diff --git a/spec/controllers/concerns/boards_responses_spec.rb b/spec/controllers/concerns/boards_responses_spec.rb deleted file mode 100644 index 553a547d42c..00000000000 --- a/spec/controllers/concerns/boards_responses_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BoardsResponses do - let(:controller_class) do - Class.new do - include BoardsResponses - end - end - - subject(:controller) { controller_class.new } - - describe '#serialize_as_json' do - let!(:board) { create(:board) } - - it 'serializes properly' do - expected = { "id" => board.id } - - expect(subject.serialize_as_json(board)).to include(expected) - end - end -end diff --git a/spec/controllers/groups/boards_controller_spec.rb b/spec/controllers/groups/boards_controller_spec.rb index 6201cddecb0..4e441f86765 100644 --- a/spec/controllers/groups/boards_controller_spec.rb +++ b/spec/controllers/groups/boards_controller_spec.rb @@ -3,11 +3,14 @@ require 'spec_helper' RSpec.describe Groups::BoardsController do - let(:group) { create(:group) } - let(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } - before do + before_all do group.add_maintainer(user) + end + + before do sign_in(user) end @@ -57,46 +60,17 @@ RSpec.describe Groups::BoardsController do end end - context 'when format is JSON' do - it 'return an array with one group board' do - create(:board, group: group) - - expect(Boards::VisitsFinder).not_to receive(:new) - - list_boards format: :json - - expect(response).to match_response_schema('boards') - expect(json_response.length).to eq 1 - end - - context 'with unauthorized user' do - before do - expect(Ability).to receive(:allowed?).with(user, :log_in, :global).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_issue_board, group).and_return(false) - end - - it 'returns a not found 404 response' do - list_boards format: :json - - expect(response).to have_gitlab_http_status(:not_found) - expect(response.media_type).to eq 'application/json' - end - end - end - it_behaves_like 'disabled when using an external authorization service' do subject { list_boards } end - def list_boards(format: :html) - get :index, params: { group_id: group }, format: format + def list_boards + get :index, params: { group_id: group } end end describe 'GET show' do - let!(:board) { create(:board, group: group) } + let_it_be(:board) { create(:board, group: group) } context 'when format is HTML' do it 'renders template' do @@ -123,12 +97,12 @@ RSpec.describe Groups::BoardsController do end context 'when user is signed out' do - let(:group) { create(:group, :public) } + let(:public_board) { create(:board, group: create(:group, :public)) } it 'does not save visit' do sign_out(user) - expect { read_board board: board }.to change(BoardGroupRecentVisit, :count).by(0) + expect { read_board board: public_board }.to change(BoardGroupRecentVisit, :count).by(0) expect(response).to render_template :show expect(response.media_type).to eq 'text/html' @@ -136,37 +110,11 @@ RSpec.describe Groups::BoardsController do end end - context 'when format is JSON' do - it 'returns project board' do - expect(Boards::Visits::CreateService).not_to receive(:new) - - read_board board: board, format: :json - - expect(response).to match_response_schema('board') - end - - context 'with unauthorized user' do - before do - expect(Ability).to receive(:allowed?).with(user, :log_in, :global).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false) - end - - it 'returns a not found 404 response' do - read_board board: board, format: :json - - expect(response).to have_gitlab_http_status(:not_found) - expect(response.media_type).to eq 'application/json' - end - end - end - context 'when board does not belong to group' do it 'returns a not found 404 response' do another_board = create(:board) - read_board board: another_board + get :show, params: { group_id: group, id: another_board.to_param } expect(response).to have_gitlab_http_status(:not_found) end @@ -176,12 +124,8 @@ RSpec.describe Groups::BoardsController do subject { read_board board: board } end - def read_board(board:, format: :html) - get :show, params: { - group_id: group, - id: board.to_param - }, - format: format + def read_board(board:) + get :show, params: { group_id: board.group, id: board.to_param } end end end diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index cde3a8d4761..7a2d9acadae 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -3,11 +3,14 @@ require 'spec_helper' RSpec.describe Projects::BoardsController do - let(:project) { create(:project) } - let(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } - before do + before_all do project.add_maintainer(user) + end + + before do sign_in(user) end @@ -22,71 +25,63 @@ RSpec.describe Projects::BoardsController do expect(assigns(:boards_endpoint)).to eq project_boards_path(project) end - context 'when format is HTML' do - it 'renders template' do - list_boards - - expect(response).to render_template :index - expect(response.media_type).to eq 'text/html' - end + it 'renders template' do + list_boards - context 'with unauthorized user' do - before do - expect(Ability).to receive(:allowed?).with(user, :log_in, :global).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_issue_board, project).and_return(false) - end + expect(response).to render_template :index + expect(response.media_type).to eq 'text/html' + end - it 'returns a not found 404 response' do - list_boards + context 'when there are recently visited boards' do + let_it_be(:boards) { create_list(:board, 3, resource_parent: project) } - expect(response).to have_gitlab_http_status(:not_found) - expect(response.media_type).to eq 'text/html' - end + before_all do + visit_board(boards[2], Time.current + 1.minute) + visit_board(boards[0], Time.current + 2.minutes) + visit_board(boards[1], Time.current + 5.minutes) end - context 'when user is signed out' do - let(:project) { create(:project, :public) } - - it 'renders template' do - sign_out(user) - - board = create(:board, project: project) - create(:board_project_recent_visit, project: board.project, board: board, user: user) + it 'redirects to latest visited board' do + list_boards - list_boards + expect(response).to redirect_to( + namespace_project_board_path(namespace_id: project.namespace, project_id: project, id: boards[1].id) + ) + end - expect(response).to render_template :index - expect(response.media_type).to eq 'text/html' - end + def visit_board(board, time) + create(:board_project_recent_visit, project: project, board: board, user: user, updated_at: time) end end - context 'when format is JSON' do - it 'returns a list of project boards' do - create_list(:board, 2, project: project) - - expect(Boards::VisitsFinder).not_to receive(:new) + context 'with unauthorized user' do + before do + expect(Ability).to receive(:allowed?).with(user, :log_in, :global).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_issue_board, project).and_return(false) + end - list_boards format: :json + it 'returns a not found 404 response' do + list_boards - expect(response).to match_response_schema('boards') - expect(json_response.length).to eq 2 + expect(response).to have_gitlab_http_status(:not_found) + expect(response.media_type).to eq 'text/html' end + end - context 'with unauthorized user' do - before do - expect(Ability).to receive(:allowed?).with(user, :log_in, :global).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_issue_board, project).and_return(false) - end + context 'when user is signed out' do + let(:project) { create(:project, :public) } - it 'returns a not found 404 response' do - list_boards format: :json + it 'renders template' do + sign_out(user) - expect(response).to have_gitlab_http_status(:not_found) - expect(response.media_type).to eq 'application/json' - end + board = create(:board, project: project) + create(:board_project_recent_visit, project: board.project, board: board, user: user) + + list_boards + + expect(response).to render_template :index + expect(response.media_type).to eq 'text/html' end end @@ -104,17 +99,16 @@ RSpec.describe Projects::BoardsController do subject { list_boards } end - def list_boards(format: :html) + def list_boards get :index, params: { namespace_id: project.namespace, project_id: project - }, - format: format + } end end describe 'GET show' do - let!(:board) { create(:board, project: project) } + let_it_be(:board) { create(:board, project: project) } it 'sets boards_endpoint instance variable to a boards path' do read_board board: board @@ -146,12 +140,12 @@ RSpec.describe Projects::BoardsController do end context 'when user is signed out' do - let(:project) { create(:project, :public) } + let(:public_board) { create(:board, project: create(:project, :public)) } it 'does not save visit' do sign_out(user) - expect { read_board board: board }.to change(BoardProjectRecentVisit, :count).by(0) + expect { read_board board: public_board }.to change(BoardProjectRecentVisit, :count).by(0) expect(response).to render_template :show expect(response.media_type).to eq 'text/html' @@ -159,48 +153,18 @@ RSpec.describe Projects::BoardsController do end end - context 'when format is JSON' do - it 'returns project board' do - expect(Boards::Visits::CreateService).not_to receive(:new) - - read_board board: board, format: :json - - expect(response).to match_response_schema('board') - end - - context 'with unauthorized user' do - before do - expect(Ability).to receive(:allowed?).with(user, :log_in, :global).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) - allow(Ability).to receive(:allowed?).with(user, :read_issue_board, project).and_return(false) - end - - it 'returns a not found 404 response' do - read_board board: board, format: :json - - expect(response).to have_gitlab_http_status(:not_found) - expect(response.media_type).to eq 'application/json' - end - end - end - context 'when board does not belong to project' do it 'returns a not found 404 response' do another_board = create(:board) - read_board board: another_board + get :show, params: { namespace_id: project.namespace, project_id: project, id: another_board.to_param } expect(response).to have_gitlab_http_status(:not_found) end end - def read_board(board:, format: :html) - get :show, params: { - namespace_id: project.namespace, - project_id: project, - id: board.to_param - }, - format: format + def read_board(board:) + get :show, params: { namespace_id: board.project.namespace, project_id: board.project, id: board.to_param } end end end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index 899aa259f8c..b29bbef0c40 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -63,9 +63,15 @@ RSpec.describe Projects::PagesDomainsController do describe 'POST create' do it "creates a new pages domain" do - expect do - post(:create, params: request_params.merge(pages_domain: pages_domain_params)) - end.to change { PagesDomain.count }.by(1) + expect { post(:create, params: request_params.merge(pages_domain: pages_domain_params)) } + .to change { PagesDomain.count }.by(1) + .and publish_event(PagesDomains::PagesDomainCreatedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: pages_domain_params[:domain] + ) created_domain = PagesDomain.reorder(:id).last @@ -213,9 +219,15 @@ RSpec.describe Projects::PagesDomainsController do describe 'DELETE destroy' do it "deletes the pages domain" do - expect do - delete(:destroy, params: request_params.merge(id: pages_domain.domain)) - end.to change { PagesDomain.count }.by(-1) + expect { delete(:destroy, params: request_params.merge(id: pages_domain.domain)) } + .to change(PagesDomain, :count).by(-1) + .and publish_event(PagesDomains::PagesDomainDeletedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: pages_domain.domain + ) expect(response).to redirect_to(project_pages_path(project)) end diff --git a/spec/fixtures/api/schemas/board.json b/spec/fixtures/api/schemas/board.json deleted file mode 100644 index 7c146647948..00000000000 --- a/spec/fixtures/api/schemas/board.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "type": "object", - "required" : [ - "id" - ], - "properties" : { - "id": { "type": "integer" }, - "name": { "type": "string" } - } -} diff --git a/spec/fixtures/api/schemas/boards.json b/spec/fixtures/api/schemas/boards.json deleted file mode 100644 index 117564ef77a..00000000000 --- a/spec/fixtures/api/schemas/boards.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "type": "array", - "items": { "$ref": "board.json" } -} diff --git a/spec/fixtures/api/schemas/current-board.json b/spec/fixtures/api/schemas/current-board.json deleted file mode 100644 index 2ddc038e908..00000000000 --- a/spec/fixtures/api/schemas/current-board.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "type": "object", - "allOf": [ - { "$ref": "board.json" }, - { - "required" : [ - "id", - "name" - ], - "properties": { - "id": { "type": "integer" }, - "name": { "type": "string" } - } - } - ] -} diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 346e43e5a72..638e4713ddc 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -13,7 +13,7 @@ import * as diffActions from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; import * as utils from '~/diffs/store/utils'; import * as treeWorkerUtils from '~/diffs/utils/tree_worker_utils'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import * as commonUtils from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; @@ -54,7 +54,7 @@ describe('DiffsStoreActions', () => { ['requestAnimationFrame', 'requestIdleCallback'].forEach((method) => { global[method] = originalMethods[method]; }); - createFlash.mockClear(); + createAlert.mockClear(); mock.restore(); }); @@ -254,8 +254,8 @@ describe('DiffsStoreActions', () => { mock.onGet(endpointCoverage).reply(400); await testAction(diffActions.fetchCoverageFiles, {}, { endpointCoverage }, [], []); - expect(createFlash).toHaveBeenCalledTimes(1); - expect(createFlash).toHaveBeenCalledWith({ + expect(createAlert).toHaveBeenCalledTimes(1); + expect(createAlert).toHaveBeenCalledWith({ message: expect.stringMatching('Something went wrong'), }); }); diff --git a/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js b/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js index fe20c23e4d7..1ff351b6554 100644 --- a/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js +++ b/spec/frontend/editor/source_editor_markdown_livepreview_ext_spec.js @@ -12,7 +12,7 @@ import { } from '~/editor/constants'; import { EditorMarkdownPreviewExtension } from '~/editor/extensions/source_editor_markdown_livepreview_ext'; import SourceEditor from '~/editor/source_editor'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import syntaxHighlight from '~/syntax_highlight'; import { spyOnApi } from './helpers'; @@ -279,7 +279,7 @@ describe('Markdown Live Preview Extension for Source Editor', () => { mockAxios.onPost().reply(500); await fetchPreview(); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); }); diff --git a/spec/frontend/environments/delete_environment_modal_spec.js b/spec/frontend/environments/delete_environment_modal_spec.js index 48e4f661c1d..cc18bf754eb 100644 --- a/spec/frontend/environments/delete_environment_modal_spec.js +++ b/spec/frontend/environments/delete_environment_modal_spec.js @@ -6,7 +6,7 @@ import { s__, sprintf } from '~/locale'; import DeleteEnvironmentModal from '~/environments/components/delete_environment_modal.vue'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { resolvedEnvironment } from './graphql/mock_data'; jest.mock('~/flash'); @@ -57,7 +57,7 @@ describe('~/environments/components/delete_environment_modal.vue', () => { await nextTick(); - expect(createFlash).not.toHaveBeenCalled(); + expect(createAlert).not.toHaveBeenCalled(); expect(deleteResolver).toHaveBeenCalledWith( expect.anything(), @@ -76,7 +76,7 @@ describe('~/environments/components/delete_environment_modal.vue', () => { await waitForPromises(); - expect(createFlash).toHaveBeenCalledWith( + expect(createAlert).toHaveBeenCalledWith( expect.objectContaining({ message: s__( 'Environments|An error occurred while deleting the environment. Check if the environment stopped; if not, stop it and try again.', diff --git a/spec/frontend/environments/edit_environment_spec.js b/spec/frontend/environments/edit_environment_spec.js index 0f2d6e95bf0..5ea23af4c16 100644 --- a/spec/frontend/environments/edit_environment_spec.js +++ b/spec/frontend/environments/edit_environment_spec.js @@ -3,7 +3,7 @@ import MockAdapter from 'axios-mock-adapter'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import EditEnvironment from '~/environments/components/edit_environment.vue'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { visitUrl } from '~/lib/utils/url_utility'; @@ -85,7 +85,7 @@ describe('~/environments/components/edit.vue', () => { await submitForm(expected, [400, { message: ['uh oh!'] }]); - expect(createFlash).toHaveBeenCalledWith({ message: 'uh oh!' }); + expect(createAlert).toHaveBeenCalledWith({ message: 'uh oh!' }); expect(showsLoading()).toBe(false); }); diff --git a/spec/frontend/environments/new_environment_spec.js b/spec/frontend/environments/new_environment_spec.js index 2405cb82eac..6dd4eea7437 100644 --- a/spec/frontend/environments/new_environment_spec.js +++ b/spec/frontend/environments/new_environment_spec.js @@ -3,7 +3,7 @@ import MockAdapter from 'axios-mock-adapter'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import NewEnvironment from '~/environments/components/new_environment.vue'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { visitUrl } from '~/lib/utils/url_utility'; @@ -94,7 +94,7 @@ describe('~/environments/components/new.vue', () => { await submitForm(expected, [400, { message: ['name taken'] }]); - expect(createFlash).toHaveBeenCalledWith({ message: 'name taken' }); + expect(createAlert).toHaveBeenCalledWith({ message: 'name taken' }); expect(showsLoading()).toBe(false); }); }); diff --git a/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js b/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js new file mode 100644 index 00000000000..155fc5342ca --- /dev/null +++ b/spec/frontend/projects/settings/branch_rules/components/view/mock_data.js @@ -0,0 +1,54 @@ +const usersMock = [ + { + username: 'usr1', + webUrl: 'http://test.test/usr1', + name: 'User 1', + avatarUrl: 'http://test.test/avt1.png', + }, + { + username: 'usr2', + webUrl: 'http://test.test/usr2', + name: 'User 2', + avatarUrl: 'http://test.test/avt2.png', + }, + { + username: 'usr3', + webUrl: 'http://test.test/usr3', + name: 'User 3', + avatarUrl: 'http://test.test/avt3.png', + }, + { + username: 'usr4', + webUrl: 'http://test.test/usr4', + name: 'User 4', + avatarUrl: 'http://test.test/avt4.png', + }, + { + username: 'usr5', + webUrl: 'http://test.test/usr5', + name: 'User 5', + avatarUrl: 'http://test.test/avt5.png', + }, +]; + +const accessLevelsMock = [ + { accessLevelDescription: 'Administrator' }, + { accessLevelDescription: 'Maintainer' }, +]; + +const groupsMock = [{ name: 'test_group_1' }, { name: 'test_group_2' }]; + +export const protectionPropsMock = { + header: 'Test protection', + headerLinkTitle: 'Test link title', + headerLinkHref: 'Test link href', + roles: accessLevelsMock, + users: usersMock, + groups: groupsMock, +}; + +export const protectionRowPropsMock = { + title: 'Test title', + users: usersMock, + accessLevels: accessLevelsMock, +}; diff --git a/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js b/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js new file mode 100644 index 00000000000..a1a2016fa17 --- /dev/null +++ b/spec/frontend/projects/settings/branch_rules/components/view/protection_row_spec.js @@ -0,0 +1,63 @@ +import { GlAvatarsInline, GlAvatar, GlAvatarLink } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import ProtectionRow, { + MAX_VISIBLE_AVATARS, + AVATAR_SIZE, +} from '~/projects/settings/branch_rules/components/view/protection_row.vue'; +import { protectionRowPropsMock } from './mock_data'; + +describe('Branch rule protection row', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMountExtended(ProtectionRow, { + propsData: protectionRowPropsMock, + stubs: { GlAvatarsInline }, + }); + }; + + beforeEach(() => createComponent()); + + afterEach(() => wrapper.destroy()); + + const findTitle = () => wrapper.findByText(protectionRowPropsMock.title); + const findAvatarsInline = () => wrapper.findComponent(GlAvatarsInline); + const findAvatarLinks = () => wrapper.findAllComponents(GlAvatarLink); + const findAvatars = () => wrapper.findAllComponents(GlAvatar); + const findAccessLevels = () => wrapper.findAllByTestId('access-level'); + + it('renders a title', () => { + expect(findTitle().exists()).toBe(true); + }); + + it('renders an avatars-inline component', () => { + expect(findAvatarsInline().props('avatars')).toMatchObject(protectionRowPropsMock.users); + expect(findAvatarsInline().props('badgeSrOnlyText')).toBe('1 additional user'); + }); + + it('renders avatar-link components', () => { + expect(findAvatarLinks().length).toBe(MAX_VISIBLE_AVATARS); + + expect(findAvatarLinks().at(1).attributes('href')).toBe(protectionRowPropsMock.users[1].webUrl); + expect(findAvatarLinks().at(1).attributes('title')).toBe(protectionRowPropsMock.users[1].name); + }); + + it('renders avatar components', () => { + expect(findAvatars().length).toBe(MAX_VISIBLE_AVATARS); + + expect(findAvatars().at(1).attributes('src')).toBe(protectionRowPropsMock.users[1].avatarUrl); + expect(findAvatars().at(1).attributes('label')).toBe(protectionRowPropsMock.users[1].name); + expect(findAvatars().at(1).props('size')).toBe(AVATAR_SIZE); + }); + + it('renders access level descriptions', () => { + expect(findAccessLevels().length).toBe(protectionRowPropsMock.accessLevels.length); + + expect(findAccessLevels().at(0).text()).toBe( + protectionRowPropsMock.accessLevels[0].accessLevelDescription, + ); + expect(findAccessLevels().at(1).text()).toBe( + protectionRowPropsMock.accessLevels[1].accessLevelDescription, + ); + }); +}); diff --git a/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js b/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js new file mode 100644 index 00000000000..91d16fd86a6 --- /dev/null +++ b/spec/frontend/projects/settings/branch_rules/components/view/protection_spec.js @@ -0,0 +1,59 @@ +import { GlCard, GlLink } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import Protection, { i18n } from '~/projects/settings/branch_rules/components/view/protection.vue'; +import ProtectionRow from '~/projects/settings/branch_rules/components/view/protection_row.vue'; +import { protectionPropsMock } from './mock_data'; + +describe('Branch rule protection', () => { + let wrapper; + + const createComponent = () => { + wrapper = shallowMountExtended(Protection, { + propsData: protectionPropsMock, + stubs: { GlCard }, + }); + }; + + beforeEach(() => createComponent()); + + afterEach(() => wrapper.destroy()); + + const findCard = () => wrapper.findComponent(GlCard); + const findHeader = () => wrapper.findByText(protectionPropsMock.header); + const findLink = () => wrapper.findComponent(GlLink); + const findProtectionRows = () => wrapper.findAllComponents(ProtectionRow); + + it('renders a card component', () => { + expect(findCard().exists()).toBe(true); + }); + + it('renders a header with a link', () => { + expect(findHeader().exists()).toBe(true); + expect(findLink().text()).toBe(protectionPropsMock.headerLinkTitle); + expect(findLink().attributes('href')).toBe(protectionPropsMock.headerLinkHref); + }); + + it('renders a protection row for roles', () => { + expect(findProtectionRows().at(0).props()).toMatchObject({ + accessLevels: protectionPropsMock.roles, + showDivider: false, + title: i18n.rolesTitle, + }); + }); + + it('renders a protection row for users', () => { + expect(findProtectionRows().at(1).props()).toMatchObject({ + users: protectionPropsMock.users, + showDivider: true, + title: i18n.usersTitle, + }); + }); + + it('renders a protection row for groups', () => { + expect(findProtectionRows().at(2).props()).toMatchObject({ + accessLevels: protectionPropsMock.groups, + showDivider: true, + title: i18n.groupsTitle, + }); + }); +}); diff --git a/spec/frontend/vue_merge_request_widget/components/states/__snapshots__/mr_widget_auto_merge_enabled_spec.js.snap b/spec/frontend/vue_merge_request_widget/components/states/__snapshots__/mr_widget_auto_merge_enabled_spec.js.snap index 635ef0f6b0d..519475f8953 100644 --- a/spec/frontend/vue_merge_request_widget/components/states/__snapshots__/mr_widget_auto_merge_enabled_spec.js.snap +++ b/spec/frontend/vue_merge_request_widget/components/states/__snapshots__/mr_widget_auto_merge_enabled_spec.js.snap @@ -72,7 +72,9 @@ exports[`MRWidgetAutoMergeEnabled when graphql is disabled template should have <div class="gl-display-flex gl-md-display-block gl-font-size-0 gl-ml-auto" > - <div> + <div + class="gl-display-flex" + > <div class="dropdown b-dropdown gl-new-dropdown gl-display-block gl-md-display-none! btn-group" lazy="" @@ -246,7 +248,9 @@ exports[`MRWidgetAutoMergeEnabled when graphql is enabled template should have c <div class="gl-display-flex gl-md-display-block gl-font-size-0 gl-ml-auto" > - <div> + <div + class="gl-display-flex" + > <div class="dropdown b-dropdown gl-new-dropdown gl-display-block gl-md-display-none! btn-group" lazy="" diff --git a/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js b/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js index a8912405fa8..d3e17065d89 100644 --- a/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js +++ b/spec/frontend/vue_merge_request_widget/deployment/deployment_actions_spec.js @@ -11,6 +11,7 @@ import { REDEPLOYING, STOPPING, } from '~/vue_merge_request_widget/components/deployment/constants'; +import eventHub from '~/vue_merge_request_widget/event_hub'; import DeploymentActions from '~/vue_merge_request_widget/components/deployment/deployment_actions.vue'; import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; import { @@ -192,6 +193,7 @@ describe('DeploymentAction component', () => { describe('it should call the executeAction method', () => { beforeEach(async () => { jest.spyOn(wrapper.vm, 'executeAction').mockImplementation(); + jest.spyOn(eventHub, '$emit'); await waitForPromises(); @@ -206,11 +208,16 @@ describe('DeploymentAction component', () => { actionButtonMocks[configConst], ); }); + + it('emits the FetchDeployments event', () => { + expect(eventHub.$emit).toHaveBeenCalledWith('FetchDeployments'); + }); }); describe('when executeInlineAction errors', () => { beforeEach(async () => { executeActionSpy.mockRejectedValueOnce(); + jest.spyOn(eventHub, '$emit'); await waitForPromises(); @@ -224,6 +231,10 @@ describe('DeploymentAction component', () => { message: actionButtonMocks[configConst].errorMessage, }); }); + + it('emits the FetchDeployments event', () => { + expect(eventHub.$emit).toHaveBeenCalledWith('FetchDeployments'); + }); }); }); }); diff --git a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js index e8233d5b2eb..30e299dea9a 100644 --- a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js @@ -368,12 +368,13 @@ describe('MrWidgetOptions', () => { describe('bindEventHubListeners', () => { it.each` - event | method | methodArgs - ${'MRWidgetUpdateRequested'} | ${'checkStatus'} | ${(x) => [x]} - ${'MRWidgetRebaseSuccess'} | ${'checkStatus'} | ${(x) => [x, true]} - ${'FetchActionsContent'} | ${'fetchActionsContent'} | ${() => []} - ${'EnablePolling'} | ${'resumePolling'} | ${() => []} - ${'DisablePolling'} | ${'stopPolling'} | ${() => []} + event | method | methodArgs + ${'MRWidgetUpdateRequested'} | ${'checkStatus'} | ${(x) => [x]} + ${'MRWidgetRebaseSuccess'} | ${'checkStatus'} | ${(x) => [x, true]} + ${'FetchActionsContent'} | ${'fetchActionsContent'} | ${() => []} + ${'EnablePolling'} | ${'resumePolling'} | ${() => []} + ${'DisablePolling'} | ${'stopPolling'} | ${() => []} + ${'FetchDeployments'} | ${'fetchPreMergeDeployments'} | ${() => []} `('should bind to $event', ({ event, method, methodArgs }) => { jest.spyOn(wrapper.vm, method).mockImplementation(); diff --git a/spec/frontend/vue_shared/components/metric_images/store/actions_spec.js b/spec/frontend/vue_shared/components/metric_images/store/actions_spec.js index 518cf354675..537367940e0 100644 --- a/spec/frontend/vue_shared/components/metric_images/store/actions_spec.js +++ b/spec/frontend/vue_shared/components/metric_images/store/actions_spec.js @@ -4,7 +4,7 @@ import actionsFactory from '~/vue_shared/components/metric_images/store/actions' import * as types from '~/vue_shared/components/metric_images/store/mutation_types'; import createStore from '~/vue_shared/components/metric_images/store'; import testAction from 'helpers/vuex_action_helper'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { fileList, initialData } from '../mock_data'; @@ -35,7 +35,7 @@ describe('Metrics tab store actions', () => { }); afterEach(() => { - createFlash.mockClear(); + createAlert.mockClear(); }); describe('fetching metric images', () => { @@ -61,7 +61,7 @@ describe('Metrics tab store actions', () => { [{ type: types.REQUEST_METRIC_IMAGES }, { type: types.RECEIVE_METRIC_IMAGES_ERROR }], [], ); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); }); @@ -98,7 +98,7 @@ describe('Metrics tab store actions', () => { [{ type: types.REQUEST_METRIC_UPLOAD }, { type: types.RECEIVE_METRIC_UPLOAD_ERROR }], [], ); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); }); @@ -129,7 +129,7 @@ describe('Metrics tab store actions', () => { [{ type: types.REQUEST_METRIC_UPLOAD }, { type: types.RECEIVE_METRIC_UPLOAD_ERROR }], [], ); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); }); diff --git a/spec/frontend/vue_shared/components/security_reports/artifact_downloads/merge_request_artifact_download_spec.js b/spec/frontend/vue_shared/components/security_reports/artifact_downloads/merge_request_artifact_download_spec.js index c5672bc28cc..09b0b3d43ad 100644 --- a/spec/frontend/vue_shared/components/security_reports/artifact_downloads/merge_request_artifact_download_spec.js +++ b/spec/frontend/vue_shared/components/security_reports/artifact_downloads/merge_request_artifact_download_spec.js @@ -6,7 +6,7 @@ import { expectedDownloadDropdownPropsWithTitle, securityReportMergeRequestDownloadPathsQueryResponse, } from 'jest/vue_shared/security_reports/mock_data'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import Component from '~/vue_shared/security_reports/components/artifact_downloads/merge_request_artifact_download.vue'; import SecurityReportDownloadDropdown from '~/vue_shared/security_reports/components/security_report_download_dropdown.vue'; import { @@ -93,8 +93,8 @@ describe('Merge request artifact Download', () => { }); }); - it('calls createFlash correctly', () => { - expect(createFlash).toHaveBeenCalledWith({ + it('calls createAlert correctly', () => { + expect(createAlert).toHaveBeenCalledWith({ message: Component.i18n.apiError, captureError: true, error: expect.any(Error), diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select_vue/store/actions_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select_vue/store/actions_spec.js index 2bc513e87bf..edd044bd754 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select_vue/store/actions_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select_vue/store/actions_spec.js @@ -1,7 +1,7 @@ import MockAdapter from 'axios-mock-adapter'; import testAction from 'helpers/vuex_action_helper'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import * as actions from '~/vue_shared/components/sidebar/labels_select_vue/store/actions'; import * as types from '~/vue_shared/components/sidebar/labels_select_vue/store/mutation_types'; @@ -102,7 +102,7 @@ describe('LabelsSelect Actions', () => { it('shows flash error', () => { actions.receiveLabelsFailure({ commit: () => {} }); - expect(createFlash).toHaveBeenCalledWith({ message: 'Error fetching labels.' }); + expect(createAlert).toHaveBeenCalledWith({ message: 'Error fetching labels.' }); }); }); @@ -186,7 +186,7 @@ describe('LabelsSelect Actions', () => { it('shows flash error', () => { actions.receiveCreateLabelFailure({ commit: () => {} }); - expect(createFlash).toHaveBeenCalledWith({ message: 'Error creating label.' }); + expect(createAlert).toHaveBeenCalledWith({ message: 'Error creating label.' }); }); }); diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view_spec.js index 9c29f304c71..237f174e048 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view_spec.js @@ -4,7 +4,7 @@ import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { workspaceLabelsQueries } from '~/sidebar/constants'; import DropdownContentsCreateView from '~/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_create_view.vue'; import createLabelMutation from '~/vue_shared/components/sidebar/labels_select_widget/graphql/create_label.mutation.graphql'; @@ -202,7 +202,7 @@ describe('DropdownContentsCreateView', () => { }); }); - it('calls createFlash is mutation has a user-recoverable error', async () => { + it('calls createAlert is mutation has a user-recoverable error', async () => { createComponent({ mutationHandler: createLabelUserRecoverableErrorHandler }); fillLabelAttributes(); await nextTick(); @@ -210,10 +210,10 @@ describe('DropdownContentsCreateView', () => { findCreateButton().vm.$emit('click'); await waitForPromises(); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); - it('calls createFlash is mutation was rejected', async () => { + it('calls createAlert is mutation was rejected', async () => { createComponent({ mutationHandler: createLabelErrorHandler }); fillLabelAttributes(); await nextTick(); @@ -221,7 +221,7 @@ describe('DropdownContentsCreateView', () => { findCreateButton().vm.$emit('click'); await waitForPromises(); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); it('displays error in alert if label title is already taken', async () => { diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view_spec.js index 7f6770e0bea..5d8ad5ddee5 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view_spec.js @@ -9,7 +9,7 @@ import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { DropdownVariant } from '~/vue_shared/components/sidebar/labels_select_widget/constants'; import DropdownContentsLabelsView from '~/vue_shared/components/sidebar/labels_select_widget/dropdown_contents_labels_view.vue'; @@ -143,13 +143,13 @@ describe('DropdownContentsLabelsView', () => { expect(findNoResultsMessage().isVisible()).toBe(true); }); - it('calls `createFlash` when fetching labels failed', async () => { + it('calls `createAlert` when fetching labels failed', async () => { createComponent({ queryHandler: jest.fn().mockRejectedValue('Houston, we have a problem!') }); await makeObserverAppear(); jest.advanceTimersByTime(DEFAULT_DEBOUNCE_AND_THROTTLE_MS); await waitForPromises(); - expect(createFlash).toHaveBeenCalled(); + expect(createAlert).toHaveBeenCalled(); }); it('emits an `input` event on label click', async () => { diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select_widget/labels_select_root_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select_widget/labels_select_root_spec.js index cad401e0013..b58c44645d6 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select_widget/labels_select_root_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select_widget/labels_select_root_spec.js @@ -3,7 +3,7 @@ import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { IssuableType } from '~/issues/constants'; import SidebarEditableItem from '~/sidebar/components/sidebar_editable_item.vue'; import DropdownContents from '~/vue_shared/components/sidebar/labels_select_widget/dropdown_contents.vue'; @@ -151,7 +151,7 @@ describe('LabelsSelectRoot', () => { it('creates flash with error message when query is rejected', async () => { createComponent({ queryHandler: errorQueryHandler }); await waitForPromises(); - expect(createFlash).toHaveBeenCalledWith({ message: 'Error fetching labels.' }); + expect(createAlert).toHaveBeenCalledWith({ message: 'Error fetching labels.' }); }); }); @@ -197,7 +197,7 @@ describe('LabelsSelectRoot', () => { findDropdownContents().vm.$emit('setLabels', [label]); await waitForPromises(); - expect(createFlash).toHaveBeenCalledWith({ + expect(createAlert).toHaveBeenCalledWith({ captureError: true, error: expect.anything(), message: 'An error occurred while updating labels.', diff --git a/spec/frontend/vue_shared/components/source_viewer/source_viewer_spec.js b/spec/frontend/vue_shared/components/source_viewer/source_viewer_spec.js index e020d9a557e..6d319b37b02 100644 --- a/spec/frontend/vue_shared/components/source_viewer/source_viewer_spec.js +++ b/spec/frontend/vue_shared/components/source_viewer/source_viewer_spec.js @@ -22,10 +22,10 @@ jest.mock('~/vue_shared/components/source_viewer/plugins/index'); Vue.use(VueRouter); const router = new VueRouter(); -const generateContent = (content, totalLines = 1) => { +const generateContent = (content, totalLines = 1, delimiter = '\n') => { let generatedContent = ''; for (let i = 0; i < totalLines; i += 1) { - generatedContent += `Line: ${i + 1} = ${content}\n`; + generatedContent += `Line: ${i + 1} = ${content}${delimiter}`; } return generatedContent; }; @@ -38,7 +38,9 @@ describe('Source Viewer component', () => { const mappedLanguage = ROUGE_TO_HLJS_LANGUAGE_MAP[language]; const chunk1 = generateContent('// Some source code 1', 70); const chunk2 = generateContent('// Some source code 2', 70); - const content = chunk1 + chunk2; + const chunk3 = generateContent('// Some source code 3', 70, '\r\n'); + const chunk3Result = generateContent('// Some source code 3', 70, '\n'); + const content = chunk1 + chunk2 + chunk3; const path = 'some/path.js'; const blamePath = 'some/blame/path.js'; const fileType = 'javascript'; @@ -152,6 +154,19 @@ describe('Source Viewer component', () => { startingFrom: 70, }); }); + + it('renders the third chunk', async () => { + const thirdChunk = findChunks().at(2); + + expect(thirdChunk.props('content')).toContain(chunk3Result.trim()); + + expect(chunk3Result).toEqual(chunk3.replace(/\r?\n/g, '\n')); + + expect(thirdChunk.props()).toMatchObject({ + totalLines: 70, + startingFrom: 140, + }); + }); }); it('emits showBlobInteractionZones on the eventHub when chunk appears', () => { diff --git a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js index e52828deb39..e47fc518b23 100644 --- a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js +++ b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js @@ -11,7 +11,7 @@ import { I18N_USER_UNFOLLOW, } from '~/vue_shared/components/user_popover/constants'; import axios from '~/lib/utils/axios_utils'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import { followUser, unfollowUser } from '~/api/user_api'; import { mockTracking } from 'helpers/tracking_helper'; @@ -388,7 +388,7 @@ describe('User Popover Component', () => { it('shows an error message', async () => { await axios.waitForAll(); - expect(createFlash).toHaveBeenCalledWith({ + expect(createAlert).toHaveBeenCalledWith({ message: 'An error occurred while trying to follow this user, please try again.', error: {}, captureError: true, @@ -437,7 +437,7 @@ describe('User Popover Component', () => { }); it('shows an error message', () => { - expect(createFlash).toHaveBeenCalledWith({ + expect(createAlert).toHaveBeenCalledWith({ message: 'An error occurred while trying to unfollow this user, please try again.', error: {}, captureError: true, diff --git a/spec/frontend/vue_shared/security_reports/security_reports_app_spec.js b/spec/frontend/vue_shared/security_reports/security_reports_app_spec.js index a9651cf8bac..43ff68e30b5 100644 --- a/spec/frontend/vue_shared/security_reports/security_reports_app_spec.js +++ b/spec/frontend/vue_shared/security_reports/security_reports_app_spec.js @@ -14,7 +14,7 @@ import { sastDiffSuccessMock, secretDetectionDiffSuccessMock, } from 'jest/vue_shared/security_reports/mock_data'; -import createFlash from '~/flash'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import HelpIcon from '~/vue_shared/security_reports/components/help_icon.vue'; import SecurityReportDownloadDropdown from '~/vue_shared/security_reports/components/security_report_download_dropdown.vue'; @@ -135,8 +135,8 @@ describe('Security reports app', () => { }); }); - it('calls createFlash correctly', () => { - expect(createFlash).toHaveBeenCalledWith({ + it('calls createAlert correctly', () => { + expect(createAlert).toHaveBeenCalledWith({ message: SecurityReportsApp.i18n.apiError, captureError: true, error: expect.any(Error), diff --git a/spec/helpers/boards_helper_spec.rb b/spec/helpers/boards_helper_spec.rb index ccc150c397a..d72e111c7cd 100644 --- a/spec/helpers/boards_helper_spec.rb +++ b/spec/helpers/boards_helper_spec.rb @@ -189,14 +189,4 @@ RSpec.describe BoardsHelper do end end end - - describe '#current_board_json' do - let(:board_json) { helper.current_board_json } - - it 'can serialise with a basic set of attributes' do - assign(:board, project_board) - - expect(board_json).to match_schema('current-board') - end - end end diff --git a/spec/models/jira_connect/public_key_spec.rb b/spec/models/jira_connect/public_key_spec.rb new file mode 100644 index 00000000000..2e79a3ca4d2 --- /dev/null +++ b/spec/models/jira_connect/public_key_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::PublicKey do + describe '.create!' do + let(:key) { 'key123' } + + subject(:create_public_key) { described_class.create!(key: key) } + + it 'only accepts valid public keys' do + expect { create_public_key }.to raise_error(ArgumentError, 'Invalid public key') + end + + shared_examples 'creates a jira connect public key' do + it 'generates a Uuid' do + expect(create_public_key.uuid).to match(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/) + end + + it 'sets the key attribute' do + expect(create_public_key.key).to eq(expected_key) + end + + it 'persists the values' do + Gitlab::Redis::SharedState.with do |redis| + expect(redis).to receive(:set).with(anything, expected_key, anything) + end + + create_public_key + end + end + + context 'with OpenSSL::PKey::RSA object' do + let(:key) { OpenSSL::PKey::RSA.generate(3072).public_key } + let(:expected_key) { key.to_s } + + it_behaves_like 'creates a jira connect public key' + end + + context 'with string public key' do + let(:key) { OpenSSL::PKey::RSA.generate(3072).public_key.to_s } + let(:expected_key) { key } + + it_behaves_like 'creates a jira connect public key' + end + end + + describe '.find' do + let(:uuid) { '1234' } + + subject(:find_public_key) { described_class.find(uuid) } + + it 'raises an error' do + expect { find_public_key }.to raise_error(ActiveRecord::RecordNotFound) + end + + context 'when the public key exists' do + let_it_be(:key) { OpenSSL::PKey::RSA.generate(3072).public_key } + let_it_be(:public_key) { described_class.create!(key: key) } + + let(:uuid) { public_key.uuid } + + it 'loads the public key', :aggregate_failures do + expect(find_public_key).to be_kind_of(described_class) + expect(find_public_key.uuid).to eq(public_key.uuid) + expect(find_public_key.key).to eq(key.to_s) + end + end + end + + describe '#save!' do + let(:key) { OpenSSL::PKey::RSA.generate(3072).public_key } + let(:public_key) { described_class.new(key: key, uuid: '123') } + let(:jira_connect_installation) { build(:jira_connect_installation) } + + subject(:save_public_key) { public_key.save! } + + it 'persists the values' do + Gitlab::Redis::SharedState.with do |redis| + expect(redis).to receive(:set).with(anything, key.to_s, ex: 5.minutes.to_i) + end + + save_public_key + end + + it 'returns itself' do + expect(save_public_key).to eq(public_key) + end + end +end diff --git a/spec/models/jira_connect_installation_spec.rb b/spec/models/jira_connect_installation_spec.rb index f0756360096..e57d3e78a4e 100644 --- a/spec/models/jira_connect_installation_spec.rb +++ b/spec/models/jira_connect_installation_spec.rb @@ -95,4 +95,46 @@ RSpec.describe JiraConnectInstallation do end end end + + describe 'audience_url' do + let_it_be(:installation) { create(:jira_connect_installation) } + + subject(:audience) { installation.audience_url } + + it { is_expected.to eq(nil) } + + context 'when proxy installation' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + + it { is_expected.to eq('https://example.com/-/jira_connect') } + end + end + + describe 'audience_installed_event_url' do + let_it_be(:installation) { create(:jira_connect_installation) } + + subject(:audience) { installation.audience_installed_event_url } + + it { is_expected.to eq(nil) } + + context 'when proxy installation' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + + it { is_expected.to eq('https://example.com/-/jira_connect/events/installed') } + end + end + + describe 'proxy?' do + let_it_be(:installation) { create(:jira_connect_installation) } + + subject { installation.proxy? } + + it { is_expected.to eq(false) } + + context 'when instance_url is present' do + let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'https://example.com') } + + it { is_expected.to eq(true) } + end + end end diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb index 8b045a32e47..8ef4e899193 100644 --- a/spec/requests/api/pages_domains_spec.rb +++ b/spec/requests/api/pages_domains_spec.rb @@ -259,7 +259,15 @@ RSpec.describe API::PagesDomains do shared_examples_for 'post pages domains' do it 'creates a new pages domain' do - post api(route, user), params: params + expect { post api(route, user), params: params } + .to publish_event(PagesDomains::PagesDomainCreatedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: params[:domain] + ) + pages_domain = PagesDomain.find_by(domain: json_response['domain']) expect(response).to have_gitlab_http_status(:created) @@ -541,7 +549,15 @@ RSpec.describe API::PagesDomains do describe 'DELETE /projects/:project_id/pages/domains/:domain' do shared_examples_for 'delete pages domain' do it 'deletes a pages domain' do - delete api(route_domain, user) + expect { delete api(route_domain, user) } + .to change(PagesDomain, :count).by(-1) + .and publish_event(PagesDomains::PagesDomainDeletedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: pages_domain.domain + ) expect(response).to have_gitlab_http_status(:no_content) end diff --git a/spec/requests/jira_connect/public_keys_controller_spec.rb b/spec/requests/jira_connect/public_keys_controller_spec.rb new file mode 100644 index 00000000000..2eca4c0ea2f --- /dev/null +++ b/spec/requests/jira_connect/public_keys_controller_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::PublicKeysController do + describe 'GET /-/jira_connect/public_keys/:uuid' do + before do + allow(Gitlab).to receive(:com?).and_return(dot_com) + end + + let(:uuid) { non_existing_record_id } + let(:dot_com) { true } + + it 'renders 404' do + get jira_connect_public_key_path(id: uuid) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'when public key exists' do + let_it_be(:public_key) { JiraConnect::PublicKey.create!(key: OpenSSL::PKey::RSA.generate(3072).public_key) } + + let(:uuid) { public_key.uuid } + + it 'renders 200' do + get jira_connect_public_key_path(id: uuid) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq(public_key.key) + end + + context 'when not on GitLab.com' do + let(:dot_com) { false } + + it 'renders 404' do + get jira_connect_public_key_path(id: uuid) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when jira_connect_oauth_self_managed disabled' do + before do + stub_feature_flags(jira_connect_oauth_self_managed: false) + end + + it 'renders 404' do + get jira_connect_public_key_path(id: uuid) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end +end diff --git a/spec/serializers/board_serializer_spec.rb b/spec/serializers/board_serializer_spec.rb deleted file mode 100644 index 9e6d5a93d53..00000000000 --- a/spec/serializers/board_serializer_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BoardSerializer do - let(:resource) { create(:board) } - let(:json_entity) do - described_class.new - .represent(resource, serializer: serializer) - .with_indifferent_access - end - - context 'serialization' do - let(:serializer) { 'board' } - - it 'matches issue_sidebar json schema' do - expect(json_entity).to match_schema('board') - end - end -end diff --git a/spec/serializers/board_simple_entity_spec.rb b/spec/serializers/board_simple_entity_spec.rb deleted file mode 100644 index c5ab9833adf..00000000000 --- a/spec/serializers/board_simple_entity_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe BoardSimpleEntity do - let_it_be(:project) { create(:project) } - let_it_be(:board) { create(:board, project: project) } - - subject { described_class.new(board).as_json } - - describe '#name' do - it 'has `name` attribute' do - is_expected.to include(:name) - end - end -end diff --git a/spec/services/jira_connect/create_asymmetric_jwt_service_spec.rb b/spec/services/jira_connect/create_asymmetric_jwt_service_spec.rb new file mode 100644 index 00000000000..f5359e5b643 --- /dev/null +++ b/spec/services/jira_connect/create_asymmetric_jwt_service_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe JiraConnect::CreateAsymmetricJwtService do + describe '#execute' do + let_it_be(:jira_connect_installation) { create(:jira_connect_installation) } + + let(:service) { described_class.new(jira_connect_installation) } + + subject(:jwt_token) { service.execute } + + it 'raises an error' do + expect { jwt_token }.to raise_error(ArgumentError, 'jira_connect_installation is not a proxy installation') + end + + context 'with proxy installation' do + let_it_be(:jira_connect_installation) { create(:jira_connect_installation, instance_url: 'https://gitlab.test') } + + let(:public_key_id) { Atlassian::Jwt.decode(jwt_token, nil, false, algorithm: 'RS256').last['kid'] } + let(:public_key_cdn) { 'https://gitlab.com/-/jira_connect/public_keys/' } + let(:jwt_verification_claims) do + { + aud: 'https://gitlab.test/-/jira_connect', + iss: jira_connect_installation.client_key, + qsh: Atlassian::Jwt.create_query_string_hash('https://gitlab.test/-/jira_connect/events/installed', 'POST', 'https://gitlab.test/-/jira_connect') + } + end + + subject(:jwt_token) { service.execute } + + it 'stores the public key' do + expect { JiraConnect::PublicKey.find(public_key_id) }.not_to raise_error + end + + it 'is produces a valid JWT' do + public_key = OpenSSL::PKey.read(JiraConnect::PublicKey.find(public_key_id).key) + options = jwt_verification_claims.except(:qsh).merge({ verify_aud: true, verify_iss: true, algorithm: 'RS256' }) + + decoded_token = Atlassian::Jwt.decode(jwt_token, public_key, true, options).first + + expect(decoded_token).to eq(jwt_verification_claims.stringify_keys) + end + end + end +end diff --git a/spec/services/pages_domains/create_service_spec.rb b/spec/services/pages_domains/create_service_spec.rb new file mode 100644 index 00000000000..cac941fb134 --- /dev/null +++ b/spec/services/pages_domains/create_service_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::PagesDomains::CreateService do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :in_subgroup) } + + let(:domain) { 'new.domain.com' } + let(:attributes) { { domain: domain } } + + subject(:service) { described_class.new(project, user, attributes) } + + context 'when the user does not have the required permissions' do + it 'does not create a pages domain and does not publish a PagesDomainCreatedEvent' do + expect(service.execute).to be_nil + + expect { service.execute } + .to not_publish_event(PagesDomains::PagesDomainCreatedEvent) + .and not_change(project.pages_domains, :count) + end + end + + context 'when the user has the required permissions' do + before do + project.add_maintainer(user) + end + + context 'when it saves the domain successfully' do + it 'creates the domain and publishes a PagesDomainCreatedEvent' do + pages_domain = nil + + expect { pages_domain = service.execute } + .to change(project.pages_domains, :count) + .and publish_event(PagesDomains::PagesDomainCreatedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: domain + ) + + expect(pages_domain).to be_persisted + end + end + + context 'when it fails to save the domain' do + let(:domain) { nil } + + it 'does not create a pages domain and does not publish a PagesDomainCreatedEvent' do + pages_domain = nil + + expect { pages_domain = service.execute } + .to not_publish_event(PagesDomains::PagesDomainCreatedEvent) + .and not_change(project.pages_domains, :count) + + expect(pages_domain).not_to be_persisted + end + end + end +end diff --git a/spec/services/pages_domains/delete_service_spec.rb b/spec/services/pages_domains/delete_service_spec.rb new file mode 100644 index 00000000000..5f98fe3c7f7 --- /dev/null +++ b/spec/services/pages_domains/delete_service_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::PagesDomains::DeleteService do + let_it_be(:user) { create(:user) } + let_it_be(:pages_domain) { create(:pages_domain, :with_project) } + + let(:params) do + attributes_for(:pages_domain, :with_trusted_chain).slice(:key, :certificate).tap do |params| + params[:user_provided_key] = params.delete(:key) + params[:user_provided_certificate] = params.delete(:certificate) + end + end + + subject(:service) { described_class.new(pages_domain.project, user, params) } + + context 'when the user does not have the required permissions' do + it 'does not delete the pages domain and does not publish a PagesDomainDeletedEvent' do + result_match = -> { expect(service.execute(pages_domain)).to be_nil } + + expect(&result_match) + .to not_publish_event(PagesDomains::PagesDomainDeletedEvent) + end + end + + context 'when the user has the required permissions' do + before do + pages_domain.project.add_maintainer(user) + end + + context 'when it updates the domain successfully' do + it 'deletes the domain and publishes a PagesDomainDeletedEvent' do + result_match = -> { expect(service.execute(pages_domain)).not_to be_nil } + + expect(&result_match) + .to publish_event(PagesDomains::PagesDomainDeletedEvent) + .with( + project_id: pages_domain.project.id, + namespace_id: pages_domain.project.namespace.id, + root_namespace_id: pages_domain.project.root_namespace.id, + domain: pages_domain.domain + ) + end + end + end +end diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index 2008de195ab..c430bb6e913 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -2,372 +2,142 @@ require 'spec_helper' -RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_redis_cache do - using RSpec::Parameterized::TableSyntax +RSpec.describe Projects::ContainerRepository::CleanupTagsService do + let_it_be_with_reload(:container_repository) { create(:container_repository) } + let_it_be(:user) { container_repository.project.owner } - include_context 'for a cleanup tags service' - - let_it_be(:user) { create(:user) } - let_it_be(:project, reload: true) { create(:project, :private) } - - let(:repository) { create(:container_repository, :root, project: project) } - let(:service) { described_class.new(container_repository: repository, current_user: user, params: params) } - let(:tags) { %w[latest A Ba Bb C D E] } + let(:params) { {} } + let(:extra_params) { {} } + let(:service) { described_class.new(container_repository: container_repository, current_user: user, params: params.merge(extra_params)) } before do - project.add_maintainer(user) if user - stub_container_registry_config(enabled: true) - - stub_container_registry_tags( - repository: repository.path, - tags: tags - ) - - stub_tag_digest('latest', 'sha256:configA') - stub_tag_digest('A', 'sha256:configA') - stub_tag_digest('Ba', 'sha256:configB') - stub_tag_digest('Bb', 'sha256:configB') - stub_tag_digest('C', 'sha256:configC') - stub_tag_digest('D', 'sha256:configD') - stub_tag_digest('E', nil) - - stub_digest_config('sha256:configA', 1.hour.ago) - stub_digest_config('sha256:configB', 5.days.ago) - stub_digest_config('sha256:configC', 1.month.ago) - stub_digest_config('sha256:configD', nil) end describe '#execute' do subject { service.execute } - it_behaves_like 'handling invalid params', - service_response_extra: { - before_truncate_size: 0, - after_truncate_size: 0, - before_delete_size: 0, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when regex matching everything is specified', - delete_expectations: [%w(A Ba Bb C D E)], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 6, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when delete regex matching specific tags is used', - service_response_extra: { - before_truncate_size: 2, - after_truncate_size: 2, - before_delete_size: 2, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex', - service_response_extra: { - before_truncate_size: 1, - after_truncate_size: 1, - before_delete_size: 1, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'with allow regex value', - delete_expectations: [%w(A C D E)], - service_response_extra: { - before_truncate_size: 4, - after_truncate_size: 4, - before_delete_size: 4, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when keeping only N tags', - delete_expectations: [%w(Bb Ba C)], - service_response_extra: { - before_truncate_size: 4, - after_truncate_size: 4, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when not keeping N tags', - delete_expectations: [%w(A Ba Bb C)], - service_response_extra: { - before_truncate_size: 4, - after_truncate_size: 4, - before_delete_size: 4, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when removing keeping only 3', - delete_expectations: [%w(Bb Ba C)], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when removing older than 1 day', - delete_expectations: [%w(Ba Bb C)], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when combining all parameters', - delete_expectations: [%w(Bb Ba C)], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true - - it_behaves_like 'when running a container_expiration_policy', - delete_expectations: [%w(Bb Ba C)], - service_response_extra: { - before_truncate_size: 6, - after_truncate_size: 6, - before_delete_size: 3, - cached_tags_count: 0 - }, - supports_caching: true - - context 'when running a container_expiration_policy with caching' do - let(:user) { nil } - let(:params) do - { - 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day', - 'container_expiration_policy' => true - } - end - - it 'expects caching to be used' do - expect_delete(%w(Bb Ba C), container_expiration_policy: true) - expect_caching + shared_examples 'returning error message' do |message| + it "returns error #{message}" do + expect(::Projects::ContainerRepository::Gitlab::CleanupTagsService).not_to receive(:new) + expect(::Projects::ContainerRepository::ThirdParty::CleanupTagsService).not_to receive(:new) + expect(service).not_to receive(:log_info) - subject - end - - context 'when setting set to false' do - before do - stub_application_setting(container_registry_expiration_policies_caching: false) - end - - it 'does not use caching' do - expect_delete(%w(Bb Ba C), container_expiration_policy: true) - expect_no_caching - - subject - end + expect(subject).to eq(status: :error, message: message) end end - context 'truncating the tags list' do - let(:params) do - { - 'name_regex_delete' => '.*', - 'keep_n' => 1 - } - end - - shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:| - it 'returns the response' do - expect_no_caching - - result = subject + shared_examples 'handling invalid regular expressions' do + shared_examples 'handling invalid regex' do + it_behaves_like 'returning error message', 'invalid regex' - service_response = expected_service_response( - status: status, - original_size: original_size, - deleted: nil - ).merge( - before_truncate_size: before_truncate_size, - after_truncate_size: after_truncate_size, - before_delete_size: before_delete_size, - cached_tags_count: 0 - ) + it 'calls error tracking service' do + expect(::Gitlab::ErrorTracking).to receive(:log_exception).and_call_original - expect(result).to eq(service_response) + subject end end - where(:max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do - 10 | :success | :success | false - 10 | :error | :error | false - 3 | :success | :error | true - 3 | :error | :error | true - 0 | :success | :success | false - 0 | :error | :error | false - end + context 'when name_regex_delete is invalid' do + let(:extra_params) { { 'name_regex_delete' => '*test*' } } - with_them do - before do - stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) - allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| - expect(service).to receive(:execute).and_return(status: delete_tags_service_status) - end - end - - original_size = 7 - keep_n = 1 - - it_behaves_like( - 'returning the response', - status: params[:expected_status], - original_size: original_size, - before_truncate_size: original_size - keep_n, - after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n, - before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter - ) + it_behaves_like 'handling invalid regex' end - end - context 'caching', :freeze_time do - let(:params) do - { - 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day', - 'container_expiration_policy' => true - } - end + context 'when name_regex is invalid' do + let(:extra_params) { { 'name_regex' => '*test*' } } - let(:tags_and_created_ats) do - { - 'A' => 1.hour.ago, - 'Ba' => 5.days.ago, - 'Bb' => 5.days.ago, - 'C' => 1.month.ago, - 'D' => nil, - 'E' => nil - } + it_behaves_like 'handling invalid regex' end - let(:cacheable_tags) { tags_and_created_ats.reject { |_, value| value.nil? } } + context 'when name_regex_keep is invalid' do + let(:extra_params) { { 'name_regex_keep' => '*test*' } } - before do - expect_delete(%w(Bb Ba C), container_expiration_policy: true) - # We froze time so we need to set the created_at stubs again - stub_digest_config('sha256:configA', 1.hour.ago) - stub_digest_config('sha256:configB', 5.days.ago) - stub_digest_config('sha256:configC', 1.month.ago) + it_behaves_like 'handling invalid regex' end + end - it 'caches the created_at values' do - expect_mget(tags_and_created_ats.keys) - expect_set(cacheable_tags) - - expect(subject).to include(cached_tags_count: 0) + shared_examples 'handling all types of container repositories' do + shared_examples 'calling service' do |service_class, extra_log_data: {}| + let(:service_double) { instance_double(service_class.to_s) } + + it "uses cleanup tags service #{service_class}" do + expect(service_class).to receive(:new).with(container_repository: container_repository, current_user: user, params: params).and_return(service_double) + expect(service_double).to receive(:execute).and_return('return value') + expect(service).to receive(:log_info) + .with( + { + container_repository_id: container_repository.id, + container_repository_path: container_repository.path, + project_id: container_repository.project.id + }.merge(extra_log_data)) + expect(subject).to eq('return value') + end end - context 'with cached values' do + context 'with a migrated repository' do before do - ::Gitlab::Redis::Cache.with do |redis| - redis.set(cache_key('C'), rfc3339(1.month.ago)) - end + container_repository.update_column(:migration_state, :import_done) end - it 'uses them' do - expect_mget(tags_and_created_ats.keys) - - # because C is already in cache, it should not be cached again - expect_set(cacheable_tags.except('C')) + context 'supporting the gitlab api' do + before do + allow(container_repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(true) + end - # We will ping the container registry for all tags *except* for C because it's cached - expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configA" }).and_call_original - expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configB" }).twice.and_call_original - expect(ContainerRegistry::Blob).not_to receive(:new).with(repository, { "digest" => "sha256:configC" }) - expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configD" }).and_call_original + it_behaves_like 'calling service', ::Projects::ContainerRepository::Gitlab::CleanupTagsService, extra_log_data: { gitlab_cleanup_tags_service: true } - expect(subject).to include(cached_tags_count: 1) - end - end + context 'with container_registry_new_cleanup_service disabled' do + before do + stub_feature_flags(container_registry_new_cleanup_service: false) + end - def expect_mget(keys) - Gitlab::Redis::Cache.with do |redis| - expect(redis).to receive(:mget).with(keys.map(&method(:cache_key))).and_call_original + it_behaves_like 'calling service', ::Projects::ContainerRepository::ThirdParty::CleanupTagsService, extra_log_data: { third_party_cleanup_tags_service: true } + end end - end - - def expect_set(tags) - selected_tags = tags.map do |tag_name, created_at| - ex = 1.day.seconds - (Time.zone.now - created_at).seconds - [tag_name, created_at, ex.to_i] if ex.positive? - end.compact - - return if selected_tags.count.zero? - - Gitlab::Redis::Cache.with do |redis| - expect(redis).to receive(:pipelined).and_call_original - expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| - selected_tags.each do |tag_name, created_at, ex| - expect(pipeline).to receive(:set).with(cache_key(tag_name), rfc3339(created_at), ex: ex) - end + context 'not supporting the gitlab api' do + before do + allow(container_repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(false) end + + it_behaves_like 'calling service', ::Projects::ContainerRepository::ThirdParty::CleanupTagsService, extra_log_data: { third_party_cleanup_tags_service: true } end end - def cache_key(tag_name) - "container_repository:{#{repository.id}}:tag:#{tag_name}:created_at" - end + context 'with a non migrated repository' do + before do + container_repository.update_column(:migration_state, :default) + container_repository.update!(created_at: ContainerRepository::MIGRATION_PHASE_1_ENDED_AT - 1.week) + end - def rfc3339(date_time) - # DateTime rfc3339 is different ActiveSupport::TimeWithZone rfc3339 - # The caching will use DateTime rfc3339 - DateTime.rfc3339(date_time.rfc3339).rfc3339 + it_behaves_like 'calling service', ::Projects::ContainerRepository::ThirdParty::CleanupTagsService, extra_log_data: { third_party_cleanup_tags_service: true } end end - end - private + context 'with valid user' do + it_behaves_like 'handling invalid regular expressions' + it_behaves_like 'handling all types of container repositories' + end - def stub_tag_digest(tag, digest) - allow_any_instance_of(ContainerRegistry::Client) - .to receive(:repository_tag_digest) - .with(repository.path, tag) { digest } + context 'for container expiration policy' do + let(:user) { nil } + let(:params) { { 'container_expiration_policy' => true } } - allow_any_instance_of(ContainerRegistry::Client) - .to receive(:repository_manifest) - .with(repository.path, tag) do - { 'config' => { 'digest' => digest } } if digest + it_behaves_like 'handling invalid regular expressions' + it_behaves_like 'handling all types of container repositories' end - end - def stub_digest_config(digest, created_at) - allow_any_instance_of(ContainerRegistry::Client) - .to receive(:blob) - .with(repository.path, digest, nil) do - { 'created' => created_at.to_datetime.rfc3339 }.to_json if created_at + context 'with not allowed user' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'returning error message', 'access denied' end - end - def expect_caching - ::Gitlab::Redis::Cache.with do |redis| - expect(redis).to receive(:mget).and_call_original - expect(redis).to receive(:pipelined).and_call_original + context 'with no user' do + let(:user) { nil } - expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| - expect(pipeline).to receive(:set).and_call_original - end + it_behaves_like 'returning error message', 'access denied' end end end diff --git a/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb index d2cdb667659..59827ea035e 100644 --- a/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/gitlab/cleanup_tags_service_spec.rb @@ -46,8 +46,6 @@ RSpec.describe Projects::ContainerRepository::Gitlab::CleanupTagsService do context 'with several tags pages' do let(:tags_page_size) { 2 } - it_behaves_like 'handling invalid params' - it_behaves_like 'when regex matching everything is specified', delete_expectations: [%w[A], %w[Ba Bb], %w[C D], %w[E]] @@ -105,8 +103,6 @@ RSpec.describe Projects::ContainerRepository::Gitlab::CleanupTagsService do context 'with a single tags page' do let(:tags_page_size) { 1000 } - it_behaves_like 'handling invalid params' - it_behaves_like 'when regex matching everything is specified', delete_expectations: [%w[A Ba Bb C D E]] diff --git a/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb new file mode 100644 index 00000000000..2d034d577ac --- /dev/null +++ b/spec/services/projects/container_repository/third_party/cleanup_tags_service_spec.rb @@ -0,0 +1,370 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::ContainerRepository::ThirdParty::CleanupTagsService, :clean_gitlab_redis_cache do + using RSpec::Parameterized::TableSyntax + + include_context 'for a cleanup tags service' + + let_it_be(:user) { create(:user) } + let_it_be(:project, reload: true) { create(:project, :private) } + + let(:repository) { create(:container_repository, :root, project: project) } + let(:service) { described_class.new(container_repository: repository, current_user: user, params: params) } + let(:tags) { %w[latest A Ba Bb C D E] } + + before do + project.add_maintainer(user) if user + + stub_container_registry_config(enabled: true) + + stub_container_registry_tags( + repository: repository.path, + tags: tags + ) + + stub_tag_digest('latest', 'sha256:configA') + stub_tag_digest('A', 'sha256:configA') + stub_tag_digest('Ba', 'sha256:configB') + stub_tag_digest('Bb', 'sha256:configB') + stub_tag_digest('C', 'sha256:configC') + stub_tag_digest('D', 'sha256:configD') + stub_tag_digest('E', nil) + + stub_digest_config('sha256:configA', 1.hour.ago) + stub_digest_config('sha256:configB', 5.days.ago) + stub_digest_config('sha256:configC', 1.month.ago) + stub_digest_config('sha256:configD', nil) + end + + describe '#execute' do + subject { service.execute } + + it_behaves_like 'when regex matching everything is specified', + delete_expectations: [%w[A Ba Bb C D E]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 6, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when delete regex matching specific tags is used', + service_response_extra: { + before_truncate_size: 2, + after_truncate_size: 2, + before_delete_size: 2, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when delete regex matching specific tags is used with overriding allow regex', + service_response_extra: { + before_truncate_size: 1, + after_truncate_size: 1, + before_delete_size: 1, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'with allow regex value', + delete_expectations: [%w[A C D E]], + service_response_extra: { + before_truncate_size: 4, + after_truncate_size: 4, + before_delete_size: 4, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when keeping only N tags', + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 4, + after_truncate_size: 4, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when not keeping N tags', + delete_expectations: [%w[A Ba Bb C]], + service_response_extra: { + before_truncate_size: 4, + after_truncate_size: 4, + before_delete_size: 4, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when removing keeping only 3', + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when removing older than 1 day', + delete_expectations: [%w[Ba Bb C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when combining all parameters', + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true + + it_behaves_like 'when running a container_expiration_policy', + delete_expectations: [%w[Bb Ba C]], + service_response_extra: { + before_truncate_size: 6, + after_truncate_size: 6, + before_delete_size: 3, + cached_tags_count: 0 + }, + supports_caching: true + + context 'when running a container_expiration_policy with caching' do + let(:user) { nil } + let(:params) do + { + 'name_regex_delete' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day', + 'container_expiration_policy' => true + } + end + + it 'expects caching to be used' do + expect_delete(%w[Bb Ba C], container_expiration_policy: true) + expect_caching + + subject + end + + context 'when setting set to false' do + before do + stub_application_setting(container_registry_expiration_policies_caching: false) + end + + it 'does not use caching' do + expect_delete(%w[Bb Ba C], container_expiration_policy: true) + expect_no_caching + + subject + end + end + end + + context 'when truncating the tags list' do + let(:params) do + { + 'name_regex_delete' => '.*', + 'keep_n' => 1 + } + end + + shared_examples 'returning the response' do + |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:| + it 'returns the response' do + expect_no_caching + + result = subject + + service_response = expected_service_response( + status: status, + original_size: original_size, + deleted: nil + ).merge( + before_truncate_size: before_truncate_size, + after_truncate_size: after_truncate_size, + before_delete_size: before_delete_size, + cached_tags_count: 0 + ) + + expect(result).to eq(service_response) + end + end + + where(:max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do + 10 | :success | :success | false + 10 | :error | :error | false + 3 | :success | :error | true + 3 | :error | :error | true + 0 | :success | :success | false + 0 | :error | :error | false + end + + with_them do + before do + stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) + allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| + allow(service).to receive(:execute).and_return(status: delete_tags_service_status) + end + end + + original_size = 7 + keep_n = 1 + + it_behaves_like( + 'returning the response', + status: params[:expected_status], + original_size: original_size, + before_truncate_size: original_size - keep_n, + after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n, + # one tag is filtered out with older_than filter + before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 + ) + end + end + + context 'with caching', :freeze_time do + let(:params) do + { + 'name_regex_delete' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day', + 'container_expiration_policy' => true + } + end + + let(:tags_and_created_ats) do + { + 'A' => 1.hour.ago, + 'Ba' => 5.days.ago, + 'Bb' => 5.days.ago, + 'C' => 1.month.ago, + 'D' => nil, + 'E' => nil + } + end + + let(:cacheable_tags) { tags_and_created_ats.reject { |_, value| value.nil? } } + + before do + expect_delete(%w[Bb Ba C], container_expiration_policy: true) + # We froze time so we need to set the created_at stubs again + stub_digest_config('sha256:configA', 1.hour.ago) + stub_digest_config('sha256:configB', 5.days.ago) + stub_digest_config('sha256:configC', 1.month.ago) + end + + it 'caches the created_at values' do + expect_mget(tags_and_created_ats.keys) + expect_set(cacheable_tags) + + expect(subject).to include(cached_tags_count: 0) + end + + context 'with cached values' do + before do + ::Gitlab::Redis::Cache.with do |redis| + redis.set(cache_key('C'), rfc3339(1.month.ago)) + end + end + + it 'uses them' do + expect_mget(tags_and_created_ats.keys) + + # because C is already in cache, it should not be cached again + expect_set(cacheable_tags.except('C')) + + # We will ping the container registry for all tags *except* for C because it's cached + expect(ContainerRegistry::Blob) + .to receive(:new).with(repository, { "digest" => "sha256:configA" }).and_call_original + expect(ContainerRegistry::Blob) + .to receive(:new).with(repository, { "digest" => "sha256:configB" }).twice.and_call_original + expect(ContainerRegistry::Blob).not_to receive(:new).with(repository, { "digest" => "sha256:configC" }) + expect(ContainerRegistry::Blob) + .to receive(:new).with(repository, { "digest" => "sha256:configD" }).and_call_original + + expect(subject).to include(cached_tags_count: 1) + end + end + + def expect_mget(keys) + Gitlab::Redis::Cache.with do |redis| + parameters = keys.map { |k| cache_key(k) } + expect(redis).to receive(:mget).with(parameters).and_call_original + end + end + + def expect_set(tags) + selected_tags = tags.map do |tag_name, created_at| + ex = 1.day.seconds - (Time.zone.now - created_at).seconds + [tag_name, created_at, ex.to_i] if ex.positive? + end.compact + + return if selected_tags.count.zero? + + Gitlab::Redis::Cache.with do |redis| + expect(redis).to receive(:pipelined).and_call_original + + expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| + selected_tags.each do |tag_name, created_at, ex| + expect(pipeline).to receive(:set).with(cache_key(tag_name), rfc3339(created_at), ex: ex) + end + end + end + end + + def cache_key(tag_name) + "container_repository:{#{repository.id}}:tag:#{tag_name}:created_at" + end + + def rfc3339(date_time) + # DateTime rfc3339 is different ActiveSupport::TimeWithZone rfc3339 + # The caching will use DateTime rfc3339 + DateTime.rfc3339(date_time.rfc3339).rfc3339 + end + end + end + + private + + def stub_tag_digest(tag, digest) + allow(repository.client) + .to receive(:repository_tag_digest) + .with(repository.path, tag) { digest } + + allow(repository.client) + .to receive(:repository_manifest) + .with(repository.path, tag) do + { 'config' => { 'digest' => digest } } if digest + end + end + + def stub_digest_config(digest, created_at) + allow(repository.client) + .to receive(:blob) + .with(repository.path, digest, nil) do + { 'created' => created_at.to_datetime.rfc3339 }.to_json if created_at + end + end + + def expect_caching + ::Gitlab::Redis::Cache.with do |redis| + expect(redis).to receive(:mget).and_call_original + expect(redis).to receive(:pipelined).and_call_original + + expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| + expect(pipeline).to receive(:set).and_call_original + end + end + end +end diff --git a/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb b/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb index 9c2d30a9c8c..f7731af8dc6 100644 --- a/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb +++ b/spec/support/shared_examples/projects/container_repository/cleanup_tags_service_shared_examples.rb @@ -1,53 +1,5 @@ # frozen_string_literal: true -RSpec.shared_examples 'handling invalid params' do |service_response_extra: {}, supports_caching: false| - context 'when no params are specified' do - let(:params) { {} } - - it_behaves_like 'not removing anything', - service_response_extra: service_response_extra, - supports_caching: supports_caching - end - - context 'with invalid regular expressions' do - shared_examples 'handling an invalid regex' do - it 'keeps all tags' do - expect(Projects::ContainerRepository::DeleteTagsService) - .not_to receive(:new) - expect_no_caching unless supports_caching - - subject - end - - it { is_expected.to eq(status: :error, message: 'invalid regex') } - - it 'calls error tracking service' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original - - subject - end - end - - context 'when name_regex_delete is invalid' do - let(:params) { { 'name_regex_delete' => '*test*' } } - - it_behaves_like 'handling an invalid regex' - end - - context 'when name_regex is invalid' do - let(:params) { { 'name_regex' => '*test*' } } - - it_behaves_like 'handling an invalid regex' - end - - context 'when name_regex_keep is invalid' do - let(:params) { { 'name_regex_keep' => '*test*' } } - - it_behaves_like 'handling an invalid regex' - end - end -end - RSpec.shared_examples 'when regex matching everything is specified' do |service_response_extra: {}, supports_caching: false, delete_expectations:| let(:params) do @@ -227,20 +179,6 @@ RSpec.shared_examples 'when running a container_expiration_policy' do is_expected.to eq(expected_service_response(deleted: delete_expectations.flatten).merge(service_response_extra)) end end - - context 'without container_expiration_policy param' do - let(:params) do - { - 'name_regex_delete' => '.*', - 'keep_n' => 1, - 'older_than' => '1 day' - } - end - - it 'fails' do - is_expected.to eq(status: :error, message: 'access denied') - end - end end RSpec.shared_examples 'not removing anything' do |service_response_extra: {}, supports_caching: false| |
