diff options
68 files changed, 1385 insertions, 508 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index d2bae1b21b3..4a7197e3bd5 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -193,6 +193,6 @@ overrides: '@graphql-eslint/no-unused-fragments': error '@graphql-eslint/no-duplicate-fields': error - files: - - 'spec/contracts/consumer/**/*' + - '{,ee/}spec/contracts/consumer/**/*' rules: '@gitlab/require-i18n-strings': off diff --git a/app/assets/javascripts/admin/users/components/actions/ban.vue b/app/assets/javascripts/admin/users/components/actions/ban.vue index 898a688c203..55938832dce 100644 --- a/app/assets/javascripts/admin/users/components/actions/ban.vue +++ b/app/assets/javascripts/admin/users/components/actions/ban.vue @@ -11,9 +11,7 @@ const messageHtml = ` <ul> <li>${s__("AdminUsers|The user can't log in.")}</li> <li>${s__("AdminUsers|The user can't access git repositories.")}</li> - <li>${s__( - 'AdminUsers|Issues and merge requests authored by this user are hidden from other users.', - )}</li> + <li>${s__('AdminUsers|Issues authored by this user are hidden from other users.')}</li> </ul> <p>${s__('AdminUsers|You can unban their account in the future. Their data remains intact.')}</p> <p>${sprintf( diff --git a/app/assets/javascripts/issuable/components/issuable_header_warnings.vue b/app/assets/javascripts/issuable/components/issuable_header_warnings.vue index a84187ab86b..543dca0afe1 100644 --- a/app/assets/javascripts/issuable/components/issuable_header_warnings.vue +++ b/app/assets/javascripts/issuable/components/issuable_header_warnings.vue @@ -1,7 +1,7 @@ <script> import { GlIcon, GlTooltipDirective } from '@gitlab/ui'; import { mapGetters } from 'vuex'; -import { sprintf, __ } from '~/locale'; +import { __ } from '~/locale'; import { IssuableType, WorkspaceType } from '~/issues/constants'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import ConfidentialityBadge from '~/vue_shared/components/confidentiality_badge.vue'; @@ -40,9 +40,7 @@ export default { iconName: 'spam', visible: this.hidden, dataTestId: 'hidden', - tooltip: sprintf(__('This %{issuable} is hidden because its author has been banned'), { - issuable: this.getNoteableData.targetType.replace('_', ' '), - }), + tooltip: __('This issue is hidden because its author has been banned'), }, ]; }, diff --git a/app/assets/javascripts/work_items/components/notes/system_note.vue b/app/assets/javascripts/work_items/components/notes/system_note.vue new file mode 100644 index 00000000000..f91a0d01581 --- /dev/null +++ b/app/assets/javascripts/work_items/components/notes/system_note.vue @@ -0,0 +1,229 @@ +<script> +/** + * Common component to render a system note, icon and user information. + * + * This component need not be used with any store neither has any vuex dependency + * + * @example + * <system-note + * :note="{ + * id: String, + * author: Object, + * createdAt: String, + * bodyHtml: String, + * systemNoteIconName: String + * }" + * /> + */ +import { GlButton, GlSkeletonLoader, GlTooltipDirective, GlIcon } from '@gitlab/ui'; +import $ from 'jquery'; +import SafeHtml from '~/vue_shared/directives/safe_html'; +import descriptionVersionHistoryMixin from 'ee_else_ce/notes/mixins/description_version_history'; +import '~/behaviors/markdown/render_gfm'; +import axios from '~/lib/utils/axios_utils'; +import { getLocationHash } from '~/lib/utils/url_utility'; +import { __ } from '~/locale'; +import NoteHeader from '~/notes/components/note_header.vue'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; + +const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; + +export default { + i18n: { + deleteButtonLabel: __('Remove description history'), + }, + name: 'SystemNote', + components: { + GlIcon, + NoteHeader, + TimelineEntryItem, + GlButton, + GlSkeletonLoader, + }, + directives: { + GlTooltip: GlTooltipDirective, + SafeHtml, + }, + mixins: [descriptionVersionHistoryMixin, glFeatureFlagsMixin()], + props: { + note: { + type: Object, + required: true, + }, + }, + data() { + return { + expanded: false, + lines: [], + showLines: false, + loadingDiff: false, + isLoadingDescriptionVersion: false, + }; + }, + computed: { + targetNoteHash() { + return getLocationHash(); + }, + descriptionVersions() { + return []; + }, + noteAnchorId() { + return `note_${this.note.id}`; + }, + isTargetNote() { + return this.targetNoteHash === this.noteAnchorId; + }, + toggleIcon() { + return this.expanded ? 'chevron-up' : 'chevron-down'; + }, + // following 2 methods taken from code in `collapseLongCommitList` of notes.js: + actionTextHtml() { + return $(this.note.bodyHtml).unwrap().html(); + }, + hasMoreCommits() { + return $(this.note.bodyHtml).filter('ul').children().length > MAX_VISIBLE_COMMIT_LIST_COUNT; + }, + descriptionVersion() { + return this.descriptionVersions[this.note.description_version_id]; + }, + }, + mounted() { + $(this.$refs['gfm-content']).renderGFM(); + }, + methods: { + fetchDescriptionVersion() {}, + softDeleteDescriptionVersion() {}, + + async toggleDiff() { + this.showLines = !this.showLines; + + if (!this.lines.length) { + this.loadingDiff = true; + const { data } = await axios.get(this.note.outdated_line_change_path); + + this.lines = data.map((l) => ({ + ...l, + rich_text: l.rich_text.replace(/^[+ -]/, ''), + })); + this.loadingDiff = false; + } + }, + }, + safeHtmlConfig: { + ADD_TAGS: ['use'], // to support icon SVGs + }, + userColorSchemeClass: window.gon.user_color_scheme, +}; +</script> + +<template> + <timeline-entry-item + :id="noteAnchorId" + :class="{ target: isTargetNote, 'pr-0': shouldShowDescriptionVersion }" + class="note system-note note-wrapper" + > + <div class="timeline-icon"><gl-icon :name="note.systemNoteIconName" /></div> + <div class="timeline-content"> + <div class="note-header"> + <note-header + :author="note.author" + :created-at="note.createdAt" + :note-id="note.id" + :is-system-note="true" + > + <span ref="gfm-content" v-safe-html="actionTextHtml"></span> + <template + v-if="canSeeDescriptionVersion || note.outdated_line_change_path" + #extra-controls + > + · + <gl-button + v-if="canSeeDescriptionVersion" + variant="link" + :icon="descriptionVersionToggleIcon" + data-testid="compare-btn" + class="gl-vertical-align-text-bottom gl-font-sm!" + @click="toggleDescriptionVersion" + >{{ __('Compare with previous version') }}</gl-button + > + <gl-button + v-if="note.outdated_line_change_path" + :icon="showLines ? 'chevron-up' : 'chevron-down'" + variant="link" + data-testid="outdated-lines-change-btn" + class="gl-vertical-align-text-bottom gl-font-sm!" + @click="toggleDiff" + > + {{ __('Compare changes') }} + </gl-button> + </template> + </note-header> + </div> + <div class="note-body"> + <div + v-safe-html="note.bodyHtml" + :class="{ 'system-note-commit-list': hasMoreCommits, 'hide-shade': expanded }" + class="note-text md" + ></div> + <div v-if="hasMoreCommits" class="flex-list"> + <div class="system-note-commit-list-toggler flex-row" @click="expanded = !expanded"> + <gl-icon :name="toggleIcon" :size="8" class="gl-mr-2" /> + <span>{{ __('Toggle commit list') }}</span> + </div> + </div> + <div v-if="shouldShowDescriptionVersion" class="description-version pt-2"> + <pre v-if="isLoadingDescriptionVersion" class="loading-state"> + <gl-skeleton-loader /> + </pre> + <pre v-else v-safe-html="descriptionVersion" class="wrapper mt-2"></pre> + <gl-button + v-if="displayDeleteButton" + v-gl-tooltip + :title="$options.i18n.deleteButtonLabel" + :aria-label="$options.i18n.deleteButtonLabel" + variant="default" + category="tertiary" + icon="remove" + class="delete-description-history" + data-testid="delete-description-version-button" + @click="deleteDescriptionVersion" + /> + </div> + <div + v-if="lines.length && showLines" + class="diff-content outdated-lines-wrapper gl-border-solid gl-border-1 gl-border-gray-200 gl-mt-4 gl-rounded-small gl-overflow-hidden" + > + <table + :class="$options.userColorSchemeClass" + class="code js-syntax-highlight" + data-testid="outdated-lines" + > + <tr v-for="line in lines" v-once :key="line.line_code" class="line_holder"> + <td + :class="line.type" + class="diff-line-num old_line gl-border-bottom-0! gl-border-top-0! gl-border-0! gl-rounded-0!" + > + {{ line.old_line }} + </td> + <td + :class="line.type" + class="diff-line-num new_line gl-border-bottom-0! gl-border-top-0!" + > + {{ line.new_line }} + </td> + <td + :class="line.type" + class="line_content gl-display-table-cell! gl-border-0! gl-rounded-0!" + v-html="line.rich_text /* eslint-disable-line vue/no-v-html */" + ></td> + </tr> + </table> + </div> + <div v-else-if="showLines" class="mt-4"> + <gl-skeleton-loader /> + </div> + </div> + </div> + </timeline-entry-item> +</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 e0ebc714dbb..4c5c5eb9de9 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail.vue @@ -30,6 +30,7 @@ import { WIDGET_TYPE_ITERATION, WORK_ITEM_TYPE_VALUE_ISSUE, WORK_ITEM_TYPE_VALUE_OBJECTIVE, + WIDGET_TYPE_NOTES, } from '../constants'; import workItemDatesSubscription from '../graphql/work_item_dates.subscription.graphql'; @@ -49,6 +50,7 @@ import WorkItemDueDate from './work_item_due_date.vue'; import WorkItemAssignees from './work_item_assignees.vue'; import WorkItemLabels from './work_item_labels.vue'; import WorkItemMilestone from './work_item_milestone.vue'; +import WorkItemNotes from './work_item_notes.vue'; export default { i18n, @@ -75,6 +77,7 @@ export default { WorkItemIteration: () => import('ee_component/work_items/components/work_item_iteration.vue'), WorkItemMilestone, WorkItemTree, + WorkItemNotes, }, mixins: [glFeatureFlagMixin()], inject: ['fullPath'], @@ -258,6 +261,9 @@ export default { workItemMilestone() { return this.isWidgetPresent(WIDGET_TYPE_MILESTONE); }, + workItemNotes() { + return this.isWidgetPresent(WIDGET_TYPE_NOTES); + }, fetchByIid() { return this.glFeatures.useIidInWorkItemsPath && parseBoolean(getParameterByName('iid_path')); }, @@ -428,7 +434,7 @@ export default { <div class="gl-display-flex gl-align-items-center" data-testid="work-item-body"> <ul v-if="parentWorkItem" - class="list-unstyled gl-display-flex gl-mr-auto gl-max-w-26 gl-md-max-w-50p gl-min-w-0 gl-mb-0" + class="list-unstyled gl-display-flex gl-mr-auto gl-max-w-26 gl-md-max-w-50p gl-min-w-0 gl-mb-0 gl-z-index-0" data-testid="work-item-parent" > <li class="gl-ml-n4 gl-display-flex gl-align-items-center gl-overflow-hidden"> @@ -589,6 +595,17 @@ export default { @addWorkItemChild="addChild" @removeChild="removeChild" /> + <template v-if="workItemsMvc2Enabled"> + <work-item-notes + v-if="workItemNotes" + :work-item-id="workItem.id" + :query-variables="queryVariables" + :full-path="fullPath" + :fetch-by-iid="fetchByIid" + class="gl-pt-5" + @error="updateError = $event" + /> + </template> <gl-empty-state v-if="error" :title="$options.i18n.fetchErrorTitle" diff --git a/app/assets/javascripts/work_items/components/work_item_detail_modal.vue b/app/assets/javascripts/work_items/components/work_item_detail_modal.vue index a6ef8886d71..e8726814aaf 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail_modal.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail_modal.vue @@ -139,6 +139,7 @@ export default { size="lg" modal-id="work-item-detail-modal" header-class="gl-p-0 gl-pb-2!" + scrollable @hide="closeModal" > <gl-alert v-if="error" variant="danger" @dismiss="error = false"> diff --git a/app/assets/javascripts/work_items/components/work_item_notes.vue b/app/assets/javascripts/work_items/components/work_item_notes.vue new file mode 100644 index 00000000000..91e90589a93 --- /dev/null +++ b/app/assets/javascripts/work_items/components/work_item_notes.vue @@ -0,0 +1,109 @@ +<script> +import { GlSkeletonLoader } from '@gitlab/ui'; +import { s__ } from '~/locale'; +import SystemNote from '~/work_items/components/notes/system_note.vue'; +import { i18n, DEFAULT_PAGE_SIZE_NOTES } from '~/work_items/constants'; +import { getWorkItemNotesQuery } from '~/work_items/utils'; + +export default { + i18n: { + ACTIVITY_LABEL: s__('WorkItem|Activity'), + }, + loader: { + repeat: 10, + width: 1000, + height: 40, + }, + components: { + SystemNote, + GlSkeletonLoader, + }, + props: { + workItemId: { + type: String, + required: true, + }, + queryVariables: { + type: Object, + required: true, + }, + fullPath: { + type: String, + required: true, + }, + fetchByIid: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + areNotesLoading() { + return this.$apollo.queries.workItemNotes.loading; + }, + notes() { + return this.workItemNotes?.nodes; + }, + pageInfo() { + return this.workItemNotes?.pageInfo; + }, + }, + apollo: { + workItemNotes: { + query() { + return getWorkItemNotesQuery(this.fetchByIid); + }, + context: { + isSingleRequest: true, + }, + variables() { + return { + ...this.queryVariables, + pageSize: DEFAULT_PAGE_SIZE_NOTES, + }; + }, + update(data) { + const workItemWidgets = this.fetchByIid + ? data.workspace?.workItems?.nodes[0]?.widgets + : data.workItem?.widgets; + return workItemWidgets.find((widget) => widget.type === 'NOTES').discussions || []; + }, + skip() { + return !this.queryVariables.id && !this.queryVariables.iid; + }, + error() { + this.$emit('error', i18n.fetchError); + }, + }, + }, +}; +</script> + +<template> + <div class="gl-border-t gl-mt-5"> + <label class="gl-mb-0">{{ $options.i18n.ACTIVITY_LABEL }}</label> + <div v-if="areNotesLoading" class="gl-mt-5"> + <gl-skeleton-loader + v-for="index in $options.loader.repeat" + :key="index" + :width="$options.loader.width" + :height="$options.loader.height" + preserve-aspect-ratio="xMinYMax meet" + > + <circle cx="20" cy="20" r="16" /> + <rect width="500" x="45" y="15" height="10" rx="4" /> + </gl-skeleton-loader> + </div> + <div v-else class="issuable-discussion gl-mb-5 work-item-notes"> + <template v-if="notes && notes.length"> + <ul class="notes main-notes-list timeline"> + <system-note + v-for="note in notes" + :key="note.notes.nodes[0].id" + :note="note.notes.nodes[0]" + /> + </ul> + </template> + </div> + </div> +</template> diff --git a/app/assets/javascripts/work_items/constants.js b/app/assets/javascripts/work_items/constants.js index 368bb6a85a4..791f06a612e 100644 --- a/app/assets/javascripts/work_items/constants.js +++ b/app/assets/javascripts/work_items/constants.js @@ -19,6 +19,7 @@ export const WIDGET_TYPE_WEIGHT = 'WEIGHT'; export const WIDGET_TYPE_HIERARCHY = 'HIERARCHY'; export const WIDGET_TYPE_MILESTONE = 'MILESTONE'; export const WIDGET_TYPE_ITERATION = 'ITERATION'; +export const WIDGET_TYPE_NOTES = 'NOTES'; export const WORK_ITEM_TYPE_ENUM_INCIDENT = 'INCIDENT'; export const WORK_ITEM_TYPE_ENUM_ISSUE = 'ISSUE'; @@ -145,3 +146,4 @@ export const FORM_TYPES = { }; export const DEFAULT_PAGE_SIZE_ASSIGNEES = 10; +export const DEFAULT_PAGE_SIZE_NOTES = 100; diff --git a/app/assets/javascripts/work_items/graphql/discussion.fragment.graphql b/app/assets/javascripts/work_items/graphql/discussion.fragment.graphql new file mode 100644 index 00000000000..62ced6bdfea --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/discussion.fragment.graphql @@ -0,0 +1,12 @@ +#import "~/graphql_shared/fragments/user.fragment.graphql" + +fragment Discussion on Note { + id + body + bodyHtml + systemNoteIconName + createdAt + author { + ...User + } +} diff --git a/app/assets/javascripts/work_items/graphql/work_item_notes.query.graphql b/app/assets/javascripts/work_items/graphql/work_item_notes.query.graphql new file mode 100644 index 00000000000..9439f22f955 --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_notes.query.graphql @@ -0,0 +1,27 @@ +#import "~/graphql_shared/fragments/page_info.fragment.graphql" +#import "~/work_items/graphql/discussion.fragment.graphql" + +query workItemNotes($id: WorkItemID!, $after: String, $pageSize: Int) { + workItem(id: $id) { + id + iid + widgets { + ... on WorkItemWidgetNotes { + type + discussions(first: $pageSize, after: $after, filter: ONLY_ACTIVITY) { + pageInfo { + ...PageInfo + } + nodes { + id + notes { + nodes { + ...Discussion + } + } + } + } + } + } + } +} diff --git a/app/assets/javascripts/work_items/graphql/work_item_notes_by_iid.query.graphql b/app/assets/javascripts/work_items/graphql/work_item_notes_by_iid.query.graphql new file mode 100644 index 00000000000..3e0960f3f54 --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_notes_by_iid.query.graphql @@ -0,0 +1,32 @@ +#import "~/graphql_shared/fragments/page_info.fragment.graphql" +#import "~/work_items/graphql/discussion.fragment.graphql" + +query workItemNotesByIid($fullPath: ID!, $iid: String, $after: String, $pageSize: Int) { + workspace: project(fullPath: $fullPath) { + id + workItems(iid: $iid) { + nodes { + id + iid + widgets { + ... on WorkItemWidgetNotes { + type + discussions(first: $pageSize, after: $after, filter: ONLY_ACTIVITY) { + pageInfo { + ...PageInfo + } + nodes { + id + notes { + nodes { + ...Discussion + } + } + } + } + } + } + } + } + } +} 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 9b802a8e8fc..a8d4392c1a5 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 @@ -79,4 +79,7 @@ fragment WorkItemWidgets on WorkItemWidget { ...MilestoneFragment } } + ... on WorkItemWidgetNotes { + type + } } diff --git a/app/assets/javascripts/work_items/utils.js b/app/assets/javascripts/work_items/utils.js index 17f9c882c2d..e58fd19ea31 100644 --- a/app/assets/javascripts/work_items/utils.js +++ b/app/assets/javascripts/work_items/utils.js @@ -1,6 +1,12 @@ import workItemQuery from './graphql/work_item.query.graphql'; import workItemByIidQuery from './graphql/work_item_by_iid.query.graphql'; +import workItemNotesIdQuery from './graphql/work_item_notes.query.graphql'; +import workItemNotesByIidQuery from './graphql/work_item_notes_by_iid.query.graphql'; export function getWorkItemQuery(isFetchedByIid) { return isFetchedByIid ? workItemByIidQuery : workItemQuery; } + +export function getWorkItemNotesQuery(isFetchedByIid) { + return isFetchedByIid ? workItemNotesByIidQuery : workItemNotesIdQuery; +} diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 76b06b2ce9d..d8da448a323 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -13,10 +13,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont @issuable = @merge_request ||= merge_request_includes(@project.merge_requests).find_by_iid!(params[:id]) - - return render_404 unless can?(current_user, :read_merge_request, @issuable) - - @issuable end def merge_request_includes(association) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index c5a3293ad2f..5fcb81949ee 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -248,10 +248,7 @@ class IssuableFinder end def init_collection - return klass.all if params.user_can_see_all_issuables? - - # Only admins and auditors can see hidden issuables, for other users we filter out hidden issuables - klass.without_hidden + klass.all end def default_or_simple_sort? diff --git a/app/finders/issuable_finder/params.rb b/app/finders/issuable_finder/params.rb index 4e17f06e1c1..32d50802537 100644 --- a/app/finders/issuable_finder/params.rb +++ b/app/finders/issuable_finder/params.rb @@ -195,11 +195,6 @@ class IssuableFinder project || group end - def user_can_see_all_issuables? - Ability.allowed?(current_user, :read_all_resources) - end - strong_memoize_attr :user_can_see_all_issuables?, :user_can_see_all_issuables - private def projects_public_or_visible_to_user diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index bd81f06f93b..e12dce744b5 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -49,7 +49,7 @@ class IssuesFinder < IssuableFinder # rubocop: disable CodeReuse/ActiveRecord def with_confidentiality_access_check - return model_class.all if params.user_can_see_all_issuables? + return model_class.all if params.user_can_see_all_issues? # Only admins can see hidden issues, so for non-admins, we filter out any hidden issues issues = model_class.without_hidden diff --git a/app/finders/issues_finder/params.rb b/app/finders/issues_finder/params.rb index 786bfbd4113..7f8acb79ed6 100644 --- a/app/finders/issues_finder/params.rb +++ b/app/finders/issues_finder/params.rb @@ -44,7 +44,7 @@ class IssuesFinder if parent Ability.allowed?(current_user, :read_confidential_issues, parent) else - user_can_see_all_issuables? + user_can_see_all_issues? end end end @@ -54,6 +54,12 @@ class IssuesFinder current_user.blank? end + + def user_can_see_all_issues? + strong_memoize(:user_can_see_all_issues) do + Ability.allowed?(current_user, :read_all_resources) + end + end end end diff --git a/app/graphql/resolvers/concerns/resolves_merge_requests.rb b/app/graphql/resolvers/concerns/resolves_merge_requests.rb index c68e120ee24..d56951bc821 100644 --- a/app/graphql/resolvers/concerns/resolves_merge_requests.rb +++ b/app/graphql/resolvers/concerns/resolves_merge_requests.rb @@ -34,7 +34,7 @@ module ResolvesMergeRequests end def unconditional_includes - [:target_project, :author] + [:target_project] end def preloads diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 7d99b0da890..2b21d8c51e6 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -275,7 +275,7 @@ module IssuablesHelper zoomMeetingUrl: ZoomMeeting.canonical_meeting_url(issuable), sentryIssueIdentifier: SentryIssue.find_by(issue: issuable)&.sentry_issue_identifier, # rubocop:disable CodeReuse/ActiveRecord iid: issuable.iid.to_s, - isHidden: issuable_hidden?(issuable), + isHidden: issue_hidden?(issuable), canCreateIncident: create_issue_type_allowed?(issuable.project, :incident) } end @@ -372,20 +372,6 @@ module IssuablesHelper end end - def issuable_hidden?(issuable) - Feature.enabled?(:ban_user_feature_flag) && issuable.hidden? - end - - def hidden_issuable_icon(issuable) - return unless issuable_hidden?(issuable) - - title = format(_('This %{issuable} is hidden because its author has been banned'), - issuable: issuable.human_class_name) - content_tag(:span, class: 'has-tooltip', title: title) do - sprite_icon('spam', css_class: 'gl-vertical-align-text-bottom') - end - end - private def sidebar_gutter_collapsed? diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 101df8cdd41..1d68dccc741 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -70,6 +70,18 @@ module IssuesHelper sprite_icon('eye-slash', css_class: 'gl-vertical-align-text-bottom') if issue.confidential? end + def issue_hidden?(issue) + Feature.enabled?(:ban_user_feature_flag) && issue.hidden? + end + + def hidden_issue_icon(issue) + return unless issue_hidden?(issue) + + content_tag(:span, class: 'has-tooltip', title: _('This issue is hidden because its author has been banned')) do + sprite_icon('spam', css_class: 'gl-vertical-align-text-bottom') + end + end + def award_user_list(awards, current_user, limit: 10) names = awards.map do |award| award.user == current_user ? 'You' : award.user.name diff --git a/app/models/bulk_imports/tracker.rb b/app/models/bulk_imports/tracker.rb index 357f4629078..b04ef1cb7ae 100644 --- a/app/models/bulk_imports/tracker.rb +++ b/app/models/bulk_imports/tracker.rb @@ -26,7 +26,7 @@ class BulkImports::Tracker < ApplicationRecord entity_scope = where(bulk_import_entity_id: entity_id) next_stage_scope = entity_scope.with_status(:created).select('MIN(stage)') - entity_scope.where(stage: next_stage_scope) + entity_scope.where(stage: next_stage_scope).with_status(:created) } def self.stage_running?(entity_id, stage) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 1a50ebde0a3..9f0cd96a8f8 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -144,14 +144,6 @@ module Issuable includes(*associations) end - scope :without_hidden, -> { - if Feature.enabled?(:ban_user_feature_flag) - where.not(author_id: Users::BannedUser.all.select(:user_id)) - else - all - end - } - attr_mentionable :title, pipeline: :single_line attr_mentionable :description @@ -235,10 +227,6 @@ module Issuable issuable_severity&.severity || IssuableSeverity::DEFAULT end - def hidden? - author&.banned? - end - private def description_max_length_for_new_records_is_valid diff --git a/app/models/issue.rb b/app/models/issue.rb index f517f42d6ba..b338ecfce88 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -178,6 +178,14 @@ class Issue < ApplicationRecord scope :confidential_only, -> { where(confidential: true) } + scope :without_hidden, -> { + if Feature.enabled?(:ban_user_feature_flag) + where.not(author_id: Users::BannedUser.all.select(:user_id)) + else + all + end + } + scope :counts_by_state, -> { reorder(nil).group(:state_id).count } scope :service_desk, -> { where(author: ::User.support_bot) } @@ -650,6 +658,10 @@ class Issue < ApplicationRecord end end + def hidden? + author&.banned? + end + # Necessary until all issues are backfilled and we add a NOT NULL constraint on the DB def work_item_type super || WorkItems::Type.default_by_type(issue_type) diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index 779384ee3fe..aa07bb7dc5f 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -16,9 +16,6 @@ class IssuablePolicy < BasePolicy condition(:is_incident) { @subject.incident? } - desc "Issuable is hidden" - condition(:hidden, scope: :subject) { @subject.hidden? } - rule { can?(:guest_access) & assignee_or_author & ~is_incident }.policy do enable :read_issue enable :update_issue diff --git a/app/policies/issue_policy.rb b/app/policies/issue_policy.rb index 7d4e42580b8..491eebe9daf 100644 --- a/app/policies/issue_policy.rb +++ b/app/policies/issue_policy.rb @@ -21,6 +21,9 @@ class IssuePolicy < IssuablePolicy desc "Issue is confidential" condition(:confidential, scope: :subject) { @subject.confidential? } + desc "Issue is hidden" + condition(:hidden, scope: :subject) { @subject.hidden? } + desc "Issue is persisted" condition(:persisted, scope: :subject) { @subject.persisted? } diff --git a/app/policies/merge_request_policy.rb b/app/policies/merge_request_policy.rb index 49f9225a1d3..1759cf057e4 100644 --- a/app/policies/merge_request_policy.rb +++ b/app/policies/merge_request_policy.rb @@ -29,10 +29,6 @@ class MergeRequestPolicy < IssuablePolicy enable :update_subscription end - rule { hidden & ~admin }.policy do - prevent :read_merge_request - end - condition(:can_merge) { @subject.can_be_merged_by?(@user) } rule { can_merge }.policy do diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index a34ed332fcf..1d3320e4f82 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -6,7 +6,7 @@ - if issue.confidential? %span.has-tooltip{ title: _('Confidential') } = confidential_icon(issue) - = hidden_issuable_icon(issue) + = hidden_issue_icon(issue) = link_to issue.title, issue_path(issue), class: 'js-prefetch-document' = render_if_exists 'projects/issues/subepic_flag', issue: issue - if issue.tasks? diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index d7e26da5f51..71f8e4c32f5 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -12,7 +12,6 @@ .issuable-main-info .merge-request-title.title %span.merge-request-title-text.js-onboarding-mr-item - = hidden_issuable_icon(merge_request) = link_to merge_request.title, merge_request_path(merge_request), class: 'js-prefetch-document' - if merge_request.tasks? %span.task-status.d-none.d-sm-inline-block diff --git a/app/views/projects/merge_requests/_mr_title.html.haml b/app/views/projects/merge_requests/_mr_title.html.haml index 6129e349df3..a73d2aa5cc4 100644 --- a/app/views/projects/merge_requests/_mr_title.html.haml +++ b/app/views/projects/merge_requests/_mr_title.html.haml @@ -16,7 +16,7 @@ .detail-page-header.border-bottom-0.pt-0.pb-0.gl-display-block{ class: "gl-md-display-flex! #{'is-merge-request' if moved_mr_sidebar_enabled? && !fluid_layout}" } .detail-page-header-body .issuable-meta.gl-display-flex - #js-issuable-header-warnings{ data: { hidden: issuable_hidden?(@merge_request).to_s } } + #js-issuable-header-warnings %h1.title.page-title.gl-font-size-h-display.gl-my-0.gl-display-inline-block{ data: { qa_selector: 'title_content' } } = markdown_field(@merge_request, :title) diff --git a/app/views/shared/issue_type/_details_header.html.haml b/app/views/shared/issue_type/_details_header.html.haml index c8762f8e060..ccb501dae11 100644 --- a/app/views/shared/issue_type/_details_header.html.haml +++ b/app/views/shared/issue_type/_details_header.html.haml @@ -13,7 +13,7 @@ %span.gl-display-none.gl-sm-display-block.gl-ml-2 = _('Open') - #js-issuable-header-warnings{ data: { hidden: issuable_hidden?(issuable).to_s } } + #js-issuable-header-warnings{ data: { hidden: issue_hidden?(issuable).to_s } } = issuable_meta(issuable, @project) %a.btn.gl-button.btn-default.btn-icon.float-right.gl-display-block.d-sm-none.gutter-toggle.issuable-gutter-toggle.js-sidebar-toggle{ href: "#" } diff --git a/app/workers/bulk_imports/entity_worker.rb b/app/workers/bulk_imports/entity_worker.rb index d23d57c33ab..fb99d63d06e 100644 --- a/app/workers/bulk_imports/entity_worker.rb +++ b/app/workers/bulk_imports/entity_worker.rb @@ -74,6 +74,8 @@ module BulkImports source_version: source_version, importer: 'gitlab_migration' ) + + entity.fail_op! end private diff --git a/app/workers/bulk_imports/export_request_worker.rb b/app/workers/bulk_imports/export_request_worker.rb index 1a5f6250429..530419dac26 100644 --- a/app/workers/bulk_imports/export_request_worker.rb +++ b/app/workers/bulk_imports/export_request_worker.rb @@ -4,11 +4,15 @@ module BulkImports class ExportRequestWorker include ApplicationWorker - data_consistency :always - idempotent! - worker_has_external_dependencies! + data_consistency :always feature_category :importers + sidekiq_options dead: false, retry: 5 + worker_has_external_dependencies! + + sidekiq_retries_exhausted do |msg, exception| + new.perform_failure(exception, msg['args'].first) + end def perform(entity_id) entity = BulkImports::Entity.find(entity_id) @@ -18,26 +22,12 @@ module BulkImports request_export(entity) BulkImports::EntityWorker.perform_async(entity_id) - rescue BulkImports::NetworkError => e - if e.retriable?(entity) - retry_request(e, entity) - else - log_exception(e, - { - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - message: "Request to export #{entity.source_type} failed", - source_version: entity.bulk_import.source_version_info.to_s, - importer: 'gitlab_migration' - } - ) - - BulkImports::Failure.create(failure_attributes(e, entity)) - - entity.fail_op! - end + end + + def perform_failure(exception, entity_id) + entity = BulkImports::Entity.find(entity_id) + + log_and_fail(exception, entity) end private @@ -104,30 +94,32 @@ module BulkImports end end - def retry_request(exception, entity) + def logger + @logger ||= Gitlab::Import::Logger.build + end + + def log_exception(exception, payload) + Gitlab::ExceptionLogFormatter.format!(exception, payload) + + logger.error(structured_payload(payload)) + end + + def log_and_fail(exception, entity) log_exception(exception, { - message: 'Retrying export request', bulk_import_entity_id: entity.id, bulk_import_id: entity.bulk_import_id, bulk_import_entity_type: entity.source_type, source_full_path: entity.source_full_path, + message: "Request to export #{entity.source_type} failed", source_version: entity.bulk_import.source_version_info.to_s, importer: 'gitlab_migration' } ) - self.class.perform_in(2.seconds, entity.id) - end - - def logger - @logger ||= Gitlab::Import::Logger.build - end - - def log_exception(exception, payload) - Gitlab::ExceptionLogFormatter.format!(exception, payload) + BulkImports::Failure.create(failure_attributes(exception, entity)) - logger.error(structured_payload(payload)) + entity.fail_op! end end end diff --git a/app/workers/bulk_imports/pipeline_worker.rb b/app/workers/bulk_imports/pipeline_worker.rb index 7eeda3efb9e..62e85d38e61 100644 --- a/app/workers/bulk_imports/pipeline_worker.rb +++ b/app/workers/bulk_imports/pipeline_worker.rb @@ -3,6 +3,7 @@ module BulkImports class PipelineWorker # rubocop:disable Scalability/IdempotentWorker include ApplicationWorker + include ExclusiveLeaseGuard FILE_EXTRACTION_PIPELINE_PERFORM_DELAY = 10.seconds @@ -10,44 +11,24 @@ module BulkImports feature_category :importers sidekiq_options retry: false, dead: false worker_has_external_dependencies! + deduplicate :until_executing def perform(pipeline_tracker_id, stage, entity_id) - @pipeline_tracker = ::BulkImports::Tracker - .with_status(:enqueued) - .find_by_id(pipeline_tracker_id) - - if pipeline_tracker.present? - @entity = @pipeline_tracker.entity - - logger.info( - structured_payload( - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_name: pipeline_tracker.pipeline_name, - message: 'Pipeline starting', - source_version: source_version, - importer: 'gitlab_migration' - ) - ) - - run - else - @entity = ::BulkImports::Entity.find(entity_id) - - logger.error( - structured_payload( - bulk_import_entity_id: entity_id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_tracker_id: pipeline_tracker_id, - message: 'Unstarted pipeline not found', - source_version: source_version, - importer: 'gitlab_migration' - ) - ) + @entity = ::BulkImports::Entity.find(entity_id) + @pipeline_tracker = ::BulkImports::Tracker.find(pipeline_tracker_id) + + try_obtain_lease do + if pipeline_tracker.enqueued? + logger.info(log_attributes(message: 'Pipeline starting')) + + run + else + message = "Pipeline in #{pipeline_tracker.human_status_name} state instead of expected enqueued state" + + logger.error(log_attributes(message: message)) + + fail_tracker(StandardError.new(message)) unless pipeline_tracker.finished? || pipeline_tracker.skipped? + end end ensure @@ -83,29 +64,9 @@ module BulkImports def fail_tracker(exception) pipeline_tracker.update!(status_event: 'fail_op', jid: jid) - log_exception(exception, - { - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_name: pipeline_tracker.pipeline_name, - message: 'Pipeline failed', - source_version: source_version, - importer: 'gitlab_migration' - } - ) + log_exception(exception, log_attributes(message: 'Pipeline failed')) - Gitlab::ErrorTracking.track_exception( - exception, - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_name: pipeline_tracker.pipeline_name, - source_version: source_version, - importer: 'gitlab_migration' - ) + Gitlab::ErrorTracking.track_exception(exception, log_attributes) BulkImports::Failure.create( bulk_import_entity_id: entity.id, @@ -171,18 +132,7 @@ module BulkImports end def retry_tracker(exception) - log_exception(exception, - { - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import_id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_name: pipeline_tracker.pipeline_name, - message: "Retrying pipeline", - source_version: source_version, - importer: 'gitlab_migration' - } - ) + log_exception(exception, log_attributes(message: "Retrying pipeline")) pipeline_tracker.update!(status_event: 'retry', jid: jid) @@ -190,29 +140,43 @@ module BulkImports end def skip_tracker - logger.info( - structured_payload( + logger.info(log_attributes(message: 'Skipping pipeline due to failed entity')) + + pipeline_tracker.update!(status_event: 'skip', jid: jid) + end + + def log_attributes(extra = {}) + structured_payload( + { bulk_import_entity_id: entity.id, bulk_import_id: entity.bulk_import_id, bulk_import_entity_type: entity.source_type, source_full_path: entity.source_full_path, + pipeline_tracker_id: pipeline_tracker.id, pipeline_name: pipeline_tracker.pipeline_name, - message: 'Skipping pipeline due to failed entity', + pipeline_tracker_state: pipeline_tracker.human_status_name, source_version: source_version, importer: 'gitlab_migration' - ) + }.merge(extra) ) - - pipeline_tracker.update!(status_event: 'skip', jid: jid) end def log_exception(exception, payload) Gitlab::ExceptionLogFormatter.format!(exception, payload) + logger.error(structured_payload(payload)) end def time_since_entity_created Time.zone.now - entity.created_at end + + def lease_timeout + 30 + end + + def lease_key + "gitlab:bulk_imports:pipeline_worker:#{pipeline_tracker.id}" + end end end diff --git a/doc/development/testing_guide/contract/index.md b/doc/development/testing_guide/contract/index.md index 8412a260c7d..08a21e58a52 100644 --- a/doc/development/testing_guide/contract/index.md +++ b/doc/development/testing_guide/contract/index.md @@ -26,6 +26,8 @@ The contracts themselves are stored in [`/spec/contracts/contracts`](https://git Before running the consumer tests, go to `spec/contracts/consumer` and run `npm install`. To run all the consumer tests, you just need to run `npm test -- /specs`. Otherwise, to run a specific spec file, replace `/specs` with the specific spec filename. +You can also run tests from the root directory of the project, using the command `yarn jest:contract`. + ### Run the provider tests Before running the provider tests, make sure your GDK (GitLab Development Kit) is fully set up and running. You can follow the setup instructions detailed in the [GDK repository](https://gitlab.com/gitlab-org/gitlab-development-kit/-/tree/main). To run the provider tests, you use Rake tasks that can be found in [`./lib/tasks/contracts`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/lib/tasks/contracts). To get a list of all the Rake tasks related to the provider tests, run `bundle exec rake -T contracts`. For example: diff --git a/doc/user/admin_area/moderate_users.md b/doc/user/admin_area/moderate_users.md index 117781f7222..c0daf029b1f 100644 --- a/doc/user/admin_area/moderate_users.md +++ b/doc/user/admin_area/moderate_users.md @@ -223,7 +223,7 @@ 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 `ban_user_feature_flag`. On GitLab.com, this feature is available to GitLab.com administrators only. -GitLab administrators can ban and unban users. Banned users are blocked, and their issues and merge requests are hidden. +GitLab administrators can ban and unban users. Banned users are blocked, and their issues are hidden. The banned user's comments are still displayed. Hiding a banned user's comments is [tracked in this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/327356). ### Ban a user diff --git a/doc/user/project/import/bitbucket_server.md b/doc/user/project/import/bitbucket_server.md index 1f34c6d4ad9..d7fa1338c55 100644 --- a/doc/user/project/import/bitbucket_server.md +++ b/doc/user/project/import/bitbucket_server.md @@ -63,6 +63,10 @@ The following items are changed when they are imported: ## User assignment +Prerequisite: + +- Authentication token with administrator access. + When issues and pull requests are importing, the importer tries to find the author's email address with a confirmed email address in the GitLab user database. If no such user is available, the project creator is set as the author. The importer appends a note in the comment to mark the diff --git a/doc/user/project/import/github.md b/doc/user/project/import/github.md index 9cd86706a2c..275343f584d 100644 --- a/doc/user/project/import/github.md +++ b/doc/user/project/import/github.md @@ -91,7 +91,10 @@ If you are using a self-managed GitLab instance or if you are importing from Git ### Use a GitHub token -NOTE: +Prerequisite: + +- Authentication token with administrator access. + Using a personal access token to import projects is not recommended. If you are a GitLab.com user, you can use a personal access token to import your project from GitHub, but this method cannot associate all user activity (such as issues and pull requests) with matching GitLab users. @@ -225,7 +228,7 @@ When they are imported, supported GitHub branch protection rules are mapped to e - Project-wide GitLab settings. | GitHub rule | GitLab rule | Introduced in | -|:------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------------------------------------------| +| :---------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------- | :------------------------------------------------------------------ | | **Require conversation resolution before merging** for the project's default branch | **All threads must be resolved** [project setting](../../discussions/index.md#prevent-merge-unless-all-threads-are-resolved) | [GitLab 15.5](https://gitlab.com/gitlab-org/gitlab/-/issues/371110) | | **Require a pull request before merging** | **No one** option in the **Allowed to push** list of [branch protection settings](../protected_branches.md#configure-a-protected-branch) | [GitLab 15.5](https://gitlab.com/gitlab-org/gitlab/-/issues/370951) | | **Require signed commits** for the project's default branch | **Reject unsigned commits** GitLab [push rule](../repository/push_rules.md#prevent-unintended-consequences) **(PREMIUM)** | [GitLab 15.5](https://gitlab.com/gitlab-org/gitlab/-/issues/370949) | diff --git a/jest.config.base.js b/jest.config.base.js index de9bff774e1..05967b51b88 100644 --- a/jest.config.base.js +++ b/jest.config.base.js @@ -26,7 +26,7 @@ module.exports = (path, options = {}) => { ]); } - const glob = `${path}/**/*_spec.js`; + const glob = `${path}/**/*@([._])spec.js`; let testMatch = [`<rootDir>/${glob}`]; if (IS_EE) { testMatch.push(`<rootDir>/ee/${glob}`); diff --git a/jest.config.contract.js b/jest.config.contract.js new file mode 100644 index 00000000000..224d50f87d6 --- /dev/null +++ b/jest.config.contract.js @@ -0,0 +1,6 @@ +module.exports = () => { + return { + modulePaths: ['<rootDir>/spec/contracts/consumer/node_modules/'], + roots: ['spec/contracts/consumer', 'ee/spec/contracts/consumer'], + }; +}; diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 26a07c17c27..420ff20e6d8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3298,7 +3298,7 @@ msgstr "" msgid "AdminUsers|Is using seat" msgstr "" -msgid "AdminUsers|Issues and merge requests authored by this user are hidden from other users." +msgid "AdminUsers|Issues authored by this user are hidden from other users." msgstr "" msgid "AdminUsers|It's you!" @@ -42070,9 +42070,6 @@ msgstr "" msgid "This %{issuableDisplayName} is locked. Only project members can comment." msgstr "" -msgid "This %{issuable} is hidden because its author has been banned" -msgstr "" - msgid "This %{issuable} is locked. Only %{strong_open}project members%{strong_close} can comment." msgstr "" @@ -46875,6 +46872,9 @@ msgstr "" msgid "WorkItem|%{workItemType} deleted" msgstr "" +msgid "WorkItem|Activity" +msgstr "" + msgid "WorkItem|Add" msgstr "" diff --git a/package.json b/package.json index e7e4e6390e8..702abbe91af 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "jest-debug": "node --inspect-brk node_modules/.bin/jest --runInBand", "jest:ci": "jest --config jest.config.js --ci --coverage --testSequencer ./scripts/frontend/parallel_ci_sequencer.js", "jest:ci:minimal": "jest --config jest.config.js --ci --coverage --findRelatedTests $(cat $RSPEC_CHANGED_FILES_PATH) --passWithNoTests --testSequencer ./scripts/frontend/parallel_ci_sequencer.js", + "jest:contract": "PACT_DO_NOT_TRACK=true jest --config jest.config.contract.js --runInBand", "jest:integration": "jest --config jest.config.integration.js", "lint:eslint": "node scripts/frontend/eslint.js", "lint:eslint:fix": "node scripts/frontend/eslint.js --fix", diff --git a/qa/Gemfile b/qa/Gemfile index 9d217a7221a..224fced35dc 100644 --- a/qa/Gemfile +++ b/qa/Gemfile @@ -2,7 +2,7 @@ source 'https://rubygems.org' -gem 'gitlab-qa', '~> 8', '>= 8.13.1', require: 'gitlab/qa' +gem 'gitlab-qa', '~> 8', '>= 8.14.0', require: 'gitlab/qa' gem 'activesupport', '~> 6.1.4.7' # This should stay in sync with the root's Gemfile gem 'allure-rspec', '~> 2.20.0' gem 'capybara', '~> 3.38.0' diff --git a/qa/Gemfile.lock b/qa/Gemfile.lock index 05185a60f16..869a207a7bb 100644 --- a/qa/Gemfile.lock +++ b/qa/Gemfile.lock @@ -100,7 +100,7 @@ GEM gitlab (4.18.0) httparty (~> 0.18) terminal-table (>= 1.5.1) - gitlab-qa (8.13.1) + gitlab-qa (8.14.0) activesupport (~> 6.1) gitlab (~> 4.18.0) http (~> 5.0) @@ -310,7 +310,7 @@ DEPENDENCIES faraday-retry (~> 2.0) fog-core (= 2.1.0) fog-google (~> 1.19) - gitlab-qa (~> 8, >= 8.13.1) + gitlab-qa (~> 8, >= 8.14.0) influxdb-client (~> 2.8) knapsack (~> 4.0) nokogiri (~> 1.13, >= 1.13.10) diff --git a/qa/README.md b/qa/README.md index a0560e1f965..4e2d688aa54 100644 --- a/qa/README.md +++ b/qa/README.md @@ -88,6 +88,43 @@ bundle exec bin/qa Test::Instance::All {GDK IP ADDRESS} - Note: If you want to run tests requiring SSH against GDK, you will need to [modify your GDK setup](https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/run_qa_against_gdk.md). - Note: If this is your first time running GDK, you can use the password pre-set for `root`. [See supported GitLab environment variables](https://gitlab.com/gitlab-org/gitlab-qa/-/blob/master/docs/what_tests_can_be_run.md#supported-gitlab-environment-variables). If you have changed your `root` password, use that when exporting `GITLAB_INITIAL_ROOT_PASSWORD`. +#### Run the end-to-end tests on GitLab in Docker + +1. [GitLab can be installed in Docker](https://docs.gitlab.com/ee/install/docker.html). You can use the following command to start an instance that you can visit at `http://127.0.0.1`: + + ``` + docker run \ + --hostname 127.0.0.1 \ + --publish 80:80 --publish 22:22 \ + --name gitlab \ + --shm-size 256m \ + --env GITLAB_OMNIBUS_CONFIG="gitlab_rails['initial_root_password']='5iveL\!fe';" \ + gitlab/gitlab-ee:nightly + ``` + + Notes: + - If you are on a Mac with [Apple Silicon](https://support.apple.com/en-us/HT211814), you will also need to add: `--platform=linux/amd64` + - If you are on Windows, please be aware that [Docker Desktop must be set to use Linux containers](https://learn.microsoft.com/en-us/virtualization/windowscontainers/quick-start/quick-start-windows-10-linux#run-your-first-linux-container). + + +2. Navigate to the QA folder and run the following commands. + + ```bash + cd gitlab/qa + bundle install + export WEBDRIVER_HEADLESS=false + export GITLAB_INITIAL_ROOT_PASSWORD=5iveL\!fe + export QA_GITLAB_URL="http://127.0.0.1" + ``` + +3. Most tests that do not require special setup could then be run with the following command. + + ```bash + bundle exec rspec <path/to/spec.rb> + ``` + +- Note: See the section above for situations that might require adjustment to the commands or to the configuration of the GitLab instance. [You can find more information in the documentation](https://docs.gitlab.com/ee/install/docker.html). + #### Running EE tests When running EE tests you'll need to have a license available. GitLab engineers can [request a license](https://about.gitlab.com/handbook/developer-onboarding/#working-on-gitlab-ee). diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index 79ea4a8d001..2e97325aff0 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -79,9 +79,12 @@ module QA super end + # @param name [Symbol] name of the data_qa_selector element + # @param page [Class] a target page class to check existence of (class must inherit from QA::Page::Base) + # @param kwargs [Hash] keyword arguments to pass to Capybara finder def click_element(name, page = nil, **kwargs) msg = ["clicking :#{highlight_element(name)}"] - msg << ", expecting to be at #{page.class}" if page + msg << "and ensuring #{page} is present" if page log(msg.join(' '), :info) log("with args #{kwargs}") diff --git a/qa/spec/page/logging_spec.rb b/qa/spec/page/logging_spec.rb index 93a08108787..1a82cda2585 100644 --- a/qa/spec/page/logging_spec.rb +++ b/qa/spec/page/logging_spec.rb @@ -5,6 +5,7 @@ require 'capybara/dsl' RSpec.describe QA::Support::Page::Logging do let(:page) { double.as_null_object } let(:logger) { Gitlab::QA::TestLogger.logger(level: ::Logger::DEBUG, source: 'QA Tests') } + let(:page_class) { class_double('QA::Page::TestPage') } before do allow(QA::Runtime::Logger).to receive(:logger).and_return(logger) @@ -66,6 +67,14 @@ RSpec.describe QA::Support::Page::Logging do .to output(/clicking :element/).to_stdout_from_any_process end + it 'logs click_element with a page' do + allow(page_class).to receive(:validate_elements_present!).and_return(true) + allow(page_class).to receive(:to_s).and_return('QA::Page::TestPage') + + expect { subject.click_element(:element, page_class) } + .to output(/clicking :element and ensuring QA::Page::TestPage is present/).to_stdout_from_any_process + end + it 'logs fill_element' do expect { subject.fill_element(:element, 'foo') } .to output(/filling :element with "foo"/).to_stdout_from_any_process diff --git a/spec/contracts/consumer/.node-version b/spec/contracts/consumer/.node-version deleted file mode 100644 index 18711d290ea..00000000000 --- a/spec/contracts/consumer/.node-version +++ /dev/null @@ -1 +0,0 @@ -14.17.5 diff --git a/spec/contracts/consumer/package.json b/spec/contracts/consumer/package.json index 6d3feaa6d4c..60f268806de 100644 --- a/spec/contracts/consumer/package.json +++ b/spec/contracts/consumer/package.json @@ -22,5 +22,8 @@ "devDependencies": { "@babel/preset-env": "^7.18.2", "babel-jest": "^28.1.1" + }, + "config": { + "pact_do_not_track": true } } diff --git a/spec/contracts/provider/spec_helper.rb b/spec/contracts/provider/spec_helper.rb index 6009d6524e1..44e4d29c18e 100644 --- a/spec/contracts/provider/spec_helper.rb +++ b/spec/contracts/provider/spec_helper.rb @@ -3,6 +3,13 @@ require 'spec_helper' require 'zeitwerk' require_relative 'helpers/users_helper' +require_relative('../../../ee/spec/contracts/provider/spec_helper') if Gitlab.ee? +require Rails.root.join("spec/support/helpers/rails_helpers.rb") +require Rails.root.join("spec/support/helpers/stub_env.rb") + +# Opt out of telemetry collection. We can't allow all engineers, and users who install GitLab from source, to be +# automatically enrolled in sending data on their usage without their knowledge. +ENV['PACT_DO_NOT_TRACK'] = 'true' RSpec.configure do |config| config.include Devise::Test::IntegrationHelpers @@ -19,6 +26,8 @@ end Pact.configure do |config| config.include FactoryBot::Syntax::Methods + config.include RailsHelpers + config.include StubENV end module SpecHelper diff --git a/spec/factories/bulk_import/trackers.rb b/spec/factories/bulk_import/trackers.rb index 22e0aa096fc..3e69ab26801 100644 --- a/spec/factories/bulk_import/trackers.rb +++ b/spec/factories/bulk_import/trackers.rb @@ -7,23 +7,22 @@ FactoryBot.define do stage { 0 } has_next_page { false } sequence(:pipeline_name) { |n| "pipeline_name_#{n}" } + sequence(:jid) { |n| "bulk_import_entity_#{n}" } trait :started do status { 1 } - - sequence(:jid) { |n| "bulk_import_entity_#{n}" } end trait :finished do status { 2 } - - sequence(:jid) { |n| "bulk_import_entity_#{n}" } end trait :failed do status { -1 } + end - sequence(:jid) { |n| "bulk_import_entity_#{n}" } + trait :skipped do + status { -2 } end end end diff --git a/spec/features/merge_request/admin_views_hidden_merge_request_spec.rb b/spec/features/merge_request/admin_views_hidden_merge_request_spec.rb deleted file mode 100644 index 99344d2cf32..00000000000 --- a/spec/features/merge_request/admin_views_hidden_merge_request_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Admin views hidden merge request', feature_category: :code_review do - context 'when signed in as admin and viewing a hidden merge request', :js do - let_it_be(:admin) { create(:admin) } - let_it_be(:author) { create(:user, :banned) } - let_it_be(:project) { create(:project, :repository) } - let!(:merge_request) { create(:merge_request, source_project: project, author: author) } - - before do - sign_in(admin) - gitlab_enable_admin_mode_sign_in(admin) - visit(project_merge_request_path(project, merge_request)) - end - - it 'shows a hidden merge request icon' do - page.within('.detail-page-header-body') do - tooltip = format(_('This %{issuable} is hidden because its author has been banned'), issuable: 'merge request') - expect(page).to have_css("div[data-testid='hidden'][title='#{tooltip}']") - expect(page).to have_css('svg[data-testid="spam-icon"]') - end - end - end -end diff --git a/spec/features/merge_requests/admin_views_hidden_merge_requests_spec.rb b/spec/features/merge_requests/admin_views_hidden_merge_requests_spec.rb deleted file mode 100644 index bc5ec124861..00000000000 --- a/spec/features/merge_requests/admin_views_hidden_merge_requests_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Admin views hidden merge requests', feature_category: :code_review do - context 'when signed in as admin and viewing a hidden merge request' do - let_it_be(:admin) { create(:admin) } - let_it_be(:author) { create(:user, :banned) } - let_it_be(:project) { create(:project) } - let!(:merge_request) { create(:merge_request, source_project: project, author: author) } - - before do - sign_in(admin) - gitlab_enable_admin_mode_sign_in(admin) - visit(project_merge_requests_path(project)) - end - - it 'shows a hidden merge request icon' do - page.within("#merge_request_#{merge_request.id}") do - tooltip = format(_('This %{issuable} is hidden because its author has been banned'), issuable: 'merge request') - expect(page).to have_css("span[title='#{tooltip}']") - expect(page).to have_css('svg[data-testid="spam-icon"]') - end - end - end -end diff --git a/spec/frontend/issuable/components/issuable_header_warnings_spec.js b/spec/frontend/issuable/components/issuable_header_warnings_spec.js index 98a81478cb6..e3a36dc8820 100644 --- a/spec/frontend/issuable/components/issuable_header_warnings_spec.js +++ b/spec/frontend/issuable/components/issuable_header_warnings_spec.js @@ -57,7 +57,6 @@ describe('IssuableHeaderWarnings', () => { beforeEach(() => { store.getters.getNoteableData.confidential = confidentialStatus; store.getters.getNoteableData.discussion_locked = lockStatus; - store.getters.getNoteableData.targetType = issuableType; createComponent({ store, provide: { hidden: hiddenStatus } }); }); @@ -85,7 +84,7 @@ describe('IssuableHeaderWarnings', () => { if (hiddenStatus) { expect(hiddenIcon.attributes('title')).toBe( - `This ${issuableType} is hidden because its author has been banned`, + 'This issue is hidden because its author has been banned', ); expect(getBinding(hiddenIcon.element, 'gl-tooltip')).not.toBeUndefined(); } diff --git a/spec/frontend/work_items/components/notes/system_note_spec.js b/spec/frontend/work_items/components/notes/system_note_spec.js new file mode 100644 index 00000000000..12a45d88843 --- /dev/null +++ b/spec/frontend/work_items/components/notes/system_note_spec.js @@ -0,0 +1,111 @@ +import { GlIcon } from '@gitlab/ui'; +import MockAdapter from 'axios-mock-adapter'; +import { shallowMount } from '@vue/test-utils'; +import $ from 'jquery'; +import waitForPromises from 'helpers/wait_for_promises'; +import WorkItemSystemNote from '~/work_items/components/notes/system_note.vue'; +import NoteHeader from '~/notes/components/note_header.vue'; +import axios from '~/lib/utils/axios_utils'; + +describe('system note component', () => { + let wrapper; + let props; + let mock; + + const findTimelineIcon = () => wrapper.findComponent(GlIcon); + const findSystemNoteMessage = () => wrapper.findComponent(NoteHeader); + const findOutdatedLineButton = () => + wrapper.findComponent('[data-testid="outdated-lines-change-btn"]'); + const findOutdatedLines = () => wrapper.findComponent('[data-testid="outdated-lines"]'); + + const createComponent = (propsData = {}) => { + wrapper = shallowMount(WorkItemSystemNote, { + propsData, + slots: { + 'extra-controls': + '<gl-button data-testid="outdated-lines-change-btn">Compare with last version</gl-button>', + }, + }); + }; + + beforeEach(() => { + props = { + note: { + id: '1424', + author: { + id: 1, + name: 'Root', + username: 'root', + state: 'active', + avatarUrl: 'path', + path: '/root', + }, + bodyHtml: '<p dir="auto">closed</p>', + systemNoteIconName: 'status_closed', + createdAt: '2017-08-02T10:51:58.559Z', + }, + }; + + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + + it('should render a list item with correct id', () => { + createComponent(props); + + expect(wrapper.attributes('id')).toBe(`note_${props.note.id}`); + }); + + // Note: The test case below is to handle a use case related to vuex store but since this does not + // have a vuex store , disabling it now will be fixing it in the next iteration + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should render target class is note is target note', () => { + createComponent(props); + + expect(wrapper.classes()).toContain('target'); + }); + + it('should render svg icon', () => { + createComponent(props); + + expect(findTimelineIcon().exists()).toBe(true); + }); + + // Redcarpet Markdown renderer wraps text in `<p>` tags + // we need to strip them because they break layout of commit lists in system notes: + // https://gitlab.com/gitlab-org/gitlab-foss/uploads/b07a10670919254f0220d3ff5c1aa110/jqzI.png + it('removes wrapping paragraph from note HTML', () => { + createComponent(props); + + expect(findSystemNoteMessage().html()).toContain('<span>closed</span>'); + }); + + it('should renderGFM onMount', () => { + const renderGFMSpy = jest.spyOn($.fn, 'renderGFM'); + + createComponent(props); + + expect(renderGFMSpy).toHaveBeenCalled(); + }); + + // eslint-disable-next-line jest/no-disabled-tests + it.skip('renders outdated code lines', async () => { + mock + .onGet('/outdated_line_change_path') + .reply(200, [ + { rich_text: 'console.log', type: 'new', line_code: '123', old_line: null, new_line: 1 }, + ]); + + createComponent({ + note: { ...props.note, outdated_line_change_path: '/outdated_line_change_path' }, + }); + + await findOutdatedLineButton().vm.$emit('click'); + await waitForPromises(); + + expect(findOutdatedLines().exists()).toBe(true); + }); +}); diff --git a/spec/frontend/work_items/components/work_item_notes_spec.js b/spec/frontend/work_items/components/work_item_notes_spec.js new file mode 100644 index 00000000000..ed68d214fc9 --- /dev/null +++ b/spec/frontend/work_items/components/work_item_notes_spec.js @@ -0,0 +1,107 @@ +import { GlSkeletonLoader } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import SystemNote from '~/work_items/components/notes/system_note.vue'; +import WorkItemNotes from '~/work_items/components/work_item_notes.vue'; +import workItemNotesQuery from '~/work_items/graphql/work_item_notes.query.graphql'; +import workItemNotesByIidQuery from '~/work_items/graphql/work_item_notes_by_iid.query.graphql'; +import { WIDGET_TYPE_NOTES } from '~/work_items/constants'; +import { + mockWorkItemNotesResponse, + workItemQueryResponse, + mockWorkItemNotesByIidResponse, +} from '../mock_data'; + +const mockWorkItemId = workItemQueryResponse.data.workItem.id; +const mockNotesWidgetResponse = mockWorkItemNotesResponse.data.workItem.widgets.find( + (widget) => widget.type === WIDGET_TYPE_NOTES, +); + +const mockNotesByIidWidgetResponse = mockWorkItemNotesByIidResponse.data.workspace.workItems.nodes[0].widgets.find( + (widget) => widget.type === WIDGET_TYPE_NOTES, +); + +describe('WorkItemNotes component', () => { + let wrapper; + + Vue.use(VueApollo); + + const findAllSystemNotes = () => wrapper.findAllComponents(SystemNote); + const findActivityLabel = () => wrapper.find('label'); + const findSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader); + const workItemNotesQueryHandler = jest.fn().mockResolvedValue(mockWorkItemNotesResponse); + const workItemNotesByIidQueryHandler = jest + .fn() + .mockResolvedValue(mockWorkItemNotesByIidResponse); + + const createComponent = ({ workItemId = mockWorkItemId, fetchByIid = false } = {}) => { + wrapper = shallowMount(WorkItemNotes, { + apolloProvider: createMockApollo([ + [workItemNotesQuery, workItemNotesQueryHandler], + [workItemNotesByIidQuery, workItemNotesByIidQueryHandler], + ]), + propsData: { + workItemId, + queryVariables: { + id: workItemId, + }, + fullPath: 'test-path', + fetchByIid, + }, + provide: { + glFeatures: { + useIidInWorkItemsPath: fetchByIid, + }, + }, + }); + }; + + beforeEach(async () => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('renders activity label', () => { + expect(findActivityLabel().exists()).toBe(true); + }); + + describe('when notes are loading', () => { + it('renders skeleton loader', () => { + expect(findSkeletonLoader().exists()).toBe(true); + }); + + it('does not render system notes', () => { + expect(findAllSystemNotes().exists()).toBe(false); + }); + }); + + describe('when notes have been loaded', () => { + it('does not render skeleton loader', () => { + expect(findSkeletonLoader().exists()).toBe(true); + }); + + it('renders system notes to the length of the response', async () => { + await waitForPromises(); + expect(findAllSystemNotes()).toHaveLength(mockNotesWidgetResponse.discussions.nodes.length); + }); + }); + + describe('when the notes are fetched by `iid`', () => { + beforeEach(async () => { + createComponent({ workItemId: mockWorkItemId, fetchByIid: true }); + await waitForPromises(); + }); + + it('shows the notes list', () => { + expect(findAllSystemNotes()).toHaveLength( + mockNotesByIidWidgetResponse.discussions.nodes.length, + ); + }); + }); +}); diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index 7bade734586..6270cfe2e9a 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -1394,3 +1394,288 @@ export const projectWorkItemResponse = { }, }, }; + +export const mockWorkItemNotesResponse = { + data: { + workItem: { + id: 'gid://gitlab/WorkItem/600', + iid: '60', + widgets: [ + { + __typename: 'WorkItemWidgetIteration', + }, + { + __typename: 'WorkItemWidgetWeight', + }, + { + __typename: 'WorkItemWidgetAssignees', + }, + { + __typename: 'WorkItemWidgetLabels', + }, + { + __typename: 'WorkItemWidgetDescription', + }, + { + __typename: 'WorkItemWidgetHierarchy', + }, + { + __typename: 'WorkItemWidgetStartAndDueDate', + }, + { + __typename: 'WorkItemWidgetMilestone', + }, + { + type: 'NOTES', + discussions: { + pageInfo: { + hasNextPage: false, + hasPreviousPage: false, + startCursor: null, + endCursor: null, + __typename: 'PageInfo', + }, + nodes: [ + { + id: + 'gid://gitlab/IndividualNoteDiscussion/8bbc4890b6ff0f2cde93a5a0947cd2b8a13d3b6e', + notes: { + nodes: [ + { + id: 'gid://gitlab/Note/2428', + body: 'added #31 as parent issue', + bodyHtml: + '<p data-sourcepos="1:1-1:25" dir="auto">added <a href="/flightjs/Flight/-/issues/31" data-reference-type="issue" data-original="#31" data-link="false" data-link-reference="false" data-project="6" data-issue="224" data-project-path="flightjs/Flight" data-iid="31" data-issue-type="issue" data-container=body data-placement="top" title="Perferendis est quae totam quia laborum tempore ut voluptatem." class="gfm gfm-issue">#31</a> as parent issue</p>', + systemNoteIconName: 'link', + createdAt: '2022-11-14T04:18:59Z', + author: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + { + id: + 'gid://gitlab/IndividualNoteDiscussion/7b08b89a728a5ceb7de8334246837ba1d07270dc', + notes: { + nodes: [ + { + id: 'gid://gitlab/MilestoneNote/not-persisted', + body: 'changed milestone to %5', + bodyHtml: + '<p data-sourcepos="1:1-1:23" dir="auto">changed milestone to <a href="/flightjs/Flight/-/milestones/5" data-reference-type="milestone" data-original="%5" data-link="false" data-link-reference="false" data-project="6" data-milestone="30" data-container=body data-placement="top" title="" class="gfm gfm-milestone has-tooltip">%v4.0</a></p>', + systemNoteIconName: 'clock', + createdAt: '2022-11-14T04:18:59Z', + author: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + { + id: + 'gid://gitlab/IndividualNoteDiscussion/0f2f195ec0d1ef95ee9d5b10446b8e96a7d83864', + notes: { + nodes: [ + { + id: 'gid://gitlab/WeightNote/not-persisted', + body: 'changed weight to 89', + bodyHtml: '<p dir="auto">changed weight to <strong>89</strong></p>', + systemNoteIconName: 'weight', + createdAt: '2022-11-25T07:16:20Z', + author: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + ], + __typename: 'DiscussionConnection', + }, + __typename: 'WorkItemWidgetNotes', + }, + ], + __typename: 'WorkItem', + }, + }, +}; +export const mockWorkItemNotesByIidResponse = { + data: { + workspace: { + id: 'gid://gitlab/Project/6', + workItems: { + nodes: [ + { + id: 'gid://gitlab/WorkItem/600', + iid: '51', + widgets: [ + { + __typename: 'WorkItemWidgetIteration', + }, + { + __typename: 'WorkItemWidgetWeight', + }, + { + __typename: 'WorkItemWidgetHealthStatus', + }, + { + __typename: 'WorkItemWidgetAssignees', + }, + { + __typename: 'WorkItemWidgetLabels', + }, + { + __typename: 'WorkItemWidgetDescription', + }, + { + __typename: 'WorkItemWidgetHierarchy', + }, + { + __typename: 'WorkItemWidgetStartAndDueDate', + }, + { + __typename: 'WorkItemWidgetMilestone', + }, + { + type: 'NOTES', + discussions: { + pageInfo: { + hasNextPage: true, + hasPreviousPage: false, + startCursor: null, + endCursor: + 'eyJjcmVhdGVkX2F0IjoiMjAyMi0xMS0xNCAwNDoxOTowMC4wOTkxMTcwMDAgKzAwMDAiLCJpZCI6IjQyNyIsIl9rZCI6Im4ifQ==', + __typename: 'PageInfo', + }, + nodes: [ + { + id: + 'gid://gitlab/IndividualNoteDiscussion/8bbc4890b6ff0f2cde93a5a0947cd2b8a13d3b6e', + notes: { + nodes: [ + { + id: 'gid://gitlab/Note/2428', + body: 'added #31 as parent issue', + bodyHtml: + '\u003cp data-sourcepos="1:1-1:25" dir="auto"\u003eadded \u003ca href="/flightjs/Flight/-/issues/31" data-reference-type="issue" data-original="#31" data-link="false" data-link-reference="false" data-project="6" data-issue="224" data-project-path="flightjs/Flight" data-iid="31" data-issue-type="issue" data-container="body" data-placement="top" title="Perferendis est quae totam quia laborum tempore ut voluptatem." class="gfm gfm-issue"\u003e#31\u003c/a\u003e as parent issue\u003c/p\u003e', + systemNoteIconName: 'link', + createdAt: '2022-11-14T04:18:59Z', + author: { + id: 'gid://gitlab/User/1', + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + { + id: + 'gid://gitlab/IndividualNoteDiscussion/7b08b89a728a5ceb7de8334246837ba1d07270dc', + notes: { + nodes: [ + { + id: + 'gid://gitlab/MilestoneNote/7b08b89a728a5ceb7de8334246837ba1d07270dc', + body: 'changed milestone to %5', + bodyHtml: + '\u003cp data-sourcepos="1:1-1:23" dir="auto"\u003echanged milestone to \u003ca href="/flightjs/Flight/-/milestones/5" data-reference-type="milestone" data-original="%5" data-link="false" data-link-reference="false" data-project="6" data-milestone="30" data-container="body" data-placement="top" title="" class="gfm gfm-milestone has-tooltip"\u003e%v4.0\u003c/a\u003e\u003c/p\u003e', + systemNoteIconName: 'clock', + createdAt: '2022-11-14T04:18:59Z', + author: { + id: 'gid://gitlab/User/1', + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + { + id: + 'gid://gitlab/IndividualNoteDiscussion/addbc177f7664699a135130ab05ffb78c57e4db3', + notes: { + nodes: [ + { + id: + 'gid://gitlab/IterationNote/addbc177f7664699a135130ab05ffb78c57e4db3', + body: 'changed iteration to *iteration:5352', + bodyHtml: + '\u003cp data-sourcepos="1:1-1:36" dir="auto"\u003echanged iteration to \u003ca href="/groups/flightjs/-/iterations/5352" data-reference-type="iteration" data-original="*iteration:5352" data-link="false" data-link-reference="false" data-project="6" data-iteration="5352" data-container="body" data-placement="top" title="Iteration" class="gfm gfm-iteration has-tooltip"\u003eEt autem debitis nam suscipit eos ut. Jul 13, 2022 - Jul 19, 2022\u003c/a\u003e\u003c/p\u003e', + systemNoteIconName: 'iteration', + createdAt: '2022-11-14T04:19:00Z', + author: { + id: 'gid://gitlab/User/1', + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + ], + __typename: 'DiscussionConnection', + }, + __typename: 'WorkItemWidgetNotes', + }, + ], + __typename: 'WorkItem', + }, + ], + __typename: 'WorkItemConnection', + }, + __typename: 'Project', + }, + }, +}; diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 2a61b38337b..15b57a4c9eb 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -629,66 +629,4 @@ RSpec.describe IssuablesHelper do expect(helper.sidebar_milestone_tooltip_label(milestone)).to eq('<img onerror=alert(1)><br/>Milestone') end end - - describe '#issuable_hidden?' do - let_it_be(:issuable) { build(:issue) } - - context 'when issuable is hidden' do - let_it_be(:banned_user) { build(:user, :banned) } - let_it_be(:hidden_issuable) { build(:issue, author: banned_user) } - - context 'when `ban_user_feature_flag` feature flag is enabled' do - it 'returns `true`' do - expect(helper.issuable_hidden?(hidden_issuable)).to eq(true) - end - end - - context 'when `ban_user_feature_flag` feature flag is disabled' do - before do - stub_feature_flags(ban_user_feature_flag: false) - end - - it 'returns `false`' do - expect(helper.issuable_hidden?(hidden_issuable)).to eq(false) - end - end - end - - context 'when issuable is not hidden' do - it 'returns `false`' do - expect(helper.issuable_hidden?(issuable)).to eq(false) - end - end - end - - describe '#hidden_issuable_icon' do - let_it_be(:banned_user) { build(:user, :banned) } - let_it_be(:hidden_issuable) { build(:issue, author: banned_user) } - let_it_be(:issuable) { build(:issue) } - let_it_be(:mock_svg) { '<svg></svg>'.html_safe } - - before do - allow(helper).to receive(:sprite_icon).and_return(mock_svg) - end - - context 'when issuable is hidden' do - it 'returns icon with tooltip' do - expect(helper.hidden_issuable_icon(hidden_issuable)).to eq("<span class=\"has-tooltip\" title=\"This issue is hidden because its author has been banned\">#{mock_svg}</span>") - end - - context 'when issuable is a merge request' do - let_it_be(:hidden_issuable) { build(:merge_request, author: banned_user) } - - it 'returns icon with tooltip' do - expect(helper.hidden_issuable_icon(hidden_issuable)).to eq("<span class=\"has-tooltip\" title=\"This merge request is hidden because its author has been banned\">#{mock_svg}</span>") - end - end - end - - context 'when issuable is not hidden' do - it 'returns `nil`' do - expect(helper.hidden_issuable_icon(issuable)).to be_nil - end - end - end end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 39e50070169..ed363268cdf 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -508,4 +508,55 @@ RSpec.describe IssuesHelper do end end end + + describe '#issue_hidden?' do + context 'when issue is hidden' do + let_it_be(:banned_user) { build(:user, :banned) } + let_it_be(:hidden_issue) { build(:issue, author: banned_user) } + + context 'when `ban_user_feature_flag` feature flag is enabled' do + it 'returns `true`' do + expect(helper.issue_hidden?(hidden_issue)).to eq(true) + end + end + + context 'when `ban_user_feature_flag` feature flag is disabled' do + before do + stub_feature_flags(ban_user_feature_flag: false) + end + + it 'returns `false`' do + expect(helper.issue_hidden?(hidden_issue)).to eq(false) + end + end + end + + context 'when issue is not hidden' do + it 'returns `false`' do + expect(helper.issue_hidden?(issue)).to eq(false) + end + end + end + + describe '#hidden_issue_icon' do + let_it_be(:banned_user) { build(:user, :banned) } + let_it_be(:hidden_issue) { build(:issue, author: banned_user) } + let_it_be(:mock_svg) { '<svg></svg>'.html_safe } + + before do + allow(helper).to receive(:sprite_icon).and_return(mock_svg) + end + + context 'when issue is hidden' do + it 'returns icon with tooltip' do + expect(helper.hidden_issue_icon(hidden_issue)).to eq("<span class=\"has-tooltip\" title=\"This issue is hidden because its author has been banned\">#{mock_svg}</span>") + end + end + + context 'when issue is not hidden' do + it 'returns `nil`' do + expect(helper.hidden_issue_icon(issue)).to be_nil + end + end + end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index 1aa76d4dadd..1516ab106cb 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -54,13 +54,16 @@ RSpec.describe BulkImports::Tracker, type: :model do it 'returns the not started pipeline trackers from the minimum stage number' do stage_1_tracker = create(:bulk_import_tracker, entity: entity, stage: 1) + stage_1_finished_tracker = create(:bulk_import_tracker, :finished, entity: entity, stage: 1) + stage_1_failed_tracker = create(:bulk_import_tracker, :failed, entity: entity, stage: 1) + stage_1_skipped_tracker = create(:bulk_import_tracker, :skipped, entity: entity, stage: 1) stage_2_tracker = create(:bulk_import_tracker, entity: entity, stage: 2) expect(described_class.next_pipeline_trackers_for(entity.id)) .to include(stage_1_tracker) expect(described_class.next_pipeline_trackers_for(entity.id)) - .not_to include(stage_2_tracker) + .not_to include(stage_2_tracker, stage_1_finished_tracker, stage_1_failed_tracker, stage_1_skipped_tracker) end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 88d44e48064..e553e34ab51 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -337,53 +337,6 @@ RSpec.describe Issuable do it { expect(MergeRequest.to_ability_name).to eq("merge_request") } end - describe '.without_hidden' do - let_it_be(:banned_user) { create(:user, :banned) } - - where(issuable_type: [:issue, :merge_request]) - - with_them do - let!(:public_issuable) { create(issuable_type, :closed) } - let!(:hidden_issuable) { create(issuable_type, :closed, author: banned_user) } - - subject { issuable_type.to_s.classify.constantize.without_hidden } - - it 'only returns public issuables' do - expect(subject).to contain_exactly(public_issuable) - end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(ban_user_feature_flag: false) - end - - it 'returns public and hidden issuables' do - expect(subject).to contain_exactly(public_issuable, hidden_issuable) - end - end - end - end - - describe '#hidden?' do - let_it_be(:author) { create(:user) } - - where(issuable_type: [:issue, :merge_request]) - - with_them do - let(:issuable) { build_stubbed(issuable_type, author: author) } - - subject { issuable.hidden? } - - it { is_expected.to eq(false) } - - context 'when the author is banned' do - let_it_be(:author) { create(:user, :banned) } - - it { is_expected.to eq(true) } - end - end - end - describe "#sort_by_attribute" do let(:project) { create(:project) } diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c8904519629..82ee062aef0 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -1432,6 +1432,26 @@ RSpec.describe Issue do end end + describe '.without_hidden' do + let_it_be(:banned_user) { create(:user, :banned) } + let_it_be(:public_issue) { create(:issue, project: reusable_project) } + let_it_be(:hidden_issue) { create(:issue, project: reusable_project, author: banned_user) } + + it 'only returns without_hidden issues' do + expect(described_class.without_hidden).to eq([public_issue]) + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(ban_user_feature_flag: false) + end + + it 'returns public and hidden issues' do + expect(described_class.without_hidden).to contain_exactly(public_issue, hidden_issue) + end + end + end + describe '.by_project_id_and_iid' do let_it_be(:issue_a) { create(:issue, project: reusable_project) } let_it_be(:issue_b) { create(:issue, iid: issue_a.iid) } diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb index 6c19590fcce..741a0db3009 100644 --- a/spec/policies/merge_request_policy_spec.rb +++ b/spec/policies/merge_request_policy_spec.rb @@ -461,20 +461,4 @@ RSpec.describe MergeRequestPolicy do end end end - - context 'when the author of the merge request is banned' do - let_it_be(:user) { create(:user) } - let_it_be(:admin) { create(:user, :admin) } - let_it_be(:author) { create(:user, :banned) } - let_it_be(:project) { create(:project, :public) } - let_it_be(:hidden_merge_request) { create(:merge_request, source_project: project, author: author) } - - it 'does not allow non-admin user to read the merge_request' do - expect(permissions(user, hidden_merge_request)).not_to be_allowed(:read_merge_request) - end - - it 'allows admin to read the merge_request', :enable_admin_mode do - expect(permissions(admin, hidden_merge_request)).to be_allowed(:read_merge_request) - end - end end diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb index 4d0e47b7f65..f5f8b5c2d83 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -8,30 +8,14 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code let_it_be(:user) { merge_request.author } describe 'GET #show' do - context 'when logged in' do - before do - login_as(user) - end - - it_behaves_like "observability csp policy", described_class do - let(:tested_path) do - project_merge_request_path(project, merge_request) - end - end + before do + login_as(user) end - context 'when the author of the merge request is banned' do - let_it_be(:user) { create(:user, :banned) } - let_it_be(:project) { create(:project, :public) } - let_it_be(:merge_request) { create(:merge_request, source_project: project, author: user) } - - subject { response } - - before do - get project_merge_request_path(project, merge_request) + it_behaves_like "observability csp policy", described_class do + let(:tested_path) do + project_merge_request_path(project, merge_request) end - - it { is_expected.to have_gitlab_http_status(:not_found) } end end diff --git a/spec/workers/bulk_imports/entity_worker_spec.rb b/spec/workers/bulk_imports/entity_worker_spec.rb index e3f0ee65205..4cd37c93d5f 100644 --- a/spec/workers/bulk_imports/entity_worker_spec.rb +++ b/spec/workers/bulk_imports/entity_worker_spec.rb @@ -114,6 +114,8 @@ RSpec.describe BulkImports::EntityWorker do ) subject + + expect(entity.reload.failed?).to eq(true) end context 'in first stage' do diff --git a/spec/workers/bulk_imports/export_request_worker_spec.rb b/spec/workers/bulk_imports/export_request_worker_spec.rb index 7eb8150fb2e..7260e0c0f67 100644 --- a/spec/workers/bulk_imports/export_request_worker_spec.rb +++ b/spec/workers/bulk_imports/export_request_worker_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' -RSpec.describe BulkImports::ExportRequestWorker do +RSpec.describe BulkImports::ExportRequestWorker, feature_category: :importers do let_it_be(:bulk_import) { create(:bulk_import) } let_it_be(:config) { create(:bulk_import_configuration, bulk_import: bulk_import) } + let_it_be(:entity) { create(:bulk_import_entity, bulk_import: bulk_import) } let_it_be(:version_url) { 'https://gitlab.example/api/v4/version' } let(:response_double) { double(code: 200, success?: true, parsed_response: {}) } @@ -31,73 +32,6 @@ RSpec.describe BulkImports::ExportRequestWorker do perform_multiple(job_args) end - context 'when network error is raised' do - let(:exception) { BulkImports::NetworkError.new('Export error') } - - before do - allow_next_instance_of(BulkImports::Clients::HTTP) do |client| - allow(client).to receive(:post).and_raise(exception).twice - end - end - - context 'when error is retriable' do - it 'logs retry request and reenqueues' do - allow(exception).to receive(:retriable?).twice.and_return(true) - - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger).to receive(:error).with( - a_hash_including( - 'bulk_import_entity_id' => entity.id, - 'bulk_import_id' => entity.bulk_import_id, - 'bulk_import_entity_type' => entity.source_type, - 'source_full_path' => entity.source_full_path, - 'exception.backtrace' => anything, - 'exception.class' => 'BulkImports::NetworkError', - 'exception.message' => 'Export error', - 'message' => 'Retrying export request', - 'importer' => 'gitlab_migration', - 'source_version' => entity.bulk_import.source_version_info.to_s - ) - ).twice - end - - expect(described_class).to receive(:perform_in).twice.with(2.seconds, entity.id) - - perform_multiple(job_args) - end - end - - context 'when error is not retriable' do - it 'logs export failure and marks entity as failed' do - allow(exception).to receive(:retriable?).twice.and_return(false) - - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger).to receive(:error).with( - a_hash_including( - 'bulk_import_entity_id' => entity.id, - 'bulk_import_id' => entity.bulk_import_id, - 'bulk_import_entity_type' => entity.source_type, - 'source_full_path' => entity.source_full_path, - 'exception.backtrace' => anything, - 'exception.class' => 'BulkImports::NetworkError', - 'exception.message' => 'Export error', - 'message' => "Request to export #{entity.source_type} failed", - 'importer' => 'gitlab_migration', - 'source_version' => entity.bulk_import.source_version_info.to_s - ) - ).twice - end - - perform_multiple(job_args) - - failure = entity.failures.last - - expect(failure.pipeline_class).to eq('ExportRequestWorker') - expect(failure.exception_message).to eq('Export error') - end - end - end - context 'when source id is nil' do let(:entity_source_id) { 'gid://gitlab/Model/1234567' } @@ -179,4 +113,24 @@ RSpec.describe BulkImports::ExportRequestWorker do it_behaves_like 'requests relations export for api resource' end end + + describe '#sidekiq_retries_exhausted' do + it 'logs export failure and marks entity as failed' do + entity = create(:bulk_import_entity, bulk_import: bulk_import) + error = 'Exhausted error!' + + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger) + .to receive(:error) + .with(hash_including('message' => "Request to export #{entity.source_type} failed")) + end + + described_class + .sidekiq_retries_exhausted_block + .call({ 'args' => [entity.id] }, StandardError.new(error)) + + expect(entity.reload.failed?).to eq(true) + expect(entity.failures.last.exception_message).to eq(error) + end + end end diff --git a/spec/workers/bulk_imports/pipeline_worker_spec.rb b/spec/workers/bulk_imports/pipeline_worker_spec.rb index 07b1bdd9d65..03ec6267ca8 100644 --- a/spec/workers/bulk_imports/pipeline_worker_spec.rb +++ b/spec/workers/bulk_imports/pipeline_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::PipelineWorker do +RSpec.describe BulkImports::PipelineWorker, feature_category: :importers do let(:pipeline_class) do Class.new do def initialize(_); end @@ -19,6 +19,15 @@ RSpec.describe BulkImports::PipelineWorker do let_it_be(:config) { create(:bulk_import_configuration, bulk_import: bulk_import) } let_it_be(:entity) { create(:bulk_import_entity, bulk_import: bulk_import) } + let(:pipeline_tracker) do + create( + :bulk_import_tracker, + entity: entity, + pipeline_name: 'FakePipeline', + status_event: 'enqueue' + ) + end + before do stub_const('FakePipeline', pipeline_class) @@ -60,45 +69,12 @@ RSpec.describe BulkImports::PipelineWorker do end end - it_behaves_like 'successfully runs the pipeline' do - let(:pipeline_tracker) do - create( - :bulk_import_tracker, - entity: entity, - pipeline_name: 'FakePipeline', - status_event: 'enqueue' - ) - end - end + it_behaves_like 'successfully runs the pipeline' - context 'when the pipeline cannot be found' do - it 'logs the error' do - pipeline_tracker = create( - :bulk_import_tracker, - :finished, - entity: entity, - pipeline_name: 'FakePipeline' - ) - - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:error) - .with( - hash_including( - 'pipeline_tracker_id' => pipeline_tracker.id, - 'bulk_import_entity_id' => entity.id, - 'bulk_import_id' => entity.bulk_import_id, - 'bulk_import_entity_type' => entity.source_type, - 'source_full_path' => entity.source_full_path, - 'source_version' => entity.bulk_import.source_version_info.to_s, - 'message' => 'Unstarted pipeline not found' - ) - ) - end - - expect(BulkImports::EntityWorker) - .to receive(:perform_async) - .with(entity.id, pipeline_tracker.stage) + context 'when exclusive lease cannot be obtained' do + it 'does not run the pipeline' do + expect(subject).to receive(:try_obtain_lease).and_return(false) + expect(subject).not_to receive(:run) subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) end @@ -145,13 +121,15 @@ RSpec.describe BulkImports::PipelineWorker do .to receive(:track_exception) .with( instance_of(StandardError), - bulk_import_entity_id: entity.id, - bulk_import_id: entity.bulk_import.id, - bulk_import_entity_type: entity.source_type, - source_full_path: entity.source_full_path, - pipeline_name: pipeline_tracker.pipeline_name, - importer: 'gitlab_migration', - source_version: entity.bulk_import.source_version_info.to_s + hash_including( + 'bulk_import_entity_id' => entity.id, + 'bulk_import_id' => entity.bulk_import.id, + 'bulk_import_entity_type' => entity.source_type, + 'source_full_path' => entity.source_full_path, + 'pipeline_name' => pipeline_tracker.pipeline_name, + 'importer' => 'gitlab_migration', + 'source_version' => entity.bulk_import.source_version_info.to_s + ) ) expect(BulkImports::EntityWorker) @@ -179,6 +157,111 @@ RSpec.describe BulkImports::PipelineWorker do expect(pipeline_tracker.jid).to eq('jid') end + shared_examples 'successfully runs the pipeline' do + it 'runs the given pipeline successfully' do + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger) + .to receive(:info) + .with( + hash_including( + 'pipeline_name' => 'FakePipeline', + 'bulk_import_id' => entity.bulk_import_id, + 'bulk_import_entity_id' => entity.id, + 'bulk_import_entity_type' => entity.source_type, + 'source_full_path' => entity.source_full_path + ) + ) + end + + expect(BulkImports::EntityWorker) + .to receive(:perform_async) + .with(entity.id, pipeline_tracker.stage) + + allow(subject).to receive(:jid).and_return('jid') + + subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + + pipeline_tracker.reload + + expect(pipeline_tracker.status_name).to eq(:finished) + expect(pipeline_tracker.jid).to eq('jid') + end + end + + context 'when enqueued pipeline cannot be found' do + shared_examples 'logs the error' do + it 'logs the error' do + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + status = pipeline_tracker.human_status_name + + expect(logger) + .to receive(:error) + .with( + hash_including( + 'bulk_import_entity_id' => entity.id, + 'bulk_import_id' => entity.bulk_import_id, + 'bulk_import_entity_type' => entity.source_type, + 'pipeline_tracker_id' => pipeline_tracker.id, + 'pipeline_tracker_state' => status, + 'pipeline_name' => pipeline_tracker.pipeline_name, + 'source_full_path' => entity.source_full_path, + 'source_version' => entity.bulk_import.source_version_info.to_s, + 'importer' => 'gitlab_migration', + 'message' => "Pipeline in #{status} state instead of expected enqueued state" + ) + ) + end + + expect(BulkImports::EntityWorker) + .to receive(:perform_async) + .with(entity.id, pipeline_tracker.stage) + + subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + end + end + + context 'when pipeline is finished' do + let(:pipeline_tracker) do + create( + :bulk_import_tracker, + :finished, + entity: entity, + pipeline_name: 'FakePipeline' + ) + end + + include_examples 'logs the error' + end + + context 'when pipeline is skipped' do + let(:pipeline_tracker) do + create( + :bulk_import_tracker, + :skipped, + entity: entity, + pipeline_name: 'FakePipeline' + ) + end + + include_examples 'logs the error' + end + + context 'when tracker is started' do + it 'marks tracker as failed' do + pipeline_tracker = create( + :bulk_import_tracker, + :started, + entity: entity, + pipeline_name: 'FakePipeline' + ) + + subject.perform(pipeline_tracker.id, pipeline_tracker.stage, entity.id) + + expect(pipeline_tracker.reload.failed?).to eq(true) + end + end + end + context 'when entity is failed' do it 'marks tracker as skipped and logs the skip' do pipeline_tracker = create( diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb index 85010ebe4c4..79b0cbf254b 100644 --- a/spec/workers/every_sidekiq_worker_spec.rb +++ b/spec/workers/every_sidekiq_worker_spec.rb @@ -139,6 +139,7 @@ RSpec.describe 'Every Sidekiq worker' do 'BuildQueueWorker' => 3, 'BuildSuccessWorker' => 3, 'BulkImportWorker' => false, + 'BulkImports::ExportRequestWorker' => 5, 'BulkImports::EntityWorker' => false, 'BulkImports::PipelineWorker' => false, 'Chaos::CpuSpinWorker' => 3, |