diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-22 18:07:44 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-22 18:07:44 +0000 |
| commit | d0f16d56f3716d4a60027eb261f12080094f8db3 (patch) | |
| tree | f9ecf9f4bda6d761f612bc4a5efb701b7c6b3d2d | |
| parent | 68aa32736b50c3609348f3bf740b81a2dfd1fb25 (diff) | |
| download | gitlab-ce-d0f16d56f3716d4a60027eb261f12080094f8db3.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
65 files changed, 953 insertions, 470 deletions
diff --git a/.rubocop_todo/gitlab/strong_memoize_attr.yml b/.rubocop_todo/gitlab/strong_memoize_attr.yml index 1eeb7c69a96..04f76fbe260 100644 --- a/.rubocop_todo/gitlab/strong_memoize_attr.yml +++ b/.rubocop_todo/gitlab/strong_memoize_attr.yml @@ -647,7 +647,6 @@ Gitlab/StrongMemoizeAttr: - 'lib/gitlab/git_access_project.rb' - 'lib/gitlab/gitaly_client/with_feature_flag_actors.rb' - 'lib/gitlab/github_import/client.rb' - - 'lib/gitlab/github_import/importer/repository_importer.rb' - 'lib/gitlab/github_import/representation/diff_notes/suggestion_formatter.rb' - 'lib/gitlab/gl_repository/identifier.rb' - 'lib/gitlab/gpg/commit.rb' diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index 48c39454d8d..be2b77ef81e 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -2048,7 +2048,6 @@ Layout/ArgumentAlignment: - 'lib/gitlab/import_export/snippets_repo_restorer.rb' - 'lib/gitlab/import_export/snippets_repo_saver.rb' - 'lib/gitlab/issuable/clone/copy_resource_events_service.rb' - - 'lib/gitlab/legacy_github_import/importer.rb' - 'lib/gitlab/mail_room.rb' - 'lib/gitlab/markdown_cache/redis/store.rb' - 'lib/gitlab/memory/reports_uploader.rb' diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 772284a6ce7..12683ec3831 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -3062,7 +3062,6 @@ Layout/LineLength: - 'lib/gitlab/kubernetes/helm/pod.rb' - 'lib/gitlab/kubernetes/kubectl_cmd.rb' - 'lib/gitlab/kubernetes/pod_cmd.rb' - - 'lib/gitlab/legacy_github_import/importer.rb' - 'lib/gitlab/legacy_github_import/project_creator.rb' - 'lib/gitlab/local_and_remote_storage_migration/base_migrater.rb' - 'lib/gitlab/lograge/custom_options.rb' diff --git a/.rubocop_todo/naming/heredoc_delimiter_naming.yml b/.rubocop_todo/naming/heredoc_delimiter_naming.yml index ae71e06b6d3..b92d316b693 100644 --- a/.rubocop_todo/naming/heredoc_delimiter_naming.yml +++ b/.rubocop_todo/naming/heredoc_delimiter_naming.yml @@ -50,7 +50,6 @@ Naming/HeredocDelimiterNaming: - 'rubocop/cop/default_scope.rb' - 'rubocop/cop/file_decompression.rb' - 'rubocop/cop/gitlab/httparty.rb' - - 'rubocop/cop/gitlab/json.rb' - 'rubocop/cop/gitlab/module_with_instance_variables.rb' - 'rubocop/cop/gitlab/predicate_memoization.rb' - 'spec/controllers/projects/pipelines_controller_spec.rb' diff --git a/.rubocop_todo/performance/map_compact.yml b/.rubocop_todo/performance/map_compact.yml index ca0e8d604fd..c3586b35cf3 100644 --- a/.rubocop_todo/performance/map_compact.yml +++ b/.rubocop_todo/performance/map_compact.yml @@ -116,7 +116,6 @@ Performance/MapCompact: - 'lib/gitlab/jira_import/metadata_collector.rb' - 'lib/gitlab/json_cache.rb' - 'lib/gitlab/language_detection.rb' - - 'lib/gitlab/legacy_github_import/importer.rb' - 'lib/gitlab/private_commit_email.rb' - 'lib/gitlab/sql/pattern.rb' - 'lib/gitlab/url_blocker.rb' diff --git a/.rubocop_todo/performance/string_include.yml b/.rubocop_todo/performance/string_include.yml deleted file mode 100644 index f2e17d3576a..00000000000 --- a/.rubocop_todo/performance/string_include.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -# Cop supports --autocorrect. -Performance/StringInclude: - Details: grace period - Exclude: - - 'lib/gitlab/github_import/importer/repository_importer.rb' - - 'lib/gitlab/legacy_github_import/importer.rb' - - 'lib/gitlab/usage_data.rb' - - 'rubocop/cop/gitlab/json.rb' diff --git a/.rubocop_todo/rake/require.yml b/.rubocop_todo/rake/require.yml index e3cd483f056..9820ce7d03c 100644 --- a/.rubocop_todo/rake/require.yml +++ b/.rubocop_todo/rake/require.yml @@ -12,8 +12,6 @@ Rake/Require: - 'lib/tasks/gitlab/packages/migrate.rake' - 'lib/tasks/gitlab/pages.rake' - 'lib/tasks/gitlab/refresh_project_statistics_build_artifacts_size.rake' - - 'lib/tasks/gitlab/terraform/migrate.rake' - - 'lib/tasks/gitlab/tw/codeowners.rake' - 'lib/tasks/gitlab/x509/update.rake' - 'lib/tasks/import.rake' - 'lib/tasks/tokens.rake' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 6c5694293f8..5f4d4dda292 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -5057,7 +5057,6 @@ RSpec/MissingFeatureCategory: - 'spec/lib/gitlab/legacy_github_import/branch_formatter_spec.rb' - 'spec/lib/gitlab/legacy_github_import/client_spec.rb' - 'spec/lib/gitlab/legacy_github_import/comment_formatter_spec.rb' - - 'spec/lib/gitlab/legacy_github_import/importer_spec.rb' - 'spec/lib/gitlab/legacy_github_import/issuable_formatter_spec.rb' - 'spec/lib/gitlab/legacy_github_import/issue_formatter_spec.rb' - 'spec/lib/gitlab/legacy_github_import/label_formatter_spec.rb' diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml index efdb0df8341..cda00940294 100644 --- a/.rubocop_todo/style/if_unless_modifier.yml +++ b/.rubocop_todo/style/if_unless_modifier.yml @@ -891,7 +891,6 @@ Style/IfUnlessModifier: - 'lib/gitlab/jira_import.rb' - 'lib/gitlab/jira_import/base_importer.rb' - 'lib/gitlab/legacy_github_import/client.rb' - - 'lib/gitlab/legacy_github_import/importer.rb' - 'lib/gitlab/legacy_github_import/issuable_formatter.rb' - 'lib/gitlab/legacy_github_import/project_creator.rb' - 'lib/gitlab/lograge/custom_options.rb' diff --git a/app/assets/javascripts/search/sidebar/components/checkbox_filter.vue b/app/assets/javascripts/search/sidebar/components/checkbox_filter.vue index b580d58b21b..36cbe9ce693 100644 --- a/app/assets/javascripts/search/sidebar/components/checkbox_filter.vue +++ b/app/assets/javascripts/search/sidebar/components/checkbox_filter.vue @@ -1,6 +1,6 @@ <script> import { GlFormCheckboxGroup, GlFormCheckbox } from '@gitlab/ui'; -import { mapState, mapActions } from 'vuex'; +import { mapState, mapActions, mapGetters } from 'vuex'; import { intersection } from 'lodash'; import { NAV_LINK_COUNT_DEFAULT_CLASSES, LABEL_DEFAULT_CLASSES } from '../constants'; import { formatSearchResultCount } from '../../store/utils'; @@ -12,31 +12,29 @@ export default { GlFormCheckbox, }, props: { - filterData: { + filtersData: { type: Object, required: true, }, }, computed: { ...mapState(['query']), + ...mapGetters(['queryLangugageFilters']), scope() { return this.query.scope; }, - queryFilters() { - return this.query[this.filterData?.filterParam] || []; - }, dataFilters() { - return Object.values(this.filterData?.filters || []); + return Object.values(this.filtersData?.filters || []); }, flatDataFilterValues() { return this.dataFilters.map(({ value }) => value); }, selectedFilter: { get() { - return intersection(this.flatDataFilterValues, this.queryFilters); + return intersection(this.flatDataFilterValues, this.queryLangugageFilters); }, set(value) { - this.setQuery({ key: this.filterData?.filterParam, value }); + this.setQuery({ key: this.filtersData?.filterParam, value }); }, }, labelCountClasses() { @@ -56,7 +54,7 @@ export default { <template> <div class="gl-mx-5"> - <h5 class="gl-mt-0">{{ filterData.header }}</h5> + <h5 class="gl-mt-0">{{ filtersData.header }}</h5> <gl-form-checkbox-group v-model="selectedFilter"> <gl-form-checkbox v-for="f in dataFilters" diff --git a/app/assets/javascripts/search/sidebar/components/language_filter.vue b/app/assets/javascripts/search/sidebar/components/language_filter.vue index 26ce204cb5c..e5306cd7b79 100644 --- a/app/assets/javascripts/search/sidebar/components/language_filter.vue +++ b/app/assets/javascripts/search/sidebar/components/language_filter.vue @@ -27,10 +27,15 @@ export default { apply: __('Apply'), showingMax: sprintf(s__('GlobalSearch|Showing top %{maxItems}'), { maxItems: MAX_ITEM_LENGTH }), loadError: s__('GlobalSearch|Aggregations load error.'), + reset: s__('GlobalSearch|Reset filters'), }, computed: { ...mapState(['aggregations', 'sidebarDirty']), - ...mapGetters(['langugageAggregationBuckets']), + ...mapGetters([ + 'langugageAggregationBuckets', + 'currentUrlQueryHasLanguageFilters', + 'queryLangugageFilters', + ]), hasBuckets() { return this.langugageAggregationBuckets.length > 0; }, @@ -55,18 +60,33 @@ export default { dividerClasses() { return [...HR_DEFAULT_CLASSES, ...ONLY_SHOW_MD]; }, + hasQueryFilters() { + return this.queryLangugageFilters.length > 0; + }, }, async created() { await this.fetchLanguageAggregation(); }, methods: { - ...mapActions(['applyQuery', 'fetchLanguageAggregation']), + ...mapActions([ + 'applyQuery', + 'resetLanguageQuery', + 'resetLanguageQueryWithRedirect', + 'fetchLanguageAggregation', + ]), onShowMore() { this.showAll = true; }, trimBuckets(length) { return this.langugageAggregationBuckets.slice(0, length); }, + cleanResetFilters() { + if (this.currentUrlQueryHasLanguageFilters) { + return this.resetLanguageQueryWithRedirect(); + } + this.showAll = false; + return this.resetLanguageQuery(); + }, }, HR_DEFAULT_CLASSES, }; @@ -84,7 +104,7 @@ export default { class="gl-overflow-x-hidden gl-overflow-y-auto" :class="{ 'language-filter-max-height': showAll }" > - <checkbox-filter class="gl-px-5" :filter-data="filtersData" /> + <checkbox-filter :filters-data="filtersData" /> <span v-if="showAll && hasOverMax" data-testid="has-over-max-text">{{ $options.i18n.showingMax }}</span> @@ -106,7 +126,9 @@ export default { </div> <div v-if="!aggregations.error"> <hr :class="$options.HR_DEFAULT_CLASSES" /> - <div class="gl-display-flex gl-align-items-center gl-mt-4 gl-mx-5 gl-px-5"> + <div + class="gl-display-flex gl-align-items-center gl-justify-content-space-between gl-mt-4 gl-mx-5" + > <gl-button category="primary" variant="confirm" @@ -116,6 +138,16 @@ export default { > {{ $options.i18n.apply }} </gl-button> + <gl-button + category="tertiary" + variant="link" + size="small" + :disabled="!hasQueryFilters && !sidebarDirty" + data-testid="reset-button" + @click="cleanResetFilters" + > + {{ $options.i18n.reset }} + </gl-button> </div> </div> </gl-form> diff --git a/app/assets/javascripts/search/sidebar/utils.js b/app/assets/javascripts/search/sidebar/utils.js index 5c08ad2f959..4357d6202df 100644 --- a/app/assets/javascripts/search/sidebar/utils.js +++ b/app/assets/javascripts/search/sidebar/utils.js @@ -1,20 +1,17 @@ import { languageFilterData } from '~/search/sidebar/constants/language_filter_data'; -export const convertFiltersData = (rawBuckets) => { - return rawBuckets.reduce( - (acc, bucket) => { - return { - ...acc, - filters: { - ...acc.filters, - [bucket.key.toUpperCase()]: { - label: bucket.key, - value: bucket.key, - count: bucket.count, - }, +export const convertFiltersData = (rawBuckets) => + rawBuckets.reduce( + (acc, bucket) => ({ + ...acc, + filters: { + ...acc.filters, + [bucket.key.toUpperCase()]: { + label: bucket.key, + value: bucket.key, + count: bucket.count, }, - }; - }, + }, + }), { ...languageFilterData, filters: {} }, ); -}; diff --git a/app/assets/javascripts/search/store/actions.js b/app/assets/javascripts/search/store/actions.js index fc0817be882..667c56cc5ad 100644 --- a/app/assets/javascripts/search/store/actions.js +++ b/app/assets/javascripts/search/store/actions.js @@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils'; import { visitUrl, setUrlParams } from '~/lib/utils/url_utility'; import { logError } from '~/lib/logger'; import { __ } from '~/locale'; +import { languageFilterData } from '~/search/sidebar/constants/language_filter_data'; import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY, SIDEBAR_PARAMS } from './constants'; import * as types from './mutation_types'; import { @@ -105,7 +106,17 @@ export const applyQuery = ({ state }) => { }; export const resetQuery = ({ state }) => { - visitUrl(setUrlParams({ ...state.query, page: null, state: null, confidential: null })); + visitUrl( + setUrlParams({ ...state.query, page: null, state: null, confidential: null }, undefined, true), + ); +}; + +export const resetLanguageQueryWithRedirect = ({ state }) => { + visitUrl(setUrlParams({ ...state.query, language: null }, undefined, true)); +}; + +export const resetLanguageQuery = ({ commit }) => { + commit(types.SET_QUERY, { key: languageFilterData?.filterParam, value: [] }); }; export const fetchSidebarCount = ({ commit, state }) => { diff --git a/app/assets/javascripts/search/store/getters.js b/app/assets/javascripts/search/store/getters.js index 0278239c144..853c83b07ee 100644 --- a/app/assets/javascripts/search/store/getters.js +++ b/app/assets/javascripts/search/store/getters.js @@ -1,4 +1,6 @@ +import { has } from 'lodash'; import { languageFilterData } from '~/search/sidebar/constants/language_filter_data'; + import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY } from './constants'; export const frequentGroups = (state) => { @@ -16,3 +18,11 @@ export const langugageAggregationBuckets = (state) => { )?.buckets || [] ); }; + +export const queryLangugageFilters = (state) => { + return state.query[languageFilterData.filterParam] || []; +}; + +export const currentUrlQueryHasLanguageFilters = (state) => + has(state.urlQuery, languageFilterData.filterParam) && + state.urlQuery[languageFilterData.filterParam]?.length > 0; diff --git a/app/assets/javascripts/search/store/mutations.js b/app/assets/javascripts/search/store/mutations.js index f9fd69d2211..b2f9f5ab225 100644 --- a/app/assets/javascripts/search/store/mutations.js +++ b/app/assets/javascripts/search/store/mutations.js @@ -24,7 +24,7 @@ export default { state.projects = []; }, [types.SET_QUERY](state, { key, value }) { - state.query[key] = value; + state.query = { ...state.query, [key]: value }; }, [types.SET_SIDEBAR_DIRTY](state, value) { state.sidebarDirty = value; diff --git a/app/assets/javascripts/search/store/utils.js b/app/assets/javascripts/search/store/utils.js index acb99c60426..1e6619ca6d5 100644 --- a/app/assets/javascripts/search/store/utils.js +++ b/app/assets/javascripts/search/store/utils.js @@ -1,3 +1,4 @@ +import { isEqual } from 'lodash'; import AccessorUtilities from '~/lib/utils/accessor'; import { formatNumber } from '~/locale'; import { joinPaths } from '~/lib/utils/url_utility'; @@ -94,6 +95,10 @@ export const isSidebarDirty = (currentQuery, urlQuery) => { const userAddedParam = !urlQuery[param] && currentQuery[param]; const userChangedExistingParam = urlQuery[param] && urlQuery[param] !== currentQuery[param]; + if (Array.isArray(currentQuery[param]) || Array.isArray(urlQuery[param])) { + return !isEqual(currentQuery[param], urlQuery[param]); + } + return userAddedParam || userChangedExistingParam; }); }; diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue index 4b65d6fd9ac..17fb2daaf17 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue @@ -5,6 +5,7 @@ import { BV_SHOW_MODAL } from '~/lib/utils/constants'; import { HTTP_STATUS_UNAUTHORIZED } from '~/lib/utils/http_status'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { s__, __ } from '~/locale'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import eventHub from '../../event_hub'; import approvalsMixin from '../../mixins/approvals'; import MrWidgetContainer from '../mr_widget_container.vue'; @@ -12,7 +13,7 @@ import MrWidgetIcon from '../mr_widget_icon.vue'; import { INVALID_RULES_DOCS_PATH } from '../../constants'; import ApprovalsSummary from './approvals_summary.vue'; import ApprovalsSummaryOptional from './approvals_summary_optional.vue'; -import { FETCH_LOADING, FETCH_ERROR, APPROVE_ERROR, UNAPPROVE_ERROR } from './messages'; +import { FETCH_LOADING, APPROVE_ERROR, UNAPPROVE_ERROR } from './messages'; import { humanizeInvalidApproversRules } from './humanized_text'; export default { @@ -59,10 +60,8 @@ export default { }, data() { return { - fetchingApprovals: true, hasApprovalAuthError: false, isApproving: false, - updatedCount: 0, }; }, computed: { @@ -70,7 +69,7 @@ export default { return this.mr.approvalsWidgetType === 'base'; }, isApproved() { - return Boolean(this.approvals.approved); + return Boolean(this.approvals.approved || this.approvedBy.length); }, isOptional() { return this.isOptionalDefault !== null ? this.isOptionalDefault : !this.approvedBy.length; @@ -78,26 +77,25 @@ export default { hasAction() { return Boolean(this.action); }, - approvals() { - return this.mr.approvals || {}; - }, invalidRules() { - return this.approvals.invalid_approvers_rules || []; + return this.approvals.approvalState?.invalidApproversRules || []; }, hasInvalidRules() { - return this.approvals.merge_request_approvers_available && this.invalidRules.length; + return this.mr.mergeRequestApproversAvailable && this.invalidRules.length; }, invalidRulesText() { return humanizeInvalidApproversRules(this.invalidRules); }, approvedBy() { - return this.approvals.approved_by ? this.approvals.approved_by.map((x) => x.user) : []; + return this.approvals.approvedBy?.nodes || []; }, userHasApproved() { - return Boolean(this.approvals.user_has_approved); + return this.approvedBy.some( + (approver) => getIdFromGraphQLId(approver.id) === gon.current_user_id, + ); }, userCanApprove() { - return Boolean(this.approvals.user_can_approve); + return Boolean(this.approvals.userPermissions.canApprove); }, showApprove() { return !this.userHasApproved && this.userCanApprove && this.mr.isOpen; @@ -135,19 +133,6 @@ export default { : this.$options.i18n.invalidRuleSingular; }, }, - created() { - this.refreshApprovals() - .then(() => { - this.fetchingApprovals = false; - }) - .catch(() => - this.alerts.push( - createAlert({ - message: FETCH_ERROR, - }), - ), - ); - }, methods: { approve() { if (this.requirePasswordToApprove) { @@ -196,16 +181,14 @@ export default { this.isApproving = true; this.clearError(); return serviceFn() - .then((data) => { - this.mr.setApprovals(data); - this.updatedCount += 1; - + .then(() => { if (!window.gon?.features?.realtimeMrStatusChange) { eventHub.$emit('MRWidgetUpdateRequested'); eventHub.$emit('ApprovalUpdated'); } - this.$emit('updated'); + // TODO: Remove this line when we move to Apollo subscriptions + this.$apollo.queries.approvals.refetch(); }) .catch(errFn) .then(() => { @@ -230,7 +213,7 @@ export default { <mr-widget-container> <div class="js-mr-approvals d-flex align-items-start align-items-md-center"> <mr-widget-icon name="approval" /> - <div v-if="fetchingApprovals">{{ $options.FETCH_LOADING }}</div> + <div v-if="$apollo.queries.approvals.loading">{{ $options.FETCH_LOADING }}</div> <template v-else> <div class="gl-display-flex gl-flex-direction-column"> <div class="gl-display-flex gl-flex-direction-row gl-align-items-center"> @@ -252,9 +235,7 @@ export default { /> <approvals-summary v-else - :project-path="mr.targetProjectFullPath" - :iid="`${mr.iid}`" - :updated-count="updatedCount" + :approval-state="approvals" :multiple-approval-rules-available="mr.multipleApprovalRulesAvailable" /> </div> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary.vue b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary.vue index 697d953874c..2af033bb80f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals_summary.vue @@ -1,5 +1,4 @@ <script> -import { GlSkeletonLoader } from '@gitlab/ui'; import { toNounSeriesText } from '~/lib/utils/grammar'; import { n__, sprintf } from '~/locale'; import { @@ -10,49 +9,21 @@ import { import UserAvatarList from '~/vue_shared/components/user_avatar/user_avatar_list.vue'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { getApprovalRuleNamesLeft } from 'ee_else_ce/vue_merge_request_widget/mappers'; -import approvedByQuery from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approved_by.query.graphql'; export default { - apollo: { - approvalState: { - query: approvedByQuery, - variables() { - return { - projectPath: this.projectPath, - iid: this.iid, - }; - }, - update: (data) => data.project.mergeRequest, - }, - }, components: { - GlSkeletonLoader, UserAvatarList, }, props: { - projectPath: { - type: String, - required: true, - }, - iid: { - type: String, - required: true, - }, - updatedCount: { - type: Number, - required: false, - default: 0, - }, multipleApprovalRulesAvailable: { type: Boolean, required: false, default: false, }, - }, - data() { - return { - approvalState: {}, - }; + approvalState: { + type: Object, + required: true, + }, }, computed: { approvers() { @@ -134,37 +105,20 @@ export default { return gon.current_user_id; }, }, - watch: { - updatedCount() { - this.$apollo.queries.approvalState.refetch(); - }, - }, }; </script> <template> <div data-qa-selector="approvals_summary_content"> - <div - v-if="$apollo.queries.approvalState.loading" - class="gl-display-inline-block gl-vertical-align-middle" - style="width: 132px; height: 24px" - > - <gl-skeleton-loader :width="132" :height="24"> - <rect width="100" height="24" x="0" y="0" rx="4" /> - <circle cx="120" cy="12" r="12" /> - </gl-skeleton-loader> - </div> - <template v-else> - <span class="gl-font-weight-bold">{{ approvalLeftMessage }}</span> - <template v-if="hasApprovers"> - <span v-if="approvalLeftMessage">{{ message }}</span> - <span v-else class="gl-font-weight-bold">{{ message }}</span> - <user-avatar-list - class="gl-display-inline-block gl-vertical-align-middle gl-pt-1" - :img-size="24" - :items="approvers" - /> - </template> + <span class="gl-font-weight-bold">{{ approvalLeftMessage }}</span> + <template v-if="hasApprovers"> + <span v-if="approvalLeftMessage">{{ message }}</span> + <span v-else class="gl-font-weight-bold">{{ message }}</span> + <user-avatar-list + class="gl-display-inline-block gl-vertical-align-middle gl-pt-1" + :img-size="24" + :items="approvers" + /> </template> </div> </template> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/approvals/queries/approved_by.query.graphql b/app/assets/javascripts/vue_merge_request_widget/components/approvals/queries/approvals.query.graphql index c8cae6a8885..437ae578cd0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/approvals/queries/approved_by.query.graphql +++ b/app/assets/javascripts/vue_merge_request_widget/components/approvals/queries/approvals.query.graphql @@ -1,3 +1,5 @@ +#import "~/graphql_shared/fragments/user.fragment.graphql" + query approvedBy($projectPath: ID!, $iid: String!) { project(fullPath: $projectPath) { id @@ -5,12 +7,12 @@ query approvedBy($projectPath: ID!, $iid: String!) { id approvedBy { nodes { - id - name - avatarUrl - webUrl + ...User } } + userPermissions { + canApprove + } } } } diff --git a/app/assets/javascripts/vue_merge_request_widget/index.js b/app/assets/javascripts/vue_merge_request_widget/index.js index 8d596465970..c66641310ee 100644 --- a/app/assets/javascripts/vue_merge_request_widget/index.js +++ b/app/assets/javascripts/vue_merge_request_widget/index.js @@ -13,7 +13,18 @@ Vue.use(Translate); Vue.use(VueApollo); const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(), + defaultClient: createDefaultClient( + {}, + { + cacheConfig: { + typePolicies: { + MergeRequestApprovalState: { + merge: true, + }, + }, + }, + }, + ), }); export default () => { diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/approvals.js b/app/assets/javascripts/vue_merge_request_widget/mixins/approvals.js index 7d0871f696b..a43c784db28 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mixins/approvals.js +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/approvals.js @@ -1,7 +1,34 @@ +import { createAlert } from '~/flash'; +import approvedByQuery from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approvals.query.graphql'; +import { FETCH_ERROR } from '../components/approvals/messages'; + export default { + apollo: { + approvals: { + query: approvedByQuery, + variables() { + return { + projectPath: this.mr.targetProjectFullPath, + iid: `${this.mr.iid}`, + }; + }, + update: (data) => data.project.mergeRequest, + result({ data }) { + const { mergeRequest } = data.project; + + this.mr.setApprovals(mergeRequest); + }, + error() { + createAlert({ + message: FETCH_ERROR, + }); + }, + }, + }, data() { return { alerts: [], + approvals: {}, }; }, methods: { diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index f6a7ef58c10..827080d9866 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -356,12 +356,11 @@ export default class MergeRequestStore { initApprovals() { this.isApproved = this.isApproved || false; - this.approvals = this.approvals || null; } setApprovals(data) { - this.approvals = data; this.isApproved = data.approved || false; + this.approvals = true; this.setState(); } diff --git a/app/graphql/mutations/ci/runner/common_mutation_arguments.rb b/app/graphql/mutations/ci/runner/common_mutation_arguments.rb new file mode 100644 index 00000000000..bfeed4881c6 --- /dev/null +++ b/app/graphql/mutations/ci/runner/common_mutation_arguments.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Mutations + module Ci + module Runner + module CommonMutationArguments + extend ActiveSupport::Concern + + included do + argument :description, GraphQL::Types::String, + required: false, + description: 'Description of the runner.' + + argument :maintenance_note, GraphQL::Types::String, + required: false, + description: 'Runner\'s maintenance notes.' + + argument :maximum_timeout, GraphQL::Types::Int, + required: false, + description: 'Maximum timeout (in seconds) for jobs processed by the runner.' + + argument :access_level, ::Types::Ci::RunnerAccessLevelEnum, + required: false, + description: 'Access level of the runner.' + + argument :paused, GraphQL::Types::Boolean, + required: false, + description: 'Indicates the runner is not allowed to receive jobs.' + + argument :locked, GraphQL::Types::Boolean, + required: false, + description: 'Indicates the runner is locked.' + + argument :run_untagged, GraphQL::Types::Boolean, + required: false, + description: 'Indicates the runner is able to run untagged jobs.' + + argument :tag_list, [GraphQL::Types::String], + required: false, + description: 'Tags associated with the runner.' + + argument :associated_projects, [::Types::GlobalIDType[::Project]], + required: false, + description: 'Projects associated with the runner. Available only for project runners.', + prepare: ->(global_ids, _ctx) { global_ids&.filter_map { |gid| gid.model_id.to_i } } + end + end + end + end +end diff --git a/app/graphql/mutations/ci/runner/create.rb b/app/graphql/mutations/ci/runner/create.rb new file mode 100644 index 00000000000..1cdee560ca8 --- /dev/null +++ b/app/graphql/mutations/ci/runner/create.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Mutations + module Ci + module Runner + class Create < BaseMutation + graphql_name 'RunnerCreate' + + authorize :create_runner + + include Mutations::Ci::Runner::CommonMutationArguments + + field :runner, + Types::Ci::RunnerType, + null: true, + description: 'Runner after mutation.' + + def resolve(**args) + if Feature.disabled?(:create_runner_workflow) + raise Gitlab::Graphql::Errors::ResourceNotAvailable, + '`create_runner_workflow` feature flag is disabled.' + end + + create_runner(args) + end + + private + + def create_runner(params) + response = { runner: nil, errors: [] } + result = ::Ci::Runners::CreateRunnerService.new(user: current_user, type: nil, params: params).execute + + if result.success? + response[:runner] = result.payload[:runner] + else + response[:errors] = result.errors + end + + response + end + end + end + end +end diff --git a/app/graphql/mutations/ci/runner/update.rb b/app/graphql/mutations/ci/runner/update.rb index 4f0bf19f09c..70f08e03553 100644 --- a/app/graphql/mutations/ci/runner/update.rb +++ b/app/graphql/mutations/ci/runner/update.rb @@ -8,54 +8,19 @@ module Mutations authorize :update_runner + include Mutations::Ci::Runner::CommonMutationArguments + RunnerID = ::Types::GlobalIDType[::Ci::Runner] argument :id, RunnerID, required: true, description: 'ID of the runner to update.' - argument :description, GraphQL::Types::String, - required: false, - description: 'Description of the runner.' - - argument :maintenance_note, GraphQL::Types::String, - required: false, - description: 'Runner\'s maintenance notes.' - - argument :maximum_timeout, GraphQL::Types::Int, - required: false, - description: 'Maximum timeout (in seconds) for jobs processed by the runner.' - - argument :access_level, ::Types::Ci::RunnerAccessLevelEnum, - required: false, - description: 'Access level of the runner.' - argument :active, GraphQL::Types::Boolean, required: false, description: 'Indicates the runner is allowed to receive jobs.', deprecated: { reason: :renamed, replacement: 'paused', milestone: '14.8' } - argument :paused, GraphQL::Types::Boolean, - required: false, - description: 'Indicates the runner is not allowed to receive jobs.' - - argument :locked, GraphQL::Types::Boolean, - required: false, - description: 'Indicates the runner is locked.' - - argument :run_untagged, GraphQL::Types::Boolean, - required: false, - description: 'Indicates the runner is able to run untagged jobs.' - - argument :tag_list, [GraphQL::Types::String], - required: false, - description: 'Tags associated with the runner.' - - argument :associated_projects, [::Types::GlobalIDType[::Project]], - required: false, - description: 'Projects associated with the runner. Available only for project runners.', - prepare: ->(global_ids, ctx) { global_ids&.filter_map { |gid| gid.model_id.to_i } } - field :runner, Types::Ci::RunnerType, null: true, diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index e48e9deae96..4b15b1f2ad8 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -139,6 +139,7 @@ module Types mount_mutation Mutations::Ci::JobArtifact::Destroy mount_mutation Mutations::Ci::JobTokenScope::AddProject mount_mutation Mutations::Ci::JobTokenScope::RemoveProject + mount_mutation Mutations::Ci::Runner::Create, alpha: { milestone: '15.10' } mount_mutation Mutations::Ci::Runner::Update mount_mutation Mutations::Ci::Runner::Delete mount_mutation Mutations::Ci::Runner::BulkDelete, alpha: { milestone: '15.3' } diff --git a/app/models/concerns/ci/has_status.rb b/app/models/concerns/ci/has_status.rb index 9a04776f1c6..c10a9221efb 100644 --- a/app/models/concerns/ci/has_status.rb +++ b/app/models/concerns/ci/has_status.rb @@ -13,7 +13,7 @@ module Ci STOPPED_STATUSES = COMPLETED_STATUSES + BLOCKED_STATUS ORDERED_STATUSES = %w[failed preparing pending running waiting_for_resource manual scheduled canceled success skipped created].freeze PASSED_WITH_WARNINGS_STATUSES = %w[failed canceled].to_set.freeze - EXCLUDE_IGNORED_STATUSES = %w[manual failed canceled].to_set.freeze + IGNORED_STATUSES = %w[manual].to_set.freeze ALIVE_STATUSES = (ACTIVE_STATUSES + ['created']).freeze CANCELABLE_STATUSES = (ALIVE_STATUSES + ['scheduled']).freeze STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3, diff --git a/app/models/namespaces/traversal/linear.rb b/app/models/namespaces/traversal/linear.rb index 0e9760832af..1aa59c4f1fe 100644 --- a/app/models/namespaces/traversal/linear.rb +++ b/app/models/namespaces/traversal/linear.rb @@ -235,8 +235,6 @@ module Namespaces # This is a temporary guard and will be removed. return if is_a?(Namespaces::ProjectNamespace) - return unless Feature.enabled?(:set_traversal_ids_on_save, root_ancestor) - self.transient_traversal_ids = if parent_id parent.traversal_ids + [id] else diff --git a/app/services/ci/runners/create_runner_service.rb b/app/services/ci/runners/create_runner_service.rb index 2de9ee4d38e..5906cdce99d 100644 --- a/app/services/ci/runners/create_runner_service.rb +++ b/app/services/ci/runners/create_runner_service.rb @@ -33,7 +33,7 @@ module Ci def normalize_params params[:registration_type] = :authenticated_user params[:runner_type] = type - params[:active] = !params.delete(:paused) if params[:paused].present? + params[:active] = !params.delete(:paused) if params.key?(:paused) params[:creator] = user strategy.normalize_params diff --git a/app/services/import/validate_remote_git_endpoint_service.rb b/app/services/import/validate_remote_git_endpoint_service.rb index 1b8fa45e979..2886bd5c9b7 100644 --- a/app/services/import/validate_remote_git_endpoint_service.rb +++ b/app/services/import/validate_remote_git_endpoint_service.rb @@ -21,7 +21,9 @@ module Import def execute uri = Gitlab::Utils.parse_url(@params[:url]) - return ServiceResponse.error(message: "#{@params[:url]} is not a valid URL") unless uri + if !uri || !uri.hostname || Project::VALID_IMPORT_PROTOCOLS.exclude?(uri.scheme) + return ServiceResponse.error(message: "#{@params[:url]} is not a valid URL") + end return ServiceResponse.success if uri.scheme == 'git' diff --git a/config/feature_flags/development/set_traversal_ids_on_save.yml b/config/feature_flags/development/set_traversal_ids_on_save.yml deleted file mode 100644 index ea07dafd9e4..00000000000 --- a/config/feature_flags/development/set_traversal_ids_on_save.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: set_traversal_ids_on_save -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/104328 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/383217 -milestone: '15.8' -type: development -group: group::organization -default_enabled: false diff --git a/danger/plugins/sidekiq_args.rb b/danger/plugins/sidekiq_args.rb new file mode 100644 index 00000000000..2210aa66b07 --- /dev/null +++ b/danger/plugins/sidekiq_args.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require_relative '../../tooling/danger/sidekiq_args' + +module Danger + class SidekiqArgs < ::Danger::Plugin + # Put the helper code somewhere it can be tested + include Tooling::Danger::SidekiqArgs + end +end diff --git a/danger/sidekiq_args/Dangerfile b/danger/sidekiq_args/Dangerfile new file mode 100644 index 00000000000..fdd582889f3 --- /dev/null +++ b/danger/sidekiq_args/Dangerfile @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +sidekiq_args.changed_worker_files.each do |filename| + sidekiq_args.add_comment_for_matched_line(filename) +end diff --git a/data/deprecations/templates/_deprecation_template.md.erb b/data/deprecations/templates/_deprecation_template.md.erb index 89e14c20050..42ab0435467 100644 --- a/data/deprecations/templates/_deprecation_template.md.erb +++ b/data/deprecations/templates/_deprecation_template.md.erb @@ -18,14 +18,14 @@ For deprecation authors (usually Product Managers and Engineering Managers): - To add a deprecation, use the example.yml file in `/data/deprecations/templates` as a template. - For more information about authoring deprecations, check the the deprecation item guidance: - https://about.gitlab.com/handbook/marketing/blog/release-posts/#creating-a-deprecation-entry + https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-and-removals-docs For deprecation reviewers (Technical Writers only): - To update the deprecation doc, run: `bin/rake gitlab:docs:compile_deprecations` - To verify the deprecations doc is up to date, run: `bin/rake gitlab:docs:check_deprecations` - For more information about updating the deprecation doc, see the deprecation doc update guidance: - https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-doc + https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-and-removals-docs --> {::options parse_block_html="true" /} diff --git a/doc/administration/lfs/index.md b/doc/administration/lfs/index.md index 3638729a6b6..4a80ab49524 100644 --- a/doc/administration/lfs/index.md +++ b/doc/administration/lfs/index.md @@ -421,6 +421,18 @@ To check an installed Git LFS client's version, run this command: git lfs version ``` +### `Connection refused` errors + +If you push or mirror LFS objects and receive errors like the following: + +- `dial tcp <IP>:443: connect: connection refused` +- `Connection refused - connect(2) for \"<target-or-proxy-IP>\" port 443` + +a firewall or proxy rule may be terminating the connection. + +If connection checks with standard Unix tools or manual Git pushes are successful, +the rule may be related to the size of the request. + ## Error viewing a PDF file When LFS has been configured with object storage and `proxy_download` set to diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a4696acdfb3..a86cd4a6975 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -4963,6 +4963,37 @@ Input type: `RepositionImageDiffNoteInput` | <a id="mutationrepositionimagediffnoteerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationrepositionimagediffnotenote"></a>`note` | [`Note`](#note) | Note after mutation. | +### `Mutation.runnerCreate` + +WARNING: +**Introduced** in 15.10. +This feature is in Alpha. It can be changed or removed at any time. + +Input type: `RunnerCreateInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationrunnercreateaccesslevel"></a>`accessLevel` | [`CiRunnerAccessLevel`](#cirunneraccesslevel) | Access level of the runner. | +| <a id="mutationrunnercreateassociatedprojects"></a>`associatedProjects` | [`[ProjectID!]`](#projectid) | Projects associated with the runner. Available only for project runners. | +| <a id="mutationrunnercreateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationrunnercreatedescription"></a>`description` | [`String`](#string) | Description of the runner. | +| <a id="mutationrunnercreatelocked"></a>`locked` | [`Boolean`](#boolean) | Indicates the runner is locked. | +| <a id="mutationrunnercreatemaintenancenote"></a>`maintenanceNote` | [`String`](#string) | Runner's maintenance notes. | +| <a id="mutationrunnercreatemaximumtimeout"></a>`maximumTimeout` | [`Int`](#int) | Maximum timeout (in seconds) for jobs processed by the runner. | +| <a id="mutationrunnercreatepaused"></a>`paused` | [`Boolean`](#boolean) | Indicates the runner is not allowed to receive jobs. | +| <a id="mutationrunnercreaterununtagged"></a>`runUntagged` | [`Boolean`](#boolean) | Indicates the runner is able to run untagged jobs. | +| <a id="mutationrunnercreatetaglist"></a>`tagList` | [`[String!]`](#string) | Tags associated with the runner. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="mutationrunnercreateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| <a id="mutationrunnercreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| <a id="mutationrunnercreaterunner"></a>`runner` | [`CiRunner`](#cirunner) | Runner after mutation. | + ### `Mutation.runnerDelete` Input type: `RunnerDeleteInput` diff --git a/doc/subscriptions/gitlab_com/index.md b/doc/subscriptions/gitlab_com/index.md index 3d6d16c21ee..ef1b6ae7f6f 100644 --- a/doc/subscriptions/gitlab_com/index.md +++ b/doc/subscriptions/gitlab_com/index.md @@ -237,7 +237,7 @@ amounts at which the alert displays. To change the namespace linked to a subscription: 1. Log in to the [Customers Portal](https://customers.gitlab.com/customers/sign_in) with a - [linked](../index.md#change-the-linked-account) GitLab.com account. + [linked](../index.md#link-a-gitlabcom-account) GitLab.com account. 1. Go to the **Manage Purchases** page. 1. Select **Change linked namespace**. 1. Select the desired group from the **This subscription is for** dropdown list. For a group to appear here, you must have the Owner role for that group. diff --git a/doc/subscriptions/index.md b/doc/subscriptions/index.md index a8241905379..f1a960a6aa9 100644 --- a/doc/subscriptions/index.md +++ b/doc/subscriptions/index.md @@ -73,6 +73,7 @@ With the [Customers Portal](https://customers.gitlab.com/) you can: - [Change account owner information](#change-account-owner-information) - [Change your company details](#change-your-company-details) - [Change your payment method](#change-your-payment-method) +- [Link a GitLab.com account](#link-a-gitlabcom-account) - [Change the linked account](#change-the-linked-account) - [Change the namespace the subscription is linked to](gitlab_com/index.md#change-the-linked-namespace) - [Change customers portal account password](#change-customers-portal-account-password) @@ -137,18 +138,26 @@ method as the default: 1. **Edit** the selected payment method and check the **Make default payment method** checkbox. 1. Select **Save Changes**. +### Link a GitLab.com account + +Follow this guideline if you have a legacy Customers Portal account and use an email and password to log in. + +To link a GitLab.com account to your Customers Portal account: + +1. Log in to the [Customers Portal](https://customers.gitlab.com/customers/sign_in?legacy=true) using email and password. +1. On the Customers Portal page, select **My account > Account details**. +1. Under **Your GitLab.com account**, select **Link account**. +1. Log in to the [GitLab.com](https://gitlab.com/users/sign_in) account you want to link to the Customers Portal account. + ### Change the linked account To change the GitLab.com account linked to your Customers Portal account: -1. Log in to the - [Customers Portal](https://customers.gitlab.com/customers/sign_in). -1. In a separate browser tab, go to [GitLab.com](https://gitlab.com/users/sign_in) and ensure you - are not logged in. +1. Log in to the [Customers Portal](https://customers.gitlab.com/customers/sign_in). +1. In a separate browser tab, go to [GitLab.com](https://gitlab.com/users/sign_in) and ensure you are not logged in. 1. On the Customers Portal page, select **My account > Account details**. 1. Under **Your GitLab.com account**, select **Change linked account**. -1. Log in to the [GitLab.com](https://gitlab.com/users/sign_in) account you want to link to the Customers Portal - account. +1. Log in to the [GitLab.com](https://gitlab.com/users/sign_in) account you want to link to the Customers Portal account. ### Change Customers Portal account password diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index 7293fc05410..188e0f7fe83 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -18,14 +18,14 @@ For deprecation authors (usually Product Managers and Engineering Managers): - To add a deprecation, use the example.yml file in `/data/deprecations/templates` as a template. - For more information about authoring deprecations, check the the deprecation item guidance: - https://about.gitlab.com/handbook/marketing/blog/release-posts/#creating-a-deprecation-entry + https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-and-removals-docs For deprecation reviewers (Technical Writers only): - To update the deprecation doc, run: `bin/rake gitlab:docs:compile_deprecations` - To verify the deprecations doc is up to date, run: `bin/rake gitlab:docs:check_deprecations` - For more information about updating the deprecation doc, see the deprecation doc update guidance: - https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-doc + https://about.gitlab.com/handbook/marketing/blog/release-posts/#update-the-deprecations-and-removals-docs --> {::options parse_block_html="true" /} diff --git a/lib/gitlab/auth/user_access_denied_reason.rb b/lib/gitlab/auth/user_access_denied_reason.rb index 322dfa74d09..3025960a8ab 100644 --- a/lib/gitlab/auth/user_access_denied_reason.rb +++ b/lib/gitlab/auth/user_access_denied_reason.rb @@ -29,7 +29,8 @@ module Gitlab "Your password expired. "\ "Please access GitLab from a web browser to update your password." else - "Your account has been blocked." + "Your request has been rejected for an unknown reason."\ + "Please contact your GitLab administrator and/or GitLab Support." end end diff --git a/lib/gitlab/ci/status/composite.rb b/lib/gitlab/ci/status/composite.rb index e854164d377..021dd8bcf9d 100644 --- a/lib/gitlab/ci/status/composite.rb +++ b/lib/gitlab/ci/status/composite.rb @@ -101,23 +101,22 @@ module Gitlab all_statuses .pluck(*columns) # rubocop: disable CodeReuse/ActiveRecord - .each(&method(:consume_status)) + .each do |status_attrs| + consume_status(Array.wrap(status_attrs)) + end end - def consume_status(description) - # convert `"status"` into `["status"]` - description = Array(description) - - status = - if success_with_warnings?(description) + def consume_status(status_attrs) + status_result = + if success_with_warnings?(status_attrs) :success_with_warnings - elsif ignored_status?(description) + elsif ignored_status?(status_attrs) :ignored else - description[@status_key].to_sym + status_attrs[@status_key].to_sym end - @status_set.add(status) + @status_set.add(status_result) end def success_with_warnings?(status) @@ -129,7 +128,7 @@ module Gitlab def ignored_status?(status) @allow_failure_key && status[@allow_failure_key] && - ::Ci::HasStatus::EXCLUDE_IGNORED_STATUSES.include?(status[@status_key]) + ::Ci::HasStatus::IGNORED_STATUSES.include?(status[@status_key]) end end end diff --git a/lib/gitlab/github_import/importer/repository_importer.rb b/lib/gitlab/github_import/importer/repository_importer.rb index d7fe01e90f8..2654812b64a 100644 --- a/lib/gitlab/github_import/importer/repository_importer.rb +++ b/lib/gitlab/github_import/importer/repository_importer.rb @@ -66,13 +66,10 @@ module Gitlab true rescue ::Gitlab::Git::CommandError => e - if e.message !~ /repository not exported/ - project.create_wiki + return true if e.message.include?('repository not exported') - raise e - else - true - end + project.create_wiki + raise e end def wiki_url @@ -89,10 +86,8 @@ module Gitlab client_repository[:default_branch] end - def client_repository - strong_memoize(:client_repository) do - client.repository(project.import_source) - end + strong_memoize_attr def client_repository + client.repository(project.import_source) end end end diff --git a/lib/gitlab/legacy_github_import/importer.rb b/lib/gitlab/legacy_github_import/importer.rb index 331eab7b62a..5eeea8f9548 100644 --- a/lib/gitlab/legacy_github_import/importer.rb +++ b/lib/gitlab/legacy_github_import/importer.rb @@ -22,7 +22,7 @@ module Gitlab unless credentials raise Projects::ImportService::Error, - "Unable to find project import data credentials for project ID: #{@project.id}" + "Unable to find project import data credentials for project ID: #{@project.id}" end opts = {} @@ -55,9 +55,7 @@ module Gitlab import_comments(:issues) # Gitea doesn't have an API endpoint for pull requests comments - unless project.gitea_import? - import_comments(:pull_requests) - end + import_comments(:pull_requests) unless project.gitea_import? import_wiki @@ -67,9 +65,7 @@ module Gitlab # See: # 1) https://gitlab.com/gitlab-org/gitlab/-/issues/343448#note_985979730 # 2) https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89694/diffs#dfc4a8141aa296465ea3c50b095a30292fb6ebc4_180_182 - unless project.gitea_import? - import_releases - end + import_releases unless project.gitea_import? handle_errors @@ -142,8 +138,8 @@ module Gitlab # rubocop: enable CodeReuse/ActiveRecord def import_pull_requests - fetch_resources(:pull_requests, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |pull_requests| - pull_requests.each do |raw| + fetch_resources(:pull_requests, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |prs| + prs.each do |raw| raw = raw.to_h gh_pull_request = PullRequestFormatter.new(project, raw, client) @@ -156,11 +152,13 @@ module Gitlab merge_request = gh_pull_request.create! # Gitea doesn't return PR in the Issue API endpoint, so labels must be assigned at this stage - if project.gitea_import? - apply_labels(merge_request, raw) - end + apply_labels(merge_request, raw) if project.gitea_import? rescue StandardError => e - errors << { type: :pull_request, url: Gitlab::UrlSanitizer.sanitize(gh_pull_request.url), errors: e.message } + errors << { + type: :pull_request, + url: Gitlab::UrlSanitizer.sanitize(gh_pull_request.url), + errors: e.message + } ensure clean_up_restored_branches(gh_pull_request) end @@ -196,9 +194,7 @@ module Gitlab return unless raw[:labels].count > 0 - label_ids = raw[:labels] - .map { |attrs| @labels[attrs[:name]] } - .compact + label_ids = raw[:labels].filter_map { |attrs| @labels[attrs[:name]] } issuable.update_attribute(:label_ids, label_ids) end @@ -208,10 +204,14 @@ module Gitlab resource_type = "#{issuable_type}_comments".to_sym # Two notes here: - # 1. We don't have a distinctive attribute for comments (unlike issues iid), so we fetch the last inserted note, - # compare it against every comment in the current imported page until we find match, and that's where start importing - # 2. GH returns comments for _both_ issues and PRs through issues_comments API, while pull_requests_comments returns - # only comments on diffs, so select last note not based on noteable_type but on line_code + # 1. We don't have a distinctive attribute for comments (unlike issues + # iid), so we fetch the last inserted note, compare it against every + # comment in the current imported page until we find match, and that's + # where start importing + # 2. GH returns comments for _both_ issues and PRs through + # issues_comments API, while pull_requests_comments returns only + # comments on diffs, so select last note not based on noteable_type but + # on line_code line_code_is = issuable_type == :pull_requests ? 'NOT NULL' : 'NULL' last_note = project.notes.where("line_code IS #{line_code_is}").last @@ -264,7 +264,8 @@ module Gitlab comment_attrs.with_indifferent_access == last_note_attrs end - # No matching resource in the collection, which means we got halted right on the end of the last page, so all good + # No matching resource in the collection, which means we got halted + # right on the end of the last page, so all good return unless cut_off_index # Otherwise, remove the resources we've already inserted @@ -280,9 +281,7 @@ module Gitlab # GitHub error message when the wiki repo has not been created, # this means that repo has wiki enabled, but have no pages. So, # we can skip the import. - if e.message !~ /repository not exported/ - errors << { type: :wiki, errors: e.message } - end + errors << { type: :wiki, errors: e.message } if e.message.exclude?('repository not exported') end def import_releases diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 3974c5f0170..f45663b41b0 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -410,7 +410,7 @@ module Gitlab end.data platform = ohai_data['platform'] - platform = 'raspbian' if ohai_data['platform'] == 'debian' && /armv/.match?(ohai_data['kernel']['machine']) + platform = 'raspbian' if ohai_data['platform'] == 'debian' && ohai_data['kernel']['machine']&.include?('armv') "#{platform}-#{ohai_data['platform_version']}" end diff --git a/lib/tasks/gitlab/terraform/migrate.rake b/lib/tasks/gitlab/terraform/migrate.rake index 99e33011cf5..1a85785c88e 100644 --- a/lib/tasks/gitlab/terraform/migrate.rake +++ b/lib/tasks/gitlab/terraform/migrate.rake @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'logger' - desc "GitLab | Terraform | Migrate Terraform states to remote storage" namespace :gitlab do namespace :terraform_states do diff --git a/lib/tasks/gitlab/tw/codeowners.rake b/lib/tasks/gitlab/tw/codeowners.rake index 0db77c54099..af016fa84e7 100644 --- a/lib/tasks/gitlab/tw/codeowners.rake +++ b/lib/tasks/gitlab/tw/codeowners.rake @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'yaml' - namespace :tw do desc 'Generates a list of codeowners for documentation pages.' task :codeowners do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index afbdddc3475..d9f62121193 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19373,6 +19373,9 @@ msgstr "" msgid "GlobalSearch|Recent merge requests" msgstr "" +msgid "GlobalSearch|Reset filters" +msgstr "" + msgid "GlobalSearch|Result count is over limit." msgstr "" diff --git a/rubocop/cop/gitlab/json.rb b/rubocop/cop/gitlab/json.rb index cf2ed0ba536..8b10850b894 100644 --- a/rubocop/cop/gitlab/json.rb +++ b/rubocop/cop/gitlab/json.rb @@ -6,9 +6,9 @@ module RuboCop class Json < RuboCop::Cop::Base extend RuboCop::Cop::AutoCorrector - MSG = <<~EOL + MSG = <<~TEXT Prefer `Gitlab::Json` over calling `JSON` directly. See https://docs.gitlab.com/ee/development/json.html - EOL + TEXT AVAILABLE_METHODS = %i[parse parse! load decode dump generate encode pretty_generate].to_set.freeze @@ -41,7 +41,7 @@ module RuboCop end def cbased(node) - return unless %r{/ee/}.match?(node.location.expression.source_buffer.name) + return unless node.location.expression.source_buffer.name.include?('/ee/') "::" end diff --git a/spec/frontend/approvals/mock_data.js b/spec/frontend/approvals/mock_data.js new file mode 100644 index 00000000000..e0e90c09791 --- /dev/null +++ b/spec/frontend/approvals/mock_data.js @@ -0,0 +1,10 @@ +import approvedByCurrentUser from 'test_fixtures/graphql/merge_requests/approvals/approvals.query.graphql.json'; + +export const createCanApproveResponse = () => { + const response = JSON.parse(JSON.stringify(approvedByCurrentUser)); + response.data.project.mergeRequest.userPermissions.canApprove = true; + response.data.project.mergeRequest.approved = false; + response.data.project.mergeRequest.approvedBy.nodes = []; + + return response; +}; diff --git a/spec/frontend/fixtures/merge_requests.rb b/spec/frontend/fixtures/merge_requests.rb index 7ee89ca3694..b6f6d149756 100644 --- a/spec/frontend/fixtures/merge_requests.rb +++ b/spec/frontend/fixtures/merge_requests.rb @@ -151,7 +151,7 @@ RSpec.describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: context 'merge request with no approvals' do base_input_path = 'vue_merge_request_widget/components/approvals/queries/' base_output_path = 'graphql/merge_requests/approvals/' - query_name = 'approved_by.query.graphql' + query_name = 'approvals.query.graphql' it "#{base_output_path}#{query_name}_no_approvals.json" do query = get_graphql_query_as_string("#{base_input_path}#{query_name}", ee: Gitlab.ee?) @@ -165,7 +165,7 @@ RSpec.describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: context 'merge request approved by current user' do base_input_path = 'vue_merge_request_widget/components/approvals/queries/' base_output_path = 'graphql/merge_requests/approvals/' - query_name = 'approved_by.query.graphql' + query_name = 'approvals.query.graphql' it "#{base_output_path}#{query_name}.json" do merge_request.approved_by_users << user @@ -181,7 +181,7 @@ RSpec.describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: context 'merge request approved by multiple users' do base_input_path = 'vue_merge_request_widget/components/approvals/queries/' base_output_path = 'graphql/merge_requests/approvals/' - query_name = 'approved_by.query.graphql' + query_name = 'approvals.query.graphql' it "#{base_output_path}#{query_name}_multiple_users.json" do merge_request.approved_by_users << user diff --git a/spec/frontend/search/sidebar/components/checkbox_filter_spec.js b/spec/frontend/search/sidebar/components/checkbox_filter_spec.js index 82017754b23..e2a3fdeeb25 100644 --- a/spec/frontend/search/sidebar/components/checkbox_filter_spec.js +++ b/spec/frontend/search/sidebar/components/checkbox_filter_spec.js @@ -17,8 +17,12 @@ describe('CheckboxFilter', () => { setQuery: jest.fn(), }; + const getterSpies = { + queryLangugageFilters: jest.fn(() => []), + }; + const defaultProps = { - filterData: convertFiltersData(MOCK_LANGUAGE_AGGREGATIONS_BUCKETS), + filtersData: convertFiltersData(MOCK_LANGUAGE_AGGREGATIONS_BUCKETS), }; const createComponent = () => { @@ -27,6 +31,7 @@ describe('CheckboxFilter', () => { query: MOCK_QUERY, }, actions: actionSpies, + getters: getterSpies, }); wrapper = shallowMountExtended(CheckboxFilter, { @@ -73,7 +78,7 @@ describe('CheckboxFilter', () => { describe('actions', () => { it('triggers setQuery', () => { const filter = - defaultProps.filterData.filters[Object.keys(defaultProps.filterData.filters)[0]].value; + defaultProps.filtersData.filters[Object.keys(defaultProps.filtersData.filters)[0]].value; findFormCheckboxGroup().vm.$emit('input', filter); expect(actionSpies.setQuery).toHaveBeenCalledWith(expect.any(Object), { diff --git a/spec/frontend/search/sidebar/components/language_filters_spec.js b/spec/frontend/search/sidebar/components/language_filter_spec.js index e297d1c33b0..6870e759110 100644 --- a/spec/frontend/search/sidebar/components/language_filters_spec.js +++ b/spec/frontend/search/sidebar/components/language_filter_spec.js @@ -23,6 +23,7 @@ describe('GlobalSearchSidebarLanguageFilter', () => { const getterSpies = { langugageAggregationBuckets: jest.fn(() => MOCK_LANGUAGE_AGGREGATIONS_BUCKETS), + queryLangugageFilters: jest.fn(() => []), }; const createComponent = (initialState) => { @@ -48,6 +49,7 @@ describe('GlobalSearchSidebarLanguageFilter', () => { const findForm = () => wrapper.findComponent(GlForm); const findCheckboxFilter = () => wrapper.findComponent(CheckboxFilter); const findApplyButton = () => wrapper.findByTestId('apply-button'); + const findResetButton = () => wrapper.findByTestId('reset-button'); const findShowMoreButton = () => wrapper.findByTestId('show-more-button'); const findAlert = () => wrapper.findComponent(GlAlert); const findAllCheckboxes = () => wrapper.findAllComponents(GlFormCheckbox); @@ -84,6 +86,25 @@ describe('GlobalSearchSidebarLanguageFilter', () => { }); }); + describe('resetButton', () => { + describe.each` + description | sidebarDirty | queryFilters | isDisabled + ${'sidebar dirty only'} | ${true} | ${[]} | ${undefined} + ${'query filters only'} | ${false} | ${['JSON', 'C']} | ${undefined} + ${'sidebar dirty and query filters'} | ${true} | ${['JSON', 'C']} | ${undefined} + ${'no sidebar and no query filters'} | ${false} | ${[]} | ${'true'} + `('$description', ({ sidebarDirty, queryFilters, isDisabled }) => { + beforeEach(() => { + getterSpies.queryLangugageFilters = jest.fn(() => queryFilters); + createComponent({ sidebarDirty, query: { ...MOCK_QUERY, language: queryFilters } }); + }); + + it(`button is ${isDisabled ? 'enabled' : 'disabled'}`, () => { + expect(findResetButton().attributes('disabled')).toBe(isDisabled); + }); + }); + }); + describe('ApplyButton', () => { describe('when sidebarDirty is false', () => { beforeEach(() => { diff --git a/spec/frontend/search/store/actions_spec.js b/spec/frontend/search/store/actions_spec.js index 2f87802dfe6..0f270ff2491 100644 --- a/spec/frontend/search/store/actions_spec.js +++ b/spec/frontend/search/store/actions_spec.js @@ -325,4 +325,26 @@ describe('Global Search Store Actions', () => { }); }); }); + + describe('resetLanguageQueryWithRedirect', () => { + it('calls visitUrl and setParams with the state.query', () => { + return testAction(actions.resetLanguageQueryWithRedirect, null, state, [], [], () => { + expect(urlUtils.setUrlParams).toHaveBeenCalledWith({ ...state.query, page: null }); + expect(urlUtils.visitUrl).toHaveBeenCalled(); + }); + }); + }); + + describe('resetLanguageQuery', () => { + it('calls commit SET_QUERY with value []', () => { + state = { ...state, query: { ...state.query, language: ['YAML', 'Text', 'Markdown'] } }; + return testAction( + actions.resetLanguageQuery, + null, + state, + [{ type: types.SET_QUERY, payload: { key: 'language', value: [] } }], + [], + ); + }); + }); }); diff --git a/spec/frontend/search/store/getters_spec.js b/spec/frontend/search/store/getters_spec.js index 818902ee720..b70a6be8d46 100644 --- a/spec/frontend/search/store/getters_spec.js +++ b/spec/frontend/search/store/getters_spec.js @@ -1,12 +1,14 @@ import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY } from '~/search/store/constants'; import * as getters from '~/search/store/getters'; import createState from '~/search/store/state'; +import { useMockLocationHelper } from 'helpers/mock_window_location_helper'; import { MOCK_QUERY, MOCK_GROUPS, MOCK_PROJECTS, MOCK_AGGREGATIONS, MOCK_LANGUAGE_AGGREGATIONS_BUCKETS, + TEST_FILTER_DATA, } from '../mock_data'; describe('Global Search Store Getters', () => { @@ -14,37 +16,48 @@ describe('Global Search Store Getters', () => { beforeEach(() => { state = createState({ query: MOCK_QUERY }); + useMockLocationHelper(); }); describe('frequentGroups', () => { - beforeEach(() => { - state.frequentItems[GROUPS_LOCAL_STORAGE_KEY] = MOCK_GROUPS; - }); - it('returns the correct data', () => { + state.frequentItems[GROUPS_LOCAL_STORAGE_KEY] = MOCK_GROUPS; expect(getters.frequentGroups(state)).toStrictEqual(MOCK_GROUPS); }); }); describe('frequentProjects', () => { - beforeEach(() => { - state.frequentItems[PROJECTS_LOCAL_STORAGE_KEY] = MOCK_PROJECTS; - }); - it('returns the correct data', () => { + state.frequentItems[PROJECTS_LOCAL_STORAGE_KEY] = MOCK_PROJECTS; expect(getters.frequentProjects(state)).toStrictEqual(MOCK_PROJECTS); }); }); describe('langugageAggregationBuckets', () => { - beforeEach(() => { - state.aggregations.data = MOCK_AGGREGATIONS; - }); - it('returns the correct data', () => { + state.aggregations.data = MOCK_AGGREGATIONS; expect(getters.langugageAggregationBuckets(state)).toStrictEqual( MOCK_LANGUAGE_AGGREGATIONS_BUCKETS, ); }); }); + + describe('queryLangugageFilters', () => { + it('returns the correct data', () => { + state.query.language = Object.keys(TEST_FILTER_DATA.filters); + expect(getters.queryLangugageFilters(state)).toStrictEqual(state.query.language); + }); + }); + + describe('currentUrlQueryHasLanguageFilters', () => { + it.each` + description | lang | result + ${'has valid language'} | ${{ language: ['a', 'b'] }} | ${true} + ${'has empty lang'} | ${{ language: [] }} | ${false} + ${'has no lang'} | ${{}} | ${false} + `('$description', ({ lang, result }) => { + state.urlQuery = lang; + expect(getters.currentUrlQueryHasLanguageFilters(state)).toBe(result); + }); + }); }); diff --git a/spec/frontend/search/store/utils_spec.js b/spec/frontend/search/store/utils_spec.js index 487ed7bfe03..8c4a17f0a5d 100644 --- a/spec/frontend/search/store/utils_spec.js +++ b/spec/frontend/search/store/utils_spec.js @@ -226,11 +226,14 @@ describe('Global Search Store Utils', () => { }); describe.each` - description | currentQuery | urlQuery | isDirty - ${'identical'} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default' }} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default' }} | ${false} - ${'different'} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'new' }} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default' }} | ${true} - ${'null/undefined'} | ${{ [SIDEBAR_PARAMS[0]]: null, [SIDEBAR_PARAMS[1]]: null }} | ${{ [SIDEBAR_PARAMS[0]]: undefined, [SIDEBAR_PARAMS[1]]: undefined }} | ${false} - ${'updated/undefined'} | ${{ [SIDEBAR_PARAMS[0]]: 'new', [SIDEBAR_PARAMS[1]]: 'new' }} | ${{ [SIDEBAR_PARAMS[0]]: undefined, [SIDEBAR_PARAMS[1]]: undefined }} | ${true} + description | currentQuery | urlQuery | isDirty + ${'identical'} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default', [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default', [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${false} + ${'different'} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'new', [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default', [SIDEBAR_PARAMS[2]]: ['a', 'c'] }} | ${true} + ${'null/undefined'} | ${{ [SIDEBAR_PARAMS[0]]: null, [SIDEBAR_PARAMS[1]]: null, [SIDEBAR_PARAMS[2]]: null }} | ${{ [SIDEBAR_PARAMS[0]]: undefined, [SIDEBAR_PARAMS[1]]: undefined, [SIDEBAR_PARAMS[2]]: undefined }} | ${false} + ${'updated/undefined'} | ${{ [SIDEBAR_PARAMS[0]]: 'new', [SIDEBAR_PARAMS[1]]: 'new', [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${{ [SIDEBAR_PARAMS[0]]: undefined, [SIDEBAR_PARAMS[1]]: undefined, [SIDEBAR_PARAMS[2]]: [] }} | ${true} + ${'language only no url params'} | ${{ [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${{ [SIDEBAR_PARAMS[2]]: undefined }} | ${true} + ${'language only url params symetric'} | ${{ [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${{ [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${false} + ${'language only url params asymetric'} | ${{ [SIDEBAR_PARAMS[2]]: ['a'] }} | ${{ [SIDEBAR_PARAMS[2]]: ['a', 'b'] }} | ${true} `('isSidebarDirty', ({ description, currentQuery, urlQuery, isDirty }) => { describe(`with ${description} sidebar query data`, () => { let res; diff --git a/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js b/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js index bf208f16d18..80a7565cbee 100644 --- a/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/approvals/approvals_spec.js @@ -1,17 +1,24 @@ -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; import { GlButton, GlSprintf } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; +import approvedByCurrentUser from 'test_fixtures/graphql/merge_requests/approvals/approvals.query.graphql.json'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { createAlert } from '~/flash'; import Approvals from '~/vue_merge_request_widget/components/approvals/approvals.vue'; import ApprovalsSummary from '~/vue_merge_request_widget/components/approvals/approvals_summary.vue'; import ApprovalsSummaryOptional from '~/vue_merge_request_widget/components/approvals/approvals_summary_optional.vue'; import { - FETCH_LOADING, - FETCH_ERROR, APPROVE_ERROR, UNAPPROVE_ERROR, } from '~/vue_merge_request_widget/components/approvals/messages'; import eventHub from '~/vue_merge_request_widget/event_hub'; +import approvedByQuery from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approvals.query.graphql'; +import { createCanApproveResponse } from 'jest/approvals/mock_data'; + +Vue.use(VueApollo); const mockAlertDismiss = jest.fn(); jest.mock('~/flash', () => ({ @@ -20,7 +27,6 @@ jest.mock('~/flash', () => ({ })), })); -const RULE_NAME = 'first_rule'; const TEST_HELP_PATH = 'help/path'; const testApprovedBy = () => [1, 7, 10].map((id) => ({ id })); const testApprovals = () => ({ @@ -34,15 +40,18 @@ const testApprovals = () => ({ require_password_to_approve: false, invalid_approvers_rules: [], }); -const testApprovalRulesResponse = () => ({ rules: [{ id: 2 }] }); describe('MRWidget approvals', () => { let wrapper; let service; let mr; - const createComponent = (props = {}) => { + const createComponent = (props = {}, response = approvedByCurrentUser) => { + const requestHandlers = [[approvedByQuery, jest.fn().mockResolvedValue(response)]]; + const apolloProvider = createMockApollo(requestHandlers); + wrapper = shallowMount(Approvals, { + apolloProvider, propsData: { mr, service, @@ -68,15 +77,10 @@ describe('MRWidget approvals', () => { }; const findSummary = () => wrapper.findComponent(ApprovalsSummary); const findOptionalSummary = () => wrapper.findComponent(ApprovalsSummaryOptional); - const findInvalidRules = () => wrapper.find('[data-testid="invalid-rules"]'); beforeEach(() => { service = { ...{ - fetchApprovals: jest.fn().mockReturnValue(Promise.resolve(testApprovals())), - fetchApprovalSettings: jest - .fn() - .mockReturnValue(Promise.resolve(testApprovalRulesResponse())), approveMergeRequest: jest.fn().mockReturnValue(Promise.resolve(testApprovals())), unapproveMergeRequest: jest.fn().mockReturnValue(Promise.resolve(testApprovals())), approveMergeRequestWithAuth: jest.fn().mockReturnValue(Promise.resolve(testApprovals())), @@ -97,55 +101,21 @@ describe('MRWidget approvals', () => { }; jest.spyOn(eventHub, '$emit').mockImplementation(() => {}); - }); - afterEach(() => { - wrapper.destroy(); - wrapper = null; - }); - - describe('when created', () => { - it('shows loading message', async () => { - service = { - fetchApprovals: jest.fn().mockReturnValue(new Promise(() => {})), - }; - - createComponent(); - await nextTick(); - expect(wrapper.text()).toContain(FETCH_LOADING); - }); - - it('fetches approvals', () => { - createComponent(); - expect(service.fetchApprovals).toHaveBeenCalled(); - }); - }); - - describe('when fetch approvals error', () => { - beforeEach(() => { - jest.spyOn(service, 'fetchApprovals').mockReturnValue(Promise.reject()); - createComponent(); - return nextTick(); - }); - - it('still shows loading message', () => { - expect(wrapper.text()).toContain(FETCH_LOADING); - }); - - it('flashes error', () => { - expect(createAlert).toHaveBeenCalledWith({ message: FETCH_ERROR }); - }); + gon.current_user_id = getIdFromGraphQLId( + approvedByCurrentUser.data.project.mergeRequest.approvedBy.nodes[0].id, + ); }); describe('action button', () => { describe('when mr is closed', () => { - beforeEach(() => { + beforeEach(async () => { + const response = createCanApproveResponse(); + mr.isOpen = false; - mr.approvals.user_has_approved = false; - mr.approvals.user_can_approve = true; - createComponent(); - return nextTick(); + createComponent({}, response); + await waitForPromises(); }); it('action is not rendered', () => { @@ -154,12 +124,12 @@ describe('MRWidget approvals', () => { }); describe('when user cannot approve', () => { - beforeEach(() => { - mr.approvals.user_has_approved = false; - mr.approvals.user_can_approve = false; + beforeEach(async () => { + const response = JSON.parse(JSON.stringify(approvedByCurrentUser)); + response.data.project.mergeRequest.approvedBy.nodes = []; - createComponent(); - return nextTick(); + createComponent({}, response); + await waitForPromises(); }); it('action is not rendered', () => { @@ -168,15 +138,16 @@ describe('MRWidget approvals', () => { }); describe('when user can approve', () => { + let canApproveResponse; + beforeEach(() => { - mr.approvals.user_has_approved = false; - mr.approvals.user_can_approve = true; + canApproveResponse = createCanApproveResponse(); }); describe('and MR is unapproved', () => { - beforeEach(() => { - createComponent(); - return nextTick(); + beforeEach(async () => { + createComponent({}, canApproveResponse); + await waitForPromises(); }); it('approve action is rendered', () => { @@ -190,30 +161,33 @@ describe('MRWidget approvals', () => { describe('and MR is approved', () => { beforeEach(() => { - mr.approvals.approved = true; + canApproveResponse.data.project.mergeRequest.approved = true; }); describe('with no approvers', () => { - beforeEach(() => { - mr.approvals.approved_by = []; - createComponent(); - return nextTick(); + beforeEach(async () => { + canApproveResponse.data.project.mergeRequest.approvedBy.nodes = []; + createComponent({}, canApproveResponse); + await nextTick(); }); - it('approve action (with inverted style) is rendered', () => { - expect(findActionData()).toEqual({ + it('approve action is rendered', () => { + expect(findActionData()).toMatchObject({ variant: 'confirm', text: 'Approve', - category: 'secondary', }); }); }); describe('with approvers', () => { - beforeEach(() => { - mr.approvals.approved_by = [{ user: { id: 7 } }]; - createComponent(); - return nextTick(); + beforeEach(async () => { + canApproveResponse.data.project.mergeRequest.approvedBy.nodes = + approvedByCurrentUser.data.project.mergeRequest.approvedBy.nodes; + + canApproveResponse.data.project.mergeRequest.approvedBy.nodes[0].id = 2; + + createComponent({}, canApproveResponse); + await waitForPromises(); }); it('approve additionally action is rendered', () => { @@ -227,9 +201,9 @@ describe('MRWidget approvals', () => { }); describe('when approve action is clicked', () => { - beforeEach(() => { - createComponent(); - return nextTick(); + beforeEach(async () => { + createComponent({}, canApproveResponse); + await waitForPromises(); }); it('shows loading icon', () => { @@ -258,10 +232,6 @@ describe('MRWidget approvals', () => { it('emits to eventHub', () => { expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested'); }); - - it('calls store setApprovals', () => { - expect(mr.setApprovals).toHaveBeenCalledWith(testApprovals()); - }); }); describe('and error', () => { @@ -286,12 +256,12 @@ describe('MRWidget approvals', () => { }); describe('when user has approved', () => { - beforeEach(() => { - mr.approvals.user_has_approved = true; - mr.approvals.user_can_approve = false; + beforeEach(async () => { + const response = JSON.parse(JSON.stringify(approvedByCurrentUser)); - createComponent(); - return nextTick(); + createComponent({}, response); + + await waitForPromises(); }); it('revoke action is rendered', () => { @@ -316,10 +286,6 @@ describe('MRWidget approvals', () => { it('emits to eventHub', () => { expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested'); }); - - it('calls store setApprovals', () => { - expect(mr.setApprovals).toHaveBeenCalledWith(testApprovals()); - }); }); describe('and error', () => { @@ -338,19 +304,24 @@ describe('MRWidget approvals', () => { }); describe('approvals optional summary', () => { + let optionalApprovalsResponse; + + beforeEach(() => { + optionalApprovalsResponse = JSON.parse(JSON.stringify(approvedByCurrentUser)); + }); + describe('when no approvals required and no approvers', () => { beforeEach(() => { - mr.approvals.approved_by = []; - mr.approvals.approvals_required = 0; - mr.approvals.user_has_approved = false; + optionalApprovalsResponse.data.project.mergeRequest.approvedBy.nodes = []; + optionalApprovalsResponse.data.project.mergeRequest.approvalsRequired = 0; }); describe('and can approve', () => { - beforeEach(() => { - mr.approvals.user_can_approve = true; + beforeEach(async () => { + optionalApprovalsResponse.data.project.mergeRequest.userPermissions.canApprove = true; - createComponent(); - return nextTick(); + createComponent({}, optionalApprovalsResponse); + await waitForPromises(); }); it('is shown', () => { @@ -363,11 +334,9 @@ describe('MRWidget approvals', () => { }); describe('and cannot approve', () => { - beforeEach(() => { - mr.approvals.user_can_approve = false; - - createComponent(); - return nextTick(); + beforeEach(async () => { + createComponent({}, optionalApprovalsResponse); + await nextTick(); }); it('is shown', () => { @@ -382,9 +351,9 @@ describe('MRWidget approvals', () => { }); describe('approvals summary', () => { - beforeEach(() => { + beforeEach(async () => { createComponent(); - return nextTick(); + await nextTick(); }); it('is rendered with props', () => { @@ -393,41 +362,7 @@ describe('MRWidget approvals', () => { expect(findOptionalSummary().exists()).toBe(false); expect(summary.exists()).toBe(true); expect(summary.props()).toMatchObject({ - projectPath: 'gitlab-org/gitlab', - iid: '1', - updatedCount: 0, - }); - }); - }); - - describe('invalid rules', () => { - beforeEach(() => { - mr.approvals.merge_request_approvers_available = true; - createComponent(); - }); - - it('does not render related components', () => { - expect(findInvalidRules().exists()).toBe(false); - }); - - describe('when invalid rules are present', () => { - beforeEach(() => { - mr.approvals.invalid_approvers_rules = [{ name: RULE_NAME }]; - createComponent(); - }); - - it('renders related components', () => { - const invalidRules = findInvalidRules(); - - expect(invalidRules.exists()).toBe(true); - - const invalidRulesText = invalidRules.text(); - - expect(invalidRulesText).toContain(RULE_NAME); - expect(invalidRulesText).toContain( - 'GitLab has approved this rule automatically to unblock the merge request.', - ); - expect(invalidRulesText).toContain('Learn more.'); + approvalState: approvedByCurrentUser.data.project.mergeRequest, }); }); }); diff --git a/spec/frontend/vue_merge_request_widget/components/approvals/approvals_summary_spec.js b/spec/frontend/vue_merge_request_widget/components/approvals/approvals_summary_spec.js index e75ce7c60c9..62eddebd9c1 100644 --- a/spec/frontend/vue_merge_request_widget/components/approvals/approvals_summary_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/approvals/approvals_summary_spec.js @@ -1,11 +1,10 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { mount } from '@vue/test-utils'; -import approvedByMultipleUsers from 'test_fixtures/graphql/merge_requests/approvals/approved_by.query.graphql_multiple_users.json'; -import noApprovalsResponse from 'test_fixtures/graphql/merge_requests/approvals/approved_by.query.graphql_no_approvals.json'; -import approvedByCurrentUser from 'test_fixtures/graphql/merge_requests/approvals/approved_by.query.graphql.json'; +import approvedByMultipleUsers from 'test_fixtures/graphql/merge_requests/approvals/approvals.query.graphql_multiple_users.json'; +import noApprovalsResponse from 'test_fixtures/graphql/merge_requests/approvals/approvals.query.graphql_no_approvals.json'; +import approvedByCurrentUser from 'test_fixtures/graphql/merge_requests/approvals/approvals.query.graphql.json'; import waitForPromises from 'helpers/wait_for_promises'; -import createMockApollo from 'helpers/mock_apollo_helper'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import ApprovalsSummary from '~/vue_merge_request_widget/components/approvals/approvals_summary.vue'; import { @@ -14,7 +13,6 @@ import { APPROVED_BY_YOU_AND_OTHERS, } from '~/vue_merge_request_widget/components/approvals/messages'; import UserAvatarList from '~/vue_shared/components/user_avatar/user_avatar_list.vue'; -import approvedByQuery from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approved_by.query.graphql'; Vue.use(VueApollo); @@ -22,13 +20,11 @@ describe('MRWidget approvals summary', () => { const originalUserId = gon.current_user_id; let wrapper; - const createComponent = (response = approvedByCurrentUser) => { + const createComponent = (data = approvedByCurrentUser) => { wrapper = mount(ApprovalsSummary, { propsData: { - projectPath: 'gitlab-org/gitlab', - iid: '1', + approvalState: data.data.project.mergeRequest, }, - apolloProvider: createMockApollo([[approvedByQuery, jest.fn().mockResolvedValue(response)]]), }); }; diff --git a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js index f37276ad594..608d513e64b 100644 --- a/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js +++ b/spec/frontend/vue_merge_request_widget/mr_widget_options_spec.js @@ -6,6 +6,7 @@ import VueApollo from 'vue-apollo'; import * as Sentry from '@sentry/browser'; import getStateQueryResponse from 'test_fixtures/graphql/merge_requests/get_state.query.graphql.json'; import readyToMergeResponse from 'test_fixtures/graphql/merge_requests/states/ready_to_merge.query.graphql.json'; +import approvedByCurrentUser from 'test_fixtures/graphql/merge_requests/approvals/approvals.query.graphql.json'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { securityReportMergeRequestDownloadPathsQueryResponse } from 'jest/vue_shared/security_reports/mock_data'; @@ -28,6 +29,7 @@ import StatusIcon from '~/vue_merge_request_widget/components/extensions/status_ import securityReportMergeRequestDownloadPathsQuery from '~/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql'; import getStateQuery from '~/vue_merge_request_widget/queries/get_state.query.graphql'; import readyToMergeQuery from 'ee_else_ce/vue_merge_request_widget/queries/states/ready_to_merge.query.graphql'; +import approvalsQuery from 'ee_else_ce/vue_merge_request_widget/components/approvals/queries/approvals.query.graphql'; import userPermissionsQuery from '~/vue_merge_request_widget/queries/permissions.query.graphql'; import conflictsStateQuery from '~/vue_merge_request_widget/queries/states/conflicts.query.graphql'; import { faviconDataUrl, overlayDataUrl } from '../lib/utils/mock_data'; @@ -100,6 +102,7 @@ describe('MrWidgetOptions', () => { ...options, apolloProvider: createMockApollo([ + [approvalsQuery, jest.fn().mockResolvedValue(approvedByCurrentUser)], [ getStateQuery, jest.fn().mockResolvedValue({ diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index cd66b93eb8b..bb38f4b1bca 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe Gitlab::LegacyGithubImport::Importer do +RSpec.describe Gitlab::LegacyGithubImport::Importer, feature_category: :importers do + subject(:importer) { described_class.new(project) } + shared_examples 'Gitlab::LegacyGithubImport::Importer#execute' do let(:expected_not_called) { [] } @@ -11,8 +13,6 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do end it 'calls import methods' do - importer = described_class.new(project) - expected_called = [ :import_labels, :import_milestones, :import_pull_requests, :import_issues, :import_wiki, :import_releases, :handle_errors, @@ -51,11 +51,13 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do allow_any_instance_of(Octokit::Client).to receive(:labels).and_return([label1, label2]) allow_any_instance_of(Octokit::Client).to receive(:milestones).and_return([milestone, milestone]) allow_any_instance_of(Octokit::Client).to receive(:issues).and_return([issue1, issue2]) - allow_any_instance_of(Octokit::Client).to receive(:pull_requests).and_return([pull_request, pull_request]) + allow_any_instance_of(Octokit::Client).to receive(:pull_requests).and_return([pull_request, pull_request_missing_source_branch]) allow_any_instance_of(Octokit::Client).to receive(:issues_comments).and_raise(Octokit::NotFound) allow_any_instance_of(Octokit::Client).to receive(:pull_requests_comments).and_return([]) allow_any_instance_of(Octokit::Client).to receive(:last_response).and_return(double(rels: { next: nil })) allow_any_instance_of(Octokit::Client).to receive(:releases).and_return([release1, release2]) + + allow(importer).to receive(:restore_source_branch).and_raise(StandardError, 'Some error') end let(:label1) do @@ -153,8 +155,6 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do } end - subject { described_class.new(project) } - it 'returns true' do expect(subject.execute).to eq true end @@ -163,18 +163,19 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do expect { subject.execute }.not_to raise_error end - it 'stores error messages' do + it 'stores error messages', :unlimited_max_formatted_output_length do error = { message: 'The remote data could not be fully imported.', errors: [ { type: :label, url: "#{api_root}/repos/octocat/Hello-World/labels/bug", errors: "Validation failed: Title can't be blank, Title is invalid" }, + { type: :pull_request, url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", errors: 'Some error' }, { type: :issue, url: "#{api_root}/repos/octocat/Hello-World/issues/1348", errors: "Validation failed: Title can't be blank" }, { type: :issues_comments, errors: 'Octokit::NotFound' }, { type: :wiki, errors: "Gitlab::Git::CommandError" } ] } - described_class.new(project).execute + importer.execute expect(project.import_state.last_error).to eq error.to_json end @@ -182,8 +183,6 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do shared_examples 'Gitlab::LegacyGithubImport unit-testing' do describe '#clean_up_restored_branches' do - subject { described_class.new(project) } - before do allow(gh_pull_request).to receive(:source_branch_exists?).at_least(:once) { false } allow(gh_pull_request).to receive(:target_branch_exists?).at_least(:once) { false } @@ -240,6 +239,16 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do } end + let(:pull_request_missing_source_branch) do + pull_request.merge( + head: { + ref: 'missing', + repo: repository, + sha: RepoHelpers.another_sample_commit + } + ) + end + let(:closed_pull_request) do { number: 1347, @@ -264,8 +273,6 @@ RSpec.describe Gitlab::LegacyGithubImport::Importer do let(:api_root) { 'https://try.gitea.io/api/v1' } let(:repo_root) { 'https://try.gitea.io' } - subject { described_class.new(project) } - before do project.update!(import_type: 'gitea', import_url: "#{repo_root}/foo/group/project.git") end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 6c991f14156..4e3d8f633b7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -571,17 +571,6 @@ RSpec.describe Namespace, feature_category: :subgroups do it { expect(child.traversal_ids).to eq [parent.id, child.id] } it { expect(parent.sync_events.count).to eq 1 } it { expect(child.sync_events.count).to eq 1 } - - context 'when set_traversal_ids_on_save feature flag is disabled' do - before do - stub_feature_flags(set_traversal_ids_on_save: false) - end - - it 'only sets traversal_ids on reload' do - expect { parent.reload }.to change(parent, :traversal_ids).from([]).to([parent.id]) - expect { child.reload }.to change(child, :traversal_ids).from([]).to([parent.id, child.id]) - end - end end context 'traversal_ids on update' do @@ -594,18 +583,6 @@ RSpec.describe Namespace, feature_category: :subgroups do it 'sets the traversal_ids attribute' do expect { subject }.to change { namespace1.traversal_ids }.from([namespace1.id]).to([namespace2.id, namespace1.id]) end - - context 'when set_traversal_ids_on_save feature flag is disabled' do - before do - stub_feature_flags(set_traversal_ids_on_save: false) - end - - it 'sets traversal_ids after reload' do - subject - - expect { namespace1.reload }.to change(namespace1, :traversal_ids).from([]).to([namespace2.id, namespace1.id]) - end - end end it 'creates a Namespaces::SyncEvent using triggers' do diff --git a/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb b/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb new file mode 100644 index 00000000000..bd22cc6706a --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'RunnerCreate', feature_category: :runner_fleet do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + + let(:mutation_params) do + { + description: 'create description', + maintenance_note: 'create maintenance note', + maximum_timeout: 900, + access_level: 'REF_PROTECTED', + paused: true, + run_untagged: false, + tag_list: %w[tag1 tag2] + } + end + + let(:mutation) do + variables = { + **mutation_params + } + + graphql_mutation( + :runner_create, + variables, + <<-QL + runner { + ephemeralAuthenticationToken + + runnerType + description + maintenanceNote + paused + tagList + accessLevel + locked + maximumTimeout + runUntagged + } + errors + QL + ) + end + + let(:mutation_response) { graphql_mutation_response(:runner_create) } + + context 'when user does not have permissions' do + let(:current_user) { user } + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(mutation_response['errors']).to contain_exactly "Insufficient permissions" + end + end + + context 'when user has permissions', :enable_admin_mode do + let(:current_user) { admin } + + context 'when :create_runner_workflow feature flag is disabled' do + before do + stub_feature_flags(create_runner_workflow: false) + end + + it 'returns an error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(graphql_errors).not_to be_empty + expect(graphql_errors[0]['message']) + .to eq("`create_runner_workflow` feature flag is disabled.") + end + end + + context 'when success' do + it do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + + mutation_params.each_key do |key| + expect(mutation_response['runner'][key.to_s.camelize(:lower)]).to eq mutation_params[key] + end + + expect(mutation_response['runner']['ephemeralAuthenticationToken']).to start_with 'glrt' + + expect(mutation_response['errors']).to eq([]) + end + end + + context 'when failure' do + let(:mutation_params) do + { + description: "", + maintenanceNote: "", + paused: true, + accessLevel: "NOT_PROTECTED", + runUntagged: false, + tagList: + [], + maximumTimeout: 1 + } + end + + it do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + + expect(mutation_response['errors']).to contain_exactly( + "Tags list can not be empty when runner is not allowed to pick untagged jobs", + "Maximum timeout needs to be at least 10 minutes" + ) + end + end + end +end diff --git a/spec/services/ci/runners/create_runner_service_spec.rb b/spec/services/ci/runners/create_runner_service_spec.rb index 673bf3ef90e..52acfcbb7af 100644 --- a/spec/services/ci/runners/create_runner_service_spec.rb +++ b/spec/services/ci/runners/create_runner_service_spec.rb @@ -83,6 +83,49 @@ RSpec.describe ::Ci::Runners::CreateRunnerService, "#execute", feature_category: expect(runner.authenticated_user_registration_type?).to be_truthy expect(runner.runner_type).to eq 'instance_type' end + + context 'with a nil paused value' do + let(:args) do + { + paused: nil, + description: 'some description', + maintenance_note: 'a note', + tag_list: %w[tag1 tag2], + access_level: 'ref_protected', + locked: true, + maximum_timeout: 600, + run_untagged: false + } + end + + it { is_expected.to be_success } + + it 'creates runner with active set to true' do + expect(runner).to be_an_instance_of(::Ci::Runner) + expect(runner.active).to eq true + end + end + + context 'with no paused value given' do + let(:args) do + { + description: 'some description', + maintenance_note: 'a note', + tag_list: %w[tag1 tag2], + access_level: 'ref_protected', + locked: true, + maximum_timeout: 600, + run_untagged: false + } + end + + it { is_expected.to be_success } + + it 'creates runner with active set to true' do + expect(runner).to be_an_instance_of(::Ci::Runner) + expect(runner.active).to eq true + end + end end end diff --git a/spec/services/import/validate_remote_git_endpoint_service_spec.rb b/spec/services/import/validate_remote_git_endpoint_service_spec.rb index 221ac2cd73a..1d2b3975832 100644 --- a/spec/services/import/validate_remote_git_endpoint_service_spec.rb +++ b/spec/services/import/validate_remote_git_endpoint_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Import::ValidateRemoteGitEndpointService do +RSpec.describe Import::ValidateRemoteGitEndpointService, feature_category: :importers do include StubRequests let_it_be(:base_url) { 'http://demo.host/path' } @@ -35,6 +35,28 @@ RSpec.describe Import::ValidateRemoteGitEndpointService do end end + context 'when uri is using an invalid protocol' do + subject { described_class.new(url: 'ssh://demo.host/repo') } + + it 'reports error when invalid URL is provided' do + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + end + end + + context 'when uri is invalid' do + subject { described_class.new(url: 'http:example.com') } + + it 'reports error when invalid URL is provided' do + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + end + end + context 'when receiving HTTP response' do subject { described_class.new(url: base_url) } diff --git a/spec/tooling/danger/sidekiq_args_spec.rb b/spec/tooling/danger/sidekiq_args_spec.rb new file mode 100644 index 00000000000..bfa9ef169de --- /dev/null +++ b/spec/tooling/danger/sidekiq_args_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require 'rspec-parameterized' +require 'gitlab-dangerfiles' +require 'danger' +require 'danger/plugins/internal/helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/sidekiq_args' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::SidekiqArgs, feature_category: :tooling do + include_context "with dangerfile" + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:fake_project_helper) { Tooling::Danger::ProjectHelper } + + subject(:specs) { fake_danger.new(helper: fake_helper) } + + before do + allow(specs).to receive(:project_helper).and_return(fake_project_helper) + end + + describe '#args_changed?' do + using RSpec::Parameterized::TableSyntax + + where(:before, :after, :result) do + " - def perform" | " + def perform(abc)" | true + " - def perform" | " + def perform(abc)" | true + " - def perform(abc)" | " + def perform(def)" | true + " - def perform(abc, def)" | " + def perform(abc)" | true + " - def perform(abc, def)" | " + def perform(def, abc)" | true + " - def perform" | " - def perform" | false + " + def perform" | " + def perform" | false + " - def perform(abc)" | " - def perform(abc)" | false + " + def perform(abc)" | " + def perform(abc)" | false + " - def perform(abc)" | " + def perform_foo(abc)" | false + end + + with_them do + it 'returns correct result' do + expect(specs.args_changed?([before, after])).to eq(result) + end + end + end + + describe '#add_comment_for_matched_line' do + let(:filename) { 'app/workers/hello_worker.rb' } + let(:file_lines) do + [ + "Module Worker", + " def perform", + " puts hello world", + " end", + "end" + ] + end + + before do + allow(specs.project_helper).to receive(:file_lines).and_return(file_lines) + end + + context 'when args are changed' do + before do + allow(specs.helper).to receive(:changed_lines).and_return([" - def perform", " + def perform(abc)"]) + allow(specs).to receive(:args_changed?).and_return(true) + end + + it 'adds suggestion at the correct lines' do + expect(specs).to receive(:markdown).with(format(described_class::SUGGEST_MR_COMMENT), file: filename, line: 2) + + specs.add_comment_for_matched_line(filename) + end + end + + context 'when args are not changed' do + before do + allow(specs.helper).to receive(:changed_lines).and_return([" - def perform", " - def perform"]) + allow(specs).to receive(:args_changed?).and_return(false) + end + + it 'does not add suggestion' do + expect(specs).not_to receive(:markdown) + + specs.add_comment_for_matched_line(filename) + end + end + end + + describe '#changed_worker_files' do + let(:base_expected_files) { %w[app/workers/a.rb app/workers/b.rb ee/app/workers/e.rb] } + + before do + all_changed_files = %w[ + app/workers/a.rb + app/workers/b.rb + ee/app/workers/e.rb + spec/foo_spec.rb + ee/spec/foo_spec.rb + spec/bar_spec.rb + ee/spec/bar_spec.rb + spec/zab_spec.rb + ee/spec/zab_spec.rb + ] + + allow(specs.helper).to receive(:all_changed_files).and_return(all_changed_files) + end + + it 'returns added, modified, and renamed_after files by default' do + expect(specs.changed_worker_files).to match_array(base_expected_files) + end + + context 'with include_ee: :exclude' do + it 'returns spec files without EE-specific files' do + expect(specs.changed_worker_files(ee: :exclude)).not_to include(%w[ee/app/workers/e.rb]) + end + end + + context 'with include_ee: :only' do + it 'returns EE-specific spec files only' do + expect(specs.changed_worker_files(ee: :only)).to match_array(%w[ee/app/workers/e.rb]) + end + end + end +end diff --git a/tooling/danger/sidekiq_args.rb b/tooling/danger/sidekiq_args.rb new file mode 100644 index 00000000000..d06bb92ca6d --- /dev/null +++ b/tooling/danger/sidekiq_args.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module SidekiqArgs + include ::Danger::Helpers + + WORKER_FILES_REGEX = 'app/workers' + EE_PREFIX = 'ee/' + DEF_PERFORM = "def perform" + DEF_PERFORM_REGEX = /[\s+-]*def perform\((.*)\)/ + BEFORE_DEF_PERFORM_REGEX = /^[\s-]*def perform\b/ + AFTER_DEF_PERFORM_REGEX = /^[\s+]*def perform\b/ + + SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT + Please follow the [sidekiq development guidelines](https://docs.gitlab.com/ee/development/sidekiq/compatibility_across_updates.html#changing-the-arguments-for-a-worker) when changing sidekiq worker arguments. + SUGGEST_COMMENT + + def changed_worker_files(ee: :include) + changed_files = helper.all_changed_files + folder_prefix = + case ee + when :include + "(#{EE_PREFIX})?" + when :only + EE_PREFIX + when :exclude + nil + end + + changed_files.grep(%r{\A#{folder_prefix}#{WORKER_FILES_REGEX}}) + end + + def args_changed?(diff) + # Find the "before" and "after" versions of the perform method definition + before_def_perform = diff.find { |line| BEFORE_DEF_PERFORM_REGEX.match?(line) } + after_def_perform = diff.find { |line| AFTER_DEF_PERFORM_REGEX.match?(line) } + + # args are not changed if there is no before or after def perform method + return false unless before_def_perform && after_def_perform + + # Extract the perform method arguments from the "before" and "after" versions + before_args, after_args = diff.flat_map { |line| line.scan(DEF_PERFORM_REGEX) } + + before_args != after_args + end + + def add_comment_for_matched_line(filename) + diff = helper.changed_lines(filename) + return unless args_changed?(diff) + + file_lines = project_helper.file_lines(filename) + + perform_method_line = file_lines.index { |line| line.include?(DEF_PERFORM) } + markdown(format(SUGGEST_MR_COMMENT), file: filename, line: perform_method_line.succ) + end + end + end +end |