diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-21 03:08:59 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-01-21 03:08:59 +0300 |
commit | 367847e266036617e540e41b7fd3c7d03033800c (patch) | |
tree | a14547ad7556d48dcdeb977021f8cd89305ea026 | |
parent | 5d7e5a8902382caaffa616e1b496b684ba72d148 (diff) |
Add latest changes from gitlab-org/gitlab@master
37 files changed, 1264 insertions, 201 deletions
diff --git a/.haml-lint.yml b/.haml-lint.yml index 3655ca44172..42b1c2b6987 100644 --- a/.haml-lint.yml +++ b/.haml-lint.yml @@ -109,6 +109,7 @@ linters: # These cops should eventually get enabled - Cop/LineBreakAfterGuardClauses - Cop/ProjectPathHelper + - Gitlab/DocUrl - Gitlab/FeatureAvailableUsage - Gitlab/Json - GitlabSecurity/PublicSend diff --git a/.rubocop.yml b/.rubocop.yml index e58dd4f9fd0..066c21e949a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -386,6 +386,18 @@ Gitlab/EventStoreSubscriber: - 'spec/**/*' - 'ee/spec/**/*' +Gitlab/DocUrl: + Enabled: true + Exclude: + - danger/**/* + - ee/spec/**/* + - haml_lint/**/* + - qa/**/* + - rubocop/**/* + - scripts/**/* + - spec/**/* + - tooling/**/* + GitlabSecurity/PublicSend: Enabled: true Exclude: diff --git a/.rubocop_todo/gitlab/doc_url.yml b/.rubocop_todo/gitlab/doc_url.yml new file mode 100644 index 00000000000..bbb477d791c --- /dev/null +++ b/.rubocop_todo/gitlab/doc_url.yml @@ -0,0 +1,56 @@ +--- +Gitlab/DocUrl: + Details: grace period + Exclude: + - 'app/controllers/jira_connect/app_descriptor_controller.rb' + - 'app/graphql/types/concerns/gitlab_style_deprecations.rb' + - 'app/graphql/types/notes/diff_position_input_type.rb' + - 'app/graphql/types/query_complexity_type.rb' + - 'app/helpers/learn_gitlab_helper.rb' + - 'app/models/integrations/apple_app_store.rb' + - 'app/models/integrations/microsoft_teams.rb' + - 'app/presenters/dev_ops_report/metric_presenter.rb' + - 'app/serializers/build_details_entity.rb' + - 'app/services/security/ci_configuration/container_scanning_create_service.rb' + - 'app/services/security/ci_configuration/dependency_scanning_create_service.rb' + - 'app/services/security/ci_configuration/sast_create_service.rb' + - 'app/services/security/ci_configuration/sast_iac_create_service.rb' + - 'app/services/security/ci_configuration/secret_detection_create_service.rb' + - 'app/services/tasks_to_be_done/create_ci_task_service.rb' + - 'app/services/tasks_to_be_done/create_code_task_service.rb' + - 'app/services/tasks_to_be_done/create_issues_task_service.rb' + - 'ee/app/components/namespaces/free_user_cap/notification_alert_component.rb' + - 'ee/app/graphql/mutations/requirements_management/export_requirements.rb' + - 'ee/app/graphql/types/vulnerability_state_enum.rb' + - 'ee/app/mailers/emails/user_cap.rb' + - 'ee/app/workers/concerns/elastic/migration_obsolete.rb' + - 'ee/lib/ee/gitlab/ci/pipeline/quota/size.rb' + - 'ee/lib/slack/block_kit/app_home_opened.rb' + - 'ee/lib/system_check/app/advanced_search_migrations_check.rb' + - 'ee/lib/tasks/gitlab/geo.rake' + - 'lib/backup/database.rb' + - 'lib/feature.rb' + - 'lib/gitlab/audit/auditor.rb' + - 'lib/gitlab/ci/config/entry/processable.rb' + - 'lib/gitlab/config_checker/external_database_checker.rb' + - 'lib/gitlab/config_checker/puma_rugged_checker.rb' + - 'lib/gitlab/database.rb' + - 'lib/gitlab/database/migration_helpers/automatic_lock_writes_on_tables.rb' + - 'lib/gitlab/database/migration_helpers/v2.rb' + - 'lib/gitlab/database/migrations/batched_background_migration_helpers.rb' + - 'lib/gitlab/database/migrations/extension_helpers.rb' + - 'lib/gitlab/database/query_analyzers/prevent_cross_database_modification.rb' + - 'lib/gitlab/database/query_analyzers/restrict_allowed_schemas.rb' + - 'lib/gitlab/i18n/po_linter.rb' + - 'lib/gitlab/instrumentation/redis_base.rb' + - 'lib/gitlab/pagination/keyset/unsupported_scope_order.rb' + - 'lib/gitlab/redis/hll.rb' + - 'lib/gitlab/slash_commands/presenters/help.rb' + - 'lib/gitlab/utils/strong_memoize.rb' + - 'lib/initializer_connections.rb' + - 'lib/security/ci_configuration/base_build_action.rb' + - 'lib/tasks/db_obsolete_ignored_columns.rake' + - 'lib/tasks/gitlab/docs/redirect.rake' + - 'lib/tasks/gitlab/shell.rake' + - 'lib/tasks/migrate/schema_check.rake' + - 'lib/tasks/test.rake' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 93540afff72..850103fba65 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -96f32d09ba110880d767a413ad302e2f9f1ffd17 +d7b9dcec61e29644ffc58dd2a59756435bf58bcb diff --git a/app/assets/javascripts/api/environments_api.js b/app/assets/javascripts/api/environments_api.js new file mode 100644 index 00000000000..9912b1ab696 --- /dev/null +++ b/app/assets/javascripts/api/environments_api.js @@ -0,0 +1,15 @@ +import axios from '../lib/utils/axios_utils'; +import { buildApiUrl } from './api_utils'; + +export const STOP_STALE_ENVIRONMENTS_PATH = '/api/:version/projects/:id/environments/stop_stale'; + +export function stopStaleEnvironments(projectId, before, query, options) { + const url = buildApiUrl(STOP_STALE_ENVIRONMENTS_PATH).replace(':id', projectId); + const defaults = { + before: before.toISOString(), + }; + + return axios.post(url, null, { + params: Object.assign(defaults, options), + }); +} diff --git a/app/assets/javascripts/environments/components/environments_app.vue b/app/assets/javascripts/environments/components/environments_app.vue index 55e6a891e27..b2a69cdb6c6 100644 --- a/app/assets/javascripts/environments/components/environments_app.vue +++ b/app/assets/javascripts/environments/components/environments_app.vue @@ -15,6 +15,7 @@ import { ENVIRONMENTS_SCOPE } from '../constants'; import EnvironmentFolder from './environment_folder.vue'; import EnableReviewAppModal from './enable_review_app_modal.vue'; import StopEnvironmentModal from './stop_environment_modal.vue'; +import StopStaleEnvironmentsModal from './stop_stale_environments_modal.vue'; import EnvironmentItem from './new_environment_item.vue'; import ConfirmRollbackModal from './confirm_rollback_modal.vue'; import DeleteEnvironmentModal from './delete_environment_modal.vue'; @@ -31,6 +32,7 @@ export default { EnableReviewAppModal, EnvironmentItem, StopEnvironmentModal, + StopStaleEnvironmentsModal, GlBadge, GlPagination, GlSearchBoxByType, @@ -75,6 +77,7 @@ export default { i18n: { newEnvironmentButtonLabel: s__('Environments|New environment'), reviewAppButtonLabel: s__('Environments|Enable review app'), + cleanUpEnvsButtonLabel: s__('Environments|Clean up environments'), available: __('Available'), stopped: __('Stopped'), prevPage: __('Go to previous page'), @@ -85,11 +88,13 @@ export default { searchPlaceholder: s__('Environments|Search by environment name'), }, modalId: 'enable-review-app-info', + stopStaleEnvsModalId: 'stop-stale-environments-modal', data() { const { page = '1', search = '', scope } = queryToObject(window.location.search); return { interval: undefined, isReviewAppModalVisible: false, + isStopStaleEnvModalVisible: false, page: parseInt(page, 10), pageInfo: {}, scope: Object.values(ENVIRONMENTS_SCOPE).includes(scope) @@ -107,6 +112,9 @@ export default { canSetupReviewApp() { return this.environmentApp?.reviewApp?.canSetupReviewApp; }, + canCleanUpEnvs() { + return this.environmentApp?.canStopStaleEnvironments; + }, folders() { return this.environmentApp?.environments?.filter((e) => e.size > 1) ?? []; }, @@ -149,6 +157,19 @@ export default { }, }; }, + openCleanUpEnvsModal() { + if (!this.canCleanUpEnvs) { + return null; + } + + return { + text: this.$options.i18n.cleanUpEnvsButtonLabel, + attributes: { + category: 'secondary', + variant: 'confirm', + }, + }; + }, stoppedCount() { return this.environmentApp?.stoppedCount; }, @@ -178,6 +199,9 @@ export default { showReviewAppModal() { this.isReviewAppModalVisible = true; }, + showCleanUpEnvsModal() { + this.isStopStaleEnvModalVisible = true; + }, setScope(scope) { this.scope = scope; this.moveToPage(1); @@ -219,16 +243,24 @@ export default { :modal-id="$options.modalId" data-testid="enable-review-app-modal" /> + <stop-stale-environments-modal + v-if="canCleanUpEnvs" + v-model="isStopStaleEnvModalVisible" + :modal-id="$options.stopStaleEnvsModalId" + data-testid="stop-stale-environments-modal" + /> <delete-environment-modal :environment="environmentToDelete" graphql /> <stop-environment-modal :environment="environmentToStop" graphql /> <confirm-rollback-modal :environment="environmentToRollback" graphql /> <canary-update-modal :environment="environmentToChangeCanary" :weight="weight" /> <gl-tabs - :action-secondary="addEnvironment" - :action-primary="openReviewAppModal" + :action-secondary="openReviewAppModal" + :action-primary="openCleanUpEnvsModal" + :action-tertiary="addEnvironment" sync-active-tab-with-query-params query-param-name="scope" - @primary="showReviewAppModal" + @secondary="showReviewAppModal" + @primary="showCleanUpEnvsModal" > <gl-tab :query-param-value="$options.ENVIRONMENTS_SCOPE.AVAILABLE" diff --git a/app/assets/javascripts/environments/components/stop_stale_environments_modal.vue b/app/assets/javascripts/environments/components/stop_stale_environments_modal.vue new file mode 100644 index 00000000000..57873b28d37 --- /dev/null +++ b/app/assets/javascripts/environments/components/stop_stale_environments_modal.vue @@ -0,0 +1,104 @@ +<script> +import { GlTooltipDirective, GlModal, GlDatepicker, GlFormGroup } from '@gitlab/ui'; +import { __, s__ } from '~/locale'; +import { stopStaleEnvironments } from '~/rest_api'; +import { MIN_STALE_ENVIRONMENT_DATE, MAX_STALE_ENVIRONMENT_DATE } from '../constants'; + +export default { + id: 'stop-stale-environments-modal', + name: 'StopStaleEnvironmentsModal', + + components: { + GlModal, + GlDatepicker, + GlFormGroup, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, + inject: { + projectId: { + default: '', + }, + }, + model: { + prop: 'visible', + event: 'change', + }, + props: { + modalId: { + type: String, + required: true, + }, + visible: { + type: Boolean, + required: false, + default: false, + }, + }, + modalProps: { + primary: { + text: s__('Environments|Clean up'), + attributes: [{ variant: 'info' }], + }, + cancel: { + text: __('Cancel'), + }, + dateRange: { + minDate: MIN_STALE_ENVIRONMENT_DATE, // 10 years ago + maxDate: MAX_STALE_ENVIRONMENT_DATE, + }, + }, + + data() { + return { + stopEnvironmentsBefore: MAX_STALE_ENVIRONMENT_DATE, + }; + }, + + methods: { + onSubmit() { + stopStaleEnvironments(this.projectId, this.stopEnvironmentsBefore || this.maxDate); + }, + }, +}; +</script> + +<template> + <gl-modal + :action-primary="$options.modalProps.primary" + :action-cancel="$options.modalProps.cancel" + :visible="visible" + :modal-id="modalId" + :title="s__('Environments|Clean up environments')" + static + @primary="onSubmit" + @change="$emit('change', $event)" + > + <p> + {{ + s__( + 'Environments|Select which environments to clean up. \ + Protected environments are excluded. Learn more about cleaning up environments.', + ) + }} + </p> + + <gl-form-group + :label="s__('Environments|Stop unused environments')" + :label-description=" + s__('Environments|Stop environments that have not been updated since the specified date:') + " + label-for="stop_environments-before" + > + <gl-datepicker + v-model="stopEnvironmentsBefore" + input-id="stop-environments-before" + data-testid="stop-environments-before" + :min-date="$options.modalProps.dateRange.minDate" + :max-date="$options.modalProps.dateRange.maxDate" + :default-date="$options.modalProps.dateRange.maxDate" + /> + </gl-form-group> + </gl-modal> +</template> diff --git a/app/assets/javascripts/environments/constants.js b/app/assets/javascripts/environments/constants.js index c4d02da9d21..f010898073c 100644 --- a/app/assets/javascripts/environments/constants.js +++ b/app/assets/javascripts/environments/constants.js @@ -1,4 +1,5 @@ import { __, s__ } from '~/locale'; +import { getDateInPast } from '~/lib/utils/datetime_utility'; // These statuses are based on how the backend defines pod phases here // lib/gitlab/kubernetes/pod.rb @@ -77,3 +78,6 @@ export const REVIEW_APP_MODAL_I18N = { viewMoreExampleProjects: s__('EnableReviewApp|View more example projects'), copyToClipboardText: s__('EnableReviewApp|Copy snippet'), }; + +export const MIN_STALE_ENVIRONMENT_DATE = getDateInPast(new Date(), 3650); // 10 years ago +export const MAX_STALE_ENVIRONMENT_DATE = getDateInPast(new Date(), 7); // one week ago diff --git a/app/assets/javascripts/environments/graphql/queries/environment_app.query.graphql b/app/assets/javascripts/environments/graphql/queries/environment_app.query.graphql index 1a572208a1c..7a50ded7d6c 100644 --- a/app/assets/javascripts/environments/graphql/queries/environment_app.query.graphql +++ b/app/assets/javascripts/environments/graphql/queries/environment_app.query.graphql @@ -4,5 +4,6 @@ query getEnvironmentApp($page: Int, $scope: String, $search: String) { stoppedCount environments reviewApp + canStopStaleEnvironments } } diff --git a/app/assets/javascripts/environments/graphql/resolvers.js b/app/assets/javascripts/environments/graphql/resolvers.js index afd56d0cf0d..e21670870b8 100644 --- a/app/assets/javascripts/environments/graphql/resolvers.js +++ b/app/assets/javascripts/environments/graphql/resolvers.js @@ -54,6 +54,7 @@ export const resolvers = (endpoint) => ({ ...convertObjectPropsToCamelCase(res.data.review_app), __typename: 'ReviewApp', }, + canStopStaleEnvironments: res.data.can_stop_stale_environments, stoppedCount: res.data.stopped_count, __typename: 'LocalEnvironmentApp', }; diff --git a/app/assets/javascripts/issues/show/components/app.vue b/app/assets/javascripts/issues/show/components/app.vue index e5428f87095..45c4e0889c0 100644 --- a/app/assets/javascripts/issues/show/components/app.vue +++ b/app/assets/javascripts/issues/show/components/app.vue @@ -453,7 +453,7 @@ export default { } }, - handleListItemReorder(description) { + handleSaveDescription(description) { this.updateFormState(); this.setFormState({ description }); this.updateIssuable(); @@ -573,7 +573,7 @@ export default { :update-url="updateEndpoint" :lock-version="state.lock_version" :is-updating="formState.updateLoading" - @listItemReorder="handleListItemReorder" + @saveDescription="handleSaveDescription" @taskListUpdateStarted="taskListUpdateStarted" @taskListUpdateSucceeded="taskListUpdateSucceeded" @taskListUpdateFailed="taskListUpdateFailed" diff --git a/app/assets/javascripts/issues/show/components/description.vue b/app/assets/javascripts/issues/show/components/description.vue index 78e729b97da..ba0f28d1c7c 100644 --- a/app/assets/javascripts/issues/show/components/description.vue +++ b/app/assets/javascripts/issues/show/components/description.vue @@ -1,6 +1,7 @@ <script> -import { GlToast, GlTooltip, GlModalDirective } from '@gitlab/ui'; +import { GlModalDirective, GlToast } from '@gitlab/ui'; import $ from 'jquery'; +import { uniqueId } from 'lodash'; import Sortable from 'sortablejs'; import Vue from 'vue'; import SafeHtml from '~/vue_shared/directives/safe_html'; @@ -18,7 +19,6 @@ import Tracking from '~/tracking'; import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; import projectWorkItemTypesQuery from '~/work_items/graphql/project_work_item_types.query.graphql'; import createWorkItemFromTaskMutation from '~/work_items/graphql/create_work_item_from_task.mutation.graphql'; - import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import WorkItemDetailModal from '~/work_items/components/work_item_detail_modal.vue'; import { @@ -29,8 +29,10 @@ import { WIDGET_TYPE_DESCRIPTION, } from '~/work_items/constants'; import { renderGFM } from '~/behaviors/markdown/render_gfm'; +import eventHub from '../event_hub'; import animateMixin from '../mixins/animate'; -import { convertDescriptionWithNewSort } from '../utils'; +import { convertDescriptionWithDeletedTaskListItem, convertDescriptionWithNewSort } from '../utils'; +import TaskListItemActions from './task_list_item_actions.vue'; Vue.use(GlToast); @@ -44,7 +46,6 @@ export default { GlModal: GlModalDirective, }, components: { - GlTooltip, WorkItemDetailModal, }, mixins: [animateMixin, glFeatureFlagMixin(), Tracking.mixin()], @@ -98,10 +99,10 @@ export default { const workItemId = getParameterByName('work_item_id'); return { + hasTaskListItemActions: false, preAnimation: false, pulseAnimation: false, initialUpdate: true, - taskButtons: [], activeTask: {}, workItemId: isPositiveInteger(workItemId) ? convertToGraphQLId(TYPE_WORK_ITEM, workItemId) @@ -164,6 +165,8 @@ export default { }, }, mounted() { + eventHub.$on('delete-task-list-item', this.deleteTaskListItem); + this.renderGFM(); this.updateTaskStatusText(); @@ -175,6 +178,8 @@ export default { } }, beforeDestroy() { + eventHub.$off('delete-task-list-item', this.deleteTaskListItem); + this.removeAllPointerEventListeners(); }, methods: { @@ -198,7 +203,7 @@ export default { this.renderSortableLists(); if (this.workItemsEnabled) { - this.renderTaskActions(); + this.renderTaskListItemActions(); } } }, @@ -223,7 +228,7 @@ export default { handle: '.drag-icon', onUpdate: (event) => { const description = convertDescriptionWithNewSort(this.descriptionText, event.to); - this.$emit('listItemReorder', description); + this.$emit('saveDescription', description); }, }), ); @@ -232,25 +237,25 @@ export default { createDragIconElement() { const container = document.createElement('div'); // eslint-disable-next-line no-unsanitized/property - container.innerHTML = `<svg class="drag-icon s14 gl-icon gl-cursor-grab gl-visibility-hidden" role="img" aria-hidden="true"> + container.innerHTML = `<svg class="drag-icon s14 gl-icon gl-cursor-grab gl-opacity-0" role="img" aria-hidden="true"> <use href="${gon.sprite_icons}#drag-vertical"></use> </svg>`; return container.firstChild; }, - addPointerEventListeners(listItem, iconSelector) { + addPointerEventListeners(listItem, elementSelector) { const pointeroverListener = (event) => { - const icon = event.target.closest('li').querySelector(iconSelector); - if (!icon || isDragging() || this.isUpdating) { + const element = event.target.closest('li').querySelector(elementSelector); + if (!element || isDragging() || this.isUpdating) { return; } - icon.style.visibility = 'visible'; + element.classList.add('gl-opacity-10'); }; const pointeroutListener = (event) => { - const icon = event.target.closest('li').querySelector(iconSelector); - if (!icon) { + const element = event.target.closest('li').querySelector(elementSelector); + if (!element) { return; } - icon.style.visibility = 'hidden'; + element.classList.remove('gl-opacity-10'); }; // We use pointerover/pointerout instead of CSS so that when we hover over a @@ -279,11 +284,9 @@ export default { taskListUpdateStarted() { this.$emit('taskListUpdateStarted'); }, - taskListUpdateSuccess() { this.$emit('taskListUpdateSucceeded'); }, - taskListUpdateError() { createAlert({ message: sprintf( @@ -298,7 +301,6 @@ export default { this.$emit('taskListUpdateFailed'); }, - updateTaskStatusText() { const taskRegexMatches = this.taskStatus.match(/(\d+) of ((?!0)\d+)/); const $issuableHeader = $('.issuable-meta'); @@ -317,15 +319,28 @@ export default { $tasksShort.text(''); } }, - renderTaskActions() { + createTaskListItemActions(toggleClass) { + const app = new Vue({ + el: document.createElement('div'), + provide: { toggleClass }, + render: (createElement) => createElement(TaskListItemActions), + }); + return app.$el; + }, + deleteTaskListItem(sourcepos) { + this.$emit( + 'saveDescription', + convertDescriptionWithDeletedTaskListItem(this.descriptionText, sourcepos), + ); + }, + renderTaskListItemActions() { if (!this.$el?.querySelectorAll) { return; } - this.taskButtons = []; const taskListFields = this.$el.querySelectorAll('.task-list-item:not(.inapplicable)'); - taskListFields.forEach((item, index) => { + taskListFields.forEach((item) => { const taskLink = item.querySelector('.gfm-issue'); if (taskLink) { const { issue, referenceType, issueType } = taskLink.dataset; @@ -351,31 +366,11 @@ export default { }); return; } - this.addPointerEventListeners(item, '.js-add-task'); - const button = document.createElement('button'); - button.classList.add( - 'btn', - 'btn-default', - 'btn-md', - 'gl-button', - 'btn-default-tertiary', - 'gl-visibility-hidden', - 'gl-p-0!', - 'gl-mt-n1', - 'gl-ml-3', - 'js-add-task', - ); - button.id = `js-task-button-${index}`; - this.taskButtons.push(button.id); - // eslint-disable-next-line no-unsanitized/property - button.innerHTML = ` - <svg data-testid="ellipsis_v-icon" role="img" aria-hidden="true" class="dropdown-icon gl-icon s14"> - <use href="${gon.sprite_icons}#doc-new"></use> - </svg> - `; - button.setAttribute('aria-label', s__('WorkItem|Create task')); - button.addEventListener('click', () => this.handleCreateTask(button)); - this.insertButtonNextToTaskText(item, button); + + const toggleClass = uniqueId('task-list-item-actions-'); + this.addPointerEventListeners(item, `.${toggleClass}`); + this.insertNextToTaskListItemText(this.createTaskListItemActions(toggleClass), item); + this.hasTaskListItemActions = true; }); }, addHoverListeners(taskLink, id) { @@ -391,19 +386,20 @@ export default { } }); }, - insertButtonNextToTaskText(listItem, button) { - const paragraph = Array.from(listItem.children).find((element) => element.tagName === 'P'); - const lastChild = listItem.lastElementChild; + insertNextToTaskListItemText(element, listItem) { + const children = Array.from(listItem.children); + const paragraph = children.find((el) => el.tagName === 'P'); + const list = children.find((el) => el.classList.contains('task-list')); if (paragraph) { // If there's a `p` element, then it's a multi-paragraph task item // and the task text exists within the `p` element as the last child - paragraph.append(button); - } else if (lastChild.tagName === 'OL' || lastChild.tagName === 'UL') { + paragraph.append(element); + } else if (list) { // Otherwise, the task item can have a child list which exists directly after the task text - lastChild.insertAdjacentElement('beforebegin', button); + list.insertAdjacentElement('beforebegin', element); } else { // Otherwise, the task item is a simple one where the task text exists as the last child - listItem.append(button); + listItem.append(element); } }, setActiveTask(el) { @@ -492,14 +488,7 @@ export default { </script> <template> - <div - v-if="descriptionHtml" - :class="{ - 'js-task-list-container': canUpdate, - 'work-items-enabled': workItemsEnabled, - }" - class="description" - > + <div v-if="descriptionHtml" :class="{ 'js-task-list-container': canUpdate }" class="description"> <div ref="gfm-content" v-safe-html:[$options.safeHtmlConfig]="descriptionHtml" @@ -507,10 +496,10 @@ export default { :class="{ 'issue-realtime-pre-pulse': preAnimation, 'issue-realtime-trigger-pulse': pulseAnimation, + 'has-task-list-item-actions': hasTaskListItemActions, }" class="md" ></div> - <textarea v-if="descriptionText" :value="descriptionText" @@ -531,10 +520,5 @@ export default { @workItemDeleted="handleDeleteTask" @close="closeWorkItemDetailModal" /> - <template v-if="workItemsEnabled"> - <gl-tooltip v-for="item in taskButtons" :key="item" :target="item"> - {{ s__('WorkItem|Create task') }} - </gl-tooltip> - </template> </div> </template> diff --git a/app/assets/javascripts/issues/show/components/task_list_item_actions.vue b/app/assets/javascripts/issues/show/components/task_list_item_actions.vue new file mode 100644 index 00000000000..084cd6062d5 --- /dev/null +++ b/app/assets/javascripts/issues/show/components/task_list_item_actions.vue @@ -0,0 +1,40 @@ +<script> +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { __, s__ } from '~/locale'; +import eventHub from '../event_hub'; + +export default { + i18n: { + delete: __('Delete'), + taskActions: s__('WorkItem|Task actions'), + }, + components: { + GlDropdown, + GlDropdownItem, + }, + inject: ['toggleClass'], + methods: { + deleteTaskListItem() { + eventHub.$emit('delete-task-list-item', this.$el.closest('li').dataset.sourcepos); + }, + }, +}; +</script> + +<template> + <gl-dropdown + class="task-list-item-actions-wrapper" + category="tertiary" + icon="ellipsis_v" + lazy + no-caret + right + :text="$options.i18n.taskActions" + text-sr-only + :toggle-class="`task-list-item-actions gl-opacity-0 gl-p-2! ${toggleClass}`" + > + <gl-dropdown-item variant="danger" @click="deleteTaskListItem"> + {{ $options.i18n.delete }} + </gl-dropdown-item> + </gl-dropdown> +</template> diff --git a/app/assets/javascripts/issues/show/utils.js b/app/assets/javascripts/issues/show/utils.js index 05b06586362..26ce5d03c2f 100644 --- a/app/assets/javascripts/issues/show/utils.js +++ b/app/assets/javascripts/issues/show/utils.js @@ -93,3 +93,78 @@ export const convertDescriptionWithNewSort = (description, list) => { return descriptionLines.join(NEWLINE); }; + +const bulletTaskListItemRegex = /^\s*[-*]\s+\[.]/; +const numericalTaskListItemRegex = /^\s*[0-9]\.\s+\[.]/; + +/** + * Checks whether the line of markdown contains a task list item, + * i.e. `- [ ]`, `* [ ]`, or `1. [ ]`. + * + * @param {String} line A line of markdown + * @returns {boolean} `true` if the line contains a task list item, otherwise `false` + */ +const containsTaskListItem = (line) => + bulletTaskListItemRegex.test(line) || numericalTaskListItemRegex.test(line); + +/** + * Deletes a task list item from the description. + * + * Starting from the task list item, it deletes each line until it hits a nested + * task list item and reduces the indentation of each line from this line onwards. + * + * For example, for a given description like: + * + * <pre> + * 1. [ ] item 1 + * + * paragraph text + * + * 1. [ ] item 2 + * + * paragraph text + * + * 1. [ ] item 3 + * </pre> + * + * Then when prompted to delete item 1, this function will return: + * + * <pre> + * 1. [ ] item 2 + * + * paragraph text + * + * 1. [ ] item 3 + * </pre> + * + * @param {String} description Description in markdown format + * @param {String} sourcepos Source position in format `23:3-23:14` + * @returns {String} Markdown with the deleted task list item + */ +export const convertDescriptionWithDeletedTaskListItem = (description, sourcepos) => { + const descriptionLines = description.split(NEWLINE); + const [startIndex, endIndex] = getSourceposRows(sourcepos); + + let indentation = 0; + let linesToDelete = 1; + let reduceIndentation = false; + + for (let i = startIndex + 1; i <= endIndex; i += 1) { + if (reduceIndentation) { + descriptionLines[i] = descriptionLines[i].slice(indentation); + } else if (containsTaskListItem(descriptionLines[i])) { + reduceIndentation = true; + const firstLine = descriptionLines[startIndex]; + const currentLine = descriptionLines[i]; + const firstLineIndentation = firstLine.length - firstLine.trimStart().length; + const currentLineIndentation = currentLine.length - currentLine.trimStart().length; + indentation = currentLineIndentation - firstLineIndentation; + descriptionLines[i] = descriptionLines[i].slice(indentation); + } else { + linesToDelete += 1; + } + } + + descriptionLines.splice(startIndex, linesToDelete); + return descriptionLines.join(NEWLINE); +}; diff --git a/app/assets/javascripts/rest_api.js b/app/assets/javascripts/rest_api.js index 7b5babdd3a6..87996d0bb85 100644 --- a/app/assets/javascripts/rest_api.js +++ b/app/assets/javascripts/rest_api.js @@ -7,6 +7,7 @@ export * from './api/namespaces_api'; export * from './api/tags_api'; export * from './api/alert_management_alerts_api'; export * from './api/harbor_registry'; +export * from './api/environments_api'; // Note: It's not possible to spy on methods imported from this file in // Jest tests. diff --git a/app/assets/stylesheets/page_bundles/issues_show.scss b/app/assets/stylesheets/page_bundles/issues_show.scss index 26d694f7421..23b210e3a8e 100644 --- a/app/assets/stylesheets/page_bundles/issues_show.scss +++ b/app/assets/stylesheets/page_bundles/issues_show.scss @@ -12,6 +12,21 @@ padding-inline-end: 1rem; width: 2rem; } + + .task-list-item-actions-wrapper { + position: absolute; + inset-block-start: 0; + inset-inline-end: -2rem; + } + + .task-list-item-actions-wrapper.show .task-list-item-actions, + .task-list-item-actions:is(:focus, :hover) { + opacity: 1; + } + } + + .md.has-task-list-item-actions > :is(ul, ol) > li { + margin-inline-end: 1.5rem; } ul.task-list > li.task-list-item { @@ -21,6 +36,10 @@ inset-inline-start: -0.6rem; } } + + .dropdown-item.text-danger p { + color: var(--red-500, $red-500); /* Override typography.scss making text black */ + } } .is-ghost { diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index ea1288c0b20..813e8d7f84d 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -19,6 +19,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action only: [:show] do push_frontend_feature_flag(:environment_details_vue, @project) end + before_action :authorize_read_environment!, except: [:metrics, :additional_metrics, :metrics_dashboard, :metrics_redirect] before_action :authorize_create_environment!, only: [:new, :create] before_action :authorize_stop_environment!, only: [:stop] @@ -57,6 +58,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController render json: { environments: serialize_environments(request, response, params[:nested]), review_app: serialize_review_app, + can_stop_stale_environments: Feature.enabled?(:stop_stale_environments, @project) && + can?(current_user, :stop_environment, @project), available_count: environments_count_by_state[:available], stopped_count: environments_count_by_state[:stopped] } diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 71cc5581ae6..d56e7858990 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -181,9 +181,9 @@ module Issues return if skip_milestone_email if issue.milestone.nil? - notification_service.async.removed_milestone_issue(issue, current_user) + notification_service.async.removed_milestone(issue, current_user) else - notification_service.async.changed_milestone_issue(issue, issue.milestone, current_user) + notification_service.async.changed_milestone(issue, issue.milestone, current_user) end end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index a273b853c0d..255d96f4969 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -215,9 +215,9 @@ module MergeRequests delete_milestone_total_merge_requests_counter_cache(previous_milestone) if merge_request.milestone.nil? - notification_service.async.removed_milestone_merge_request(merge_request, current_user) + notification_service.async.removed_milestone(merge_request, current_user) else - notification_service.async.changed_milestone_merge_request(merge_request, merge_request.milestone, current_user) + notification_service.async.changed_milestone(merge_request, merge_request.milestone, current_user) delete_milestone_total_merge_requests_counter_cache(merge_request.milestone) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 777d02c590d..1be240356c6 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -212,14 +212,6 @@ class NotificationService relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email) end - def removed_milestone_issue(issue, current_user) - removed_milestone_resource_email(issue, current_user, :removed_milestone_issue_email) - end - - def changed_milestone_issue(issue, new_milestone, current_user) - changed_milestone_resource_email(issue, new_milestone, current_user, :changed_milestone_issue_email) - end - # When create a merge request we should send an email to: # # * mr author @@ -366,14 +358,6 @@ class NotificationService relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email) end - def removed_milestone_merge_request(merge_request, current_user) - removed_milestone_resource_email(merge_request, current_user, :removed_milestone_merge_request_email) - end - - def changed_milestone_merge_request(merge_request, new_milestone, current_user) - changed_milestone_resource_email(merge_request, new_milestone, current_user, :changed_milestone_merge_request_email) - end - def close_mr(merge_request, current_user) close_resource_email(merge_request, current_user, :closed_merge_request_email) end @@ -788,6 +772,44 @@ class NotificationService end end + def removed_milestone(target, current_user) + method = case target + when Issue + :removed_milestone_issue_email + when MergeRequest + :removed_milestone_merge_request_email + end + + recipients = NotificationRecipients::BuildService.build_recipients( + target, + current_user, + action: 'removed_milestone' + ) + + recipients.each do |recipient| + mailer.send(method, recipient.user.id, target.id, current_user.id).deliver_later + end + end + + def changed_milestone(target, milestone, current_user) + method = case target + when Issue + :changed_milestone_issue_email + when MergeRequest + :changed_milestone_merge_request_email + end + + recipients = NotificationRecipients::BuildService.build_recipients( + target, + current_user, + action: 'changed_milestone' + ) + + recipients.each do |recipient| + mailer.send(method, recipient.user.id, target.id, milestone, current_user.id).deliver_later + end + end + protected def new_resource_email(target, current_user, method) @@ -847,30 +869,6 @@ class NotificationService end end - def removed_milestone_resource_email(target, current_user, method) - recipients = NotificationRecipients::BuildService.build_recipients( - target, - current_user, - action: 'removed_milestone' - ) - - recipients.each do |recipient| - mailer.send(method, recipient.user.id, target.id, current_user.id).deliver_later - end - end - - def changed_milestone_resource_email(target, milestone, current_user, method) - recipients = NotificationRecipients::BuildService.build_recipients( - target, - current_user, - action: 'changed_milestone' - ) - - recipients.each do |recipient| - mailer.send(method, recipient.user.id, target.id, milestone, current_user.id).deliver_later - end - end - def reopen_resource_email(target, current_user, method, status) recipients = NotificationRecipients::BuildService.build_recipients(target, current_user, action: "reopen") diff --git a/config/feature_flags/development/stop_stale_environments.yml b/config/feature_flags/development/stop_stale_environments.yml new file mode 100644 index 00000000000..ea1484f0970 --- /dev/null +++ b/config/feature_flags/development/stop_stale_environments.yml @@ -0,0 +1,8 @@ +--- +name: stop_stale_environments +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108616 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/387820 +milestone: '15.8' +type: development +group: group::release +default_enabled: false diff --git a/doc/administration/geo/replication/troubleshooting.md b/doc/administration/geo/replication/troubleshooting.md index 39828ca21a2..bf9a7be8b1f 100644 --- a/doc/administration/geo/replication/troubleshooting.md +++ b/doc/administration/geo/replication/troubleshooting.md @@ -401,6 +401,39 @@ generate an error because containers in Kubernetes do not have access to the hos Machine clock is synchronized ... Exception: getaddrinfo: Servname not supported for ai_socktype ``` +### Message: `ActiveRecord::StatementInvalid: PG::ReadOnlySqlTransaction: ERROR: cannot execute INSERT in a read-only transaction` + +When this error is encountered on a secondary site, it likely affects all usages of GitLab Rails such as `gitlab-rails` or `gitlab-rake` commands, as well the Puma, Sidekiq, and Geo Log Cursor services. + +```plaintext +ActiveRecord::StatementInvalid: PG::ReadOnlySqlTransaction: ERROR: cannot execute INSERT in a read-only transaction +/opt/gitlab/embedded/service/gitlab-rails/app/models/application_record.rb:86:in `block in safe_find_or_create_by' +/opt/gitlab/embedded/service/gitlab-rails/app/models/concerns/cross_database_modification.rb:92:in `block in transaction' +/opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/database.rb:332:in `block in transaction' +/opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/database.rb:331:in `transaction' +/opt/gitlab/embedded/service/gitlab-rails/app/models/concerns/cross_database_modification.rb:83:in `transaction' +/opt/gitlab/embedded/service/gitlab-rails/app/models/application_record.rb:86:in `safe_find_or_create_by' +/opt/gitlab/embedded/service/gitlab-rails/app/models/shard.rb:21:in `by_name' +/opt/gitlab/embedded/service/gitlab-rails/app/models/shard.rb:17:in `block in populate!' +/opt/gitlab/embedded/service/gitlab-rails/app/models/shard.rb:17:in `map' +/opt/gitlab/embedded/service/gitlab-rails/app/models/shard.rb:17:in `populate!' +/opt/gitlab/embedded/service/gitlab-rails/config/initializers/fill_shards.rb:9:in `<top (required)>' +/opt/gitlab/embedded/service/gitlab-rails/config/environment.rb:7:in `<top (required)>' +/opt/gitlab/embedded/bin/bundle:23:in `load' +/opt/gitlab/embedded/bin/bundle:23:in `<main>' +``` + +The PostgreSQL read-replica database would be producing these errors: + +```plaintext +2023-01-17_17:44:54.64268 ERROR: cannot execute INSERT in a read-only transaction +2023-01-17_17:44:54.64271 STATEMENT: /*application:web,db_config_name:main*/ INSERT INTO "shards" ("name") VALUES ('storage1') RETURNING "id" +``` + +This situation can occur during initial configuration when a secondary site is not yet aware that it is a secondary site. + +To resolve the error, follow [Step 3. Add the secondary site](configuration.md#step-3-add-the-secondary-site). + ## Fixing PostgreSQL database replication errors The following sections outline troubleshooting steps for fixing replication diff --git a/doc/development/merge_request_concepts/diffs/frontend.md b/doc/development/merge_request_concepts/diffs/frontend.md new file mode 100644 index 00000000000..6bd6d80af94 --- /dev/null +++ b/doc/development/merge_request_concepts/diffs/frontend.md @@ -0,0 +1,208 @@ +--- +stage: Create +group: Code Review +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +--- + +# Merge request diffs frontend overview + +This document provides an overview on how the frontend diffs Vue application works, and +the various different parts that exist. It should help contributors: + +- Understand how the diffs Vue app is set up. +- Identify any areas that need improvement. + +This document is a living document. Update it whenever anything significant changes in +the diffs application. + +## Diffs Vue app + +### Components + +The Vue app for rendering diffs uses many different Vue components, some of which get shared +with other areas of the GitLab app. The below chart shows the direction for which components +get rendered. + +NOTE: +[Issue #388843](https://gitlab.com/gitlab-org/gitlab/-/issues/388843) is open to +generate a Mermaid graph of the components diagram. An image version of the +diagram is available in the issue. + +Some of the components are rendered more than others, but the main component is `diff_row.vue`. +This component renders every diff line in a diff file. For performance reasons, this +component is a functional component. However, when we upgrade to Vue 3, this is no longer +required. + +The main diff app component is the main entry point to the diffs app. One of the most important parts +of this component is to dispatch the action that assigns discussions to diff lines. This action +gets dispatched after the metadata request is completed, and after the batch diffs requests are +finished. There is also a watcher set up to watches for changes in both the diff files array and the notes +array. Whenever a change happens here, the set discussion action gets dispatched. + +The DiffRow component is set up in a way that allows for us to store the diff line data in one format. +Previously, we had to request two different formats for inline and side-by-side. The DiffRow component +then uses this standard format to render the diff line data. With this standard format, the user +can then switch between inline and side-by-side without the need to re-fetch any data. + +NOTE: +For this component, a lot of the data used and rendered gets memoized and cached, based on +various conditions. It is possible that data sometimes gets cached between each different +component render. + +### Vuex store + +The Vuex store for the diffs app consists of 3 different modules: + +- Notes +- Diffs +- Batch comments + +The notes module is responsible for the discussions, including diff discussions. In this module, +the discussions get fetched, and the polling for new discussions is setup. This module gets shared +with the issue app as well, so changes here need to be tested in both issues and merge requests. + +The diffs module is responsible for the everything related to diffs. This includes, but is not limited +to, fetching diffs, assigning diff discussions to lines, and creating diff discussions. + +Finally, the batch comments module is not complex, and is responsible only for the draft comments feature. +However, this module does dispatch actions in the notes and diff modules whenever draft comments +are published. + +### API Requests + +#### Metadata + +The diffs metadata endpoint exists to fetch the base data the diffs app requires quickly, without +the need to fetch all the diff files. This includes, but is not limited to: + +- Diff file names, including some extra meta data for diff files +- Added and removed line numbers +- Branch names +- Diff versions + +The most important part of the metadata response is the diff file names. This data allows the diffs +app to render the file browser inside of the diffs app, without waiting for all batch diffs +requests to complete. + +When the metadata response is received, the diff file data gets sent to a web worker. The web worker +exists to allow for this data, which for larger merge requests could be huge, to be processed off +the main thread. Processing this data involves getting the data into the correct structure +that the frontend requires to render the file browser in either tree view or list view. + +```mermaid +graph TD + A[fetchDiffFilesMeta] + B[Create web worker] + C[Fetch data] + D[SET_DIFF_METADATA] + E[Post worker data] + F[Worker message event listener] + K[SET_TREE_DATA] + + G[TreeWorker] + H[onMessage] + I[generateTreeList] + J[postMessage sortTree] + + A -->B + E -->F + B -->C + C -->D + D -->E + + G -->H + H -->I + I -->J + J -->F + + F -->K +``` + +The structure for this file object is: + +```javascript +{ + "key": "", + "path": "", + "name": "", + "type": "", + "tree": [], + "changed": true, + "tempFile": false, + "deleted": false, + "fileHash": "", + "addedLines": 1, + "removedLines": 1, + "parentPath": "/", + "submodule": false +} +``` + +#### Batch diffs + +To reduce the response size for the diffs endpoint, we are splitting this response up into different +requests, to: + +- Reduces the response size of each request. +- Allows the diffs app to start rendering diffs as quickly as the first request finishes. + +To make the first request quicker, the request gets sent asking for a small amount of +diffs. The number of diffs requested then increases, until the maximum number of diffs per request is 30. + +When the request finishes, the diffs app formats the data received into a format that makes +it easier for the diffs app to render the diffs lines. + +```mermaid +graph TD + A[fetchDiffFilesBatch] --> + B[commit SET_DIFF_DATA_BATCH] --> + C[prepareDiffData] --> + D[prepareRawDiffFile] --> + E[ensureBasicDiffFileLines] --> + F[prepareDiffFileLines] --> + G[finalizeDiffFile] --> + H[deduplicateFilesList] +``` + +After this has been completed, the diffs app can now begin to render the diff lines. However, before +anything can be rendered the diffs app does one more format. It takes the diff line data, and maps +the data into a format for easier switching between inline and side-by-side modes. This +formatting happens in a computed property inside the `diff_content.vue` component. + +### Render queue + +NOTE: +This _might_ not be required any more. Some investigation work is required to decide +the future of the render queue. The virtual scroll bar we created has probably removed +any performance benefit we got from this approach. + +To render diffs quickly, we have a render queue that allows the diffs to render only if the +browser is idle. This saves the browser getting frozen when rendering a lot of large diffs at once, +and allows us to reduce the total blocking time. + +This pipeline of rendering files happens only if all the below conditions are `true` for every +diff file. If any of these are `false`, then this render queue does not happen and the diffs get +rendered as expected. + +- Are the diffs in this file already rendered? +- Does this diff have a viewer? (Meaning, is it not a download?) +- Is the diff expanded? + +This chart gives a brief overview of the pipeline that happens: + +```mermaid +graph TD + A[startRenderDiffsQueue] -->B + B[commit RENDER_FILE current file index] -->C + C[canRenderNextFile?] + C -->|Yes| D[Render file] -->B + C -->|No| E[Re-run requestIdleCallback] -->C +``` + +The checks that happen: + +- Is the idle time remaining less than 5 ms? +- Have we already tried to render this file 4 times? + +After these checks happen, the file is marked in Vuex as `renderable`, which allows the diffs +app to start rendering the diff lines and discussions. diff --git a/doc/user/project/merge_requests/approvals/rules.md b/doc/user/project/merge_requests/approvals/rules.md index 51f7254bfda..0a5e7c29860 100644 --- a/doc/user/project/merge_requests/approvals/rules.md +++ b/doc/user/project/merge_requests/approvals/rules.md @@ -254,6 +254,15 @@ the API. For more information about this validation error, read [issue 285129](https://gitlab.com/gitlab-org/gitlab/-/issues/285129). +### Groups need explicit or inherited Developer role on a project + +A group created to handle approvals may be created in a different area of the +project hierarchy than the project requiring review. If this happens, the approvals group +isn't recognized as a valid Code Owner for the project, nor does it display in the +project's **Approvals** list. To fix this problem, add the approval group as a shared group +high enough in the shared hierarchy so the project requiring review inherits this +group of users. + ## Security Approvals **(ULTIMATE)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/357021) in GitLab 15.0. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d82867713c5..006243bb9ec 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15747,6 +15747,12 @@ msgstr "" msgid "Environments|Auto stops %{autoStopAt}" msgstr "" +msgid "Environments|Clean up" +msgstr "" + +msgid "Environments|Clean up environments" +msgstr "" + msgid "Environments|Commit" msgstr "" @@ -15840,6 +15846,9 @@ msgstr "" msgid "Environments|Search by environment name" msgstr "" +msgid "Environments|Select which environments to clean up. Protected environments are excluded. Learn more about cleaning up environments." +msgstr "" + msgid "Environments|Show all" msgstr "" @@ -15849,6 +15858,12 @@ msgstr "" msgid "Environments|Stop environment" msgstr "" +msgid "Environments|Stop environments that have not been updated since the specified date:" +msgstr "" + +msgid "Environments|Stop unused environments" +msgstr "" + msgid "Environments|Stopping %{environmentName}" msgstr "" @@ -47760,9 +47775,6 @@ msgstr "" msgid "WorkItem|Create objective" msgstr "" -msgid "WorkItem|Create task" -msgstr "" - msgid "WorkItem|Create work item" msgstr "" @@ -47901,6 +47913,9 @@ msgstr "" msgid "WorkItem|Task" msgstr "" +msgid "WorkItem|Task actions" +msgstr "" + msgid "WorkItem|Task deleted" msgstr "" diff --git a/rubocop/cop/gitlab/doc_url.rb b/rubocop/cop/gitlab/doc_url.rb new file mode 100644 index 00000000000..cbfbdf7eb57 --- /dev/null +++ b/rubocop/cop/gitlab/doc_url.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # This cop encourages using helper to link to documentation + # in string literals. + # + # @example + # # bad + # 'See [the docs](https://docs.gitlab.com/ee/user/permissions#roles).' + # _('See [the docs](https://docs.gitlab.com/ee/user/permissions#roles).') + # + # # good + # docs_link = link_to _('the docs'), help_page_url('user/permissions', anchor: 'roles') + # "See #{docs_link}." + # _('See %{docs_link}.').html_safe % { docs_link: docs_link.html_safe } + class DocUrl < RuboCop::Cop::Base + include RangeHelp + + MSG = 'Use `#help_page_url` instead of directly including link. ' \ + 'See https://docs.gitlab.com/ee/development/documentation/#linking-to-help-in-ruby.' + + DOCS_URL_REGEXP = %r{https://docs.gitlab.com/ee/[\w#%./-]+}.freeze + + def on_str(node) + match = DOCS_URL_REGEXP.match(node.source) + return unless match + + add_offense(bad_range(node, match)) + end + + private + + def bad_range(node, match) + url_begin_pos, url_end_pos = match.offset(0) + begin_pos = node.loc.expression.begin_pos + url_begin_pos + + range_between(begin_pos, begin_pos + (url_end_pos - url_begin_pos)) + end + end + end + end +end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index dddefbac163..48e58cc0e38 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::EnvironmentsController do +RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_delivery do include MetricsDashboardHelpers include KubernetesHelpers @@ -103,6 +103,38 @@ RSpec.describe Projects::EnvironmentsController do expect(json_response['stopped_count']).to eq 1 end + context 'can access stop stale environments feature' do + context 'when stop_stale_environments FF is enabled' do + it 'maintainers can access the feature' do + get :index, params: environment_params(format: :json) + + expect(json_response['can_stop_stale_environments']).to be_truthy + end + + context 'when user is a reporter' do + let(:user) { reporter } + + it 'reporters cannot access the feature' do + get :index, params: environment_params(format: :json) + + expect(json_response['can_stop_stale_environments']).to be_falsey + end + end + end + + context 'when stop_stale_environments FF is disabled' do + before do + stub_feature_flags(stop_stale_environments: false) + end + + it 'maintainers cannot access the feature' do + get :index, params: environment_params(format: :json) + + expect(json_response['can_stop_stale_environments']).to be_falsey + end + end + end + context 'when enable_environments_search_within_folder FF is disabled' do before do stub_feature_flags(enable_environments_search_within_folder: false) diff --git a/spec/frontend/environments/environments_app_spec.js b/spec/frontend/environments/environments_app_spec.js index 65a9f2907d2..986ecca4e84 100644 --- a/spec/frontend/environments/environments_app_spec.js +++ b/spec/frontend/environments/environments_app_spec.js @@ -195,6 +195,36 @@ describe('~/environments/components/environments_app.vue', () => { expect(button.exists()).toBe(false); }); + it('should not show a button to clean up environments if the user has no permissions', async () => { + await createWrapperWithMocked({ + environmentsApp: { + ...resolvedEnvironmentsApp, + canStopStaleEnvironments: false, + }, + folder: resolvedFolder, + }); + + const button = wrapper.findByRole('button', { + name: s__('Environments|Clean up environments'), + }); + expect(button.exists()).toBe(false); + }); + + it('should show a button to clean up environments if the user has permissions', async () => { + await createWrapperWithMocked({ + environmentsApp: { + ...resolvedEnvironmentsApp, + canStopStaleEnvironments: true, + }, + folder: resolvedFolder, + }); + + const button = wrapper.findByRole('button', { + name: s__('Environments|Clean up environments'), + }); + expect(button.exists()).toBe(true); + }); + describe('tabs', () => { it('should show tabs for available and stopped environmets', async () => { await createWrapperWithMocked({ diff --git a/spec/frontend/environments/graphql/mock_data.js b/spec/frontend/environments/graphql/mock_data.js index 355b77b55c3..0b1c8dc6fb8 100644 --- a/spec/frontend/environments/graphql/mock_data.js +++ b/spec/frontend/environments/graphql/mock_data.js @@ -265,6 +265,7 @@ export const environmentsApp = { review_snippet: '{"deploy_review"=>{"stage"=>"deploy", "script"=>["echo \\"Deploy a review app\\""], "environment"=>{"name"=>"review/$CI_COMMIT_REF_NAME", "url"=>"https://$CI_ENVIRONMENT_SLUG.example.com"}, "only"=>["branches"]}}', }, + can_stop_stale_environments: true, available_count: 4, stopped_count: 0, }; @@ -474,6 +475,7 @@ export const resolvedEnvironmentsApp = { '{"deploy_review"=>{"stage"=>"deploy", "script"=>["echo \\"Deploy a review app\\""], "environment"=>{"name"=>"review/$CI_COMMIT_REF_NAME", "url"=>"https://$CI_ENVIRONMENT_SLUG.example.com"}, "only"=>["branches"]}}', __typename: 'ReviewApp', }, + canStopStaleEnvironments: true, stoppedCount: 0, __typename: 'LocalEnvironmentApp', }; diff --git a/spec/frontend/environments/stop_stale_environments_modal_spec.js b/spec/frontend/environments/stop_stale_environments_modal_spec.js new file mode 100644 index 00000000000..a2ab4f707b5 --- /dev/null +++ b/spec/frontend/environments/stop_stale_environments_modal_spec.js @@ -0,0 +1,60 @@ +import MockAdapter from 'axios-mock-adapter'; +import { GlModal } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import StopStaleEnvironmentsModal from '~/environments/components/stop_stale_environments_modal.vue'; +import axios from '~/lib/utils/axios_utils'; +import { getDateInPast } from '~/lib/utils/datetime_utility'; +import { STOP_STALE_ENVIRONMENTS_PATH } from '~/api/environments_api'; +import { HTTP_STATUS_OK } from '~/lib/utils/http_status'; + +const DEFAULT_OPTS = { + provide: { projectId: 1 }, +}; + +const ONE_WEEK_AGO = getDateInPast(new Date(), 7); +const TEN_YEARS_AGO = getDateInPast(new Date(), 3650); + +describe('~/environments/components/stop_stale_environments_modal.vue', () => { + let wrapper; + let mock; + let before; + let originalGon; + + const createWrapper = (opts = {}) => + shallowMount(StopStaleEnvironmentsModal, { + ...DEFAULT_OPTS, + ...opts, + propsData: { modalId: 'stop-stale-environments-modal', visible: true }, + }); + + beforeEach(() => { + originalGon = window.gon; + window.gon = { api_version: 'v4' }; + + mock = new MockAdapter(axios); + jest.spyOn(axios, 'post'); + wrapper = createWrapper(); + before = wrapper.find("[data-testid='stop-environments-before']"); + }); + + afterEach(() => { + mock.restore(); + wrapper.destroy(); + jest.resetAllMocks(); + window.gon = originalGon; + }); + + it('sets the correct min and max dates', async () => { + expect(before.props().minDate.toISOString()).toBe(TEN_YEARS_AGO.toISOString()); + expect(before.props().maxDate.toISOString()).toBe(ONE_WEEK_AGO.toISOString()); + }); + + it('requests cleanup when submit is clicked', async () => { + mock.onPost().replyOnce(HTTP_STATUS_OK); + wrapper.findComponent(GlModal).vm.$emit('primary'); + const url = STOP_STALE_ENVIRONMENTS_PATH.replace(':id', 1).replace(':version', 'v4'); + expect(axios.post).toHaveBeenCalledWith(url, null, { + params: { before: ONE_WEEK_AGO.toISOString() }, + }); + }); +}); diff --git a/spec/frontend/issues/show/components/app_spec.js b/spec/frontend/issues/show/components/app_spec.js index 6cf44e60092..7c1d643dc0f 100644 --- a/spec/frontend/issues/show/components/app_spec.js +++ b/spec/frontend/issues/show/components/app_spec.js @@ -645,10 +645,10 @@ describe('Issuable output', () => { }); }); - describe('listItemReorder event', () => { + describe('saveDescription event', () => { it('makes request to update issue', async () => { const description = 'I have been updated!'; - findDescription().vm.$emit('listItemReorder', description); + findDescription().vm.$emit('saveDescription', description); await waitForPromises(); expect(mock.history.put[0].data).toContain(description); diff --git a/spec/frontend/issues/show/components/description_spec.js b/spec/frontend/issues/show/components/description_spec.js index 889ff450825..9f36204139f 100644 --- a/spec/frontend/issues/show/components/description_spec.js +++ b/spec/frontend/issues/show/components/description_spec.js @@ -1,8 +1,7 @@ import $ from 'jquery'; import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; -import { GlTooltip, GlModal } from '@gitlab/ui'; - +import { GlModal } from '@gitlab/ui'; import setWindowLocation from 'helpers/set_window_location_helper'; import { stubComponent } from 'helpers/stub_component'; import { TEST_HOST } from 'helpers/test_constants'; @@ -10,9 +9,8 @@ import { mockTracking } from 'helpers/tracking_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; - -import { createAlert } from '~/flash'; import Description from '~/issues/show/components/description.vue'; +import eventHub from '~/issues/show/event_hub'; import { updateHistory } from '~/lib/utils/url_utility'; import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; import workItemTypesQuery from '~/work_items/graphql/project_work_item_types.query.graphql'; @@ -31,7 +29,6 @@ import { descriptionHtmlWithTask, } from '../mock_data/mock_data'; -jest.mock('~/flash'); jest.mock('~/lib/utils/url_utility', () => ({ ...jest.requireActual('~/lib/utils/url_utility'), updateHistory: jest.fn(), @@ -65,11 +62,8 @@ describe('Description component', () => { const findGfmContent = () => wrapper.find('[data-testid="gfm-content"]'); const findTextarea = () => wrapper.find('[data-testid="textarea"]'); - const findTaskActionButtons = () => wrapper.findAll('.js-add-task'); - const findConvertToTaskButton = () => wrapper.find('.js-add-task'); + const findTaskActionButtons = () => wrapper.findAll('.task-list-item-actions'); const findTaskLink = () => wrapper.find('a.gfm-issue'); - - const findTooltips = () => wrapper.findAllComponents(GlTooltip); const findModal = () => wrapper.findComponent(GlModal); const findWorkItemDetailModal = () => wrapper.findComponent(WorkItemDetailModal); @@ -125,10 +119,6 @@ describe('Description component', () => { } }); - afterEach(() => { - wrapper.destroy(); - }); - afterAll(() => { $('.issuable-meta .flash-container').remove(); }); @@ -311,13 +301,6 @@ describe('Description component', () => { expect(findTaskActionButtons()).toHaveLength(3); }); - it('renders a list of tooltips corresponding to checkboxes in description HTML', () => { - expect(findTooltips()).toHaveLength(3); - expect(findTooltips().at(0).props('target')).toBe( - findTaskActionButtons().at(0).attributes('id'), - ); - }); - it('does not show a modal by default', () => { expect(findModal().exists()).toBe(false); }); @@ -331,50 +314,29 @@ describe('Description component', () => { }); }); - describe('creating work item from checklist item', () => { - it('emits `updateDescription` after creating new work item', async () => { - createComponent({ - props: { - descriptionHtml: descriptionHtmlWithCheckboxes, - }, - provide: { - glFeatures: { - workItemsCreateFromMarkdown: true, - }, - }, - }); - - const newDescription = `<p>New description</p>`; + describe('task list item actions', () => { + describe('deleting the task list item', () => { + it('emits an event to update the description with the deleted task list item', () => { + const descriptionText = `Tasks - await findConvertToTaskButton().trigger('click'); +1. [ ] item 1 + 1. [ ] item 2 + 1. [ ] item 3 + 1. [ ] item 4;`; + const newDescriptionText = `Tasks - await waitForPromises(); +1. [ ] item 1 + 1. [ ] item 3 + 1. [ ] item 4;`; + createComponent({ + props: { descriptionText }, + provide: { glFeatures: { workItemsCreateFromMarkdown: true } }, + }); - expect(wrapper.emitted('updateDescription')).toEqual([[newDescription]]); - }); + eventHub.$emit('delete-task-list-item', '4:4-5:19'); - it('shows flash message when creating task fails', async () => { - createComponent({ - props: { - descriptionHtml: descriptionHtmlWithCheckboxes, - }, - provide: { - glFeatures: { - workItemsCreateFromMarkdown: true, - }, - }, - createWorkItemFromTaskHandler: jest.fn().mockRejectedValue({}), + expect(wrapper.emitted('saveDescription')).toEqual([[newDescriptionText]]); }); - - await findConvertToTaskButton().trigger('click'); - - await waitForPromises(); - - expect(createAlert).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'Something went wrong when creating task. Please try again.', - }), - ); }); }); diff --git a/spec/frontend/issues/show/components/task_list_item_actions_spec.js b/spec/frontend/issues/show/components/task_list_item_actions_spec.js new file mode 100644 index 00000000000..d1879510d59 --- /dev/null +++ b/spec/frontend/issues/show/components/task_list_item_actions_spec.js @@ -0,0 +1,45 @@ +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import TaskListItemActions from '~/issues/show/components/task_list_item_actions.vue'; +import eventHub from '~/issues/show/event_hub'; + +describe('TaskListItemActions component', () => { + let wrapper; + + const findGlDropdown = () => wrapper.findComponent(GlDropdown); + const findGlDropdownItem = () => wrapper.findComponent(GlDropdownItem); + + const mountComponent = () => { + const li = document.createElement('li'); + li.dataset.sourcepos = '3:1-3:10'; + li.appendChild(document.createElement('div')); + document.body.appendChild(li); + + wrapper = shallowMount(TaskListItemActions, { + provide: { toggleClass: 'task-list-item-actions' }, + attachTo: document.querySelector('div'), + }); + }; + + beforeEach(() => { + mountComponent(); + }); + + it('renders dropdown', () => { + expect(findGlDropdown().props()).toMatchObject({ + category: 'tertiary', + icon: 'ellipsis_v', + right: true, + text: TaskListItemActions.i18n.taskActions, + textSrOnly: true, + }); + }); + + it('emits event when `Delete` dropdown item is clicked', () => { + jest.spyOn(eventHub, '$emit'); + + findGlDropdownItem().vm.$emit('click'); + + expect(eventHub.$emit).toHaveBeenCalledWith('delete-task-list-item', '3:1-3:10'); + }); +}); diff --git a/spec/frontend/issues/show/utils_spec.js b/spec/frontend/issues/show/utils_spec.js index 603fb5cc2a6..58966d51be2 100644 --- a/spec/frontend/issues/show/utils_spec.js +++ b/spec/frontend/issues/show/utils_spec.js @@ -1,4 +1,7 @@ -import { convertDescriptionWithNewSort } from '~/issues/show/utils'; +import { + convertDescriptionWithDeletedTaskListItem, + convertDescriptionWithNewSort, +} from '~/issues/show/utils'; describe('app/assets/javascripts/issues/show/utils.js', () => { describe('convertDescriptionWithNewSort', () => { @@ -137,4 +140,201 @@ describe('app/assets/javascripts/issues/show/utils.js', () => { expect(convertDescriptionWithNewSort(description, list.firstChild)).toBe(expected); }); }); + + describe('convertDescriptionWithDeletedTaskListItem', () => { + const description = `Tasks + +1. [ ] item 1 + 1. [ ] item 2 + 1. [ ] item 3 + 1. [ ] item 4 + 1. [ ] item 5 + 1. [ ] item 6 + + paragraph text + + 1. [ ] item 7 + + paragraph text + + 1. [ ] item 8 + + paragraph text + + 1. [ ] item 9 + 1. [ ] item 10`; + + /* The equivalent HTML for the above markdown + <ol data-sourcepos="3:1-21:17"> + <li data-sourcepos="3:1-21:17">item 1 + <ol data-sourcepos="4:4-21:17"> + <li data-sourcepos="4:4-4:16"> + <p data-sourcepos="4:7-4:16">item 2</p> + </li> + <li data-sourcepos="5:4-7:19"> + <p data-sourcepos="5:7-5:16">item 3</p> + <ol data-sourcepos="6:7-7:19"> + <li data-sourcepos="6:7-6:19">item 4</li> + <li data-sourcepos="7:7-7:19">item 5</li> + </ol> + </li> + <li data-sourcepos="8:4-11:0"> + <p data-sourcepos="8:7-8:16">item 6</p> + <p data-sourcepos="10:7-10:20">paragraph text</p> + </li> + <li data-sourcepos="12:4-20:19"> + <p data-sourcepos="12:7-12:16">item 7</p> + <p data-sourcepos="14:7-14:20">paragraph text</p> + <ol data-sourcepos="16:7-20:19"> + <li data-sourcepos="16:7-19:0"> + <p data-sourcepos="16:10-16:19">item 8</p> + <p data-sourcepos="18:10-18:23">paragraph text</p> + </li> + <li data-sourcepos="20:7-20:19"> + <p data-sourcepos="20:10-20:19">item 9</p> + </li> + </ol> + </li> + <li data-sourcepos="21:4-21:17"> + <p data-sourcepos="21:7-21:17">item 10</p> + </li> + </ol> + </li> + </ol> + */ + + it('deletes item with no children', () => { + const sourcepos = '4:4-4:14'; + const newDescription = `Tasks + +1. [ ] item 1 + 1. [ ] item 3 + 1. [ ] item 4 + 1. [ ] item 5 + 1. [ ] item 6 + + paragraph text + + 1. [ ] item 7 + + paragraph text + + 1. [ ] item 8 + + paragraph text + + 1. [ ] item 9 + 1. [ ] item 10`; + + expect(convertDescriptionWithDeletedTaskListItem(description, sourcepos)).toBe( + newDescription, + ); + }); + + it('deletes deeply nested item with no children', () => { + const sourcepos = '6:7-6:19'; + const newDescription = `Tasks + +1. [ ] item 1 + 1. [ ] item 2 + 1. [ ] item 3 + 1. [ ] item 5 + 1. [ ] item 6 + + paragraph text + + 1. [ ] item 7 + + paragraph text + + 1. [ ] item 8 + + paragraph text + + 1. [ ] item 9 + 1. [ ] item 10`; + + expect(convertDescriptionWithDeletedTaskListItem(description, sourcepos)).toBe( + newDescription, + ); + }); + + it('deletes item with children and moves sub-tasks up a level', () => { + const sourcepos = '5:4-7:19'; + const newDescription = `Tasks + +1. [ ] item 1 + 1. [ ] item 2 + 1. [ ] item 4 + 1. [ ] item 5 + 1. [ ] item 6 + + paragraph text + + 1. [ ] item 7 + + paragraph text + + 1. [ ] item 8 + + paragraph text + + 1. [ ] item 9 + 1. [ ] item 10`; + + expect(convertDescriptionWithDeletedTaskListItem(description, sourcepos)).toBe( + newDescription, + ); + }); + + it('deletes item with associated paragraph text', () => { + const sourcepos = '8:4-11:0'; + const newDescription = `Tasks + +1. [ ] item 1 + 1. [ ] item 2 + 1. [ ] item 3 + 1. [ ] item 4 + 1. [ ] item 5 + 1. [ ] item 7 + + paragraph text + + 1. [ ] item 8 + + paragraph text + + 1. [ ] item 9 + 1. [ ] item 10`; + + expect(convertDescriptionWithDeletedTaskListItem(description, sourcepos)).toBe( + newDescription, + ); + }); + + it('deletes item with associated paragraph text and moves sub-tasks up a level', () => { + const sourcepos = '12:4-20:19'; + const newDescription = `Tasks + +1. [ ] item 1 + 1. [ ] item 2 + 1. [ ] item 3 + 1. [ ] item 4 + 1. [ ] item 5 + 1. [ ] item 6 + + paragraph text + + 1. [ ] item 8 + + paragraph text + + 1. [ ] item 9 + 1. [ ] item 10`; + + expect(convertDescriptionWithDeletedTaskListItem(description, sourcepos)).toBe( + newDescription, + ); + }); + }); }); diff --git a/spec/frontend/members/utils_spec.js b/spec/frontend/members/utils_spec.js index 9f200324c02..4f276e8c9df 100644 --- a/spec/frontend/members/utils_spec.js +++ b/spec/frontend/members/utils_spec.js @@ -166,9 +166,9 @@ describe('Members Utils', () => { describe('canDisableTwoFactor', () => { it.each` - member | expected - ${{ ...memberMock, canGetTwoFactorDisabled: true }} | ${false} - ${{ ...memberMock, canGetTwoFactorDisabled: false }} | ${false} + member | expected + ${{ ...memberMock, canDisableTwoFactor: true }} | ${false} + ${{ ...memberMock, canDisableTwoFactor: false }} | ${false} `( 'returns $expected for members whose two factor authentication can be disabled', ({ member, expected }) => { diff --git a/spec/rubocop/cop/gitlab/doc_url_spec.rb b/spec/rubocop/cop/gitlab/doc_url_spec.rb new file mode 100644 index 00000000000..4a7ef14ccbc --- /dev/null +++ b/spec/rubocop/cop/gitlab/doc_url_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' +require_relative '../../../../rubocop/cop/gitlab/doc_url' + +RSpec.describe RuboCop::Cop::Gitlab::DocUrl, feature_category: :not_owned do + context 'when string literal is added with docs url prefix' do + context 'when inlined' do + it 'registers an offense' do + expect_offense(<<~RUBY) + 'See [the docs](https://docs.gitlab.com/ee/user/permissions#roles).' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#help_page_url` instead of directly including link. See https://docs.gitlab.com/ee/development/documentation/#linking-to-help-in-ruby. + RUBY + end + end + + context 'when multilined' do + it 'registers an offense' do + expect_offense(<<~'RUBY') + 'See the docs: ' \ + 'https://docs.gitlab.com/ee/user/permissions#roles' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#help_page_url` instead of directly including link. See https://docs.gitlab.com/ee/development/documentation/#linking-to-help-in-ruby. + RUBY + end + end + + context 'with heredoc' do + it 'registers an offense' do + expect_offense(<<~RUBY) + <<-HEREDOC + See the docs: + https://docs.gitlab.com/ee/user/permissions#roles + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#help_page_url` instead of directly including link. See https://docs.gitlab.com/ee/development/documentation/#linking-to-help-in-ruby. + HEREDOC + RUBY + end + end + end + + context 'when string literal is added without docs url prefix' do + context 'when inlined' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + '[The DevSecOps Platform](https://about.gitlab.com/)' + RUBY + end + end + + context 'when multilined' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + 'The DevSecOps Platform: ' \ + 'https://about.gitlab.com/' + RUBY + end + end + + context 'with heredoc' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + <<-HEREDOC + The DevSecOps Platform: + https://about.gitlab.com/ + HEREDOC + RUBY + end + end + end +end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 1ad9234c939..7e59a4e5bc5 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1571,25 +1571,25 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end end - describe '#removed_milestone_issue' do + describe '#removed_milestone on Issue' do context do let(:milestone) { create(:milestone, project: project, issues: [issue]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } it_behaves_like 'altered milestone notification on issue' do before do - notification.removed_milestone_issue(issue, issue.author) + notification.removed_milestone(issue, issue.author) end end it_behaves_like 'project emails are disabled' do let(:notification_target) { issue } - let(:notification_trigger) { notification.removed_milestone_issue(issue, issue.author) } + let(:notification_trigger) { notification.removed_milestone(issue, issue.author) } end it_behaves_like 'participating by confidential note notification' do let(:issuable) { issue } - let(:notification_trigger) { notification.removed_milestone_issue(issue, issue.author) } + let(:notification_trigger) { notification.removed_milestone(issue, issue.author) } end end @@ -1615,7 +1615,7 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do reset_delivered_emails! - notification.removed_milestone_issue(confidential_issue, @u_disabled) + notification.removed_milestone(confidential_issue, @u_disabled) should_not_email(non_member) should_not_email(guest) @@ -1627,20 +1627,20 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end end - describe '#changed_milestone_issue' do + describe '#changed_milestone on Issue' do context do let(:new_milestone) { create(:milestone, project: project, issues: [issue]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } it_behaves_like 'altered milestone notification on issue' do before do - notification.changed_milestone_issue(issue, new_milestone, issue.author) + notification.changed_milestone(issue, new_milestone, issue.author) end end it_behaves_like 'project emails are disabled' do let(:notification_target) { issue } - let(:notification_trigger) { notification.changed_milestone_issue(issue, new_milestone, issue.author) } + let(:notification_trigger) { notification.changed_milestone(issue, new_milestone, issue.author) } end end @@ -1666,7 +1666,7 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do reset_delivered_emails! - notification.changed_milestone_issue(confidential_issue, new_milestone, @u_disabled) + notification.changed_milestone(confidential_issue, new_milestone, @u_disabled) should_not_email(non_member) should_not_email(guest) @@ -2395,35 +2395,35 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end end - describe '#removed_milestone_merge_request' do + describe '#removed_milestone on MergeRequest' do let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } it_behaves_like 'altered milestone notification on merge request' do before do - notification.removed_milestone_merge_request(merge_request, merge_request.author) + notification.removed_milestone(merge_request, merge_request.author) end end it_behaves_like 'project emails are disabled' do let(:notification_target) { merge_request } - let(:notification_trigger) { notification.removed_milestone_merge_request(merge_request, merge_request.author) } + let(:notification_trigger) { notification.removed_milestone(merge_request, merge_request.author) } end end - describe '#changed_milestone_merge_request' do + describe '#changed_milestone on MergeRequest' do let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } it_behaves_like 'altered milestone notification on merge request' do before do - notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) + notification.changed_milestone(merge_request, new_milestone, merge_request.author) end end it_behaves_like 'project emails are disabled' do let(:notification_target) { merge_request } - let(:notification_trigger) { notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) } + let(:notification_trigger) { notification.changed_milestone(merge_request, new_milestone, merge_request.author) } end end |