diff options
158 files changed, 1978 insertions, 992 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index 85c40e7ec05..ae0d4610927 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -28,11 +28,11 @@ rules: import/no-unresolved: - error - ignore: - # In FOSS, these import paths are rewritten using - # NormalModuleReplacementPlugin, which import/no-unresolved doesn't - # consider. See - # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89831. - - '^(ee|jh)_component/' + # In FOSS, these import paths are rewritten using + # NormalModuleReplacementPlugin, which import/no-unresolved doesn't + # consider. See + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89831. + - '^(ee|jh)_component/' # Disabled for now, to make the airbnb-base 12.1.0 -> 13.1.0 update smoother no-else-return: - error @@ -48,12 +48,12 @@ rules: '@gitlab/vue-no-new-non-primitive-in-template': - error - allowNames: - - 'class(es)?$' - - '^style$' - - '^to$' - - '^$' - - '^variables$' - - 'attrs?$' + - 'class(es)?$' + - '^style$' + - '^to$' + - '^$' + - '^variables$' + - 'attrs?$' no-param-reassign: - error - props: true @@ -120,8 +120,8 @@ rules: no-restricted-imports: - error - paths: - - name: mousetrap - message: "Import { Mousetrap } from ~/lib/mousetrap instead." + - name: mousetrap + message: 'Import { Mousetrap } from ~/lib/mousetrap instead.' # See https://gitlab.com/gitlab-org/gitlab/-/issues/360551 vue/multi-word-component-names: off unicorn/prefer-dom-node-dataset: @@ -136,7 +136,7 @@ rules: methods: 'sanitize' overrides: - files: - - '{,ee/,jh/}spec/frontend*/**/*' + - '{,ee/,jh/}spec/frontend*/**/*' rules: '@gitlab/require-i18n-strings': off '@gitlab/no-runtime-template-compiler': off @@ -153,6 +153,8 @@ overrides: message: 'Prefer explicit waitForPromises (or equivalent), or jest.runAllTimers (or equivalent) to vague setImmediate calls.' - selector: ImportSpecifier[imported.name='GlSkeletonLoading'] message: 'Migrate to GlSkeletonLoader, or import GlDeprecatedSkeletonLoading.' + - selector: CallExpression[arguments.length=1][arguments.0.type='Literal'] CallExpression[callee.property.name='toBe'] CallExpression[callee.property.name='attributes'][arguments.length=1][arguments.0.value='disabled'] + message: Avoid asserting disabled attribute exact value, because Vue.js 2 and Vue.js 3 renders it differently. Use toBeDefined / toBeUndefined instead no-unsanitized/method: off no-unsanitized/property: off - files: @@ -198,6 +200,6 @@ overrides: '@graphql-eslint/no-unused-fragments': error '@graphql-eslint/no-duplicate-fields': error - files: - - '{,ee/}spec/contracts/consumer/**/*' + - '{,ee/}spec/contracts/consumer/**/*' rules: '@gitlab/require-i18n-strings': off diff --git a/.gitlab/issue_templates/AI Project Proposal.md b/.gitlab/issue_templates/AI Project Proposal.md index 94c4dbdeca1..072e7ed9ed3 100644 --- a/.gitlab/issue_templates/AI Project Proposal.md +++ b/.gitlab/issue_templates/AI Project Proposal.md @@ -50,6 +50,7 @@ _What job to be done will this solve?_ ### Problem validation _What validation exists that customers have this problem?_ +<!-- Refer to https://about.gitlab.com/handbook/product/ux/ux-research/research-in-the-AI-space/#guideline-1-problem-validation --- to help identify and understand user needs --> ### Business objective _What business objective will be achieved with this proposal?_ @@ -110,6 +111,7 @@ _What tasks or actions should the user be capable of performing with this featur <details> <summary> Technical needs </summary> - [ ] Please consider the operational aspects of the feature you are creating. A list of things to think about is in: https://gitlab.com/gitlab-org/gitlab/-/issues/403859. We will be improving this process in the future: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117637#note_1353253349. +- [ ] @ mention your [AppSec Stable Counterpart](https://about.gitlab.com/handbook/product/categories/) and read the [AI secure coding guidelines](https://docs.gitlab.com/ee/development/secure_coding_guidelines.html#artificial-intelligence-ai-features) 1. Work estimate and skills needs to build an ML viable feature: To build any ML feature depending on the work, there are many personas that contribute including, Data Scientist, NLP engineer, ML Engineer, MLOps Engineer, ML Infra engineers, and Fullstack engineer to integrate the ML Services with Gitlab. Post-prototype we would assess the skills needed to build a production-grade ML feature for the prototype. 2. Data Limitation: We would like to upfront validate if we have viable data for the feature including whether we can use the DataOps pipeline of ModelOps or create a custom one. We would want to understand the training data, test data, and feedback data to dial up the accuracy and the limitations of the data. diff --git a/.gitlab/merge_request_templates/Default.md b/.gitlab/merge_request_templates/Default.md index 2f5e0406ab4..404a18ad074 100644 --- a/.gitlab/merge_request_templates/Default.md +++ b/.gitlab/merge_request_templates/Default.md @@ -27,10 +27,6 @@ _Numbered steps to set up and validate the change are strongly suggested._ <!-- Example below: -1. Enable the invite modal - ```ruby - Feature.enable(:invite_members_group_modal) - ``` 1. In rails console enable the experiment fully ```ruby Feature.enable(:member_areas_of_focus) diff --git a/.rubocop_todo/cop/user_admin.yml b/.rubocop_todo/cop/user_admin.yml index d0e1d035a8d..ce16309d3f8 100644 --- a/.rubocop_todo/cop/user_admin.yml +++ b/.rubocop_todo/cop/user_admin.yml @@ -5,7 +5,6 @@ Cop/UserAdmin: - 'app/controllers/sessions_controller.rb' - 'app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb' - 'app/graphql/resolvers/admin/analytics/usage_trends/measurements_resolver.rb' - - 'app/models/concerns/protected_ref_access.rb' - 'app/models/concerns/spammable.rb' - 'app/models/merge_requests_closing_issues.rb' - 'app/models/protected_branch.rb' diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index a7d176a4fc1..b73074fe8ec 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -1418,13 +1418,6 @@ Layout/ArgumentAlignment: - 'ee/spec/requests/users/identity_verification_controller_spec.rb' - 'ee/spec/services/analytics/cycle_analytics/aggregator_service_spec.rb' - 'ee/spec/services/analytics/devops_adoption/enabled_namespaces/find_or_create_service_spec.rb' - - 'ee/spec/services/app_sec/dast/profiles/create_associations_service_spec.rb' - - 'ee/spec/services/app_sec/dast/profiles/create_service_spec.rb' - - 'ee/spec/services/app_sec/dast/profiles/update_service_spec.rb' - - 'ee/spec/services/app_sec/dast/scan_configs/build_service_spec.rb' - - 'ee/spec/services/app_sec/dast/scanner_profiles/create_service_spec.rb' - - 'ee/spec/services/app_sec/dast/scanner_profiles/destroy_service_spec.rb' - - 'ee/spec/services/app_sec/dast/scanner_profiles/update_service_spec.rb' - 'ee/spec/services/arkose/blocked_users_report_service_spec.rb' - 'ee/spec/services/audit_events/protected_branch_audit_event_service_spec.rb' - 'ee/spec/services/audit_events/streaming/event_type_filters/create_service_spec.rb' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9394f0c9861..a6075bcfe8f 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -2df2ef004c6fece1192069a862c584425cc54cdf +da9aed3fa5fed505898e09126ee97b34ed7fccd7 diff --git a/app/assets/javascripts/emoji/awards_app/store/actions.js b/app/assets/javascripts/emoji/awards_app/store/actions.js index 427a504e038..677c11277a3 100644 --- a/app/assets/javascripts/emoji/awards_app/store/actions.js +++ b/app/assets/javascripts/emoji/awards_app/store/actions.js @@ -2,8 +2,6 @@ import * as Sentry from '@sentry/browser'; import axios from '~/lib/utils/axios_utils'; import { normalizeHeaders } from '~/lib/utils/common_utils'; import { joinPaths } from '~/lib/utils/url_utility'; -import { __ } from '~/locale'; -import showToast from '~/vue_shared/plugins/global_toast'; import { SET_INITIAL_DATA, FETCH_AWARDS_SUCCESS, @@ -62,8 +60,6 @@ export const toggleAward = async ({ commit, state }, name) => { throw err; }); - - showToast(__('Award removed')); } else { const optimisticAward = newOptimisticAward(name, state); @@ -78,8 +74,6 @@ export const toggleAward = async ({ commit, state }, name) => { }); commit(ADD_NEW_AWARD, data); - - showToast(__('Award added')); } } catch (error) { Sentry.captureException(error); diff --git a/app/assets/javascripts/mr_notes/init_notes.js b/app/assets/javascripts/mr_notes/init_notes.js index adee18184aa..1795363f24c 100644 --- a/app/assets/javascripts/mr_notes/init_notes.js +++ b/app/assets/javascripts/mr_notes/init_notes.js @@ -43,6 +43,7 @@ export default ({ editorAiActions = [] } = {}) => { reportAbusePath: notesDataset.reportAbusePath, newCommentTemplatePath: notesDataset.newCommentTemplatePath, editorAiActions, + mrFilter: true, }, data() { const noteableData = JSON.parse(notesDataset.noteableData); diff --git a/app/assets/javascripts/notes/components/mr_discussion_filter.vue b/app/assets/javascripts/notes/components/mr_discussion_filter.vue new file mode 100644 index 00000000000..2338c9eef67 --- /dev/null +++ b/app/assets/javascripts/notes/components/mr_discussion_filter.vue @@ -0,0 +1,109 @@ +<script> +import { GlCollapsibleListbox, GlButton, GlIcon, GlSprintf, GlButtonGroup } from '@gitlab/ui'; +import { mapActions, mapState } from 'vuex'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; +import { __ } from '~/locale'; +import { MR_FILTER_OPTIONS } from '~/notes/constants'; + +export default { + components: { + GlCollapsibleListbox, + GlButton, + GlButtonGroup, + GlIcon, + GlSprintf, + LocalStorageSync, + }, + data() { + return { + selectedFilters: MR_FILTER_OPTIONS.map((f) => f.value), + }; + }, + computed: { + ...mapState({ + mergeRequestFilters: (state) => state.notes.mergeRequestFilters, + discussionSortOrder: (state) => state.notes.discussionSortOrder, + }), + selectedFilterText() { + const { length } = this.mergeRequestFilters; + + if (length === 0) return __('None'); + + const firstSelected = MR_FILTER_OPTIONS.find( + ({ value }) => this.mergeRequestFilters[0] === value, + ); + + if (length === MR_FILTER_OPTIONS.length) { + return __('All activity'); + } else if (length > 1) { + return `%{strongStart}${firstSelected.text}%{strongEnd} +${length - 1} more`; + } + + return firstSelected.text; + }, + isSortAsc() { + return this.discussionSortOrder === 'asc'; + }, + sortIcon() { + return this.isSortAsc ? 'sort-lowest' : 'sort-highest'; + }, + }, + methods: { + ...mapActions(['updateMergeRequestFilters', 'setDiscussionSortDirection']), + updateSortDirection() { + this.setDiscussionSortDirection({ + direction: this.isSortAsc ? 'desc' : 'asc', + }); + }, + applyFilters() { + this.updateMergeRequestFilters(this.selectedFilters); + }, + localSyncFilters(filters) { + this.updateMergeRequestFilters(filters); + this.selectedFilters = filters; + }, + }, + MR_FILTER_OPTIONS, +}; +</script> + +<template> + <div> + <local-storage-sync + :value="discussionSortOrder" + storage-key="sort_direction_merge_request" + as-string + @input="setDiscussionSortDirection({ direction: $event })" + /> + <local-storage-sync + :value="mergeRequestFilters" + storage-key="mr_activity_filters" + @input="localSyncFilters" + /> + <gl-button-group> + <gl-collapsible-listbox + v-model="selectedFilters" + :items="$options.MR_FILTER_OPTIONS" + multiple + placement="right" + @hidden="applyFilters" + > + <template #toggle> + <gl-button class="gl-rounded-top-right-none! gl-rounded-bottom-right-none!"> + <gl-sprintf :message="selectedFilterText"> + <template #strong="{ content }"> + <strong>{{ content }}</strong> + </template> + </gl-sprintf> + <gl-icon name="chevron-down" /> + </gl-button> + </template> + <template #list-item="{ item }"> + <strong v-if="item.value === '*'">{{ item.text }}</strong> + <span v-else>{{ item.text }}</span> + </template> + </gl-collapsible-listbox> + <gl-button :icon="sortIcon" @click="updateSortDirection" /> + </gl-button-group> + </div> +</template> diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index 7dc6b045b4d..5e776639a7a 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -73,6 +73,11 @@ export default { required: false, default: '', }, + emailParticipant: { + type: String, + required: false, + default: '', + }, }, data() { return { @@ -97,6 +102,11 @@ export default { hasAuthor() { return this.author && Object.keys(this.author).length; }, + isServiceDeskEmailParticipant() { + return ( + !this.isInternalNote && this.author.username === 'support-bot' && this.emailParticipant + ); + }, authorLinkClasses() { return { hover: this.isUsernameLinkHovered, @@ -108,7 +118,7 @@ export default { }; }, authorName() { - return this.author.name; + return this.isServiceDeskEmailParticipant ? this.emailParticipant : this.author.name; }, internalNoteTooltip() { return s__('Notes|This internal note will always remain confidential'); @@ -159,16 +169,27 @@ export default { </button> </div> <template v-if="hasAuthor"> + <span + v-if="emailParticipant" + class="note-header-author-name gl-font-weight-bold" + data-testid="author-name" + v-text="authorName" + ></span> <a + v-else ref="authorNameLink" :href="authorHref" :class="authorLinkClasses" :data-user-id="authorId" :data-username="author.username" > - <span class="note-header-author-name gl-font-weight-bold" v-text="authorName"></span> + <span + class="note-header-author-name gl-font-weight-bold" + data-testid="author-name" + v-text="authorName" + ></span> </a> - <span v-if="!isSystemNote" class="text-nowrap author-username"> + <span v-if="!isSystemNote && !emailParticipant" class="text-nowrap author-username"> <a ref="authorUsernameLink" class="author-username-link" @@ -180,6 +201,9 @@ export default { <slot name="note-header-info"></slot> <gitlab-team-member-badge v-if="author && author.is_gitlab_employee" /> </span> + <span v-if="emailParticipant" class="note-headline-light">{{ + __('(external participant)') + }}</span> </template> <span v-else>{{ __('A deleted user') }}</span> <span class="note-headline-light note-headline-meta"> diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index ae2f94a5a80..5929e419247 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -477,6 +477,7 @@ export default { :note-id="note.id" :is-internal-note="note.internal" :noteable-type="noteableType" + :email-participant="note.external_author" > <template #note-header-info> <slot name="note-header-info"></slot> diff --git a/app/assets/javascripts/notes/components/notes_activity_header.vue b/app/assets/javascripts/notes/components/notes_activity_header.vue index 679c38d7721..a91c825710d 100644 --- a/app/assets/javascripts/notes/components/notes_activity_header.vue +++ b/app/assets/javascripts/notes/components/notes_activity_header.vue @@ -8,6 +8,7 @@ export default { DiscussionFilter, AiSummarizeNotes: () => import('ee_component/notes/components/note_actions/ai_summarize_notes.vue'), + MrDiscussionFilter: () => import('./mr_discussion_filter.vue'), }, mixins: [glFeatureFlagsMixin()], inject: { @@ -15,6 +16,9 @@ export default { default: false, }, resourceGlobalId: { default: null }, + mrFilter: { + default: false, + }, }, props: { notesFilters: { @@ -52,7 +56,8 @@ export default { :loading="aiLoading" /> <timeline-toggle v-if="showTimelineViewToggle" /> - <discussion-filter :filters="notesFilters" :selected-value="notesFilterValue" /> + <mr-discussion-filter v-if="mrFilter && glFeatures.mrActivityFilters" /> + <discussion-filter v-else :filters="notesFilters" :selected-value="notesFilterValue" /> </div> </div> </template> diff --git a/app/assets/javascripts/notes/constants.js b/app/assets/javascripts/notes/constants.js index 15eb4f95910..e7c3385ae5c 100644 --- a/app/assets/javascripts/notes/constants.js +++ b/app/assets/javascripts/notes/constants.js @@ -1,5 +1,5 @@ import { STATUS_CLOSED, STATUS_OPEN, STATUS_REOPENED } from '~/issues/constants'; -import { __ } from '~/locale'; +import { __, s__ } from '~/locale'; export const DISCUSSION_NOTE = 'DiscussionNote'; export const DIFF_NOTE = 'DiffNote'; @@ -56,3 +56,63 @@ export const toggleStateErrorMessage = { ), }, }; + +export const MR_FILTER_OPTIONS = [ + { + text: __('Approvals'), + value: 'approval', + systemNoteIcons: ['approval', 'unapproval'], + }, + { + text: __('Commits & branches'), + value: 'commit_branches', + systemNoteIcons: ['commit', 'fork'], + }, + { + text: __('Merge request status'), + value: 'status', + systemNoteIcons: ['git-merge', 'issue-close', 'issues'], + }, + { + text: __('Assignees & reviewers'), + value: 'assignees_reviewers', + noteText: [ + s__('IssuableEvents|requested review from'), + s__('IssuableEvents|removed review request for'), + s__('IssuableEvents|assigned to'), + s__('IssuableEvents|unassigned'), + ], + }, + { + text: __('Edits'), + value: 'edits', + systemNoteIcons: ['pencil', 'task-done'], + }, + { + text: __('Labels'), + value: 'labels', + systemNoteIcons: ['label'], + }, + { + text: __('Mentions'), + value: 'mentions', + systemNoteIcons: ['comment-dots'], + }, + { + text: __('Tracking'), + value: 'tracking', + noteType: ['MilestoneNote'], + systemNoteIcons: ['timer'], + }, + { + text: __('Comments'), + value: 'comments', + noteType: ['DiscussionNote', 'DiffNote'], + individualNote: true, + }, + { + text: __('Lock status'), + value: 'lock_status', + systemNoteIcons: ['lock', 'lock-open'], + }, +]; diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index cdfa0d11f56..dc7f1577bbb 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -897,3 +897,6 @@ export const updateAssignees = ({ commit }, assignees) => { export const updateDiscussionPosition = ({ commit }, updatedPosition) => { commit(types.UPDATE_DISCUSSION_POSITION, updatedPosition); }; + +export const updateMergeRequestFilters = ({ commit }, newFilters) => + commit(types.SET_MERGE_REQUEST_FILTERS, newFilters); diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index f6373f24b74..3fb9913bdcb 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -22,10 +22,51 @@ const getDraftComments = (state) => { .sort((a, b) => a.id - b.id); }; +const hideActivity = (filters, discussion) => { + const firstNote = discussion.notes[0]; + + return constants.MR_FILTER_OPTIONS.some((f) => { + if (filters.includes(f.value) || f.value === '*') return false; + + if ( + // For all of the below firstNote is the first note of a discussion, whether that be + // the first in a discussion or a single note + // If the filter option filters based on icon check against the first notes system note icon + f.systemNoteIcons?.includes(firstNote.system_note_icon_name) || + // If the filter option filters based on note type user the first notes type + f.noteType?.includes(firstNote.type) || + // If the filter option filters based on the note text then check if it is sytem + // and filter based on the text of the system note + (firstNote.system && f.noteText?.some((t) => firstNote.note.includes(t))) || + // For individual notes we filter if the discussion is a single note and is not a sytem + (f.individualNote === discussion.individual_note && !firstNote.system) + ) { + return true; + } + + return false; + }); +}; + export const discussions = (state, getters, rootState) => { let discussionsInState = clone(state.discussions); // NOTE: not testing bc will be removed when backend is finished. + if ( + state.noteableData.targetType === 'merge_request' && + window.gon?.features?.mrActivityFilters + ) { + discussionsInState = discussionsInState.reduce((acc, discussion) => { + if (hideActivity(state.mergeRequestFilters, discussion)) { + return acc; + } + + acc.push(discussion); + + return acc; + }, []); + } + if (state.isTimelineEnabled) { discussionsInState = discussionsInState .reduce((acc, discussion) => { diff --git a/app/assets/javascripts/notes/stores/modules/index.js b/app/assets/javascripts/notes/stores/modules/index.js index 81c4c42a49a..317fe6442d4 100644 --- a/app/assets/javascripts/notes/stores/modules/index.js +++ b/app/assets/javascripts/notes/stores/modules/index.js @@ -1,4 +1,4 @@ -import { ASC } from '../../constants'; +import { ASC, MR_FILTER_OPTIONS } from '../../constants'; import * as actions from '../actions'; import * as getters from '../getters'; import mutations from '../mutations'; @@ -51,6 +51,7 @@ export default () => ({ isTimelineEnabled: false, isFetching: false, isPollingInitialized: false, + mergeRequestFilters: MR_FILTER_OPTIONS.map((f) => f.value), }, actions, getters, diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index bc1d5b5bba4..4008b40b57f 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -61,3 +61,5 @@ export const RECEIVE_DELETE_DESCRIPTION_VERSION_ERROR = 'RECEIVE_DELETE_DESCRIPT // Incidents export const SET_PROMOTE_COMMENT_TO_TIMELINE_PROGRESS = 'SET_PROMOTE_COMMENT_TO_TIMELINE_PROGRESS'; + +export const SET_MERGE_REQUEST_FILTERS = 'SET_MERGE_REQUEST_FILTERS'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 7a7aa0deb1d..c3407936847 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -432,4 +432,7 @@ export default { [types.SET_IS_POLLING_INITIALIZED](state, value) { state.isPollingInitialized = value; }, + [types.SET_MERGE_REQUEST_FILTERS](state, value) { + state.mergeRequestFilters = value; + }, }; diff --git a/app/assets/javascripts/work_items/components/work_item_notes.vue b/app/assets/javascripts/work_items/components/work_item_notes.vue index 3f6a0643313..e34578e1f46 100644 --- a/app/assets/javascripts/work_items/components/work_item_notes.vue +++ b/app/assets/javascripts/work_items/components/work_item_notes.vue @@ -97,7 +97,6 @@ export default { data() { return { isLoadingMore: false, - perPage: DEFAULT_PAGE_SIZE_NOTES, sortOrder: ASC, noteToDelete: null, discussionFilter: WORK_ITEM_NOTES_FILTER_ALL_NOTES, @@ -117,9 +116,6 @@ export default { hasNextPage() { return this.pageInfo?.hasNextPage; }, - showLoadingMoreSkeleton() { - return this.isLoadingMore && !this.changeNotesSortOrderAfterLoading; - }, disableActivityFilterSort() { return this.initialLoading || this.isLoadingMore; }, @@ -204,8 +200,6 @@ export default { this.$emit('error', i18n.fetchError); }, result() { - this.updateSortingOrderIfApplicable(); - if (this.hasNextPage) { this.fetchMoreNotes(); } else if (this.targetNoteHash) { @@ -268,17 +262,6 @@ export default { isSystemNote(note) { return note.notes.nodes[0].system; }, - updateSortingOrderIfApplicable() { - // when the sort order is DESC in local storage and there is only a single page, call - // changeSortOrder manually - if ( - this.changeNotesSortOrderAfterLoading && - this.perPage === DEFAULT_PAGE_SIZE_NOTES && - !this.hasNextPage - ) { - this.changeNotesSortOrder(DESC); - } - }, changeNotesSortOrder(direction) { this.sortOrder = direction; }, @@ -293,14 +276,10 @@ export default { }, async fetchMoreNotes() { this.isLoadingMore = true; - // copied from discussions batch logic - every fetchMore call has a higher - // amount of page size than the previous one with the limit being 100 - this.perPage = Math.min(Math.round(this.perPage * 1.5), 100); await this.$apollo.queries.workItemNotes .fetchMore({ variables: { ...this.queryVariables, - pageSize: this.perPage, after: this.pageInfo?.endCursor, }, }) @@ -429,7 +408,7 @@ export default { </div> </template> - <template v-if="showLoadingMoreSkeleton"> + <template v-if="isLoadingMore"> <gl-skeleton-loader v-for="index in $options.loader.repeat" :key="index" diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 89b7767aa40..d967aa89eb7 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -52,6 +52,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:hide_create_issue_resolve_all, project) push_frontend_feature_flag(:auto_merge_labels_mr_widget, project) push_frontend_feature_flag(:summarize_my_code_review, current_user) + push_frontend_feature_flag(:mr_activity_filters, current_user) end around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :diffs, :discussions] diff --git a/app/models/concerns/protected_ref_access.rb b/app/models/concerns/protected_ref_access.rb index f5c4ec0c3b3..964a862d415 100644 --- a/app/models/concerns/protected_ref_access.rb +++ b/app/models/concerns/protected_ref_access.rb @@ -47,7 +47,6 @@ module ProtectedRefAccess def check_access(user) return false unless user - return true if user.admin? user.can?(:push_code, project) && project.team.max_member_access(user.id) >= access_level diff --git a/app/models/note.rb b/app/models/note.rb index d2f2a71b027..597ba767a11 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -87,6 +87,7 @@ class Note < ApplicationRecord inverse_of: :note, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_one :system_note_metadata + has_one :note_metadata, inverse_of: :note, class_name: 'Notes::NoteMetadata' has_one :note_diff_file, inverse_of: :diff_note, foreign_key: :diff_note_id has_many :diff_note_positions @@ -95,6 +96,8 @@ class Note < ApplicationRecord delegate :name, :email, to: :author, prefix: true delegate :title, to: :noteable, allow_nil: true + accepts_nested_attributes_for :note_metadata + validates :note, presence: true validates :note, length: { maximum: Gitlab::Database::MAX_TEXT_SIZE_LIMIT } validates :project, presence: true, if: :for_project_noteable? diff --git a/app/models/notes/note_metadata.rb b/app/models/notes/note_metadata.rb new file mode 100644 index 00000000000..96e0917734b --- /dev/null +++ b/app/models/notes/note_metadata.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Notes + class NoteMetadata < ApplicationRecord + self.table_name = :note_metadata + + belongs_to :note, inverse_of: :note_metadata + validates :email_participant, length: { maximum: 255 } + + alias_attribute :external_author, :email_participant + end +end diff --git a/app/models/packages/dependency.rb b/app/models/packages/dependency.rb index ad3944b5f21..c39b46dcc20 100644 --- a/app/models/packages/dependency.rb +++ b/app/models/packages/dependency.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true class Packages::Dependency < ApplicationRecord + include EachBatch + has_many :dependency_links, class_name: 'Packages::DependencyLink' validates :name, :version_pattern, presence: true @@ -41,6 +43,11 @@ class Packages::Dependency < ApplicationRecord pluck(:id, :name) end + def self.orphaned + subquery = Packages::DependencyLink.where(Packages::DependencyLink.arel_table[:dependency_id].eq(Packages::Dependency.arel_table[:id])) + where_not_exists(subquery) + end + def orphaned? self.dependency_links.empty? end diff --git a/app/models/sent_notification.rb b/app/models/sent_notification.rb index 8a3449e8f7c..580e4cd277c 100644 --- a/app/models/sent_notification.rb +++ b/app/models/sent_notification.rb @@ -103,9 +103,18 @@ class SentNotification < ApplicationRecord self.reply_key end - def create_reply(message, dryrun: false) + def create_reply(message, external_author = nil, dryrun: false) klass = dryrun ? Notes::BuildService : Notes::CreateService - klass.new(self.project, self.recipient, reply_params.merge(note: message)).execute + params = reply_params.merge( + note: message + ) + + params[:external_author] = external_author if external_author.present? + + klass.new(self.project, + self.recipient, + params + ).execute end private diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index fc154e6b465..73e4cbee54a 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -81,7 +81,7 @@ module Ci # Authorizing the user to access to protected entities. # There is a "jailbreak" mode to exceptionally bypass the authorization, # however, you should NEVER allow it, rather suspect it's a wrong feature/product design. - rule { ~can?(:jailbreak) & (archived | protected_ref | protected_environment) }.policy do + rule { ~can?(:jailbreak) & (archived | (protected_ref & ~admin) | protected_environment) }.policy do prevent :update_build prevent :update_commit_status prevent :erase_build diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb index 679f829e852..e80b3be98bd 100644 --- a/app/serializers/note_entity.rb +++ b/app/serializers/note_entity.rb @@ -12,6 +12,8 @@ class NoteEntity < API::Entities::Note expose :type + expose :external_author + expose :author, using: NoteUserEntity unexpose :note, as: :body @@ -105,6 +107,18 @@ class NoteEntity < API::Entities::Note def with_base_discussion? options.fetch(:with_base_discussion, true) end + + def external_author + return unless Feature.enabled?(:external_note_author_service_desk, type: :ops) + + return unless object.note_metadata&.external_author + + if can?(current_user, :read_external_emails, object.project) + object.note_metadata.external_author + else + Gitlab::Utils::Email.obfuscated_email(object.note_metadata.external_author, deform: true) + end + end end NoteEntity.prepend_mod_with('NoteEntity') diff --git a/app/services/notes/build_service.rb b/app/services/notes/build_service.rb index e6766273441..91993700e25 100644 --- a/app/services/notes/build_service.rb +++ b/app/services/notes/build_service.rb @@ -4,8 +4,15 @@ module Notes class BuildService < ::BaseService def execute in_reply_to_discussion_id = params.delete(:in_reply_to_discussion_id) + external_author = params.delete(:external_author) + discussion = nil + if external_author.present? + note_metadata = Notes::NoteMetadata.new(email_participant: external_author) + params[:note_metadata] = note_metadata + end + if in_reply_to_discussion_id.present? discussion = find_discussion(in_reply_to_discussion_id) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 77b6bf573df..8d100f8b456 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -570,6 +570,15 @@ :weight: 1 :idempotent: false :tags: [] +- :name: cronjob:packages_cleanup_delete_orphaned_dependencies + :worker_name: Packages::Cleanup::DeleteOrphanedDependenciesWorker + :feature_category: :package_registry + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: cronjob:packages_cleanup_package_registry :worker_name: Packages::CleanupPackageRegistryWorker :feature_category: :package_registry diff --git a/app/workers/packages/cleanup/delete_orphaned_dependencies_worker.rb b/app/workers/packages/cleanup/delete_orphaned_dependencies_worker.rb new file mode 100644 index 00000000000..0b3d3c98742 --- /dev/null +++ b/app/workers/packages/cleanup/delete_orphaned_dependencies_worker.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +module Packages + module Cleanup + class DeleteOrphanedDependenciesWorker + include ApplicationWorker + include CronjobQueue # rubocop:disable Scalability/CronWorkerContext + + data_consistency :sticky + feature_category :package_registry + urgency :low + idempotent! + + # This cron worker is executed at an interval of 10 minutes and should not run for + # more than 2 minutes nor process more than 10 batches. + MAX_RUN_TIME = 2.minutes + MAX_BATCHES = 10 + BATCH_SIZE = 100 + LAST_PROCESSED_PACKAGES_DEPENDENCY_REDIS_KEY = 'last_processed_packages_dependency_id' + REDIS_EXPIRATION_TIME = 2.hours.to_i + + def perform + return unless enabled? + + start_time + + dependency_id = last_processed_dependency_id + batches_count = 0 + deleted_rows_count = 0 + + ::Packages::Dependency.id_in(dependency_id..).each_batch(of: BATCH_SIZE) do |batch| + batches_count += 1 + deleted_rows_count += batch.orphaned.delete_all + + if batches_count == MAX_BATCHES || over_time? + save_last_processed_dependency_id(batch.maximum(:id)) + break + end + end + + log_extra_metadata(deleted_rows_count) + reset_last_processed_dependency_id if batches_count < MAX_BATCHES && !over_time? + end + + private + + def enabled? + Feature.enabled?(:packages_delete_orphaned_dependencies_worker) + end + + def start_time + @start_time ||= ::Gitlab::Metrics::System.monotonic_time + end + + def over_time? + (::Gitlab::Metrics::System.monotonic_time - start_time) > MAX_RUN_TIME + end + + def save_last_processed_dependency_id(dependency_id) + with_redis do |redis| + redis.set(LAST_PROCESSED_PACKAGES_DEPENDENCY_REDIS_KEY, dependency_id, ex: REDIS_EXPIRATION_TIME) + end + end + + def last_processed_dependency_id + with_redis do |redis| + redis.get(LAST_PROCESSED_PACKAGES_DEPENDENCY_REDIS_KEY).to_i + end + end + + def reset_last_processed_dependency_id + with_redis do |redis| + redis.del(LAST_PROCESSED_PACKAGES_DEPENDENCY_REDIS_KEY) + end + end + + def with_redis(&block) + Gitlab::Redis::SharedState.with(&block) # rubocop:disable CodeReuse/ActiveRecord + end + + def log_extra_metadata(deleted_rows_count) + log_extra_metadata_on_done(:last_processed_packages_dependency_id, last_processed_dependency_id) + log_extra_metadata_on_done(:deleted_rows_count, deleted_rows_count) + end + end + end +end diff --git a/config/feature_flags/development/mr_activity_filters.yml b/config/feature_flags/development/mr_activity_filters.yml new file mode 100644 index 00000000000..fcad25e3ba8 --- /dev/null +++ b/config/feature_flags/development/mr_activity_filters.yml @@ -0,0 +1,8 @@ +--- +name: mr_activity_filters +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115383 +rollout_issue_url: +milestone: '15.11' +type: development +group: group::code review +default_enabled: false diff --git a/config/feature_flags/development/packages_delete_orphaned_dependencies_worker.yml b/config/feature_flags/development/packages_delete_orphaned_dependencies_worker.yml new file mode 100644 index 00000000000..2966d6b9bc3 --- /dev/null +++ b/config/feature_flags/development/packages_delete_orphaned_dependencies_worker.yml @@ -0,0 +1,8 @@ +--- +name: packages_delete_orphaned_dependencies_worker +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113076 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/409458 +milestone: '16.0' +type: development +group: group::package registry +default_enabled: false diff --git a/config/feature_flags/ops/external_note_author_service_desk.yml b/config/feature_flags/ops/external_note_author_service_desk.yml new file mode 100644 index 00000000000..044dc59262b --- /dev/null +++ b/config/feature_flags/ops/external_note_author_service_desk.yml @@ -0,0 +1,8 @@ +--- +name: external_note_author_service_desk +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117149 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408932 +milestone: '16.0' +type: ops +group: group::respond +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 5b2d7e6793d..6b41b32e15c 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -680,6 +680,9 @@ Settings.cron_jobs['users_migrate_records_to_ghost_user_in_batches_worker']['job Settings.cron_jobs['ci_runners_stale_machines_cleanup_worker'] ||= {} Settings.cron_jobs['ci_runners_stale_machines_cleanup_worker']['cron'] ||= '36 4 * * *' Settings.cron_jobs['ci_runners_stale_machines_cleanup_worker']['job_class'] = 'Ci::Runners::StaleMachinesCleanupCronWorker' +Settings.cron_jobs['packages_cleanup_delete_orphaned_dependencies_worker'] ||= {} +Settings.cron_jobs['packages_cleanup_delete_orphaned_dependencies_worker']['cron'] ||= '*/10 * * * *' +Settings.cron_jobs['packages_cleanup_delete_orphaned_dependencies_worker']['job_class'] = 'Packages::Cleanup::DeleteOrphanedDependenciesWorker' Settings.cron_jobs['cleanup_dangling_debian_package_files_worker'] ||= {} Settings.cron_jobs['cleanup_dangling_debian_package_files_worker']['cron'] ||= '20 21 * * *' Settings.cron_jobs['cleanup_dangling_debian_package_files_worker']['job_class'] = 'Packages::Debian::CleanupDanglingPackageFilesWorker' diff --git a/data/deprecations/15-6-deprecate-post-api-v4-runner.yml b/data/deprecations/15-6-deprecate-post-api-v4-runner.yml index b841451b1d1..8c2931faf78 100644 --- a/data/deprecations/15-6-deprecate-post-api-v4-runner.yml +++ b/data/deprecations/15-6-deprecate-post-api-v4-runner.yml @@ -9,15 +9,18 @@ body: | # (required) Do not modify this line, instead modify the lines below. The support for registration tokens and certain runner configuration arguments in the `POST` method operation on the `/api/v4/runners` endpoint is deprecated. This endpoint [registers](https://docs.gitlab.com/ee/api/runners.html#register-a-new-runner) a runner - with a GitLab instance at the instance, group, or project level through the API. We plan to remove the support for - registration tokens and certain configuration arguments in this endpoint in GitLab 17.0. + with a GitLab instance at the instance, group, or project level through the API. Registration tokens, and support for certain configuration arguments, + will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for authentication tokens are: - We plan to implement a new method to bind runners to a GitLab instance - as part of the new [GitLab Runner token architecture](https://docs.gitlab.com/ee/architecture/blueprints/runner_tokens/). - The work is planned in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/7633). - This new architecture introduces a new method for registering runners and will eliminate the legacy - [runner registration token](https://docs.gitlab.com/ee/security/token_overview.html#runner-registration-tokens). - From GitLab 17.0 and later, the runner registration methods implemented by the new GitLab Runner token architecture will be the only supported methods. + - `--locked` + - `--access-level` + - `--run-untagged` + - `--maximum-timeout` + - `--paused` + - `--tag-list` + - `--maintenance-note` + + This change is a breaking change. You should [create a runner in the UI](../ci/runners/register_runner.md) to add configurations, and use the authentication token in the `gitlab-runner register` command instead. end_of_support_milestone: "17.0" # (optional) Use "XX.YY" format. The milestone when support for this feature will end. tiers: # (optional - may be required in the future) An array of tiers that the feature is available in currently. e.g., [Free, Silver, Gold, Core, Premium, Ultimate] documentation_url: https://docs.gitlab.com/ee/api/runners.html#register-a-new-runner # (optional) This is a link to the current documentation page diff --git a/data/deprecations/15-6-deprecate-runner-register-command.yml b/data/deprecations/15-6-deprecate-runner-register-command.yml index 1311451abb8..cb9b9f517cd 100644 --- a/data/deprecations/15-6-deprecate-runner-register-command.yml +++ b/data/deprecations/15-6-deprecate-runner-register-command.yml @@ -7,11 +7,17 @@ stage: Verify # (required) String value of the stage that the feature was created in. e.g., Growth issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/380872 # (required) Link to the deprecation issue in GitLab body: | # (required) Do not modify this line, instead modify the lines below. - The support for registration tokens and certain configuration arguments in the command to [register](https://docs.gitlab.com/runner/register/) a runner, `gitlab-runner register` is deprecated. - We plan to implement a new method to bind runners to a GitLab instance - as part of the new [GitLab Runner token architecture](https://docs.gitlab.com/ee/architecture/blueprints/runner_tokens/). - The work is planned in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/7633). - The new method will involve creating the runner in the GitLab UI and passing the - [runner authentication token](https://docs.gitlab.com/ee/security/token_overview.html#runner-authentication-tokens-also-called-runner-tokens) - to the `gitlab-runner register` command. + Registration tokens and certain configuration arguments in the command `gitlab-runner register` that [registers](https://docs.gitlab.com/runner/register/) a runner, are deprecated. + Authentication tokens will be used to register runners instead. Registration tokens, and support for certain configuration arguments, + will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for authentication tokens are: + + - `--locked` + - `--access-level` + - `--run-untagged` + - `--maximum-timeout` + - `--paused` + - `--tag-list` + - `--maintenance-note` + + This change is a breaking change. You should [create a runner in the UI](../ci/runners/register_runner.md) to add configurations, and use the authentication token in the `gitlab-runner register` command instead. end_of_support_milestone: "17.0" # (optional) Use "XX.YY" format. The milestone when support for this feature will end. diff --git a/data/deprecations/15-6-deprecate-runner-register-token-k8s-operator.yml b/data/deprecations/15-6-deprecate-runner-register-token-k8s-operator.yml index 56d40620047..da184cfbe43 100644 --- a/data/deprecations/15-6-deprecate-runner-register-token-k8s-operator.yml +++ b/data/deprecations/15-6-deprecate-runner-register-token-k8s-operator.yml @@ -7,10 +7,15 @@ stage: Verify # (required) String value of the stage that the feature was created in. e.g., Growth issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/382077 # (required) Link to the deprecation issue in GitLab body: | # (required) Do not modify this line, instead modify the lines below. - The [`runner-registration-token`](https://docs.gitlab.com/runner/install/operator.html#install-the-kubernetes-operator) parameter that uses the OpenShift and k8s Vanilla Operator to install a runner on Kubernetes is deprecated. - We plan to implement a new method to bind runners to a GitLab instance - as part of the new [GitLab Runner token architecture](https://docs.gitlab.com/ee/architecture/blueprints/runner_tokens/). - The work is planned in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/7633). + The [`runner-registration-token`](https://docs.gitlab.com/runner/install/operator.html#install-the-kubernetes-operator) parameter that uses the OpenShift and Kubernetes Vanilla Operator to install a runner on Kubernetes is deprecated. Authentication tokens will be used to register runners instead. Registration tokens, and support for certain configuration arguments, + will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for authentication tokens are: + + - `--locked` + - `--access-level` + - `--run-untagged` + - `--tag-list` + + This change is a breaking change. You should use an [authentication token](../ci/runners/register_runner.md) in the `gitlab-runner register` command instead. end_of_support_milestone: "17.0" # (optional) Use "XX.YY" format. The milestone when support for this feature will end. tiers: # (optional - may be required in the future) An array of tiers that the feature is available in currently. e.g., [Free, Silver, Gold, Core, Premium, Ultimate] documentation_url: https://docs.gitlab.com/runner/install/operator.html#install-the-kubernetes-operator # (optional) This is a link to the current documentation page diff --git a/db/docs/note_metadata.yml b/db/docs/note_metadata.yml new file mode 100644 index 00000000000..63527152abe --- /dev/null +++ b/db/docs/note_metadata.yml @@ -0,0 +1,14 @@ +--- +table_name: note_metadata +classes: +- Notes::NoteMetadata +feature_categories: +- code_review_workflow +- portfolio_management +- service_desk +- source_code_management +- team_planning +description: Store any extra metadata for notes +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117149 +milestone: '16.0' +gitlab_schema: gitlab_main diff --git a/db/migrate/20230421035557_create_note_metadata.rb b/db/migrate/20230421035557_create_note_metadata.rb new file mode 100644 index 00000000000..fd15d8510d1 --- /dev/null +++ b/db/migrate/20230421035557_create_note_metadata.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CreateNoteMetadata < Gitlab::Database::Migration[2.1] + enable_lock_retries! + + def change + create_table :note_metadata, id: false do |t| + t.references :note, + primary_key: true, + null: false, + type: :bigint, + index: true, + foreign_key: { on_delete: :cascade } + t.text :email_participant, null: true, limit: 255 + t.timestamps_with_timezone null: true + end + end +end diff --git a/db/schema_migrations/20230421035557 b/db/schema_migrations/20230421035557 new file mode 100644 index 00000000000..5e4f51509be --- /dev/null +++ b/db/schema_migrations/20230421035557 @@ -0,0 +1 @@ +d685a5657a16728099225cb8f1545e09b317dc1608521d5df1272160cce46ddc
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index f67a3c225ec..7f07a820220 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18969,6 +18969,23 @@ CREATE SEQUENCE note_diff_files_id_seq ALTER SEQUENCE note_diff_files_id_seq OWNED BY note_diff_files.id; +CREATE TABLE note_metadata ( + note_id bigint NOT NULL, + email_participant text, + created_at timestamp with time zone, + updated_at timestamp with time zone, + CONSTRAINT check_40aa5ff1c6 CHECK ((char_length(email_participant) <= 255)) +); + +CREATE SEQUENCE note_metadata_note_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE note_metadata_note_id_seq OWNED BY note_metadata.note_id; + CREATE TABLE notes ( id integer NOT NULL, note text, @@ -25448,6 +25465,8 @@ ALTER TABLE ONLY namespaces_sync_events ALTER COLUMN id SET DEFAULT nextval('nam ALTER TABLE ONLY note_diff_files ALTER COLUMN id SET DEFAULT nextval('note_diff_files_id_seq'::regclass); +ALTER TABLE ONLY note_metadata ALTER COLUMN note_id SET DEFAULT nextval('note_metadata_note_id_seq'::regclass); + ALTER TABLE ONLY notes ALTER COLUMN id SET DEFAULT nextval('notes_id_seq'::regclass); ALTER TABLE ONLY notification_settings ALTER COLUMN id SET DEFAULT nextval('notification_settings_id_seq'::regclass); @@ -27668,6 +27687,9 @@ ALTER TABLE ONLY namespaces_sync_events ALTER TABLE ONLY note_diff_files ADD CONSTRAINT note_diff_files_pkey PRIMARY KEY (id); +ALTER TABLE ONLY note_metadata + ADD CONSTRAINT note_metadata_pkey PRIMARY KEY (note_id); + ALTER TABLE ONLY notes ADD CONSTRAINT notes_pkey PRIMARY KEY (id); @@ -31630,6 +31652,8 @@ CREATE INDEX index_non_requested_project_members_on_source_id_and_type ON member CREATE UNIQUE INDEX index_note_diff_files_on_diff_note_id ON note_diff_files USING btree (diff_note_id); +CREATE INDEX index_note_metadata_on_note_id ON note_metadata USING btree (note_id); + CREATE INDEX index_notes_for_cherry_picked_merge_requests ON notes USING btree (project_id, commit_id) WHERE ((noteable_type)::text = 'MergeRequest'::text); CREATE INDEX index_notes_on_author_id_and_created_at_and_id ON notes USING btree (author_id, created_at, id); @@ -37134,6 +37158,9 @@ ALTER TABLE ONLY packages_rpm_repository_files ALTER TABLE ONLY packages_rpm_metadata ADD CONSTRAINT fk_rails_d79f02264b FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE; +ALTER TABLE ONLY note_metadata + ADD CONSTRAINT fk_rails_d853224d37 FOREIGN KEY (note_id) REFERENCES notes(id) ON DELETE CASCADE; + ALTER TABLE ONLY merge_request_reviewers ADD CONSTRAINT fk_rails_d9fec24b9d FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; diff --git a/doc/api/merge_request_approvals.md b/doc/api/merge_request_approvals.md index 1d0397aaa2b..796971e20eb 100644 --- a/doc/api/merge_request_approvals.md +++ b/doc/api/merge_request_approvals.md @@ -972,11 +972,11 @@ Supported attributes: | Attribute | Type | Required | Description | |------------------------|-------------------|------------------------|-------------------------------------------------------------------------------| | `id` | integer or string | **{check-circle}** Yes | The ID or [URL-encoded path of a project](rest/index.md#namespaced-path-encoding). | -| `approvals_required` | integer | **{check-circle}** Yes | The number of required approvals for this rule. | | `approval_rule_id` | integer | **{check-circle}** Yes | The ID of an approval rule. | | `merge_request_iid` | integer | **{check-circle}** Yes | The IID of a merge request. | -| `name` | string | **{check-circle}** Yes | The name of the approval rule. | +| `approvals_required` | integer | **{check-circle}** No | The number of required approvals for this rule. | | `group_ids` | Array | **{dotted-circle}** No | The IDs of groups as approvers. | +| `name` | string | **{check-circle}** No | The name of the approval rule. | | `remove_hidden_groups` | boolean | **{dotted-circle}** No | Whether hidden groups should be removed. | | `user_ids` | Array | **{dotted-circle}** No | The IDs of users as approvers. | | `usernames` | string array | **{dotted-circle}** No | The usernames for this rule. | diff --git a/doc/ci/runners/register_runner.md b/doc/ci/runners/register_runner.md index 7b22e6215ba..21f72552ce3 100644 --- a/doc/ci/runners/register_runner.md +++ b/doc/ci/runners/register_runner.md @@ -47,11 +47,11 @@ and is then saved in `config.toml`. ## Generate a registration token (deprecated) WARNING: -The ability to pass a runner registration token was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/380872) in GitLab 15.6 and is -planned for removal in 17.0, along with support for certain configuration arguments. This change is a breaking change. GitLab plans to introduce a new -[GitLab Runner token architecture](../../architecture/blueprints/runner_tokens/index.md), which introduces -a new method for registering runners and eliminates the legacy -[runner registration token](../../security/token_overview.md#runner-registration-tokens-deprecated). +The ability to pass a runner registration token, and support for certain configuration arguments was +[deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/380872) in GitLab 15.6. Authentication tokens +will be used instead to register runners. Registration tokens, and support for certain configuration arguments +will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for `glrt-` tokens are `--locked`, `--access-level`, `--run-untagged`, `--maximum-timeout`, `--paused`, `--tag-list`, and `--maintenance-note`. This change is a breaking +change. ### For a shared runner diff --git a/doc/integration/jira/development_panel.md b/doc/integration/jira/development_panel.md index f8c37394f85..e4fe94476bd 100644 --- a/doc/integration/jira/development_panel.md +++ b/doc/integration/jira/development_panel.md @@ -45,9 +45,9 @@ The Jira development panel connects a Jira instance with all its projects to the ## Information displayed in the development panel -You can [view GitLab activity for a Jira issue](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/) in the Jira development panel by referring to the Jira issue by ID in GitLab. - -The information displayed in the development panel depends on where you mention the Jira issue ID in GitLab. +You can [view GitLab activity for a Jira issue](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/) +in the Jira development panel by referring to the Jira issue by ID in GitLab. The information displayed in the development panel +depends on where you mention the Jira issue ID in GitLab. | GitLab: where you mention the Jira issue ID | Jira development panel: what information is displayed | |------------------------------------------------|-------------------------------------------------------| @@ -70,8 +70,6 @@ Jira Smart Commits are special commands to process a Jira issue. With these comm - Log time against a Jira issue. - Transition a Jira issue to any status defined in the project workflow. -### Smart Commit syntax - Smart Commits must follow this syntax: ```plaintext @@ -80,6 +78,8 @@ Smart Commits must follow this syntax: You can execute one or more commands in a single commit. +### Smart Commit syntax + | Commands | Syntax | |-------------------------------------------------|--------------------------------------------------------------| | Add a comment | `KEY-123 #comment Bug is fixed` | diff --git a/doc/integration/jira/dvcs/index.md b/doc/integration/jira/dvcs/index.md index ceaa6255047..dc2b488e043 100644 --- a/doc/integration/jira/dvcs/index.md +++ b/doc/integration/jira/dvcs/index.md @@ -26,15 +26,14 @@ If you're on Jira Cloud, migrate to the GitLab for Jira Cloud app. For more info ### Create a GitLab application for DVCS -For projects in a single group, you should create a [group application](../../oauth_provider.md#create-a-group-owned-application). +- **For projects in a single group**, you should create a [group application](../../oauth_provider.md#create-a-group-owned-application). +- **For projects across multiple groups**, you should create a separate GitLab user account for Jira integration work only. + This account ensures regular maintenance does not affect your integration. +- **If you cannot create a group application or separate user account**, you can create instead: + - [An instance-wide application](../../oauth_provider.md#create-an-instance-wide-application) + - [A user-owned application](../../oauth_provider.md#create-a-user-owned-application) -For projects across multiple groups, you should create a new user account in GitLab for Jira integration work only. -A separate account ensures regular account maintenance does not affect your integration. - -If it's not possible to create a separate user account or group application, you can set up this integration by creating: - -- [An instance-wide application](../../oauth_provider.md#create-an-instance-wide-application) -- [A user-owned application](../../oauth_provider.md#create-a-user-owned-application) +To create a GitLab application for DVCS: 1. Go to the [appropriate **Applications** section](../../oauth_provider.md). 1. In the **Name** text box, enter a descriptive name for the integration (for example, `Jira`). @@ -48,14 +47,14 @@ If it's not possible to create a separate user account or group application, you ### Configure Jira for DVCS -1. Go to your DVCS account: - - **For Jira Server**, select **Settings (gear) > Applications > DVCS accounts**. +To configure Jira for DVCS: + +1. Go to your DVCS account. **For Jira Server**, select **Settings (gear) > Applications > DVCS accounts**. 1. To create a new integration, for **Host**, select **GitLab** or **GitLab Self-Managed**. 1. For **Team or User Account**, enter the relative path of a top-level GitLab group that [the GitLab user](#create-a-gitlab-application-for-dvcs) can access. 1. In the **Host URL** text box, enter the appropriate URL. Replace `<gitlab.example.com>` with your GitLab instance domain. Use `https://<gitlab.example.com>`. - 1. For **Client ID**, use the [**Application ID** value](#create-a-gitlab-application-for-dvcs). 1. For **Client Secret**, use the [**Secret** value](#create-a-gitlab-application-for-dvcs). 1. Ensure that all other checkboxes are selected. diff --git a/doc/integration/jira/index.md b/doc/integration/jira/index.md index 17d68349cb1..2b6395f437b 100644 --- a/doc/integration/jira/index.md +++ b/doc/integration/jira/index.md @@ -12,8 +12,8 @@ If you want to continue to use Jira, you can integrate Jira with GitLab instead. ## Jira integrations -GitLab offers two types of Jira integrations. You can -use one or both depending on the capabilities you need. +GitLab offers two types of Jira integrations. You can use one or both integrations +[depending on the capabilities you need](#jira-integration-capabilities). ### Jira issue integration @@ -33,18 +33,20 @@ including related branches, commits, and merge requests. To configure the Jira d ## Jira integration capabilities +This table shows the capabilities available with the Jira issue integration and the Jira development panel: + | Capability | Jira issue integration | Jira development panel | |-|-|-| -| Mention a Jira issue ID in a GitLab commit or merge request, and a link to the Jira issue is created. | Yes. | No. | -| Mention a Jira issue ID in GitLab and the Jira issue shows the GitLab issue or merge request. | Yes. A Jira comment with the GitLab issue or MR title links to GitLab. The first mention is also added to the Jira issue under **Web links**. | Yes, in the issue's [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/). | -| Mention a Jira issue ID in a GitLab commit message and the Jira issue shows the commit message. | Yes. The entire commit message is displayed in the Jira issue as a comment and under **Web links**. Each message links back to the commit in GitLab. | Yes, in the issue's [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/) and optionally with a custom comment on the Jira issue using Jira [Smart Commits](https://confluence.atlassian.com/fisheye/using-smart-commits-960155400.html). | -| Mention a Jira issue ID in a GitLab branch name and the Jira issue shows the branch name. | No. | Yes, in the issue's [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/). | -| Add Jira time tracking to an issue. | No. | Yes. Time can be specified using Jira Smart Commits. | -| Use a Git commit or merge request to transition or close a Jira issue. | Yes. Only a single transition type, typically configured to close the issue by setting it to Done. | Yes. Transition to any state using Jira Smart Commits. | -| Display a list of [Jira issues](issues.md#view-jira-issues). | Yes. | No. | -| Create a Jira issue from a [vulnerability or finding](../../user/application_security/vulnerabilities/index.md#create-a-jira-issue-for-a-vulnerability). | Yes. | No. | -| Create a GitLab branch from a Jira issue. | No. | Yes, in the issue's [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/). | -| Mention a Jira issue ID in a GitLab merge request, and deployments are synced. | No. | Yes, in the issue's [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/). | +| Mention a Jira issue ID in a GitLab commit or merge request, and a link to the Jira issue is created | **{check-circle}** Yes | **{dotted-circle}** No | +| Mention a Jira issue ID in GitLab, and the Jira issue shows the GitLab issue or merge request | **{check-circle}** Yes, a Jira comment with the GitLab issue or merge request title links to GitLab. The first mention is also added to the Jira issue under **Web links** | **{check-circle}** Yes, in the issue's [development panel](https://support.atlassian.com/jira-software-cloud/docs/view-development-information-for-an-issue/) | +| Mention a Jira issue ID in a GitLab commit message, and the Jira issue shows the commit message | **{check-circle}** Yes, the entire commit message is displayed in the Jira issue as a comment and under **Web links**. Each message links back to the commit in GitLab | **{check-circle}** Yes, in the issue's development panel and optionally with a custom comment on the Jira issue by using [Jira Smart Commits](https://confluence.atlassian.com/fisheye/using-smart-commits-960155400.html) | +| Mention a Jira issue ID in a GitLab branch name, and the Jira issue shows the branch name | **{dotted-circle}** No | **{check-circle}** Yes, in the issue's development panel | +| Add time tracking to a Jira issue | **{dotted-circle}** No | **{check-circle}** Yes, time can be specified by using Jira Smart Commits | +| Use a Git commit or merge request to transition or close a Jira issue |**{check-circle}** Yes, only a single transition type. Typically configured to close the issue by setting it to **Done** | **{check-circle}** Yes, transition to any state by using Jira Smart Commits | +| [View a list of Jira issues](issues.md#view-jira-issues) | **{check-circle}** Yes | **{dotted-circle}** No | +| [Create a Jira issue for a vulnerability](../../user/application_security/vulnerabilities/index.md#create-a-jira-issue-for-a-vulnerability) | **{check-circle}** Yes | **{dotted-circle}** No | +| Create a GitLab branch from a Jira issue | **{dotted-circle}** No | **{check-circle}** Yes, in the issue's development panel | +| Mention a Jira issue ID in a GitLab merge request, and deployments are synced | **{dotted-circle}** No | **{check-circle}** Yes, in the issue's development panel | ## Privacy considerations diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index c7a190964c8..7b63cf2a103 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -231,10 +231,15 @@ are deprecated and will be removed from the GraphQL API. For installation instru - [Breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/) </div> -The [`runner-registration-token`](https://docs.gitlab.com/runner/install/operator.html#install-the-kubernetes-operator) parameter that uses the OpenShift and k8s Vanilla Operator to install a runner on Kubernetes is deprecated. -We plan to implement a new method to bind runners to a GitLab instance -as part of the new [GitLab Runner token architecture](https://docs.gitlab.com/ee/architecture/blueprints/runner_tokens/). -The work is planned in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/7633). +The [`runner-registration-token`](https://docs.gitlab.com/runner/install/operator.html#install-the-kubernetes-operator) parameter that uses the OpenShift and Kubernetes Vanilla Operator to install a runner on Kubernetes is deprecated. Authentication tokens will be used to register runners instead. Registration tokens, and support for certain configuration arguments, +will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for authentication tokens are: + +- `--locked` +- `--access-level` +- `--run-untagged` +- `--tag-list` + +This change is a breaking change. You should use an [authentication token](../ci/runners/register_runner.md) in the `gitlab-runner register` command instead. </div> @@ -294,15 +299,18 @@ While the above approach is recommended for most instances, Sidekiq can also be The support for registration tokens and certain runner configuration arguments in the `POST` method operation on the `/api/v4/runners` endpoint is deprecated. This endpoint [registers](https://docs.gitlab.com/ee/api/runners.html#register-a-new-runner) a runner -with a GitLab instance at the instance, group, or project level through the API. We plan to remove the support for -registration tokens and certain configuration arguments in this endpoint in GitLab 17.0. +with a GitLab instance at the instance, group, or project level through the API. Registration tokens, and support for certain configuration arguments, +will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for authentication tokens are: -We plan to implement a new method to bind runners to a GitLab instance -as part of the new [GitLab Runner token architecture](https://docs.gitlab.com/ee/architecture/blueprints/runner_tokens/). -The work is planned in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/7633). -This new architecture introduces a new method for registering runners and will eliminate the legacy -[runner registration token](https://docs.gitlab.com/ee/security/token_overview.html#runner-registration-tokens). -From GitLab 17.0 and later, the runner registration methods implemented by the new GitLab Runner token architecture will be the only supported methods. +- `--locked` +- `--access-level` +- `--run-untagged` +- `--maximum-timeout` +- `--paused` +- `--tag-list` +- `--maintenance-note` + +This change is a breaking change. You should [create a runner in the UI](../ci/runners/register_runner.md) to add configurations, and use the authentication token in the `gitlab-runner register` command instead. </div> @@ -316,13 +324,19 @@ From GitLab 17.0 and later, the runner registration methods implemented by the n - [Breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/) </div> -The support for registration tokens and certain configuration arguments in the command to [register](https://docs.gitlab.com/runner/register/) a runner, `gitlab-runner register` is deprecated. -We plan to implement a new method to bind runners to a GitLab instance -as part of the new [GitLab Runner token architecture](https://docs.gitlab.com/ee/architecture/blueprints/runner_tokens/). -The work is planned in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/7633). -The new method will involve creating the runner in the GitLab UI and passing the -[runner authentication token](https://docs.gitlab.com/ee/security/token_overview.html#runner-authentication-tokens-also-called-runner-tokens) -to the `gitlab-runner register` command. +Registration tokens and certain configuration arguments in the command `gitlab-runner register` that [registers](https://docs.gitlab.com/runner/register/) a runner, are deprecated. +Authentication tokens will be used to register runners instead. Registration tokens, and support for certain configuration arguments, +will be disabled behind a feature flag in GitLab 16.6 and removed in GitLab 17.0. The configuration arguments disabled for authentication tokens are: + +- `--locked` +- `--access-level` +- `--run-untagged` +- `--maximum-timeout` +- `--paused` +- `--tag-list` +- `--maintenance-note` + +This change is a breaking change. You should [create a runner in the UI](../ci/runners/register_runner.md) to add configurations, and use the authentication token in the `gitlab-runner register` command instead. </div> diff --git a/doc/user/project/protected_branches.md b/doc/user/project/protected_branches.md index 307a17a53d1..8d4d16ffbc3 100644 --- a/doc/user/project/protected_branches.md +++ b/doc/user/project/protected_branches.md @@ -27,7 +27,7 @@ When a branch is protected, the default behavior enforces these restrictions on | Action | Who can do it | |:-------------------------|:------------------------------------------------------------------| | Protect a branch | At least the Maintainer role. | -| Push to the branch | GitLab administrators and anyone with **Allowed** permission. (1) | +| Push to the branch | Anyone with **Allowed** permission. (1) | | Force push to the branch | No one. | | Delete the branch | No one. (2) | diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb index b168efaac11..e6c64e2b1d6 100644 --- a/lib/gitlab/email/handler/create_note_handler.rb +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -50,7 +50,9 @@ module Gitlab end def create_note - sent_notification.create_reply(note_message) + external_author = from_address if author == User.support_bot + + sent_notification.create_reply(note_message, external_author) end def note_message diff --git a/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing.rb b/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing.rb index f02a7430b6b..851750163af 100644 --- a/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing.rb +++ b/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing.rb @@ -5,50 +5,49 @@ module Gitlab module Subscriptions class ActionCableWithLoadBalancing < ::GraphQL::Subscriptions::ActionCableSubscriptions extend ::Gitlab::Utils::Override + include Gitlab::Database::LoadBalancing::WalTrackingSender include Gitlab::Database::LoadBalancing::WalTrackingReceiver - def initialize(**options) - super(serializer: WalInjectingSerializer.new, **options) + KEY_PAYLOAD = 'gql_payload' + KEY_WAL_LOCATIONS = 'wal_locations' + + override :execute_all + def execute_all(event, object) + super(event, { + KEY_WAL_LOCATIONS => current_wal_locations, + KEY_PAYLOAD => object + }) end # We fall back to the primary in case no replica is sufficiently caught up. override :execute_update def execute_update(subscription_id, event, object) - ::Gitlab::Database::LoadBalancing::Session.current.use_primary! if use_primary? + # Make sure we do not accidentally try to unwrap messages that are not wrapped. + # This could in theory happen if workers roll over where some send wrapped payload + # and others expect the original payload. + return super(subscription_id, event, object) unless wrapped_payload?(object) - super + wal_locations = object[KEY_WAL_LOCATIONS] + ::Gitlab::Database::LoadBalancing::Session.current.use_primary! if use_primary?(wal_locations) + + super(subscription_id, event, object[KEY_PAYLOAD]) end private - def use_primary? - @serializer.wal_locations.blank? || !databases_in_sync?(@serializer.wal_locations) + def wrapped_payload?(object) + object.try(:key?, KEY_PAYLOAD) end - end - - class WalInjectingSerializer - include Gitlab::Database::LoadBalancing::WalTrackingSender - - DEFAULT_SERIALIZER = GraphQL::Subscriptions::Serialize - - attr_reader :wal_locations - # rubocop: disable GitlabSecurity/PublicSend - def load(str) - value = Gitlab::Json.parse(str) - - @wal_locations = value['wal_locations'] - - DEFAULT_SERIALIZER.send(:load_value, value['payload']) + def use_primary?(wal_locations) + wal_locations.blank? || !databases_in_sync?(wal_locations) end - def dump(obj) - Gitlab::Json.dump({ - 'wal_locations' => wal_locations_by_db_name, - 'payload' => DEFAULT_SERIALIZER.send(:dump_value, obj) - }) + # We stringify keys since otherwise the graphql-ruby serializer will inject additional metadata + # to keep track of which keys used to be symbols. + def current_wal_locations + wal_locations_by_db_name&.stringify_keys end - # rubocop: enable GitlabSecurity/PublicSend end end end diff --git a/lib/sidebars/admin/menus/monitoring_menu.rb b/lib/sidebars/admin/menus/monitoring_menu.rb index 71a9d4b8a03..2da56e87144 100644 --- a/lib/sidebars/admin/menus/monitoring_menu.rb +++ b/lib/sidebars/admin/menus/monitoring_menu.rb @@ -44,7 +44,7 @@ module Sidebars title: _('Background Migrations'), link: admin_background_migrations_path, active_routes: { controller: 'background_migrations' }, - item_id: :usage_trends + item_id: :background_migrations ) end diff --git a/lib/sidebars/projects/menus/settings_menu.rb b/lib/sidebars/projects/menus/settings_menu.rb index 8b6d85e718d..5138c1534b8 100644 --- a/lib/sidebars/projects/menus/settings_menu.rb +++ b/lib/sidebars/projects/menus/settings_menu.rb @@ -168,7 +168,7 @@ module Sidebars title: _('Merge requests'), link: project_settings_merge_requests_path(context.project), active_routes: { path: 'projects/settings/merge_requests#show' }, - item_id: :merge_requests + item_id: context.is_super_sidebar ? :merge_request_settings : :merge_requests ) end end diff --git a/lib/sidebars/projects/super_sidebar_menus/analyze_menu.rb b/lib/sidebars/projects/super_sidebar_menus/analyze_menu.rb index 78a988fffaf..58b231a269c 100644 --- a/lib/sidebars/projects/super_sidebar_menus/analyze_menu.rb +++ b/lib/sidebars/projects/super_sidebar_menus/analyze_menu.rb @@ -23,7 +23,7 @@ module Sidebars :ci_cd_analytics, :repository_analytics, :code_review, - :merge_requests, + :merge_request_analytics, :issues, :insights, :model_experiments diff --git a/lib/sidebars/projects/super_sidebar_menus/operations_menu.rb b/lib/sidebars/projects/super_sidebar_menus/operations_menu.rb index a6279c8e116..64cf4aee9c5 100644 --- a/lib/sidebars/projects/super_sidebar_menus/operations_menu.rb +++ b/lib/sidebars/projects/super_sidebar_menus/operations_menu.rb @@ -22,7 +22,6 @@ module Sidebars :kubernetes, :terraform_states, :infrastructure_registry, - :activity, :google_cloud, :aws ].each { |id| add_item(::Sidebars::NilMenuItem.new(item_id: id)) } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index affd72a8def..5faf66858b5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1354,6 +1354,9 @@ msgstr "" msgid "(external link)" msgstr "" +msgid "(external participant)" +msgstr "" + msgid "(leave blank if you don't want to change it)" msgstr "" @@ -4385,6 +4388,9 @@ msgstr "" msgid "All Members" msgstr "" +msgid "All activity" +msgstr "" + msgid "All branch names must match %{link_start}this regular expression%{link_end}. If empty, any branch name is allowed." msgstr "" @@ -5651,6 +5657,9 @@ msgstr "" msgid "ApprovalSettings|When a commit is added:" msgstr "" +msgid "Approvals" +msgstr "" + msgid "Approvals are optional." msgstr "" @@ -6163,6 +6172,9 @@ msgstr "" msgid "Assignees" msgstr "" +msgid "Assignees & reviewers" +msgstr "" + msgid "Assigns %{assignee_users_sentence}." msgstr "" @@ -6635,12 +6647,6 @@ msgstr "" msgid "Awaiting user signup" msgstr "" -msgid "Award added" -msgstr "" - -msgid "Award removed" -msgstr "" - msgid "AwardEmoji|No emojis found." msgstr "" @@ -10966,6 +10972,9 @@ msgstr "" msgid "Commits" msgstr "" +msgid "Commits & branches" +msgstr "" + msgid "Commits feed" msgstr "" @@ -16218,6 +16227,9 @@ msgstr "" msgid "Editing rich text" msgstr "" +msgid "Edits" +msgstr "" + msgid "Elapsed time" msgstr "" @@ -26705,6 +26717,9 @@ msgstr "" msgid "Lock not found" msgstr "" +msgid "Lock status" +msgstr "" + msgid "Lock the discussion" msgstr "" @@ -27606,6 +27621,9 @@ msgstr "" msgid "Memory Usage" msgstr "" +msgid "Mentions" +msgstr "" + msgid "Menu" msgstr "" @@ -27696,6 +27714,9 @@ msgstr "" msgid "Merge request reports" msgstr "" +msgid "Merge request status" +msgstr "" + msgid "Merge request unlocked." msgstr "" @@ -46992,6 +47013,9 @@ msgstr "" msgid "Track time with quick actions" msgstr "" +msgid "Tracking" +msgstr "" + msgid "Training mode" msgstr "" diff --git a/spec/factories/notes/notes_metadata.rb b/spec/factories/notes/notes_metadata.rb new file mode 100644 index 00000000000..555debbc0e5 --- /dev/null +++ b/spec/factories/notes/notes_metadata.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :note_metadata, class: 'Notes::NoteMetadata' do + note + email_participant { 'email@example.com' } + end +end diff --git a/spec/fixtures/api/schemas/entities/discussion.json b/spec/fixtures/api/schemas/entities/discussion.json index 4af36b5814b..f91571fbc48 100644 --- a/spec/fixtures/api/schemas/entities/discussion.json +++ b/spec/fixtures/api/schemas/entities/discussion.json @@ -194,6 +194,12 @@ "boolean", "null" ] + }, + "external_author": { + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/spec/frontend/admin/abuse_reports/components/abuse_report_actions_spec.js b/spec/frontend/admin/abuse_reports/components/abuse_report_actions_spec.js index 9708de69caa..571d01a2fb5 100644 --- a/spec/frontend/admin/abuse_reports/components/abuse_report_actions_spec.js +++ b/spec/frontend/admin/abuse_reports/components/abuse_report_actions_spec.js @@ -63,7 +63,7 @@ describe('AbuseReportActions', () => { const button = findBlockUserButton(); expect(button.text()).toBe(ACTIONS_I18N.alreadyBlocked); - expect(button.attributes('disabled')).toBe('disabled'); + expect(button.attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/admin/users/components/modals/delete_user_modal_spec.js b/spec/frontend/admin/users/components/modals/delete_user_modal_spec.js index efb951f4ad2..b2a0c201893 100644 --- a/spec/frontend/admin/users/components/modals/delete_user_modal_spec.js +++ b/spec/frontend/admin/users/components/modals/delete_user_modal_spec.js @@ -84,8 +84,8 @@ describe('Delete user modal', () => { }); it('has disabled buttons', () => { - expect(findPrimaryButton().attributes('disabled')).toBe('true'); - expect(findSecondaryButton().attributes('disabled')).toBe('true'); + expect(findPrimaryButton().attributes('disabled')).toBeDefined(); + expect(findSecondaryButton().attributes('disabled')).toBeDefined(); }); }); @@ -102,8 +102,8 @@ describe('Delete user modal', () => { }); it('has disabled buttons', () => { - expect(findPrimaryButton().attributes('disabled')).toBe('true'); - expect(findSecondaryButton().attributes('disabled')).toBe('true'); + expect(findPrimaryButton().attributes('disabled')).toBeDefined(); + expect(findSecondaryButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/alerts_settings/components/alerts_settings_form_spec.js b/spec/frontend/alerts_settings/components/alerts_settings_form_spec.js index b8575d8ab26..5eb5ae2f783 100644 --- a/spec/frontend/alerts_settings/components/alerts_settings_form_spec.js +++ b/spec/frontend/alerts_settings/components/alerts_settings_form_spec.js @@ -99,7 +99,7 @@ describe('AlertsSettingsForm', () => { it('disables the dropdown and shows help text when multi integrations are not supported', () => { createComponent({ props: { canAddIntegration: false } }); - expect(findSelect().attributes('disabled')).toBe('disabled'); + expect(findSelect().attributes('disabled')).toBeDefined(); expect(findMultiSupportText().exists()).toBe(true); }); @@ -433,13 +433,13 @@ describe('AlertsSettingsForm', () => { it('should not be able to submit when no integration type is selected', async () => { await selectOptionAtIndex(0); - expect(findSubmitButton().attributes('disabled')).toBe('disabled'); + expect(findSubmitButton().attributes('disabled')).toBeDefined(); }); it('should not be able to submit when HTTP integration form is invalid', async () => { await selectOptionAtIndex(1); await findFormFields().at(0).vm.$emit('input', ''); - expect(findSubmitButton().attributes('disabled')).toBe('disabled'); + expect(findSubmitButton().attributes('disabled')).toBeDefined(); }); it('should be able to submit when HTTP integration form is valid', async () => { @@ -452,7 +452,7 @@ describe('AlertsSettingsForm', () => { await selectOptionAtIndex(2); await findFormFields().at(0).vm.$emit('input', ''); - expect(findSubmitButton().attributes('disabled')).toBe('disabled'); + expect(findSubmitButton().attributes('disabled')).toBeDefined(); }); it('should be able to submit when Prometheus integration form is valid', async () => { @@ -482,7 +482,7 @@ describe('AlertsSettingsForm', () => { }); await nextTick(); - expect(findSubmitButton().attributes('disabled')).toBe('disabled'); + expect(findSubmitButton().attributes('disabled')).toBeDefined(); }); it('should disable submit button after click on validation failure', async () => { @@ -490,7 +490,7 @@ describe('AlertsSettingsForm', () => { findSubmitButton().trigger('click'); await nextTick(); - expect(findSubmitButton().attributes('disabled')).toBe('disabled'); + expect(findSubmitButton().attributes('disabled')).toBeDefined(); }); it('should scroll to invalid field on validation failure', async () => { diff --git a/spec/frontend/blob/components/blob_header_default_actions_spec.js b/spec/frontend/blob/components/blob_header_default_actions_spec.js index 6e001846bb6..4c8c256121f 100644 --- a/spec/frontend/blob/components/blob_header_default_actions_spec.js +++ b/spec/frontend/blob/components/blob_header_default_actions_spec.js @@ -67,7 +67,7 @@ describe('Blob Header Default Actions', () => { }); buttons = wrapper.findAllComponents(GlButton); - expect(buttons.at(0).attributes('disabled')).toBe('true'); + expect(buttons.at(0).attributes('disabled')).toBeDefined(); }); it('does not render the copy button if a rendering error is set', () => { diff --git a/spec/frontend/boards/components/board_configuration_options_spec.js b/spec/frontend/boards/components/board_configuration_options_spec.js index d2948daf121..199a08c5d83 100644 --- a/spec/frontend/boards/components/board_configuration_options_spec.js +++ b/spec/frontend/boards/components/board_configuration_options_spec.js @@ -62,8 +62,8 @@ describe('BoardConfigurationOptions', () => { it('renders checkboxes disabled when user does not have edit rights', () => { createComponent({ readonly: true }); - expect(closedListCheckbox().attributes('disabled')).toBe('true'); - expect(backlogListCheckbox().attributes('disabled')).toBe('true'); + expect(closedListCheckbox().attributes('disabled')).toBeDefined(); + expect(backlogListCheckbox().attributes('disabled')).toBeDefined(); }); it('renders checkboxes enabled when user has edit rights', () => { diff --git a/spec/frontend/ci/artifacts/components/artifact_row_spec.js b/spec/frontend/ci/artifacts/components/artifact_row_spec.js index ee5d62c2d48..96ddedc3a9d 100644 --- a/spec/frontend/ci/artifacts/components/artifact_row_spec.js +++ b/spec/frontend/ci/artifacts/components/artifact_row_spec.js @@ -106,7 +106,7 @@ describe('ArtifactRow component', () => { props: { isSelected: false, isSelectedArtifactsLimitReached: true }, }); - expect(findCheckbox().attributes('disabled')).toBe('true'); + expect(findCheckbox().attributes('disabled')).toBeDefined(); expect(findCheckbox().attributes('title')).toBe(I18N_BULK_DELETE_MAX_SELECTED); }); }); diff --git a/spec/frontend/ci/artifacts/components/job_artifacts_table_spec.js b/spec/frontend/ci/artifacts/components/job_artifacts_table_spec.js index fe58f302a1d..514644a92f2 100644 --- a/spec/frontend/ci/artifacts/components/job_artifacts_table_spec.js +++ b/spec/frontend/ci/artifacts/components/job_artifacts_table_spec.js @@ -324,7 +324,7 @@ describe('JobArtifactsTable component', () => { await waitForPromises(); - expect(findDownloadButton().attributes('disabled')).toBe('disabled'); + expect(findDownloadButton().attributes('disabled')).toBeDefined(); }); }); @@ -350,7 +350,7 @@ describe('JobArtifactsTable component', () => { await waitForPromises(); - expect(findBrowseButton().attributes('disabled')).toBe('disabled'); + expect(findBrowseButton().attributes('disabled')).toBeDefined(); }); }); @@ -463,7 +463,7 @@ describe('JobArtifactsTable component', () => { await waitForPromises(); - expect(findDeleteButton().attributes('disabled')).toBe('disabled'); + expect(findDeleteButton().attributes('disabled')).toBeDefined(); }); it('is hidden when user does not have delete permission', async () => { diff --git a/spec/frontend/ci/artifacts/components/job_checkbox_spec.js b/spec/frontend/ci/artifacts/components/job_checkbox_spec.js index 56f1d8cc47b..8b47571239c 100644 --- a/spec/frontend/ci/artifacts/components/job_checkbox_spec.js +++ b/spec/frontend/ci/artifacts/components/job_checkbox_spec.js @@ -33,7 +33,7 @@ describe('JobCheckbox component', () => { it('is disabled when the job has no artifacts', () => { createComponent({ hasArtifacts: false }); - expect(findCheckbox().attributes('disabled')).toBe('true'); + expect(findCheckbox().attributes('disabled')).toBeDefined(); }); describe('when some artifacts from this job are selected', () => { @@ -124,7 +124,7 @@ describe('JobCheckbox component', () => { it('is disabled when the selected artifacts limit has been reached', () => { // job checkbox is disabled to block further selection - expect(findCheckbox().attributes('disabled')).toBe('true'); + expect(findCheckbox().attributes('disabled')).toBeDefined(); expect(findCheckbox().attributes('title')).toBe(I18N_BULK_DELETE_MAX_SELECTED); }); }); diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js index e8bfb370fb4..b6ffde9b33f 100644 --- a/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js @@ -95,7 +95,7 @@ describe('Ci variable modal', () => { }); it('shows the submit button as disabled', () => { - expect(findAddorUpdateButton().attributes('disabled')).toBe('true'); + expect(findAddorUpdateButton().attributes('disabled')).toBeDefined(); }); }); @@ -507,7 +507,7 @@ describe('Ci variable modal', () => { }); it('disables the submit button', () => { - expect(findAddorUpdateButton().attributes('disabled')).toBe('disabled'); + expect(findAddorUpdateButton().attributes('disabled')).toBeDefined(); }); it('shows the correct error text', () => { diff --git a/spec/frontend/ci/pipeline_editor/components/commit/commit_form_spec.js b/spec/frontend/ci/pipeline_editor/components/commit/commit_form_spec.js index 03f346181e4..4b0ddacef93 100644 --- a/spec/frontend/ci/pipeline_editor/components/commit/commit_form_spec.js +++ b/spec/frontend/ci/pipeline_editor/components/commit/commit_form_spec.js @@ -93,7 +93,7 @@ describe('Pipeline Editor | Commit Form', () => { createComponent({ props: { hasUnsavedChanges, isNewCiConfigFile } }); if (isDisabled) { - expect(findSubmitBtn().attributes('disabled')).toBe('true'); + expect(findSubmitBtn().attributes('disabled')).toBeDefined(); } else { expect(findSubmitBtn().attributes('disabled')).toBeUndefined(); } @@ -132,7 +132,7 @@ describe('Pipeline Editor | Commit Form', () => { it('when the commit message is empty, submit button is disabled', async () => { await findCommitTextarea().setValue(''); - expect(findSubmitBtn().attributes('disabled')).toBe('disabled'); + expect(findSubmitBtn().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/ci/runner/admin_runners/admin_runners_app_spec.js b/spec/frontend/ci/runner/admin_runners/admin_runners_app_spec.js index 2cd1bc0b2f8..6fc6c0b6085 100644 --- a/spec/frontend/ci/runner/admin_runners/admin_runners_app_spec.js +++ b/spec/frontend/ci/runner/admin_runners/admin_runners_app_spec.js @@ -388,7 +388,7 @@ describe('AdminRunnersApp', () => { it('when runners have not loaded, shows a loading state', () => { createComponent(); expect(findRunnerList().props('loading')).toBe(true); - expect(findRunnerPagination().attributes('disabled')).toBe('true'); + expect(findRunnerPagination().attributes('disabled')).toBeDefined(); }); describe('Bulk delete', () => { diff --git a/spec/frontend/ci/runner/components/runner_jobs_spec.js b/spec/frontend/ci/runner/components/runner_jobs_spec.js index 365b0f1f5ba..367b9ce395d 100644 --- a/spec/frontend/ci/runner/components/runner_jobs_spec.js +++ b/spec/frontend/ci/runner/components/runner_jobs_spec.js @@ -99,7 +99,7 @@ describe('RunnerJobs', () => { expect(findGlSkeletonLoading().exists()).toBe(true); expect(findRunnerJobsTable().exists()).toBe(false); - expect(findRunnerPagination().attributes('disabled')).toBe('true'); + expect(findRunnerPagination().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/ci/runner/components/runner_projects_spec.js b/spec/frontend/ci/runner/components/runner_projects_spec.js index afdc54d8ebc..736a1f7d3ce 100644 --- a/spec/frontend/ci/runner/components/runner_projects_spec.js +++ b/spec/frontend/ci/runner/components/runner_projects_spec.js @@ -194,7 +194,7 @@ describe('RunnerProjects', () => { expect(wrapper.findByText(I18N_NO_PROJECTS_FOUND).exists()).toBe(false); expect(findRunnerAssignedItems().length).toBe(0); - expect(findRunnerPagination().attributes('disabled')).toBe('true'); + expect(findRunnerPagination().attributes('disabled')).toBeDefined(); expect(findGlSearchBoxByType().props('isLoading')).toBe(true); }); }); diff --git a/spec/frontend/ci/runner/group_runners/group_runners_app_spec.js b/spec/frontend/ci/runner/group_runners/group_runners_app_spec.js index 6824242cba9..5b2ddeafe04 100644 --- a/spec/frontend/ci/runner/group_runners/group_runners_app_spec.js +++ b/spec/frontend/ci/runner/group_runners/group_runners_app_spec.js @@ -391,7 +391,7 @@ describe('GroupRunnersApp', () => { it('when runners have not loaded, shows a loading state', () => { createComponent(); expect(findRunnerList().props('loading')).toBe(true); - expect(findRunnerPagination().attributes('disabled')).toBe('true'); + expect(findRunnerPagination().attributes('disabled')).toBeDefined(); }); it('runners can be deleted in bulk', () => { diff --git a/spec/frontend/clusters/agents/components/create_token_button_spec.js b/spec/frontend/clusters/agents/components/create_token_button_spec.js index 5a8906813cf..2bbde33d6f4 100644 --- a/spec/frontend/clusters/agents/components/create_token_button_spec.js +++ b/spec/frontend/clusters/agents/components/create_token_button_spec.js @@ -55,7 +55,7 @@ describe('CreateTokenButton', () => { }); it('disabled the button', () => { - expect(findButton().attributes('disabled')).toBe('true'); + expect(findButton().attributes('disabled')).toBeDefined(); }); it('shows a disabled tooltip', () => { diff --git a/spec/frontend/clusters/agents/components/create_token_modal_spec.js b/spec/frontend/clusters/agents/components/create_token_modal_spec.js index fe3e1844118..f0fded7b7b2 100644 --- a/spec/frontend/clusters/agents/components/create_token_modal_spec.js +++ b/spec/frontend/clusters/agents/components/create_token_modal_spec.js @@ -61,7 +61,7 @@ describe('CreateTokenModal', () => { const expectDisabledAttribute = (element, disabled) => { if (disabled) { - expect(element.attributes('disabled')).toBe('true'); + expect(element.attributes('disabled')).toBeDefined(); } else { expect(element.attributes('disabled')).toBeUndefined(); } diff --git a/spec/frontend/clusters/agents/components/revoke_token_button_spec.js b/spec/frontend/clusters/agents/components/revoke_token_button_spec.js index 76c32abb5e4..970782a8e58 100644 --- a/spec/frontend/clusters/agents/components/revoke_token_button_spec.js +++ b/spec/frontend/clusters/agents/components/revoke_token_button_spec.js @@ -119,7 +119,7 @@ describe('RevokeTokenButton', () => { }); it('disabled the button', () => { - expect(findRevokeBtn().attributes('disabled')).toBe('true'); + expect(findRevokeBtn().attributes('disabled')).toBeDefined(); }); it('shows a disabled tooltip', () => { @@ -217,7 +217,7 @@ describe('RevokeTokenButton', () => { it('reenables the button', async () => { expect(findPrimaryActionAttributes('loading')).toBe(true); - expect(findRevokeBtn().attributes('disabled')).toBe('true'); + expect(findRevokeBtn().attributes('disabled')).toBeDefined(); await findModal().vm.$emit('hide'); diff --git a/spec/frontend/clusters_list/components/delete_agent_button_spec.js b/spec/frontend/clusters_list/components/delete_agent_button_spec.js index 53cf67bca0f..2c9a6b11671 100644 --- a/spec/frontend/clusters_list/components/delete_agent_button_spec.js +++ b/spec/frontend/clusters_list/components/delete_agent_button_spec.js @@ -140,7 +140,7 @@ describe('DeleteAgentButton', () => { }); it('disables the button', () => { - expect(findDeleteBtn().attributes('disabled')).toBe('true'); + expect(findDeleteBtn().attributes('disabled')).toBeDefined(); }); it('shows a disabled tooltip', () => { @@ -230,7 +230,7 @@ describe('DeleteAgentButton', () => { it('reenables the button', async () => { expect(findPrimaryActionAttributes('loading')).toBe(true); - expect(findDeleteBtn().attributes('disabled')).toBe('true'); + expect(findDeleteBtn().attributes('disabled')).toBeDefined(); await findModal().vm.$emit('hide'); diff --git a/spec/frontend/clusters_list/components/install_agent_modal_spec.js b/spec/frontend/clusters_list/components/install_agent_modal_spec.js index f9009696c7b..e1306e2738f 100644 --- a/spec/frontend/clusters_list/components/install_agent_modal_spec.js +++ b/spec/frontend/clusters_list/components/install_agent_modal_spec.js @@ -74,7 +74,7 @@ describe('InstallAgentModal', () => { const expectDisabledAttribute = (element, disabled) => { if (disabled) { - expect(element.attributes('disabled')).toBe('true'); + expect(element.attributes('disabled')).toBeDefined(); } else { expect(element.attributes('disabled')).toBeUndefined(); } diff --git a/spec/frontend/deploy_freeze/components/deploy_freeze_modal_spec.js b/spec/frontend/deploy_freeze/components/deploy_freeze_modal_spec.js index 77118ae140a..1cd16e39417 100644 --- a/spec/frontend/deploy_freeze/components/deploy_freeze_modal_spec.js +++ b/spec/frontend/deploy_freeze/components/deploy_freeze_modal_spec.js @@ -48,7 +48,7 @@ describe('Deploy freeze modal', () => { describe('Basic interactions', () => { it('button is disabled when freeze period is invalid', () => { - expect(submitDeployFreezeButton().attributes('disabled')).toBe('true'); + expect(submitDeployFreezeButton().attributes('disabled')).toBeDefined(); }); }); @@ -88,7 +88,7 @@ describe('Deploy freeze modal', () => { }); it('disables the add deploy freeze button', () => { - expect(submitDeployFreezeButton().attributes('disabled')).toBe('true'); + expect(submitDeployFreezeButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/design_management/components/delete_button_spec.js b/spec/frontend/design_management/components/delete_button_spec.js index 81e3b21a910..cacda9a475e 100644 --- a/spec/frontend/design_management/components/delete_button_spec.js +++ b/spec/frontend/design_management/components/delete_button_spec.js @@ -30,7 +30,7 @@ describe('Batch delete button component', () => { it('renders disabled button when design is deleting', () => { createComponent({ isDeleting: true }); - expect(findButton().attributes('disabled')).toBe('true'); + expect(findButton().attributes('disabled')).toBeDefined(); }); it('emits `delete-selected-designs` event on modal ok click', async () => { diff --git a/spec/frontend/environments/environment_actions_spec.js b/spec/frontend/environments/environment_actions_spec.js index dcfefbb2072..b7e192839da 100644 --- a/spec/frontend/environments/environment_actions_spec.js +++ b/spec/frontend/environments/environment_actions_spec.js @@ -84,7 +84,7 @@ describe('EnvironmentActions Component', () => { it("should render a disabled action when it's not playable", () => { const dropdownItems = findDropdownItems(); const lastDropdownItem = dropdownItems.at(dropdownItems.length - 1); - expect(lastDropdownItem.find('button').attributes('disabled')).toBe('disabled'); + expect(lastDropdownItem.find('button').attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/error_tracking_settings/components/app_spec.js b/spec/frontend/error_tracking_settings/components/app_spec.js index 7a714cc1ebc..dfe9652073e 100644 --- a/spec/frontend/error_tracking_settings/components/app_spec.js +++ b/spec/frontend/error_tracking_settings/components/app_spec.js @@ -92,7 +92,7 @@ describe('error tracking settings app', () => { store.state.settingsLoading = true; await nextTick(); - expect(wrapper.find('.js-error-tracking-button').attributes('disabled')).toBe('true'); + expect(wrapper.find('.js-error-tracking-button').attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js b/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js index a12c25c6897..b75e2f653e9 100644 --- a/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js +++ b/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js @@ -152,7 +152,7 @@ describe('Configure Feature Flags Modal', () => { beforeEach(factory.bind(null, { isRotating: true })); it('should disable the project name input', () => { - expect(findProjectNameInput().attributes('disabled')).toBe('true'); + expect(findProjectNameInput().attributes('disabled')).toBeDefined(); }); }); }); diff --git a/spec/frontend/ide/components/commit_sidebar/new_merge_request_option_spec.js b/spec/frontend/ide/components/commit_sidebar/new_merge_request_option_spec.js index adc9a0f1421..ce26519abc9 100644 --- a/spec/frontend/ide/components/commit_sidebar/new_merge_request_option_spec.js +++ b/spec/frontend/ide/components/commit_sidebar/new_merge_request_option_spec.js @@ -66,7 +66,7 @@ describe('NewMergeRequestOption component', () => { }); it('disables the new MR checkbox', () => { - expect(findCheckbox().attributes('disabled')).toBe('true'); + expect(findCheckbox().attributes('disabled')).toBeDefined(); }); it('adds `is-disabled` class to the fieldset', () => { diff --git a/spec/frontend/ide/components/jobs/detail/scroll_button_spec.js b/spec/frontend/ide/components/jobs/detail/scroll_button_spec.js index eec1bd6b123..450c6cb357c 100644 --- a/spec/frontend/ide/components/jobs/detail/scroll_button_spec.js +++ b/spec/frontend/ide/components/jobs/detail/scroll_button_spec.js @@ -41,6 +41,6 @@ describe('IDE job log scroll button', () => { it('disables button when disabled is true', () => { createComponent({ disabled: true }); - expect(wrapper.find('button').attributes('disabled')).toBe('disabled'); + expect(wrapper.find('button').attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/ide/components/jobs/detail_spec.js b/spec/frontend/ide/components/jobs/detail_spec.js index 60e03a7b882..334501bbca7 100644 --- a/spec/frontend/ide/components/jobs/detail_spec.js +++ b/spec/frontend/ide/components/jobs/detail_spec.js @@ -119,8 +119,8 @@ describe('IDE jobs detail view', () => { await findBuildJobLog().trigger('scroll'); - expect(findScrollToBottomButton().attributes('disabled')).toBe('disabled'); - expect(findScrollToTopButton().attributes('disabled')).not.toBe('disabled'); + expect(findScrollToBottomButton().attributes('disabled')).toBeDefined(); + expect(findScrollToTopButton().attributes('disabled')).toBeUndefined(); }); it('keeps scroll at top when already at top', async () => { @@ -128,8 +128,8 @@ describe('IDE jobs detail view', () => { await findBuildJobLog().trigger('scroll'); - expect(findScrollToBottomButton().attributes('disabled')).not.toBe('disabled'); - expect(findScrollToTopButton().attributes('disabled')).toBe('disabled'); + expect(findScrollToBottomButton().attributes('disabled')).toBeUndefined(); + expect(findScrollToTopButton().attributes('disabled')).toBeDefined(); }); it('resets scroll when not at top or bottom', async () => { @@ -137,8 +137,8 @@ describe('IDE jobs detail view', () => { await findBuildJobLog().trigger('scroll'); - expect(findScrollToBottomButton().attributes('disabled')).not.toBe('disabled'); - expect(findScrollToTopButton().attributes('disabled')).not.toBe('disabled'); + expect(findScrollToBottomButton().attributes('disabled')).toBeUndefined(); + expect(findScrollToTopButton().attributes('disabled')).toBeUndefined(); }); }); }); diff --git a/spec/frontend/ide/components/new_dropdown/upload_spec.js b/spec/frontend/ide/components/new_dropdown/upload_spec.js index 40780c7f0bd..883b365c99a 100644 --- a/spec/frontend/ide/components/new_dropdown/upload_spec.js +++ b/spec/frontend/ide/components/new_dropdown/upload_spec.js @@ -1,32 +1,31 @@ import { mount } from '@vue/test-utils'; import Upload from '~/ide/components/new_dropdown/upload.vue'; +import waitForPromises from 'helpers/wait_for_promises'; describe('new dropdown upload', () => { let wrapper; - beforeEach(() => { + function createComponent() { wrapper = mount(Upload, { propsData: { path: '', }, }); - }); - - describe('openFile', () => { - it('calls for each file', () => { - const files = ['test', 'test2', 'test3']; + } - jest.spyOn(wrapper.vm, 'readFile').mockImplementation(() => {}); - jest.spyOn(wrapper.vm.$refs.fileUpload, 'files', 'get').mockReturnValue(files); + const uploadFile = (file) => { + const input = wrapper.find('input[type="file"]'); + Object.defineProperty(input.element, 'files', { value: [file] }); + input.trigger('change', file); + }; - wrapper.vm.openFile(); + const waitForFileToLoad = async () => { + await waitForPromises(); + return waitForPromises(); + }; - expect(wrapper.vm.readFile.mock.calls.length).toBe(3); - - files.forEach((file, i) => { - expect(wrapper.vm.readFile.mock.calls[i]).toEqual([file]); - }); - }); + beforeEach(() => { + createComponent(); }); describe('readFile', () => { @@ -39,20 +38,13 @@ describe('new dropdown upload', () => { type: 'images/png', }; - wrapper.vm.readFile(file); + uploadFile(file); expect(FileReader.prototype.readAsDataURL).toHaveBeenCalledWith(file); }); }); describe('createFile', () => { - const textTarget = { - result: 'base64,cGxhaW4gdGV4dA==', - }; - const binaryTarget = { - result: 'base64,8PDw8A==', // ðððð - }; - const textFile = new File(['plain text'], 'textFile', { type: 'test/mime-text' }); const binaryFile = new File(['😺'], 'binaryFile', { type: 'test/mime-binary' }); @@ -61,15 +53,13 @@ describe('new dropdown upload', () => { }); it('calls readAsText and creates file in plain text (without encoding) if the file content is plain text', async () => { - const waitForCreate = new Promise((resolve) => { - wrapper.vm.$on('create', resolve); - }); + uploadFile(textFile); - wrapper.vm.createFile(textTarget, textFile); + // Text file has an additional load, so need to wait twice + await waitForFileToLoad(); + await waitForFileToLoad(); expect(FileReader.prototype.readAsText).toHaveBeenCalledWith(textFile); - - await waitForCreate; expect(wrapper.emitted('create')[0]).toStrictEqual([ { name: textFile.name, @@ -81,8 +71,10 @@ describe('new dropdown upload', () => { ]); }); - it('creates a blob URL for the content if binary', () => { - wrapper.vm.createFile(binaryTarget, binaryFile); + it('creates a blob URL for the content if binary', async () => { + uploadFile(binaryFile); + + await waitForFileToLoad(); expect(FileReader.prototype.readAsText).not.toHaveBeenCalled(); @@ -90,7 +82,7 @@ describe('new dropdown upload', () => { { name: binaryFile.name, type: 'blob', - content: 'ðððð', + content: 'ðº', // '😺' rawPath: 'blob:https://gitlab.com/048c7ac1-98de-4a37-ab1b-0206d0ea7e1b', mimeType: 'test/mime-binary', }, diff --git a/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js b/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js index a957e85723f..46884a42707 100644 --- a/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_target_cell_spec.js @@ -141,7 +141,7 @@ describe('import target cell', () => { }); it('renders namespace dropdown as disabled', () => { - expect(findNamespaceDropdown().attributes('disabled')).toBe('true'); + expect(findNamespaceDropdown().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/integrations/edit/components/active_checkbox_spec.js b/spec/frontend/integrations/edit/components/active_checkbox_spec.js index 8afff842a85..3a78140d0b1 100644 --- a/spec/frontend/integrations/edit/components/active_checkbox_spec.js +++ b/spec/frontend/integrations/edit/components/active_checkbox_spec.js @@ -26,7 +26,7 @@ describe('ActiveCheckbox', () => { createComponent({}, { isInheriting: true }); expect(findGlFormCheckbox().exists()).toBe(true); - expect(findInputInCheckbox().attributes('disabled')).toBe('disabled'); + expect(findInputInCheckbox().attributes('disabled')).toBeDefined(); }); }); @@ -35,7 +35,7 @@ describe('ActiveCheckbox', () => { createComponent({ activateDisabled: true }); expect(findGlFormCheckbox().exists()).toBe(true); - expect(findInputInCheckbox().attributes('disabled')).toBe('disabled'); + expect(findInputInCheckbox().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js b/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js index f876a497f98..a038b63d28c 100644 --- a/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js +++ b/spec/frontend/integrations/edit/components/jira_trigger_fields_spec.js @@ -187,7 +187,7 @@ describe('JiraTriggerFields', () => { ); wrapper.findAll('[type=text], [type=checkbox], [type=radio]').wrappers.forEach((input) => { - expect(input.attributes('disabled')).toBe('disabled'); + expect(input.attributes('disabled')).toBeDefined(); }); }); }); diff --git a/spec/frontend/integrations/edit/components/trigger_field_spec.js b/spec/frontend/integrations/edit/components/trigger_field_spec.js index 3b736b33a2f..b3d6784959f 100644 --- a/spec/frontend/integrations/edit/components/trigger_field_spec.js +++ b/spec/frontend/integrations/edit/components/trigger_field_spec.js @@ -37,7 +37,7 @@ describe('TriggerField', () => { it('when isInheriting is true, renders disabled GlFormCheckbox', () => { createComponent({ isInheriting: true }); - expect(findGlFormCheckbox().attributes('disabled')).toBe('true'); + expect(findGlFormCheckbox().attributes('disabled')).toBeDefined(); }); it('renders correct title', () => { diff --git a/spec/frontend/issuable/components/related_issuable_item_spec.js b/spec/frontend/issuable/components/related_issuable_item_spec.js index ca217a9fa7f..7322894164b 100644 --- a/spec/frontend/issuable/components/related_issuable_item_spec.js +++ b/spec/frontend/issuable/components/related_issuable_item_spec.js @@ -195,7 +195,7 @@ describe('RelatedIssuableItem', () => { }); it('renders disabled button when removeDisabled', () => { - expect(findRemoveButton().attributes('disabled')).toBe('true'); + expect(findRemoveButton().attributes('disabled')).toBeDefined(); }); it('triggers onRemoveRequest when clicked', () => { diff --git a/spec/frontend/issues/show/components/edit_actions_spec.js b/spec/frontend/issues/show/components/edit_actions_spec.js index ca561149806..0ebeb1b7b56 100644 --- a/spec/frontend/issues/show/components/edit_actions_spec.js +++ b/spec/frontend/issues/show/components/edit_actions_spec.js @@ -66,7 +66,7 @@ describe('Edit Actions component', () => { it('disables save button when title is blank', () => { createComponent({ props: { formState: { title: '', issue_type: '' } } }); - expect(findSaveButton().attributes('disabled')).toBe('true'); + expect(findSaveButton().attributes('disabled')).toBeDefined(); }); describe('updateIssuable', () => { diff --git a/spec/frontend/jobs/components/job/job_log_controllers_spec.js b/spec/frontend/jobs/components/job/job_log_controllers_spec.js index 9917c63b2d0..db060c0d606 100644 --- a/spec/frontend/jobs/components/job/job_log_controllers_spec.js +++ b/spec/frontend/jobs/components/job/job_log_controllers_spec.js @@ -133,7 +133,7 @@ describe('Job log controllers', () => { }); it('renders disabled scroll top button', () => { - expect(findScrollTop().attributes('disabled')).toBe('disabled'); + expect(findScrollTop().attributes('disabled')).toBeDefined(); }); it('does not emit scrollJobLogTop event on click', async () => { diff --git a/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js b/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js index 2758103fd6e..c54acf3cbee 100644 --- a/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js +++ b/spec/frontend/monitoring/components/dashboard_actions_menu_spec.js @@ -319,7 +319,7 @@ describe('Actions menu', () => { await nextTick(); expect(findStarDashboardItem().exists()).toBe(true); - expect(findStarDashboardItem().attributes('disabled')).toBe('true'); + expect(findStarDashboardItem().attributes('disabled')).toBeDefined(); }); it('on click it dispatches a toggle star action', async () => { @@ -365,7 +365,7 @@ describe('Actions menu', () => { }); it('is rendered by default but it is disabled', () => { - expect(findCreateDashboardItem().attributes('disabled')).toBe('true'); + expect(findCreateDashboardItem().attributes('disabled')).toBeDefined(); }); describe('when project path is set', () => { @@ -410,7 +410,7 @@ describe('Actions menu', () => { }); it('is disabled', () => { - expect(findCreateDashboardItem().attributes('disabled')).toBe('true'); + expect(findCreateDashboardItem().attributes('disabled')).toBeDefined(); }); it('does not render a modal for creating a dashboard', () => { diff --git a/spec/frontend/notes/components/comment_form_spec.js b/spec/frontend/notes/components/comment_form_spec.js index 04143bb5b60..fb64551c76b 100644 --- a/spec/frontend/notes/components/comment_form_spec.js +++ b/spec/frontend/notes/components/comment_form_spec.js @@ -303,7 +303,7 @@ describe('issue_comment_form component', () => { await findCommentButton().trigger('click'); - expect(findMarkdownEditor().find('textarea').attributes('disabled')).toBe('disabled'); + expect(findMarkdownEditor().find('textarea').attributes('disabled')).toBeDefined(); }); it('should support quick actions', () => { diff --git a/spec/frontend/notes/components/mr_discussion_filter_spec.js b/spec/frontend/notes/components/mr_discussion_filter_spec.js new file mode 100644 index 00000000000..405043ff2a0 --- /dev/null +++ b/spec/frontend/notes/components/mr_discussion_filter_spec.js @@ -0,0 +1,110 @@ +import { mount } from '@vue/test-utils'; +import { GlCollapsibleListbox, GlListboxItem, GlButton } from '@gitlab/ui'; +import Vue, { nextTick } from 'vue'; +import Vuex from 'vuex'; +import DiscussionFilter from '~/notes/components/mr_discussion_filter.vue'; +import { MR_FILTER_OPTIONS } from '~/notes/constants'; + +Vue.use(Vuex); + +describe('Merge request discussion filter component', () => { + let wrapper; + let store; + let updateMergeRequestFilters; + let setDiscussionSortDirection; + + function createComponent(mergeRequestFilters = MR_FILTER_OPTIONS.map((f) => f.value)) { + updateMergeRequestFilters = jest.fn(); + setDiscussionSortDirection = jest.fn(); + + store = new Vuex.Store({ + modules: { + notes: { + state: { + mergeRequestFilters, + discussionSortOrder: 'asc', + }, + actions: { + updateMergeRequestFilters, + setDiscussionSortDirection, + }, + }, + }, + }); + + wrapper = mount(DiscussionFilter, { + store, + }); + } + + afterEach(() => { + localStorage.removeItem('mr_activity_filters'); + localStorage.removeItem('sort_direction_merge_request'); + }); + + describe('local sync sort direction', () => { + it('calls setDiscussionSortDirection when mounted', () => { + localStorage.setItem('sort_direction_merge_request', 'desc'); + + createComponent(); + + expect(setDiscussionSortDirection).toHaveBeenCalledWith(expect.anything(), { + direction: 'desc', + }); + }); + }); + + describe('local sync sort filters', () => { + it('calls setDiscussionSortDirection when mounted', () => { + localStorage.setItem('mr_activity_filters', '["comments"]'); + + createComponent(); + + expect(updateMergeRequestFilters).toHaveBeenCalledWith(expect.anything(), ['comments']); + }); + }); + + it('lists current filters', () => { + createComponent(); + + expect(wrapper.findAllComponents(GlListboxItem).length).toBe(MR_FILTER_OPTIONS.length); + }); + + it('updates store when selecting filter', async () => { + createComponent(); + + wrapper.findComponent(GlListboxItem).vm.$emit('select'); + + await nextTick(); + + wrapper.findComponent(GlCollapsibleListbox).vm.$emit('hidden'); + + expect(updateMergeRequestFilters).toHaveBeenCalledWith(expect.anything(), [ + 'commit_branches', + 'status', + 'assignees_reviewers', + 'edits', + 'labels', + 'mentions', + 'tracking', + 'comments', + 'lock_status', + ]); + }); + + it.each` + state | expectedText + ${['status']} | ${'Merge request status'} + ${['status', 'comments']} | ${'Merge request status +1 more'} + ${[]} | ${'None'} + ${MR_FILTER_OPTIONS.map((f) => f.value)} | ${'All activity'} + `('updates toggle text to $expectedText with $state', async ({ state, expectedText }) => { + createComponent(); + + store.state.notes.mergeRequestFilters = state; + + await nextTick(); + + expect(wrapper.findComponent(GlButton).text()).toBe(expectedText); + }); +}); diff --git a/spec/frontend/notes/components/note_form_spec.js b/spec/frontend/notes/components/note_form_spec.js index d6413d33c99..9423af4f058 100644 --- a/spec/frontend/notes/components/note_form_spec.js +++ b/spec/frontend/notes/components/note_form_spec.js @@ -177,7 +177,7 @@ describe('issue_note_form component', () => { await nextTick(); - expect(textarea.attributes('disabled')).toBe('disabled'); + expect(textarea.attributes('disabled')).toBeDefined(); }); }); }); diff --git a/spec/frontend/notes/components/note_header_spec.js b/spec/frontend/notes/components/note_header_spec.js index b3d6fab7f91..60ad9e3344a 100644 --- a/spec/frontend/notes/components/note_header_spec.js +++ b/spec/frontend/notes/components/note_header_spec.js @@ -19,7 +19,9 @@ describe('NoteHeader component', () => { const findTimestampLink = () => wrapper.findComponent({ ref: 'noteTimestampLink' }); const findTimestamp = () => wrapper.findComponent({ ref: 'noteTimestamp' }); const findInternalNoteIndicator = () => wrapper.findByTestId('internal-note-indicator'); + const findAuthorName = () => wrapper.findByTestId('author-name'); const findSpinner = () => wrapper.findComponent({ ref: 'spinner' }); + const authorUsernameLink = () => wrapper.findComponent({ ref: 'authorUsernameLink' }); const statusHtml = '"<span class="user-status-emoji has-tooltip" title="foo bar" data-html="true" data-placement="top"><gl-emoji title="basketball and hoop" data-name="basketball" data-unicode-version="6.0">🏀</gl-emoji></span>"'; @@ -35,6 +37,17 @@ describe('NoteHeader component', () => { status_tooltip_html: statusHtml, }; + const supportBotAuthor = { + avatar_url: null, + id: 1, + name: 'Gitlab Support Bot', + path: '/support-bot', + state: 'active', + username: 'support-bot', + show_status: true, + status_tooltip_html: statusHtml, + }; + const createComponent = (props) => { wrapper = shallowMountExtended(NoteHeader, { store: new Vuex.Store({ @@ -114,6 +127,16 @@ describe('NoteHeader component', () => { expect(wrapper.text()).toContain('A deleted user'); }); + it('renders participant email when author is a support-bot', () => { + createComponent({ + author: supportBotAuthor, + emailParticipant: 'email@example.com', + }); + + expect(findAuthorName().text()).toBe('email@example.com'); + expect(authorUsernameLink().exists()).toBe(false); + }); + it('does not render created at information if createdAt is not passed as a prop', () => { createComponent(); @@ -204,16 +227,15 @@ describe('NoteHeader component', () => { it('toggles hover specific CSS classes on author name link', async () => { createComponent({ author }); - const authorUsernameLink = wrapper.findComponent({ ref: 'authorUsernameLink' }); const authorNameLink = wrapper.findComponent({ ref: 'authorNameLink' }); - authorUsernameLink.trigger('mouseenter'); + authorUsernameLink().trigger('mouseenter'); await nextTick(); expect(authorNameLink.classes()).toContain('hover'); expect(authorNameLink.classes()).toContain('text-underline'); - authorUsernameLink.trigger('mouseleave'); + authorUsernameLink().trigger('mouseleave'); await nextTick(); expect(authorNameLink.classes()).not.toContain('hover'); diff --git a/spec/frontend/packages_and_registries/container_registry/explorer/components/details_page/tags_list_row_spec.js b/spec/frontend/packages_and_registries/container_registry/explorer/components/details_page/tags_list_row_spec.js index bfefe46c09b..f74dfcb029d 100644 --- a/spec/frontend/packages_and_registries/container_registry/explorer/components/details_page/tags_list_row_spec.js +++ b/spec/frontend/packages_and_registries/container_registry/explorer/components/details_page/tags_list_row_spec.js @@ -153,7 +153,7 @@ describe('tags list row', () => { it('is disabled when the component is disabled', () => { mountComponent({ ...defaultProps, disabled: true }); - expect(findClipboardButton().attributes('disabled')).toBe('true'); + expect(findClipboardButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/packages_and_registries/container_registry/explorer/components/list_page/image_list_row_spec.js b/spec/frontend/packages_and_registries/container_registry/explorer/components/list_page/image_list_row_spec.js index 7da9c7533a0..5d8df45415e 100644 --- a/spec/frontend/packages_and_registries/container_registry/explorer/components/list_page/image_list_row_spec.js +++ b/spec/frontend/packages_and_registries/container_registry/explorer/components/list_page/image_list_row_spec.js @@ -145,7 +145,7 @@ describe('Image List Row', () => { }); it('the clipboard button is disabled', () => { - expect(findClipboardButton().attributes('disabled')).toBe('true'); + expect(findClipboardButton().attributes('disabled')).toBeDefined(); }); }); }); diff --git a/spec/frontend/packages_and_registries/infrastructure_registry/components/shared/package_list_row_spec.js b/spec/frontend/packages_and_registries/infrastructure_registry/components/shared/package_list_row_spec.js index 37ca420ae77..d00d7180f75 100644 --- a/spec/frontend/packages_and_registries/infrastructure_registry/components/shared/package_list_row_spec.js +++ b/spec/frontend/packages_and_registries/infrastructure_registry/components/shared/package_list_row_spec.js @@ -138,7 +138,7 @@ describe('packages_list_row', () => { }); it('details link is disabled', () => { - expect(findPackageLink().attributes('disabled')).toBe('true'); + expect(findPackageLink().attributes('disabled')).toBeDefined(); }); it('has a warning icon', () => { diff --git a/spec/frontend/packages_and_registries/settings/group/components/exceptions_input_spec.js b/spec/frontend/packages_and_registries/settings/group/components/exceptions_input_spec.js index 461200e6983..dd1edbaa3fd 100644 --- a/spec/frontend/packages_and_registries/settings/group/components/exceptions_input_spec.js +++ b/spec/frontend/packages_and_registries/settings/group/components/exceptions_input_spec.js @@ -98,7 +98,7 @@ describe('Exceptions Input', () => { }); it('disables the form input', () => { - expect(findInput().attributes('disabled')).toBe('true'); + expect(findInput().attributes('disabled')).toBeDefined(); }); }); }); diff --git a/spec/frontend/packages_and_registries/shared/components/package_path_spec.js b/spec/frontend/packages_and_registries/shared/components/package_path_spec.js index 2490e9a1f6a..3ffbb6f435c 100644 --- a/spec/frontend/packages_and_registries/shared/components/package_path_spec.js +++ b/spec/frontend/packages_and_registries/shared/components/package_path_spec.js @@ -86,12 +86,12 @@ describe('PackagePath', () => { }); it('root link is disabled', () => { - expect(findItem(ROOT_LINK).attributes('disabled')).toBe('true'); + expect(findItem(ROOT_LINK).attributes('disabled')).toBeDefined(); }); if (shouldExist.includes(LEAF_LINK)) { it('the last link is disabled', () => { - expect(findItem(LEAF_LINK).attributes('disabled')).toBe('true'); + expect(findItem(LEAF_LINK).attributes('disabled')).toBeDefined(); }); } }); diff --git a/spec/frontend/packages_and_registries/shared/components/registry_list_spec.js b/spec/frontend/packages_and_registries/shared/components/registry_list_spec.js index 85b4ca95d5d..66fca2ce12e 100644 --- a/spec/frontend/packages_and_registries/shared/components/registry_list_spec.js +++ b/spec/frontend/packages_and_registries/shared/components/registry_list_spec.js @@ -58,7 +58,7 @@ describe('Registry List', () => { it('sets disabled prop to true when items length is 0', () => { mountComponent({ propsData: { ...defaultPropsData, items: [] } }); - expect(findSelectAll().attributes('disabled')).toBe('true'); + expect(findSelectAll().attributes('disabled')).toBeDefined(); }); it('when few are selected, sets indeterminate prop to true', async () => { diff --git a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js index d8a848f0a2e..a7a1e649cd0 100644 --- a/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js +++ b/spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js @@ -158,7 +158,7 @@ describe('Settings Panel', () => { it('should disable the visibility level dropdown', () => { wrapper = mountComponent({ canChangeVisibilityLevel: false }); - expect(findProjectVisibilityLevelInput().attributes('disabled')).toBe('disabled'); + expect(findProjectVisibilityLevelInput().attributes('disabled')).toBeDefined(); }); it.each` diff --git a/spec/frontend/pipeline_wizard/components/step_nav_spec.js b/spec/frontend/pipeline_wizard/components/step_nav_spec.js index e80eb01ea7a..8a94f58523a 100644 --- a/spec/frontend/pipeline_wizard/components/step_nav_spec.js +++ b/spec/frontend/pipeline_wizard/components/step_nav_spec.js @@ -56,13 +56,13 @@ describe('Pipeline Wizard - Step Navigation Component', () => { it('enables the next button if nextButtonEnabled ist set to true', () => { createComponent({ nextButtonEnabled: true }); - expect(nextButton.attributes('disabled')).not.toBe('disabled'); + expect(nextButton.attributes('disabled')).toBeUndefined(); }); it('disables the next button if nextButtonEnabled ist set to false', () => { createComponent({ nextButtonEnabled: false }); - expect(nextButton.attributes('disabled')).toBe('disabled'); + expect(nextButton.attributes('disabled')).toBeDefined(); }); it('does not emit "next" event when clicking next button while nextButtonEnabled ist set to false', async () => { diff --git a/spec/frontend/pipelines/graph/job_item_spec.js b/spec/frontend/pipelines/graph/job_item_spec.js index 5cc2c76f3dd..2a5dfd7e0ee 100644 --- a/spec/frontend/pipelines/graph/job_item_spec.js +++ b/spec/frontend/pipelines/graph/job_item_spec.js @@ -121,7 +121,7 @@ describe('pipeline graph job item', () => { expect(actionComponent.exists()).toBe(true); expect(actionComponent.props('actionIcon')).toBe('retry'); - expect(actionComponent.attributes('disabled')).not.toBe('disabled'); + expect(actionComponent.attributes('disabled')).toBeUndefined(); }); it('should render disabled action icon when user cannot run the action', () => { @@ -135,7 +135,7 @@ describe('pipeline graph job item', () => { expect(actionComponent.exists()).toBe(true); expect(actionComponent.props('actionIcon')).toBe('stop'); - expect(actionComponent.attributes('disabled')).toBe('disabled'); + expect(actionComponent.attributes('disabled')).toBeDefined(); }); it('action icon tooltip text when job has passed but can be ran again', () => { diff --git a/spec/frontend/pipelines/pipelines_manual_actions_spec.js b/spec/frontend/pipelines/pipelines_manual_actions_spec.js index e47e57db887..82cab88c9eb 100644 --- a/spec/frontend/pipelines/pipelines_manual_actions_spec.js +++ b/spec/frontend/pipelines/pipelines_manual_actions_spec.js @@ -101,7 +101,7 @@ describe('Pipeline manual actions', () => { }); it("displays a disabled action when it's not playable", () => { - expect(findAllDropdownItems().at(0).attributes('disabled')).toBe('true'); + expect(findAllDropdownItems().at(0).attributes('disabled')).toBeDefined(); }); describe('on action click', () => { diff --git a/spec/frontend/profile/account/components/update_username_spec.js b/spec/frontend/profile/account/components/update_username_spec.js index 3cb9cf3622a..fa107600d64 100644 --- a/spec/frontend/profile/account/components/update_username_spec.js +++ b/spec/frontend/profile/account/components/update_username_spec.js @@ -117,7 +117,7 @@ describe('UpdateUsername component', () => { const { input, openModalBtn, modal } = findElements(); axiosMock.onPut(actionUrl).replyOnce(() => { - expect(input.attributes('disabled')).toBe('disabled'); + expect(input.attributes('disabled')).toBeDefined(); expect(openModalBtn.props('disabled')).toBe(false); expect(openModalBtn.props('loading')).toBe(true); @@ -136,7 +136,7 @@ describe('UpdateUsername component', () => { const { input, openModalBtn } = findElements(); axiosMock.onPut(actionUrl).replyOnce(() => { - expect(input.attributes('disabled')).toBe('disabled'); + expect(input.attributes('disabled')).toBeDefined(); expect(openModalBtn.props('disabled')).toBe(false); expect(openModalBtn.props('loading')).toBe(true); diff --git a/spec/frontend/projects/commits/components/author_select_spec.js b/spec/frontend/projects/commits/components/author_select_spec.js index b06463e73a7..630b8feafbc 100644 --- a/spec/frontend/projects/commits/components/author_select_spec.js +++ b/spec/frontend/projects/commits/components/author_select_spec.js @@ -81,7 +81,7 @@ describe('Author Select', () => { }); it('disables dropdown', () => { - expect(findDropdown().attributes('disabled')).toBe('true'); + expect(findDropdown().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/projects/components/shared/delete_button_spec.js b/spec/frontend/projects/components/shared/delete_button_spec.js index 364a29d0e41..6b4ef341b0c 100644 --- a/spec/frontend/projects/components/shared/delete_button_spec.js +++ b/spec/frontend/projects/components/shared/delete_button_spec.js @@ -69,7 +69,7 @@ describe('Project remove modal', () => { }); it('the confirm button is disabled', () => { - expect(findConfirmButton().attributes('disabled')).toBe('true'); + expect(findConfirmButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/projects/new/components/deployment_target_select_spec.js b/spec/frontend/projects/new/components/deployment_target_select_spec.js index bec738f7765..57b804b632a 100644 --- a/spec/frontend/projects/new/components/deployment_target_select_spec.js +++ b/spec/frontend/projects/new/components/deployment_target_select_spec.js @@ -56,7 +56,7 @@ describe('Deployment target select', () => { it('renders a select with the disabled default option', () => { expect(findSelect().find('option').text()).toBe('Select the deployment target'); - expect(findSelect().find('option').attributes('disabled')).toBe('disabled'); + expect(findSelect().find('option').attributes('disabled')).toBeDefined(); }); describe.each` diff --git a/spec/frontend/projects/settings/components/transfer_project_form_spec.js b/spec/frontend/projects/settings/components/transfer_project_form_spec.js index a92ac1bed9d..e12938c3bab 100644 --- a/spec/frontend/projects/settings/components/transfer_project_form_spec.js +++ b/spec/frontend/projects/settings/components/transfer_project_form_spec.js @@ -49,7 +49,7 @@ describe('Transfer project form', () => { it('disables the confirm button by default', () => { createComponent(); - expect(findConfirmDanger().attributes('disabled')).toBe('true'); + expect(findConfirmDanger().attributes('disabled')).toBeDefined(); }); describe('with a selected namespace', () => { diff --git a/spec/frontend/releases/components/app_edit_new_spec.js b/spec/frontend/releases/components/app_edit_new_spec.js index d253c42e03f..69d8969f0ad 100644 --- a/spec/frontend/releases/components/app_edit_new_spec.js +++ b/spec/frontend/releases/components/app_edit_new_spec.js @@ -309,7 +309,7 @@ describe('Release edit/new component', () => { }); it('renders the submit button as disabled', () => { - expect(findSubmitButton().attributes('disabled')).toBe('disabled'); + expect(findSubmitButton().attributes('disabled')).toBeDefined(); }); it('does not allow the form to be submitted', () => { diff --git a/spec/frontend/search/sidebar/components/filters_spec.js b/spec/frontend/search/sidebar/components/filters_spec.js index 51c7bdd9609..d189c695467 100644 --- a/spec/frontend/search/sidebar/components/filters_spec.js +++ b/spec/frontend/search/sidebar/components/filters_spec.js @@ -66,7 +66,7 @@ describe('GlobalSearchSidebarFilters', () => { }); it('disables the button', () => { - expect(findApplyButton().attributes('disabled')).toBe('true'); + expect(findApplyButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/search/sidebar/components/language_filter_spec.js b/spec/frontend/search/sidebar/components/language_filter_spec.js index 5821def5b43..9ad9d095aca 100644 --- a/spec/frontend/search/sidebar/components/language_filter_spec.js +++ b/spec/frontend/search/sidebar/components/language_filter_spec.js @@ -132,7 +132,7 @@ describe('GlobalSearchSidebarLanguageFilter', () => { }); it('disables the button', () => { - expect(findApplyButton().attributes('disabled')).toBe('true'); + expect(findApplyButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/security_configuration/components/training_provider_list_spec.js b/spec/frontend/security_configuration/components/training_provider_list_spec.js index d35fc00057a..2982cef7c74 100644 --- a/spec/frontend/security_configuration/components/training_provider_list_spec.js +++ b/spec/frontend/security_configuration/components/training_provider_list_spec.js @@ -401,7 +401,7 @@ describe('TrainingProviderList component', () => { it('has disabled state for radio', () => { findPrimaryProviderRadios().wrappers.forEach((radio) => { - expect(radio.attributes('disabled')).toBe('true'); + expect(radio.attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/sidebar/components/lock/edit_form_buttons_spec.js b/spec/frontend/sidebar/components/lock/edit_form_buttons_spec.js index ad9efc371f0..2c256a67bb0 100644 --- a/spec/frontend/sidebar/components/lock/edit_form_buttons_spec.js +++ b/spec/frontend/sidebar/components/lock/edit_form_buttons_spec.js @@ -69,7 +69,7 @@ describe('EditFormButtons', () => { }); it('disables the toggle button', () => { - expect(findLockToggle().attributes('disabled')).toBe('disabled'); + expect(findLockToggle().attributes('disabled')).toBeDefined(); }); it('sets loading on the toggle button', () => { diff --git a/spec/frontend/sidebar/components/move/issuable_move_dropdown_spec.js b/spec/frontend/sidebar/components/move/issuable_move_dropdown_spec.js index ab3e71bdddb..56c915c4cae 100644 --- a/spec/frontend/sidebar/components/move/issuable_move_dropdown_spec.js +++ b/spec/frontend/sidebar/components/move/issuable_move_dropdown_spec.js @@ -279,7 +279,7 @@ describe('IssuableMoveDropdown', () => { const moveButtonEl = findFooter().findComponent(GlButton); expect(moveButtonEl.text()).toBe('Move'); - expect(moveButtonEl.attributes('disabled')).toBe('true'); + expect(moveButtonEl.attributes('disabled')).toBeDefined(); findDropdownEl().vm.$emit('shown'); await waitForPromises(); diff --git a/spec/frontend/sidebar/components/move/move_issues_button_spec.js b/spec/frontend/sidebar/components/move/move_issues_button_spec.js index 2c7982a4b7f..83b32d04fcf 100644 --- a/spec/frontend/sidebar/components/move/move_issues_button_spec.js +++ b/spec/frontend/sidebar/components/move/move_issues_button_spec.js @@ -166,7 +166,7 @@ describe('MoveIssuesButton', () => { it('renders disabled by default', () => { createComponent(); expect(findDropdown().exists()).toBe(true); - expect(findDropdown().attributes('disabled')).toBe('true'); + expect(findDropdown().attributes('disabled')).toBeDefined(); }); it.each` @@ -185,7 +185,7 @@ describe('MoveIssuesButton', () => { await nextTick(); if (disabled) { - expect(findDropdown().attributes('disabled')).toBe('true'); + expect(findDropdown().attributes('disabled')).toBeDefined(); } else { expect(findDropdown().attributes('disabled')).toBeUndefined(); } diff --git a/spec/frontend/super_sidebar/components/super_sidebar_toggle_spec.js b/spec/frontend/super_sidebar/components/super_sidebar_toggle_spec.js index a12434a9e48..8bb20186e16 100644 --- a/spec/frontend/super_sidebar/components/super_sidebar_toggle_spec.js +++ b/spec/frontend/super_sidebar/components/super_sidebar_toggle_spec.js @@ -57,7 +57,7 @@ describe('SuperSidebarToggle component', () => { it('is disabled when isPeek is true', () => { createWrapper({ sidebarState: { isPeek: true } }); - expect(findButton().attributes('disabled')).toBe('true'); + expect(findButton().attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/terms/components/app_spec.js b/spec/frontend/terms/components/app_spec.js index c60c6c79f17..cab7fbe18b0 100644 --- a/spec/frontend/terms/components/app_spec.js +++ b/spec/frontend/terms/components/app_spec.js @@ -65,7 +65,6 @@ describe('TermsApp', () => { describe('accept button', () => { it('is disabled until user scrolls to the bottom of the terms', async () => { createComponent(); - expect(findButton(defaultProvide.paths.accept).attributes('disabled')).toBe('disabled'); wrapper.findComponent(GlIntersectionObserver).vm.$emit('appear'); diff --git a/spec/frontend/vue_merge_request_widget/components/artifacts_list_app_spec.js b/spec/frontend/vue_merge_request_widget/components/artifacts_list_app_spec.js index 332f14a1721..9516aacea0a 100644 --- a/spec/frontend/vue_merge_request_widget/components/artifacts_list_app_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/artifacts_list_app_spec.js @@ -70,8 +70,8 @@ describe('Merge Requests Artifacts list app', () => { it('renders disabled buttons', () => { const buttons = findButtons(); - expect(buttons.at(0).attributes('disabled')).toBe('disabled'); - expect(buttons.at(1).attributes('disabled')).toBe('disabled'); + expect(buttons.at(0).attributes('disabled')).toBeDefined(); + expect(buttons.at(1).attributes('disabled')).toBeDefined(); }); }); diff --git a/spec/frontend/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed_spec.js b/spec/frontend/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed_spec.js index 9b043bda72d..e65deb2db3d 100644 --- a/spec/frontend/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed_spec.js @@ -44,7 +44,7 @@ describe('MRWidgetAutoMergeFailed', () => { await nextTick(); - expect(findButton().attributes('disabled')).toBe('disabled'); + expect(findButton().attributes('disabled')).toBeDefined(); expect(wrapper.findComponent(GlLoadingIcon).exists()).toBe(true); }); }); diff --git a/spec/frontend/vue_shared/components/confirm_danger/confirm_danger_spec.js b/spec/frontend/vue_shared/components/confirm_danger/confirm_danger_spec.js index 379b5cde4d5..e082fa4085f 100644 --- a/spec/frontend/vue_shared/components/confirm_danger/confirm_danger_spec.js +++ b/spec/frontend/vue_shared/components/confirm_danger/confirm_danger_spec.js @@ -48,7 +48,7 @@ describe('Confirm Danger Modal', () => { wrapper = createComponent({ disabled: true }); - expect(findBtn().attributes('disabled')).toBe('true'); + expect(findBtn().attributes('disabled')).toBeDefined(); }); it('passes `buttonClass` prop to button', () => { diff --git a/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js b/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js index e9d5da4edcf..63a689088c7 100644 --- a/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js +++ b/spec/frontend/vue_shared/components/markdown/markdown_editor_spec.js @@ -155,7 +155,7 @@ describe('vue_shared/component/markdown/markdown_editor', () => { it('disables markdown field when disabled prop is true', () => { buildWrapper({ propsData: { disabled: true } }); - expect(findMarkdownField().find('textarea').attributes('disabled')).toBe('disabled'); + expect(findMarkdownField().find('textarea').attributes('disabled')).toBeDefined(); }); it('enables markdown field when disabled prop is false', () => { diff --git a/spec/frontend/webhooks/components/form_url_mask_item_spec.js b/spec/frontend/webhooks/components/form_url_mask_item_spec.js index 06c743749a6..6bae0ca9854 100644 --- a/spec/frontend/webhooks/components/form_url_mask_item_spec.js +++ b/spec/frontend/webhooks/components/form_url_mask_item_spec.js @@ -57,12 +57,12 @@ describe('FormUrlMaskItem', () => { }); it('renders disabled key and value', () => { - expect(findMaskItemKey().findComponent(GlFormInput).attributes('disabled')).toBe('true'); - expect(findMaskItemValue().findComponent(GlFormInput).attributes('disabled')).toBe('true'); + expect(findMaskItemKey().findComponent(GlFormInput).attributes('disabled')).toBeDefined(); + expect(findMaskItemValue().findComponent(GlFormInput).attributes('disabled')).toBeDefined(); }); it('renders disabled remove button', () => { - expect(findRemoveButton().attributes('disabled')).toBe('true'); + expect(findRemoveButton().attributes('disabled')).toBeDefined(); }); it('displays ************ as input value', () => { diff --git a/spec/frontend/work_items/components/notes/__snapshots__/work_item_note_replying_spec.js.snap b/spec/frontend/work_items/components/notes/__snapshots__/work_item_note_replying_spec.js.snap index 1c01451f047..30577dc60cf 100644 --- a/spec/frontend/work_items/components/notes/__snapshots__/work_item_note_replying_spec.js.snap +++ b/spec/frontend/work_items/components/notes/__snapshots__/work_item_note_replying_spec.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Work Item Note Replying should have the note body and header 1`] = `"<note-header-stub author=\\"[object Object]\\" actiontext=\\"\\" noteabletype=\\"\\" expanded=\\"true\\" showspinner=\\"true\\" noteurl=\\"\\"></note-header-stub>"`; +exports[`Work Item Note Replying should have the note body and header 1`] = `"<note-header-stub author=\\"[object Object]\\" actiontext=\\"\\" noteabletype=\\"\\" expanded=\\"true\\" showspinner=\\"true\\" noteurl=\\"\\" emailparticipant=\\"\\"></note-header-stub>"`; diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js index 5184b24d202..6100bbea4a1 100644 --- a/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js +++ b/spec/frontend/work_items/components/work_item_links/work_item_links_form_spec.js @@ -150,7 +150,7 @@ describe('WorkItemLinksForm', () => { const confidentialCheckbox = findConfidentialCheckbox(); const confidentialTooltip = wrapper.findComponent(GlTooltip); - expect(confidentialCheckbox.attributes('disabled')).toBe('true'); + expect(confidentialCheckbox.attributes('disabled')).toBeDefined(); expect(confidentialCheckbox.attributes('checked')).toBe('true'); expect(confidentialTooltip.exists()).toBe(true); expect(confidentialTooltip.text()).toBe( diff --git a/spec/frontend/work_items/components/work_item_notes_spec.js b/spec/frontend/work_items/components/work_item_notes_spec.js index 7dbf828c44a..9cbe8283468 100644 --- a/spec/frontend/work_items/components/work_item_notes_spec.js +++ b/spec/frontend/work_items/components/work_item_notes_spec.js @@ -208,7 +208,7 @@ describe('WorkItemNotes component', () => { await nextTick(); expect(workItemMoreNotesQueryHandler).toHaveBeenCalledWith({ - pageSize: 45, + pageSize: DEFAULT_PAGE_SIZE_NOTES, id: 'gid://gitlab/WorkItem/1', after: mockMoreNotesWidgetResponse.discussions.pageInfo.endCursor, }); diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb index f70645a8272..e3b0e90bff9 100644 --- a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -206,4 +206,26 @@ RSpec.describe Gitlab::Email::Handler::CreateNoteHandler do it_behaves_like 'a reply to existing comment' end + + context 'when note is authored from external author for service desk' do + before do + SentNotification.find_by(reply_key: mail_key).update!(recipient: User.support_bot) + end + + context 'when email contains text, quoted text and quick commands' do + let(:email_raw) { fixture_file('emails/commands_in_reply.eml') } + + it 'creates a discussion' do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + end + + it 'links external participant' do + receiver.execute + + new_note = noteable.notes.last + + expect(new_note.note_metadata.external_author).to eq('jake@adventuretime.ooo') + end + end + end end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 13e9aeb4c53..1b205aa5c85 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -869,11 +869,13 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures, feature_category: :system check = -> { push_changes(changes[action]) } if allowed - expect(&check).not_to raise_error, - -> { "expected #{action} to be allowed" } + expect(&check).not_to raise_error, -> do + "expected #{action} for #{role} to be allowed while #{who_can_action}" + end else - expect(&check).to raise_error(Gitlab::GitAccess::ForbiddenError), - -> { "expected #{action} to be disallowed" } + expect(&check).to raise_error(Gitlab::GitAccess::ForbiddenError), -> do + "expected #{action} for #{role} to be disallowed while #{who_can_action}" + end end end end @@ -886,12 +888,12 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures, feature_category: :system any: true, push_new_branch: true, push_master: true, - push_protected_branch: true, + push_protected_branch: false, push_remove_protected_branch: false, push_tag: true, push_new_tag: true, - push_all: true, - merge_into_protected_branch: true + push_all: false, + merge_into_protected_branch: false }, admin_without_admin_mode: { @@ -957,19 +959,22 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures, feature_category: :system [%w(feature exact), ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type| context do - let(:protected_branch) { create(:protected_branch, :maintainers_can_push, name: protected_branch_name, project: project) } + let(:who_can_action) { :maintainers_can_push } + let(:protected_branch) { create(:protected_branch, who_can_action, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix) end context "when developers are allowed to push into the #{protected_branch_type} protected branch" do - let(:protected_branch) { create(:protected_branch, :developers_can_push, name: protected_branch_name, project: project) } + let(:who_can_action) { :developers_can_push } + let(:protected_branch) { create(:protected_branch, who_can_action, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) end - context "developers are allowed to merge into the #{protected_branch_type} protected branch" do - let(:protected_branch) { create(:protected_branch, :developers_can_merge, name: protected_branch_name, project: project) } + context "when developers are allowed to merge into the #{protected_branch_type} protected branch" do + let(:who_can_action) { :developers_can_merge } + let(:protected_branch) { create(:protected_branch, who_can_action, name: protected_branch_name, project: project) } context "when a merge request exists for the given source/target branch" do context "when the merge request is in progress" do @@ -996,6 +1001,7 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures, feature_category: :system end context "when developers are allowed to push and merge into the #{protected_branch_type} protected branch" do + let(:who_can_action) { :developers_can_push_and_merge } let(:protected_branch) { create(:protected_branch, :developers_can_merge, :developers_can_push, name: protected_branch_name, project: project) } run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) diff --git a/spec/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing_spec.rb b/spec/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing_spec.rb index f7adb44b54a..8d8b879a90f 100644 --- a/spec/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing_spec.rb +++ b/spec/lib/gitlab/graphql/subscriptions/action_cable_with_load_balancing_spec.rb @@ -22,8 +22,8 @@ RSpec.describe Gitlab::Graphql::Subscriptions::ActionCableWithLoadBalancing, fea expect(action_cable).to receive(:broadcast) do |topic, payload| expect(topic).to match(/^graphql-event/) expect(Gitlab::Json.parse(payload)).to match({ - 'wal_locations' => expected_locations, - 'payload' => { '__gid__' => be_instance_of(String) } + described_class::KEY_WAL_LOCATIONS => expected_locations, + described_class::KEY_PAYLOAD => { '__gid__' => 'Z2lkOi8vZ2l0bGFiL1Byb2plY3QvMQ' } }) end @@ -77,28 +77,32 @@ RSpec.describe Gitlab::Graphql::Subscriptions::ActionCableWithLoadBalancing, fea end context 'when handling event' do - def handle_event! - subscriptions.execute_update('sub:123', event, object) + def handle_event!(wal_locations: nil) + subscriptions.execute_update('sub:123', event, { + described_class::KEY_WAL_LOCATIONS => wal_locations || { + 'main' => current_location + }, + described_class::KEY_PAYLOAD => { '__gid__' => 'Z2lkOi8vZ2l0bGFiL1Byb2plY3QvMQ' } + }) end before do allow(action_cable).to receive(:broadcast) + end - subscriptions.load_action_cable_message(Gitlab::Json.dump({ - 'wal_locations' => { - 'main' => current_location - }, - 'payload' => {} - }), nil) + context 'when event payload is not wrapped' do + it 'does not attempt to unwrap it' do + expect(object).not_to receive(:[]).with(described_class::KEY_PAYLOAD) + + subscriptions.execute_update('sub:123', event, object) + end end context 'when WAL locations are not present' do it 'uses the primary' do - subscriptions.load_action_cable_message(Gitlab::Json.dump({}), nil) - expect(::Gitlab::Database::LoadBalancing::Session.current).to receive(:use_primary!) - handle_event! + handle_event!(wal_locations: {}) end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 5e00fc80b16..e6f281ae35b 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -93,6 +93,10 @@ notes: - suggestions - diff_note_positions - review +- note_metadata +note_metadata: + - note + - email_participant commit_notes: - award_emoji - noteable @@ -107,6 +111,7 @@ commit_notes: - suggestions - diff_note_positions - review +- note_metadata label_links: - target - label diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index a2087695d31..faf345e8f78 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -87,6 +87,11 @@ Note: - confidential - last_edited_at - internal +Notes::NoteMetadata: +- note_id +- email_participant +- created_at +- updated_at LabelLink: - id - target_type diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 1ae45d41f2d..7d09330d185 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::UserAccess do +RSpec.describe Gitlab::UserAccess, feature_category: :system_access do include ProjectForksHelper let(:access) { described_class.new(user, container: project) } @@ -85,10 +85,10 @@ RSpec.describe Gitlab::UserAccess do let(:not_existing_branch) { create :protected_branch, :developers_can_merge, project: project } context 'when admin mode is enabled', :enable_admin_mode do - it 'returns true for admins' do + it 'returns false for admins' do user.update!(admin: true) - expect(access.can_push_to_branch?(branch.name)).to be_truthy + expect(access.can_push_to_branch?(branch.name)).to be_falsey end end diff --git a/spec/lib/sidebars/admin/panel_spec.rb b/spec/lib/sidebars/admin/panel_spec.rb index a12fc8f8d2a..12bd83a4b88 100644 --- a/spec/lib/sidebars/admin/panel_spec.rb +++ b/spec/lib/sidebars/admin/panel_spec.rb @@ -12,4 +12,6 @@ RSpec.describe Sidebars::Admin::Panel, feature_category: :navigation do it 'implements #super_sidebar_context_header' do expect(subject.super_sidebar_context_header).to eq({ title: 'Admin Area' }) end + + it_behaves_like 'a panel with uniquely identifiable menu items' end diff --git a/spec/lib/sidebars/groups/super_sidebar_panel_spec.rb b/spec/lib/sidebars/groups/super_sidebar_panel_spec.rb index e9e9b87b588..7362f88ab3c 100644 --- a/spec/lib/sidebars/groups/super_sidebar_panel_spec.rb +++ b/spec/lib/sidebars/groups/super_sidebar_panel_spec.rb @@ -3,12 +3,18 @@ require 'spec_helper' RSpec.describe Sidebars::Groups::SuperSidebarPanel, feature_category: :navigation do - let_it_be(:group) { create(:group) } - - let(:user) { group.first_owner } + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group).tap { |group| group.add_owner(user) } } let(:context) do - double("Stubbed context", current_user: user, container: group, group: group).as_null_object # rubocop:disable RSpec/VerifiedDoubles + Sidebars::Groups::Context.new( + current_user: user, + container: group, + is_super_sidebar: true, + # Turn features off that do not add/remove menu items + show_promotions: false, + show_discover_group_security: false + ) end subject { described_class.new(context) } @@ -42,4 +48,7 @@ RSpec.describe Sidebars::Groups::SuperSidebarPanel, feature_category: :navigatio expect(subject.instance_variable_get(:@menus).map(&:class)).to eq(category_menu) end end + + it_behaves_like 'a panel with uniquely identifiable menu items' + it_behaves_like 'a panel with all menu_items categorized' end diff --git a/spec/lib/sidebars/projects/super_sidebar_menus/analyze_menu_spec.rb b/spec/lib/sidebars/projects/super_sidebar_menus/analyze_menu_spec.rb index 8f07241d2e2..d459d47c31a 100644 --- a/spec/lib/sidebars/projects/super_sidebar_menus/analyze_menu_spec.rb +++ b/spec/lib/sidebars/projects/super_sidebar_menus/analyze_menu_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Sidebars::Projects::SuperSidebarMenus::AnalyzeMenu, feature_categ :ci_cd_analytics, :repository_analytics, :code_review, - :merge_requests, + :merge_request_analytics, :issues, :insights, :model_experiments diff --git a/spec/lib/sidebars/projects/super_sidebar_menus/operations_menu_spec.rb b/spec/lib/sidebars/projects/super_sidebar_menus/operations_menu_spec.rb index 8113c2b5b64..6ab070c40ae 100644 --- a/spec/lib/sidebars/projects/super_sidebar_menus/operations_menu_spec.rb +++ b/spec/lib/sidebars/projects/super_sidebar_menus/operations_menu_spec.rb @@ -20,7 +20,6 @@ RSpec.describe Sidebars::Projects::SuperSidebarMenus::OperationsMenu, feature_ca :kubernetes, :terraform_states, :infrastructure_registry, - :activity, :google_cloud, :aws ]) diff --git a/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb b/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb index 25554bba6f1..b6672f2c820 100644 --- a/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb +++ b/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb @@ -6,9 +6,20 @@ RSpec.describe Sidebars::Projects::SuperSidebarPanel, feature_category: :navigat let_it_be(:project) { create(:project, :repository) } let(:user) { project.first_owner } - let(:context) do - double("Stubbed context", current_user: user, container: project, project: project, current_ref: 'master').as_null_object # rubocop:disable RSpec/VerifiedDoubles + Sidebars::Projects::Context.new( + current_user: user, + container: project, + current_ref: project.repository.root_ref, + is_super_sidebar: true, + # Turn features on that impact the list of items rendered + can_view_pipeline_editor: true, + learn_gitlab_enabled: true, + show_discover_project_security: true, + # Turn features off that do not add/remove items + show_cluster_hint: false, + show_promotions: false + ) end subject { described_class.new(context) } @@ -43,4 +54,7 @@ RSpec.describe Sidebars::Projects::SuperSidebarPanel, feature_category: :navigat expect(subject.instance_variable_get(:@menus).map(&:class)).to eq(category_menu) end end + + it_behaves_like 'a panel with uniquely identifiable menu items' + it_behaves_like 'a panel with all menu_items categorized' end diff --git a/spec/lib/sidebars/search/panel_spec.rb b/spec/lib/sidebars/search/panel_spec.rb index 8561dc0b875..22c2bff0a32 100644 --- a/spec/lib/sidebars/search/panel_spec.rb +++ b/spec/lib/sidebars/search/panel_spec.rb @@ -11,6 +11,8 @@ RSpec.describe Sidebars::Search::Panel, feature_category: :navigation do subject { described_class.new(context) } + it_behaves_like 'a panel with uniquely identifiable menu items' + describe '#aria_label' do it 'returns the correct aria label' do expect(panel.aria_label).to eq(_('Search')) diff --git a/spec/lib/sidebars/user_profile/panel_spec.rb b/spec/lib/sidebars/user_profile/panel_spec.rb index af261dce3f3..c62c7f9fd96 100644 --- a/spec/lib/sidebars/user_profile/panel_spec.rb +++ b/spec/lib/sidebars/user_profile/panel_spec.rb @@ -10,6 +10,8 @@ RSpec.describe Sidebars::UserProfile::Panel, feature_category: :navigation do subject { described_class.new(context) } + it_behaves_like 'a panel with uniquely identifiable menu items' + it 'implements #aria_label' do expect(subject.aria_label).to eq(s_('UserProfile|User profile navigation')) end diff --git a/spec/lib/sidebars/user_settings/panel_spec.rb b/spec/lib/sidebars/user_settings/panel_spec.rb index aa05d99912a..0c02bf77d0e 100644 --- a/spec/lib/sidebars/user_settings/panel_spec.rb +++ b/spec/lib/sidebars/user_settings/panel_spec.rb @@ -9,6 +9,8 @@ RSpec.describe Sidebars::UserSettings::Panel, feature_category: :navigation do subject { described_class.new(context) } + it_behaves_like 'a panel with uniquely identifiable menu items' + it 'implements #super_sidebar_context_header' do expect(subject.super_sidebar_context_header).to eq({ title: _('User settings'), avatar: user.avatar_url }) end diff --git a/spec/lib/sidebars/your_work/panel_spec.rb b/spec/lib/sidebars/your_work/panel_spec.rb index 97da94e4114..ae9c3aa18e6 100644 --- a/spec/lib/sidebars/your_work/panel_spec.rb +++ b/spec/lib/sidebars/your_work/panel_spec.rb @@ -9,6 +9,8 @@ RSpec.describe Sidebars::YourWork::Panel, feature_category: :navigation do subject { described_class.new(context) } + it_behaves_like 'a panel with uniquely identifiable menu items' + it 'implements #super_sidebar_context_header' do expect(subject.super_sidebar_context_header).to eq({ title: 'Your work', icon: 'work' }) end diff --git a/spec/models/concerns/protected_ref_access_spec.rb b/spec/models/concerns/protected_ref_access_spec.rb index 750a5eba303..3cf195704ce 100644 --- a/spec/models/concerns/protected_ref_access_spec.rb +++ b/spec/models/concerns/protected_ref_access_spec.rb @@ -12,10 +12,10 @@ RSpec.describe ProtectedRefAccess do let(:project) { protected_ref_access.project } describe '#check_access' do - it 'is always true for admins' do + it 'is not bypassed for admins' do admin = create(:admin) - expect(protected_ref_access.check_access(admin)).to be_truthy + expect(protected_ref_access.check_access(admin)).to be_falsey end it 'is true for maintainers' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index bcfcfa05ddf..f722415d428 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -11,6 +11,7 @@ RSpec.describe Note, feature_category: :team_planning do it { is_expected.to belong_to(:author).class_name('User') } it { is_expected.to have_many(:todos) } + it { is_expected.to have_one(:note_metadata).inverse_of(:note).class_name('Notes::NoteMetadata') } it { is_expected.to belong_to(:review).inverse_of(:notes) } end diff --git a/spec/models/notes/note_metadata_spec.rb b/spec/models/notes/note_metadata_spec.rb new file mode 100644 index 00000000000..8a11dd1a758 --- /dev/null +++ b/spec/models/notes/note_metadata_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Notes::NoteMetadata, feature_category: :team_planning do + describe 'associations' do + it { is_expected.to belong_to(:note) } + end + + describe 'validation' do + it { is_expected.to validate_length_of(:email_participant).is_at_most(255) } + end +end diff --git a/spec/models/packages/dependency_spec.rb b/spec/models/packages/dependency_spec.rb index 1575dec98c9..80ec7f77fda 100644 --- a/spec/models/packages/dependency_spec.rb +++ b/spec/models/packages/dependency_spec.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Packages::Dependency, type: :model do +RSpec.describe Packages::Dependency, type: :model, feature_category: :package_registry do + describe 'included modules' do + it { is_expected.to include_module(EachBatch) } + end + describe 'relationships' do it { is_expected.to have_many(:dependency_links) } end @@ -110,6 +114,19 @@ RSpec.describe Packages::Dependency, type: :model do end end + describe '.orphaned' do + let_it_be(:orphaned_dependencies) { create_list(:packages_dependency, 2) } + let_it_be(:linked_dependency) do + create(:packages_dependency).tap do |dependency| + create(:packages_dependency_link, dependency: dependency) + end + end + + it 'returns orphaned dependency records' do + expect(described_class.orphaned).to contain_exactly(*orphaned_dependencies) + end + end + def build_names_and_version_patterns(*package_dependencies) result = Hash.new { |h, dependency| h[dependency.name] = dependency.version_pattern } package_dependencies.each { |dependency| result[dependency] } diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 77cfcab5c3e..ec3b3fde719 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -258,6 +258,8 @@ RSpec.describe Ci::BuildPolicy do context 'when the build was created for a protected tag' do before do create(:protected_tag, :developers_can_create, name: build.ref, project: project) + + build.update!(tag: true) end it { expect(policy).to be_allowed :erase_build } diff --git a/spec/requests/api/ml/mlflow/experiments_spec.rb b/spec/requests/api/ml/mlflow/experiments_spec.rb new file mode 100644 index 00000000000..1a2577e69e7 --- /dev/null +++ b/spec/requests/api/ml/mlflow/experiments_spec.rb @@ -0,0 +1,215 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:another_project) { build(:project).tap { |p| p.add_developer(developer) } } + let_it_be(:experiment) do + create(:ml_experiments, :with_metadata, project: project) + end + + let_it_be(:tokens) do + { + write: create(:personal_access_token, scopes: %w[read_api api], user: developer), + read: create(:personal_access_token, scopes: %w[read_api], user: developer), + no_access: create(:personal_access_token, scopes: %w[read_user], user: developer), + different_user: create(:personal_access_token, scopes: %w[read_api api], user: build(:user)) + } + end + + let(:current_user) { developer } + let(:ff_value) { true } + let(:access_token) { tokens[:write] } + let(:headers) { { 'Authorization' => "Bearer #{access_token.token}" } } + let(:project_id) { project.id } + let(:default_params) { {} } + let(:params) { default_params } + let(:request) { get api(route), params: params, headers: headers } + let(:json_response) { Gitlab::Json.parse(api_response.body) } + let(:presented_experiment) do + { + 'experiment_id' => experiment.iid.to_s, + 'name' => experiment.name, + 'lifecycle_stage' => 'active', + 'artifact_location' => 'not_implemented', + 'tags' => [ + { + 'key' => experiment.metadata[0].name, + 'value' => experiment.metadata[0].value + }, + { + 'key' => experiment.metadata[1].name, + 'value' => experiment.metadata[1].value + } + ] + } + end + + subject(:api_response) do + request + response + end + + before do + stub_feature_flags(ml_experiment_tracking: ff_value) + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get' do + let(:experiment_iid) { experiment.iid.to_s } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } + + it 'returns the experiment', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/get_experiment') + expect(json_response).to include({ 'experiment' => presented_experiment }) + end + + describe 'Error States' do + context 'when has access' do + context 'and experiment does not exist' do + let(:experiment_iid) { non_existing_record_iid.to_s } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + context 'and experiment_id is not passed' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get" } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires read_api scope' + end + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/list' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/list" } + + it 'returns the experiments', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/list_experiments') + expect(json_response).to include({ 'experiments' => [presented_experiment] }) + end + + context 'when there are no experiments' do + let(:project_id) { another_project.id } + + it 'returns an empty list' do + expect(json_response).to include({ 'experiments' => [] }) + end + end + + describe 'Error States' do + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires read_api scope' + end + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get-by-name' do + let(:experiment_name) { experiment.name } + let(:route) do + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" + end + + it 'returns the experiment', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/get_experiment') + expect(json_response).to include({ 'experiment' => presented_experiment }) + end + + describe 'Error States' do + context 'when has access but experiment does not exist' do + let(:experiment_name) { "random_experiment" } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + context 'when has access but experiment_name is not passed' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name" } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires read_api scope' + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/create' do + let(:route) do + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/create" + end + + let(:params) { { name: 'new_experiment' } } + let(:request) { post api(route), params: params, headers: headers } + + it 'creates the experiment', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response).to include('experiment_id') + end + + describe 'Error States' do + context 'when experiment name is not passed' do + let(:params) { {} } + + it_behaves_like 'MLflow|Bad Request' + end + + context 'when experiment name already exists' do + let(:existing_experiment) do + create(:ml_experiments, user: current_user, project: project) + end + + let(:params) { { name: existing_experiment.name } } + + it "is Bad Request", :aggregate_failures do + is_expected.to have_gitlab_http_status(:bad_request) + + expect(json_response).to include({ 'error_code' => 'RESOURCE_ALREADY_EXISTS' }) + end + end + + context 'when project does not exist' do + let(:route) { "/projects/#{non_existing_record_id}/ml/mlflow/api/2.0/mlflow/experiments/create" } + + it "is Not Found", :aggregate_failures do + is_expected.to have_gitlab_http_status(:not_found) + + expect(json_response['message']).to eq('404 Project Not Found') + end + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/set-experiment-tag' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/set-experiment-tag" } + let(:default_params) { { experiment_id: experiment.iid.to_s, key: 'some_key', value: 'value' } } + let(:params) { default_params } + let(:request) { post api(route), params: params, headers: headers } + + it 'logs the tag', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(experiment.reload.metadata.map(&:name)).to include('some_key') + end + + describe 'Error Cases' do + context 'when tag was already set' do + let(:params) { default_params.merge(key: experiment.metadata[0].name) } + + it_behaves_like 'MLflow|Bad Request' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] + end + end +end diff --git a/spec/requests/api/ml/mlflow/runs_spec.rb b/spec/requests/api/ml/mlflow/runs_spec.rb new file mode 100644 index 00000000000..746372b7978 --- /dev/null +++ b/spec/requests/api/ml/mlflow/runs_spec.rb @@ -0,0 +1,354 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Ml::Mlflow::Runs, feature_category: :mlops do + let_it_be(:project) { create(:project) } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:another_project) { build(:project).tap { |p| p.add_developer(developer) } } + let_it_be(:experiment) do + create(:ml_experiments, :with_metadata, project: project) + end + + let_it_be(:candidate) do + create(:ml_candidates, + :with_metrics_and_params, :with_metadata, + user: experiment.user, start_time: 1234, experiment: experiment, project: project) + end + + let_it_be(:tokens) do + { + write: create(:personal_access_token, scopes: %w[read_api api], user: developer), + read: create(:personal_access_token, scopes: %w[read_api], user: developer), + no_access: create(:personal_access_token, scopes: %w[read_user], user: developer), + different_user: create(:personal_access_token, scopes: %w[read_api api], user: build(:user)) + } + end + + let(:current_user) { developer } + let(:ff_value) { true } + let(:access_token) { tokens[:write] } + let(:headers) { { 'Authorization' => "Bearer #{access_token.token}" } } + let(:project_id) { project.id } + let(:default_params) { {} } + let(:params) { default_params } + let(:request) { get api(route), params: params, headers: headers } + let(:json_response) { Gitlab::Json.parse(api_response.body) } + + subject(:api_response) do + request + response + end + + before do + stub_feature_flags(ml_experiment_tracking: ff_value) + end + + RSpec.shared_examples 'MLflow|run_id param error cases' do + context 'when run id is not passed' do + let(:params) { {} } + + it "is Bad Request" do + is_expected.to have_gitlab_http_status(:bad_request) + end + end + + context 'when run_id is invalid' do + let(:params) { default_params.merge(run_id: non_existing_record_iid.to_s) } + + it "is Resource Does Not Exist", :aggregate_failures do + is_expected.to have_gitlab_http_status(:not_found) + + expect(json_response).to include({ "error_code" => 'RESOURCE_DOES_NOT_EXIST' }) + end + end + + context 'when run_id is not in in the project' do + let(:project_id) { another_project.id } + + it "is Resource Does Not Exist", :aggregate_failures do + is_expected.to have_gitlab_http_status(:not_found) + + expect(json_response).to include({ "error_code" => 'RESOURCE_DOES_NOT_EXIST' }) + end + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/create' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/create" } + let(:params) do + { + experiment_id: experiment.iid.to_s, + start_time: Time.now.to_i, + run_name: "A new Run", + tags: [ + { key: 'hello', value: 'world' } + ] + } + end + + let(:request) { post api(route), params: params, headers: headers } + + it 'creates the run', :aggregate_failures do + expected_properties = { + 'experiment_id' => params[:experiment_id], + 'user_id' => current_user.id.to_s, + 'run_name' => "A new Run", + 'start_time' => params[:start_time], + 'status' => 'RUNNING', + 'lifecycle_stage' => 'active' + } + + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/run') + expect(json_response['run']).to include('info' => hash_including(**expected_properties), + 'data' => { + 'metrics' => [], + 'params' => [], + 'tags' => [{ 'key' => 'hello', 'value' => 'world' }] + }) + end + + describe 'Error States' do + context 'when experiment id is not passed' do + let(:params) { {} } + + it_behaves_like 'MLflow|Bad Request' + end + + context 'when experiment id does not exist' do + let(:params) { { experiment_id: non_existing_record_iid.to_s } } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + context 'when experiment exists but is not part of the project' do + let(:project_id) { another_project.id } + + it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + end + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/runs/get' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/get" } + let(:default_params) { { 'run_id' => candidate.eid } } + + it 'gets the run', :aggregate_failures do + expected_properties = { + 'experiment_id' => candidate.experiment.iid.to_s, + 'user_id' => candidate.user.id.to_s, + 'start_time' => candidate.start_time, + 'artifact_uri' => "http://www.example.com/api/v4/projects/#{project_id}/packages/generic/ml_experiment_#{experiment.iid}/#{candidate.iid}/", + 'status' => "RUNNING", + 'lifecycle_stage' => "active" + } + + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/run') + expect(json_response['run']).to include( + 'info' => hash_including(**expected_properties), + 'data' => { + 'metrics' => [ + hash_including('key' => candidate.metrics[0].name), + hash_including('key' => candidate.metrics[1].name) + ], + 'params' => [ + { 'key' => candidate.params[0].name, 'value' => candidate.params[0].value }, + { 'key' => candidate.params[1].name, 'value' => candidate.params[1].value } + ], + 'tags' => [ + { 'key' => candidate.metadata[0].name, 'value' => candidate.metadata[0].value }, + { 'key' => candidate.metadata[1].name, 'value' => candidate.metadata[1].value } + ] + }) + end + + describe 'Error States' do + it_behaves_like 'MLflow|run_id param error cases' + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires read_api scope' + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/update' do + let(:default_params) { { run_id: candidate.eid.to_s, status: 'FAILED', end_time: Time.now.to_i } } + let(:request) { post api(route), params: params, headers: headers } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/update" } + + it 'updates the run', :aggregate_failures do + expected_properties = { + 'experiment_id' => candidate.experiment.iid.to_s, + 'user_id' => candidate.user.id.to_s, + 'start_time' => candidate.start_time, + 'end_time' => params[:end_time], + 'artifact_uri' => "http://www.example.com/api/v4/projects/#{project_id}/packages/generic/ml_experiment_#{experiment.iid}/#{candidate.iid}/", + 'status' => 'FAILED', + 'lifecycle_stage' => 'active' + } + + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/update_run') + expect(json_response).to include('run_info' => hash_including(**expected_properties)) + end + + describe 'Error States' do + context 'when status in invalid' do + let(:params) { default_params.merge(status: 'YOLO') } + + it_behaves_like 'MLflow|Bad Request' + end + + context 'when end_time is invalid' do + let(:params) { default_params.merge(end_time: 's') } + + it_behaves_like 'MLflow|Bad Request' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|run_id param error cases' + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-metric' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-metric" } + let(:default_params) { { run_id: candidate.eid.to_s, key: 'some_key', value: 10.0, timestamp: Time.now.to_i } } + let(:request) { post api(route), params: params, headers: headers } + + it 'logs the metric', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate.metrics.reload.length).to eq(3) + end + + describe 'Error Cases' do + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|run_id param error cases' + it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value, :timestamp] + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-parameter' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-parameter" } + let(:default_params) { { run_id: candidate.eid.to_s, key: 'some_key', value: 'value' } } + let(:request) { post api(route), params: params, headers: headers } + + it 'logs the parameter', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate.params.reload.length).to eq(3) + end + + describe 'Error Cases' do + context 'when parameter was already logged' do + let(:params) { default_params.tap { |p| p[:key] = candidate.params[0].name } } + + it_behaves_like 'MLflow|Bad Request' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|run_id param error cases' + it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/set-tag' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/set-tag" } + let(:default_params) { { run_id: candidate.eid.to_s, key: 'some_key', value: 'value' } } + let(:request) { post api(route), params: params, headers: headers } + + it 'logs the tag', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate.reload.metadata.map(&:name)).to include('some_key') + end + + describe 'Error Cases' do + context 'when tag was already logged' do + let(:params) { default_params.tap { |p| p[:key] = candidate.metadata[0].name } } + + it_behaves_like 'MLflow|Bad Request' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|run_id param error cases' + it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] + end + end + + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-batch' do + let_it_be(:candidate2) do + create(:ml_candidates, user: experiment.user, start_time: 1234, experiment: experiment, project: project) + end + + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-batch" } + let(:default_params) do + { + run_id: candidate2.eid.to_s, + metrics: [ + { key: 'mae', value: 2.5, timestamp: 1552550804 }, + { key: 'rmse', value: 2.7, timestamp: 1552550804 } + ], + params: [{ key: 'model_class', value: 'LogisticRegression' }], + tags: [{ key: 'tag1', value: 'tag.value.1' }] + } + end + + let(:request) { post api(route), params: params, headers: headers } + + it 'logs parameters and metrics', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate2.params.size).to eq(1) + expect(candidate2.metadata.size).to eq(1) + expect(candidate2.metrics.size).to eq(2) + end + + context 'when parameter was already logged' do + let(:params) do + default_params.tap { |p| p[:params] = [{ key: 'hello', value: 'a' }, { key: 'hello', value: 'b' }] } + end + + it 'does not log', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(candidate2.params.reload.size).to eq(1) + end + end + + context 'when tag was already logged' do + let(:params) do + default_params.tap { |p| p[:tags] = [{ key: 'tag1', value: 'a' }, { key: 'tag1', value: 'b' }] } + end + + it 'logs only 1', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + expect(candidate2.metadata.reload.size).to eq(1) + end + end + + describe 'Error Cases' do + context 'when required metric key is missing' do + let(:params) { default_params.tap { |p| p[:metrics] = [p[:metrics][0].delete(:key)] } } + + it_behaves_like 'MLflow|Bad Request' + end + + context 'when required param key is missing' do + let(:params) { default_params.tap { |p| p[:params] = [p[:params][0].delete(:key)] } } + + it_behaves_like 'MLflow|Bad Request' + end + + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope' + it_behaves_like 'MLflow|run_id param error cases' + end + end +end diff --git a/spec/requests/api/ml/mlflow_spec.rb b/spec/requests/api/ml/mlflow_spec.rb deleted file mode 100644 index 5c6289948cc..00000000000 --- a/spec/requests/api/ml/mlflow_spec.rb +++ /dev/null @@ -1,630 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require 'mime/types' - -RSpec.describe API::Ml::Mlflow, feature_category: :mlops do - include SessionHelpers - include ApiHelpers - include HttpBasicAuthHelpers - - let_it_be(:project) { create(:project, :private) } - let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } - let_it_be(:another_project) { build(:project).tap { |p| p.add_developer(developer) } } - let_it_be(:experiment) do - create(:ml_experiments, :with_metadata, project: project) - end - - let_it_be(:candidate) do - create(:ml_candidates, - :with_metrics_and_params, :with_metadata, - user: experiment.user, start_time: 1234, experiment: experiment, project: project) - end - - let_it_be(:tokens) do - { - write: create(:personal_access_token, scopes: %w[read_api api], user: developer), - read: create(:personal_access_token, scopes: %w[read_api], user: developer), - no_access: create(:personal_access_token, scopes: %w[read_user], user: developer), - different_user: create(:personal_access_token, scopes: %w[read_api api], user: build(:user)) - } - end - - let(:current_user) { developer } - let(:ff_value) { true } - let(:access_token) { tokens[:write] } - let(:headers) do - { 'Authorization' => "Bearer #{access_token.token}" } - end - - let(:project_id) { project.id } - let(:default_params) { {} } - let(:params) { default_params } - let(:request) { get api(route), params: params, headers: headers } - - before do - stub_feature_flags(ml_experiment_tracking: ff_value) - - request - end - - shared_examples 'Not Found' do |message| - it "is Not Found" do - expect(response).to have_gitlab_http_status(:not_found) - - expect(json_response['message']).to eq(message) if message.present? - end - end - - shared_examples 'Not Found - Resource Does Not Exist' do - it "is Resource Does Not Exist" do - expect(response).to have_gitlab_http_status(:not_found) - - expect(json_response).to include({ "error_code" => 'RESOURCE_DOES_NOT_EXIST' }) - end - end - - shared_examples 'Requires api scope' do - context 'when user has access but token has wrong scope' do - let(:access_token) { tokens[:read] } - - it { expect(response).to have_gitlab_http_status(:forbidden) } - end - end - - shared_examples 'Requires read_api scope' do - context 'when user has access but token has wrong scope' do - let(:access_token) { tokens[:no_access] } - - it { expect(response).to have_gitlab_http_status(:forbidden) } - end - end - - shared_examples 'Bad Request' do |error_code = nil| - it "is Bad Request" do - expect(response).to have_gitlab_http_status(:bad_request) - - expect(json_response).to include({ 'error_code' => error_code }) if error_code.present? - end - end - - shared_examples 'shared error cases' do - context 'when not authenticated' do - let(:headers) { {} } - - it "is Unauthorized" do - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context 'when user does not have access' do - let(:access_token) { tokens[:different_user] } - - it_behaves_like 'Not Found' - end - - context 'when ff is disabled' do - let(:ff_value) { false } - - it_behaves_like 'Not Found' - end - end - - shared_examples 'run_id param error cases' do - context 'when run id is not passed' do - let(:params) { {} } - - it_behaves_like 'Bad Request' - end - - context 'when run_id is invalid' do - let(:params) { default_params.merge(run_id: non_existing_record_iid.to_s) } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - - context 'when run_id is not in in the project' do - let(:project_id) { another_project.id } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - end - - shared_examples 'Bad Request on missing required' do |keys| - keys.each do |key| - context "when \"#{key}\" is missing" do - let(:params) { default_params.tap { |p| p.delete(key) } } - - it_behaves_like 'Bad Request' - end - end - end - - describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get' do - let(:experiment_iid) { experiment.iid.to_s } - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } - - it 'returns the experiment', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('ml/get_experiment') - expect(json_response).to include({ - 'experiment' => { - 'experiment_id' => experiment_iid, - 'name' => experiment.name, - 'lifecycle_stage' => 'active', - 'artifact_location' => 'not_implemented', - 'tags' => [ - { - 'key' => experiment.metadata[0].name, - 'value' => experiment.metadata[0].value - }, - { - 'key' => experiment.metadata[1].name, - 'value' => experiment.metadata[1].value - } - ] - } - }) - end - - describe 'Error States' do - context 'when has access' do - context 'and experiment does not exist' do - let(:experiment_iid) { non_existing_record_iid.to_s } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - - context 'and experiment_id is not passed' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get" } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires read_api scope' - end - end - - describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/list' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/list" } - - it 'returns the experiments' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('ml/list_experiments') - expect(json_response).to include({ - 'experiments' => [ - 'experiment_id' => experiment.iid.to_s, - 'name' => experiment.name, - 'lifecycle_stage' => 'active', - 'artifact_location' => 'not_implemented', - 'tags' => [ - { - 'key' => experiment.metadata[0].name, - 'value' => experiment.metadata[0].value - }, - { - 'key' => experiment.metadata[1].name, - 'value' => experiment.metadata[1].value - } - ] - ] - }) - end - - context 'when there are no experiments' do - let(:project_id) { another_project.id } - - it 'returns an empty list' do - expect(json_response).to include({ 'experiments' => [] }) - end - end - - describe 'Error States' do - it_behaves_like 'shared error cases' - it_behaves_like 'Requires read_api scope' - end - end - - describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get-by-name' do - let(:experiment_name) { experiment.name } - let(:route) do - "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" - end - - it 'returns the experiment', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('ml/get_experiment') - expect(json_response).to include({ - 'experiment' => { - 'experiment_id' => experiment.iid.to_s, - 'name' => experiment_name, - 'lifecycle_stage' => 'active', - 'artifact_location' => 'not_implemented', - 'tags' => [ - { - 'key' => experiment.metadata[0].name, - 'value' => experiment.metadata[0].value - }, - { - 'key' => experiment.metadata[1].name, - 'value' => experiment.metadata[1].value - } - ] - } - }) - end - - describe 'Error States' do - context 'when has access but experiment does not exist' do - let(:experiment_name) { "random_experiment" } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - - context 'when has access but experiment_name is not passed' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name" } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires read_api scope' - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/create' do - let(:route) do - "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/create" - end - - let(:params) { { name: 'new_experiment' } } - let(:request) { post api(route), params: params, headers: headers } - - it 'creates the experiment', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('experiment_id') - end - - describe 'Error States' do - context 'when experiment name is not passed' do - let(:params) { {} } - - it_behaves_like 'Bad Request' - end - - context 'when experiment name already exists' do - let(:existing_experiment) do - create(:ml_experiments, user: current_user, project: project) - end - - let(:params) { { name: existing_experiment.name } } - - it_behaves_like 'Bad Request', 'RESOURCE_ALREADY_EXISTS' - end - - context 'when project does not exist' do - let(:route) { "/projects/#{non_existing_record_id}/ml/mlflow/api/2.0/mlflow/experiments/create" } - - it_behaves_like 'Not Found', '404 Project Not Found' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/set-experiment-tag' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/set-experiment-tag" } - let(:default_params) { { experiment_id: experiment.iid.to_s, key: 'some_key', value: 'value' } } - let(:params) { default_params } - let(:request) { post api(route), params: params, headers: headers } - - it 'logs the tag', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty - expect(experiment.reload.metadata.map(&:name)).to include('some_key') - end - - describe 'Error Cases' do - context 'when tag was already set' do - let(:params) { default_params.merge(key: experiment.metadata[0].name) } - - it_behaves_like 'Bad Request' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - it_behaves_like 'Bad Request on missing required', [:key, :value] - end - end - - describe 'Runs' do - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/create' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/create" } - let(:params) do - { - experiment_id: experiment.iid.to_s, - start_time: Time.now.to_i, - run_name: "A new Run", - tags: [ - { key: 'hello', value: 'world' } - ] - } - end - - let(:request) { post api(route), params: params, headers: headers } - - it 'creates the run', :aggregate_failures do - expected_properties = { - 'experiment_id' => params[:experiment_id], - 'user_id' => current_user.id.to_s, - 'run_name' => "A new Run", - 'start_time' => params[:start_time], - 'status' => 'RUNNING', - 'lifecycle_stage' => 'active' - } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('ml/run') - expect(json_response['run']).to include('info' => hash_including(**expected_properties), - 'data' => { - 'metrics' => [], - 'params' => [], - 'tags' => [{ 'key' => 'hello', 'value' => 'world' }] - }) - end - - describe 'Error States' do - context 'when experiment id is not passed' do - let(:params) { {} } - - it_behaves_like 'Bad Request' - end - - context 'when experiment id does not exist' do - let(:params) { { experiment_id: non_existing_record_iid.to_s } } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - - context 'when experiment exists but is not part of the project' do - let(:project_id) { another_project.id } - - it_behaves_like 'Not Found - Resource Does Not Exist' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - end - end - - describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/runs/get' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/get" } - let(:default_params) { { 'run_id' => candidate.eid } } - - it 'gets the run', :aggregate_failures do - expected_properties = { - 'experiment_id' => candidate.experiment.iid.to_s, - 'user_id' => candidate.user.id.to_s, - 'start_time' => candidate.start_time, - 'artifact_uri' => "http://www.example.com/api/v4/projects/#{project_id}/packages/generic/ml_experiment_#{experiment.iid}/#{candidate.iid}/", - 'status' => "RUNNING", - 'lifecycle_stage' => "active" - } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('ml/run') - expect(json_response['run']).to include( - 'info' => hash_including(**expected_properties), - 'data' => { - 'metrics' => [ - hash_including('key' => candidate.metrics[0].name), - hash_including('key' => candidate.metrics[1].name) - ], - 'params' => [ - { 'key' => candidate.params[0].name, 'value' => candidate.params[0].value }, - { 'key' => candidate.params[1].name, 'value' => candidate.params[1].value } - ], - 'tags' => [ - { 'key' => candidate.metadata[0].name, 'value' => candidate.metadata[0].value }, - { 'key' => candidate.metadata[1].name, 'value' => candidate.metadata[1].value } - ] - }) - end - - describe 'Error States' do - it_behaves_like 'run_id param error cases' - it_behaves_like 'shared error cases' - it_behaves_like 'Requires read_api scope' - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/update' do - let(:default_params) { { run_id: candidate.eid.to_s, status: 'FAILED', end_time: Time.now.to_i } } - let(:request) { post api(route), params: params, headers: headers } - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/update" } - - it 'updates the run', :aggregate_failures do - expected_properties = { - 'experiment_id' => candidate.experiment.iid.to_s, - 'user_id' => candidate.user.id.to_s, - 'start_time' => candidate.start_time, - 'end_time' => params[:end_time], - 'artifact_uri' => "http://www.example.com/api/v4/projects/#{project_id}/packages/generic/ml_experiment_#{experiment.iid}/#{candidate.iid}/", - 'status' => 'FAILED', - 'lifecycle_stage' => 'active' - } - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('ml/update_run') - expect(json_response).to include('run_info' => hash_including(**expected_properties)) - end - - describe 'Error States' do - context 'when status in invalid' do - let(:params) { default_params.merge(status: 'YOLO') } - - it_behaves_like 'Bad Request' - end - - context 'when end_time is invalid' do - let(:params) { default_params.merge(end_time: 's') } - - it_behaves_like 'Bad Request' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - it_behaves_like 'run_id param error cases' - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-metric' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-metric" } - let(:default_params) { { run_id: candidate.eid.to_s, key: 'some_key', value: 10.0, timestamp: Time.now.to_i } } - let(:request) { post api(route), params: params, headers: headers } - - it 'logs the metric', :aggregate_failures do - candidate.metrics.reload - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty - expect(candidate.metrics.length).to eq(3) - end - - describe 'Error Cases' do - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - it_behaves_like 'run_id param error cases' - it_behaves_like 'Bad Request on missing required', [:key, :value, :timestamp] - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-parameter' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-parameter" } - let(:default_params) { { run_id: candidate.eid.to_s, key: 'some_key', value: 'value' } } - let(:request) { post api(route), params: params, headers: headers } - - it 'logs the parameter', :aggregate_failures do - candidate.params.reload - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty - expect(candidate.params.length).to eq(3) - end - - describe 'Error Cases' do - context 'when parameter was already logged' do - let(:params) { default_params.tap { |p| p[:key] = candidate.params[0].name } } - - it_behaves_like 'Bad Request' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - it_behaves_like 'run_id param error cases' - it_behaves_like 'Bad Request on missing required', [:key, :value] - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/set-tag' do - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/set-tag" } - let(:default_params) { { run_id: candidate.eid.to_s, key: 'some_key', value: 'value' } } - let(:request) { post api(route), params: params, headers: headers } - - it 'logs the tag', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty - expect(candidate.reload.metadata.map(&:name)).to include('some_key') - end - - describe 'Error Cases' do - context 'when tag was already logged' do - let(:params) { default_params.tap { |p| p[:key] = candidate.metadata[0].name } } - - it_behaves_like 'Bad Request' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - it_behaves_like 'run_id param error cases' - it_behaves_like 'Bad Request on missing required', [:key, :value] - end - end - - describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-batch' do - let(:candidate2) do - create(:ml_candidates, user: experiment.user, start_time: 1234, experiment: experiment, project: project) - end - - let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-batch" } - let(:default_params) do - { - run_id: candidate2.eid.to_s, - metrics: [ - { key: 'mae', value: 2.5, timestamp: 1552550804 }, - { key: 'rmse', value: 2.7, timestamp: 1552550804 } - ], - params: [{ key: 'model_class', value: 'LogisticRegression' }], - tags: [{ key: 'tag1', value: 'tag.value.1' }] - } - end - - let(:request) { post api(route), params: params, headers: headers } - - it 'logs parameters and metrics', :aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_empty - expect(candidate2.params.size).to eq(1) - expect(candidate2.metadata.size).to eq(1) - expect(candidate2.metrics.size).to eq(2) - end - - context 'when parameter was already logged' do - let(:params) do - default_params.tap { |p| p[:params] = [{ key: 'hello', value: 'a' }, { key: 'hello', value: 'b' }] } - end - - it 'does not log', :aggregate_failures do - candidate.params.reload - - expect(response).to have_gitlab_http_status(:ok) - expect(candidate2.params.size).to eq(1) - end - end - - context 'when tag was already logged' do - let(:params) do - default_params.tap { |p| p[:tags] = [{ key: 'tag1', value: 'a' }, { key: 'tag1', value: 'b' }] } - end - - it 'logs only 1', :aggregate_failures do - candidate.metadata.reload - - expect(response).to have_gitlab_http_status(:ok) - expect(candidate2.metadata.size).to eq(1) - end - end - - describe 'Error Cases' do - context 'when required metric key is missing' do - let(:params) { default_params.tap { |p| p[:metrics] = [p[:metrics][0].delete(:key)] } } - - it_behaves_like 'Bad Request' - end - - context 'when required param key is missing' do - let(:params) { default_params.tap { |p| p[:params] = [p[:params][0].delete(:key)] } } - - it_behaves_like 'Bad Request' - end - - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' - it_behaves_like 'run_id param error cases' - end - end - end -end diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index 54ee5e489f7..9933bf1545d 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -27,6 +27,21 @@ RSpec.describe 'merge requests discussions', feature_category: :source_code_mana end # rubocop:enable RSpec/InstanceVariable + shared_examples 'N+1 queries' do + it 'avoids N+1 DB queries', :request_store do + send_request # warm up + + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) + control = ActiveRecord::QueryRecorder.new { send_request } + + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) + + expect do + send_request + end.not_to exceed_query_limit(control).with_threshold(notes_metadata_threshold) + end + end + it 'returns 200' do send_request @@ -34,17 +49,20 @@ RSpec.describe 'merge requests discussions', feature_category: :source_code_mana end # https://docs.gitlab.com/ee/development/query_recorder.html#use-request-specs-instead-of-controller-specs - it 'avoids N+1 DB queries', :request_store do - send_request # warm up + context 'with notes_metadata_threshold' do + let(:notes_metadata_threshold) { 1 } - create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) - control = ActiveRecord::QueryRecorder.new { send_request } + it_behaves_like 'N+1 queries' - create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) + context 'when external_note_author_service_desk feature flag is disabled' do + let(:notes_metadata_threshold) { 0 } - expect do - send_request - end.not_to exceed_query_limit(control) + before do + stub_feature_flags(external_note_author_service_desk: false) + end + + it_behaves_like 'N+1 queries' + end end it 'limits Gitaly queries', :request_store do diff --git a/spec/serializers/note_entity_spec.rb b/spec/serializers/note_entity_spec.rb index 19438e69a10..bbb1d2ca164 100644 --- a/spec/serializers/note_entity_spec.rb +++ b/spec/serializers/note_entity_spec.rb @@ -14,4 +14,67 @@ RSpec.describe NoteEntity do subject { entity.as_json } it_behaves_like 'note entity' + + shared_examples 'external author' do + context 'when anonymous' do + let(:user) { nil } + + it { is_expected.to eq(obfuscated_email) } + end + + context 'with signed in user' do + before do + stub_member_access_level(note.project, access_level => user) if access_level + end + + context 'when user has no role in project' do + let(:access_level) { nil } + + it { is_expected.to eq(obfuscated_email) } + end + + context 'when user has guest role in project' do + let(:access_level) { :guest } + + it { is_expected.to eq(obfuscated_email) } + end + + context 'when user has reporter role in project' do + let(:access_level) { :reporter } + + it { is_expected.to eq(email) } + end + + context 'when user has developer role in project' do + let(:access_level) { :developer } + + it { is_expected.to eq(email) } + end + end + end + + describe 'with email participant' do + let_it_be(:note) { create(:note) } + let_it_be(:note_metadata) { create(:note_metadata, note: note) } + + subject { entity.as_json[:external_author] } + + context 'when external_note_author_service_desk feature flag is enabled' do + let(:obfuscated_email) { 'em*****@e*****.c**' } + let(:email) { 'email@example.com' } + + it_behaves_like 'external author' + end + + context 'when external_note_author_service_desk feature flag is disabled' do + let(:email) { nil } + let(:obfuscated_email) { nil } + + before do + stub_feature_flags(external_note_author_service_desk: false) + end + + it_behaves_like 'external author' + end + end end diff --git a/spec/support/shared_examples/lib/menus_shared_examples.rb b/spec/support/shared_examples/lib/menus_shared_examples.rb index ed3165079fb..0aa98517444 100644 --- a/spec/support/shared_examples/lib/menus_shared_examples.rb +++ b/spec/support/shared_examples/lib/menus_shared_examples.rb @@ -61,3 +61,34 @@ RSpec.shared_examples_for 'not serializable as super_sidebar_menu_args' do expect(menu.serialize_as_menu_item_args).to be_nil end end + +RSpec.shared_examples_for 'a panel with uniquely identifiable menu items' do + let(:menu_items) do + subject.instance_variable_get(:@menus) + .flat_map { |menu| menu.instance_variable_get(:@items) } + end + + it 'all menu_items have unique item_id' do + duplicated_ids = menu_items.group_by(&:item_id).reject { |_, v| (v.size < 2) } + + expect(duplicated_ids).to eq({}) + end + + it 'all menu_items have an item_id' do + items_with_nil_id = menu_items.select { |item| item.item_id.nil? } + + expect(items_with_nil_id).to match_array([]) + end +end + +RSpec.shared_examples_for 'a panel with all menu_items categorized' do + let(:uncategorized_menu) do + subject.instance_variable_get(:@menus) + .find { |menu| menu.instance_of?(::Sidebars::UncategorizedMenu) } + end + + it 'has no uncategorized menu_items' do + uncategorized_menu_items = uncategorized_menu.instance_variable_get(:@items) + expect(uncategorized_menu_items).to eq([]) + end +end diff --git a/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb new file mode 100644 index 00000000000..2ca62698daf --- /dev/null +++ b/spec/support/shared_examples/requests/api/ml/mlflow/mlflow_shared_examples.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'MLflow|Not Found - Resource Does Not Exist' do + it "is Resource Does Not Exist", :aggregate_failures do + is_expected.to have_gitlab_http_status(:not_found) + + expect(json_response).to include({ "error_code" => 'RESOURCE_DOES_NOT_EXIST' }) + end +end + +RSpec.shared_examples 'MLflow|Requires api scope' do + context 'when user has access but token has wrong scope' do + let(:access_token) { tokens[:read] } + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end +end + +RSpec.shared_examples 'MLflow|Requires read_api scope' do + context 'when user has access but token has wrong scope' do + let(:access_token) { tokens[:no_access] } + + it { is_expected.to have_gitlab_http_status(:forbidden) } + end +end + +RSpec.shared_examples 'MLflow|Bad Request' do + it "is Bad Request" do + is_expected.to have_gitlab_http_status(:bad_request) + end +end + +RSpec.shared_examples 'MLflow|shared error cases' do + context 'when not authenticated' do + let(:headers) { {} } + + it "is Unauthorized" do + is_expected.to have_gitlab_http_status(:unauthorized) + end + end + + context 'when user does not have access' do + let(:access_token) { tokens[:different_user] } + + it "is Not Found" do + is_expected.to have_gitlab_http_status(:not_found) + end + end + + context 'when ff is disabled' do + let(:ff_value) { false } + + it "is Not Found" do + is_expected.to have_gitlab_http_status(:not_found) + end + end +end + +RSpec.shared_examples 'MLflow|Bad Request on missing required' do |keys| + keys.each do |key| + context "when \"#{key}\" is missing" do + let(:params) { default_params.tap { |p| p.delete(key) } } + + it "is Bad Request" do + is_expected.to have_gitlab_http_status(:bad_request) + end + end + end +end diff --git a/spec/support/shared_examples/serializers/note_entity_shared_examples.rb b/spec/support/shared_examples/serializers/note_entity_shared_examples.rb index b5e3a407b53..e8238480ced 100644 --- a/spec/support/shared_examples/serializers/note_entity_shared_examples.rb +++ b/spec/support/shared_examples/serializers/note_entity_shared_examples.rb @@ -18,7 +18,8 @@ RSpec.shared_examples 'note entity' do :noteable_note_url, :report_abuse_path, :resolvable, - :type + :type, + :external_author ) end diff --git a/spec/workers/packages/cleanup/delete_orphaned_dependencies_worker_spec.rb b/spec/workers/packages/cleanup/delete_orphaned_dependencies_worker_spec.rb new file mode 100644 index 00000000000..ffa7767075e --- /dev/null +++ b/spec/workers/packages/cleanup/delete_orphaned_dependencies_worker_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Cleanup::DeleteOrphanedDependenciesWorker, feature_category: :package_registry do + let(:worker) { described_class.new } + + it { is_expected.to include_module(CronjobQueue) } + it { expect(described_class.idempotent?).to be_truthy } + + describe '#perform', :clean_gitlab_redis_shared_state do + let_it_be(:orphaned_dependencies) { create_list(:packages_dependency, 2) } + let_it_be(:linked_dependency) do + create(:packages_dependency).tap do |dependency| + create(:packages_dependency_link, dependency: dependency) + end + end + + subject { worker.perform } + + it 'deletes only orphaned dependencies' do + expect { subject }.to change { Packages::Dependency.count }.by(-2) + expect(Packages::Dependency.all).to contain_exactly(linked_dependency) + end + + it 'executes 3 queries' do + queries = ActiveRecord::QueryRecorder.new { subject } + + # 1. (each_batch lower bound) SELECT packages_dependencies.id FROM packages_dependencies + # WHERE packages_dependencies.id >= 0 + # ORDER BY packages_dependencies.id ASC LIMIT 1; + # 2. (each_batch upper bound) SELECT packages_dependencies.id FROM packages_dependencies + # WHERE packages_dependencies.id >= 0 + # AND packages_dependencies.id >= 1 ORDER BY packages_dependencies.id ASC + # LIMIT 1 OFFSET 100; + # 3. (delete query) DELETE FROM packages_dependencies WHERE packages_dependencies.id >= 0 + # AND packages_dependencies.id >= 1 + # AND (NOT EXISTS ( + # SELECT 1 FROM packages_dependency_links + # WHERE packages_dependency_links.dependency_id = packages_dependencies.id + # )); + expect(queries.count).to eq(3) + end + + context 'when the worker is running for more than the max time' do + before do + allow(worker).to receive(:over_time?).and_return(true) + end + + it 'sets the last processed dependency id in redis cache' do + subject + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.get('last_processed_packages_dependency_id').to_i).to eq(Packages::Dependency.last.id) + end + end + end + + context 'when the worker reaches the maximum number of batches' do + before do + stub_const('Packages::Cleanup::DeleteOrphanedDependenciesWorker::MAX_BATCHES', 1) + end + + it 'iterates over only 1 batch' do + expect { subject }.to change { Packages::Dependency.count }.by(-2) + end + + it 'sets the last processed dependency id in redis cache' do + subject + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.get('last_processed_packages_dependency_id').to_i).to eq(Packages::Dependency.last.id) + end + end + end + + context 'when the worker finishes processing in less than the max time' do + before do + Gitlab::Redis::SharedState.with do |redis| + redis.set('last_processed_packages_dependency_id', orphaned_dependencies.first.id) + end + end + + it 'clears the last processed last_processed_packages_dependency_id from redis cache' do + Gitlab::Redis::SharedState.with do |redis| + expect { subject } + .to change { redis.get('last_processed_packages_dependency_id') }.to(nil) + end + end + end + + context 'when logging extra metadata' do + before do + stub_const('Packages::Cleanup::DeleteOrphanedDependenciesWorker::MAX_BATCHES', 1) + end + + it 'logs the last proccessed id & the deleted rows count', :aggregate_failures do + expect(worker).to receive(:log_extra_metadata_on_done).with( + :last_processed_packages_dependency_id, + Packages::Dependency.last.id + ) + expect(worker).to receive(:log_extra_metadata_on_done).with(:deleted_rows_count, 2) + + subject + end + end + + context 'when the FF is disabled' do + before do + stub_feature_flags(packages_delete_orphaned_dependencies_worker: false) + end + + it 'does not execute the worker' do + expect { subject }.not_to change { Packages::Dependency.count } + end + end + end +end |