diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-12 15:13:54 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-12 15:13:54 +0000 |
| commit | 98638cd5e43611aac2193a5c2f80f72374040430 (patch) | |
| tree | 6605f0f284efed1d05708b3799f093eb5e305a8f | |
| parent | 43d816ebc20da6ff959176248c70d8c4c7c9345a (diff) | |
| download | gitlab-ce-98638cd5e43611aac2193a5c2f80f72374040430.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
202 files changed, 3176 insertions, 1284 deletions
diff --git a/.gitlab/ci/qa-common/main.gitlab-ci.yml b/.gitlab/ci/qa-common/main.gitlab-ci.yml index 7a812ed5dad..025ea61bcdb 100644 --- a/.gitlab/ci/qa-common/main.gitlab-ci.yml +++ b/.gitlab/ci/qa-common/main.gitlab-ci.yml @@ -216,7 +216,7 @@ stages: bundle exec relate-failure-issue \ --input-files "${CI_PROJECT_DIR}/gitlab-qa-run-*/**/rspec-*.json" \ --project "gitlab-org/gitlab" \ - --token "${RELATE_TEST_FAILURE_TOKEN}" + --token "${QA_RELATE_FAILURE_ISSUE_TOKEN}" .generate-test-session: extends: @@ -230,7 +230,8 @@ stages: --input-files "${CI_PROJECT_DIR}/gitlab-qa-run-*/**/rspec-*.json" \ --project "gitlab-org/quality/testcase-sessions" \ --token "${QA_TEST_SESSION_TOKEN}" \ - --ci-project-token "${GENERATE_TEST_SESSION_READ_API_REPORTER_TOKEN}" > REPORT_ISSUE_URL + --ci-project-token "${GENERATE_TEST_SESSION_READ_API_REPORTER_TOKEN}" \ + --issue-url-file REPORT_ISSUE_URL artifacts: when: always expire_in: 1d diff --git a/.rubocop_todo/lint/empty_block.yml b/.rubocop_todo/lint/empty_block.yml index ae61e7cb862..e111703ce3b 100644 --- a/.rubocop_todo/lint/empty_block.yml +++ b/.rubocop_todo/lint/empty_block.yml @@ -108,7 +108,6 @@ Lint/EmptyBlock: - 'spec/lib/gitlab/database/migrations/instrumentation_spec.rb' - 'spec/lib/gitlab/database/migrations/lock_retries_helpers_spec.rb' - 'spec/lib/gitlab/database/migrations/observers/transaction_duration_spec.rb' - - 'spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb' - 'spec/lib/gitlab/database/shared_model_spec.rb' - 'spec/lib/gitlab/database/with_lock_retries_outside_transaction_spec.rb' - 'spec/lib/gitlab/database/with_lock_retries_spec.rb' diff --git a/.rubocop_todo/performance/map_compact.yml b/.rubocop_todo/performance/map_compact.yml index 21cf14275f8..2ea13f71fa4 100644 --- a/.rubocop_todo/performance/map_compact.yml +++ b/.rubocop_todo/performance/map_compact.yml @@ -101,7 +101,6 @@ Performance/MapCompact: - 'lib/gitlab/ci/reports/security/finding.rb' - 'lib/gitlab/ci/reports/test_suite_summary.rb' - 'lib/gitlab/database/load_balancing/service_discovery.rb' - - 'lib/gitlab/database/obsolete_ignored_columns.rb' - 'lib/gitlab/git/commit.rb' - 'lib/gitlab/git/conflict/file.rb' - 'lib/gitlab/git/rugged_impl/commit.rb' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 943fded777c..1c887978de8 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -3509,7 +3509,6 @@ RSpec/MissingFeatureCategory: - 'spec/lib/gitlab/database/migrations/sidekiq_helpers_spec.rb' - 'spec/lib/gitlab/database/migrations/test_background_runner_spec.rb' - 'spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb' - - 'spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb' - 'spec/lib/gitlab/database/partitioning/detached_partition_dropper_spec.rb' - 'spec/lib/gitlab/database/partitioning/monthly_strategy_spec.rb' - 'spec/lib/gitlab/database/partitioning/partition_manager_spec.rb' @@ -341,7 +341,7 @@ gem 'pg_query', '~> 2.2', '>= 2.2.1' gem 'premailer-rails', '~> 1.10.3' -gem 'gitlab-labkit', '~> 0.31.1' +gem 'gitlab-labkit', '~> 0.32.0' gem 'thrift', '>= 0.16.0' # I18n diff --git a/Gemfile.checksum b/Gemfile.checksum index 0cc74cbdd8a..ebb89765d59 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -213,7 +213,7 @@ {"name":"gitlab-dangerfiles","version":"3.10.0","platform":"ruby","checksum":"df4cfe051f52529c0256346d89d06d5ef2bb630928754eb620b5233eb9b14041"}, {"name":"gitlab-experiment","version":"0.7.1","platform":"ruby","checksum":"166dddb3aa83428bcaa93c35684ed01dc4d61f321fd2ae40b020806dc54a7824"}, {"name":"gitlab-fog-azure-rm","version":"1.7.0","platform":"ruby","checksum":"969c67943c54ad4c259a6acd040493f13922fbdf2211bb4eca00e71505263dc2"}, -{"name":"gitlab-labkit","version":"0.31.1","platform":"ruby","checksum":"3e3a39370966b5d2739c2d9d9005c0ea27541d32cb7292e856e8bd74c720bffb"}, +{"name":"gitlab-labkit","version":"0.32.0","platform":"ruby","checksum":"f30a33edc53586c059fd0b5d748acd2a12be75f6fc72a87669a0a08fe922866e"}, {"name":"gitlab-license","version":"2.2.2","platform":"ruby","checksum":"2ccbc763828d013524b0b3b9ee671e58d5277693e5ffb2e5463cbac87e8aed1e"}, {"name":"gitlab-mail_room","version":"0.0.23","platform":"ruby","checksum":"23564fa4dab24ec5011d4c64a801fc0228301d5b0f046a26a1d8e96e36c19997"}, {"name":"gitlab-markup","version":"1.9.0","platform":"ruby","checksum":"7eda045a08ec2d110084252fa13a8c9eac8bdac0e302035ca7db4b82bcbd7ed4"}, diff --git a/Gemfile.lock b/Gemfile.lock index fb7ff0a481e..d26ca466ce6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -602,7 +602,7 @@ GEM fog-json (~> 1.2.0) mime-types ms_rest_azure (~> 0.12.0) - gitlab-labkit (0.31.1) + gitlab-labkit (0.32.0) actionpack (>= 5.0.0, < 8.0.0) activesupport (>= 5.0.0, < 8.0.0) grpc (>= 1.37) @@ -1747,7 +1747,7 @@ DEPENDENCIES gitlab-dangerfiles (~> 3.10.0) gitlab-experiment (~> 0.7.1) gitlab-fog-azure-rm (~> 1.7.0) - gitlab-labkit (~> 0.31.1) + gitlab-labkit (~> 0.32.0) gitlab-license (~> 2.2.1) gitlab-mail_room (~> 0.0.23) gitlab-markup (~> 1.9.0) diff --git a/app/assets/javascripts/code_review/signals.js b/app/assets/javascripts/code_review/signals.js new file mode 100644 index 00000000000..101b7996bb5 --- /dev/null +++ b/app/assets/javascripts/code_review/signals.js @@ -0,0 +1,51 @@ +import createApolloClient from '../lib/graphql'; + +import { getDerivedMergeRequestInformation } from '../diffs/utils/merge_request'; +import { EVT_MR_PREPARED } from '../diffs/constants'; + +import getMr from '../graphql_shared/queries/merge_request.query.graphql'; +import mrPreparation from '../graphql_shared/subscriptions/merge_request_prepared.subscription.graphql'; + +function required(name) { + throw new Error(`${name} is a required argument`); +} + +async function observeMergeRequestFinishingPreparation({ apollo, signaler }) { + const { namespace, project, id: iid } = getDerivedMergeRequestInformation({ + endpoint: document.location.pathname, + }); + const projectPath = `${namespace}/${project}`; + + if (projectPath && iid) { + const currentStatus = await apollo.query({ + query: getMr, + variables: { projectPath, iid }, + }); + const { id: gqlMrId, preparedAt } = currentStatus.data.project.mergeRequest; + let preparationObservable; + let preparationSubscriber; + + if (!preparedAt) { + preparationObservable = apollo.subscribe({ + query: mrPreparation, + variables: { + issuableId: gqlMrId, + }, + }); + + preparationSubscriber = preparationObservable.subscribe((preparationUpdate) => { + if (preparationUpdate.data.mergeRequestMergeStatusUpdated?.preparedAt) { + signaler.$emit(EVT_MR_PREPARED); + preparationSubscriber.unsubscribe(); + } + }); + } + } +} + +export async function start({ + signalBus = required('signalBus'), + apolloClient = createApolloClient(), +} = {}) { + await observeMergeRequestFinishingPreparation({ signaler: signalBus, apollo: apolloClient }); +} diff --git a/app/assets/javascripts/content_editor/components/bubble_menus/formatting_bubble_menu.vue b/app/assets/javascripts/content_editor/components/bubble_menus/formatting_bubble_menu.vue deleted file mode 100644 index cef446c4cf8..00000000000 --- a/app/assets/javascripts/content_editor/components/bubble_menus/formatting_bubble_menu.vue +++ /dev/null @@ -1,130 +0,0 @@ -<script> -import { GlButtonGroup } from '@gitlab/ui'; -import { BUBBLE_MENU_TRACKING_ACTION } from '../../constants'; -import trackUIControl from '../../services/track_ui_control'; -import Paragraph from '../../extensions/paragraph'; -import Heading from '../../extensions/heading'; -import Audio from '../../extensions/audio'; -import Video from '../../extensions/video'; -import Image from '../../extensions/image'; -import DrawioDiagram from '../../extensions/drawio_diagram'; -import ToolbarButton from '../toolbar_button.vue'; -import BubbleMenu from './bubble_menu.vue'; - -export default { - components: { - BubbleMenu, - GlButtonGroup, - ToolbarButton, - }, - inject: ['tiptapEditor'], - methods: { - trackToolbarControlExecution({ contentType, value }) { - trackUIControl({ action: BUBBLE_MENU_TRACKING_ACTION, property: contentType, value }); - }, - - shouldShow: ({ editor, from, to }) => { - if (from === to) return false; - - const includes = [Paragraph.name, Heading.name]; - const excludes = [Image.name, Audio.name, Video.name, DrawioDiagram.name]; - - return ( - includes.some((type) => editor.isActive(type)) && - !excludes.some((type) => editor.isActive(type)) - ); - }, - }, -}; -</script> -<template> - <bubble-menu - data-testid="formatting-bubble-menu" - class="gl-shadow gl-rounded-base gl-bg-white" - :should-show="shouldShow" - :plugin-key="'formatting'" - > - <gl-button-group> - <toolbar-button - data-testid="bold" - content-type="bold" - icon-name="bold" - editor-command="toggleBold" - category="tertiary" - size="medium" - :label="__('Bold text')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="italic" - content-type="italic" - icon-name="italic" - editor-command="toggleItalic" - category="tertiary" - size="medium" - :label="__('Italic text')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="strike" - content-type="strike" - icon-name="strikethrough" - editor-command="toggleStrike" - category="tertiary" - size="medium" - :label="__('Strikethrough')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="code" - content-type="code" - icon-name="code" - editor-command="toggleCode" - category="tertiary" - size="medium" - :label="__('Code')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="superscript" - content-type="superscript" - icon-name="superscript" - editor-command="toggleSuperscript" - category="tertiary" - size="medium" - :label="__('Superscript')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="subscript" - content-type="subscript" - icon-name="subscript" - editor-command="toggleSubscript" - category="tertiary" - size="medium" - :label="__('Subscript')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="highlight" - content-type="highlight" - icon-name="highlight" - editor-command="toggleHighlight" - category="tertiary" - size="medium" - :label="__('Highlight')" - @execute="trackToolbarControlExecution" - /> - <toolbar-button - data-testid="link" - content-type="link" - icon-name="link" - editor-command="editLink" - category="tertiary" - size="medium" - :label="__('Insert link')" - @execute="trackToolbarControlExecution" - /> - </gl-button-group> - </bubble-menu> -</template> diff --git a/app/assets/javascripts/content_editor/components/content_editor.vue b/app/assets/javascripts/content_editor/components/content_editor.vue index 7fee798c65a..4c5bbca4110 100644 --- a/app/assets/javascripts/content_editor/components/content_editor.vue +++ b/app/assets/javascripts/content_editor/components/content_editor.vue @@ -8,7 +8,6 @@ import { ALERT_EVENT, TIPTAP_AUTOFOCUS_OPTIONS } from '../constants'; import ContentEditorAlert from './content_editor_alert.vue'; import ContentEditorProvider from './content_editor_provider.vue'; import EditorStateObserver from './editor_state_observer.vue'; -import FormattingBubbleMenu from './bubble_menus/formatting_bubble_menu.vue'; import CodeBlockBubbleMenu from './bubble_menus/code_block_bubble_menu.vue'; import LinkBubbleMenu from './bubble_menus/link_bubble_menu.vue'; import MediaBubbleMenu from './bubble_menus/media_bubble_menu.vue'; @@ -24,7 +23,6 @@ export default { ContentEditorProvider, TiptapEditorContent, FormattingToolbar, - FormattingBubbleMenu, CodeBlockBubbleMenu, LinkBubbleMenu, MediaBubbleMenu, @@ -225,7 +223,6 @@ export default { > <formatting-toolbar ref="toolbar" @enableMarkdownEditor="$emit('enableMarkdownEditor')" /> <div class="gl-relative"> - <formatting-bubble-menu /> <code-block-bubble-menu /> <link-bubble-menu /> <media-bubble-menu /> diff --git a/app/assets/javascripts/content_editor/components/formatting_toolbar.vue b/app/assets/javascripts/content_editor/components/formatting_toolbar.vue index 1ef38df0f78..fac259cf6a1 100644 --- a/app/assets/javascripts/content_editor/components/formatting_toolbar.vue +++ b/app/assets/javascripts/content_editor/components/formatting_toolbar.vue @@ -54,6 +54,14 @@ export default { @execute="trackToolbarControlExecution" /> <toolbar-button + data-testid="strike" + content-type="strike" + icon-name="strikethrough" + editor-command="toggleStrike" + :label="__('Strikethrough')" + @execute="trackToolbarControlExecution" + /> + <toolbar-button data-testid="blockquote" content-type="blockquote" icon-name="quote" diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index bfabac3123b..02307150e2f 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -41,6 +41,7 @@ import { TRACKING_WHITESPACE_HIDE, TRACKING_SINGLE_FILE_MODE, TRACKING_MULTIPLE_FILES_MODE, + EVT_MR_PREPARED, } from '../constants'; import diffsEventHub from '../event_hub'; @@ -470,8 +471,10 @@ export default { diffsEventHub.$on('diffFilesModified', this.setDiscussions); notesEventHub.$on('fetchedNotesData', this.rereadNoteHash); } + diffsEventHub.$on(EVT_MR_PREPARED, this.fetchData); }, unsubscribeFromEvents() { + diffsEventHub.$off(EVT_MR_PREPARED, this.fetchData); if (this.glFeatures.singleFileFileByFile) { notesEventHub.$off('fetchedNotesData', this.rereadNoteHash); diffsEventHub.$off('diffFilesModified', this.setDiscussions); diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index a459def6b4b..063e36fa7fb 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -79,6 +79,7 @@ export const RENAMED_DIFF_TRANSITIONS = { }; // MR Diffs known events +export const EVT_MR_PREPARED = 'mr:asyncPreparationFinished'; export const EVT_EXPAND_ALL_FILES = 'mr:diffs:expandAllFiles'; export const EVT_PERF_MARK_FILE_TREE_START = 'mr:diffs:perf:fileTreeStart'; export const EVT_PERF_MARK_FILE_TREE_END = 'mr:diffs:perf:fileTreeEnd'; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 2a7b4da684b..0668551902a 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -49,6 +49,7 @@ import { TRACKING_CLICK_SINGLE_FILE_SETTING, TRACKING_SINGLE_FILE_MODE, TRACKING_MULTIPLE_FILES_MODE, + EVT_MR_PREPARED, } from '../constants'; import { DISCUSSION_SINGLE_DIFF_FAILED, LOAD_SINGLE_DIFF_FAILED } from '../i18n'; import eventHub from '../event_hub'; @@ -287,10 +288,14 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { }) .catch((error) => { if (error.response.status === HTTP_STATUS_NOT_FOUND) { - createAlert({ - message: __('Building your merge request. Wait a few moments, then refresh this page.'), + const alert = createAlert({ + message: __( + 'Building your merge request… This page will update when the build is complete.', + ), variant: VARIANT_WARNING, }); + + eventHub.$once(EVT_MR_PREPARED, () => alert.dismiss()); } else { throw error; } diff --git a/app/assets/javascripts/environments/components/deploy_freeze_alert.vue b/app/assets/javascripts/environments/components/deploy_freeze_alert.vue new file mode 100644 index 00000000000..aaa7e71758c --- /dev/null +++ b/app/assets/javascripts/environments/components/deploy_freeze_alert.vue @@ -0,0 +1,79 @@ +<script> +import { GlAlert, GlLink, GlSprintf } from '@gitlab/ui'; +import { sortBy } from 'lodash'; +import { formatDate } from '~/lib/utils/datetime/date_format_utility'; +import { helpPagePath } from '~/helpers/help_page_helper'; +import { s__ } from '~/locale'; +import deployFreezesQuery from '../graphql/queries/deploy_freezes.query.graphql'; + +export default { + components: { + GlAlert, + GlLink, + GlSprintf, + }, + inject: ['projectFullPath'], + props: { + name: { + type: String, + required: true, + }, + }, + data() { + return { deployFreezes: [] }; + }, + + apollo: { + deployFreezes: { + query: deployFreezesQuery, + update(data) { + const freezes = data?.project?.environment?.deployFreezes; + return sortBy(freezes, [(freeze) => freeze.startTime]); + }, + variables() { + return { + projectFullPath: this.projectFullPath, + environmentName: this.name, + }; + }, + }, + }, + computed: { + shouldShowDeployFreezeAlert() { + return this.deployFreezes.length > 0; + }, + nextDeployFreeze() { + return this.deployFreezes[0]; + }, + deployFreezeStartTime() { + return formatDate(this.nextDeployFreeze.startTime); + }, + deployFreezeEndTime() { + return formatDate(this.nextDeployFreeze.endTime); + }, + }, + i18n: { + deployFreezeAlert: s__( + 'Environments|A freeze period is in effect from %{startTime} to %{endTime}. Deployments might fail during this time. For more information, see the %{docsLinkStart}deploy freeze documentation%{docsLinkEnd}.', + ), + }, + deployFreezeDocsPath: helpPagePath('user/project/releases/index', { + anchor: 'prevent-unintentional-releases-by-setting-a-deploy-freeze', + }), +}; +</script> +<template> + <gl-alert v-if="shouldShowDeployFreezeAlert" :dismissible="false" class="gl-mt-4"> + <gl-sprintf :message="$options.i18n.deployFreezeAlert"> + <template #startTime + ><span class="gl-font-weight-bold">{{ deployFreezeStartTime }}</span></template + > + <template #endTime + ><span class="gl-font-weight-bold">{{ deployFreezeEndTime }}</span></template + > + <template #docsLink="{ content }" + ><gl-link :href="$options.deployFreezeDocsPath">{{ content }}</gl-link></template + > + </gl-sprintf> + </gl-alert> +</template> diff --git a/app/assets/javascripts/environments/components/environments_detail_header.vue b/app/assets/javascripts/environments/components/environments_detail_header.vue index 1e555347011..0507abf3eaf 100644 --- a/app/assets/javascripts/environments/components/environments_detail_header.vue +++ b/app/assets/javascripts/environments/components/environments_detail_header.vue @@ -7,6 +7,7 @@ import timeagoMixin from '~/vue_shared/mixins/timeago'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import DeleteEnvironmentModal from './delete_environment_modal.vue'; import StopEnvironmentModal from './stop_environment_modal.vue'; +import DeployFreezeAlert from './deploy_freeze_alert.vue'; export default { name: 'EnvironmentsDetailHeader', @@ -15,6 +16,7 @@ export default { GlButton, GlSprintf, TimeAgo, + DeployFreezeAlert, DeleteEnvironmentModal, StopEnvironmentModal, }, @@ -96,81 +98,88 @@ export default { }; </script> <template> - <header class="top-area gl-justify-content-between"> - <div class="gl-display-flex gl-flex-grow-1 gl-align-items-center"> - <h1 class="page-title gl-font-size-h-display"> - {{ environment.name }} - </h1> - <p v-if="shouldShowCancelAutoStopButton" class="gl-mb-0 gl-ml-3" data-testid="auto-stops-at"> - <gl-sprintf :message="$options.i18n.autoStopAtText"> - <template #autoStopAt> - <time-ago :time="environment.autoStopAt" /> - </template> - </gl-sprintf> - </p> - </div> - <div class="nav-controls gl-my-1"> - <form method="POST" :action="cancelAutoStopPath" data-testid="cancel-auto-stop-form"> - <input :value="$options.csrf.token" type="hidden" name="authenticity_token" /> - <gl-button + <div> + <deploy-freeze-alert :name="environment.name" /> + <header class="top-area gl-justify-content-between"> + <div class="gl-display-flex gl-flex-grow-1 gl-align-items-center"> + <h1 class="page-title gl-font-size-h-display"> + {{ environment.name }} + </h1> + <p v-if="shouldShowCancelAutoStopButton" - v-gl-tooltip.hover - data-testid="cancel-auto-stop-button" - :title="$options.i18n.cancelAutoStopButtonTitle" - type="submit" - icon="thumbtack" + class="gl-mb-0 gl-ml-3" + data-testid="auto-stops-at" + > + <gl-sprintf :message="$options.i18n.autoStopAtText"> + <template #autoStopAt> + <time-ago :time="environment.autoStopAt" /> + </template> + </gl-sprintf> + </p> + </div> + <div class="nav-controls gl-my-1"> + <form method="POST" :action="cancelAutoStopPath" data-testid="cancel-auto-stop-form"> + <input :value="$options.csrf.token" type="hidden" name="authenticity_token" /> + <gl-button + v-if="shouldShowCancelAutoStopButton" + v-gl-tooltip.hover + data-testid="cancel-auto-stop-button" + :title="$options.i18n.cancelAutoStopButtonTitle" + type="submit" + icon="thumbtack" + /> + </form> + <gl-button + v-if="shouldShowTerminalButton" + data-testid="terminal-button" + :href="terminalPath" + icon="terminal" /> - </form> - <gl-button - v-if="shouldShowTerminalButton" - data-testid="terminal-button" - :href="terminalPath" - icon="terminal" - /> - <gl-button - v-if="shouldShowExternalUrlButton" - v-gl-tooltip.hover - data-testid="external-url-button" - :title="$options.i18n.externalButtonTitle" - :href="environment.externalUrl" - is-unsafe-link - icon="external-link" - target="_blank" - >{{ $options.i18n.externalButtonText }}</gl-button - > - <gl-button - v-if="shouldShowMetricsButton" - v-gl-tooltip.hover - data-testid="metrics-button" - :href="metricsPath" - :title="$options.i18n.metricsButtonTitle" - icon="chart" - class="gl-mr-2" - > - {{ $options.i18n.metricsButtonText }} - </gl-button> - <gl-button v-if="canUpdateEnvironment" data-testid="edit-button" :href="updatePath"> - {{ $options.i18n.editButtonText }} - </gl-button> - <gl-button - v-if="shouldShowStopButton" - v-gl-modal-directive="'stop-environment-modal'" - data-testid="stop-button" - icon="stop" - variant="danger" - > - {{ $options.i18n.stopButtonText }} - </gl-button> - <gl-button - v-if="canDestroyEnvironment" - v-gl-modal-directive="'delete-environment-modal'" - data-testid="destroy-button" - variant="danger" - > - {{ $options.i18n.deleteButtonText }} - </gl-button> - </div> - <delete-environment-modal v-if="canDestroyEnvironment" :environment="environment" /> - <stop-environment-modal v-if="shouldShowStopButton" :environment="environment" /> - </header> + <gl-button + v-if="shouldShowExternalUrlButton" + v-gl-tooltip.hover + data-testid="external-url-button" + :title="$options.i18n.externalButtonTitle" + :href="environment.externalUrl" + is-unsafe-link + icon="external-link" + target="_blank" + >{{ $options.i18n.externalButtonText }}</gl-button + > + <gl-button + v-if="shouldShowMetricsButton" + v-gl-tooltip.hover + data-testid="metrics-button" + :href="metricsPath" + :title="$options.i18n.metricsButtonTitle" + icon="chart" + class="gl-mr-2" + > + {{ $options.i18n.metricsButtonText }} + </gl-button> + <gl-button v-if="canUpdateEnvironment" data-testid="edit-button" :href="updatePath"> + {{ $options.i18n.editButtonText }} + </gl-button> + <gl-button + v-if="shouldShowStopButton" + v-gl-modal-directive="'stop-environment-modal'" + data-testid="stop-button" + icon="stop" + variant="danger" + > + {{ $options.i18n.stopButtonText }} + </gl-button> + <gl-button + v-if="canDestroyEnvironment" + v-gl-modal-directive="'delete-environment-modal'" + data-testid="destroy-button" + variant="danger" + > + {{ $options.i18n.deleteButtonText }} + </gl-button> + </div> + <delete-environment-modal v-if="canDestroyEnvironment" :environment="environment" /> + <stop-environment-modal v-if="shouldShowStopButton" :environment="environment" /> + </header> + </div> </template> diff --git a/app/assets/javascripts/environments/graphql/queries/deploy_freezes.query.graphql b/app/assets/javascripts/environments/graphql/queries/deploy_freezes.query.graphql new file mode 100644 index 00000000000..7d701b95bbf --- /dev/null +++ b/app/assets/javascripts/environments/graphql/queries/deploy_freezes.query.graphql @@ -0,0 +1,12 @@ +query getEnvironmentFreezes($projectFullPath: ID!, $environmentName: String) { + project(fullPath: $projectFullPath) { + id + environment(name: $environmentName) { + id + deployFreezes { + startTime + endTime + } + } + } +} diff --git a/app/assets/javascripts/environments/mount_show.js b/app/assets/javascripts/environments/mount_show.js index cc13a237aca..f73cb7fe1bc 100644 --- a/app/assets/javascripts/environments/mount_show.js +++ b/app/assets/javascripts/environments/mount_show.js @@ -3,9 +3,13 @@ import VueApollo from 'vue-apollo'; import VueRouter from 'vue-router'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import EnvironmentsDetailHeader from './components/environments_detail_header.vue'; -import { apolloProvider } from './graphql/client'; +import { apolloProvider as createApolloProvider } from './graphql/client'; import environmentsMixin from './mixins/environments_mixin'; +Vue.use(VueApollo); + +const apolloProvider = createApolloProvider(); + export const initHeader = () => { const el = document.getElementById('environments-detail-view-header'); const container = document.getElementById('environments-detail-view'); @@ -13,7 +17,11 @@ export const initHeader = () => { return new Vue({ el, + apolloProvider, mixins: [environmentsMixin], + provide: { + projectFullPath: dataset.projectFullPath, + }, data() { const environment = { name: dataset.name, @@ -60,7 +68,6 @@ export const initPage = async () => { const dataElement = document.getElementById('environments-detail-view'); const dataSet = convertObjectPropsToCamelCase(JSON.parse(dataElement.dataset.details)); - Vue.use(VueApollo); Vue.use(VueRouter); const el = document.getElementById('environment_details_page'); @@ -90,7 +97,7 @@ export const initPage = async () => { return new Vue({ el, - apolloProvider: apolloProvider(), + apolloProvider, router, provide: { projectPath: dataSet.projectFullPath, diff --git a/app/assets/javascripts/graphql_shared/queries/merge_request.query.graphql b/app/assets/javascripts/graphql_shared/queries/merge_request.query.graphql new file mode 100644 index 00000000000..a6ef5935162 --- /dev/null +++ b/app/assets/javascripts/graphql_shared/queries/merge_request.query.graphql @@ -0,0 +1,9 @@ +query mergeRequestId($projectPath: ID!, $iid: String!) { + project(fullPath: $projectPath) { + id + mergeRequest(iid: $iid) { + id + preparedAt + } + } +} diff --git a/app/assets/javascripts/graphql_shared/subscriptions/merge_request_prepared.subscription.graphql b/app/assets/javascripts/graphql_shared/subscriptions/merge_request_prepared.subscription.graphql new file mode 100644 index 00000000000..ba658f56ebd --- /dev/null +++ b/app/assets/javascripts/graphql_shared/subscriptions/merge_request_prepared.subscription.graphql @@ -0,0 +1,8 @@ +subscription mergeRequestPrepared($issuableId: IssuableID!) { + mergeRequestMergeStatusUpdated(issuableId: $issuableId) { + ... on MergeRequest { + id + preparedAt + } + } +} diff --git a/app/assets/javascripts/pages/projects/merge_requests/page.js b/app/assets/javascripts/pages/projects/merge_requests/page.js index fbd45f4bd7d..552e75da9b8 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/page.js +++ b/app/assets/javascripts/pages/projects/merge_requests/page.js @@ -3,6 +3,8 @@ import VueApollo from 'vue-apollo'; import initMrNotes from 'ee_else_ce/mr_notes'; import StickyHeader from '~/merge_requests/components/sticky_header.vue'; import { initIssuableHeaderWarnings } from '~/issuable'; +import { start as startCodeReviewMessaging } from '~/code_review/signals'; +import diffsEventHub from '~/diffs/event_hub'; import store from '~/mr_notes/stores'; import initSidebarBundle from '~/sidebar/sidebar_bundle'; import { apolloProvider } from '~/graphql_shared/issuable_client'; @@ -15,6 +17,7 @@ Vue.use(VueApollo); export function initMrPage() { initMrNotes(); initShow(); + startCodeReviewMessaging({ signalBus: diffsEventHub }); } requestIdleCallback(() => { diff --git a/app/assets/javascripts/pipelines/graphql/queries/get_failed_jobs.query.graphql b/app/assets/javascripts/pipelines/graphql/queries/get_failed_jobs.query.graphql index 13c9f0ff8ee..5bdafa15f72 100644 --- a/app/assets/javascripts/pipelines/graphql/queries/get_failed_jobs.query.graphql +++ b/app/assets/javascripts/pipelines/graphql/queries/get_failed_jobs.query.graphql @@ -3,7 +3,7 @@ query getFailedJobs($fullPath: ID!, $pipelineIid: ID!) { id pipeline(iid: $pipelineIid) { id - jobs(statuses: FAILED) { + jobs(statuses: FAILED, retried: false) { nodes { status detailedStatus { diff --git a/app/assets/javascripts/projects/commit_box/info/init_details_button.js b/app/assets/javascripts/projects/commit_box/info/init_details_button.js index 667d6bd0250..520b20fcb86 100644 --- a/app/assets/javascripts/projects/commit_box/info/init_details_button.js +++ b/app/assets/javascripts/projects/commit_box/info/init_details_button.js @@ -6,7 +6,7 @@ export const initDetailsButton = () => { } expandButton.addEventListener('click', (event) => { - const btn = event.target; + const btn = event.currentTarget; const contentEl = btn.parentElement.querySelector('.js-details-content'); if (contentEl) { diff --git a/app/assets/javascripts/work_items/components/work_item_award_emoji.vue b/app/assets/javascripts/work_items/components/work_item_award_emoji.vue new file mode 100644 index 00000000000..91f87be1233 --- /dev/null +++ b/app/assets/javascripts/work_items/components/work_item_award_emoji.vue @@ -0,0 +1,144 @@ +<script> +import * as Sentry from '@sentry/browser'; +import { getIdFromGraphQLId, convertToGraphQLId } from '~/graphql_shared/utils'; +import AwardsList from '~/vue_shared/components/awards_list.vue'; +import { isLoggedIn } from '~/lib/utils/common_utils'; +import { TYPENAME_USER } from '~/graphql_shared/constants'; +import updateWorkItemMutation from '../graphql/update_work_item.mutation.graphql'; +import { + EMOJI_ACTION_REMOVE, + EMOJI_ACTION_ADD, + WIDGET_TYPE_AWARD_EMOJI, + EMOJI_THUMBSDOWN, + EMOJI_THUMBSUP, +} from '../constants'; + +export default { + defaultAwards: [EMOJI_THUMBSUP, EMOJI_THUMBSDOWN], + isLoggedIn: isLoggedIn(), + components: { + AwardsList, + }, + props: { + workItem: { + type: Object, + required: true, + }, + awardEmoji: { + type: Object, + required: true, + }, + }, + computed: { + currentUserId() { + return window.gon.current_user_id; + }, + /** + * Parse and convert award emoji list to a format that AwardsList can understand + */ + awards() { + return this.awardEmoji.nodes.map((emoji, index) => ({ + id: index + 1, + name: emoji.name, + user: { + id: getIdFromGraphQLId(emoji.user.id), + }, + })); + }, + }, + methods: { + handleAward(name) { + // Decide action based on emoji is already present + const action = + this.awards.findIndex((emoji) => emoji.name === name) > -1 + ? EMOJI_ACTION_REMOVE + : EMOJI_ACTION_ADD; + const inputVariables = { + id: this.workItem.id, + awardEmojiWidget: { + action, + name, + }, + }; + + this.$apollo + .mutate({ + mutation: updateWorkItemMutation, + variables: { + input: inputVariables, + }, + optimisticResponse: this.getOptimisticResponse({ name, action }), + }) + .then( + ({ + data: { + workItemUpdate: { errors }, + }, + }) => { + if (errors?.length) { + throw new Error(errors[0]); + } + }, + ) + .catch((error) => { + this.$emit('error', error.message); + Sentry.captureException(error); + }); + }, + /** + * Prepare workItemUpdate for optimistic response + */ + getOptimisticResponse({ name, action }) { + let awardEmojiNodes = [ + ...this.awardEmoji.nodes, + { + name, + __typename: 'AwardEmoji', + user: { + id: convertToGraphQLId(TYPENAME_USER, this.currentUserId), + __typename: 'UserCore', + }, + }, + ]; + // Exclude the award emoji node in case of remove action + if (action === EMOJI_ACTION_REMOVE) { + awardEmojiNodes = [...this.awardEmoji.nodes.filter((emoji) => emoji.name !== name)]; + } + return { + workItemUpdate: { + errors: [], + workItem: { + ...this.workItem, + widgets: [ + { + type: WIDGET_TYPE_AWARD_EMOJI, + awardEmoji: { + nodes: awardEmojiNodes, + __typename: 'AwardEmojiConnection', + }, + __typename: 'WorkItemWidgetAwardEmoji', + }, + ], + __typename: 'WorkItem', + }, + __typename: 'WorkItemUpdatePayload', + }, + }; + }, + }, +}; +</script> + +<template> + <div class="gl-mt-3"> + <awards-list + data-testid="work-item-award-list" + :awards="awards" + :can-award-emoji="$options.isLoggedIn" + :current-user-id="currentUserId" + :default-awards="$options.defaultAwards" + selected-class="selected" + @award="handleAward" + /> + </div> +</template> diff --git a/app/assets/javascripts/work_items/components/work_item_detail.vue b/app/assets/javascripts/work_items/components/work_item_detail.vue index e3ac49b90d5..0f1af44e8a1 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail.vue @@ -18,6 +18,7 @@ import { getParameterByName, updateHistory, setUrlParams } from '~/lib/utils/url import { isPositiveInteger } from '~/lib/utils/number_utils'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { isLoggedIn } from '~/lib/utils/common_utils'; import { TYPENAME_WORK_ITEM } from '~/graphql_shared/constants'; import WorkItemTypeIcon from '~/work_items/components/work_item_type_icon.vue'; import AbuseCategorySelector from '~/abuse_reports/components/abuse_category_selector.vue'; @@ -27,7 +28,9 @@ import { WIDGET_TYPE_ASSIGNEES, WIDGET_TYPE_LABELS, WIDGET_TYPE_NOTIFICATIONS, + WIDGET_TYPE_CURRENT_USER_TODOS, WIDGET_TYPE_DESCRIPTION, + WIDGET_TYPE_AWARD_EMOJI, WIDGET_TYPE_START_AND_DUE_DATE, WIDGET_TYPE_WEIGHT, WIDGET_TYPE_PROGRESS, @@ -51,10 +54,12 @@ import { findHierarchyWidgetChildren } from '../utils'; import WorkItemTree from './work_item_links/work_item_tree.vue'; import WorkItemActions from './work_item_actions.vue'; +import WorkItemTodos from './work_item_todos.vue'; import WorkItemState from './work_item_state.vue'; import WorkItemTitle from './work_item_title.vue'; import WorkItemCreatedUpdated from './work_item_created_updated.vue'; import WorkItemDescription from './work_item_description.vue'; +import WorkItemAwardEmoji from './work_item_award_emoji.vue'; import WorkItemDueDate from './work_item_due_date.vue'; import WorkItemAssignees from './work_item_assignees.vue'; import WorkItemLabels from './work_item_labels.vue'; @@ -67,6 +72,7 @@ export default { directives: { GlTooltip: GlTooltipDirective, }, + isLoggedIn: isLoggedIn(), components: { GlAlert, GlBadge, @@ -77,8 +83,10 @@ export default { GlEmptyState, WorkItemAssignees, WorkItemActions, + WorkItemTodos, WorkItemCreatedUpdated, WorkItemDescription, + WorkItemAwardEmoji, WorkItemDueDate, WorkItemLabels, WorkItemTitle, @@ -287,6 +295,15 @@ export default { workItemNotificationsSubscribed() { return Boolean(this.isWidgetPresent(WIDGET_TYPE_NOTIFICATIONS)?.subscribed); }, + workItemCurrentUserTodos() { + return this.isWidgetPresent(WIDGET_TYPE_CURRENT_USER_TODOS); + }, + showWorkItemCurrentUserTodos() { + return this.$options.isLoggedIn && this.workItemCurrentUserTodos; + }, + currentUserTodos() { + return this.workItemCurrentUserTodos?.currentUserTodos?.edges; + }, workItemAssignees() { return this.isWidgetPresent(WIDGET_TYPE_ASSIGNEES); }, @@ -302,6 +319,9 @@ export default { workItemProgress() { return this.isWidgetPresent(WIDGET_TYPE_PROGRESS); }, + workItemAwardEmoji() { + return this.isWidgetPresent(WIDGET_TYPE_AWARD_EMOJI); + }, workItemHierarchy() { return this.isWidgetPresent(WIDGET_TYPE_HIERARCHY); }, @@ -566,6 +586,12 @@ export default { class="gl-mr-3 gl-cursor-help" >{{ __('Confidential') }}</gl-badge > + <work-item-todos + v-if="showWorkItemCurrentUserTodos" + :work-item="workItem" + :current-user-todos="currentUserTodos" + @error="updateError = $event" + /> <work-item-actions v-if="canUpdate || canDelete" :work-item-id="workItem.id" @@ -685,6 +711,12 @@ export default { class="gl-pt-5" @error="updateError = $event" /> + <work-item-award-emoji + v-if="workItemAwardEmoji" + :work-item="workItem" + :award-emoji="workItemAwardEmoji.awardEmoji" + @error="updateError = $event" + /> <work-item-tree v-if="workItemType === $options.WORK_ITEM_TYPE_VALUE_OBJECTIVE" :work-item-type="workItemType" diff --git a/app/assets/javascripts/work_items/components/work_item_todos.vue b/app/assets/javascripts/work_items/components/work_item_todos.vue new file mode 100644 index 00000000000..4e787720a42 --- /dev/null +++ b/app/assets/javascripts/work_items/components/work_item_todos.vue @@ -0,0 +1,116 @@ +<script> +import { GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui'; +import { s__ } from '~/locale'; +import { updateGlobalTodoCount } from '~/sidebar/utils'; +import { getWorkItemTodoOptimisticResponse } from '../utils'; +import { ADD, MARK_AS_DONE, TODO_ADD_ICON, TODO_DONE_ICON } from '../constants'; +import updateWorkItemMutation from '../graphql/update_work_item.mutation.graphql'; + +export default { + i18n: { + addATodo: s__('WorkItem|Add a to do'), + markAsDone: s__('WorkItem|Mark as done'), + }, + directives: { + GlTooltip: GlTooltipDirective, + }, + components: { + GlIcon, + GlButton, + }, + props: { + workItem: { + type: Object, + required: true, + }, + currentUserTodos: { + type: Array, + required: false, + default: () => [], + }, + }, + data() { + return { + isLoading: false, + buttonLabel: + this.currentUserTodos.length > 0 + ? this.$options.i18n.markAsDone + : this.$options.i18n.addATodo, + }; + }, + computed: { + pendingTodo() { + return this.currentUserTodos.length > 0; + }, + buttonIcon() { + return this.pendingTodo ? TODO_DONE_ICON : TODO_ADD_ICON; + }, + }, + methods: { + onToggle() { + this.isLoading = true; + this.buttonLabel = ''; + const action = this.pendingTodo ? MARK_AS_DONE : ADD; + const inputVariables = { + id: this.workItem.id, + currentUserTodosWidget: { + action, + }, + }; + this.$apollo + .mutate({ + mutation: updateWorkItemMutation, + variables: { + input: inputVariables, + }, + optimisticResponse: getWorkItemTodoOptimisticResponse({ + workItem: this.workItem, + pendingTodo: this.pendingTodo, + }), + }) + .then( + ({ + data: { + workItemUpdate: { errors }, + }, + }) => { + if (errors?.length) { + throw new Error(errors[0]); + } + if (this.pendingTodo) { + updateGlobalTodoCount(1); + this.buttonLabel = this.$options.i18n.markAsDone; + } else { + updateGlobalTodoCount(-1); + this.buttonLabel = this.$options.i18n.addATodo; + } + }, + ) + .catch((error) => { + this.$emit('error', error.message); + }) + .finally(() => { + this.isLoading = false; + }); + }, + }, +}; +</script> + +<template> + <gl-button + v-gl-tooltip.hover + data-testid="work-item-todos-action" + :loading="isLoading" + :title="buttonLabel" + category="tertiary" + :aria-label="buttonLabel" + @click="onToggle" + > + <gl-icon + data-testid="work-item-todos-icon" + :class="{ 'gl-fill-blue-500': pendingTodo }" + :name="buttonIcon" + /> + </gl-button> +</template> diff --git a/app/assets/javascripts/work_items/constants.js b/app/assets/javascripts/work_items/constants.js index 6523734a382..6710c762c2e 100644 --- a/app/assets/javascripts/work_items/constants.js +++ b/app/assets/javascripts/work_items/constants.js @@ -14,7 +14,9 @@ export const TASK_TYPE_NAME = 'Task'; export const WIDGET_TYPE_ASSIGNEES = 'ASSIGNEES'; export const WIDGET_TYPE_DESCRIPTION = 'DESCRIPTION'; +export const WIDGET_TYPE_AWARD_EMOJI = 'AWARD_EMOJI'; export const WIDGET_TYPE_NOTIFICATIONS = 'NOTIFICATIONS'; +export const WIDGET_TYPE_CURRENT_USER_TODOS = 'CURRENT_USER_TODOS'; export const WIDGET_TYPE_LABELS = 'LABELS'; export const WIDGET_TYPE_START_AND_DUE_DATE = 'START_AND_DUE_DATE'; export const WIDGET_TYPE_WEIGHT = 'WEIGHT'; @@ -215,3 +217,19 @@ export const TEST_ID_NOTIFICATIONS_TOGGLE_ACTION = 'notifications-toggle-action' export const TEST_ID_NOTIFICATIONS_TOGGLE_FORM = 'notifications-toggle-form'; export const TEST_ID_DELETE_ACTION = 'delete-action'; export const TEST_ID_PROMOTE_ACTION = 'promote-action'; + +export const ADD = 'ADD'; +export const MARK_AS_DONE = 'MARK_AS_DONE'; +export const TODO_ADD_ICON = 'todo-add'; +export const TODO_DONE_ICON = 'todo-done'; +export const TODO_TYPENAME = 'Todo'; +export const TODO_EDGE_TYPENAME = 'TodoEdge'; +export const TODO_CONNECTION_TYPENAME = 'TodoConnection'; +export const CURRENT_USER_TODOS_TYPENAME = 'WorkItemWidgetCurrentUserTodos'; +export const WORK_ITEM_TYPENAME = 'WorkItem'; +export const WORK_ITEM_UPDATE_PAYLOAD_TYPENAME = 'WorkItemUpdatePayload'; + +export const EMOJI_ACTION_ADD = 'ADD'; +export const EMOJI_ACTION_REMOVE = 'REMOVE'; +export const EMOJI_THUMBSUP = 'thumbsup'; +export const EMOJI_THUMBSDOWN = 'thumbsdown'; diff --git a/app/assets/javascripts/work_items/graphql/award_emoji.fragment.graphql b/app/assets/javascripts/work_items/graphql/award_emoji.fragment.graphql new file mode 100644 index 00000000000..85b88990cd6 --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/award_emoji.fragment.graphql @@ -0,0 +1,6 @@ +fragment AwardEmojiFragment on AwardEmoji { + name + user { + id + } +} diff --git a/app/assets/javascripts/work_items/graphql/work_item_metadata_widgets.fragment.graphql b/app/assets/javascripts/work_items/graphql/work_item_metadata_widgets.fragment.graphql index 44fda3ee894..42c057fb8fe 100644 --- a/app/assets/javascripts/work_items/graphql/work_item_metadata_widgets.fragment.graphql +++ b/app/assets/javascripts/work_items/graphql/work_item_metadata_widgets.fragment.graphql @@ -1,6 +1,7 @@ #import "~/graphql_shared/fragments/label.fragment.graphql" #import "~/graphql_shared/fragments/user.fragment.graphql" #import "~/work_items/graphql/milestone.fragment.graphql" +#import "~/work_items/graphql/award_emoji.fragment.graphql" fragment WorkItemMetadataWidgets on WorkItemWidget { ... on WorkItemWidgetDescription { @@ -36,9 +37,28 @@ fragment WorkItemMetadataWidgets on WorkItemWidget { } } } - ... on WorkItemWidgetNotifications { type subscribed } + + ... on WorkItemWidgetCurrentUserTodos { + type + currentUserTodos(state: pending) { + edges { + node { + id + state + } + } + } + } + ... on WorkItemWidgetAwardEmoji { + type + awardEmoji { + nodes { + ...AwardEmojiFragment + } + } + } } diff --git a/app/assets/javascripts/work_items/graphql/work_item_widgets.fragment.graphql b/app/assets/javascripts/work_items/graphql/work_item_widgets.fragment.graphql index 8039ef53f98..bf8dc9ce9b0 100644 --- a/app/assets/javascripts/work_items/graphql/work_item_widgets.fragment.graphql +++ b/app/assets/javascripts/work_items/graphql/work_item_widgets.fragment.graphql @@ -1,6 +1,7 @@ #import "~/graphql_shared/fragments/label.fragment.graphql" #import "~/graphql_shared/fragments/user.fragment.graphql" #import "~/work_items/graphql/milestone.fragment.graphql" +#import "~/work_items/graphql/award_emoji.fragment.graphql" #import "ee_else_ce/work_items/graphql/work_item_metadata_widgets.fragment.graphql" fragment WorkItemWidgets on WorkItemWidget { @@ -89,4 +90,23 @@ fragment WorkItemWidgets on WorkItemWidget { type subscribed } + ... on WorkItemWidgetCurrentUserTodos { + type + currentUserTodos(state: pending) { + edges { + node { + id + state + } + } + } + } + ... on WorkItemWidgetAwardEmoji { + type + awardEmoji { + nodes { + ...AwardEmojiFragment + } + } + } } diff --git a/app/assets/javascripts/work_items/utils.js b/app/assets/javascripts/work_items/utils.js index c5b937ef132..653819904af 100644 --- a/app/assets/javascripts/work_items/utils.js +++ b/app/assets/javascripts/work_items/utils.js @@ -1,4 +1,14 @@ -import { WIDGET_TYPE_HIERARCHY } from '~/work_items/constants'; +import { uniqueId } from 'lodash'; +import { + WIDGET_TYPE_HIERARCHY, + WIDGET_TYPE_CURRENT_USER_TODOS, + CURRENT_USER_TODOS_TYPENAME, + TODO_CONNECTION_TYPENAME, + TODO_EDGE_TYPENAME, + TODO_TYPENAME, + WORK_ITEM_TYPENAME, + WORK_ITEM_UPDATE_PAYLOAD_TYPENAME, +} from '~/work_items/constants'; import workItemQuery from './graphql/work_item.query.graphql'; import workItemByIidQuery from './graphql/work_item_by_iid.query.graphql'; @@ -28,3 +38,38 @@ export const markdownPreviewPath = (fullPath, iid) => `${ gon.relative_url_root || '' }/${fullPath}/preview_markdown?target_type=WorkItem&target_id=${iid}`; + +export const getWorkItemTodoOptimisticResponse = ({ workItem, pendingTodo }) => { + const todo = pendingTodo + ? [ + { + node: { + id: -uniqueId(), + state: 'pending', + __typename: TODO_TYPENAME, + }, + __typename: TODO_EDGE_TYPENAME, + }, + ] + : []; + return { + workItemUpdate: { + errors: [], + workItem: { + ...workItem, + widgets: [ + { + type: WIDGET_TYPE_CURRENT_USER_TODOS, + currentUserTodos: { + edges: todo, + __typename: TODO_CONNECTION_TYPENAME, + }, + __typename: CURRENT_USER_TODOS_TYPENAME, + }, + ], + __typename: WORK_ITEM_TYPENAME, + }, + __typename: WORK_ITEM_UPDATE_PAYLOAD_TYPENAME, + }, + }; +}; diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 56ef6d13252..f828129cdf1 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -36,7 +36,7 @@ } .right-sidebar-collapsed { - --application-bar-right: #{$gutter-collapsed-width}; + --application-bar-right: #{$right-sidebar-collapsed-width}; &.is-merge-request { --application-bar-right: 0px; @@ -44,7 +44,7 @@ } .right-sidebar-expanded { - --application-bar-right: #{$gutter-width}; + --application-bar-right: #{$right-sidebar-width}; } } diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index f2486640650..9fdf889f4e9 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -22,7 +22,7 @@ &:not(.is-merge-request) { @include media-breakpoint-up(sm) { &:not(.wiki-sidebar):not(.build-sidebar):not(.issuable-bulk-update-sidebar) .content-wrapper { - padding-right: $gutter-collapsed-width; + padding-right: $right-sidebar-collapsed-width; } } } @@ -30,7 +30,7 @@ &.is-merge-request { @include media-breakpoint-up(lg) { .content-wrapper { - padding-right: $gutter-collapsed-width; + padding-right: $right-sidebar-collapsed-width; } } } @@ -56,7 +56,7 @@ z-index: $zindex-dropdown-menu; &.right-sidebar-merge-requests { - width: $gutter-width; + width: $right-sidebar-width; @include media-breakpoint-up(md) { z-index: auto; @@ -69,14 +69,14 @@ @include media-breakpoint-only(sm) { &:not(.wiki-sidebar):not(.build-sidebar):not(.issuable-bulk-update-sidebar) .content-wrapper { - padding-right: $gutter-collapsed-width; + padding-right: $right-sidebar-collapsed-width; } } &:not(.is-merge-request) { @include media-breakpoint-up(md) { .content-wrapper { - padding-right: $gutter-width; + padding-right: $right-sidebar-width; } } } @@ -102,7 +102,7 @@ @mixin maintain-sidebar-dimensions { display: block; - width: $gutter-width; + width: $right-sidebar-width; } .issues-bulk-update.right-sidebar { @@ -113,7 +113,7 @@ &.right-sidebar-expanded { @include maintain-sidebar-dimensions; - width: $gutter-width; + width: $right-sidebar-width; .issuable-sidebar-header { // matches `.top-area .nav-controls` for issuable index pages @@ -401,7 +401,7 @@ border-bottom: 1px solid $border-gray-normal; // This prevents the mess when resizing the sidebar // of elements repositioning themselves.. - width: $gutter-inner-width; + width: $right-sidebar-inner-width; // -- &:last-child { @@ -474,7 +474,7 @@ &.right-sidebar-expanded { &:not(.right-sidebar-merge-requests) { - width: $gutter-width; + width: $right-sidebar-width; } .value { @@ -554,13 +554,13 @@ } } - width: $gutter-collapsed-width; + width: $right-sidebar-collapsed-width; padding: 0; .block, .sidebar-contained-width, .issuable-sidebar-header { - width: $gutter-collapsed-width - 2px; + width: $right-sidebar-collapsed-width - 2px; padding: 0; border-bottom: 0; overflow: hidden; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 6cf231fbcef..dc6a5c5479c 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -2,9 +2,9 @@ * Layout */ $grid-size: 8px; -$gutter-collapsed-width: 62px; -$gutter-width: 290px; -$gutter-inner-width: 250px; +$right-sidebar-collapsed-width: 62px; +$right-sidebar-width: 290px; +$right-sidebar-inner-width: 250px; $sidebar-breakpoint: 1024px; $default-transition-duration: 0.15s; $contextual-sidebar-width: 256px; diff --git a/app/assets/stylesheets/page_bundles/boards.scss b/app/assets/stylesheets/page_bundles/boards.scss index 32a6af0dc36..5aca697ae26 100644 --- a/app/assets/stylesheets/page_bundles/boards.scss +++ b/app/assets/stylesheets/page_bundles/boards.scss @@ -15,7 +15,7 @@ width: 100%; &.is-compact { - width: calc(100% - #{$gutter-width}); + width: calc(100% - #{$right-sidebar-width}); } } } diff --git a/app/assets/stylesheets/page_bundles/login.scss b/app/assets/stylesheets/page_bundles/login.scss index 1ae7230772d..6175bba6ba7 100644 --- a/app/assets/stylesheets/page_bundles/login.scss +++ b/app/assets/stylesheets/page_bundles/login.scss @@ -292,3 +292,9 @@ } } } + +@include media-breakpoint-down(sm) { + .sm-bg-gray-10 { + @include gl-bg-gray-10; + } +} diff --git a/app/assets/stylesheets/page_bundles/merge_requests.scss b/app/assets/stylesheets/page_bundles/merge_requests.scss index 17ca7828a20..b282d83b74b 100644 --- a/app/assets/stylesheets/page_bundles/merge_requests.scss +++ b/app/assets/stylesheets/page_bundles/merge_requests.scss @@ -964,7 +964,7 @@ $tabs-holder-z-index: 250; .merge-request-overview { @include media-breakpoint-up(lg) { display: grid; - grid-template-columns: calc(97% - #{$gutter-width}) auto; + grid-template-columns: calc(97% - #{$right-sidebar-width}) auto; grid-gap: 3%; } } @@ -1131,7 +1131,7 @@ $tabs-holder-z-index: 250; width: 100%; height: $toggle-sidebar-height; padding-left: $contextual-sidebar-width; - padding-right: $gutter_collapsed_width; + padding-right: $right-sidebar-collapsed-width; background: var(--white, $white); border-top: 1px solid var(--border-color, $border-color); transition: padding $gl-transition-duration-medium; diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index aed08328546..f91ec55573d 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -176,10 +176,14 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def metrics_redirect + return not_found if Feature.enabled?(:remove_monitor_metrics) + redirect_to project_metrics_dashboard_path(project) end def metrics + return not_found if Feature.enabled?(:remove_monitor_metrics) + respond_to do |format| format.html do redirect_to project_metrics_dashboard_path(project, environment: environment) @@ -195,6 +199,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def additional_metrics + return not_found if Feature.enabled?(:remove_monitor_metrics) + respond_to do |format| format.json do additional_metrics = environment.additional_metrics(*metrics_params) || {} diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 478d9f0b38e..952c9e90a2c 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/app/controllers/projects/settings/operations_controller.rb @@ -133,7 +133,11 @@ module Projects ], grafana_integration_attributes: [:token, :grafana_url, :enabled] - } + }.tap do |potential_params| + if Feature.enabled?(:remove_monitor_metrics) + potential_params.except!(:metrics_setting_attributes, :grafana_integration_attributes) + end + end end end end diff --git a/app/graphql/resolvers/ci/jobs_resolver.rb b/app/graphql/resolvers/ci/jobs_resolver.rb index 31cc350f331..95816a1ac1a 100644 --- a/app/graphql/resolvers/ci/jobs_resolver.rb +++ b/app/graphql/resolvers/ci/jobs_resolver.rb @@ -23,12 +23,17 @@ module Resolvers required: false, description: 'Filter jobs by when they are executed.' - def resolve(statuses: nil, security_report_types: [], retried: nil, when_executed: nil) + argument :job_kind, ::Types::Ci::JobKindEnum, + required: false, + description: 'Filter jobs by kind.' + + def resolve(statuses: nil, security_report_types: [], retried: nil, when_executed: nil, job_kind: nil) jobs = init_collection(security_report_types) jobs = jobs.with_status(statuses) if statuses.present? jobs = jobs.retried if retried jobs = jobs.with_when_executed(when_executed) if when_executed.present? jobs = jobs.latest if retried == false + jobs = jobs.with_type(job_kind) if job_kind jobs end diff --git a/app/graphql/resolvers/metrics/dashboard_resolver.rb b/app/graphql/resolvers/metrics/dashboard_resolver.rb index d2be9fcdd89..5abad0de539 100644 --- a/app/graphql/resolvers/metrics/dashboard_resolver.rb +++ b/app/graphql/resolvers/metrics/dashboard_resolver.rb @@ -15,6 +15,7 @@ module Resolvers alias_method :environment, :object def resolve(path:) + return if Feature.enabled?(:remove_monitor_metrics) return unless environment ::PerformanceMonitoring::PrometheusDashboard.find_for(path: path, **service_params) diff --git a/app/graphql/types/environment_type.rb b/app/graphql/types/environment_type.rb index 5f58fc38540..a3737cbcd0d 100644 --- a/app/graphql/types/environment_type.rb +++ b/app/graphql/types/environment_type.rb @@ -53,7 +53,8 @@ module Types field :metrics_dashboard, Types::Metrics::DashboardType, null: true, description: 'Metrics dashboard schema for the environment.', - resolver: Resolvers::Metrics::DashboardResolver + resolver: Resolvers::Metrics::DashboardResolver, + deprecated: { reason: 'Returns no data. Underlying feature was removed in 16.0', milestone: '16.0' } field :latest_opened_most_severe_alert, Types::AlertManagement::AlertType, diff --git a/app/helpers/environment_helper.rb b/app/helpers/environment_helper.rb index d914c63b753..00109212934 100644 --- a/app/helpers/environment_helper.rb +++ b/app/helpers/environment_helper.rb @@ -79,7 +79,7 @@ module EnvironmentHelper can_destroy_environment: can_destroy_environment?(environment), can_stop_environment: can?(current_user, :stop_environment, environment), can_admin_environment: can?(current_user, :admin_environment, project), - environment_metrics_path: project_metrics_dashboard_path(project, environment: environment), + **environment_metrics_path(project, environment), environments_fetch_path: project_environments_path(project, format: :json), environment_edit_path: edit_project_environment_path(project, environment), environment_stop_path: stop_project_environment_path(project, environment), @@ -96,4 +96,10 @@ module EnvironmentHelper def environments_detail_data_json(user, project, environment) environments_detail_data(user, project, environment).to_json end + + def environment_metrics_path(project, environment) + return {} if Feature.enabled?(:remove_monitor_metrics) + + { environment_metrics_path: project_metrics_dashboard_path(project, environment: environment) } + end end diff --git a/app/helpers/environments_helper.rb b/app/helpers/environments_helper.rb index 5bf4fa2ffcc..525fdd3e9f6 100644 --- a/app/helpers/environments_helper.rb +++ b/app/helpers/environments_helper.rb @@ -22,6 +22,8 @@ module EnvironmentsHelper end def metrics_data(project, environment) + return {} if Feature.enabled?(:remove_monitor_metrics) + metrics_data = {} metrics_data.merge!(project_metrics_data(project)) if project metrics_data.merge!(environment_metrics_data(environment, project)) if environment diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 3d31d697452..d7ca76f6a8a 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -7,8 +7,8 @@ module SystemNoteHelper 'cherry_pick' => 'cherry-pick-commit', 'commit' => 'commit', 'description' => 'pencil', - 'merge' => 'merge', 'merged' => 'merge', + 'merge' => 'merge', 'opened' => 'issues', 'closed' => 'issue-close', 'time_tracking' => 'timer', @@ -53,6 +53,8 @@ module SystemNoteHelper def system_note_icon_name(note) if note.system_note_metadata&.action == 'closed' && note.for_merge_request? 'merge-request-close' + elsif note.system_note_metadata&.action == 'merge' && note.for_merge_request? + 'mr-system-note-empty' else ICON_NAMES_BY_ACTION[note.system_note_metadata&.action] end diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index b77e0f1d5c1..7cdd0d56a98 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -79,7 +79,9 @@ module Ci case pipeline.status when 'success' success! - when 'failed', 'canceled', 'skipped' + when 'canceled' + cancel! + when 'failed', 'skipped' drop! else false diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index da427a7e068..6dfea7ef9a7 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -68,6 +68,7 @@ class CommitStatus < Ci::ApplicationRecord where("#{quoted_table_name}.scheduled_at IS NOT NULL AND #{quoted_table_name}.scheduled_at < ?", date) } scope :with_when_executed, ->(when_executed) { where(when: when_executed) } + scope :with_type, ->(type) { where(type: type) } # The scope applies `pluck` to split the queries. Use with care. scope :for_project_paths, -> (paths) do diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 146ce2aa5b6..7c6fa24cd4d 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -396,12 +396,8 @@ class Namespace < ApplicationRecord # Includes projects from this namespace and projects from all subgroups # that belongs to this namespace def all_projects - if Feature.enabled?(:recursive_approach_for_all_projects) - namespace = user_namespace? ? self : self_and_descendant_ids - Project.where(namespace: namespace) - else - Project.inside_path(full_path) - end + namespace = user_namespace? ? self : self_and_descendant_ids + Project.where(namespace: namespace) end def has_parent? diff --git a/app/models/user_preference.rb b/app/models/user_preference.rb index 9c40376bb13..90449411f8a 100644 --- a/app/models/user_preference.rb +++ b/app/models/user_preference.rb @@ -29,6 +29,8 @@ class UserPreference < ApplicationRecord ignore_columns :experience_level, remove_with: '14.10', remove_after: '2021-03-22' ignore_columns :time_format_in_24h, remove_with: '16.2', remove_after: '2023-07-22' + # 2023-06-22 is after 16.1 release and during 16.2 release https://docs.gitlab.com/ee/development/database/avoiding_downtime_in_migrations.html#ignoring-the-column-release-m + ignore_columns :use_legacy_web_ide, remove_with: '16.2', remove_after: '2023-06-22' attribute :tab_width, default: -> { Gitlab::TabWidth::DEFAULT } attribute :time_display_relative, default: true diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb index 3473b4aebc8..6457127d831 100644 --- a/app/serializers/environment_entity.rb +++ b/app/serializers/environment_entity.rb @@ -27,7 +27,7 @@ class EnvironmentEntity < Grape::Entity ops.merge(except: UNNECESSARY_ENTRIES_FOR_UPCOMING_DEPLOYMENT)) end - expose :metrics_path, if: -> (*) { environment.has_metrics? } do |environment| + expose :metrics_path, if: -> (*) { expose_metrics_path? } do |environment| metrics_project_environment_path(environment.project, environment) end @@ -101,6 +101,10 @@ class EnvironmentEntity < Grape::Entity def cluster deployment_platform.cluster end + + def expose_metrics_path? + !Feature.enabled?(:remove_monitor_metrics) && environment.has_metrics? + end end EnvironmentEntity.prepend_mod_with('EnvironmentEntity') diff --git a/app/services/ci/reset_skipped_jobs_service.rb b/app/services/ci/reset_skipped_jobs_service.rb index eb809b0162c..cb793eb3e06 100644 --- a/app/services/ci/reset_skipped_jobs_service.rb +++ b/app/services/ci/reset_skipped_jobs_service.rb @@ -4,8 +4,10 @@ module Ci # This service resets skipped jobs so they can be processed again. # It affects the jobs that depend on the passed in job parameter. class ResetSkippedJobsService < ::BaseService - def execute(processable) - @processable = processable + def execute(processables) + @processables = Array.wrap(processables) + @pipeline = @processables.first.pipeline + @processable = @processables.first # Remove with FF `ci_support_reset_skipped_jobs_for_multiple_jobs` process_subsequent_jobs reset_source_bridge @@ -20,13 +22,13 @@ module Ci end def reset_source_bridge - @processable.pipeline.reset_source_bridge!(current_user) + @pipeline.reset_source_bridge!(current_user) end # rubocop: disable CodeReuse/ActiveRecord def dependent_jobs ordered_by_dag( - @processable.pipeline.processables + @pipeline.processables .from_union(needs_dependent_jobs, stage_dependent_jobs) .skipped .ordered_by_stage @@ -41,13 +43,27 @@ module Ci end def stage_dependent_jobs - @processable.pipeline.processables.after_stage(@processable.stage_idx) + if ::Feature.enabled?(:ci_support_reset_skipped_jobs_for_multiple_jobs, project) + # Get all jobs after the earliest stage of the inputted jobs + min_stage_idx = @processables.map(&:stage_idx).min + @pipeline.processables.after_stage(min_stage_idx) + else + @pipeline.processables.after_stage(@processable.stage_idx) + end end def needs_dependent_jobs - ::Gitlab::Ci::ProcessableObjectHierarchy.new( - ::Ci::Processable.where(id: @processable.id) - ).descendants + if ::Feature.enabled?(:ci_support_reset_skipped_jobs_for_multiple_jobs, project) + # We must include the hierarchy base here because @processables may include both a parent job + # and its dependents, and we do not want to exclude those dependents from being processed. + ::Gitlab::Ci::ProcessableObjectHierarchy.new( + ::Ci::Processable.where(id: @processables.map(&:id)) + ).base_and_descendants + else + ::Gitlab::Ci::ProcessableObjectHierarchy.new( + ::Ci::Processable.where(id: @processable.id) + ).descendants + end end def ordered_by_dag(jobs) diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb index 8921acb9ff1..0c13c32e236 100644 --- a/app/services/ci/runners/register_runner_service.rb +++ b/app/services/ci/runners/register_runner_service.rb @@ -14,7 +14,9 @@ module Ci return ServiceResponse.error(message: 'invalid token supplied', http_status: :forbidden) unless attrs_from_token unless registration_token_allowed?(attrs_from_token) - return ServiceResponse.error(message: 'runner registration disallowed', http_status: :forbidden) + return ServiceResponse.error( + message: 'runner registration disallowed', + reason: :runner_registration_disallowed) end runner = ::Ci::Runner.new(attributes.merge(attrs_from_token)) diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb index 6906ab2b642..0ab46bf236c 100644 --- a/app/services/protected_branches/base_service.rb +++ b/app/services/protected_branches/base_service.rb @@ -18,6 +18,8 @@ module ProtectedBranches def refresh_cache CacheService.new(@project_or_group, @current_user, @params).refresh + rescue StandardError => e + Gitlab::ErrorTracking.track_exception(e) end end end diff --git a/app/views/groups/settings/_git_access_protocols.html.haml b/app/views/groups/settings/_git_access_protocols.html.haml index 01e8536c7ad..db177da1d84 100644 --- a/app/views/groups/settings/_git_access_protocols.html.haml +++ b/app/views/groups/settings/_git_access_protocols.html.haml @@ -1,4 +1,4 @@ -- if group.root? && Feature.enabled?(:group_level_git_protocol_control, group) +- if group.root? .form-group = f.label _('Enabled Git access protocols'), class: 'label-bold' = f.select :enabled_git_access_protocol, options_for_select(enabled_git_access_protocol_options_for_group, group.enabled_git_access_protocol), {}, class: 'form-control', data: { qa_selector: 'enabled_git_access_protocol_dropdown' }, disabled: !::Gitlab::CurrentSettings.enabled_git_access_protocol.blank? diff --git a/app/views/layouts/devise.html.haml b/app/views/layouts/devise.html.haml index bed3995cb35..71771dd7cb6 100644 --- a/app/views/layouts/devise.html.haml +++ b/app/views/layouts/devise.html.haml @@ -10,18 +10,29 @@ .container.navless-container .content = render "layouts/flash" - .mt-3 - .col-sm-12.gl-text-center - = brand_image - %h1.mb-3.gl-font-size-h2 - = brand_title - - if current_appearance&.description? - = brand_text - = render_if_exists 'layouts/devise_help_text' - .mb-3 - .gl-w-half.gl-xs-w-full.gl-ml-auto.gl-mr-auto.bar - = yield - + - if current_appearance&.description? + .row + .col-md.order-12.sm-bg-gray-10 + .col-sm-12 + %h1.mb-3.gl-font-size-h2 + = brand_title + = brand_text + = render_if_exists 'layouts/devise_help_text' + .col-md.order-md-12 + .col-sm-12.bar + .gl-text-center + = brand_image + = yield + - else + .mt-3 + .col-sm-12.gl-text-center + = brand_image + %h1.mb-3.gl-font-size-h2 + = brand_title + = render_if_exists 'layouts/devise_help_text' + .mb-3 + .gl-w-half.gl-xs-w-full.gl-ml-auto.gl-mr-auto.bar + = yield = render 'devise/shared/footer', footer_message: footer_message - else diff --git a/app/workers/namespaces/process_sync_events_worker.rb b/app/workers/namespaces/process_sync_events_worker.rb index 112badd08b5..54169165f64 100644 --- a/app/workers/namespaces/process_sync_events_worker.rb +++ b/app/workers/namespaces/process_sync_events_worker.rb @@ -13,7 +13,7 @@ module Namespaces urgency :high idempotent! - deduplicate :until_executed, if_deduplicated: :reschedule_once + deduplicate :until_executed, if_deduplicated: :reschedule_once, ttl: 1.minute def perform results = ::Ci::ProcessSyncEventsService.new( diff --git a/app/workers/projects/process_sync_events_worker.rb b/app/workers/projects/process_sync_events_worker.rb index b088aed8fb7..93b6b6bfabd 100644 --- a/app/workers/projects/process_sync_events_worker.rb +++ b/app/workers/projects/process_sync_events_worker.rb @@ -13,7 +13,7 @@ module Projects urgency :high idempotent! - deduplicate :until_executed, if_deduplicated: :reschedule_once + deduplicate :until_executed, if_deduplicated: :reschedule_once, ttl: 1.minute def perform results = ::Ci::ProcessSyncEventsService.new( diff --git a/config/feature_flags/development/group_level_git_protocol_control.yml b/config/feature_flags/development/ci_support_reset_skipped_jobs_for_multiple_jobs.yml index ad9ba309d69..3dc826b627f 100644 --- a/config/feature_flags/development/group_level_git_protocol_control.yml +++ b/config/feature_flags/development/ci_support_reset_skipped_jobs_for_multiple_jobs.yml @@ -1,8 +1,8 @@ --- -name: group_level_git_protocol_control -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89817 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/365357 -milestone: '15.1' +name: ci_support_reset_skipped_jobs_for_multiple_jobs +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120060 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/410415 +milestone: '16.1' type: development -group: group::source code +group: group::pipeline authoring default_enabled: false diff --git a/config/feature_flags/development/disable_follow_users.yml b/config/feature_flags/development/disable_follow_users.yml index ead3687f302..1c9879899a2 100644 --- a/config/feature_flags/development/disable_follow_users.yml +++ b/config/feature_flags/development/disable_follow_users.yml @@ -5,4 +5,4 @@ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408886 milestone: '16.0' type: development group: group::authentication and authorization -default_enabled: false +default_enabled: enable diff --git a/config/feature_flags/development/recursive_approach_for_all_projects.yml b/config/feature_flags/development/recursive_approach_for_all_projects.yml deleted file mode 100644 index e2d656b7de2..00000000000 --- a/config/feature_flags/development/recursive_approach_for_all_projects.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: recursive_approach_for_all_projects -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64632 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334817 -milestone: '14.1' -type: development -group: group::fulfillment -default_enabled: true diff --git a/config/webpack.config.js b/config/webpack.config.js index 64a06912838..e3505ae47b8 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -891,6 +891,15 @@ module.exports = { ...(DEV_SERVER_ALLOWED_HOSTS ? { allowedHosts: DEV_SERVER_ALLOWED_HOSTS } : {}), client: { ...(DEV_SERVER_PUBLIC_ADDR ? { webSocketURL: DEV_SERVER_PUBLIC_ADDR } : {}), + overlay: { + runtimeErrors: (error) => { + if (error instanceof DOMException && error.message === 'The user aborted a request.') { + return false; + } + + return true; + }, + }, }, }, diff --git a/data/deprecations/16-0-deprecate-sidekiq-delivery-method-for-mailroom.yml b/data/deprecations/16-0-deprecate-sidekiq-delivery-method-for-mailroom.yml new file mode 100644 index 00000000000..a7715f89abb --- /dev/null +++ b/data/deprecations/16-0-deprecate-sidekiq-delivery-method-for-mailroom.yml @@ -0,0 +1,40 @@ +- title: '`sidekiq` delivery method for `incoming_email` and `service_desk_email` is deprecated' + announcement_milestone: '16.0' + removal_milestone: '17.0' + breaking_change: true + reporter: msaleiko + stage: Monitor + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/398132 + body: | + The `sidekiq` delivery method for `incoming_email` and `service_desk_email` is deprecated and is + scheduled for removal in GitLab 17.0. + + GitLab uses a separate process called `mail_room` to ingest emails. Currently, GitLab administrators + can configure their GitLab instances to use `sidekiq` or `webhook` delivery methods to deliver ingested + emails from `mail_room` to GitLab. + + Using the deprecated `sidekiq` delivery method, `mail_room` writes the job data directly to the GitLab + Redis queue. This means that there is a hard coupling between the delivery method and the Redis + configuration. Another disadvantage is that framework optimizations such as job payload compression are missed. + + Using the `webhook` delivery method, `mail_room` pushes the ingested email body to the GitLab + API. That way `mail_room` does not need to know your Redis configuration and the GitLab application + adds the processing job. `mail_room` authenticates with a shared secret key. + + Reconfiguring an Omnibus installation generates this secret key file automatically, + so no secret file configuration setting is needed. + + You can configure a custom secret key file (32 characters base 64 encoded) by running a command + like below and referencing the secret file in `incoming_email_secret_file` and + `service_desk_email_secret_file` (always specify the absolute path): + + ```shell + echo $( ruby -rsecurerandom -e "puts SecureRandom.base64(32)" ) > ~/.gitlab-mailroom-secret + ``` + + If you run GitLab on more than one machine, you need to provide the secret key file for each machine. + + We highly encourage GitLab administrators to start using the `webhook` delivery method for + `incoming_email_delivery_method` and `service_desk_email_delivery_method` instead of `sidekiq`. + tiers: [Free, Silver, Gold, Core, Premium, Ultimate] + documentation_url: https://docs.gitlab.com/ee/user/project/service_desk.html#use-a-custom-email-address diff --git a/db/docs/audit_events_google_cloud_logging_configurations.yml b/db/docs/audit_events_google_cloud_logging_configurations.yml new file mode 100644 index 00000000000..bd6c13a1fdf --- /dev/null +++ b/db/docs/audit_events_google_cloud_logging_configurations.yml @@ -0,0 +1,10 @@ +--- +table_name: audit_events_google_cloud_logging_configurations +classes: + - AuditEvents::GoogleCloudLoggingConfiguration +feature_categories: + - audit_events +description: Stores Google Cloud Logging configurations associated with IAM service accounts, used for generating access tokens. +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/issues/409421 +milestone: '16.0' +gitlab_schema: gitlab_main diff --git a/db/migrate/20230507192028_create_audit_events_google_cloud_logging_configurations.rb b/db/migrate/20230507192028_create_audit_events_google_cloud_logging_configurations.rb new file mode 100644 index 00000000000..1a32367382a --- /dev/null +++ b/db/migrate/20230507192028_create_audit_events_google_cloud_logging_configurations.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class CreateAuditEventsGoogleCloudLoggingConfigurations < Gitlab::Database::Migration[2.1] + enable_lock_retries! + + UNIQUE_INDEX_NAME = "unique_google_cloud_logging_configurations_on_namespace_id" + + # rubocop:disable Migration/AddLimitToTextColumns + def change + create_table :audit_events_google_cloud_logging_configurations do |t| + t.references :namespace, index: false, null: false, foreign_key: { on_delete: :cascade } + t.timestamps_with_timezone null: false + t.text :google_project_id_name, null: false, limit: 30 + t.text :client_email, null: false, limit: 254 + t.text :log_id_name, default: "audit_events", limit: 511 + t.binary :encrypted_private_key, null: false + t.binary :encrypted_private_key_iv, null: false + + t.index [:namespace_id, :google_project_id_name, :log_id_name], unique: true, name: UNIQUE_INDEX_NAME + end + end + # rubocop:enable Migration/AddLimitToTextColumns +end diff --git a/db/migrate/20230508074515_add_google_cloud_logging_configuration_limit_to_plan_limits.rb b/db/migrate/20230508074515_add_google_cloud_logging_configuration_limit_to_plan_limits.rb new file mode 100644 index 00000000000..a3a54fb55ea --- /dev/null +++ b/db/migrate/20230508074515_add_google_cloud_logging_configuration_limit_to_plan_limits.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddGoogleCloudLoggingConfigurationLimitToPlanLimits < Gitlab::Database::Migration[2.1] + def change + add_column(:plan_limits, :google_cloud_logging_configurations, :integer, default: 5, null: false) + end +end diff --git a/db/post_migrate/20230502014227_drop_partial_index_deployments_for_project_id_and_tag.rb b/db/post_migrate/20230502014227_drop_partial_index_deployments_for_project_id_and_tag.rb new file mode 100644 index 00000000000..864e0e74b97 --- /dev/null +++ b/db/post_migrate/20230502014227_drop_partial_index_deployments_for_project_id_and_tag.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class DropPartialIndexDeploymentsForProjectIdAndTag < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'partial_index_deployments_for_project_id_and_tag' + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name :deployments, name: INDEX_NAME + end + + def down + # This is based on the following `CREATE INDEX` command in db/init_structure.sql: + # CREATE INDEX partial_index_deployments_for_project_id_and_tag ON deployments + # USING btree (project_id) WHERE (tag IS TRUE); + add_concurrent_index :deployments, :project_id, name: INDEX_NAME, where: 'tag IS TRUE' + end +end diff --git a/db/post_migrate/20230512023321_prepare_audit_events_group_index.rb b/db/post_migrate/20230512023321_prepare_audit_events_group_index.rb new file mode 100644 index 00000000000..c3b27d6a909 --- /dev/null +++ b/db/post_migrate/20230512023321_prepare_audit_events_group_index.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +class PrepareAuditEventsGroupIndex < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + TABLE_NAME = :audit_events + COLUMN_NAMES = [:entity_id, :entity_type, :created_at, :id] + INDEX_NAME = 'index_audit_events_on_entity_id_and_entity_type_and_created_at' + + disable_ddl_transaction! + + def up + # Since audit_events is a partitioned table, we need to prepare the index + # for each partition individually. We can't use the `prepare_async_index` + # method directly because it will try to prepare the index for the whole + # table, which will fail. + + # In a future migration after this one, we will create the index on the + # parent table itself. + # TODO: Issue for future migration https://gitlab.com/gitlab-org/gitlab/-/issues/411129 + each_partition(TABLE_NAME) do |partition, partition_index_name| + prepare_async_index( + partition.identifier, + COLUMN_NAMES, + name: partition_index_name + ) + end + end + + def down + each_partition(TABLE_NAME) do |partition, partition_index_name| + unprepare_async_index_by_name(partition.identifier, partition_index_name) + end + end + + private + + def each_partition(table_name) + partitioned_table = find_partitioned_table(table_name) + partitioned_table.postgres_partitions.order(:name).each do |partition| + partition_index_name = generated_index_name(partition.identifier, INDEX_NAME) + + yield partition, partition_index_name + end + end +end diff --git a/db/schema_migrations/20230502014227 b/db/schema_migrations/20230502014227 new file mode 100644 index 00000000000..a5ed25b30de --- /dev/null +++ b/db/schema_migrations/20230502014227 @@ -0,0 +1 @@ +d1948970874f890d178db6b1df9053bf5bb45d701c8c295e1e8e3d7d6b4d175d
\ No newline at end of file diff --git a/db/schema_migrations/20230507192028 b/db/schema_migrations/20230507192028 new file mode 100644 index 00000000000..f6e1ec5c167 --- /dev/null +++ b/db/schema_migrations/20230507192028 @@ -0,0 +1 @@ +f248bac33290d490c88e79445a7600cb120761e3a8ee73e9e6ceb46d934399f2
\ No newline at end of file diff --git a/db/schema_migrations/20230508074515 b/db/schema_migrations/20230508074515 new file mode 100644 index 00000000000..f62368fbba6 --- /dev/null +++ b/db/schema_migrations/20230508074515 @@ -0,0 +1 @@ +7f3a70214dc73e754311019b208284cd2784ca4331458a98ec109e50598e7900
\ No newline at end of file diff --git a/db/schema_migrations/20230512023321 b/db/schema_migrations/20230512023321 new file mode 100644 index 00000000000..ad6c781e164 --- /dev/null +++ b/db/schema_migrations/20230512023321 @@ -0,0 +1 @@ +f2461838b62f7449f6b436c259724cb14b1ad5cd29cbff6f9e80e8b9e6f38984
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a57d1cbebdd..5280c06145f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12194,6 +12194,30 @@ CREATE SEQUENCE audit_events_external_audit_event_destinations_id_seq ALTER SEQUENCE audit_events_external_audit_event_destinations_id_seq OWNED BY audit_events_external_audit_event_destinations.id; +CREATE TABLE audit_events_google_cloud_logging_configurations ( + id bigint NOT NULL, + namespace_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + google_project_id_name text NOT NULL, + client_email text NOT NULL, + log_id_name text DEFAULT 'audit_events'::text, + encrypted_private_key bytea NOT NULL, + encrypted_private_key_iv bytea NOT NULL, + CONSTRAINT check_0ef835c61e CHECK ((char_length(client_email) <= 254)), + CONSTRAINT check_55783c7c19 CHECK ((char_length(google_project_id_name) <= 30)), + CONSTRAINT check_898a76b005 CHECK ((char_length(log_id_name) <= 511)) +); + +CREATE SEQUENCE audit_events_google_cloud_logging_configurations_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE audit_events_google_cloud_logging_configurations_id_seq OWNED BY audit_events_google_cloud_logging_configurations.id; + CREATE SEQUENCE audit_events_id_seq START WITH 1 INCREMENT BY 1 @@ -20054,7 +20078,8 @@ CREATE TABLE plan_limits ( notification_limit integer DEFAULT 0 NOT NULL, dashboard_limit_enabled_at timestamp with time zone, web_hook_calls integer DEFAULT 0 NOT NULL, - project_access_token_limit integer DEFAULT 0 NOT NULL + project_access_token_limit integer DEFAULT 0 NOT NULL, + google_cloud_logging_configurations integer DEFAULT 5 NOT NULL ); CREATE SEQUENCE plan_limits_id_seq @@ -24783,6 +24808,8 @@ ALTER TABLE ONLY audit_events ALTER COLUMN id SET DEFAULT nextval('audit_events_ ALTER TABLE ONLY audit_events_external_audit_event_destinations ALTER COLUMN id SET DEFAULT nextval('audit_events_external_audit_event_destinations_id_seq'::regclass); +ALTER TABLE ONLY audit_events_google_cloud_logging_configurations ALTER COLUMN id SET DEFAULT nextval('audit_events_google_cloud_logging_configurations_id_seq'::regclass); + ALTER TABLE ONLY audit_events_instance_external_audit_event_destinations ALTER COLUMN id SET DEFAULT nextval('audit_events_instance_external_audit_event_destinations_id_seq'::regclass); ALTER TABLE ONLY audit_events_streaming_event_type_filters ALTER COLUMN id SET DEFAULT nextval('audit_events_streaming_event_type_filters_id_seq'::regclass); @@ -26535,6 +26562,9 @@ ALTER TABLE ONLY atlassian_identities ALTER TABLE ONLY audit_events_external_audit_event_destinations ADD CONSTRAINT audit_events_external_audit_event_destinations_pkey PRIMARY KEY (id); +ALTER TABLE ONLY audit_events_google_cloud_logging_configurations + ADD CONSTRAINT audit_events_google_cloud_logging_configurations_pkey PRIMARY KEY (id); + ALTER TABLE ONLY audit_events_instance_external_audit_event_destinations ADD CONSTRAINT audit_events_instance_external_audit_event_destinations_pkey PRIMARY KEY (id); @@ -33033,8 +33063,6 @@ CREATE INDEX partial_index_ci_builds_on_scheduled_at_with_scheduled_jobs ON ci_b CREATE INDEX partial_index_deployments_for_legacy_successful_deployments ON deployments USING btree (id) WHERE ((finished_at IS NULL) AND (status = 2)); -CREATE INDEX partial_index_deployments_for_project_id_and_tag ON deployments USING btree (project_id) WHERE (tag IS TRUE); - CREATE INDEX partial_index_slack_integrations_with_bot_user_id ON slack_integrations USING btree (id) WHERE (bot_user_id IS NOT NULL); CREATE UNIQUE INDEX partial_index_sop_configs_on_namespace_id ON security_orchestration_policy_configurations USING btree (namespace_id) WHERE (namespace_id IS NOT NULL); @@ -33129,6 +33157,8 @@ CREATE UNIQUE INDEX uniq_pkgs_debian_project_distributions_project_id_and_suite CREATE UNIQUE INDEX unique_ci_builds_token_encrypted_and_partition_id ON ci_builds USING btree (token_encrypted, partition_id) WHERE (token_encrypted IS NOT NULL); +CREATE UNIQUE INDEX unique_google_cloud_logging_configurations_on_namespace_id ON audit_events_google_cloud_logging_configurations USING btree (namespace_id, google_project_id_name, log_id_name); + CREATE UNIQUE INDEX unique_idx_namespaces_storage_limit_exclusions_on_namespace_id ON namespaces_storage_limit_exclusions USING btree (namespace_id); CREATE UNIQUE INDEX unique_index_ci_build_pending_states_on_partition_id_build_id ON ci_build_pending_states USING btree (partition_id, build_id); @@ -35658,6 +35688,9 @@ ALTER TABLE ONLY operations_user_lists ALTER TABLE ONLY resource_link_events ADD CONSTRAINT fk_rails_0cea73eba5 FOREIGN KEY (child_work_item_id) REFERENCES issues(id) ON DELETE CASCADE; +ALTER TABLE ONLY audit_events_google_cloud_logging_configurations + ADD CONSTRAINT fk_rails_0eb52fc617 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY geo_node_statuses ADD CONSTRAINT fk_rails_0ecc699c2a FOREIGN KEY (geo_node_id) REFERENCES geo_nodes(id) ON DELETE CASCADE; diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 938c2b812e6..bca97d3d596 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -13999,6 +13999,10 @@ Returns [`Deployment`](#deployment). Metrics dashboard schema for the environment. +WARNING: +**Deprecated** in 16.0. +Returns no data. Underlying feature was removed in 16.0. + Returns [`MetricsDashboard`](#metricsdashboard). ###### Arguments @@ -18689,6 +18693,7 @@ four standard [pagination arguments](#connection-pagination-arguments): | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="pipelinejobsjobkind"></a>`jobKind` | [`CiJobKind`](#cijobkind) | Filter jobs by kind. | | <a id="pipelinejobsretried"></a>`retried` | [`Boolean`](#boolean) | Filter jobs by retry-status. | | <a id="pipelinejobssecurityreporttypes"></a>`securityReportTypes` | [`[SecurityReportTypeEnum!]`](#securityreporttypeenum) | Filter jobs by the type of security report they produce. | | <a id="pipelinejobsstatuses"></a>`statuses` | [`[CiJobStatus!]`](#cijobstatus) | Filter jobs by status. | diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index e4be851ec82..1be5f6204a1 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -12,9 +12,15 @@ info: To determine the technical writer assigned to the Stage/Group associated w > - `merge_user` was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/349031) as an eventual replacement for `merged_by` in GitLab 14.7. > - `merge_status` was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/3169#note_1162532204) in favor of `detailed_merge_status` in GitLab 15.6. > - `with_merge_status_recheck` [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115948) in GitLab 15.11 [with a flag](../administration/feature_flags.md) named `restrict_merge_status_recheck` to be ignored for requests from users insufficient permissions. Disabled by default. +> - `approvals_before_merge` was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/119503) in GitLab 16.0. Every API call to merge requests must be authenticated. +## Removals in API v5 + +The `approvals_before_merge` attribute has been deprecated, and is scheduled to be removed +in API v5 in favor of the [Merge request approvals API](merge_request_approvals.md). + ## List merge requests Get all merge requests the authenticated user has access to. By @@ -196,20 +202,6 @@ Supported attributes: ] ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -[ - { - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... - } -] -``` - ### Merge requests list response notes - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31890) in GitLab 13.0, listing merge requests may @@ -400,20 +392,6 @@ Supported attributes: ] ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -[ - { - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... - } -] -``` - For important notes on response data, read [Merge requests list response notes](#merge-requests-list-response-notes). ## List group merge requests @@ -587,20 +565,6 @@ Supported attributes: ] ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -[ - { - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... - } -] -``` - For important notes on response data, read [Merge requests list response notes](#merge-requests-list-response-notes). ## Get single MR @@ -630,7 +594,7 @@ Supported attributes: | Attribute | Type | Description | |----------------------------------|------|-------------| -| `approvals_before_merge` | integer | **(PREMIUM)** Number of approvals required before this merge request can merge. | +| `approvals_before_merge`| integer | **(PREMIUM)** Number of approvals required before this merge request can merge. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/353097) in GitLab 16.0. | | `assignee` | object | First assignee of the merge request. | | `assignees` | array | Assignees of the merge request. | | `author` | object | User who created this merge request. | @@ -753,7 +717,7 @@ Supported attributes: }, "has_conflicts": false, "blocking_discussions_resolved": true, - "approvals_before_merge": null, + "approvals_before_merge": null, // deprecated, use [Merge request approvals API](merge_request_approvals.md) "subscribed": true, "changes_count": "1", "latest_build_started_at": "2022-05-13T09:46:50.032Z", @@ -1280,8 +1244,8 @@ POST /projects/:id/merge_requests | `target_branch` | string | **{check-circle}** Yes | The target branch. | | `title` | string | **{check-circle}** Yes | Title of MR. | | `allow_collaboration` | boolean | **{dotted-circle}** No | Allow commits from members who can merge to the target branch. | +| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | Number of approvals required before this can be merged (see below). To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/353097) in GitLab 16.0. | | `allow_maintainer_to_push` | boolean | **{dotted-circle}** No | Alias of `allow_collaboration`. | -| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | Number of approvals required before this can be merged (see below). | | `assignee_id` | integer | **{dotted-circle}** No | Assignee user ID. | | `assignee_ids` | integer array | **{dotted-circle}** No | The ID of the users to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. | | `description` | string | **{dotted-circle}** No | Description of the merge request. Limited to 1,048,576 characters. | @@ -1293,13 +1257,6 @@ POST /projects/:id/merge_requests | `squash_on_merge` | boolean | no | Indicates if the merge request will be squashed when merged. | | `target_project_id` | integer | **{dotted-circle}** No | Numeric ID of the target project. | -If `approvals_before_merge` is not provided, it inherits the value from the target project. If provided, the following conditions must hold for it to take effect: - -- The target project's `approvals_before_merge` must be greater than zero. A - value of zero disables approvals for that project. -- The provided value of `approvals_before_merge` must be greater than the - target project's `approvals_before_merge`. - ```json { "id": 1, @@ -1421,18 +1378,6 @@ If `approvals_before_merge` is not provided, it inherits the value from the targ } ``` -Users of [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -{ - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... -} -``` - For important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes). ## Update MR @@ -1604,18 +1549,6 @@ Must include at least one non-required attribute from above. } ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -{ - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... -} -``` - For important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes). ## Delete a merge request @@ -1793,18 +1726,6 @@ Supported attributes: } ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -{ - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... -} -``` - This API returns specific HTTP status codes on failure: | HTTP Status | Message | Reason | @@ -2006,18 +1927,6 @@ Supported attributes: } ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -{ - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... -} -``` - For important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes). ## Rebase a merge request @@ -2318,18 +2227,6 @@ Example response: } ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -{ - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... -} -``` - For important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes). ## Unsubscribe from a merge request @@ -2489,18 +2386,6 @@ Example response: } ``` -Users on [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) also see -the `approvals_before_merge` parameter: - -```json -{ - "id": 1, - "title": "test1", - "approvals_before_merge": null - ... -} -``` - For important notes on response data, read [Single merge request response notes](#single-merge-request-response-notes). ## Create a to-do item diff --git a/doc/api/projects.md b/doc/api/projects.md index ca57ca76a81..3105da44906 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -17,6 +17,16 @@ For details, see [Project visibility](../user/public_access.md). The fields returned in responses vary based on the [permissions](../user/permissions.md) of the authenticated user. +## Removals in API v5 + +These attributes are deprecated, and are scheduled to be removed in v5 of the API: + +- `tag_list`: Use the `topics` attribute instead. +- `marked_for_deletion_at`: Use the `marked_for_deletion_on` attribute instead. + Available only to [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/). +- `approvals_before_merge`: Use the [Merge request approvals API](merge_request_approvals.md) instead. + Available only to [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/). + ## Project merge method The `merge_method` can use these options: @@ -266,29 +276,6 @@ When the user is authenticated and `simple` is not set this returns something li ] ``` -NOTE: -The `tag_list` attribute has been deprecated -and is removed in API v5 in favor of the `topics` attribute. - -NOTE: -For users of [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/), -the `marked_for_deletion_at` attribute has been deprecated, and is removed -in API v5 in favor of the `marked_for_deletion_on` attribute. - -Users of [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) -can also see the `approvals_before_merge` parameter: - -```json -[ - { - "id": 4, - "description": null, - "approvals_before_merge": 0, - ... - } -] -``` - You can filter by [custom attributes](custom_attributes.md) with: ```plaintext @@ -548,7 +535,7 @@ GET /users/:user_id/projects "auto_devops_enabled": true, "auto_devops_deploy_strategy": "continuous", "repository_storage": "default", - "approvals_before_merge": 0, + "approvals_before_merge": 0, // Deprecated. Use merge request approvals API instead. "mirror": false, "mirror_user_id": 45, "mirror_trigger_builds": false, @@ -807,7 +794,7 @@ Example response: "auto_devops_enabled": true, "auto_devops_deploy_strategy": "continuous", "repository_storage": "default", - "approvals_before_merge": 0, + "approvals_before_merge": 0, // Deprecated. Use merge request approvals API instead. "mirror": false, "mirror_user_id": 45, "mirror_trigger_builds": false, @@ -989,7 +976,7 @@ GET /projects/:id "squash_option": "default_on", "auto_devops_enabled": true, "auto_devops_deploy_strategy": "continuous", - "approvals_before_merge": 0, + "approvals_before_merge": 0, // Deprecated. Use merge request approvals API instead. "mirror": false, "mirror_user_id": 45, "mirror_trigger_builds": false, @@ -1034,22 +1021,6 @@ GET /projects/:id } ``` -NOTE: -The `tag_list` attribute has been deprecated -and is removed in API v5 in favor of the `topics` attribute. - -Users of [GitLab Premium or Ultimate](https://about.gitlab.com/pricing/) -can also see the `approvals_before_merge` parameter: - -```json -{ - "id": 3, - "description": null, - "approvals_before_merge": 0, - ... -} -``` - Users of [GitLab Ultimate](https://about.gitlab.com/pricing/) can also see the `only_allow_merge_if_all_status_checks_passed` parameters using GitLab 15.5 and later: @@ -1275,7 +1246,7 @@ curl --request POST --header "PRIVATE-TOKEN: <your-token>" \ | `allow_merge_on_skipped_pipeline` | boolean | **{dotted-circle}** No | Set whether or not merge requests can be merged with skipped jobs. | | `only_allow_merge_if_all_status_checks_passed` **(ULTIMATE)** | boolean | **{dotted-circle}** No | Indicates that merges of merge requests should be blocked unless all status checks have passed. Defaults to false. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/369859) in GitLab 15.5 with feature flag `only_allow_merge_if_all_status_checks_passed` disabled by default. | | `analytics_access_level` | string | **{dotted-circle}** No | One of `disabled`, `private` or `enabled` | -| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | How many approvers should approve merge requests by default. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). | +| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | How many approvers should approve merge requests by default. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/353097) in GitLab 16.0. | | `auto_cancel_pending_pipelines` | string | **{dotted-circle}** No | Auto-cancel pending pipelines. This action toggles between an enabled state and a disabled state; it is not a boolean. | | `auto_devops_deploy_strategy` | string | **{dotted-circle}** No | Auto Deploy strategy (`continuous`, `manual` or `timed_incremental`). | | `auto_devops_enabled` | boolean | **{dotted-circle}** No | Enable Auto DevOps for this project. | @@ -1361,7 +1332,7 @@ POST /projects/user/:user_id | `allow_merge_on_skipped_pipeline` | boolean | **{dotted-circle}** No | Set whether or not merge requests can be merged with skipped jobs. | | `only_allow_merge_if_all_status_checks_passed` **(ULTIMATE)** | boolean | **{dotted-circle}** No | Indicates that merges of merge requests should be blocked unless all status checks have passed. Defaults to false. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/369859) in GitLab 15.5 with feature flag `only_allow_merge_if_all_status_checks_passed` disabled by default. | | `analytics_access_level` | string | **{dotted-circle}** No | One of `disabled`, `private` or `enabled` | -| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | How many approvers should approve merge requests by default. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). | +| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | How many approvers should approve merge requests by default. [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/353097) in GitLab 16.0. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). | | `auto_cancel_pending_pipelines` | string | **{dotted-circle}** No | Auto-cancel pending pipelines. This action toggles between an enabled state and a disabled state; it is not a boolean. | | `auto_devops_deploy_strategy` | string | **{dotted-circle}** No | Auto Deploy strategy (`continuous`, `manual` or `timed_incremental`). | | `auto_devops_enabled` | boolean | **{dotted-circle}** No | Enable Auto DevOps for this project. | @@ -1460,7 +1431,7 @@ Supported attributes: | `allow_pipeline_trigger_approve_deployment` **(PREMIUM)** | boolean | **{dotted-circle}** No | Set whether or not a pipeline triggerer is allowed to approve deployments. | | `only_allow_merge_if_all_status_checks_passed` **(ULTIMATE)** | boolean | **{dotted-circle}** No | Indicates that merges of merge requests should be blocked unless all status checks have passed. Defaults to false.<br/><br/>[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/369859) in GitLab 15.5 with feature flag `only_allow_merge_if_all_status_checks_passed` disabled by default. The feature flag was enabled by default in GitLab 15.9. | | `analytics_access_level` | string | **{dotted-circle}** No | One of `disabled`, `private` or `enabled` | -| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | How many approvers should approve merge request by default. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). | +| `approvals_before_merge` **(PREMIUM)** | integer | **{dotted-circle}** No | How many approvers should approve merge requests by default. [Deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/353097) in GitLab 16.0. To configure approval rules, see [Merge request approvals API](merge_request_approvals.md). | | `auto_cancel_pending_pipelines` | string | **{dotted-circle}** No | Auto-cancel pending pipelines. This action toggles between an enabled state and a disabled state; it is not a boolean. | | `auto_devops_deploy_strategy` | string | **{dotted-circle}** No | Auto Deploy strategy (`continuous`, `manual`, or `timed_incremental`). | | `auto_devops_enabled` | boolean | **{dotted-circle}** No | Enable Auto DevOps for this project. | @@ -2882,7 +2853,7 @@ Example response: "auto_devops_enabled": true, "auto_devops_deploy_strategy": "continuous", "autoclose_referenced_issues": true, - "approvals_before_merge": 0, + "approvals_before_merge": 0, // Deprecated. Use merge request approvals API instead. "mirror": false, "compliance_frameworks": [] } diff --git a/doc/api/runners.md b/doc/api/runners.md index 8d0be1c3aba..a2614b95bb9 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -679,9 +679,11 @@ curl --request POST "https://gitlab.example.com/api/v4/runners" \ Response: -| Status | Description | -|-----------|---------------------------------| -| 201 | Runner was created | +| Status | Description | +|-----------|-----------------------------------| +| 201 | Runner was created | +| 403 | Invalid runner registration token | +| 410 | Runner registration disabled | Example response: diff --git a/doc/api/users.md b/doc/api/users.md index f4547c06582..a2293090209 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -875,7 +875,7 @@ Parameters: | :------------------------------- | :------- | :--------------------------------------------------------------------------- | | `view_diffs_file_by_file` | Yes | Flag indicating the user sees only one file diff per page. | | `show_whitespace_in_diffs` | Yes | Flag indicating the user sees whitespace changes in diffs. | -| `pass_user_identities_to_ci_jwt` | Yes | Flag indicating the user passes their external identities as CI information. | +| `pass_user_identities_to_ci_jwt` | Yes | Flag indicating the user passes their external identities as CI information. This attribute does not contain enough information to identify or authorize the user in an external system. The attribute is internal to GitLab, and must not be passed to third-party services. | ## User follow diff --git a/doc/architecture/blueprints/organization/index.md b/doc/architecture/blueprints/organization/index.md index 8f835a33a4f..bd8d085413c 100644 --- a/doc/architecture/blueprints/organization/index.md +++ b/doc/architecture/blueprints/organization/index.md @@ -16,34 +16,37 @@ This document is a work in progress and represents the current state of the Orga ## Glossary -- Organization: An organization is the umbrella for one or multiple top-level groups. Organizations are isolated from each other by default meaning that cross-namespace features will only work for namespaces that exist in a single organization. +- Organization: An Organization is the umbrella for one or multiple top-level groups. Organizations are isolated from each other by default meaning that cross-namespace features will only work for namespaces that exist in a single Organization. - Top-level group: Top-level group is the name given to the topmost group of all other groups. Groups and projects are nested underneath the top-level group. -- Cell: A Cell is a set of infrastructure components that contains multiple organizations. The infrastructure components provided in a Cell are shared among organizations, but not shared with other Cells. This isolation of infrastructure components means that Cells are independent from each other. +- Cell: A Cell is a set of infrastructure components that contains multiple Organizations. The infrastructure components provided in a Cell are shared among Organizations, but not shared with other Cells. This isolation of infrastructure components means that Cells are independent from each other. +- User: An Organization has many users. Joining an Organization makes someone a user of that Organization. +- Member: Adding a user to a group or project within an Organization makes them a member. Members are always users, but users are not necessarily members of a group or project within an Organization. For instance, a user could just have accepted the invitation to join an Organization, but not be a member of any group or project it contains. +- Non-user: A non-user of an Organization means a user is not part of that specific Organization. ## Summary Organizations solve the following problems: -1. Enables grouping of top-level groups. For example, the following top-level groups would belong to the organization `GitLab`: +1. Enables grouping of top-level groups. For example, the following top-level groups would belong to the Organization `GitLab`: 1. `https://gitlab.com/gitlab-org/` 1. `https://gitlab.com/gitlab-com/` -1. Allows different organizations to be isolated. Top-level groups of the same organization can interact with each other but not with groups in other organizations, providing clear boundaries for an organization, similar to a self-managed instance. Isolation should have a positive impact on performance and availability as things like user dashboards can be scoped to organizations. -1. Allows integration with Cells. Isolating organizations makes it possible to allocate and distribute them across different Cells. -1. Removes the need to define hierarchies. An organization is a container that could be filled with whatever hierarchy/entity set makes sense (organization, top-level groups, etc.) -1. Enables centralized control of user profiles. With an organization-specific user profile, administrators can control the user's role in a company, enforce user emails, or show a graphical indicator that a user as part of the organization. An example could be adding a "GitLab employee" stamp on comments. -1. Organizations bring an on-premise-like experience to SaaS (GitLab.com). The organization admin will have access to instance-equivalent Admin Area settings with most of the configuration controlled on organization level. +1. Allows different Organizations to be isolated. Top-level groups of the same Organization can interact with each other but not with groups in other Organizations, providing clear boundaries for an Organization, similar to a self-managed instance. Isolation should have a positive impact on performance and availability as things like user dashboards can be scoped to Organizations. +1. Allows integration with Cells. Isolating Organizations makes it possible to allocate and distribute them across different Cells. +1. Removes the need to define hierarchies. An Organization is a container that could be filled with whatever hierarchy/entity set makes sense (Organization, top-level groups, etc.) +1. Enables centralized control of user profiles. With an Organization-specific user profile, administrators can control the user's role in a company, enforce user emails, or show a graphical indicator that a user as part of the Organization. An example could be adding a "GitLab employee" stamp on comments. +1. Organizations bring an on-premise-like experience to SaaS (GitLab.com). The Organization admin will have access to instance-equivalent Admin Area settings with most of the configuration controlled on Organization level. ## Motivation ### Goals -The Organization focuses on creating a better experience for organizations to manage their GitLab experience. By introducing Organizations and [Cells](../cells/index.md) we can improve the reliability, performance and availability of our SaaS Platforms. +The Organization focuses on creating a better experience for Organizations to manage their GitLab experience. By introducing Organizations and [Cells](../cells/index.md) we can improve the reliability, performance and availability of our SaaS Platforms. - Wider audience: Many instance-level features are admin only. We do not want to lock out users of GitLab.com in that way. We want to make administrative capabilities that previously only existed for self-managed users available to our SaaS users as well. This also means we would give users of GitLab.com more independence from GitLab.com admins in the long run. Today, there are actions that self-managed admins can perform that GitLab.com users have to request from GitLab.com admins. -- Improved UX: Inconsistencies between the features available at the project and group levels create navigation and usability issues. Moreover, there isn't a dedicated place for organization-level features. -- Aggregation: Data from all groups and projects in an organization can be aggregated. -- An organization includes settings, data, and features from all groups and projects under the same owner (including personal namespaces). -- Cascading behavior: Organization cascades behavior to all the projects and groups that are owned by the same organization. It can be decided at the organization level whether a setting can be overridden or not on the levels beneath. +- Improved UX: Inconsistencies between the features available at the project and group levels create navigation and usability issues. Moreover, there isn't a dedicated place for Organization-level features. +- Aggregation: Data from all groups and projects in an Organization can be aggregated. +- An Organization includes settings, data, and features from all groups and projects under the same owner (including personal namespaces). +- Cascading behavior: Organization cascades behavior to all the projects and groups that are owned by the same Organization. It can be decided at the Organization level whether a setting can be overridden or not on the levels beneath. ### Non-Goals @@ -64,29 +67,109 @@ graph TD ns[Namespace] -. has many .- ns[Namespace] ``` -Self-managed instances would set a default organization. +Self-managed instances would set a default Organization. ### Benefits -- No changes to URL's for groups moving under an organization, which makes moving around top-level groups very easy. +- No changes to URL's for groups moving under an Organization, which makes moving around top-level groups very easy. - Low risk rollout strategy, as there is no conversion process for existing top-level groups. - Organization becomes the key for identifying what is part of an Organization, which is likely on its own table for performance and clarity. ### Drawbacks -- It is unclear right now how we would avoid continuing to spend effort to build instance (or not Organization) features, in particular much of the reporting. This is not an issue on SaaS as top-level groups already have this capability, however it is a challenge on self-managed. If we introduce a built-in Organization (or just none at all) for self-managed, it seems like we would need to continue to build instance/organization level reporting features as we would not get that for free along with the work to add to groups. +- It is unclear right now how we would avoid continuing to spend effort to build instance (or not Organization) features, in particular much of the reporting. This is not an issue on SaaS as top-level groups already have this capability, however it is a challenge on self-managed. If we introduce a built-in Organization (or just none at all) for self-managed, it seems like we would need to continue to build instance/Organization level reporting features as we would not get that for free along with the work to add to groups. - Billing may need to be moved from top-level groups to Organization level. ## Design and Implementation Details +### Organization MVC + +The Organization MVC will contain the following functionality: + +- Instance setting to allow the creation of multiple Organizations. This will be enabled by default on GitLab.com, and disabled for self-managed GitLab. +- Every instance will have a default organization. Initially, all users will be managed by this default Organization. +- Organization Owner. The creation of an Organization appoints that user as the Organization Owner. Once established, the Organization Owner can appoint other Organization Owners. +- Organization users. A user is managed by one Organization, but can be part of multiple Organizations. +- Setup settings. Containing the Organization name, ID, description, README, and avatar. Settings are editable by the Organization Owner. +- Setup flow. Users are able to build an Organization on top of an existing top-level group. New users are able to create an Organization from scratch and to start building top-level groups from there. +- Visibility. Options will be `public` and `private`. A nonuser of a specific Organization will not see private Organizations in the explore section. Visibility is editable by the Organization Owner. +- Organization settings page with the added ability to remove an Organization. Deletion of the default Organization is prevented. +- Groups. This includes the ability to create, edit, and delete groups, as well as a Groups overview that can be accessed by the Organization Owner. +- Projects. This includes the ability to create, edit, and delete projects, as well as a Projects overview that can be accessed by the Organization Owner. + +### Organization Access + +#### Organization Users + +Organization Users can get access to groups and projects as: + +- A group member: this grants access to the group and all its projects, regardless of their visibility. +- A project member: this grants access to the project, and limited access to parent groups, regardless of their visibility. +- A non-member: this grants access to public and internal groups and projects of that Organization. To access a private group or project in an Organization, a user must become a member. + +Organization Users can be managed by the Organization as: + +- Enterprise Users, managed by the Organization. This includes control over their user account and the ability to block the user. +- Non-Enterprise Users, managed by the User. Non-Enterprise Users can be removed from an Organization, but their user account remains in their control. + +Enterprise Users are only available to Organizations with a Premium or Ultimate subscription. Organizations on the free tier will only be able to host Non-Enterprise Users. + +#### Organization Non-Users + +Non-users are external to the Organization and can only access the public resources of an Organization, such as public projects. + ## Iteration Plan -### Iteration 0: Organization MVC +The following iteration plan outlines how we intend to arrive at the Organization MVC. We are following the guidelines for [Experiment, Beta, and Generally Available features](../../../policy/alpha-beta-support.md). + +### Iteration 1: Organization Prototype (FY24Q2) + +In iteration 1, we introduce the concept of an Organization as a way to group top-level groups together. Support for Organizations does not require any [Cells](../cells/index.md) work, but having them will make all subsequent iterations of Cells simpler. The goal of iteration 1 will be to generate a prototype that can be used by GitLab teams to test moving functionality to the Organization. It contains everything that is necessary to move an Organization to a Cell: -In the first iteration, we introduce the concept of an Organization as a way to group top-level groups together. Support for Organizations does not require any [Cells](../cells/index.md) work, but having them will make all subsequent iterations of Cells simpler. +- The Organization can be named, has an ID and an avatar. +- Only non-enterprise user can be part of an Organization. +- A user can be part of multiple Organizations. +- A single Organization Owner can be assigned. +- Groups can be created in an Organization. Groups are listed in the Groups overview. +- Projects can be created in a Group. Projects are listed in the Projects overview. -### Iteration 1 +### Iteration 2: Organization MVC Experiment (FY24Q3) + +In iteration 2, an Organization MVC Experiment will be released. We will test the functionality with a select set of customers and improve the MVC based on these learnings. Users will be able to build an Organization on top of their existing top-level group. + +- The Organization has a description and a README. + +### Iteration 3: Organization MVC Beta (FY24Q4) + +In iteration 3, the Organization MVC Beta will be released. + +- Multiple Organization Owners can be assigned. +- Enterprise users can be added to an Organization. + +### Iteration 4: Organization MVC GA (FY25Q1) + +### Post-MVC Iterations + +After the initial rollout of Organizations, the following functionality will be added to address customer needs relating to their implementation of GitLab: + +1. Internal visibility will be made available on Organizations that are part of GitLab.com. +1. Move billing from top-level group to Organization. +1. Audit events at the Organization level. +1. Set merge request approval rules at the Organization level and cascade to all groups and projects. +1. Security policies at the Organization level. +1. Vulnerability reports at the Organization level. +1. Cascading Organization setting to enforce security scans. +1. Scan result policies at the Organization level. +1. Compliance frameworks. ## Alternative Solutions An alternative approach to building Organizations is to convert top-level groups into Organizations. The main advantage of this approach is that features could be built on top of the namespace framework and therewith leverage functionality that is already available at the group level. We would avoid building the same feature multiple times. However, Organizations have been identified as a critical driver of Cells. Due to the urgency of delivering Cells, we decided to opt for the quickest and most straightforward solution to deliver an Organization, which is the lightweight design described above. More details on comparing the two Organization proposals can be found [here](https://gitlab.com/gitlab-org/tenant-scale-group/group-tasks/-/issues/56). + +## Decision Log + +- 2023-05-10: [Billing is not part of the Organization MVC](https://gitlab.com/gitlab-org/gitlab/-/issues/406614#note_1384055365) + +## Links + +- [Organization epic](https://gitlab.com/groups/gitlab-org/-/epics/9265) diff --git a/doc/ci/components/index.md b/doc/ci/components/index.md index 82040d5990c..95a513220a2 100644 --- a/doc/ci/components/index.md +++ b/doc/ci/components/index.md @@ -177,7 +177,7 @@ For example, for a component repository located at `gitlab-org/dast` on `gitlab. - If a tag is named the same as a commit SHA that exists, like `e3262fdd0914fa823210cdb79a8c421e2cef79d8`, the commit SHA takes precedence over the tag. -## Components catalog +## CI/CD Catalog The CI/CD Catalog is a list of [components repositories](#components-repository), each containing resources that you can add to your CI/CD pipelines. diff --git a/doc/ci/docker/using_docker_build.md b/doc/ci/docker/using_docker_build.md index 3f47af11380..fe57b451146 100644 --- a/doc/ci/docker/using_docker_build.md +++ b/doc/ci/docker/using_docker_build.md @@ -716,3 +716,10 @@ services: - name: registry.hub.docker.com/library/docker:20.10.16-dind alias: docker ``` + +### Error response from daemon: Get "https://registry-1.docker.io/v2/": unauthorized: incorrect username or password + +This error appears when you use the deprecated variable, `CI_BUILD_TOKEN`. To prevent users from receiving this error, you should: + +- Use [CI_JOB_TOKEN](../jobs/ci_job_token.md) instead. +- Change from `gitlab-ci-token/CI_BUILD_TOKEN` to `$CI_REGISTRY_USER/$CI_REGISTRY_PASSWORD`. diff --git a/doc/ci/environments/deployment_safety.md b/doc/ci/environments/deployment_safety.md index 0cfaf2b3fe9..8be46da3fa8 100644 --- a/doc/ci/environments/deployment_safety.md +++ b/doc/ci/environments/deployment_safety.md @@ -126,6 +126,10 @@ vacation period when most employees are out, you can set up a [Deploy Freeze](.. During a deploy freeze period, no deployment can be executed. This is helpful to ensure that deployments do not happen unexpectedly. +The next configured deploy freeze is displayed at the top of the +[environment deployments list](index.md#view-environments-and-deployments) +page. + ## Protect production secrets Production secrets are needed to deploy successfully. For example, when deploying to the cloud, diff --git a/doc/ci/jobs/job_control.md b/doc/ci/jobs/job_control.md index 3cd57ff6a6a..fa045a898fa 100644 --- a/doc/ci/jobs/job_control.md +++ b/doc/ci/jobs/job_control.md @@ -10,11 +10,7 @@ When a new pipeline starts, GitLab checks the pipeline configuration to determin which jobs should run in that pipeline. You can configure jobs to run depending on factors like the status of variables, or the pipeline type. -To configure a job to be included or excluded from certain pipelines, you can use: - -- [`rules`](../yaml/index.md#rules) -- [`only`](../yaml/index.md#only--except) -- [`except`](../yaml/index.md#only--except) +To configure a job to be included or excluded from certain pipelines, use [`rules`](../yaml/index.md#rules). Use [`needs`](../yaml/index.md#needs) to configure a job to run as soon as the earlier jobs it depends on finish running. diff --git a/doc/development/logging.md b/doc/development/logging.md index 084bad70b90..908fa16d3f8 100644 --- a/doc/development/logging.md +++ b/doc/development/logging.md @@ -87,6 +87,28 @@ importer progresses. Here's what to do: end ``` + Note that by default, `Gitlab::JsonLogger` will include application context metadata in the log entry. If your + logger is expected to be called outside of an application request (for example, in a `rake` task) or by low-level + code that may be involved in building the application context (for example, database connection code), you should + call the class method `exclude_context!` for your logger class, like so: + + ```ruby + module Gitlab + module Database + module LoadBalancing + class Logger < ::Gitlab::JsonLogger + exclude_context! + + def self.file_name_noext + 'database_load_balancing' + end + end + end + end + end + + ``` + 1. In your class where you want to log, you might initialize the logger as an instance variable: ```ruby diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index a5345527203..8261330223d 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -424,7 +424,7 @@ a file for quick reference. ## Show obsolete `ignored_columns` -To see a list of all obsolete `ignored_columns` run: +To see a list of all obsolete `ignored_columns` definitions run: ```shell bundle exec rake db:obsolete_ignored_columns diff --git a/doc/install/installation.md b/doc/install/installation.md index a8e498674a6..28aa37f0d1b 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -291,7 +291,7 @@ sudo adduser --disabled-login --gecos 'GitLab' git ## 7. Database NOTE: -In GitLab 12.1 and later, only PostgreSQL is supported. In GitLab 14.0 and later, we [require PostgreSQL 12+](requirements.md#postgresql-requirements). +In GitLab 12.1 and later, only PostgreSQL is supported. In GitLab 16.0 and later, we [require PostgreSQL 13+](requirements.md#postgresql-requirements). 1. Install the database packages. diff --git a/doc/install/requirements.md b/doc/install/requirements.md index 8d779ec978d..7fdbdfc2b24 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -89,7 +89,7 @@ the following table) as these were used for development and testing: | 13.0 | 11 | | 14.0 | 12.7 | | 15.0 | 12.10 | -| 16.0 (planned) | 13.6 | +| 16.0 | 13.6 | You must also ensure the following extensions are loaded into every GitLab database. [Read more about this requirement, and troubleshooting](postgresql_extensions.md). diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index 9d82bbafe88..18303a5f45c 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -665,11 +665,20 @@ You may need to reconfigure or restart GitLab for the changes to take effect. 1. Clear all the tokens for pending jobs: + For GitLab 15.3 and earlier: + ```sql -- Clear build tokens UPDATE ci_builds SET token = null, token_encrypted = null; ``` + For GitLab 15.4 and later: + + ```sql + -- Clear build tokens + UPDATE ci_builds SET token_encrypted = null; + ``` + A similar strategy can be employed for the remaining features. By removing the data that can't be decrypted, GitLab can be returned to operation, and the lost data can be manually replaced. diff --git a/doc/raketasks/restore_gitlab.md b/doc/raketasks/restore_gitlab.md index c0e62187e9b..1a8a95f8b42 100644 --- a/doc/raketasks/restore_gitlab.md +++ b/doc/raketasks/restore_gitlab.md @@ -97,6 +97,9 @@ sudo gitlab-ctl stop sidekiq sudo gitlab-ctl status ``` +Next, ensure you have completed the [restore prerequisites](#restore-prerequisites) steps and have run `gitlab-ctl reconfigure` +after copying over the GitLab secrets file from the original installation. + Next, restore the backup, specifying the timestamp of the backup you wish to restore: @@ -123,13 +126,9 @@ WARNING: The restore command requires [additional parameters](backup_restore.md#back-up-and-restore-for-installations-using-pgbouncer) when your installation is using PgBouncer, for either performance reasons or when using it with a Patroni cluster. -Next, restore `/etc/gitlab/gitlab-secrets.json` if necessary, -[as previously mentioned](#restore-prerequisites). - -Reconfigure, restart and [check](../administration/raketasks/maintenance.md#check-gitlab-configuration) GitLab: +Next, restart and [check](../administration/raketasks/maintenance.md#check-gitlab-configuration) GitLab: ```shell -sudo gitlab-ctl reconfigure sudo gitlab-ctl restart sudo gitlab-rake gitlab:check SANITIZE=true ``` diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index 5d0985ca4a1..e92bfa96a4b 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -614,6 +614,49 @@ From GitLab 17.0 and later, the methods to register runners introduced by the ne <div class="deprecation breaking-change" data-milestone="17.0"> +### `sidekiq` delivery method for `incoming_email` and `service_desk_email` is deprecated + +<div class="deprecation-notes"> +- Announced in: GitLab <span class="milestone">16.0</span> +- This is a [breaking change](https://docs.gitlab.com/ee/development/deprecation_guidelines/). +- To discuss this change or learn more, see the [deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/398132). +</div> + +The `sidekiq` delivery method for `incoming_email` and `service_desk_email` is deprecated and is +scheduled for removal in GitLab 17.0. + +GitLab uses a separate process called `mail_room` to ingest emails. Currently, GitLab administrators +can configure their GitLab instances to use `sidekiq` or `webhook` delivery methods to deliver ingested +emails from `mail_room` to GitLab. + +Using the deprecated `sidekiq` delivery method, `mail_room` writes the job data directly to the GitLab +Redis queue. This means that there is a hard coupling between the delivery method and the Redis +configuration. Another disadvantage is that framework optimizations such as job payload compression are missed. + +Using the `webhook` delivery method, `mail_room` pushes the ingested email body to the GitLab +API. That way `mail_room` does not need to know your Redis configuration and the GitLab application +adds the processing job. `mail_room` authenticates with a shared secret key. + +Reconfiguring an Omnibus installation generates this secret key file automatically, +so no secret file configuration setting is needed. + +You can configure a custom secret key file (32 characters base 64 encoded) by running a command +like below and referencing the secret file in `incoming_email_secret_file` and +`service_desk_email_secret_file` (always specify the absolute path): + +```shell +echo $( ruby -rsecurerandom -e "puts SecureRandom.base64(32)" ) > ~/.gitlab-mailroom-secret +``` + +If you run GitLab on more than one machine, you need to provide the secret key file for each machine. + +We highly encourage GitLab administrators to start using the `webhook` delivery method for +`incoming_email_delivery_method` and `service_desk_email_delivery_method` instead of `sidekiq`. + +</div> + +<div class="deprecation breaking-change" data-milestone="17.0"> + ### project.pipeline.securityReportFindings GraphQL query <div class="deprecation-notes"> diff --git a/doc/update/upgrading_from_source.md b/doc/update/upgrading_from_source.md index 8e4b6aa62db..7e2c9bf53dd 100644 --- a/doc/update/upgrading_from_source.md +++ b/doc/update/upgrading_from_source.md @@ -131,7 +131,7 @@ Remember to set `git -> bin_path` to `/usr/local/bin/git` in `config/gitlab.yml` ### 7. Update PostgreSQL WARNING: -GitLab 14.0 requires at least PostgreSQL 12. +GitLab 16.0 requires at least PostgreSQL 13. The latest version of GitLab might depend on a more recent PostgreSQL version than what you are running. You may also have to enable some diff --git a/doc/user/analytics/value_streams_dashboard.md b/doc/user/analytics/value_streams_dashboard.md index 771a7334c4e..384f4ce3455 100644 --- a/doc/user/analytics/value_streams_dashboard.md +++ b/doc/user/analytics/value_streams_dashboard.md @@ -107,10 +107,10 @@ title: 'Custom Dashboard title' # description - Change the description of the Value Streams Dashboard. [optional] description: 'Custom description' -# widgets - List of widgets that contain panel settings. +# panels - List of panels that contain panel settings. # title - Change the title of the panel. [optional] # data.namespace - The Group or Project path to use for the chart panel. -widgets: +panels: - title: 'My Custom Project' data: namespace: group/my-custom-project @@ -123,10 +123,10 @@ widgets: namespace: group/another-group ``` - The following example has an option configuration for a widget for the `my-group` namespace: + The following example has an option configuration for a panel for the `my-group` namespace: ```yaml - widgets: + panels: - data: namespace: my-group ``` diff --git a/doc/user/award_emojis.md b/doc/user/award_emojis.md index 919b76e930f..29ca6fe8d98 100644 --- a/doc/user/award_emojis.md +++ b/doc/user/award_emojis.md @@ -6,9 +6,18 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Award emojis **(FREE)** +> Awarding emojis to work items (such as tasks, objectives, and key results) [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/393599) in GitLab 16.0. + When you're collaborating online, you get fewer opportunities for high-fives -and thumbs-ups. Emojis can be awarded to [issues](project/issues/index.md), [merge requests](project/merge_requests/index.md), -[snippets](snippets.md), and anywhere you can have a thread. +and thumbs-ups. Add award emojis to: + +- [Issues](project/issues/index.md). +- [Tasks](tasks.md). +- [Merge requests](project/merge_requests/index.md), +[snippets](snippets.md). +- [Epics](../user/group/epics/index.md). +- [Objectives and key results](okrs.md). +- Anywhere else you can have a comment thread.  diff --git a/doc/user/group/access_and_permissions.md b/doc/user/group/access_and_permissions.md index 5092779b63d..50506d005b0 100644 --- a/doc/user/group/access_and_permissions.md +++ b/doc/user/group/access_and_permissions.md @@ -37,12 +37,8 @@ The group's new subgroups have push rules set for them based on either: ## Restrict Git access protocols -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/365601) in GitLab 15.1 [with a flag](../../administration/feature_flags.md) named `group_level_git_protocol_control`. Disabled by default. - -FLAG: -On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to -[enable the feature flag](../../administration/feature_flags.md) named `group_level_git_protocol_control`. On GitLab.com, -this feature is available. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/365601) in GitLab 15.1. +> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/365357) in GitLab 16.0. You can set the permitted protocols used to access a group's repositories to either SSH, HTTPS, or both. This setting is disabled when the [instance setting](../admin_area/settings/visibility_and_access_controls.md#configure-enabled-git-access-protocols) is diff --git a/doc/user/img/todos_add_okrs_v16_0.png b/doc/user/img/todos_add_okrs_v16_0.png Binary files differnew file mode 100644 index 00000000000..45c04e647e2 --- /dev/null +++ b/doc/user/img/todos_add_okrs_v16_0.png diff --git a/doc/user/img/todos_mark_done_okrs_v16_0.png b/doc/user/img/todos_mark_done_okrs_v16_0.png Binary files differnew file mode 100644 index 00000000000..545b3569ed5 --- /dev/null +++ b/doc/user/img/todos_mark_done_okrs_v16_0.png diff --git a/doc/user/project/git_attributes.md b/doc/user/project/git_attributes.md index 034fc1dc079..698b888a32a 100644 --- a/doc/user/project/git_attributes.md +++ b/doc/user/project/git_attributes.md @@ -5,7 +5,7 @@ info: "To determine the technical writer assigned to the Stage/Group associated type: reference --- -# Git Attributes **(FREE)** +# Git attributes **(FREE)** GitLab supports defining custom [Git attributes](https://git-scm.com/docs/gitattributes) such as what files to treat as binary, and what language to use for syntax highlighting @@ -14,13 +14,13 @@ diffs. To define these attributes, create a file called `.gitattributes` in the root directory of your repository and push it to the default branch of your project. -## Encoding Requirements +## Encoding requirements The `.gitattributes` file _must_ be encoded in UTF-8 and _must not_ contain a Byte Order Mark. If a different encoding is used, the file's contents are ignored. -## Support for Mixed File Encodings +## Support for mixed file encodings GitLab attempts to detect the encoding of files automatically, but defaults to UTF-8 unless the detector is confident of a different type (such as `ISO-8859-1`). Incorrect encoding @@ -47,8 +47,8 @@ be performed for the whole repository by running `git add --renormalize .`. For more information, see [working-tree-encoding](https://git-scm.com/docs/gitattributes#_working_tree_encoding). -## Syntax Highlighting +## Syntax highlighting The `.gitattributes` file can be used to define which language to use when -syntax highlighting files and diffs. See -["Syntax Highlighting"](highlighting.md) for more information. +syntax highlighting files and diffs. For more information, see +[Syntax highlighting](highlighting.md). diff --git a/doc/user/project/merge_requests/approvals/index.md b/doc/user/project/merge_requests/approvals/index.md index 6b50aaf1128..717358df9f3 100644 --- a/doc/user/project/merge_requests/approvals/index.md +++ b/doc/user/project/merge_requests/approvals/index.md @@ -109,12 +109,11 @@ Without the approvals, the work cannot merge. Required approvals enable multiple > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/334698) in GitLab 15.1. > - [Changed](https://gitlab.com/gitlab-org/gitlab/-/issues/389905) in GitLab 15.11 [with a flag](../../../../administration/feature_flags.md) named `invalid_scan_result_policy_prevents_merge`. Disabled by default. -> - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/405023) in GitLab 16.0. FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, -ask an administrator to [disable the feature flag](../../../../administration/feature_flags.md) named `invalid_scan_result_policy_prevents_merge`. -On GitLab.com, this feature is enabled by default. +On self-managed GitLab, by default this feature is not available. To make it available per project or for your entire instance, +ask an administrator to [enable the feature flag](../../../../administration/feature_flags.md) named `invalid_scan_result_policy_prevents_merge`. +On GitLab.com, this feature is available but can be configured by GitLab.com administrators only. Whenever an approval rule cannot be satisfied, the rule is displayed as **(!) Auto approved**. This applies to the following conditions: diff --git a/doc/user/todos.md b/doc/user/todos.md index 29fe3336ea2..b05e968dd11 100644 --- a/doc/user/todos.md +++ b/doc/user/todos.md @@ -81,6 +81,8 @@ When you enable this feature: ## Create a to-do item +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/390549) in objectives, key results and, tasks in GitLab 16.0. + You can manually add an item to your To-Do List. 1. Go to your: @@ -90,11 +92,15 @@ You can manually add an item to your To-Do List. - [Epic](group/epics/index.md) - [Design](project/issues/design_management.md) - [Incident](../operations/incident_management/incidents.md) + - [Objective or key result](../user/okrs.md) + - [Task](tasks.md) -1. On the right sidebar, at the top, select **Add a to do**. +1. In the upper-right corner, select **Add a to do** (**{todo-add}**).  +  + ## Create a to-do item by mentioning someone You can create a to-do item by mentioning someone anywhere except for a code block. Mentioning a user many times in one message only creates one to-do item. @@ -150,10 +156,12 @@ You can manually mark a to-do item as done. There are two ways to do this: - In the To-Do List, to the right of the to-do item, select **Mark as done** (**{check}**). -- In the sidebar of an issue, merge request, or epic, select **Mark as done**. +- In the upper-right corner of the resource (for example, issue or merge request), select **Mark as done** (**{todo-done}**).  +  + ## Mark all to-do items as done You can mark all your to-do items as done at the same time. diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index d8f4b228797..0247ce301e2 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -11,7 +11,7 @@ module API desc 'Register a new runner' do detail "Register a new runner for the instance" success Entities::Ci::RunnerRegistrationDetails - failure [[400, 'Bad Request'], [403, 'Forbidden']] + failure [[400, 'Bad Request'], [403, 'Forbidden'], [410, 'Gone']] end params do requires :token, type: String, desc: 'Registration token' @@ -52,9 +52,17 @@ module API attributes[:active] = !attributes.delete(:paused) if attributes.include?(:paused) result = ::Ci::Runners::RegisterRunnerService.new(params[:token], attributes).execute - @runner = result.success? ? result.payload[:runner] : nil - forbidden!(result.message) unless @runner + if result.error? + case result.reason + when :runner_registration_disallowed + render_api_error_with_reason!(410, '410 Gone', result.message) + else + forbidden!(result.message) + end + end + + @runner = result.payload[:runner] if @runner.persisted? present @runner, with: Entities::Ci::RunnerRegistrationDetails else diff --git a/lib/feature/logger.rb b/lib/feature/logger.rb index 95e160273b6..337d3a4ccc2 100644 --- a/lib/feature/logger.rb +++ b/lib/feature/logger.rb @@ -2,6 +2,8 @@ module Feature class Logger < ::Gitlab::JsonLogger + exclude_context! + def self.file_name_noext 'features_json' end diff --git a/lib/gitlab/background_migration/logger.rb b/lib/gitlab/background_migration/logger.rb index 4ea89771eff..d338c214140 100644 --- a/lib/gitlab/background_migration/logger.rb +++ b/lib/gitlab/background_migration/logger.rb @@ -4,6 +4,8 @@ module Gitlab module BackgroundMigration # Logger that can be used for migrations logging class Logger < ::Gitlab::JsonLogger + exclude_context! + def self.file_name_noext 'migrations' end diff --git a/lib/gitlab/backup_logger.rb b/lib/gitlab/backup_logger.rb index fad36b860ae..ec85c55d4a4 100644 --- a/lib/gitlab/backup_logger.rb +++ b/lib/gitlab/backup_logger.rb @@ -2,6 +2,8 @@ module Gitlab class BackupLogger < Gitlab::JsonLogger + exclude_context! + def self.file_name_noext 'backup_json' end diff --git a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb index 4d609de10f6..92d9d170575 100644 --- a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb +++ b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb @@ -131,11 +131,6 @@ module Gitlab end def report_uses_deprecated_schema_version? - # Avoid deprecation warnings for GitLab security scanners - # To be removed via https://gitlab.com/gitlab-org/gitlab/-/issues/386798 - return if report_data.dig('scan', 'scanner', 'vendor', 'name')&.downcase == 'gitlab' - return if report_data.dig('scan', 'analyzer', 'vendor', 'name')&.downcase == 'gitlab' - DEPRECATED_VERSIONS[report_type].include?(report_version) end diff --git a/lib/gitlab/ci/templates/Terraform/Base.gitlab-ci.yml b/lib/gitlab/ci/templates/Terraform/Base.gitlab-ci.yml index cfade84a533..f16c28e7b60 100644 --- a/lib/gitlab/ci/templates/Terraform/Base.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Terraform/Base.gitlab-ci.yml @@ -52,6 +52,7 @@ cache: - gitlab-terraform apply resource_group: ${TF_STATE_NAME} rules: + - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH && $TF_AUTO_DEPLOY == "true" - if: $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH when: manual diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 57caf1b07e6..093667dc624 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -18,7 +18,7 @@ module Gitlab # Minimum PostgreSQL version requirement per documentation: # https://docs.gitlab.com/ee/install/requirements.html#postgresql-requirements - MINIMUM_POSTGRES_VERSION = 12 + MINIMUM_POSTGRES_VERSION = 13 # https://www.postgresql.org/docs/9.2/static/datatype-numeric.html MAX_INT_VALUE = 2147483647 diff --git a/lib/gitlab/database/load_balancing/logger.rb b/lib/gitlab/database/load_balancing/logger.rb index ee67ffcc99c..df6ae47bb84 100644 --- a/lib/gitlab/database/load_balancing/logger.rb +++ b/lib/gitlab/database/load_balancing/logger.rb @@ -4,6 +4,8 @@ module Gitlab module Database module LoadBalancing class Logger < ::Gitlab::JsonLogger + exclude_context! + def self.file_name_noext 'database_load_balancing' end diff --git a/lib/gitlab/database/obsolete_ignored_columns.rb b/lib/gitlab/database/obsolete_ignored_columns.rb index 2b88ab12380..c172d15301a 100644 --- a/lib/gitlab/database/obsolete_ignored_columns.rb +++ b/lib/gitlab/database/obsolete_ignored_columns.rb @@ -2,15 +2,15 @@ module Gitlab module Database - # Checks which `ignored_columns` can be safely removed by scanning - # the current schema for all `ApplicationRecord` descendants. + # Checks which `ignored_columns` definitions can be safely removed by + # scanning the current schema for all `ApplicationRecord` descendants. class ObsoleteIgnoredColumns def initialize(base = ApplicationRecord) @base = base end def execute - @base.descendants.map do |klass| + @base.descendants.filter_map do |klass| next if klass.abstract_class? safe_to_remove = ignored_columns_safe_to_remove_for(klass) diff --git a/lib/gitlab/deprecation_json_logger.rb b/lib/gitlab/deprecation_json_logger.rb index 9796b24868b..5b0900a86dd 100644 --- a/lib/gitlab/deprecation_json_logger.rb +++ b/lib/gitlab/deprecation_json_logger.rb @@ -2,6 +2,8 @@ module Gitlab class DeprecationJsonLogger < Gitlab::JsonLogger + exclude_context! + def self.file_name_noext 'deprecation_json' end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index a907af6891b..77d2ba315a8 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -24,8 +24,6 @@ module Gitlab end end - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION' MAXIMUM_GITALY_CALLS = 30 CLIENT_NAME = (Gitlab::Runtime.sidekiq? ? 'gitlab-sidekiq' : 'gitlab-web').freeze @@ -188,15 +186,15 @@ module Gitlab end def self.query_time - query_time = InstrumentationStorage[:gitaly_query_time] || 0 + query_time = Gitlab::SafeRequestStore[:gitaly_query_time] || 0 query_time.round(Gitlab::InstrumentationHelper::DURATION_PRECISION) end def self.add_query_time(duration) - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? - InstrumentationStorage[:gitaly_query_time] ||= 0 - InstrumentationStorage[:gitaly_query_time] += duration + Gitlab::SafeRequestStore[:gitaly_query_time] ||= 0 + Gitlab::SafeRequestStore[:gitaly_query_time] += duration end # For some time related tasks we can't rely on `Time.now` since it will be @@ -255,8 +253,9 @@ module Gitlab # forced to route all requests to the primary node which has injected the # quarantine object directory to us. def self.route_to_primary - return {} unless InstrumentationStorage.active? - return {} if InstrumentationStorage[:gitlab_git_env].blank? + return {} unless Gitlab::SafeRequestStore.active? + + return {} if Gitlab::SafeRequestStore[:gitlab_git_env].blank? { 'gitaly-route-repository-accessor-policy' => 'primary-only' } end @@ -279,7 +278,7 @@ module Gitlab private_class_method :request_deadline def self.session_id - InstrumentationStorage[:gitaly_session_id] ||= SecureRandom.uuid + Gitlab::SafeRequestStore[:gitaly_session_id] ||= SecureRandom.uuid end def self.token(storage) @@ -291,8 +290,8 @@ module Gitlab # Ensures that Gitaly is not being abuse through n+1 misuse etc def self.enforce_gitaly_request_limits(call_site) - # Only count limits in live environments - return unless InstrumentationStorage.active? + # Only count limits in request-response environments + return unless Gitlab::SafeRequestStore.active? # This is this actual number of times this call was made. Used for information purposes only actual_call_count = increment_call_count("gitaly_#{call_site}_actual") @@ -330,7 +329,7 @@ module Gitlab private_class_method :enforce_gitaly_request_limits? def self.allow_n_plus_1_calls - return yield unless InstrumentationStorage.active? + return yield unless Gitlab::SafeRequestStore.active? begin increment_call_count(:gitaly_call_count_exception_block_depth) @@ -345,34 +344,34 @@ module Gitlab # afterwards. However, for read-only requests that never mutate the # branch, this method allows caching of the ref name directly. def self.allow_ref_name_caching - return yield unless InstrumentationStorage.active? + return yield unless Gitlab::SafeRequestStore.active? return yield if ref_name_caching_allowed? begin - InstrumentationStorage[:allow_ref_name_caching] = true + Gitlab::SafeRequestStore[:allow_ref_name_caching] = true yield ensure - InstrumentationStorage[:allow_ref_name_caching] = false + Gitlab::SafeRequestStore[:allow_ref_name_caching] = false end end def self.ref_name_caching_allowed? - InstrumentationStorage[:allow_ref_name_caching] + Gitlab::SafeRequestStore[:allow_ref_name_caching] end def self.get_call_count(key) - InstrumentationStorage[key] || 0 + Gitlab::SafeRequestStore[key] || 0 end private_class_method :get_call_count def self.increment_call_count(key) - InstrumentationStorage[key] ||= 0 - InstrumentationStorage[key] += 1 + Gitlab::SafeRequestStore[key] ||= 0 + Gitlab::SafeRequestStore[key] += 1 end private_class_method :increment_call_count def self.decrement_call_count(key) - InstrumentationStorage[key] -= 1 + Gitlab::SafeRequestStore[key] -= 1 end private_class_method :decrement_call_count @@ -382,21 +381,21 @@ module Gitlab end def self.reset_counts - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? - InstrumentationStorage["gitaly_call_actual"] = 0 - InstrumentationStorage["gitaly_call_permitted"] = 0 + Gitlab::SafeRequestStore["gitaly_call_actual"] = 0 + Gitlab::SafeRequestStore["gitaly_call_permitted"] = 0 end def self.add_call_details(details) - InstrumentationStorage['gitaly_call_details'] ||= [] - InstrumentationStorage['gitaly_call_details'] << details + Gitlab::SafeRequestStore['gitaly_call_details'] ||= [] + Gitlab::SafeRequestStore['gitaly_call_details'] << details end def self.list_call_details return [] unless Gitlab::PerformanceBar.enabled_for_request? - InstrumentationStorage['gitaly_call_details'] || [] + Gitlab::SafeRequestStore['gitaly_call_details'] || [] end def self.expected_server_version @@ -481,22 +480,22 @@ module Gitlab # Count a stack. Used for n+1 detection def self.count_stack - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? stack_string = Gitlab::BacktraceCleaner.clean_backtrace(caller).drop(1).join("\n") - InstrumentationStorage[:stack_counter] ||= {} + Gitlab::SafeRequestStore[:stack_counter] ||= {} - count = InstrumentationStorage[:stack_counter][stack_string] || 0 - InstrumentationStorage[:stack_counter][stack_string] = count + 1 + count = Gitlab::SafeRequestStore[:stack_counter][stack_string] || 0 + Gitlab::SafeRequestStore[:stack_counter][stack_string] = count + 1 end private_class_method :count_stack # Returns a count for the stack which called Gitaly the most times. Used for n+1 detection def self.max_call_count - return 0 unless InstrumentationStorage.active? + return 0 unless Gitlab::SafeRequestStore.active? - stack_counter = InstrumentationStorage[:stack_counter] + stack_counter = Gitlab::SafeRequestStore[:stack_counter] return 0 unless stack_counter stack_counter.values.max @@ -505,9 +504,9 @@ module Gitlab # Returns the stacks that calls Gitaly the most times. Used for n+1 detection def self.max_stacks - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? - stack_counter = InstrumentationStorage[:stack_counter] + stack_counter = Gitlab::SafeRequestStore[:stack_counter] return unless stack_counter max = max_call_count @@ -545,8 +544,8 @@ module Gitlab end def self.feature_flag_actors - if InstrumentationStorage.active? - InstrumentationStorage[:gitaly_feature_flag_actors] ||= {} + if Gitlab::SafeRequestStore.active? + Gitlab::SafeRequestStore[:gitaly_feature_flag_actors] ||= {} else Thread.current[:gitaly_feature_flag_actors] ||= {} end diff --git a/lib/gitlab/graphql/calls_gitaly/field_extension.rb b/lib/gitlab/graphql/calls_gitaly/field_extension.rb index 014ce9fb0ee..32530b47ce3 100644 --- a/lib/gitlab/graphql/calls_gitaly/field_extension.rb +++ b/lib/gitlab/graphql/calls_gitaly/field_extension.rb @@ -67,14 +67,14 @@ module Gitlab end def accounted_for(count = nil) - return 0 unless ::Gitlab::Instrumentation::Storage.active? + return 0 unless Gitlab::SafeRequestStore.active? - ::Gitlab::Instrumentation::Storage["graphql_gitaly_accounted_for"] ||= 0 + Gitlab::SafeRequestStore["graphql_gitaly_accounted_for"] ||= 0 if count.nil? - ::Gitlab::Instrumentation::Storage["graphql_gitaly_accounted_for"] + Gitlab::SafeRequestStore["graphql_gitaly_accounted_for"] else - ::Gitlab::Instrumentation::Storage["graphql_gitaly_accounted_for"] += count + Gitlab::SafeRequestStore["graphql_gitaly_accounted_for"] += count end end diff --git a/lib/gitlab/instrumentation/elasticsearch_transport.rb b/lib/gitlab/instrumentation/elasticsearch_transport.rb index 791cf691112..4bef043ecb0 100644 --- a/lib/gitlab/instrumentation/elasticsearch_transport.rb +++ b/lib/gitlab/instrumentation/elasticsearch_transport.rb @@ -4,8 +4,6 @@ require 'elasticsearch-transport' module Gitlab module Instrumentation - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - module ElasticsearchTransportInterceptor def perform_request(method, path, params = {}, body = nil, headers = nil) start = Time.now @@ -13,7 +11,7 @@ module Gitlab .reverse_merge({ 'X-Opaque-Id': Labkit::Correlation::CorrelationId.current_or_new_id }) response = super ensure - if InstrumentationStorage.active? + if ::Gitlab::SafeRequestStore.active? duration = (Time.now - start) ::Gitlab::Instrumentation::ElasticsearchTransport.increment_request_count @@ -35,35 +33,35 @@ module Gitlab ELASTICSEARCH_TIMED_OUT_COUNT = :elasticsearch_timed_out_count def self.get_request_count - InstrumentationStorage[ELASTICSEARCH_REQUEST_COUNT] || 0 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_REQUEST_COUNT] || 0 end def self.increment_request_count - InstrumentationStorage[ELASTICSEARCH_REQUEST_COUNT] ||= 0 - InstrumentationStorage[ELASTICSEARCH_REQUEST_COUNT] += 1 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_REQUEST_COUNT] ||= 0 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_REQUEST_COUNT] += 1 end def self.detail_store - InstrumentationStorage[ELASTICSEARCH_CALL_DETAILS] ||= [] + ::Gitlab::SafeRequestStore[ELASTICSEARCH_CALL_DETAILS] ||= [] end def self.query_time - query_time = InstrumentationStorage[ELASTICSEARCH_CALL_DURATION] || 0 + query_time = ::Gitlab::SafeRequestStore[ELASTICSEARCH_CALL_DURATION] || 0 query_time.round(::Gitlab::InstrumentationHelper::DURATION_PRECISION) end def self.add_duration(duration) - InstrumentationStorage[ELASTICSEARCH_CALL_DURATION] ||= 0 - InstrumentationStorage[ELASTICSEARCH_CALL_DURATION] += duration + ::Gitlab::SafeRequestStore[ELASTICSEARCH_CALL_DURATION] ||= 0 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_CALL_DURATION] += duration end def self.increment_timed_out_count - InstrumentationStorage[ELASTICSEARCH_TIMED_OUT_COUNT] ||= 0 - InstrumentationStorage[ELASTICSEARCH_TIMED_OUT_COUNT] += 1 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_TIMED_OUT_COUNT] ||= 0 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_TIMED_OUT_COUNT] += 1 end def self.get_timed_out_count - InstrumentationStorage[ELASTICSEARCH_TIMED_OUT_COUNT] || 0 + ::Gitlab::SafeRequestStore[ELASTICSEARCH_TIMED_OUT_COUNT] || 0 end def self.add_call_details(duration, method, path, params, body) diff --git a/lib/gitlab/instrumentation/global_search_api.rb b/lib/gitlab/instrumentation/global_search_api.rb index d475a58c36c..ea2f5702364 100644 --- a/lib/gitlab/instrumentation/global_search_api.rb +++ b/lib/gitlab/instrumentation/global_search_api.rb @@ -8,22 +8,20 @@ module Gitlab SCOPE = 'meta.search.scope' SEARCH_DURATION_S = :global_search_duration_s - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - def self.get_type - InstrumentationStorage[TYPE] + ::Gitlab::SafeRequestStore[TYPE] end def self.get_level - InstrumentationStorage[LEVEL] + ::Gitlab::SafeRequestStore[LEVEL] end def self.get_scope - InstrumentationStorage[SCOPE] + ::Gitlab::SafeRequestStore[SCOPE] end def self.get_search_duration_s - InstrumentationStorage[SEARCH_DURATION_S] + ::Gitlab::SafeRequestStore[SEARCH_DURATION_S] end def self.payload @@ -36,11 +34,11 @@ module Gitlab end def self.set_information(type:, level:, scope:, search_duration_s:) - if InstrumentationStorage.active? - InstrumentationStorage[TYPE] = type - InstrumentationStorage[LEVEL] = level - InstrumentationStorage[SCOPE] = scope - InstrumentationStorage[SEARCH_DURATION_S] = search_duration_s + if ::Gitlab::SafeRequestStore.active? + ::Gitlab::SafeRequestStore[TYPE] = type + ::Gitlab::SafeRequestStore[LEVEL] = level + ::Gitlab::SafeRequestStore[SCOPE] = scope + ::Gitlab::SafeRequestStore[SEARCH_DURATION_S] = search_duration_s end end end diff --git a/lib/gitlab/instrumentation/rate_limiting_gates.rb b/lib/gitlab/instrumentation/rate_limiting_gates.rb index 01b362f0cf4..960b6995030 100644 --- a/lib/gitlab/instrumentation/rate_limiting_gates.rb +++ b/lib/gitlab/instrumentation/rate_limiting_gates.rb @@ -5,11 +5,9 @@ module Gitlab class RateLimitingGates GATES = :rate_limiting_gates - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - class << self def track(key) - if InstrumentationStorage.active? + if ::Gitlab::SafeRequestStore.active? gates_set << key end end @@ -27,7 +25,7 @@ module Gitlab private def gates_set - InstrumentationStorage[GATES] ||= Set.new + ::Gitlab::SafeRequestStore[GATES] ||= Set.new end end end diff --git a/lib/gitlab/instrumentation/redis_base.rb b/lib/gitlab/instrumentation/redis_base.rb index 39290c109d7..00a7387afe2 100644 --- a/lib/gitlab/instrumentation/redis_base.rb +++ b/lib/gitlab/instrumentation/redis_base.rb @@ -9,8 +9,6 @@ module Gitlab include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Instrumentation::RedisPayload - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - # TODO: To be used by https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/395 # as a 'label' alias. def storage_key @@ -18,10 +16,8 @@ module Gitlab end def add_duration(duration) - return unless InstrumentationStorage.active? - - InstrumentationStorage[call_duration_key] ||= 0 - InstrumentationStorage[call_duration_key] += duration + ::RequestStore[call_duration_key] ||= 0 + ::RequestStore[call_duration_key] += duration end def add_call_details(duration, commands) @@ -35,66 +31,56 @@ module Gitlab end def increment_request_count(amount = 1) - return unless InstrumentationStorage.active? - - InstrumentationStorage[request_count_key] ||= 0 - InstrumentationStorage[request_count_key] += amount + ::RequestStore[request_count_key] ||= 0 + ::RequestStore[request_count_key] += amount end def increment_read_bytes(num_bytes) - return unless InstrumentationStorage.active? - - InstrumentationStorage[read_bytes_key] ||= 0 - InstrumentationStorage[read_bytes_key] += num_bytes + ::RequestStore[read_bytes_key] ||= 0 + ::RequestStore[read_bytes_key] += num_bytes end def increment_write_bytes(num_bytes) - return unless InstrumentationStorage.active? - - InstrumentationStorage[write_bytes_key] ||= 0 - InstrumentationStorage[write_bytes_key] += num_bytes + ::RequestStore[write_bytes_key] ||= 0 + ::RequestStore[write_bytes_key] += num_bytes end def increment_cross_slot_request_count(amount = 1) - return unless InstrumentationStorage.active? - - InstrumentationStorage[cross_slots_key] ||= 0 - InstrumentationStorage[cross_slots_key] += amount + ::RequestStore[cross_slots_key] ||= 0 + ::RequestStore[cross_slots_key] += amount end def increment_allowed_cross_slot_request_count(amount = 1) - return unless InstrumentationStorage.active? - - InstrumentationStorage[allowed_cross_slots_key] ||= 0 - InstrumentationStorage[allowed_cross_slots_key] += amount + ::RequestStore[allowed_cross_slots_key] ||= 0 + ::RequestStore[allowed_cross_slots_key] += amount end def get_request_count - InstrumentationStorage[request_count_key] || 0 + ::RequestStore[request_count_key] || 0 end def read_bytes - InstrumentationStorage[read_bytes_key] || 0 + ::RequestStore[read_bytes_key] || 0 end def write_bytes - InstrumentationStorage[write_bytes_key] || 0 + ::RequestStore[write_bytes_key] || 0 end def detail_store - InstrumentationStorage[call_details_key] ||= [] + ::RequestStore[call_details_key] ||= [] end def get_cross_slot_request_count - InstrumentationStorage[cross_slots_key] || 0 + ::RequestStore[cross_slots_key] || 0 end def get_allowed_cross_slot_request_count - InstrumentationStorage[allowed_cross_slots_key] || 0 + ::RequestStore[allowed_cross_slots_key] || 0 end def query_time - query_time = InstrumentationStorage[call_duration_key] || 0 + query_time = ::RequestStore[call_duration_key] || 0 query_time.round(::Gitlab::InstrumentationHelper::DURATION_PRECISION) end diff --git a/lib/gitlab/instrumentation/redis_interceptor.rb b/lib/gitlab/instrumentation/redis_interceptor.rb index c81f070219e..b3fbe30e583 100644 --- a/lib/gitlab/instrumentation/redis_interceptor.rb +++ b/lib/gitlab/instrumentation/redis_interceptor.rb @@ -55,7 +55,7 @@ module Gitlab commands.each { instrumentation_class.instance_observe_duration(duration / commands.size) } end - if ::Gitlab::Instrumentation::Storage.active? + if ::RequestStore.active? # These metrics measure total Redis usage per Rails request / job. instrumentation_class.increment_request_count(commands.size) instrumentation_class.add_duration(duration) diff --git a/lib/gitlab/instrumentation/storage.rb b/lib/gitlab/instrumentation/storage.rb deleted file mode 100644 index 20c1b69acdd..00000000000 --- a/lib/gitlab/instrumentation/storage.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Instrumentation - module Storage - extend self - - delegate :active?, to: ::Gitlab::SafeRequestStore - delegate :[], :[]=, to: :storage - - def clear! - storage.clear - end - - private - - def storage - ::Gitlab::SafeRequestStore.fetch(:instrumentation) { {} } - end - end - end -end diff --git a/lib/gitlab/instrumentation/throttle.rb b/lib/gitlab/instrumentation/throttle.rb index 837313127e7..0b7e990fb2e 100644 --- a/lib/gitlab/instrumentation/throttle.rb +++ b/lib/gitlab/instrumentation/throttle.rb @@ -3,16 +3,14 @@ module Gitlab module Instrumentation class Throttle - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - KEY = :instrumentation_throttle_safelist def self.safelist - InstrumentationStorage[KEY] + Gitlab::SafeRequestStore[KEY] end def self.safelist=(name) - InstrumentationStorage[KEY] = name + Gitlab::SafeRequestStore[KEY] = name end end end diff --git a/lib/gitlab/instrumentation/uploads.rb b/lib/gitlab/instrumentation/uploads.rb index 226c4e73d01..02e457453cd 100644 --- a/lib/gitlab/instrumentation/uploads.rb +++ b/lib/gitlab/instrumentation/uploads.rb @@ -6,21 +6,19 @@ module Gitlab UPLOAD_DURATION = :uploaded_file_upload_duration_s UPLOADED_FILE_SIZE = :uploaded_file_size_bytes - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - def self.track(uploaded_file) - if InstrumentationStorage.active? - InstrumentationStorage[UPLOAD_DURATION] = uploaded_file.upload_duration - InstrumentationStorage[UPLOADED_FILE_SIZE] = uploaded_file.size + if ::Gitlab::SafeRequestStore.active? + ::Gitlab::SafeRequestStore[UPLOAD_DURATION] = uploaded_file.upload_duration + ::Gitlab::SafeRequestStore[UPLOADED_FILE_SIZE] = uploaded_file.size end end def self.get_upload_duration - InstrumentationStorage[UPLOAD_DURATION] + ::Gitlab::SafeRequestStore[UPLOAD_DURATION] end def self.get_uploaded_file_size - InstrumentationStorage[UPLOADED_FILE_SIZE] + ::Gitlab::SafeRequestStore[UPLOADED_FILE_SIZE] end def self.payload diff --git a/lib/gitlab/instrumentation/zoekt.rb b/lib/gitlab/instrumentation/zoekt.rb index 4d2933680c5..cd9b15bcee8 100644 --- a/lib/gitlab/instrumentation/zoekt.rb +++ b/lib/gitlab/instrumentation/zoekt.rb @@ -3,34 +3,32 @@ module Gitlab module Instrumentation class Zoekt - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - ZOEKT_REQUEST_COUNT = :zoekt_request_count ZOEKT_CALL_DURATION = :zoekt_call_duration ZOEKT_CALL_DETAILS = :zoekt_call_details class << self def get_request_count - InstrumentationStorage[ZOEKT_REQUEST_COUNT] || 0 + ::Gitlab::SafeRequestStore[ZOEKT_REQUEST_COUNT] || 0 end def increment_request_count - InstrumentationStorage[ZOEKT_REQUEST_COUNT] ||= 0 - InstrumentationStorage[ZOEKT_REQUEST_COUNT] += 1 + ::Gitlab::SafeRequestStore[ZOEKT_REQUEST_COUNT] ||= 0 + ::Gitlab::SafeRequestStore[ZOEKT_REQUEST_COUNT] += 1 end def detail_store - InstrumentationStorage[ZOEKT_CALL_DETAILS] ||= [] + ::Gitlab::SafeRequestStore[ZOEKT_CALL_DETAILS] ||= [] end def query_time - query_time = InstrumentationStorage[ZOEKT_CALL_DURATION] || 0 + query_time = ::Gitlab::SafeRequestStore[ZOEKT_CALL_DURATION] || 0 query_time.round(::Gitlab::InstrumentationHelper::DURATION_PRECISION) end def add_duration(duration) - InstrumentationStorage[ZOEKT_CALL_DURATION] ||= 0 - InstrumentationStorage[ZOEKT_CALL_DURATION] += duration + ::Gitlab::SafeRequestStore[ZOEKT_CALL_DURATION] ||= 0 + ::Gitlab::SafeRequestStore[ZOEKT_CALL_DURATION] += duration end def add_call_details(duration:, method:, path:, params: nil, body: nil) diff --git a/lib/gitlab/instrumentation_helper.rb b/lib/gitlab/instrumentation_helper.rb index 686142e338a..2a3c4db5ffa 100644 --- a/lib/gitlab/instrumentation_helper.rb +++ b/lib/gitlab/instrumentation_helper.rb @@ -6,18 +6,8 @@ module Gitlab DURATION_PRECISION = 6 # microseconds - def init_instrumentation_data(request_ip: nil) - ::Gitlab::Instrumentation::Storage.clear! - - # Set `request_start_time` only if this is request - # This is done, as `request_start_time` imply `request_deadline` - if request_ip - Gitlab::RequestContext.instance.client_ip = request_ip - Gitlab::RequestContext.instance.request_start_time = Gitlab::Metrics::System.real_time - end - - Gitlab::RequestContext.instance.start_thread_cpu_time = Gitlab::Metrics::System.thread_cpu_time - Gitlab::RequestContext.instance.thread_memory_allocations = Gitlab::Memory::Instrumentation.start_thread_memory_allocations + def init_instrumentation_data + Gitlab::RequestContext.start_thread_context end def add_instrumentation_data(payload) diff --git a/lib/gitlab/metrics/subscribers/active_record.rb b/lib/gitlab/metrics/subscribers/active_record.rb index dd99d4d770c..10bb358a292 100644 --- a/lib/gitlab/metrics/subscribers/active_record.rb +++ b/lib/gitlab/metrics/subscribers/active_record.rb @@ -21,8 +21,6 @@ module Gitlab SQL_WAL_LOCATION_REGEX = /(pg_current_wal_insert_lsn\(\)::text|pg_last_wal_replay_lsn\(\)::text)/.freeze - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - # This event is published from ActiveRecordBaseTransactionMetrics and # used to record a database transaction duration when calling # ApplicationRecord.transaction {} block. @@ -58,20 +56,20 @@ module Gitlab end def self.db_counter_payload - return {} unless InstrumentationStorage.active? + return {} unless Gitlab::SafeRequestStore.active? {}.tap do |payload| db_counter_keys.each do |key| - payload[key] = InstrumentationStorage[key].to_i + payload[key] = Gitlab::SafeRequestStore[key].to_i end - if InstrumentationStorage.active? + if ::Gitlab::SafeRequestStore.active? load_balancing_metric_counter_keys.each do |counter| - payload[counter] = InstrumentationStorage[counter].to_i + payload[counter] = ::Gitlab::SafeRequestStore[counter].to_i end load_balancing_metric_duration_keys.each do |duration| - payload[duration] = InstrumentationStorage[duration].to_f.round(3) + payload[duration] = ::Gitlab::SafeRequestStore[duration].to_f.round(3) end end end @@ -102,16 +100,16 @@ module Gitlab buckets ::Gitlab::Metrics::Subscribers::ActiveRecord::SQL_DURATION_BUCKET end - return unless InstrumentationStorage.active? + return unless ::Gitlab::SafeRequestStore.active? duration = event.duration / 1000.0 duration_key = compose_metric_key(:duration_s, db_role) - InstrumentationStorage[duration_key] = (InstrumentationStorage[duration_key].presence || 0) + duration + ::Gitlab::SafeRequestStore[duration_key] = (::Gitlab::SafeRequestStore[duration_key].presence || 0) + duration # Per database metrics db_config_name = db_config_name(event.payload) duration_key = compose_metric_key(:duration_s, nil, db_config_name) - InstrumentationStorage[duration_key] = (InstrumentationStorage[duration_key].presence || 0) + duration + ::Gitlab::SafeRequestStore[duration_key] = (::Gitlab::SafeRequestStore[duration_key].presence || 0) + duration end def ignored_query?(payload) @@ -137,14 +135,14 @@ module Gitlab current_transaction&.increment(prometheus_key, 1, { db_config_name: db_config_name }) - InstrumentationStorage[log_key] = InstrumentationStorage[log_key].to_i + 1 + Gitlab::SafeRequestStore[log_key] = Gitlab::SafeRequestStore[log_key].to_i + 1 # To avoid confusing log keys we only log the db_config_name metrics # when we are also logging the db_role. Otherwise it will be hard to # tell if the log key is referring to a db_role OR a db_config_name. if db_role.present? && db_config_name.present? log_key = compose_metric_key(counter, nil, db_config_name) - InstrumentationStorage[log_key] = InstrumentationStorage[log_key].to_i + 1 + Gitlab::SafeRequestStore[log_key] = Gitlab::SafeRequestStore[log_key].to_i + 1 end end diff --git a/lib/gitlab/metrics/subscribers/external_http.rb b/lib/gitlab/metrics/subscribers/external_http.rb index a5bfc80b3bf..87756b14887 100644 --- a/lib/gitlab/metrics/subscribers/external_http.rb +++ b/lib/gitlab/metrics/subscribers/external_http.rb @@ -6,8 +6,6 @@ module Gitlab # Class for tracking the total time spent in external HTTP # See more at https://gitlab.com/gitlab-org/labkit-ruby/-/blob/v0.14.0/lib/gitlab-labkit.rb#L18 class ExternalHttp < ActiveSupport::Subscriber - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - attach_to :external_http DEFAULT_STATUS_CODE = 'undefined' @@ -21,19 +19,19 @@ module Gitlab MAX_SLOW_REQUESTS = 10 def self.detail_store - InstrumentationStorage[DETAIL_STORE] ||= [] + ::Gitlab::SafeRequestStore[DETAIL_STORE] ||= [] end def self.duration - InstrumentationStorage[DURATION].to_f + Gitlab::SafeRequestStore[DURATION].to_f end def self.request_count - InstrumentationStorage[COUNTER].to_i + Gitlab::SafeRequestStore[COUNTER].to_i end def self.slow_requests - InstrumentationStorage[SLOW_REQUESTS] + Gitlab::SafeRequestStore[SLOW_REQUESTS] end def self.top_slowest_requests @@ -84,14 +82,14 @@ module Gitlab end def add_to_request_store(payload) - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? - InstrumentationStorage[COUNTER] = InstrumentationStorage[COUNTER].to_i + 1 - InstrumentationStorage[DURATION] = InstrumentationStorage[DURATION].to_f + payload[:duration].to_f + Gitlab::SafeRequestStore[COUNTER] = Gitlab::SafeRequestStore[COUNTER].to_i + 1 + Gitlab::SafeRequestStore[DURATION] = Gitlab::SafeRequestStore[DURATION].to_f + payload[:duration].to_f if payload[:duration].to_f > THRESHOLD_SLOW_REQUEST_S - InstrumentationStorage[SLOW_REQUESTS] ||= [] - InstrumentationStorage[SLOW_REQUESTS] << { + Gitlab::SafeRequestStore[SLOW_REQUESTS] ||= [] + Gitlab::SafeRequestStore[SLOW_REQUESTS] << { method: payload[:method], host: payload[:host], port: payload[:port], diff --git a/lib/gitlab/metrics/subscribers/ldap.rb b/lib/gitlab/metrics/subscribers/ldap.rb index 409ecbb6e8a..3dae2d1fd88 100644 --- a/lib/gitlab/metrics/subscribers/ldap.rb +++ b/lib/gitlab/metrics/subscribers/ldap.rb @@ -8,8 +8,6 @@ module Gitlab # at the end of the event key, e.g. `open.net_ldap` attach_to :net_ldap - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - COUNTER = :net_ldap_count DURATION = :net_ldap_duration_s @@ -28,12 +26,12 @@ module Gitlab class << self # @return [Integer] the total number of LDAP requests def count - InstrumentationStorage[COUNTER].to_i + Gitlab::SafeRequestStore[COUNTER].to_i end # @return [Float] the total duration spent on LDAP requests def duration - InstrumentationStorage[DURATION].to_f + Gitlab::SafeRequestStore[DURATION].to_f end # Used in Gitlab::InstrumentationHelper to merge the LDAP stats @@ -73,10 +71,10 @@ module Gitlab # Track these events as statistics for the current requests, for logging purposes def add_to_request_store(event) - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? - InstrumentationStorage[COUNTER] = self.class.count + 1 - InstrumentationStorage[DURATION] = self.class.duration + convert_to_seconds(event.duration) + Gitlab::SafeRequestStore[COUNTER] = self.class.count + 1 + Gitlab::SafeRequestStore[DURATION] = self.class.duration + convert_to_seconds(event.duration) end # Converts the observed events into Prometheus metrics diff --git a/lib/gitlab/metrics/subscribers/load_balancing.rb b/lib/gitlab/metrics/subscribers/load_balancing.rb index d7fe33dbe89..bd77e8c3c3f 100644 --- a/lib/gitlab/metrics/subscribers/load_balancing.rb +++ b/lib/gitlab/metrics/subscribers/load_balancing.rb @@ -6,13 +6,11 @@ module Gitlab class LoadBalancing < ActiveSupport::Subscriber attach_to :load_balancing - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - PROMETHEUS_COUNTER = :gitlab_transaction_caught_up_replica_pick_count_total LOG_COUNTERS = { true => :caught_up_replica_pick_ok, false => :caught_up_replica_pick_fail }.freeze def caught_up_replica_pick(event) - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? result = event.payload[:result] counter_name = counter(result) @@ -22,17 +20,17 @@ module Gitlab # we want to update Prometheus counter after the controller/action are set def web_transaction_completed(_event) - return unless InstrumentationStorage.active? + return unless Gitlab::SafeRequestStore.active? LOG_COUNTERS.keys.each { |result| increment_prometheus_for_result_label(result) } end def self.load_balancing_payload - return {} unless InstrumentationStorage.active? + return {} unless Gitlab::SafeRequestStore.active? {}.tap do |payload| LOG_COUNTERS.values.each do |counter| - value = InstrumentationStorage[counter] + value = Gitlab::SafeRequestStore[counter] payload[counter] = value.to_i if value end @@ -42,12 +40,12 @@ module Gitlab private def increment(counter) - InstrumentationStorage[counter] = InstrumentationStorage[counter].to_i + 1 + Gitlab::SafeRequestStore[counter] = Gitlab::SafeRequestStore[counter].to_i + 1 end def increment_prometheus_for_result_label(label_value) counter_name = counter(label_value) - return unless (counter_value = InstrumentationStorage[counter_name]) + return unless (counter_value = Gitlab::SafeRequestStore[counter_name]) increment_prometheus(labels: { result: label_value }, value: counter_value.to_i) end diff --git a/lib/gitlab/metrics/subscribers/rack_attack.rb b/lib/gitlab/metrics/subscribers/rack_attack.rb index 705536039ed..2196122df01 100644 --- a/lib/gitlab/metrics/subscribers/rack_attack.rb +++ b/lib/gitlab/metrics/subscribers/rack_attack.rb @@ -13,12 +13,10 @@ module Gitlab class RackAttack < ActiveSupport::Subscriber attach_to 'rack_attack' - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - INSTRUMENTATION_STORE_KEY = :rack_attack_instrumentation def self.payload - InstrumentationStorage[INSTRUMENTATION_STORE_KEY] ||= { + Gitlab::SafeRequestStore[INSTRUMENTATION_STORE_KEY] ||= { rack_attack_redis_count: 0, rack_attack_redis_duration_s: 0.0 } diff --git a/lib/gitlab/middleware/request_context.rb b/lib/gitlab/middleware/request_context.rb index 07f6f87a68c..f609002007c 100644 --- a/lib/gitlab/middleware/request_context.rb +++ b/lib/gitlab/middleware/request_context.rb @@ -8,15 +8,9 @@ module Gitlab end def call(env) - # We should be using ActionDispatch::Request instead of - # Rack::Request to be consistent with Rails, but due to a Rails - # bug described in - # https://gitlab.com/gitlab-org/gitlab-foss/issues/58573#note_149799010 - # hosts behind a load balancer will only see 127.0.0.1 for the - # load balancer's IP. - req = Rack::Request.new(env) - - ::Gitlab::InstrumentationHelper.init_instrumentation_data(request_ip: req.ip) + request = ActionDispatch::Request.new(env) + Gitlab::RequestContext.start_request_context(request: request) + Gitlab::RequestContext.start_thread_context @app.call(env) end diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb index c9eefe9a647..813468ece90 100644 --- a/lib/gitlab/request_context.rb +++ b/lib/gitlab/request_context.rb @@ -7,12 +7,28 @@ module Gitlab RequestDeadlineExceeded = Class.new(StandardError) - attr_accessor :client_ip, :start_thread_cpu_time, :request_start_time, :thread_memory_allocations + attr_accessor :client_ip, :spam_params, :start_thread_cpu_time, :request_start_time, :thread_memory_allocations class << self def instance Gitlab::SafeRequestStore[:request_context] ||= new end + + def start_request_context(request:) + # We need to use Rack::Request to be consistent with Rails due to a Rails bug described in + # https://gitlab.com/gitlab-org/gitlab-foss/issues/58573#note_149799010 + # Hosts behind a load balancer will only see 127.0.0.1 for the load balancer's IP. + rack_req = Rack::Request.new(request.env) + instance.client_ip = rack_req.ip + + instance.spam_params = ::Spam::SpamParams.new_from_request(request: request) + instance.request_start_time = Gitlab::Metrics::System.real_time + end + + def start_thread_context + instance.start_thread_cpu_time = Gitlab::Metrics::System.thread_cpu_time + instance.thread_memory_allocations = Gitlab::Memory::Instrumentation.start_thread_memory_allocations + end end def request_deadline diff --git a/lib/gitlab/rugged_instrumentation.rb b/lib/gitlab/rugged_instrumentation.rb index b768e89b1e4..36a3a491de6 100644 --- a/lib/gitlab/rugged_instrumentation.rb +++ b/lib/gitlab/rugged_instrumentation.rb @@ -2,16 +2,14 @@ module Gitlab module RuggedInstrumentation - InstrumentationStorage = ::Gitlab::Instrumentation::Storage - def self.query_time - query_time = InstrumentationStorage[:rugged_query_time] || 0 + query_time = SafeRequestStore[:rugged_query_time] || 0 query_time.round(Gitlab::InstrumentationHelper::DURATION_PRECISION) end def self.add_query_time(duration) - InstrumentationStorage[:rugged_query_time] ||= 0 - InstrumentationStorage[:rugged_query_time] += duration + SafeRequestStore[:rugged_query_time] ||= 0 + SafeRequestStore[:rugged_query_time] += duration end def self.query_time_ms @@ -19,29 +17,29 @@ module Gitlab end def self.query_count - InstrumentationStorage[:rugged_call_count] ||= 0 + SafeRequestStore[:rugged_call_count] ||= 0 end def self.increment_query_count - InstrumentationStorage[:rugged_call_count] ||= 0 - InstrumentationStorage[:rugged_call_count] += 1 + SafeRequestStore[:rugged_call_count] ||= 0 + SafeRequestStore[:rugged_call_count] += 1 end def self.active? - InstrumentationStorage.active? + SafeRequestStore.active? end def self.add_call_details(details) return unless Gitlab::PerformanceBar.enabled_for_request? - InstrumentationStorage[:rugged_call_details] ||= [] - InstrumentationStorage[:rugged_call_details] << details + Gitlab::SafeRequestStore[:rugged_call_details] ||= [] + Gitlab::SafeRequestStore[:rugged_call_details] << details end def self.list_call_details return [] unless Gitlab::PerformanceBar.enabled_for_request? - InstrumentationStorage[:rugged_call_details] || [] + Gitlab::SafeRequestStore[:rugged_call_details] || [] end end end diff --git a/lib/sidebars/projects/menus/confluence_menu.rb b/lib/sidebars/projects/menus/confluence_menu.rb index 0fd42a57da3..43ef7ac73c4 100644 --- a/lib/sidebars/projects/menus/confluence_menu.rb +++ b/lib/sidebars/projects/menus/confluence_menu.rb @@ -42,6 +42,14 @@ module Sidebars def active_routes { controller: :confluences } end + + override :serialize_as_menu_item_args + def serialize_as_menu_item_args + super.merge({ + item_id: :confluence, + super_sidebar_parent: ::Sidebars::Projects::SuperSidebarMenus::PlanMenu + }) + end end end end diff --git a/lib/sidebars/projects/menus/external_issue_tracker_menu.rb b/lib/sidebars/projects/menus/external_issue_tracker_menu.rb index 136d30f38c3..f088ccce9f5 100644 --- a/lib/sidebars/projects/menus/external_issue_tracker_menu.rb +++ b/lib/sidebars/projects/menus/external_issue_tracker_menu.rb @@ -48,6 +48,14 @@ module Sidebars external_issue_tracker.present? end + override :serialize_as_menu_item_args + def serialize_as_menu_item_args + super.merge({ + item_id: :external_issue_tracker, + super_sidebar_parent: ::Sidebars::Projects::SuperSidebarMenus::PlanMenu + }) + end + private def external_issue_tracker diff --git a/lib/sidebars/projects/menus/external_wiki_menu.rb b/lib/sidebars/projects/menus/external_wiki_menu.rb index 825f0ca5e8b..1af9abc33ff 100644 --- a/lib/sidebars/projects/menus/external_wiki_menu.rb +++ b/lib/sidebars/projects/menus/external_wiki_menu.rb @@ -41,6 +41,14 @@ module Sidebars external_wiki.present? end + override :serialize_as_menu_item_args + def serialize_as_menu_item_args + super.merge({ + item_id: :external_wiki, + super_sidebar_parent: ::Sidebars::Projects::SuperSidebarMenus::PlanMenu + }) + end + private def external_wiki diff --git a/lib/tasks/db_obsolete_ignored_columns.rake b/lib/tasks/db_obsolete_ignored_columns.rake index a689a9bf2d8..c71e3169723 100644 --- a/lib/tasks/db_obsolete_ignored_columns.rake +++ b/lib/tasks/db_obsolete_ignored_columns.rake @@ -5,9 +5,9 @@ task 'db:obsolete_ignored_columns' => :environment do list = Gitlab::Database::ObsoleteIgnoredColumns.new.execute if list.empty? - puts 'No obsolete `ignored_columns` found.' + puts 'No obsolete `ignored_columns` definitions found.' else - puts 'The following `ignored_columns` are obsolete and can be removed:' + puts 'The following `ignored_columns` definitions are obsolete and can be removed:' list.each do |name, ignored_columns| puts "#{name}:" diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8f934560f07..9f097e63b8b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8167,7 +8167,7 @@ msgstr "" msgid "BuildArtifacts|Loading artifacts" msgstr "" -msgid "Building your merge request. Wait a few moments, then refresh this page." +msgid "Building your merge request… This page will update when the build is complete." msgstr "" msgid "Built-in" @@ -16869,6 +16869,9 @@ msgstr "" msgid "EnvironmentsDashboard|This dashboard displays 3 environments per project, and is linked to the Operations Dashboard. When you add or remove a project from one dashboard, GitLab adds or removes the project from the other. %{linkStart}More information%{linkEnd}" msgstr "" +msgid "Environments|A freeze period is in effect from %{startTime} to %{endTime}. Deployments might fail during this time. For more information, see the %{docsLinkStart}deploy freeze documentation%{docsLinkEnd}." +msgstr "" + msgid "Environments|An error occurred while canceling the auto stop, please try again" msgstr "" @@ -22069,9 +22072,6 @@ msgstr "" msgid "Highest role:" msgstr "" -msgid "Highlight" -msgstr "" - msgid "HighlightBar|Alert events:" msgstr "" @@ -31561,7 +31561,7 @@ msgstr "" msgid "Package registry rate limits" msgstr "" -msgid "Package registry types for which metadata is stored, required for License Compliance for CycloneDX files" +msgid "Package registry types for which metadata is stored, required for License Compliance for CycloneDX files." msgstr "" msgid "Package type" @@ -43440,9 +43440,6 @@ msgstr "" msgid "Subscribes to this %{quick_action_target}." msgstr "" -msgid "Subscript" -msgstr "" - msgid "Subscription" msgstr "" @@ -43958,9 +43955,6 @@ msgstr "" msgid "SuperSonics|past subscriptions" msgstr "" -msgid "Superscript" -msgstr "" - msgid "Support" msgstr "" @@ -44953,6 +44947,9 @@ msgstr[1] "" msgid "The form contains the following warning:" msgstr "" +msgid "The full package metadata sync can add up to 30 GB to GitLab PostgreSQL database. Ensure you have provisioned enough disk space for the database before enabling this feature. We are actively working on reducing this data size in %{link_start}epic 10415%{link_end}." +msgstr "" + msgid "The git server, Gitaly, is not available at this time. Please contact your administrator." msgstr "" @@ -50716,6 +50713,9 @@ msgstr "" msgid "WorkItem|Add a title" msgstr "" +msgid "WorkItem|Add a to do" +msgstr "" + msgid "WorkItem|Add assignee" msgstr "" @@ -50814,6 +50814,9 @@ msgstr "" msgid "WorkItem|Key Result" msgstr "" +msgid "WorkItem|Mark as done" +msgstr "" + msgid "WorkItem|Milestone" msgstr "" diff --git a/qa/Gemfile b/qa/Gemfile index 9189ca8a002..a7e42e32be5 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -3,7 +3,7 @@ source 'https://rubygems.org' gem 'gitlab-qa', '~> 10', '>= 10.3.0', require: 'gitlab/qa' -gem 'gitlab_quality-test_tooling', '~> 0.3.0', require: false +gem 'gitlab_quality-test_tooling', '~> 0.4.0', require: false gem 'activesupport', '~> 6.1.7.2' # This should stay in sync with the root's Gemfile gem 'allure-rspec', '~> 2.20.0' gem 'capybara', '~> 3.39.0' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index f6344da8393..a6d82f117e9 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -111,7 +111,7 @@ GEM rainbow (>= 3, < 4) table_print (= 1.5.7) zeitwerk (>= 2, < 3) - gitlab_quality-test_tooling (0.3.0) + gitlab_quality-test_tooling (0.4.0) activesupport (~> 6.1) gitlab (~> 4.18.0) http (~> 5.0) @@ -328,7 +328,7 @@ DEPENDENCIES fog-core (= 2.1.0) fog-google (~> 1.19) gitlab-qa (~> 10, >= 10.3.0) - gitlab_quality-test_tooling (~> 0.3.0) + gitlab_quality-test_tooling (~> 0.4.0) influxdb-client (~> 2.9) knapsack (~> 4.0) nokogiri (~> 1.14, >= 1.14.3) diff --git a/qa/qa/page/group/menu.rb b/qa/qa/page/group/menu.rb index 752fa37094f..157bc3abaf6 100644 --- a/qa/qa/page/group/menu.rb +++ b/qa/qa/page/group/menu.rb @@ -107,12 +107,6 @@ module QA end end - def go_to_workspaces - within_sidebar do - click_element(:sidebar_menu_link, menu_item: "Workspaces") - end - end - private def hover_settings diff --git a/qa/qa/page/main/menu.rb b/qa/qa/page/main/menu.rb index bb037037bd5..934aa182b12 100644 --- a/qa/qa/page/main/menu.rb +++ b/qa/qa/page/main/menu.rb @@ -129,6 +129,12 @@ module QA click_element(:sidebar_menu_link, menu_item: 'Snippets') end + def go_to_workspaces + return click_element(:nav_item_link, submenu_item: 'Workspaces') if Runtime::Env.super_sidebar_enabled? + + click_element(:sidebar_menu_link, menu_item: 'Workspaces') + end + def go_to_create_project click_element(:new_menu_toggle) click_element(:global_new_project_link) diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index cbf632bfdb0..f097d08fe1b 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -15,6 +15,7 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d let!(:environment) { create(:environment, name: 'production', project: project) } before do + stub_feature_flags(remove_monitor_metrics: false) sign_in(user) end @@ -543,6 +544,18 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d expect(response).to redirect_to(project_metrics_dashboard_path(project)) end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + get :metrics_redirect, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET #metrics' do @@ -614,6 +627,20 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d expect(response).to redirect_to(project_metrics_dashboard_path(project, environment: environment)) end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + expect(environment).not_to receive(:metrics) + + get :metrics, params: environment_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET #additional_metrics' do @@ -709,6 +736,18 @@ RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_d expect(response).to have_gitlab_http_status(:ok) end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns 404 not found' do + additional_metrics(window_params) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe 'GET #metrics_dashboard' do diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 76d8191e342..04dbd9ab671 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Settings::OperationsController do +RSpec.describe Projects::Settings::OperationsController, feature_category: :incident_management do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project) } @@ -11,6 +11,8 @@ RSpec.describe Projects::Settings::OperationsController do end before do + stub_feature_flags(remove_monitor_metrics: false) + sign_in(user) end @@ -65,6 +67,20 @@ RSpec.describe Projects::Settings::OperationsController do end end + shared_examples 'PATCHable without metrics dashboard' do + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + include_examples 'PATCHable' do + let(:permitted_params) do + ActionController::Parameters.new({}).permit! + end + end + end + end + describe 'GET #show' do it 'renders show template' do get :show, params: project_params(project) @@ -124,7 +140,7 @@ RSpec.describe Projects::Settings::OperationsController do end end - context 'incident management' do + context 'incident management', feature_category: :incident_management do describe 'GET #show' do context 'with existing setting' do let!(:incident_management_setting) do @@ -278,7 +294,7 @@ RSpec.describe Projects::Settings::OperationsController do end end - context 'error tracking' do + context 'error tracking', feature_category: :error_tracking do describe 'GET #show' do context 'with existing setting' do let!(:error_tracking_setting) do @@ -323,7 +339,7 @@ RSpec.describe Projects::Settings::OperationsController do end end - context 'metrics dashboard setting' do + context 'metrics dashboard setting', feature_category: :metrics do describe 'PATCH #update' do let(:params) do { @@ -333,11 +349,12 @@ RSpec.describe Projects::Settings::OperationsController do } end - it_behaves_like 'PATCHable' + include_examples 'PATCHable' + include_examples 'PATCHable without metrics dashboard' end end - context 'grafana integration' do + context 'grafana integration', feature_category: :metrics do describe 'PATCH #update' do let(:params) do { @@ -349,7 +366,8 @@ RSpec.describe Projects::Settings::OperationsController do } end - it_behaves_like 'PATCHable' + include_examples 'PATCHable' + include_examples 'PATCHable without metrics dashboard' end end diff --git a/spec/features/merge_request/user_sees_discussions_navigation_spec.rb b/spec/features/merge_request/user_sees_discussions_navigation_spec.rb index e5352ad88ce..5f815bffb22 100644 --- a/spec/features/merge_request/user_sees_discussions_navigation_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_navigation_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe 'Merge request > User sees discussions navigation', :js, feature_category: :code_review_workflow do +RSpec.describe 'Merge request > User sees discussions navigation', + :js, feature_category: :code_review_workflow, + quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/410678' do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:user) { project.creator } let_it_be(:merge_request) { create(:merge_request, source_project: project) } diff --git a/spec/features/projects/environments/environment_metrics_spec.rb b/spec/features/projects/environments/environment_metrics_spec.rb index 0983bfa7abd..e212d464029 100644 --- a/spec/features/projects/environments/environment_metrics_spec.rb +++ b/spec/features/projects/environments/environment_metrics_spec.rb @@ -14,6 +14,8 @@ RSpec.describe 'Environment > Metrics', feature_category: :projects do let!(:staging) { create(:environment, name: 'staging', project: project) } before do + stub_feature_flags(remove_monitor_metrics: false) + project.add_developer(user) stub_any_prometheus_request @@ -66,6 +68,18 @@ RSpec.describe 'Environment > Metrics', feature_category: :projects do it_behaves_like 'has environment selector' end + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'does not provide a link to the monitoring dashboard' do + visit_environment(environment) + + expect(page).not_to have_link('Monitoring') + end + end + def visit_environment(environment) visit project_environment_path(environment.project, environment) end diff --git a/spec/features/projects/work_items/work_item_spec.rb b/spec/features/projects/work_items/work_item_spec.rb index d202f6ad500..b706a624fc5 100644 --- a/spec/features/projects/work_items/work_item_spec.rb +++ b/spec/features/projects/work_items/work_item_spec.rb @@ -41,6 +41,8 @@ RSpec.describe 'Work item', :js, feature_category: :team_planning do it_behaves_like 'work items description' it_behaves_like 'work items milestone' it_behaves_like 'work items notifications' + it_behaves_like 'work items todos' + it_behaves_like 'work items award emoji' end context 'for signed in owner' do @@ -75,5 +77,16 @@ RSpec.describe 'Work item', :js, feature_category: :team_planning do it 'actions dropdown is not displayed' do expect(page).not_to have_selector('[data-testid="work-item-actions-dropdown"]') end + + it 'todos action is not displayed' do + expect(page).not_to have_selector('[data-testid="work-item-todos-action"]') + end + + it 'award button is disabled and add reaction is not displayed' do + within('[data-testid="work-item-award-list"]') do + expect(page).not_to have_selector('[data-testid="emoji-picker"]') + expect(page).to have_selector('[data-testid="award-button"].disabled') + end + end end end diff --git a/spec/frontend/code_review/signals_spec.js b/spec/frontend/code_review/signals_spec.js new file mode 100644 index 00000000000..03c3580860e --- /dev/null +++ b/spec/frontend/code_review/signals_spec.js @@ -0,0 +1,145 @@ +import { start } from '~/code_review/signals'; + +import diffsEventHub from '~/diffs/event_hub'; +import { EVT_MR_PREPARED } from '~/diffs/constants'; +import { getDerivedMergeRequestInformation } from '~/diffs/utils/merge_request'; + +jest.mock('~/diffs/utils/merge_request'); + +describe('~/code_review', () => { + const io = diffsEventHub; + + beforeAll(() => { + getDerivedMergeRequestInformation.mockImplementation(() => ({ + namespace: 'x', + project: 'y', + id: '1', + })); + }); + + describe('start', () => { + it.each` + description | argument + ${'no event hub is provided'} | ${{}} + ${'no parameters are provided'} | ${undefined} + `('throws an error if $description', async ({ argument }) => { + await expect(() => start(argument)).rejects.toThrow('signalBus is a required argument'); + }); + + describe('observeMergeRequestFinishingPreparation', () => { + const callArgs = {}; + const apollo = {}; + let querySpy; + let apolloSubscribeSpy; + let subscribeSpy; + let nextSpy; + let unsubscribeSpy; + let observable; + + beforeEach(() => { + querySpy = jest.fn(); + apolloSubscribeSpy = jest.fn(); + subscribeSpy = jest.fn(); + unsubscribeSpy = jest.fn(); + nextSpy = jest.fn(); + observable = { + next: nextSpy, + subscribe: subscribeSpy.mockReturnValue({ + unsubscribe: unsubscribeSpy, + }), + }; + + querySpy.mockResolvedValue({ + data: { project: { mergeRequest: { id: 'gql:id:1', preparedAt: 'x' } } }, + }); + apolloSubscribeSpy.mockReturnValue(observable); + + apollo.query = querySpy; + apollo.subscribe = apolloSubscribeSpy; + + callArgs.signalBus = io; + callArgs.apolloClient = apollo; + }); + + it('does not query at all if the page does not seem like a merge request', async () => { + getDerivedMergeRequestInformation.mockImplementationOnce(() => ({})); + + await start(callArgs); + + expect(querySpy).not.toHaveBeenCalled(); + expect(apolloSubscribeSpy).not.toHaveBeenCalled(); + }); + + describe('on a merge request page', () => { + it('requests the preparedAt (and id) for the current merge request', async () => { + await start(callArgs); + + expect(querySpy).toHaveBeenCalledWith( + expect.objectContaining({ + variables: { + projectPath: 'x/y', + iid: '1', + }, + }), + ); + }); + + it('does not subscribe to any updates if the preparedAt value is already populated', async () => { + await start(callArgs); + + expect(apolloSubscribeSpy).not.toHaveBeenCalled(); + }); + + describe('if the merge request is still asynchronously preparing', () => { + beforeEach(() => { + querySpy.mockResolvedValue({ + data: { project: { mergeRequest: { id: 'gql:id:1', preparedAt: null } } }, + }); + }); + + it('subscribes to updates', async () => { + await start(callArgs); + + expect(apolloSubscribeSpy).toHaveBeenCalledWith( + expect.objectContaining({ variables: { issuableId: 'gql:id:1' } }), + ); + expect(observable.subscribe).toHaveBeenCalled(); + }); + + describe('when the MR has been updated', () => { + let emitSpy; + let behavior; + + beforeEach(() => { + emitSpy = jest.spyOn(diffsEventHub, '$emit'); + nextSpy.mockImplementation((data) => behavior?.(data)); + subscribeSpy.mockImplementation((handler) => { + behavior = handler; + + return { unsubscribe: unsubscribeSpy }; + }); + }); + + it('does nothing if the MR has not yet finished preparing', async () => { + await start(callArgs); + + observable.next({ data: { mergeRequestMergeStatusUpdated: { preparedAt: null } } }); + + expect(unsubscribeSpy).not.toHaveBeenCalled(); + expect(emitSpy).not.toHaveBeenCalled(); + }); + + it('emits an event and unsubscribes when the MR is prepared', async () => { + await start(callArgs); + + observable.next({ data: { mergeRequestMergeStatusUpdated: { preparedAt: 'x' } } }); + + expect(unsubscribeSpy).toHaveBeenCalled(); + expect(emitSpy).toHaveBeenCalledWith(EVT_MR_PREPARED); + }); + }); + }); + }); + }); + }); +}); diff --git a/spec/frontend/content_editor/components/bubble_menus/formatting_bubble_menu_spec.js b/spec/frontend/content_editor/components/bubble_menus/formatting_bubble_menu_spec.js deleted file mode 100644 index c4bc29adb52..00000000000 --- a/spec/frontend/content_editor/components/bubble_menus/formatting_bubble_menu_spec.js +++ /dev/null @@ -1,87 +0,0 @@ -import { mockTracking } from 'helpers/tracking_helper'; -import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; -import FormattingBubbleMenu from '~/content_editor/components/bubble_menus/formatting_bubble_menu.vue'; -import BubbleMenu from '~/content_editor/components/bubble_menus/bubble_menu.vue'; -import { stubComponent } from 'helpers/stub_component'; - -import { - BUBBLE_MENU_TRACKING_ACTION, - CONTENT_EDITOR_TRACKING_LABEL, -} from '~/content_editor/constants'; -import { createTestEditor } from '../../test_utils'; - -describe('content_editor/components/bubble_menus/formatting_bubble_menu', () => { - let wrapper; - let trackingSpy; - let tiptapEditor; - - const buildEditor = () => { - tiptapEditor = createTestEditor(); - - jest.spyOn(tiptapEditor, 'isActive'); - }; - - const buildWrapper = () => { - wrapper = shallowMountExtended(FormattingBubbleMenu, { - provide: { - tiptapEditor, - }, - stubs: { - BubbleMenu: stubComponent(BubbleMenu), - }, - }); - }; - - beforeEach(() => { - trackingSpy = mockTracking(undefined, null, jest.spyOn); - buildEditor(); - }); - - it('renders bubble menu component', () => { - buildWrapper(); - const bubbleMenu = wrapper.findComponent(BubbleMenu); - - expect(bubbleMenu.classes()).toEqual(['gl-shadow', 'gl-rounded-base', 'gl-bg-white']); - }); - - describe.each` - testId | controlProps - ${'bold'} | ${{ contentType: 'bold', iconName: 'bold', label: 'Bold text', editorCommand: 'toggleBold' }} - ${'italic'} | ${{ contentType: 'italic', iconName: 'italic', label: 'Italic text', editorCommand: 'toggleItalic' }} - ${'strike'} | ${{ contentType: 'strike', iconName: 'strikethrough', label: 'Strikethrough', editorCommand: 'toggleStrike' }} - ${'code'} | ${{ contentType: 'code', iconName: 'code', label: 'Code', editorCommand: 'toggleCode' }} - ${'superscript'} | ${{ contentType: 'superscript', iconName: 'superscript', label: 'Superscript', editorCommand: 'toggleSuperscript' }} - ${'subscript'} | ${{ contentType: 'subscript', iconName: 'subscript', label: 'Subscript', editorCommand: 'toggleSubscript' }} - ${'highlight'} | ${{ contentType: 'highlight', iconName: 'highlight', label: 'Highlight', editorCommand: 'toggleHighlight' }} - ${'link'} | ${{ contentType: 'link', iconName: 'link', label: 'Insert link', editorCommand: 'editLink' }} - `('given a $testId toolbar control', ({ testId, controlProps }) => { - beforeEach(() => { - buildWrapper(); - }); - - it('renders the toolbar control with the provided properties', () => { - expect(wrapper.findByTestId(testId).exists()).toBe(true); - - expect(wrapper.findByTestId(testId).props()).toEqual( - expect.objectContaining({ - ...controlProps, - size: 'medium', - category: 'tertiary', - }), - ); - }); - - it('tracks the execution of toolbar controls', () => { - const eventData = { contentType: 'italic', value: 1 }; - const { contentType, value } = eventData; - - wrapper.findByTestId(testId).vm.$emit('execute', eventData); - - expect(trackingSpy).toHaveBeenCalledWith(undefined, BUBBLE_MENU_TRACKING_ACTION, { - label: CONTENT_EDITOR_TRACKING_LABEL, - property: contentType, - value, - }); - }); - }); -}); diff --git a/spec/frontend/content_editor/components/content_editor_spec.js b/spec/frontend/content_editor/components/content_editor_spec.js index 8bbd79a61af..852c8a9591a 100644 --- a/spec/frontend/content_editor/components/content_editor_spec.js +++ b/spec/frontend/content_editor/components/content_editor_spec.js @@ -6,7 +6,6 @@ import ContentEditor from '~/content_editor/components/content_editor.vue'; import ContentEditorAlert from '~/content_editor/components/content_editor_alert.vue'; import ContentEditorProvider from '~/content_editor/components/content_editor_provider.vue'; import EditorStateObserver from '~/content_editor/components/editor_state_observer.vue'; -import FormattingBubbleMenu from '~/content_editor/components/bubble_menus/formatting_bubble_menu.vue'; import CodeBlockBubbleMenu from '~/content_editor/components/bubble_menus/code_block_bubble_menu.vue'; import LinkBubbleMenu from '~/content_editor/components/bubble_menus/link_bubble_menu.vue'; import MediaBubbleMenu from '~/content_editor/components/bubble_menus/media_bubble_menu.vue'; @@ -264,11 +263,10 @@ describe('ContentEditor', () => { }); it.each` - name | component - ${'formatting'} | ${FormattingBubbleMenu} - ${'link'} | ${LinkBubbleMenu} - ${'media'} | ${MediaBubbleMenu} - ${'codeBlock'} | ${CodeBlockBubbleMenu} + name | component + ${'link'} | ${LinkBubbleMenu} + ${'media'} | ${MediaBubbleMenu} + ${'codeBlock'} | ${CodeBlockBubbleMenu} `('renders formatting bubble menu', ({ component }) => { createWrapper(); diff --git a/spec/frontend/content_editor/components/formatting_toolbar_spec.js b/spec/frontend/content_editor/components/formatting_toolbar_spec.js index 2fc7e5e2e1b..e04c6a00765 100644 --- a/spec/frontend/content_editor/components/formatting_toolbar_spec.js +++ b/spec/frontend/content_editor/components/formatting_toolbar_spec.js @@ -31,6 +31,7 @@ describe('content_editor/components/formatting_toolbar', () => { ${'text-styles'} | ${{}} ${'bold'} | ${{ contentType: 'bold', iconName: 'bold', label: 'Bold text', editorCommand: 'toggleBold' }} ${'italic'} | ${{ contentType: 'italic', iconName: 'italic', label: 'Italic text', editorCommand: 'toggleItalic' }} + ${'strike'} | ${{ contentType: 'strike', iconName: 'strikethrough', label: 'Strikethrough', editorCommand: 'toggleStrike' }} ${'blockquote'} | ${{ contentType: 'blockquote', iconName: 'quote', label: 'Insert a quote', editorCommand: 'toggleBlockquote' }} ${'code'} | ${{ contentType: 'code', iconName: 'code', label: 'Code', editorCommand: 'toggleCode' }} ${'link'} | ${{}} diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 7f9cd1a274d..f883aea764f 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -9,6 +9,7 @@ import { DIFF_VIEW_COOKIE_NAME, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE, + EVT_MR_PREPARED, } from '~/diffs/constants'; import { LOAD_SINGLE_DIFF_FAILED } from '~/diffs/i18n'; import * as diffActions from '~/diffs/store/actions'; @@ -396,23 +397,46 @@ describe('DiffsStoreActions', () => { ); }); - it('should show a warning on 404 reponse', async () => { - mock.onGet(endpointMetadata).reply(HTTP_STATUS_NOT_FOUND); + describe('on a 404 response', () => { + let dismissAlert; - await testAction( - diffActions.fetchDiffFilesMeta, - {}, - { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, - [{ type: types.SET_LOADING, payload: true }], - [], - ); + beforeAll(() => { + dismissAlert = jest.fn(); - expect(createAlert).toHaveBeenCalledTimes(1); - expect(createAlert).toHaveBeenCalledWith({ - message: expect.stringMatching( - 'Building your merge request. Wait a few moments, then refresh this page.', - ), - variant: 'warning', + mock.onGet(endpointMetadata).reply(HTTP_STATUS_NOT_FOUND); + createAlert.mockImplementation(() => ({ dismiss: dismissAlert })); + }); + + it('should show a warning', async () => { + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + + expect(createAlert).toHaveBeenCalledTimes(1); + expect(createAlert).toHaveBeenCalledWith({ + message: expect.stringMatching( + 'Building your merge request… This page will update when the build is complete.', + ), + variant: 'warning', + }); + }); + + it("should attempt to close the alert if the MR reports that it's been prepared", async () => { + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + + diffsEventHub.$emit(EVT_MR_PREPARED); + + expect(dismissAlert).toHaveBeenCalled(); }); }); diff --git a/spec/frontend/environments/deploy_freeze_alert_spec.js b/spec/frontend/environments/deploy_freeze_alert_spec.js new file mode 100644 index 00000000000..b7202253e61 --- /dev/null +++ b/spec/frontend/environments/deploy_freeze_alert_spec.js @@ -0,0 +1,111 @@ +import { GlAlert, GlLink } from '@gitlab/ui'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import DeployFreezeAlert from '~/environments/components/deploy_freeze_alert.vue'; +import deployFreezesQuery from '~/environments/graphql/queries/deploy_freezes.query.graphql'; +import { formatDate } from '~/lib/utils/datetime/date_format_utility'; + +const ENVIRONMENT_NAME = 'staging'; + +Vue.use(VueApollo); +describe('~/environments/components/deploy_freeze_alert.vue', () => { + let wrapper; + + const createWrapper = (deployFreezes = []) => { + const mockApollo = createMockApollo([ + [ + deployFreezesQuery, + jest.fn().mockResolvedValue({ + data: { + project: { + id: '1', + __typename: 'Project', + environment: { + id: '1', + __typename: 'Environment', + deployFreezes, + }, + }, + }, + }), + ], + ]); + wrapper = mountExtended(DeployFreezeAlert, { + apolloProvider: mockApollo, + provide: { + projectFullPath: 'gitlab-org/gitlab', + }, + propsData: { + name: ENVIRONMENT_NAME, + }, + }); + }; + + describe('with deploy freezes', () => { + let deployFreezes; + let alert; + + beforeEach(async () => { + deployFreezes = [ + { + __typename: 'CiFreezePeriod', + startTime: new Date('2020-02-01'), + endTime: new Date('2020-02-02'), + }, + { + __typename: 'CiFreezePeriod', + startTime: new Date('2020-01-01'), + endTime: new Date('2020-01-02'), + }, + ]; + + createWrapper(deployFreezes); + + await waitForPromises(); + + alert = wrapper.findComponent(GlAlert); + }); + + it('shows an alert', () => { + expect(alert.exists()).toBe(true); + }); + + it('shows the start time of the most recent freeze period', () => { + expect(alert.text()).toContain(`from ${formatDate(deployFreezes[1].startTime)}`); + }); + + it('shows the end time of the most recent freeze period', () => { + expect(alert.text()).toContain(`to ${formatDate(deployFreezes[1].endTime)}`); + }); + + it('shows a link to the docs', () => { + const link = alert.findComponent(GlLink); + expect(link.attributes('href')).toBe( + '/help/user/project/releases/index#prevent-unintentional-releases-by-setting-a-deploy-freeze', + ); + expect(link.text()).toBe('deploy freeze documentation'); + }); + }); + + describe('without deploy freezes', () => { + let deployFreezes; + let alert; + + beforeEach(async () => { + deployFreezes = []; + + createWrapper(deployFreezes); + + await waitForPromises(); + + alert = wrapper.findComponent(GlAlert); + }); + + it('does not show an alert', () => { + expect(alert.exists()).toBe(false); + }); + }); +}); diff --git a/spec/frontend/environments/environments_detail_header_spec.js b/spec/frontend/environments/environments_detail_header_spec.js index dc6f9ce91ca..9464aeff028 100644 --- a/spec/frontend/environments/environments_detail_header_spec.js +++ b/spec/frontend/environments/environments_detail_header_spec.js @@ -5,6 +5,7 @@ import DeleteEnvironmentModal from '~/environments/components/delete_environment import EnvironmentsDetailHeader from '~/environments/components/environments_detail_header.vue'; import StopEnvironmentModal from '~/environments/components/stop_environment_modal.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; +import DeployFreezeAlert from '~/environments/components/deploy_freeze_alert.vue'; import { createEnvironment } from './mock_data'; describe('Environments detail header component', () => { @@ -27,6 +28,7 @@ describe('Environments detail header component', () => { const findDestroyButton = () => wrapper.findByTestId('destroy-button'); const findStopEnvironmentModal = () => wrapper.findComponent(StopEnvironmentModal); const findDeleteEnvironmentModal = () => wrapper.findComponent(DeleteEnvironmentModal); + const findDeployFreezeAlert = () => wrapper.findComponent(DeployFreezeAlert); const buttons = [ ['Cancel Auto Stop At', findCancelAutoStopAtButton], @@ -44,6 +46,9 @@ describe('Environments detail header component', () => { GlSprintf, TimeAgo, }, + provide: { + glFeatures, + }, directives: { GlTooltip: createMockDirective('gl-tooltip'), }, @@ -54,9 +59,6 @@ describe('Environments detail header component', () => { canDestroyEnvironment: false, ...props, }, - provide: { - glFeatures, - }, }); }; @@ -262,4 +264,13 @@ describe('Environments detail header component', () => { expect(findDeleteEnvironmentModal().exists()).toBe(true); }); }); + + describe('deploy freeze alert', () => { + it('passes the environment name to the alert', () => { + const environment = createEnvironment(); + createWrapper({ props: { environment } }); + + expect(findDeployFreezeAlert().props('name')).toBe(environment.name); + }); + }); }); diff --git a/spec/frontend/fixtures/metrics_dashboard.rb b/spec/frontend/fixtures/metrics_dashboard.rb index 109b016d980..036ce9eea3a 100644 --- a/spec/frontend/fixtures/metrics_dashboard.rb +++ b/spec/frontend/fixtures/metrics_dashboard.rb @@ -17,6 +17,7 @@ RSpec.describe MetricsDashboard, '(JavaScript fixtures)', type: :controller do end before do + stub_feature_flags(remove_monitor_metrics: false) sign_in(user) project.add_maintainer(user) diff --git a/spec/frontend/projects/commit_box/info/init_details_button_spec.js b/spec/frontend/projects/commit_box/info/init_details_button_spec.js index 8aaba31e23e..bf9c6a4c998 100644 --- a/spec/frontend/projects/commit_box/info/init_details_button_spec.js +++ b/spec/frontend/projects/commit_box/info/init_details_button_spec.js @@ -3,13 +3,14 @@ import { initDetailsButton } from '~/projects/commit_box/info/init_details_butto const htmlFixture = ` <span> - <a href="#" class="js-details-expand">Expand</a> + <a href="#" class="js-details-expand"><span class="sub-element">Expand</span></a> <span class="js-details-content hide">Some branch</span> </span>`; describe('~/projects/commit_box/info/init_details_button', () => { const findExpandButton = () => document.querySelector('.js-details-expand'); const findContent = () => document.querySelector('.js-details-content'); + const findExpandSubElement = () => document.querySelector('.sub-element'); beforeEach(() => { setHTMLFixture(htmlFixture); @@ -29,4 +30,18 @@ describe('~/projects/commit_box/info/init_details_button', () => { expect(findExpandButton().classList).toContain('gl-display-none'); }); }); + + describe('when user clicks on element inside of expand button', () => { + it('renders the content by removing the `hide` class', () => { + expect(findContent().classList).toContain('hide'); + findExpandSubElement().click(); + expect(findContent().classList).not.toContain('hide'); + }); + + it('hides the expand button by adding the `gl-display-none` class', () => { + expect(findExpandButton().classList).not.toContain('gl-display-none'); + findExpandSubElement().click(); + expect(findExpandButton().classList).toContain('gl-display-none'); + }); + }); }); diff --git a/spec/frontend/work_items/components/work_item_award_emoji_spec.js b/spec/frontend/work_items/components/work_item_award_emoji_spec.js new file mode 100644 index 00000000000..f87c0e3f357 --- /dev/null +++ b/spec/frontend/work_items/components/work_item_award_emoji_spec.js @@ -0,0 +1,170 @@ +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import { shallowMount } from '@vue/test-utils'; + +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; + +import { isLoggedIn } from '~/lib/utils/common_utils'; +import AwardList from '~/vue_shared/components/awards_list.vue'; +import WorkItemAwardEmoji from '~/work_items/components/work_item_award_emoji.vue'; +import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; +import { + EMOJI_ACTION_REMOVE, + EMOJI_ACTION_ADD, + EMOJI_THUMBSUP, + EMOJI_THUMBSDOWN, +} from '~/work_items/constants'; + +import { + workItemByIidResponseFactory, + mockAwardsWidget, + updateWorkItemMutationResponseFactory, + mockAwardEmojiThumbsUp, +} from '../mock_data'; + +jest.mock('~/lib/utils/common_utils'); +Vue.use(VueApollo); + +describe('WorkItemAwardEmoji component', () => { + let wrapper; + + const errorMessage = 'Failed to update the award'; + + const workItemQueryResponse = workItemByIidResponseFactory(); + const workItemSuccessHandler = jest + .fn() + .mockResolvedValue(updateWorkItemMutationResponseFactory()); + const awardEmojiAddSuccessHandler = jest.fn().mockResolvedValue( + updateWorkItemMutationResponseFactory({ + awardEmoji: { + ...mockAwardsWidget, + nodes: [mockAwardEmojiThumbsUp], + }, + }), + ); + const awardEmojiRemoveSuccessHandler = jest.fn().mockResolvedValue( + updateWorkItemMutationResponseFactory({ + awardEmoji: { + ...mockAwardsWidget, + nodes: [], + }, + }), + ); + const workItemUpdateFailureHandler = jest.fn().mockRejectedValue(new Error(errorMessage)); + const mockWorkItem = workItemQueryResponse.data.workspace.workItems.nodes[0]; + + const createComponent = ({ + mockWorkItemUpdateMutationHandler = [updateWorkItemMutation, workItemSuccessHandler], + workItem = mockWorkItem, + awardEmoji = { ...mockAwardsWidget, nodes: [] }, + } = {}) => { + wrapper = shallowMount(WorkItemAwardEmoji, { + isLoggedIn: isLoggedIn(), + apolloProvider: createMockApollo([mockWorkItemUpdateMutationHandler]), + propsData: { + workItem, + awardEmoji, + }, + }); + }; + + const findAwardsList = () => wrapper.findComponent(AwardList); + + beforeEach(() => { + isLoggedIn.mockReturnValue(true); + window.gon = { + current_user_id: 1, + }; + + createComponent(); + }); + + it('renders the award-list component with default props', () => { + expect(findAwardsList().exists()).toBe(true); + expect(findAwardsList().props()).toEqual({ + boundary: '', + canAwardEmoji: true, + currentUserId: 1, + defaultAwards: [EMOJI_THUMBSUP, EMOJI_THUMBSDOWN], + selectedClass: 'selected', + awards: [], + }); + }); + + it('renders awards-list component with awards present', () => { + createComponent({ awardEmoji: mockAwardsWidget }); + + expect(findAwardsList().props('awards')).toEqual([ + { + id: 1, + name: EMOJI_THUMBSUP, + user: { + id: 5, + }, + }, + { + id: 2, + name: EMOJI_THUMBSDOWN, + user: { + id: 5, + }, + }, + ]); + }); + + it.each` + expectedAssertion | action | successHandler | mockAwardEmojiNodes + ${'added'} | ${EMOJI_ACTION_ADD} | ${awardEmojiAddSuccessHandler} | ${[]} + ${'removed'} | ${EMOJI_ACTION_REMOVE} | ${awardEmojiRemoveSuccessHandler} | ${[mockAwardEmojiThumbsUp]} + `( + 'calls mutation when an award emoji is $expectedAssertion', + async ({ action, successHandler, mockAwardEmojiNodes }) => { + createComponent({ + mockWorkItemUpdateMutationHandler: [updateWorkItemMutation, successHandler], + awardEmoji: { + ...mockAwardsWidget, + nodes: mockAwardEmojiNodes, + }, + }); + + findAwardsList().vm.$emit('award', EMOJI_THUMBSUP); + + await waitForPromises(); + + expect(successHandler).toHaveBeenCalledWith({ + input: { + id: mockWorkItem.id, + awardEmojiWidget: { + action, + name: EMOJI_THUMBSUP, + }, + }, + }); + }, + ); + + it('emits error when the update mutation fails', async () => { + createComponent({ + mockWorkItemUpdateMutationHandler: [updateWorkItemMutation, workItemUpdateFailureHandler], + }); + + findAwardsList().vm.$emit('award', EMOJI_THUMBSUP); + + await waitForPromises(); + + expect(wrapper.emitted('error')).toEqual([[errorMessage]]); + }); + + describe('when user is not logged in', () => { + beforeEach(() => { + isLoggedIn.mockReturnValue(false); + + createComponent(); + }); + + it('renders the component with required props and canAwardEmoji false', () => { + expect(findAwardsList().props('canAwardEmoji')).toBe(false); + }); + }); +}); diff --git a/spec/frontend/work_items/components/work_item_detail_spec.js b/spec/frontend/work_items/components/work_item_detail_spec.js index 630eb78d5c0..557ae07969e 100644 --- a/spec/frontend/work_items/components/work_item_detail_spec.js +++ b/spec/frontend/work_items/components/work_item_detail_spec.js @@ -9,6 +9,7 @@ import { import { shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; +import { isLoggedIn } from '~/lib/utils/common_utils'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import setWindowLocation from 'helpers/set_window_location_helper'; @@ -27,6 +28,7 @@ import WorkItemTree from '~/work_items/components/work_item_links/work_item_tree import WorkItemNotes from '~/work_items/components/work_item_notes.vue'; import WorkItemDetailModal from '~/work_items/components/work_item_detail_modal.vue'; import AbuseCategorySelector from '~/abuse_reports/components/abuse_category_selector.vue'; +import WorkItemTodos from '~/work_items/components/work_item_todos.vue'; import { i18n } from '~/work_items/constants'; import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; import workItemDatesSubscription from '~/graphql_shared/subscriptions/work_item_dates.subscription.graphql'; @@ -47,6 +49,8 @@ import { mockWorkItemCommentNote, } from '../mock_data'; +jest.mock('~/lib/utils/common_utils'); + describe('WorkItemDetail component', () => { let wrapper; @@ -91,6 +95,7 @@ describe('WorkItemDetail component', () => { const findNotesWidget = () => wrapper.findComponent(WorkItemNotes); const findModal = () => wrapper.findComponent(WorkItemDetailModal); const findAbuseCategorySelector = () => wrapper.findComponent(AbuseCategorySelector); + const findWorkItemTodos = () => wrapper.findComponent(WorkItemTodos); const createComponent = ({ isModal = false, @@ -114,6 +119,7 @@ describe('WorkItemDetail component', () => { wrapper = shallowMount(WorkItemDetail, { apolloProvider: createMockApollo(handlers), + isLoggedIn: isLoggedIn(), propsData: { isModal, workItemId, workItemIid }, data() { return { @@ -146,6 +152,10 @@ describe('WorkItemDetail component', () => { }); }; + beforeEach(() => { + isLoggedIn.mockReturnValue(true); + }); + afterEach(() => { setWindowLocation(''); }); @@ -187,6 +197,10 @@ describe('WorkItemDetail component', () => { it('updates the document title', () => { expect(document.title).toEqual('Updated title · Task · test-project-path'); }); + + it('renders todos widget if logged in', () => { + expect(findWorkItemTodos().exists()).toBe(true); + }); }); describe('close button', () => { @@ -768,4 +782,16 @@ describe('WorkItemDetail component', () => { expect(findAbuseCategorySelector().exists()).toBe(false); }); }); + + describe('todos widget', () => { + beforeEach(async () => { + isLoggedIn.mockReturnValue(false); + createComponent(); + await waitForPromises(); + }); + + it('does not renders if not logged in', () => { + expect(findWorkItemTodos().exists()).toBe(false); + }); + }); }); diff --git a/spec/frontend/work_items/components/work_item_todos_spec.js b/spec/frontend/work_items/components/work_item_todos_spec.js new file mode 100644 index 00000000000..83b61a04298 --- /dev/null +++ b/spec/frontend/work_items/components/work_item_todos_spec.js @@ -0,0 +1,97 @@ +import { GlButton, GlIcon } from '@gitlab/ui'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import WorkItemTodos from '~/work_items/components/work_item_todos.vue'; +import { ADD, TODO_DONE_ICON, TODO_ADD_ICON } from '~/work_items/constants'; +import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; +import { updateGlobalTodoCount } from '~/sidebar/utils'; +import { workItemResponseFactory, updateWorkItemMutationResponseFactory } from '../mock_data'; + +jest.mock('~/sidebar/utils'); + +describe('WorkItemTodo component', () => { + Vue.use(VueApollo); + + let wrapper; + + const findTodoWidget = () => wrapper.findComponent(GlButton); + const findTodoIcon = () => wrapper.findComponent(GlIcon); + + const errorMessage = 'Failed to add item'; + const workItemQueryResponse = workItemResponseFactory({ canUpdate: true }); + const successHandler = jest + .fn() + .mockResolvedValue(updateWorkItemMutationResponseFactory({ canUpdate: true })); + const failureHandler = jest.fn().mockRejectedValue(new Error(errorMessage)); + + const inputVariables = { + id: 'gid://gitlab/WorkItem/1', + currentUserTodosWidget: { + action: ADD, + }, + }; + + const createComponent = ({ + currentUserTodosMock = [updateWorkItemMutation, successHandler], + currentUserTodos = [], + } = {}) => { + const handlers = [currentUserTodosMock]; + wrapper = shallowMountExtended(WorkItemTodos, { + apolloProvider: createMockApollo(handlers), + propsData: { + workItem: workItemQueryResponse.data.workItem, + currentUserTodos, + }, + }); + }; + + it('renders the widget', () => { + createComponent(); + + expect(findTodoWidget().exists()).toBe(true); + expect(findTodoIcon().props('name')).toEqual(TODO_ADD_ICON); + expect(findTodoIcon().classes('gl-fill-blue-500')).toBe(false); + }); + + it('renders mark as done button when there is pending item', () => { + createComponent({ + currentUserTodos: [ + { + node: { + id: 'gid://gitlab/Todo/1', + state: 'pending', + }, + }, + ], + }); + + expect(findTodoIcon().props('name')).toEqual(TODO_DONE_ICON); + expect(findTodoIcon().classes('gl-fill-blue-500')).toBe(true); + }); + + it('calls update mutation when to do button is clicked', async () => { + createComponent(); + + findTodoWidget().vm.$emit('click'); + + await waitForPromises(); + + expect(successHandler).toHaveBeenCalledWith({ + input: inputVariables, + }); + expect(updateGlobalTodoCount).toHaveBeenCalled(); + }); + + it('emits error when the update mutation fails', async () => { + createComponent({ currentUserTodosMock: [updateWorkItemMutation, failureHandler] }); + + findTodoWidget().vm.$emit('click'); + + await waitForPromises(); + + expect(wrapper.emitted('error')).toEqual([[errorMessage]]); + }); +}); diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index 0d5f92d0f4a..05c6a21bb38 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -46,6 +46,29 @@ export const mockMilestone = { dueDate: '2022-10-24', }; +export const mockAwardEmojiThumbsUp = { + name: 'thumbsup', + __typename: 'AwardEmoji', + user: { + id: 'gid://gitlab/User/5', + __typename: 'UserCore', + }, +}; + +export const mockAwardEmojiThumbsDown = { + name: 'thumbsdown', + __typename: 'AwardEmoji', + user: { + id: 'gid://gitlab/User/5', + __typename: 'UserCore', + }, +}; + +export const mockAwardsWidget = { + nodes: [mockAwardEmojiThumbsUp, mockAwardEmojiThumbsDown], + __typename: 'AwardEmojiConnection', +}; + export const workItemQueryResponse = { data: { workItem: { @@ -386,6 +409,8 @@ export const workItemResponseFactory = ({ canDelete = false, adminParentLink = false, notificationsWidgetPresent = true, + currentUserTodosWidgetPresent = true, + awardEmojiWidgetPresent = true, subscribed = true, allowsMultipleAssignees = true, assigneesWidgetPresent = true, @@ -409,6 +434,7 @@ export const workItemResponseFactory = ({ author = mockAssignees[0], createdAt = '2022-08-03T12:41:54Z', updatedAt = '2022-08-08T12:32:54Z', + awardEmoji = mockAwardsWidget, } = {}) => ({ data: { workItem: { @@ -579,6 +605,32 @@ export const workItemResponseFactory = ({ subscribed, } : { type: 'MOCK TYPE' }, + currentUserTodosWidgetPresent + ? { + type: 'CURRENT_USER_TODOS', + currentUserTodos: { + edges: [ + { + node: { + id: 'gid://gitlab/Todo/1', + state: 'pending', + __typename: 'Todo', + }, + __typename: 'TodoEdge', + }, + ], + __typename: 'TodoConnection', + }, + __typename: 'WorkItemWidgetCurrentUserTodos', + } + : { type: 'MOCK TYPE' }, + awardEmojiWidgetPresent + ? { + __typename: 'WorkItemWidgetAwardEmoji', + type: 'AWARD_EMOJI', + awardEmoji, + } + : { type: 'MOCK TYPE' }, ], }, }, @@ -599,6 +651,18 @@ export const workItemByIidResponseFactory = (options) => { }; }; +export const updateWorkItemMutationResponseFactory = (options) => { + const response = workItemResponseFactory(options); + return { + data: { + workItemUpdate: { + workItem: response.data.workItem, + errors: [], + }, + }, + }; +}; + export const getIssueDetailsResponse = ({ confidential = false } = {}) => ({ data: { issue: { diff --git a/spec/frontend/work_items/utils_spec.js b/spec/frontend/work_items/utils_spec.js index aa24b80cf08..b8af5f10a5a 100644 --- a/spec/frontend/work_items/utils_spec.js +++ b/spec/frontend/work_items/utils_spec.js @@ -1,4 +1,9 @@ -import { autocompleteDataSources, markdownPreviewPath } from '~/work_items/utils'; +import { + autocompleteDataSources, + markdownPreviewPath, + getWorkItemTodoOptimisticResponse, +} from '~/work_items/utils'; +import { workItemResponseFactory } from './mock_data'; describe('autocompleteDataSources', () => { beforeEach(() => { @@ -25,3 +30,17 @@ describe('markdownPreviewPath', () => { ); }); }); + +describe('getWorkItemTodoOptimisticResponse', () => { + it.each` + scenario | pendingTodo | result + ${'empty'} | ${false} | ${0} + ${'present'} | ${true} | ${1} + `('returns correct response when pending item list is $scenario', ({ pendingTodo, result }) => { + const workItem = workItemResponseFactory({ canUpdate: true }); + expect( + getWorkItemTodoOptimisticResponse({ workItem, pendingTodo }).workItemUpdate.workItem + .widgets[0].currentUserTodos.edges.length, + ).toBe(result); + }); +}); diff --git a/spec/graphql/resolvers/ci/jobs_resolver_spec.rb b/spec/graphql/resolvers/ci/jobs_resolver_spec.rb index 1e9559b738b..b99eb56d6ab 100644 --- a/spec/graphql/resolvers/ci/jobs_resolver_spec.rb +++ b/spec/graphql/resolvers/ci/jobs_resolver_spec.rb @@ -14,10 +14,11 @@ RSpec.describe Resolvers::Ci::JobsResolver, feature_category: :continuous_integr create(:ci_build, :dast, name: 'SAST job', pipeline: pipeline) create(:ci_build, :container_scanning, name: 'Container scanning job', pipeline: pipeline) create(:ci_build, name: 'Job with tags', pipeline: pipeline, tag_list: ['review']) + create(:ci_bridge, name: 'Bridge job', pipeline: pipeline) end describe '#resolve' do - context 'when security_report_types is empty' do + context 'when none of the optional params are given' do it "returns all of the pipeline's jobs" do jobs = resolve(described_class, obj: pipeline, arg_style: :internal) @@ -26,7 +27,8 @@ RSpec.describe Resolvers::Ci::JobsResolver, feature_category: :continuous_integr have_attributes(name: 'DAST job'), have_attributes(name: 'SAST job'), have_attributes(name: 'Container scanning job'), - have_attributes(name: 'Job with tags') + have_attributes(name: 'Job with tags'), + have_attributes(name: 'Bridge job') ) end end @@ -50,12 +52,14 @@ RSpec.describe Resolvers::Ci::JobsResolver, feature_category: :continuous_integr context 'when a job has tags' do it "returns jobs with tags when applicable" do jobs = resolve(described_class, obj: pipeline, arg_style: :internal) + expect(jobs).to contain_exactly( have_attributes(tag_list: []), have_attributes(tag_list: []), have_attributes(tag_list: []), have_attributes(tag_list: []), - have_attributes(tag_list: ['review']) + have_attributes(tag_list: ['review']), + have_attributes(name: 'Bridge job') # A bridge job has no tag list ) end end @@ -72,5 +76,14 @@ RSpec.describe Resolvers::Ci::JobsResolver, feature_category: :continuous_integr ) end end + + context 'when filtering by job kind' do + it "returns jobs with that type" do + jobs = resolve(described_class, obj: pipeline, arg_style: :internal, args: { job_kind: ::Ci::Bridge }) + expect(jobs).to contain_exactly( + have_attributes(name: 'Bridge job') + ) + end + end end end diff --git a/spec/graphql/resolvers/metrics/dashboard_resolver_spec.rb b/spec/graphql/resolvers/metrics/dashboard_resolver_spec.rb index 4112e3d4fe6..354fd350aa7 100644 --- a/spec/graphql/resolvers/metrics/dashboard_resolver_spec.rb +++ b/spec/graphql/resolvers/metrics/dashboard_resolver_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Resolvers::Metrics::DashboardResolver do +RSpec.describe Resolvers::Metrics::DashboardResolver, feature_category: :metrics do include GraphqlHelpers let_it_be(:current_user) { create(:user) } @@ -21,6 +21,7 @@ RSpec.describe Resolvers::Metrics::DashboardResolver do let(:parent_object) { create(:environment, project: project) } before do + stub_feature_flags(remove_monitor_metrics: false) project.add_developer(current_user) end @@ -39,6 +40,17 @@ RSpec.describe Resolvers::Metrics::DashboardResolver do expect(resolve_dashboard).to be_nil end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'returns nil', :aggregate_failures do + expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:find_for) + expect(resolve_dashboard).to be_nil + end + end end end end diff --git a/spec/helpers/environment_helper_spec.rb b/spec/helpers/environment_helper_spec.rb index c2d17832e8c..64735f8b23b 100644 --- a/spec/helpers/environment_helper_spec.rb +++ b/spec/helpers/environment_helper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe EnvironmentHelper do +RSpec.describe EnvironmentHelper, feature_category: :environment_management do describe '#render_deployment_status' do context 'when using a manual deployment' do it 'renders a span tag' do @@ -56,7 +56,6 @@ RSpec.describe EnvironmentHelper do can_destroy_environment: true, can_stop_environment: true, can_admin_environment: true, - environment_metrics_path: project_metrics_dashboard_path(project, environment: environment), environments_fetch_path: project_environments_path(project, format: :json), environment_edit_path: edit_project_environment_path(project, environment), environment_stop_path: stop_project_environment_path(project, environment), @@ -69,5 +68,17 @@ RSpec.describe EnvironmentHelper do graphql_etag_key: environment.etag_cache_key }.to_json) end + + context 'when metrics dashboard feature is available' do + before do + stub_feature_flags(remove_monitor_metrics: false) + end + + it 'includes metrics path' do + expect(Gitlab::Json.parse(subject)).to include( + 'environment_metrics_path' => project_metrics_dashboard_path(project, environment: environment) + ) + end + end end end diff --git a/spec/helpers/environments_helper_spec.rb b/spec/helpers/environments_helper_spec.rb index cf33f8a4939..0ebec3ed6d0 100644 --- a/spec/helpers/environments_helper_spec.rb +++ b/spec/helpers/environments_helper_spec.rb @@ -2,13 +2,15 @@ require 'spec_helper' -RSpec.describe EnvironmentsHelper do +RSpec.describe EnvironmentsHelper, feature_category: :environment_management do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, :repository) } let_it_be(:environment) { create(:environment, project: project) } - describe '#metrics_data' do + describe '#metrics_data', feature_category: :metrics do before do + stub_feature_flags(remove_monitor_metrics: false) + # This is so that this spec also passes in EE. allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:can?).and_return(true) @@ -103,9 +105,19 @@ RSpec.describe EnvironmentsHelper do end end end + + context 'when metrics dashboard feature is unavailable' do + before do + stub_feature_flags(remove_monitor_metrics: true) + end + + it 'does not return data' do + expect(metrics_data).to be_empty + end + end end - describe '#custom_metrics_available?' do + describe '#custom_metrics_available?', feature_category: :metrics do subject { helper.custom_metrics_available?(project) } before do diff --git a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb index 18ac8a7b42b..2064a592246 100644 --- a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb @@ -7,20 +7,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu let(:supported_dast_versions) { described_class::SUPPORTED_VERSIONS[:dast].join(', ') } - let(:analyzer_vendor) do - { 'name' => 'A DAST analyzer' } - end - - let(:scanner_vendor) do - { 'name' => 'A DAST scanner' } - end - let(:scanner) do { 'id' => 'my-dast-scanner', 'name' => 'My DAST scanner', 'version' => '0.2.0', - 'vendor' => scanner_vendor + 'vendor' => { 'name' => 'A DAST scanner' } } end @@ -33,7 +25,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu 'id' => 'my-dast-analyzer', 'name' => 'My DAST analyzer', 'version' => '0.1.0', - 'vendor' => analyzer_vendor + 'vendor' => { 'name' => 'A DAST analyzer' } }, 'end_time' => '2020-01-28T03:26:02', 'scanned_resources' => [], @@ -441,22 +433,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu it_behaves_like 'report with expected warnings' end - - context 'and the report passes schema validation as a GitLab-vendored analyzer' do - let(:analyzer_vendor) do - { 'name' => 'GitLab' } - end - - it { is_expected.to be_empty } - end - - context 'and the report passes schema validation as a GitLab-vendored scanner' do - let(:scanner_vendor) do - { 'name' => 'GitLab' } - end - - it { is_expected.to be_empty } - end end context 'when given an unsupported schema version' do diff --git a/spec/lib/gitlab/database/load_balancing/logger_spec.rb b/spec/lib/gitlab/database/load_balancing/logger_spec.rb new file mode 100644 index 00000000000..81883fa6f1a --- /dev/null +++ b/spec/lib/gitlab/database/load_balancing/logger_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::LoadBalancing::Logger, feature_category: :database do + subject { described_class.new('/dev/null') } + + it_behaves_like 'a json logger', {} + + it 'excludes context' do + expect(described_class.exclude_context?).to be(true) + end +end diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index b39b273bba9..fa7645d581c 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do +RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns, feature_category: :database do before do stub_const('Testing', Module.new) stub_const('Testing::MyBase', Class.new(ActiveRecord::Base)) @@ -16,11 +16,10 @@ RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do Testing.module_eval do Testing::MyBase.class_eval do + include IgnorableColumns end SomeAbstract.class_eval do - include IgnorableColumns - self.abstract_class = true self.table_name = 'projects' @@ -29,8 +28,6 @@ RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do end Testing::B.class_eval do - include IgnorableColumns - self.table_name = 'issues' ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0' diff --git a/spec/lib/gitlab/database/pg_depend_spec.rb b/spec/lib/gitlab/database/pg_depend_spec.rb index f8b0e1af3a5..547a2c84b76 100644 --- a/spec/lib/gitlab/database/pg_depend_spec.rb +++ b/spec/lib/gitlab/database/pg_depend_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Gitlab::Database::PgDepend, type: :model, feature_category: :data connection.execute('CREATE EXTENSION IF NOT EXISTS pg_stat_statements;') end - it 'returns pg_stat_statements' do + it 'returns pg_stat_statements', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/410508' do expect(subject.pluck('relname')).to eq(['pg_stat_statements']) end end diff --git a/spec/lib/gitlab/database/reflection_spec.rb b/spec/lib/gitlab/database/reflection_spec.rb index 779bdbe50f0..641dd48be36 100644 --- a/spec/lib/gitlab/database/reflection_spec.rb +++ b/spec/lib/gitlab/database/reflection_spec.rb @@ -191,9 +191,15 @@ RSpec.describe Gitlab::Database::Reflection, feature_category: :database do expect(database.postgresql_minimum_supported_version?).to eq(false) end - it 'returns true when using PostgreSQL 12' do + it 'returns false when using PostgreSQL 12' do allow(database).to receive(:version).and_return('12') + expect(database.postgresql_minimum_supported_version?).to eq(false) + end + + it 'returns true when using PostgreSQL 13' do + allow(database).to receive(:version).and_return('13') + expect(database.postgresql_minimum_supported_version?).to eq(true) end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 1ba526a6eb3..0073d2ebe80 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -534,7 +534,7 @@ RSpec.describe Gitlab::GitalyClient, feature_category: :gitaly do context 'when RequestStore is enabled with empty git_env', :request_store do before do - ::Gitlab::Instrumentation::Storage[:gitlab_git_env] = {} + Gitlab::SafeRequestStore[:gitlab_git_env] = {} end it 'disables force-routing to primary' do @@ -544,7 +544,7 @@ RSpec.describe Gitlab::GitalyClient, feature_category: :gitaly do context 'when RequestStore is enabled with populated git_env', :request_store do before do - ::Gitlab::Instrumentation::Storage[:gitlab_git_env] = { + Gitlab::SafeRequestStore[:gitlab_git_env] = { "GIT_OBJECT_DIRECTORY_RELATIVE" => "foo/bar" } end diff --git a/spec/lib/gitlab/github_import/logger_spec.rb b/spec/lib/gitlab/github_import/logger_spec.rb index 6fd0f5db93e..97806872746 100644 --- a/spec/lib/gitlab/github_import/logger_spec.rb +++ b/spec/lib/gitlab/github_import/logger_spec.rb @@ -5,37 +5,5 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Logger do subject(:logger) { described_class.new('/dev/null') } - let(:now) { Time.zone.now } - - describe '#format_message' do - before do - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') - end - - it 'formats strings' do - output = subject.format_message('INFO', now, 'test', 'Hello world') - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'message' => 'Hello world', - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers', - 'import_type' => 'github' - }) - end - - it 'formats hashes' do - output = subject.format_message('INFO', now, 'test', { hello: 1 }) - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'hello' => 1, - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers', - 'import_type' => 'github' - }) - end - end + it_behaves_like 'a json logger', { 'feature_category' => 'importers', 'import_type' => 'github' } end diff --git a/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb b/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb index 1899697c78e..33f49dbc8d4 100644 --- a/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb +++ b/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::Graphql::CallsGitaly::FieldExtension, :request_store do context 'when the field calls gitaly' do before do owner.define_method :value do - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 1 + Gitlab::SafeRequestStore['gitaly_call_actual'] = 1 'fresh-from-the-gitaly-mines!' end end @@ -64,22 +64,22 @@ RSpec.describe Gitlab::Graphql::CallsGitaly::FieldExtension, :request_store do object = :anything arguments = :any_args - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 3 - ::Gitlab::Instrumentation::Storage['graphql_gitaly_accounted_for'] = 0 + ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 3 + ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 0 expect do |b| extension.resolve(object: object, arguments: arguments, &b) end.to yield_with_args(object, arguments, [3, 0]) - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 13 - ::Gitlab::Instrumentation::Storage['graphql_gitaly_accounted_for'] = 10 + ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 13 + ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 10 expect { extension.after_resolve(value: 'foo', memo: [3, 0]) }.not_to raise_error end it 'is unacceptable if some of the calls are unaccounted for' do - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 10 - ::Gitlab::Instrumentation::Storage['graphql_gitaly_accounted_for'] = 9 + ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 10 + ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 9 expect { extension.after_resolve(value: 'foo', memo: [0, 0]) }.to raise_error(include('Object.value')) end diff --git a/spec/lib/gitlab/import/logger_spec.rb b/spec/lib/gitlab/import/logger_spec.rb index 60978aaa25c..a85ba84108e 100644 --- a/spec/lib/gitlab/import/logger_spec.rb +++ b/spec/lib/gitlab/import/logger_spec.rb @@ -5,35 +5,5 @@ require 'spec_helper' RSpec.describe Gitlab::Import::Logger do subject { described_class.new('/dev/null') } - let(:now) { Time.zone.now } - - describe '#format_message' do - before do - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') - end - - it 'formats strings' do - output = subject.format_message('INFO', now, 'test', 'Hello world') - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'message' => 'Hello world', - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers' - }) - end - - it 'formats hashes' do - output = subject.format_message('INFO', now, 'test', { hello: 1 }) - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'hello' => 1, - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers' - }) - end - end + it_behaves_like 'a json logger', { 'feature_category' => 'importers' } end diff --git a/spec/lib/gitlab/instrumentation/storage_spec.rb b/spec/lib/gitlab/instrumentation/storage_spec.rb deleted file mode 100644 index afff8f6251b..00000000000 --- a/spec/lib/gitlab/instrumentation/storage_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::Gitlab::Instrumentation::Storage, :request_store, feature_category: :shared do - subject(:storage) { described_class } - - describe '.active?' do - context 'when SafeRequestStore is active' do - it 'returns true' do - allow(Gitlab::SafeRequestStore).to receive(:active?).and_return(true) - - expect(storage.active?).to be(true) - end - end - - context 'when SafeRequestStore is not active' do - it 'returns false' do - allow(Gitlab::SafeRequestStore).to receive(:active?).and_return(false) - - expect(storage.active?).to be(false) - end - end - end - - it 'stores data' do - storage[:a] = 1 - storage[:b] = 'hey' - - expect(storage[:a]).to eq(1) - expect(storage[:b]).to eq('hey') - end - - describe '.clear!' do - it 'removes all values' do - storage[:a] = 1 - storage[:b] = 'hey' - - storage.clear! - - expect(storage[:a]).to be_nil - expect(storage[:b]).to be_nil - end - end - - # This is testing implementation details, but until we have a truly segregated - # instrumentation data store, we need to make sure we do not "pollute" the - # underlying RequestStore or interfere with other co-located data. - describe 'backing storage' do - it 'stores data in the instrumentation bucket' do - storage[:a] = 1 - - expect(::RequestStore[:instrumentation]).to eq({ a: 1 }) - end - - describe '.clear!' do - it 'resets only the instrumentation bucket' do - storage[:a] = 1 - storage[:b] = 'hey' - ::RequestStore[:b] = 2 - - storage.clear! - - expect(::RequestStore[:instrumentation]).to eq({}) - expect(::RequestStore[:b]).to eq(2) - end - end - end -end diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb index b934f2261a4..8a88328e0c1 100644 --- a/spec/lib/gitlab/instrumentation_helper_spec.rb +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -8,14 +8,6 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac :use_null_store_as_repository_cache, feature_category: :scalability do using RSpec::Parameterized::TableSyntax - describe '.init_instrumentation_data' do - it 'clears instrumentation storage' do - expect(::Gitlab::Instrumentation::Storage).to receive(:clear!) - - described_class.init_instrumentation_data - end - end - describe '.add_instrumentation_data', :request_store do let(:payload) { {} } @@ -34,7 +26,7 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac context 'when Gitaly calls are made' do it 'adds Gitaly and Redis data' do project = create(:project) - ::Gitlab::Instrumentation::Storage.clear! + RequestStore.clear! project.repository.exists? subject @@ -189,8 +181,8 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac context 'when replica caught up search was made' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end it 'includes related metrics' do @@ -203,8 +195,8 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac context 'when only a single counter was updated' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 1 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil end it 'includes only that counter into logging' do diff --git a/spec/lib/gitlab/json_logger_spec.rb b/spec/lib/gitlab/json_logger_spec.rb index 801de357ddc..87df20c066b 100644 --- a/spec/lib/gitlab/json_logger_spec.rb +++ b/spec/lib/gitlab/json_logger_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::JsonLogger do subject { described_class.new('/dev/null') } - let(:now) { Time.now } + it_behaves_like 'a json logger', {} describe '#file_name' do let(:subclass) do @@ -26,31 +26,4 @@ RSpec.describe Gitlab::JsonLogger do expect(subclass.file_name).to eq('testlogger.log') end end - - describe '#format_message' do - before do - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') - end - - it 'formats strings' do - output = subject.format_message('INFO', now, 'test', 'Hello world') - data = Gitlab::Json.parse(output) - - expect(data['severity']).to eq('INFO') - expect(data['time']).to eq(now.utc.iso8601(3)) - expect(data['message']).to eq('Hello world') - expect(data['correlation_id']).to eq('new-correlation-id') - end - - it 'formats hashes' do - output = subject.format_message('INFO', now, 'test', { hello: 1 }) - data = Gitlab::Json.parse(output) - - expect(data['severity']).to eq('INFO') - expect(data['time']).to eq(now.utc.iso8601(3)) - expect(data['hello']).to eq(1) - expect(data['message']).to be_nil - expect(data['correlation_id']).to eq('new-correlation-id') - end - end end diff --git a/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb b/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb index cccfb5d8c99..18a5d2c2c3f 100644 --- a/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb @@ -67,7 +67,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu context 'when external HTTP detail store has some values' do before do Gitlab::SafeRequestStore[:peek_enabled] = true - ::Gitlab::Instrumentation::Storage[:external_http_detail_store] = [{ + Gitlab::SafeRequestStore[:external_http_detail_store] = [{ method: 'POST', code: "200", duration: 0.321 }] end @@ -87,8 +87,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu context 'when external HTTP recorded some values' do before do - ::Gitlab::Instrumentation::Storage[:external_http_count] = 7 - ::Gitlab::Instrumentation::Storage[:external_http_duration_s] = 1.2 + Gitlab::SafeRequestStore[:external_http_count] = 7 + Gitlab::SafeRequestStore[:external_http_duration_s] = 1.2 end it 'returns the external http detailed store' do @@ -133,7 +133,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu ) expect(described_class.top_slowest_requests).to eq(slow_requests) - expect(::Gitlab::Instrumentation::Storage[:external_http_slow_requests].length).to eq(3) + expect(Gitlab::SafeRequestStore[:external_http_slow_requests].length).to eq(3) end end end @@ -181,8 +181,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu subscriber.request(event_2) subscriber.request(event_3) - expect(::Gitlab::Instrumentation::Storage[:external_http_count]).to eq(3) - expect(::Gitlab::Instrumentation::Storage[:external_http_duration_s]).to eq(5.741) # 0.321 + 0.12 + 5.3 + expect(Gitlab::SafeRequestStore[:external_http_count]).to eq(3) + expect(Gitlab::SafeRequestStore[:external_http_duration_s]).to eq(5.741) # 0.321 + 0.12 + 5.3 end it 'stores a portion of events into the detail store' do @@ -190,22 +190,22 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu subscriber.request(event_2) subscriber.request(event_3) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store].length).to eq(3) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store][0]).to match a_hash_including( + expect(Gitlab::SafeRequestStore[:external_http_detail_store].length).to eq(3) + expect(Gitlab::SafeRequestStore[:external_http_detail_store][0]).to match a_hash_including( start: be_like_time(Time.current), method: 'POST', code: "200", duration: 0.321, scheme: 'https', host: 'gitlab.com', port: 443, path: '/api/v4/projects', query: 'current=true', exception_object: nil, backtrace: be_a(Array) ) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store][1]).to match a_hash_including( + expect(Gitlab::SafeRequestStore[:external_http_detail_store][1]).to match a_hash_including( start: be_like_time(Time.current), method: 'GET', code: "301", duration: 0.12, scheme: 'http', host: 'gitlab.com', port: 80, path: '/api/v4/projects/2', query: 'current=true', exception_object: nil, backtrace: be_a(Array) ) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store][2]).to match a_hash_including( + expect(Gitlab::SafeRequestStore[:external_http_detail_store][2]).to match a_hash_including( start: be_like_time(Time.current), method: 'POST', duration: 5.3, scheme: 'http', host: 'gitlab.com', port: 80, path: '/api/v4/projects/2/issues', @@ -225,7 +225,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu subscriber.request(event_2) subscriber.request(event_3) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store]).to be(nil) + expect(Gitlab::SafeRequestStore[:external_http_detail_store]).to be(nil) end end end diff --git a/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb b/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb index 1db8659943e..fb822c8d779 100644 --- a/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb @@ -66,7 +66,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_categ end describe ".payload" do - context "when instrumentation storage is empty" do + context "when SafeRequestStore is empty" do it "returns an empty array" do expect(described_class.payload).to eql(net_ldap_count: 0, net_ldap_duration_s: 0.0) end @@ -74,8 +74,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_categ context "when LDAP recorded some values" do before do - ::Gitlab::Instrumentation::Storage[:net_ldap_count] = 7 - ::Gitlab::Instrumentation::Storage[:net_ldap_duration_s] = 1.2 + Gitlab::SafeRequestStore[:net_ldap_count] = 7 + Gitlab::SafeRequestStore[:net_ldap_duration_s] = 1.2 end it "returns the populated payload" do @@ -117,8 +117,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_categ subscriber.observe_event(event_2) subscriber.observe_event(event_3) - expect(::Gitlab::Instrumentation::Storage[:net_ldap_count]).to eq(3) - expect(::Gitlab::Instrumentation::Storage[:net_ldap_duration_s]).to eq(0.005741) # (0.321 + 0.12 + 5.3) / 1000 + expect(Gitlab::SafeRequestStore[:net_ldap_count]).to eq(3) + expect(Gitlab::SafeRequestStore[:net_ldap_duration_s]).to eq(0.005741) # (0.321 + 0.12 + 5.3) / 1000 end end end diff --git a/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb b/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb index be04167a588..c2c3bb29b16 100644 --- a/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat it 'stores per-request caught up replica search result' do subject - expect(::Gitlab::Instrumentation::Storage[counter_name]).to eq(1) + expect(Gitlab::SafeRequestStore[counter_name]).to eq(1) end end @@ -50,7 +50,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when no data in request store' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick] = nil end it 'does not change the counters' do @@ -60,8 +60,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when request store was updated' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end it 'increments :caught_up_replica_pick count with proper label' do @@ -78,8 +78,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when no data in request store' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = nil - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil end it 'returns empty hash' do @@ -89,7 +89,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when request store was updated for a single counter' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 end it 'returns proper payload with only that counter' do @@ -99,8 +99,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when both counters were updated' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end it 'return proper payload' do diff --git a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb index a87b1ab4a71..13965bf1244 100644 --- a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do context 'when the request store already has data' do before do - ::Gitlab::Instrumentation::Storage[:rack_attack_instrumentation] = { + Gitlab::SafeRequestStore[:rack_attack_instrumentation] = { rack_attack_redis_count: 10, rack_attack_redis_duration_s: 9.0 } @@ -239,7 +239,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do it 'adds the matched name to safe request store' do subscriber.safelist(event) - expect(::Gitlab::Instrumentation::Storage[:instrumentation_throttle_safelist]).to eql('throttle_unauthenticated') + expect(Gitlab::SafeRequestStore[:instrumentation_throttle_safelist]).to eql('throttle_unauthenticated') end end end diff --git a/spec/lib/gitlab/middleware/request_context_spec.rb b/spec/lib/gitlab/middleware/request_context_spec.rb index 6d5b581feaa..cd21209bcee 100644 --- a/spec/lib/gitlab/middleware/request_context_spec.rb +++ b/spec/lib/gitlab/middleware/request_context_spec.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' require 'rack' require 'request_store' require_relative '../../../support/helpers/next_instance_of' -RSpec.describe Gitlab::Middleware::RequestContext do +RSpec.describe Gitlab::Middleware::RequestContext, feature_category: :application_instrumentation do include NextInstanceOf let(:app) { -> (env) {} } @@ -55,6 +55,10 @@ RSpec.describe Gitlab::Middleware::RequestContext do it 'sets the `request_start_time`' do expect { subject }.to change { instance.request_start_time }.from(nil).to(Float) end + + it 'sets the `spam_params`' do + expect { subject }.to change { instance.spam_params }.from(nil).to(::Spam::SpamParams) + end end end end diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index b9acfa4a841..44664be7d39 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::RequestContext, :request_store do +RSpec.describe Gitlab::RequestContext, :request_store, feature_category: :application_instrumentation do subject { described_class.instance } before do @@ -11,6 +11,44 @@ RSpec.describe Gitlab::RequestContext, :request_store do it { is_expected.to have_attributes(client_ip: nil, start_thread_cpu_time: nil, request_start_time: nil) } + describe '.start_request_context' do + let(:request) { ActionDispatch::Request.new({ 'REMOTE_ADDR' => '1.2.3.4' }) } + let(:start_request_context) { described_class.start_request_context(request: request) } + + before do + allow(Gitlab::Metrics::System).to receive(:real_time).and_return(123) + end + + it 'sets the client IP' do + expect { start_request_context }.to change { subject.client_ip }.from(nil).to('1.2.3.4') + end + + it 'sets the spam params' do + expect { start_request_context }.to change { subject.spam_params }.from(nil).to(::Spam::SpamParams) + end + + it 'sets the request start time' do + expect { start_request_context }.to change { subject.request_start_time }.from(nil).to(123) + end + end + + describe '.start_thread_context' do + let(:start_thread_context) { described_class.start_thread_context } + + before do + allow(Gitlab::Metrics::System).to receive(:thread_cpu_time).and_return(123) + allow(Gitlab::Memory::Instrumentation).to receive(:start_thread_memory_allocations).and_return(456) + end + + it 'sets the thread cpu time' do + expect { start_thread_context }.to change { subject.start_thread_cpu_time }.from(nil).to(123) + end + + it 'sets the thread memory allocations' do + expect { start_thread_context }.to change { subject.thread_memory_allocations }.from(nil).to(456) + end + end + describe '#request_deadline' do let(:request_start_time) { 1575982156.206008 } diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 459c926fa89..4b589dc43af 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -328,7 +328,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ApplicationRecord.connection.execute('SELECT pg_sleep(0.1);') end - ::Gitlab::Instrumentation::Storage.clear! + Gitlab::SafeRequestStore.clear! call_subject(job.dup, 'test_queue') {} end diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index 7f22dea8528..af9075f5aa0 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -18,8 +18,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do include ApplicationWorker def perform(*args) - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 1 - ::Gitlab::Instrumentation::Storage[:gitaly_query_time] = 5 + Gitlab::SafeRequestStore['gitaly_call_actual'] = 1 + Gitlab::SafeRequestStore[:gitaly_query_time] = 5 end end end diff --git a/spec/lib/sidebars/projects/menus/confluence_menu_spec.rb b/spec/lib/sidebars/projects/menus/confluence_menu_spec.rb index 836c6d26c6c..55c55b70a43 100644 --- a/spec/lib/sidebars/projects/menus/confluence_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/confluence_menu_spec.rb @@ -41,4 +41,13 @@ RSpec.describe Sidebars::Projects::Menus::ConfluenceMenu do end end end + + describe 'serialize_as_menu_item_args' do + it 'renders as part of the Plan section' do + expect(subject.serialize_as_menu_item_args).to include({ + item_id: :confluence, + super_sidebar_parent: ::Sidebars::Projects::SuperSidebarMenus::PlanMenu + }) + end + end end diff --git a/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb b/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb index b6672f2c820..93f0072a111 100644 --- a/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb +++ b/spec/lib/sidebars/projects/super_sidebar_panel_spec.rb @@ -24,6 +24,12 @@ RSpec.describe Sidebars::Projects::SuperSidebarPanel, feature_category: :navigat subject { described_class.new(context) } + before do + # Enable integrations with menu items + allow(project).to receive(:external_wiki).and_return(build(:external_wiki_integration, project: project)) + allow(project).to receive(:external_issue_tracker).and_return(build(:bugzilla_integration, project: project)) + end + it 'implements #super_sidebar_context_header' do expect(subject.super_sidebar_context_header).to eq( { diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 49b32c6f6b8..ac994735928 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -160,7 +160,9 @@ RSpec.describe Ci::Bridge, feature_category: :continuous_integration do where(:downstream_status, :upstream_status) do [ %w[success success], - *::Ci::Pipeline.completed_statuses.without(:success).map { |status| [status.to_s, 'failed'] } + %w[canceled canceled], + %w[failed failed], + %w[skipped failed] ] end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index d033d4adba7..38c45e8c975 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -565,6 +565,15 @@ RSpec.describe CommitStatus, feature_category: :continuous_integration do end end + describe '.with_type' do + let_it_be(:build_job) { create_status(name: 'build job', type: ::Ci::Build) } + let_it_be(:bridge_job) { create_status(name: 'bridge job', type: ::Ci::Bridge) } + + it 'returns statuses that match type' do + expect(described_class.with_type(::Ci::Build)).to contain_exactly(have_attributes(name: 'build job')) + end + end + describe '#before_sha' do subject { commit_status.before_sha } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index d62d7162497..87beba680d8 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -2009,10 +2009,9 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end end - describe '#deploy_freezes', :request_store do + describe '#deploy_freezes' do let(:environment) { create(:environment, project: project, name: 'staging') } let(:freeze_period) { create(:ci_freeze_period, project: project) } - let(:cache_key) { "project:#{project.id}:freeze_periods_for_environments" } subject { environment.deploy_freezes } @@ -2021,9 +2020,11 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching, feature_categ end it 'caches the freeze periods' do - expect { subject }.to( - change { Gitlab::SafeRequestStore[cache_key] }.from(nil).to([freeze_period]) - ) + expect(Gitlab::SafeRequestStore).to receive(:fetch) + .at_least(:once) + .and_return([freeze_period]) + + subject end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index d628812e081..3ff49938de5 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1733,14 +1733,6 @@ RSpec.describe Namespace, feature_category: :subgroups do end describe '#all_projects' do - context 'when recursive approach is disabled' do - before do - stub_feature_flags(recursive_approach_for_all_projects: false) - end - - include_examples '#all_projects' - end - context 'with use_traversal_ids feature flag enabled' do before do stub_feature_flags(use_traversal_ids: true) diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 91cb0a572a1..a36ea2115cf 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -237,6 +237,18 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end end + + context 'when runner registration is disallowed' do + before do + stub_application_setting(allow_runner_registration_token: false) + end + + it 'returns 410 Gone status' do + post api('/runners'), params: { token: registration_token } + + expect(response).to have_gitlab_http_status(:gone) + end + end end end end diff --git a/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb b/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb index 4dd47142c40..a3103b1af57 100644 --- a/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/graphql/metrics/dashboard/annotations_spec.rb @@ -50,6 +50,7 @@ RSpec.describe 'Getting Metrics Dashboard Annotations', feature_category: :metri end before do + stub_feature_flags(remove_monitor_metrics: false) project.add_developer(current_user) post_graphql(query, current_user: current_user) end diff --git a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb index 8db0844c6d7..b7d9b59f5fe 100644 --- a/spec/requests/api/graphql/metrics/dashboard_query_spec.rb +++ b/spec/requests/api/graphql/metrics/dashboard_query_spec.rb @@ -45,7 +45,10 @@ RSpec.describe 'Getting Metrics Dashboard', feature_category: :metrics do end context 'for user with developer access' do + let(:remove_monitor_metrics) { false } + before do + stub_feature_flags(remove_monitor_metrics: remove_monitor_metrics) project.add_developer(current_user) post_graphql(query, current_user: current_user) end @@ -82,6 +85,18 @@ RSpec.describe 'Getting Metrics Dashboard', feature_category: :metrics do expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: should be an array of panel_groups objects"]) end end + + context 'metrics dashboard feature is unavailable' do + let(:remove_monitor_metrics) { true } + + it_behaves_like 'a working graphql query' + + it 'returns nil' do + dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard') + + expect(dashboard).to be_nil + end + end end context 'requested dashboard can not be found' do diff --git a/spec/serializers/environment_entity_spec.rb b/spec/serializers/environment_entity_spec.rb index d5c3f64f52d..c60bead12c2 100644 --- a/spec/serializers/environment_entity_spec.rb +++ b/spec/serializers/environment_entity_spec.rb @@ -83,26 +83,36 @@ RSpec.describe EnvironmentEntity do end end - context 'metrics disabled' do + context 'when metrics dashboard feature is available' do before do - allow(environment).to receive(:has_metrics?).and_return(false) + stub_feature_flags(remove_monitor_metrics: false) end - it "doesn't expose metrics path" do - expect(subject).not_to include(:metrics_path) - end - end + context 'metrics disabled' do + before do + allow(environment).to receive(:has_metrics?).and_return(false) + end - context 'metrics enabled' do - before do - allow(environment).to receive(:has_metrics?).and_return(true) + it "doesn't expose metrics path" do + expect(subject).not_to include(:metrics_path) + end end - it 'exposes metrics path' do - expect(subject).to include(:metrics_path) + context 'metrics enabled' do + before do + allow(environment).to receive(:has_metrics?).and_return(true) + end + + it 'exposes metrics path' do + expect(subject).to include(:metrics_path) + end end end + it "doesn't expose metrics path" do + expect(subject).not_to include(:metrics_path) + end + context 'with deployment platform' do let(:project) { create(:project, :repository) } let(:environment) { create(:environment, project: project) } diff --git a/spec/services/ci/reset_skipped_jobs_service_spec.rb b/spec/services/ci/reset_skipped_jobs_service_spec.rb index 712a21e665b..ba6a4a4e822 100644 --- a/spec/services/ci/reset_skipped_jobs_service_spec.rb +++ b/spec/services/ci/reset_skipped_jobs_service_spec.rb @@ -6,13 +6,22 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : let_it_be(:project) { create(:project, :empty_repo) } let_it_be(:user) { project.first_owner } + let(:pipeline) do + Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload + end + + let(:a1) { find_job('a1') } + let(:a2) { find_job('a2') } + let(:b1) { find_job('b1') } + let(:input_processables) { a1 } # This is the input used when running service.execute() + before_all do project.repository.create_file(user, 'init', 'init', message: 'init', branch_name: 'master') end subject(:service) { described_class.new(project, user) } - context 'with a stage-dag mixed pipeline' do + shared_examples 'with a stage-dag mixed pipeline' do let(:config) do <<-YAML stages: [a, b, c] @@ -52,13 +61,6 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : YAML end - let(:pipeline) do - Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload - end - - let(:a1) { find_job('a1') } - let(:b1) { find_job('b1') } - before do stub_ci_pipeline_yaml_file(config) check_jobs_statuses( @@ -107,7 +109,7 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : end it 'marks subsequent skipped jobs as processable' do - execute_after_requeue_service(a1) + service.execute(input_processables) check_jobs_statuses( a1: 'pending', @@ -135,7 +137,7 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : { 'name' => 'c2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => [] } ) - execute_after_requeue_service(a1) + service.execute(input_processables) expect(jobs_name_status_owner_needs).to contain_exactly( { 'name' => 'a1', 'status' => 'pending', 'user_id' => user.id, 'needs' => [] }, @@ -150,7 +152,7 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : end end - context 'with stage-dag mixed pipeline with some same-stage needs' do + shared_examples 'with stage-dag mixed pipeline with some same-stage needs' do let(:config) do <<-YAML stages: [a, b, c] @@ -184,12 +186,6 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : YAML end - let(:pipeline) do - Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload - end - - let(:a1) { find_job('a1') } - before do stub_ci_pipeline_yaml_file(config) check_jobs_statuses( @@ -224,7 +220,7 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : end it 'marks subsequent skipped jobs as processable' do - execute_after_requeue_service(a1) + service.execute(input_processables) check_jobs_statuses( a1: 'pending', @@ -237,61 +233,465 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : end end - context 'with same-stage needs' do + shared_examples 'with same-stage needs' do let(:config) do <<-YAML - a: + a1: script: exit $(($RANDOM % 2)) - b: + b1: script: exit 0 - needs: [a] + needs: [a1] - c: + c1: script: exit 0 - needs: [b] + needs: [b1] YAML end - let(:pipeline) do - Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload + before do + stub_ci_pipeline_yaml_file(config) + check_jobs_statuses( + a1: 'pending', + b1: 'created', + c1: 'created' + ) + + a1.drop! + check_jobs_statuses( + a1: 'failed', + b1: 'skipped', + c1: 'skipped' + ) + + new_a1 = Ci::RetryJobService.new(project, user).clone!(a1) + new_a1.enqueue! + check_jobs_statuses( + a1: 'pending', + b1: 'skipped', + c1: 'skipped' + ) end - let(:a) { find_job('a') } + it 'marks subsequent skipped jobs as processable' do + service.execute(input_processables) + + check_jobs_statuses( + a1: 'pending', + b1: 'created', + c1: 'created' + ) + end + end + + context 'with same-stage needs where the parent jobs do not share the same descendants' do + let(:config) do + <<-YAML + a1: + script: exit $(($RANDOM % 2)) + + a2: + script: exit $(($RANDOM % 2)) + + b1: + script: exit 0 + needs: [a1] + + b2: + script: exit 0 + needs: [a2] + + c1: + script: exit 0 + needs: [b1] + + c2: + script: exit 0 + needs: [b2] + YAML + end before do stub_ci_pipeline_yaml_file(config) check_jobs_statuses( - a: 'pending', - b: 'created', - c: 'created' + a1: 'pending', + a2: 'pending', + b1: 'created', + b2: 'created', + c1: 'created', + c2: 'created' ) - a.drop! + a1.drop! + a2.drop! + check_jobs_statuses( - a: 'failed', - b: 'skipped', - c: 'skipped' + a1: 'failed', + a2: 'failed', + b1: 'skipped', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' + ) + + new_a1 = Ci::RetryJobService.new(project, user).clone!(a1) + new_a1.enqueue! + + check_jobs_statuses( + a1: 'pending', + a2: 'failed', + b1: 'skipped', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' ) - new_a = Ci::RetryJobService.new(project, user).clone!(a) - new_a.enqueue! + new_a2 = Ci::RetryJobService.new(project, user).clone!(a2) + new_a2.enqueue! + check_jobs_statuses( - a: 'pending', - b: 'skipped', - c: 'skipped' + a1: 'pending', + a2: 'pending', + b1: 'skipped', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' ) end + # This demonstrates that when only a1 is inputted, only the *1 subsequent jobs are reset. + # This is in contrast to the following example when both a1 and a2 are inputted. it 'marks subsequent skipped jobs as processable' do - execute_after_requeue_service(a) + service.execute(input_processables) check_jobs_statuses( - a: 'pending', - b: 'created', - c: 'created' + a1: 'pending', + a2: 'pending', + b1: 'created', + b2: 'skipped', + c1: 'created', + c2: 'skipped' ) end + + context 'when multiple processables are inputted' do + # When both a1 and a2 are inputted, all subsequent jobs are reset. + it 'marks subsequent skipped jobs as processable' do + input_processables = [a1, a2] + service.execute(input_processables) + + check_jobs_statuses( + a1: 'pending', + a2: 'pending', + b1: 'created', + b2: 'created', + c1: 'created', + c2: 'created' + ) + end + end + end + + context 'when a single processable is inputted' do + it_behaves_like 'with a stage-dag mixed pipeline' + it_behaves_like 'with stage-dag mixed pipeline with some same-stage needs' + it_behaves_like 'with same-stage needs' + end + + context 'when multiple processables are inputted' do + let(:input_processables) { [a1, b1] } + + it_behaves_like 'with a stage-dag mixed pipeline' + it_behaves_like 'with stage-dag mixed pipeline with some same-stage needs' + it_behaves_like 'with same-stage needs' + end + + context 'when FF is `ci_support_reset_skipped_jobs_for_multiple_jobs` disabled' do + before do + stub_feature_flags(ci_support_reset_skipped_jobs_for_multiple_jobs: false) + end + + context 'with a stage-dag mixed pipeline' do + let(:config) do + <<-YAML + stages: [a, b, c] + + a1: + stage: a + script: exit $(($RANDOM % 2)) + + a2: + stage: a + script: exit 0 + needs: [a1] + + a3: + stage: a + script: exit 0 + needs: [a2] + + b1: + stage: b + script: exit 0 + needs: [] + + b2: + stage: b + script: exit 0 + needs: [a2] + + c1: + stage: c + script: exit 0 + needs: [b2] + + c2: + stage: c + script: exit 0 + YAML + end + + let(:pipeline) do + Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload + end + + let(:a1) { find_job('a1') } + let(:b1) { find_job('b1') } + + before do + stub_ci_pipeline_yaml_file(config) + check_jobs_statuses( + a1: 'pending', + a2: 'created', + a3: 'created', + b1: 'pending', + b2: 'created', + c1: 'created', + c2: 'created' + ) + + b1.success! + check_jobs_statuses( + a1: 'pending', + a2: 'created', + a3: 'created', + b1: 'success', + b2: 'created', + c1: 'created', + c2: 'created' + ) + + a1.drop! + check_jobs_statuses( + a1: 'failed', + a2: 'skipped', + a3: 'skipped', + b1: 'success', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' + ) + + new_a1 = Ci::RetryJobService.new(project, user).clone!(a1) + new_a1.enqueue! + check_jobs_statuses( + a1: 'pending', + a2: 'skipped', + a3: 'skipped', + b1: 'success', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' + ) + end + + it 'marks subsequent skipped jobs as processable' do + execute_after_requeue_service(a1) + + check_jobs_statuses( + a1: 'pending', + a2: 'created', + a3: 'created', + b1: 'success', + b2: 'created', + c1: 'created', + c2: 'created' + ) + end + + context 'when executed by a different user than the original owner' do + let(:retryer) { create(:user).tap { |u| project.add_maintainer(u) } } + let(:service) { described_class.new(project, retryer) } + + it 'reassigns jobs with updated statuses to the retryer' do + expect(jobs_name_status_owner_needs).to contain_exactly( + { 'name' => 'a1', 'status' => 'pending', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'a2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a1'] }, + { 'name' => 'a3', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a2'] }, + { 'name' => 'b1', 'status' => 'success', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'b2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['a2'] }, + { 'name' => 'c1', 'status' => 'skipped', 'user_id' => user.id, 'needs' => ['b2'] }, + { 'name' => 'c2', 'status' => 'skipped', 'user_id' => user.id, 'needs' => [] } + ) + + execute_after_requeue_service(a1) + + expect(jobs_name_status_owner_needs).to contain_exactly( + { 'name' => 'a1', 'status' => 'pending', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'a2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a1'] }, + { 'name' => 'a3', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a2'] }, + { 'name' => 'b1', 'status' => 'success', 'user_id' => user.id, 'needs' => [] }, + { 'name' => 'b2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['a2'] }, + { 'name' => 'c1', 'status' => 'created', 'user_id' => retryer.id, 'needs' => ['b2'] }, + { 'name' => 'c2', 'status' => 'created', 'user_id' => retryer.id, 'needs' => [] } + ) + end + end + end + + context 'with stage-dag mixed pipeline with some same-stage needs' do + let(:config) do + <<-YAML + stages: [a, b, c] + + a1: + stage: a + script: exit $(($RANDOM % 2)) + + a2: + stage: a + script: exit 0 + needs: [a1] + + b1: + stage: b + script: exit 0 + needs: [b2] + + b2: + stage: b + script: exit 0 + + c1: + stage: c + script: exit 0 + needs: [b2] + + c2: + stage: c + script: exit 0 + YAML + end + + let(:pipeline) do + Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload + end + + let(:a1) { find_job('a1') } + + before do + stub_ci_pipeline_yaml_file(config) + check_jobs_statuses( + a1: 'pending', + a2: 'created', + b1: 'created', + b2: 'created', + c1: 'created', + c2: 'created' + ) + + a1.drop! + check_jobs_statuses( + a1: 'failed', + a2: 'skipped', + b1: 'skipped', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' + ) + + new_a1 = Ci::RetryJobService.new(project, user).clone!(a1) + new_a1.enqueue! + check_jobs_statuses( + a1: 'pending', + a2: 'skipped', + b1: 'skipped', + b2: 'skipped', + c1: 'skipped', + c2: 'skipped' + ) + end + + it 'marks subsequent skipped jobs as processable' do + execute_after_requeue_service(a1) + + check_jobs_statuses( + a1: 'pending', + a2: 'created', + b1: 'created', + b2: 'created', + c1: 'created', + c2: 'created' + ) + end + end + + context 'with same-stage needs' do + let(:config) do + <<-YAML + a: + script: exit $(($RANDOM % 2)) + + b: + script: exit 0 + needs: [a] + + c: + script: exit 0 + needs: [b] + YAML + end + + let(:pipeline) do + Ci::CreatePipelineService.new(project, user, { ref: 'master' }).execute(:push).payload + end + + let(:a) { find_job('a') } + + before do + stub_ci_pipeline_yaml_file(config) + check_jobs_statuses( + a: 'pending', + b: 'created', + c: 'created' + ) + + a.drop! + check_jobs_statuses( + a: 'failed', + b: 'skipped', + c: 'skipped' + ) + + new_a = Ci::RetryJobService.new(project, user).clone!(a) + new_a.enqueue! + check_jobs_statuses( + a: 'pending', + b: 'skipped', + c: 'skipped' + ) + end + + it 'marks subsequent skipped jobs as processable' do + execute_after_requeue_service(a) + + check_jobs_statuses( + a: 'pending', + b: 'created', + c: 'created' + ) + end + end end private @@ -314,6 +714,7 @@ RSpec.describe Ci::ResetSkippedJobsService, :sidekiq_inline, feature_category: : end end + # Remove this method when FF is `ci_support_reset_skipped_jobs_for_multiple_jobs` is removed def execute_after_requeue_service(processable) service.execute(processable) end diff --git a/spec/services/ci/runners/register_runner_service_spec.rb b/spec/services/ci/runners/register_runner_service_spec.rb index 53e313f8256..b5921773364 100644 --- a/spec/services/ci/runners/register_runner_service_spec.rb +++ b/spec/services/ci/runners/register_runner_service_spec.rb @@ -18,10 +18,10 @@ RSpec.describe ::Ci::Runners::RegisterRunnerService, '#execute', feature_categor subject(:execute) { described_class.new(token, args).execute } shared_examples 'runner registration is disallowed' do - it 'returns error response' do + it 'returns error response with runner_registration_disallowed reason' do expect(execute).to be_error expect(execute.message).to eq 'runner registration disallowed' - expect(execute.http_status).to eq :forbidden + expect(execute.reason).to eq :runner_registration_disallowed end end diff --git a/spec/support/fast_quarantine.rb b/spec/support/fast_quarantine.rb index 18d4df887a3..b5ed1a2aa96 100644 --- a/spec/support/fast_quarantine.rb +++ b/spec/support/fast_quarantine.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +return unless ENV['CI'] return if ENV['FAST_QUARANTINE'] == "false" return if ENV['CI_MERGE_REQUEST_LABELS'].to_s.include?('pipeline:run-flaky-tests') diff --git a/spec/support/helpers/content_editor_helpers.rb b/spec/support/helpers/content_editor_helpers.rb index f19af0c9af8..83c18f8073f 100644 --- a/spec/support/helpers/content_editor_helpers.rb +++ b/spec/support/helpers/content_editor_helpers.rb @@ -17,14 +17,6 @@ module ContentEditorHelpers find('.js-gfm-input').set content end - def expect_formatting_menu_to_be_visible - expect(page).to have_css('[data-testid="formatting-bubble-menu"]') - end - - def expect_formatting_menu_to_be_hidden - expect(page).not_to have_css('[data-testid="formatting-bubble-menu"]') - end - def expect_media_bubble_menu_to_be_visible expect(page).to have_css('[data-testid="media-bubble-menu"]') end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 724f45524d1..36118361546 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -6151,7 +6151,6 @@ - './spec/lib/gitlab/database/migrations/test_background_runner_spec.rb' - './spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb' - './spec/lib/gitlab/database/no_cross_db_foreign_keys_spec.rb' -- './spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb' - './spec/lib/gitlab/database/partitioning/detached_partition_dropper_spec.rb' - './spec/lib/gitlab/database/partitioning_migration_helpers/backfill_partitioned_table_spec.rb' - './spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb' diff --git a/spec/support/shared_examples/features/content_editor_shared_examples.rb b/spec/support/shared_examples/features/content_editor_shared_examples.rb index 7bcc82590ee..41114197ff5 100644 --- a/spec/support/shared_examples/features/content_editor_shared_examples.rb +++ b/spec/support/shared_examples/features/content_editor_shared_examples.rb @@ -27,27 +27,6 @@ RSpec.shared_examples 'edits content using the content editor' do expect(page).to have_text('Typing text in the content editor') end - describe 'formatting bubble menu' do - it 'shows a formatting bubble menu for a regular paragraph and headings' do - switch_to_content_editor - - expect(page).to have_css(content_editor_testid) - - type_in_content_editor 'Typing text in the content editor' - type_in_content_editor [:shift, :left] - - expect_formatting_menu_to_be_visible - - type_in_content_editor [:right, :right, :enter, '## Heading'] - - expect_formatting_menu_to_be_hidden - - type_in_content_editor [:shift, :left] - - expect_formatting_menu_to_be_visible - end - end - describe 'creating and editing links' do before do switch_to_content_editor @@ -319,14 +298,12 @@ RSpec.shared_examples 'edits content using the content editor' do it 'displays correct media bubble menu for images', :js do display_media_bubble_menu '[data-testid="content_editor_editablebox"] img[src]', 'dk.png' - expect_formatting_menu_to_be_hidden expect_media_bubble_menu_to_be_visible end it 'displays correct media bubble menu for video', :js do display_media_bubble_menu '[data-testid="content_editor_editablebox"] video', 'video_sample.mp4' - expect_formatting_menu_to_be_hidden expect_media_bubble_menu_to_be_visible end end @@ -367,7 +344,6 @@ RSpec.shared_examples 'edits content using the content editor' do type_in_content_editor 'var a = 0' type_in_content_editor [:shift, :left] - expect_formatting_menu_to_be_hidden expect(page).to have_css('[data-testid="code-block-bubble-menu"]') end @@ -618,7 +594,6 @@ RSpec.shared_examples 'inserts diagrams.net diagram using the content editor' do it 'displays correct media bubble menu with edit diagram button' do display_media_bubble_menu '[data-testid="content_editor_editablebox"] img[src]', 'diagram.drawio.svg' - expect_formatting_menu_to_be_hidden expect_media_bubble_menu_to_be_visible click_edit_diagram_button 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 6d4d0a5dd0a..526a56e7dab 100644 --- a/spec/support/shared_examples/features/work_items_shared_examples.rb +++ b/spec/support/shared_examples/features/work_items_shared_examples.rb @@ -307,3 +307,82 @@ RSpec.shared_examples 'work items notifications' do end end end + +RSpec.shared_examples 'work items todos' do + let(:todos_action_selector) { '[data-testid="work-item-todos-action"]' } + let(:todos_icon_selector) { '[data-testid="work-item-todos-icon"]' } + let(:header_section_selector) { '[data-testid="work-item-body"]' } + + def toggle_todo_action + find(todos_action_selector).click + wait_for_requests + end + + it 'adds item to the list' do + page.within(header_section_selector) do + expect(find(todos_action_selector)['aria-label']).to eq('Add a to do') + + toggle_todo_action + + expect(find(todos_action_selector)['aria-label']).to eq('Mark as done') + end + + page.within ".header-content span[aria-label='#{_('Todos count')}']" do + expect(page).to have_content '1' + end + end + + it 'marks a todo as done' do + page.within(header_section_selector) do + toggle_todo_action + toggle_todo_action + end + + expect(find(todos_action_selector)['aria-label']).to eq('Add a to do') + expect(page).to have_selector(".header-content span[aria-label='#{_('Todos count')}']", visible: :hidden) + end +end + +RSpec.shared_examples 'work items award emoji' do + let(:award_section_selector) { '[data-testid="work-item-award-list"]' } + let(:award_action_selector) { '[data-testid="award-button"]' } + let(:selected_award_action_selector) { '[data-testid="award-button"].selected' } + let(:emoji_picker_action_selector) { '[data-testid="emoji-picker"]' } + let(:basketball_emoji_selector) { 'gl-emoji[data-name="basketball"]' } + + def select_emoji + first(award_action_selector).click + + wait_for_requests + end + + it 'adds award to the work item' do + within(award_section_selector) do + select_emoji + + expect(page).to have_selector(selected_award_action_selector) + expect(first(award_action_selector)).to have_content '1' + end + end + + it 'removes award from work item' do + within(award_section_selector) do + select_emoji + + expect(first(award_action_selector)).to have_content '1' + + select_emoji + + expect(first(award_action_selector)).to have_content '0' + end + end + + it 'add custom award to the work item' do + within(award_section_selector) do + find(emoji_picker_action_selector).click + find(basketball_emoji_selector).click + + expect(page).to have_selector(basketball_emoji_selector) + end + end +end diff --git a/spec/support/shared_examples/lib/gitlab/json_logger_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/json_logger_shared_examples.rb new file mode 100644 index 00000000000..8a5e8397c3d --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/json_logger_shared_examples.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a json logger' do |extra_params| + let(:now) { Time.now } + let(:correlation_id) { Labkit::Correlation::CorrelationId.current_id } + + it 'formats strings' do + output = subject.format_message('INFO', now, 'test', 'Hello world') + data = Gitlab::Json.parse(output) + + expect(data['severity']).to eq('INFO') + expect(data['time']).to eq(now.utc.iso8601(3)) + expect(data['message']).to eq('Hello world') + expect(data['correlation_id']).to eq(correlation_id) + expect(data).to include(extra_params) + end + + it 'formats hashes' do + output = subject.format_message('INFO', now, 'test', { hello: 1 }) + data = Gitlab::Json.parse(output) + + expect(data['severity']).to eq('INFO') + expect(data['time']).to eq(now.utc.iso8601(3)) + expect(data['hello']).to eq(1) + expect(data['message']).to be_nil + expect(data['correlation_id']).to eq(correlation_id) + expect(data).to include(extra_params) + end +end diff --git a/spec/workers/namespaces/process_sync_events_worker_spec.rb b/spec/workers/namespaces/process_sync_events_worker_spec.rb index c11cd32cfc7..a5afb5d5cbf 100644 --- a/spec/workers/namespaces/process_sync_events_worker_spec.rb +++ b/spec/workers/namespaces/process_sync_events_worker_spec.rb @@ -20,8 +20,8 @@ RSpec.describe Namespaces::ProcessSyncEventsWorker, feature_category: :cell do expect(described_class.get_deduplicate_strategy).to eq(:until_executed) end - it 'has an option to reschedule once if deduplicated' do - expect(described_class.get_deduplication_options).to include({ if_deduplicated: :reschedule_once }) + it 'has the option to reschedule once if deduplicated and a TTL of 1 minute' do + expect(described_class.get_deduplication_options).to include({ if_deduplicated: :reschedule_once, ttl: 1.minute }) end it 'expect the job to enqueue itself again if there was more items to be processed', :sidekiq_inline do diff --git a/spec/workers/projects/process_sync_events_worker_spec.rb b/spec/workers/projects/process_sync_events_worker_spec.rb index fe53b6d6d8c..7047d8e8653 100644 --- a/spec/workers/projects/process_sync_events_worker_spec.rb +++ b/spec/workers/projects/process_sync_events_worker_spec.rb @@ -14,8 +14,8 @@ RSpec.describe Projects::ProcessSyncEventsWorker, feature_category: :cell do expect(described_class.get_deduplicate_strategy).to eq(:until_executed) end - it 'has an option to reschedule once if deduplicated' do - expect(described_class.get_deduplication_options).to include({ if_deduplicated: :reschedule_once }) + it 'has the option to reschedule once if deduplicated and a TTL of 1 minute' do + expect(described_class.get_deduplication_options).to include({ if_deduplicated: :reschedule_once, ttl: 1.minute }) end describe '#perform' do diff --git a/workhorse/internal/headers/content_headers.go b/workhorse/internal/headers/content_headers.go index 54c7c1bdd95..3dd8216eec5 100644 --- a/workhorse/internal/headers/content_headers.go +++ b/workhorse/internal/headers/content_headers.go @@ -53,13 +53,15 @@ func SafeContentHeaders(data []byte, contentDisposition string) (string, string) // Some browsers will render XML inline unless a filename directive is provided with a non-xml file extension // This overrides the filename directive in the case of XML data - for _, element := range htmlRenderingTypes { - if isType(detectedContentType, element) { - disposition, directives, err := mime.ParseMediaType(contentDisposition) - if err == nil { - directives["filename"] = dummyFilename - contentDisposition = mime.FormatMediaType(disposition, directives) - break + if !attachmentRegex.MatchString(contentDisposition) { + for _, element := range htmlRenderingTypes { + if isType(detectedContentType, element) { + disposition, directives, err := mime.ParseMediaType(contentDisposition) + if err == nil { + directives["filename"] = dummyFilename + contentDisposition = mime.FormatMediaType(disposition, directives) + break + } } } } diff --git a/workhorse/internal/headers/content_headers_test.go b/workhorse/internal/headers/content_headers_test.go index 7cfce335d88..bd329dbc199 100644 --- a/workhorse/internal/headers/content_headers_test.go +++ b/workhorse/internal/headers/content_headers_test.go @@ -13,33 +13,63 @@ func fileContents(fileName string) []byte { } func TestHeaders(t *testing.T) { + xmlFileContents := fileContents("../../testdata/test.xml") + svgFileContents := fileContents("../../testdata/xml.svg") + xhtmlFileContents := fileContents("../../testdata/index.xhtml") + tests := []struct { desc string fileContents []byte + contentDisposition string expectedContentType string expectedContentDisposition string }{ { - desc: "XML file", - fileContents: fileContents("../../testdata/test.xml"), + desc: "inline XML file", + fileContents: xmlFileContents, + contentDisposition: "inline; filename=test.xml", expectedContentType: "text/plain; charset=utf-8", expectedContentDisposition: "inline; filename=blob", }, { - desc: "XHTML file", - fileContents: fileContents("../../testdata/index.xhtml"), + desc: "attachment XML file", + fileContents: xmlFileContents, + contentDisposition: "attachment; filename=test.xml", + expectedContentType: "application/octet-stream", + expectedContentDisposition: "attachment; filename=test.xml", + }, + { + desc: "inline XHTML file", + fileContents: xhtmlFileContents, + contentDisposition: "inline; filename=index.xhtml", expectedContentType: "text/plain; charset=utf-8", expectedContentDisposition: "inline; filename=blob", }, { + desc: "attachment XHTML file", + fileContents: xhtmlFileContents, + contentDisposition: "attachment; filename=index.xhtml", + expectedContentType: "application/octet-stream", + expectedContentDisposition: "attachment; filename=index.xhtml", + }, + { desc: "svg+xml file", - fileContents: fileContents("../../testdata/xml.svg"), + fileContents: svgFileContents, + contentDisposition: "", expectedContentType: "image/svg+xml", expectedContentDisposition: "attachment", }, { + desc: "svg+xml file", + fileContents: svgFileContents, + contentDisposition: "inline; filename=xml.svg", + expectedContentType: "image/svg+xml", + expectedContentDisposition: "attachment; filename=xml.svg", + }, + { desc: "text file", fileContents: []byte(`a text file`), + contentDisposition: "", expectedContentType: "text/plain; charset=utf-8", expectedContentDisposition: "inline", }, @@ -47,7 +77,7 @@ func TestHeaders(t *testing.T) { for _, test := range tests { t.Run(test.desc, func(t *testing.T) { - contentType, newContentDisposition := SafeContentHeaders(test.fileContents, "") + contentType, newContentDisposition := SafeContentHeaders(test.fileContents, test.contentDisposition) require.Equal(t, test.expectedContentType, contentType) require.Equal(t, test.expectedContentDisposition, newContentDisposition) |
