diff options
66 files changed, 1061 insertions, 341 deletions
diff --git a/.gitlab/issue_templates/Performance Indicator Metric.md b/.gitlab/issue_templates/Performance Indicator Metric.md deleted file mode 100644 index 8019be8cad5..00000000000 --- a/.gitlab/issue_templates/Performance Indicator Metric.md +++ /dev/null @@ -1,23 +0,0 @@ -<!-- -Performance Indicator Metric issues are used for adding, updating, or removing performance indicator type in Service Ping metrics. - -Please title your issue with the following format: "{action}(Add|Update|Remove) Metric name as performance indicator" - -Example of title: "Add some_feature_views as gmau" - ---> - -## Summary - -<!-- -Summary of the changes ---> - -## Tasks - -- [ ] [Link to metric definition]() -- [ ] Create issue in GitLab Data Team project using [Product Performance Indicator template](https://gitlab.com/gitlab-data/analytics/-/issues/new?issuable_template=Product%20Performance%20Indicator%20Template) - -See [Product Intelligence Guide](https://docs.gitlab.com/ee/development/service_ping/performance_indicator_metrics.html) for details - -/label ~"product intelligence" ~"Data Warehouse::Impact Check" diff --git a/.rubocop.yml b/.rubocop.yml index 0aa37f2d9f0..8ddd5087bb7 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -566,9 +566,6 @@ Graphql/Descriptions: RSpec/ImplicitSubject: Enabled: false -RSpec/Be: - Enabled: false - RSpec/DescribedClass: Enabled: false diff --git a/.rubocop_todo/rspec/be.yml b/.rubocop_todo/rspec/be.yml new file mode 100644 index 00000000000..dad8a7d730b --- /dev/null +++ b/.rubocop_todo/rspec/be.yml @@ -0,0 +1,22 @@ +--- +RSpec/Be: + Exclude: + - 'ee/spec/services/groups/transfer_service_spec.rb' + - 'spec/lib/bulk_imports/common/pipelines/boards_pipeline_spec.rb' + - 'spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb' + - 'spec/lib/gitlab/lets_encrypt/client_spec.rb' + - 'spec/lib/gitlab/search_context/builder_spec.rb' + - 'spec/migrations/20220503035221_add_gitlab_schema_to_batched_background_migrations_spec.rb' + - 'spec/models/concerns/issuable_spec.rb' + - 'spec/models/identity_spec.rb' + - 'spec/models/snippet_repository_spec.rb' + - 'spec/presenters/packages/nuget/search_results_presenter_spec.rb' + - 'spec/requests/api/graphql/mutations/snippets/create_spec.rb' + - 'spec/requests/api/pages_domains_spec.rb' + - 'spec/services/pages/delete_service_spec.rb' + - 'spec/services/pages/destroy_deployments_service_spec.rb' + - 'spec/services/pages/migrate_from_legacy_storage_service_spec.rb' + - 'spec/services/projects/update_pages_service_spec.rb' + - 'spec/support/shared_examples/requests/api/packages_shared_examples.rb' + - 'spec/uploaders/file_uploader_spec.rb' + - 'spec/uploaders/namespace_file_uploader_spec.rb' diff --git a/app/assets/javascripts/crm/constants.js b/app/assets/javascripts/crm/constants.js index 3b085837aea..815289e075e 100644 --- a/app/assets/javascripts/crm/constants.js +++ b/app/assets/javascripts/crm/constants.js @@ -1,3 +1,7 @@ export const INDEX_ROUTE_NAME = 'index'; export const NEW_ROUTE_NAME = 'new'; export const EDIT_ROUTE_NAME = 'edit'; +export const trackViewsOptions = { + category: 'Customer Relations' /* eslint-disable-line @gitlab/require-i18n-strings */, + action: 'view_contacts_list', +}; diff --git a/app/assets/javascripts/crm/contacts/bundle.js b/app/assets/javascripts/crm/contacts/bundle.js index f49ec64210f..fe62b7cfbe3 100644 --- a/app/assets/javascripts/crm/contacts/bundle.js +++ b/app/assets/javascripts/crm/contacts/bundle.js @@ -3,6 +3,7 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import VueRouter from 'vue-router'; import createDefaultClient from '~/lib/graphql'; +import { parseBoolean } from '~/lib/utils/common_utils'; import CrmContactsRoot from './components/contacts_root.vue'; import routes from './routes'; @@ -21,7 +22,14 @@ export default () => { return false; } - const { basePath, groupFullPath, groupIssuesPath, canAdminCrmContact, groupId } = el.dataset; + const { + basePath, + groupFullPath, + groupIssuesPath, + canAdminCrmContact, + groupId, + textQuery, + } = el.dataset; const router = new VueRouter({ base: basePath, @@ -33,7 +41,13 @@ export default () => { el, router, apolloProvider, - provide: { groupFullPath, groupIssuesPath, canAdminCrmContact, groupId }, + provide: { + groupFullPath, + groupIssuesPath, + canAdminCrmContact: parseBoolean(canAdminCrmContact), + groupId, + textQuery, + }, render(createElement) { return createElement(CrmContactsRoot); }, diff --git a/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue b/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue index be3b9bd72b8..b29089519e2 100644 --- a/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue +++ b/app/assets/javascripts/crm/contacts/components/contact_form_wrapper.vue @@ -57,7 +57,7 @@ export default { getQuery() { return { query: getGroupContactsQuery, - variables: { groupFullPath: this.groupFullPath }, + variables: { groupFullPath: this.groupFullPath, ids: [this.contactGraphQLId] }, }; }, title() { diff --git a/app/assets/javascripts/crm/contacts/components/contacts_root.vue b/app/assets/javascripts/crm/contacts/components/contacts_root.vue index 9d6f34c73b7..562363ff88e 100644 --- a/app/assets/javascripts/crm/contacts/components/contacts_root.vue +++ b/app/assets/javascripts/crm/contacts/components/contacts_root.vue @@ -1,36 +1,54 @@ <script> -import { GlAlert, GlButton, GlLoadingIcon, GlTable, GlTooltipDirective } from '@gitlab/ui'; -import { parseBoolean } from '~/lib/utils/common_utils'; +import { GlButton, GlLoadingIcon, GlTable, GlTooltipDirective } from '@gitlab/ui'; import { s__, __ } from '~/locale'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; -import { EDIT_ROUTE_NAME, NEW_ROUTE_NAME } from '../../constants'; -import getGroupContactsQuery from './graphql/get_group_contacts.query.graphql'; +import PaginatedTableWithSearchAndTabs from '~/vue_shared/components/paginated_table_with_search_and_tabs/paginated_table_with_search_and_tabs.vue'; +import { + bodyTrClass, + initialPaginationState, +} from '~/vue_shared/components/paginated_table_with_search_and_tabs/constants'; +import { convertToSnakeCase } from '~/lib/utils/text_utility'; +import { EDIT_ROUTE_NAME, NEW_ROUTE_NAME, trackViewsOptions } from '../../constants'; +import getGroupContacts from './graphql/get_group_contacts.query.graphql'; +import getGroupContactsCountByState from './graphql/get_group_contacts_count_by_state.graphql'; export default { components: { - GlAlert, GlButton, GlLoadingIcon, GlTable, + PaginatedTableWithSearchAndTabs, }, directives: { GlTooltip: GlTooltipDirective, }, - inject: ['canAdminCrmContact', 'groupFullPath', 'groupIssuesPath'], + inject: ['canAdminCrmContact', 'groupFullPath', 'groupIssuesPath', 'textQuery'], data() { return { - contacts: [], + contacts: { list: [] }, + contactsCount: {}, error: false, + filteredByStatus: '', + pagination: initialPaginationState, + statusFilter: 'all', + searchTerm: this.textQuery, + sort: 'LAST_NAME_ASC', + sortDesc: false, }; }, apollo: { contacts: { - query() { - return getGroupContactsQuery; - }, + query: getGroupContacts, variables() { return { groupFullPath: this.groupFullPath, + searchTerm: this.searchTerm, + state: this.statusFilter, + sort: this.sort, + firstPageSize: this.pagination.firstPageSize, + lastPageSize: this.pagination.lastPageSize, + prevPageCursor: this.pagination.prevPageCursor, + nextPageCursor: this.pagination.nextPageCursor, }; }, update(data) { @@ -40,19 +58,52 @@ export default { this.error = true; }, }, + contactsCount: { + query: getGroupContactsCountByState, + variables() { + return { + groupFullPath: this.groupFullPath, + searchTerm: this.searchTerm, + }; + }, + update(data) { + return data?.group?.contactStateCounts; + }, + error() { + this.error = true; + }, + }, }, computed: { isLoading() { return this.$apollo.queries.contacts.loading; }, - canAdmin() { - return parseBoolean(this.canAdminCrmContact); + tbodyTrClass() { + return { + [bodyTrClass]: !this.loading && !this.isEmpty, + }; }, }, methods: { + errorAlertDismissed() { + this.error = true; + }, extractContacts(data) { const contacts = data?.group?.contacts?.nodes || []; - return contacts.slice().sort((a, b) => a.firstName.localeCompare(b.firstName)); + const pageInfo = data?.group?.contacts?.pageInfo || {}; + return { + list: contacts, + pageInfo, + }; + }, + fetchSortedData({ sortBy, sortDesc }) { + const sortingColumn = convertToSnakeCase(sortBy).toUpperCase(); + const sortingDirection = sortDesc ? 'DESC' : 'ASC'; + this.pagination = initialPaginationState; + this.sort = `${sortingColumn}_${sortingDirection}`; + }, + filtersChanged({ searchTerm }) { + this.searchTerm = searchTerm; }, getIssuesPath(path, value) { return `${path}?crm_contact_id=${value}`; @@ -60,6 +111,13 @@ export default { getEditRoute(id) { return { name: this.$options.EDIT_ROUTE_NAME, params: { id } }; }, + pageChanged(pagination) { + this.pagination = pagination; + }, + statusChanged({ filters, status }) { + this.statusFilter = filters; + this.filteredByStatus = status; + }, }, fields: [ { key: 'firstName', sortable: true }, @@ -92,57 +150,109 @@ export default { }, EDIT_ROUTE_NAME, NEW_ROUTE_NAME, + statusTabs: [ + { + title: __('Active'), + status: 'ACTIVE', + filters: 'active', + }, + { + title: __('Inactive'), + status: 'INACTIVE', + filters: 'inactive', + }, + { + title: __('All'), + status: 'ALL', + filters: 'all', + }, + ], + trackViewsOptions, + emptyArray: [], }; </script> <template> <div> - <gl-alert v-if="error" variant="danger" class="gl-mt-6" @dismiss="error = false"> - {{ $options.i18n.errorText }} - </gl-alert> - <div - class="gl-display-flex gl-align-items-baseline gl-flex-direction-row gl-justify-content-space-between gl-mt-6" + <paginated-table-with-search-and-tabs + :show-items="true" + :show-error-msg="false" + :i18n="$options.i18n" + :items="contacts.list" + :page-info="contacts.pageInfo" + :items-count="contactsCount" + :status-tabs="$options.statusTabs" + :track-views-options="$options.trackViewsOptions" + :filter-search-tokens="$options.emptyArray" + filter-search-key="contacts" + @page-changed="pageChanged" + @tabs-changed="statusChanged" + @filters-changed="filtersChanged" + @error-alert-dismissed="errorAlertDismissed" > - <h2 class="gl-font-size-h2 gl-my-0"> - {{ $options.i18n.title }} - </h2> - <div v-if="canAdmin"> - <router-link :to="{ name: $options.NEW_ROUTE_NAME }"> - <gl-button variant="confirm" data-testid="new-contact-button"> + <template #header-actions> + <router-link v-if="canAdminCrmContact" :to="{ name: $options.NEW_ROUTE_NAME }"> + <gl-button class="gl-my-3 gl-mr-5" variant="confirm" data-testid="new-contact-button"> {{ $options.i18n.newContact }} </gl-button> </router-link> - </div> - </div> - <router-view /> - <gl-loading-icon v-if="isLoading" class="gl-mt-5" size="lg" /> - <gl-table - v-else - class="gl-mt-5" - :items="contacts" - :fields="$options.fields" - :empty-text="$options.i18n.emptyText" - show-empty - > - <template #cell(id)="{ value: id }"> - <gl-button - v-gl-tooltip.hover.bottom="$options.i18n.issuesButtonLabel" - class="gl-mr-3" - data-testid="issues-link" - icon="issues" - :aria-label="$options.i18n.issuesButtonLabel" - :href="getIssuesPath(groupIssuesPath, id)" - /> - <router-link :to="getEditRoute(id)"> - <gl-button - v-if="canAdmin" - v-gl-tooltip.hover.bottom="$options.i18n.editButtonLabel" - data-testid="edit-contact-button" - icon="pencil" - :aria-label="$options.i18n.editButtonLabel" - /> - </router-link> </template> - </gl-table> + + <template #title> + {{ $options.i18n.title }} + </template> + + <template #table> + <gl-table + :items="contacts.list" + :fields="$options.fields" + :busy="isLoading" + stacked="md" + :tbody-tr-class="tbodyTrClass" + sort-direction="asc" + :sort-desc.sync="sortDesc" + sort-by="createdAt" + show-empty + no-local-sorting + sort-icon-left + fixed + @sort-changed="fetchSortedData" + > + <template #cell(id)="{ value: id }"> + <gl-button + v-gl-tooltip.hover.bottom="$options.i18n.issuesButtonLabel" + class="gl-mr-3" + data-testid="issues-link" + icon="issues" + :aria-label="$options.i18n.issuesButtonLabel" + :href="getIssuesPath(groupIssuesPath, id)" + /> + <router-link :to="getEditRoute(id)"> + <gl-button + v-if="canAdminCrmContact" + v-gl-tooltip.hover.bottom="$options.i18n.editButtonLabel" + data-testid="edit-contact-button" + icon="pencil" + :aria-label="$options.i18n.editButtonLabel" + /> + </router-link> + </template> + + <template #table-busy> + <gl-loading-icon size="lg" color="dark" class="mt-3" /> + </template> + + <template #empty> + <span v-if="error"> + {{ $options.i18n.errorText }} + </span> + <span v-else> + {{ $options.i18n.emptyText }} + </span> + </template> + </gl-table> + </template> + </paginated-table-with-search-and-tabs> + <router-view /> </div> </template> diff --git a/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts.query.graphql b/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts.query.graphql index 2a8150e42e3..1d23a33460e 100644 --- a/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts.query.graphql +++ b/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts.query.graphql @@ -1,13 +1,38 @@ #import "./crm_contact_fields.fragment.graphql" -query contacts($groupFullPath: ID!) { +query contacts( + $groupFullPath: ID! + $state: CustomerRelationsContactState + $searchTerm: String + $sort: ContactSort + $firstPageSize: Int + $lastPageSize: Int + $prevPageCursor: String = "" + $nextPageCursor: String = "" + $ids: [CustomerRelationsContactID!] +) { group(fullPath: $groupFullPath) { __typename id - contacts { + contacts( + state: $state + search: $searchTerm + sort: $sort + first: $firstPageSize + last: $lastPageSize + after: $nextPageCursor + before: $prevPageCursor + ids: $ids + ) { nodes { ...ContactFragment } + pageInfo { + hasNextPage + endCursor + hasPreviousPage + startCursor + } } } } diff --git a/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts_count_by_state.graphql b/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts_count_by_state.graphql new file mode 100644 index 00000000000..6b591240096 --- /dev/null +++ b/app/assets/javascripts/crm/contacts/components/graphql/get_group_contacts_count_by_state.graphql @@ -0,0 +1,11 @@ +query contactsCountByState($groupFullPath: ID!, $searchTerm: String) { + group(fullPath: $groupFullPath) { + __typename + id + contactStateCounts(search: $searchTerm) { + all + active + inactive + } + } +} diff --git a/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue b/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue index 15c4e503685..3b1509e5be5 100644 --- a/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue +++ b/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue @@ -1,7 +1,6 @@ <script> import { mapState } from 'vuex'; import { GlBadge } from '@gitlab/ui'; -import { helpPagePath } from '~/helpers/help_page_helper'; import { timeIntervalInWords } from '~/lib/utils/datetime_utility'; import { __, sprintf } from '~/locale'; import timeagoMixin from '~/vue_shared/mixins/timeago'; @@ -47,11 +46,6 @@ export default { this.job.coverage, ); }, - runnerHelpUrl() { - return helpPagePath('ci/runners/configure_runners.html', { - anchor: 'set-maximum-job-timeout-for-a-runner', - }); - }, runnerId() { const { id, short_sha: token, description } = this.job.runner; @@ -85,6 +79,7 @@ export default { TAGS: __('Tags:'), TIMEOUT: __('Timeout'), }, + RUNNER_HELP_URL: 'https://docs.gitlab.com/runner/register/index.html', }; </script> @@ -101,7 +96,7 @@ export default { <detail-row v-if="job.queued_duration" :value="queuedDuration" :title="$options.i18n.QUEUED" /> <detail-row v-if="hasTimeout" - :help-url="runnerHelpUrl" + :help-url="$options.RUNNER_HELP_URL" :value="timeout" data-testid="job-timeout" :title="$options.i18n.TIMEOUT" diff --git a/app/assets/javascripts/pipeline_editor/components/drawer/cards/first_pipeline_card.vue b/app/assets/javascripts/pipeline_editor/components/drawer/cards/first_pipeline_card.vue index 1f74e89f90c..0b57433e894 100644 --- a/app/assets/javascripts/pipeline_editor/components/drawer/cards/first_pipeline_card.vue +++ b/app/assets/javascripts/pipeline_editor/components/drawer/cards/first_pipeline_card.vue @@ -28,13 +28,13 @@ export default { GlSprintf, }, mixins: [Tracking.mixin()], - inject: ['runnerHelpPagePath'], methods: { trackHelpPageClick() { const { label, actions } = pipelineEditorTrackingOptions; this.track(actions.helpDrawerLinks.runners, { label }); }, }, + RUNNER_HELP_URL: 'https://docs.gitlab.com/runner/register/index.html', }; </script> <template> @@ -47,7 +47,7 @@ export default { <p class="gl-mb-0"> <gl-sprintf :message="$options.i18n.note"> <template #link="{ content }"> - <gl-link :href="runnerHelpPagePath" target="_blank" @click="trackHelpPageClick()"> + <gl-link :href="$options.RUNNER_HELP_URL" target="_blank" @click="trackHelpPageClick()"> {{ content }} </gl-link> </template> diff --git a/app/assets/javascripts/pipeline_editor/index.js b/app/assets/javascripts/pipeline_editor/index.js index 4f5b69107bf..13dad0b2459 100644 --- a/app/assets/javascripts/pipeline_editor/index.js +++ b/app/assets/javascripts/pipeline_editor/index.js @@ -40,7 +40,6 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => { projectFullPath, projectPath, projectNamespace, - runnerHelpPagePath, simulatePipelineHelpPagePath, totalBranches, validateTabIllustrationPath, @@ -132,7 +131,6 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => { projectFullPath, projectPath, projectNamespace, - runnerHelpPagePath, simulatePipelineHelpPagePath, totalBranches: parseInt(totalBranches, 10), validateTabIllustrationPath, diff --git a/app/assets/javascripts/related_issues/components/related_issues_block.vue b/app/assets/javascripts/related_issues/components/related_issues_block.vue index 553904765a4..5dbdce11ea2 100644 --- a/app/assets/javascripts/related_issues/components/related_issues_block.vue +++ b/app/assets/javascripts/related_issues/components/related_issues_block.vue @@ -156,6 +156,10 @@ export default { handleToggle() { this.isOpen = !this.isOpen; }, + addButtonClick(event) { + this.isOpen = true; + this.$emit('toggleAddRelatedIssuesForm', event); + }, }, linkedIssueTypesTextMap, }; @@ -203,7 +207,7 @@ export default { :aria-label="addIssuableButtonText" :class="qaClass" class="gl-ml-3" - @click="$emit('toggleAddRelatedIssuesForm', $event)" + @click="addButtonClick" > <slot name="add-button-text">{{ __('Add') }}</slot> </gl-button> diff --git a/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_form.vue b/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_form.vue index c44ce8b0057..336c291d4f1 100644 --- a/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_form.vue +++ b/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_form.vue @@ -88,7 +88,10 @@ export default { .then( ({ data: { - issuableSetConfidential: { errors }, + issuableSetConfidential: { + issuable: { confidential }, + errors, + }, }, }) => { if (errors.length) { @@ -96,7 +99,7 @@ export default { message: errors[0], }); } else { - this.$emit('closeForm'); + this.$emit('closeForm', { confidential }); } }, ) diff --git a/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_widget.vue b/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_widget.vue index f234c5ea3c9..eec083f23f3 100644 --- a/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_widget.vue +++ b/app/assets/javascripts/sidebar/components/confidential/sidebar_confidentiality_widget.vue @@ -95,10 +95,10 @@ export default { confidentialWidget.setConfidentiality = null; }, methods: { - closeForm() { + closeForm({ confidential } = {}) { this.$refs.editable.collapse(); this.$el.dispatchEvent(hideDropdownEvent); - this.$emit('closeForm'); + this.$emit('closeForm', { confidential }); }, // synchronizing the quick action with the sidebar widget // this is a temporary solution until we have confidentiality real-time updates diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index 84e7c0f876a..fec4d0e346d 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -39,6 +39,7 @@ import SidebarTimeTracking from './components/time_tracking/sidebar_time_trackin import { IssuableAttributeType } from './constants'; import SidebarMoveIssue from './lib/sidebar_move_issue'; import CrmContacts from './components/crm_contacts/crm_contacts.vue'; +import SidebarEventHub from './event_hub'; Vue.use(Translate); Vue.use(VueApollo); @@ -359,6 +360,13 @@ function mountConfidentialComponent() { ? IssuableType.Issue : IssuableType.MergeRequest, }, + on: { + closeForm({ confidential }) { + if (confidential !== undefined) { + SidebarEventHub.$emit('confidentialityUpdated', confidential); + } + }, + }, }), }); } diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue index ebae1245810..fe7582a6cc3 100644 --- a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue @@ -7,6 +7,8 @@ import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { TYPE_WORK_ITEM } from '~/graphql_shared/constants'; import { isMetaKey } from '~/lib/utils/common_utils'; import { setUrlParams, updateHistory } from '~/lib/utils/url_utility'; +import SidebarEventHub from '~/sidebar/event_hub'; + import { STATE_OPEN, WIDGET_ICONS, @@ -111,7 +113,16 @@ export default { return this.isLoading && this.children.length === 0 ? '...' : this.children.length; }, }, + mounted() { + SidebarEventHub.$on('confidentialityUpdated', this.refetchWorkItems); + }, + destroyed() { + SidebarEventHub.$off('confidentialityUpdated', this.refetchWorkItems); + }, methods: { + refetchWorkItems() { + this.$apollo.queries.workItem.refetch(); + }, badgeVariant(state) { return state === STATE_OPEN ? 'success' : 'info'; }, @@ -122,6 +133,7 @@ export default { this.isOpen = !this.isOpen; }, showAddForm() { + this.isOpen = true; this.isShownAddForm = true; this.$nextTick(() => { this.$refs.wiLinksForm.$refs.wiTitleInput?.$el.focus(); diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index cc405cb8f5e..d88653f2f8c 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -93,6 +93,11 @@ module Types field :merge_status_enum, ::Types::MergeRequests::MergeStatusEnum, method: :public_merge_status, null: true, description: 'Merge status of the merge request.' + + field :detailed_merge_status, ::Types::MergeRequests::DetailedMergeStatusEnum, method: :detailed_merge_status, null: true, + calls_gitaly: true, + description: 'Detailed merge status of the merge request.', alpha: { milestone: '15.3' } + field :mergeable_discussions_state, GraphQL::Types::Boolean, null: true, calls_gitaly: true, description: 'Indicates if all discussions in the merge request have been resolved, allowing the merge request to be merged.' diff --git a/app/graphql/types/merge_requests/detailed_merge_status_enum.rb b/app/graphql/types/merge_requests/detailed_merge_status_enum.rb new file mode 100644 index 00000000000..58104159303 --- /dev/null +++ b/app/graphql/types/merge_requests/detailed_merge_status_enum.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Types + module MergeRequests + class DetailedMergeStatusEnum < BaseEnum + graphql_name 'DetailedMergeStatus' + description 'Detailed representation of whether a GitLab merge request can be merged.' + + value 'UNCHECKED', + value: :unchecked, + description: 'Merge status has not been checked.' + value 'CHECKING', + value: :checking, + description: 'Currently checking for mergeability.' + value 'MERGEABLE', + value: :mergeable, + description: 'Branch can be merged.' + value 'BROKEN_STATUS', + value: :broken_status, + description: 'Can not merge the source into the target branch, potential conflict.' + value 'CI_MUST_PASS', + value: :ci_must_pass, + description: 'Pipeline must succeed before merging.' + value 'DISCUSSIONS_NOT_RESOLVED', + value: :discussions_not_resolved, + description: 'Discussions must be resolved before merging.' + value 'DRAFT_STATUS', + value: :draft_status, + description: 'Merge request must not be draft before merging.' + value 'NOT_OPEN', + value: :not_open, + description: 'Merge request must be open before merging.' + value 'NOT_APPROVED', + value: :not_approved, + description: 'Merge request must be approved before merging.' + value 'BLOCKED_STATUS', + value: :merge_request_blocked, + description: 'Merge request is blocked by another merge request.' + value 'POLICIES_DENIED', + value: :policies_denied, + description: 'There are denied policies for the merge request.' + end + end +end diff --git a/app/helpers/ci/pipeline_editor_helper.rb b/app/helpers/ci/pipeline_editor_helper.rb index 48eb7b4c41a..d00301678dd 100644 --- a/app/helpers/ci/pipeline_editor_helper.rb +++ b/app/helpers/ci/pipeline_editor_helper.rb @@ -32,7 +32,6 @@ module Ci "project-path" => project.path, "project-full-path" => project.full_path, "project-namespace" => project.namespace.full_path, - "runner-help-page-path" => help_page_path('ci/runners/index'), "simulate-pipeline-help-page-path" => help_page_path('ci/pipeline_editor/index', anchor: 'simulate-a-cicd-pipeline'), "total-branches" => total_branches, "validate-tab-illustration-path" => image_path('illustrations/project-run-CICD-pipelines-sm.svg'), diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b2dd2264046..3c06e1aa983 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1187,17 +1187,30 @@ class MergeRequest < ApplicationRecord ] end + def detailed_merge_status + if cannot_be_merged_rechecking? || preparing? || checking? + return :checking + elsif unchecked? + return :unchecked + end + + checks = execute_merge_checks + + if checks.success? + :mergeable + else + checks.failure_reason + end + end + # rubocop: disable CodeReuse/ServiceClass def mergeable_state?(skip_ci_check: false, skip_discussions_check: false) if Feature.enabled?(:improved_mergeability_checks, self.project) - additional_checks = MergeRequests::Mergeability::RunChecksService.new( - merge_request: self, - params: { - skip_ci_check: skip_ci_check, - skip_discussions_check: skip_discussions_check - } - ) - additional_checks.execute.all?(&:success?) + additional_checks = execute_merge_checks(params: { + skip_ci_check: skip_ci_check, + skip_discussions_check: skip_discussions_check + }) + additional_checks.execute.success? else return false unless open? return false if draft? @@ -2059,6 +2072,12 @@ class MergeRequest < ApplicationRecord def report_type_enabled?(report_type) !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(report_type) end + + def execute_merge_checks(params: {}) + # rubocop: disable CodeReuse/ServiceClass + MergeRequests::Mergeability::RunChecksService.new(merge_request: self, params: params).execute + # rubocop: enable CodeReuse/ServiceClass + end end MergeRequest.prepend_mod_with('MergeRequest') diff --git a/app/services/merge_requests/mergeability/check_base_service.rb b/app/services/merge_requests/mergeability/check_base_service.rb index d5ddcb4b828..e614a7c27fe 100644 --- a/app/services/merge_requests/mergeability/check_base_service.rb +++ b/app/services/merge_requests/mergeability/check_base_service.rb @@ -24,12 +24,12 @@ module MergeRequests private - def success(*args) - Gitlab::MergeRequests::Mergeability::CheckResult.success(*args) + def success(**args) + Gitlab::MergeRequests::Mergeability::CheckResult.success(payload: args) end - def failure(*args) - Gitlab::MergeRequests::Mergeability::CheckResult.failed(*args) + def failure(**args) + Gitlab::MergeRequests::Mergeability::CheckResult.failed(payload: args) end end end diff --git a/app/services/merge_requests/mergeability/check_broken_status_service.rb b/app/services/merge_requests/mergeability/check_broken_status_service.rb index 9a54a4292c8..6fe4eb4a57f 100644 --- a/app/services/merge_requests/mergeability/check_broken_status_service.rb +++ b/app/services/merge_requests/mergeability/check_broken_status_service.rb @@ -4,7 +4,7 @@ module MergeRequests class CheckBrokenStatusService < CheckBaseService def execute if merge_request.broken? - failure + failure(reason: failure_reason) else success end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :broken_status + end end end end diff --git a/app/services/merge_requests/mergeability/check_ci_status_service.rb b/app/services/merge_requests/mergeability/check_ci_status_service.rb index c0ef5ba1c30..9e09b513c57 100644 --- a/app/services/merge_requests/mergeability/check_ci_status_service.rb +++ b/app/services/merge_requests/mergeability/check_ci_status_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.mergeable_ci_state? success else - failure + failure(reason: failure_reason) end end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :ci_must_pass + end end end end diff --git a/app/services/merge_requests/mergeability/check_discussions_status_service.rb b/app/services/merge_requests/mergeability/check_discussions_status_service.rb index 9b4eab9d399..3421d96e8ae 100644 --- a/app/services/merge_requests/mergeability/check_discussions_status_service.rb +++ b/app/services/merge_requests/mergeability/check_discussions_status_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.mergeable_discussions_state? success else - failure + failure(reason: failure_reason) end end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :discussions_not_resolved + end end end end diff --git a/app/services/merge_requests/mergeability/check_draft_status_service.rb b/app/services/merge_requests/mergeability/check_draft_status_service.rb index bc940e2116d..a1524317155 100644 --- a/app/services/merge_requests/mergeability/check_draft_status_service.rb +++ b/app/services/merge_requests/mergeability/check_draft_status_service.rb @@ -5,7 +5,7 @@ module MergeRequests class CheckDraftStatusService < CheckBaseService def execute if merge_request.draft? - failure + failure(reason: failure_reason) else success end @@ -18,6 +18,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :draft_status + end end end end diff --git a/app/services/merge_requests/mergeability/check_open_status_service.rb b/app/services/merge_requests/mergeability/check_open_status_service.rb index 361af946e3f..29f3d0d3ccb 100644 --- a/app/services/merge_requests/mergeability/check_open_status_service.rb +++ b/app/services/merge_requests/mergeability/check_open_status_service.rb @@ -7,7 +7,7 @@ module MergeRequests if merge_request.open? success else - failure + failure(reason: failure_reason) end end @@ -18,6 +18,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :not_open + end end end end diff --git a/app/services/merge_requests/mergeability/run_checks_service.rb b/app/services/merge_requests/mergeability/run_checks_service.rb index 1d4b96b3090..68f842b3322 100644 --- a/app/services/merge_requests/mergeability/run_checks_service.rb +++ b/app/services/merge_requests/mergeability/run_checks_service.rb @@ -10,36 +10,50 @@ module MergeRequests end def execute - merge_request.mergeability_checks.each_with_object([]) do |check_class, results| + @results = merge_request.mergeability_checks.each_with_object([]) do |check_class, result_hash| check = check_class.new(merge_request: merge_request, params: params) next if check.skip? check_result = run_check(check) - results << check_result + result_hash << check_result - break results if check_result.failed? + break result_hash if check_result.failed? end + + self + end + + def success? + raise 'Execute needs to be called before' if results.nil? + + results.all?(&:success?) + end + + def failure_reason + raise 'Execute needs to be called before' if results.nil? + + results.find(&:failed?)&.payload&.fetch(:reason) end private - attr_reader :merge_request, :params + attr_reader :merge_request, :params, :results def run_check(check) return check.execute unless Feature.enabled?(:mergeability_caching, merge_request.project) return check.execute unless check.cacheable? - cached_result = results.read(merge_check: check) + cached_result = cached_results.read(merge_check: check) return cached_result if cached_result.respond_to?(:status) check.execute.tap do |result| - results.write(merge_check: check, result_hash: result.to_hash) + cached_results.write(merge_check: check, result_hash: result.to_hash) end end - def results - strong_memoize(:results) do + def cached_results + strong_memoize(:cached_results) do Gitlab::MergeRequests::Mergeability::ResultsStore.new(merge_request: merge_request) end end diff --git a/app/views/admin/application_settings/_ci_cd.html.haml b/app/views/admin/application_settings/_ci_cd.html.haml index ba2a2f34d63..77170761448 100644 --- a/app/views/admin/application_settings/_ci_cd.html.haml +++ b/app/views/admin/application_settings/_ci_cd.html.haml @@ -22,7 +22,7 @@ .form-group = f.label :shared_runners_text, _('Shared runners details'), class: 'label-bold' = f.text_area :shared_runners_text, class: 'form-control gl-form-input', rows: 4 - .form-text.text-muted= _("Add a custom message with details about the instance's shared runners. The message is visible in group and project CI/CD settings, in the Runners section. Markdown is supported.") + .form-text.text-muted= _("Add a custom message with details about the instance's shared runners. The message is visible when you view runners for projects and groups. Markdown is supported.") .form-group = f.label :max_artifacts_size, _('Maximum artifacts size (MB)'), class: 'label-bold' = f.number_field :max_artifacts_size, class: 'form-control gl-form-input' diff --git a/app/views/groups/crm/contacts/index.html.haml b/app/views/groups/crm/contacts/index.html.haml index 8a971e451a4..27f18ac1c57 100644 --- a/app/views/groups/crm/contacts/index.html.haml +++ b/app/views/groups/crm/contacts/index.html.haml @@ -5,4 +5,4 @@ = content_for :after_content do #js-crm-form-portal -#js-crm-contacts-app{ data: { group_full_path: @group.full_path, group_issues_path: issues_group_path(@group), group_id: @group.id, can_admin_crm_contact: can?(current_user, :admin_crm_contact, @group).to_s, base_path: group_crm_contacts_path(@group) } } +#js-crm-contacts-app{ data: { group_full_path: @group.full_path, group_issues_path: issues_group_path(@group), group_id: @group.id, can_admin_crm_contact: can?(current_user, :admin_crm_contact, @group).to_s, base_path: group_crm_contacts_path(@group), text_query: params[:search] } } diff --git a/data/deprecations/15-3-deprecate-redis-5.yml b/data/deprecations/15-3-deprecate-redis-5.yml new file mode 100644 index 00000000000..f0e350cc62b --- /dev/null +++ b/data/deprecations/15-3-deprecate-redis-5.yml @@ -0,0 +1,16 @@ +- name: "Redis 5 deprecated" # (required) The name of the feature to be deprecated + announcement_milestone: "15.3" # (required) The milestone when this feature was first announced as deprecated. + announcement_date: "2022-08-22" # (required) The date of the milestone release when this feature was first announced as deprecated. This should almost always be the 22nd of a month (YYYY-MM-22), unless you did an out of band blog post. + removal_milestone: "16.0" # (required) The milestone when this feature is planned to be removed + removal_date: "2023-05-22" # (required) The date of the milestone release when this feature is planned to be removed. This should almost always be the 22nd of a month (YYYY-MM-22), unless you did an out of band blog post. + breaking_change: true # (required) If this deprecation is a breaking change, set this value to true + reporter: tnir + stage: Enablement + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/331468 + body: | # (required) Do not modify this line, instead modify the lines below. + With GitLab 13.9, in the Omnibus GitLab package and GitLab Helm chart 4.9, the Redis version [was updated to Redis 6](https://about.gitlab.com/releases/2021/02/22/gitlab-13-9-released/#omnibus-improvements). + Redis 5 has reached the end of life in April 2022 and will no longer be supported as of GitLab 15.6. + If you are using your own Redis 5.0 instance, you should upgrade it to Redis 6.0 or higher before upgrading to GitLab 16.0 or higher. + end_of_support_milestone: "15.6" + end_of_support_date: "2022-11-22" + documentation_url: https://docs.gitlab.com/ee/install/requirements.html diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 8f6c2f19446..11d179ffcfa 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -13528,6 +13528,7 @@ Maven metadata. | <a id="mergerequestdefaultsquashcommitmessage"></a>`defaultSquashCommitMessage` | [`String`](#string) | Default squash commit message of the merge request. | | <a id="mergerequestdescription"></a>`description` | [`String`](#string) | Description of the merge request (Markdown rendered as HTML for caching). | | <a id="mergerequestdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. | +| <a id="mergerequestdetailedmergestatus"></a>`detailedMergeStatus` **{warning-solid}** | [`DetailedMergeStatus`](#detailedmergestatus) | **Introduced** in 15.3. This feature is in Alpha. It can be changed or removed at any time. Detailed merge status of the merge request. | | <a id="mergerequestdiffheadsha"></a>`diffHeadSha` | [`String`](#string) | Diff head SHA of the merge request. | | <a id="mergerequestdiffrefs"></a>`diffRefs` | [`DiffRefs`](#diffrefs) | References of the base SHA, the head SHA, and the start SHA for this merge request. | | <a id="mergerequestdiffstatssummary"></a>`diffStatsSummary` | [`DiffStatsSummary`](#diffstatssummary) | Summary of which files were changed in this merge request. | @@ -19580,6 +19581,24 @@ Mutation event of a design within a version. | <a id="designversioneventmodification"></a>`MODIFICATION` | A modification event. | | <a id="designversioneventnone"></a>`NONE` | No change. | +### `DetailedMergeStatus` + +Detailed representation of whether a GitLab merge request can be merged. + +| Value | Description | +| ----- | ----------- | +| <a id="detailedmergestatusblocked_status"></a>`BLOCKED_STATUS` | Merge request is blocked by another merge request. | +| <a id="detailedmergestatusbroken_status"></a>`BROKEN_STATUS` | Can not merge the source into the target branch, potential conflict. | +| <a id="detailedmergestatuschecking"></a>`CHECKING` | Currently checking for mergeability. | +| <a id="detailedmergestatusci_must_pass"></a>`CI_MUST_PASS` | Pipeline must succeed before merging. | +| <a id="detailedmergestatusdiscussions_not_resolved"></a>`DISCUSSIONS_NOT_RESOLVED` | Discussions must be resolved before merging. | +| <a id="detailedmergestatusdraft_status"></a>`DRAFT_STATUS` | Merge request must not be draft before merging. | +| <a id="detailedmergestatusmergeable"></a>`MERGEABLE` | Branch can be merged. | +| <a id="detailedmergestatusnot_approved"></a>`NOT_APPROVED` | Merge request must be approved before merging. | +| <a id="detailedmergestatusnot_open"></a>`NOT_OPEN` | Merge request must be open before merging. | +| <a id="detailedmergestatuspolicies_denied"></a>`POLICIES_DENIED` | There are denied policies for the merge request. | +| <a id="detailedmergestatusunchecked"></a>`UNCHECKED` | Merge status has not been checked. | + ### `DiffPositionType` Type of file the position refers to. diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 2536eb4899e..0673a072234 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -447,7 +447,7 @@ You can also use `workflow::ready for review` label. That means that your merge When your merge request receives an approval from the first reviewer it can be passed to a maintainer. You should default to choosing a maintainer with [domain expertise](#domain-experts), and otherwise follow the Reviewer Roulette recommendation or use the label `ready for merge`. -Sometimes, a maintainer may not be available for review. They could be out of the office or [at capacity](#review-response-slo). +Sometimes, a maintainer may not be available for review. They could be out of the office or [at capacity](https://about.gitlab.com/handbook/engineering/workflow/code-review/#review-response-slo). You can and should check the maintainer's availability in their profile. If the maintainer recommended by the roulette is not available, choose someone else from that list. @@ -679,42 +679,6 @@ Enterprise Edition instance. This has some implications: Ensure that we support object storage for any file storage we need to perform. For more information, see the [uploads documentation](uploads/index.md). -### Review turnaround time - -Because [unblocking others is always a top priority](https://about.gitlab.com/handbook/values/#global-optimization), -reviewers are expected to review merge requests in a timely manner, -even when this may negatively impact their other tasks and priorities. - -Doing so allows everyone involved in the merge request to iterate faster as the -context is fresh in memory, and improves contributors' experience significantly. - -#### Review-response SLO - -To ensure swift feedback to ready-to-review code, we maintain a `Review-response` Service-level Objective (SLO). The SLO is defined as: - -> Review-response SLO = (time when first review is provided) - (time MR is assigned to reviewer) < 2 business days - -If you don't think you can review a merge request in the `Review-response` SLO -time frame, let the author know as soon as possible in the comments -(no later than 36 hours after first receiving the review request) -and try to help them find another reviewer or maintainer who is able to, so that they can be unblocked -and get on with their work quickly. Remove yourself as a reviewer. - -If you think you are at capacity and are unable to accept any more reviews until -some have been completed, communicate this through your GitLab status by setting -the 🔴 `:red_circle:` emoji and mentioning that you are at capacity in the status -text. This guides contributors to pick a different reviewer, helping us to -meet the SLO. - -Of course, if you are out of office and have -[communicated](https://about.gitlab.com/handbook/paid-time-off/#communicating-your-time-off) -this through your GitLab.com Status, authors are expected to realize this and -find a different reviewer themselves. - -When a merge request author has been blocked for longer than -the `Review-response` SLO, they are free to remind the reviewer through Slack or add -another reviewer. - ### Customer critical merge requests A merge request may benefit from being considered a customer critical priority because there is a significant benefit to the business in doing so. diff --git a/doc/development/database/batched_background_migrations.md b/doc/development/database/batched_background_migrations.md index f267930f3da..edb22fcf436 100644 --- a/doc/development/database/batched_background_migrations.md +++ b/doc/development/database/batched_background_migrations.md @@ -233,6 +233,77 @@ class CopyColumnUsingBackgroundMigrationJob < BatchedMigrationJob end ``` +### Additional filters + +By default, when creating background jobs to perform the migration, batched background migrations +iterate over the full specified table. This iteration is done using the +[`PrimaryKeyBatchingStrategy`](https://gitlab.com/gitlab-org/gitlab/-/blob/c9dabd1f4b8058eece6d8cb4af95e9560da9a2ee/lib/gitlab/database/migrations/batched_background_migration_helpers.rb#L17). If the table has 1000 records +and the batch size is 100, the work is batched into 10 jobs. For illustrative purposes, +`EachBatch` is used like this: + +```ruby +# PrimaryKeyBatchingStrategy +Namespace.each_batch(of: 100) do |relation| + relation.where(type: nil).update_all(type: 'User') # this happens in each background job +end +``` + +In some cases, only a subset of records must be examined. If only 10% of the 1000 records +need examination, apply a filter to the initial relation when the jobs are created: + +```ruby +Namespace.where(type: nil).each_batch(of: 100) do |relation| + relation.update_all(type: 'User') +end +``` + +In the first example, we don't know how many records will be updated in each batch. +In the second (filtered) example, we know exactly 100 will be updated with each batch. + +`BatchedMigrationJob` provides a `scope_to` helper method to apply additional filters and achieve this: + +1. Create a new migration job class that inherits from `BatchedMigrationJob` and defines the additional filter: + + ```ruby + class BackfillNamespaceType < BatchedMigrationJob + scope_to ->(relation) { relation.where(type: nil) } + + def perform + each_sub_batch(operation_name: :update_all) do |sub_batch| + sub_batch.update_all(type: 'User') + end + end + end + ``` + +1. In the post-deployment migration, enqueue the batched background migration: + + ```ruby + class BackfillNamespaceType < Gitlab::Database::Migration[2.0] + MIGRATION = 'BackfillNamespaceType' + DELAY_INTERVAL = 2.minutes + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + queue_batched_background_migration( + MIGRATION, + :namespaces, + :id, + job_interval: DELAY_INTERVAL + ) + end + + def down + delete_batched_background_migration(MIGRATION, :namespaces, :id, []) + end + end + ``` + +NOTE: +When applying additional filters, it is important to ensure they are properly covered by an index to optimize `EachBatch` performance. +In the example above we need an index on `(type, id)` to support the filters. See [the `EachBatch` docs for more information](../iterating_tables_in_batches.md). + ## Example The `routes` table has a `source_type` field that's used for a polymorphic relationship. @@ -419,76 +490,8 @@ module Gitlab end ``` -### Adding filters to the initial batching - -By default, when creating background jobs to perform the migration, batched background migrations will iterate over the full specified table. This is done using the [`PrimaryKeyBatchingStrategy`](https://gitlab.com/gitlab-org/gitlab/-/blob/c9dabd1f4b8058eece6d8cb4af95e9560da9a2ee/lib/gitlab/database/migrations/batched_background_migration_helpers.rb#L17). This means if there are 1000 records in the table and the batch size is 100, there will be 10 jobs. For illustrative purposes, `EachBatch` is used like this: - -```ruby -# PrimaryKeyBatchingStrategy -Projects.all.each_batch(of: 100) do |relation| - relation.where(foo: nil).update_all(foo: 'bar') # this happens in each background job -end -``` - -There are cases where we only need to look at a subset of records. Perhaps we only need to update 1 out of every 10 of those 1000 records. It would be best if we could apply a filter to the initial relation when the jobs are created: - -```ruby -Projects.where(foo: nil).each_batch(of: 100) do |relation| - relation.update_all(foo: 'bar') -end -``` - -In the `PrimaryKeyBatchingStrategy` example, we do not know how many records will be updated in each batch. In the filtered example, we know exactly 100 will be updated with each batch. - -The `PrimaryKeyBatchingStrategy` contains [a method that can be overwritten](https://gitlab.com/gitlab-org/gitlab/-/blob/dd1e70d3676891025534dc4a1e89ca9383178fe7/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy.rb#L38-52) to apply additional filtering on the initial `EachBatch`. - -We can accomplish this by: - -1. Create a new class that inherits from `PrimaryKeyBatchingStrategy` and overrides the method using the desired filter (this may be the same filter used in the sub-batch): - - ```ruby - # frozen_string_literal: true - - module GitLab - module BackgroundMigration - module BatchingStrategies - class FooStrategy < PrimaryKeyBatchingStrategy - def apply_additional_filters(relation, job_arguments: [], job_class: nil) - relation.where(foo: nil) - end - end - end - end - end - ``` - -1. In the post-deployment migration that queues the batched background migration, specify the new batching strategy using the `batch_class_name` parameter: - - ```ruby - class BackfillProjectsFoo < Gitlab::Database::Migration[2.0] - MIGRATION = 'BackfillProjectsFoo' - DELAY_INTERVAL = 2.minutes - BATCH_CLASS_NAME = 'FooStrategy' - - restrict_gitlab_migration gitlab_schema: :gitlab_main - - def up - queue_batched_background_migration( - MIGRATION, - :routes, - :id, - job_interval: DELAY_INTERVAL, - batch_class_name: BATCH_CLASS_NAME - ) - end - - def down - delete_batched_background_migration(MIGRATION, :routes, :id, []) - end - end - ``` - -When applying a batching strategy, it is important to ensure the filter properly covered by an index to optimize `EachBatch` performance. See [the `EachBatch` docs for more information](iterating_tables_in_batches.md). +NOTE: +[Additional filters](#additional-filters) defined with `scope_to` will be ignored by `LooseIndexScanBatchingStrategy` and `distinct_each_batch`. ## Testing diff --git a/doc/development/database/database_reviewer_guidelines.md b/doc/development/database/database_reviewer_guidelines.md index 1bdbf69d4fe..a85f399a447 100644 --- a/doc/development/database/database_reviewer_guidelines.md +++ b/doc/development/database/database_reviewer_guidelines.md @@ -95,7 +95,7 @@ are three times as likely to be picked by the [Danger bot](../dangerbot.md) as o ## What to do if you feel overwhelmed Similar to all types of reviews, [unblocking others is always a top priority](https://about.gitlab.com/handbook/values/#global-optimization). -Database reviewers are expected to [review assigned merge requests in a timely manner](../code_review.md#review-turnaround-time) +Database reviewers are expected to [review assigned merge requests in a timely manner](https://about.gitlab.com/handbook/engineering/workflow/code-review/#review-turnaround-time) or let the author know as soon as possible and help them find another reviewer or maintainer. We are doing reviews to help the rest of the GitLab team and, at the same time, get exposed diff --git a/doc/development/service_ping/performance_indicator_metrics.md b/doc/development/service_ping/performance_indicator_metrics.md index bdd4c319d41..d2abc597a22 100644 --- a/doc/development/service_ping/performance_indicator_metrics.md +++ b/doc/development/service_ping/performance_indicator_metrics.md @@ -10,8 +10,7 @@ This guide describes how to use metrics definitions to define [performance indic To use a metric definition to manage a performance indicator: -1. Create a new issue and use the [Performance Indicator Metric issue template](https://gitlab.com/gitlab-org/gitlab/-/issues/new?issuable_template=Performance%20Indicator%20Metric). +1. Create a merge request that includes related changes. 1. Use labels `~"product intelligence"`, `"~Data Warehouse::Impact Check"`. -1. Create a merge request that includes changes related only to the metric performance indicator. 1. Update the metric definition `performance_indicator_type` [field](metrics_dictionary.md#metrics-definition-and-validation). -1. Create an issue in GitLab Data Team project with the [Product Performance Indicator template](https://gitlab.com/gitlab-data/analytics/-/issues/new?issuable_template=Product%20Performance%20Indicator%20Template). +1. Create an issue in GitLab Product Data Insights project with the [PI Chart Help template](https://gitlab.com/gitlab-data/product-analytics/-/issues/new?issuable_template=PI%20Chart%20Help) to have the new metric visualized. diff --git a/doc/development/work_items.md b/doc/development/work_items.md index 9a17a152525..3625f85eb82 100644 --- a/doc/development/work_items.md +++ b/doc/development/work_items.md @@ -36,14 +36,12 @@ Here are some problems with current issues usage and why we are looking into wor differences in common interactions that the user needs to hold a complicated mental model of how they each behave. - Issues are not extensible enough to support all of the emerging jobs they need to facilitate. -- Codebase maintainability and feature development become bigger challenges as we grow the Issue type +- Codebase maintainability and feature development becomes a bigger challenge as we grow the Issue type. beyond its core role of issue tracking into supporting the different work item types and handling logic and structure differences. - New functionality is typically implemented with first class objects that import behavior from issues via shared concerns. This leads to duplicated effort and ultimately small differences between common interactions. This leads to inconsistent UX. -- Codebase maintainability and feature development becomes a bigger challenges as we grow issues - beyond its core role of issue tracking into supporting the different types and subtle differences between them. ## Work item terminology diff --git a/doc/tutorials/make_your_first_git_commit.md b/doc/tutorials/make_your_first_git_commit.md index a35137e158b..be9023c6ae0 100644 --- a/doc/tutorials/make_your_first_git_commit.md +++ b/doc/tutorials/make_your_first_git_commit.md @@ -37,7 +37,7 @@ Each time you push a change, Git records it as a unique *commit*. These commits the history of when and how a file changed, and who changed it. ```mermaid -graph LR +graph TB subgraph Repository commit history A(Author: Alex<br>Date: 3 Jan at 1PM<br>Commit message: Added sales figures for January<br> Commit ID: 123abc12) ---> B B(Author: Sam<br>Date: 4 Jan at 10AM<br>Commit message: Removed outdated marketing information<br> Commit ID: aabb1122) ---> C @@ -54,7 +54,7 @@ of a repository are in a default branch. To make changes, you: 1. When you're ready, *merge* your branch into the default branch. ```mermaid -flowchart LR +flowchart TB subgraph Default branch A[Commit] --> B[Commit] --> C[Commit] --> D[Commit] end diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index 40209e4f5e8..dcadddd28bb 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -92,6 +92,23 @@ The [**Maximum number of active pipelines per project** limit](https://docs.gitl - [**Total number of jobs in currently active pipelines**](https://docs.gitlab.com/ee/user/admin_area/settings/continuous_integration.html#set-cicd-limits). </div> + +<div class="deprecation removal-160 breaking-change"> + +### Redis 5 deprecated + +End of Support: GitLab <span class="removal-milestone">15.6</span> (2022-11-22) +Planned removal: GitLab <span class="removal-milestone">16.0</span> (2023-05-22) + +WARNING: +This is a [breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/). +Review the details carefully before upgrading. + +With GitLab 13.9, in the Omnibus GitLab package and GitLab Helm chart 4.9, the Redis version [was updated to Redis 6](https://about.gitlab.com/releases/2021/02/22/gitlab-13-9-released/#omnibus-improvements). +Redis 5 has reached the end of life in April 2022 and will no longer be supported as of GitLab 15.6. +If you are using your own Redis 5.0 instance, you should upgrade it to Redis 6.0 or higher before upgrading to GitLab 16.0 or higher. + +</div> </div> <div class="announcement-milestone"> diff --git a/doc/user/application_security/api_fuzzing/index.md b/doc/user/application_security/api_fuzzing/index.md index 2c011d3c940..80e4700c34c 100644 --- a/doc/user/application_security/api_fuzzing/index.md +++ b/doc/user/application_security/api_fuzzing/index.md @@ -1844,7 +1844,7 @@ API Security uses the specified media types in the OpenAPI document to generate To get support for your particular problem please use the [getting help channels](https://about.gitlab.com/get-help/). The [GitLab issue tracker on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues) is the right place for bugs and feature proposals about API Security and API Fuzzing. -Please use `~"Category:API Security"` [label](../../../development/contributing/issue_workflow.md#labels) when opening a new issue regarding API fuzzing to ensure it is quickly reviewed by the right people. Please refer to our [review response SLO](../../../development/code_review.md#review-response-slo) to understand when you should receive a response. +Please use `~"Category:API Security"` [label](../../../development/contributing/issue_workflow.md#labels) when opening a new issue regarding API fuzzing to ensure it is quickly reviewed by the right people. Please refer to our [review response SLO](https://about.gitlab.com/handbook/engineering/workflow/code-review/#review-response-slo) to understand when you should receive a response. [Search the issue tracker](https://gitlab.com/gitlab-org/gitlab/-/issues) for similar entries before submitting your own, there's a good chance somebody else had the same issue or feature proposal. Show your support with an award emoji and or join the discussion. diff --git a/doc/user/application_security/dast_api/index.md b/doc/user/application_security/dast_api/index.md index 319b5bd26b2..1f86f2ffa49 100644 --- a/doc/user/application_security/dast_api/index.md +++ b/doc/user/application_security/dast_api/index.md @@ -1654,7 +1654,7 @@ API Security uses the specified media types in the OpenAPI document to generate To get support for your particular problem please use the [getting help channels](https://about.gitlab.com/get-help/). The [GitLab issue tracker on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues) is the right place for bugs and feature proposals about API Security and DAST API. -Please use `~"Category:API Security"` [label](../../../development/contributing/issue_workflow.md#labels) when opening a new issue regarding DAST API to ensure it is quickly reviewed by the right people. Please refer to our [review response SLO](../../../development/code_review.md#review-response-slo) to understand when you should receive a response. +Please use `~"Category:API Security"` [label](../../../development/contributing/issue_workflow.md#labels) when opening a new issue regarding DAST API to ensure it is quickly reviewed by the right people. Please refer to our [review response SLO](https://about.gitlab.com/handbook/engineering/workflow/code-review/#review-response-slo) to understand when you should receive a response. [Search the issue tracker](https://gitlab.com/gitlab-org/gitlab/-/issues) for similar entries before submitting your own, there's a good chance somebody else had the same issue or feature proposal. Show your support with an award emoji and or join the discussion. diff --git a/doc/user/group/subgroups/index.md b/doc/user/group/subgroups/index.md index 0d84606eb11..12434de5efc 100644 --- a/doc/user/group/subgroups/index.md +++ b/doc/user/group/subgroups/index.md @@ -47,6 +47,28 @@ graph TD end ``` +## View subgroups of a group + +Prerequisite: + +- To view private nested subgroups, you must be a direct or inherited member of +the private subgroup. + +To view the subgroups of a group: + +1. On the top bar, select **Menu > Groups** and find your group. +1. Select the **Subgroups and projects** tab. +1. To view a nested subgroup, expand a subgroup in the hierarchy list. + +### Private subgroups in public parent groups + +In the hierarchy list, public groups with a private subgroup have an expand option (**{chevron-down}**) +for all users that indicate there is a subgroup. When users who are not direct or inherited members of +the private subgroup select expand (**{chevron-down}**), the nested subgroup does not display. + +If you prefer to keep information about the presence of nested subgroups private, we advise that you only +add private subgroups to private parent groups. + ## Create a subgroup Prerequisites: diff --git a/doc/user/packages/container_registry/reduce_container_registry_storage.md b/doc/user/packages/container_registry/reduce_container_registry_storage.md index 3e41a260854..788e57b6410 100644 --- a/doc/user/packages/container_registry/reduce_container_registry_storage.md +++ b/doc/user/packages/container_registry/reduce_container_registry_storage.md @@ -16,9 +16,26 @@ to automatically manage your container registry usage. ## Check Container Registry Storage Use -The Usage Quotas page (**Settings > Usage Quotas > Storage**) displays storage usage for Packages, -which doesn't include the Container Registry. To track work on this, see the epic -[Storage management for the Container Registry](https://gitlab.com/groups/gitlab-org/-/epics/7226). +The Usage Quotas page (**Settings > Usage Quotas > Storage**) displays storage usage for Packages. +This page includes the Container Registry usage but is currently only available on GitLab.com. +Measuring usage is only possible on the new version of the GitLab Container Registry backed by a +metadata database. We are completing the [upgrade and migration of the GitLab.com Container Registry](https://gitlab.com/groups/gitlab-org/-/epics/5523) +first and only then will work on [making this available to self-managed installs](https://gitlab.com/groups/gitlab-org/-/epics/5521). + +Image layers stored in the Container Registry are deduplicated at the root namespace level. +Therefore, if you tag the same 500MB image twice (either in the same repository or across distinct +repositories under the same root namespace), it will only count towards the root namespace usage +once. Similarly, if a given image layer is shared across multiple images, be those under the same +container repository, project, group, or across different ones, that layer will count only once +towards the root namespace usage. + +Only layers that are referenced by tagged images are accounted for. Untagged images and any layers +referenced exclusively by them are subject to [online garbage collection](index.md#delete-images) +and automatically deleted after 24 hours if they remain unreferenced during that period. + +Image layers are stored on the storage backend in the original (usually compressed) format. This +means that the measured size for any given image layer should match the size displayed on the +corresponding [image manifest](https://github.com/opencontainers/image-spec/blob/main/manifest.md#example-image-manifest). ## Cleanup policy diff --git a/doc/user/project/insights/index.md b/doc/user/project/insights/index.md index ff95945a64a..53547e69e00 100644 --- a/doc/user/project/insights/index.md +++ b/doc/user/project/insights/index.md @@ -157,6 +157,9 @@ Supported values are: | `line` |  | | `stacked-bar` |  | +NOTE: +The `dora` data source only supports the `bar` chart type. + ### `query` `query` allows to define the data source and various filtering conditions for the chart. @@ -349,7 +352,7 @@ dora: title: "DORA charts" charts: - title: "DORA deployment frequency" - type: bar + type: bar # only bar chart is supported at the moment query: data_source: dora params: @@ -372,7 +375,7 @@ dora: period_limit: 30 ``` -#### `query.metric` +##### `query.metric` Defines which DORA metric to query. The available values are: @@ -398,7 +401,7 @@ Define how far the metrics are queried in the past (default: 15). Maximum lookba ##### `query.environment_tiers` -An array of environments to include into the calculation (default: production). +An array of environments to include into the calculation (default: production). Available options: `production`, `staging`, `testing`, `development`, `other`. ### `projects` diff --git a/doc/user/project/merge_requests/methods/index.md b/doc/user/project/merge_requests/methods/index.md index c9c76706e77..c4e4b40dc48 100644 --- a/doc/user/project/merge_requests/methods/index.md +++ b/doc/user/project/merge_requests/methods/index.md @@ -150,7 +150,8 @@ considered equivalent to rebasing. ### Rebase without CI/CD pipeline -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/118825) in GitLab 14.7. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/118825) in GitLab 14.7 [with a flag](../../../../administration/feature_flags.md) named `rebase_without_ci_ui`. Disabled by default. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/350262) in GitLab 15.3. Feature flag `rebase_without_ci_ui` removed. To rebase a merge request's branch without triggering a CI/CD pipeline, select **Rebase without pipeline** from the merge request reports section. diff --git a/lib/gitlab/background_migration/batched_migration_job.rb b/lib/gitlab/background_migration/batched_migration_job.rb index dedcdf5d745..11d15804344 100644 --- a/lib/gitlab/background_migration/batched_migration_job.rb +++ b/lib/gitlab/background_migration/batched_migration_job.rb @@ -24,6 +24,14 @@ module Gitlab @connection = connection end + def self.generic_instance(batch_table:, batch_column:, job_arguments: [], connection:) + new( + batch_table: batch_table, batch_column: batch_column, + job_arguments: job_arguments, connection: connection, + start_id: 0, end_id: 0, sub_batch_size: 0, pause_ms: 0 + ) + end + def self.job_arguments_count 0 end @@ -40,6 +48,16 @@ module Gitlab end end + def self.scope_to(scope) + define_method(:filter_batch) do |relation| + instance_exec(relation, &scope) + end + end + + def filter_batch(relation) + relation + end + def perform raise NotImplementedError, "subclasses of #{self.class.name} must implement #{__method__}" end @@ -55,9 +73,10 @@ module Gitlab def each_sub_batch(operation_name: :default, batching_arguments: {}, batching_scope: nil) all_batching_arguments = { column: batch_column, of: sub_batch_size }.merge(batching_arguments) - parent_relation = parent_batch_relation(batching_scope) + relation = filter_batch(base_relation) + sub_batch_relation = filter_sub_batch(relation, batching_scope) - parent_relation.each_batch(**all_batching_arguments) do |relation| + sub_batch_relation.each_batch(**all_batching_arguments) do |relation| batch_metrics.instrument_operation(operation_name) do yield relation end @@ -67,9 +86,13 @@ module Gitlab end def distinct_each_batch(operation_name: :default, batching_arguments: {}) + if base_relation != filter_batch(base_relation) + raise 'distinct_each_batch can not be used when additional filters are defined with scope_to' + end + all_batching_arguments = { column: batch_column, of: sub_batch_size }.merge(batching_arguments) - parent_batch_relation.distinct_each_batch(**all_batching_arguments) do |relation| + base_relation.distinct_each_batch(**all_batching_arguments) do |relation| batch_metrics.instrument_operation(operation_name) do yield relation end @@ -78,13 +101,15 @@ module Gitlab end end - def parent_batch_relation(batching_scope = nil) - parent_relation = define_batchable_model(batch_table, connection: connection) + def base_relation + define_batchable_model(batch_table, connection: connection) .where(batch_column => start_id..end_id) + end - return parent_relation unless batching_scope + def filter_sub_batch(relation, batching_scope = nil) + return relation unless batching_scope - batching_scope.call(parent_relation) + batching_scope.call(relation) end end end diff --git a/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy.rb b/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy.rb index 751e483ad24..1ffa4a052e5 100644 --- a/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy.rb +++ b/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy.rb @@ -24,6 +24,14 @@ module Gitlab quoted_column_name = model_class.connection.quote_column_name(column_name) relation = model_class.where("#{quoted_column_name} >= ?", batch_min_value) + + if job_class + relation = filter_batch(relation, + table_name: table_name, column_name: column_name, + job_class: job_class, job_arguments: job_arguments + ) + end + relation = apply_additional_filters(relation, job_arguments: job_arguments, job_class: job_class) next_batch_bounds = nil @@ -36,13 +44,27 @@ module Gitlab next_batch_bounds end + # Deprecated + # + # Use `scope_to` to define additional filters on the migration job class. + # + # see https://docs.gitlab.com/ee/development/database/batched_background_migrations.html#adding-additional-filters. def apply_additional_filters(relation, job_arguments: [], job_class: nil) - if job_class.respond_to?(:batching_scope) - return job_class.batching_scope(relation, job_arguments: job_arguments) - end - relation end + + private + + def filter_batch(relation, table_name:, column_name:, job_class:, job_arguments: []) + return relation unless job_class.respond_to?(:generic_instance) + + job = job_class.generic_instance( + batch_table: table_name, batch_column: column_name, + job_arguments: job_arguments, connection: connection + ) + + job.filter_batch(relation) + end end end end diff --git a/lib/gitlab/merge_requests/mergeability/check_result.rb b/lib/gitlab/merge_requests/mergeability/check_result.rb index 5284d20d423..a25156661af 100644 --- a/lib/gitlab/merge_requests/mergeability/check_result.rb +++ b/lib/gitlab/merge_requests/mergeability/check_result.rb @@ -13,11 +13,11 @@ module Gitlab end def self.success(payload: {}) - new(status: SUCCESS_STATUS, payload: default_payload.merge(payload)) + new(status: SUCCESS_STATUS, payload: default_payload.merge(**payload)) end def self.failed(payload: {}) - new(status: FAILED_STATUS, payload: default_payload.merge(payload)) + new(status: FAILED_STATUS, payload: default_payload.merge(**payload)) end def self.from_hash(data) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ed3e2e25d96..261dca88d54 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2145,7 +2145,7 @@ msgstr "" msgid "Add a confidential internal note to this %{noteableDisplayName}." msgstr "" -msgid "Add a custom message with details about the instance's shared runners. The message is visible in group and project CI/CD settings, in the Runners section. Markdown is supported." +msgid "Add a custom message with details about the instance's shared runners. The message is visible when you view runners for projects and groups. Markdown is supported." msgstr "" msgid "Add a general comment to this %{noteableDisplayName}." @@ -16799,8 +16799,8 @@ msgstr "" msgid "From %{providerTitle}" msgstr "" -msgid "From October 19, 2022, free groups will be limited to %d member" -msgid_plural "From October 19, 2022, free groups will be limited to %d members" +msgid "From October 19, 2022, free private groups will be limited to %d member" +msgid_plural "From October 19, 2022, free private groups will be limited to %d members" msgstr[0] "" msgstr[1] "" diff --git a/spec/frontend/crm/contacts_root_spec.js b/spec/frontend/crm/contacts_root_spec.js index 3a6989a00f1..7aaaf480c44 100644 --- a/spec/frontend/crm/contacts_root_spec.js +++ b/spec/frontend/crm/contacts_root_spec.js @@ -1,14 +1,16 @@ -import { GlAlert, GlLoadingIcon } from '@gitlab/ui'; +import { GlLoadingIcon } from '@gitlab/ui'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import VueRouter from 'vue-router'; -import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import ContactsRoot from '~/crm/contacts/components/contacts_root.vue'; import getGroupContactsQuery from '~/crm/contacts/components/graphql/get_group_contacts.query.graphql'; +import getGroupContactsCountByStateQuery from '~/crm/contacts/components/graphql/get_group_contacts_count_by_state.graphql'; import routes from '~/crm/contacts/routes'; -import { getGroupContactsQueryResponse } from './mock_data'; +import PaginatedTableWithSearchAndTabs from '~/vue_shared/components/paginated_table_with_search_and_tabs/paginated_table_with_search_and_tabs.vue'; +import { getGroupContactsQueryResponse, getGroupContactsCountQueryResponse } from './mock_data'; describe('Customer relations contacts root app', () => { Vue.use(VueApollo); @@ -21,24 +23,30 @@ describe('Customer relations contacts root app', () => { const findRowByName = (rowName) => wrapper.findAllByRole('row', { name: rowName }); const findIssuesLinks = () => wrapper.findAllByTestId('issues-link'); const findNewContactButton = () => wrapper.findByTestId('new-contact-button'); - const findError = () => wrapper.findComponent(GlAlert); + const findTable = () => wrapper.findComponent(PaginatedTableWithSearchAndTabs); const successQueryHandler = jest.fn().mockResolvedValue(getGroupContactsQueryResponse); + const successCountQueryHandler = jest.fn().mockResolvedValue(getGroupContactsCountQueryResponse); const basePath = '/groups/flightjs/-/crm/contacts'; const mountComponent = ({ queryHandler = successQueryHandler, - mountFunction = shallowMountExtended, + countQueryHandler = successCountQueryHandler, canAdminCrmContact = true, + textQuery = null, } = {}) => { - fakeApollo = createMockApollo([[getGroupContactsQuery, queryHandler]]); - wrapper = mountFunction(ContactsRoot, { + fakeApollo = createMockApollo([ + [getGroupContactsQuery, queryHandler], + [getGroupContactsCountByStateQuery, countQueryHandler], + ]); + wrapper = mountExtended(ContactsRoot, { router, provide: { groupFullPath: 'flightjs', groupId: 26, groupIssuesPath: '/issues', canAdminCrmContact, + textQuery, }, apolloProvider: fakeApollo, }); @@ -58,9 +66,33 @@ describe('Customer relations contacts root app', () => { router = null; }); - it('should render loading spinner', () => { + it('should render table with default props and loading state', () => { mountComponent(); + expect(findTable().props()).toMatchObject({ + items: [], + itemsCount: {}, + pageInfo: {}, + statusTabs: [ + { title: 'Active', status: 'ACTIVE', filters: 'active' }, + { title: 'Inactive', status: 'INACTIVE', filters: 'inactive' }, + { title: 'All', status: 'ALL', filters: 'all' }, + ], + showItems: true, + showErrorMsg: false, + trackViewsOptions: { category: 'Customer Relations', action: 'view_contacts_list' }, + i18n: { + emptyText: 'No contacts found', + issuesButtonLabel: 'View issues', + editButtonLabel: 'Edit', + title: 'Customer relations contacts', + newContact: 'New contact', + errorText: 'Something went wrong. Please try again.', + }, + serverErrorMessage: '', + filterSearchKey: 'contacts', + filterSearchTokens: [], + }); expect(findLoadingIcon().exists()).toBe(true); }); @@ -83,7 +115,7 @@ describe('Customer relations contacts root app', () => { mountComponent({ queryHandler: jest.fn().mockRejectedValue('ERROR') }); await waitForPromises(); - expect(findError().exists()).toBe(true); + expect(wrapper.text()).toContain('Something went wrong. Please try again.'); }); }); @@ -92,11 +124,11 @@ describe('Customer relations contacts root app', () => { mountComponent(); await waitForPromises(); - expect(findError().exists()).toBe(false); + expect(wrapper.text()).not.toContain('Something went wrong. Please try again.'); }); it('renders correct results', async () => { - mountComponent({ mountFunction: mountExtended }); + mountComponent(); await waitForPromises(); expect(findRowByName(/Marty/i)).toHaveLength(1); @@ -105,7 +137,7 @@ describe('Customer relations contacts root app', () => { const issueLink = findIssuesLinks().at(0); expect(issueLink.exists()).toBe(true); - expect(issueLink.attributes('href')).toBe('/issues?crm_contact_id=16'); + expect(issueLink.attributes('href')).toBe('/issues?crm_contact_id=12'); }); }); }); diff --git a/spec/frontend/crm/mock_data.js b/spec/frontend/crm/mock_data.js index 3f3f076bf39..a2e2e88ac60 100644 --- a/spec/frontend/crm/mock_data.js +++ b/spec/frontend/crm/mock_data.js @@ -43,6 +43,28 @@ export const getGroupContactsQueryResponse = { organization: null, }, ], + pageInfo: { + __typename: 'PageInfo', + hasNextPage: false, + endCursor: 'eyJsYXN0X25hbWUiOiJMZWRuZXIiLCJpZCI6IjE3OSJ9', + hasPreviousPage: false, + startCursor: 'eyJsYXN0X25hbWUiOiJCYXJ0b24iLCJpZCI6IjE5MyJ9', + }, + }, + }, + }, +}; + +export const getGroupContactsCountQueryResponse = { + data: { + group: { + __typename: 'Group', + id: 'gid://gitlab/Group/26', + contactStateCounts: { + all: 241, + active: 239, + inactive: 2, + __typename: 'ContactStateCountsType', }, }, }, diff --git a/spec/frontend/jobs/components/sidebar_detail_row_spec.js b/spec/frontend/jobs/components/sidebar_detail_row_spec.js index 43f2e022dd8..8d2680608ab 100644 --- a/spec/frontend/jobs/components/sidebar_detail_row_spec.js +++ b/spec/frontend/jobs/components/sidebar_detail_row_spec.js @@ -7,7 +7,7 @@ describe('Sidebar detail row', () => { const title = 'this is the title'; const value = 'this is the value'; - const helpUrl = '/help/ci/runners/index.html'; + const helpUrl = 'https://docs.gitlab.com/runner/register/index.html'; const findHelpLink = () => wrapper.findComponent(GlLink); diff --git a/spec/frontend/pipeline_editor/components/drawer/cards/first_pipeline_card_spec.js b/spec/frontend/pipeline_editor/components/drawer/cards/first_pipeline_card_spec.js index bf5d15516c2..7e1e5004d91 100644 --- a/spec/frontend/pipeline_editor/components/drawer/cards/first_pipeline_card_spec.js +++ b/spec/frontend/pipeline_editor/components/drawer/cards/first_pipeline_card_spec.js @@ -8,16 +8,8 @@ describe('First pipeline card', () => { let wrapper; let trackingSpy; - const defaultProvide = { - runnerHelpPagePath: '/help/runners', - }; - const createComponent = () => { - wrapper = mount(FirstPipelineCard, { - provide: { - ...defaultProvide, - }, - }); + wrapper = mount(FirstPipelineCard); }; const getLinkByName = (name) => getByRole(wrapper.element, 'link', { name }); @@ -43,7 +35,7 @@ describe('First pipeline card', () => { }); it('renders the link', () => { - expect(findRunnersLink().href).toContain(defaultProvide.runnerHelpPagePath); + expect(findRunnersLink().href).toBe(wrapper.vm.$options.RUNNER_HELP_URL); }); describe('tracking', () => { diff --git a/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_form_spec.js b/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_form_spec.js index 1ea035c7184..7775ed6aa37 100644 --- a/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_form_spec.js +++ b/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_form_spec.js @@ -71,7 +71,12 @@ describe('Sidebar Confidentiality Form', () => { it('creates a flash if mutation contains errors', async () => { createComponent({ mutate: jest.fn().mockResolvedValue({ - data: { issuableSetConfidential: { errors: ['Houston, we have a problem!'] } }, + data: { + issuableSetConfidential: { + issuable: { confidential: false }, + errors: ['Houston, we have a problem!'], + }, + }, }), }); findConfidentialToggle().vm.$emit('click', new MouseEvent('click')); @@ -82,6 +87,24 @@ describe('Sidebar Confidentiality Form', () => { }); }); + it('emits `closeForm` event with confidentiality value when mutation is successful', async () => { + createComponent({ + mutate: jest.fn().mockResolvedValue({ + data: { + issuableSetConfidential: { + issuable: { confidential: true }, + errors: [], + }, + }, + }), + }); + + findConfidentialToggle().vm.$emit('click', new MouseEvent('click')); + await waitForPromises(); + + expect(wrapper.emitted('closeForm')).toEqual([[{ confidential: true }]]); + }); + describe('when issue is not confidential', () => { beforeEach(() => { createComponent(); diff --git a/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_widget_spec.js b/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_widget_spec.js index 1de71e52264..18ee423d12e 100644 --- a/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_widget_spec.js +++ b/spec/frontend/sidebar/components/confidential/sidebar_confidentiality_widget_spec.js @@ -132,6 +132,7 @@ describe('Sidebar Confidentiality Widget', () => { it('closes the form and dispatches an event when `closeForm` is emitted', async () => { createComponent(); const el = wrapper.vm.$el; + const closeFormPayload = { confidential: true }; jest.spyOn(el, 'dispatchEvent'); await waitForPromises(); @@ -140,12 +141,12 @@ describe('Sidebar Confidentiality Widget', () => { expect(findConfidentialityForm().isVisible()).toBe(true); - findConfidentialityForm().vm.$emit('closeForm'); + findConfidentialityForm().vm.$emit('closeForm', closeFormPayload); await nextTick(); expect(findConfidentialityForm().isVisible()).toBe(false); expect(el.dispatchEvent).toHaveBeenCalled(); - expect(wrapper.emitted('closeForm')).toHaveLength(1); + expect(wrapper.emitted('closeForm')).toEqual([[closeFormPayload]]); }); it('emits `expandSidebar` event when it is emitted from child component', async () => { diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js index 17c0277d461..d22602e35b1 100644 --- a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js +++ b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js @@ -4,6 +4,7 @@ import VueApollo from 'vue-apollo'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import SidebarEventHub from '~/sidebar/event_hub'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import WorkItemLinks from '~/work_items/components/work_item_links/work_item_links.vue'; import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; @@ -162,6 +163,21 @@ describe('WorkItemLinks', () => { expect(findChildrenCount().text()).toContain('4'); }); + it('refetches child items when `confidentialityUpdated` event is emitted on SidebarEventhub', async () => { + const fetchHandler = jest.fn().mockResolvedValue(workItemHierarchyResponse); + await createComponent({ + fetchHandler, + }); + await waitForPromises(); + + SidebarEventHub.$emit('confidentialityUpdated'); + await nextTick(); + + // First call is done on component mount. + // Second call is done on confidentialityUpdated event. + expect(fetchHandler).toHaveBeenCalledTimes(2); + }); + describe('when no permission to update', () => { beforeEach(async () => { await createComponent({ diff --git a/spec/helpers/ci/pipeline_editor_helper_spec.rb b/spec/helpers/ci/pipeline_editor_helper_spec.rb index 59f24e7ceb8..1950d685980 100644 --- a/spec/helpers/ci/pipeline_editor_helper_spec.rb +++ b/spec/helpers/ci/pipeline_editor_helper_spec.rb @@ -64,7 +64,6 @@ RSpec.describe Ci::PipelineEditorHelper do "project-path" => project.path, "project-full-path" => project.full_path, "project-namespace" => project.namespace.full_path, - "runner-help-page-path" => help_page_path('ci/runners/index'), "simulate-pipeline-help-page-path" => help_page_path('ci/pipeline_editor/index', anchor: 'simulate-a-cicd-pipeline'), "total-branches" => project.repository.branches.length, "validate-tab-illustration-path" => 'illustrations/validate.svg', @@ -95,7 +94,6 @@ RSpec.describe Ci::PipelineEditorHelper do "project-path" => project.path, "project-full-path" => project.full_path, "project-namespace" => project.namespace.full_path, - "runner-help-page-path" => help_page_path('ci/runners/index'), "simulate-pipeline-help-page-path" => help_page_path('ci/pipeline_editor/index', anchor: 'simulate-a-cicd-pipeline'), "total-branches" => 0, "validate-tab-illustration-path" => 'illustrations/validate.svg', diff --git a/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb b/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb index 6b6ee148bbf..f03f90ddbbb 100644 --- a/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb +++ b/spec/lib/gitlab/background_migration/batched_migration_job_spec.rb @@ -5,6 +5,24 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do let(:connection) { Gitlab::Database.database_base_models[:main].connection } + describe '.generic_instance' do + it 'defines generic instance with only some of the attributes set' do + generic_instance = described_class.generic_instance( + batch_table: 'projects', batch_column: 'id', + job_arguments: %w(x y), connection: connection + ) + + expect(generic_instance.send(:batch_table)).to eq('projects') + expect(generic_instance.send(:batch_column)).to eq('id') + expect(generic_instance.instance_variable_get('@job_arguments')).to eq(%w(x y)) + expect(generic_instance.send(:connection)).to eq(connection) + + %i(start_id end_id sub_batch_size pause_ms).each do |attr| + expect(generic_instance.send(attr)).to eq(0) + end + end + end + describe '.job_arguments' do let(:job_class) do Class.new(described_class) do @@ -39,6 +57,59 @@ RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do end end + describe '.scope_to' do + subject(:job_instance) do + job_class.new(start_id: 1, end_id: 10, + batch_table: '_test_table', + batch_column: 'id', + sub_batch_size: 2, + pause_ms: 1000, + job_arguments: %w(a b), + connection: connection) + end + + context 'when additional scoping is defined' do + let(:job_class) do + Class.new(described_class) do + job_arguments :value_a, :value_b + scope_to ->(r) { "#{r}-#{value_a}-#{value_b}".upcase } + end + end + + it 'applies additional scope to the provided relation' do + expect(job_instance.filter_batch('relation')).to eq('RELATION-A-B') + end + end + + context 'when there is no additional scoping defined' do + let(:job_class) do + Class.new(described_class) do + end + end + + it 'returns provided relation as is' do + expect(job_instance.filter_batch('relation')).to eq('relation') + end + end + end + + describe 'descendants', :eager_load do + it 'have the same method signature for #perform' do + expected_arity = described_class.instance_method(:perform).arity + offences = described_class.descendants.select { |klass| klass.instance_method(:perform).arity != expected_arity } + + expect(offences).to be_empty, "expected no descendants of #{described_class} to accept arguments for " \ + "'#perform', but some do: #{offences.join(", ")}" + end + + it 'do not use .batching_scope' do + offences = described_class.descendants.select { |klass| klass.respond_to?(:batching_scope) } + + expect(offences).to be_empty, "expected no descendants of #{described_class} to define '.batching_scope', " \ + "but some do: #{offences.join(", ")}" + end + end + describe '#perform' do let(:connection) { Gitlab::Database.database_base_models[:main].connection } @@ -59,14 +130,6 @@ RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do expect { perform_job }.to raise_error(NotImplementedError, /must implement perform/) end - it 'expects descendants to have the same method signature', :eager_load do - expected_arity = described_class.instance_method(:perform).arity - offences = described_class.descendants.select { |klass| klass.instance_method(:perform).arity != expected_arity } - - expect(offences).to be_empty, "expected no descendants of #{described_class} to accept arguments for #perform, " \ - "but some do: #{offences.join(", ")}" - end - context 'when the subclass uses sub-batching' do let(:job_class) do Class.new(described_class) do @@ -110,6 +173,30 @@ RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do expect(test_table.order(:id).pluck(:to_column)).to contain_exactly(5, 10, nil, 20) end + context 'with additional scoping' do + let(:job_class) do + Class.new(described_class) do + scope_to ->(r) { r.where('mod(id, 2) = 0') } + + def perform(*job_arguments) + each_sub_batch( + operation_name: :update, + batching_arguments: { order_hint: :updated_at }, + batching_scope: -> (relation) { relation.where.not(bar: nil) } + ) do |sub_batch| + sub_batch.update_all('to_column = from_column') + end + end + end + end + + it 'respects #filter_batch' do + expect { perform_job }.to change { test_table.where(to_column: nil).count }.from(4).to(2) + + expect(test_table.order(:id).pluck(:to_column)).to contain_exactly(nil, 10, nil, 20) + end + end + it 'instruments the batch operation' do expect(job_instance.batch_metrics.affected_rows).to be_empty @@ -128,7 +215,7 @@ RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do context 'when batching_arguments are given' do it 'forwards them for batching' do - expect(job_instance).to receive(:parent_batch_relation).and_return(test_table) + expect(job_instance).to receive(:base_relation).and_return(test_table) expect(test_table).to receive(:each_batch).with(column: 'id', of: 2, order_hint: :updated_at) @@ -199,6 +286,24 @@ RSpec.describe Gitlab::BackgroundMigration::BatchedMigrationJob do expect(job_instance.batch_metrics.affected_rows[:insert]).to contain_exactly(2, 1) end + + context 'when used in combination with scope_to' do + let(:job_class) do + Class.new(described_class) do + scope_to ->(r) { r.where.not(from_column: 10) } + + def perform(*job_arguments) + distinct_each_batch(operation_name: :insert) do |sub_batch| + end + end + end + end + + it 'raises an error' do + expect { perform_job }.to raise_error RuntimeError, + /distinct_each_batch can not be used when additional filters are defined with scope_to/ + end + end end end end diff --git a/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb b/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb index 943b5744b64..9fdd7bf8adc 100644 --- a/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb +++ b/spec/lib/gitlab/background_migration/batching_strategies/primary_key_batching_strategy_spec.rb @@ -45,19 +45,16 @@ RSpec.describe Gitlab::BackgroundMigration::BatchingStrategies::PrimaryKeyBatchi end end - context 'when job_class is provided with a batching_scope' do + context 'when job class supports batch scope DSL' do let(:job_class) do - Class.new(described_class) do - def self.batching_scope(relation, job_arguments:) - min_id = job_arguments.first - - relation.where.not(type: 'Project').where('id >= ?', min_id) - end + Class.new(Gitlab::BackgroundMigration::BatchedMigrationJob) do + job_arguments :min_id + scope_to ->(r) { r.where.not(type: 'Project').where('id >= ?', min_id) } end end - it 'applies the batching scope' do - expect(job_class).to receive(:batching_scope).and_call_original + it 'applies the additional scope' do + expect(job_class).to receive(:generic_instance).and_call_original batch_bounds = batching_strategy.next_batch(:namespaces, :id, batch_min_value: namespace4.id, batch_size: 3, job_arguments: [1], job_class: job_class) diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d15f0bd4103..19026a4772d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3232,6 +3232,62 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#detailed_merge_status' do + subject(:detailed_merge_status) { merge_request.detailed_merge_status } + + context 'when merge status is cannot_be_merged_rechecking' do + let(:merge_request) { create(:merge_request, merge_status: :cannot_be_merged_rechecking) } + + it 'returns :checking' do + expect(detailed_merge_status).to eq(:checking) + end + end + + context 'when merge status is preparing' do + let(:merge_request) { create(:merge_request, merge_status: :preparing) } + + it 'returns :checking' do + expect(detailed_merge_status).to eq(:checking) + end + end + + context 'when merge status is checking' do + let(:merge_request) { create(:merge_request, merge_status: :checking) } + + it 'returns :checking' do + expect(detailed_merge_status).to eq(:checking) + end + end + + context 'when merge status is unchecked' do + let(:merge_request) { create(:merge_request, merge_status: :unchecked) } + + it 'returns :unchecked' do + expect(detailed_merge_status).to eq(:unchecked) + end + end + + context 'when merge checks are a success' do + let(:merge_request) { create(:merge_request) } + + it 'returns :mergeable' do + expect(detailed_merge_status).to eq(:mergeable) + end + end + + context 'when merge status have a failure' do + let(:merge_request) { create(:merge_request) } + + before do + merge_request.close! + end + + it 'returns the failure reason' do + expect(detailed_merge_status).to eq(:not_open) + end + end + end + describe '#mergeable_state?' do it_behaves_like 'for mergeable_state' diff --git a/spec/services/merge_requests/mergeability/check_broken_status_service_spec.rb b/spec/services/merge_requests/mergeability/check_broken_status_service_spec.rb index 9e178c121ef..6cc1079c94a 100644 --- a/spec/services/merge_requests/mergeability/check_broken_status_service_spec.rb +++ b/spec/services/merge_requests/mergeability/check_broken_status_service_spec.rb @@ -8,6 +8,8 @@ RSpec.describe MergeRequests::Mergeability::CheckBrokenStatusService do let(:merge_request) { build(:merge_request) } describe '#execute' do + let(:result) { check_broken_status.execute } + before do expect(merge_request).to receive(:broken?).and_return(broken) end @@ -16,7 +18,8 @@ RSpec.describe MergeRequests::Mergeability::CheckBrokenStatusService do let(:broken) { true } it 'returns a check result with status failed' do - expect(check_broken_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.payload[:reason]).to eq(:broken_status) end end @@ -24,7 +27,7 @@ RSpec.describe MergeRequests::Mergeability::CheckBrokenStatusService do let(:broken) { false } it 'returns a check result with status success' do - expect(check_broken_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS end end end diff --git a/spec/services/merge_requests/mergeability/check_ci_status_service_spec.rb b/spec/services/merge_requests/mergeability/check_ci_status_service_spec.rb index 6fbbecd7c0e..def3cb0ca28 100644 --- a/spec/services/merge_requests/mergeability/check_ci_status_service_spec.rb +++ b/spec/services/merge_requests/mergeability/check_ci_status_service_spec.rb @@ -10,6 +10,8 @@ RSpec.describe MergeRequests::Mergeability::CheckCiStatusService do let(:skip_check) { false } describe '#execute' do + let(:result) { check_ci_status.execute } + before do expect(merge_request).to receive(:mergeable_ci_state?).and_return(mergeable) end @@ -18,7 +20,7 @@ RSpec.describe MergeRequests::Mergeability::CheckCiStatusService do let(:mergeable) { true } it 'returns a check result with status success' do - expect(check_ci_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS end end @@ -26,7 +28,8 @@ RSpec.describe MergeRequests::Mergeability::CheckCiStatusService do let(:mergeable) { false } it 'returns a check result with status failed' do - expect(check_ci_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.payload[:reason]).to eq :ci_must_pass end end end diff --git a/spec/services/merge_requests/mergeability/check_discussions_status_service_spec.rb b/spec/services/merge_requests/mergeability/check_discussions_status_service_spec.rb index c24d40967c4..9f107ce046a 100644 --- a/spec/services/merge_requests/mergeability/check_discussions_status_service_spec.rb +++ b/spec/services/merge_requests/mergeability/check_discussions_status_service_spec.rb @@ -10,6 +10,8 @@ RSpec.describe MergeRequests::Mergeability::CheckDiscussionsStatusService do let(:skip_check) { false } describe '#execute' do + let(:result) { check_discussions_status.execute } + before do expect(merge_request).to receive(:mergeable_discussions_state?).and_return(mergeable) end @@ -18,7 +20,7 @@ RSpec.describe MergeRequests::Mergeability::CheckDiscussionsStatusService do let(:mergeable) { true } it 'returns a check result with status success' do - expect(check_discussions_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS end end @@ -26,7 +28,8 @@ RSpec.describe MergeRequests::Mergeability::CheckDiscussionsStatusService do let(:mergeable) { false } it 'returns a check result with status failed' do - expect(check_discussions_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.payload[:reason]).to eq(:discussions_not_resolved) end end end diff --git a/spec/services/merge_requests/mergeability/check_draft_status_service_spec.rb b/spec/services/merge_requests/mergeability/check_draft_status_service_spec.rb index 923cff220ef..e9363e5d676 100644 --- a/spec/services/merge_requests/mergeability/check_draft_status_service_spec.rb +++ b/spec/services/merge_requests/mergeability/check_draft_status_service_spec.rb @@ -8,6 +8,8 @@ RSpec.describe MergeRequests::Mergeability::CheckDraftStatusService do let(:merge_request) { build(:merge_request) } describe '#execute' do + let(:result) { check_draft_status.execute } + before do expect(merge_request).to receive(:draft?).and_return(draft) end @@ -16,7 +18,8 @@ RSpec.describe MergeRequests::Mergeability::CheckDraftStatusService do let(:draft) { true } it 'returns a check result with status failed' do - expect(check_draft_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.payload[:reason]).to eq(:draft_status) end end @@ -24,7 +27,7 @@ RSpec.describe MergeRequests::Mergeability::CheckDraftStatusService do let(:draft) { false } it 'returns a check result with status success' do - expect(check_draft_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS end end end diff --git a/spec/services/merge_requests/mergeability/check_open_status_service_spec.rb b/spec/services/merge_requests/mergeability/check_open_status_service_spec.rb index b1c9a930317..936524b020a 100644 --- a/spec/services/merge_requests/mergeability/check_open_status_service_spec.rb +++ b/spec/services/merge_requests/mergeability/check_open_status_service_spec.rb @@ -8,6 +8,8 @@ RSpec.describe MergeRequests::Mergeability::CheckOpenStatusService do let(:merge_request) { build(:merge_request) } describe '#execute' do + let(:result) { check_open_status.execute } + before do expect(merge_request).to receive(:open?).and_return(open) end @@ -16,7 +18,7 @@ RSpec.describe MergeRequests::Mergeability::CheckOpenStatusService do let(:open) { true } it 'returns a check result with status success' do - expect(check_open_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS end end @@ -24,7 +26,8 @@ RSpec.describe MergeRequests::Mergeability::CheckOpenStatusService do let(:open) { false } it 'returns a check result with status failed' do - expect(check_open_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS + expect(result.payload[:reason]).to eq(:not_open) end end end diff --git a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb index 2bb7dc3eef7..afea3e952a1 100644 --- a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb +++ b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb @@ -5,11 +5,11 @@ require 'spec_helper' RSpec.describe MergeRequests::Mergeability::RunChecksService do subject(:run_checks) { described_class.new(merge_request: merge_request, params: {}) } - let_it_be(:merge_request) { create(:merge_request) } - describe '#execute' do subject(:execute) { run_checks.execute } + let_it_be(:merge_request) { create(:merge_request) } + let(:params) { {} } let(:success_result) { Gitlab::MergeRequests::Mergeability::CheckResult.success } @@ -23,7 +23,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do end it 'is still a success' do - expect(execute.all?(&:success?)).to eq(true) + expect(execute.success?).to eq(true) end end @@ -41,13 +41,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do expect(service).not_to receive(:execute) end - # Since we're only marking one check to be skipped, we expect to receive - # `# of checks - 1` success result objects in return - # - check_count = merge_request.mergeability_checks.count - 1 - success_array = (1..check_count).each_with_object([]) { |_, array| array << success_result } - - expect(execute).to match_array(success_array) + expect(execute.success?).to eq(true) end end @@ -75,7 +69,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do expect(service).to receive(:read).with(merge_check: merge_check).and_return(success_result) end - expect(execute).to match_array([success_result]) + expect(execute.success?).to eq(true) end end @@ -86,7 +80,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do expect(service).to receive(:write).with(merge_check: merge_check, result_hash: success_result.to_hash).and_return(true) end - expect(execute).to match_array([success_result]) + expect(execute.success?).to eq(true) end end end @@ -97,7 +91,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do it 'does not call the results store' do expect(Gitlab::MergeRequests::Mergeability::ResultsStore).not_to receive(:new) - expect(execute).to match_array([success_result]) + expect(execute.success?).to eq(true) end end @@ -109,9 +103,81 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do it 'does not call the results store' do expect(Gitlab::MergeRequests::Mergeability::ResultsStore).not_to receive(:new) - expect(execute).to match_array([success_result]) + expect(execute.success?).to eq(true) end end end end + + describe '#success?' do + subject(:success) { run_checks.success? } + + let_it_be(:merge_request) { create(:merge_request) } + + context 'when the execute method has been executed' do + before do + run_checks.execute + end + + context 'when all the checks succeed' do + it 'returns true' do + expect(success).to eq(true) + end + end + + context 'when one check fails' do + before do + allow(merge_request).to receive(:open?).and_return(false) + run_checks.execute + end + + it 'returns false' do + expect(success).to eq(false) + end + end + end + + context 'when execute has not been exectued' do + it 'raises an error' do + expect { subject } + .to raise_error(/Execute needs to be called before/) + end + end + end + + describe '#failure_reason' do + subject(:failure_reason) { run_checks.failure_reason } + + let_it_be(:merge_request) { create(:merge_request) } + + context 'when the execute method has been executed' do + before do + run_checks.execute + end + + context 'when all the checks succeed' do + it 'returns nil' do + expect(failure_reason).to eq(nil) + end + end + + context 'when one check fails' do + before do + allow(merge_request).to receive(:open?).and_return(false) + run_checks.execute + end + + it 'returns the open reason' do + expect(failure_reason).to eq(:not_open) + end + end + end + + context 'when execute has not been exectued' do + it 'raises an error' do + expect { subject } + .to raise_error(/Execute needs to be called before/) + end + end + end end |