diff options
67 files changed, 1243 insertions, 477 deletions
@@ -1 +1 @@ -18.16.0 +18.16.1
\ No newline at end of file diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index b933c562d84..01d46a19ca4 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -1539,23 +1539,6 @@ Layout/ArgumentAlignment: - 'spec/features/issues/user_filters_issues_spec.rb' - 'spec/features/jira_oauth_provider_authorize_spec.rb' - 'spec/features/markdown/gitlab_flavored_markdown_spec.rb' - - 'spec/features/merge_request/maintainer_edits_fork_spec.rb' - - 'spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb' - - 'spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb' - - 'spec/features/merge_request/user_creates_merge_request_spec.rb' - - 'spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb' - - 'spec/features/merge_request/user_merges_immediately_spec.rb' - - 'spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb' - - 'spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb' - - 'spec/features/merge_request/user_opens_checkout_branch_modal_spec.rb' - - 'spec/features/merge_request/user_posts_notes_spec.rb' - - 'spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb' - - 'spec/features/merge_request/user_squashes_merge_request_spec.rb' - - 'spec/features/merge_request/user_suggests_changes_on_diff_spec.rb' - - 'spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb' - - 'spec/features/merge_request/user_uses_quick_actions_spec.rb' - - 'spec/features/merge_requests/user_lists_merge_requests_spec.rb' - - 'spec/features/merge_requests/user_views_open_merge_requests_spec.rb' - 'spec/features/nav/top_nav_tooltip_spec.rb' - 'spec/features/oauth_provider_authorize_spec.rb' - 'spec/features/participants_autocomplete_spec.rb' diff --git a/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js b/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js index 3d81e77f879..f71a1041068 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/creations/new/index.js @@ -1,9 +1,11 @@ import Vue from 'vue'; + +import { mountMarkdownEditor } from 'ee_else_ce/vue_shared/components/markdown/mount_markdown_editor'; + import initPipelines from '~/commit/pipelines/pipelines_bundle'; import MergeRequest from '~/merge_request'; import CompareApp from '~/merge_requests/components/compare_app.vue'; import { __ } from '~/locale'; -import { mountMarkdownEditor } from '~/vue_shared/components/markdown/mount_markdown_editor'; import IssuableTemplateSelectors from '~/issuable/issuable_template_selectors'; const mrNewCompareNode = document.querySelector('.js-merge-request-new-compare'); diff --git a/app/assets/javascripts/pages/projects/merge_requests/edit/index.js b/app/assets/javascripts/pages/projects/merge_requests/edit/index.js index 6127adc3584..79d771ab993 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/edit/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/edit/index.js @@ -1,7 +1,8 @@ +import { mountMarkdownEditor } from 'ee_else_ce/vue_shared/components/markdown/mount_markdown_editor'; + import { createAlert } from '~/alert'; import axios from '~/lib/utils/axios_utils'; import { __ } from '~/locale'; -import { mountMarkdownEditor } from '~/vue_shared/components/markdown/mount_markdown_editor'; import { GitLabDropdown } from '~/deprecated_jquery_dropdown/gl_dropdown'; import initMergeRequest from '~/pages/projects/merge_requests/init_merge_request'; diff --git a/app/assets/javascripts/projects/compare/components/app.vue b/app/assets/javascripts/projects/compare/components/app.vue index bc573461b1f..b40b28adab9 100644 --- a/app/assets/javascripts/projects/compare/components/app.vue +++ b/app/assets/javascripts/projects/compare/components/app.vue @@ -1,7 +1,21 @@ <script> -import { GlCollapsibleListbox, GlButton } from '@gitlab/ui'; +import { + GlButton, + GlFormGroup, + GlFormRadioGroup, + GlIcon, + GlTooltipDirective, + GlSprintf, + GlLink, +} from '@gitlab/ui'; import csrf from '~/lib/utils/csrf'; import { joinPaths } from '~/lib/utils/url_utility'; +import { + I18N, + COMPARE_OPTIONS, + COMPARE_REVISIONS_DOCS_URL, + COMPARE_OPTIONS_INPUT_NAME, +} from '../constants'; import RevisionCard from './revision_card.vue'; export default { @@ -9,7 +23,14 @@ export default { components: { RevisionCard, GlButton, - GlCollapsibleListbox, + GlFormRadioGroup, + GlFormGroup, + GlIcon, + GlLink, + GlSprintf, + }, + directives: { + GlTooltip: GlTooltipDirective, }, props: { projectCompareIndexPath: { @@ -72,23 +93,9 @@ export default { revision: this.paramsTo, refsProjectPath: this.sourceProjectRefsPath, }, - isStraight: this.straight.toString(), + isStraight: this.straight, }; }, - computed: { - dropdownItems() { - return [ - { - text: '..', - value: 'false', - }, - { - text: '...', - value: 'true', - }, - ]; - }, - }, methods: { onSubmit() { this.$refs.form.submit(); @@ -106,6 +113,10 @@ export default { [this.from, this.to] = [this.to, this.from]; // swaps 'from' and 'to' }, }, + i18n: I18N, + compareOptions: COMPARE_OPTIONS, + docsLink: COMPARE_REVISIONS_DOCS_URL, + inputName: COMPARE_OPTIONS_INPUT_NAME, }; </script> @@ -117,13 +128,26 @@ export default { :action="projectCompareIndexPath" > <input :value="$options.csrf.token" type="hidden" name="authenticity_token" /> + <h1 class="gl-font-size-h1 gl-mt-4">{{ $options.i18n.title }}</h1> + <p> + <gl-sprintf :message="$options.i18n.subtitle"> + <template #bold="{ content }"> + <strong>{{ content }}</strong> + </template> + <template #link="{ content }"> + <gl-link target="_blank" :href="$options.docsLink" data-testid="help-link">{{ + content + }}</gl-link> + </template> + </gl-sprintf> + </p> <div class="gl-lg-flex-direction-row gl-lg-display-flex gl-align-items-center compare-revision-cards" > <revision-card data-testid="sourceRevisionCard" :refs-project-path="to.refsProjectPath" - :revision-text="__('Source')" + :revision-text="$options.i18n.source" params-name="to" :params-branch="to.revision" :projects="to.projects" @@ -131,17 +155,26 @@ export default { @selectProject="onSelectProject" @selectRevision="onSelectRevision" /> - <div - class="gl-display-flex gl-justify-content-center gl-align-items-center gl-align-self-end gl-md-my-0 gl-pl-3 gl-pr-3" - data-testid="ellipsis" + <gl-button + v-gl-tooltip="$options.i18n.swapRevisions" + class="gl-display-flex gl-mx-3 gl-align-self-end swap-button" + data-testid="swapRevisionsButton" + category="tertiary" + @click="onSwapRevision" + > + <gl-icon name="substitute" /> + </gl-button> + <gl-button + v-gl-tooltip="$options.i18n.swapRevisions" + class="gl-display-none gl-align-self-end gl-my-5 swap-button-mobile" + @click="onSwapRevision" > - <input :value="isStraight" type="hidden" name="straight" /> - <gl-collapsible-listbox v-model="isStraight" :items="dropdownItems" size="medium" /> - </div> + {{ $options.i18n.swap }} + </gl-button> <revision-card data-testid="targetRevisionCard" :refs-project-path="from.refsProjectPath" - :revision-text="__('Target')" + :revision-text="$options.i18n.target" params-name="from" :params-branch="from.revision" :projects="from.projects" @@ -150,22 +183,32 @@ export default { @selectRevision="onSelectRevision" /> </div> - <div class="gl-display-flex gl-mt-6 gl-gap-3"> - <gl-button category="primary" variant="confirm" @click="onSubmit"> - {{ s__('CompareRevisions|Compare') }} - </gl-button> - <gl-button data-testid="swapRevisionsButton" @click="onSwapRevision"> - {{ s__('CompareRevisions|Swap revisions') }} + <gl-form-group :label="$options.i18n.optionsLabel" class="gl-mt-4"> + <gl-form-radio-group + v-model="isStraight" + :options="$options.compareOptions" + :name="$options.inputName" + required + /> + </gl-form-group> + <div class="gl-display-flex gl-gap-3 gl-pb-4"> + <gl-button + category="primary" + variant="confirm" + data-testid="compare-button" + @click="onSubmit" + > + {{ $options.i18n.compare }} </gl-button> <gl-button v-if="projectMergeRequestPath" :href="projectMergeRequestPath" data-testid="projectMrButton" > - {{ s__('CompareRevisions|View open merge request') }} + {{ $options.i18n.viewMr }} </gl-button> <gl-button v-else-if="createMrPath" :href="createMrPath" data-testid="createMrButton"> - {{ s__('CompareRevisions|Create merge request') }} + {{ $options.i18n.openMr }} </gl-button> </div> </form> diff --git a/app/assets/javascripts/projects/compare/components/revision_card.vue b/app/assets/javascripts/projects/compare/components/revision_card.vue index 162aca44f9d..212937c87c6 100644 --- a/app/assets/javascripts/projects/compare/components/revision_card.vue +++ b/app/assets/javascripts/projects/compare/components/revision_card.vue @@ -40,7 +40,7 @@ export default { <template> <div class="revision-card gl-flex-basis-half"> - <h2 class="gl-font-size-h2"> + <h2 class="gl-font-base gl-mt-0"> {{ s__(`CompareRevisions|${revisionText}`) }} </h2> <div class="gl-sm-display-flex gl-align-items-center gl-gap-3"> diff --git a/app/assets/javascripts/projects/compare/constants.js b/app/assets/javascripts/projects/compare/constants.js new file mode 100644 index 00000000000..f689d543455 --- /dev/null +++ b/app/assets/javascripts/projects/compare/constants.js @@ -0,0 +1,25 @@ +import { __, s__ } from '~/locale'; + +export const COMPARE_OPTIONS_INPUT_NAME = 'straight'; +export const COMPARE_OPTIONS = [ + { value: false, text: s__('CompareRevisions|Only incoming changes from source') }, + { value: true, text: s__('CompareRevisions|Include changes to target since source was created') }, +]; + +export const I18N = { + title: s__('CompareRevisions|Compare revisions'), + subtitle: s__( + 'CompareRevisions|Changes are shown as if the %{boldStart}source%{boldEnd} revision was being merged into the %{boldStart}target%{boldEnd} revision. %{linkStart}Learn more about comparing revisions.%{linkEnd}', + ), + source: __('Source'), + swap: s__('CompareRevisions|Swap'), + target: __('Target'), + swapRevisions: s__('CompareRevisions|Swap revisions'), + compare: s__('CompareRevisions|Compare'), + optionsLabel: s__('CompareRevisions|Show changes'), + viewMr: s__('CompareRevisions|View open merge request'), + openMr: s__('CompareRevisions|Create merge request'), +}; + +export const COMPARE_REVISIONS_DOCS_URL = + 'https://docs.gitlab.com/ee/user/project/repository/branches/#compare-branches'; diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index 9baa2ed15d6..d26dc7d7ba0 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -556,7 +556,7 @@ export default { }; </script> <template> - <div v-if="!loading" class="mr-state-widget gl-mt-3"> + <div v-if="!loading" id="widget-state" class="mr-state-widget gl-mt-3"> <header v-if="shouldRenderCollaborationStatus" class="gl-rounded-base gl-border-solid gl-border-1 gl-border-gray-100 gl-overflow-hidden mr-widget-workflow gl-mt-0!" diff --git a/app/assets/javascripts/vue_shared/components/markdown/header.vue b/app/assets/javascripts/vue_shared/components/markdown/header.vue index bf070943fe6..758254b3cc0 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/header.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/header.vue @@ -13,7 +13,8 @@ import { import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { getModifierKey } from '~/constants'; import { getSelectedFragment } from '~/lib/utils/common_utils'; -import { s__, __ } from '~/locale'; +import { truncateSha } from '~/lib/utils/text_utility'; +import { s__, __, sprintf } from '~/locale'; import { CopyAsGFM } from '~/behaviors/markdown/copy_as_gfm'; import { updateText } from '~/lib/utils/text_markdown'; import ToolbarButton from './toolbar_button.vue'; @@ -203,6 +204,23 @@ export default { }); } }, + replaceTextarea(text) { + const { description, descriptionForSha } = this.$options.i18n; + const headSha = document.getElementById('merge_request_diff_head_sha').value; + const textArea = this.$el.closest('.md-area')?.querySelector('textarea'); + const addendum = headSha + ? sprintf(descriptionForSha, { revision: truncateSha(headSha) }) + : description; + + if (textArea) { + updateText({ + textArea, + tag: `${text}\n\n---\n\n_${addendum}_`, + cursorOffset: 0, + wrap: false, + }); + } + }, switchPreview() { if (this.previewMarkdown) { this.hideMarkdownPreview(); @@ -220,8 +238,13 @@ export default { outdent: keysFor(OUTDENT_LINE), }, i18n: { - preview: __('Preview'), + comment: __('This comment was generated by AI'), + description: s__('MergeRequest|This description was generated using AI'), + descriptionForSha: s__( + 'MergeRequest|This description was generated for revision %{revision} using AI', + ), hidePreview: __('Continue editing'), + preview: __('Preview'), }, }; </script> @@ -289,6 +312,7 @@ export default { v-if="editorAiActions.length" :actions="editorAiActions" @input="insertIntoTextarea" + @replace="replaceTextarea" /> <toolbar-button tag="**" diff --git a/app/assets/javascripts/vue_shared/components/markdown/mount_markdown_editor.js b/app/assets/javascripts/vue_shared/components/markdown/mount_markdown_editor.js index 8ff14220eab..e12815f0094 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/mount_markdown_editor.js +++ b/app/assets/javascripts/vue_shared/components/markdown/mount_markdown_editor.js @@ -1,12 +1,15 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createApolloClient from '~/lib/graphql'; import { queryToObject, objectToQuery } from '~/lib/utils/url_utility'; import { parseBoolean } from '~/lib/utils/common_utils'; + import { CLEAR_AUTOSAVE_ENTRY_EVENT } from '../../constants'; import MarkdownEditor from './markdown_editor.vue'; import eventHub from './eventhub'; -const MR_SOURCE_BRANCH = 'merge_request[source_branch]'; -const MR_TARGET_BRANCH = 'merge_request[target_branch]'; +export const MR_SOURCE_BRANCH = 'merge_request[source_branch]'; +export const MR_TARGET_BRANCH = 'merge_request[target_branch]'; function organizeQuery(obj, isFallbackKey = false) { if (!obj[MR_SOURCE_BRANCH] && !obj[MR_TARGET_BRANCH]) { @@ -51,8 +54,13 @@ function mountAutosaveClearOnSubmit(autosaveKey) { } } -export function mountMarkdownEditor() { +export function mountMarkdownEditor(options = {}) { const el = document.querySelector('.js-markdown-editor'); + const componentConfiguration = { + provide: { + ...options.provide, + }, + }; if (!el) { return null; @@ -86,6 +94,16 @@ export function mountMarkdownEditor() { const setFacade = (props) => Object.assign(facade, props); const autosaveKey = `autosave/${document.location.pathname}/${searchTerm}/description`; + if (options.useApollo || options.apolloProvider) { + let { apolloProvider } = options; + + if (!apolloProvider) { + apolloProvider = new VueApollo({ defaultClient: createApolloClient() }); + } + + componentConfiguration.apolloProvider = apolloProvider; + } + // eslint-disable-next-line no-new new Vue({ el, @@ -114,6 +132,7 @@ export function mountMarkdownEditor() { }, }); }, + ...componentConfiguration, }); mountAutosaveClearOnSubmit(autosaveKey); diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss index 192cb82aaab..6c40781670a 100644 --- a/app/assets/stylesheets/framework/diffs.scss +++ b/app/assets/stylesheets/framework/diffs.scss @@ -1,6 +1,6 @@ // Common .diff-file { - padding-bottom: $gl-padding; + margin-bottom: $gl-padding; &.has-body { .file-title { diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index ff1987f35b3..8cf0bebfc4e 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -518,64 +518,24 @@ } } -.project-refs-form .dropdown-menu { - width: 300px; - @include media-breakpoint-up(sm) { - width: 500px; - } - - a { - white-space: normal; - } -} - -.compare-form-group { - .dropdown-menu, - .inline-input-group { - width: 100%; - - @include media-breakpoint-up(sm) { - width: 300px; +.compare-revision-cards { + @media (max-width: $breakpoint-lg) { + .swap-button { + display: none; } } - + .compare-ellipsis { - width: 100%; - vertical-align: middle; - text-align: center; - margin-top: -20px; - - @include media-breakpoint-up(sm) { - margin: 0 $gl-padding-8; - width: auto; + @media (max-width: $breakpoint-lg) { + .swap-button-mobile { + display: flex; } } - // Remove once gitlab/ui solution is implemented: - // https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1157 - // https://gitlab.com/gitlab-org/gitlab/-/issues/300405 - .gl-search-box-by-type-input { - width: 100%; - } - - // Remove once gitlab/ui solution is implemented - // https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1158 - // https://gitlab.com/gitlab-org/gitlab/-/issues/300405 - .gl-dropdown-button-text { - @include str-truncated; - } -} - -.compare-revision-cards { @media (min-width: $breakpoint-lg) { .gl-card { width: calc(50% - 15px); } - - .compare-ellipsis { - width: 30px; - } } } diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index ceff5d5f510..6a3523b82d9 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -11,6 +11,12 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path] before_action :build_merge_request, except: [:create] + before_action only: [:new] do + if can?(current_user, :fill_in_merge_request_template, project) + push_frontend_feature_flag(:fill_in_mr_template, project) + end + end + urgency :low, [ :new, :create, diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 6e9ab5bd3e5..f1f55d5af9b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -52,6 +52,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:ci_job_failures_in_mr, project) end + before_action only: [:edit] do + if can?(current_user, :fill_in_merge_request_template, project) + push_frontend_feature_flag(:fill_in_mr_template, project) + end + end + around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :diffs, :discussions] after_action :log_merge_request_show, only: [:show, :diffs] diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index fb332fec3b5..4fd307b5105 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -25,14 +25,25 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController end def create - @schedule = Ci::CreatePipelineScheduleService - .new(@project, current_user, schedule_params) - .execute - - if @schedule.persisted? - redirect_to pipeline_schedules_path(@project) + if ::Feature.enabled?(:ci_refactoring_pipeline_schedule_create_service, @project) + response = Ci::PipelineSchedules::CreateService.new(@project, current_user, schedule_params).execute + @schedule = response.payload + + if response.success? + redirect_to pipeline_schedules_path(@project) + else + render :new + end else - render :new + @schedule = Ci::CreatePipelineScheduleService + .new(@project, current_user, schedule_params) + .execute + + if @schedule.persisted? + redirect_to pipeline_schedules_path(@project) + else + render :new + end end end diff --git a/app/graphql/mutations/ci/pipeline_schedule/create.rb b/app/graphql/mutations/ci/pipeline_schedule/create.rb index 65b355cd80f..71a366ed342 100644 --- a/app/graphql/mutations/ci/pipeline_schedule/create.rb +++ b/app/graphql/mutations/ci/pipeline_schedule/create.rb @@ -51,14 +51,28 @@ module Mutations params = pipeline_schedule_attrs.merge(variables_attributes: variables.map(&:to_h)) - schedule = ::Ci::CreatePipelineScheduleService - .new(project, current_user, params) - .execute - - unless schedule.persisted? - return { - pipeline_schedule: nil, errors: schedule.errors.full_messages - } + if ::Feature.enabled?(:ci_refactoring_pipeline_schedule_create_service, project) + response = ::Ci::PipelineSchedules::CreateService + .new(project, current_user, params) + .execute + + schedule = response.payload + + unless response.success? + return { + pipeline_schedule: nil, errors: response.errors + } + end + else + schedule = ::Ci::CreatePipelineScheduleService + .new(project, current_user, params) + .execute + + unless schedule.persisted? + return { + pipeline_schedule: nil, errors: schedule.errors.full_messages + } + end end { diff --git a/app/models/release.rb b/app/models/release.rb index 7f74872cf67..f0ba56390ab 100644 --- a/app/models/release.rb +++ b/app/models/release.rb @@ -64,10 +64,10 @@ class Release < ApplicationRecord end # This query uses LATERAL JOIN to find the latest release for each project. To avoid - # joining the `releases` table, we build an in-memory table using the project ids. + # joining the `projects` table, we build an in-memory table using the project ids. # Example: # SELECT ... - # FROM (VALUES (PROJECT_ID_1),(PROJECT_ID_2)) project_ids (id) + # FROM (VALUES (PROJECT_ID_1),(PROJECT_ID_2)) projects (id) # INNER JOIN LATERAL (...) def latest_for_projects(projects, order_by: 'released_at') return Release.none if projects.empty? diff --git a/app/services/ci/create_pipeline_schedule_service.rb b/app/services/ci/create_pipeline_schedule_service.rb index 0d5f50c26a1..4fdd65bcdb4 100644 --- a/app/services/ci/create_pipeline_schedule_service.rb +++ b/app/services/ci/create_pipeline_schedule_service.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true module Ci + # This class is deprecated and will be removed with the FF ci_refactoring_pipeline_schedule_create_service class CreatePipelineScheduleService < BaseService def execute project.pipeline_schedules.create(pipeline_schedule_params) diff --git a/app/services/ci/pipeline_schedules/create_service.rb b/app/services/ci/pipeline_schedules/create_service.rb new file mode 100644 index 00000000000..c1825865bc0 --- /dev/null +++ b/app/services/ci/pipeline_schedules/create_service.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Ci + module PipelineSchedules + class CreateService + def initialize(project, user, params) + @project = project + @user = user + @params = params + + @schedule = project.pipeline_schedules.new + end + + def execute + return forbidden unless allowed? + + schedule.assign_attributes(params.merge(owner: user)) + + if schedule.save + ServiceResponse.success(payload: schedule) + else + ServiceResponse.error(payload: schedule, message: schedule.errors.full_messages) + end + end + + private + + attr_reader :project, :user, :params, :schedule + + def allowed? + user.can?(:create_pipeline_schedule, schedule) + end + + def forbidden + # We add the error to the base object too + # because model errors are used in the API responses and the `form_errors` helper. + schedule.errors.add(:base, forbidden_message) + + ServiceResponse.error(payload: schedule, message: [forbidden_message], reason: :forbidden) + end + + def forbidden_message + _('The current user is not authorized to create the pipeline schedule') + end + end + end +end diff --git a/app/services/ci/pipeline_schedules/update_service.rb b/app/services/ci/pipeline_schedules/update_service.rb index 2412b5cbd81..28c22e0a868 100644 --- a/app/services/ci/pipeline_schedules/update_service.rb +++ b/app/services/ci/pipeline_schedules/update_service.rb @@ -12,7 +12,9 @@ module Ci def execute return forbidden unless allowed? - if schedule.update(@params) + schedule.assign_attributes(params) + + if schedule.save ServiceResponse.success(payload: schedule) else ServiceResponse.error(message: schedule.errors.full_messages) @@ -21,17 +23,22 @@ module Ci private - attr_reader :schedule, :user + attr_reader :schedule, :user, :params def allowed? user.can?(:update_pipeline_schedule, schedule) end def forbidden - ServiceResponse.error( - message: _('The current user is not authorized to update the pipeline schedule'), - reason: :forbidden - ) + # We add the error to the base object too + # because model errors are used in the API responses and the `form_errors` helper. + schedule.errors.add(:base, forbidden_message) + + ServiceResponse.error(message: [forbidden_message], reason: :forbidden) + end + + def forbidden_message + _('The current user is not authorized to update the pipeline schedule') end end end diff --git a/app/views/projects/commits/_commit_list.html.haml b/app/views/projects/commits/_commit_list.html.haml index 22f4594c1d5..721040f9a09 100644 --- a/app/views/projects/commits/_commit_list.html.haml +++ b/app/views/projects/commits/_commit_list.html.haml @@ -4,7 +4,7 @@ = render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5'}, body_options: { class: 'gl-py-0'}) do |c| - c.with_header do - Commits (#{@total_commit_count}) + = s_('CompareRevisions|Commits on Source (%{commits_amount})').html_safe % { commits_amount: @total_commit_count } - c.with_body do - if hidden > 0 %ul.content-list diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index 9185afc0771..19db86a086e 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -1,7 +1,7 @@ - add_to_breadcrumbs _("Compare revisions"), project_compare_index_path(@project) -- page_title "#{params[:from]}...#{params[:to]}" +- page_title "#{params[:from]} to #{params[:to]}" -.sub-header-block.gl-border-b-0.gl-mb-0 +.sub-header-block.gl-border-b-0.gl-mb-0.gl-pt-4 .js-signature-container{ data: { 'signatures-path' => signatures_namespace_project_compare_index_path } } #js-compare-selector{ data: project_compare_selector_data(@project, @merge_request, params) } diff --git a/app/views/shared/doorkeeper/applications/_form.html.haml b/app/views/shared/doorkeeper/applications/_form.html.haml index 628a34e1278..ae539c46cf1 100644 --- a/app/views/shared/doorkeeper/applications/_form.html.haml +++ b/app/views/shared/doorkeeper/applications/_form.html.haml @@ -1,4 +1,4 @@ -= gitlab_ui_form_for @application, url: url, html: { role: 'form', class: 'doorkeeper-app-form' } do |f| += gitlab_ui_form_for @application, url: url, html: { role: 'form', class: 'doorkeeper-app-form gl-max-w-80' } do |f| = form_errors(@application) .form-group diff --git a/app/views/shared/doorkeeper/applications/_index.html.haml b/app/views/shared/doorkeeper/applications/_index.html.haml index abfe3baf8b4..cffe645d698 100644 --- a/app/views/shared/doorkeeper/applications/_index.html.haml +++ b/app/views/shared/doorkeeper/applications/_index.html.haml @@ -1,10 +1,10 @@ - @force_desktop_expanded_sidebar = true -.row.gl-mt-3.js-search-settings-section - .col-lg-4.profile-settings-sidebar - %h4.gl-mt-0 +.js-search-settings-section + .profile-settings-sidebar + %h4.gl-my-0 = page_title - %p + %p.gl-text-secondary - if oauth_applications_enabled - if oauth_authorized_applications_enabled = _("Manage applications that can use GitLab as an OAuth provider, and applications that you've authorized to use your account.") @@ -12,77 +12,77 @@ = _("Manage applications that use GitLab as an OAuth provider.") - else = _("Manage applications that you've authorized to use your account.") - .col-lg-8 - - if oauth_applications_enabled - %h5.gl-mt-0 - = _('Add new application') + - if oauth_applications_enabled + %h5.gl-mt-0 + = _('Add new application') + .gl-border-b.gl-pb-6 = render 'shared/doorkeeper/applications/form', url: form_url - %hr - - else - .bs-callout.bs-callout-disabled - = _('Adding new applications is disabled in your GitLab instance. Please contact your GitLab administrator to get the permission') - - if oauth_applications_enabled - .oauth-applications - %h5 - = _("Your applications (%{size})") % { size: @applications.size } - - if @applications.any? - .table-responsive - %table.table - %thead - %tr - %th= _('Name') - %th= _('Callback URL') - %th= _('Clients') - %th.last-heading - %tbody - - @applications.each do |application| - %tr{ id: "application_#{application.id}" } - %td= link_to application.name, application_url.call(application) - %td - - application.redirect_uri.split.each do |uri| - %div= uri - %td= application.access_tokens.count - %td.gl-display-flex - = link_to edit_application_url.call(application), class: "gl-button btn btn-default btn-icon gl-mr-3" do - %span.sr-only - = _('Edit') - = sprite_icon('pencil') - = render 'shared/doorkeeper/applications/delete_form', path: application_url.call(application), small: true - - else - .settings-message.text-center - = _("You don't have any applications") - - if oauth_authorized_applications_enabled - .oauth-authorized-applications.prepend-top-20.gl-mb-3 - - if oauth_applications_enabled - %h5 - = _("Authorized applications (%{size})") % { size: @authorized_tokens.size } - - if @authorized_tokens.any? - .table-responsive - %table.table.table-striped - %thead - %tr - %th= _('Name') - %th= _('Authorized At') - %th= _('Scope') - %th - %tbody - - @authorized_tokens.each do |token| - %tr{ id: ("application_#{token.application.id}" if token.application) } - %td - - if token.application - = token.application.name - - else - = _('Anonymous') - .form-text.text-muted - %em= _("Authorization was granted by entering your username and password in the application.") - %td= token.created_at - %td= token.scopes - %td - - if token.application - = render 'doorkeeper/authorized_applications/delete_form', application: token.application - - else - = render 'doorkeeper/authorized_applications/delete_form', token: token - - else - .settings-message.text-center - = _("You don't have any authorized applications") + - else + .bs-callout.bs-callout-disabled + = _('Adding new applications is disabled in your GitLab instance. Please contact your GitLab administrator to get the permission') + - if oauth_applications_enabled + .oauth-applications.gl-pt-6 + %h5.gl-mt-0 + = _("Your applications (%{size})") % { size: @applications.size } + - if @applications.any? + .table-responsive + %table.table + %thead + %tr + %th= _('Name') + %th= _('Callback URL') + %th= _('Clients') + %th.last-heading + %tbody + - @applications.each do |application| + %tr{ id: "application_#{application.id}" } + %td= link_to application.name, application_url.call(application) + %td + - application.redirect_uri.split.each do |uri| + %div= uri + %td= application.access_tokens.count + %td.gl-display-flex + = link_to edit_application_url.call(application), class: "gl-button btn btn-default btn-icon gl-mr-3" do + %span.sr-only + = _('Edit') + = sprite_icon('pencil') + = render 'shared/doorkeeper/applications/delete_form', path: application_url.call(application), small: true + - else + .settings-message + = _("You don't have any applications") + - if oauth_authorized_applications_enabled + .oauth-authorized-applications.gl-mt-4 + - if oauth_applications_enabled + %h5.gl-mt-0 + = _("Authorized applications (%{size})") % { size: @authorized_tokens.size } + + - if @authorized_tokens.any? + .table-responsive + %table.table.table-striped + %thead + %tr + %th= _('Name') + %th= _('Authorized At') + %th= _('Scope') + %th + %tbody + - @authorized_tokens.each do |token| + %tr{ id: ("application_#{token.application.id}" if token.application) } + %td + - if token.application + = token.application.name + - else + = _('Anonymous') + .form-text.text-muted + %em= _("Authorization was granted by entering your username and password in the application.") + %td= token.created_at + %td= token.scopes + %td + - if token.application + = render 'doorkeeper/authorized_applications/delete_form', application: token.application + - else + = render 'doorkeeper/authorized_applications/delete_form', token: token + - else + .settings-message + = _("You don't have any authorized applications") diff --git a/config/feature_flags/development/ci_refactoring_pipeline_schedule_create_service.yml b/config/feature_flags/development/ci_refactoring_pipeline_schedule_create_service.yml new file mode 100644 index 00000000000..40f2af0cc34 --- /dev/null +++ b/config/feature_flags/development/ci_refactoring_pipeline_schedule_create_service.yml @@ -0,0 +1,8 @@ +--- +name: ci_refactoring_pipeline_schedule_create_service +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/124696 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/416359 +milestone: '16.2' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/doc/architecture/blueprints/gitlab_ci_events/index.md b/doc/architecture/blueprints/gitlab_ci_events/index.md index aab3a54cbd2..fb78c0f5d9d 100644 --- a/doc/architecture/blueprints/gitlab_ci_events/index.md +++ b/doc/architecture/blueprints/gitlab_ci_events/index.md @@ -46,7 +46,7 @@ Events" blueprint is about making it possible to: ## Proposals -For now, we have technical 4 proposals; +For now, we have technical 5 proposals; 1. [Proposal 1: Using the `.gitlab-ci.yml` file](proposal-1-using-the-gitlab-ci-file.md) Based on; @@ -56,9 +56,7 @@ For now, we have technical 4 proposals; Highly inefficient way. 1. [Proposal 3: Using the `.gitlab/ci/events` folder](proposal-3-using-the-gitlab-ci-events-folder.md) Involves file reading for every event. -1. [Proposal 4: Creating events via CI files](proposal-4-creating-events-via-ci-files.md) - Combination of some proposals. - -Each of them has its pros and cons. There could be many more proposals and we -would like to discuss them all. We can combine the best part of those proposals -and create a new one. +1. [Proposal 4: Creating events via a CI config file](proposal-4-creating-events-via-ci-files.md) + Separate configuration files for defininig events. +1. [Proposal 5: Combined proposal](proposal-5-combined-proposal.md) + Combination of all of the proposals listed above. diff --git a/doc/architecture/blueprints/gitlab_ci_events/proposal-1-using-the-gitlab-ci-file.md b/doc/architecture/blueprints/gitlab_ci_events/proposal-1-using-the-gitlab-ci-file.md index 7dfc3873ada..f4cde963224 100644 --- a/doc/architecture/blueprints/gitlab_ci_events/proposal-1-using-the-gitlab-ci-file.md +++ b/doc/architecture/blueprints/gitlab_ci_events/proposal-1-using-the-gitlab-ci-file.md @@ -12,7 +12,7 @@ Currently, we have two proof-of-concept (POC) implementations: They both have similar ideas; -1. Find a new CI Config syntax to define the pipeline events. +1. Find a new CI Config syntax to define pipeline events. Example 1: @@ -42,19 +42,13 @@ They both have similar ideas; script: echo "Hello World" ``` -1. Upsert an event to the database when creating a pipeline. -1. Create [EventStore subscriptions](../../../development/event_store.md) to handle the events. +1. Upsert a workflow definition to the database when new configuration gets + pushed. +1. Match subscriptions and publishers whenever something happens at GitLab. -## Problems & Questions +## Discussion -1. The CI config of a project can be anything; - - `.gitlab-ci.yml` by default - - another file in the project - - another file in another project - - completely a remote/external file - - How do we handle these cases? -1. Since we have these problems above, should we keep the events in its own file? (`.gitlab-ci-events.yml`) -1. Do we only accept the changes in the main branch? -1. We try to create event subscriptions every time a pipeline is created. -1. Can we move the existing workflows into the new CI events, for example, `merge_request_event`? +1. How to efficiently detect changes to the subscriptions? +1. How do we handle differences between workflows / events / subscriptions on + different branches? +1. Do we need to upsert subscriptions on every push? diff --git a/doc/architecture/blueprints/gitlab_ci_events/proposal-2-using-the-rules-keyword.md b/doc/architecture/blueprints/gitlab_ci_events/proposal-2-using-the-rules-keyword.md index 6f69a0f11f0..1f59a8ccf20 100644 --- a/doc/architecture/blueprints/gitlab_ci_events/proposal-2-using-the-rules-keyword.md +++ b/doc/architecture/blueprints/gitlab_ci_events/proposal-2-using-the-rules-keyword.md @@ -23,16 +23,13 @@ test_package_removed: - events: ["package/removed"] ``` -1. We don't upsert anything to the database. -1. We'll have a single worker which subcribes to events -like `store.subscribe ::Ci::CreatePipelineFromEventWorker, to: ::Issues::CreatedEvent`. -1. The worker just runs `Ci::CreatePipelineService` with the correct parameters, the rest -will be handled by the `rules` system. Of course, we'll need modifications to the `rules` system to support `events`. - -## Problems & Questions - -1. For every defined event run, we need to enqueue a new `Ci::CreatePipelineFromEventWorker` job. -1. The worker will need to run `Ci::CreatePipelineService` for every event run. -This may be costly because we go through every cycle of `Ci::CreatePipelineService`. -1. This would be highly inefficient. -1. Can we move the existing workflows into the new CI events, for example, `merge_request_event`? +1. We don't upsert subscriptions to the database. +1. We'll have a single worker which runs when something happens in GitLab. +1. The worker just tries to create a pipeline with the correct parameters. +1. Pipeline runs when `rules` subsystem finds a job to run. + +## Challenges + +1. For every defined event run, we need to enqueue a new pipeline creation worker. +1. Creating pipelines and selecting builds to run is a relatively expensive operation +1. This will not work on GitLab.com scale. diff --git a/doc/architecture/blueprints/gitlab_ci_events/proposal-3-using-the-gitlab-ci-events-folder.md b/doc/architecture/blueprints/gitlab_ci_events/proposal-3-using-the-gitlab-ci-events-folder.md index ad76b7f8dd4..8a8efe2be08 100644 --- a/doc/architecture/blueprints/gitlab_ci_events/proposal-3-using-the-gitlab-ci-events-folder.md +++ b/doc/architecture/blueprints/gitlab_ci_events/proposal-3-using-the-gitlab-ci-events-folder.md @@ -5,11 +5,8 @@ description: 'GitLab CI Events Proposal 3: Using the .gitlab/ci/events folder' # GitLab CI Events Proposal 3: Using the `.gitlab/ci/events` folder -We can also approach this problem by creating separate files for events. - -Let's say we'll have the `.gitlab/ci/events` folder (or `.gitlab/workflows/ci`). - -We can define events in the following format: +In this proposal we want to create separate files for each group of events. We +can define events in the following format: ```yaml # .gitlab/ci/events/package-published.yml @@ -17,9 +14,7 @@ We can define events in the following format: spec: events: - name: package/published - --- - include: - local: .gitlab-ci.yml with: @@ -35,9 +30,7 @@ spec: inputs: event: default: push - --- - job1: script: echo "Hello World" @@ -61,4 +54,4 @@ When an event happens; 1. For every defined event run, we need to enqueue a new job. 1. Every event-job will need to search for files. 1. This would be only for the project-scope events. -1. This can be inefficient because of searching for files for the project for every event. +1. This will not work for GitLab.com scale. diff --git a/doc/architecture/blueprints/gitlab_ci_events/proposal-4-creating-events-via-ci-files.md b/doc/architecture/blueprints/gitlab_ci_events/proposal-4-creating-events-via-ci-files.md index 5f10ba1fbb2..debca82d148 100644 --- a/doc/architecture/blueprints/gitlab_ci_events/proposal-4-creating-events-via-ci-files.md +++ b/doc/architecture/blueprints/gitlab_ci_events/proposal-4-creating-events-via-ci-files.md @@ -1,12 +1,13 @@ --- owning-stage: "~devops::verify" -description: 'GitLab CI Events Proposal 4: Creating events via CI files' +description: 'GitLab CI Events Proposal 4: Defining subscriptions in a dedicated configuration file' --- -# GitLab CI Events Proposal 4: Creating events via CI files +# GitLab CI Events Proposal 4: Defining subscriptions in a dedicated configuration file -Each project can have its own event configuration file. Let's call it `.gitlab-ci-event.yml` for now. -In this file, we can define events in the following format: +Each project can have its own configuration file for defining subscriptions to +events. For example, `.gitlab-ci-event.yml`. In this file, we can define events +in the following format: ```yaml events: @@ -14,12 +15,13 @@ events: - issue/created ``` -When this file is changed in the project repository, it is parsed and the events are created, updated, or deleted. -This is highly similar to [Proposal 1](proposal-1-using-the-gitlab-ci-file.md) except that we don't need to -track pipeline creations every time. +When this file is changed in the project repository, it is parsed and the +events are created, updated, or deleted. This is highly similar to +[Proposal 1](proposal-1-using-the-gitlab-ci-file.md) except that we don't need +to track pipeline creations every time. -1. Upsert events to the database when `.gitlab-ci-event.yml` is updated. -1. Create [EventStore subscriptions](../../../development/event_store.md) to handle the events. +1. Upsert events to the database when `.gitlab-ci-event.yml` gets updated. +1. Create inline reactions to events in code to trigger pipelines. ## Filtering jobs @@ -51,7 +53,7 @@ test_package_removed: - if: $CI_EVENT == "package/removed" ``` -or an input like in the [Proposal 3](proposal-3-using-the-gitlab-ci-events-folder.md); +or an input like in the [Proposal 3](proposal-3-using-the-gitlab-ci-events-folder.md): ```yaml spec: @@ -71,3 +73,7 @@ test_package_removed: rules: - if: $[[ inputs.event ]] == "package/removed" ``` + +## Challenges + +1. This will not work on GitLab.com scale. diff --git a/doc/architecture/blueprints/gitlab_ci_events/proposal-5-combined-proposal.md b/doc/architecture/blueprints/gitlab_ci_events/proposal-5-combined-proposal.md new file mode 100644 index 00000000000..3a596b21526 --- /dev/null +++ b/doc/architecture/blueprints/gitlab_ci_events/proposal-5-combined-proposal.md @@ -0,0 +1,99 @@ +--- +owning-stage: "~devops::verify" +description: 'GitLab CI Events Proposal 5: Combined proposal' +--- + +# GitLab CI Events Proposal 5: Combined proposal + +In this proposal we have separate files for cohesive groups of events. The +files are being included into the main `.gitlab-ci.yml` configuration file. + +```yaml +# my/events/packages.yaml + +spec: + events: + - events/package/published + - events/audit/package/* + inputs: + env: +--- +do_something: + script: ./run_for $[[ event.name ]] --env $[[ inputs.env ]] + rules: + - if: $[[ event.payload.package.name ]] == "my_package" +``` + +In the `.gitlab-ci.yml` file, we can enable the subscription: + +```yaml +# .gitlab-ci.yml + +include: + - local: my/events/packages.yaml + inputs: + env: test + +``` + +GitLab will detect changes in the included files, and parse their specs. All +the information required to define a subscription will be encapsulated in the +spec, hence we will not need to read a whole file. We can easily read `spec` +header and calculate its checksum what can become a workflow identifier. + +Once we see a new identifier, we can redefine subscriptions for a particular +project and then to upsert them into the database. + +We will use an efficient GIN index matching technique to match publishers with +the subscribers to run pipelines. + +The syntax is also compatible with CI Components, and make it easier to define +components that will only be designed to run for events happening inside +GitLab. + +## No entrypoint file variant + +Another variant of this proposal is to move away from the single GitLab CI YAML +configuration file. In such case we would define another search **directory**, +like `.gitlab/workflows/` where we would store all YAML files. + +We wouldn't need to `include` workflow / events files anywhere, because these +would be found by GitLab automatically. In order to implement this feature this +way we would need to extend features like "custom location for `.gitlab-ci.yml` +file". + +Example, without using a main configuration file (the GitLab CI YAML file would +be still supported): + +```yaml +# .gitlab/workflows/push.yml + +spec: + events: + - events/repository/push +--- +rspec-on-push: + script: bundle exec rspec +``` + +```yaml +# .gitlab/workflows/merge_requests.yml + +spec: + events: + - events/merge_request/push +--- +rspec-on-mr-push: + script: bundle exec rspec +``` + +```yaml +# .gitlab/workflows/schedules.yml + +spec: + events: + - events/pipeline/schedule/run +--- +smoke-test: + script: bundle exec rspec --smoke +``` diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index da3a6eff79d..915877d3980 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -1418,6 +1418,48 @@ wrapper = mount(SomeComponent, { }); ``` +#### Testing subscriptions + +When testing subscriptions, be aware that default behavior for subscription in `vue-apollo@4` is to re-subscribe and immediatelly issue new request on error (unless value of `skip` restricts us from doing that) + +```javascript +import waitForPromises from 'helpers/wait_for_promises'; + +// subscriptionMock is registered as handler function for subscription +// in our helper +const subcriptionMock = jest.fn().mockResolvedValue(okResponse); + +// ... + +it('testing error state', () => { + // Avoid: will stuck below! + subscriptionMock = jest.fn().mockRejectedValue({ errors: [] }); + + // component calls subscription mock as part of + createComponent(); + // will be stuck forever: + // * rejected promise will trigger resubscription + // * re-subscription will call subscriptionMock again, resulting in rejected promise + // * rejected promise will trigger next re-subscription, + await waitForPromises(); + // ... +}) +``` + +To avoid such infinite loops when using `vue@3` and `vue-apollo@4` consider using one-time rejections + +```javascript +it('testing failure', () => { + // OK: subscription will fail once + subscriptionMock.mockRejectedValueOnce({ errors: [] }); + // component calls subscription mock as part of + createComponent(); + await waitForPromises(); + + // code below now will be executred +}) +``` + #### Testing `@client` queries ##### Using mock resolvers diff --git a/doc/install/installation.md b/doc/install/installation.md index 11f2760cee8..82c5807131e 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -260,7 +260,7 @@ GitLab requires the use of Node to compile JavaScript assets, and Yarn to manage JavaScript dependencies. The current minimum requirements for these are: -- `node` 18.x releases (v18.16.0 or later). +- `node` 18.x releases (v18.16.1 or later). [Other LTS versions of Node.js](https://github.com/nodejs/release#release-schedule) might be able to build assets, but we only guarantee Node.js 18.x. - `yarn` = v1.22.x (Yarn 2 is not supported yet) diff --git a/doc/integration/advanced_search/elasticsearch.md b/doc/integration/advanced_search/elasticsearch.md index 7b23eaa278a..d515289dfb5 100644 --- a/doc/integration/advanced_search/elasticsearch.md +++ b/doc/integration/advanced_search/elasticsearch.md @@ -217,6 +217,7 @@ The following Elasticsearch settings are available: | `Elasticsearch indexing` | Enables or disables Elasticsearch indexing and creates an empty index if one does not already exist. You may want to enable indexing but disable search to give the index time to be fully completed, for example. Also, keep in mind that this option doesn't have any impact on existing data, this only enables/disables the background indexer which tracks data changes and ensures new data is indexed. | | `Pause Elasticsearch indexing` | Enables or disables temporary indexing pause. This is useful for cluster migration/reindexing. All changes are still tracked, but they are not committed to the Elasticsearch index until resumed. | | `Search with Elasticsearch enabled` | Enables or disables using Elasticsearch in search. | +| `Requeue indexing workers` | Enable automatic requeuing of indexing workers. This improves non-code indexing throughput by enqueuing Sidekiq jobs until all documents are processed. Requeuing indexing workers is not recommended for smaller instances or instances with few Sidekiq processes. | | `URL` | The URL of your Elasticsearch instance. Use a comma-separated list to support clustering (for example, `http://host1, https://host2:9200`). If your Elasticsearch instance is password-protected, use the `Username` and `Password` fields described below. Alternatively, use inline credentials such as `http://<username>:<password>@<elastic_host>:9200/`. | | `Username` | The `username` of your Elasticsearch instance. | | `Password` | The password of your Elasticsearch instance. | @@ -228,6 +229,7 @@ The following Elasticsearch settings are available: | `AWS Secret Access Key` | The AWS secret access key. | | `Maximum file size indexed` | See [the explanation in instance limits.](../../administration/instance_limits.md#maximum-file-size-indexed). | | `Maximum field length` | See [the explanation in instance limits.](../../administration/instance_limits.md#maximum-field-length). | +| `Number of shards for non-code indexing` | Number of indexing worker shards. This improves non-code indexing throughput by enqueuing more parallel Sidekiq jobs. Increasing the number of shards is not recommended for smaller instances or instances with few Sidekiq processes. Default is `2`. | | `Maximum bulk request size (MiB)` | Used by the GitLab Ruby and Go-based indexer processes. This setting indicates how much data must be collected (and stored in memory) in a given indexing process before submitting the payload to the Elasticsearch Bulk API. For the GitLab Go-based indexer, you should use this setting with `Bulk request concurrency`. `Maximum bulk request size (MiB)` must accommodate the resource constraints of both the Elasticsearch hosts and the hosts running the GitLab Go-based indexer from either the `gitlab-rake` command or the Sidekiq tasks. | | `Bulk request concurrency` | The Bulk request concurrency indicates how many of the GitLab Go-based indexer processes (or threads) can run in parallel to collect data to subsequently submit to the Elasticsearch Bulk API. This increases indexing performance, but fills the Elasticsearch bulk requests queue faster. This setting should be used together with the Maximum bulk request size setting (see above) and needs to accommodate the resource constraints of both the Elasticsearch hosts and the hosts running the GitLab Go-based indexer either from the `gitlab-rake` command or the Sidekiq tasks. | | `Client request timeout` | Elasticsearch HTTP client request timeout value in seconds. `0` means using the system default timeout value, which depends on the libraries that GitLab application is built upon. | diff --git a/doc/user/application_security/dast/browser_based.md b/doc/user/application_security/dast/browser_based.md index 7b263e5817d..c8ddcfbb201 100644 --- a/doc/user/application_security/dast/browser_based.md +++ b/doc/user/application_security/dast/browser_based.md @@ -189,7 +189,7 @@ For authentication CI/CD variables, see [Authentication](authentication.md). | `DAST_BROWSER_MAX_ACTIONS` | number | `10000` | The maximum number of actions that the crawler performs. For example, selecting a link, or filling a form. | | `DAST_BROWSER_MAX_DEPTH` | number | `10` | The maximum number of chained actions that the crawler takes. For example, `Click -> Form Fill -> Click` is a depth of three. | | `DAST_BROWSER_MAX_RESPONSE_SIZE_MB` | number | `15` | The maximum size of a HTTP response body. Responses with bodies larger than this are blocked by the browser. Defaults to 10 MB. | -| `DAST_BROWSER_NAVIGATION_STABILITY_TIMEOUT` | [Duration string](https://pkg.go.dev/time#ParseDuration) | `7s` | The maximum amount of time to wait for a browser to consider a page loaded and ready for analysis after a navigation completes. | +| `DAST_BROWSER_NAVIGATION_STABILITY_TIMEOUT` | [Duration string](https://pkg.go.dev/time#ParseDuration) | `7s` | The maximum amount of time to wait for a browser to consider a page loaded and ready for analysis after a navigation completes. Defaults to `800ms`.| | `DAST_BROWSER_NAVIGATION_TIMEOUT` | [Duration string](https://pkg.go.dev/time#ParseDuration) | `15s` | The maximum amount of time to wait for a browser to navigate from one page to another. | | `DAST_BROWSER_NUMBER_OF_BROWSERS` | number | `3` | The maximum number of concurrent browser instances to use. For shared runners on GitLab.com, we recommended a maximum of three. Private runners with more resources may benefit from a higher number, but are likely to produce little benefit after five to seven instances. | | `DAST_BROWSER_PAGE_LOADING_SELECTOR` | selector | `css:#page-is-loading` | Selector that when is no longer visible on the page, indicates to the analyzer that the page has finished loading and the scan can continue. Cannot be used with `DAST_BROWSER_PAGE_READY_SELECTOR`. | diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 1179985d43b..9cc167e0496 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -1025,24 +1025,6 @@ See explanations of the variables above in the [configuration section](#configur See the following sections for configuring specific languages and package managers. -#### JavaScript (npm and yarn) projects - -Add the following to the variables section of `.gitlab-ci.yml`: - -```yaml -RETIREJS_JS_ADVISORY_DB: "example.com/jsrepository.json" -RETIREJS_NODE_ADVISORY_DB: "example.com/npmrepository.json" -``` - -#### Ruby (gem) projects - -Add the following to the variables section of `.gitlab-ci.yml`: - -```yaml -BUNDLER_AUDIT_ADVISORY_DB_REF_NAME: "master" -BUNDLER_AUDIT_ADVISORY_DB_URL: "gitlab.example.com/ruby-advisory-db.git" -``` - #### Python (pip) If you need to install Python packages before the analyzer runs, you should use `pip install --user` in the `before_script` of the scanning job. The `--user` flag causes project dependencies to be installed in the user directory. If you do not pass the `--user` option, packages are installed globally, and they are not scanned and don't show up when listing project dependencies. diff --git a/doc/user/group/saml_sso/scim_setup.md b/doc/user/group/saml_sso/scim_setup.md index e5d6c86a5ac..4ebff6fe584 100644 --- a/doc/user/group/saml_sso/scim_setup.md +++ b/doc/user/group/saml_sso/scim_setup.md @@ -183,7 +183,11 @@ The following diagram describes what happens when you add users to your SCIM app graph TD A[Add User to SCIM app] -->|IdP sends user info to GitLab| B(GitLab: Does the email exist?) B -->|No| C[GitLab creates user with SCIM identity] - B -->|Yes| D[GitLab sends message back 'Email exists'] + B -->|Yes| D(GitLab: Is the user part of the group?) + D -->|No| E(GitLab: Is SSO enforcement enabled?) + E -->|No| G + E -->|Yes| F[GitLab sends message back:\nThe member's email address is not linked to a SAML account] + D -->|Yes| G[Associate SCIM identity to user] ``` During provisioning: diff --git a/doc/user/workspace/index.md b/doc/user/workspace/index.md index 4ad651f08a7..2fe033db880 100644 --- a/doc/user/workspace/index.md +++ b/doc/user/workspace/index.md @@ -62,6 +62,17 @@ To create a workspace: The workspace might take a few minutes to start. To access the workspace, under **Preview**, select the workspace link. You also have access to the terminal and can install any necessary dependencies. +## Deleting data associated with a workspace + +When you delete a project, agent, user, or token associated with a workspace: + +- The workspace is deleted from both the user interface and the Kubernetes cluster. +- In the Kubernetes cluster, the running workspace resources become orphaned. + +To clean up orphaned resources, a cluster administrator must manually delete the namespace. + +For more information about our plans to change the current behavior, see [issue 414384](https://gitlab.com/gitlab-org/gitlab/-/issues/414384). + ## Devfile A devfile is a file that defines a development environment by specifying the necessary tools, languages, runtimes, and other components for a GitLab project. diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb index e27ec24fb44..1606d5ba649 100644 --- a/lib/api/ci/pipeline_schedules.rb +++ b/lib/api/ci/pipeline_schedules.rb @@ -90,14 +90,28 @@ module API post ':id/pipeline_schedules' do authorize! :create_pipeline_schedule, user_project - pipeline_schedule = ::Ci::CreatePipelineScheduleService - .new(user_project, current_user, declared_params(include_missing: false)) - .execute + if ::Feature.enabled?(:ci_refactoring_pipeline_schedule_create_service, @project) + response = ::Ci::PipelineSchedules::CreateService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute - if pipeline_schedule.persisted? - present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails + pipeline_schedule = response.payload + + if response.success? + present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails + else + render_validation_error!(pipeline_schedule) + end else - render_validation_error!(pipeline_schedule) + pipeline_schedule = ::Ci::CreatePipelineScheduleService + .new(user_project, current_user, declared_params(include_missing: false)) + .execute + + if pipeline_schedule.persisted? + present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails + else + render_validation_error!(pipeline_schedule) + end end end @@ -121,10 +135,22 @@ module API put ':id/pipeline_schedules/:pipeline_schedule_id' do authorize! :update_pipeline_schedule, pipeline_schedule - if pipeline_schedule.update(declared_params(include_missing: false)) - present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails + if ::Feature.enabled?(:ci_refactoring_pipeline_schedule_create_service, @project) + response = ::Ci::PipelineSchedules::UpdateService + .new(pipeline_schedule, current_user, declared_params(include_missing: false)) + .execute + + if response.success? + present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails + else + render_validation_error!(pipeline_schedule) + end else - render_validation_error!(pipeline_schedule) + if pipeline_schedule.update(declared_params(include_missing: false)) # rubocop:disable Style/IfInsideElse + present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails + else + render_validation_error!(pipeline_schedule) + end end end diff --git a/lib/gitlab/ci/config/README.md b/lib/gitlab/ci/config/README.md new file mode 100644 index 00000000000..e850afc5253 --- /dev/null +++ b/lib/gitlab/ci/config/README.md @@ -0,0 +1,178 @@ +# `::Gitlab::Ci::Config` module overview + +`::Gitlab::Ci::Config` is a concrete implementation of abstract +`::Gitlab::Config` module. It's being used to build, traverse and translate +hierarchical, user-provided, CI configuration, usually provided in +`.gitlab-ci.yml` and included files. + +## High-level Overview + +`::Gitlab::Ci::Config` is an indirection layer between user-provided data and +GitLab itself. + +1. A user provides YAML configuration in `.gitlab-ci.yml` and all included files. +1. `::Gitlab::Ci::Config` loads the provided YAML using Ruby standard `Psych` library. +1. The resulting Hash is then passed to the module to build an Abstract Syntax Tree. +1. The module validates, transforms, translates and augments the data to build + a stable representation of user-provided configuration. + +This additional layer helps us to validate the user-provided configuration and +surface any errors to a user if it is not valid. In case of a valid +configuration, it makes it possible to build a stable representation of +config that we can depend on. + +For example, both following configurations using the +[environment](https://docs.gitlab.com/ee/ci/yaml/#environment) +keyword are correct: + +```yaml +# First way to define an environment: + +deploy: + environment: production + script: cap deploy + +# Second way to define an environment: + +deploy: + environment: + name: production + url: https://prod.example.com + kubernetes: + namespace: production +``` + +This demonstrates the concept of hidden / expanding complexity: if users need +more flexibility, they can opt-in into using a much more elaborate syntax to +configure their environments. **We use this technique to make it possible for +simplicity to coexist with flexibility without additional complexity**. + +`::Gitlab::Ci::Config` allows us to achieve this, because it is an indirection +layer, that translates user-provided configuration into a known and expected +format when users can achieve the same thing in `.gitlab-ci.yml` in a few +different ways. + +## Hierarchical configuration + +`.gitlab-ci.yml` configuration is hierarchical but same keywords can often be +used on different levels in the hierarchy. `::Gitlab::Ci::Config` module makes +it easier to manage the complexity that stems from having same keyword +available in [many different places](https://docs.gitlab.com/ee/ci/yaml/#default): + +```yaml +default: + image: ruby:3.0 + +rspec: + script: bundle exec rspec + +rspec 2.7: + image: ruby:2.7 + script: bundle exec rspec +``` + +We can achieve that, because in `::Gitlab::Ci::Config` most of the keywords are +implemented within separate Ruby classes, that then can be reused: + +```ruby +# Simplified version of an entry class that describes a Docker image. +# +class Gitlab::Ci::Config::Entry + class Image < ::Gitlab::Config::Entry::Node + + validates :config, allowed_keys: ALLOWED_IMAGE_CONFIG_KEYS + + def value + if string? + { name: @config } + elsif hash? + { + name: @config[:name], + entrypoint: @config[:entrypoint], + ports: (ports_value if ports_defined?), + pull_policy: pull_policy_value + } + else + {} + end + end + end +end +``` + +The config above is a simple demonstration of the translation layer, into a +stable configuration, depending on what simplification strategy has been used +by a user. There more complex examples, though: + +```ruby +module Gitlab::Ci::Config::Entry + class Need < ::Gitlab::Config::Entry::Simplifiable + strategy :JobString, if: -> (config) { config.is_a?(String) } + + strategy :JobHash, + if: -> (config) { config.is_a?(Hash) && same_pipeline_need?(config) } + + strategy :CrossPipelineDependency, + if: -> (config) { config.is_a?(Hash) && cross_pipeline_need?(config) } + + # [ ... ] + end +end +``` + +Every time we load config, an Abstract Syntax Tree is being built, because +nodes / entries know what the child nodes can be: + +```ruby +# Simplified root entry code +# +module Gitlab::Ci::Config::Entry + class Root < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Configurable + + entry :default, Entry::Default, + description: 'Default configuration for all jobs.' + + entry :include, Entry::Includes, + description: 'List of external YAML files to include.' + + entry :before_script, Entry::Commands, + description: 'Script that will be executed before each job.' + + entry :image, Entry::Image, + description: 'Docker image that will be used to execute jobs.' + + entry :services, Entry::Services, + description: 'Docker images that will be linked to the container.' + + entry :after_script, Entry::Commands, + description: 'Script that will be executed after each job.' + + entry :variables, Entry::Variables, + description: 'Environment variables that will be used.' + + # [ ... ] + end +end +``` + +Loading the configuration script mentioned at the beginning of this pargraph +will result in build a following AST: + +``` +Entry::Root +`- + |- Entry::Default + | `- Entry::Image('ruby:3.0') + | + |- Entry::Job('rspec') + | `- Entry::Script('bundle exec rspec') + | + |- Entry::Job('rspec 2.7') + | |- Entry::Image('ruby:2.7) + | `- Entry::Script('bundle exec rspec') +``` + +The AST will be validated, and eventually will generate a stable representation +of configuration that we can use to persist pipelines / stages / jobs in the +database, and start pipeline processing. diff --git a/lib/gitlab/config/README.md b/lib/gitlab/config/README.md new file mode 100644 index 00000000000..355dbdc8cfe --- /dev/null +++ b/lib/gitlab/config/README.md @@ -0,0 +1,29 @@ +# `::Gitlab::Config` module overview + +`::Gitlab::Config` is an abstract module used to build, traverse and translate +any kind of hierarchical, user-provided configuration. + +The most complex and widely used implementation is `::Gitlab::Ci::Config` +facade class. Please see `lib/gitlab/ci/config/README.md` for more information +around how it works. + +## High-level Overview + +The main motivation behind how `::Gitlab::Config` and `::Gitlab::Ci::Config` +work is to build an indirection layer between complex user-provided +configuration and GitLab itself. This helps us to extend configuration keywords +in a backwards-compatible way, and make sure that validation and transformation +rules are encapsulated within domain classes, what significantly helps to +reduce cognitive load on Engineers working on that part of the codebase. + +`Gitlab::Config` is a tool to work with hierarchical configuration: + +1. First we parse YAML with Ruby standard library `Psych`. +1. The resulting hash is being used to initialize a concrete implementation of `Gitlab::Config`. +1. In `::Gitlab::Ci::Config` abstract classes from `::Gitlab::Config` have their implementations. +1. Each domain class represents one or a group of hierarchical YAML entries, like `job:artifacts`. +1. Each entry knows what subentires are supported and how to validate them. +1. Upon loading a configuration we build an abstract syntax tree, and validate configuration. +1. If there are errors, the module can surface them to a user. +1. In case of config being valid, the config gets translated and augmented. +1. The result is a consistent representation that we can depend on in other parts of the codebase. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 08bdf1df796..0bd5f0dcbbd 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11549,15 +11549,30 @@ msgstr "" msgid "CompareRevisions|Branches" msgstr "" +msgid "CompareRevisions|Changes are shown as if the %{boldStart}source%{boldEnd} revision was being merged into the %{boldStart}target%{boldEnd} revision. %{linkStart}Learn more about comparing revisions.%{linkEnd}" +msgstr "" + +msgid "CompareRevisions|Commits on Source (%{commits_amount})" +msgstr "" + msgid "CompareRevisions|Compare" msgstr "" +msgid "CompareRevisions|Compare revisions" +msgstr "" + msgid "CompareRevisions|Create merge request" msgstr "" msgid "CompareRevisions|Filter by Git revision" msgstr "" +msgid "CompareRevisions|Include changes to target since source was created" +msgstr "" + +msgid "CompareRevisions|Only incoming changes from source" +msgstr "" + msgid "CompareRevisions|Select Git revision" msgstr "" @@ -11567,6 +11582,12 @@ msgstr "" msgid "CompareRevisions|Select target project" msgstr "" +msgid "CompareRevisions|Show changes" +msgstr "" + +msgid "CompareRevisions|Swap" +msgstr "" + msgid "CompareRevisions|Swap revisions" msgstr "" @@ -19279,6 +19300,9 @@ msgstr "" msgid "Files, directories, and submodules in the path %{path} for commit reference %{ref}" msgstr "" +msgid "Fill in merge request template" +msgstr "" + msgid "Fill in the fields below, turn on %{strong_open}Enable SAML authentication for this group%{strong_close}, and press %{strong_open}Save changes%{strong_close}" msgstr "" @@ -28792,6 +28816,12 @@ msgstr "" msgid "MergeRequest|Search (e.g. *.vue) (%{MODIFIER_KEY}P)" msgstr "" +msgid "MergeRequest|This description was generated for revision %{revision} using AI" +msgstr "" + +msgid "MergeRequest|This description was generated using AI" +msgstr "" + msgid "MergeTopics|%{sourceTopic} will be removed" msgstr "" @@ -38532,6 +38562,9 @@ msgstr "" msgid "Replace audio" msgstr "" +msgid "Replace current template with filled in placeholders" +msgstr "" + msgid "Replace file" msgstr "" @@ -45896,6 +45929,9 @@ msgstr "" msgid "The current user is not authorized to access the job log." msgstr "" +msgid "The current user is not authorized to create the pipeline schedule" +msgstr "" + msgid "The current user is not authorized to update the pipeline schedule" msgstr "" diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 6d810fdcd51..486062fe52b 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -106,7 +106,8 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end - describe 'POST #create' do + # Move this from `shared_context` to `describe` when `ci_refactoring_pipeline_schedule_create_service` is removed. + shared_context 'POST #create' do # rubocop:disable RSpec/ContextWording describe 'functionality' do before do project.add_developer(user) @@ -184,6 +185,16 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end + it_behaves_like 'POST #create' + + context 'when the FF ci_refactoring_pipeline_schedule_create_service is disabled' do + before do + stub_feature_flags(ci_refactoring_pipeline_schedule_create_service: false) + end + + it_behaves_like 'POST #create' + end + describe 'PUT #update' do describe 'functionality' do let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index c9aa22e396b..7603696c60c 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'a maintainer edits files on a source-branch of an MR from a fork', :js, :sidekiq_might_not_need_inline, -feature_category: :code_review_workflow do + feature_category: :code_review_workflow do include Features::SourceEditorSpecHelpers include ProjectForksHelper let(:user) { create(:user, username: 'the-maintainer') } @@ -12,13 +12,15 @@ feature_category: :code_review_workflow do let(:source_project) { fork_project(target_project, author, repository: true) } let(:merge_request) do - create(:merge_request, - source_project: source_project, - target_project: target_project, - source_branch: 'fix', - target_branch: 'master', - author: author, - allow_collaboration: true) + create( + :merge_request, + source_project: source_project, + target_project: target_project, + source_branch: 'fix', + target_branch: 'master', + author: author, + allow_collaboration: true + ) end before do diff --git a/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb index 0ff773ef02d..149b2e2bb0f 100644 --- a/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb +++ b/spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'create a merge request, allowing commits from members who can merge to the target branch', :js, -feature_category: :code_review_workflow do + feature_category: :code_review_workflow do include ProjectForksHelper let(:user) { create(:user) } let(:target_project) { create(:project, :public, :repository) } @@ -67,10 +67,12 @@ feature_category: :code_review_workflow do context 'when a member who can merge tries to edit the option' do let(:member) { create(:user) } let(:merge_request) do - create(:merge_request, - source_project: source_project, - target_project: target_project, - source_branch: 'fixes') + create( + :merge_request, + source_project: source_project, + target_project: target_project, + source_branch: 'fixes' + ) end before do diff --git a/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb b/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb index 537702df12d..446f6a470de 100644 --- a/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb +++ b/spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297500', - feature_category: :code_review_workflow do + feature_category: :code_review_workflow do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } diff --git a/spec/features/merge_request/user_creates_merge_request_spec.rb b/spec/features/merge_request/user_creates_merge_request_spec.rb index 97b423f2cc2..eab5cee976e 100644 --- a/spec/features/merge_request/user_creates_merge_request_spec.rb +++ b/spec/features/merge_request/user_creates_merge_request_spec.rb @@ -110,11 +110,13 @@ RSpec.describe 'User creates a merge request', :js, feature_category: :code_revi context 'when project is public and merge requests are private' do let_it_be(:project) do - create(:project, - :public, - :repository, - group: group, - merge_requests_access_level: ProjectFeature::DISABLED) + create( + :project, + :public, + :repository, + group: group, + merge_requests_access_level: ProjectFeature::DISABLED + ) end context 'and user is a guest' do diff --git a/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb b/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb index a013666a496..a96ec1f68aa 100644 --- a/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb +++ b/spec/features/merge_request/user_interacts_with_batched_mr_diffs_spec.rb @@ -64,7 +64,7 @@ RSpec.describe 'Batch diffs', :js, feature_category: :code_review_workflow do context 'which is in at least page 2 of the batched pages of diffs' do it 'scrolls to the correct discussion', - quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/293814' } do + quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/293814' } do page.within get_first_diff do click_link('just now') end diff --git a/spec/features/merge_request/user_merges_immediately_spec.rb b/spec/features/merge_request/user_merges_immediately_spec.rb index d47968ebc6b..71af2045bab 100644 --- a/spec/features/merge_request/user_merges_immediately_spec.rb +++ b/spec/features/merge_request/user_merges_immediately_spec.rb @@ -6,17 +6,23 @@ RSpec.describe 'Merge requests > User merges immediately', :js, feature_category let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let!(:merge_request) do - create(:merge_request_with_diffs, source_project: project, - author: user, - title: 'Bug NS-04', - head_pipeline: pipeline, - source_branch: pipeline.ref) + create( + :merge_request_with_diffs, + source_project: project, + author: user, + title: 'Bug NS-04', + head_pipeline: pipeline, + source_branch: pipeline.ref + ) end let(:pipeline) do - create(:ci_pipeline, project: project, - ref: 'master', - sha: project.repository.commit('master').id) + create( + :ci_pipeline, + project: project, + ref: 'master', + sha: project.repository.commit('master').id + ) end context 'when there is active pipeline for merge request' do diff --git a/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb index a4c03dc4e73..78814e36cfe 100644 --- a/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb +++ b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb @@ -25,11 +25,14 @@ RSpec.describe 'Merge request > User merges only if pipeline succeeds', :js, fea context 'when project has CI enabled' do let!(:pipeline) do - create(:ci_empty_pipeline, - project: project, - sha: merge_request.diff_head_sha, - ref: merge_request.source_branch, - status: status, head_pipeline_of: merge_request) + create( + :ci_empty_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + status: status, + head_pipeline_of: merge_request + ) end context 'when merge requests can only be merged if the pipeline succeeds' do diff --git a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb index 9a8384bfc9f..ebec8a6d2ea 100644 --- a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb +++ b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb @@ -6,17 +6,23 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js, featur let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) do - create(:merge_request_with_diffs, source_project: project, - author: user, - title: 'Bug NS-04', - merge_params: { force_remove_source_branch: '1' }) + create( + :merge_request_with_diffs, + source_project: project, + author: user, + title: 'Bug NS-04', + merge_params: { force_remove_source_branch: '1' } + ) end let(:pipeline) do - create(:ci_pipeline, project: project, - sha: merge_request.diff_head_sha, - ref: merge_request.source_branch, - head_pipeline_of: merge_request) + create( + :ci_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + head_pipeline_of: merge_request + ) end before do @@ -67,12 +73,14 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js, featur context 'when it was enabled and then canceled' do let(:merge_request) do - create(:merge_request_with_diffs, - :merge_when_pipeline_succeeds, - source_project: project, - title: 'Bug NS-04', - author: user, - merge_user: user) + create( + :merge_request_with_diffs, + :merge_when_pipeline_succeeds, + source_project: project, + title: 'Bug NS-04', + author: user, + merge_user: user + ) end before do @@ -88,11 +96,15 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js, featur context 'when merge when pipeline succeeds is enabled' do let(:merge_request) do - create(:merge_request_with_diffs, :simple, :merge_when_pipeline_succeeds, - source_project: project, - author: user, - merge_user: user, - title: 'MepMep') + create( + :merge_request_with_diffs, + :simple, + :merge_when_pipeline_succeeds, + source_project: project, + author: user, + merge_user: user, + title: 'MepMep' + ) end let!(:build) do diff --git a/spec/features/merge_request/user_opens_checkout_branch_modal_spec.rb b/spec/features/merge_request/user_opens_checkout_branch_modal_spec.rb index 601310cbacf..63f03ae64e0 100644 --- a/spec/features/merge_request/user_opens_checkout_branch_modal_spec.rb +++ b/spec/features/merge_request/user_opens_checkout_branch_modal_spec.rb @@ -20,13 +20,15 @@ RSpec.describe 'Merge request > User opens checkout branch modal', :js, feature_ let(:source_project) { fork_project(project, author, repository: true) } let(:merge_request) do - create(:merge_request, - source_project: source_project, - target_project: project, - source_branch: 'fix', - target_branch: 'master', - author: author, - allow_collaboration: true) + create( + :merge_request, + source_project: source_project, + target_project: project, + source_branch: 'fix', + target_branch: 'master', + author: author, + allow_collaboration: true + ) end it 'shows instructions' do diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb index d31777db42e..d1e00d730b7 100644 --- a/spec/features/merge_request/user_posts_notes_spec.rb +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -13,8 +13,7 @@ RSpec.describe 'Merge request > User posts notes', :js, feature_category: :code_ end let!(:note) do - create(:note_on_merge_request, :with_attachment, noteable: merge_request, - project: project) + create(:note_on_merge_request, :with_attachment, noteable: merge_request, project: project) end before do diff --git a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb index dae28cbb05c..e3be99254dc 100644 --- a/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb +++ b/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb @@ -144,9 +144,7 @@ RSpec.describe 'Merge request > User selects branches for new MR', :js, feature_ context 'when a new merge request has a pipeline' do let!(:pipeline) do - create(:ci_pipeline, sha: project.commit('fix').id, - ref: 'fix', - project: project) + create(:ci_pipeline, sha: project.commit('fix').id, ref: 'fix', project: project) end it 'shows pipelines for a new merge request' do diff --git a/spec/features/merge_request/user_squashes_merge_request_spec.rb b/spec/features/merge_request/user_squashes_merge_request_spec.rb index 63faf830f7e..5fd0f353e56 100644 --- a/spec/features/merge_request/user_squashes_merge_request_spec.rb +++ b/spec/features/merge_request/user_squashes_merge_request_spec.rb @@ -16,15 +16,19 @@ RSpec.describe 'User squashes a merge request', :js, feature_category: :code_rev latest_master_commits = project.repository.commits_between(original_head.sha, 'master').map(&:raw) - squash_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/), - message: a_string_starting_with(project.merge_requests.first.default_squash_commit_message), - author_name: user.name, - committer_name: user.name) - - merge_commit = an_object_having_attributes(sha: a_string_matching(/\h{40}/), - message: a_string_starting_with("Merge branch '#{source_branch}' into 'master'"), - author_name: user.name, - committer_name: user.name) + squash_commit = an_object_having_attributes( + sha: a_string_matching(/\h{40}/), + message: a_string_starting_with(project.merge_requests.first.default_squash_commit_message), + author_name: user.name, + committer_name: user.name + ) + + merge_commit = an_object_having_attributes( + sha: a_string_matching(/\h{40}/), + message: a_string_starting_with("Merge branch '#{source_branch}' into 'master'"), + author_name: user.name, + committer_name: user.name + ) expect(project.repository).not_to be_merged_to_root_ref(source_branch) expect(latest_master_commits).to match([squash_commit, merge_commit]) diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index efd88df0f97..6152d9f8259 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -303,13 +303,17 @@ RSpec.describe 'User comments on a diff', :js, feature_category: :code_review_wo "5 # heh" ] - expect_suggestion_has_content(suggestion_1, - suggestion_1_expected_changing_content, - suggestion_1_expected_suggested_content) - - expect_suggestion_has_content(suggestion_2, - suggestion_2_expected_changing_content, - suggestion_2_expected_suggested_content) + expect_suggestion_has_content( + suggestion_1, + suggestion_1_expected_changing_content, + suggestion_1_expected_suggested_content + ) + + expect_suggestion_has_content( + suggestion_2, + suggestion_2_expected_changing_content, + suggestion_2_expected_suggested_content + ) end end end diff --git a/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb b/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb index 5770f5ab94d..1232e19d22b 100644 --- a/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb +++ b/spec/features/merge_request/user_tries_to_access_private_project_info_through_new_mr_spec.rb @@ -3,19 +3,27 @@ require 'spec_helper' RSpec.describe 'Merge Request > User tries to access private project information through the new mr page', -feature_category: :code_review_workflow do + feature_category: :code_review_workflow do let(:current_user) { create(:user) } let(:private_project) do - create(:project, :public, :repository, - path: 'nothing-to-see-here', - name: 'nothing to see here', - repository_access_level: ProjectFeature::PRIVATE) + create( + :project, + :public, + :repository, + path: 'nothing-to-see-here', + name: 'nothing to see here', + repository_access_level: ProjectFeature::PRIVATE + ) end let(:owned_project) do - create(:project, :public, :repository, - namespace: current_user.namespace, - creator: current_user) + create( + :project, + :public, + :repository, + namespace: current_user.namespace, + creator: current_user + ) end context 'when the user enters the querystring info for the other project' do diff --git a/spec/features/merge_request/user_uses_quick_actions_spec.rb b/spec/features/merge_request/user_uses_quick_actions_spec.rb index 1ec86948065..1c63f5b56b0 100644 --- a/spec/features/merge_request/user_uses_quick_actions_spec.rb +++ b/spec/features/merge_request/user_uses_quick_actions_spec.rb @@ -8,7 +8,7 @@ require 'spec_helper' # Because this kind of spec takes more time to run there is no need to add new ones # for each existing quick action unless they test something not tested by existing tests. RSpec.describe 'Merge request > User uses quick actions', :js, :use_clean_rails_redis_caching, -feature_category: :code_review_workflow do + feature_category: :code_review_workflow do include Features::NotesHelpers let(:project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index 371c40b40a5..f594e39b2b7 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -14,44 +14,52 @@ RSpec.describe 'Merge requests > User lists merge requests', feature_category: : let(:user5) { create(:user) } before do - @fix = create(:merge_request, - title: 'fix', - source_project: project, - source_branch: 'fix', - assignees: [user], - reviewers: [user, user2, user3, user4, user5], - milestone: create(:milestone, project: project, due_date: '2013-12-11'), - created_at: 1.minute.ago, - updated_at: 1.minute.ago) + @fix = create( + :merge_request, + title: 'fix', + source_project: project, + source_branch: 'fix', + assignees: [user], + reviewers: [user, user2, user3, user4, user5], + milestone: create(:milestone, project: project, due_date: '2013-12-11'), + created_at: 1.minute.ago, + updated_at: 1.minute.ago + ) @fix.metrics.update!(merged_at: 10.seconds.ago, latest_closed_at: 20.seconds.ago) - @markdown = create(:merge_request, - title: 'markdown', - source_project: project, - source_branch: 'markdown', - assignees: [user], - reviewers: [user, user2, user3, user4], - milestone: create(:milestone, project: project, due_date: '2013-12-12'), - created_at: 2.minutes.ago, - updated_at: 2.minutes.ago, - state: 'merged') + @markdown = create( + :merge_request, + title: 'markdown', + source_project: project, + source_branch: 'markdown', + assignees: [user], + reviewers: [user, user2, user3, user4], + milestone: create(:milestone, project: project, due_date: '2013-12-12'), + created_at: 2.minutes.ago, + updated_at: 2.minutes.ago, + state: 'merged' + ) @markdown.metrics.update!(merged_at: 10.minutes.ago, latest_closed_at: 10.seconds.ago) - @merge_test = create(:merge_request, - title: 'merge-test', - source_project: project, - source_branch: 'merge-test', - created_at: 3.minutes.ago, - updated_at: 10.seconds.ago) + @merge_test = create( + :merge_request, + title: 'merge-test', + source_project: project, + source_branch: 'merge-test', + created_at: 3.minutes.ago, + updated_at: 10.seconds.ago + ) @merge_test.metrics.update!(merged_at: 10.seconds.ago, latest_closed_at: 10.seconds.ago) - @feature = create(:merge_request, - title: 'feature', - source_project: project, - source_branch: 'feautre', - created_at: 2.minutes.ago, - updated_at: 1.minute.ago, - state: 'merged') + @feature = create( + :merge_request, + title: 'feature', + source_project: project, + source_branch: 'feautre', + created_at: 2.minutes.ago, + updated_at: 1.minute.ago, + state: 'merged' + ) @feature.metrics.update!(merged_at: 10.seconds.ago, latest_closed_at: 10.minutes.ago) end @@ -134,8 +142,7 @@ RSpec.describe 'Merge requests > User lists merge requests', feature_category: : label = create(:label, project: project) create(:label_link, label: label, target: @fix) - visit_merge_requests(project, label_name: [label.name], - sort: sort_value_milestone) + visit_merge_requests(project, label_name: [label.name], sort: sort_value_milestone) expect(first_merge_request).to include('fix') expect(count_merge_requests).to eq(1) @@ -160,8 +167,7 @@ RSpec.describe 'Merge requests > User lists merge requests', feature_category: : end it 'sorts by milestone due date' do - visit_merge_requests(project, label_name: [label.name, label2.name], - sort: sort_value_milestone) + visit_merge_requests(project, label_name: [label.name, label2.name], sort: sort_value_milestone) expect(first_merge_request).to include('fix') expect(count_merge_requests).to eq(1) @@ -169,9 +175,12 @@ RSpec.describe 'Merge requests > User lists merge requests', feature_category: : context 'filter on assignee and' do it 'sorts by milestone due date' do - visit_merge_requests(project, label_name: [label.name, label2.name], - assignee_id: user.id, - sort: sort_value_milestone) + visit_merge_requests( + project, + label_name: [label.name, label2.name], + assignee_id: user.id, + sort: sort_value_milestone + ) expect(first_merge_request).to include('fix') expect(count_merge_requests).to eq(1) diff --git a/spec/features/merge_requests/user_views_open_merge_requests_spec.rb b/spec/features/merge_requests/user_views_open_merge_requests_spec.rb index 1a2024a5511..0021f701290 100644 --- a/spec/features/merge_requests/user_views_open_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_views_open_merge_requests_spec.rb @@ -57,10 +57,12 @@ RSpec.describe 'User views open merge requests', feature_category: :code_review_ let!(:build) { create :ci_build, pipeline: pipeline } let(:merge_request) do - create(:merge_request_with_diffs, - source_project: project, - target_project: project, - source_branch: 'merge-test') + create( + :merge_request_with_diffs, + source_project: project, + target_project: project, + source_branch: 'merge-test' + ) end let(:pipeline) do diff --git a/spec/features/projects/compare_spec.rb b/spec/features/projects/compare_spec.rb index beb5fa7822b..eff538513c1 100644 --- a/spec/features/projects/compare_spec.rb +++ b/spec/features/projects/compare_spec.rb @@ -24,7 +24,7 @@ RSpec.describe "Compare", :js, feature_category: :groups_and_projects do click_button 'Compare' - expect(page).to have_content 'Commits' + expect(page).to have_content 'Commits on Source' expect(page).to have_link 'Create merge request' end end @@ -53,7 +53,7 @@ RSpec.describe "Compare", :js, feature_category: :groups_and_projects do select_using_dropdown('to', RepoHelpers.sample_commit.id, commit: true) click_button 'Compare' - expect(page).to have_content 'Commits (1)' + expect(page).to have_content 'Commits on Source (1)' expect(page).to have_content "Showing 2 changed files" diff = first('.js-unfold') @@ -85,7 +85,7 @@ RSpec.describe "Compare", :js, feature_category: :groups_and_projects do click_button 'Compare' - expect(page).to have_content 'Commits (1)' + expect(page).to have_content 'Commits on Source (1)' expect(page).to have_content 'Showing 1 changed file with 5 additions and 0 deletions' expect(page).to have_link 'View open merge request', href: project_merge_request_path(project, merge_request) expect(page).not_to have_link 'Create merge request' @@ -136,14 +136,14 @@ RSpec.describe "Compare", :js, feature_category: :groups_and_projects do visit project_compare_index_path(project, from: "feature", to: "master") click_button('Compare') - expect(page).to have_content 'Commits (29)' + expect(page).to have_content 'Commits on Source (29)' # go to the second page within(".files .gl-pagination") do click_on("2") end - expect(page).not_to have_content 'Commits (29)' + expect(page).not_to have_content 'Commits on Source (29)' end end end @@ -159,7 +159,7 @@ RSpec.describe "Compare", :js, feature_category: :groups_and_projects do expect(find(".js-compare-to-dropdown .gl-dropdown-button-text")).to have_content("v1.1.0") click_button "Compare" - expect(page).to have_content "Commits" + expect(page).to have_content "Commits on Source" end end diff --git a/spec/frontend/projects/compare/components/app_spec.js b/spec/frontend/projects/compare/components/app_spec.js index f8c10311974..6cc76d4a573 100644 --- a/spec/frontend/projects/compare/components/app_spec.js +++ b/spec/frontend/projects/compare/components/app_spec.js @@ -1,24 +1,37 @@ -import { GlButton, GlCollapsibleListbox } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; +import { GlIcon, GlLink, GlSprintf, GlFormGroup, GlFormRadioGroup, GlFormRadio } from '@gitlab/ui'; import { nextTick } from 'vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import CompareApp from '~/projects/compare/components/app.vue'; +import { + COMPARE_REVISIONS_DOCS_URL, + I18N, + COMPARE_OPTIONS, + COMPARE_OPTIONS_INPUT_NAME, +} from '~/projects/compare/constants'; import RevisionCard from '~/projects/compare/components/revision_card.vue'; +import { createMockDirective, getBinding } from 'helpers/vue_mock_directive'; import { appDefaultProps as defaultProps } from './mock_data'; jest.mock('~/lib/utils/csrf', () => ({ token: 'mock-csrf-token' })); describe('CompareApp component', () => { let wrapper; - const findSourceRevisionCard = () => wrapper.find('[data-testid="sourceRevisionCard"]'); - const findTargetRevisionCard = () => wrapper.find('[data-testid="targetRevisionCard"]'); + const findSourceRevisionCard = () => wrapper.findByTestId('sourceRevisionCard'); + const findTargetRevisionCard = () => wrapper.findByTestId('targetRevisionCard'); const createComponent = (props = {}) => { - wrapper = shallowMount(CompareApp, { + wrapper = shallowMountExtended(CompareApp, { propsData: { ...defaultProps, ...props, }, - stubs: { GlCollapsibleListbox }, + directives: { + GlTooltip: createMockDirective('gl-tooltip'), + }, + stubs: { + GlSprintf, + GlFormRadioGroup, + }, }); }; @@ -38,6 +51,21 @@ describe('CompareApp component', () => { ); }); + it('renders title', () => { + const title = wrapper.find('h1'); + expect(title.text()).toBe(I18N.title); + }); + + it('renders subtitle', () => { + const subtitle = wrapper.find('p'); + expect(subtitle.text()).toMatchInterpolatedText(I18N.subtitle); + }); + + it('renders link to docs', () => { + const docsLink = wrapper.findComponent(GlLink); + expect(docsLink.attributes('href')).toBe(COMPARE_REVISIONS_DOCS_URL); + }); + it('contains the correct form attributes', () => { expect(wrapper.attributes('action')).toBe(defaultProps.projectCompareIndexPath); expect(wrapper.attributes('method')).toBe('POST'); @@ -49,20 +77,16 @@ describe('CompareApp component', () => { ); }); - it('has ellipsis', () => { - expect(wrapper.find('[data-testid="ellipsis"]').exists()).toBe(true); - }); - it('render Source and Target BranchDropdown components', () => { const revisionCards = wrapper.findAllComponents(RevisionCard); expect(revisionCards.length).toBe(2); - expect(revisionCards.at(0).props('revisionText')).toBe('Source'); - expect(revisionCards.at(1).props('revisionText')).toBe('Target'); + expect(revisionCards.at(0).props('revisionText')).toBe(I18N.source); + expect(revisionCards.at(1).props('revisionText')).toBe(I18N.target); }); describe('compare button', () => { - const findCompareButton = () => wrapper.findComponent(GlButton); + const findCompareButton = () => wrapper.findByTestId('compare-button'); it('renders button', () => { expect(findCompareButton().exists()).toBe(true); @@ -110,14 +134,19 @@ describe('CompareApp component', () => { }); describe('swap revisions button', () => { - const findSwapRevisionsButton = () => wrapper.find('[data-testid="swapRevisionsButton"]'); + const findSwapRevisionsButton = () => wrapper.findByTestId('swapRevisionsButton'); it('renders the swap revisions button', () => { expect(findSwapRevisionsButton().exists()).toBe(true); }); - it('has the correct text', () => { - expect(findSwapRevisionsButton().text()).toBe('Swap revisions'); + it('renders icon', () => { + expect(findSwapRevisionsButton().findComponent(GlIcon).props('name')).toBe('substitute'); + }); + + it('has tooltip', () => { + const tooltip = getBinding(findSwapRevisionsButton().element, 'gl-tooltip'); + expect(tooltip.value).toBe(I18N.swapRevisions); }); it('swaps revisions when clicked', async () => { @@ -130,39 +159,43 @@ describe('CompareApp component', () => { }); }); - describe('mode dropdown', () => { - const findGlDropdown = () => wrapper.findComponent(GlCollapsibleListbox); - const findEnableStraightModeButton = () => - wrapper.findComponent('[data-testid="listbox-item-true"]'); - const findDisableStraightModeButton = () => - wrapper.findComponent('[data-testid="listbox-item-false"]'); + describe('compare options', () => { + const findGroup = () => wrapper.findComponent(GlFormGroup); + const findOptionsGroup = () => wrapper.findComponent(GlFormRadioGroup); - it('renders the mode dropdown button', () => { - expect(findGlDropdown().exists()).toBe(true); + const findOptions = () => wrapper.findAllComponents(GlFormRadio); + + it('renders label for the compare options', () => { + expect(findGroup().attributes('label')).toBe(I18N.optionsLabel); }); - it('has the correct text', () => { - expect(findEnableStraightModeButton().text()).toBe('...'); - expect(findDisableStraightModeButton().text()).toBe('..'); + it('correct input name', () => { + expect(findOptionsGroup().attributes('name')).toBe(COMPARE_OPTIONS_INPUT_NAME); + }); + + it('renders "only incoming changes" option', () => { + expect(findOptions().at(0).text()).toBe(COMPARE_OPTIONS[0].text); + }); + + it('renders "since source was created" option', () => { + expect(findOptions().at(1).text()).toBe(COMPARE_OPTIONS[1].text); }); it('straight mode button when clicked', async () => { - expect(wrapper.find('input[name="straight"]').attributes('value')).toBe('false'); + expect(wrapper.props('straight')).toBe(false); + expect(wrapper.vm.isStraight).toBe(false); - findGlDropdown().vm.$emit('select', 'true'); - await nextTick(); + findOptionsGroup().vm.$emit('input', COMPARE_OPTIONS[1].value); - expect(wrapper.find('input[name="straight"]').attributes('value')).toBe('true'); - findGlDropdown().vm.$emit('select', 'false'); await nextTick(); - expect(wrapper.find('input[name="straight"]').attributes('value')).toBe('false'); + expect(wrapper.vm.isStraight).toBe(true); }); }); describe('merge request buttons', () => { - const findProjectMrButton = () => wrapper.find('[data-testid="projectMrButton"]'); - const findCreateMrButton = () => wrapper.find('[data-testid="createMrButton"]'); + const findProjectMrButton = () => wrapper.findByTestId('projectMrButton'); + const findCreateMrButton = () => wrapper.findByTestId('createMrButton'); it('does not have merge request buttons', () => { createComponent(); diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index d760e4ddf28..d5f60e62b06 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -311,7 +311,8 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end end - describe 'POST /projects/:id/pipeline_schedules' do + # Move this from `shared_context` to `describe` when `ci_refactoring_pipeline_schedule_create_service` is removed. + shared_context 'POST /projects/:id/pipeline_schedules' do # rubocop:disable RSpec/ContextWording let(:params) { attributes_for(:ci_pipeline_schedule) } context 'authenticated user with valid permissions' do @@ -368,7 +369,8 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end end - describe 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' do + # Move this from `shared_context` to `describe` when `ci_refactoring_pipeline_schedule_create_service` is removed. + shared_context 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' do let(:pipeline_schedule) do create(:ci_pipeline_schedule, project: project, owner: developer) end @@ -437,6 +439,18 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end end + it_behaves_like 'POST /projects/:id/pipeline_schedules' + it_behaves_like 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' + + context 'when the FF ci_refactoring_pipeline_schedule_create_service is disabled' do + before do + stub_feature_flags(ci_refactoring_pipeline_schedule_create_service: false) + end + + it_behaves_like 'POST /projects/:id/pipeline_schedules' + it_behaves_like 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id' + end + describe 'POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do let(:pipeline_schedule) do create(:ci_pipeline_schedule, project: project, owner: developer) diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_create_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_create_spec.rb index 4a45d255d99..a89fc6eb6f1 100644 --- a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_create_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'PipelineSchedulecreate' do +RSpec.describe 'PipelineSchedulecreate', feature_category: :continuous_integration do include GraphqlHelpers let_it_be(:user) { create(:user) } @@ -68,7 +68,8 @@ RSpec.describe 'PipelineSchedulecreate' do end end - context 'when authorized' do + # Move this from `shared_context` to `context` when `ci_refactoring_pipeline_schedule_create_service` is removed. + shared_context 'when authorized' do # rubocop:disable RSpec/ContextWording before do project.add_developer(user) end @@ -148,4 +149,14 @@ RSpec.describe 'PipelineSchedulecreate' do end end end + + it_behaves_like 'when authorized' + + context 'when the FF ci_refactoring_pipeline_schedule_create_service is disabled' do + before do + stub_feature_flags(ci_refactoring_pipeline_schedule_create_service: false) + end + + it_behaves_like 'when authorized' + end end diff --git a/spec/services/ci/pipeline_schedules/create_service_spec.rb b/spec/services/ci/pipeline_schedules/create_service_spec.rb new file mode 100644 index 00000000000..a01c71432c3 --- /dev/null +++ b/spec/services/ci/pipeline_schedules/create_service_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::PipelineSchedules::CreateService, feature_category: :continuous_integration do + let_it_be(:user) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + + before_all do + project.add_maintainer(user) + project.add_reporter(reporter) + end + + describe "execute" do + context 'when user does not have permission' do + subject(:service) { described_class.new(project, reporter, {}) } + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + + error_message = _('The current user is not authorized to create the pipeline schedule') + expect(result.message).to match_array([error_message]) + expect(result.payload.errors).to match_array([error_message]) + end + end + + context 'when user has permission' do + let(:params) do + { + description: 'desc', + ref: 'patch-x', + active: false, + cron: '*/1 * * * *', + cron_timezone: 'UTC' + } + end + + subject(:service) { described_class.new(project, user, params) } + + it 'saves values with passed params' do + result = service.execute + + expect(result.payload).to have_attributes( + description: 'desc', + ref: 'patch-x', + active: false, + cron: '*/1 * * * *', + cron_timezone: 'UTC' + ) + end + + it 'returns ServiceResponse.success' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.success?).to be(true) + end + + context 'when schedule save fails' do + subject(:service) { described_class.new(project, user, {}) } + + before do + errors = ActiveModel::Errors.new(project) + errors.add(:base, 'An error occurred') + + allow_next_instance_of(Ci::PipelineSchedule) do |instance| + allow(instance).to receive(:save).and_return(false) + allow(instance).to receive(:errors).and_return(errors) + end + end + + it 'returns ServiceResponse.error' do + result = service.execute + + expect(result).to be_a(ServiceResponse) + expect(result.error?).to be(true) + expect(result.message).to match_array(['An error occurred']) + end + end + end + end +end diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb index 838f49f6dea..6899f6c7d63 100644 --- a/spec/services/ci/pipeline_schedules/update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -22,7 +22,10 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo expect(result).to be_a(ServiceResponse) expect(result.error?).to be(true) - expect(result.message).to eq(_('The current user is not authorized to update the pipeline schedule')) + + error_message = _('The current user is not authorized to update the pipeline schedule') + expect(result.message).to match_array([error_message]) + expect(pipeline_schedule.errors).to match_array([error_message]) end end @@ -58,7 +61,7 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo subject(:service) { described_class.new(pipeline_schedule, user, {}) } before do - allow(pipeline_schedule).to receive(:update).and_return(false) + allow(pipeline_schedule).to receive(:save).and_return(false) errors = ActiveModel::Errors.new(pipeline_schedule) errors.add(:base, 'An error occurred') diff --git a/spec/support/shared_examples/features/work_items_shared_examples.rb b/spec/support/shared_examples/features/work_items_shared_examples.rb index 128bd28410c..4c15b682458 100644 --- a/spec/support/shared_examples/features/work_items_shared_examples.rb +++ b/spec/support/shared_examples/features/work_items_shared_examples.rb @@ -166,7 +166,8 @@ RSpec.shared_examples 'work items comments' do |type| end RSpec.shared_examples 'work items assignees' do - it 'successfully assigns the current user by searching' do + it 'successfully assigns the current user by searching', + quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/413074' do # The button is only when the mouse is over the input find('[data-testid="work-item-assignees-input"]').fill_in(with: user.username) wait_for_requests |