diff options
181 files changed, 3713 insertions, 549 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 4c2a8041846..4db8830b115 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.128.0 +0.129.0 diff --git a/app/assets/javascripts/badges/components/badge.vue b/app/assets/javascripts/badges/components/badge.vue index 97232d7f783..8512bf9dd7b 100644 --- a/app/assets/javascripts/badges/components/badge.vue +++ b/app/assets/javascripts/badges/components/badge.vue @@ -1,12 +1,14 @@ <script> import Icon from '~/vue_shared/components/icon.vue'; import Tooltip from '~/vue_shared/directives/tooltip'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { name: 'Badge', components: { Icon, Tooltip, + GlLoadingIcon, }, directives: { Tooltip, diff --git a/app/assets/javascripts/badges/components/badge_form.vue b/app/assets/javascripts/badges/components/badge_form.vue index aff7c4180e3..47e6e618219 100644 --- a/app/assets/javascripts/badges/components/badge_form.vue +++ b/app/assets/javascripts/badges/components/badge_form.vue @@ -4,6 +4,7 @@ import { mapActions, mapState } from 'vuex'; import createFlash from '~/flash'; import { s__, sprintf } from '~/locale'; import LoadingButton from '~/vue_shared/components/loading_button.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import createEmptyBadge from '../empty_badge'; import Badge from './badge.vue'; @@ -14,6 +15,7 @@ export default { components: { Badge, LoadingButton, + GlLoadingIcon, }, props: { isEditing: { diff --git a/app/assets/javascripts/badges/components/badge_list.vue b/app/assets/javascripts/badges/components/badge_list.vue index 359d3e10380..ab518820378 100644 --- a/app/assets/javascripts/badges/components/badge_list.vue +++ b/app/assets/javascripts/badges/components/badge_list.vue @@ -1,5 +1,6 @@ <script> import { mapState } from 'vuex'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import BadgeListRow from './badge_list_row.vue'; import { GROUP_BADGE } from '../constants'; @@ -7,6 +8,7 @@ export default { name: 'BadgeList', components: { BadgeListRow, + GlLoadingIcon, }, computed: { ...mapState(['badges', 'isLoading', 'kind']), diff --git a/app/assets/javascripts/badges/components/badge_list_row.vue b/app/assets/javascripts/badges/components/badge_list_row.vue index 5d16ba3ce6d..f28eff18f03 100644 --- a/app/assets/javascripts/badges/components/badge_list_row.vue +++ b/app/assets/javascripts/badges/components/badge_list_row.vue @@ -2,6 +2,7 @@ import { mapActions, mapState } from 'vuex'; import { s__ } from '~/locale'; import Icon from '~/vue_shared/components/icon.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import { PROJECT_BADGE } from '../constants'; import Badge from './badge.vue'; @@ -10,6 +11,7 @@ export default { components: { Badge, Icon, + GlLoadingIcon, }, props: { badge: { diff --git a/app/assets/javascripts/boards/components/board_list.vue b/app/assets/javascripts/boards/components/board_list.vue index 4dc56c670f0..5e28fc396ab 100644 --- a/app/assets/javascripts/boards/components/board_list.vue +++ b/app/assets/javascripts/boards/components/board_list.vue @@ -1,5 +1,6 @@ <script> import Sortable from 'sortablejs'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import boardNewIssue from './board_new_issue.vue'; import boardCard from './board_card.vue'; import eventHub from '../eventhub'; @@ -11,6 +12,7 @@ export default { components: { boardCard, boardNewIssue, + GlLoadingIcon, }, props: { groupId: { diff --git a/app/assets/javascripts/boards/components/modal/index.vue b/app/assets/javascripts/boards/components/modal/index.vue index 40949cc0656..fdd1346d4c7 100644 --- a/app/assets/javascripts/boards/components/modal/index.vue +++ b/app/assets/javascripts/boards/components/modal/index.vue @@ -6,6 +6,7 @@ import ModalList from './list.vue'; import ModalFooter from './footer.vue'; import EmptyState from './empty_state.vue'; import ModalStore from '../../stores/modal_store'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { components: { @@ -13,6 +14,7 @@ export default { ModalHeader, ModalList, ModalFooter, + GlLoadingIcon, }, props: { newIssuePath: { diff --git a/app/assets/javascripts/boards/components/project_select.vue b/app/assets/javascripts/boards/components/project_select.vue index 0f01a2a6c09..503417644fa 100644 --- a/app/assets/javascripts/boards/components/project_select.vue +++ b/app/assets/javascripts/boards/components/project_select.vue @@ -2,6 +2,7 @@ import $ from 'jquery'; import _ from 'underscore'; import Icon from '~/vue_shared/components/icon.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import eventHub from '../eventhub'; import Api from '../../api'; @@ -9,6 +10,7 @@ export default { name: 'BoardProjectSelect', components: { Icon, + GlLoadingIcon, }, props: { groupId: { diff --git a/app/assets/javascripts/commons/gitlab_ui.js b/app/assets/javascripts/commons/gitlab_ui.js deleted file mode 100644 index 6c18a0fd390..00000000000 --- a/app/assets/javascripts/commons/gitlab_ui.js +++ /dev/null @@ -1,4 +0,0 @@ -import Vue from 'vue'; -import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; - -Vue.component('gl-loading-icon', GlLoadingIcon); diff --git a/app/assets/javascripts/commons/index.js b/app/assets/javascripts/commons/index.js index ea945cd3fa5..0d2fe2925d8 100644 --- a/app/assets/javascripts/commons/index.js +++ b/app/assets/javascripts/commons/index.js @@ -3,5 +3,4 @@ import './polyfills'; import './jquery'; import './bootstrap'; import './vue'; -import './gitlab_ui'; import '../lib/utils/axios_utils'; diff --git a/app/assets/javascripts/deploy_keys/components/action_btn.vue b/app/assets/javascripts/deploy_keys/components/action_btn.vue index 10548da8ec5..ea74fd27ff6 100644 --- a/app/assets/javascripts/deploy_keys/components/action_btn.vue +++ b/app/assets/javascripts/deploy_keys/components/action_btn.vue @@ -1,7 +1,11 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import eventHub from '../eventhub'; export default { + components: { + GlLoadingIcon, + }, props: { deployKey: { type: Object, diff --git a/app/assets/javascripts/deploy_keys/components/app.vue b/app/assets/javascripts/deploy_keys/components/app.vue index 3589599986d..631a9673b3e 100644 --- a/app/assets/javascripts/deploy_keys/components/app.vue +++ b/app/assets/javascripts/deploy_keys/components/app.vue @@ -6,11 +6,13 @@ import eventHub from '../eventhub'; import DeployKeysService from '../service'; import DeployKeysStore from '../store'; import KeysPanel from './keys_panel.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { components: { KeysPanel, NavigationTabs, + GlLoadingIcon, }, props: { endpoint: { diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 59680959bb1..7c60fb3da42 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -3,6 +3,7 @@ import { mapState, mapGetters, mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; import { __ } from '~/locale'; import createFlash from '~/flash'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import eventHub from '../../notes/event_hub'; import CompareVersions from './compare_versions.vue'; import DiffFile from './diff_file.vue'; @@ -21,6 +22,7 @@ export default { HiddenFilesWarning, CommitWidget, TreeList, + GlLoadingIcon, }, props: { endpoint: { @@ -223,7 +225,10 @@ export default { :commit="commit" /> - <div class="files d-flex prepend-top-default"> + <div + :data-can-create-note="getNoteableData.current_user.can_create_note" + class="files d-flex prepend-top-default" + > <div v-show="showTreeList" class="diff-tree-list" diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index fb5556e3cd7..547742a5ff4 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -1,15 +1,22 @@ <script> -import { mapGetters, mapState } from 'vuex'; +import { mapActions, mapGetters, mapState } from 'vuex'; import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; -import { diffModes } from '~/ide/constants'; import InlineDiffView from './inline_diff_view.vue'; import ParallelDiffView from './parallel_diff_view.vue'; +import NoteForm from '../../notes/components/note_form.vue'; +import ImageDiffOverlay from './image_diff_overlay.vue'; +import DiffDiscussions from './diff_discussions.vue'; +import { IMAGE_DIFF_POSITION_TYPE } from '../constants'; +import { getDiffMode } from '../store/utils'; export default { components: { InlineDiffView, ParallelDiffView, DiffViewer, + NoteForm, + DiffDiscussions, + ImageDiffOverlay, }, props: { diffFile: { @@ -23,13 +30,38 @@ export default { endpoint: state => state.diffs.endpoint, }), ...mapGetters('diffs', ['isInlineView', 'isParallelView']), + ...mapGetters('diffs', ['getCommentFormForDiffFile']), + ...mapGetters(['getNoteableData', 'noteableType']), diffMode() { - const diffModeKey = Object.keys(diffModes).find(key => this.diffFile[`${key}File`]); - return diffModes[diffModeKey] || diffModes.replaced; + return getDiffMode(this.diffFile); }, isTextFile() { return this.diffFile.viewer.name === 'text'; }, + diffFileCommentForm() { + return this.getCommentFormForDiffFile(this.diffFile.fileHash); + }, + showNotesContainer() { + return this.diffFile.discussions.length || this.diffFileCommentForm; + }, + }, + methods: { + ...mapActions('diffs', ['saveDiffDiscussion', 'closeDiffFileCommentForm']), + handleSaveNote(note) { + this.saveDiffDiscussion({ + note, + formData: { + noteableData: this.getNoteableData, + noteableType: this.noteableType, + diffFile: this.diffFile, + positionType: IMAGE_DIFF_POSITION_TYPE, + x: this.diffFileCommentForm.x, + y: this.diffFileCommentForm.y, + width: this.diffFileCommentForm.width, + height: this.diffFileCommentForm.height, + }, + }); + }, }, }; </script> @@ -56,7 +88,37 @@ export default { :new-sha="diffFile.diffRefs.headSha" :old-path="diffFile.oldPath" :old-sha="diffFile.diffRefs.baseSha" - :project-path="projectPath"/> + :file-hash="diffFile.fileHash" + :project-path="projectPath" + > + <image-diff-overlay + slot="image-overlay" + :discussions="diffFile.discussions" + :file-hash="diffFile.fileHash" + :can-comment="getNoteableData.current_user.can_create_note" + /> + <div + v-if="showNotesContainer" + class="note-container" + > + <diff-discussions + v-if="diffFile.discussions.length" + class="diff-file-discussions" + :discussions="diffFile.discussions" + :should-collapse-discussions="true" + :render-avatar-badge="true" + /> + <note-form + v-if="diffFileCommentForm" + ref="noteForm" + :is-editing="false" + :save-button-title="__('Comment')" + class="diff-comment-form new-note discussion-form discussion-form-container" + @handleFormUpdate="handleSaveNote" + @cancelForm="closeDiffFileCommentForm(diffFile.fileHash)" + /> + </div> + </diff-viewer> </div> </div> </template> diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index cddbe554fbd..e19207bdc95 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -1,24 +1,40 @@ <script> import { mapActions } from 'vuex'; +import Icon from '~/vue_shared/components/icon.vue'; import noteableDiscussion from '../../notes/components/noteable_discussion.vue'; export default { components: { noteableDiscussion, + Icon, }, props: { discussions: { type: Array, required: true, }, + shouldCollapseDiscussions: { + type: Boolean, + required: false, + default: false, + }, + renderAvatarBadge: { + type: Boolean, + required: false, + default: false, + }, }, methods: { + ...mapActions(['toggleDiscussion']), ...mapActions('diffs', ['removeDiscussionsFromDiff']), deleteNoteHandler(discussion) { if (discussion.notes.length <= 1) { this.removeDiscussionsFromDiff(discussion); } }, + isExpanded(discussion) { + return this.shouldCollapseDiscussions ? discussion.expanded : true; + }, }, }; </script> @@ -26,22 +42,54 @@ export default { <template> <div> <div - v-for="discussion in discussions" + v-for="(discussion, index) in discussions" :key="discussion.id" - class="discussion-notes diff-discussions" + :class="{ + collapsed: !isExpanded(discussion) + }" + class="discussion-notes diff-discussions position-relative" > <ul :data-discussion-id="discussion.id" class="notes" > + <template v-if="shouldCollapseDiscussions"> + <button + :class="{ + 'diff-notes-collapse': discussion.expanded, + 'btn-transparent badge badge-pill': !discussion.expanded + }" + type="button" + class="js-diff-notes-toggle" + @click="toggleDiscussion({ discussionId: discussion.id })" + > + <icon + v-if="discussion.expanded" + name="collapse" + class="collapse-icon" + /> + <template v-else> + {{ index + 1 }} + </template> + </button> + </template> <noteable-discussion + v-show="isExpanded(discussion)" :discussion="discussion" :render-header="false" :render-diff-file="false" :always-expanded="true" :discussions-by-diff-order="true" @noteDeleted="deleteNoteHandler" - /> + > + <span + v-if="renderAvatarBadge" + slot="avatar-badge" + class="badge badge-pill" + > + {{ index + 1 }} + </span> + </noteable-discussion> </ul> </div> </div> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 958e57c5652..e76c7afd863 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -3,6 +3,7 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import _ from 'underscore'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import DiffFileHeader from './diff_file_header.vue'; import DiffContent from './diff_content.vue'; @@ -10,6 +11,7 @@ export default { components: { DiffFileHeader, DiffContent, + GlLoadingIcon, }, props: { file: { diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index f4a9be19496..e31a3546b69 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -55,11 +55,6 @@ export default { required: false, default: false, }, - isContextLine: { - type: Boolean, - required: false, - default: false, - }, isHover: { type: Boolean, required: false, @@ -81,7 +76,6 @@ export default { this.showCommentButton && this.isHover && !this.isMatchLine && - !this.isContextLine && !this.isMetaLine && !this.hasDiscussions ); diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 5d9a0b123fe..e26aa9c9b00 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -3,7 +3,6 @@ import { mapGetters } from 'vuex'; import DiffLineGutterContent from './diff_line_gutter_content.vue'; import { MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, EMPTY_CELL_TYPE, OLD_LINE_TYPE, OLD_NO_NEW_LINE_TYPE, @@ -71,9 +70,6 @@ export default { isMatchLine() { return this.line.type === MATCH_LINE_TYPE; }, - isContextLine() { - return this.line.type === CONTEXT_LINE_TYPE; - }, isMetaLine() { const { type } = this.line; @@ -88,11 +84,7 @@ export default { [type]: type, [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine, [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && - this.isHover && - !this.isMatchLine && - !this.isContextLine && - !this.isMetaLine, + this.isLoggedIn && this.isHover && !this.isMatchLine && !this.isMetaLine, }; }, lineNumber() { diff --git a/app/assets/javascripts/diffs/components/image_diff_overlay.vue b/app/assets/javascripts/diffs/components/image_diff_overlay.vue new file mode 100644 index 00000000000..ae1b0a52901 --- /dev/null +++ b/app/assets/javascripts/diffs/components/image_diff_overlay.vue @@ -0,0 +1,139 @@ +<script> +import { mapActions, mapGetters } from 'vuex'; +import _ from 'underscore'; +import Icon from '~/vue_shared/components/icon.vue'; + +export default { + name: 'ImageDiffOverlay', + components: { + Icon, + }, + props: { + discussions: { + type: [Array, Object], + required: true, + }, + fileHash: { + type: String, + required: true, + }, + canComment: { + type: Boolean, + required: false, + default: false, + }, + showCommentIcon: { + type: Boolean, + required: false, + default: false, + }, + badgeClass: { + type: String, + required: false, + default: 'badge badge-pill', + }, + shouldToggleDiscussion: { + type: Boolean, + required: false, + default: true, + }, + }, + computed: { + ...mapGetters('diffs', ['getDiffFileByHash', 'getCommentFormForDiffFile']), + currentCommentForm() { + return this.getCommentFormForDiffFile(this.fileHash); + }, + allDiscussions() { + return _.isArray(this.discussions) ? this.discussions : [this.discussions]; + }, + }, + methods: { + ...mapActions(['toggleDiscussion']), + ...mapActions('diffs', ['openDiffFileCommentForm']), + getImageDimensions() { + return { + width: this.$parent.width, + height: this.$parent.height, + }; + }, + getPositionForObject(meta) { + const { x, y, width, height } = meta; + const imageWidth = this.getImageDimensions().width; + const imageHeight = this.getImageDimensions().height; + const widthRatio = imageWidth / width; + const heightRatio = imageHeight / height; + + return { + x: Math.round(x * widthRatio), + y: Math.round(y * heightRatio), + }; + }, + getPosition(discussion) { + const { x, y } = this.getPositionForObject(discussion.position); + + return { + left: `${x}px`, + top: `${y}px`, + }; + }, + clickedImage(x, y) { + const { width, height } = this.getImageDimensions(); + + this.openDiffFileCommentForm({ + fileHash: this.fileHash, + width, + height, + x, + y, + }); + }, + }, +}; +</script> + +<template> + <div class="position-absolute w-100 h-100 image-diff-overlay"> + <button + v-if="canComment" + type="button" + class="btn-transparent position-absolute image-diff-overlay-add-comment w-100 h-100 js-add-image-diff-note-button" + @click="clickedImage($event.offsetX, $event.offsetY)" + > + <span class="sr-only"> + {{ __('Add image comment') }} + </span> + </button> + <button + v-for="(discussion, index) in allDiscussions" + :key="discussion.id" + :style="getPosition(discussion)" + :class="badgeClass" + :disabled="!shouldToggleDiscussion" + class="js-image-badge" + type="button" + @click="toggleDiscussion({ discussionId: discussion.id })" + > + <icon + v-if="showCommentIcon" + name="image-comment-dark" + /> + <template v-else> + {{ index + 1 }} + </template> + </button> + <button + v-if="currentCommentForm" + :style="{ + left: `${currentCommentForm.x}px`, + top: `${currentCommentForm.y}px` + }" + :aria-label="__('Comment form position')" + class="btn-transparent comment-indicator" + type="button" + > + <icon + name="image-comment-dark" + /> + </button> + </div> +</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 542acd3d930..44c05e4b634 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -4,8 +4,6 @@ import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, OLD_LINE_TYPE, - CONTEXT_LINE_TYPE, - CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE, LINE_POSITION_LEFT, LINE_POSITION_RIGHT, @@ -41,13 +39,9 @@ export default { }, computed: { ...mapGetters('diffs', ['isInlineView']), - isContextLine() { - return this.line.type === CONTEXT_LINE_TYPE; - }, classNameMap() { return { [this.line.type]: this.line.type, - [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView, }; }, 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 fcc3b3e9117..39312cddfce 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -5,8 +5,6 @@ import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, OLD_LINE_TYPE, - CONTEXT_LINE_TYPE, - CONTEXT_LINE_CLASS_NAME, OLD_NO_NEW_LINE_TYPE, PARALLEL_DIFF_VIEW_TYPE, NEW_NO_NEW_LINE_TYPE, @@ -43,12 +41,8 @@ export default { }; }, computed: { - isContextLine() { - return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE; - }, classNameMap() { return { - [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 6a50d2c1426..f5f5c0ffc29 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -3,7 +3,6 @@ export const PARALLEL_DIFF_VIEW_TYPE = 'parallel'; export const MATCH_LINE_TYPE = 'match'; export const OLD_NO_NEW_LINE_TYPE = 'old-nonewline'; export const NEW_NO_NEW_LINE_TYPE = 'new-nonewline'; -export const CONTEXT_LINE_TYPE = 'context'; export const EMPTY_CELL_TYPE = 'empty-cell'; export const COMMENT_FORM_TYPE = 'commentForm'; export const DIFF_NOTE_TYPE = 'DiffNote'; @@ -12,6 +11,7 @@ export const NOTE_TYPE = 'Note'; export const NEW_LINE_TYPE = 'new'; export const OLD_LINE_TYPE = 'old'; export const TEXT_DIFF_POSITION_TYPE = 'text'; +export const IMAGE_DIFF_POSITION_TYPE = 'image'; export const LINE_POSITION_LEFT = 'left'; export const LINE_POSITION_RIGHT = 'right'; @@ -21,7 +21,6 @@ export const LINE_SIDE_RIGHT = 'right-side'; export const DIFF_VIEW_COOKIE_NAME = 'diff_view'; export const LINE_HOVER_CLASS_NAME = 'is-over'; export const LINE_UNFOLD_CLASS_NAME = 'unfold js-unfold'; -export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded'; export const UNFOLD_COUNT = 20; export const COUNT_OF_AVATARS_IN_GUTTER = 3; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index ca8ae605cb4..d3e9c7c88f0 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -50,8 +50,8 @@ export const assignDiscussionsToDiff = ( }; export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { - const { fileHash, line_code } = removeDiscussion; - commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code }); + const { fileHash, line_code, id } = removeDiscussion; + commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code, id }); }; export const startRenderDiffsQueue = ({ state, commit }) => { @@ -189,6 +189,7 @@ export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => { return dispatch('saveNote', postData, { root: true }) .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) .then(discussion => dispatch('assignDiscussionsToDiff', [discussion])) + .then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.fileHash)) .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); }; @@ -210,5 +211,19 @@ export const toggleShowTreeList = ({ commit, state }) => { localStorage.setItem(MR_TREE_SHOW_KEY, state.showTreeList); }; +export const openDiffFileCommentForm = ({ commit, getters }, formData) => { + const form = getters.getCommentFormForDiffFile(formData.fileHash); + + if (form) { + commit(types.UPDATE_DIFF_FILE_COMMENT_FORM, formData); + } else { + commit(types.OPEN_DIFF_FILE_COMMENT_FORM, formData); + } +}; + +export const closeDiffFileCommentForm = ({ commit }, fileHash) => { + commit(types.CLOSE_DIFF_FILE_COMMENT_FORM, fileHash); +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index d4c205882ff..2bf0ad99c22 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -114,5 +114,8 @@ export const allBlobs = state => Object.values(state.treeEntries).filter(f => f. export const diffFilesLength = state => state.diffFiles.length; +export const getCommentFormForDiffFile = state => fileHash => + state.commentForms.find(form => form.fileHash === fileHash); + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 1c5c35071de..085e255f1d3 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -24,4 +24,6 @@ export default () => ({ showTreeList: storedTreeShow === null ? bp.getBreakpointSize() !== 'xs' : storedTreeShow === 'true', currentDiffFileId: '', + projectPath: '', + commentForms: [], }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 6474ee628e2..e011031e72c 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -14,3 +14,7 @@ export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FIL export const TOGGLE_FOLDER_OPEN = 'TOGGLE_FOLDER_OPEN'; export const TOGGLE_SHOW_TREE_LIST = 'TOGGLE_SHOW_TREE_LIST'; export const UPDATE_CURRENT_DIFF_FILE_ID = 'UPDATE_CURRENT_DIFF_FILE_ID'; + +export const OPEN_DIFF_FILE_COMMENT_FORM = 'OPEN_DIFF_FILE_COMMENT_FORM'; +export const UPDATE_DIFF_FILE_COMMENT_FORM = 'UPDATE_DIFF_FILE_COMMENT_FORM'; +export const CLOSE_DIFF_FILE_COMMENT_FORM = 'CLOSE_DIFF_FILE_COMMENT_FORM'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 38a65f111a2..e651c197968 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -65,7 +65,13 @@ export default { const { highlightedDiffLines, parallelDiffLines } = diffFile; removeMatchLine(diffFile, lineNumbers, bottom); - const lines = addLineReferences(contextLines, lineNumbers, bottom); + + const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({ + ...line, + lineCode: line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`, + discussions: line.discussions || [], + })); + addContextLines({ inlineLines: highlightedDiffLines, parallelLines: parallelDiffLines, @@ -153,20 +159,22 @@ export default { }); }, - [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { + [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode, id }) { const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); if (selectedFile) { - const targetLine = selectedFile.parallelDiffLines.find( - line => - (line.left && line.left.lineCode === lineCode) || - (line.right && line.right.lineCode === lineCode), - ); - if (targetLine) { - const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right'; - - Object.assign(targetLine[side], { - discussions: [], - }); + if (selectedFile.parallelDiffLines) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === lineCode) || + (line.right && line.right.lineCode === lineCode), + ); + if (targetLine) { + const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right'; + + Object.assign(targetLine[side], { + discussions: [], + }); + } } if (selectedFile.highlightedDiffLines) { @@ -180,6 +188,12 @@ export default { }); } } + + if (selectedFile.discussions && selectedFile.discussions.length) { + selectedFile.discussions = selectedFile.discussions.filter( + discussion => discussion.id !== id, + ); + } } }, [types.TOGGLE_FOLDER_OPEN](state, path) { @@ -191,4 +205,25 @@ export default { [types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) { state.currentDiffFileId = fileId; }, + [types.OPEN_DIFF_FILE_COMMENT_FORM](state, formData) { + state.commentForms.push({ + ...formData, + }); + }, + [types.UPDATE_DIFF_FILE_COMMENT_FORM](state, formData) { + const { fileHash } = formData; + + state.commentForms = state.commentForms.map(form => { + if (form.fileHash === fileHash) { + return { + ...formData, + }; + } + + return form; + }); + }, + [types.CLOSE_DIFF_FILE_COMMENT_FORM](state, fileHash) { + state.commentForms = state.commentForms.filter(form => form.fileHash !== fileHash); + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index a482a2b82c0..a935b9b1ffa 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -1,5 +1,6 @@ import _ from 'underscore'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import { diffModes } from '~/ide/constants'; import { LINE_POSITION_LEFT, LINE_POSITION_RIGHT, @@ -34,6 +35,7 @@ export function getFormData(params) { noteTargetLine, diffViewType, linePosition, + positionType, } = params; const position = JSON.stringify({ @@ -42,9 +44,13 @@ export function getFormData(params) { head_sha: diffFile.diffRefs.headSha, old_path: diffFile.oldPath, new_path: diffFile.newPath, - position_type: TEXT_DIFF_POSITION_TYPE, - old_line: noteTargetLine.oldLine, - new_line: noteTargetLine.newLine, + position_type: positionType || TEXT_DIFF_POSITION_TYPE, + old_line: noteTargetLine ? noteTargetLine.oldLine : null, + new_line: noteTargetLine ? noteTargetLine.newLine : null, + x: params.x, + y: params.y, + width: params.width, + height: params.height, }); const postData = { @@ -66,7 +72,7 @@ export function getFormData(params) { diffFile.diffRefs.startSha && diffFile.diffRefs.headSha ? DIFF_NOTE_TYPE : LEGACY_DIFF_NOTE_TYPE, - line_code: noteTargetLine.lineCode, + line_code: noteTargetLine ? noteTargetLine.lineCode : null, }, }; @@ -225,6 +231,7 @@ export function prepareDiffData(diffData) { Object.assign(file, { renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, + discussions: [], }); } } @@ -320,3 +327,8 @@ export const generateTreeList = files => }, { treeEntries: {}, tree: [] }, ); + +export const getDiffMode = diffFile => { + const diffModeKey = Object.keys(diffModes).find(key => diffFile[`${key}File`]); + return diffModes[diffModeKey] || diffModes.replaced; +}; diff --git a/app/assets/javascripts/environments/components/container.vue b/app/assets/javascripts/environments/components/container.vue index 00d197d294f..a48f5fcb7d6 100644 --- a/app/assets/javascripts/environments/components/container.vue +++ b/app/assets/javascripts/environments/components/container.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import tablePagination from '../../vue_shared/components/table_pagination.vue'; import environmentTable from '../components/environments_table.vue'; @@ -6,6 +7,7 @@ export default { components: { environmentTable, tablePagination, + GlLoadingIcon, }, props: { isLoading: { diff --git a/app/assets/javascripts/environments/components/environment_actions.vue b/app/assets/javascripts/environments/components/environment_actions.vue index 0a3ae384afa..03c3ad0401f 100644 --- a/app/assets/javascripts/environments/components/environment_actions.vue +++ b/app/assets/javascripts/environments/components/environment_actions.vue @@ -4,6 +4,7 @@ import { formatTime } from '~/lib/utils/datetime_utility'; import Icon from '~/vue_shared/components/icon.vue'; import eventHub from '../event_hub'; import tooltip from '../../vue_shared/directives/tooltip'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { directives: { @@ -11,6 +12,7 @@ export default { }, components: { Icon, + GlLoadingIcon, }, props: { actions: { diff --git a/app/assets/javascripts/environments/components/environment_rollback.vue b/app/assets/javascripts/environments/components/environment_rollback.vue index 9e137f79dcc..69856abc2d5 100644 --- a/app/assets/javascripts/environments/components/environment_rollback.vue +++ b/app/assets/javascripts/environments/components/environment_rollback.vue @@ -9,10 +9,12 @@ import { s__ } from '~/locale'; import Icon from '~/vue_shared/components/icon.vue'; import tooltip from '~/vue_shared/directives/tooltip'; import eventHub from '../event_hub'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { components: { Icon, + GlLoadingIcon, }, directives: { diff --git a/app/assets/javascripts/environments/components/environments_table.vue b/app/assets/javascripts/environments/components/environments_table.vue index 16abafebbc0..c03d4f29ff9 100644 --- a/app/assets/javascripts/environments/components/environments_table.vue +++ b/app/assets/javascripts/environments/components/environments_table.vue @@ -2,11 +2,13 @@ /** * Render environments table. */ +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import environmentItem from './environment_item.vue'; export default { components: { environmentItem, + GlLoadingIcon, }, props: { diff --git a/app/assets/javascripts/frequent_items/components/app.vue b/app/assets/javascripts/frequent_items/components/app.vue index 70a8838b772..159c0bdc992 100644 --- a/app/assets/javascripts/frequent_items/components/app.vue +++ b/app/assets/javascripts/frequent_items/components/app.vue @@ -1,6 +1,7 @@ <script> import { mapState, mapActions, mapGetters } from 'vuex'; import AccessorUtilities from '~/lib/utils/accessor'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import eventHub from '../event_hub'; import store from '../store/'; import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants'; @@ -14,6 +15,7 @@ export default { components: { FrequentItemsSearchInput, FrequentItemsList, + GlLoadingIcon, }, mixins: [frequentItemsMixin], props: { diff --git a/app/assets/javascripts/groups/components/app.vue b/app/assets/javascripts/groups/components/app.vue index a032f291546..2a4a39436e7 100644 --- a/app/assets/javascripts/groups/components/app.vue +++ b/app/assets/javascripts/groups/components/app.vue @@ -8,6 +8,7 @@ import { HIDDEN_CLASS } from '~/lib/utils/constants'; import { getParameterByName } from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import eventHub from '../event_hub'; import { COMMON_STR, CONTENT_LIST_CLASS } from '../constants'; import groupsComponent from './groups.vue'; @@ -16,6 +17,7 @@ export default { components: { DeprecatedModal, groupsComponent, + GlLoadingIcon, }, props: { action: { diff --git a/app/assets/javascripts/ide/components/branches/search_list.vue b/app/assets/javascripts/ide/components/branches/search_list.vue index 52ccc537c9d..358f1153de2 100644 --- a/app/assets/javascripts/ide/components/branches/search_list.vue +++ b/app/assets/javascripts/ide/components/branches/search_list.vue @@ -2,12 +2,14 @@ import { mapActions, mapState } from 'vuex'; import _ from 'underscore'; import Icon from '~/vue_shared/components/icon.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Item from './item.vue'; export default { components: { Item, Icon, + GlLoadingIcon, }, data() { return { diff --git a/app/assets/javascripts/ide/components/error_message.vue b/app/assets/javascripts/ide/components/error_message.vue index a20dc0a7006..2d9bd99e82a 100644 --- a/app/assets/javascripts/ide/components/error_message.vue +++ b/app/assets/javascripts/ide/components/error_message.vue @@ -1,7 +1,11 @@ <script> import { mapActions } from 'vuex'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { + components: { + GlLoadingIcon, + }, props: { message: { type: Object, diff --git a/app/assets/javascripts/ide/components/file_templates/dropdown.vue b/app/assets/javascripts/ide/components/file_templates/dropdown.vue index 94222c08e91..891f7d48b4c 100644 --- a/app/assets/javascripts/ide/components/file_templates/dropdown.vue +++ b/app/assets/javascripts/ide/components/file_templates/dropdown.vue @@ -2,10 +2,12 @@ import $ from 'jquery'; import { mapActions, mapState } from 'vuex'; import DropdownButton from '~/vue_shared/components/dropdown/dropdown_button.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { components: { DropdownButton, + GlLoadingIcon, }, props: { data: { diff --git a/app/assets/javascripts/ide/components/jobs/list.vue b/app/assets/javascripts/ide/components/jobs/list.vue index acd37605d16..57da8b4e2cb 100644 --- a/app/assets/javascripts/ide/components/jobs/list.vue +++ b/app/assets/javascripts/ide/components/jobs/list.vue @@ -1,10 +1,12 @@ <script> import { mapActions } from 'vuex'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Stage from './stage.vue'; export default { components: { Stage, + GlLoadingIcon, }, props: { stages: { diff --git a/app/assets/javascripts/ide/components/jobs/stage.vue b/app/assets/javascripts/ide/components/jobs/stage.vue index ec168d36b9e..5644759d2f9 100644 --- a/app/assets/javascripts/ide/components/jobs/stage.vue +++ b/app/assets/javascripts/ide/components/jobs/stage.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import tooltip from '../../../vue_shared/directives/tooltip'; import Icon from '../../../vue_shared/components/icon.vue'; import CiIcon from '../../../vue_shared/components/ci_icon.vue'; @@ -12,6 +13,7 @@ export default { Icon, CiIcon, Item, + GlLoadingIcon, }, props: { stage: { diff --git a/app/assets/javascripts/ide/components/merge_requests/list.vue b/app/assets/javascripts/ide/components/merge_requests/list.vue index f5e42e87f1b..e4000f588bd 100644 --- a/app/assets/javascripts/ide/components/merge_requests/list.vue +++ b/app/assets/javascripts/ide/components/merge_requests/list.vue @@ -3,6 +3,7 @@ import { mapActions, mapState } from 'vuex'; import _ from 'underscore'; import { __ } from '~/locale'; import Icon from '~/vue_shared/components/icon.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Item from './item.vue'; import TokenedInput from '../shared/tokened_input.vue'; @@ -16,6 +17,7 @@ export default { TokenedInput, Item, Icon, + GlLoadingIcon, }, data() { return { diff --git a/app/assets/javascripts/ide/components/pipelines/list.vue b/app/assets/javascripts/ide/components/pipelines/list.vue index b670b0355b7..16aec1decd6 100644 --- a/app/assets/javascripts/ide/components/pipelines/list.vue +++ b/app/assets/javascripts/ide/components/pipelines/list.vue @@ -1,6 +1,7 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import _ from 'underscore'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import { sprintf, __ } from '../../../locale'; import Icon from '../../../vue_shared/components/icon.vue'; import CiIcon from '../../../vue_shared/components/ci_icon.vue'; @@ -17,6 +18,7 @@ export default { Tab, JobsList, EmptyState, + GlLoadingIcon, }, computed: { ...mapState(['pipelinesEmptyStateSvgPath', 'links']), diff --git a/app/assets/javascripts/ide/components/preview/clientside.vue b/app/assets/javascripts/ide/components/preview/clientside.vue index 37a8ad36507..0bd56ff6e9b 100644 --- a/app/assets/javascripts/ide/components/preview/clientside.vue +++ b/app/assets/javascripts/ide/components/preview/clientside.vue @@ -3,6 +3,7 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import _ from 'underscore'; import { Manager } from 'smooshpack'; import { listen } from 'codesandbox-api'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Navigator from './navigator.vue'; import { packageJsonPath } from '../../constants'; import { createPathWithExt } from '../../utils'; @@ -10,6 +11,7 @@ import { createPathWithExt } from '../../utils'; export default { components: { Navigator, + GlLoadingIcon, }, data() { return { diff --git a/app/assets/javascripts/ide/components/preview/navigator.vue b/app/assets/javascripts/ide/components/preview/navigator.vue index 42f23801692..af8959186f9 100644 --- a/app/assets/javascripts/ide/components/preview/navigator.vue +++ b/app/assets/javascripts/ide/components/preview/navigator.vue @@ -1,10 +1,12 @@ <script> import { listen } from 'codesandbox-api'; import Icon from '~/vue_shared/components/icon.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { components: { Icon, + GlLoadingIcon, }, props: { manager: { diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue index d23915966de..35104c80694 100644 --- a/app/assets/javascripts/jobs/components/job_app.vue +++ b/app/assets/javascripts/jobs/components/job_app.vue @@ -16,6 +16,8 @@ import Log from './job_log.vue'; import LogTopBar from './job_log_controllers.vue'; import StuckBlock from './stuck_block.vue'; import Sidebar from './sidebar.vue'; +import { sprintf } from '~/locale'; +import delayedJobMixin from '../mixins/delayed_job_mixin'; export default { name: 'JobPageApp', @@ -26,13 +28,14 @@ export default { EmptyState, EnvironmentsBlock, ErasedBlock, - GlLoadingIcon, Icon, Log, LogTopBar, StuckBlock, Sidebar, + GlLoadingIcon, }, + mixins: [delayedJobMixin], props: { runnerSettingsUrl: { type: String, @@ -92,6 +95,17 @@ export default { shouldRenderContent() { return !this.isLoading && !this.hasError; }, + + emptyStateTitle() { + const { emptyStateIllustration, remainingTime } = this; + const { title } = emptyStateIllustration; + + if (this.isDelayedJob) { + return sprintf(title, { remainingTime }); + } + + return title; + }, }, watch: { // Once the job log is loaded, @@ -272,7 +286,7 @@ export default { class="js-job-empty-state" :illustration-path="emptyStateIllustration.image" :illustration-size-class="emptyStateIllustration.size" - :title="emptyStateIllustration.title" + :title="emptyStateTitle" :content="emptyStateIllustration.content" :action="emptyStateAction" /> diff --git a/app/assets/javascripts/jobs/components/job_container_item.vue b/app/assets/javascripts/jobs/components/job_container_item.vue index cdac8a391d1..3ddcfd11dca 100644 --- a/app/assets/javascripts/jobs/components/job_container_item.vue +++ b/app/assets/javascripts/jobs/components/job_container_item.vue @@ -1,7 +1,10 @@ <script> -import { GlTooltipDirective, GlLink } from '@gitlab-org/gitlab-ui'; +import { GlLink } from '@gitlab-org/gitlab-ui'; +import tooltip from '~/vue_shared/directives/tooltip'; import CiIcon from '~/vue_shared/components/ci_icon.vue'; import Icon from '~/vue_shared/components/icon.vue'; +import delayedJobMixin from '~/jobs/mixins/delayed_job_mixin'; +import { sprintf } from '~/locale'; export default { components: { @@ -10,8 +13,9 @@ export default { GlLink, }, directives: { - GlTooltip: GlTooltipDirective, + tooltip, }, + mixins: [delayedJobMixin], props: { job: { type: Object, @@ -24,7 +28,14 @@ export default { }, computed: { tooltipText() { - return `${this.job.name} - ${this.job.status.tooltip}`; + const { name, status } = this.job; + const text = `${name} - ${status.tooltip}`; + + if (this.isDelayedJob) { + return sprintf(text, { remainingTime: this.remainingTime }); + } + + return text; }, }, }; @@ -39,7 +50,7 @@ export default { }" > <gl-link - v-gl-tooltip + v-tooltip :href="job.status.details_path" :title="tooltipText" data-boundary="viewport" diff --git a/app/assets/javascripts/jobs/mixins/delayed_job_mixin.js b/app/assets/javascripts/jobs/mixins/delayed_job_mixin.js new file mode 100644 index 00000000000..8c7fb785a61 --- /dev/null +++ b/app/assets/javascripts/jobs/mixins/delayed_job_mixin.js @@ -0,0 +1,50 @@ +import { calculateRemainingMilliseconds, formatTime } from '~/lib/utils/datetime_utility'; + +export default { + data() { + return { + remainingTime: formatTime(0), + remainingTimeIntervalId: null, + }; + }, + + mounted() { + this.startRemainingTimeInterval(); + }, + + beforeDestroy() { + if (this.remainingTimeIntervalId) { + clearInterval(this.remainingTimeIntervalId); + } + }, + + computed: { + isDelayedJob() { + return this.job && this.job.scheduled; + }, + }, + + watch: { + isDelayedJob() { + this.startRemainingTimeInterval(); + }, + }, + + methods: { + startRemainingTimeInterval() { + if (this.remainingTimeIntervalId) { + clearInterval(this.remainingTimeIntervalId); + } + + if (this.isDelayedJob) { + this.updateRemainingTime(); + this.remainingTimeIntervalId = setInterval(() => this.updateRemainingTime(), 1000); + } + }, + + updateRemainingTime() { + const remainingMilliseconds = calculateRemainingMilliseconds(this.job.scheduled_at); + this.remainingTime = formatTime(remainingMilliseconds); + }, + }, +}; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 2950c2299ab..d8255181574 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -11,7 +11,6 @@ import bp from './breakpoints'; import { parseUrlPathname, handleLocationHash, isMetaClick } from './lib/utils/common_utils'; import { isInVueNoteablePage } from './lib/utils/dom_utils'; import { getLocationHash } from './lib/utils/url_utility'; -import initDiscussionTab from './image_diff/init_discussion_tab'; import Diff from './diff'; import { localTimeAgo } from './lib/utils/datetime_utility'; import syntaxHighlight from './syntax_highlight'; @@ -207,8 +206,6 @@ export default class MergeRequestTabs { } this.resetViewContainer(); this.destroyPipelinesView(); - - initDiscussionTab(); } if (this.setUrl) { this.setCurrentAction(action); diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index eaa0cded224..b209f736c3f 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -1,15 +1,18 @@ <script> import { mapState, mapActions } from 'vuex'; -import imageDiffHelper from '~/image_diff/helpers/index'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; +import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; +import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; import { GlSkeletonLoading } from '@gitlab-org/gitlab-ui'; -import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; +import { trimFirstCharOfLineContent, getDiffMode } from '~/diffs/store/utils'; export default { components: { DiffFileHeader, GlSkeletonLoading, + DiffViewer, + ImageDiffOverlay, }, props: { discussion: { @@ -25,7 +28,11 @@ export default { computed: { ...mapState({ noteableData: state => state.notes.noteableData, + projectPath: state => state.diffs.projectPath, }), + diffMode() { + return getDiffMode(this.diffFile); + }, hasTruncatedDiffLines() { return this.discussion.truncatedDiffLines && this.discussion.truncatedDiffLines.length !== 0; }, @@ -62,11 +69,7 @@ export default { }, }, mounted() { - if (this.isImageDiff) { - const canCreateNote = false; - const renderCommentBadge = true; - imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge); - } else if (!this.hasTruncatedDiffLines) { + if (!this.hasTruncatedDiffLines) { this.fetchDiff(); } }, @@ -160,7 +163,24 @@ export default { <div v-else > - <div v-html="imageDiffHtml"></div> + <diff-viewer + :diff-mode="diffMode" + :new-path="diffFile.newPath" + :new-sha="diffFile.diffRefs.headSha" + :old-path="diffFile.oldPath" + :old-sha="diffFile.diffRefs.baseSha" + :file-hash="diffFile.fileHash" + :project-path="projectPath" + > + <image-diff-overlay + slot="image-overlay" + :discussions="discussion" + :file-hash="diffFile.fileHash" + :show-comment-icon="true" + :should-toggle-discussion="false" + badge-class="image-comment-badge" + /> + </diff-viewer> <slot></slot> </div> </div> diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index e075f94b82b..01cbe40f444 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -9,11 +9,13 @@ import resolvedDiscussionSvg from 'icons/_icon_status_success_solid.svg'; import ellipsisSvg from 'icons/_ellipsis_v.svg'; import Icon from '~/vue_shared/components/icon.vue'; import tooltip from '~/vue_shared/directives/tooltip'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { name: 'NoteActions', components: { Icon, + GlLoadingIcon, }, directives: { tooltip, @@ -246,7 +248,7 @@ export default { <ul class="dropdown-menu more-actions-dropdown dropdown-open-left"> <li v-if="canReportAsAbuse"> <a :href="reportAbusePath"> - Report as abuse + {{ __('Report abuse to GitLab') }} </a> </li> <li v-if="noteUrl"> @@ -255,7 +257,7 @@ export default { type="button" class="btn-default btn-transparent js-btn-copy-note-link" > - Copy link + {{ __('Copy link') }} </button> </li> <li v-if="canEdit"> @@ -264,7 +266,7 @@ export default { type="button" @click.prevent="onDelete"> <span class="text-danger"> - Delete comment + {{ __('Delete comment') }} </span> </button> </li> diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 6293dd5b7e1..07115ca07c4 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -350,11 +350,18 @@ Please check your network connection and try again.`; <ul class="notes"> <component :is="componentName(note)" - v-for="note in discussion.notes" + v-for="(note, index) in discussion.notes" :key="note.id" :note="componentData(note)" @handleDeleteNote="deleteNoteHandler" - /> + > + <slot + v-if="index === 0" + slot="avatar-badge" + name="avatar-badge" + > + </slot> + </component> </ul> <div :class="{ 'is-replying': isReplying }" diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index f391ed848a4..40222ac4a80 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -182,7 +182,13 @@ export default { :img-src="author.avatar_url" :img-alt="author.name" :img-size="40" - /> + > + <slot + slot="avatar-badge" + name="avatar-badge" + > + </slot> + </user-avatar-link> </div> <div class="timeline-content"> <div class="note-header"> diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue index 23c0be7742e..4de8b3401e8 100644 --- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue @@ -1,10 +1,12 @@ <script> import _ from 'underscore'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import StageColumnComponent from './stage_column_component.vue'; export default { components: { StageColumnComponent, + GlLoadingIcon, }, props: { isLoading: { diff --git a/app/assets/javascripts/pipelines/components/graph/job_item.vue b/app/assets/javascripts/pipelines/components/graph/job_item.vue index a1504592bbc..7cdde8a53b3 100644 --- a/app/assets/javascripts/pipelines/components/graph/job_item.vue +++ b/app/assets/javascripts/pipelines/components/graph/job_item.vue @@ -2,6 +2,8 @@ import ActionComponent from './action_component.vue'; import JobNameComponent from './job_name_component.vue'; import tooltip from '../../../vue_shared/directives/tooltip'; +import { sprintf } from '~/locale'; +import delayedJobMixin from '~/jobs/mixins/delayed_job_mixin'; /** * Renders the badge for the pipeline graph and the job's dropdown. @@ -36,6 +38,7 @@ export default { directives: { tooltip, }, + mixins: [delayedJobMixin], props: { job: { type: Object, @@ -52,6 +55,7 @@ export default { default: Infinity, }, }, + computed: { status() { return this.job && this.job.status ? this.job.status : {}; @@ -59,17 +63,23 @@ export default { tooltipText() { const textBuilder = []; + const { name: jobName } = this.job; - if (this.job.name) { - textBuilder.push(this.job.name); + if (jobName) { + textBuilder.push(jobName); } - if (this.job.name && this.status.tooltip) { + const { tooltip: statusTooltip } = this.status; + if (jobName && statusTooltip) { textBuilder.push('-'); } - if (this.status.tooltip) { - textBuilder.push(this.job.status.tooltip); + if (statusTooltip) { + if (this.isDelayedJob) { + textBuilder.push(sprintf(statusTooltip, { remainingTime: this.remainingTime })); + } else { + textBuilder.push(statusTooltip); + } } return textBuilder.join(' '); @@ -88,6 +98,7 @@ export default { return this.job.status && this.job.status.action && this.job.status.action.path; }, }, + methods: { pipelineActionRequestComplete() { this.$emit('pipelineActionRequestComplete'); diff --git a/app/assets/javascripts/pipelines/components/header_component.vue b/app/assets/javascripts/pipelines/components/header_component.vue index 1f9187c3d65..8f004b491c8 100644 --- a/app/assets/javascripts/pipelines/components/header_component.vue +++ b/app/assets/javascripts/pipelines/components/header_component.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import ciHeader from '../../vue_shared/components/header_ci_component.vue'; import eventHub from '../event_hub'; @@ -6,6 +7,7 @@ export default { name: 'PipelineHeaderSection', components: { ciHeader, + GlLoadingIcon, }, props: { pipeline: { diff --git a/app/assets/javascripts/pipelines/components/pipelines_actions.vue b/app/assets/javascripts/pipelines/components/pipelines_actions.vue index 07a4af3e61e..cb47704ca26 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_actions.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_actions.vue @@ -4,6 +4,7 @@ import eventHub from '../event_hub'; import Icon from '../../vue_shared/components/icon.vue'; import tooltip from '../../vue_shared/directives/tooltip'; import GlCountdown from '~/vue_shared/components/gl_countdown.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { directives: { @@ -12,6 +13,7 @@ export default { components: { Icon, GlCountdown, + GlLoadingIcon, }, props: { actions: { diff --git a/app/assets/javascripts/pipelines/components/stage.vue b/app/assets/javascripts/pipelines/components/stage.vue index 7ec55792850..3df8f7a6da6 100644 --- a/app/assets/javascripts/pipelines/components/stage.vue +++ b/app/assets/javascripts/pipelines/components/stage.vue @@ -13,6 +13,7 @@ */ import $ from 'jquery'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import { __ } from '../../locale'; import Flash from '../../flash'; import axios from '../../lib/utils/axios_utils'; @@ -26,6 +27,7 @@ export default { components: { Icon, JobItem, + GlLoadingIcon, }, directives: { diff --git a/app/assets/javascripts/pipelines/mixins/pipelines.js b/app/assets/javascripts/pipelines/mixins/pipelines.js index 85781f548c6..41bc5dcce5c 100644 --- a/app/assets/javascripts/pipelines/mixins/pipelines.js +++ b/app/assets/javascripts/pipelines/mixins/pipelines.js @@ -1,4 +1,5 @@ import Visibility from 'visibilityjs'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import { __ } from '../../locale'; import Flash from '../../flash'; import Poll from '../../lib/utils/poll'; @@ -13,6 +14,7 @@ export default { PipelinesTableComponent, SvgBlankState, EmptyState, + GlLoadingIcon, }, data() { return { diff --git a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_dropdown_mixin.js b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_dropdown_mixin.js index d5266544307..f5dae5ad808 100644 --- a/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_dropdown_mixin.js +++ b/app/assets/javascripts/projects/gke_cluster_dropdowns/components/gke_dropdown_mixin.js @@ -2,6 +2,7 @@ import _ from 'underscore'; import DropdownSearchInput from '~/vue_shared/components/dropdown/dropdown_search_input.vue'; import DropdownHiddenInput from '~/vue_shared/components/dropdown/dropdown_hidden_input.vue'; import DropdownButton from '~/vue_shared/components/dropdown/dropdown_button.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import store from '../store'; @@ -11,6 +12,7 @@ export default { DropdownButton, DropdownSearchInput, DropdownHiddenInput, + GlLoadingIcon, }, props: { fieldId: { diff --git a/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue b/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue index 120b4fc2f2b..9a729ca9b91 100644 --- a/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue +++ b/app/assets/javascripts/projects/tree/components/commit_pipeline_status_component.vue @@ -5,6 +5,7 @@ import Poll from '~/lib/utils/poll'; import Flash from '~/flash'; import { s__, sprintf } from '~/locale'; import tooltip from '~/vue_shared/directives/tooltip'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import CommitPipelineService from '../services/commit_pipeline_service'; export default { @@ -13,6 +14,7 @@ export default { }, components: { ciIcon, + GlLoadingIcon, }, props: { endpoint: { diff --git a/app/assets/javascripts/registry/components/app.vue b/app/assets/javascripts/registry/components/app.vue index 9dd1c87a87d..0a906f40f5a 100644 --- a/app/assets/javascripts/registry/components/app.vue +++ b/app/assets/javascripts/registry/components/app.vue @@ -1,5 +1,6 @@ <script> import { mapGetters, mapActions } from 'vuex'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Flash from '../../flash'; import store from '../stores'; import collapsibleContainer from './collapsible_container.vue'; @@ -9,6 +10,7 @@ export default { name: 'RegistryListApp', components: { collapsibleContainer, + GlLoadingIcon, }, props: { endpoint: { diff --git a/app/assets/javascripts/registry/components/collapsible_container.vue b/app/assets/javascripts/registry/components/collapsible_container.vue index 501b2625ae5..be9816a55c4 100644 --- a/app/assets/javascripts/registry/components/collapsible_container.vue +++ b/app/assets/javascripts/registry/components/collapsible_container.vue @@ -1,5 +1,6 @@ <script> import { mapActions } from 'vuex'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Flash from '../../flash'; import clipboardButton from '../../vue_shared/components/clipboard_button.vue'; import tooltip from '../../vue_shared/directives/tooltip'; @@ -12,6 +13,7 @@ export default { components: { clipboardButton, tableRegistry, + GlLoadingIcon, }, directives: { tooltip, diff --git a/app/assets/javascripts/reports/components/summary_row.vue b/app/assets/javascripts/reports/components/summary_row.vue index 51188981bed..a44ba833b63 100644 --- a/app/assets/javascripts/reports/components/summary_row.vue +++ b/app/assets/javascripts/reports/components/summary_row.vue @@ -1,6 +1,7 @@ <script> import CiIcon from '~/vue_shared/components/ci_icon.vue'; import Popover from '~/vue_shared/components/help_popover.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; /** * Renders the summary row for each report @@ -15,6 +16,7 @@ export default { components: { CiIcon, Popover, + GlLoadingIcon, }, props: { summary: { diff --git a/app/assets/javascripts/sidebar/components/participants/participants.vue b/app/assets/javascripts/sidebar/components/participants/participants.vue index 11b5dbe5f8e..fe73f6a0cef 100644 --- a/app/assets/javascripts/sidebar/components/participants/participants.vue +++ b/app/assets/javascripts/sidebar/components/participants/participants.vue @@ -2,6 +2,7 @@ import { __, n__, sprintf } from '~/locale'; import tooltip from '~/vue_shared/directives/tooltip'; import userAvatarImage from '~/vue_shared/components/user_avatar/user_avatar_image.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { directives: { @@ -9,6 +10,7 @@ export default { }, components: { userAvatarImage, + GlLoadingIcon, }, props: { loading: { diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue index bc59774f0a8..913a616d9f1 100644 --- a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue +++ b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue @@ -1,6 +1,7 @@ <script> import { __ } from '~/locale'; import tooltip from '~/vue_shared/directives/tooltip'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import Icon from '~/vue_shared/components/icon.vue'; @@ -13,6 +14,7 @@ export default { }, components: { Icon, + GlLoadingIcon, }, props: { issuableId: { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue index ba6a1687e51..b3340290ed3 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_status_icon.vue @@ -1,9 +1,11 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import ciIcon from '../../vue_shared/components/ci_icon.vue'; export default { components: { ciIcon, + GlLoadingIcon, }, props: { status: { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed.vue index 4f8b07484c0..4bfbdcf1404 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import eventHub from '../../event_hub'; import statusIcon from '../mr_widget_status_icon.vue'; @@ -6,6 +7,7 @@ export default { name: 'MRWidgetAutoMergeFailed', components: { statusIcon, + GlLoadingIcon, }, props: { mr: { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue index 656c3b5c47e..7e33021e4b4 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue @@ -6,6 +6,7 @@ import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import MrWidgetAuthorTime from '../../components/mr_widget_author_time.vue'; import statusIcon from '../mr_widget_status_icon.vue'; import eventHub from '../../event_hub'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { name: 'MRWidgetMerged', @@ -16,6 +17,7 @@ export default { MrWidgetAuthorTime, statusIcon, ClipboardButton, + GlLoadingIcon, }, props: { mr: { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue index 041fa13a8f5..0e714cc2aa1 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import simplePoll from '../../../lib/utils/simple_poll'; import eventHub from '../../event_hub'; import statusIcon from '../mr_widget_status_icon.vue'; @@ -8,6 +9,7 @@ export default { name: 'MRWidgetRebase', components: { statusIcon, + GlLoadingIcon, }, props: { mr: { diff --git a/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue b/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue index 8163947cd0c..6f2f0f98690 100644 --- a/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue @@ -17,19 +17,37 @@ export default { type: Boolean, default: true, }, + innerCssClasses: { + type: [Array, Object, String], + required: false, + default: '', + }, }, data() { return { width: 0, height: 0, - isZoomable: false, - isZoomed: false, + isLoaded: false, }; }, computed: { fileSizeReadable() { return numberToHumanSize(this.fileSize); }, + dimensionStyles() { + if (!this.isLoaded) return {}; + + return { + width: `${this.width}px`, + height: `${this.height}px`, + }; + }, + hasFileSize() { + return this.fileSize > 0; + }, + hasDimensions() { + return this.width && this.height; + }, }, beforeDestroy() { window.removeEventListener('resize', this.resizeThrottled, false); @@ -48,51 +66,52 @@ export default { const { contentImg } = this.$refs; if (contentImg) { - this.isZoomable = - contentImg.naturalWidth > contentImg.width || - contentImg.naturalHeight > contentImg.height; - this.width = contentImg.naturalWidth; this.height = contentImg.naturalHeight; - this.$emit('imgLoaded', { - width: this.width, - height: this.height, - renderedWidth: contentImg.clientWidth, - renderedHeight: contentImg.clientHeight, + this.$nextTick(() => { + this.isLoaded = true; + + this.$emit('imgLoaded', { + width: this.width, + height: this.height, + renderedWidth: contentImg.clientWidth, + renderedHeight: contentImg.clientHeight, + }); }); } }, - onImgClick() { - if (this.isZoomable) this.isZoomed = !this.isZoomed; - }, }, }; </script> <template> - <div class="file-container"> - <div class="file-content image_file"> + <div> + <div + :class="innerCssClasses" + :style="dimensionStyles" + class="position-relative" + > <img ref="contentImg" - :class="{ 'is-zoomable': isZoomable, 'is-zoomed': isZoomed }" :src="path" - :alt="path" @load="onImgLoad" - @click="onImgClick"/> - <p - v-if="renderInfo" - class="file-info prepend-top-10"> - <template v-if="fileSize>0"> - {{ fileSizeReadable }} - </template> - <template v-if="fileSize>0 && width && height"> - | - </template> - <template v-if="width && height"> - W: {{ width }} | H: {{ height }} - </template> - </p> + /> + <slot name="image-overlay"></slot> </div> + <p + v-if="renderInfo" + class="image-info" + > + <template v-if="hasFileSize"> + {{ fileSizeReadable }} + </template> + <template v-if="hasFileSize && hasDimensions"> + | + </template> + <template v-if="hasDimensions"> + <strong>W</strong>: {{ width }} | <strong>H</strong>: {{ height }} + </template> + </p> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue index cfc5343217c..9c3f3e7f7a9 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue @@ -69,6 +69,13 @@ export default { :new-path="fullNewPath" :old-path="fullOldPath" :project-path="projectPath" - /> + > + <slot + slot="image-overlay" + name="image-overlay" + > + </slot> + </component> + <slot></slot> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue index 38e881d17a2..cd0c1e850af 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/onion_skin_viewer.vue @@ -15,11 +15,6 @@ export default { type: String, required: true, }, - projectPath: { - type: String, - required: false, - default: '', - }, }, data() { return { @@ -120,7 +115,6 @@ export default { key="onionOldImg" :render-info="false" :path="oldPath" - :project-path="projectPath" @imgLoaded="onionOldImgLoaded" /> </div> @@ -136,9 +130,14 @@ export default { key="onionNewImg" :render-info="false" :path="newPath" - :project-path="projectPath" @imgLoaded="onionNewImgLoaded" - /> + > + <slot + slot="image-overlay" + name="image-overlay" + > + </slot> + </image-viewer> </div> <div class="controls"> <div class="transparent"></div> diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue index 86366c799a2..c3cfe54eb4d 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue @@ -16,11 +16,6 @@ export default { type: String, required: true, }, - projectPath: { - type: String, - required: false, - default: '', - }, }, data() { return { @@ -117,16 +112,14 @@ export default { 'height': swipeMaxPixelHeight, }" class="swipe-frame"> - <div class="frame deleted"> - <image-viewer - key="swipeOldImg" - ref="swipeOldImg" - :render-info="false" - :path="oldPath" - :project-path="projectPath" - @imgLoaded="swipeOldImgLoaded" - /> - </div> + <image-viewer + key="swipeOldImg" + ref="swipeOldImg" + :render-info="false" + :path="oldPath" + class="frame deleted" + @imgLoaded="swipeOldImgLoaded" + /> <div ref="swipeWrap" :style="{ @@ -134,15 +127,19 @@ export default { 'height': swipeMaxPixelHeight, }" class="swipe-wrap"> - <div class="frame added"> - <image-viewer - key="swipeNewImg" - :render-info="false" - :path="newPath" - :project-path="projectPath" - @imgLoaded="swipeNewImgLoaded" - /> - </div> + <image-viewer + key="swipeNewImg" + :render-info="false" + :path="newPath" + class="frame added" + @imgLoaded="swipeNewImgLoaded" + > + <slot + slot="image-overlay" + name="image-overlay" + > + </slot> + </image-viewer> </div> <span ref="swipeBar" diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue index 9c19266ecdf..9806d65e940 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/two_up_viewer.vue @@ -14,28 +14,29 @@ export default { type: String, required: true, }, - projectPath: { - type: String, - required: false, - default: '', - }, }, }; </script> <template> - <div class="two-up view row"> - <div class="col-sm-6 frame deleted"> - <image-viewer - :path="oldPath" - :project-path="projectPath" - /> - </div> - <div class="col-sm-6 frame added"> - <image-viewer - :path="newPath" - :project-path="projectPath" - /> - </div> + <div class="two-up view"> + <image-viewer + :path="oldPath" + :render-info="true" + inner-css-classes="frame deleted" + class="wrap" + /> + <image-viewer + :path="newPath" + :render-info="true" + :inner-css-classes="['frame', 'added']" + class="wrap" + > + <slot + slot="image-overlay" + name="image-overlay" + > + </slot> + </image-viewer> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue index 1af85283277..e68a2aa73fa 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer.vue @@ -8,9 +8,6 @@ import { diffModes, imageViewMode } from '../constants'; export default { components: { ImageViewer, - TwoUpViewer, - SwipeViewer, - OnionSkinViewer, }, props: { diffMode: { @@ -25,17 +22,32 @@ export default { type: String, required: true, }, - projectPath: { - type: String, - required: false, - default: '', - }, }, data() { return { mode: imageViewMode.twoup, }; }, + computed: { + imageViewComponent() { + switch (this.mode) { + case imageViewMode.twoup: + return TwoUpViewer; + case imageViewMode.swipe: + return SwipeViewer; + case imageViewMode.onion: + return OnionSkinViewer; + default: + return undefined; + } + }, + isNew() { + return this.diffMode === diffModes.new; + }, + imagePath() { + return this.isNew ? this.newPath : this.oldPath; + }, + }, methods: { changeMode(newMode) { this.mode = newMode; @@ -52,15 +64,16 @@ export default { v-if="diffMode === $options.diffModes.replaced" class="diff-viewer"> <div class="image js-replaced-image"> - <two-up-viewer - v-if="mode === $options.imageViewMode.twoup" - v-bind="$props"/> - <swipe-viewer - v-else-if="mode === $options.imageViewMode.swipe" - v-bind="$props"/> - <onion-skin-viewer - v-else-if="mode === $options.imageViewMode.onion" - v-bind="$props"/> + <component + :is="imageViewComponent" + v-bind="$props" + > + <slot + slot="image-overlay" + name="image-overlay" + > + </slot> + </component> </div> <div class="view-modes"> <ul class="view-modes-menu"> @@ -87,23 +100,27 @@ export default { </li> </ul> </div> - <div class="note-container"></div> - </div> - <div - v-else-if="diffMode === $options.diffModes.new" - class="diff-viewer added"> - <image-viewer - :path="newPath" - :project-path="projectPath" - /> </div> <div v-else - class="diff-viewer deleted"> - <image-viewer - :path="oldPath" - :project-path="projectPath" - /> + class="diff-viewer" + > + <div class="image"> + <image-viewer + :path="imagePath" + :inner-css-classes="['frame', { + 'added': isNew, + 'deleted': diffMode === $options.diffModes.deleted + }]" + > + <slot + v-if="isNew" + slot="image-overlay" + name="image-overlay" + > + </slot> + </image-viewer> + </div> </div> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/dropdown/dropdown_button.vue b/app/assets/javascripts/vue_shared/components/dropdown/dropdown_button.vue index 31087017968..0e194eaaed5 100644 --- a/app/assets/javascripts/vue_shared/components/dropdown/dropdown_button.vue +++ b/app/assets/javascripts/vue_shared/components/dropdown/dropdown_button.vue @@ -1,7 +1,11 @@ <script> import { __ } from '~/locale'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; export default { + components: { + GlLoadingIcon, + }, props: { isDisabled: { type: Boolean, diff --git a/app/assets/javascripts/vue_shared/components/file_icon.vue b/app/assets/javascripts/vue_shared/components/file_icon.vue index 408f7d7965f..03818be6a69 100644 --- a/app/assets/javascripts/vue_shared/components/file_icon.vue +++ b/app/assets/javascripts/vue_shared/components/file_icon.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import getIconForFile from './file_icon/file_icon_map'; import icon from '../../vue_shared/components/icon.vue'; @@ -17,6 +18,7 @@ import icon from '../../vue_shared/components/icon.vue'; export default { components: { icon, + GlLoadingIcon, }, props: { fileName: { diff --git a/app/assets/javascripts/vue_shared/components/loading_button.vue b/app/assets/javascripts/vue_shared/components/loading_button.vue index f9b7fd5b1f9..69d7e5c46f5 100644 --- a/app/assets/javascripts/vue_shared/components/loading_button.vue +++ b/app/assets/javascripts/vue_shared/components/loading_button.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; /* eslint-disable vue/require-default-prop */ /* This is a re-usable vue component for rendering a button that will probably be sending off ajax requests and need @@ -18,6 +19,9 @@ */ export default { + components: { + GlLoadingIcon, + }, props: { loading: { type: Boolean, diff --git a/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue b/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue index 500586302cf..5b12bb6b59e 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/date_picker.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import datePicker from '../pikaday.vue'; import toggleSidebar from './toggle_sidebar.vue'; import collapsedCalendarIcon from './collapsed_calendar_icon.vue'; @@ -10,6 +11,7 @@ export default { datePicker, toggleSidebar, collapsedCalendarIcon, + GlLoadingIcon, }, props: { blockClass: { diff --git a/app/assets/javascripts/vue_shared/components/sidebar/labels_select/base.vue b/app/assets/javascripts/vue_shared/components/sidebar/labels_select/base.vue index 3df286de129..e50d612ce36 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/labels_select/base.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/labels_select/base.vue @@ -4,6 +4,7 @@ import { __ } from '~/locale'; import LabelsSelect from '~/labels_select'; import DropdownHiddenInput from '~/vue_shared/components/dropdown/dropdown_hidden_input.vue'; +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import DropdownTitle from './dropdown_title.vue'; import DropdownValue from './dropdown_value.vue'; import DropdownValueCollapsed from './dropdown_value_collapsed.vue'; @@ -24,6 +25,7 @@ export default { DropdownSearchInput, DropdownFooter, DropdownCreateLabel, + GlLoadingIcon, }, props: { showCreate: { diff --git a/app/assets/javascripts/vue_shared/components/toggle_button.vue b/app/assets/javascripts/vue_shared/components/toggle_button.vue index 4e9289cbed8..e7cb5cfac12 100644 --- a/app/assets/javascripts/vue_shared/components/toggle_button.vue +++ b/app/assets/javascripts/vue_shared/components/toggle_button.vue @@ -1,4 +1,5 @@ <script> +import { GlLoadingIcon } from '@gitlab-org/gitlab-ui'; import { s__ } from '../../locale'; import icon from './icon.vue'; @@ -10,6 +11,7 @@ const LABEL_OFF = s__('ToggleButton|Toggle Status: OFF'); export default { components: { icon, + GlLoadingIcon, }, model: { diff --git a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue index 86c7498a092..dd6f96e2609 100644 --- a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue +++ b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue @@ -99,6 +99,6 @@ export default { v-tooltip :title="tooltipText" :tooltip-placement="tooltipPlacement" - >{{ username }}</span> + >{{ username }}</span><slot name="avatar-badge"></slot> </gl-link> </template> diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 52c91266ff4..19bc4262e21 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -421,21 +421,13 @@ .diff-file-container { .frame.deleted { - border: 0; + border: 1px solid $deleted; background-color: inherit; - - .image_file img { - border: 1px solid $deleted; - } } .frame.added { - border: 0; + border: 1px solid $added; background-color: inherit; - - .image_file img { - border: 1px solid $added; - } } .swipe.view, @@ -481,6 +473,11 @@ bottom: -25px; } } + + .discussion-notes .discussion-notes { + margin-left: 0; + border-left: 0; + } } .file-content .diff-file { @@ -804,7 +801,7 @@ // double jagged line divider .discussion-notes + .discussion-notes::before, - .discussion-notes + .discussion-form::before { + .diff-file-discussions + .discussion-form::before { content: ''; position: relative; display: block; @@ -844,6 +841,13 @@ background-repeat: repeat; } + .diff-file-discussions + .discussion-form::before { + width: auto; + margin-left: -16px; + margin-right: -16px; + margin-bottom: 16px; + } + .notes { position: relative; } @@ -870,11 +874,13 @@ } } -.files:not([data-can-create-note]) .frame { +.files:not([data-can-create-note="true"]) .frame { cursor: auto; } -.frame.click-to-comment { +.frame, +.frame.click-to-comment, +.btn-transparent.image-diff-overlay-add-comment { position: relative; cursor: image-url('illustrations/image_comment_light_cursor.svg') $image-comment-cursor-left-offset $image-comment-cursor-top-offset, @@ -910,6 +916,7 @@ .frame .badge.badge-pill, .image-diff-avatar-link .badge.badge-pill, +.user-avatar-link .badge.badge-pill, .notes > .badge.badge-pill { position: absolute; background-color: $blue-400; @@ -944,7 +951,8 @@ } } -.image-diff-avatar-link { +.image-diff-avatar-link, +.user-avatar-link { position: relative; .badge.badge-pill, @@ -1073,3 +1081,14 @@ top: 0; } } + +.image-diff-overlay, +.image-diff-overlay-add-comment { + top: 0; + left: 0; + + &:active, + &:focus { + outline: 0; + } +} diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index c02ec407262..2a6fe3b9c97 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -122,7 +122,7 @@ class Projects::BlobController < Projects::ApplicationController @lines.map! do |line| # These are marked as context lines but are loaded from blobs. # We also have context lines loaded from diffs in other places. - diff_line = Gitlab::Diff::Line.new(line, 'context', nil, nil, nil) + diff_line = Gitlab::Diff::Line.new(line, nil, nil, nil, nil) diff_line.rich_text = line diff_line end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 5307cd0720a..740f41c0642 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -22,6 +22,12 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def render_diffs @environment = @merge_request.environments_for(current_user).last + notes_grouped_by_path = @notes.group_by { |note| note.position.file_path } + + @diffs.diff_files.each do |diff_file| + notes = notes_grouped_by_path.fetch(diff_file.file_path, []) + notes.each { |note| diff_file.unfold_diff_lines(note.position) } + end @diffs.write_cache diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index f90971bb9f6..d3774746cb8 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -1,138 +1,149 @@ # frozen_string_literal: true -# Snippets Finder +# Finder for retrieving snippets that a user can see, optionally scoped to a +# project or snippets author. # -# Used to filter Snippets collections by a set of params +# Basic usage: # -# Arguments. +# user = User.find(1) # -# current_user - The current user, nil also can be used. -# params: -# visibility (integer) - Individual snippet visibility: Public(20), internal(10) or private(0). -# project (Project) - Project related. -# author (User) - Author related. +# SnippetsFinder.new(user).execute # -# params are optional +# To limit the snippets to a specific project, supply the `project:` option: +# +# user = User.find(1) +# project = Project.find(1) +# +# SnippetsFinder.new(user, project: project).execute +# +# Limiting snippets to an author can be done by supplying the `author:` option: +# +# user = User.find(1) +# project = Project.find(1) +# +# SnippetsFinder.new(user, author: user).execute +# +# To filter snippets using a specific visibility level, you can provide the +# `scope:` option: +# +# user = User.find(1) +# project = Project.find(1) +# +# SnippetsFinder.new(user, author: user, scope: :are_public).execute +# +# Valid `scope:` values are: +# +# * `:are_private` +# * `:are_internal` +# * `:are_public` +# +# Any other value will be ignored. class SnippetsFinder < UnionFinder - include Gitlab::Allowable include FinderMethods - attr_accessor :current_user, :project, :params + attr_accessor :current_user, :project, :author, :scope - def initialize(current_user, params = {}) + def initialize(current_user = nil, params = {}) @current_user = current_user - @params = params @project = params[:project] - end - - def execute - items = init_collection - items = by_author(items) - items = by_visibility(items) - - items.fresh - end - - private - - def init_collection - if project.present? - authorized_snippets_from_project - else - authorized_snippets + @author = params[:author] + @scope = params[:scope].to_s + + if project && author + raise( + ArgumentError, + 'Filtering by both an author and a project is not supported, ' \ + 'as this finder is not optimised for this use case' + ) end end - def authorized_snippets_from_project - if can?(current_user, :read_project_snippet, project) - if project.team.member?(current_user) - project.snippets + def execute + base = + if project + snippets_for_a_single_project else - project.snippets.public_to_user(current_user) + snippets_for_multiple_projects end - else - Snippet.none - end - end - # rubocop: disable CodeReuse/ActiveRecord - def authorized_snippets - # This query was intentionally converted to a raw one to get it work in Rails 5.0. - # In Rails 5.0 and 5.1 there's a bug: https://github.com/rails/arel/issues/531 - # Please convert it back when on rails 5.2 as it works again as expected since 5.2. - Snippet.where("#{feature_available_projects} OR #{not_project_related}") - .public_or_visible_to_user(current_user) + base.with_optional_visibility(visibility_from_scope).fresh end - # rubocop: enable CodeReuse/ActiveRecord - # Returns a collection of projects that is either public or visible to the - # logged in user. + # Produces a query that retrieves snippets from multiple projects. # - # A caller must pass in a block to modify individual parts of - # the query, e.g. to apply .with_feature_available_for_user on top of it. - # This is useful for performance as we can stick those additional filters - # at the bottom of e.g. the UNION. - # rubocop: disable CodeReuse/ActiveRecord - def projects_for_user - return yield(Project.public_to_user) unless current_user - - # If the current_user is allowed to see all projects, - # we can shortcut and just return. - return yield(Project.all) if current_user.full_private_access? - - authorized_projects = yield(Project.where('EXISTS (?)', current_user.authorizations_for_projects)) - - levels = Gitlab::VisibilityLevel.levels_for_user(current_user) - visible_projects = yield(Project.where(visibility_level: levels)) - - # We use a UNION here instead of OR clauses since this results in better - # performance. - Project.from_union([authorized_projects, visible_projects]) - end - # rubocop: enable CodeReuse/ActiveRecord - - def feature_available_projects - # Don't return any project related snippets if the user cannot read cross project - return table[:id].eq(nil).to_sql unless Ability.allowed?(current_user, :read_cross_project) - - projects = projects_for_user do |part| - part.with_feature_available_for_user(:snippets, current_user) - end.select(:id) + # The resulting query will, depending on the user's permissions, include the + # following collections of snippets: + # + # 1. Snippets that don't belong to any project. + # 2. Snippets of projects that are visible to the current user (e.g. snippets + # in public projects). + # 3. Snippets of projects that the current user is a member of. + # + # Each collection is constructed in isolation, allowing for greater control + # over the resulting SQL query. + def snippets_for_multiple_projects + queries = [global_snippets] + + if Ability.allowed?(current_user, :read_cross_project) + queries << snippets_of_visible_projects + queries << snippets_of_authorized_projects if current_user + end - # This query was intentionally converted to a raw one to get it work in Rails 5.0. - # In Rails 5.0 and 5.1 there's a bug: https://github.com/rails/arel/issues/531 - # Please convert it back when on rails 5.2 as it works again as expected since 5.2. - "snippets.project_id IN (#{projects.to_sql})" + find_union(queries, Snippet) end - def not_project_related - table[:project_id].eq(nil).to_sql + def snippets_for_a_single_project + Snippet.for_project_with_user(project, current_user) end - def table - Snippet.arel_table + def global_snippets + snippets_for_author_or_visible_to_user.only_global_snippets end - # rubocop: disable CodeReuse/ActiveRecord - def by_visibility(items) - visibility = params[:visibility] || visibility_from_scope + # Returns the snippets that the current user (logged in or not) can view. + def snippets_of_visible_projects + snippets_for_author_or_visible_to_user + .only_include_projects_visible_to(current_user) + .only_include_projects_with_snippets_enabled + end - return items unless visibility + # Returns the snippets that the currently logged in user has access to by + # being a member of the project the snippets belong to. + # + # This method requires that `current_user` returns a `User` instead of `nil`, + # and is optimised for this specific scenario. + def snippets_of_authorized_projects + base = author ? snippets_for_author : Snippet.all + + base + .only_include_projects_with_snippets_enabled(include_private: true) + .only_include_authorized_projects(current_user) + end - items.where(visibility_level: visibility) + def snippets_for_author_or_visible_to_user + if author + snippets_for_author + elsif current_user + Snippet.visible_to_or_authored_by(current_user) + else + Snippet.public_to_user + end end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord - def by_author(items) - return items unless params[:author] + def snippets_for_author + base = author.snippets - items.where(author_id: params[:author].id) + if author == current_user + # If the current user is also the author of all snippets, then we can + # include private snippets. + base + else + base.public_to_user(current_user) + end end - # rubocop: enable CodeReuse/ActiveRecord def visibility_from_scope - case params[:scope].to_s + case scope when 'are_private' Snippet::PRIVATE when 'are_internal' diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 360c9924a7d..25596581d0f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -815,7 +815,7 @@ module Ci end end - def predefined_variables + def predefined_variables # rubocop:disable Metrics/AbcSize Gitlab::Ci::Variables::Collection.new.tap do |variables| variables.append(key: 'CI', value: 'true') variables.append(key: 'GITLAB_CI', value: 'true') @@ -835,6 +835,8 @@ module Ci variables.append(key: "CI_COMMIT_TAG", value: ref) if tag? variables.append(key: "CI_PIPELINE_TRIGGERED", value: 'true') if trigger_request variables.append(key: "CI_JOB_MANUAL", value: 'true') if action? + variables.append(key: "CI_NODE_INDEX", value: self.options[:instance].to_s) if self.options&.include?(:instance) + variables.append(key: "CI_NODE_TOTAL", value: (self.options&.dig(:parallel) || 1).to_s) variables.concat(legacy_variables) end end diff --git a/app/models/compare.rb b/app/models/compare.rb index b2d46ada831..f1ed84ab5a5 100644 --- a/app/models/compare.rb +++ b/app/models/compare.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'set' + class Compare include Gitlab::Utils::StrongMemoize @@ -77,4 +79,13 @@ class Compare head_sha: head_commit_sha ) end + + def modified_paths + paths = Set.new + diffs.diff_files.each do |diff| + paths.add diff.old_path + paths.add diff.new_path + end + paths.to_a + end end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 5f59e4832db..c32008aa9c7 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -66,6 +66,10 @@ class DiffNote < Note self.original_position.diff_refs == diff_refs end + def discussion_first_note? + self == discussion.first_note + end + private def enqueue_diff_file_creation_job @@ -78,26 +82,33 @@ class DiffNote < Note end def should_create_diff_file? - on_text? && note_diff_file.nil? && self == discussion.first_note + on_text? && note_diff_file.nil? && discussion_first_note? end def fetch_diff_file - if note_diff_file - diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) - Gitlab::Diff::File.new(diff, - repository: project.repository, - diff_refs: original_position.diff_refs) - elsif created_at_diff?(noteable.diff_refs) - # We're able to use the already persisted diffs (Postgres) if we're - # presenting a "current version" of the MR discussion diff. - # So no need to make an extra Gitaly diff request for it. - # As an extra benefit, the returned `diff_file` already - # has `highlighted_diff_lines` data set from Redis on - # `Diff::FileCollection::MergeRequestDiff`. - noteable.diffs(original_position.diff_options).diff_files.first - else - original_position.diff_file(self.project.repository) - end + file = + if note_diff_file + diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) + Gitlab::Diff::File.new(diff, + repository: project.repository, + diff_refs: original_position.diff_refs) + elsif created_at_diff?(noteable.diff_refs) + # We're able to use the already persisted diffs (Postgres) if we're + # presenting a "current version" of the MR discussion diff. + # So no need to make an extra Gitaly diff request for it. + # As an extra benefit, the returned `diff_file` already + # has `highlighted_diff_lines` data set from Redis on + # `Diff::FileCollection::MergeRequestDiff`. + noteable.diffs(original_position.diff_options).diff_files.first + else + original_position.diff_file(self.project.repository) + end + + # Since persisted diff files already have its content "unfolded" + # there's no need to make it pass through the unfolding process. + file&.unfold_diff_lines(position) unless note_diff_file + + file end def supported? diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 735d9fba966..df5678ec2f1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -409,6 +409,18 @@ class MergeRequest < ActiveRecord::Base merge_request_diff&.real_size || diffs.real_size end + def modified_paths(past_merge_request_diff: nil) + diffs = if past_merge_request_diff + past_merge_request_diff + elsif compare + compare + else + self.merge_request_diff + end + + diffs.modified_paths + end + def diff_base_commit if persisted? merge_request_diff.base_commit diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index bb6ff8921df..74583af1a29 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -6,6 +6,7 @@ class MergeRequestDiff < ActiveRecord::Base include ManualInverseAssociation include IgnorableColumn include EachBatch + include Gitlab::Utils::StrongMemoize # Don't display more than 100 commits at once COMMITS_SAFE_SIZE = 100 @@ -234,6 +235,12 @@ class MergeRequestDiff < ActiveRecord::Base end # rubocop: enable CodeReuse/ServiceClass + def modified_paths + strong_memoize(:modified_paths) do + merge_request_diff_files.pluck(:new_path, :old_path).flatten.uniq + end + end + private def create_merge_request_diff_files(diffs) diff --git a/app/models/project.rb b/app/models/project.rb index d5a4ae79c47..48905547ab4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2073,6 +2073,10 @@ class Project < ActiveRecord::Base storage_version != LATEST_STORAGE_VERSION end + def snippets_visible?(user = nil) + Ability.allowed?(user, :read_project_snippet, self) + end + private def use_hashed_storage diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 1c5846b4023..11856b55902 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -63,6 +63,62 @@ class Snippet < ActiveRecord::Base attr_spammable :title, spam_title: true attr_spammable :content, spam_description: true + def self.with_optional_visibility(value = nil) + if value + where(visibility_level: value) + else + all + end + end + + def self.only_global_snippets + where(project_id: nil) + end + + def self.only_include_projects_visible_to(current_user = nil) + levels = Gitlab::VisibilityLevel.levels_for_user(current_user) + + joins(:project).where('projects.visibility_level IN (?)', levels) + end + + def self.only_include_projects_with_snippets_enabled(include_private: false) + column = ProjectFeature.access_level_attribute(:snippets) + levels = [ProjectFeature::ENABLED, ProjectFeature::PUBLIC] + + levels << ProjectFeature::PRIVATE if include_private + + joins(project: :project_feature) + .where(project_features: { column => levels }) + end + + def self.only_include_authorized_projects(current_user) + where( + 'EXISTS (?)', + ProjectAuthorization + .select(1) + .where('project_id = snippets.project_id') + .where(user_id: current_user.id) + ) + end + + def self.for_project_with_user(project, user = nil) + return none unless project.snippets_visible?(user) + + if user && project.team.member?(user) + project.snippets + else + project.snippets.public_to_user(user) + end + end + + def self.visible_to_or_authored_by(user) + where( + 'snippets.visibility_level IN (?) OR snippets.author_id = ?', + Gitlab::VisibilityLevel.levels_for_user(user), + user.id + ) + end + def self.reference_prefix '$' end @@ -81,27 +137,6 @@ class Snippet < ActiveRecord::Base @link_reference_pattern ||= super("snippets", /(?<snippet>\d+)/) end - # Returns a collection of snippets that are either public or visible to the - # logged in user. - # - # This method does not verify the user actually has the access to the project - # the snippet is in, so it should be only used on a relation that's already scoped - # for project access - def self.public_or_visible_to_user(user = nil) - if user - authorized = user - .project_authorizations - .select(1) - .where('project_authorizations.project_id = snippets.project_id') - - levels = Gitlab::VisibilityLevel.levels_for_user(user) - - where('EXISTS (?) OR snippets.visibility_level IN (?) or snippets.author_id = (?)', authorized, levels, user.id) - else - public_to_user - end - end - def to_reference(from = nil, full: false) reference = "#{self.class.reference_prefix}#{id}" diff --git a/app/models/upload.rb b/app/models/upload.rb index 23bc9ca42fc..e01e9c6a4f0 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -11,7 +11,8 @@ class Upload < ActiveRecord::Base validates :model, presence: true validates :uploader, presence: true - scope :with_files_stored_locally, -> { where(store: [nil, ObjectStorage::Store::LOCAL]) } + scope :with_files_stored_locally, -> { where(store: ObjectStorage::Store::LOCAL) } + scope :with_files_stored_remotely, -> { where(store: ObjectStorage::Store::REMOTE) } before_save :calculate_checksum!, if: :foreground_checksummable? after_commit :schedule_checksum, if: :checksummable? @@ -46,7 +47,18 @@ class Upload < ActiveRecord::Base end def exist? - File.exist?(absolute_path) + exist = File.exist?(absolute_path) + + # Help sysadmins find missing upload files + if persisted? && !exist + if Gitlab::Sentry.enabled? + Raven.capture_message("Upload file does not exist", extra: self.attributes) + end + + Gitlab::Metrics.counter(:upload_file_does_not_exist_total, 'The number of times an upload record could not find its file').increment + end + + exist end def uploader_context @@ -57,8 +69,6 @@ class Upload < ActiveRecord::Base end def local? - return true if store.nil? - store == ObjectStorage::Store::LOCAL end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 53768ff2cbe..5fe48da1cd6 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -2,18 +2,18 @@ module MergeRequests class RefreshService < MergeRequests::BaseService + attr_reader :push + def execute(oldrev, newrev, ref) - push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref) - return true unless push.branch_push? + @push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref) + return true unless @push.branch_push? - refresh_merge_requests!(push) + refresh_merge_requests! end private - def refresh_merge_requests!(push) - @push = push - + def refresh_merge_requests! Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits)) # Be sure to close outstanding MRs before reloading them to avoid generating an # empty diff during a manual merge diff --git a/app/services/merge_requests/reload_diffs_service.rb b/app/services/merge_requests/reload_diffs_service.rb index b47d8f3f63a..c64b2e99b52 100644 --- a/app/services/merge_requests/reload_diffs_service.rb +++ b/app/services/merge_requests/reload_diffs_service.rb @@ -29,10 +29,6 @@ module MergeRequests # rubocop: disable CodeReuse/ActiveRecord def clear_cache(new_diff) - # Executing the iteration we cache highlighted diffs for each diff file of - # MergeRequestDiff. - cacheable_collection(new_diff).write_cache - # Remove cache for all diffs on this MR. Do not use the association on the # model, as that will interfere with other actions happening when # reloading the diff. diff --git a/app/services/notes/base_service.rb b/app/services/notes/base_service.rb new file mode 100644 index 00000000000..431ff6c11c4 --- /dev/null +++ b/app/services/notes/base_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Notes + class BaseService < ::BaseService + def clear_noteable_diffs_cache(note) + noteable = note.noteable + + if note.is_a?(DiffNote) && + note.discussion_first_note? && + note.position.unfolded_diff?(project.repository) + noteable.diffs.clear_cache + end + end + end +end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 049e6c5a871..e03789e3ca9 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Notes - class CreateService < ::BaseService + class CreateService < ::Notes::BaseService def execute merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha) @@ -35,6 +35,7 @@ module Notes if !only_commands && note.save todo_service.new_note(note, current_user) + clear_noteable_diffs_cache(note) end if command_params.present? diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index 64e9accd97f..fa0c2c5c86b 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true module Notes - class DestroyService < BaseService + class DestroyService < ::Notes::BaseService def execute(note) TodoService.new.destroy_target(note) do |note| note.destroy end + + clear_noteable_diffs_cache(note) end end end diff --git a/app/views/abuse_reports/new.html.haml b/app/views/abuse_reports/new.html.haml index 391115a67b5..84c3dfd8b91 100644 --- a/app/views/abuse_reports/new.html.haml +++ b/app/views/abuse_reports/new.html.haml @@ -1,6 +1,10 @@ -- page_title "Report abuse" -%h3.page-title Report abuse -%p Please use this form to report users who create spam issues, comments or behave inappropriately. +- page_title _("Report abuse to GitLab") +%h3.page-title + = _('Report abuse to GitLab') +%p + = _('Please use this form to report users to GitLab who create spam issues, comments or behave inappropriately.') +%p + = _("A member of GitLab's abuse team will review your report as soon as possible.") %hr = form_for @abuse_report, html: { class: 'js-quick-submit js-requires-input'} do |f| = form_errors(@abuse_report) @@ -16,7 +20,7 @@ .col-sm-10 = f.text_area :message, class: "form-control", rows: 2, required: true, value: sanitize(@ref_url) .form-text.text-muted - Explain the problem with this user. If appropriate, provide a link to the relevant issue or comment. + = _('Explain the problem. If appropriate, provide a link to the relevant issue or comment.') .form-actions = f.submit "Send report", class: "btn btn-success" diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 0904e44a658..51dcc9d0cda 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -35,7 +35,7 @@ %p = _('Who will be able to see this group?') = link_to _('View the documentation'), help_page_path("public_access/public_access"), target: '_blank' - = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group, with_label: false + = render 'shared/visibility_level', f: f, visibility_level: default_group_visibility, can_change_visibility_level: true, form_model: @group, with_label: false = render 'create_chat_team', f: f if Gitlab.config.mattermost.enabled diff --git a/app/views/projects/notes/_more_actions_dropdown.html.haml b/app/views/projects/notes/_more_actions_dropdown.html.haml index 88085c7185b..8de84f82e9f 100644 --- a/app/views/projects/notes/_more_actions_dropdown.html.haml +++ b/app/views/projects/notes/_more_actions_dropdown.html.haml @@ -11,8 +11,9 @@ - unless is_current_user %li = link_to new_abuse_report_path(user_id: note.author.id, ref_url: noteable_note_url(note)) do - Report as abuse + = _('Report abuse to GitLab') - if note_editable %li = link_to note_url(note), method: :delete, data: { confirm: 'Are you sure you want to delete this comment?' }, remote: true, class: 'js-note-delete' do - %span.text-danger Delete comment + %span.text-danger + = _('Delete comment') diff --git a/changelogs/unreleased/21480-parallel-job-keyword-mvc.yml b/changelogs/unreleased/21480-parallel-job-keyword-mvc.yml new file mode 100644 index 00000000000..7ac2410b18c --- /dev/null +++ b/changelogs/unreleased/21480-parallel-job-keyword-mvc.yml @@ -0,0 +1,5 @@ +--- +title: Implement parallel job keyword. +merge_request: 22631 +author: +type: added diff --git a/changelogs/unreleased/52771-ldap-users-can-t-choose-private-or-internal-when-creating-a-new-group.yml b/changelogs/unreleased/52771-ldap-users-can-t-choose-private-or-internal-when-creating-a-new-group.yml new file mode 100644 index 00000000000..a05ef75b6a6 --- /dev/null +++ b/changelogs/unreleased/52771-ldap-users-can-t-choose-private-or-internal-when-creating-a-new-group.yml @@ -0,0 +1,5 @@ +--- +title: Fix bug stopping non-admin users from changing visibility level on group creation +merge_request: 22468 +author: +type: fixed diff --git a/changelogs/unreleased/fj-bump-gitaly-0-129-0.yml b/changelogs/unreleased/fj-bump-gitaly-0-129-0.yml new file mode 100644 index 00000000000..9d44e46c0ed --- /dev/null +++ b/changelogs/unreleased/fj-bump-gitaly-0-129-0.yml @@ -0,0 +1,5 @@ +--- +title: Bump Gitaly to 0.129.0 +merge_request: 22868 +author: +type: added diff --git a/changelogs/unreleased/gl-ui-loading-icon.yml b/changelogs/unreleased/gl-ui-loading-icon.yml new file mode 100644 index 00000000000..5540fc7d7ea --- /dev/null +++ b/changelogs/unreleased/gl-ui-loading-icon.yml @@ -0,0 +1,5 @@ +--- +title: Remove gitlab-ui's loading icon from global +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/mr-image-commenting.yml b/changelogs/unreleased/mr-image-commenting.yml new file mode 100644 index 00000000000..3cc3becc795 --- /dev/null +++ b/changelogs/unreleased/mr-image-commenting.yml @@ -0,0 +1,5 @@ +--- +title: Reimplemented image commenting in merge request diffs +merge_request: +author: +type: added diff --git a/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml b/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml new file mode 100644 index 00000000000..7b48a94a993 --- /dev/null +++ b/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml @@ -0,0 +1,5 @@ +--- +title: Allow commenting on any diff line in Merge Requests +merge_request: 22398 +author: +type: added diff --git a/changelogs/unreleased/refactor-snippets-finder.yml b/changelogs/unreleased/refactor-snippets-finder.yml new file mode 100644 index 00000000000..37cacf71c14 --- /dev/null +++ b/changelogs/unreleased/refactor-snippets-finder.yml @@ -0,0 +1,5 @@ +--- +title: Rewrite SnippetsFinder to improve performance by a factor of 1500 +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/tc-index-uploads-file-store.yml b/changelogs/unreleased/tc-index-uploads-file-store.yml new file mode 100644 index 00000000000..fa3b3164e38 --- /dev/null +++ b/changelogs/unreleased/tc-index-uploads-file-store.yml @@ -0,0 +1,5 @@ +--- +title: Enhance performance of counting local Uploads +merge_request: 22522 +author: +type: performance diff --git a/changelogs/unreleased/winh-delayed-jobs-dynamic-timer.yml b/changelogs/unreleased/winh-delayed-jobs-dynamic-timer.yml new file mode 100644 index 00000000000..fbedd2796b2 --- /dev/null +++ b/changelogs/unreleased/winh-delayed-jobs-dynamic-timer.yml @@ -0,0 +1,5 @@ +--- +title: Add dynamic timer to delayed jobs +merge_request: 22382 +author: +type: changed diff --git a/db/migrate/20181005125926_add_index_to_uploads_store.rb b/db/migrate/20181005125926_add_index_to_uploads_store.rb new file mode 100644 index 00000000000..d32ca05e980 --- /dev/null +++ b/db/migrate/20181005125926_add_index_to_uploads_store.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddIndexToUploadsStore < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :uploads, :store + end + + def down + remove_concurrent_index :uploads, :store + end +end diff --git a/db/migrate/20181026143227_migrate_snippets_access_level_default_value.rb b/db/migrate/20181026143227_migrate_snippets_access_level_default_value.rb new file mode 100644 index 00000000000..ede0ee27b8a --- /dev/null +++ b/db/migrate/20181026143227_migrate_snippets_access_level_default_value.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateSnippetsAccessLevelDefaultValue < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + ENABLED = 20 + + disable_ddl_transaction! + + class ProjectFeature < ActiveRecord::Base + include EachBatch + + self.table_name = 'project_features' + end + + def up + change_column_default :project_features, :snippets_access_level, ENABLED + + # On GitLab.com this will update about 28 000 rows. Since our updates are + # very small and this column is not indexed, these updates should be very + # lightweight. + ProjectFeature.where(snippets_access_level: nil).each_batch do |batch| + batch.update_all(snippets_access_level: ENABLED) + end + + # We do not need to perform this in a post-deployment migration as the + # ProjectFeature model already enforces a default value for all new rows. + change_column_null :project_features, :snippets_access_level, false + end + + def down + change_column_null :project_features, :snippets_access_level, true + change_column_default :project_features, :snippets_access_level, nil + + # We can't migrate from 20 -> NULL, as some projects may have explicitly set + # the access level to 20. + end +end diff --git a/db/post_migrate/20181105201455_steal_fill_store_upload.rb b/db/post_migrate/20181105201455_steal_fill_store_upload.rb new file mode 100644 index 00000000000..982001fedbe --- /dev/null +++ b/db/post_migrate/20181105201455_steal_fill_store_upload.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class StealFillStoreUpload < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 10_000 + + disable_ddl_transaction! + + class Upload < ActiveRecord::Base + include EachBatch + + self.table_name = 'uploads' + self.inheritance_column = :_type_disabled # Disable STI + end + + def up + Gitlab::BackgroundMigration.steal('FillStoreUpload') + + Upload.where(store: nil).each_batch(of: BATCH_SIZE) do |batch| + range = batch.pluck('MIN(id)', 'MAX(id)').first + + Gitlab::BackgroundMigration::FillStoreUpload.new.perform(*range) + end + end + + def down + # noop + end +end diff --git a/db/schema.rb b/db/schema.rb index 4695d923b79..765cbf0606a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1632,7 +1632,7 @@ ActiveRecord::Schema.define(version: 20181107054254) do t.integer "merge_requests_access_level" t.integer "issues_access_level" t.integer "wiki_access_level" - t.integer "snippets_access_level" + t.integer "snippets_access_level", default: 20, null: false t.integer "builds_access_level" t.datetime "created_at" t.datetime "updated_at" @@ -2156,6 +2156,7 @@ ActiveRecord::Schema.define(version: 20181107054254) do add_index "uploads", ["checksum"], name: "index_uploads_on_checksum", using: :btree add_index "uploads", ["model_id", "model_type"], name: "index_uploads_on_model_id_and_model_type", using: :btree + add_index "uploads", ["store"], name: "index_uploads_on_store", using: :btree add_index "uploads", ["uploader", "path"], name: "index_uploads_on_uploader_and_path", using: :btree create_table "user_agent_details", force: :cascade do |t| diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index c6fd7ef7360..5700f640e4c 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -45,6 +45,7 @@ The following metrics are available: | redis_ping_success | Gauge | 9.4 | Whether or not the last redis ping succeeded | | redis_ping_latency_seconds | Gauge | 9.4 | Round trip time of the redis ping | | user_session_logins_total | Counter | 9.4 | Counter of how many users have logged in | +| upload_file_does_not_exist | Counter | 10.7 | Number of times an upload record could not find its file | | failed_login_captcha_total | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login | | successful_login_captcha_total | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login | diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 2d23bf6d2fd..bdbcf8c9435 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -65,6 +65,8 @@ future GitLab releases.** | **CI_JOB_NAME** | 9.0 | 0.5 | The name of the job as defined in `.gitlab-ci.yml` | | **CI_JOB_STAGE** | 9.0 | 0.5 | The name of the stage as defined in `.gitlab-ci.yml` | | **CI_JOB_TOKEN** | 9.0 | 1.2 | Token used for authenticating with the [GitLab Container Registry][registry] and downloading [dependent repositories][dependent-repositories] | +| **CI_NODE_INDEX** | 11.5 | all | Index of the job in the job set. If the job is not parallelized, this variable is not set. | +| **CI_NODE_TOTAL** | 11.5 | all | Total number of instances of this job running in parallel. If the job is not parallelized, this variable is set to `1`. | | **CI_JOB_URL** | 11.1 | 0.5 | Job details URL | | **CI_REPOSITORY_URL** | 9.0 | all | The URL to clone the Git repository | | **CI_RUNNER_DESCRIPTION** | 8.10 | 0.5 | The description of the runner as saved in GitLab | diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 981aa101dd3..b3a55e48f4e 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -75,6 +75,7 @@ A job is defined by a list of parameters that define the job behavior. | environment | no | Defines a name of environment to which deployment is done by this job | | coverage | no | Define code coverage settings for a given job | | retry | no | Define how many times a job can be auto-retried in case of a failure | +| parallel | no | Defines how many instances of a job should be run in parallel | ### `extends` @@ -1451,6 +1452,26 @@ test: retry: 2 ``` +## `parallel` + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22631) in GitLab 11.5. + +`parallel` allows you to configure how many instances of a job to run in +parallel. This value has to be greater than or equal to two (2). + +This creates N instances of the same job that run in parallel. They're named +sequentially from `job_name 1/N` to `job_name N/N`. + +For every job, `CI_NODE_INDEX` and `CI_NODE_TOTAL` [environment variables](../variables/README.html#predefined-variables-environment-variables) are set. + +A simple example: + +```yaml +test: + script: rspec + parallel: 5 +``` + ## `include` > Introduced in [GitLab Edition Premium][ee] 10.5. @@ -2034,4 +2055,4 @@ CI with various languages. [schedules]: ../../user/project/pipelines/schedules.md [variables-expressions]: ../variables/README.md#variables-expressions [ee]: https://about.gitlab.com/gitlab-ee/ -[gitlab-versions]: https://about.gitlab.com/products/
\ No newline at end of file +[gitlab-versions]: https://about.gitlab.com/products/ diff --git a/doc/install/openshift_and_gitlab/index.md b/doc/install/openshift_and_gitlab/index.md index 5c8a830ac8f..4c88b6f97fc 100644 --- a/doc/install/openshift_and_gitlab/index.md +++ b/doc/install/openshift_and_gitlab/index.md @@ -505,7 +505,7 @@ PaaS and managing your applications with the ease of containers. [RedHat]: https://www.redhat.com/en "RedHat website" [openshift]: https://www.openshift.org "OpenShift Origin website" [vm]: https://www.openshift.org/vm/ "OpenShift All-in-one VM" -[vm-new]: https://atlas.hashicorp.com/openshift/boxes/origin-all-in-one "Official OpenShift Vagrant box on Atlas" +[vm-new]: https://app.vagrantup.com/openshift/boxes/origin-all-in-one "Official OpenShift Vagrant box on Vagrant Cloud" [template]: https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/docker/openshift-template.json "OpenShift template for GitLab" [openshift.com]: https://openshift.com "OpenShift Online" [kubernetes]: http://kubernetes.io/ "Kubernetes website" diff --git a/doc/university/README.md b/doc/university/README.md index f19b1ffd3d9..3e7d02770e4 100644 --- a/doc/university/README.md +++ b/doc/university/README.md @@ -104,7 +104,7 @@ The curriculum is composed of GitLab videos, screencasts, presentations, project 1. [Due Dates and Milestones for GitLab Issues](https://about.gitlab.com/2016/08/05/feature-highlight-set-dates-for-issues/) 1. [How to Use GitLab Labels](https://about.gitlab.com/2016/08/17/using-gitlab-labels/) 1. [Applying GitLab Labels Automatically](https://about.gitlab.com/2016/08/19/applying-gitlab-labels-automatically/) -1. [GitLab Issue Board - Product Page](https://about.gitlab.com/solutions/issueboard/) +1. [GitLab Issue Board - Product Page](https://about.gitlab.com/product/issueboard/) 1. [An Overview of GitLab Issue Board](https://about.gitlab.com/2016/08/22/announcing-the-gitlab-issue-board/) 1. [Designing GitLab Issue Board](https://about.gitlab.com/2016/08/31/designing-issue-boards/) 1. [From Idea to Production with GitLab - Video](https://www.youtube.com/watch?v=25pHyknRgEo&index=14&list=PLFGfElNsQthbQu_IWlNOxul0TbS_2JH-e) @@ -125,7 +125,7 @@ The curriculum is composed of GitLab videos, screencasts, presentations, project 1. [Setting up GitLab CI for iOS projects](https://about.gitlab.com/2016/03/10/setting-up-gitlab-ci-for-ios-projects/) 1. [IBM: Continuous Delivery vs Continuous Deployment - Video](https://www.youtube.com/watch?v=igwFj8PPSnw) 1. [Amazon: Transition to Continuous Delivery - Video](https://www.youtube.com/watch?v=esEFaY0FDKc) -2. [TechBeacon: Doing continuous delivery? Focus first on reducing release cycle times](https://techbeacon.com/doing-continuous-delivery-focus-first-reducing-release-cycle-times) +1. [TechBeacon: Doing continuous delivery? Focus first on reducing release cycle times](https://techbeacon.com/doing-continuous-delivery-focus-first-reducing-release-cycle-times) 1. See **[Integrations](#39-integrations)** for integrations with other CI services. #### 2.4. Workflow @@ -140,7 +140,7 @@ The curriculum is composed of GitLab videos, screencasts, presentations, project 1. [GitLab Compared to Other Tools](https://about.gitlab.com/comparison/) 1. [Comparing GitLab Terminology](https://about.gitlab.com/2016/01/27/comparing-terms-gitlab-github-bitbucket/) -1. [GitLab Compared to Atlassian (Recording 2016-03-03) ](https://youtu.be/Nbzp1t45ERo) +1. [GitLab Compared to Atlassian (Recording 2016-03-03)](https://youtu.be/Nbzp1t45ERo) 1. [GitLab Position FAQ](https://about.gitlab.com/handbook/positioning-faq) 1. [Customer review of GitLab with points on why they prefer GitLab](https://www.enovate.co.uk/web-design-blog/2015/11/25/gitlab-review/) @@ -189,7 +189,7 @@ The curriculum is composed of GitLab videos, screencasts, presentations, project #### 3.8 Cycle Analytics 1. [GitLab Cycle Analytics Overview](https://about.gitlab.com/2016/09/21/cycle-analytics-feature-highlight/) -1. [GitLab Cycle Analytics - Product Page](https://about.gitlab.com/solutions/cycle-analytics/) +1. [GitLab Cycle Analytics - Product Page](https://about.gitlab.com/product/cycle-analytics/) #### 3.9. Integrations @@ -213,7 +213,8 @@ The curriculum is composed of GitLab videos, screencasts, presentations, project ### 5. Resources for GitLab Team Members -*Some content can only be accessed by GitLab team members* +NOTE: **Note:** +Some content can only be accessed by GitLab team members 1. [Support Path](support/README.md) 1. [Sales Path (redirect to sales handbook)](https://about.gitlab.com/handbook/sales-onboarding/) diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index f1786c15f4f..1ae144ca9c1 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -14,7 +14,7 @@ module API end def public_snippets - SnippetsFinder.new(current_user, visibility: Snippet::PUBLIC).execute + SnippetsFinder.new(current_user, scope: :are_public).execute end end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index e4610faa327..362014b1a09 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -14,7 +14,7 @@ module Gitlab ALLOWED_KEYS = %i[tags script only except type image services allow_failure type stage when start_in artifacts cache dependencies before_script after_script variables - environment coverage retry extends].freeze + environment coverage retry parallel extends].freeze validations do validates :config, allowed_keys: ALLOWED_KEYS @@ -29,6 +29,8 @@ module Gitlab validates :retry, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 2 } + validates :parallel, numericality: { only_integer: true, + greater_than_or_equal_to: 2 } validates :when, inclusion: { in: %w[on_success on_failure always manual delayed], message: 'should be on_success, on_failure, ' \ @@ -79,17 +81,18 @@ module Gitlab description: 'Artifacts configuration for this job.' entry :environment, Entry::Environment, - description: 'Environment configuration for this job.' + description: 'Environment configuration for this job.' entry :coverage, Entry::Coverage, - description: 'Coverage configuration for this job.' + description: 'Coverage configuration for this job.' helpers :before_script, :script, :stage, :type, :after_script, :cache, :image, :services, :only, :except, :variables, - :artifacts, :commands, :environment, :coverage, :retry + :artifacts, :commands, :environment, :coverage, :retry, + :parallel attributes :script, :tags, :allow_failure, :when, :dependencies, - :retry, :extends, :start_in + :retry, :parallel, :extends, :start_in def compose!(deps = nil) super do @@ -158,6 +161,7 @@ module Gitlab environment_name: environment_defined? ? environment_value[:name] : nil, coverage: coverage_defined? ? coverage_value : nil, retry: retry_defined? ? retry_value.to_i : nil, + parallel: parallel_defined? ? parallel_value.to_i : nil, artifacts: artifacts_value, after_script: after_script_value, ignore: ignored? } diff --git a/lib/gitlab/ci/config/normalizer.rb b/lib/gitlab/ci/config/normalizer.rb new file mode 100644 index 00000000000..b7743bd2090 --- /dev/null +++ b/lib/gitlab/ci/config/normalizer.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + class Normalizer + def initialize(jobs_config) + @jobs_config = jobs_config + end + + def normalize_jobs + extract_parallelized_jobs! + return @jobs_config if @parallelized_jobs.empty? + + parallelized_config = parallelize_jobs + parallelize_dependencies(parallelized_config) + end + + private + + def extract_parallelized_jobs! + @parallelized_jobs = {} + + @jobs_config.each do |job_name, config| + if config[:parallel] + @parallelized_jobs[job_name] = self.class.parallelize_job_names(job_name, config[:parallel]) + end + end + + @parallelized_jobs + end + + def parallelize_jobs + @jobs_config.each_with_object({}) do |(job_name, config), hash| + if @parallelized_jobs.key?(job_name) + @parallelized_jobs[job_name].each { |name, index| hash[name.to_sym] = config.merge(name: name, instance: index) } + else + hash[job_name] = config + end + + hash + end + end + + def parallelize_dependencies(parallelized_config) + parallelized_job_names = @parallelized_jobs.keys.map(&:to_s) + parallelized_config.each_with_object({}) do |(job_name, config), hash| + if config[:dependencies] && (intersection = config[:dependencies] & parallelized_job_names).any? + deps = intersection.map { |dep| @parallelized_jobs[dep.to_sym].map(&:first) }.flatten + hash[job_name] = config.merge(dependencies: deps) + else + hash[job_name] = config + end + + hash + end + end + + def self.parallelize_job_names(name, total) + Array.new(total) { |index| ["#{name} #{index + 1}/#{total}", index + 1] } + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/scheduled.rb b/lib/gitlab/ci/status/build/scheduled.rb index f443dbee120..b3452eae189 100644 --- a/lib/gitlab/ci/status/build/scheduled.rb +++ b/lib/gitlab/ci/status/build/scheduled.rb @@ -9,7 +9,7 @@ module Gitlab { image: 'illustrations/illustrations_scheduled-job_countdown.svg', size: 'svg-394', - title: _("This is a delayed to run in ") + " #{execute_in}", + title: _("This is a delayed job to run in %{remainingTime}"), content: _("This job will automatically run after it's timer finishes. " \ "Often they are used for incremental roll-out deploys " \ "to production environments. When unscheduled it converts " \ @@ -18,21 +18,12 @@ module Gitlab end def status_tooltip - "delayed manual action (#{execute_in})" + "delayed manual action (%{remainingTime})" end def self.matches?(build, user) build.scheduled? && build.scheduled_at end - - private - - include TimeHelper - - def execute_in - remaining_seconds = [0, subject.scheduled_at - Time.now].max - duration_in_numbers(remaining_seconds) - end end end end diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 39a1b52e531..e6ec400e476 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -52,6 +52,8 @@ module Gitlab after_script: job[:after_script], environment: job[:environment], retry: job[:retry], + parallel: job[:parallel], + instance: job[:instance], start_in: job[:start_in] }.compact } end @@ -104,7 +106,7 @@ module Gitlab ## # Jobs # - @jobs = @ci_config.jobs + @jobs = Ci::Config::Normalizer.new(@ci_config.jobs).normalize_jobs @jobs.each do |name, job| # logical validation for job diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index fb117baca9e..84595f8afd7 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -26,6 +26,7 @@ module Gitlab @repository = repository @diff_refs = diff_refs @fallback_diff_refs = fallback_diff_refs + @unfolded = false # Ensure items are collected in the the batch new_blob_lazy @@ -135,6 +136,24 @@ module Gitlab Gitlab::Diff::Parser.new.parse(raw_diff.each_line, diff_file: self).to_a end + # Changes diff_lines according to the given position. That is, + # it checks whether the position requires blob lines into the diff + # in order to be presented. + def unfold_diff_lines(position) + return unless position + + unfolder = Gitlab::Diff::LinesUnfolder.new(self, position) + + if unfolder.unfold_required? + @diff_lines = unfolder.unfolded_diff_lines + @unfolded = true + end + end + + def unfolded? + @unfolded + end + def highlighted_diff_lines @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 5b67cd46c48..70063071ee7 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -3,9 +3,9 @@ module Gitlab class Line SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze - attr_reader :line_code, :type, :index, :old_pos, :new_pos + attr_reader :line_code, :type, :old_pos, :new_pos attr_writer :rich_text - attr_accessor :text + attr_accessor :text, :index def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil) @text, @type, @index = text, type, index @@ -19,7 +19,14 @@ module Gitlab end def self.init_from_hash(hash) - new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code], rich_text: hash[:rich_text]) + new(hash[:text], + hash[:type], + hash[:index], + hash[:old_pos], + hash[:new_pos], + parent_file: hash[:parent_file], + line_code: hash[:line_code], + rich_text: hash[:rich_text]) end def to_hash diff --git a/lib/gitlab/diff/lines_unfolder.rb b/lib/gitlab/diff/lines_unfolder.rb new file mode 100644 index 00000000000..9306b7e16a2 --- /dev/null +++ b/lib/gitlab/diff/lines_unfolder.rb @@ -0,0 +1,235 @@ +# frozen_string_literal: true + +# Given a position, calculates which Blob lines should be extracted, treated and +# injected in the current diff file lines in order to present a "unfolded" diff. +module Gitlab + module Diff + class LinesUnfolder + include Gitlab::Utils::StrongMemoize + + UNFOLD_CONTEXT_SIZE = 3 + + def initialize(diff_file, position) + @diff_file = diff_file + @blob = diff_file.old_blob + @position = position + @generate_top_match_line = true + @generate_bottom_match_line = true + + # These methods update `@generate_top_match_line` and + # `@generate_bottom_match_line`. + @from_blob_line = calculate_from_blob_line! + @to_blob_line = calculate_to_blob_line! + end + + # Returns merged diff lines with required blob lines with correct + # positions. + def unfolded_diff_lines + strong_memoize(:unfolded_diff_lines) do + next unless unfold_required? + + merged_diff_with_blob_lines + end + end + + # Returns the extracted lines from the old blob which should be merged + # with the current diff lines. + def blob_lines + strong_memoize(:blob_lines) do + # Blob lines, unlike diffs, doesn't start with an empty space for + # unchanged line, so the parsing and highlighting step can get fuzzy + # without the following change. + line_prefix = ' ' + blob_as_diff_lines = @blob.data.each_line.map { |line| "#{line_prefix}#{line}" } + + lines = Gitlab::Diff::Parser.new.parse(blob_as_diff_lines, diff_file: @diff_file).to_a + + from = from_blob_line - 1 + to = to_blob_line - 1 + + lines[from..to] + end + end + + def unfold_required? + strong_memoize(:unfold_required) do + next false unless @diff_file.text? + next false unless @position.unchanged? + next false if @diff_file.new_file? || @diff_file.deleted_file? + next false unless @position.old_line + # Invalid position (MR import scenario) + next false if @position.old_line > @blob.lines.size + next false if @diff_file.diff_lines.empty? + next false if @diff_file.line_for_position(@position) + next false unless unfold_line + + true + end + end + + private + + attr_reader :from_blob_line, :to_blob_line + + def merged_diff_with_blob_lines + lines = @diff_file.diff_lines + match_line = unfold_line + insert_index = bottom? ? -1 : match_line.index + + lines -= [match_line] unless bottom? + + lines.insert(insert_index, *blob_lines_with_matches) + + # The inserted blob lines have invalid indexes, so we need + # to reindex them. + reindex(lines) + + lines + end + + # Returns 'unchanged' blob lines with recalculated `old_pos` and + # `new_pos` and the recalculated new match line (needed if we for instance + # we unfolded once, but there are still folded lines). + def blob_lines_with_matches + old_pos = from_blob_line + new_pos = from_blob_line + offset + + new_blob_lines = [] + + new_blob_lines.push(top_blob_match_line) if top_blob_match_line + + blob_lines.each do |line| + new_blob_lines << Gitlab::Diff::Line.new(line.text, line.type, nil, old_pos, new_pos, + parent_file: @diff_file) + + old_pos += 1 + new_pos += 1 + end + + new_blob_lines.push(bottom_blob_match_line) if bottom_blob_match_line + + new_blob_lines + end + + def reindex(lines) + lines.each_with_index { |line, i| line.index = i } + end + + def top_blob_match_line + strong_memoize(:top_blob_match_line) do + next unless @generate_top_match_line + + old_pos = from_blob_line + new_pos = from_blob_line + offset + + build_match_line(old_pos, new_pos) + end + end + + def bottom_blob_match_line + strong_memoize(:bottom_blob_match_line) do + # The bottom line match addition is already handled on + # Diff::File#diff_lines_for_serializer + next if bottom? + next unless @generate_bottom_match_line + + position = line_after_unfold_position.old_pos + + old_pos = position + new_pos = position + offset + + build_match_line(old_pos, new_pos) + end + end + + def build_match_line(old_pos, new_pos) + blob_lines_length = blob_lines.length + old_line_ref = [old_pos, blob_lines_length].join(',') + new_line_ref = [new_pos, blob_lines_length].join(',') + new_match_line_str = "@@ -#{old_line_ref}+#{new_line_ref} @@" + + Gitlab::Diff::Line.new(new_match_line_str, 'match', nil, old_pos, new_pos) + end + + # Returns the first line position that should be extracted + # from `blob_lines`. + def calculate_from_blob_line! + return unless unfold_required? + + from = comment_position - UNFOLD_CONTEXT_SIZE + + # There's no line before the match if it's in the top-most + # position. + prev_line_number = line_before_unfold_position&.old_pos || 0 + + if from <= prev_line_number + 1 + @generate_top_match_line = false + from = prev_line_number + 1 + end + + from + end + + # Returns the last line position that should be extracted + # from `blob_lines`. + def calculate_to_blob_line! + return unless unfold_required? + + to = comment_position + UNFOLD_CONTEXT_SIZE + + return to if bottom? + + next_line_number = line_after_unfold_position.old_pos + + if to >= next_line_number - 1 + @generate_bottom_match_line = false + to = next_line_number - 1 + end + + to + end + + def offset + unfold_line.new_pos - unfold_line.old_pos + end + + def line_before_unfold_position + return unless index = unfold_line&.index + + @diff_file.diff_lines[index - 1] if index > 0 + end + + def line_after_unfold_position + return unless index = unfold_line&.index + + @diff_file.diff_lines[index + 1] if index >= 0 + end + + def bottom? + strong_memoize(:bottom) do + @position.old_line > last_line.old_pos + end + end + + # Returns the line which needed to be expanded in order to send a comment + # in `@position`. + def unfold_line + strong_memoize(:unfold_line) do + next last_line if bottom? + + @diff_file.diff_lines.find do |line| + line.old_pos > comment_position && line.type == 'match' + end + end + end + + def comment_position + @position.old_line + end + + def last_line + @diff_file.diff_lines.last + end + end + end +end diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb index f967494199e..7bfab2d808f 100644 --- a/lib/gitlab/diff/position.rb +++ b/lib/gitlab/diff/position.rb @@ -101,6 +101,10 @@ module Gitlab @diff_refs ||= DiffRefs.new(base_sha: base_sha, start_sha: start_sha, head_sha: head_sha) end + def unfolded_diff?(repository) + diff_file(repository)&.unfolded? + end + def diff_file(repository) return @diff_file if defined?(@diff_file) @@ -134,7 +138,13 @@ module Gitlab return unless diff_refs.complete? return unless comparison = diff_refs.compare_in(repository.project) - comparison.diffs(diff_options).diff_files.first + file = comparison.diffs(diff_options).diff_files.first + + # We need to unfold diff lines according to the position in order + # to correctly calculate the line code and trace position changes. + file&.unfold_diff_lines(self) + + file end def get_formatter_class(type) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7d15d6a11fd..99f3138501b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -264,6 +264,9 @@ msgstr "" msgid "A deleted user" msgstr "" +msgid "A member of GitLab's abuse team will review your report as soon as possible." +msgstr "" + msgid "A new branch will be created in your fork and a new merge request will be started." msgstr "" @@ -336,6 +339,9 @@ msgstr "" msgid "Add a table" msgstr "" +msgid "Add image comment" +msgstr "" + msgid "Add license" msgstr "" @@ -1718,12 +1724,18 @@ msgstr "" msgid "Collapse sidebar" msgstr "" +msgid "Comment" +msgstr "" + msgid "Comment & resolve discussion" msgstr "" msgid "Comment & unresolve discussion" msgstr "" +msgid "Comment form position" +msgstr "" + msgid "Comments" msgstr "" @@ -1971,6 +1983,9 @@ msgstr "" msgid "Copy file path to clipboard" msgstr "" +msgid "Copy link" +msgstr "" + msgid "Copy reference to clipboard" msgstr "" @@ -2193,6 +2208,9 @@ msgstr "" msgid "Delete Snippet" msgstr "" +msgid "Delete comment" +msgstr "" + msgid "Delete list" msgstr "" @@ -2729,6 +2747,9 @@ msgstr "" msgid "Expiration date" msgstr "" +msgid "Explain the problem. If appropriate, provide a link to the relevant issue or comment." +msgstr "" + msgid "Explore" msgstr "" @@ -4593,6 +4614,9 @@ msgstr "" msgid "Please try again" msgstr "" +msgid "Please use this form to report users to GitLab who create spam issues, comments or behave inappropriately." +msgstr "" + msgid "Please wait while we import the repository for you. Refresh at will." msgstr "" @@ -5171,6 +5195,9 @@ msgstr "" msgid "Reply to this email directly or %{view_it_on_gitlab}." msgstr "" +msgid "Report abuse to GitLab" +msgstr "" + msgid "Reporting" msgstr "" @@ -6200,7 +6227,7 @@ msgstr "" msgid "This is a confidential issue." msgstr "" -msgid "This is a delayed to run in " +msgid "This is a delayed job to run in %{remainingTime}" msgstr "" msgid "This is the author's first Merge Request to this project." diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 64b589a6d83..f58aa25cbdd 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -157,7 +157,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).first - expect(match_line['type']).to eq('context') + expect(match_line['type']).to be_nil end it 'adds bottom match line when "t"o is less than blob size' do @@ -177,7 +177,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).last - expect(match_line['type']).to eq('context') + expect(match_line['type']).to be_nil end end end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 3190f1ce9d4..ccd4fc4db3a 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Projects::MilestonesController do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:milestone) { create(:milestone, project: project) } let(:issue) { create(:issue, project: project, milestone: milestone) } diff --git a/spec/factories/merge_request_diff_files.rb b/spec/factories/merge_request_diff_files.rb new file mode 100644 index 00000000000..469a7a0ac8d --- /dev/null +++ b/spec/factories/merge_request_diff_files.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_request_diff_file do + association :merge_request_diff + + relative_order 0 + new_file true + renamed_file false + deleted_file false + too_large false + a_mode 0 + b_mode 100644 + new_path 'foo' + old_path 'foo' + diff '' + binary false + + trait :new_file do + relative_order 0 + new_file true + renamed_file false + deleted_file false + too_large false + a_mode 0 + b_mode 100644 + new_path 'foo' + old_path 'foo' + diff '' + binary false + end + + trait :renamed_file do + relative_order 662 + new_file false + renamed_file true + deleted_file false + too_large false + a_mode 100644 + b_mode 100644 + new_path 'bar' + old_path 'baz' + diff '' + binary false + end + end +end diff --git a/spec/factories/merge_request_diffs.rb b/spec/factories/merge_request_diffs.rb new file mode 100644 index 00000000000..e7b51189538 --- /dev/null +++ b/spec/factories/merge_request_diffs.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_request_diff do + association :merge_request + state :collected + commits_count 1 + + base_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } + head_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } + start_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } + end +end diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index 4d04b8043ec..d01fc04311a 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -1,8 +1,10 @@ require 'spec_helper' describe 'Group' do + let(:user) { create(:admin) } + before do - sign_in(create(:admin)) + sign_in(user) end matcher :have_namespace_error_message do @@ -16,6 +18,24 @@ describe 'Group' do visit new_group_path end + describe 'as a non-admin' do + let(:user) { create(:user) } + + it 'creates a group and persists visibility radio selection', :js do + stub_application_setting(default_group_visibility: :private) + + fill_in 'Group name', with: 'test-group' + find("input[name='group[visibility_level]'][value='#{Gitlab::VisibilityLevel::PUBLIC}']").click + click_button 'Create group' + + group = Group.find_by(name: 'test-group') + + expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC) + expect(current_path).to eq(group_path(group)) + expect(page).to have_selector '.visibility-icon .fa-globe' + end + end + describe 'with space in group path' do it 'renders new group form with validation errors' do fill_in 'Group URL', with: 'space group' diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index f0d38dc6a0c..d790bdc82ce 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -114,10 +114,9 @@ describe 'Merge request > User creates image diff notes', :js do create_image_diff_note end - # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 - xit 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do + it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes' do indicator = find('.js-image-badge', match: :first) - badge = find('.image-diff-avatar-link .badge', match: :first) + badge = find('.user-avatar-link .badge', match: :first) expect(indicator).to have_content('1') expect(badge).to have_content('1') @@ -157,8 +156,7 @@ describe 'Merge request > User creates image diff notes', :js do visit project_merge_request_path(project, merge_request) end - # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 - xit 'render diff indicators within the image frame' do + it 'render diff indicators within the image frame' do diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) wait_for_requests @@ -200,7 +198,6 @@ describe 'Merge request > User creates image diff notes', :js do def create_image_diff_note find('.js-add-image-diff-note-button', match: :first).click - page.all('.js-add-image-diff-note-button')[0].click find('.diff-content .note-textarea').native.send_keys('image diff test comment') click_button 'Comment' wait_for_requests diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index fa148715855..51b78d3e7d1 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -85,12 +85,13 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') } - it 'does not allow commenting on the left side' do - should_not_allow_commenting(line_holder, 'left') + it 'allows commenting on the left side' do + should_allow_commenting(line_holder, 'left') end - it 'does not allow commenting on the right side' do - should_not_allow_commenting(line_holder, 'right') + it 'allows commenting on the right side' do + # Automatically shifts comment box to left side. + should_allow_commenting(line_holder, 'right') end end end @@ -147,8 +148,8 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') } - it 'does not allow commenting' do - should_not_allow_commenting line_holder + it 'allows commenting' do + should_allow_commenting line_holder end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index cbb935abd53..a1323699969 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -595,7 +595,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do end it 'shows delayed job', :js do - expect(page).to have_content('This is a delayed to run in') + expect(page).to have_content('This is a delayed job to run in') expect(page).to have_content("This job will automatically run after it's timer finishes.") expect(page).to have_link('Unschedule job') end diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 1ae0bd988f2..dfeeb3040c6 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -4,16 +4,13 @@ describe SnippetsFinder do include Gitlab::Allowable using RSpec::Parameterized::TableSyntax - context 'filter by visibility' do - let!(:snippet1) { create(:personal_snippet, :private) } - let!(:snippet2) { create(:personal_snippet, :internal) } - let!(:snippet3) { create(:personal_snippet, :public) } + describe '#initialize' do + it 'raises ArgumentError when a project and author are given' do + user = build(:user) + project = build(:project) - it "returns public snippets when visibility is PUBLIC" do - snippets = described_class.new(nil, visibility: Snippet::PUBLIC).execute - - expect(snippets).to include(snippet3) - expect(snippets).not_to include(snippet1, snippet2) + expect { described_class.new(user, author: user, project: project) } + .to raise_error(ArgumentError) end end @@ -66,21 +63,21 @@ describe SnippetsFinder do end it "returns internal snippets" do - snippets = described_class.new(user, author: user, visibility: Snippet::INTERNAL).execute + snippets = described_class.new(user, author: user, scope: :are_internal).execute expect(snippets).to include(snippet2) expect(snippets).not_to include(snippet1, snippet3) end it "returns private snippets" do - snippets = described_class.new(user, author: user, visibility: Snippet::PRIVATE).execute + snippets = described_class.new(user, author: user, scope: :are_private).execute expect(snippets).to include(snippet1) expect(snippets).not_to include(snippet2, snippet3) end it "returns public snippets" do - snippets = described_class.new(user, author: user, visibility: Snippet::PUBLIC).execute + snippets = described_class.new(user, author: user, scope: :are_public).execute expect(snippets).to include(snippet3) expect(snippets).not_to include(snippet1, snippet2) @@ -98,6 +95,13 @@ describe SnippetsFinder do expect(snippets).to include(snippet3) expect(snippets).not_to include(snippet2, snippet1) end + + it 'returns all snippets for an admin' do + admin = create(:user, :admin) + snippets = described_class.new(admin, author: user).execute + + expect(snippets).to include(snippet1, snippet2, snippet3) + end end context 'filter by project' do @@ -126,21 +130,21 @@ describe SnippetsFinder do end it "returns public snippets for non project members" do - snippets = described_class.new(user, project: project1, visibility: Snippet::PUBLIC).execute + snippets = described_class.new(user, project: project1, scope: :are_public).execute expect(snippets).to include(@snippet3) expect(snippets).not_to include(@snippet1, @snippet2) end it "returns internal snippets for non project members" do - snippets = described_class.new(user, project: project1, visibility: Snippet::INTERNAL).execute + snippets = described_class.new(user, project: project1, scope: :are_internal).execute expect(snippets).to include(@snippet2) expect(snippets).not_to include(@snippet1, @snippet3) end it "does not return private snippets for non project members" do - snippets = described_class.new(user, project: project1, visibility: Snippet::PRIVATE).execute + snippets = described_class.new(user, project: project1, scope: :are_private).execute expect(snippets).not_to include(@snippet1, @snippet2, @snippet3) end @@ -156,10 +160,17 @@ describe SnippetsFinder do it "returns private snippets for project members" do project1.add_developer(user) - snippets = described_class.new(user, project: project1, visibility: Snippet::PRIVATE).execute + snippets = described_class.new(user, project: project1, scope: :are_private).execute expect(snippets).to include(@snippet1) end + + it 'returns all snippets for an admin' do + admin = create(:user, :admin) + snippets = described_class.new(admin, project: project1).execute + + expect(snippets).to include(@snippet1, @snippet2, @snippet3) + end end describe '#execute' do @@ -184,4 +195,6 @@ describe SnippetsFinder do end end end + + it_behaves_like 'snippet visibility' end diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js index 67f7b569f47..36bd042f3c4 100644 --- a/spec/javascripts/diffs/components/diff_content_spec.js +++ b/spec/javascripts/diffs/components/diff_content_spec.js @@ -1,15 +1,24 @@ import Vue from 'vue'; import DiffContentComponent from '~/diffs/components/diff_content.vue'; -import store from '~/mr_notes/stores'; +import { createStore } from '~/mr_notes/stores'; import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants'; +import '~/behaviors/markdown/render_gfm'; import diffFileMockData from '../mock_data/diff_file'; +import discussionsMockData from '../mock_data/diff_discussions'; describe('DiffContent', () => { const Component = Vue.extend(DiffContentComponent); let vm; beforeEach(() => { + const store = createStore(); + store.state.notes.noteableData = { + current_user: { + can_create_note: false, + }, + }; + vm = mountComponentWithStore(Component, { store, props: { @@ -46,21 +55,57 @@ describe('DiffContent', () => { }); describe('image diff', () => { - beforeEach(() => { + beforeEach(done => { vm.diffFile.newPath = GREEN_BOX_IMAGE_URL; vm.diffFile.newSha = 'DEF'; vm.diffFile.oldPath = RED_BOX_IMAGE_URL; vm.diffFile.oldSha = 'ABC'; vm.diffFile.viewPath = ''; + vm.diffFile.discussions = [{ ...discussionsMockData }]; + vm.$store.state.diffs.commentForms.push({ + fileHash: vm.diffFile.fileHash, + x: 10, + y: 20, + width: 100, + height: 200, + }); + + vm.$nextTick(done); }); - it('should have image diff view in place', done => { - vm.$nextTick(() => { - expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0); + it('should have image diff view in place', () => { + expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0); - expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1); + expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1); + }); - done(); + it('renders image diff overlay', () => { + expect(vm.$el.querySelector('.image-diff-overlay')).not.toBe(null); + }); + + it('renders diff file discussions', () => { + expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5); + }); + + describe('handleSaveNote', () => { + it('dispatches handleSaveNote', () => { + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.handleSaveNote('test'); + + expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/saveDiffDiscussion', { + note: 'test', + formData: { + noteableData: jasmine.anything(), + noteableType: jasmine.anything(), + diffFile: vm.diffFile, + positionType: 'image', + x: 10, + y: 20, + width: 100, + height: 200, + }, + }); }); }); }); diff --git a/spec/javascripts/diffs/components/diff_discussions_spec.js b/spec/javascripts/diffs/components/diff_discussions_spec.js index 270f363825f..0bc9da5ad0f 100644 --- a/spec/javascripts/diffs/components/diff_discussions_spec.js +++ b/spec/javascripts/diffs/components/diff_discussions_spec.js @@ -1,24 +1,90 @@ import Vue from 'vue'; import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; -import store from '~/mr_notes/stores'; +import { createStore } from '~/mr_notes/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import '~/behaviors/markdown/render_gfm'; import discussionsMockData from '../mock_data/diff_discussions'; describe('DiffDiscussions', () => { - let component; + let vm; const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; - beforeEach(() => { - component = createComponentWithStore(Vue.extend(DiffDiscussions), store, { + function createComponent(props = {}) { + const store = createStore(); + + vm = createComponentWithStore(Vue.extend(DiffDiscussions), store, { discussions: getDiscussionsMockData(), + ...props, }).$mount(); + } + + afterEach(() => { + vm.$destroy(); }); describe('template', () => { it('should have notes list', () => { - const { $el } = component; + createComponent(); + + expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5); + }); + }); + + describe('image commenting', () => { + it('renders collapsible discussion button', () => { + createComponent({ shouldCollapseDiscussions: true }); + + expect(vm.$el.querySelector('.js-diff-notes-toggle')).not.toBe(null); + expect(vm.$el.querySelector('.js-diff-notes-toggle svg')).not.toBe(null); + expect(vm.$el.querySelector('.js-diff-notes-toggle').classList).toContain( + 'diff-notes-collapse', + ); + }); + + it('dispatches toggleDiscussion when clicking collapse button', () => { + createComponent({ shouldCollapseDiscussions: true }); + + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.$el.querySelector('.js-diff-notes-toggle').click(); + + expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', { + discussionId: vm.discussions[0].id, + }); + }); + + it('renders expand button when discussion is collapsed', done => { + createComponent({ shouldCollapseDiscussions: true }); + + vm.discussions[0].expanded = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.js-diff-notes-toggle').textContent.trim()).toBe('1'); + expect(vm.$el.querySelector('.js-diff-notes-toggle').className).toContain( + 'btn-transparent badge badge-pill', + ); + + done(); + }); + }); + + it('hides discussion when collapsed', done => { + createComponent({ shouldCollapseDiscussions: true }); + + vm.discussions[0].expanded = false; + + vm.$nextTick(() => { + expect(vm.$el.querySelector('.note-discussion').style.display).toBe('none'); + + done(); + }); + }); + + it('renders badge on avatar', () => { + createComponent({ renderAvatarBadge: true, discussions: [{ ...discussionsMockData }] }); - expect($el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5); + expect(vm.$el.querySelector('.user-avatar-link .badge-pill')).not.toBe(null); + expect(vm.$el.querySelector('.user-avatar-link .badge-pill').textContent.trim()).toBe('1'); }); }); }); diff --git a/spec/javascripts/diffs/components/image_diff_overlay_spec.js b/spec/javascripts/diffs/components/image_diff_overlay_spec.js new file mode 100644 index 00000000000..d76ab745fe1 --- /dev/null +++ b/spec/javascripts/diffs/components/image_diff_overlay_spec.js @@ -0,0 +1,146 @@ +import Vue from 'vue'; +import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; +import { createStore } from '~/mr_notes/stores'; +import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; +import { imageDiffDiscussions } from '../mock_data/diff_discussions'; + +describe('Diffs image diff overlay component', () => { + const dimensions = { + width: 100, + height: 200, + }; + let Component; + let vm; + + function createComponent(props = {}, extendStore = () => {}) { + const store = createStore(); + + extendStore(store); + + vm = createComponentWithStore(Component, store, { + discussions: [...imageDiffDiscussions], + fileHash: 'ABC', + ...props, + }); + } + + beforeAll(() => { + Component = Vue.extend(ImageDiffOverlay); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders comment badges', () => { + createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(2); + }); + + it('renders index of discussion in badge', () => { + createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + expect(vm.$el.querySelectorAll('.js-image-badge')[0].textContent.trim()).toBe('1'); + expect(vm.$el.querySelectorAll('.js-image-badge')[1].textContent.trim()).toBe('2'); + }); + + it('renders icon when showCommentIcon is true', () => { + createComponent({ showCommentIcon: true }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + expect(vm.$el.querySelector('.js-image-badge svg')).not.toBe(null); + }); + + it('sets badge comment positions', () => { + createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.left).toBe('10px'); + expect(vm.$el.querySelectorAll('.js-image-badge')[0].style.top).toBe('10px'); + + expect(vm.$el.querySelectorAll('.js-image-badge')[1].style.left).toBe('5px'); + expect(vm.$el.querySelectorAll('.js-image-badge')[1].style.top).toBe('5px'); + }); + + it('renders single badge for discussion object', () => { + createComponent({ + discussions: { + ...imageDiffDiscussions[0], + }, + }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + expect(vm.$el.querySelectorAll('.js-image-badge').length).toBe(1); + }); + + it('dispatches openDiffFileCommentForm when clicking overlay', () => { + createComponent({ canComment: true }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.$el.querySelector('.js-add-image-diff-note-button').click(); + + expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/openDiffFileCommentForm', { + fileHash: 'ABC', + x: 0, + y: 0, + width: 100, + height: 200, + }); + }); + + describe('toggle discussion', () => { + it('disables buttons when shouldToggleDiscussion is false', () => { + createComponent({ shouldToggleDiscussion: false }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + expect(vm.$el.querySelector('.js-image-badge').hasAttribute('disabled')).toBe(true); + }); + + it('dispatches toggleDiscussion when clicking image badge', () => { + createComponent(); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.$el.querySelector('.js-image-badge').click(); + + expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', { discussionId: '1' }); + }); + }); + + describe('comment form', () => { + beforeEach(() => { + createComponent({}, store => { + store.state.diffs.commentForms.push({ + fileHash: 'ABC', + x: 20, + y: 10, + }); + }); + spyOn(vm, 'getImageDimensions').and.returnValue(dimensions); + vm.$mount(); + }); + + it('renders comment form badge', () => { + expect(vm.$el.querySelector('.comment-indicator')).not.toBe(null); + }); + + it('sets comment form badge position', () => { + expect(vm.$el.querySelector('.comment-indicator').style.left).toBe('20px'); + expect(vm.$el.querySelector('.comment-indicator').style.top).toBe('10px'); + }); + }); +}); diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js index 0ad214ea4a4..5ffe5a366ba 100644 --- a/spec/javascripts/diffs/mock_data/diff_discussions.js +++ b/spec/javascripts/diffs/mock_data/diff_discussions.js @@ -492,3 +492,24 @@ export default { image_diff_html: '<div class="image js-replaced-image" data="">\n<div class="two-up view">\n<div class="wrap">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n<div class="wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{"base_sha":"e63f41fe459e62e1228fcef60d7189127aeba95a","start_sha":"d9eaefe5a676b820c57ff18cf5b68316025f7962","head_sha":"c48ee0d1bf3b30453f5b32250ce03134beaa6d13","old_path":"CHANGELOG","new_path":"CHANGELOG","position_type":"text","old_line":null,"new_line":2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n</div>\n<div class="swipe view hide">\n<div class="swipe-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="swipe-wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{"base_sha":"e63f41fe459e62e1228fcef60d7189127aeba95a","start_sha":"d9eaefe5a676b820c57ff18cf5b68316025f7962","head_sha":"c48ee0d1bf3b30453f5b32250ce03134beaa6d13","old_path":"CHANGELOG","new_path":"CHANGELOG","position_type":"text","old_line":null,"new_line":2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n</div>\n<span class="swipe-bar">\n<span class="top-handle"></span>\n<span class="bottom-handle"></span>\n</span>\n</div>\n</div>\n<div class="onion-skin view hide">\n<div class="onion-skin-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{"base_sha":"e63f41fe459e62e1228fcef60d7189127aeba95a","start_sha":"d9eaefe5a676b820c57ff18cf5b68316025f7962","head_sha":"c48ee0d1bf3b30453f5b32250ce03134beaa6d13","old_path":"CHANGELOG","new_path":"CHANGELOG","position_type":"text","old_line":null,"new_line":2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<div class="controls">\n<div class="transparent"></div>\n<div class="drag-track">\n<div class="dragger" style="left: 0px;"></div>\n</div>\n<div class="opaque"></div>\n</div>\n</div>\n</div>\n</div>\n<div class="view-modes hide">\n<ul class="view-modes-menu">\n<li class="two-up" data-mode="two-up">2-up</li>\n<li class="swipe" data-mode="swipe">Swipe</li>\n<li class="onion-skin" data-mode="onion-skin">Onion skin</li>\n</ul>\n</div>\n', }; + +export const imageDiffDiscussions = [ + { + id: '1', + position: { + x: 10, + y: 10, + width: 100, + height: 200, + }, + }, + { + id: '2', + position: { + x: 5, + y: 5, + width: 100, + height: 200, + }, + }, +]; diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js index d7bc0dbe431..be194ab414f 100644 --- a/spec/javascripts/diffs/mock_data/diff_file.js +++ b/spec/javascripts/diffs/mock_data/diff_file.js @@ -237,4 +237,5 @@ export default { }, }, ], + discussions: [], }; diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index bb623953710..17d0f31bdd3 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -218,6 +218,7 @@ describe('DiffsStoreActions', () => { ], }; const singleDiscussion = { + id: '1', fileHash: 'ABC', line_code: 'ABC_1_1', }; @@ -230,6 +231,7 @@ describe('DiffsStoreActions', () => { { type: types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, payload: { + id: '1', fileHash: 'ABC', lineCode: 'ABC_1_1', }, diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index fed04cbaed8..8821cde76f4 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -98,7 +98,7 @@ describe('DiffsStoreMutations', () => { it('should call utils.addContextLines with proper params', () => { const options = { lineNumbers: { oldLineNumber: 1, newLineNumber: 2 }, - contextLines: [{ oldLine: 1 }], + contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }], fileHash: 'ff9200', params: { bottom: true, @@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => { parallelDiffLines: [], }; const state = { diffFiles: [diffFile] }; - const lines = [{ oldLine: 1 }]; + const lines = [{ oldLine: 1, newLine: 1 }]; const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile); const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine'); diff --git a/spec/javascripts/fixtures/jobs.rb b/spec/javascripts/fixtures/jobs.rb index 6d5c6d5334f..82d7a5e394e 100644 --- a/spec/javascripts/fixtures/jobs.rb +++ b/spec/javascripts/fixtures/jobs.rb @@ -5,16 +5,24 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do let(:admin) { create(:admin) } let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} - let(:project) { create(:project_empty_repo, namespace: namespace, path: 'builds-project') } - let(:pipeline) { create(:ci_empty_pipeline, project: project) } + let(:project) { create(:project, :repository, namespace: namespace, path: 'builds-project') } + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id) } let!(:build_with_artifacts) { create(:ci_build, :success, :artifacts, :trace_artifact, pipeline: pipeline, stage: 'test', artifacts_expire_at: Time.now + 18.months) } let!(:failed_build) { create(:ci_build, :failed, pipeline: pipeline, stage: 'build') } let!(:pending_build) { create(:ci_build, :pending, pipeline: pipeline, stage: 'deploy') } + let!(:delayed_job) do + create(:ci_build, :scheduled, + pipeline: pipeline, + name: 'delayed job', + stage: 'test', + commands: 'test') + end render_views before(:all) do clean_frontend_fixtures('builds/') + clean_frontend_fixtures('jobs/') end before do @@ -34,4 +42,15 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do expect(response).to be_success store_frontend_fixture(response, example.description) end + + it 'jobs/delayed.json' do |example| + get :show, + namespace_id: project.namespace.to_param, + project_id: project, + id: delayed_job.to_param, + format: :json + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end end diff --git a/spec/javascripts/jobs/components/job_app_spec.js b/spec/javascripts/jobs/components/job_app_spec.js index f8ca43fc150..98c995393b9 100644 --- a/spec/javascripts/jobs/components/job_app_spec.js +++ b/spec/javascripts/jobs/components/job_app_spec.js @@ -8,6 +8,7 @@ import { resetStore } from '../store/helpers'; import job from '../mock_data'; describe('Job App ', () => { + const delayedJobFixture = getJSONFixture('jobs/delayed.json'); const Component = Vue.extend(jobApp); let store; let vm; @@ -420,6 +421,36 @@ describe('Job App ', () => { done(); }, 0); }); + + it('displays remaining time for a delayed job', done => { + const oneHourInMilliseconds = 3600000; + spyOn(Date, 'now').and.callFake( + () => new Date(delayedJobFixture.scheduled_at).getTime() - oneHourInMilliseconds, + ); + mock.onGet(props.endpoint).replyOnce(200, { ...delayedJobFixture }); + + vm = mountComponentWithStore(Component, { + props, + store, + }); + + store.subscribeAction(action => { + if (action.type !== 'receiveJobSuccess') { + return; + } + + Vue.nextTick() + .then(() => { + expect(vm.$el.querySelector('.js-job-empty-state')).not.toBeNull(); + + const title = vm.$el.querySelector('.js-job-empty-state-title'); + + expect(title).toContainText('01:00:00'); + done(); + }) + .catch(done.fail); + }); + }); }); }); diff --git a/spec/javascripts/jobs/components/job_container_item_spec.js b/spec/javascripts/jobs/components/job_container_item_spec.js index 8588eda19c8..2d108f1ad7f 100644 --- a/spec/javascripts/jobs/components/job_container_item_spec.js +++ b/spec/javascripts/jobs/components/job_container_item_spec.js @@ -4,6 +4,7 @@ import mountComponent from 'spec/helpers/vue_mount_component_helper'; import job from '../mock_data'; describe('JobContainerItem', () => { + const delayedJobFixture = getJSONFixture('jobs/delayed.json'); const Component = Vue.extend(JobContainerItem); let vm; @@ -70,4 +71,29 @@ describe('JobContainerItem', () => { expect(vm.$el).toHaveSpriteIcon('retry'); }); }); + + describe('for delayed job', () => { + beforeEach(() => { + const remainingMilliseconds = 1337000; + spyOn(Date, 'now').and.callFake( + () => new Date(delayedJobFixture.scheduled_at).getTime() - remainingMilliseconds, + ); + }); + + it('displays remaining time in tooltip', done => { + vm = mountComponent(Component, { + job: delayedJobFixture, + isActive: false, + }); + + Vue.nextTick() + .then(() => { + expect(vm.$el.querySelector('.js-job-link').getAttribute('data-original-title')).toEqual( + 'delayed job - delayed manual action (00:22:17)', + ); + }) + .then(done) + .catch(done.fail); + }); + }); }); diff --git a/spec/javascripts/jobs/mixins/delayed_job_mixin_spec.js b/spec/javascripts/jobs/mixins/delayed_job_mixin_spec.js new file mode 100644 index 00000000000..48a6b80b365 --- /dev/null +++ b/spec/javascripts/jobs/mixins/delayed_job_mixin_spec.js @@ -0,0 +1,93 @@ +import Vue from 'vue'; +import delayedJobMixin from '~/jobs/mixins/delayed_job_mixin'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +describe('DelayedJobMixin', () => { + const delayedJobFixture = getJSONFixture('jobs/delayed.json'); + const dummyComponent = Vue.extend({ + mixins: [delayedJobMixin], + props: { + job: { + type: Object, + required: true, + }, + }, + template: '<div>{{ remainingTime }}</div>', + }); + + let vm; + + beforeEach(() => { + jasmine.clock().install(); + }); + + afterEach(() => { + vm.$destroy(); + jasmine.clock().uninstall(); + }); + + describe('if job is empty object', () => { + beforeEach(() => { + vm = mountComponent(dummyComponent, { + job: {}, + }); + }); + + it('sets remaining time to 00:00:00', () => { + expect(vm.$el.innerText).toBe('00:00:00'); + }); + + describe('after mounting', () => { + beforeEach(done => { + Vue.nextTick() + .then(done) + .catch(done.fail); + }); + + it('doe not update remaining time', () => { + expect(vm.$el.innerText).toBe('00:00:00'); + }); + }); + }); + + describe('if job is delayed job', () => { + let remainingTimeInMilliseconds = 42000; + + beforeEach(() => { + spyOn(Date, 'now').and.callFake( + () => new Date(delayedJobFixture.scheduled_at).getTime() - remainingTimeInMilliseconds, + ); + vm = mountComponent(dummyComponent, { + job: delayedJobFixture, + }); + }); + + it('sets remaining time to 00:00:00', () => { + expect(vm.$el.innerText).toBe('00:00:00'); + }); + + describe('after mounting', () => { + beforeEach(done => { + Vue.nextTick() + .then(done) + .catch(done.fail); + }); + + it('sets remaining time', () => { + expect(vm.$el.innerText).toBe('00:00:42'); + }); + + it('updates remaining time', done => { + remainingTimeInMilliseconds = 41000; + jasmine.clock().tick(1000); + + Vue.nextTick() + .then(() => { + expect(vm.$el.innerText).toBe('00:00:41'); + }) + .then(done) + .catch(done.fail); + }); + }); + }); +}); diff --git a/spec/javascripts/notes/components/diff_with_note_spec.js b/spec/javascripts/notes/components/diff_with_note_spec.js index 239d7950907..0c16103714a 100644 --- a/spec/javascripts/notes/components/diff_with_note_spec.js +++ b/spec/javascripts/notes/components/diff_with_note_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import DiffWithNote from '~/notes/components/diff_with_note.vue'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import createStore from '~/notes/stores'; +import { createStore } from '~/mr_notes/stores'; import { mountComponentWithStore } from 'spec/helpers'; const discussionFixture = 'merge_requests/diff_discussion.json'; diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js index d7298cb3483..f6c854e6def 100644 --- a/spec/javascripts/notes/components/note_actions_spec.js +++ b/spec/javascripts/notes/components/note_actions_spec.js @@ -55,7 +55,7 @@ describe('issue_note_actions component', () => { expect(vm.$el.querySelector('.js-note-edit')).toBeDefined(); }); - it('should be possible to report as abuse', () => { + it('should be possible to report abuse to GitLab', () => { expect(vm.$el.querySelector(`a[href="${props.reportAbusePath}"]`)).toBeDefined(); }); diff --git a/spec/javascripts/pipelines/graph/job_item_spec.js b/spec/javascripts/pipelines/graph/job_item_spec.js index 7cbcdc791e7..41b614cc95e 100644 --- a/spec/javascripts/pipelines/graph/job_item_spec.js +++ b/spec/javascripts/pipelines/graph/job_item_spec.js @@ -6,6 +6,7 @@ describe('pipeline graph job item', () => { const JobComponent = Vue.extend(JobItem); let component; + const delayedJobFixture = getJSONFixture('jobs/delayed.json'); const mockJob = { id: 4256, name: 'test', @@ -167,4 +168,30 @@ describe('pipeline graph job item', () => { expect(component.$el.querySelector(tooltipBoundary)).toBeNull(); }); }); + + describe('for delayed job', () => { + beforeEach(() => { + const fifteenMinutesInMilliseconds = 900000; + spyOn(Date, 'now').and.callFake( + () => new Date(delayedJobFixture.scheduled_at).getTime() - fifteenMinutesInMilliseconds, + ); + }); + + it('displays remaining time in tooltip', done => { + component = mountComponent(JobComponent, { + job: delayedJobFixture, + }); + + Vue.nextTick() + .then(() => { + expect( + component.$el + .querySelector('.js-pipeline-graph-job-link') + .getAttribute('data-original-title'), + ).toEqual('delayed job - delayed manual action (00:15:00)'); + }) + .then(done) + .catch(done.fail); + }); + }); }); diff --git a/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js b/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js index e2c34508b0d..4da8c6196b1 100644 --- a/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js +++ b/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js @@ -47,7 +47,7 @@ describe('ContentViewer', () => { }); setTimeout(() => { - expect(vm.$el.querySelector('.image_file img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL); + expect(vm.$el.querySelector('img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL); done(); }); diff --git a/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js index fcd231ec693..67a3a2e08bc 100644 --- a/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js +++ b/spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js @@ -30,11 +30,11 @@ describe('DiffViewer', () => { }); setTimeout(() => { - expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe( + expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe( `//raw/DEF/${RED_BOX_IMAGE_URL}`, ); - expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe( + expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe( `//raw/ABC/${GREEN_BOX_IMAGE_URL}`, ); diff --git a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js index 380effdb669..2d3e178d249 100644 --- a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js +++ b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js @@ -52,13 +52,9 @@ describe('ImageDiffViewer', () => { }); setTimeout(() => { - expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe( - GREEN_BOX_IMAGE_URL, - ); + expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL); - expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe( - RED_BOX_IMAGE_URL, - ); + expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL); expect(vm.$el.querySelector('.view-modes-menu li.active').textContent.trim()).toBe('2-up'); expect(vm.$el.querySelector('.view-modes-menu li:nth-child(2)').textContent.trim()).toBe( @@ -81,9 +77,7 @@ describe('ImageDiffViewer', () => { }); setTimeout(() => { - expect(vm.$el.querySelector('.added .image_file img').getAttribute('src')).toBe( - GREEN_BOX_IMAGE_URL, - ); + expect(vm.$el.querySelector('.added img').getAttribute('src')).toBe(GREEN_BOX_IMAGE_URL); done(); }); @@ -97,9 +91,7 @@ describe('ImageDiffViewer', () => { }); setTimeout(() => { - expect(vm.$el.querySelector('.deleted .image_file img').getAttribute('src')).toBe( - RED_BOX_IMAGE_URL, - ); + expect(vm.$el.querySelector('.deleted img').getAttribute('src')).toBe(RED_BOX_IMAGE_URL); done(); }); diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 1169938b80c..f1a2946acda 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -1,5 +1,4 @@ -require 'fast_spec_helper' -require_dependency 'active_model' +require 'spec_helper' describe Gitlab::Ci::Config::Entry::Job do let(:entry) { described_class.new(config, name: :rspec) } @@ -138,6 +137,36 @@ describe Gitlab::Ci::Config::Entry::Job do end end + context 'when parallel value is not correct' do + context 'when it is not a numeric value' do + let(:config) { { parallel: true } } + + it 'returns error about invalid type' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job parallel is not a number' + end + end + + context 'when it is lower than two' do + let(:config) { { parallel: 1 } } + + it 'returns error about value too low' do + expect(entry).not_to be_valid + expect(entry.errors) + .to include 'job parallel must be greater than or equal to 2' + end + end + + context 'when it is not an integer' do + let(:config) { { parallel: 1.5 } } + + it 'returns error about wrong value' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'job parallel must be an integer' + end + end + end + context 'when delayed job' do context 'when start_in is specified' do let(:config) { { script: 'echo', when: 'delayed', start_in: '1 day' } } diff --git a/spec/lib/gitlab/ci/config/normalizer_spec.rb b/spec/lib/gitlab/ci/config/normalizer_spec.rb new file mode 100644 index 00000000000..97926695b6e --- /dev/null +++ b/spec/lib/gitlab/ci/config/normalizer_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::Normalizer do + let(:job_name) { :rspec } + let(:job_config) { { script: 'rspec', parallel: 5, name: 'rspec' } } + let(:config) { { job_name => job_config } } + + describe '.normalize_jobs' do + subject { described_class.new(config).normalize_jobs } + + it 'does not have original job' do + is_expected.not_to include(job_name) + end + + it 'has parallelized jobs' do + job_names = [:"rspec 1/5", :"rspec 2/5", :"rspec 3/5", :"rspec 4/5", :"rspec 5/5"] + + is_expected.to include(*job_names) + end + + it 'sets job instance in options' do + expect(subject.values).to all(include(:instance)) + end + + it 'parallelizes jobs with original config' do + original_config = config[job_name].except(:name) + configs = subject.values.map { |config| config.except(:name, :instance) } + + expect(configs).to all(eq(original_config)) + end + + context 'when the job is not parallelized' do + let(:job_config) { { script: 'rspec', name: 'rspec' } } + + it 'returns the same hash' do + is_expected.to eq(config) + end + end + + context 'when there is a job with a slash in it' do + let(:job_name) { :"rspec 35/2" } + + it 'properly parallelizes job names' do + job_names = [:"rspec 35/2 1/5", :"rspec 35/2 2/5", :"rspec 35/2 3/5", :"rspec 35/2 4/5", :"rspec 35/2 5/5"] + + is_expected.to include(*job_names) + end + end + + context 'when jobs depend on parallelized jobs' do + let(:config) { { job_name => job_config, other_job: { script: 'echo 1', dependencies: [job_name.to_s] } } } + + it 'parallelizes dependencies' do + job_names = ["rspec 1/5", "rspec 2/5", "rspec 3/5", "rspec 4/5", "rspec 5/5"] + + expect(subject[:other_job][:dependencies]).to include(*job_names) + end + + it 'does not include original job name in dependencies' do + expect(subject[:other_job][:dependencies]).not_to include(job_name) + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/build/scheduled_spec.rb b/spec/lib/gitlab/ci/status/build/scheduled_spec.rb index 4a52b3ab8de..68b87fea75d 100644 --- a/spec/lib/gitlab/ci/status/build/scheduled_spec.rb +++ b/spec/lib/gitlab/ci/status/build/scheduled_spec.rb @@ -13,24 +13,10 @@ describe Gitlab::Ci::Status::Build::Scheduled do end describe '#status_tooltip' do - context 'when scheduled_at is not expired' do - let(:build) { create(:ci_build, scheduled_at: 1.minute.since, project: project) } - - it 'shows execute_in of the scheduled job' do - Timecop.freeze(Time.now.change(usec: 0)) do - expect(subject.status_tooltip).to include('00:01:00') - end - end - end - - context 'when scheduled_at is expired' do - let(:build) { create(:ci_build, :expired_scheduled, project: project) } + let(:build) { create(:ci_build, scheduled_at: 1.minute.since, project: project) } - it 'shows 00:00' do - Timecop.freeze do - expect(subject.status_tooltip).to include('00:00') - end - end + it 'has a placeholder for the remaining time' do + expect(subject.status_tooltip).to include('%{remainingTime}') end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 85b23edce9f..dcfd54107a3 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -645,6 +645,33 @@ module Gitlab end end + describe 'Parallel' do + context 'when job is parallelized' do + let(:parallel) { 5 } + + let(:config) do + YAML.dump(rspec: { script: 'rspec', + parallel: parallel }) + end + + it 'returns parallelized jobs' do + config_processor = Gitlab::Ci::YamlProcessor.new(config) + builds = config_processor.stage_builds_attributes('test') + build_options = builds.map { |build| build[:options] } + + expect(builds.size).to eq(5) + expect(build_options).to all(include(:instance, parallel: parallel)) + end + + it 'does not have the original job' do + config_processor = Gitlab::Ci::YamlProcessor.new(config) + builds = config_processor.stage_builds_attributes('test') + + expect(builds).not_to include(:rspec) + end + end + end + describe 'cache' do context 'when cache definition has unknown keys' do it 'raises relevant validation error' do diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 2f51642b58e..3417896e259 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -41,6 +41,52 @@ describe Gitlab::Diff::File do end end + describe '#unfold_diff_lines' do + let(:unfolded_lines) { double('expanded-lines') } + let(:unfolder) { instance_double(Gitlab::Diff::LinesUnfolder) } + let(:position) { instance_double(Gitlab::Diff::Position, old_line: 10) } + + before do + allow(Gitlab::Diff::LinesUnfolder).to receive(:new) { unfolder } + end + + context 'when unfold required' do + before do + allow(unfolder).to receive(:unfold_required?) { true } + allow(unfolder).to receive(:unfolded_diff_lines) { unfolded_lines } + end + + it 'changes @unfolded to true' do + diff_file.unfold_diff_lines(position) + + expect(diff_file).to be_unfolded + end + + it 'updates @diff_lines' do + diff_file.unfold_diff_lines(position) + + expect(diff_file.diff_lines).to eq(unfolded_lines) + end + end + + context 'when unfold not required' do + before do + allow(unfolder).to receive(:unfold_required?) { false } + end + + it 'keeps @unfolded false' do + diff_file.unfold_diff_lines(position) + + expect(diff_file).not_to be_unfolded + end + + it 'does not update @diff_lines' do + expect { diff_file.unfold_diff_lines(position) } + .not_to change(diff_file, :diff_lines) + end + end + end + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb new file mode 100644 index 00000000000..8e00c8e0e30 --- /dev/null +++ b/spec/lib/gitlab/diff/lines_unfolder_spec.rb @@ -0,0 +1,750 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Diff::LinesUnfolder do + let(:raw_diff) do + <<-DIFF.strip_heredoc + @@ -7,9 +7,6 @@ + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": [ + - "--share=ipc", "--socket=x11", + - "--socket=wayland", + - "--talk-name=org.gnome.OnlineAccounts", + "--talk-name=org.freedesktop.Tracker1", + "--filesystem=home", + "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", + @@ -62,7 +59,7 @@ + }, + { + "name": "gnome-desktop", + - "config-opts": ["--disable-debug-tools", "--disable-udev"], + + "config-opts": ["--disable-debug-tools", "--disable-"], + "sources": [ + { + "type": "git", + @@ -83,11 +80,6 @@ + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + - "config-opts": [ + - "-Denable-desktop=false", + - "-Denable-selinux=false", + - "--libdir=/app/lib" + - ], + "sources": [ + { + "type": "git", + DIFF + end + + let(:raw_old_blob) do + <<-BLOB.strip_heredoc + { + "app-id": "org.gnome.Nautilus", + "runtime": "org.gnome.Platform", + "runtime-version": "master", + "sdk": "org.gnome.Sdk", + "command": "nautilus", + "tags": ["devel", "development", "nightly"], + "desktop-file-name-prefix": "(Development) ", + "finish-args": [ + "--share=ipc", "--socket=x11", + "--socket=wayland", + "--talk-name=org.gnome.OnlineAccounts", + "--talk-name=org.freedesktop.Tracker1", + "--filesystem=home", + "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", + "--filesystem=xdg-run/dconf", "--filesystem=~/.config/dconf:ro", + "--talk-name=ca.desrt.dconf", "--env=DCONF_USER_CONFIG_DIR=.config/dconf" + ], + "cleanup": [ "/include", "/share/bash-completion" ], + "modules": [ + { + "name": "exiv2", + "sources": [ + { + "type": "archive", + "url": "http://exiv2.org/builds/exiv2-0.26-trunk.tar.gz", + "sha256": "c75e3c4a0811bf700d92c82319373b7a825a2331c12b8b37d41eb58e4f18eafb" + }, + { + "type": "shell", + "commands": [ + "cp -f /usr/share/gnu-config/config.sub ./config/", + "cp -f /usr/share/gnu-config/config.guess ./config/" + ] + } + ] + }, + { + "name": "gexiv2", + "config-opts": [ "--disable-introspection" ], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gexiv2" + } + ] + }, + { + "name": "tracker", + "cleanup": [ "/bin", "/etc", "/libexec" ], + "config-opts": [ "--disable-miner-apps", "--disable-static", + "--disable-tracker-extract", "--disable-tracker-needle", + "--disable-tracker-preferences", "--disable-artwork", + "--disable-tracker-writeback", "--disable-miner-user-guides", + "--with-bash-completion-dir=no" ], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/tracker" + } + ] + }, + { + "name": "gnome-desktop", + "config-opts": ["--disable-debug-tools", "--disable-udev"], + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gnome-desktop" + } + ] + }, + { + "name": "gnome-autoar", + "sources": [ + { + "type": "git", + "url": "https://git.gnome.org/browse/gnome-autoar" + } + ] + }, + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "config-opts": [ + "-Denable-desktop=false", + "-Denable-selinux=false", + "--libdir=/app/lib" + ], + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + ] + }, + { + "app-id": "foo", + "runtime": "foo", + "runtime-version": "foo", + "sdk": "foo", + "command": "foo", + "tags": ["foo", "bar", "kux"], + "desktop-file-name-prefix": "(Foo) ", + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + }, + { + "app-id": "foo", + "runtime": "foo", + "runtime-version": "foo", + "sdk": "foo", + "command": "foo", + "tags": ["foo", "bar", "kux"], + "desktop-file-name-prefix": "(Foo) ", + { + "buildsystem": "meson", + "builddir": true, + "name": "nautilus", + "sources": [ + { + "type": "git", + "url": "https://gitlab.gnome.org/GNOME/nautilus.git" + } + ] + } + } + BLOB + end + + let(:project) { create(:project) } + + let(:old_blob) { Gitlab::Git::Blob.new(data: raw_old_blob) } + + let(:diff) do + Gitlab::Git::Diff.new(diff: raw_diff, + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + a_mode: "100644", + b_mode: "100644", + new_file: false, + renamed_file: false, + deleted_file: false, + too_large: false) + end + + let(:diff_file) do + Gitlab::Diff::File.new(diff, repository: project.repository) + end + + before do + allow(old_blob).to receive(:load_all_data!) + allow(diff_file).to receive(:old_blob) { old_blob } + end + + subject { described_class.new(diff_file, position) } + + context 'position requires a middle expansion and new match lines' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 43, + new_line: 40) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"], + [41, 41, " \"sources\": ["], + [42, 42, " {"], + [43, 43, " \"type\": \"git\","], + [44, 44, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], + [45, 45, " }"], + [46, 46, " ]"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(7) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + + # New match line + [40, 37, "@@ -40,7+37,7 @@"], + + # Injected blob lines + [40, 37, " \"config-opts\": [ \"--disable-introspection\" ],"], + [41, 38, " \"sources\": ["], + [42, 39, " {"], + [43, 40, " \"type\": \"git\","], # comment + [44, 41, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], + [45, 42, " }"], + [46, 43, " ]"], + # end + + # Second match line + [62, 59, "@@ -62,7+59,7 @@"], + + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a middle expansion and no top match line' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 16, + new_line: 17) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[16, 16, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], + [17, 17, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], + [18, 18, " ],"], + [19, 19, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + # No new match needed + + # Injected blob lines + [16, 13, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], + [17, 14, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], + [18, 15, " ],"], + [19, 16, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"], + # end + + # Second match line + [62, 59, "@@ -62,4+59,4 @@"], + + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a middle expansion and no bottom match line' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 82, + new_line: 79) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[79, 79, " }"], + [80, 80, " ]"], + [81, 81, " },"], + [82, 82, " {"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + + # New top match line + [79, 76, "@@ -79,4+76,4 @@"], + + # Injected blob lines + [79, 76, " }"], + [80, 77, " ]"], + [81, 78, " },"], + [82, 79, " {"], + # end + + # No new second match line + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position requires a short top expansion' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 6, + new_line: 6) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[3, 3, " \"runtime\": \"org.gnome.Platform\","], + [4, 4, " \"runtime-version\": \"master\","], + [5, 5, " \"sdk\": \"org.gnome.Sdk\","], + [6, 6, " \"command\": \"nautilus\","]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(4) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + # New match line + [[3, 3, "@@ -3,4+3,4 @@"], + + # Injected blob lines + [3, 3, " \"runtime\": \"org.gnome.Platform\","], + [4, 4, " \"runtime-version\": \"master\","], + [5, 5, " \"sdk\": \"org.gnome.Sdk\","], + [6, 6, " \"command\": \"nautilus\","], + # end + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","]] + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end + + context 'position sits between two match lines (no expasion needed)' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 64, + new_line: 61) + end + + context 'diff lines' do + it 'returns nil' do + expect(subject.unfolded_diff_lines).to be_nil + end + end + end + + context 'position requires bottom expansion and new match lines' do + let(:position) do + Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", + head_sha: "1487062132228de836236c522fe52fed4980a46c", + old_path: "build-aux/flatpak/org.gnome.Nautilus.json", + new_path: "build-aux/flatpak/org.gnome.Nautilus.json", + position_type: "text", + old_line: 107, + new_line: 99) + end + + context 'blob lines' do + let(:expected_blob_lines) do + [[104, 104, " \"sdk\": \"foo\","], + [105, 105, " \"command\": \"foo\","], + [106, 106, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], + [107, 107, " \"desktop-file-name-prefix\": \"(Foo) \","], + [108, 108, " {"], + [109, 109, " \"buildsystem\": \"meson\","], + [110, 110, " \"builddir\": true,"]] + end + + it 'returns the extracted blob lines correctly' do + extracted_lines = subject.blob_lines + + expect(extracted_lines.size).to eq(7) + + extracted_lines.each_with_index do |line, i| + expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) + end + end + end + + context 'diff lines' do + let(:expected_diff_lines) do + [[7, 7, "@@ -7,9 +7,6 @@"], + [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], + [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], + [9, 9, " \"finish-args\": ["], + [10, 10, "- \"--share=ipc\", \"--socket=x11\","], + [11, 10, "- \"--socket=wayland\","], + [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], + [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], + [14, 11, " \"--filesystem=home\","], + [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], + [62, 59, "@@ -62,7 +59,7 @@"], + [62, 59, " },"], + [63, 60, " {"], + [64, 61, " \"name\": \"gnome-desktop\","], + [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], + [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], + [66, 63, " \"sources\": ["], + [67, 64, " {"], + [68, 65, " \"type\": \"git\","], + [83, 80, "@@ -83,11 +80,6 @@"], + [83, 80, " \"buildsystem\": \"meson\","], + [84, 81, " \"builddir\": true,"], + [85, 82, " \"name\": \"nautilus\","], + [86, 83, "- \"config-opts\": ["], + [87, 83, "- \"-Denable-desktop=false\","], + [88, 83, "- \"-Denable-selinux=false\","], + [89, 83, "- \"--libdir=/app/lib\""], + [90, 83, "- ],"], + [91, 83, " \"sources\": ["], + [92, 84, " {"], + [93, 85, " \"type\": \"git\","], + # New match line + [104, 96, "@@ -104,7+96,7 @@"], + + # Injected blob lines + [104, 96, " \"sdk\": \"foo\","], + [105, 97, " \"command\": \"foo\","], + [106, 98, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], + [107, 99, " \"desktop-file-name-prefix\": \"(Foo) \","], + [108, 100, " {"], + [109, 101, " \"buildsystem\": \"meson\","], + [110, 102, " \"builddir\": true,"]] + # end + end + + it 'return merge of blob lines with diff lines correctly' do + new_diff_lines = subject.unfolded_diff_lines + + expected_diff_lines.each_with_index do |expected_line, i| + line = new_diff_lines[i] + + expect([line.old_pos, line.new_pos, line.text]) + .to eq([expected_line[0], expected_line[1], expected_line[2]]) + end + end + + it 'merged lines have correct line codes' do + new_diff_lines = subject.unfolded_diff_lines + + new_diff_lines.each_with_index do |line, i| + old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] + + unless line.type == 'match' + expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) + end + end + end + end + end +end diff --git a/spec/migrations/steal_fill_store_upload_spec.rb b/spec/migrations/steal_fill_store_upload_spec.rb new file mode 100644 index 00000000000..ed809baf2b5 --- /dev/null +++ b/spec/migrations/steal_fill_store_upload_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20181105201455_steal_fill_store_upload.rb') + +describe StealFillStoreUpload, :migration do + let(:uploads) { table(:uploads) } + + describe '#up' do + it 'steals the FillStoreUpload background migration' do + expect(Gitlab::BackgroundMigration).to receive(:steal).with('FillStoreUpload').and_call_original + + migrate! + end + + it 'does not run migration if not needed' do + uploads.create(size: 100.kilobytes, + uploader: 'AvatarUploader', + path: 'uploads/-/system/avatar.jpg', + store: 1) + + expect_any_instance_of(Gitlab::BackgroundMigration::FillStoreUpload).not_to receive(:perform) + + migrate! + end + + it 'ensures all rows are migrated' do + uploads.create(size: 100.kilobytes, + uploader: 'AvatarUploader', + path: 'uploads/-/system/avatar.jpg', + store: nil) + + expect_any_instance_of(Gitlab::BackgroundMigration::FillStoreUpload).to receive(:perform).and_call_original + + expect do + migrate! + end.to change { uploads.where(store: nil).count }.from(1).to(0) + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2e65a6a2a0f..5bd2f096656 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2015,6 +2015,7 @@ describe Ci::Build do { key: 'CI_COMMIT_BEFORE_SHA', value: build.before_sha, public: true }, { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_NODE_TOTAL', value: '1', public: true }, { key: 'CI_BUILD_REF', value: build.sha, public: true }, { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true }, @@ -2476,6 +2477,29 @@ describe Ci::Build do end end + context 'when build is parallelized' do + let(:total) { 5 } + let(:index) { 3 } + + before do + build.options[:parallel] = total + build.options[:instance] = index + build.name = "#{build.name} #{index}/#{total}" + end + + it 'includes CI_NODE_INDEX' do + is_expected.to include( + { key: 'CI_NODE_INDEX', value: index.to_s, public: true } + ) + end + + it 'includes correct CI_NODE_TOTAL' do + is_expected.to include( + { key: 'CI_NODE_TOTAL', value: total.to_s, public: true } + ) + end + end + describe 'variables ordering' do context 'when variables hierarchy is stubbed' do let(:build_pre_var) { { key: 'build', value: 'value', public: true } } diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb index 8e88bb81162..0bc3ee014e6 100644 --- a/spec/models/compare_spec.rb +++ b/spec/models/compare_spec.rb @@ -92,4 +92,33 @@ describe Compare do expect(subject.diff_refs.head_sha).to eq(head_commit.id) end end + + describe '#modified_paths' do + context 'changes are present' do + let(:raw_compare) do + Gitlab::Git::Compare.new( + project.repository.raw_repository, 'before-create-delete-modify-move', 'after-create-delete-modify-move' + ) + end + + it 'returns affected file paths, without duplication' do + expect(subject.modified_paths).to contain_exactly(*%w{ + foo/for_move.txt + foo/bar/for_move.txt + foo/for_create.txt + foo/for_delete.txt + foo/for_edit.txt + }) + end + end + + context 'changes are absent' do + let(:start_commit) { sample_commit } + let(:head_commit) { sample_commit } + + it 'returns empty array' do + expect(subject.modified_paths).to eq([]) + end + end + end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 47e8f04e728..cbe60b3a4a5 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -232,4 +232,17 @@ describe MergeRequestDiff do expect(commits.map(&:sha)).to match_array(commit_shas) end end + + describe '#modified_paths' do + subject do + diff = create(:merge_request_diff) + create(:merge_request_diff_file, :new_file, merge_request_diff: diff) + create(:merge_request_diff_file, :renamed_file, merge_request_diff: diff) + diff + end + + it 'returns affected file paths' do + expect(subject.modified_paths).to eq(%w{foo bar baz}) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 3a54725c7ec..c7202b481d3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -631,6 +631,44 @@ describe MergeRequest do end end + describe '#modified_paths' do + let(:paths) { double(:paths) } + subject(:merge_request) { build(:merge_request) } + + before do + expect(diff).to receive(:modified_paths).and_return(paths) + end + + context 'when past_merge_request_diff is specified' do + let(:another_diff) { double(:merge_request_diff) } + let(:diff) { another_diff } + + it 'returns affected file paths from specified past_merge_request_diff' do + expect(merge_request.modified_paths(past_merge_request_diff: another_diff)).to eq(paths) + end + end + + context 'when compare is present' do + let(:compare) { double(:compare) } + let(:diff) { compare } + + it 'returns affected file paths from compare' do + merge_request.compare = compare + + expect(merge_request.modified_paths).to eq(paths) + end + end + + context 'when no arguments provided' do + let(:diff) { merge_request.merge_request_diff } + subject(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') } + + it 'returns affected file paths for merge_request_diff' do + expect(merge_request.modified_paths).to eq(paths) + end + end + end + describe "#related_notes" do let!(:merge_request) { create(:merge_request) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f020557e4af..471f19f9b7c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4010,6 +4010,28 @@ describe Project do end end + describe '#snippets_visible?' do + it 'returns true when a logged in user can read snippets' do + project = create(:project, :public) + user = create(:user) + + expect(project.snippets_visible?(user)).to eq(true) + end + + it 'returns true when an anonymous user can read snippets' do + project = create(:project, :public) + + expect(project.snippets_visible?).to eq(true) + end + + it 'returns false when a user can not read snippets' do + project = create(:project, :private) + user = create(:user) + + expect(project.snippets_visible?(user)).to eq(false) + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index e09d89d235d..7a7272ccb60 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -131,6 +131,217 @@ describe Snippet do end end + describe '.with_optional_visibility' do + context 'when a visibility level is provided' do + it 'returns snippets with the given visibility' do + create(:snippet, :private) + + snippet = create(:snippet, :public) + snippets = described_class + .with_optional_visibility(Gitlab::VisibilityLevel::PUBLIC) + + expect(snippets).to eq([snippet]) + end + end + + context 'when a visibility level is not provided' do + it 'returns all snippets' do + snippet1 = create(:snippet, :public) + snippet2 = create(:snippet, :private) + snippets = described_class.with_optional_visibility + + expect(snippets).to include(snippet1, snippet2) + end + end + end + + describe '.only_global_snippets' do + it 'returns snippets not associated with any projects' do + create(:project_snippet) + + snippet = create(:snippet) + snippets = described_class.only_global_snippets + + expect(snippets).to eq([snippet]) + end + end + + describe '.only_include_projects_visible_to' do + let!(:project1) { create(:project, :public) } + let!(:project2) { create(:project, :internal) } + let!(:project3) { create(:project, :private) } + let!(:snippet1) { create(:project_snippet, project: project1) } + let!(:snippet2) { create(:project_snippet, project: project2) } + let!(:snippet3) { create(:project_snippet, project: project3) } + + context 'when a user is provided' do + it 'returns snippets visible to the user' do + user = create(:user) + + snippets = described_class.only_include_projects_visible_to(user) + + expect(snippets).to include(snippet1, snippet2) + expect(snippets).not_to include(snippet3) + end + end + + context 'when a user is not provided' do + it 'returns snippets visible to anonymous users' do + snippets = described_class.only_include_projects_visible_to + + expect(snippets).to include(snippet1) + expect(snippets).not_to include(snippet2, snippet3) + end + end + end + + describe 'only_include_projects_with_snippets_enabled' do + context 'when the include_private option is enabled' do + it 'includes snippets for projects with snippets set to private' do + project = create(:project) + + project.project_feature + .update(snippets_access_level: ProjectFeature::PRIVATE) + + snippet = create(:project_snippet, project: project) + + snippets = described_class + .only_include_projects_with_snippets_enabled(include_private: true) + + expect(snippets).to eq([snippet]) + end + end + + context 'when the include_private option is not enabled' do + it 'does not include snippets for projects that have snippets set to private' do + project = create(:project) + + project.project_feature + .update(snippets_access_level: ProjectFeature::PRIVATE) + + create(:project_snippet, project: project) + + snippets = described_class.only_include_projects_with_snippets_enabled + + expect(snippets).to be_empty + end + end + + it 'includes snippets for projects with snippets enabled' do + project = create(:project) + + project.project_feature + .update(snippets_access_level: ProjectFeature::ENABLED) + + snippet = create(:project_snippet, project: project) + snippets = described_class.only_include_projects_with_snippets_enabled + + expect(snippets).to eq([snippet]) + end + end + + describe '.only_include_authorized_projects' do + it 'only includes snippets for projects the user is authorized to see' do + user = create(:user) + project1 = create(:project, :private) + project2 = create(:project, :private) + + project1.team.add_developer(user) + + create(:project_snippet, project: project2) + + snippet = create(:project_snippet, project: project1) + snippets = described_class.only_include_authorized_projects(user) + + expect(snippets).to eq([snippet]) + end + end + + describe '.for_project_with_user' do + context 'when a user is provided' do + it 'returns an empty collection if the user can not view the snippets' do + project = create(:project, :private) + user = create(:user) + + project.project_feature + .update(snippets_access_level: ProjectFeature::ENABLED) + + create(:project_snippet, :public, project: project) + + expect(described_class.for_project_with_user(project, user)).to be_empty + end + + it 'returns the snippets if the user is a member of the project' do + project = create(:project, :private) + user = create(:user) + snippet = create(:project_snippet, project: project) + + project.team.add_developer(user) + + snippets = described_class.for_project_with_user(project, user) + + expect(snippets).to eq([snippet]) + end + + it 'returns public snippets for a public project the user is not a member of' do + project = create(:project, :public) + + project.project_feature + .update(snippets_access_level: ProjectFeature::ENABLED) + + user = create(:user) + snippet = create(:project_snippet, :public, project: project) + + create(:project_snippet, :private, project: project) + + snippets = described_class.for_project_with_user(project, user) + + expect(snippets).to eq([snippet]) + end + end + + context 'when a user is not provided' do + it 'returns an empty collection for a private project' do + project = create(:project, :private) + + project.project_feature + .update(snippets_access_level: ProjectFeature::ENABLED) + + create(:project_snippet, :public, project: project) + + expect(described_class.for_project_with_user(project)).to be_empty + end + + it 'returns public snippets for a public project' do + project = create(:project, :public) + snippet = create(:project_snippet, :public, project: project) + + project.project_feature + .update(snippets_access_level: ProjectFeature::PUBLIC) + + create(:project_snippet, :private, project: project) + + snippets = described_class.for_project_with_user(project) + + expect(snippets).to eq([snippet]) + end + end + end + + describe '.visible_to_or_authored_by' do + it 'returns snippets visible to the user' do + user = create(:user) + snippet1 = create(:snippet, :public) + snippet2 = create(:snippet, :private, author: user) + snippet3 = create(:snippet, :private) + + snippets = described_class.visible_to_or_authored_by(user) + + expect(snippets).to include(snippet1, snippet2) + expect(snippets).not_to include(snippet3) + end + end + describe '#participants' do let(:project) { create(:project, :public) } let(:snippet) { create(:snippet, content: 'foo', project: project) } diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 3c89e99abf0..5a0df9fbbb0 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -21,7 +21,8 @@ describe Upload do path: __FILE__, size: described_class::CHECKSUM_THRESHOLD + 1.kilobyte, model: build_stubbed(:user), - uploader: double('ExampleUploader') + uploader: double('ExampleUploader'), + store: ObjectStorage::Store::LOCAL ) expect(UploadChecksumWorker) @@ -35,7 +36,8 @@ describe Upload do path: __FILE__, size: described_class::CHECKSUM_THRESHOLD, model: build_stubbed(:user), - uploader: double('ExampleUploader') + uploader: double('ExampleUploader'), + store: ObjectStorage::Store::LOCAL ) expect { upload.save } @@ -60,7 +62,7 @@ describe Upload do describe '#absolute_path' do it 'returns the path directly when already absolute' do path = '/path/to/namespace/project/secret/file.jpg' - upload = described_class.new(path: path) + upload = described_class.new(path: path, store: ObjectStorage::Store::LOCAL) expect(upload).not_to receive(:uploader_class) @@ -69,7 +71,7 @@ describe Upload do it "delegates to the uploader's absolute_path method" do uploader = spy('FakeUploader') - upload = described_class.new(path: 'secret/file.jpg') + upload = described_class.new(path: 'secret/file.jpg', store: ObjectStorage::Store::LOCAL) expect(upload).to receive(:uploader_class).and_return(uploader) upload.absolute_path @@ -81,7 +83,8 @@ describe Upload do describe '#calculate_checksum!' do let(:upload) do described_class.new(path: __FILE__, - size: described_class::CHECKSUM_THRESHOLD - 1.megabyte) + size: described_class::CHECKSUM_THRESHOLD - 1.megabyte, + store: ObjectStorage::Store::LOCAL) end it 'sets `checksum` to SHA256 sum of the file' do @@ -104,15 +107,56 @@ describe Upload do describe '#exist?' do it 'returns true when the file exists' do - upload = described_class.new(path: __FILE__) + upload = described_class.new(path: __FILE__, store: ObjectStorage::Store::LOCAL) expect(upload).to exist end - it 'returns false when the file does not exist' do - upload = described_class.new(path: "#{__FILE__}-nope") + context 'when the file does not exist' do + it 'returns false' do + upload = described_class.new(path: "#{__FILE__}-nope", store: ObjectStorage::Store::LOCAL) - expect(upload).not_to exist + expect(upload).not_to exist + end + + context 'when the record is persisted' do + it 'sends a message to Sentry' do + upload = create(:upload, :issuable_upload) + + expect(Gitlab::Sentry).to receive(:enabled?).and_return(true) + expect(Raven).to receive(:capture_message).with("Upload file does not exist", extra: upload.attributes) + + upload.exist? + end + + it 'increments a metric counter to signal a problem' do + upload = create(:upload, :issuable_upload) + + counter = double(:counter) + expect(counter).to receive(:increment) + expect(Gitlab::Metrics).to receive(:counter).with(:upload_file_does_not_exist_total, 'The number of times an upload record could not find its file').and_return(counter) + + upload.exist? + end + end + + context 'when the record is not persisted' do + it 'does not send a message to Sentry' do + upload = described_class.new(path: "#{__FILE__}-nope", store: ObjectStorage::Store::LOCAL) + + expect(Raven).not_to receive(:capture_message) + + upload.exist? + end + + it 'does not increment a metric counter' do + upload = described_class.new(path: "#{__FILE__}-nope", store: ObjectStorage::Store::LOCAL) + + expect(Gitlab::Metrics).not_to receive(:counter) + + upload.exist? + end + end end end diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index 53c85f73cde..f0b0f7956ce 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Issuable::BulkUpdateService do let(:user) { create(:user) } - let(:project) { create(:project, namespace: user.namespace) } + let(:project) { create(:project, :repository, namespace: user.namespace) } def bulk_update(issuables, extra_params = {}) bulk_update_params = extra_params diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 546c9f277c5..5acd01828cb 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -31,32 +31,11 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin end context 'cache clearing' do - before do - allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true) - allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true) - end - - it 'retrieves the diff files to cache the highlighted result' do - new_diff = merge_request.create_merge_request_diff - cache_key = new_diff.diffs_collection.cache_key - - expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) - expect(Rails.cache).to receive(:read).with(cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original - - subject.execute - end - it 'clears the cache for older diffs on the merge request' do old_diff = merge_request.merge_request_diff old_cache_key = old_diff.diffs_collection.cache_key - new_diff = merge_request.create_merge_request_diff - new_cache_key = new_diff.diffs_collection.cache_key - expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original - expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original - expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original subject.execute end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 1b599ba11b6..be5ad849ba7 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -593,8 +593,8 @@ describe MergeRequests::UpdateService, :mailer do end context 'setting `allow_collaboration`' do - let(:target_project) { create(:project, :public) } - let(:source_project) { fork_project(target_project) } + let(:target_project) { create(:project, :repository, :public) } + let(:source_project) { fork_project(target_project, nil, repository: true) } let(:user) { create(:user) } let(:merge_request) do create(:merge_request, diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb index 8680e428517..9d2be30c636 100644 --- a/spec/services/milestones/destroy_service_spec.rb +++ b/spec/services/milestones/destroy_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Milestones::DestroyService do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) } before do diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index b1290fd0d47..80b015d4cd0 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -57,6 +57,57 @@ describe Notes::CreateService do end end + context 'noteable highlight cache clearing' do + let(:project_with_repo) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, source_project: project_with_repo, + target_project: project_with_repo) + end + + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: merge_request.diff_refs) + end + + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end + + before do + allow_any_instance_of(Gitlab::Diff::Position) + .to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear) + + described_class.new(project_with_repo, user, new_opts).execute + end + + it 'does not clear cache when note is not the first of the discussion' do + prev_note = + create(:diff_note_on_merge_request, noteable: merge_request, + project: project_with_repo) + reply_opts = + opts.merge(in_reply_to_discussion_id: prev_note.discussion_id, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + + expect(merge_request).not_to receive(:diffs) + + described_class.new(project_with_repo, user, reply_opts).execute + end + end + context 'note diff file' do let(:project_with_repo) { create(:project, :repository) } let(:merge_request) do diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index 64445be560e..b1f4e87e8ea 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -21,5 +21,38 @@ describe Notes::DestroyService do expect { described_class.new(project, user).execute(note) } .to change { user.todos_pending_count }.from(1).to(0) end + + context 'noteable highlight cache clearing' do + let(:repo_project) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request, source_project: repo_project, + target_project: repo_project) + end + + let(:note) do + create(:diff_note_on_merge_request, project: repo_project, + noteable: merge_request) + end + + before do + allow(note.position).to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + described_class.new(repo_project, user).execute(note) + end + + it 'does not clear cache when note is not the first of the discussion' do + reply_note = create(:diff_note_on_merge_request, in_reply_to: note, + project: repo_project, + noteable: merge_request) + + expect(merge_request).not_to receive(:diffs) + + described_class.new(repo_project, user).execute(reply_note) + end + end end end diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb index a8c994c101c..14d62763a5b 100644 --- a/spec/services/notes/quick_actions_service_spec.rb +++ b/spec/services/notes/quick_actions_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Notes::QuickActionsService do shared_context 'note on noteable' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } } let(:assignee) { create(:user) } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 1746721b0d0..c52515aefd8 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -10,7 +10,7 @@ describe TodoService do let(:john_doe) { create(:user) } let(:skipped) { create(:user) } let(:skip_users) { [skipped] } - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:mentions) { 'FYI: ' + [author, assignee, john_doe, member, guest, non_member, admin, skipped].map(&:to_reference).join(' ') } let(:directly_addressed) { [author, assignee, john_doe, member, guest, non_member, admin, skipped].map(&:to_reference).join(' ') } let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin, skipped].map(&:to_reference).join(' ') } diff --git a/spec/support/features/reportable_note_shared_examples.rb b/spec/support/features/reportable_note_shared_examples.rb index 89a5518239d..8cfce49da8a 100644 --- a/spec/support/features/reportable_note_shared_examples.rb +++ b/spec/support/features/reportable_note_shared_examples.rb @@ -20,7 +20,7 @@ shared_examples 'reportable note' do |type| dropdown = comment.find(more_actions_selector) open_dropdown(dropdown) - expect(dropdown).to have_link('Report as abuse', href: abuse_report_path) + expect(dropdown).to have_link('Report abuse to GitLab', href: abuse_report_path) if type == 'issue' || type == 'merge_request' expect(dropdown).to have_button('Delete comment') @@ -33,7 +33,7 @@ shared_examples 'reportable note' do |type| dropdown = comment.find(more_actions_selector) open_dropdown(dropdown) - dropdown.click_link('Report as abuse') + dropdown.click_link('Report abuse to GitLab') expect(find('#user_name')['value']).to match(note.author.username) expect(find('#abuse_report_message')['value']).to match(noteable_note_url(note)) diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 71d72ff27e9..80b96f20e3f 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -55,6 +55,9 @@ module TestEnv 'update-gitlab-shell-v-6-0-1' => '2f61d70', 'update-gitlab-shell-v-6-0-3' => 'de78448', '2-mb-file' => 'bf12d25', + 'before-create-delete-modify-move' => '845009f', + 'between-create-delete-modify-move' => '3f5f443', + 'after-create-delete-modify-move' => 'ba3faa7', 'with-codeowners' => '219560e' }.freeze diff --git a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb index 6fcfae358ec..9588e8be5dc 100644 --- a/spec/tasks/gitlab/uploads/migrate_rake_spec.rb +++ b/spec/tasks/gitlab/uploads/migrate_rake_spec.rb @@ -38,14 +38,6 @@ describe 'gitlab:uploads:migrate rake tasks' do let!(:projects) { create_list(:project, 10, :with_avatar) } it_behaves_like 'enqueue jobs in batch', batch: 4 - - context 'Upload has store = nil' do - before do - Upload.where(model: projects).update_all(store: nil) - end - - it_behaves_like 'enqueue jobs in batch', batch: 4 - end end context "for Group" do diff --git a/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb b/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb index 9c0be249a50..8a9ab02eaca 100644 --- a/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb +++ b/spec/views/projects/notes/_more_actions_dropdown.html.haml_spec.rb @@ -12,10 +12,10 @@ describe 'projects/notes/_more_actions_dropdown' do assign(:project, project) end - it 'shows Report as abuse button if not editable and not current users comment' do + it 'shows Report abuse to GitLab button if not editable and not current users comment' do render 'projects/notes/more_actions_dropdown', current_user: not_author_user, note_editable: false, note: note - expect(rendered).to have_link('Report as abuse') + expect(rendered).to have_link('Report abuse to GitLab') end it 'does not show the More actions button if not editable and current users comment' do @@ -24,10 +24,10 @@ describe 'projects/notes/_more_actions_dropdown' do expect(rendered).not_to have_selector('.dropdown.more-actions') end - it 'shows Report as abuse and Delete buttons if editable and not current users comment' do + it 'shows Report abuse to GitLab and Delete buttons if editable and not current users comment' do render 'projects/notes/more_actions_dropdown', current_user: not_author_user, note_editable: true, note: note - expect(rendered).to have_link('Report as abuse') + expect(rendered).to have_link('Report abuse to GitLab') expect(rendered).to have_link('Delete comment') end |