diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-10 12:08:57 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-11-10 12:08:57 +0000 |
| commit | a08f8baa63c0aea7fcf969da40d30e6cf56365cc (patch) | |
| tree | 57b5d1964407332189ce027bc3c99301b7a1f515 | |
| parent | 01c201bc6a9b99e1f3095f4139110c6fd0cf7aa9 (diff) | |
| download | gitlab-ce-a08f8baa63c0aea7fcf969da40d30e6cf56365cc.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
162 files changed, 2633 insertions, 1243 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index ad340ae0947..8ea542caa2e 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -fa974a4ab21aa6acc4c3a00456265248a4d70703 +9cde939eef5182b062f9aa04a3a3f78d8264af4b diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_form_new.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_form_new.vue index a08100f3938..81ee86fe797 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_form_new.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_form_new.vue @@ -17,6 +17,7 @@ import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { s__ } from '~/locale'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import AlertSettingsFormHelpBlock from './alert_settings_form_help_block.vue'; +import service from '../services'; import { integrationTypesNew, JSON_VALIDATE_DELAY, @@ -89,7 +90,7 @@ export default { MappingBuilder, }, directives: { - 'gl-modal': GlModalDirective, + GlModal: GlModalDirective, }, inject: { generic: { @@ -150,6 +151,13 @@ export default { apiUrl: this.currentIntegration?.apiUrl || '', }; }, + testAlertPayload() { + return { + data: this.integrationTestPayload.json, + endpoint: this.integrationForm.url, + token: this.integrationForm.token, + }; + }, }, watch: { currentIntegration(val) { @@ -170,8 +178,14 @@ export default { } }, submitWithTestPayload() { - // TODO: Test payload before saving via GraphQL - this.submit(); + return service + .updateTestAlert(this.testAlertPayload) + .then(() => { + this.submit(); + }) + .catch(() => { + this.$emit('test-payload-failure'); + }); }, submit() { const { name, apiUrl } = this.integrationForm; @@ -359,7 +373,6 @@ export default { </div> </gl-form-group> <gl-form-group - id="test-integration" :label="$options.i18n.integrationFormSteps.step4.label" label-for="test-integration" :invalid-feedback="integrationTestPayload.error" @@ -395,6 +408,8 @@ export default { <div class="gl-display-flex gl-justify-content-end"> <gl-button type="reset" class="gl-mr-3 js-no-auto-disable">{{ __('Cancel') }}</gl-button> <gl-button + data-testid="integration-test-and-submit" + :disabled="Boolean(integrationTestPayload.error)" category="secondary" variant="success" class="gl-mr-1 js-no-auto-disable" diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue index 57fc1984990..3904a5b0fe2 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_wrapper.vue @@ -24,6 +24,7 @@ import { ADD_INTEGRATION_ERROR, RESET_INTEGRATION_TOKEN_ERROR, UPDATE_INTEGRATION_ERROR, + INTEGRATION_PAYLOAD_TEST_ERROR, } from '../utils/error_messages'; export default { @@ -244,6 +245,9 @@ export default { clearCurrentIntegration() { this.currentIntegration = null; }, + testPayloadFailure() { + createFlash({ message: INTEGRATION_PAYLOAD_TEST_ERROR }); + }, }, }; </script> @@ -266,6 +270,7 @@ export default { @update-integration="updateIntegration" @reset-token="resetToken" @clear-current-integration="clearCurrentIntegration" + @test-payload-failure="testPayloadFailure" /> <settings-form-old v-else /> </div> diff --git a/app/assets/javascripts/alerts_settings/services/index.js b/app/assets/javascripts/alerts_settings/services/index.js index c49992d4f57..1835d6b46aa 100644 --- a/app/assets/javascripts/alerts_settings/services/index.js +++ b/app/assets/javascripts/alerts_settings/services/index.js @@ -2,6 +2,7 @@ import axios from '~/lib/utils/axios_utils'; export default { + // TODO: All this code save updateTestAlert will be deleted as part of: https://gitlab.com/gitlab-org/gitlab/-/issues/255501 updateGenericKey({ endpoint, params }) { return axios.put(endpoint, params); }, @@ -25,11 +26,11 @@ export default { }, }); }, - updateTestAlert({ endpoint, data, authKey }) { + updateTestAlert({ endpoint, data, token }) { return axios.post(endpoint, data, { headers: { 'Content-Type': 'application/json', - Authorization: `Bearer ${authKey}`, + Authorization: `Bearer ${token}`, }, }); }, diff --git a/app/assets/javascripts/alerts_settings/utils/error_messages.js b/app/assets/javascripts/alerts_settings/utils/error_messages.js index 7df5d444a53..979d1ca3ccc 100644 --- a/app/assets/javascripts/alerts_settings/utils/error_messages.js +++ b/app/assets/javascripts/alerts_settings/utils/error_messages.js @@ -15,3 +15,7 @@ export const UPDATE_INTEGRATION_ERROR = s__( export const RESET_INTEGRATION_TOKEN_ERROR = s__( 'AlertsIntegrations|The integration token could not be reset. Please try again.', ); + +export const INTEGRATION_PAYLOAD_TEST_ERROR = s__( + 'AlertsIntegrations|Integration payload is invalid. You can still save your changes.', +); diff --git a/app/assets/javascripts/batch_comments/components/inline_draft_comment_row.vue b/app/assets/javascripts/batch_comments/components/inline_draft_comment_row.vue deleted file mode 100644 index 385725cd109..00000000000 --- a/app/assets/javascripts/batch_comments/components/inline_draft_comment_row.vue +++ /dev/null @@ -1,32 +0,0 @@ -<script> -import DraftNote from './draft_note.vue'; - -export default { - components: { - DraftNote, - }, - props: { - draft: { - type: Object, - required: true, - }, - diffFile: { - type: Object, - required: true, - }, - line: { - type: Object, - required: false, - default: null, - }, - }, -}; -</script> - -<template> - <tr class="notes_holder js-temp-notes-holder"> - <td class="notes-content" colspan="4"> - <div class="content"><draft-note :draft="draft" :diff-file="diffFile" :line="line" /></div> - </td> - </tr> -</template> diff --git a/app/assets/javascripts/batch_comments/components/parallel_draft_comment_row.vue b/app/assets/javascripts/batch_comments/components/parallel_draft_comment_row.vue deleted file mode 100644 index b0916623cd2..00000000000 --- a/app/assets/javascripts/batch_comments/components/parallel_draft_comment_row.vue +++ /dev/null @@ -1,49 +0,0 @@ -<script> -import { mapGetters } from 'vuex'; -import DraftNote from './draft_note.vue'; - -export default { - components: { - DraftNote, - }, - props: { - line: { - type: Object, - required: true, - }, - diffFileContentSha: { - type: String, - required: true, - }, - }, - computed: { - ...mapGetters('batchComments', ['draftForLine']), - className() { - return this.leftDraft > 0 || this.rightDraft > 0 ? '' : 'js-temp-notes-holder'; - }, - leftDraft() { - return this.draftForLine(this.diffFileContentSha, this.line, 'left'); - }, - rightDraft() { - return this.draftForLine(this.diffFileContentSha, this.line, 'right'); - }, - }, -}; -</script> - -<template> - <tr :class="className" class="notes_holder"> - <td class="notes_line old"></td> - <td class="notes-content parallel old" colspan="2"> - <div v-if="leftDraft.isDraft" class="content"> - <draft-note :draft="leftDraft" :line="line.left" /> - </div> - </td> - <td class="notes_line new"></td> - <td class="notes-content parallel new" colspan="2"> - <div v-if="rightDraft.isDraft" class="content"> - <draft-note :draft="rightDraft" :line="line.right" /> - </div> - </td> - </tr> -</template> diff --git a/app/assets/javascripts/boards/components/board_assignee_dropdown.vue b/app/assets/javascripts/boards/components/board_assignee_dropdown.vue index b2c3b6aa8ab..2245c0d0f44 100644 --- a/app/assets/javascripts/boards/components/board_assignee_dropdown.vue +++ b/app/assets/javascripts/boards/components/board_assignee_dropdown.vue @@ -4,7 +4,7 @@ import { GlDropdownItem, GlDropdownDivider, GlAvatarLabeled, GlAvatarLink } from import { __, n__ } from '~/locale'; import IssuableAssignees from '~/sidebar/components/assignees/issuable_assignees.vue'; import BoardEditableItem from '~/boards/components/sidebar/board_editable_item.vue'; -import AssigneesDropdown from '~/vue_shared/components/sidebar/assignees_dropdown.vue'; +import MultiSelectDropdown from '~/vue_shared/components/sidebar/multiselect_dropdown.vue'; import getIssueParticipants from '~/vue_shared/components/sidebar/queries/getIssueParticipants.query.graphql'; export default { @@ -17,7 +17,7 @@ export default { components: { BoardEditableItem, IssuableAssignees, - AssigneesDropdown, + MultiSelectDropdown, GlDropdownItem, GlDropdownDivider, GlAvatarLabeled, @@ -92,7 +92,7 @@ export default { </template> <template #default> - <assignees-dropdown + <multi-select-dropdown class="w-100" :text="$options.i18n.assignees" :header-text="$options.i18n.assignTo" @@ -138,7 +138,7 @@ export default { </gl-avatar-link> </gl-dropdown-item> </template> - </assignees-dropdown> + </multi-select-dropdown> </template> </board-editable-item> </template> diff --git a/app/assets/javascripts/design_management/components/design_overlay.vue b/app/assets/javascripts/design_management/components/design_overlay.vue index 88f3ce0b8ea..3c2ce693bc0 100644 --- a/app/assets/javascripts/design_management/components/design_overlay.vue +++ b/app/assets/javascripts/design_management/components/design_overlay.vue @@ -112,9 +112,9 @@ export default { }, canMoveNote(note) { const { userPermissions } = note; - const { adminNote } = userPermissions || {}; + const { repositionNote } = userPermissions || {}; - return Boolean(adminNote); + return Boolean(repositionNote); }, isPositionInOverlay(position) { const { top, left } = this.getNoteRelativePosition(position); diff --git a/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql b/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql index c243e39f3d3..e599ab19c2d 100644 --- a/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql +++ b/app/assets/javascripts/design_management/graphql/fragments/note_permissions.fragment.graphql @@ -1,3 +1,4 @@ fragment DesignNotePermissions on NotePermissions { adminNote + repositionNote } diff --git a/app/assets/javascripts/design_management/graphql/mutations/reposition_image_diff_note.mutation.graphql b/app/assets/javascripts/design_management/graphql/mutations/reposition_image_diff_note.mutation.graphql new file mode 100644 index 00000000000..78fbcf1c3c7 --- /dev/null +++ b/app/assets/javascripts/design_management/graphql/mutations/reposition_image_diff_note.mutation.graphql @@ -0,0 +1,10 @@ +#import "../fragments/design_note.fragment.graphql" + +mutation repositionImageDiffNote($input: RepositionImageDiffNoteInput!) { + repositionImageDiffNote(input: $input) { + errors + note { + ...DesignNote + } + } +} diff --git a/app/assets/javascripts/design_management/graphql/mutations/update_image_diff_note.mutation.graphql b/app/assets/javascripts/design_management/graphql/mutations/update_image_diff_note.mutation.graphql deleted file mode 100644 index 5562ca9d89f..00000000000 --- a/app/assets/javascripts/design_management/graphql/mutations/update_image_diff_note.mutation.graphql +++ /dev/null @@ -1,10 +0,0 @@ -#import "../fragments/design_note.fragment.graphql" - -mutation updateImageDiffNote($input: UpdateImageDiffNoteInput!) { - updateImageDiffNote(input: $input) { - errors - note { - ...DesignNote - } - } -} diff --git a/app/assets/javascripts/design_management/pages/design/index.vue b/app/assets/javascripts/design_management/pages/design/index.vue index f410a976676..d1bbb5239d0 100644 --- a/app/assets/javascripts/design_management/pages/design/index.vue +++ b/app/assets/javascripts/design_management/pages/design/index.vue @@ -13,19 +13,19 @@ import DesignReplyForm from '../../components/design_notes/design_reply_form.vue import DesignSidebar from '../../components/design_sidebar.vue'; import getDesignQuery from '../../graphql/queries/get_design.query.graphql'; import createImageDiffNoteMutation from '../../graphql/mutations/create_image_diff_note.mutation.graphql'; -import updateImageDiffNoteMutation from '../../graphql/mutations/update_image_diff_note.mutation.graphql'; +import repositionImageDiffNoteMutation from '../../graphql/mutations/reposition_image_diff_note.mutation.graphql'; import updateActiveDiscussionMutation from '../../graphql/mutations/update_active_discussion.mutation.graphql'; import { extractDiscussions, extractDesign, - updateImageDiffNoteOptimisticResponse, + repositionImageDiffNoteOptimisticResponse, toDiffNoteGid, extractDesignNoteId, getPageLayoutElement, } from '../../utils/design_management_utils'; import { updateStoreAfterAddImageDiffNote, - updateStoreAfterUpdateImageDiffNote, + updateStoreAfterRepositionImageDiffNote, } from '../../utils/cache_update'; import { ADD_DISCUSSION_COMMENT_ERROR, @@ -182,12 +182,12 @@ export default { updateImageDiffNoteInStore( store, { - data: { updateImageDiffNote }, + data: { repositionImageDiffNote }, }, ) { - return updateStoreAfterUpdateImageDiffNote( + return updateStoreAfterRepositionImageDiffNote( store, - updateImageDiffNote, + repositionImageDiffNote, getDesignQuery, this.designVariables, ); @@ -199,7 +199,7 @@ export default { ); const mutationPayload = { - optimisticResponse: updateImageDiffNoteOptimisticResponse(note, { + optimisticResponse: repositionImageDiffNoteOptimisticResponse(note, { position, }), variables: { @@ -208,7 +208,7 @@ export default { position, }, }, - mutation: updateImageDiffNoteMutation, + mutation: repositionImageDiffNoteMutation, update: this.updateImageDiffNoteInStore, }; diff --git a/app/assets/javascripts/design_management/utils/cache_update.js b/app/assets/javascripts/design_management/utils/cache_update.js index ac4763853ef..5bd0288d037 100644 --- a/app/assets/javascripts/design_management/utils/cache_update.js +++ b/app/assets/javascripts/design_management/utils/cache_update.js @@ -101,7 +101,7 @@ const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) = }); }; -const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables) => { +const updateImageDiffNoteInStore = (store, repositionImageDiffNote, query, variables) => { const sourceData = store.readQuery({ query, variables, @@ -111,12 +111,12 @@ const updateImageDiffNoteInStore = (store, updateImageDiffNote, query, variables const design = extractDesign(draftData); const discussion = extractCurrentDiscussion( design.discussions, - updateImageDiffNote.note.discussion.id, + repositionImageDiffNote.note.discussion.id, ); discussion.notes = { ...discussion.notes, - nodes: [updateImageDiffNote.note, ...discussion.notes.nodes.slice(1)], + nodes: [repositionImageDiffNote.note, ...discussion.notes.nodes.slice(1)], }; }); @@ -268,7 +268,7 @@ export const updateStoreAfterAddImageDiffNote = (store, data, query, queryVariab } }; -export const updateStoreAfterUpdateImageDiffNote = (store, data, query, queryVariables) => { +export const updateStoreAfterRepositionImageDiffNote = (store, data, query, queryVariables) => { if (hasErrors(data)) { onError(data, UPDATE_IMAGE_DIFF_NOTE_ERROR); } else { diff --git a/app/assets/javascripts/design_management/utils/design_management_utils.js b/app/assets/javascripts/design_management/utils/design_management_utils.js index 687e793d3df..a905230811c 100644 --- a/app/assets/javascripts/design_management/utils/design_management_utils.js +++ b/app/assets/javascripts/design_management/utils/design_management_utils.js @@ -107,12 +107,12 @@ export const designUploadOptimisticResponse = files => { * @param {Object} note * @param {Object} position */ -export const updateImageDiffNoteOptimisticResponse = (note, { position }) => ({ +export const repositionImageDiffNoteOptimisticResponse = (note, { position }) => ({ // False positive i18n lint: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26 // eslint-disable-next-line @gitlab/require-i18n-strings __typename: 'Mutation', - updateImageDiffNote: { - __typename: 'UpdateImageDiffNotePayload', + repositionImageDiffNote: { + __typename: 'RepositionImageDiffNotePayload', note: { ...note, position: { diff --git a/app/assets/javascripts/diffs/components/diff_comment_cell.vue b/app/assets/javascripts/diffs/components/diff_comment_cell.vue new file mode 100644 index 00000000000..4b0b603f5a5 --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_comment_cell.vue @@ -0,0 +1,69 @@ +<script> +import { mapActions } from 'vuex'; +import DiffDiscussions from './diff_discussions.vue'; +import DiffLineNoteForm from './diff_line_note_form.vue'; +import DiffDiscussionReply from './diff_discussion_reply.vue'; + +export default { + components: { + DiffDiscussions, + DiffLineNoteForm, + DiffDiscussionReply, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFileHash: { + type: String, + required: true, + }, + helpPagePath: { + type: String, + required: false, + default: '', + }, + hasDraft: { + type: Boolean, + required: false, + default: false, + }, + linePosition: { + type: String, + required: false, + default: '', + }, + }, + methods: { + ...mapActions('diffs', ['showCommentForm']), + }, +}; +</script> + +<template> + <div class="content"> + <diff-discussions + v-if="line.renderDiscussion" + :line="line" + :discussions="line.discussions" + :help-page-path="helpPagePath" + /> + <diff-discussion-reply + v-if="!hasDraft" + :has-form="line.hasCommentForm" + :render-reply-placeholder="Boolean(line.discussions.length)" + @showNewDiscussionForm="showCommentForm({ lineCode: line.line_code, fileHash: diffFileHash })" + > + <template #form> + <diff-line-note-form + :diff-file-hash="diffFileHash" + :line="line" + :note-target-line="line" + :help-page-path="helpPagePath" + :line-position="linePosition" + /> + </template> + </diff-discussion-reply> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index e68260b3e62..401064fb18f 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -10,6 +10,7 @@ import NoPreviewViewer from '~/vue_shared/components/diff_viewer/viewers/no_prev import DiffFileDrafts from '~/batch_comments/components/diff_file_drafts.vue'; import InlineDiffView from './inline_diff_view.vue'; import ParallelDiffView from './parallel_diff_view.vue'; +import DiffView from './diff_view.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import NoteForm from '../../notes/components/note_form.vue'; import ImageDiffOverlay from './image_diff_overlay.vue'; @@ -18,12 +19,14 @@ import eventHub from '../../notes/event_hub'; import { IMAGE_DIFF_POSITION_TYPE } from '../constants'; import { getDiffMode } from '../store/utils'; import { diffViewerModes } from '~/ide/constants'; +import { mapInline, mapParallel } from './diff_row_utils'; export default { components: { GlLoadingIcon, InlineDiffView, ParallelDiffView, + DiffView, DiffViewer, NoteForm, DiffDiscussions, @@ -83,6 +86,19 @@ export default { author() { return this.getUserData; }, + mappedLines() { + if (this.glFeatures.unifiedDiffLines && this.glFeatures.unifiedDiffComponents) { + return this.diffLines(this.diffFile, true).map(mapParallel(this)) || []; + } + + // TODO: Everything below this line can be deleted when unifiedDiffComponents FF is removed + if (this.isInlineView) { + return this.diffFile.highlighted_diff_lines.map(mapInline(this)); + } + return this.glFeatures.unifiedDiffLines + ? this.diffLines(this.diffFile).map(mapParallel(this)) + : this.diffFile.parallel_diff_lines.map(mapParallel(this)) || []; + }, }, updated() { this.$nextTick(() => { @@ -113,19 +129,28 @@ export default { <template> <div class="diff-content"> <div class="diff-viewer"> - <template v-if="isTextFile"> + <template + v-if="isTextFile && glFeatures.unifiedDiffLines && glFeatures.unifiedDiffComponents" + > + <diff-view + :diff-file="diffFile" + :diff-lines="mappedLines" + :help-page-path="helpPagePath" + :inline="isInlineView" + /> + <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" /> + </template> + <template v-else-if="isTextFile"> <inline-diff-view v-if="isInlineView" :diff-file="diffFile" - :diff-lines="diffFile.highlighted_diff_lines" + :diff-lines="mappedLines" :help-page-path="helpPagePath" /> <parallel-diff-view v-else-if="isParallelView" :diff-file="diffFile" - :diff-lines=" - glFeatures.unifiedDiffLines ? diffLines(diffFile) : diffFile.parallel_diff_lines || [] - " + :diff-lines="mappedLines" :help-page-path="helpPagePath" /> <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" /> diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index ac8eb6ba8f9..4c49dfb5de9 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -54,11 +54,6 @@ export default { required: false, default: false, }, - colspan: { - type: Number, - required: false, - default: 4, - }, }, computed: { ...mapState({ @@ -231,28 +226,26 @@ export default { </script> <template> - <td :colspan="colspan" class="text-center gl-font-regular"> - <div class="content js-line-expansion-content"> - <a - v-if="canExpandDown" - class="gl-mx-2 gl-cursor-pointer js-unfold-down gl-display-inline-block gl-py-4" - @click="handleExpandLines(EXPAND_DOWN)" - > - <gl-icon :size="12" name="expand-down" aria-hidden="true" /> - <span>{{ $options.i18n.showMore }}</span> - </a> - <a class="gl-mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()"> - <gl-icon :size="12" name="expand" aria-hidden="true" /> - <span>{{ $options.i18n.showAll }}</span> - </a> - <a - v-if="canExpandUp" - class="gl-mx-2 gl-cursor-pointer js-unfold gl-display-inline-block gl-py-4" - @click="handleExpandLines(EXPAND_UP)" - > - <gl-icon :size="12" name="expand-up" aria-hidden="true" /> - <span>{{ $options.i18n.showMore }}</span> - </a> - </div> - </td> + <div class="content js-line-expansion-content"> + <a + v-if="canExpandDown" + class="gl-mx-2 gl-cursor-pointer js-unfold-down gl-display-inline-block gl-py-4" + @click="handleExpandLines(EXPAND_DOWN)" + > + <gl-icon :size="12" name="expand-down" aria-hidden="true" /> + <span>{{ $options.i18n.showMore }}</span> + </a> + <a class="gl-mx-2 cursor-pointer js-unfold-all" @click="handleExpandLines()"> + <gl-icon :size="12" name="expand" aria-hidden="true" /> + <span>{{ $options.i18n.showAll }}</span> + </a> + <a + v-if="canExpandUp" + class="gl-mx-2 gl-cursor-pointer js-unfold gl-display-inline-block gl-py-4" + @click="handleExpandLines(EXPAND_UP)" + > + <gl-icon :size="12" name="expand-up" aria-hidden="true" /> + <span>{{ $options.i18n.showMore }}</span> + </a> + </div> </template> diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue new file mode 100644 index 00000000000..77a97c67f3b --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -0,0 +1,271 @@ +<script> +import { mapActions, mapGetters, mapState } from 'vuex'; +import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; +import { CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; +import DiffGutterAvatars from './diff_gutter_avatars.vue'; +import * as utils from './diff_row_utils'; + +export default { + components: { + GlIcon, + DiffGutterAvatars, + }, + directives: { + GlTooltip: GlTooltipDirective, + SafeHtml, + }, + props: { + fileHash: { + type: String, + required: true, + }, + filePath: { + type: String, + required: true, + }, + line: { + type: Object, + required: true, + }, + isCommented: { + type: Boolean, + required: false, + default: false, + }, + inline: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapGetters('diffs', ['fileLineCoverage']), + ...mapGetters(['isLoggedIn']), + ...mapState({ + isHighlighted(state) { + const line = this.line.left?.line_code ? this.line.left : this.line.right; + return utils.isHighlighted(state, line, this.isCommented); + }, + }), + classNameMap() { + return { + [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLineLeft, + [PARALLEL_DIFF_VIEW_TYPE]: true, + }; + }, + parallelViewLeftLineType() { + return utils.parallelViewLeftLineType(this.line, this.isHighlighted); + }, + coverageState() { + return this.fileLineCoverage(this.filePath, this.line.right.new_line); + }, + classNameMapCellLeft() { + return utils.classNameMapCell(this.line.left, this.isHighlighted, this.isLoggedIn); + }, + classNameMapCellRight() { + return utils.classNameMapCell(this.line.right, this.isHighlighted, this.isLoggedIn); + }, + addCommentTooltipLeft() { + return utils.addCommentTooltip(this.line.left); + }, + addCommentTooltipRight() { + return utils.addCommentTooltip(this.line.right); + }, + shouldRenderCommentButton() { + return ( + this.isLoggedIn && + !this.line.isContextLineLeft && + !this.line.isMetaLineLeft && + !this.line.hasDiscussionsLeft && + !this.line.hasDiscussionsRight + ); + }, + }, + mounted() { + this.scrollToLineIfNeededParallel(this.line); + }, + methods: { + ...mapActions('diffs', [ + 'scrollToLineIfNeededParallel', + 'showCommentForm', + 'setHighlightedRow', + 'toggleLineDiscussions', + ]), + // Prevent text selecting on both sides of parallel diff view + // Backport of the same code from legacy diff notes. + handleParallelLineMouseDown(e) { + const line = e.currentTarget; + const table = line.closest('.diff-table'); + + table.classList.remove('left-side-selected', 'right-side-selected'); + const [lineClass] = ['left-side', 'right-side'].filter(name => line.classList.contains(name)); + + if (lineClass) { + table.classList.add(`${lineClass}-selected`); + } + }, + handleCommentButton(line) { + this.showCommentForm({ lineCode: line.line_code, fileHash: this.fileHash }); + }, + }, +}; +</script> + +<template> + <div :class="classNameMap" class="diff-grid-row diff-tr line_holder"> + <div class="diff-grid-left left-side"> + <template v-if="line.left"> + <div + :class="classNameMapCellLeft" + data-testid="leftLineNumber" + class="diff-td diff-line-num old_line" + > + <span + v-if="shouldRenderCommentButton" + v-gl-tooltip + data-testid="leftCommentButton" + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipLeft" + > + <button + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :disabled="line.left.commentsDisabled" + @click="handleCommentButton(line.left)" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + <a + v-if="line.left.old_line" + :data-linenumber="line.left.old_line" + :href="line.lineHrefOld" + @click="setHighlightedRow(line.lineCode)" + > + </a> + <diff-gutter-avatars + v-if="line.hasDiscussionsLeft" + :discussions="line.left.discussions" + :discussions-expanded="line.left.discussionsExpanded" + data-testid="leftDiscussions" + @toggleLineDiscussions=" + toggleLineDiscussions({ + lineCode: line.left.line_code, + fileHash, + expanded: !line.left.discussionsExpanded, + }) + " + /> + </div> + <div :class="classNameMapCellLeft" class="diff-td diff-line-num old_line"> + <a + v-if="line.left.old_line" + :data-linenumber="line.left.old_line" + :href="line.lineHrefOld" + @click="setHighlightedRow(line.lineCode)" + > + </a> + </div> + <div :class="parallelViewLeftLineType" class="diff-td line-coverage left-side"></div> + <div + :id="line.left.line_code" + :key="line.left.line_code" + v-safe-html="line.left.rich_text" + :class="parallelViewLeftLineType" + class="diff-td line_content with-coverage parallel left-side" + data-testid="leftContent" + @mousedown="handleParallelLineMouseDown" + ></div> + </template> + <template v-else> + <div data-testid="leftEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td line-coverage left-side empty-cell"></div> + <div class="diff-td line_content with-coverage parallel left-side empty-cell"></div> + </template> + </div> + <div + v-if="!inline || (line.right && Boolean(line.right.type))" + class="diff-grid-right right-side" + > + <template v-if="line.right"> + <div + :class="classNameMapCellRight" + data-testid="rightLineNumber" + class="diff-td diff-line-num new_line" + > + <span + v-if="shouldRenderCommentButton" + v-gl-tooltip + data-testid="rightCommentButton" + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipRight" + > + <button + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :disabled="line.right.commentsDisabled" + @click="handleCommentButton(line.right)" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + <a + v-if="line.right.new_line" + :data-linenumber="line.right.new_line" + :href="line.lineHrefNew" + @click="setHighlightedRow(line.lineCode)" + > + </a> + <diff-gutter-avatars + v-if="line.hasDiscussionsRight" + :discussions="line.right.discussions" + :discussions-expanded="line.right.discussionsExpanded" + data-testid="rightDiscussions" + @toggleLineDiscussions=" + toggleLineDiscussions({ + lineCode: line.right.line_code, + fileHash, + expanded: !line.right.discussionsExpanded, + }) + " + /> + </div> + <div :class="classNameMapCellRight" class="diff-td diff-line-num new_line"> + <a + v-if="line.right.new_line" + :data-linenumber="line.right.new_line" + :href="line.lineHrefNew" + @click="setHighlightedRow(line.lineCode)" + > + </a> + </div> + <div + v-gl-tooltip.hover + :title="coverageState.text" + :class="[line.right.type, coverageState.class, { hll: isHighlighted }]" + class="diff-td line-coverage right-side" + ></div> + <div + :id="line.right.line_code" + :key="line.right.rich_text" + v-safe-html="line.right.rich_text" + :class="[ + line.right.type, + { + hll: isHighlighted, + }, + ]" + class="diff-td line_content with-coverage parallel right-side" + @mousedown="handleParallelLineMouseDown" + ></div> + </template> + <template v-else> + <div data-testid="rightEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td diff-line-num old_line empty-cell"></div> + <div class="diff-td line-coverage right-side empty-cell"></div> + <div class="diff-td line_content with-coverage parallel right-side empty-cell"></div> + </template> + </div> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_row_utils.js b/app/assets/javascripts/diffs/components/diff_row_utils.js index 08b87a4bade..d5491d3cd56 100644 --- a/app/assets/javascripts/diffs/components/diff_row_utils.js +++ b/app/assets/javascripts/diffs/components/diff_row_utils.js @@ -83,3 +83,76 @@ export const parallelViewLeftLineType = (line, hll) => { export const shouldShowCommentButton = (hover, context, meta, discussions) => { return hover && !context && !meta && !discussions; }; + +export const mapParallel = content => line => { + let { left, right } = line; + + // Dicussions/Comments + const hasExpandedDiscussionOnLeft = + left?.discussions?.length > 0 ? left?.discussionsExpanded : false; + const hasExpandedDiscussionOnRight = + right?.discussions?.length > 0 ? right?.discussionsExpanded : false; + + const renderCommentRow = + hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight || left?.hasForm || right?.hasForm; + + if (left) { + left = { + ...left, + renderDiscussion: hasExpandedDiscussionOnLeft, + hasDraft: content.hasParallelDraftLeft(content.diffFile.file_hash, line), + lineDraft: content.draftForLine(content.diffFile.file_hash, line, 'left'), + hasCommentForm: left.hasForm, + }; + } + if (right) { + right = { + ...right, + renderDiscussion: Boolean(hasExpandedDiscussionOnRight && right.type), + hasDraft: content.hasParallelDraftRight(content.diffFile.file_hash, line), + lineDraft: content.draftForLine(content.diffFile.file_hash, line, 'right'), + hasCommentForm: Boolean(right.hasForm && right.type), + }; + } + + return { + ...line, + left, + right, + isMatchLineLeft: isMatchLine(left?.type), + isMatchLineRight: isMatchLine(right?.type), + isContextLineLeft: isContextLine(left?.type), + isContextLineRight: isContextLine(right?.type), + hasDiscussionsLeft: hasDiscussions(left), + hasDiscussionsRight: hasDiscussions(right), + lineHrefOld: lineHref(left), + lineHrefNew: lineHref(right), + lineCode: lineCode(line), + isMetaLineLeft: isMetaLine(left?.type), + isMetaLineRight: isMetaLine(right?.type), + draftRowClasses: left?.lineDraft > 0 || right?.lineDraft > 0 ? '' : 'js-temp-notes-holder', + renderCommentRow, + commentRowClasses: hasDiscussions(left) || hasDiscussions(right) ? '' : 'js-temp-notes-holder', + }; +}; + +// TODO: Delete this function when unifiedDiffComponents FF is removed +export const mapInline = content => line => { + // Discussions/Comments + const renderCommentRow = line.hasForm || (line.discussions?.length && line.discussionsExpanded); + + return { + ...line, + renderDiscussion: Boolean(line.discussions?.length), + isMatchLine: isMatchLine(line.type), + commentRowClasses: line.discussions?.length ? '' : 'js-temp-notes-holder', + renderCommentRow, + hasDraft: content.shouldRenderDraftRow(content.diffFile.file_hash, line), + hasCommentForm: line.hasForm, + isMetaLine: isMetaLine(line.type), + isContextLine: isContextLine(line.type), + hasDiscussions: hasDiscussions(line), + lineHref: lineHref(line), + lineCode: lineCode(line), + }; +}; diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue new file mode 100644 index 00000000000..8747678e69b --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -0,0 +1,149 @@ +<script> +import { mapGetters, mapState } from 'vuex'; +import draftCommentsMixin from '~/diffs/mixins/draft_comments'; +import DraftNote from '~/batch_comments/components/draft_note.vue'; +import DiffRow from './diff_row.vue'; +import DiffCommentCell from './diff_comment_cell.vue'; +import DiffExpansionCell from './diff_expansion_cell.vue'; +import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; + +export default { + components: { + DiffExpansionCell, + DiffRow, + DiffCommentCell, + DraftNote, + }, + mixins: [draftCommentsMixin], + props: { + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + helpPagePath: { + type: String, + required: false, + default: '', + }, + inline: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapGetters('diffs', ['commitId']), + ...mapState({ + selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition, + selectedCommentPositionHover: ({ notes }) => notes.selectedCommentPositionHover, + }), + diffLinesLength() { + return this.diffLines.length; + }, + commentedLines() { + return getCommentedLines( + this.selectedCommentPosition || this.selectedCommentPositionHover, + this.diffLines, + ); + }, + }, + userColorScheme: window.gon.user_color_scheme, +}; +</script> + +<template> + <div + :class="[$options.userColorScheme, { inline }]" + :data-commit-id="commitId" + class="diff-grid diff-table code diff-wrap-lines js-syntax-highlight text-file" + > + <template v-for="(line, index) in diffLines"> + <div + v-if="line.isMatchLineLeft || line.isMatchLineRight" + :key="`expand-${index}`" + class="diff-tr line_expansion match" + > + <div class="diff-td text-center gl-font-regular"> + <diff-expansion-cell + :file-hash="diffFile.file_hash" + :context-lines-path="diffFile.context_lines_path" + :line="line.left" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + /> + </div> + </div> + <diff-row + v-if="!line.isMatchLineLeft && !line.isMatchLineRight" + :key="line.line_code" + :file-hash="diffFile.file_hash" + :file-path="diffFile.file_path" + :line="line" + :is-bottom="index + 1 === diffLinesLength" + :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" + :inline="inline" + /> + <div + v-if="line.renderCommentRow" + :key="`dcr-${line.line_code || index}`" + :class="line.commentRowClasses" + class="diff-grid-comments diff-tr notes_holder" + > + <div + v-if="!inline || (line.left && line.left.discussions.length)" + class="diff-td notes-content parallel old" + > + <diff-comment-cell + v-if="line.left" + :line="line.left" + :diff-file-hash="diffFile.file_hash" + :help-page-path="helpPagePath" + :has-draft="line.left.hasDraft" + line-position="left" + /> + </div> + <div + v-if="!inline || (line.right && line.right.discussions.length)" + class="diff-td notes-content parallel new" + > + <diff-comment-cell + v-if="line.right" + :line="line.right" + :diff-file-hash="diffFile.file_hash" + :line-index="index" + :help-page-path="helpPagePath" + :has-draft="line.right.hasDraft" + line-position="right" + /> + </div> + </div> + <div + v-if="shouldRenderParallelDraftRow(diffFile.file_hash, line)" + :key="`drafts-${index}`" + :class="line.draftRowClasses" + class="diff-grid-drafts diff-tr notes_holder" + > + <div + v-if="!inline || (line.left && line.left.lineDraft.isDraft)" + class="diff-td notes-content parallel old" + > + <div v-if="line.left && line.left.lineDraft.isDraft" class="content"> + <draft-note :draft="line.left.lineDraft" :line="line.left" /> + </div> + </div> + <div + v-if="!inline || (line.right && line.right.lineDraft.isDraft)" + class="diff-td notes-content parallel new" + > + <div v-if="line.right && line.right.lineDraft.isDraft" class="content"> + <draft-note :draft="line.right.lineDraft" :line="line.right" /> + </div> + </div> + </div> + </template> + </div> +</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue deleted file mode 100644 index 87f0396cf72..00000000000 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ /dev/null @@ -1,82 +0,0 @@ -<script> -import { mapActions } from 'vuex'; -import DiffDiscussions from './diff_discussions.vue'; -import DiffLineNoteForm from './diff_line_note_form.vue'; -import DiffDiscussionReply from './diff_discussion_reply.vue'; - -export default { - components: { - DiffDiscussions, - DiffLineNoteForm, - DiffDiscussionReply, - }, - props: { - line: { - type: Object, - required: true, - }, - diffFileHash: { - type: String, - required: true, - }, - helpPagePath: { - type: String, - required: false, - default: '', - }, - hasDraft: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - className() { - return this.line.discussions.length ? '' : 'js-temp-notes-holder'; - }, - shouldRender() { - if (this.line.hasForm) return true; - - if (!this.line.discussions || !this.line.discussions.length) { - return false; - } - return this.line.discussionsExpanded; - }, - }, - methods: { - ...mapActions('diffs', ['showCommentForm']), - }, -}; -</script> - -<template> - <tr v-if="shouldRender" :class="className" class="notes_holder"> - <td class="notes-content" colspan="4"> - <div class="content"> - <diff-discussions - v-if="line.discussions.length" - :line="line" - :discussions="line.discussions" - :help-page-path="helpPagePath" - /> - <diff-discussion-reply - v-if="!hasDraft" - :has-form="line.hasForm" - :render-reply-placeholder="Boolean(line.discussions.length)" - @showNewDiscussionForm=" - showCommentForm({ lineCode: line.line_code, fileHash: diffFileHash }) - " - > - <template #form> - <diff-line-note-form - :diff-file-hash="diffFileHash" - :line="line" - :note-target-line="line" - :help-page-path="helpPagePath" - /> - </template> - </diff-discussion-reply> - </div> - </td> - </tr> -</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_expansion_row.vue b/app/assets/javascripts/diffs/components/inline_diff_expansion_row.vue deleted file mode 100644 index 071a988d789..00000000000 --- a/app/assets/javascripts/diffs/components/inline_diff_expansion_row.vue +++ /dev/null @@ -1,51 +0,0 @@ -<script> -import DiffExpansionCell from './diff_expansion_cell.vue'; -import { MATCH_LINE_TYPE } from '../constants'; - -export default { - components: { - DiffExpansionCell, - }, - props: { - fileHash: { - type: String, - required: true, - }, - contextLinesPath: { - type: String, - required: true, - }, - line: { - type: Object, - required: true, - }, - isTop: { - type: Boolean, - required: false, - default: false, - }, - isBottom: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - isMatchLine() { - return this.line.type === MATCH_LINE_TYPE; - }, - }, -}; -</script> - -<template> - <tr v-if="isMatchLine" class="line_expansion match"> - <diff-expansion-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line" - :is-top="isTop" - :is-bottom="isBottom" - /> - </tr> -</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue index 99cf79a70d4..2d8ffb047ca 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -3,7 +3,13 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; import { CONTEXT_LINE_CLASS_NAME } from '../constants'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; -import * as utils from './diff_row_utils'; +import { + isHighlighted, + shouldShowCommentButton, + shouldRenderCommentButton, + classNameMapCell, + addCommentTooltip, +} from './diff_row_utils'; export default { components: { @@ -48,60 +54,42 @@ export default { ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { - return utils.isHighlighted(state, this.line, this.isCommented); + return isHighlighted(state, this.line, this.isCommented); }, }), - isContextLine() { - return utils.isContextLine(this.line.type); - }, classNameMap() { return [ this.line.type, { - [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, + [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLine, }, ]; }, inlineRowId() { return this.line.line_code || `${this.fileHash}_${this.line.old_line}_${this.line.new_line}`; }, - isMatchLine() { - return utils.isMatchLine(this.line.type); - }, coverageState() { return this.fileLineCoverage(this.filePath, this.line.new_line); }, - isMetaLine() { - return utils.isMetaLine(this.line.type); - }, classNameMapCell() { - return utils.classNameMapCell(this.line, this.isHighlighted, this.isLoggedIn, this.isHover); + return classNameMapCell(this.line, this.isHighlighted, this.isLoggedIn, this.isHover); }, addCommentTooltip() { - return utils.addCommentTooltip(this.line); + return addCommentTooltip(this.line); }, shouldRenderCommentButton() { - return utils.shouldRenderCommentButton(this.isLoggedIn, true); + return shouldRenderCommentButton(this.isLoggedIn, true); }, shouldShowCommentButton() { - return utils.shouldShowCommentButton( + return shouldShowCommentButton( this.isHover, - this.isContextLine, - this.isMetaLine, - this.hasDiscussions, + this.line.isContextLine, + this.line.isMetaLine, + this.line.hasDiscussions, ); }, - hasDiscussions() { - return utils.hasDiscussions(this.line); - }, - lineHref() { - return utils.lineHref(this.line); - }, - lineCode() { - return utils.lineCode(this.line); - }, shouldShowAvatarsOnGutter() { - return this.hasDiscussions; + return this.line.hasDiscussions; }, }, mounted() { @@ -128,7 +116,6 @@ export default { <template> <tr - v-if="!isMatchLine" :id="inlineRowId" :class="classNameMap" class="line_holder" @@ -158,8 +145,8 @@ export default { v-if="line.old_line" ref="lineNumberRefOld" :data-linenumber="line.old_line" - :href="lineHref" - @click="setHighlightedRow(lineCode)" + :href="line.lineHref" + @click="setHighlightedRow(line.lineCode)" > </a> <diff-gutter-avatars @@ -167,7 +154,11 @@ export default { :discussions="line.discussions" :discussions-expanded="line.discussionsExpanded" @toggleLineDiscussions=" - toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded }) + toggleLineDiscussions({ + lineCode: line.lineCode, + fileHash, + expanded: !line.discussionsExpanded, + }) " /> </td> @@ -176,8 +167,8 @@ export default { v-if="line.new_line" ref="lineNumberRefNew" :data-linenumber="line.new_line" - :href="lineHref" - @click="setHighlightedRow(lineCode)" + :href="line.lineHref" + @click="setHighlightedRow(line.lineCode)" > </a> </td> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 13805910648..05f5461054f 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -2,18 +2,18 @@ import { mapGetters, mapState } from 'vuex'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; -import InlineDraftCommentRow from '~/batch_comments/components/inline_draft_comment_row.vue'; +import DraftNote from '~/batch_comments/components/draft_note.vue'; import inlineDiffTableRow from './inline_diff_table_row.vue'; -import inlineDiffCommentRow from './inline_diff_comment_row.vue'; -import inlineDiffExpansionRow from './inline_diff_expansion_row.vue'; +import DiffCommentCell from './diff_comment_cell.vue'; +import DiffExpansionCell from './diff_expansion_cell.vue'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; export default { components: { - inlineDiffCommentRow, + DiffCommentCell, inlineDiffTableRow, - InlineDraftCommentRow, - inlineDiffExpansionRow, + DraftNote, + DiffExpansionCell, }, mixins: [draftCommentsMixin, glFeatureFlagsMixin()], props: { @@ -65,15 +65,19 @@ export default { </colgroup> <tbody> <template v-for="(line, index) in diffLines"> - <inline-diff-expansion-row - :key="`expand-${index}`" - :file-hash="diffFile.file_hash" - :context-lines-path="diffFile.context_lines_path" - :line="line" - :is-top="index === 0" - :is-bottom="index + 1 === diffLinesLength" - /> + <tr v-if="line.isMatchLine" :key="`expand-${index}`" class="line_expansion match"> + <td colspan="4" class="text-center gl-font-regular"> + <diff-expansion-cell + :file-hash="diffFile.file_hash" + :context-lines-path="diffFile.context_lines_path" + :line="line" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + /> + </td> + </tr> <inline-diff-table-row + v-if="!line.isMatchLine" :key="`${line.line_code || index}`" :file-hash="diffFile.file_hash" :file-path="diffFile.file_path" @@ -81,20 +85,32 @@ export default { :is-bottom="index + 1 === diffLinesLength" :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" /> - <inline-diff-comment-row + <tr + v-if="line.renderCommentRow" :key="`icr-${line.line_code || index}`" - :diff-file-hash="diffFile.file_hash" - :line="line" - :help-page-path="helpPagePath" - :has-draft="shouldRenderDraftRow(diffFile.file_hash, line) || false" - /> - <inline-draft-comment-row - v-if="shouldRenderDraftRow(diffFile.file_hash, line)" - :key="`draft_${index}`" - :draft="draftForLine(diffFile.file_hash, line)" - :diff-file="diffFile" - :line="line" - /> + :class="line.commentRowClasses" + class="notes_holder" + > + <td class="notes-content" colspan="4"> + <diff-comment-cell + :diff-file-hash="diffFile.file_hash" + :line="line" + :help-page-path="helpPagePath" + :has-draft="line.hasDraft" + /> + </td> + </tr> + <tr v-if="line.hasDraft" :key="`draft_${index}`" class="notes_holder js-temp-notes-holder"> + <td class="notes-content" colspan="4"> + <div class="content"> + <draft-note + :draft="draftForLine(diffFile.file_hash, line)" + :diff-file="diffFile" + :line="line" + /> + </div> + </td> + </tr> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue deleted file mode 100644 index 127e3f214cf..00000000000 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ /dev/null @@ -1,175 +0,0 @@ -<script> -import { mapActions } from 'vuex'; -import DiffDiscussions from './diff_discussions.vue'; -import DiffLineNoteForm from './diff_line_note_form.vue'; -import DiffDiscussionReply from './diff_discussion_reply.vue'; - -export default { - components: { - DiffDiscussions, - DiffLineNoteForm, - DiffDiscussionReply, - }, - props: { - line: { - type: Object, - required: true, - }, - diffFileHash: { - type: String, - required: true, - }, - lineIndex: { - type: Number, - required: true, - }, - helpPagePath: { - type: String, - required: false, - default: '', - }, - hasDraftLeft: { - type: Boolean, - required: false, - default: false, - }, - hasDraftRight: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - hasExpandedDiscussionOnLeft() { - return this.line.left && this.line.left.discussions.length - ? this.line.left.discussionsExpanded - : false; - }, - hasExpandedDiscussionOnRight() { - return this.line.right && this.line.right.discussions.length - ? this.line.right.discussionsExpanded - : false; - }, - hasAnyExpandedDiscussion() { - return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; - }, - shouldRenderDiscussionsOnLeft() { - return ( - this.line.left && - this.line.left.discussions && - this.line.left.discussions.length && - this.hasExpandedDiscussionOnLeft - ); - }, - shouldRenderDiscussionsOnRight() { - return ( - this.line.right && - this.line.right.discussions && - this.line.right.discussions.length && - this.hasExpandedDiscussionOnRight && - this.line.right.type - ); - }, - showRightSideCommentForm() { - return this.line.right && this.line.right.type && this.line.right.hasForm; - }, - showLeftSideCommentForm() { - return this.line.left && this.line.left.hasForm; - }, - className() { - return (this.left && this.line.left.discussions.length > 0) || - (this.right && this.line.right.discussions.length > 0) - ? '' - : 'js-temp-notes-holder'; - }, - shouldRender() { - const { line } = this; - const hasDiscussion = - (line.left && line.left.discussions && line.left.discussions.length) || - (line.right && line.right.discussions && line.right.discussions.length); - - if ( - hasDiscussion && - (this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight) - ) { - return true; - } - - const hasCommentFormOnLeft = line.left && line.left.hasForm; - const hasCommentFormOnRight = line.right && line.right.hasForm; - - return hasCommentFormOnLeft || hasCommentFormOnRight; - }, - shouldRenderReplyPlaceholderOnLeft() { - return Boolean( - this.line.left && this.line.left.discussions && this.line.left.discussions.length, - ); - }, - shouldRenderReplyPlaceholderOnRight() { - return Boolean( - this.line.right && this.line.right.discussions && this.line.right.discussions.length, - ); - }, - }, - methods: { - ...mapActions('diffs', ['showCommentForm']), - showNewDiscussionForm(lineCode) { - this.showCommentForm({ lineCode, fileHash: this.diffFileHash }); - }, - }, -}; -</script> - -<template> - <tr v-if="shouldRender" :class="className" class="notes_holder"> - <td class="notes-content parallel old" colspan="3"> - <div v-if="shouldRenderDiscussionsOnLeft" class="content"> - <diff-discussions - :discussions="line.left.discussions" - :line="line.left" - :help-page-path="helpPagePath" - /> - </div> - <diff-discussion-reply - v-if="!hasDraftLeft" - :has-form="showLeftSideCommentForm" - :render-reply-placeholder="shouldRenderReplyPlaceholderOnLeft" - @showNewDiscussionForm="showNewDiscussionForm(line.left.line_code)" - > - <template #form> - <diff-line-note-form - :diff-file-hash="diffFileHash" - :line="line.left" - :note-target-line="line.left" - :help-page-path="helpPagePath" - line-position="left" - /> - </template> - </diff-discussion-reply> - </td> - <td class="notes-content parallel new" colspan="3"> - <div v-if="shouldRenderDiscussionsOnRight" class="content"> - <diff-discussions - :discussions="line.right.discussions" - :line="line.right" - :help-page-path="helpPagePath" - /> - </div> - <diff-discussion-reply - v-if="!hasDraftRight" - :has-form="showRightSideCommentForm" - :render-reply-placeholder="shouldRenderReplyPlaceholderOnRight" - @showNewDiscussionForm="showNewDiscussionForm(line.right.line_code)" - > - <template #form> - <diff-line-note-form - :diff-file-hash="diffFileHash" - :line="line.right" - :note-target-line="line.right" - line-position="right" - /> - </template> - </diff-discussion-reply> - </td> - </tr> -</template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue deleted file mode 100644 index 0a80107ced4..00000000000 --- a/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue +++ /dev/null @@ -1,56 +0,0 @@ -<script> -import { MATCH_LINE_TYPE } from '../constants'; -import DiffExpansionCell from './diff_expansion_cell.vue'; - -export default { - components: { - DiffExpansionCell, - }, - props: { - fileHash: { - type: String, - required: true, - }, - contextLinesPath: { - type: String, - required: true, - }, - line: { - type: Object, - required: true, - }, - isTop: { - type: Boolean, - required: false, - default: false, - }, - isBottom: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - isMatchLineLeft() { - return this.line.left && this.line.left.type === MATCH_LINE_TYPE; - }, - isMatchLineRight() { - return this.line.right && this.line.right.type === MATCH_LINE_TYPE; - }, - }, -}; -</script> -<template> - <tr class="line_expansion match"> - <template v-if="isMatchLineLeft || isMatchLineRight"> - <diff-expansion-cell - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line="line.left" - :is-top="isTop" - :is-bottom="isBottom" - :colspan="6" - /> - </template> - </tr> -</template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index cdc6db791f0..13cd0651ff2 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -55,27 +55,15 @@ export default { return utils.isHighlighted(state, line, this.isCommented); }, }), - isContextLineLeft() { - return utils.isContextLine(this.line.left?.type); - }, - isContextLineRight() { - return utils.isContextLine(this.line.right?.type); - }, classNameMap() { return { - [CONTEXT_LINE_CLASS_NAME]: this.isContextLineLeft, + [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLineLeft, [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, parallelViewLeftLineType() { return utils.parallelViewLeftLineType(this.line, this.isHighlighted); }, - isMatchLineLeft() { - return utils.isMatchLine(this.line.left?.type); - }, - isMatchLineRight() { - return utils.isMatchLine(this.line.right?.type); - }, coverageState() { return this.fileLineCoverage(this.filePath, this.line.right.new_line); }, @@ -107,40 +95,19 @@ export default { shouldShowCommentButtonLeft() { return utils.shouldShowCommentButton( this.isLeftHover, - this.isContextLineLeft, - this.isMetaLineLeft, - this.hasDiscussionsLeft, + this.line.isContextLineLeft, + this.line.isMetaLineLeft, + this.line.hasDiscussionsLeft, ); }, shouldShowCommentButtonRight() { return utils.shouldShowCommentButton( this.isRightHover, - this.isContextLineRight, - this.isMetaLineRight, - this.hasDiscussionsRight, + this.line.isContextLineRight, + this.line.isMetaLineRight, + this.line.hasDiscussionsRight, ); }, - hasDiscussionsLeft() { - return utils.hasDiscussions(this.line.left); - }, - hasDiscussionsRight() { - return utils.hasDiscussions(this.line.right); - }, - lineHrefOld() { - return utils.lineHref(this.line.left); - }, - lineHrefNew() { - return utils.lineHref(this.line.right); - }, - lineCode() { - return utils.lineCode(this.line); - }, - isMetaLineLeft() { - return utils.isMetaLine(this.line.left?.type); - }, - isMetaLineRight() { - return utils.isMetaLine(this.line.right?.type); - }, }, mounted() { this.scrollToLineIfNeededParallel(this.line); @@ -203,7 +170,7 @@ export default { @mouseover="handleMouseMove" @mouseout="handleMouseMove" > - <template v-if="line.left && !isMatchLineLeft"> + <template v-if="line.left && !line.isMatchLineLeft"> <td ref="oldTd" :class="classNameMapCellLeft" class="diff-line-num old_line"> <span v-if="shouldRenderCommentButton" @@ -227,12 +194,12 @@ export default { v-if="line.left.old_line" ref="lineNumberRefOld" :data-linenumber="line.left.old_line" - :href="lineHrefOld" - @click="setHighlightedRow(lineCode)" + :href="line.lineHrefOld" + @click="setHighlightedRow(line.lineCode)" > </a> <diff-gutter-avatars - v-if="hasDiscussionsLeft" + v-if="line.hasDiscussionsLeft" :discussions="line.left.discussions" :discussions-expanded="line.left.discussionsExpanded" @toggleLineDiscussions=" @@ -259,7 +226,7 @@ export default { <td class="line-coverage left-side empty-cell"></td> <td class="line_content with-coverage parallel left-side empty-cell"></td> </template> - <template v-if="line.right && !isMatchLineRight"> + <template v-if="line.right && !line.isMatchLineRight"> <td ref="newTd" :class="classNameMapCellRight" class="diff-line-num new_line"> <span v-if="shouldRenderCommentButton" @@ -283,12 +250,12 @@ export default { v-if="line.right.new_line" ref="lineNumberRefNew" :data-linenumber="line.right.new_line" - :href="lineHrefNew" - @click="setHighlightedRow(lineCode)" + :href="line.lineHrefNew" + @click="setHighlightedRow(line.lineCode)" > </a> <diff-gutter-avatars - v-if="hasDiscussionsRight" + v-if="line.hasDiscussionsRight" :discussions="line.right.discussions" :discussions-expanded="line.right.discussionsExpanded" @toggleLineDiscussions=" diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 46a691ad22d..67b599fe163 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -1,18 +1,18 @@ <script> import { mapGetters, mapState } from 'vuex'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; -import ParallelDraftCommentRow from '~/batch_comments/components/parallel_draft_comment_row.vue'; +import DraftNote from '~/batch_comments/components/draft_note.vue'; import parallelDiffTableRow from './parallel_diff_table_row.vue'; -import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; -import parallelDiffExpansionRow from './parallel_diff_expansion_row.vue'; +import DiffCommentCell from './diff_comment_cell.vue'; +import DiffExpansionCell from './diff_expansion_cell.vue'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; export default { components: { - parallelDiffExpansionRow, + DiffExpansionCell, parallelDiffTableRow, - parallelDiffCommentRow, - ParallelDraftCommentRow, + DiffCommentCell, + DraftNote, }, mixins: [draftCommentsMixin], props: { @@ -66,14 +66,21 @@ export default { </colgroup> <tbody> <template v-for="(line, index) in diffLines"> - <parallel-diff-expansion-row + <tr + v-if="line.isMatchLineLeft || line.isMatchLineRight" :key="`expand-${index}`" - :file-hash="diffFile.file_hash" - :context-lines-path="diffFile.context_lines_path" - :line="line" - :is-top="index === 0" - :is-bottom="index + 1 === diffLinesLength" - /> + class="line_expansion match" + > + <td colspan="6" class="text-center gl-font-regular"> + <diff-expansion-cell + :file-hash="diffFile.file_hash" + :context-lines-path="diffFile.context_lines_path" + :line="line.left" + :is-top="index === 0" + :is-bottom="index + 1 === diffLinesLength" + /> + </td> + </tr> <parallel-diff-table-row :key="line.line_code" :file-hash="diffFile.file_hash" @@ -82,21 +89,53 @@ export default { :is-bottom="index + 1 === diffLinesLength" :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" /> - <parallel-diff-comment-row + <tr + v-if="line.renderCommentRow" :key="`dcr-${line.line_code || index}`" - :line="line" - :diff-file-hash="diffFile.file_hash" - :line-index="index" - :help-page-path="helpPagePath" - :has-draft-left="hasParallelDraftLeft(diffFile.file_hash, line) || false" - :has-draft-right="hasParallelDraftRight(diffFile.file_hash, line) || false" - /> - <parallel-draft-comment-row + :class="line.commentRowClasses" + class="notes_holder" + > + <td class="notes-content parallel old" colspan="3"> + <diff-comment-cell + v-if="line.left" + :line="line.left" + :diff-file-hash="diffFile.file_hash" + :help-page-path="helpPagePath" + :has-draft="line.left.hasDraft" + line-position="left" + /> + </td> + <td class="notes-content parallel new" colspan="3"> + <diff-comment-cell + v-if="line.right" + :line="line.right" + :diff-file-hash="diffFile.file_hash" + :line-index="index" + :help-page-path="helpPagePath" + :has-draft="line.right.hasDraft" + line-position="right" + /> + </td> + </tr> + <tr v-if="shouldRenderParallelDraftRow(diffFile.file_hash, line)" :key="`drafts-${index}`" - :line="line" - :diff-file-content-sha="diffFile.file_hash" - /> + :class="line.draftRowClasses" + class="notes_holder" + > + <td class="notes_line old"></td> + <td class="notes-content parallel old" colspan="2"> + <div v-if="line.left && line.left.lineDraft.isDraft" class="content"> + <draft-note :draft="line.left.lineDraft" :line="line.left" /> + </div> + </td> + <td class="notes_line new"></td> + <td class="notes-content parallel new" colspan="2"> + <div v-if="line.right && line.right.lineDraft.isDraft" class="content"> + <draft-note :draft="line.right.lineDraft" :line="line.right" /> + </div> + </td> + </tr> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 4900dcd473d..3ec9720c907 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -165,8 +165,8 @@ export const fileLineCoverage = state => (file, line) => { export const currentDiffIndex = state => Math.max(0, state.diffFiles.findIndex(diff => diff.file_hash === state.currentDiffFileId)); -export const diffLines = state => file => { - if (state.diffViewType === INLINE_DIFF_VIEW_TYPE) { +export const diffLines = state => (file, unifiedDiffComponents) => { + if (!unifiedDiffComponents && state.diffViewType === INLINE_DIFF_VIEW_TYPE) { return null; } diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js index 0a0cfe918af..f65d9259e7b 100644 --- a/app/assets/javascripts/issue.js +++ b/app/assets/javascripts/issue.js @@ -84,13 +84,7 @@ export default class Issue { projectIssuesCounter.text(addDelimiter(numProjectIssues)); if (this.createMergeRequestDropdown) { - if (isClosed) { - this.createMergeRequestDropdown.unavailable(); - this.createMergeRequestDropdown.disable(); - } else { - // We should check in case a branch was created in another tab - this.createMergeRequestDropdown.checkAbilityToCreateBranch(); - } + this.createMergeRequestDropdown.checkAbilityToCreateBranch(); } } else { flash(issueFailMessage); diff --git a/app/assets/javascripts/jobs/components/job_retry_forward_deployment_modal.vue b/app/assets/javascripts/jobs/components/job_retry_forward_deployment_modal.vue new file mode 100644 index 00000000000..5ce9d08035d --- /dev/null +++ b/app/assets/javascripts/jobs/components/job_retry_forward_deployment_modal.vue @@ -0,0 +1,66 @@ +<script> +import { GlLink, GlModal } from '@gitlab/ui'; +import { JOB_RETRY_FORWARD_DEPLOYMENT_MODAL } from '../constants'; + +export default { + name: 'JobRetryForwardDeploymentModal', + components: { + GlLink, + GlModal, + }, + i18n: { + ...JOB_RETRY_FORWARD_DEPLOYMENT_MODAL, + }, + props: { + modalId: { + type: String, + required: true, + }, + href: { + type: String, + required: true, + }, + }, + inject: { + retryOutdatedJobDocsUrl: { + default: '', + }, + }, + data() { + return { + primaryProps: { + text: this.$options.i18n.primaryText, + attributes: [ + { + 'data-method': 'post', + 'data-testid': 'retry-button-modal', + href: this.href, + variant: 'danger', + }, + ], + }, + cancelProps: { + text: this.$options.i18n.cancel, + attributes: [{ category: 'secondary', variant: 'default' }], + }, + }; + }, +}; +</script> + +<template> + <gl-modal + :action-cancel="cancelProps" + :action-primary="primaryProps" + :modal-id="modalId" + :title="$options.i18n.title" + > + <p> + {{ $options.i18n.info }} + <gl-link v-if="retryOutdatedJobDocsUrl" :href="retryOutdatedJobDocsUrl" target="_blank"> + {{ $options.i18n.moreInfo }} + </gl-link> + </p> + <p>{{ $options.i18n.areYouSure }}</p> + </gl-modal> +</template> diff --git a/app/assets/javascripts/jobs/components/job_sidebar_retry_button.vue b/app/assets/javascripts/jobs/components/job_sidebar_retry_button.vue new file mode 100644 index 00000000000..258b8cadd63 --- /dev/null +++ b/app/assets/javascripts/jobs/components/job_sidebar_retry_button.vue @@ -0,0 +1,45 @@ +<script> +import { GlButton, GlLink, GlModalDirective } from '@gitlab/ui'; +import { mapGetters } from 'vuex'; +import { JOB_SIDEBAR } from '../constants'; + +export default { + name: 'JobSidebarRetryButton', + i18n: { + retryLabel: JOB_SIDEBAR.retry, + }, + components: { + GlButton, + GlLink, + }, + directives: { + GlModal: GlModalDirective, + }, + props: { + modalId: { + type: String, + required: true, + }, + href: { + type: String, + required: true, + }, + }, + computed: { + ...mapGetters(['hasForwardDeploymentFailure']), + }, +}; +</script> +<template> + <gl-button + v-if="hasForwardDeploymentFailure" + v-gl-modal="modalId" + :aria-label="$options.i18n.retryLabel" + category="primary" + variant="info" + >{{ $options.i18n.retryLabel }}</gl-button + > + <gl-link v-else :href="href" data-method="post" rel="nofollow" + >{{ $options.i18n.retryLabel }} + </gl-link> +</template> diff --git a/app/assets/javascripts/jobs/components/jobs_container.vue b/app/assets/javascripts/jobs/components/jobs_container.vue index 951bcb36600..df64b6422c7 100644 --- a/app/assets/javascripts/jobs/components/jobs_container.vue +++ b/app/assets/javascripts/jobs/components/jobs_container.vue @@ -24,7 +24,7 @@ export default { }; </script> <template> - <div class="js-jobs-container builds-container"> + <div class="builds-container"> <job-container-item v-for="job in jobs" :key="job.id" diff --git a/app/assets/javascripts/jobs/components/sidebar.vue b/app/assets/javascripts/jobs/components/sidebar.vue index e1372a5c05e..0789bb54f0f 100644 --- a/app/assets/javascripts/jobs/components/sidebar.vue +++ b/app/assets/javascripts/jobs/components/sidebar.vue @@ -1,28 +1,39 @@ <script> import { isEmpty } from 'lodash'; -import { mapActions, mapState } from 'vuex'; +import { mapActions, mapGetters, mapState } from 'vuex'; import { GlButton, GlIcon, GlLink } from '@gitlab/ui'; import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate.vue'; import ArtifactsBlock from './artifacts_block.vue'; +import JobSidebarRetryButton from './job_sidebar_retry_button.vue'; +import JobRetryForwardDeploymentModal from './job_retry_forward_deployment_modal.vue'; import TriggerBlock from './trigger_block.vue'; import CommitBlock from './commit_block.vue'; import StagesDropdown from './stages_dropdown.vue'; import JobsContainer from './jobs_container.vue'; -import SidebarJobDetailsContainer from './sidebar_job_details_container.vue'; +import JobSidebarDetailsContainer from './sidebar_job_details_container.vue'; +import { JOB_SIDEBAR } from '../constants'; + +export const forwardDeploymentFailureModalId = 'forward-deployment-failure'; export default { name: 'JobSidebar', + i18n: { + ...JOB_SIDEBAR, + }, + forwardDeploymentFailureModalId, components: { ArtifactsBlock, CommitBlock, + GlButton, + GlLink, GlIcon, - TriggerBlock, - StagesDropdown, JobsContainer, - GlLink, - GlButton, - SidebarJobDetailsContainer, + JobSidebarRetryButton, + JobRetryForwardDeploymentModal, + JobSidebarDetailsContainer, + StagesDropdown, TooltipOnTruncate, + TriggerBlock, }, props: { artifactHelpUrl: { @@ -37,9 +48,10 @@ export default { }, }, computed: { + ...mapGetters(['hasForwardDeploymentFailure']), ...mapState(['job', 'stages', 'jobs', 'selectedStage']), retryButtonClass() { - let className = 'js-retry-button btn btn-retry'; + let className = 'btn btn-retry'; className += this.job.status && this.job.recoverable ? ' btn-primary' : ' btn-inverted-secondary'; return className; @@ -56,6 +68,9 @@ export default { commit() { return this.job?.pipeline?.commit || {}; }, + shouldShowJobRetryForwardDeploymentModal() { + return this.job.retry_path && this.hasForwardDeploymentFailure; + }, }, methods: { ...mapActions(['fetchJobsForStage', 'toggleSidebar']), @@ -73,27 +88,27 @@ export default { </h4> </tooltip-on-truncate> <div class="flex-grow-1 flex-shrink-0 text-right"> - <gl-link + <job-sidebar-retry-button v-if="job.retry_path" :class="retryButtonClass" :href="job.retry_path" - data-method="post" + :modal-id="$options.forwardDeploymentFailureModalId" data-qa-selector="retry_button" - rel="nofollow" - >{{ __('Retry') }} - </gl-link> + data-testid="retry-button" + /> <gl-link v-if="job.cancel_path" :href="job.cancel_path" - class="js-cancel-job btn btn-default" + class="btn btn-default" data-method="post" + data-testid="cancel-button" rel="nofollow" - >{{ __('Cancel') }} + >{{ $options.i18n.cancel }} </gl-link> </div> <gl-button - :aria-label="__('Toggle Sidebar')" + :aria-label="$options.i18n.toggleSidebar" category="tertiary" class="gl-display-md-none gl-ml-2 js-sidebar-build-toggle" icon="chevron-double-lg-right" @@ -107,19 +122,20 @@ export default { :href="job.new_issue_path" class="btn btn-success btn-inverted float-left mr-2" data-testid="job-new-issue" - >{{ __('New issue') }} + >{{ $options.i18n.newIssue }} </gl-link> <gl-link v-if="job.terminal_path" :href="job.terminal_path" - class="js-terminal-link btn btn-primary btn-inverted visible-md-block visible-lg-block float-left" + class="btn btn-primary btn-inverted visible-md-block visible-lg-block float-left" target="_blank" + data-testid="terminal-link" > - {{ __('Debug') }} + {{ $options.i18n.debug }} <gl-icon :size="14" name="external-link" /> </gl-link> </div> - <sidebar-job-details-container :runner-help-url="runnerHelpUrl" /> + <job-sidebar-details-container :runner-help-url="runnerHelpUrl" /> <artifacts-block v-if="hasArtifact" :artifact="job.artifact" :help-url="artifactHelpUrl" /> <trigger-block v-if="hasTriggers" :trigger="job.trigger" /> <commit-block @@ -139,5 +155,10 @@ export default { <jobs-container v-if="jobs.length" :job-id="job.id" :jobs="jobs" /> </div> + <job-retry-forward-deployment-modal + v-if="shouldShowJobRetryForwardDeploymentModal" + :modal-id="$options.forwardDeploymentFailureModalId" + :href="job.retry_path" + /> </aside> </template> diff --git a/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue b/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue index 887fea982ad..8ad1008278e 100644 --- a/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue +++ b/app/assets/javascripts/jobs/components/sidebar_job_details_container.vue @@ -6,7 +6,7 @@ import timeagoMixin from '~/vue_shared/mixins/timeago'; import { timeIntervalInWords } from '~/lib/utils/datetime_utility'; export default { - name: 'SidebarJobDetailsContainer', + name: 'JobSidebarDetailsContainer', components: { DetailRow, }, diff --git a/app/assets/javascripts/jobs/components/stages_dropdown.vue b/app/assets/javascripts/jobs/components/stages_dropdown.vue index 116331d9549..aeae9f26ed3 100644 --- a/app/assets/javascripts/jobs/components/stages_dropdown.vue +++ b/app/assets/javascripts/jobs/components/stages_dropdown.vue @@ -1,11 +1,13 @@ <script> import { isEmpty } from 'lodash'; -import { GlLink } from '@gitlab/ui'; +import { GlLink, GlDropdown, GlDropdownItem } from '@gitlab/ui'; import CiIcon from '~/vue_shared/components/ci_icon.vue'; export default { components: { CiIcon, + GlDropdown, + GlDropdownItem, GlLink, }, props: { @@ -78,20 +80,15 @@ export default { </template> </div> - <button - type="button" - data-toggle="dropdown" - class="js-selected-stage dropdown-menu-toggle gl-mt-3" - > - {{ selectedStage }} <i class="fa fa-chevron-down"></i> - </button> - - <ul class="dropdown-menu"> - <li v-for="stage in stages" :key="stage.name"> - <button type="button" class="js-stage-item stage-item" @click="onStageClick(stage)"> - {{ stage.name }} - </button> - </li> - </ul> + <gl-dropdown :text="selectedStage" class="js-selected-stage gl-w-full gl-mt-3"> + <gl-dropdown-item + v-for="stage in stages" + :key="stage.name" + class="js-stage-item stage-item" + @click="onStageClick(stage)" + > + {{ stage.name }} + </gl-dropdown-item> + </gl-dropdown> </div> </template> diff --git a/app/assets/javascripts/jobs/constants.js b/app/assets/javascripts/jobs/constants.js new file mode 100644 index 00000000000..d0d625d794d --- /dev/null +++ b/app/assets/javascripts/jobs/constants.js @@ -0,0 +1,24 @@ +import { __, s__ } from '~/locale'; + +const cancel = __('Cancel'); +const moreInfo = __('More information'); + +export const JOB_SIDEBAR = { + cancel, + debug: __('Debug'), + newIssue: __('New issue'), + retry: __('Retry'), + toggleSidebar: __('Toggle Sidebar'), +}; + +export const JOB_RETRY_FORWARD_DEPLOYMENT_MODAL = { + cancel, + info: s__( + `Jobs|You're about to retry a job that failed because it attempted to deploy code that is older than the latest deployment. + Retrying this job could result in overwriting the environment with the older source code.`, + ), + areYouSure: s__('Jobs|Are you sure you want to proceed?'), + moreInfo, + primaryText: __('Retry job'), + title: s__('Jobs|Are you sure you want to retry this job?'), +}; diff --git a/app/assets/javascripts/jobs/index.js b/app/assets/javascripts/jobs/index.js index 6e15360b66c..1ad6292a030 100644 --- a/app/assets/javascripts/jobs/index.js +++ b/app/assets/javascripts/jobs/index.js @@ -10,27 +10,31 @@ export default () => { // Let's start initializing the store (i.e. fetching data) right away store.dispatch('init', element.dataset); + const { + artifactHelpUrl, + deploymentHelpUrl, + runnerHelpUrl, + runnerSettingsUrl, + variablesSettingsUrl, + subscriptionsMoreMinutesUrl, + endpoint, + pagePath, + logState, + buildStatus, + projectPath, + retryOutdatedJobDocsUrl, + } = element.dataset; + return new Vue({ el: element, store, components: { JobApp, }, + provide: { + retryOutdatedJobDocsUrl, + }, render(createElement) { - const { - artifactHelpUrl, - deploymentHelpUrl, - runnerHelpUrl, - runnerSettingsUrl, - variablesSettingsUrl, - subscriptionsMoreMinutesUrl, - endpoint, - pagePath, - logState, - buildStatus, - projectPath, - } = element.dataset; - return createElement('job-app', { props: { artifactHelpUrl, diff --git a/app/assets/javascripts/jobs/store/getters.js b/app/assets/javascripts/jobs/store/getters.js index bf924bc1917..8c2d1dd8ab2 100644 --- a/app/assets/javascripts/jobs/store/getters.js +++ b/app/assets/javascripts/jobs/store/getters.js @@ -3,6 +3,9 @@ import { isScrolledToBottom } from '~/lib/utils/scroll_utils'; export const headerTime = state => (state.job.started ? state.job.started : state.job.created_at); +export const hasForwardDeploymentFailure = state => + state?.job?.failure_reason === 'forward_deployment_failure'; + export const hasUnmetPrerequisitesFailure = state => state?.job?.failure_reason === 'unmet_prerequisites'; diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 7845f2968db..42a5de68cfa 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -741,6 +741,24 @@ export const roundOffFloat = (number, precision = 0) => { }; /** + * Method to round down values with decimal places + * with provided precision. + * + * Eg; roundDownFloat(3.141592, 3) = 3.141 + * + * Refer to spec/javascripts/lib/utils/common_utils_spec.js for + * more supported examples. + * + * @param {Float} number + * @param {Number} precision + */ +export const roundDownFloat = (number, precision = 0) => { + // eslint-disable-next-line no-restricted-properties + const multiplier = Math.pow(10, precision); + return Math.floor(number * multiplier) / multiplier; +}; + +/** * Represents navigation type constants of the Performance Navigation API. * Detailed explanation see https://developer.mozilla.org/en-US/docs/Web/API/PerformanceNavigation. */ diff --git a/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue b/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue index cfd787b3f52..8308ac7e327 100644 --- a/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue +++ b/app/assets/javascripts/registry/explorer/components/list_page/image_list_row.vue @@ -37,15 +37,6 @@ export default { ROW_SCHEDULED_FOR_DELETION, }, computed: { - encodedItem() { - const params = JSON.stringify({ - name: this.item.path, - tags_path: this.item.tags_path, - id: this.item.id, - cleanup_policy_started_at: this.item.cleanup_policy_started_at, - }); - return window.btoa(params); - }, disabledDelete() { return !this.item.destroy_path || this.item.deleting; }, @@ -82,7 +73,7 @@ export default { <router-link class="gl-text-body gl-font-weight-bold" data-testid="detailsLink" - :to="{ name: 'details', params: { id: encodedItem } }" + :to="{ name: 'details', params: { id: item.id } }" > {{ item.path }} </router-link> diff --git a/app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue b/app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue index 146d1434b18..666d8b042da 100644 --- a/app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue +++ b/app/assets/javascripts/registry/explorer/components/registry_breadcrumb.vue @@ -30,7 +30,7 @@ export default { return { tagName, className, - text: this.$route.meta.nameGenerator(this.$route), + text: this.$route.meta.nameGenerator(this.$store.state), path: { to: this.$route.name }, }; }, @@ -48,7 +48,7 @@ export default { ></li> <li v-if="!isRootRoute"> <router-link ref="rootRouteLink" :to="rootRoute.path"> - {{ rootRoute.meta.nameGenerator(rootRoute) }} + {{ rootRoute.meta.nameGenerator($store.state) }} </router-link> <component :is="divider.tagName" v-safe-html="divider.innerHTML" :class="divider.classList" /> </li> diff --git a/app/assets/javascripts/registry/explorer/pages/details.vue b/app/assets/javascripts/registry/explorer/pages/details.vue index e110c9c9c70..a60ef5c4982 100644 --- a/app/assets/javascripts/registry/explorer/pages/details.vue +++ b/app/assets/javascripts/registry/explorer/pages/details.vue @@ -11,7 +11,6 @@ import TagsList from '../components/details_page/tags_list.vue'; import TagsLoader from '../components/details_page/tags_loader.vue'; import EmptyTagsState from '../components/details_page/empty_tags_state.vue'; -import { decodeAndParse } from '../utils'; import { ALERT_SUCCESS_TAG, ALERT_DANGER_TAG, @@ -43,12 +42,9 @@ export default { }; }, computed: { - ...mapState(['tagsPagination', 'isLoading', 'config', 'tags']), - queryParameters() { - return decodeAndParse(this.$route.params.id); - }, + ...mapState(['tagsPagination', 'isLoading', 'config', 'tags', 'imageDetails']), showPartialCleanupWarning() { - return this.queryParameters.cleanup_policy_started_at && !this.dismissPartialCleanupWarning; + return this.imageDetails?.cleanup_policy_started_at && !this.dismissPartialCleanupWarning; }, tracking() { return { @@ -61,15 +57,20 @@ export default { return this.tagsPagination.page; }, set(page) { - this.requestTagsList({ pagination: { page }, params: this.$route.params.id }); + this.requestTagsList({ page }); }, }, }, mounted() { - this.requestTagsList({ params: this.$route.params.id }); + this.requestImageDetailsAndTagsList(this.$route.params.id); }, methods: { - ...mapActions(['requestTagsList', 'requestDeleteTag', 'requestDeleteTags']), + ...mapActions([ + 'requestTagsList', + 'requestDeleteTag', + 'requestDeleteTags', + 'requestImageDetailsAndTagsList', + ]), deleteTags(toBeDeleted) { this.itemsToBeDeleted = this.tags.filter(tag => toBeDeleted[tag.name]); this.track('click_button'); @@ -78,7 +79,7 @@ export default { handleSingleDelete() { const [itemToDelete] = this.itemsToBeDeleted; this.itemsToBeDeleted = []; - return this.requestDeleteTag({ tag: itemToDelete, params: this.$route.params.id }) + return this.requestDeleteTag({ tag: itemToDelete }) .then(() => { this.deleteAlertType = ALERT_SUCCESS_TAG; }) @@ -92,7 +93,6 @@ export default { return this.requestDeleteTags({ ids: itemsToBeDeleted.map(x => x.name), - params: this.$route.params.id, }) .then(() => { this.deleteAlertType = ALERT_SUCCESS_TAGS; @@ -132,7 +132,7 @@ export default { @dismiss="dismissPartialCleanupWarning = true" /> - <details-header :image-name="queryParameters.name" /> + <details-header :image-name="imageDetails.name" /> <tags-loader v-if="isLoading" /> <template v-else> diff --git a/app/assets/javascripts/registry/explorer/router.js b/app/assets/javascripts/registry/explorer/router.js index f570987023b..dcf1c77329d 100644 --- a/app/assets/javascripts/registry/explorer/router.js +++ b/app/assets/javascripts/registry/explorer/router.js @@ -2,7 +2,6 @@ import Vue from 'vue'; import VueRouter from 'vue-router'; import List from './pages/list.vue'; import Details from './pages/details.vue'; -import { decodeAndParse } from './utils'; import { CONTAINER_REGISTRY_TITLE } from './constants/index'; Vue.use(VueRouter); @@ -26,7 +25,7 @@ export default function createRouter(base) { path: '/:id', component: Details, meta: { - nameGenerator: route => decodeAndParse(route.params.id).name, + nameGenerator: ({ imageDetails }) => imageDetails?.name, }, }, ], diff --git a/app/assets/javascripts/registry/explorer/stores/actions.js b/app/assets/javascripts/registry/explorer/stores/actions.js index b9b0e1704d8..3d5b9a36543 100644 --- a/app/assets/javascripts/registry/explorer/stores/actions.js +++ b/app/assets/javascripts/registry/explorer/stores/actions.js @@ -9,7 +9,7 @@ import { FETCH_TAGS_LIST_ERROR_MESSAGE, FETCH_IMAGE_DETAILS_ERROR_MESSAGE, } from '../constants/index'; -import { decodeAndParse } from '../utils'; +import { pathGenerator } from '../utils'; export const setInitialState = ({ commit }, data) => commit(types.SET_INITIAL_STATE, data); export const setShowGarbageCollectionTip = ({ commit }, data) => @@ -45,13 +45,13 @@ export const requestImagesList = ( }); }; -export const requestTagsList = ({ commit, dispatch }, { pagination = {}, params }) => { +export const requestTagsList = ({ commit, dispatch, state: { imageDetails } }, pagination = {}) => { commit(types.SET_MAIN_LOADING, true); - const { tags_path } = decodeAndParse(params); + const tagsPath = pathGenerator(imageDetails); const { page = DEFAULT_PAGE, perPage = DEFAULT_PAGE_SIZE } = pagination; return axios - .get(tags_path, { params: { page, per_page: perPage } }) + .get(tagsPath, { params: { page, per_page: perPage } }) .then(({ data, headers }) => { dispatch('receiveTagsListSuccess', { data, headers }); }) @@ -76,30 +76,30 @@ export const requestImageDetailsAndTagsList = ({ dispatch, commit }, id) => { }); }; -export const requestDeleteTag = ({ commit, dispatch, state }, { tag, params }) => { +export const requestDeleteTag = ({ commit, dispatch, state }, { tag }) => { commit(types.SET_MAIN_LOADING, true); return axios .delete(tag.destroy_path) .then(() => { dispatch('setShowGarbageCollectionTip', true); - return dispatch('requestTagsList', { pagination: state.tagsPagination, params }); + + return dispatch('requestTagsList', state.tagsPagination); }) .finally(() => { commit(types.SET_MAIN_LOADING, false); }); }; -export const requestDeleteTags = ({ commit, dispatch, state }, { ids, params }) => { +export const requestDeleteTags = ({ commit, dispatch, state }, { ids }) => { commit(types.SET_MAIN_LOADING, true); - const { tags_path } = decodeAndParse(params); - const url = tags_path.replace('?format=json', '/bulk_destroy'); + const tagsPath = pathGenerator(state.imageDetails, '/bulk_destroy'); return axios - .delete(url, { params: { ids } }) + .delete(tagsPath, { params: { ids } }) .then(() => { dispatch('setShowGarbageCollectionTip', true); - return dispatch('requestTagsList', { pagination: state.tagsPagination, params }); + return dispatch('requestTagsList', state.tagsPagination); }) .finally(() => { commit(types.SET_MAIN_LOADING, false); diff --git a/app/assets/javascripts/registry/explorer/utils.js b/app/assets/javascripts/registry/explorer/utils.js index 7c6348151c1..a3478756c9b 100644 --- a/app/assets/javascripts/registry/explorer/utils.js +++ b/app/assets/javascripts/registry/explorer/utils.js @@ -1,9 +1,6 @@ -export const decodeAndParse = param => JSON.parse(window.atob(param)); - -// eslint-disable-next-line @gitlab/require-i18n-strings -export const pathGenerator = (imageDetails, ending = 'tags?format=json') => { +export const pathGenerator = (imageDetails, ending = '?format=json') => { // this method is a temporary workaround, to be removed with graphql implementation // https://gitlab.com/gitlab-org/gitlab/-/issues/276432 const basePath = imageDetails.path.replace(`/${imageDetails.name}`, ''); - return `/${basePath}/registry/repository/${imageDetails.id}/${ending}`; + return `/${basePath}/registry/repository/${imageDetails.id}/tags${ending}`; }; diff --git a/app/assets/javascripts/vue_shared/components/sidebar/assignees_dropdown.vue b/app/assets/javascripts/vue_shared/components/sidebar/multiselect_dropdown.vue index 68c97ed13d7..68c97ed13d7 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/assignees_dropdown.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/multiselect_dropdown.vue diff --git a/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue b/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue index f2e9c4a4fbb..9b6d0a87374 100644 --- a/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue +++ b/app/assets/javascripts/vue_shared/components/stacked_progress_bar.vue @@ -1,7 +1,7 @@ <script> import { GlTooltipDirective } from '@gitlab/ui'; import { __ } from '~/locale'; -import { roundOffFloat } from '~/lib/utils/common_utils'; +import { roundDownFloat } from '~/lib/utils/common_utils'; export default { directives: { @@ -89,7 +89,7 @@ export default { return 0; } - const percent = roundOffFloat((count / this.totalCount) * 100, 1); + const percent = roundDownFloat((count / this.totalCount) * 100, 1); if (percent > 0 && percent < 1) { return '< 1'; } diff --git a/app/assets/stylesheets/framework/diffs.scss b/app/assets/stylesheets/framework/diffs.scss index 62c2e15fcc8..f193785140d 100644 --- a/app/assets/stylesheets/framework/diffs.scss +++ b/app/assets/stylesheets/framework/diffs.scss @@ -449,6 +449,7 @@ } } +.diff-table.code, table.code { width: 100%; font-family: $monospace-font; @@ -459,10 +460,12 @@ table.code { table-layout: fixed; border-radius: 0 0 $border-radius-default $border-radius-default; + .diff-tr:first-of-type.line_expansion > .diff-td, tr:first-of-type.line_expansion > td { border-top: 0; } + .diff-tr:nth-last-of-type(2).line_expansion > .diff-td, tr:nth-last-of-type(2).line_expansion, tr:last-of-type.line_expansion { > td { @@ -470,6 +473,7 @@ table.code { } } + .diff-tr.line_holder .diff-td, tr.line_holder td { line-height: $code-line-height; font-size: $code-font-size; @@ -565,24 +569,95 @@ table.code { } .line_holder:last-of-type { + .diff-td:first-child, td:first-child { border-bottom-left-radius: $border-radius-default; } } &.left-side-selected { + .diff-td.line_content.parallel.right-side, td.line_content.parallel.right-side { user-select: none; } } &.right-side-selected { + .diff-td.line_content.parallel.left-side, td.line_content.parallel.left-side { user-select: none; } } } +// Merge request diff grid layout +.diff-grid { + .diff-grid-row { + display: grid; + grid-template-columns: 1fr 1fr; + } + + .diff-grid-left, + .diff-grid-right { + display: grid; + grid-template-columns: 50px 8px 1fr; + + .diff-td:nth-child(2) { + display: none; + } + } + + .diff-grid-comments { + display: grid; + grid-template-columns: 1fr 1fr; + } + + .diff-grid-drafts { + display: grid; + grid-template-columns: 1fr 1fr; + } + + &.inline { + .diff-grid-comments { + display: grid; + grid-template-columns: 1fr; + } + + .diff-grid-drafts { + display: grid; + grid-template-columns: 1fr; + } + + .diff-grid-row { + grid-template-columns: 1fr; + } + + .diff-grid-left, + .diff-grid-right { + grid-template-columns: 50px 50px 8px 1fr; + + .diff-td:nth-child(2) { + display: block; + } + } + + .diff-grid-left .old:nth-child(2) [data-linenumber], + .diff-grid-right .new:nth-child(2) [data-linenumber] { + display: inline; + } + + .diff-grid-left .old:nth-child(3) [data-linenumber], + .diff-grid-right .new:nth-child(1) [data-linenumber] { + display: none; + } + } +} + +// Merge request diff grid layout overrides +.diff-table.code .diff-tr.line_holder .diff-td.line_content.parallel { + width: unset; +} + .diff-stats { align-items: center; padding: 0 1rem; diff --git a/app/assets/stylesheets/highlight/common.scss b/app/assets/stylesheets/highlight/common.scss index 31075b09b83..d9b9f3694c1 100644 --- a/app/assets/stylesheets/highlight/common.scss +++ b/app/assets/stylesheets/highlight/common.scss @@ -20,6 +20,7 @@ @mixin diff-expansion($background, $border, $link) { background-color: $background; + .diff-td, td { border-top: 1px solid $border; border-bottom: 1px solid $border; @@ -41,3 +42,12 @@ border-left: 3px solid $no-coverage; } } + +@mixin line-number-hover($color) { + background-color: $color; + border-color: darken($color, 5%); + + a { + color: darken($color, 15%); + } +} diff --git a/app/assets/stylesheets/highlight/themes/dark.scss b/app/assets/stylesheets/highlight/themes/dark.scss index 8d965ea4309..d51d5b7137d 100644 --- a/app/assets/stylesheets/highlight/themes/dark.scss +++ b/app/assets/stylesheets/highlight/themes/dark.scss @@ -125,6 +125,9 @@ $dark-il: #de935f; @include dark-diff-match-line; } + .diff-td.diff-line-num.hll:not(.empty-cell), + .diff-td.line-coverage.hll:not(.empty-cell), + .diff-td.line_content.hll:not(.empty-cell), td.diff-line-num.hll:not(.empty-cell), td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { @@ -158,15 +161,17 @@ $dark-il: #de935f; } } + .diff-grid-left:hover, + .diff-grid-right:hover { + .diff-line-num:not(.empty-cell) { + @include line-number-hover($dark-over-bg); + } + } + .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - background-color: $dark-over-bg; - border-color: darken($dark-over-bg, 5%); - - a { - color: darken($dark-over-bg, 15%); - } + @include line-number-hover($dark-over-bg); } } diff --git a/app/assets/stylesheets/highlight/themes/monokai.scss b/app/assets/stylesheets/highlight/themes/monokai.scss index 5ef2b9dcc36..e690f9c7c74 100644 --- a/app/assets/stylesheets/highlight/themes/monokai.scss +++ b/app/assets/stylesheets/highlight/themes/monokai.scss @@ -125,6 +125,9 @@ $monokai-gi: #a6e22e; @include dark-diff-match-line; } + .diff-td.diff-line-num.hll:not(.empty-cell), + .diff-td.line-coverage.hll:not(.empty-cell), + .diff-td.line_content.hll:not(.empty-cell), td.diff-line-num.hll:not(.empty-cell), td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { @@ -158,15 +161,17 @@ $monokai-gi: #a6e22e; } } + .diff-grid-left:hover, + .diff-grid-right:hover { + .diff-line-num:not(.empty-cell) { + @include line-number-hover($monokai-over-bg); + } + } + .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - background-color: $monokai-over-bg; - border-color: darken($monokai-over-bg, 5%); - - a { - color: darken($monokai-over-bg, 15%); - } + @include line-number-hover($monokai-over-bg); } } diff --git a/app/assets/stylesheets/highlight/themes/none.scss b/app/assets/stylesheets/highlight/themes/none.scss index fb548a00526..4fc6e5dba39 100644 --- a/app/assets/stylesheets/highlight/themes/none.scss +++ b/app/assets/stylesheets/highlight/themes/none.scss @@ -59,6 +59,13 @@ } } + .diff-grid-left:hover, + .diff-grid-right:hover { + .diff-line-num:not(.empty-cell) { + @include line-number-hover($none-over-bg); + } + } + .diff-line-num { &.old { a { @@ -74,12 +81,7 @@ &.is-over, &.hll:not(.empty-cell).is-over { - background-color: $none-over-bg; - border-color: darken($none-over-bg, 5%); - - a { - color: darken($none-over-bg, 15%); - } + @include line-number-hover($none-over-bg); } &.hll:not(.empty-cell) { diff --git a/app/assets/stylesheets/highlight/themes/solarized-dark.scss b/app/assets/stylesheets/highlight/themes/solarized-dark.scss index 190a6e6156a..8c532f53182 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-dark.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-dark.scss @@ -129,6 +129,9 @@ $solarized-dark-il: #2aa198; @include dark-diff-match-line; } + .diff-td.diff-line-num.hll:not(.empty-cell), + .diff-td.line-coverage.hll:not(.empty-cell), + .diff-td.line_content.hll:not(.empty-cell), td.diff-line-num.hll:not(.empty-cell), td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { @@ -140,6 +143,13 @@ $solarized-dark-il: #2aa198; @include line-coverage-border-color($solarized-dark-coverage, $solarized-dark-no-coverage); } + .diff-grid-left:hover, + .diff-grid-right:hover { + .diff-line-num:not(.empty-cell) { + @include line-number-hover($solarized-dark-over-bg); + } + } + .diff-line-num.new, .line-coverage.new, .line_content.new { @@ -165,12 +175,7 @@ $solarized-dark-il: #2aa198; .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - background-color: $solarized-dark-over-bg; - border-color: darken($solarized-dark-over-bg, 5%); - - a { - color: darken($solarized-dark-over-bg, 15%); - } + @include line-number-hover($solarized-dark-over-bg); } } diff --git a/app/assets/stylesheets/highlight/themes/solarized-light.scss b/app/assets/stylesheets/highlight/themes/solarized-light.scss index 71d8dd06834..1f9042a9534 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-light.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-light.scss @@ -136,6 +136,9 @@ $solarized-light-il: #2aa198; @include match-line; } + .diff-td.diff-line-num.hll:not(.empty-cell), + .diff-td.line-coverage.hll:not(.empty-cell), + .diff-td.line_content.hll:not(.empty-cell), td.diff-line-num.hll:not(.empty-cell), td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { @@ -159,6 +162,13 @@ $solarized-light-il: #2aa198; } } + .diff-grid-left:hover, + .diff-grid-right:hover { + .diff-line-num:not(.empty-cell) { + @include line-number-hover($solarized-light-over-bg); + } + } + .diff-line-num.old, .line-coverage.old, .line_content.old { @@ -173,12 +183,7 @@ $solarized-light-il: #2aa198; .diff-line-num { &.is-over, &.hll:not(.empty-cell).is-over { - background-color: $solarized-light-over-bg; - border-color: darken($solarized-light-over-bg, 5%); - - a { - color: darken($solarized-light-over-bg, 15%); - } + @include line-number-hover($solarized-light-over-bg); } } diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 3e126a52c4b..bb5ca94af33 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/app/assets/stylesheets/highlight/white_base.scss @@ -113,6 +113,13 @@ pre.code, @include match-line; } + .diff-grid-left:hover, + .diff-grid-right:hover { + .diff-line-num:not(.empty-cell) { + @include line-number-hover($white-over-bg); + } + } + .diff-line-num { &.old { background-color: $line-number-old; @@ -134,12 +141,7 @@ pre.code, &.is-over, &.hll:not(.empty-cell).is-over { - background-color: $white-over-bg; - border-color: darken($white-over-bg, 5%); - - a { - color: darken($white-over-bg, 15%); - } + @include line-number-hover($white-over-bg); } &.hll:not(.empty-cell) { diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index f4a8ab58297..0c24ea9ccc6 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -2,6 +2,7 @@ * Note Form */ .diff-file .diff-content { + .diff-tr.line_holder:hover > .diff-td .line_note_link, tr.line_holder:hover > td .line_note_link { opacity: 1; filter: alpha(opacity = 100); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 55250f9244a..6578b5ed337 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -453,6 +453,8 @@ $note-form-margin-left: 72px; } .diff-file { + .diff-grid-left:hover, + .diff-grid-right:hover, .is-over { .add-diff-note { display: inline-flex; @@ -490,6 +492,7 @@ $note-form-margin-left: 72px; .notes_holder { font-family: $regular-font; + .diff-td, td { border: 1px solid $border-color; border-left: 0; @@ -805,6 +808,8 @@ $note-form-margin-left: 72px; * Line note button on the side of diffs */ +.diff-grid-left:hover, +.diff-grid-right:hover, .line_holder .is-over:not(.no-comment-btn) { .add-diff-note { opacity: 1; diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 63e8a4dc3e8..9b380049583 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -218,8 +218,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController # TODO Remove domain_denylist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) params.delete(:domain_denylist_raw) if params[:domain_denylist_file] - params.delete(:domain_denylist_raw) if params[:domain_blacklist] - params.delete(:domain_allowlist_raw) if params[:domain_whitelist] + params.delete(:domain_denylist_raw) if params[:domain_denylist] + params.delete(:domain_allowlist_raw) if params[:domain_allowlist] params.require(:application_setting).permit( visible_application_setting_attributes diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 1d28c828464..2c7b571baa8 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -37,6 +37,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:hide_jump_to_next_unresolved_in_threads, default_enabled: true) push_frontend_feature_flag(:merge_request_widget_graphql, @project) push_frontend_feature_flag(:unified_diff_lines, @project, default_enabled: true) + push_frontend_feature_flag(:unified_diff_components, @project) push_frontend_feature_flag(:highlight_current_diff_row, @project) push_frontend_feature_flag(:default_merge_ref_for_diffs, @project) push_frontend_feature_flag(:core_security_mr_widget, @project, default_enabled: true) diff --git a/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb b/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb index a3a421f8938..17f9b5b5637 100644 --- a/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb +++ b/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb @@ -33,9 +33,9 @@ module Mutations super end - def resolve(args) + def resolve(queue_name:, **args) { - result: Gitlab::SidekiqQueue.new(args[:queue_name]).drop_jobs!(args, timeout: 30), + result: Gitlab::SidekiqQueue.new(queue_name).drop_jobs!(args, timeout: 30), errors: [] } rescue Gitlab::SidekiqQueue::NoMetadataError @@ -44,7 +44,7 @@ module Mutations errors: ['No metadata provided'] } rescue Gitlab::SidekiqQueue::InvalidQueueError - raise Gitlab::Graphql::Errors::ResourceNotAvailable, "Queue #{args[:queue_name]} not found" + raise Gitlab::Graphql::Errors::ResourceNotAvailable, "Queue #{queue_name} not found" end end end diff --git a/app/graphql/mutations/alert_management/base.rb b/app/graphql/mutations/alert_management/base.rb index 0ccfcf34180..81d5ee95f06 100644 --- a/app/graphql/mutations/alert_management/base.rb +++ b/app/graphql/mutations/alert_management/base.rb @@ -4,7 +4,6 @@ module Mutations module AlertManagement class Base < BaseMutation include Gitlab::Utils::UsageData - include ResolvesProject argument :project_path, GraphQL::ID_TYPE, required: true, @@ -33,13 +32,12 @@ module Mutations private - def find_object(project_path:, iid:) - project = resolve_project(full_path: project_path) + def find_object(project_path:, **args) + project = Project.find_by_full_path(project_path) return unless project - resolver = Resolvers::AlertManagement::AlertResolver.single.new(object: project, context: context, field: nil) - resolver.resolve(iid: iid) + ::AlertManagement::AlertsFinder.new(current_user, project, args).execute.first end end end diff --git a/app/graphql/mutations/alert_management/update_alert_status.rb b/app/graphql/mutations/alert_management/update_alert_status.rb index 1e14bae048a..74185dca529 100644 --- a/app/graphql/mutations/alert_management/update_alert_status.rb +++ b/app/graphql/mutations/alert_management/update_alert_status.rb @@ -9,9 +9,9 @@ module Mutations required: true, description: 'The status to set the alert' - def resolve(args) - alert = authorized_find!(project_path: args[:project_path], iid: args[:iid]) - result = update_status(alert, args[:status]) + def resolve(project_path:, iid:, status:) + alert = authorized_find!(project_path: project_path, iid: iid) + result = update_status(alert, status) track_usage_event(:incident_management_alert_status_changed, current_user.id) diff --git a/app/graphql/mutations/todos/base.rb b/app/graphql/mutations/todos/base.rb index 6db863796bc..4dab3bbc3f4 100644 --- a/app/graphql/mutations/todos/base.rb +++ b/app/graphql/mutations/todos/base.rb @@ -11,16 +11,6 @@ module Mutations id = ::Types::GlobalIDType[::Todo].coerce_isolated_input(id) GitlabSchema.find_by_gid(id) end - - def map_to_global_ids(ids) - return [] if ids.blank? - - ids.map { |id| to_global_id(id) } - end - - def to_global_id(id) - Gitlab::GlobalId.as_global_id(id, model_name: Todo.name).to_s - end end end end diff --git a/app/graphql/mutations/todos/mark_all_done.rb b/app/graphql/mutations/todos/mark_all_done.rb index 8b53658ddd5..97bbbeeaa2f 100644 --- a/app/graphql/mutations/todos/mark_all_done.rb +++ b/app/graphql/mutations/todos/mark_all_done.rb @@ -8,7 +8,7 @@ module Mutations authorize :update_user field :updated_ids, - [GraphQL::ID_TYPE], + [::Types::GlobalIDType[::Todo]], null: false, deprecated: { reason: 'Use todos', milestone: '13.2' }, description: 'Ids of the updated todos' @@ -23,7 +23,7 @@ module Mutations updated_ids = mark_all_todos_done { - updated_ids: map_to_global_ids(updated_ids), + updated_ids: updated_ids, todos: Todo.id_in(updated_ids), errors: [] } diff --git a/app/graphql/mutations/todos/restore_many.rb b/app/graphql/mutations/todos/restore_many.rb index ea5f5414134..9e0a95c48ec 100644 --- a/app/graphql/mutations/todos/restore_many.rb +++ b/app/graphql/mutations/todos/restore_many.rb @@ -12,7 +12,7 @@ module Mutations required: true, description: 'The global ids of the todos to restore (a maximum of 50 is supported at once)' - field :updated_ids, [GraphQL::ID_TYPE], + field :updated_ids, [::Types::GlobalIDType[Todo]], null: false, description: 'The ids of the updated todo items', deprecated: { reason: 'Use todos', milestone: '13.2' } @@ -28,7 +28,7 @@ module Mutations updated_ids = restore(todos) { - updated_ids: gids_of(updated_ids), + updated_ids: updated_ids, todos: Todo.id_in(updated_ids), errors: errors_on_objects(todos) } @@ -36,10 +36,6 @@ module Mutations private - def gids_of(ids) - ids.map { |id| Gitlab::GlobalId.as_global_id(id, model_name: Todo.name).to_s } - end - def model_ids_of(ids) ids.map do |gid| # TODO: remove this line when the compatibility layer is removed diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 3668d09e284..f566f145aed 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -199,11 +199,11 @@ module ApplicationSettingsHelper :default_projects_limit, :default_snippet_visibility, :disabled_oauth_sign_in_sources, - :domain_blacklist, - :domain_blacklist_enabled, + :domain_denylist, + :domain_denylist_enabled, # TODO Remove domain_denylist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) :domain_denylist_raw, - :domain_whitelist, + :domain_allowlist, # TODO Remove domain_allowlist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) :domain_allowlist_raw, :outbound_local_requests_allowlist_raw, diff --git a/app/helpers/ci/jobs_helper.rb b/app/helpers/ci/jobs_helper.rb index e876eb64029..d47f6195c61 100644 --- a/app/helpers/ci/jobs_helper.rb +++ b/app/helpers/ci/jobs_helper.rb @@ -15,7 +15,8 @@ module Ci "build_status" => @build.status, "build_stage" => @build.stage, "log_state" => '', - "build_options" => javascript_build_options + "build_options" => javascript_build_options, + "retry_outdated_job_docs_url" => help_page_path('ci/pipelines/settings', anchor: 'retry-outdated-jobs') } end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index abd58ab62a2..8077a4eb3c9 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -40,8 +40,8 @@ class ApplicationSetting < ApplicationRecord serialize :restricted_visibility_levels # rubocop:disable Cop/ActiveRecordSerialize serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize - serialize :domain_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize - serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize + serialize :domain_allowlist, Array # rubocop:disable Cop/ActiveRecordSerialize + serialize :domain_denylist, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize serialize :asset_proxy_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize @@ -184,9 +184,9 @@ class ApplicationSetting < ApplicationRecord validates :enabled_git_access_protocol, inclusion: { in: %w(ssh http), allow_blank: true } - validates :domain_blacklist, - presence: { message: 'Domain blacklist cannot be empty if Blacklist is enabled.' }, - if: :domain_blacklist_enabled? + validates :domain_denylist, + presence: { message: 'Domain denylist cannot be empty if denylist is enabled.' }, + if: :domain_denylist_enabled? validates :housekeeping_incremental_repack_period, presence: true, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 6eed627b502..63ed65e6725 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -60,7 +60,7 @@ module ApplicationSettingImplementation diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES, disabled_oauth_sign_in_sources: [], dns_rebinding_protection_enabled: true, - domain_whitelist: Settings.gitlab['domain_whitelist'], + domain_allowlist: Settings.gitlab['domain_allowlist'], dsa_key_restriction: 0, ecdsa_key_restriction: 0, ed25519_key_restriction: 0, @@ -203,19 +203,19 @@ module ApplicationSettingImplementation end def domain_allowlist_raw - array_to_string(self.domain_whitelist) + array_to_string(self.domain_allowlist) end def domain_denylist_raw - array_to_string(self.domain_blacklist) + array_to_string(self.domain_denylist) end def domain_allowlist_raw=(values) - self.domain_whitelist = strings_to_array(values) + self.domain_allowlist = strings_to_array(values) end def domain_denylist_raw=(values) - self.domain_blacklist = strings_to_array(values) + self.domain_denylist = strings_to_array(values) end def domain_denylist_file=(file) @@ -242,7 +242,7 @@ module ApplicationSettingImplementation end # This method separates out the strings stored in the - # application_setting.outbound_local_requests_allowlist array into 2 arrays; + # application_setting.outbound_local_requests_whitelist array into 2 arrays; # an array of IPAddr objects (`[IPAddr.new('127.0.0.1')]`), and an array of # domain strings (`['www.example.com']`). def outbound_local_requests_allowlist_arrays diff --git a/app/models/user.rb b/app/models/user.rb index ef77e207215..ef30cce9a6c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1843,15 +1843,15 @@ class User < ApplicationRecord valid = true error = nil - if Gitlab::CurrentSettings.domain_blacklist_enabled? - blocked_domains = Gitlab::CurrentSettings.domain_blacklist + if Gitlab::CurrentSettings.domain_denylist_enabled? + blocked_domains = Gitlab::CurrentSettings.domain_denylist if domain_matches?(blocked_domains, email) error = 'is not from an allowed domain.' valid = false end end - allowed_domains = Gitlab::CurrentSettings.domain_whitelist + allowed_domains = Gitlab::CurrentSettings.domain_allowlist unless allowed_domains.blank? if domain_matches?(allowed_domains, email) valid = true diff --git a/app/views/admin/application_settings/_signup.html.haml b/app/views/admin/application_settings/_signup.html.haml index 19324b1a002..c3deb8af99e 100644 --- a/app/views/admin/application_settings/_signup.html.haml +++ b/app/views/admin/application_settings/_signup.html.haml @@ -31,14 +31,14 @@ .form-text.text-muted = _("See GitLab's %{password_policy_guidelines}").html_safe % { password_policy_guidelines: password_policy_guidelines_link } .form-group - = f.label :domain_whitelist, _('Allowed domains for sign-ups'), class: 'label-bold' + = f.label :domain_allowlist, _('Allowed domains for sign-ups'), class: 'label-bold' = f.text_area :domain_allowlist_raw, placeholder: 'domain.com', class: 'form-control', rows: 8 .form-text.text-muted ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com .form-group - = f.label :domain_blacklist_enabled, _('Domain denylist'), class: 'label-bold' + = f.label :domain_denylist_enabled, _('Domain denylist'), class: 'label-bold' .form-check - = f.check_box :domain_blacklist_enabled, class: 'form-check-input' - = f.label :domain_blacklist_enabled, class: 'form-check-label' do + = f.check_box :domain_denylist_enabled, class: 'form-check-input' + = f.label :domain_denylist_enabled, class: 'form-check-label' do Enable domain denylist for sign ups .form-group .form-check @@ -47,7 +47,7 @@ .option-title Upload denylist file .form-check - = radio_button_tag :denylist_type, :raw, @application_setting.domain_blacklist.present? || @application_setting.domain_blacklist.blank?, class: 'form-check-input' + = radio_button_tag :denylist_type, :raw, @application_setting.domain_denylist.present? || @application_setting.domain_denylist.blank?, class: 'form-check-input' = label_tag :denylist_type_raw, class: 'form-check-label' do .option-title Enter denylist manually @@ -56,7 +56,7 @@ = f.file_field :domain_denylist_file, class: 'form-control', accept: '.txt,.conf' .form-text.text-muted Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines or commas for multiple entries. .form-group.js-denylist-raw - = f.label :domain_blacklist, _('Denied domains for sign-ups'), class: 'label-bold' + = f.label :domain_denylist, _('Denied domains for sign-ups'), class: 'label-bold' = f.text_area :domain_denylist_raw, placeholder: 'domain.com', class: 'form-control', rows: 8 .form-text.text-muted Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com .form-group diff --git a/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb b/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb index 5619fdd7f38..8c3c2e9e103 100644 --- a/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb +++ b/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb @@ -16,15 +16,16 @@ module ContainerExpirationPolicies return unless throttling_enabled? return unless container_repository + log_extra_metadata_on_done(:container_repository_id, container_repository.id) + unless allowed_to_run?(container_repository) container_repository.cleanup_unscheduled! - log_info(container_repository_id: container_repository.id, cleanup_status: :skipped) + log_extra_metadata_on_done(:cleanup_status, :skipped) return end result = ContainerExpirationPolicies::CleanupService.new(container_repository) .execute - log_extra_metadata_on_done(:container_repository_id, result.payload[:container_repository_id]) log_extra_metadata_on_done(:cleanup_status, result.payload[:cleanup_status]) end diff --git a/changelogs/unreleased/207334-design-view-relax-permissions-for-updateimagediffnote-mutation.yml b/changelogs/unreleased/207334-design-view-relax-permissions-for-updateimagediffnote-mutation.yml new file mode 100644 index 00000000000..31a23da9708 --- /dev/null +++ b/changelogs/unreleased/207334-design-view-relax-permissions-for-updateimagediffnote-mutation.yml @@ -0,0 +1,5 @@ +--- +title: Change the mutation and permissions for image note reposition +merge_request: 47161 +author: +type: changed diff --git a/changelogs/unreleased/211339-forward-deployment-warn-users-on-retry_followup.yml b/changelogs/unreleased/211339-forward-deployment-warn-users-on-retry_followup.yml new file mode 100644 index 00000000000..3f72e0b8cb2 --- /dev/null +++ b/changelogs/unreleased/211339-forward-deployment-warn-users-on-retry_followup.yml @@ -0,0 +1,5 @@ +--- +title: Forward deployment, add modal to warn users on Retry action +merge_request: 46416 +author: +type: added diff --git a/changelogs/unreleased/263482-container-scanning-version-3.yml b/changelogs/unreleased/263482-container-scanning-version-3.yml new file mode 100644 index 00000000000..cd3412b3c10 --- /dev/null +++ b/changelogs/unreleased/263482-container-scanning-version-3.yml @@ -0,0 +1,5 @@ +--- +title: Update container_scanning to version 3 to support FIPS +merge_request: 47099 +author: +type: added diff --git a/changelogs/unreleased/Migrate-Bootstrap-dropdown-to-GitLab-UI-GlDropdown-in-app-assets-javascri.yml b/changelogs/unreleased/Migrate-Bootstrap-dropdown-to-GitLab-UI-GlDropdown-in-app-assets-javascri.yml new file mode 100644 index 00000000000..4329e48355a --- /dev/null +++ b/changelogs/unreleased/Migrate-Bootstrap-dropdown-to-GitLab-UI-GlDropdown-in-app-assets-javascri.yml @@ -0,0 +1,5 @@ +--- +title: Migrate-Bootstrap-dropdown-to-GitLab-UI-GlDropdown-in-app/assets/javascripts/jobs/components/stages_dropdown.vue +merge_request: 41452 +author: nuwe1 +type: other diff --git a/changelogs/unreleased/dstull-update-whitelist-blacklist-to-allowlist-denylist-in-signup-restric.yml b/changelogs/unreleased/dstull-update-whitelist-blacklist-to-allowlist-denylist-in-signup-restric.yml new file mode 100644 index 00000000000..a213f1e4585 --- /dev/null +++ b/changelogs/unreleased/dstull-update-whitelist-blacklist-to-allowlist-denylist-in-signup-restric.yml @@ -0,0 +1,5 @@ +--- +title: Use allowlist/denylist in application settings backend +merge_request: 46170 +author: +type: changed diff --git a/changelogs/unreleased/nmezzopera-refactor-details-fetch-2.yml b/changelogs/unreleased/nmezzopera-refactor-details-fetch-2.yml new file mode 100644 index 00000000000..2e1324f15d5 --- /dev/null +++ b/changelogs/unreleased/nmezzopera-refactor-details-fetch-2.yml @@ -0,0 +1,5 @@ +--- +title: Use new image details API in container registry details +merge_request: 47054 +author: +type: changed diff --git a/changelogs/unreleased/ph-225303-fixCreateBranchButtonSpinner.yml b/changelogs/unreleased/ph-225303-fixCreateBranchButtonSpinner.yml new file mode 100644 index 00000000000..5d906cff257 --- /dev/null +++ b/changelogs/unreleased/ph-225303-fixCreateBranchButtonSpinner.yml @@ -0,0 +1,5 @@ +--- +title: Fixed create branch button not hiding when issue is closed +merge_request: 47187 +author: +type: fixed diff --git a/config/feature_flags/development/unified_diff_components.yml b/config/feature_flags/development/unified_diff_components.yml new file mode 100644 index 00000000000..63470c2a28d --- /dev/null +++ b/config/feature_flags/development/unified_diff_components.yml @@ -0,0 +1,7 @@ +--- +name: unified_diff_components +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44974 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/268039 +type: development +group: group::source code +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 4a9c6669bb6..39c5e91cf46 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -198,7 +198,7 @@ Settings.gitlab.default_projects_features['snippets'] = true if Settin Settings.gitlab.default_projects_features['builds'] = true if Settings.gitlab.default_projects_features['builds'].nil? Settings.gitlab.default_projects_features['container_registry'] = true if Settings.gitlab.default_projects_features['container_registry'].nil? Settings.gitlab.default_projects_features['visibility_level'] = Settings.__send__(:verify_constant, Gitlab::VisibilityLevel, Settings.gitlab.default_projects_features['visibility_level'], Gitlab::VisibilityLevel::PRIVATE) -Settings.gitlab['domain_whitelist'] ||= [] +Settings.gitlab['domain_allowlist'] ||= [] Settings.gitlab['import_sources'] ||= Gitlab::ImportSources.values Settings.gitlab['trusted_proxies'] ||= [] Settings.gitlab['content_security_policy'] ||= Gitlab::ContentSecurityPolicy::ConfigLoader.default_settings_hash diff --git a/db/migrate/20201029143650_rename_application_settings_to_allow_deny_names.rb b/db/migrate/20201029143650_rename_application_settings_to_allow_deny_names.rb new file mode 100644 index 00000000000..2d96da91069 --- /dev/null +++ b/db/migrate/20201029143650_rename_application_settings_to_allow_deny_names.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class RenameApplicationSettingsToAllowDenyNames < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :application_settings, :domain_blacklist_enabled, :domain_denylist_enabled + rename_column_concurrently :application_settings, :domain_blacklist, :domain_denylist + rename_column_concurrently :application_settings, :domain_whitelist, :domain_allowlist + end + + def down + undo_rename_column_concurrently :application_settings, :domain_blacklist_enabled, :domain_denylist_enabled + undo_rename_column_concurrently :application_settings, :domain_blacklist, :domain_denylist + undo_rename_column_concurrently :application_settings, :domain_whitelist, :domain_allowlist + end +end diff --git a/db/post_migrate/20201029144157_cleanup_application_settings_to_allow_deny_rename.rb b/db/post_migrate/20201029144157_cleanup_application_settings_to_allow_deny_rename.rb new file mode 100644 index 00000000000..bb6fe4258c5 --- /dev/null +++ b/db/post_migrate/20201029144157_cleanup_application_settings_to_allow_deny_rename.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class CleanupApplicationSettingsToAllowDenyRename < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :application_settings, :domain_blacklist_enabled, :domain_denylist_enabled + cleanup_concurrent_column_rename :application_settings, :domain_blacklist, :domain_denylist + cleanup_concurrent_column_rename :application_settings, :domain_whitelist, :domain_allowlist + end + + def down + undo_cleanup_concurrent_column_rename :application_settings, :domain_blacklist_enabled, :domain_denylist_enabled + undo_cleanup_concurrent_column_rename :application_settings, :domain_blacklist, :domain_denylist + undo_cleanup_concurrent_column_rename :application_settings, :domain_whitelist, :domain_allowlist + end +end diff --git a/db/schema_migrations/20201029143650 b/db/schema_migrations/20201029143650 new file mode 100644 index 00000000000..c6f00890f4f --- /dev/null +++ b/db/schema_migrations/20201029143650 @@ -0,0 +1 @@ +c718bc731f7dc3e1f0104dfdb79a3dc46c46849153ec9b228600eeb5a92465e7
\ No newline at end of file diff --git a/db/schema_migrations/20201029144157 b/db/schema_migrations/20201029144157 new file mode 100644 index 00000000000..7fdac19230f --- /dev/null +++ b/db/schema_migrations/20201029144157 @@ -0,0 +1 @@ +a61310c95a1302871ea18881d45bc0c7357baa8f24daa31b7e2174318dab5707
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d461af965f5..d411fa028cc 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9083,7 +9083,6 @@ CREATE TABLE application_settings ( max_attachment_size integer DEFAULT 10 NOT NULL, default_project_visibility integer DEFAULT 0 NOT NULL, default_snippet_visibility integer DEFAULT 0 NOT NULL, - domain_whitelist text, user_oauth_applications boolean DEFAULT true, after_sign_out_path character varying, session_expire_delay integer DEFAULT 10080 NOT NULL, @@ -9119,8 +9118,6 @@ CREATE TABLE application_settings ( elasticsearch_search boolean DEFAULT false NOT NULL, repository_storages character varying DEFAULT 'default'::character varying, enabled_git_access_protocol character varying, - domain_blacklist_enabled boolean DEFAULT false, - domain_blacklist text, usage_ping_enabled boolean DEFAULT true NOT NULL, sign_in_text_html text, help_page_text_html text, @@ -9341,6 +9338,9 @@ CREATE TABLE application_settings ( secret_detection_token_revocation_url text, encrypted_secret_detection_token_revocation_token text, encrypted_secret_detection_token_revocation_token_iv text, + domain_denylist_enabled boolean DEFAULT false, + domain_denylist text, + domain_allowlist text, new_user_signups_cap integer, CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)), CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)), diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 6cb5159264b..e214ed7f028 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -20841,7 +20841,7 @@ type TodoRestoreManyPayload { """ The ids of the updated todo items. Deprecated in 13.2: Use todos """ - updatedIds: [ID!]! @deprecated(reason: "Use todos. Deprecated in 13.2") + updatedIds: [TodoID!]! @deprecated(reason: "Use todos. Deprecated in 13.2") } """ @@ -20938,7 +20938,7 @@ type TodosMarkAllDonePayload { """ Ids of the updated todos. Deprecated in 13.2: Use todos """ - updatedIds: [ID!]! @deprecated(reason: "Use todos. Deprecated in 13.2") + updatedIds: [TodoID!]! @deprecated(reason: "Use todos. Deprecated in 13.2") } """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 18a43ad1deb..1b5f3b7f1f9 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -60657,7 +60657,7 @@ "name": null, "ofType": { "kind": "SCALAR", - "name": "ID", + "name": "TodoID", "ofType": null } } @@ -60934,7 +60934,7 @@ "name": null, "ofType": { "kind": "SCALAR", - "name": "ID", + "name": "TodoID", "ofType": null } } diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 3c22ab72446..914f7ae2edd 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3077,7 +3077,7 @@ Autogenerated return type of TodoRestoreMany. | `clientMutationId` | String | A unique identifier for the client performing the mutation. | | `errors` | String! => Array | Errors encountered during execution of the mutation. | | `todos` | Todo! => Array | Updated todos | -| `updatedIds` **{warning-solid}** | ID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 | +| `updatedIds` **{warning-solid}** | TodoID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 | ### TodoRestorePayload @@ -3098,7 +3098,7 @@ Autogenerated return type of TodosMarkAllDone. | `clientMutationId` | String | A unique identifier for the client performing the mutation. | | `errors` | String! => Array | Errors encountered during execution of the mutation. | | `todos` | Todo! => Array | Updated todos | -| `updatedIds` **{warning-solid}** | ID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 | +| `updatedIds` **{warning-solid}** | TodoID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 | ### ToggleAwardEmojiPayload diff --git a/doc/api/groups.md b/doc/api/groups.md index 34b1c0f6d82..0a584795d21 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -95,7 +95,7 @@ GET /groups?statistics=true "parent_id": null, "created_at": "2020-01-15T12:36:29.590Z", "statistics": { - "storage_size" : 212, + "storage_size" : 363, "repository_size" : 33, "wiki_size" : 100, "lfs_objects_size" : 123, diff --git a/doc/api/settings.md b/doc/api/settings.md index fdce87aec78..5b04ee9d368 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -43,9 +43,9 @@ Example response: "home_page_url" : null, "default_snippet_visibility" : "private", "outbound_local_requests_whitelist": [], - "domain_whitelist" : [], - "domain_blacklist_enabled" : false, - "domain_blacklist" : [], + "domain_allowlist" : [], + "domain_denylist_enabled" : false, + "domain_denylist" : [], "created_at" : "2016-01-04T15:44:55.176Z", "default_ci_config_path" : null, "default_project_visibility" : "private", @@ -134,9 +134,9 @@ Example response: "default_snippet_visibility": "private", "default_group_visibility": "private", "outbound_local_requests_whitelist": [], - "domain_whitelist": [], - "domain_blacklist_enabled" : false, - "domain_blacklist" : [], + "domain_allowlist": [], + "domain_denylist_enabled" : false, + "domain_denylist" : [], "external_authorization_service_enabled": true, "external_authorization_service_url": "https://authorize.me", "external_authorization_service_default_label": "default", @@ -233,9 +233,9 @@ listed in the descriptions of the relevant settings. | `diff_max_patch_bytes` | integer | no | Maximum diff patch size (Bytes). | | `disabled_oauth_sign_in_sources` | array of strings | no | Disabled OAuth sign-in sources. | | `dns_rebinding_protection_enabled` | boolean | no | Enforce DNS rebinding attack protection. | -| `domain_blacklist_enabled` | boolean | no | (**If enabled, requires:** `domain_blacklist`) Allows blocking sign-ups from emails from specific domains. | -| `domain_blacklist` | array of strings | no | Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. | -| `domain_whitelist` | array of strings | no | Force people to use only corporate emails for sign-up. Default is `null`, meaning there is no restriction. | +| `domain_denylist_enabled` | boolean | no | (**If enabled, requires:** `domain_denylist`) Allows blocking sign-ups from emails from specific domains. | +| `domain_denylist` | array of strings | no | Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: `domain.com`, `*.domain.com`. | +| `domain_allowlist` | array of strings | no | Force people to use only corporate emails for sign-up. Default is `null`, meaning there is no restriction. | | `dsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys. | | `ecdsa_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `0` (no restriction). `-1` disables ECDSA keys. | | `ed25519_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ED25519 key. Default is `0` (no restriction). `-1` disables ED25519 keys. | diff --git a/doc/ci/pipelines/settings.md b/doc/ci/pipelines/settings.md index 26e2a09fdef..92a5e9ed03d 100644 --- a/doc/ci/pipelines/settings.md +++ b/doc/ci/pipelines/settings.md @@ -231,6 +231,16 @@ When enabled, any older deployments job are skipped when a new deployment starts For more information, see [Deployment safety](../environments/deployment_safety.md). +## Retry outdated jobs + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211339) in GitLab 13.6. + +A deployment job can fail because a newer one has run. If you retry the failed deployment job, the +environment could be overwritten with older source code. If you click **Retry**, a modal warns you +about this and asks for confirmation. + +For more information, see [Deployment safety](../environments/deployment_safety.md). + ## Pipeline Badges In the pipelines settings page you can find pipeline status and test coverage diff --git a/doc/development/database_review.md b/doc/development/database_review.md index 3da961ce35c..f5c03b9e8e6 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -201,8 +201,8 @@ estimated to keep migration timing to a minimum. NOTE: **Note:** Keep in mind that all runtimes should be measured against GitLab.com. -| Migration Type | Execution Time Recommended | Notes | +| Migration Type | Execution Time Recommended | Notes | |----|----|---| | Regular migrations on `db/migrate` | `3 minutes` | A valid exception are index creation as this can take a long time. | -| Post migrations on `db/post_migrate` | `10 minutes` | | -| Background migrations | --- | Since these are suitable for larger tables, it's not possible to set a precise timing guideline, however, any single query must stay below `1 second` execution time with cold caches. | +| Post migrations on `db/post_migrate` | `10 minutes` | | +| Background migrations | --- | Since these are suitable for larger tables, it's not possible to set a precise timing guideline, however, any single query must stay below `1 second` execution time with cold caches. | diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index e843126d50b..e6cea6bad1c 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -1210,10 +1210,10 @@ When you take screenshots: or concept in the image. If the image is of the GitLab interface, append the GitLab version to the file name, based on the following format: `image_name_vX_Y.png`. For example, for a screenshot taken from the pipelines - page of GitLab 11.1, a valid name is `pipelines_v11_1.png`. If you're adding an + page of GitLab 11.1, a valid name is `pipelines_v11_1.png`. If you're adding an illustration that doesn't include parts of the user interface, add the release number corresponding to the release the image was added to; for an MR added to - 11.1's milestone, a valid name for an illustration is `devops_diagram_v11_1.png`. + 11.1's milestone, a valid name for an illustration is `devops_diagram_v11_1.png`. - Place images in a separate directory named `img/` in the same directory where the `.md` document that you're working on is located. - Consider using PNG images instead of JPEG. diff --git a/doc/development/git_object_deduplication.md b/doc/development/git_object_deduplication.md index a9e9c19fbf2..4f1afed24ba 100644 --- a/doc/development/git_object_deduplication.md +++ b/doc/development/git_object_deduplication.md @@ -162,7 +162,7 @@ repository and a pool. ### Pool existence -If GitLab thinks a pool repository exists (i.e. it exists according to +If GitLab thinks a pool repository exists (i.e. it exists according to SQL), but it does not on the Gitaly server, then it will be created on the fly by Gitaly. diff --git a/doc/development/i18n/translation.md b/doc/development/i18n/translation.md index a98d6d758f6..5d8af8a096e 100644 --- a/doc/development/i18n/translation.md +++ b/doc/development/i18n/translation.md @@ -100,7 +100,7 @@ To propose additions to the glossary please ### Inclusive language in French <!-- vale gitlab.Spelling = NO --> -In French, the "écriture inclusive" is now over (see on [Legifrance](https://www.legifrance.gouv.fr/affichTexte.do?cidTexte=JORFTEXT000036068906&categorieLien=id)). +In French, the "écriture inclusive" is now over (see on [Legifrance](https://www.legifrance.gouv.fr/affichTexte.do?cidTexte=JORFTEXT000036068906&categorieLien=id)). So, to include both genders, write “Utilisateurs et utilisatrices” instead of “Utilisateur·rice·s”. When space is missing, the male gender should be used alone. <!-- vale gitlab.Spelling = YES --> diff --git a/doc/development/import_export.md b/doc/development/import_export.md index 59228126d7b..77c71f0f991 100644 --- a/doc/development/import_export.md +++ b/doc/development/import_export.md @@ -360,28 +360,28 @@ The NDJSON tree will look like this: ```shell tree ├── project -│ ├── auto_devops.ndjson -│ ├── boards.ndjson -│ ├── ci_cd_settings.ndjson -│ ├── ci_pipelines.ndjson -│ ├── container_expiration_policy.ndjson -│ ├── custom_attributes.ndjson -│ ├── error_tracking_setting.ndjson -│ ├── external_pull_requests.ndjson -│ ├── issues.ndjson -│ ├── labels.ndjson -│ ├── merge_requests.ndjson -│ ├── milestones.ndjson -│ ├── pipeline_schedules.ndjson -│ ├── project_badges.ndjson -│ ├── project_feature.ndjson -│ ├── project_members.ndjson -│ ├── protected_branches.ndjson -│ ├── protected_tags.ndjson -│ ├── releases.ndjson -│ ├── services.ndjson -│ ├── snippets.ndjson -│ └── triggers.ndjson +│ ├── auto_devops.ndjson +│ ├── boards.ndjson +│ ├── ci_cd_settings.ndjson +│ ├── ci_pipelines.ndjson +│ ├── container_expiration_policy.ndjson +│ ├── custom_attributes.ndjson +│ ├── error_tracking_setting.ndjson +│ ├── external_pull_requests.ndjson +│ ├── issues.ndjson +│ ├── labels.ndjson +│ ├── merge_requests.ndjson +│ ├── milestones.ndjson +│ ├── pipeline_schedules.ndjson +│ ├── project_badges.ndjson +│ ├── project_feature.ndjson +│ ├── project_members.ndjson +│ ├── protected_branches.ndjson +│ ├── protected_tags.ndjson +│ ├── releases.ndjson +│ ├── services.ndjson +│ ├── snippets.ndjson +│ └── triggers.ndjson └── project.json ``` @@ -395,19 +395,19 @@ The NDJSON tree will look like this: tree └── groups ├── 4351 - │ ├── badges.ndjson - │ ├── boards.ndjson - │ ├── epics.ndjson - │ ├── labels.ndjson - │ ├── members.ndjson - │ └── milestones.ndjson + │ ├── badges.ndjson + │ ├── boards.ndjson + │ ├── epics.ndjson + │ ├── labels.ndjson + │ ├── members.ndjson + │ └── milestones.ndjson ├── 4352 - │ ├── badges.ndjson - │ ├── boards.ndjson - │ ├── epics.ndjson - │ ├── labels.ndjson - │ ├── members.ndjson - │ └── milestones.ndjson + │ ├── badges.ndjson + │ ├── boards.ndjson + │ ├── epics.ndjson + │ ├── labels.ndjson + │ ├── members.ndjson + │ └── milestones.ndjson ├── _all.ndjson ├── 4351.json └── 4352.json diff --git a/doc/development/integrations/secure.md b/doc/development/integrations/secure.md index d437e180dfd..94f1ccdb1e6 100644 --- a/doc/development/integrations/secure.md +++ b/doc/development/integrations/secure.md @@ -256,7 +256,7 @@ to `info`. When executing command lines, scanners should use the `debug` level to log the command line and its output. For instance, the [bundler-audit](https://gitlab.com/gitlab-org/security-products/analyzers/bundler-audit) scanner -uses the `debug` level to log the command line `bundle audit check --quiet`, +uses the `debug` level to log the command line `bundle audit check --quiet`, and what `bundle audit` writes to the standard output. #### common logutil package @@ -298,7 +298,7 @@ The `vulnerabilities` field of the report is an array of vulnerability objects. #### ID -The `id` field is the unique identifier of the vulnerability. +The `id` field is the unique identifier of the vulnerability. It is used to reference a fixed vulnerability from a [remediation objects](#remediations). We recommend that you generate a UUID and use it as the `id` field's value. diff --git a/doc/development/namespaces_storage_statistics.md b/doc/development/namespaces_storage_statistics.md index 563b397ac2d..b4a7c8c3132 100644 --- a/doc/development/namespaces_storage_statistics.md +++ b/doc/development/namespaces_storage_statistics.md @@ -6,18 +6,18 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Database case study: Namespaces storage statistics -## Introduction +## Introduction On [Storage and limits management for groups](https://gitlab.com/groups/gitlab-org/-/epics/886), we want to facilitate a method for easily viewing the amount of storage consumed by a group, and allow easy management. -## Proposal +## Proposal 1. Create a new ActiveRecord model to hold the namespaces' statistics in an aggregated form (only for root namespaces). 1. Refresh the statistics in this model every time a project belonging to this namespace is changed. -## Problem +## Problem In GitLab, we update the project storage statistics through a [callback](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/app/models/project.rb#L97) @@ -42,7 +42,7 @@ alternative method. ## Attempts -### Attempt A: PostgreSQL materialized view +### Attempt A: PostgreSQL materialized view Model can be updated through a refresh strategy based on a project routes SQL and a [materialized view](https://www.postgresql.org/docs/11/rules-materializedviews.html): @@ -71,7 +71,7 @@ While this implied a single query update (and probably a fast one), it has some - Materialized views syntax varies from PostgreSQL and MySQL. While this feature was worked on, MySQL was still supported by GitLab. - Rails does not have native support for materialized views. We'd need to use a specialized gem to take care of the management of the database views, which implies additional work. -### Attempt B: An update through a CTE +### Attempt B: An update through a CTE Similar to Attempt A: Model update done through a refresh strategy with a [Common Table Expression](https://www.postgresql.org/docs/9.1/queries-with.html) @@ -140,7 +140,7 @@ Even though this approach would make aggregating much easier, it has some major - We'd have to migrate **all namespaces** by adding and filling a new column. Because of the size of the table, dealing with time/cost will not be great. The background migration will take approximately `153h`, see <https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/29772>. - Background migration has to be shipped one release before, delaying the functionality by another milestone. -### Attempt E (final): Update the namespace storage statistics in async way +### Attempt E (final): Update the namespace storage statistics in async way This approach consists of keep using the incremental statistics updates we currently already have, but we refresh them through Sidekiq jobs and in different transactions: @@ -170,7 +170,7 @@ The only downside of this approach is that namespaces' statistics are updated up which means there's a time window in which the statistics are inaccurate. Because we're still not [enforcing storage limits](https://gitlab.com/gitlab-org/gitlab/-/issues/17664), this is not a major problem. -## Conclusion +## Conclusion Updating the storage statistics asynchronously, was the less problematic and performant approach of aggregating the root namespaces. diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index 484437f1907..9b3c4e8a684 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -94,6 +94,8 @@ renaming. For example class RenameUsersUpdatedAtToUpdatedAtTimestamp < ActiveRecord::Migration[4.2] include Gitlab::Database::MigrationHelpers + DOWNTIME = false + disable_ddl_transaction! def up diff --git a/doc/policy/maintenance.md b/doc/policy/maintenance.md index 2cfc2923616..9edec660d5f 100644 --- a/doc/policy/maintenance.md +++ b/doc/policy/maintenance.md @@ -74,7 +74,7 @@ A step-by-step guide to [upgrading the Omnibus-bundled PostgreSQL is documented ## Upgrading major versions -Backward-incompatible changes and migrations are reserved for major versions. See the [upgrade guide](../update/README.md#upgrading-to-a-new-major-version). +Backward-incompatible changes and migrations are reserved for major versions. See the [upgrade guide](../update/README.md#upgrading-to-a-new-major-version). ## Patch releases diff --git a/doc/update/README.md b/doc/update/README.md index d09c5f45446..774d468cb76 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -164,7 +164,7 @@ upgrade paths. ## Upgrading to a new major version Upgrading the *major* version requires more attention. -Backward-incompatible changes and migrations are reserved for major versions. +Backward-incompatible changes and migrations are reserved for major versions. We cannot guarantee that upgrading between major versions will be seamless. It is suggested to upgrade to the latest available *minor* version within your major version before proceeding to the next major version. diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index 6d9e017a599..62a0ade9f54 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -53,7 +53,7 @@ is **not** `19.03.0`. See [troubleshooting information](#error-response-from-dae ## Supported languages and package managers GitLab relies on [`rules`](../../../ci/yaml/README.md#rules) to start relevant analyzers depending on the languages detected in the repository. -The current detection logic limits the maximum search depth to two levels. For example, the `gemnasium-dependency_scanning` job is enabled if a repository contains either a `Gemfile` or `api/Gemfile` file, but not if the only supported dependency file is `api/client/Gemfile`. +The current detection logic limits the maximum search depth to two levels. For example, the `gemnasium-dependency_scanning` job is enabled if a repository contains either a `Gemfile` or `api/Gemfile` file, but not if the only supported dependency file is `api/client/Gemfile`. The following languages and dependency managers are supported: diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 2cf4c7a7484..bbea5b802ed 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -110,7 +110,7 @@ The scanning tools and vulnerabilities database are updated regularly. | Secure scanning tool | Vulnerabilities database updates | |:-------------------------------------------------------------|-------------------------------------------| | [Container Scanning](container_scanning/index.md) | Uses `clair`. The latest `clair-db` version is used for each job by running the [`latest` Docker image tag](https://gitlab.com/gitlab-org/gitlab/blob/438a0a56dc0882f22bdd82e700554525f552d91b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml#L37). The `clair-db` database [is updated daily according to the author](https://github.com/arminc/clair-local-scan#clair-server-or-local). | -| [Dependency Scanning](dependency_scanning/index.md) | Relies on `bundler-audit` (for Ruby gems), `retire.js` (for NPM packages), and `gemnasium` (GitLab's own tool for all libraries). Both `bundler-audit` and `retire.js` fetch their vulnerabilities data from GitHub repositories, so vulnerabilities added to `ruby-advisory-db` and `retire.js` are immediately available. The tools themselves are updated once per month if there's a new version. The [Gemnasium DB](https://gitlab.com/gitlab-org/security-products/gemnasium-db) is updated at least once a week. See our [current measurement of time from CVE being issued to our product being updated](https://about.gitlab.com/handbook/engineering/development/performance-indicators/#cve-issue-to-update). | +| [Dependency Scanning](dependency_scanning/index.md) | Relies on `bundler-audit` (for Ruby gems), `retire.js` (for NPM packages), and `gemnasium` (GitLab's own tool for all libraries). Both `bundler-audit` and `retire.js` fetch their vulnerabilities data from GitHub repositories, so vulnerabilities added to `ruby-advisory-db` and `retire.js` are immediately available. The tools themselves are updated once per month if there's a new version. The [Gemnasium DB](https://gitlab.com/gitlab-org/security-products/gemnasium-db) is updated at least once a week. See our [current measurement of time from CVE being issued to our product being updated](https://about.gitlab.com/handbook/engineering/development/performance-indicators/#cve-issue-to-update). | | [Dynamic Application Security Testing (DAST)](dast/index.md) | The scanning engine is updated on a periodic basis. See the [version of the underlying tool `zaproxy`](https://gitlab.com/gitlab-org/security-products/dast/blob/master/Dockerfile#L1). The scanning rules are downloaded at scan runtime. | | [Static Application Security Testing (SAST)](sast/index.md) | Relies exclusively on [the tools GitLab wraps](sast/index.md#supported-languages-and-frameworks). The underlying analyzers are updated at least once per month if a relevant update is available. The vulnerabilities database is updated by the upstream tools. | diff --git a/doc/user/application_security/security_dashboard/img/project_security_dashboard_chart_v13_6.png b/doc/user/application_security/security_dashboard/img/project_security_dashboard_chart_v13_6.png Binary files differnew file mode 100644 index 00000000000..6ccae80e80e --- /dev/null +++ b/doc/user/application_security/security_dashboard/img/project_security_dashboard_chart_v13_6.png diff --git a/doc/user/application_security/security_dashboard/index.md b/doc/user/application_security/security_dashboard/index.md index d124b595617..3c6e3c170a3 100644 --- a/doc/user/application_security/security_dashboard/index.md +++ b/doc/user/application_security/security_dashboard/index.md @@ -65,11 +65,24 @@ the analyzer outputs an ## Project Security Dashboard +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/235558) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 13.6. + +At the project level, the Security Dashboard displays a chart with the number of vulnerabilities over time. +Access it by navigating to **Security & Compliance > Security Dashboard**. Currently, we display historical +data up to 365 days. + + + +Filter the historical data by clicking on the corresponding legend name. The image above, for example, shows +only the graph for vulnerabilities with **high** severity. + +### Vulnerability Report + > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/6165) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.1. -At the project level, the Security Dashboard displays the vulnerabilities that exist in your project's -[default branch](../../project/repository/branches/index.md#default-branch). Access it by navigating -to **Security & Compliance > Security Dashboard**. By default, the Security Dashboard is filtered to +The vulnerabilities that exist in your project's +[default branch](../../project/repository/branches/index.md#default-branch) are accessed by navigating to +**Security & Compliance > Vulnerability Report**. By default, the Security Dashboard is filtered to display all detected and confirmed vulnerabilities. The Security Dashboard first displays the time at which the last pipeline completed on the project's diff --git a/doc/user/project/clusters/serverless/aws.md b/doc/user/project/clusters/serverless/aws.md index db91f78fc20..0de0fd38336 100644 --- a/doc/user/project/clusters/serverless/aws.md +++ b/doc/user/project/clusters/serverless/aws.md @@ -335,7 +335,7 @@ Some steps in this documentation use SAM CLI. Follow the instructions for [installing SAM CLI](https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/serverless-sam-cli-install.html) to install and configure SAM CLI. -If you use [AWS Cloud9](https://aws.amazon.com/cloud9/) as your integrated development +If you use [AWS Cloud9](https://aws.amazon.com/cloud9/) as your integrated development environment (IDE), the following are installed for you: - [AWS Command Line Interface](https://docs.aws.amazon.com/en_pv/cli/latest/userguide/cli-chap-install.html) @@ -357,7 +357,7 @@ To create a new AWS SAM application: 1. `git push` the application back to the GitLab project. This creates a SAM app named `gitlabpoc` using the default configuration, a single -Python 3.8 function invoked by an [Amazon API Gateway](https://aws.amazon.com/api-gateway/) +Python 3.8 function invoked by an [Amazon API Gateway](https://aws.amazon.com/api-gateway/) endpoint. To see additional runtimes supported by SAM and options for `sam init`, run: ```shell @@ -367,13 +367,13 @@ sam init -h ### Setting up your AWS credentials with your GitLab account In order to interact with your AWS account, the GitLab CI/CD pipelines require both -`AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` to be set in the project's CI/CD +`AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` to be set in the project's CI/CD variables. To set these: -1. Navigate to the project's **Settings > CI / CD**. -1. Expand the **Variables** section and create entries for `AWS_ACCESS_KEY_ID` and +1. Navigate to the project's **Settings > CI / CD**. +1. Expand the **Variables** section and create entries for `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY`. 1. Mask the credentials so they do not show in logs using the **Masked** toggle. @@ -460,7 +460,7 @@ CLI installed locally for you to test locally. First, test the function. -SAM provides a default event in `events/event.json` that includes a message body of: +SAM provides a default event in `events/event.json` that includes a message body of: ```plaintext {\"message\": \"hello world\"} @@ -491,7 +491,7 @@ sam local start-api ``` SAM again launches a Docker container, this time with a mocked Amazon API Gateway -listening on `localhost:3000`. +listening on `localhost:3000`. Call the `hello` API by running: diff --git a/doc/user/project/merge_requests/code_quality.md b/doc/user/project/merge_requests/code_quality.md index f3a676e2b48..d50056c9450 100644 --- a/doc/user/project/merge_requests/code_quality.md +++ b/doc/user/project/merge_requests/code_quality.md @@ -348,7 +348,7 @@ This can be due to multiple reasons: nothing will be displayed. - The [`artifacts:expire_in`](../../../ci/yaml/README.md#artifactsexpire_in) CI/CD setting can cause the Code Quality artifact(s) to expire faster than desired. -- Large `codeclimate.json` files (esp. >10 MB) are [known to prevent the report from being displayed](https://gitlab.com/gitlab-org/gitlab/-/issues/2737). +- Large `codeclimate.json` files (esp. >10 MB) are [known to prevent the report from being displayed](https://gitlab.com/gitlab-org/gitlab/-/issues/2737). As a work-around, try removing [properties](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#data-types) that are [ignored by GitLab](#implementing-a-custom-tool). You can: - Configure the Code Quality tool to not output those types. diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 16ac4ec4ef2..b95856d99d1 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -53,9 +53,9 @@ module API optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' optional :default_snippet_visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The default snippet visibility' optional :disabled_oauth_sign_in_sources, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Disable certain OAuth sign-in sources' - optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups' - optional :domain_blacklist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' - optional :domain_whitelist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + optional :domain_denylist_enabled, type: Boolean, desc: 'Enable domain denylist for sign ups' + optional :domain_denylist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + optional :domain_allowlist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' optional :eks_integration_enabled, type: Boolean, desc: 'Enable integration with Amazon EKS' given eks_integration_enabled: -> (val) { val } do requires :eks_account_id, type: String, desc: 'Amazon account ID for EKS integration' diff --git a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml index 21bcdd8d9b5..3cbde9d30c8 100644 --- a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml @@ -4,8 +4,7 @@ variables: # Setting this variable will affect all Security templates # (SAST, Dependency Scanning, ...) SECURE_ANALYZERS_PREFIX: "registry.gitlab.com/gitlab-org/security-products/analyzers" - - CS_MAJOR_VERSION: 2 + CS_MAJOR_VERSION: 3 container_scanning: stage: test diff --git a/lib/gitlab/dependency_linker/base_linker.rb b/lib/gitlab/dependency_linker/base_linker.rb index a4e265eba88..d735fb55652 100644 --- a/lib/gitlab/dependency_linker/base_linker.rb +++ b/lib/gitlab/dependency_linker/base_linker.rb @@ -6,6 +6,7 @@ module Gitlab URL_REGEX = %r{https?://[^'" ]+}.freeze GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze + VALID_LINK_ATTRIBUTES = %w[href rel target].freeze include ActionView::Helpers::SanitizeHelper @@ -66,7 +67,7 @@ module Gitlab def link_tag(name, url) sanitize( %{<a href="#{ERB::Util.html_escape_once(url)}" rel="nofollow noreferrer noopener" target="_blank">#{ERB::Util.html_escape_once(name)}</a>}, - attributes: %w[href rel target] + attributes: VALID_LINK_ATTRIBUTES ) end @@ -77,7 +78,7 @@ module Gitlab # # Will link `user/repo` in `github: "user/repo"` or `:github => "user/repo"` def link_regex(regex, &url_proc) highlighted_lines.map!.with_index do |rich_line, i| - marker = StringRegexMarker.new(plain_lines[i].chomp, rich_line.html_safe) + marker = StringRegexMarker.new((plain_lines[i].chomp! || plain_lines[i]), rich_line.html_safe) marker.mark(regex, group: :name) do |text, left:, right:| url = yield(text) diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index a1c11b38f1a..eece2c343d2 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -113,7 +113,7 @@ module Gitlab end rescue SocketError # If the dns rebinding protection is not enabled or the domain - # is whitelisted we avoid the dns rebinding checks + # is allowed we avoid the dns rebinding checks return if domain_allowed?(uri) || !dns_rebind_protection # In the test suite we use a lot of mocked urls that are either invalid or diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f88260804d4..5e25af512dc 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2668,6 +2668,9 @@ msgstr "" msgid "AlertsIntegrations|Integration Name" msgstr "" +msgid "AlertsIntegrations|Integration payload is invalid. You can still save your changes." +msgstr "" + msgid "AlertsIntegrations|No integrations have been added yet" msgstr "" @@ -15334,6 +15337,15 @@ msgstr "" msgid "Jobs" msgstr "" +msgid "Jobs|Are you sure you want to proceed?" +msgstr "" + +msgid "Jobs|Are you sure you want to retry this job?" +msgstr "" + +msgid "Jobs|You're about to retry a job that failed because it attempted to deploy code that is older than the latest deployment. Retrying this job could result in overwriting the environment with the older source code." +msgstr "" + msgid "Job|Browse" msgstr "" @@ -23088,6 +23100,9 @@ msgstr "" msgid "Retry" msgstr "" +msgid "Retry job" +msgstr "" + msgid "Retry this job" msgstr "" diff --git a/package.json b/package.json index 644b326146d..ffd539e445b 100644 --- a/package.json +++ b/package.json @@ -91,8 +91,8 @@ "font-awesome": "4.7.0", "fuzzaldrin-plus": "^0.6.0", "glob": "^7.1.6", - "graphql": "^14.7.0", - "graphql-tag": "^2.10.1", + "graphql": "^15.4.0", + "graphql-tag": "^2.11.0", "immer": "^7.0.7", "imports-loader": "^0.8.0", "ipaddr.js": "^1.9.1", diff --git a/qa/qa/specs/features/browser_ui/5_package/composer_registry_spec.rb b/qa/qa/specs/features/browser_ui/5_package/composer_registry_spec.rb new file mode 100644 index 00000000000..7783dba3fa7 --- /dev/null +++ b/qa/qa/specs/features/browser_ui/5_package/composer_registry_spec.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +module QA + RSpec.describe 'Package', :orchestrated, :packages do + describe 'Composer Repository' do + include Runtime::Fixtures + + let(:package_name) { 'my_package' } + + let(:project) do + Resource::Project.fabricate_via_api! do |project| + project.name = 'composer-package-project' + end + end + + let!(:runner) do + Resource::Runner.fabricate! do |runner| + runner.name = "qa-runner-#{Time.now.to_i}" + runner.tags = ["runner-for-#{project.name}"] + runner.executor = :docker + runner.project = project + end + end + + let!(:gitlab_address_with_port) do + uri = URI.parse(Runtime::Scenario.gitlab_address) + "#{uri.scheme}://#{uri.host}:#{uri.port}" + end + + let(:composer_json_file) do + <<~EOF + { + "name": "#{project.path_with_namespace}/#{package_name}", + "description": "Library XY", + "type": "library", + "license": "GPL-3.0-only", + "authors": [ + { + "name": "John Doe", + "email": "john@example.com" + } + ], + "require": {} + } + EOF + end + + let(:gitlab_ci_yaml) do + <<~YAML + publish: + image: curlimages/curl:latest + stage: build + variables: + URL: "$CI_SERVER_PROTOCOL://$CI_SERVER_HOST:$CI_SERVER_PORT/api/v4/projects/$CI_PROJECT_ID/packages/composer?job_token=$CI_JOB_TOKEN" + script: + - version=$([[ -z "$CI_COMMIT_TAG" ]] && echo "branch=$CI_COMMIT_REF_NAME" || echo "tag=$CI_COMMIT_TAG") + - insecure=$([ "$CI_SERVER_PROTOCOL" = "http" ] && echo "--insecure" || echo "") + - response=$(curl -s -w "%{http_code}" $insecure --data $version $URL) + - code=$(echo "$response" | tail -n 1) + - body=$(echo "$response" | head -n 1) + tags: + - "runner-for-#{project.name}" + YAML + end + + before do + Flow::Login.sign_in + + Resource::Repository::Commit.fabricate_via_api! do |commit| + commit.project = project + commit.commit_message = 'Add .gitlab-ci.yml' + commit.add_files([{ + file_path: '.gitlab-ci.yml', + content: gitlab_ci_yaml + }, + { + file_path: 'composer.json', + content: composer_json_file + }] + ) + end + + project.visit! + Page::Project::Menu.perform(&:click_ci_cd_pipelines) + Page::Project::Pipeline::Index.perform(&:click_on_latest_pipeline) + + Page::Project::Pipeline::Show.perform do |pipeline| + pipeline.click_job('publish') + end + + Page::Project::Job::Show.perform do |job| + expect(job).to be_successful(timeout: 800) + end + end + + after do + runner.remove_via_api! + end + + it 'publishes a composer package and deletes it', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1088' do + Page::Project::Menu.perform(&:click_packages_link) + + Page::Project::Packages::Index.perform do |index| + expect(index).to have_package(package_name) + index.click_package(package_name) + end + + Page::Project::Packages::Show.perform do |package| + package.click_delete + end + + Page::Project::Packages::Index.perform do |index| + aggregate_failures 'package deletion' do + expect(index).to have_content("Package deleted successfully") + expect(index).to have_no_package(package_name) + end + end + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index eadabd820e9..09a356b7e5f 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -504,6 +504,11 @@ FactoryBot.define do failure_reason { 10 } end + trait :forward_deployment_failure do + failed + failure_reason { 13 } + end + trait :with_runner_session do after(:build) do |build| build.build_runner_session(url: 'https://localhost') diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 404c3e93586..e19337e1ff5 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -1013,7 +1013,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do before do job.run! visit project_job_path(project, job) - find('.js-cancel-job').click + find('[data-testid="cancel-button"]').click end it 'loads the page and shows all needed controls' do @@ -1030,7 +1030,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do visit project_job_path(project, job) wait_for_requests - find('.js-retry-button').click + find('[data-testid="retry-button"]').click end it 'shows the right status and buttons' do @@ -1057,6 +1057,31 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do end end end + + context "Job that failed because of a forward deployment failure" do + let(:job) { create(:ci_build, :forward_deployment_failure, pipeline: pipeline) } + + before do + visit project_job_path(project, job) + wait_for_requests + + find('[data-testid="retry-button"]').click + end + + it 'shows a modal to warn the user' do + page.within('.modal-header') do + expect(page).to have_content 'Are you sure you want to retry this job?' + end + end + + it 'retries the job' do + find('[data-testid="retry-button-modal"]').click + + within '[data-testid="ci-header-content"]' do + expect(page).to have_content('pending') + end + end + end end describe "GET /:project/jobs/:id/download", :js do diff --git a/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_new_spec.js.snap b/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_new_spec.js.snap index 629cbd68bd6..70c38249a36 100644 --- a/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_new_spec.js.snap +++ b/spec/frontend/alerts_settings/__snapshots__/alerts_settings_form_new_spec.js.snap @@ -72,7 +72,7 @@ exports[`AlertsSettingsFormNew with default values renders the initial template <!----> </div> </div> - <div id=\\"test-integration\\" role=\\"group\\" class=\\"form-group gl-form-group\\"><label id=\\"test-integration__BV_label_\\" for=\\"test-integration\\" class=\\"d-block col-form-label\\">4. Test integration(optional)</label> + <div role=\\"group\\" class=\\"form-group gl-form-group\\" id=\\"__BVID__40\\"><label for=\\"test-integration\\" class=\\"d-block col-form-label\\" id=\\"__BVID__40__BV_label_\\">4. Test integration(optional)</label> <div class=\\"bv-no-focus-ring\\"><span class=\\"gl-text-gray-500\\">Provide an example payload from the monitoring tool you intend to integrate with to send a test alert to the <a rel=\\"noopener noreferrer\\" target=\\"_blank\\" href=\\"http://invalid\\" class=\\"gl-link gl-display-inline-block\\">alerts page</a>. This payload can be used to test the integration using the \\"save and test payload\\" button.</span> <textarea id=\\"test-integration\\" disabled=\\"disabled\\" placeholder=\\"Enter test alert JSON....\\" wrap=\\"soft\\" class=\\"gl-form-input gl-form-textarea gl-my-4 form-control is-valid\\" style=\\"resize: none; overflow-y: scroll;\\"></textarea> <!----> <!----> @@ -82,7 +82,7 @@ exports[`AlertsSettingsFormNew with default values renders the initial template <!----> <div class=\\"gl-display-flex gl-justify-content-end\\"><button type=\\"reset\\" class=\\"btn gl-mr-3 js-no-auto-disable btn-default btn-md gl-button\\"> <!----> - <!----> <span class=\\"gl-button-text\\">Cancel</span></button> <button type=\\"button\\" class=\\"btn gl-mr-1 js-no-auto-disable btn-success btn-md gl-button btn-success-secondary\\"> + <!----> <span class=\\"gl-button-text\\">Cancel</span></button> <button data-testid=\\"integration-test-and-submit\\" type=\\"button\\" class=\\"btn gl-mr-1 js-no-auto-disable btn-success btn-md gl-button btn-success-secondary\\"> <!----> <!----> <span class=\\"gl-button-text\\">Save and test payload</span></button> <button data-testid=\\"integration-form-submit\\" type=\\"submit\\" class=\\"btn js-no-auto-disable btn-success btn-md gl-button\\"> <!----> diff --git a/spec/frontend/alerts_settings/alerts_settings_form_new_spec.js b/spec/frontend/alerts_settings/alerts_settings_form_new_spec.js index 8ab760dd962..5436ecd8f4c 100644 --- a/spec/frontend/alerts_settings/alerts_settings_form_new_spec.js +++ b/spec/frontend/alerts_settings/alerts_settings_form_new_spec.js @@ -37,6 +37,8 @@ describe('AlertsSettingsFormNew', () => { const findSubmitButton = () => wrapper.find(`[type = "submit"]`); const findMultiSupportText = () => wrapper.find(`[data-testid="multi-integrations-not-supported"]`); + const findJsonTestSubmit = () => wrapper.find(`[data-testid="integration-test-and-submit"]`); + const findJsonTextArea = () => wrapper.find(`[id = "test-integration"]`); afterEach(() => { if (wrapper) { @@ -203,6 +205,42 @@ describe('AlertsSettingsFormNew', () => { }); }); + describe('submitting the integration with a JSON test payload', () => { + beforeEach(() => { + createComponent({ + data: { + selectedIntegration: typeSet.http, + active: true, + }, + props: { + currentIntegration: { id: '1', name: 'Test' }, + loading: false, + }, + }); + }); + + it('should not allow a user to test invalid JSON', async () => { + jest.useFakeTimers(); + await findJsonTextArea().setValue('Invalid JSON'); + + jest.runAllTimers(); + await wrapper.vm.$nextTick(); + + expect(findJsonTestSubmit().exists()).toBe(true); + expect(findJsonTestSubmit().text()).toBe('Save and test payload'); + expect(findJsonTestSubmit().props('disabled')).toBe(true); + }); + + it('should allow for the form to be automatically saved if the test payload is successfully submitted', async () => { + jest.useFakeTimers(); + await findJsonTextArea().setValue('{ "value": "value" }'); + + jest.runAllTimers(); + await wrapper.vm.$nextTick(); + expect(findJsonTestSubmit().props('disabled')).toBe(false); + }); + }); + describe('Mapping builder section', () => { beforeEach(() => { createComponent({}); diff --git a/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js b/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js index dc3a490154b..4bd881d0460 100644 --- a/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js +++ b/spec/frontend/alerts_settings/alerts_settings_wrapper_spec.js @@ -20,6 +20,7 @@ import { ADD_INTEGRATION_ERROR, RESET_INTEGRATION_TOKEN_ERROR, UPDATE_INTEGRATION_ERROR, + INTEGRATION_PAYLOAD_TEST_ERROR, } from '~/alerts_settings/utils/error_messages'; import createFlash from '~/flash'; import { defaultAlertSettingsConfig } from './util'; @@ -327,6 +328,20 @@ describe('AlertsSettingsWrapper', () => { await waitForPromises(); expect(createFlash).toHaveBeenCalledWith({ message: UPDATE_INTEGRATION_ERROR }); }); + + it('shows an error alert when integration test payload fails ', async () => { + createComponent({ + data: { integrations: { list: mockIntegrations }, currentIntegration: mockIntegrations[0] }, + provide: { glFeatures: { httpIntegrationsList: true } }, + loading: false, + }); + + wrapper.find(AlertsSettingsFormNew).vm.$emit('test-payload-failure'); + + await waitForPromises(); + expect(createFlash).toHaveBeenCalledWith({ message: INTEGRATION_PAYLOAD_TEST_ERROR }); + expect(createFlash).toHaveBeenCalledTimes(1); + }); }); describe('with mocked Apollo client', () => { diff --git a/spec/frontend/boards/components/board_assignee_dropdown_spec.js b/spec/frontend/boards/components/board_assignee_dropdown_spec.js index f81a96c645d..846f8acfa5d 100644 --- a/spec/frontend/boards/components/board_assignee_dropdown_spec.js +++ b/spec/frontend/boards/components/board_assignee_dropdown_spec.js @@ -2,7 +2,7 @@ import { mount } from '@vue/test-utils'; import { GlDropdownItem, GlAvatarLink, GlAvatarLabeled } from '@gitlab/ui'; import BoardAssigneeDropdown from '~/boards/components/board_assignee_dropdown.vue'; import IssuableAssignees from '~/sidebar/components/assignees/issuable_assignees.vue'; -import AssigneesDropdown from '~/vue_shared/components/sidebar/assignees_dropdown.vue'; +import MultiSelectDropdown from '~/vue_shared/components/sidebar/multiselect_dropdown.vue'; import BoardEditableItem from '~/boards/components/sidebar/board_editable_item.vue'; import store from '~/boards/stores'; import getIssueParticipants from '~/vue_shared/components/sidebar/queries/getIssueParticipants.query.graphql'; @@ -109,7 +109,7 @@ describe('BoardCardAssigneeDropdown', () => { createComponent(); expect(wrapper.find(IssuableAssignees).isVisible()).toBe(true); - expect(wrapper.find(AssigneesDropdown).isVisible()).toBe(false); + expect(wrapper.find(MultiSelectDropdown).isVisible()).toBe(false); }); }); @@ -122,7 +122,7 @@ describe('BoardCardAssigneeDropdown', () => { it('shows assignees dropdown', async () => { expect(wrapper.find(IssuableAssignees).isVisible()).toBe(false); - expect(wrapper.find(AssigneesDropdown).isVisible()).toBe(true); + expect(wrapper.find(MultiSelectDropdown).isVisible()).toBe(true); }); it('shows the issue returned as the activeIssue', async () => { diff --git a/spec/frontend/design_management/components/design_overlay_spec.js b/spec/frontend/design_management/components/design_overlay_spec.js index 4ef067e3f5e..f4fd4c70dfc 100644 --- a/spec/frontend/design_management/components/design_overlay_spec.js +++ b/spec/frontend/design_management/components/design_overlay_spec.js @@ -243,11 +243,11 @@ describe('Design overlay component', () => { }); }); - describe('without [adminNote] permission', () => { + describe('without [repositionNote] permission', () => { const mockNoteNotAuthorised = { ...notes[0], userPermissions: { - adminNote: false, + repositionNote: false, }, }; @@ -412,18 +412,18 @@ describe('Design overlay component', () => { describe('canMoveNote', () => { it.each` - adminNotePermission | canMoveNoteResult - ${true} | ${true} - ${false} | ${false} - ${undefined} | ${false} + repositionNotePermission | canMoveNoteResult + ${true} | ${true} + ${false} | ${false} + ${undefined} | ${false} `( - 'returns [$canMoveNoteResult] when [adminNote permission] is [$adminNotePermission]', - ({ adminNotePermission, canMoveNoteResult }) => { + 'returns [$canMoveNoteResult] when [repositionNote permission] is [$repositionNotePermission]', + ({ repositionNotePermission, canMoveNoteResult }) => { createComponent(); const note = { userPermissions: { - adminNote: adminNotePermission, + repositionNote: repositionNotePermission, }, }; expect(wrapper.vm.canMoveNote(note)).toBe(canMoveNoteResult); diff --git a/spec/frontend/design_management/mock_data/discussion.js b/spec/frontend/design_management/mock_data/discussion.js index fbf9a2fdcc1..0e59ef29f8f 100644 --- a/spec/frontend/design_management/mock_data/discussion.js +++ b/spec/frontend/design_management/mock_data/discussion.js @@ -18,7 +18,7 @@ export default { }, createdAt: '2020-05-08T07:10:45Z', userPermissions: { - adminNote: true, + repositionNote: true, }, resolved: false, }, diff --git a/spec/frontend/design_management/utils/cache_update_spec.js b/spec/frontend/design_management/utils/cache_update_spec.js index 56c9b91bef0..2fb08c3ef05 100644 --- a/spec/frontend/design_management/utils/cache_update_spec.js +++ b/spec/frontend/design_management/utils/cache_update_spec.js @@ -3,7 +3,7 @@ import { updateStoreAfterDesignsDelete, updateStoreAfterAddImageDiffNote, updateStoreAfterUploadDesign, - updateStoreAfterUpdateImageDiffNote, + updateStoreAfterRepositionImageDiffNote, } from '~/design_management/utils/cache_update'; import { designDeletionError, @@ -26,11 +26,11 @@ describe('Design Management cache update', () => { describe('error handling', () => { it.each` - fnName | subject | errorMessage | extraArgs - ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} - ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} - ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} - ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]} + fnName | subject | errorMessage | extraArgs + ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} + ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} + ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} + ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterRepositionImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]} `('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => { expect(createFlash).not.toHaveBeenCalled(); expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow(); diff --git a/spec/frontend/design_management/utils/design_management_utils_spec.js b/spec/frontend/design_management/utils/design_management_utils_spec.js index 232cfa2f4ca..368448ead10 100644 --- a/spec/frontend/design_management/utils/design_management_utils_spec.js +++ b/spec/frontend/design_management/utils/design_management_utils_spec.js @@ -3,7 +3,7 @@ import { extractDiscussions, findVersionId, designUploadOptimisticResponse, - updateImageDiffNoteOptimisticResponse, + repositionImageDiffNoteOptimisticResponse, isValidDesignFile, extractDesign, extractDesignNoteId, @@ -112,7 +112,7 @@ describe('optimistic responses', () => { expect(designUploadOptimisticResponse([{ name: 'test' }])).toEqual(expectedResponse); }); - it('correctly generated for updateImageDiffNoteOptimisticResponse', () => { + it('correctly generated for repositionImageDiffNoteOptimisticResponse', () => { const mockNote = { id: 'test-note-id', }; @@ -126,8 +126,8 @@ describe('optimistic responses', () => { const expectedResponse = { __typename: 'Mutation', - updateImageDiffNote: { - __typename: 'UpdateImageDiffNotePayload', + repositionImageDiffNote: { + __typename: 'RepositionImageDiffNotePayload', note: { ...mockNote, position: mockPosition, @@ -135,7 +135,7 @@ describe('optimistic responses', () => { errors: [], }, }; - expect(updateImageDiffNoteOptimisticResponse(mockNote, { position: mockPosition })).toEqual( + expect(repositionImageDiffNoteOptimisticResponse(mockNote, { position: mockPosition })).toEqual( expectedResponse, ); }); diff --git a/spec/frontend/diffs/components/diff_comment_cell_spec.js b/spec/frontend/diffs/components/diff_comment_cell_spec.js new file mode 100644 index 00000000000..d6b68fc52d7 --- /dev/null +++ b/spec/frontend/diffs/components/diff_comment_cell_spec.js @@ -0,0 +1,43 @@ +import { shallowMount } from '@vue/test-utils'; +import DiffCommentCell from '~/diffs/components/diff_comment_cell.vue'; +import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; +import DiffDiscussionReply from '~/diffs/components/diff_discussion_reply.vue'; + +describe('DiffCommentCell', () => { + const createWrapper = (props = {}) => { + const { renderDiscussion, ...otherProps } = props; + const line = { + discussions: [], + renderDiscussion, + }; + const diffFileHash = 'abc'; + + return shallowMount(DiffCommentCell, { + propsData: { line, diffFileHash, ...otherProps }, + }); + }; + + it('renders discussions if line has discussions', () => { + const wrapper = createWrapper({ renderDiscussion: true }); + + expect(wrapper.find(DiffDiscussions).exists()).toBe(true); + }); + + it('does not render discussions if line has no discussions', () => { + const wrapper = createWrapper(); + + expect(wrapper.find(DiffDiscussions).exists()).toBe(false); + }); + + it('renders discussion reply if line has no draft', () => { + const wrapper = createWrapper(); + + expect(wrapper.find(DiffDiscussionReply).exists()).toBe(true); + }); + + it('does not render discussion reply if line has draft', () => { + const wrapper = createWrapper({ hasDraft: true }); + + expect(wrapper.find(DiffDiscussionReply).exists()).toBe(false); + }); +}); diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js index 6d0120d888e..e3a6f7f16a9 100644 --- a/spec/frontend/diffs/components/diff_content_spec.js +++ b/spec/frontend/diffs/components/diff_content_spec.js @@ -12,6 +12,8 @@ import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants'; import diffFileMockData from '../mock_data/diff_file'; import { diffViewerModes } from '~/ide/constants'; +import { diffLines } from '~/diffs/store/getters'; +import DiffView from '~/diffs/components/diff_view.vue'; const localVue = createLocalVue(); localVue.use(Vuex); @@ -33,7 +35,7 @@ describe('DiffContent', () => { diffFile: JSON.parse(JSON.stringify(diffFileMockData)), }; - const createComponent = ({ props, state } = {}) => { + const createComponent = ({ props, state, provide } = {}) => { const fakeStore = new Vuex.Store({ getters: { getNoteableData() { @@ -55,6 +57,10 @@ describe('DiffContent', () => { namespaced: true, getters: { draftsForFile: () => () => true, + draftForLine: () => () => true, + shouldRenderDraftRow: () => () => true, + hasParallelDraftLeft: () => () => true, + hasParallelDraftRight: () => () => true, }, }, diffs: { @@ -68,6 +74,7 @@ describe('DiffContent', () => { isInlineView: isInlineViewGetterMock, isParallelView: isParallelViewGetterMock, getCommentFormForDiffFile: getCommentFormForDiffFileGetterMock, + diffLines, }, actions: { saveDiffDiscussion: saveDiffDiscussionMock, @@ -77,6 +84,8 @@ describe('DiffContent', () => { }, }); + const glFeatures = provide ? { ...provide.glFeatures } : {}; + wrapper = shallowMount(DiffContentComponent, { propsData: { ...defaultProps, @@ -84,6 +93,7 @@ describe('DiffContent', () => { }, localVue, store: fakeStore, + provide: { glFeatures }, }); }; @@ -112,6 +122,16 @@ describe('DiffContent', () => { expect(wrapper.find(ParallelDiffView).exists()).toBe(true); }); + it('should render diff view if `unifiedDiffLines` & `unifiedDiffComponents` are true', () => { + isParallelViewGetterMock.mockReturnValue(true); + createComponent({ + props: { diffFile: textDiffFile }, + provide: { glFeatures: { unifiedDiffLines: true, unifiedDiffComponents: true } }, + }); + + expect(wrapper.find(DiffView).exists()).toBe(true); + }); + it('renders rendering more lines loading icon', () => { createComponent({ props: { diffFile: { ...textDiffFile, renderingLines: true } } }); diff --git a/spec/frontend/diffs/components/diff_row_spec.js b/spec/frontend/diffs/components/diff_row_spec.js new file mode 100644 index 00000000000..f9e76cf8107 --- /dev/null +++ b/spec/frontend/diffs/components/diff_row_spec.js @@ -0,0 +1,127 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import diffsModule from '~/diffs/store/modules'; +import DiffRow from '~/diffs/components/diff_row.vue'; + +describe('DiffRow', () => { + const testLines = [ + { + left: { old_line: 1, discussions: [] }, + right: { new_line: 1, discussions: [] }, + hasDiscussionsLeft: true, + hasDiscussionsRight: true, + }, + { + left: {}, + right: {}, + isMatchLineLeft: true, + isMatchLineRight: true, + }, + {}, + { + left: { old_line: 1, discussions: [] }, + right: { new_line: 1, discussions: [] }, + }, + ]; + + const createWrapper = ({ props, state, isLoggedIn = true }) => { + const localVue = createLocalVue(); + localVue.use(Vuex); + + const diffs = diffsModule(); + diffs.state = { ...diffs.state, ...state }; + + const getters = { isLoggedIn: () => isLoggedIn }; + + const store = new Vuex.Store({ + modules: { diffs }, + getters, + }); + + const propsData = { + fileHash: 'abc', + filePath: 'abc', + line: {}, + ...props, + }; + return shallowMount(DiffRow, { propsData, localVue, store }); + }; + + it('isHighlighted returns true if isCommented is true', () => { + const props = { isCommented: true }; + const wrapper = createWrapper({ props }); + expect(wrapper.vm.isHighlighted).toBe(true); + }); + + it('isHighlighted returns true given line.left', () => { + const props = { + line: { + left: { + line_code: 'abc', + }, + }, + }; + const state = { highlightedRow: 'abc' }; + const wrapper = createWrapper({ props, state }); + expect(wrapper.vm.isHighlighted).toBe(true); + }); + + it('isHighlighted returns true given line.right', () => { + const props = { + line: { + right: { + line_code: 'abc', + }, + }, + }; + const state = { highlightedRow: 'abc' }; + const wrapper = createWrapper({ props, state }); + expect(wrapper.vm.isHighlighted).toBe(true); + }); + + it('isHighlighted returns false given line.left', () => { + const props = { + line: { + left: { + line_code: 'abc', + }, + }, + }; + const wrapper = createWrapper({ props }); + expect(wrapper.vm.isHighlighted).toBe(false); + }); + + describe.each` + side + ${'left'} + ${'right'} + `('$side side', ({ side }) => { + it(`renders empty cells if ${side} is unavailable`, () => { + const wrapper = createWrapper({ props: { line: testLines[2] } }); + expect(wrapper.find(`[data-testid="${side}LineNumber"]`).exists()).toBe(false); + expect(wrapper.find(`[data-testid="${side}EmptyCell"]`).exists()).toBe(true); + }); + + it('renders comment button', () => { + const wrapper = createWrapper({ props: { line: testLines[3] } }); + expect(wrapper.find(`[data-testid="${side}CommentButton"]`).exists()).toBe(true); + }); + + it('renders avatars', () => { + const wrapper = createWrapper({ props: { line: testLines[0] } }); + expect(wrapper.find(`[data-testid="${side}Discussions"]`).exists()).toBe(true); + }); + }); + + it('renders left line numbers', () => { + const wrapper = createWrapper({ props: { line: testLines[0] } }); + const lineNumber = testLines[0].left.old_line; + expect(wrapper.find(`[data-linenumber="${lineNumber}"]`).exists()).toBe(true); + }); + + it('renders right line numbers', () => { + const wrapper = createWrapper({ props: { line: testLines[0] } }); + const lineNumber = testLines[0].right.new_line; + expect(wrapper.find(`[data-linenumber="${lineNumber}"]`).exists()).toBe(true); + }); +}); diff --git a/spec/frontend/diffs/components/diff_row_utils_spec.js b/spec/frontend/diffs/components/diff_row_utils_spec.js index 394b6cb1914..c001857fa49 100644 --- a/spec/frontend/diffs/components/diff_row_utils_spec.js +++ b/spec/frontend/diffs/components/diff_row_utils_spec.js @@ -201,3 +201,76 @@ describe('shouldShowCommentButton', () => { }, ); }); + +describe('mapParallel', () => { + it('should assign computed properties to the line object', () => { + const side = { + discussions: [{}], + discussionsExpanded: true, + hasForm: true, + }; + const content = { + diffFile: {}, + hasParallelDraftLeft: () => false, + hasParallelDraftRight: () => false, + draftForLine: () => ({}), + }; + const line = { left: side, right: side }; + const expectation = { + commentRowClasses: '', + draftRowClasses: 'js-temp-notes-holder', + hasDiscussionsLeft: true, + hasDiscussionsRight: true, + isContextLineLeft: false, + isContextLineRight: false, + isMatchLineLeft: false, + isMatchLineRight: false, + isMetaLineLeft: false, + isMetaLineRight: false, + }; + const leftExpectation = { + renderDiscussion: true, + hasDraft: false, + lineDraft: {}, + hasCommentForm: true, + }; + const rightExpectation = { + renderDiscussion: false, + hasDraft: false, + lineDraft: {}, + hasCommentForm: false, + }; + const mapped = utils.mapParallel(content)(line); + + expect(mapped).toMatchObject(expectation); + expect(mapped.left).toMatchObject(leftExpectation); + expect(mapped.right).toMatchObject(rightExpectation); + }); +}); + +describe('mapInline', () => { + it('should assign computed properties to the line object', () => { + const content = { + diffFile: {}, + shouldRenderDraftRow: () => false, + }; + const line = { + discussions: [{}], + discussionsExpanded: true, + hasForm: true, + }; + const expectation = { + commentRowClasses: '', + hasDiscussions: true, + isContextLine: false, + isMatchLine: false, + isMetaLine: false, + renderDiscussion: true, + hasDraft: false, + hasCommentForm: true, + }; + const mapped = utils.mapInline(content)(line); + + expect(mapped).toMatchObject(expectation); + }); +}); diff --git a/spec/frontend/diffs/components/diff_view_spec.js b/spec/frontend/diffs/components/diff_view_spec.js new file mode 100644 index 00000000000..20ac844fe00 --- /dev/null +++ b/spec/frontend/diffs/components/diff_view_spec.js @@ -0,0 +1,65 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import DiffView from '~/diffs/components/diff_view.vue'; +// import DraftNote from '~/batch_comments/components/draft_note.vue'; +// import DiffRow from '~/diffs/components/diff_row.vue'; +// import DiffCommentCell from '~/diffs/components/diff_comment_cell.vue'; +// import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue'; + +describe('DiffView', () => { + const DiffExpansionCell = { template: `<div/>` }; + const DiffRow = { template: `<div/>` }; + const DiffCommentCell = { template: `<div/>` }; + const DraftNote = { template: `<div/>` }; + const createWrapper = props => { + const localVue = createLocalVue(); + localVue.use(Vuex); + + const batchComments = { + getters: { + shouldRenderDraftRow: () => false, + shouldRenderParallelDraftRow: () => () => true, + draftForLine: () => false, + draftsForFile: () => false, + hasParallelDraftLeft: () => false, + hasParallelDraftRight: () => false, + }, + namespaced: true, + }; + const diffs = { getters: { commitId: () => 'abc123' }, namespaced: true }; + const notes = { + state: { selectedCommentPosition: null, selectedCommentPositionHover: null }, + }; + + const store = new Vuex.Store({ + modules: { diffs, notes, batchComments }, + }); + + const propsData = { + diffFile: {}, + diffLines: [], + ...props, + }; + const stubs = { DiffExpansionCell, DiffRow, DiffCommentCell, DraftNote }; + return shallowMount(DiffView, { propsData, store, localVue, stubs }); + }; + + it('renders a match line', () => { + const wrapper = createWrapper({ diffLines: [{ isMatchLineLeft: true }] }); + expect(wrapper.find(DiffExpansionCell).exists()).toBe(true); + }); + + it('renders a comment row', () => { + const wrapper = createWrapper({ + diffLines: [{ renderCommentRow: true, left: { lineDraft: {} } }], + }); + expect(wrapper.find(DiffCommentCell).exists()).toBe(true); + }); + + it('renders a draft row', () => { + const wrapper = createWrapper({ + diffLines: [{ renderCommentRow: true, left: { lineDraft: { isDraft: true } } }], + }); + expect(wrapper.find(DraftNote).exists()).toBe(true); + }); +}); diff --git a/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js b/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js deleted file mode 100644 index 81e5403d502..00000000000 --- a/spec/frontend/diffs/components/inline_diff_expansion_row_spec.js +++ /dev/null @@ -1,32 +0,0 @@ -import Vue from 'vue'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; -import { createStore } from '~/mr_notes/stores'; -import InlineDiffExpansionRow from '~/diffs/components/inline_diff_expansion_row.vue'; -import diffFileMockData from '../mock_data/diff_file'; - -describe('InlineDiffExpansionRow', () => { - const mockData = { ...diffFileMockData }; - const matchLine = mockData.highlighted_diff_lines.pop(); - - const createComponent = (options = {}) => { - const cmp = Vue.extend(InlineDiffExpansionRow); - const defaults = { - fileHash: mockData.file_hash, - contextLinesPath: 'contextLinesPath', - line: matchLine, - isTop: false, - isBottom: false, - }; - const props = { ...defaults, ...options }; - - return createComponentWithStore(cmp, createStore(), props).$mount(); - }; - - describe('template', () => { - it('should render expansion row for match lines', () => { - const vm = createComponent(); - - expect(vm.$el.classList.contains('line_expansion')).toBe(true); - }); - }); -}); diff --git a/spec/frontend/diffs/components/inline_diff_table_row_spec.js b/spec/frontend/diffs/components/inline_diff_table_row_spec.js index c65a39b9083..21e7d7397a0 100644 --- a/spec/frontend/diffs/components/inline_diff_table_row_spec.js +++ b/spec/frontend/diffs/components/inline_diff_table_row_spec.js @@ -4,6 +4,7 @@ import InlineDiffTableRow from '~/diffs/components/inline_diff_table_row.vue'; import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; import diffFileMockData from '../mock_data/diff_file'; import discussionsMockData from '../mock_data/diff_discussions'; +import { mapInline } from '~/diffs/components/diff_row_utils'; const TEST_USER_ID = 'abc123'; const TEST_USER = { id: TEST_USER_ID }; @@ -11,7 +12,16 @@ const TEST_USER = { id: TEST_USER_ID }; describe('InlineDiffTableRow', () => { let wrapper; let store; - const thisLine = diffFileMockData.highlighted_diff_lines[0]; + const mockDiffContent = { + diffFile: diffFileMockData, + shouldRenderDraftRow: jest.fn(), + hasParallelDraftLeft: jest.fn(), + hasParallelDraftRight: jest.fn(), + draftForLine: jest.fn(), + }; + + const applyMap = mapInline(mockDiffContent); + const thisLine = applyMap(diffFileMockData.highlighted_diff_lines[0]); const createComponent = (props = {}, propsStore = store) => { wrapper = shallowMount(InlineDiffTableRow, { @@ -132,7 +142,7 @@ describe('InlineDiffTableRow', () => { ${true} | ${{ ...thisLine, type: 'old-nonewline', discussions: [] }} | ${false} ${true} | ${{ ...thisLine, discussions: [{}] }} | ${false} `('visible is $expectation - line ($line)', ({ isHover, line, expectation }) => { - createComponent({ line }); + createComponent({ line: applyMap(line) }); wrapper.setData({ isHover }); return wrapper.vm.$nextTick().then(() => { @@ -148,7 +158,7 @@ describe('InlineDiffTableRow', () => { 'has attribute disabled=$disabled when the outer component has prop commentsDisabled=$commentsDisabled', ({ disabled, commentsDisabled }) => { createComponent({ - line: { ...thisLine, commentsDisabled }, + line: applyMap({ ...thisLine, commentsDisabled }), }); wrapper.setData({ isHover: true }); @@ -177,7 +187,7 @@ describe('InlineDiffTableRow', () => { 'has the correct tooltip when commentsDisabled=$commentsDisabled', ({ tooltip, commentsDisabled }) => { createComponent({ - line: { ...thisLine, commentsDisabled }, + line: applyMap({ ...thisLine, commentsDisabled }), }); wrapper.setData({ isHover: true }); @@ -216,7 +226,7 @@ describe('InlineDiffTableRow', () => { beforeEach(() => { jest.spyOn(store, 'dispatch').mockImplementation(); createComponent({ - line: { ...thisLine, ...lineProps }, + line: applyMap({ ...thisLine, ...lineProps }), }); }); @@ -268,7 +278,7 @@ describe('InlineDiffTableRow', () => { describe('with showCommentButton', () => { it('renders if line has discussions', () => { - createComponent({ line }); + createComponent({ line: applyMap(line) }); expect(findAvatars().props()).toEqual({ discussions: line.discussions, @@ -278,13 +288,13 @@ describe('InlineDiffTableRow', () => { it('does notrender if line has no discussions', () => { line.discussions = []; - createComponent({ line }); + createComponent({ line: applyMap(line) }); expect(findAvatars().exists()).toEqual(false); }); it('toggles line discussion', () => { - createComponent({ line }); + createComponent({ line: applyMap(line) }); expect(store.dispatch).toHaveBeenCalledTimes(1); diff --git a/spec/frontend/diffs/components/inline_diff_view_spec.js b/spec/frontend/diffs/components/inline_diff_view_spec.js index 39c581e2796..6a1791509fd 100644 --- a/spec/frontend/diffs/components/inline_diff_view_spec.js +++ b/spec/frontend/diffs/components/inline_diff_view_spec.js @@ -1,54 +1,57 @@ -import Vue from 'vue'; import '~/behaviors/markdown/render_gfm'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; +import { mount } from '@vue/test-utils'; +import { getByText } from '@testing-library/dom'; import { createStore } from '~/mr_notes/stores'; import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; +import { mapInline } from '~/diffs/components/diff_row_utils'; import diffFileMockData from '../mock_data/diff_file'; import discussionsMockData from '../mock_data/diff_discussions'; describe('InlineDiffView', () => { - let component; + let wrapper; const getDiffFileMock = () => ({ ...diffFileMockData }); const getDiscussionsMockData = () => [{ ...discussionsMockData }]; const notesLength = getDiscussionsMockData()[0].notes.length; - beforeEach(done => { - const diffFile = getDiffFileMock(); + const setup = (diffFile, diffLines) => { + const mockDiffContent = { + diffFile, + shouldRenderDraftRow: jest.fn(), + }; const store = createStore(); store.dispatch('diffs/setInlineDiffViewType'); - component = createComponentWithStore(Vue.extend(InlineDiffView), store, { - diffFile, - diffLines: diffFile.highlighted_diff_lines, - }).$mount(); - - Vue.nextTick(done); - }); + wrapper = mount(InlineDiffView, { + store, + propsData: { + diffFile, + diffLines: diffLines.map(mapInline(mockDiffContent)), + }, + }); + }; describe('template', () => { it('should have rendered diff lines', () => { - const el = component.$el; + const diffFile = getDiffFileMock(); + setup(diffFile, diffFile.highlighted_diff_lines); - expect(el.querySelectorAll('tr.line_holder').length).toEqual(8); - expect(el.querySelectorAll('tr.line_holder.new').length).toEqual(4); - expect(el.querySelectorAll('tr.line_expansion.match').length).toEqual(1); - expect(el.textContent.indexOf('Bad dates')).toBeGreaterThan(-1); + expect(wrapper.findAll('tr.line_holder').length).toEqual(8); + expect(wrapper.findAll('tr.line_holder.new').length).toEqual(4); + expect(wrapper.findAll('tr.line_expansion.match').length).toEqual(1); + getByText(wrapper.element, /Bad dates/i); }); - it('should render discussions', done => { - const el = component.$el; - component.diffLines[1].discussions = getDiscussionsMockData(); - component.diffLines[1].discussionsExpanded = true; - - Vue.nextTick(() => { - expect(el.querySelectorAll('.notes_holder').length).toEqual(1); - expect(el.querySelectorAll('.notes_holder .note').length).toEqual(notesLength + 1); - expect(el.innerText.indexOf('comment 5')).toBeGreaterThan(-1); - component.$store.dispatch('setInitialNotes', []); + it('should render discussions', () => { + const diffFile = getDiffFileMock(); + diffFile.highlighted_diff_lines[1].discussions = getDiscussionsMockData(); + diffFile.highlighted_diff_lines[1].discussionsExpanded = true; + setup(diffFile, diffFile.highlighted_diff_lines); - done(); - }); + expect(wrapper.findAll('.notes_holder').length).toEqual(1); + expect(wrapper.findAll('.notes_holder .note').length).toEqual(notesLength + 1); + getByText(wrapper.element, 'comment 5'); + wrapper.vm.$store.dispatch('setInitialNotes', []); }); }); }); diff --git a/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js b/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js deleted file mode 100644 index 38112445e8d..00000000000 --- a/spec/frontend/diffs/components/parallel_diff_expansion_row_spec.js +++ /dev/null @@ -1,31 +0,0 @@ -import Vue from 'vue'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; -import { createStore } from '~/mr_notes/stores'; -import ParallelDiffExpansionRow from '~/diffs/components/parallel_diff_expansion_row.vue'; -import diffFileMockData from '../mock_data/diff_file'; - -describe('ParallelDiffExpansionRow', () => { - const matchLine = diffFileMockData.highlighted_diff_lines[5]; - - const createComponent = (options = {}) => { - const cmp = Vue.extend(ParallelDiffExpansionRow); - const defaults = { - fileHash: diffFileMockData.file_hash, - contextLinesPath: 'contextLinesPath', - line: matchLine, - isTop: false, - isBottom: false, - }; - const props = { ...defaults, ...options }; - - return createComponentWithStore(cmp, createStore(), props).$mount(); - }; - - describe('template', () => { - it('should render expansion row for match lines', () => { - const vm = createComponent(); - - expect(vm.$el.classList.contains('line_expansion')).toBe(true); - }); - }); -}); diff --git a/spec/frontend/diffs/components/parallel_diff_table_row_spec.js b/spec/frontend/diffs/components/parallel_diff_table_row_spec.js index 13031bd8b66..57eff177261 100644 --- a/spec/frontend/diffs/components/parallel_diff_table_row_spec.js +++ b/spec/frontend/diffs/components/parallel_diff_table_row_spec.js @@ -3,11 +3,22 @@ import { shallowMount } from '@vue/test-utils'; import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; import { createStore } from '~/mr_notes/stores'; import ParallelDiffTableRow from '~/diffs/components/parallel_diff_table_row.vue'; +import { mapParallel } from '~/diffs/components/diff_row_utils'; import diffFileMockData from '../mock_data/diff_file'; import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; import discussionsMockData from '../mock_data/diff_discussions'; describe('ParallelDiffTableRow', () => { + const mockDiffContent = { + diffFile: diffFileMockData, + shouldRenderDraftRow: jest.fn(), + hasParallelDraftLeft: jest.fn(), + hasParallelDraftRight: jest.fn(), + draftForLine: jest.fn(), + }; + + const applyMap = mapParallel(mockDiffContent); + describe('when one side is empty', () => { let wrapper; let vm; @@ -18,7 +29,7 @@ describe('ParallelDiffTableRow', () => { wrapper = shallowMount(ParallelDiffTableRow, { store: createStore(), propsData: { - line: thisLine, + line: applyMap(thisLine), fileHash: diffFileMockData.file_hash, filePath: diffFileMockData.file_path, contextLinesPath: 'contextLinesPath', @@ -67,7 +78,7 @@ describe('ParallelDiffTableRow', () => { beforeEach(() => { vm = createComponentWithStore(Vue.extend(ParallelDiffTableRow), createStore(), { - line: thisLine, + line: applyMap(thisLine), fileHash: diffFileMockData.file_hash, filePath: diffFileMockData.file_path, contextLinesPath: 'contextLinesPath', @@ -243,7 +254,10 @@ describe('ParallelDiffTableRow', () => { ${{ ...thisLine, left: { type: 'old-nonewline', discussions: [] } }} | ${false} ${{ ...thisLine, left: { discussions: [{}] } }} | ${false} `('visible is $expectation - line ($line)', async ({ line, expectation }) => { - createComponent({ line }, store, { isLeftHover: true, isCommentButtonRendered: true }); + createComponent({ line: applyMap(line) }, store, { + isLeftHover: true, + isCommentButtonRendered: true, + }); expect(findNoteButton().isVisible()).toBe(expectation); }); @@ -320,7 +334,7 @@ describe('ParallelDiffTableRow', () => { Object.assign(thisLine.left, lineProps); Object.assign(thisLine.right, lineProps); createComponent({ - line: { ...thisLine }, + line: applyMap({ ...thisLine }), }); }); @@ -357,7 +371,7 @@ describe('ParallelDiffTableRow', () => { beforeEach(() => { jest.spyOn(store, 'dispatch').mockImplementation(); - line = { + line = applyMap({ left: { line_code: TEST_LINE_CODE, type: 'new', @@ -369,7 +383,7 @@ describe('ParallelDiffTableRow', () => { rich_text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', meta_data: null, }, - }; + }); }); describe('with showCommentButton', () => { @@ -384,7 +398,7 @@ describe('ParallelDiffTableRow', () => { it('does notrender if line has no discussions', () => { line.left.discussions = []; - createComponent({ line }); + createComponent({ line: applyMap(line) }); expect(findAvatars().exists()).toEqual(false); }); diff --git a/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js b/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js index 1fd1bacf2a2..67f4bee766b 100644 --- a/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js +++ b/spec/frontend/feature_flags/components/configure_feature_flags_modal_spec.js @@ -129,7 +129,7 @@ describe('Configure Feature Flags Modal', () => { expect(findPrimaryAction()).toBe(null); }); - it('shold not display regenerating instance ID', async () => { + it('should not display regenerating instance ID', async () => { expect(findDangerCallout().exists()).toBe(false); }); diff --git a/spec/frontend/jobs/components/job_retry_forward_deployment_modal_spec.js b/spec/frontend/jobs/components/job_retry_forward_deployment_modal_spec.js new file mode 100644 index 00000000000..08973223c08 --- /dev/null +++ b/spec/frontend/jobs/components/job_retry_forward_deployment_modal_spec.js @@ -0,0 +1,76 @@ +import { GlLink, GlModal } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import JobRetryForwardDeploymentModal from '~/jobs/components/job_retry_forward_deployment_modal.vue'; +import { JOB_RETRY_FORWARD_DEPLOYMENT_MODAL } from '~/jobs/constants'; +import createStore from '~/jobs/store'; +import job from '../mock_data'; + +describe('Job Retry Forward Deployment Modal', () => { + let store; + let wrapper; + + const retryOutdatedJobDocsUrl = 'url-to-docs'; + const findLink = () => wrapper.find(GlLink); + const findModal = () => wrapper.find(GlModal); + + const createWrapper = ({ props = {}, provide = {}, stubs = {} } = {}) => { + store = createStore(); + wrapper = shallowMount(JobRetryForwardDeploymentModal, { + propsData: { + modalId: 'modal-id', + href: job.retry_path, + ...props, + }, + provide, + store, + stubs, + }); + }; + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + wrapper = null; + } + }); + + beforeEach(createWrapper); + + describe('Modal configuration', () => { + it('should display the correct messages', () => { + const modal = findModal(); + expect(modal.attributes('title')).toMatch(JOB_RETRY_FORWARD_DEPLOYMENT_MODAL.title); + expect(modal.text()).toMatch(JOB_RETRY_FORWARD_DEPLOYMENT_MODAL.info); + expect(modal.text()).toMatch(JOB_RETRY_FORWARD_DEPLOYMENT_MODAL.areYouSure); + }); + }); + + describe('Modal docs help link', () => { + it('should not display an info link when none is provided', () => { + createWrapper(); + + expect(findLink().exists()).toBe(false); + }); + + it('should display an info link when one is provided', () => { + createWrapper({ provide: { retryOutdatedJobDocsUrl } }); + + expect(findLink().attributes('href')).toBe(retryOutdatedJobDocsUrl); + expect(findLink().text()).toMatch(JOB_RETRY_FORWARD_DEPLOYMENT_MODAL.moreInfo); + }); + }); + + describe('Modal actions', () => { + beforeEach(createWrapper); + + it('should correctly configure the primary action', () => { + expect(findModal().props('actionPrimary').attributes).toMatchObject([ + { + 'data-method': 'post', + href: job.retry_path, + variant: 'danger', + }, + ]); + }); + }); +}); diff --git a/spec/frontend/jobs/components/sidebar_job_details_container_spec.js b/spec/frontend/jobs/components/job_sidebar_details_container_spec.js index 1807ed2fae8..be684769b46 100644 --- a/spec/frontend/jobs/components/sidebar_job_details_container_spec.js +++ b/spec/frontend/jobs/components/job_sidebar_details_container_spec.js @@ -5,7 +5,7 @@ import createStore from '~/jobs/store'; import { extendedWrapper } from '../../helpers/vue_test_utils_helper'; import job from '../mock_data'; -describe('Sidebar Job Details Container', () => { +describe('Job Sidebar Details Container', () => { let store; let wrapper; diff --git a/spec/frontend/jobs/components/job_sidebar_retry_button_spec.js b/spec/frontend/jobs/components/job_sidebar_retry_button_spec.js new file mode 100644 index 00000000000..4bf697ab7cc --- /dev/null +++ b/spec/frontend/jobs/components/job_sidebar_retry_button_spec.js @@ -0,0 +1,70 @@ +import { GlButton, GlLink } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import job from '../mock_data'; +import JobsSidebarRetryButton from '~/jobs/components/job_sidebar_retry_button.vue'; +import createStore from '~/jobs/store'; + +describe('Job Sidebar Retry Button', () => { + let store; + let wrapper; + + const forwardDeploymentFailure = 'forward_deployment_failure'; + const findRetryButton = () => wrapper.find(GlButton); + const findRetryLink = () => wrapper.find(GlLink); + + const createWrapper = ({ props = {} } = {}) => { + store = createStore(); + wrapper = shallowMount(JobsSidebarRetryButton, { + propsData: { + href: job.retry_path, + modalId: 'modal-id', + ...props, + }, + store, + }); + }; + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + wrapper = null; + } + }); + + beforeEach(createWrapper); + + it.each([ + [null, false, true], + ['unmet_prerequisites', false, true], + [forwardDeploymentFailure, true, false], + ])( + 'when error is: %s, should render button: %s | should render link: %s', + async (failureReason, buttonExists, linkExists) => { + await store.dispatch('receiveJobSuccess', { ...job, failure_reason: failureReason }); + + expect(findRetryButton().exists()).toBe(buttonExists); + expect(findRetryLink().exists()).toBe(linkExists); + expect(wrapper.text()).toMatch('Retry'); + }, + ); + + describe('Button', () => { + it('should have the correct configuration', async () => { + await store.dispatch('receiveJobSuccess', { failure_reason: forwardDeploymentFailure }); + + expect(findRetryButton().attributes()).toMatchObject({ + category: 'primary', + variant: 'info', + }); + }); + }); + + describe('Link', () => { + it('should have the correct configuration', () => { + expect(findRetryLink().attributes()).toMatchObject({ + 'data-method': 'post', + href: job.retry_path, + }); + }); + }); +}); diff --git a/spec/frontend/jobs/components/sidebar_spec.js b/spec/frontend/jobs/components/sidebar_spec.js index a591e8777b0..1d4be2fb81e 100644 --- a/spec/frontend/jobs/components/sidebar_spec.js +++ b/spec/frontend/jobs/components/sidebar_spec.js @@ -1,7 +1,9 @@ import { shallowMount } from '@vue/test-utils'; -import Sidebar from '~/jobs/components/sidebar.vue'; +import Sidebar, { forwardDeploymentFailureModalId } from '~/jobs/components/sidebar.vue'; import StagesDropdown from '~/jobs/components/stages_dropdown.vue'; import JobsContainer from '~/jobs/components/jobs_container.vue'; +import JobRetryForwardDeploymentModal from '~/jobs/components/job_retry_forward_deployment_modal.vue'; +import JobRetryButton from '~/jobs/components/job_sidebar_retry_button.vue'; import createStore from '~/jobs/store'; import job, { jobsInStage } from '../mock_data'; import { extendedWrapper } from '../../helpers/vue_test_utils_helper'; @@ -10,6 +12,13 @@ describe('Sidebar details block', () => { let store; let wrapper; + const forwardDeploymentFailure = 'forward_deployment_failure'; + const findModal = () => wrapper.find(JobRetryForwardDeploymentModal); + const findCancelButton = () => wrapper.findByTestId('cancel-button'); + const findNewIssueButton = () => wrapper.findByTestId('job-new-issue'); + const findRetryButton = () => wrapper.find(JobRetryButton); + const findTerminalLink = () => wrapper.findByTestId('terminal-link'); + const createWrapper = ({ props = {} } = {}) => { store = createStore(); wrapper = extendedWrapper( @@ -33,7 +42,7 @@ describe('Sidebar details block', () => { const copy = { ...job, retry_path: null }; await store.dispatch('receiveJobSuccess', copy); - expect(wrapper.find('.js-retry-button').exists()).toBe(false); + expect(findRetryButton().exists()).toBe(false); }); }); @@ -42,7 +51,7 @@ describe('Sidebar details block', () => { createWrapper(); await store.dispatch('receiveJobSuccess', job); - expect(wrapper.find('.js-terminal-link').exists()).toBe(false); + expect(findTerminalLink().exists()).toBe(false); }); }); @@ -51,7 +60,7 @@ describe('Sidebar details block', () => { createWrapper(); await store.dispatch('receiveJobSuccess', { ...job, terminal_path: 'job/43123/terminal' }); - expect(wrapper.find('.js-terminal-link').exists()).toBe(true); + expect(findTerminalLink().exists()).toBe(true); }); }); @@ -62,16 +71,65 @@ describe('Sidebar details block', () => { }); it('should render link to new issue', () => { - expect(wrapper.findByTestId('job-new-issue').attributes('href')).toBe(job.new_issue_path); - expect(wrapper.find('[data-testid="job-new-issue"]').text()).toBe('New issue'); + expect(findNewIssueButton().attributes('href')).toBe(job.new_issue_path); + expect(findNewIssueButton().text()).toBe('New issue'); }); - it('should render link to retry job', () => { - expect(wrapper.find('.js-retry-button').attributes('href')).toBe(job.retry_path); + it('should render the retry button', () => { + expect(findRetryButton().props('href')).toBe(job.retry_path); }); it('should render link to cancel job', () => { - expect(wrapper.find('.js-cancel-job').attributes('href')).toBe(job.cancel_path); + expect(findCancelButton().text()).toMatch('Cancel'); + expect(findCancelButton().attributes('href')).toBe(job.cancel_path); + }); + }); + + describe('forward deployment failure', () => { + describe('when the relevant data is missing', () => { + it.each` + retryPath | failureReason + ${null} | ${null} + ${''} | ${''} + ${job.retry_path} | ${''} + ${''} | ${forwardDeploymentFailure} + ${job.retry_path} | ${'unmet_prerequisites'} + `( + 'should not render the modal when path and failure are $retryPath, $failureReason', + async ({ retryPath, failureReason }) => { + createWrapper(); + await store.dispatch('receiveJobSuccess', { + ...job, + failure_reason: failureReason, + retry_path: retryPath, + }); + expect(findModal().exists()).toBe(false); + }, + ); + }); + + describe('when there is the relevant error', () => { + beforeEach(() => { + createWrapper(); + return store.dispatch('receiveJobSuccess', { + ...job, + failure_reason: forwardDeploymentFailure, + }); + }); + + it('should render the modal', () => { + expect(findModal().exists()).toBe(true); + }); + + it('should provide the modal id to the button and modal', () => { + expect(findRetryButton().props('modalId')).toBe(forwardDeploymentFailureModalId); + expect(findModal().props('modalId')).toBe(forwardDeploymentFailureModalId); + }); + + it('should provide the retry path to the button and modal', () => { + expect(findRetryButton().props('href')).toBe(job.retry_path); + expect(findModal().props('href')).toBe(job.retry_path); + }); }); }); @@ -90,8 +148,8 @@ describe('Sidebar details block', () => { describe('without jobs for stages', () => { beforeEach(() => store.dispatch('receiveJobSuccess', job)); - it('does not render job container', () => { - expect(wrapper.find('.js-jobs-container').exists()).toBe(false); + it('does not render jobs container', () => { + expect(wrapper.find(JobsContainer).exists()).toBe(false); }); }); diff --git a/spec/frontend/lib/utils/common_utils_spec.js b/spec/frontend/lib/utils/common_utils_spec.js index effc446d846..09eb362c77e 100644 --- a/spec/frontend/lib/utils/common_utils_spec.js +++ b/spec/frontend/lib/utils/common_utils_spec.js @@ -959,6 +959,25 @@ describe('common_utils', () => { }); }); + describe('roundDownFloat', () => { + it('Rounds down decimal places of a float number with provided precision', () => { + expect(commonUtils.roundDownFloat(3.141592, 3)).toBe(3.141); + }); + + it('Rounds down a float number to a whole number when provided precision is zero', () => { + expect(commonUtils.roundDownFloat(3.141592, 0)).toBe(3); + expect(commonUtils.roundDownFloat(3.9, 0)).toBe(3); + }); + + it('Rounds down float number to nearest 0, 10, 100, 1000 and so on when provided precision is below 0', () => { + expect(commonUtils.roundDownFloat(34567.14159, -1)).toBeCloseTo(34560); + expect(commonUtils.roundDownFloat(34567.14159, -2)).toBeCloseTo(34500); + expect(commonUtils.roundDownFloat(34567.14159, -3)).toBeCloseTo(34000); + expect(commonUtils.roundDownFloat(34567.14159, -4)).toBeCloseTo(30000); + expect(commonUtils.roundDownFloat(34567.14159, -5)).toBeCloseTo(0); + }); + }); + describe('searchBy', () => { const searchSpace = { iid: 1, diff --git a/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js b/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js index ce446e6d93e..e85ee39f165 100644 --- a/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js +++ b/spec/frontend/registry/explorer/components/list_page/image_list_row_spec.js @@ -67,7 +67,12 @@ describe('Image List Row', () => { mountComponent(); const link = findDetailsLink(); expect(link.html()).toContain(item.path); - expect(link.props('to').name).toBe('details'); + expect(link.props('to')).toMatchObject({ + name: 'details', + params: { + id: item.id, + }, + }); }); it('contains a clipboard button', () => { diff --git a/spec/frontend/registry/explorer/components/registry_breadcrumb_spec.js b/spec/frontend/registry/explorer/components/registry_breadcrumb_spec.js index b906e44a4f7..d730bfcde24 100644 --- a/spec/frontend/registry/explorer/components/registry_breadcrumb_spec.js +++ b/spec/frontend/registry/explorer/components/registry_breadcrumb_spec.js @@ -32,6 +32,10 @@ describe('Registry Breadcrumb', () => { { name: 'baz', meta: { nameGenerator } }, ]; + const state = { + imageDetails: { foo: 'bar' }, + }; + const findDivider = () => wrapper.find('.js-divider'); const findRootRoute = () => wrapper.find({ ref: 'rootRouteLink' }); const findChildRoute = () => wrapper.find({ ref: 'childRouteLink' }); @@ -52,6 +56,9 @@ describe('Registry Breadcrumb', () => { routes, }, }, + $store: { + state, + }, }, }); }; @@ -80,7 +87,7 @@ describe('Registry Breadcrumb', () => { }); it('the link text is calculated by nameGenerator', () => { - expect(nameGenerator).toHaveBeenCalledWith(routes[0]); + expect(nameGenerator).toHaveBeenCalledWith(state); expect(nameGenerator).toHaveBeenCalledTimes(1); }); }); @@ -104,7 +111,7 @@ describe('Registry Breadcrumb', () => { }); it('the link text is calculated by nameGenerator', () => { - expect(nameGenerator).toHaveBeenCalledWith(routes[1]); + expect(nameGenerator).toHaveBeenCalledWith(state); expect(nameGenerator).toHaveBeenCalledTimes(2); }); }); diff --git a/spec/frontend/registry/explorer/mock_data.js b/spec/frontend/registry/explorer/mock_data.js index a7ffed4c9fd..da5f1840b5c 100644 --- a/spec/frontend/registry/explorer/mock_data.js +++ b/spec/frontend/registry/explorer/mock_data.js @@ -97,3 +97,14 @@ export const imagePagination = { totalPages: 2, nextPage: 2, }; + +export const imageDetailsMock = { + id: 1, + name: 'rails-32309', + path: 'gitlab-org/gitlab-test/rails-32309', + project_id: 1, + location: '0.0.0.0:5000/gitlab-org/gitlab-test/rails-32309', + created_at: '2020-06-29T10:23:47.838Z', + cleanup_policy_started_at: null, + delete_api_path: 'http://0.0.0.0:3000/api/v4/projects/1/registry/repositories/1', +}; diff --git a/spec/frontend/registry/explorer/pages/details_spec.js b/spec/frontend/registry/explorer/pages/details_spec.js index 372875914a5..c09b7e0c067 100644 --- a/spec/frontend/registry/explorer/pages/details_spec.js +++ b/spec/frontend/registry/explorer/pages/details_spec.js @@ -14,9 +14,10 @@ import { SET_TAGS_LIST_SUCCESS, SET_TAGS_PAGINATION, SET_INITIAL_STATE, + SET_IMAGE_DETAILS, } from '~/registry/explorer/stores/mutation_types'; -import { tagsListResponse } from '../mock_data'; +import { tagsListResponse, imageDetailsMock } from '../mock_data'; import { DeleteModal } from '../stubs'; describe('Details Page', () => { @@ -33,8 +34,7 @@ describe('Details Page', () => { const findEmptyTagsState = () => wrapper.find(EmptyTagsState); const findPartialCleanupAlert = () => wrapper.find(PartialCleanupAlert); - const routeIdGenerator = override => - window.btoa(JSON.stringify({ name: 'foo', tags_path: 'bar', ...override })); + const routeId = 1; const tagsArrayToSelectedTags = tags => tags.reduce((acc, c) => { @@ -42,7 +42,7 @@ describe('Details Page', () => { return acc; }, {}); - const mountComponent = ({ options, routeParams } = {}) => { + const mountComponent = ({ options } = {}) => { wrapper = shallowMount(component, { store, stubs: { @@ -51,7 +51,7 @@ describe('Details Page', () => { mocks: { $route: { params: { - id: routeIdGenerator(routeParams), + id: routeId, }, }, }, @@ -65,6 +65,7 @@ describe('Details Page', () => { dispatchSpy.mockResolvedValue(); store.commit(SET_TAGS_LIST_SUCCESS, tagsListResponse.data); store.commit(SET_TAGS_PAGINATION, tagsListResponse.headers); + store.commit(SET_IMAGE_DETAILS, imageDetailsMock); jest.spyOn(Tracking, 'event'); }); @@ -73,6 +74,13 @@ describe('Details Page', () => { wrapper = null; }); + describe('lifecycle events', () => { + it('calls the appropriate action on mount', () => { + mountComponent(); + expect(dispatchSpy).toHaveBeenCalledWith('requestImageDetailsAndTagsList', routeId); + }); + }); + describe('when isLoading is true', () => { beforeEach(() => { store.commit(SET_MAIN_LOADING, true); @@ -194,8 +202,7 @@ describe('Details Page', () => { dispatchSpy.mockResolvedValue(); findPagination().vm.$emit(GlPagination.model.event, 2); expect(store.dispatch).toHaveBeenCalledWith('requestTagsList', { - params: wrapper.vm.$route.params.id, - pagination: { page: 2 }, + page: 2, }); }); }); @@ -227,7 +234,6 @@ describe('Details Page', () => { findDeleteModal().vm.$emit('confirmDelete'); expect(dispatchSpy).toHaveBeenCalledWith('requestDeleteTag', { tag: store.state.tags[0], - params: routeIdGenerator(), }); }); }); @@ -242,7 +248,6 @@ describe('Details Page', () => { findDeleteModal().vm.$emit('confirmDelete'); expect(dispatchSpy).toHaveBeenCalledWith('requestDeleteTags', { ids: store.state.tags.map(t => t.name), - params: routeIdGenerator(), }); }); }); @@ -257,7 +262,7 @@ describe('Details Page', () => { it('has the correct props', () => { mountComponent(); - expect(findDetailsHeader().props()).toEqual({ imageName: 'foo' }); + expect(findDetailsHeader().props()).toEqual({ imageName: imageDetailsMock.name }); }); }); @@ -293,10 +298,14 @@ describe('Details Page', () => { }; describe('when expiration_policy_started is not null', () => { - const routeParams = { cleanup_policy_started_at: Date.now().toString() }; - + beforeEach(() => { + store.commit(SET_IMAGE_DETAILS, { + ...imageDetailsMock, + cleanup_policy_started_at: Date.now().toString(), + }); + }); it('exists', () => { - mountComponent({ routeParams }); + mountComponent(); expect(findPartialCleanupAlert().exists()).toBe(true); }); @@ -304,13 +313,13 @@ describe('Details Page', () => { it('has the correct props', () => { store.commit(SET_INITIAL_STATE, { ...config }); - mountComponent({ routeParams }); + mountComponent(); expect(findPartialCleanupAlert().props()).toEqual({ ...config }); }); it('dismiss hides the component', async () => { - mountComponent({ routeParams }); + mountComponent(); expect(findPartialCleanupAlert().exists()).toBe(true); findPartialCleanupAlert().vm.$emit('dismiss'); diff --git a/spec/frontend/registry/explorer/stores/actions_spec.js b/spec/frontend/registry/explorer/stores/actions_spec.js index 66efbf6446b..07294d34277 100644 --- a/spec/frontend/registry/explorer/stores/actions_spec.js +++ b/spec/frontend/registry/explorer/stores/actions_spec.js @@ -7,13 +7,18 @@ import axios from '~/lib/utils/axios_utils'; import * as actions from '~/registry/explorer/stores/actions'; import * as types from '~/registry/explorer/stores/mutation_types'; import { reposServerResponse, registryServerResponse } from '../mock_data'; +import * as utils from '~/registry/explorer/utils'; jest.mock('~/flash.js'); +jest.mock('~/registry/explorer/utils'); describe('Actions RegistryExplorer Store', () => { let mock; const endpoint = `${TEST_HOST}/endpoint.json`; + const url = `${endpoint}/1}`; + jest.spyOn(utils, 'pathGenerator').mockReturnValue(url); + beforeEach(() => { mock = new MockAdapter(axios); }); @@ -132,15 +137,12 @@ describe('Actions RegistryExplorer Store', () => { }); describe('fetch tags list', () => { - const url = `${endpoint}/1}`; - const params = window.btoa(JSON.stringify({ tags_path: `${endpoint}/1}` })); - it('sets the tagsList', done => { mock.onGet(url).replyOnce(200, registryServerResponse, {}); testAction( actions.requestTagsList, - { params }, + {}, {}, [ { type: types.SET_MAIN_LOADING, payload: true }, @@ -159,7 +161,7 @@ describe('Actions RegistryExplorer Store', () => { it('should create flash on error', done => { testAction( actions.requestTagsList, - { params }, + {}, {}, [ { type: types.SET_MAIN_LOADING, payload: true }, @@ -177,8 +179,6 @@ describe('Actions RegistryExplorer Store', () => { describe('request delete single tag', () => { it('successfully performs the delete request', done => { const deletePath = 'delete/path'; - const params = window.btoa(JSON.stringify({ tags_path: `${endpoint}/1}`, id: 1 })); - mock.onDelete(deletePath).replyOnce(200); testAction( @@ -187,7 +187,6 @@ describe('Actions RegistryExplorer Store', () => { tag: { destroy_path: deletePath, }, - params, }, { tagsPagination: {}, @@ -203,7 +202,7 @@ describe('Actions RegistryExplorer Store', () => { }, { type: 'requestTagsList', - payload: { pagination: {}, params }, + payload: {}, }, ], done, @@ -270,17 +269,13 @@ describe('Actions RegistryExplorer Store', () => { }); describe('request delete multiple tags', () => { - const url = `project-path/registry/repository/foo/tags`; - const params = window.btoa(JSON.stringify({ tags_path: `${url}?format=json` })); - it('successfully performs the delete request', done => { - mock.onDelete(`${url}/bulk_destroy`).replyOnce(200); + mock.onDelete(url).replyOnce(200); testAction( actions.requestDeleteTags, { ids: [1, 2], - params, }, { tagsPagination: {}, @@ -296,7 +291,7 @@ describe('Actions RegistryExplorer Store', () => { }, { type: 'requestTagsList', - payload: { pagination: {}, params }, + payload: {}, }, ], done, @@ -310,7 +305,6 @@ describe('Actions RegistryExplorer Store', () => { actions.requestDeleteTags, { ids: [1, 2], - params, }, { tagsPagination: {}, diff --git a/spec/frontend/registry/explorer/utils_spec.js b/spec/frontend/registry/explorer/utils_spec.js index 77c14ccee65..d26cf3e56da 100644 --- a/spec/frontend/registry/explorer/utils_spec.js +++ b/spec/frontend/registry/explorer/utils_spec.js @@ -13,7 +13,7 @@ describe('Utils', () => { }); it('returns the url with an ending when is passed', () => { - expect(pathGenerator(imageDetails, 'foo')).toBe('/foo/bar/registry/repository/1/foo'); + expect(pathGenerator(imageDetails, '/foo')).toBe('/foo/bar/registry/repository/1/tags/foo'); }); }); }); diff --git a/spec/frontend/vue_shared/components/assignees_dropdown_spec.js b/spec/frontend/vue_shared/components/multiselect_dropdown_spec.js index c34dfffa0cc..9c5c97c57e8 100644 --- a/spec/frontend/vue_shared/components/assignees_dropdown_spec.js +++ b/spec/frontend/vue_shared/components/multiselect_dropdown_spec.js @@ -1,10 +1,10 @@ import { shallowMount } from '@vue/test-utils'; import { getByText } from '@testing-library/dom'; -import AssigneesDropdown from '~/vue_shared/components/sidebar/assignees_dropdown.vue'; +import MultiSelectDropdown from '~/vue_shared/components/sidebar/multiselect_dropdown.vue'; -describe('AssigneesDropdown Component', () => { +describe('MultiSelectDropdown Component', () => { it('renders items slot', () => { - const wrapper = shallowMount(AssigneesDropdown, { + const wrapper = shallowMount(MultiSelectDropdown, { propsData: { text: '', headerText: '', diff --git a/spec/frontend/vue_shared/components/stacked_progress_bar_spec.js b/spec/frontend/vue_shared/components/stacked_progress_bar_spec.js index bc86ee5a0c6..0786882f527 100644 --- a/spec/frontend/vue_shared/components/stacked_progress_bar_spec.js +++ b/spec/frontend/vue_shared/components/stacked_progress_bar_spec.js @@ -29,6 +29,13 @@ describe('StackedProgressBarComponent', () => { vm.$destroy(); }); + const findSuccessBarText = wrapper => wrapper.$el.querySelector('.status-green').innerText.trim(); + const findNeutralBarText = wrapper => + wrapper.$el.querySelector('.status-neutral').innerText.trim(); + const findFailureBarText = wrapper => wrapper.$el.querySelector('.status-red').innerText.trim(); + const findUnavailableBarText = wrapper => + wrapper.$el.querySelector('.status-unavailable').innerText.trim(); + describe('computed', () => { describe('neutralCount', () => { it('returns neutralCount based on totalCount, successCount and failureCount', () => { @@ -37,24 +44,54 @@ describe('StackedProgressBarComponent', () => { }); }); - describe('methods', () => { + describe('template', () => { + it('renders container element', () => { + expect(vm.$el.classList.contains('stacked-progress-bar')).toBeTruthy(); + }); + + it('renders empty state when count is unavailable', () => { + const vmX = createComponent({ totalCount: 0, successCount: 0, failureCount: 0 }); + + expect(findUnavailableBarText(vmX)).not.toBeUndefined(); + }); + + it('renders bar elements when count is available', () => { + expect(findSuccessBarText(vm)).not.toBeUndefined(); + expect(findNeutralBarText(vm)).not.toBeUndefined(); + expect(findFailureBarText(vm)).not.toBeUndefined(); + }); + describe('getPercent', () => { - it('returns percentage from provided count based on `totalCount`', () => { - expect(vm.getPercent(500)).toBe(10); + it('returns correct percentages from provided count based on `totalCount`', () => { + vm = createComponent({ totalCount: 100, successCount: 25, failureCount: 10 }); + + expect(findSuccessBarText(vm)).toBe('25%'); + expect(findNeutralBarText(vm)).toBe('65%'); + expect(findFailureBarText(vm)).toBe('10%'); }); - it('returns percentage with decimal place from provided count based on `totalCount`', () => { - expect(vm.getPercent(67)).toBe(1.3); + it('returns percentage with decimal place when decimal is greater than 1', () => { + vm = createComponent({ successCount: 67 }); + + expect(findSuccessBarText(vm)).toBe('1.3%'); }); - it('returns percentage as `< 1` from provided count based on `totalCount` when evaluated value is less than 1', () => { - expect(vm.getPercent(10)).toBe('< 1'); + it('returns percentage as `< 1%` from provided count based on `totalCount` when evaluated value is less than 1', () => { + vm = createComponent({ successCount: 10 }); + + expect(findSuccessBarText(vm)).toBe('< 1%'); }); - it('returns 0 if totalCount is falsy', () => { + it('returns not available if totalCount is falsy', () => { vm = createComponent({ totalCount: 0 }); - expect(vm.getPercent(100)).toBe(0); + expect(findUnavailableBarText(vm)).toBe('Not available'); + }); + + it('returns 99.9% when numbers are extreme decimals', () => { + vm = createComponent({ totalCount: 1000000 }); + + expect(findNeutralBarText(vm)).toBe('99.9%'); }); }); @@ -82,23 +119,4 @@ describe('StackedProgressBarComponent', () => { }); }); }); - - describe('template', () => { - it('renders container element', () => { - expect(vm.$el.classList.contains('stacked-progress-bar')).toBeTruthy(); - }); - - it('renders empty state when count is unavailable', () => { - const vmX = createComponent({ totalCount: 0, successCount: 0, failureCount: 0 }); - - expect(vmX.$el.querySelectorAll('.status-unavailable').length).not.toBe(0); - vmX.$destroy(); - }); - - it('renders bar elements when count is available', () => { - expect(vm.$el.querySelectorAll('.status-green').length).not.toBe(0); - expect(vm.$el.querySelectorAll('.status-neutral').length).not.toBe(0); - expect(vm.$el.querySelectorAll('.status-red').length).not.toBe(0); - }); - }); }); diff --git a/spec/graphql/mutations/alert_management/update_alert_status_spec.rb b/spec/graphql/mutations/alert_management/update_alert_status_spec.rb index ab98088ebcd..08761ce64c2 100644 --- a/spec/graphql/mutations/alert_management/update_alert_status_spec.rb +++ b/spec/graphql/mutations/alert_management/update_alert_status_spec.rb @@ -37,8 +37,8 @@ RSpec.describe Mutations::AlertManagement::UpdateAlertStatus do context 'error occurs when updating' do it 'returns the alert with errors' do # Stub an error on the alert - allow_next_instance_of(Resolvers::AlertManagement::AlertResolver) do |resolver| - allow(resolver).to receive(:resolve).and_return(alert) + allow_next_instance_of(::AlertManagement::AlertsFinder) do |finder| + allow(finder).to receive(:execute).and_return([alert]) end allow(alert).to receive(:save).and_return(false) diff --git a/spec/graphql/mutations/todos/mark_all_done_spec.rb b/spec/graphql/mutations/todos/mark_all_done_spec.rb index 2f167164050..f3b6bf52ef7 100644 --- a/spec/graphql/mutations/todos/mark_all_done_spec.rb +++ b/spec/graphql/mutations/todos/mark_all_done_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Mutations::Todos::MarkAllDone do expect(todo3.reload.state).to eq('done') expect(other_user_todo.reload.state).to eq('pending') - expect(updated_todo_ids).to contain_exactly(global_id_of(todo1), global_id_of(todo3)) + expect(updated_todo_ids).to contain_exactly(todo1.id, todo3.id) expect(todos).to contain_exactly(todo1, todo3) end diff --git a/spec/graphql/mutations/todos/restore_many_spec.rb b/spec/graphql/mutations/todos/restore_many_spec.rb index 59995e33f2d..dc10355ef22 100644 --- a/spec/graphql/mutations/todos/restore_many_spec.rb +++ b/spec/graphql/mutations/todos/restore_many_spec.rb @@ -24,11 +24,11 @@ RSpec.describe Mutations::Todos::RestoreMany do expect(todo2.reload.state).to eq('pending') expect(other_user_todo.reload.state).to eq('done') - todo_ids = result[:updated_ids] - expect(todo_ids.size).to eq(1) - expect(todo_ids.first).to eq(todo1.to_global_id.to_s) - - expect(result[:todos]).to contain_exactly(todo1) + expect(result).to match( + errors: be_empty, + updated_ids: contain_exactly(todo1.id), + todos: contain_exactly(todo1) + ) end it 'handles a todo which is already pending as expected' do @@ -36,8 +36,11 @@ RSpec.describe Mutations::Todos::RestoreMany do expect_states_were_not_changed - expect(result[:updated_ids]).to eq([]) - expect(result[:todos]).to be_empty + expect(result).to match( + errors: be_empty, + updated_ids: be_empty, + todos: be_empty + ) end it 'ignores requests for todos which do not belong to the current user' do @@ -61,7 +64,7 @@ RSpec.describe Mutations::Todos::RestoreMany do expect(result[:updated_ids].size).to eq(2) returned_todo_ids = result[:updated_ids] - expect(returned_todo_ids).to contain_exactly(todo1.to_global_id.to_s, todo4.to_global_id.to_s) + expect(returned_todo_ids).to contain_exactly(todo1.id, todo4.id) expect(result[:todos]).to contain_exactly(todo1, todo4) expect(todo1.reload.state).to eq('pending') diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index b49efd6a092..f466d117851 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -350,7 +350,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do expect(described_class).to be_blocked_url('http://[fe80::c800:eff:fe74:8]', allow_local_network: false) end - context 'when local domain/IP is whitelisted' do + context 'when local domain/IP is allowed' do let(:url_blocker_attributes) do { allow_localhost: false, @@ -360,11 +360,11 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do before do allow(ApplicationSetting).to receive(:current).and_return(ApplicationSetting.new) - stub_application_setting(outbound_local_requests_whitelist: whitelist) + stub_application_setting(outbound_local_requests_whitelist: allowlist) end - context 'with IPs in whitelist' do - let(:whitelist) do + context 'with IPs in allowlist' do + let(:allowlist) do [ '0.0.0.0', '127.0.0.1', @@ -396,7 +396,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do it_behaves_like 'allows local requests', { allow_localhost: false, allow_local_network: false } - it 'whitelists IP when dns_rebind_protection is disabled' do + it 'allows IP when dns_rebind_protection is disabled' do url = "http://example.com" attrs = url_blocker_attributes.merge(dns_rebind_protection: false) @@ -410,8 +410,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do end end - context 'with domains in whitelist' do - let(:whitelist) do + context 'with domains in allowlist' do + let(:allowlist) do [ 'www.example.com', 'example.com', @@ -420,7 +420,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do ] end - it 'allows domains present in whitelist' do + it 'allows domains present in allowlist' do domain = 'example.com' subdomain1 = 'www.example.com' subdomain2 = 'subdomain.example.com' @@ -435,7 +435,7 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do url_blocker_attributes) end - # subdomain2 is not part of the whitelist so it should be blocked + # subdomain2 is not part of the allowlist so it should be blocked stub_domain_resolv(subdomain2, '192.168.1.1') do expect(described_class).to be_blocked_url("http://#{subdomain2}", url_blocker_attributes) @@ -458,8 +458,8 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do end shared_examples 'dns rebinding checks' do - shared_examples 'whitelists the domain' do - let(:whitelist) { [domain] } + shared_examples 'allowlists the domain' do + let(:allowlist) { [domain] } let(:url) { "http://#{domain}" } before do @@ -475,13 +475,13 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do context 'enabled' do let(:dns_rebind_value) { true } - it_behaves_like 'whitelists the domain' + it_behaves_like 'allowlists the domain' end context 'disabled' do let(:dns_rebind_value) { false } - it_behaves_like 'whitelists the domain' + it_behaves_like 'allowlists the domain' end end end @@ -504,11 +504,11 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do end context 'with ports' do - let(:whitelist) do + let(:allowlist) do ["127.0.0.1:2000"] end - it 'allows domain with port when resolved ip has port whitelisted' do + it 'allows domain with port when resolved ip has port allowed' do stub_domain_resolv("www.resolve-domain.com", '127.0.0.1') do expect(described_class).not_to be_blocked_url("http://www.resolve-domain.com:2000", url_blocker_attributes) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 64bff5d00aa..4c254f54590 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -319,7 +319,7 @@ RSpec.describe User do expect(subject).to validate_presence_of(:username) end - it 'rejects blacklisted names' do + it 'rejects denied names' do user = build(:user, username: 'dashboard') expect(user).not_to be_valid @@ -442,9 +442,9 @@ RSpec.describe User do end describe 'email' do - context 'when no signup domains whitelisted' do + context 'when no signup domains allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return([]) + allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return([]) end it 'accepts any email' do @@ -455,7 +455,7 @@ RSpec.describe User do context 'bad regex' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['([a-zA-Z0-9]+)+\.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['([a-zA-Z0-9]+)+\.com']) end it 'does not hang on evil input' do @@ -467,9 +467,9 @@ RSpec.describe User do end end - context 'when a signup domain is whitelisted and subdomains are allowed' do + context 'when a signup domain is allowed and subdomains are allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['example.com', '*.example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['example.com', '*.example.com']) end it 'accepts info@example.com' do @@ -488,9 +488,9 @@ RSpec.describe User do end end - context 'when a signup domain is whitelisted and subdomains are not allowed' do + context 'when a signup domain is allowed and subdomains are not allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['example.com']) end it 'accepts info@example.com' do @@ -514,15 +514,15 @@ RSpec.describe User do end end - context 'domain blacklist' do + context 'domain denylist' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist_enabled?).and_return(true) - allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist).and_return(['example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist_enabled?).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist).and_return(['example.com']) end context 'bad regex' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist).and_return(['([a-zA-Z0-9]+)+\.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist).and_return(['([a-zA-Z0-9]+)+\.com']) end it 'does not hang on evil input' do @@ -534,7 +534,7 @@ RSpec.describe User do end end - context 'when a signup domain is blacklisted' do + context 'when a signup domain is denied' do it 'accepts info@test.com' do user = build(:user, email: 'info@test.com') expect(user).to be_valid @@ -551,13 +551,13 @@ RSpec.describe User do end end - context 'when a signup domain is blacklisted but a wildcard subdomain is allowed' do + context 'when a signup domain is denied but a wildcard subdomain is allowed' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_blacklist).and_return(['test.example.com']) - allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['*.example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_denylist).and_return(['test.example.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['*.example.com']) end - it 'gives priority to whitelist and allow info@test.example.com' do + it 'gives priority to allowlist and allow info@test.example.com' do user = build(:user, email: 'info@test.example.com') expect(user).to be_valid end @@ -565,7 +565,7 @@ RSpec.describe User do context 'with both lists containing a domain' do before do - allow_any_instance_of(ApplicationSetting).to receive(:domain_whitelist).and_return(['test.com']) + allow_any_instance_of(ApplicationSetting).to receive(:domain_allowlist).and_return(['test.com']) end it 'accepts info@test.com' do @@ -3637,9 +3637,9 @@ RSpec.describe User do end end - context 'when a domain whitelist is in place' do + context 'when a domain allowlist is in place' do before do - stub_application_setting(domain_whitelist: ['gitlab.com']) + stub_application_setting(domain_allowlist: ['gitlab.com']) end it 'creates a ghost user' do diff --git a/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb b/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb new file mode 100644 index 00000000000..3e96d5c5058 --- /dev/null +++ b/spec/requests/api/graphql/mutations/todos/restore_many_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Restoring many Todos' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:author) { create(:user) } + let_it_be(:other_user) { create(:user) } + + let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done) } + let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done) } + + let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :done) } + + let(:input_ids) { [todo1, todo2].map { |obj| global_id_of(obj) } } + let(:input) { { ids: input_ids } } + + let(:mutation) do + graphql_mutation(:todo_restore_many, input, + <<-QL.strip_heredoc + clientMutationId + errors + updatedIds + todos { + id + state + } + QL + ) + end + + def mutation_response + graphql_mutation_response(:todo_restore_many) + end + + it 'restores many todos' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(todo1.reload.state).to eq('pending') + expect(todo2.reload.state).to eq('pending') + expect(other_user_todo.reload.state).to eq('done') + + expect(mutation_response).to include( + 'errors' => be_empty, + 'updatedIds' => match_array(input_ids), + 'todos' => contain_exactly( + { 'id' => global_id_of(todo1), 'state' => 'pending' }, + { 'id' => global_id_of(todo2), 'state' => 'pending' } + ) + ) + end + + context 'when using an invalid gid' do + let(:input_ids) { [global_id_of(author)] } + let(:invalid_gid_error) { /does not represent an instance of #{todo1.class}/ } + + it 'contains the expected error' do + post_graphql_mutation(mutation, current_user: current_user) + + errors = json_response['errors'] + expect(errors).not_to be_blank + expect(errors.first['message']).to match(invalid_gid_error) + + expect(todo1.reload.state).to eq('done') + expect(todo2.reload.state).to eq('done') + end + end +end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 3348c103918..e03c0d37f3d 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -379,41 +379,41 @@ RSpec.describe API::Settings, 'Settings' do end end - context 'domain_blacklist settings' do - it 'rejects domain_blacklist_enabled when domain_blacklist is empty' do + context 'domain_denylist settings' do + it 'rejects domain_denylist_enabled when domain_denylist is empty' do put api('/application/settings', admin), params: { - domain_blacklist_enabled: true, - domain_blacklist: [] + domain_denylist_enabled: true, + domain_denylist: [] } expect(response).to have_gitlab_http_status(:bad_request) message = json_response["message"] - expect(message["domain_blacklist"]).to eq(["Domain blacklist cannot be empty if Blacklist is enabled."]) + expect(message["domain_denylist"]).to eq(["Domain denylist cannot be empty if denylist is enabled."]) end - it 'allows array for domain_blacklist' do + it 'allows array for domain_denylist' do put api('/application/settings', admin), params: { - domain_blacklist_enabled: true, - domain_blacklist: ['domain1.com', 'domain2.com'] + domain_denylist_enabled: true, + domain_denylist: ['domain1.com', 'domain2.com'] } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['domain_blacklist_enabled']).to be(true) - expect(json_response['domain_blacklist']).to eq(['domain1.com', 'domain2.com']) + expect(json_response['domain_denylist_enabled']).to be(true) + expect(json_response['domain_denylist']).to eq(['domain1.com', 'domain2.com']) end - it 'allows a string for domain_blacklist' do + it 'allows a string for domain_denylist' do put api('/application/settings', admin), params: { - domain_blacklist_enabled: true, - domain_blacklist: 'domain3.com, *.domain4.com' + domain_denylist_enabled: true, + domain_denylist: 'domain3.com, *.domain4.com' } expect(response).to have_gitlab_http_status(:ok) - expect(json_response['domain_blacklist_enabled']).to be(true) - expect(json_response['domain_blacklist']).to eq(['domain3.com', '*.domain4.com']) + expect(json_response['domain_denylist_enabled']).to be(true) + expect(json_response['domain_denylist']).to eq(['domain3.com', '*.domain4.com']) end end diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb index 7e05104984e..03e55b3ec46 100644 --- a/spec/services/application_settings/update_service_spec.rb +++ b/spec/services/application_settings/update_service_spec.rb @@ -66,7 +66,7 @@ RSpec.describe ApplicationSettings::UpdateService do context 'when params is blank' do let(:params) { {} } - it 'does not add to whitelist' do + it 'does not add to allowlist' do expect { subject.execute }.not_to change { application_settings.outbound_local_requests_whitelist } @@ -80,7 +80,7 @@ RSpec.describe ApplicationSettings::UpdateService do let(:params) { { add_to_outbound_local_requests_whitelist: ['example.com', ''] } } - it 'adds to whitelist' do + it 'adds to allowlist' do expect { subject.execute }.to change { application_settings.outbound_local_requests_whitelist } @@ -98,7 +98,7 @@ RSpec.describe ApplicationSettings::UpdateService do let(:params) { { outbound_local_requests_allowlist_raw: 'example.com;gitlab.com' } } - it 'overwrites the existing whitelist' do + it 'overwrites the existing allowlist' do expect { subject.execute }.to change { application_settings.outbound_local_requests_whitelist } diff --git a/spec/support/shared_examples/models/application_setting_shared_examples.rb b/spec/support/shared_examples/models/application_setting_shared_examples.rb index c4b58cc6985..92fd4363134 100644 --- a/spec/support/shared_examples/models/application_setting_shared_examples.rb +++ b/spec/support/shared_examples/models/application_setting_shared_examples.rb @@ -49,15 +49,15 @@ end RSpec.shared_examples 'application settings examples' do context 'restricted signup domains' do - it_behaves_like 'string of domains', :domain_allowlist, :domain_whitelist + it_behaves_like 'string of domains', :domain_allowlist, :domain_allowlist end - context 'blacklisted signup domains' do - it_behaves_like 'string of domains', :domain_denylist, :domain_blacklist + context 'denied signup domains' do + it_behaves_like 'string of domains', :domain_denylist, :domain_denylist it 'sets multiple domain with file' do setting.domain_denylist_file = File.open(Rails.root.join('spec/fixtures/', 'domain_denylist.txt')) - expect(setting.domain_blacklist).to contain_exactly('example.com', 'test.com', 'foo.bar') + expect(setting.domain_denylist).to contain_exactly('example.com', 'test.com', 'foo.bar') end end diff --git a/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb b/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb index 7c4cf8cdafa..d98ea1b6ab2 100644 --- a/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb +++ b/spec/workers/container_expiration_policies/cleanup_container_repository_worker_spec.rb @@ -47,6 +47,8 @@ RSpec.describe ContainerExpirationPolicies::CleanupContainerRepositoryWorker do it 'skips the repository' do expect(ContainerExpirationPolicies::CleanupService).not_to receive(:new) + expect(worker).to receive(:log_extra_metadata_on_done).with(:container_repository_id, repository.id) + expect(worker).to receive(:log_extra_metadata_on_done).with(:cleanup_status, :skipped) expect { subject }.to change { ContainerRepository.waiting_for_cleanup.count }.from(1).to(0) expect(repository.reload.cleanup_unscheduled?).to be_truthy diff --git a/yarn.lock b/yarn.lock index d26e14eb03a..2ac639f47db 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5628,17 +5628,15 @@ graphlib@^2.1.7, graphlib@^2.1.8: dependencies: lodash "^4.17.15" -graphql-tag@^2.10.1: - version "2.10.3" - resolved "https://registry.yarnpkg.com/graphql-tag/-/graphql-tag-2.10.3.tgz#ea1baba5eb8fc6339e4c4cf049dabe522b0edf03" - integrity sha512-4FOv3ZKfA4WdOKJeHdz6B3F/vxBLSgmBcGeAFPf4n1F64ltJUvOOerNj0rsJxONQGdhUMynQIvd6LzB+1J5oKA== +graphql-tag@^2.11.0: + version "2.11.0" + resolved "https://registry.yarnpkg.com/graphql-tag/-/graphql-tag-2.11.0.tgz#1deb53a01c46a7eb401d6cb59dec86fa1cccbffd" + integrity sha512-VmsD5pJqWJnQZMUeRwrDhfgoyqcfwEkvtpANqcoUG8/tOLkwNgU9mzub/Mc78OJMhHjx7gfAMTxzdG43VGg3bA== -graphql@^14.7.0: - version "14.7.0" - resolved "https://registry.yarnpkg.com/graphql/-/graphql-14.7.0.tgz#7fa79a80a69be4a31c27dda824dc04dac2035a72" - integrity sha512-l0xWZpoPKpppFzMfvVyFmp9vLN7w/ZZJPefUicMCepfJeQ8sMcztloGYY9DfjVPo6tIUDzU5Hw3MUbIjj9AVVA== - dependencies: - iterall "^1.2.2" +graphql@^15.4.0: + version "15.4.0" + resolved "https://registry.yarnpkg.com/graphql/-/graphql-15.4.0.tgz#e459dea1150da5a106486ba7276518b5295a4347" + integrity sha512-EB3zgGchcabbsU9cFe1j+yxdzKQKAbGUWRb13DsrsMN1yyfmmIq+2+L5MqVWcDCE4V89R5AyUOi7sMOGxdsYtA== growly@^1.3.0: version "1.3.0" @@ -6719,11 +6717,6 @@ istextorbinary@^2.2.1: editions "^1.3.3" textextensions "2" -iterall@^1.2.2: - version "1.2.2" - resolved "https://registry.yarnpkg.com/iterall/-/iterall-1.2.2.tgz#92d70deb8028e0c39ff3164fdbf4d8b088130cd7" - integrity sha512-yynBb1g+RFUPY64fTrFv7nsjRrENBQJaX2UL+2Szc9REFrSNm1rpSXHGzhmAy7a9uv3vlvgBlXnf9RqmPH1/DA== - jasmine-core@^2.9.0: version "2.9.0" resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-2.9.0.tgz#bfbb56defcd30789adec5a3fbba8504233289c72" |
