diff options
50 files changed, 1107 insertions, 418 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index e19207bdc95..b9de487a737 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -76,7 +76,6 @@ export default { <noteable-discussion v-show="isExpanded(discussion)" :discussion="discussion" - :render-header="false" :render-diff-file="false" :always-expanded="true" :discussions-by-diff-order="true" diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index 3339c56cbb6..3b71c0a1fd4 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -76,8 +76,9 @@ export default { :class="className" class="notes_holder" > - <td class="notes_line old"></td> - <td class="notes_content parallel old"> + <td + class="notes_content parallel old" + colspan="2"> <div v-if="shouldRenderDiscussionsOnLeft" class="content" @@ -95,8 +96,9 @@ export default { line-position="left" /> </td> - <td class="notes_line new"></td> - <td class="notes_content parallel new"> + <td + class="notes_content parallel new" + colspan="2"> <div v-if="shouldRenderDiscussionsOnRight" class="content" diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 554db102027..754c6e79ee4 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -321,10 +321,10 @@ Please check your network connection and try again.`; v-else-if="!canCreateNote" :issuable-type="issuableTypeTitle" /> - <ul + <div v-else-if="canCreateNote" class="notes notes-form timeline"> - <li class="timeline-entry"> + <div class="timeline-entry note-form"> <div class="timeline-entry-inner"> <div class="flash-container error-alert timeline-content"></div> <div class="timeline-icon d-none d-sm-none d-md-block"> @@ -462,7 +462,7 @@ append-right-10 comment-type-dropdown js-comment-type-dropdown droplab-dropdown" </form> </div> </div> - </li> - </ul> + </div> + </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/discussion_counter.vue b/app/assets/javascripts/notes/components/discussion_counter.vue index 1f80f24e045..a4d76a70696 100644 --- a/app/assets/javascripts/notes/components/discussion_counter.vue +++ b/app/assets/javascripts/notes/components/discussion_counter.vue @@ -1,9 +1,6 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import resolveSvg from 'icons/_icon_resolve_discussion.svg'; -import resolvedSvg from 'icons/_icon_status_success_solid.svg'; -import mrIssueSvg from 'icons/_icon_mr_issue.svg'; -import nextDiscussionSvg from 'icons/_next_discussion.svg'; +import Icon from '~/vue_shared/components/icon.vue'; import { pluralize } from '../../lib/utils/text_utility'; import discussionNavigation from '../mixins/discussion_navigation'; import tooltip from '../../vue_shared/directives/tooltip'; @@ -12,6 +9,9 @@ export default { directives: { tooltip, }, + components: { + Icon, + }, mixins: [discussionNavigation], computed: { ...mapGetters([ @@ -37,12 +37,6 @@ export default { return this.getNoteableData.create_issue_to_resolve_discussions_path; }, }, - created() { - this.resolveSvg = resolveSvg; - this.resolvedSvg = resolvedSvg; - this.mrIssueSvg = mrIssueSvg; - this.nextDiscussionSvg = nextDiscussionSvg; - }, methods: { ...mapActions(['expandDiscussion']), jumpToFirstUnresolvedDiscussion() { @@ -66,15 +60,9 @@ export default { <span :class="{ 'is-active': allResolved }" class="line-resolve-btn is-disabled" - type="button"> - <span - v-if="allResolved" - v-html="resolvedSvg" - ></span> - <span - v-else - v-html="resolveSvg" - ></span> + type="button" + > + <icon name="check-circle" /> </span> <span class="line-resolve-text"> {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ countText }} resolved @@ -90,7 +78,7 @@ export default { :title="s__('Resolve all discussions in new issue')" data-container="body" class="new-issue-for-discussion btn btn-default discussion-create-issue-btn"> - <span v-html="mrIssueSvg"></span> + <icon name="issue-new" /> </a> </div> <div @@ -103,7 +91,7 @@ export default { data-container="body" class="btn btn-default discussion-next-btn" @click="jumpToFirstUnresolvedDiscussion"> - <span v-html="nextDiscussionSvg"></span> + <icon name="comment-next" /> </button> </div> </div> diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index 01cbe40f444..f7a61fbfcd4 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -1,12 +1,5 @@ <script> import { mapGetters } from 'vuex'; -import emojiSmiling from 'icons/_emoji_slightly_smiling_face.svg'; -import emojiSmile from 'icons/_emoji_smile.svg'; -import emojiSmiley from 'icons/_emoji_smiley.svg'; -import editSvg from 'icons/_icon_pencil.svg'; -import resolveDiscussionSvg from 'icons/_icon_resolve_discussion.svg'; -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'; @@ -110,15 +103,6 @@ export default { return title; }, }, - created() { - this.emojiSmiling = emojiSmiling; - this.emojiSmile = emojiSmile; - this.emojiSmiley = emojiSmiley; - this.editSvg = editSvg; - this.ellipsisSvg = ellipsisSvg; - this.resolveDiscussionSvg = resolveDiscussionSvg; - this.resolvedDiscussionSvg = resolvedDiscussionSvg; - }, methods: { onEdit() { this.$emit('handleEdit'); @@ -152,12 +136,7 @@ export default { class="line-resolve-btn note-action-button" @click="onResolve"> <template v-if="!isResolving"> - <div - v-if="isResolved" - v-html="resolvedDiscussionSvg"></div> - <div - v-else - v-html="resolveDiscussionSvg"></div> + <icon name="check-circle" /> </template> <gl-loading-icon v-else @@ -179,18 +158,18 @@ export default { title="Add reaction" > <gl-loading-icon inline/> - <span - class="link-highlight award-control-icon-neutral" - v-html="emojiSmiling"> - </span> - <span - class="link-highlight award-control-icon-positive" - v-html="emojiSmiley"> - </span> - <span - class="link-highlight award-control-icon-super-positive" - v-html="emojiSmile"> - </span> + <icon + css-classes="link-highlight award-control-icon-neutral" + name="emoji_slightly_smiling_face" + /> + <icon + css-classes="link-highlight award-control-icon-positive" + name="emoji_smiley" + /> + <icon + css-classes="link-highlight award-control-icon-super-positive" + name="emoji_smiley" + /> </a> </div> <div @@ -204,10 +183,10 @@ export default { data-container="body" data-placement="bottom" @click="onEdit"> - <span - class="link-highlight" - v-html="editSvg"> - </span> + <icon + name="pencil" + css-classes="link-highlight" + /> </button> </div> <div @@ -240,10 +219,10 @@ export default { data-toggle="dropdown" data-container="body" data-placement="bottom"> - <span - class="icon" - v-html="ellipsisSvg"> - </span> + <icon + css-classes="icon" + name="ellipsis_v" + /> </button> <ul class="dropdown-menu more-actions-dropdown dropdown-open-left"> <li v-if="canReportAsAbuse"> diff --git a/app/assets/javascripts/notes/components/note_awards_list.vue b/app/assets/javascripts/notes/components/note_awards_list.vue index df7ab4502a6..401bcfabbe4 100644 --- a/app/assets/javascripts/notes/components/note_awards_list.vue +++ b/app/assets/javascripts/notes/components/note_awards_list.vue @@ -1,13 +1,14 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import emojiSmiling from 'icons/_emoji_slightly_smiling_face.svg'; -import emojiSmile from 'icons/_emoji_smile.svg'; -import emojiSmiley from 'icons/_emoji_smiley.svg'; +import Icon from '~/vue_shared/components/icon.vue'; import Flash from '../../flash'; import { glEmojiTag } from '../../emoji'; import tooltip from '../../vue_shared/directives/tooltip'; export default { + components: { + Icon, + }, directives: { tooltip, }, @@ -72,11 +73,6 @@ export default { return this.noteAuthorId === this.getUserData.id; }, }, - created() { - this.emojiSmiling = emojiSmiling; - this.emojiSmile = emojiSmile; - this.emojiSmiley = emojiSmiley; - }, methods: { ...mapActions(['toggleAwardRequest']), getAwardHTML(name) { @@ -196,17 +192,14 @@ export default { data-boundary="viewport" data-placement="bottom" type="button"> - <span - class="award-control-icon award-control-icon-neutral" - v-html="emojiSmiling"> + <span class="award-control-icon award-control-icon-neutral"> + <icon name="emoji_slightly_smiling_face" /> </span> - <span - class="award-control-icon award-control-icon-positive" - v-html="emojiSmiley"> + <span class="award-control-icon award-control-icon-positive"> + <icon name="emoji_smiley" /> </span> - <span - class="award-control-icon award-control-icon-super-positive" - v-html="emojiSmile"> + <span class="award-control-icon award-control-icon-super-positive"> + <icon name="emoji_smiley" /> </span> <i aria-hidden="true" diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index 7b6e7b72caf..dd7313d7b10 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -45,6 +45,9 @@ export default { noteTimestampLink() { return `#note_${this.noteId}`; }, + hasAuthor() { + return this.author && Object.keys(this.author).length; + }, }, methods: { ...mapActions(['setTargetNoteHash']), @@ -76,7 +79,7 @@ export default { </button> </div> <a - v-if="Object.keys(author).length" + v-if="hasAuthor" :href="author.path" > <span class="note-header-author-name">{{ author.name }}</span> @@ -92,9 +95,6 @@ export default { </span> <span class="note-headline-light"> <span class="note-headline-meta"> - <template v-if="actionText"> - {{ actionText }} - </template> <span class="system-note-message"> <slot></slot> </span> @@ -102,7 +102,9 @@ export default { v-if="createdAt" > <span class="system-note-separator"> - · + <template v-if="actionText"> + {{ actionText }} + </template> </span> <a :href="noteTimestampLink" diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 07115ca07c4..a153edd0476 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -1,16 +1,16 @@ <script> import { mapActions, mapGetters } from 'vuex'; -import resolveDiscussionsSvg from 'icons/_icon_mr_issue.svg'; -import nextDiscussionsSvg from 'icons/_next_discussion.svg'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { truncateSha } from '~/lib/utils/text_utility'; -import systemNote from '~/vue_shared/components/notes/system_note.vue'; import { s__ } from '~/locale'; +import systemNote from '~/vue_shared/components/notes/system_note.vue'; +import icon from '~/vue_shared/components/icon.vue'; import Flash from '../../flash'; import { SYSTEM_NOTE } from '../constants'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import noteableNote from './noteable_note.vue'; import noteHeader from './note_header.vue'; +import toggleRepliesWidget from './toggle_replies_widget.vue'; import noteSignedOutWidget from './note_signed_out_widget.vue'; import noteEditedText from './note_edited_text.vue'; import noteForm from './note_form.vue'; @@ -26,6 +26,7 @@ import tooltip from '../../vue_shared/directives/tooltip'; export default { name: 'NoteableDiscussion', components: { + icon, noteableNote, diffWithNote, userAvatarLink, @@ -33,6 +34,7 @@ export default { noteSignedOutWidget, noteEditedText, noteForm, + toggleRepliesWidget, placeholderNote, placeholderSystemNote, systemNote, @@ -46,11 +48,6 @@ export default { type: Object, required: true, }, - renderHeader: { - type: Boolean, - required: false, - default: true, - }, renderDiffFile: { type: Boolean, required: false, @@ -72,6 +69,7 @@ export default { isReplying: false, isResolving: false, resolveAsThread: true, + isRepliesCollapsed: (!this.discussion.diff_discussion && this.discussion.resolved) || false, }; }, computed: { @@ -112,6 +110,15 @@ export default { newNotePath() { return this.getNoteableData.create_note_path; }, + hasReplies() { + return this.discussion.notes.length > 1; + }, + initialDiscussion() { + return this.discussion.notes.slice(0, 1)[0]; + }, + replies() { + return this.discussion.notes.slice(1); + }, lastUpdatedBy() { const { notes } = this.discussion; @@ -147,6 +154,12 @@ export default { return diffDiscussion && diffFile && this.renderDiffFile; }, + shouldGroupReplies() { + return !this.shouldRenderDiffs && !this.transformedDiscussion.diffDiscussion; + }, + shouldRenderHeader() { + return this.shouldRenderDiffs; + }, wrapperComponent() { return this.shouldRenderDiffs ? diffWithNote : 'div'; }, @@ -160,6 +173,22 @@ export default { wrapperClass() { return this.isDiffDiscussion ? '' : 'card discussion-wrapper'; }, + componentClassName() { + if (this.shouldRenderDiffs) { + if (!this.lastUpdatedAt && !this.discussion.resolved) { + return 'unresolved'; + } + } + + return ''; + }, + shouldShowDiscussions() { + const isExpanded = this.discussion.expanded; + const { diffDiscussion, resolved } = this.transformedDiscussion; + const isResolvedNonDiffDiscussion = !diffDiscussion && resolved; + + return isExpanded || this.alwaysExpanded || isResolvedNonDiffDiscussion; + }, }, watch: { isReplying() { @@ -173,10 +202,6 @@ export default { } }, }, - created() { - this.resolveDiscussionsSvg = resolveDiscussionsSvg; - this.nextDiscussionsSvg = nextDiscussionsSvg; - }, methods: { ...mapActions([ 'saveNote', @@ -207,6 +232,9 @@ export default { toggleDiscussionHandler() { this.toggleDiscussion({ discussionId: this.discussion.id }); }, + toggleReplies() { + this.isRepliesCollapsed = !this.isRepliesCollapsed; + }, showReplyForm() { this.isReplying = true; }, @@ -274,26 +302,29 @@ Please check your network connection and try again.`; </script> <template> - <li class="note note-discussion timeline-entry"> + <li + class="note note-discussion timeline-entry" + :class="componentClassName" + > <div class="timeline-entry-inner"> - <div class="timeline-icon"> - <user-avatar-link - v-if="author" - :link-href="author.path" - :img-src="author.avatar_url" - :img-alt="author.name" - :img-size="40" - /> - </div> <div class="timeline-content"> <div :data-discussion-id="transformedDiscussion.discussion_id" class="discussion js-discussion-container" > <div - v-if="renderHeader" - class="discussion-header" + v-if="shouldRenderHeader" + class="discussion-header note-wrapper" > + <div class="timeline-icon"> + <user-avatar-link + v-if="author" + :link-href="author.path" + :img-src="author.avatar_url" + :img-alt="author.name" + :img-size="40" + /> + </div> <note-header :author="author" :created-at="transformedDiscussion.created_at" @@ -339,7 +370,7 @@ Please check your network connection and try again.`; /> </div> <div - v-if="discussion.expanded || alwaysExpanded" + v-if="shouldShowDiscussions" class="discussion-body"> <component :is="wrapperComponent" @@ -348,45 +379,70 @@ Please check your network connection and try again.`; > <div class="discussion-notes"> <ul class="notes"> - <component - :is="componentName(note)" - 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" + <template v-if="shouldGroupReplies"> + <component + :is="componentName(initialDiscussion)" + :note="componentData(initialDiscussion)" + @handleDeleteNote="deleteNoteHandler" > - </slot> - </component> + <slot + slot="avatar-badge" + name="avatar-badge" + > + </slot> + </component> + <toggle-replies-widget + v-if="hasReplies" + :collapsed="isRepliesCollapsed" + :replies="replies" + @toggle="toggleReplies" + /> + <template v-if="!isRepliesCollapsed"> + <component + :is="componentName(note)" + v-for="note in replies" + :key="note.id" + :note="componentData(note)" + @handleDeleteNote="deleteNoteHandler" + /> + </template> + </template> + <template v-else> + <component + :is="componentName(note)" + 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> + </template> </ul> <div + v-if="!isRepliesCollapsed" :class="{ 'is-replying': isReplying }" class="discussion-reply-holder" > <template v-if="!isReplying && canReply"> - <div - class="btn-group d-flex discussion-with-resolve-btn" - role="group"> - <div - class="btn-group w-100" - role="group"> - <button - type="button" - class="js-vue-discussion-reply btn btn-text-field mr-2 qa-discussion-reply" - title="Add a reply" - @click="showReplyForm">Reply...</button> - </div> - <div - v-if="discussion.resolvable" - class="btn-group" - role="group"> + <div class="discussion-with-resolve-btn"> + <button + type="button" + class="js-vue-discussion-reply btn btn-text-field mr-2 qa-discussion-reply" + title="Add a reply" + @click="showReplyForm" + > + Reply... + </button> + <div v-if="discussion.resolvable"> <button type="button" - class="btn btn-default" + class="btn btn-default mx-sm-2" @click="resolveHandler()" > <i @@ -414,7 +470,7 @@ Please check your network connection and try again.`; btn-default discussion-create-issue-btn" data-container="body" > - <span v-html="resolveDiscussionsSvg"></span> + <icon name="issue-new" /> </a> </div> <div @@ -428,7 +484,7 @@ Please check your network connection and try again.`; data-container="body" @click="jumpToNextDiscussion" > - <span v-html="nextDiscussionsSvg"></span> + <icon name="comment-next" /> </button> </div> </div> diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 40222ac4a80..e302a89ee95 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -173,7 +173,7 @@ export default { :class="classNameBindings" :data-award-url="note.toggle_award_path" :data-note-id="note.id" - class="note timeline-entry" + class="note timeline-entry note-wrapper" > <div class="timeline-entry-inner"> <div class="timeline-icon"> @@ -196,6 +196,7 @@ export default { :author="author" :created-at="note.created_at" :note-id="note.id" + action-text="commented" /> <note-actions :author-id="author.id" diff --git a/app/assets/javascripts/notes/components/toggle_replies_widget.vue b/app/assets/javascripts/notes/components/toggle_replies_widget.vue new file mode 100644 index 00000000000..78ecbbb9247 --- /dev/null +++ b/app/assets/javascripts/notes/components/toggle_replies_widget.vue @@ -0,0 +1,94 @@ +<script> +import _ from 'underscore'; +import Icon from '~/vue_shared/components/icon.vue'; +import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; +import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; + +export default { + components: { + Icon, + UserAvatarLink, + TimeAgoTooltip, + }, + props: { + collapsed: { + type: Boolean, + required: true, + }, + replies: { + type: Array, + required: true, + }, + }, + computed: { + lastReply() { + return this.replies[this.replies.length - 1]; + }, + uniqueAuthors() { + const authors = this.replies.map(reply => reply.author || {}); + + return _.uniq(authors, author => author.username); + }, + className() { + return this.collapsed ? 'collapsed' : 'expanded'; + }, + }, + methods: { + toggle() { + this.$emit('toggle'); + }, + }, +}; +</script> + +<template> + <li + :class="className" + class="replies-toggle" + > + <template v-if="collapsed"> + <icon + name="chevron-right" + @click.native="toggle" + /> + <div> + <user-avatar-link + v-for="author in uniqueAuthors" + :key="author.username" + :link-href="author.path" + :img-alt="author.name" + :img-src="author.avatar_url" + :img-size="26" + :tooltip-text="author.name" + tooltip-placement="bottom" + /> + </div> + <button + class="btn btn-link js-replies-text" + type="button" + @click="toggle" + > + {{ replies.length }} {{ n__('reply', 'replies', replies.length) }} + </button> + {{ __('Last reply by') }} + <a + :href="lastReply.author.path" + class="btn btn-link author-link" + > + {{ lastReply.author.name }} + </a> + <time-ago-tooltip + :time="lastReply.created_at" + tooltip-placement="bottom" + /> + </template> + <span + v-else + class="collapse-replies-btn js-collapse-replies" + @click="toggle" + > + <icon name="chevron-down" /> + {{ s__('Notes|Collapse replies') }} + </span> + </li> +</template> diff --git a/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue b/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue index f56414c3c63..8cb72afcdc0 100644 --- a/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/skeleton_note.vue @@ -10,7 +10,7 @@ export default { </script> <template> - <li class="timeline-entry note"> + <li class="timeline-entry note note-wrapper"> <div class="timeline-entry-inner"> <div class="timeline-icon"> </div> diff --git a/app/assets/javascripts/vue_shared/components/notes/system_note.vue b/app/assets/javascripts/vue_shared/components/notes/system_note.vue index de3c7a80365..6a44e6a29ed 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -76,7 +76,7 @@ export default { <li :id="noteAnchorId" :class="{ target: isTargetNote }" - class="note system-note timeline-entry"> + class="note system-note timeline-entry note-wrapper"> <div class="timeline-entry-inner"> <div class="timeline-icon" diff --git a/app/assets/stylesheets/framework/awards.scss b/app/assets/stylesheets/framework/awards.scss index 702276780e9..7a95db5976d 100644 --- a/app/assets/stylesheets/framework/awards.scss +++ b/app/assets/stylesheets/framework/awards.scss @@ -148,10 +148,7 @@ .award-control-icon svg { background: $award-emoji-positive-add-bg; - - path { - fill: $award-emoji-positive-add-lines; - } + fill: $award-emoji-positive-add-lines; } .award-control-icon-neutral { diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index c4296c7a88a..219fd99b097 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -207,6 +207,10 @@ @include btn-with-margin; } + &.btn-icon { + color: $gl-gray-700; + } + .fa-caret-down, .fa-chevron-down { margin-left: 5px; @@ -218,6 +222,25 @@ } } + &.btn-text-field { + width: 100%; + text-align: left; + padding: 6px 16px; + border-color: $border-color; + color: $gray-darkest; + background-color: $gray-light; + + &:hover, + &:active, + &:focus { + cursor: text; + box-shadow: none; + border-color: lighten($blue-300, 20%); + color: $gray-darkest; + background-color: $gray-light; + } + } + &.dot-highlight::after { content: ''; background-color: $blue-500; @@ -335,25 +358,6 @@ } } -.btn-text-field { - width: 100%; - text-align: left; - padding: 6px 16px; - border-color: $border-color; - color: $gray-darkest; - background-color: $gray-light; - - &:hover, - &:active, - &:focus { - cursor: text; - box-shadow: none; - border-color: lighten($blue-300, 20%); - color: $gray-darkest; - background-color: $gray-light; - } -} - .btn-build { margin-left: 10px; diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 53f198b47c6..6bdcb20210b 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -36,7 +36,6 @@ text-align: left; padding: 10px $gl-padding; word-wrap: break-word; - border-radius: $border-radius-default $border-radius-default 0 0; &.file-title-clear { padding-left: 0; diff --git a/app/assets/stylesheets/framework/timeline.scss b/app/assets/stylesheets/framework/timeline.scss index dfb145debe7..4a311da1675 100644 --- a/app/assets/stylesheets/framework/timeline.scss +++ b/app/assets/stylesheets/framework/timeline.scss @@ -1,7 +1,7 @@ .timeline { - @include basic-list; margin: 0; padding: 0; + list-style: none; &::before { @include notes-media('max', map-get($grid-breakpoints, sm)) { @@ -26,10 +26,8 @@ } .timeline-entry { - border-color: $white-normal; color: $gl-text-color; - border-bottom: 1px solid $border-white-light; - background: $white-light; + background-color: $white-light; .timeline-entry-inner { position: relative; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 19bc4262e21..6d998fa1e07 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -59,6 +59,7 @@ margin: 0; padding: 0; table-layout: fixed; + border-radius: 0 0 $border-radius-default $border-radius-default; .diff-line-num { width: 50px; @@ -859,7 +860,7 @@ } .diff-file .note-container > .new-note, -.note-container .discussion-notes { +.note-container .discussion-notes.diff-discussions { margin-left: 100px; border-left: 1px solid $white-normal; } diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index c60bb360a03..855d73a9939 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -239,6 +239,7 @@ .discussion-reply-holder { background-color: $white-light; padding: 10px 16px; + border-radius: 0 0 $border-radius-default $border-radius-default; &.is-replying { padding-bottom: $gl-padding; @@ -247,10 +248,15 @@ } .discussion-with-resolve-btn { + @include media-breakpoint-up(sm) { + display: flex; + } + + .discussion-actions { display: table; - .btn-default path { + svg { fill: $gray-darkest; } @@ -270,6 +276,12 @@ .btn { width: 100%; } + + .btn-text-field { + @include media-breakpoint-down(xs) { + margin-bottom: $gl-padding-8; + } + } } .discussion-notes-count { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index be535ade0a6..dcb1275d182 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -1,26 +1,135 @@ -/** - * Notes - */ +$system-note-icon-size: 32px; +$system-note-svg-size: 16px; +$note-form-margin-left: 70px; -@-webkit-keyframes targe3-note { - from { - background: $note-targe3-outside; +@mixin vertical-line($left) { + &::before { + content: ''; + border-left: 2px solid $theme-gray-100; + position: absolute; + top: 0; + bottom: 0; + left: $left; } +} - 50% { - background: $note-targe3-inside; - } +.note-wrapper { + padding: $gl-padding; +} + +.issuable-discussion { + .notes.timeline > .timeline-entry { + border: 1px solid $border-color; + border-radius: $border-radius-default; + margin: $gl-padding 0; + + &.system-note, + &.note-form { + border: 0; + } + + &.note-form { + margin-left: 0; - to { - background: $note-targe3-outside; + @include notes-media('min', map-get($grid-breakpoints, md)) { + margin-left: $note-form-margin-left; + } + + .timeline-icon { + @include notes-media('min', map-get($grid-breakpoints, sm)) { + margin-left: -$note-icon-gutter-width; + } + } + + .timeline-content { + margin-left: 0; + } + } + + .notes_content { + border: 0; + border-top: 1px solid $border-color; + } } } -ul.notes { +.main-notes-list { + @include vertical-line(39px); +} + +.notes { display: block; list-style: none; margin: 0; padding: 0; + position: relative; + + > .note-discussion { + .card { + border: 0; + } + + li.note { + border-bottom: 1px solid $border-color; + + &:first-child { + border-radius: $border-radius-default $border-radius-default 0 0; + } + } + } + + .replies-toggle { + background-color: $gray-light; + padding: $gl-padding-8 $gl-padding; + + .collapse-replies-btn:hover { + color: $blue-600; + } + + &.expanded { + border-bottom: 1px solid $border-color; + + span { + cursor: pointer; + } + + svg { + position: relative; + top: 3px; + } + } + + &.collapsed { + color: $gl-text-color-secondary; + + svg { + float: left; + position: relative; + top: $gl-padding-4; + margin-right: $gl-padding-8; + cursor: pointer; + } + + img { + margin: -2px 4px 0 0; + } + + .author-link { + color: $gl-text-color; + } + } + + .user-avatar-link { + &:last-child img { + margin-right: $gl-padding-8; + } + } + + .btn-link { + border: 0; + vertical-align: baseline; + } + } .note-created-ago, .note-updated-at { @@ -28,8 +137,6 @@ ul.notes { } .discussion-body { - padding-top: 8px; - .card { margin-bottom: 0; } @@ -46,21 +153,10 @@ ul.notes { } > li { - // .timeline-entry - padding: 0; display: block; position: relative; border-bottom: 0; - @include notes-media('min', map-get($grid-breakpoints, sm)) { - padding-left: $note-icon-gutter-width; - } - - .timeline-entry-inner { - padding: $gl-padding $gl-btn-padding; - border-bottom: 1px solid $white-normal; - } - &:target, &.target { border-bottom: 1px solid $white-normal; @@ -75,23 +171,10 @@ ul.notes { } } - .timeline-icon { - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: -$note-icon-gutter-width; - } - } - - .timeline-content { - margin-left: $note-icon-gutter-width; - - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: 0; - } - } - &.being-posted { pointer-events: none; opacity: 0.5; + padding: $gl-padding; .dummy-avatar { background-color: $gl-gray-200; @@ -104,12 +187,6 @@ ul.notes { } } - &.note-discussion { - .timeline-entry-inner { - padding: $gl-padding 10px; - } - } - .editing-spinner { display: none; } @@ -191,8 +268,9 @@ ul.notes { } .system-note { - font-size: 14px; - clear: both; + padding: 6px $gl-padding-24; + margin: $gl-padding-24 0; + background-color: transparent; .note-header-info { padding-bottom: 0; @@ -225,17 +303,21 @@ ul.notes { .timeline-icon { float: left; - - @include notes-media('min', map-get($grid-breakpoints, sm)) { - margin-left: 0; - width: auto; - } + display: flex; + align-items: center; + background-color: $white-light; + width: $system-note-icon-size; + height: $system-note-icon-size; + border: 1px solid $border-color; + border-radius: $system-note-icon-size; + margin: -6px $gl-padding 0 0; svg { - width: 16px; - height: 16px; + width: $system-note-svg-size; + height: $system-note-svg-size; fill: $gray-darkest; - margin-top: 2px; + display: block; + margin: 0 auto; } } @@ -302,10 +384,17 @@ ul.notes { .discussion-body .diff-file { .file-title { cursor: default; + line-height: 42px; + padding: 0 $gl-padding; + border-top: 1px solid $border-color; &:hover { background-color: $gray-light; } + + .btn-clipboard { + top: 10px; + } } .line_content { @@ -320,6 +409,23 @@ ul.notes { } } + .discussion-notes { + &:not(:first-child) { + border-top: 1px solid $white-normal; + margin-top: 20px; + } + + &:not(:last-child) { + border-bottom: 1px solid $white-normal; + margin-bottom: 20px; + } + + .system-note { + margin: 0; + padding: $gl-padding; + } + } + // Merge request notes in diffs // Diff is inline .notes_content .note-header .note-headline-light { @@ -335,7 +441,6 @@ ul.notes { border-left: 0; &.notes_content { - background-color: $gray-light; border-width: 1px 0; padding: 0; vertical-align: top; @@ -349,18 +454,6 @@ ul.notes { } } - .discussion-notes { - &:not(:first-child) { - border-top: 1px solid $white-normal; - margin-top: 20px; - } - - &:not(:last-child) { - border-bottom: 1px solid $white-normal; - margin-bottom: 20px; - } - } - .notes { background-color: $white-light; } @@ -374,6 +467,30 @@ ul.notes { } } +.diffs { + .discussion-notes { + margin-left: 0; + border-left: 0; + + .notes { + position: relative; + @include vertical-line(52px); + } + } + + .note-wrapper { + margin: $gl-padding; + border: 1px solid $border-color; + border-radius: $border-radius-default; + } + + .discussion-reply-holder { + border-radius: 0 0 $border-radius-default $border-radius-default; + border-top: 1px solid $border-color; + position: relative; + } +} + .discussion-header, .note-header-info { a { @@ -399,7 +516,17 @@ ul.notes { } .discussion-header { - font-size: 14px; + min-height: 72px; + + .note-header-info { + padding-bottom: 0; + } +} + +.unresolved { + .note-header-info { + margin-top: $gl-padding-8; + } } .note-header { @@ -409,7 +536,7 @@ ul.notes { .note-header-info { min-width: 0; - padding-bottom: 8px; + padding-bottom: $gl-padding-8; &.discussion { padding-bottom: 0; @@ -471,9 +598,18 @@ ul.notes { margin-left: 10px; color: $gray-darkest; + @include media-breakpoint-down(xs) { + width: 100%; + margin: $gl-padding-8 0; + } + .btn-group > .discussion-next-btn { margin-left: -1px; } + + svg { + height: 15px; + } } .note-actions { @@ -585,19 +721,6 @@ ul.notes { z-index: 10; } -.discussion-body, -.diff-file { - .notes .note { - border-bottom: 1px solid $white-normal; - - .timeline-entry-inner { - padding-left: $gl-padding; - padding-right: $gl-padding; - border-bottom: 0; - } - } -} - .disabled-comment { background-color: $gray-light; border-radius: $border-radius-base; @@ -634,7 +757,7 @@ ul.notes { } .btn { - svg path { + svg { fill: $gray-darkest; } @@ -659,7 +782,7 @@ ul.notes { .line-resolve-all { vertical-align: middle; display: inline-block; - padding: 5px 10px 6px; + padding: 6px 10px; background-color: $gray-light; border: 1px solid $border-color; border-radius: $border-radius-default; diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d9713f9c9b0..0a7f930110a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -540,4 +540,13 @@ module ProjectsHelper network ] end + + def sidebar_operations_paths + %w[ + environments + clusters + user + gcp + ] + end end diff --git a/app/models/project_import_state.rb b/app/models/project_import_state.rb index d59cb43dea4..7126bb66d80 100644 --- a/app/models/project_import_state.rb +++ b/app/models/project_import_state.rb @@ -56,4 +56,17 @@ class ProjectImportState < ActiveRecord::Base end end end + + def mark_as_failed(error_message) + original_errors = errors.dup + sanitized_message = Gitlab::UrlSanitizer.sanitize(error_message) + + fail_op + + update_column(:last_error, sanitized_message) + rescue ActiveRecord::ActiveRecordError => e + Rails.logger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}") + ensure + @errors = original_errors + end end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 174033f3d49..ab15889a465 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -195,7 +195,7 @@ = _('Charts') - if project_nav_tab? :operations - = nav_link(controller: [:environments, :clusters, :user, :gcp]) do + = nav_link(controller: sidebar_operations_paths) do = link_to metrics_project_environments_path(@project), class: 'shortcuts-operations' do .nav-icon-container = sprite_icon('cloud-gear') @@ -203,7 +203,7 @@ = _('Operations') %ul.sidebar-sub-level-items - = nav_link(controller: [:environments, :clusters, :user, :gcp], html_options: { class: "fly-out-top-item" } ) do + = nav_link(controller: sidebar_operations_paths, html_options: { class: "fly-out-top-item" } ) do = link_to metrics_project_environments_path(@project) do %strong.fly-out-top-item-name = _('Operations') @@ -215,6 +215,8 @@ %span = _('Metrics') + = render_if_exists "layouts/nav/sidebar/tracing_link" + = nav_link(controller: :environments, action: [:index, :folder, :show, :new, :edit, :create, :update, :stop, :terminal]) do = link_to project_environments_path(@project), title: _('Environments'), class: 'shortcuts-environments qa-operations-environments-link' do %span @@ -326,6 +328,7 @@ = link_to project_settings_ci_cd_path(@project), title: _('CI / CD') do %span = _('CI / CD') + = render_if_exists 'projects/sidebar/settings_operations' - if @project.pages_available? = nav_link(controller: :pages) do = link_to project_pages_path(@project), title: _('Pages') do diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml index 9dd1c24fdfa..ec1e10bb0c1 100644 --- a/app/views/shared/notes/_notes_with_form.html.haml +++ b/app/views/shared/notes/_notes_with_form.html.haml @@ -7,8 +7,8 @@ = render 'shared/notes/edit_form', project: @project - if can_create_note? - %ul.notes.notes-form.timeline - %li.timeline-entry + .notes.notes-form.timeline + .timeline-entry .timeline-entry-inner .flash-container.timeline-content diff --git a/app/workers/stuck_import_jobs_worker.rb b/app/workers/stuck_import_jobs_worker.rb index de92f3eca6a..667a4121131 100644 --- a/app/workers/stuck_import_jobs_worker.rb +++ b/app/workers/stuck_import_jobs_worker.rb @@ -7,60 +7,58 @@ class StuckImportJobsWorker IMPORT_JOBS_EXPIRATION = 15.hours.to_i def perform - projects_without_jid_count = mark_projects_without_jid_as_failed! - projects_with_jid_count = mark_projects_with_jid_as_failed! + import_state_without_jid_count = mark_import_states_without_jid_as_failed! + import_state_with_jid_count = mark_import_states_with_jid_as_failed! Gitlab::Metrics.add_event(:stuck_import_jobs, - projects_without_jid_count: projects_without_jid_count, - projects_with_jid_count: projects_with_jid_count) + projects_without_jid_count: import_state_without_jid_count, + projects_with_jid_count: import_state_with_jid_count) end private - def mark_projects_without_jid_as_failed! - enqueued_projects_without_jid.each do |project| - project.mark_import_as_failed(error_message) + def mark_import_states_without_jid_as_failed! + enqueued_import_states_without_jid.each do |import_state| + import_state.mark_as_failed(error_message) end.count end # rubocop: disable CodeReuse/ActiveRecord - def mark_projects_with_jid_as_failed! - # TODO: Rollback this change to use SQL through #pluck - jids_and_ids = enqueued_projects_with_jid.map { |project| [project.import_jid, project.id] }.to_h + def mark_import_states_with_jid_as_failed! + jids_and_ids = enqueued_import_states_with_jid.pluck(:jid, :id).to_h # Find the jobs that aren't currently running or that exceeded the threshold. completed_jids = Gitlab::SidekiqStatus.completed_jids(jids_and_ids.keys) return unless completed_jids.any? - completed_project_ids = jids_and_ids.values_at(*completed_jids) + completed_import_state_ids = jids_and_ids.values_at(*completed_jids) - # We select the projects again, because they may have transitioned from + # We select the import states again, because they may have transitioned from # scheduled/started to finished/failed while we were looking up their Sidekiq status. - completed_projects = enqueued_projects_with_jid.where(id: completed_project_ids) + completed_import_states = enqueued_import_states_with_jid.where(id: completed_import_state_ids) - Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_projects.map(&:import_jid).join(', ')}") + completed_import_state_jids = completed_import_states.map { |import_state| import_state.jid }.join(', ') + Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_import_state_jids}") - completed_projects.each do |project| - project.mark_import_as_failed(error_message) + completed_import_states.each do |import_state| + import_state.mark_as_failed(error_message) end.count end # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord - def enqueued_projects - Project.joins_import_state.where("(import_state.status = 'scheduled' OR import_state.status = 'started') OR (projects.import_status = 'scheduled' OR projects.import_status = 'started')") + def enqueued_import_states + ProjectImportState.with_status([:scheduled, :started]) end - # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord - def enqueued_projects_with_jid - enqueued_projects.where.not("import_state.jid IS NULL AND projects.import_jid IS NULL") + def enqueued_import_states_with_jid + enqueued_import_states.where.not(jid: nil) end # rubocop: enable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord - def enqueued_projects_without_jid - enqueued_projects.where("import_state.jid IS NULL AND projects.import_jid IS NULL") + def enqueued_import_states_without_jid + enqueued_import_states.where(jid: nil) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/changelogs/unreleased/fix-stuck-import-jobs-query-performance-issue.yml b/changelogs/unreleased/fix-stuck-import-jobs-query-performance-issue.yml new file mode 100644 index 00000000000..d8455a8509f --- /dev/null +++ b/changelogs/unreleased/fix-stuck-import-jobs-query-performance-issue.yml @@ -0,0 +1,5 @@ +--- +title: Improves performance of stuck import jobs detection +merge_request: 22879 +author: +type: performance diff --git a/doc/administration/auth/okta.md b/doc/administration/auth/okta.md index 664657650d4..ae38094391b 100644 --- a/doc/administration/auth/okta.md +++ b/doc/administration/auth/okta.md @@ -11,7 +11,7 @@ The following documentation enables Okta as a SAML provider. 1. When the app screen comes up you see another button to **Create an App** and choose SAML 2.0 on the next screen. 1. Now, very important, add a logo - (you can choose it from https://about.gitlab.com/press/). You'll have to + (you can choose it from <https://about.gitlab.com/press/>). You'll have to crop and resize it. 1. Next, you'll need the to fill in the SAML general config. Here's an example image. diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index 890b780fe80..cfe7b0e05e3 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -71,7 +71,7 @@ A Registry init file is not shipped with GitLab if you install it from source. Hence, [restarting GitLab][restart gitlab] will not restart the Registry should you modify its settings. Read the upstream documentation on how to achieve that. -At the absolute minimum, make sure your [Registry configuration][registry-auth] +At the **absolute** minimum, make sure your [Registry configuration][registry-auth] has `container_registry` as the service and `https://gitlab.example.com/jwt/auth` as the realm: @@ -84,6 +84,9 @@ auth: rootcertbundle: /root/certs/certbundle ``` +CAUTION: **Caution:** +If `auth` is not set up, users will be able to pull docker images without authentication. + ## Container Registry domain configuration There are two ways you can configure the Registry's external domain. diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 5700f640e4c..c9a2778b3a4 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -45,7 +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 | +| upload_file_does_not_exist | Counter | 10.7 in EE, 11.5 in CE | 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/install/database_mysql.md b/doc/install/database_mysql.md index acaed53e052..4cb8ca4f3e7 100644 --- a/doc/install/database_mysql.md +++ b/doc/install/database_mysql.md @@ -1,15 +1,20 @@ # Database MySQL -> **Note:** -> - We do not recommend using MySQL due to various issues. For example, case - [(in)sensitivity](https://dev.mysql.com/doc/refman/5.0/en/case-sensitivity.html) - and [problems](https://bugs.mysql.com/bug.php?id=65830) that - [suggested](https://bugs.mysql.com/bug.php?id=50909) - [fixes](https://bugs.mysql.com/bug.php?id=65830) [have](https://bugs.mysql.com/bug.php?id=63164). +NOTE: **Note:** +We do not recommend using MySQL due to various issues. +For example, there have been bugs with case +[(in)sensitivity](https://dev.mysql.com/doc/refman/5.7/en/case-sensitivity.html). + +Bugs relating to case sensitivity: + +- <https://bugs.mysql.com/bug.php?id=65830> +- <https://bugs.mysql.com/bug.php?id=50909> +- <https://bugs.mysql.com/bug.php?id=65830> +- <https://bugs.mysql.com/bug.php?id=63164> ## Initial database setup -``` +```sh # Install the database packages sudo apt-get install -y mysql-server mysql-client libmysqlclient-dev @@ -84,8 +89,9 @@ GitLab 8.14 has introduced [a feature](https://gitlab.com/gitlab-org/gitlab-ce/m Follow the below instructions to ensure you use the most up to date requirements for your GitLab MySQL Database. **We are about to do the following:** + - Ensure you can enable `utf8mb4` encoding and `utf8mb4_general_ci` collation for your GitLab DB, tables and data. -- Convert your GitLab tables and data from `utf8`/`utf8_general_ci` to `utf8mb4`/`utf8mb4_general_ci` +- Convert your GitLab tables and data from `utf8`/`utf8_general_ci` to `utf8mb4`/`utf8mb4_general_ci`. ### Check for utf8mb4 support @@ -130,7 +136,8 @@ We need to check, enable and maybe convert your existing GitLab DB tables to the Whatever the results of your checks above, we now need to check if your GitLab database has been created using [InnoDB File-Per-Table Tablespaces](http://dev.mysql.com/doc/refman/5.7/en/innodb-multiple-tablespaces.html) (i.e. `innodb_file_per_table` was set to **1** at initial setup time). -> Note: This setting is [enabled by default](http://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_file_per_table) since MySQL 5.6.6. +NOTE: **Note:** +This setting is [enabled by default](http://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_file_per_table) since MySQL 5.6.6. # Run this command with root privileges, replace the data dir if different: sudo ls -lh /var/lib/mysql/gitlabhq_production/*.ibd | wc -l @@ -138,20 +145,19 @@ Whatever the results of your checks above, we now need to check if your GitLab d # Run this command with root privileges, replace the data dir if different: sudo ls -lh /var/lib/mysql/gitlabhq_production/*.frm | wc -l - - **Case 1: a result > 0 for both commands** -Congrats, your GitLab database uses the right InnoDB tablespace format. +Congratulations, your GitLab database uses the right InnoDB tablespace format. However, you must still ensure that any **future tables** created by GitLab will still use the right format: - If `SELECT @@innodb_file_per_table` returned **1** previously, your server is running correctly. - > It's however a requirement to check *now* that this setting is indeed persisted in your [my.cnf](https://dev.mysql.com/doc/refman/5.7/en/tablespace-enabling.html) file! + > It's however a requirement to check *now* that this setting is indeed persisted in your [`my.cnf`](https://dev.mysql.com/doc/refman/5.7/en/innodb-multiple-tablespaces.html) file! - If `SELECT @@innodb_file_per_table` returned **0** previously, your server is not running correctly. - > [Enable innodb_file_per_table](https://dev.mysql.com/doc/refman/5.7/en/tablespace-enabling.html) by running in a MySQL session as root the command `SET GLOBAL innodb_file_per_table=1, innodb_file_format=Barracuda;` and persist the two settings in your [my.cnf](https://dev.mysql.com/doc/refman/5.7/en/tablespace-enabling.html) file + > [Enable innodb_file_per_table](https://dev.mysql.com/doc/refman/5.7/en/innodb-multiple-tablespaces.html) by running in a MySQL session as root the command `SET GLOBAL innodb_file_per_table=1, innodb_file_format=Barracuda;` and persist the two settings in your [`my.cnf`](https://dev.mysql.com/doc/refman/5.7/en/innodb-multiple-tablespaces.html) file. Now, if you have a **different result** returned by the 2 commands above, it means you have a **mix of tables format** uses in your GitLab database. This can happen if your MySQL server had different values for `innodb_file_per_table` in its life and you updated GitLab at different moments with those inconsistent values. So keep reading. @@ -172,7 +178,7 @@ Let's enable what we need on the running server: # You can now quit the database session mysql> \q -> Now, **persist** [innodb_file_per_table](https://dev.mysql.com/doc/refman/5.6/en/tablespace-enabling.html) and [innodb_file_format](https://dev.mysql.com/doc/refman/5.6/en/innodb-file-format-enabling.html) in your `my.cnf` file. +> Now, **persist** [innodb_file_per_table](https://dev.mysql.com/doc/refman/5.7/en/innodb-multiple-tablespaces.html) and [innodb_file_format](https://dev.mysql.com/doc/refman/5.7/en/innodb-file-format-enabling.html) in your `my.cnf` file. Ensure at this stage that your GitLab instance is indeed **stopped**. @@ -184,7 +190,7 @@ Now, let's convert all the GitLab database tables to the new tablespace format: # Type the MySQL root password mysql > use gitlabhq_production; - # Safety check: you should still have those values set as follow: + # Safety check: you should still have those values set as follows: mysql> SELECT @@innodb_file_per_table, @@innodb_file_format; +-------------------------+----------------------+ | @@innodb_file_per_table | @@innodb_file_format | @@ -203,7 +209,7 @@ Now, let's convert all the GitLab database tables to the new tablespace format: #### Check for proper InnoDB File Format, Row Format, Large Prefix and tables conversion -We need to check, enable and probably convert your existing GitLab DB tables to use the [Barracuda InnoDB file format](https://dev.mysql.com/doc/refman/5.6/en/innodb-file-format.html), the [DYNAMIC row format](https://dev.mysql.com/doc/refman/5.6/en/glossary.html#glos_dynamic_row_format) and [innodb_large_prefix](http://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_large_prefix) as a second prerequisite for supporting **utfb8mb4 with long indexes** used by recent GitLab databases. +We need to check, enable and probably convert your existing GitLab DB tables to use the [Barracuda InnoDB file format](https://dev.mysql.com/doc/refman/5.7/en/innodb-file-format.html), the [DYNAMIC row format](https://dev.mysql.com/doc/refman/5.7/en/glossary.html#glos_dynamic_row_format) and [innodb_large_prefix](http://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_large_prefix) as a second prerequisite for supporting **utfb8mb4 with long indexes** used by recent GitLab databases. # Login to MySQL mysql -u root -p @@ -229,7 +235,7 @@ We need to check, enable and probably convert your existing GitLab DB tables to | utf8 | utf8_general_ci | +--------------------------+----------------------+ -> Now, ensure that [innodb_file_format](https://dev.mysql.com/doc/refman/5.6/en/tablespace-enabling.html) and [innodb_large_prefix](http://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_large_prefix) are **persisted** in your `my.cnf` file. +> Now, ensure that [innodb_file_format](https://dev.mysql.com/doc/refman/5.7/en/innodb-multiple-tablespaces.html) and [innodb_large_prefix](http://dev.mysql.com/doc/refman/5.7/en/innodb-parameters.html#sysvar_innodb_large_prefix) are **persisted** in your `my.cnf` file. #### Tables and data conversion to utf8mb4 @@ -257,7 +263,7 @@ Now that you have a persistent MySQL setup, you can safely upgrade tables after Ensure your GitLab database configuration file uses a proper connection encoding and collation: -```sudo -u git -H editor config/database.yml``` +`sudo -u git -H editor config/database.yml` production: adapter: mysql2 @@ -266,19 +272,19 @@ Ensure your GitLab database configuration file uses a proper connection encoding [Restart your GitLab instance](../administration/restart_gitlab.md). - ## MySQL strings limits After installation or upgrade, remember to run the `add_limits_mysql` Rake task: **Omnibus GitLab installations** -``` + +```sh sudo gitlab-rake add_limits_mysql ``` **Installations from source** -``` +```sh bundle exec rake add_limits_mysql RAILS_ENV=production ``` diff --git a/doc/raketasks/import.md b/doc/raketasks/import.md index 97e9b36d1a6..bb316df5b9a 100644 --- a/doc/raketasks/import.md +++ b/doc/raketasks/import.md @@ -6,6 +6,7 @@ - The groups will be created as needed, including subgroups - The owner of the group will be the first admin - Existing projects will be skipped +- Projects in hashed storage may be skipped (see [Importing bare repositories from hashed storage](#importing-bare-repositories-from-hashed-storage)) - The existing Git repos will be moved from disk (removed from the original path) ## How to use @@ -26,7 +27,6 @@ sudo -u git mkdir /var/opt/gitlab/git-data/repository-import-<date>/new_group If we copy the repos to `/var/opt/gitlab/git-data/repository-import-<date>`, and repo A needs to be under the groups G1 and G2, it will have to be created under those folders: `/var/opt/gitlab/git-data/repository-import-<date>/G1/G2/A.git`. - ``` sudo cp -r /old/git/foo.git /var/opt/gitlab/git-data/repository-import-<date>/new_group/ @@ -70,3 +70,73 @@ Processing /var/opt/gitlab/git-data/repository-import-1/group/xyz.git * Skipping repo /var/opt/gitlab/git-data/repository-import-1/@shared/a/b/abcd.git [...] ``` + +## Importing bare repositories from hashed storage + +### Background + +Projects in legacy storage have a directory structure that mirrors their full +project path in GitLab, including their namespace structure. This information is +leveraged by the bare repository importer to import projects into their proper +locations. Each project and its parent namespaces are meaningfully named. + +However, the directory structure of projects in hashed storage do not contain +this information. This is beneficial for a variety of reasons, especially +improved performance and data integrity. See +[Repository Storage Types](../administration/repository_storage_types.md) for +more details. + +### Which repositories are importable? + +#### GitLab 10.3 or earlier + +Importing bare repositories from hashed storage is unsupported. + +#### GitLab 10.4 and later + +To support importing bare repositories from hashed storage, GitLab 10.4 and +later stores the full project path with each repository, in a special section of +the git repository's config file. This section is formatted as follows: + +``` +[gitlab] + fullpath = gitlab-org/gitlab-ce +``` + +However, existing repositories were not migrated to include this path. + +Bare repositories are importable if the following events occurred to the +repository in GitLab 10.4 and later: + +- Created +- Migrated to hashed storage +- Renamed +- Transferred to another namespace +- Ancestor renamed +- Ancestor transferred to another namespace + +Bare repositories are **not** importable by GitLab 10.4 and later when all the following are true about the repository: + +- It was created in GitLab 10.3 or earlier. +- It was not renamed, transferred, or migrated to hashed storage in GitLab 10.4 and later. +- Its ancestor namespaces were not renamed or transferred in GitLab 10.4 and later. + +There is an [open issue to add a migration to make all bare repositories +importable](https://gitlab.com/gitlab-org/gitlab-ce/issues/41776). + +Until then, you may wish to manually migrate repositories yourself. You can use +[Rails console](https://docs.gitlab.com/omnibus/maintenance/#starting-a-rails-console-session) +to do so. In a Rails console session, run the following to migrate a project: + +``` +project = Project.find_by_full_path('gitlab-org/gitlab-ce') +project.write_repository_config +``` + +In a Rails console session, run the following to migrate all of a namespace's +projects (this may take a while if there are 1000s of projects in a namespace): + +``` +namespace = Namespace.find_by_full_path('gitlab-org') +namespace.send(:write_projects_repository_config) +``` diff --git a/doc/university/training/topics/additional_resources.md b/doc/university/training/topics/additional_resources.md index d01634df744..4871372d105 100644 --- a/doc/university/training/topics/additional_resources.md +++ b/doc/university/training/topics/additional_resources.md @@ -4,9 +4,9 @@ comments: false # Additional Resources -1. GitLab Documentation [http://docs.gitlab.com](http://docs.gitlab.com/) -2. GUI Clients [http://git-scm.com/downloads/guis](http://git-scm.com/downloads/guis) -3. Pro git book [http://git-scm.com/book](http://git-scm.com/book) -4. Platzi Course [https://courses.platzi.com/courses/git-gitlab/](https://courses.platzi.com/courses/git-gitlab/) -5. Code School tutorial [http://try.github.io/](http://try.github.io/) -6. Contact Us at `subscribers@gitlab.com` +1. GitLab Documentation: <http://docs.gitlab.com>. +1. GUI Clients: <http://git-scm.com/downloads/guis>. +1. Pro Git book: <http://git-scm.com/book>. +1. Platzi Course: <https://courses.platzi.com/courses/git-gitlab/>. +1. Code School tutorial: <http://try.github.io/>. +1. Contact us at `subscribers@gitlab.com`. diff --git a/doc/university/training/topics/tags.md b/doc/university/training/topics/tags.md index 6333ceedbd7..9526bcbfb82 100644 --- a/doc/university/training/topics/tags.md +++ b/doc/university/training/topics/tags.md @@ -22,7 +22,7 @@ comments: false **Additional resources** -[http://git-scm.com/book/en/Git-Basics-Tagging](http://git-scm.com/book/en/Git-Basics-Tagging) +<http://git-scm.com/book/en/Git-Basics-Tagging> ---------- diff --git a/doc/university/training/user_training.md b/doc/university/training/user_training.md index f3a4d766522..ca3f777f403 100644 --- a/doc/university/training/user_training.md +++ b/doc/university/training/user_training.md @@ -385,7 +385,7 @@ Thank you for your hard work! - GitLab Documentation: <http://docs.gitlab.com/>. - GUI Clients: <http://git-scm.com/downloads/guis>. -- Pro git book: <http://git-scm.com/book>. +- Pro Git book: <http://git-scm.com/book>. - Platzi Course: <https://courses.platzi.com/courses/git-gitlab/>. - Code School tutorial: <http://try.github.io/>. - Contact us at `subscribers@gitlab.com`. diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 94744cf8500..233ed205790 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -132,59 +132,62 @@ functionalities needed to successfully build and deploy a containerized application. Bare in mind that the same credentials are used for all the applications running on the cluster. -When GitLab creates the cluster, it enables and uses the legacy -[Attribute-based access control (ABAC)](https://kubernetes.io/docs/admin/authorization/abac/). -The newer [RBAC](https://kubernetes.io/docs/admin/authorization/rbac/) -authorization is [experimental](#role-based-access-control-rbac). - -### Role-based access control (RBAC) **[CORE ONLY]** - -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21401) in GitLab 11.4. - -CAUTION: **Warning:** -The RBAC authorization is experimental. - -Once RBAC is enabled for a cluster, GitLab will create the necessary service accounts -and privileges in order to install and run [GitLab managed applications](#installing-applications). - -If you are creating a [new GKE cluster via -GitLab](#adding-and-creating-a-new-gke-cluster-via-gitlab), you will be -asked if you would like to create an RBAC-enabled cluster. Enabling this -setting will create a `gitlab` service account which will be used by -GitLab to manage the newly created cluster. To enable this, this service -account will have the `cluster-admin` privilege. - -If you are [adding an existing Kubernetes -cluster](#adding-an-existing-kubernetes-cluster), you will be asked if -the cluster you are adding is a RBAC-enabled cluster. Ensure the -token of the account has administrator privileges for the cluster. - -In both cases above, when you install Helm Tiller into your cluster, an -RBAC-enabled cluster will create a `tiller` service account, with `cluster-admin` -privileges in the `gitlab-managed-apps` namespace. This service account will be -added to the installed Helm Tiller and will be used by Helm to install and run -[GitLab managed applications](#installing-applications). - -The table below summarizes which resources will be created in a -RBAC-enabled cluster : - -| Name | Kind | Details | Created when | -| --- | --- | --- | --- | -| `gitlab` | `ServiceAccount` | `default` namespace | Creating a new GKE Cluster | -| `gitlab-admin` | `ClusterRoleBinding` | `cluster-admin` roleRef | Creating a new GKE Cluster | -| `gitlab-token` | `Secret` | Token for `gitlab` ServiceAccount | Creating a new GKE Cluster | -| `tiller` | `ServiceAccount` | `gitlab-managed-apps` namespace | Installing Helm Tiller | -| `tiller-admin` | `ClusterRoleBinding` | `cluster-admin` roleRef | Installing Helm Tiller | - - -Helm Tiller will also create additional service accounts and other RBAC -resources for each installed application. Consult the documentation for the -Helm charts for each application for details. - -NOTE: **Note:** -Auto DevOps will not successfully complete in a cluster that only has RBAC -authorization enabled. RBAC support for Auto DevOps is planned in a -[future release](https://gitlab.com/gitlab-org/gitlab-ce/issues/44597). +## Access controls + +When creating a cluster in GitLab, you will be asked if you would like to create an +[Attribute-based access control (ABAC)](https://kubernetes.io/docs/admin/authorization/abac/) cluster, or +a [Role-based access control (RBAC)](https://kubernetes.io/docs/admin/authorization/rbac/) one. + +Whether ABAC or RBAC is enabled, GitLab will create the necessary +service accounts and privileges in order to install and run +[GitLab managed applications](#installing-applications): + +- A `gitlab` service account with `cluster-admin` privileges will be created in the + `default` namespace, which will be used by GitLab to manage the newly created cluster. + +- A project service account with `edit` privileges will be created in + the project namespace (also created by GitLab), which will be used in + [deployment jobs](#deployment-variables). + + NOTE: **Note:** + Restricted service account for deployment was [introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/51716) in GitLab 11.5. + +- When you install Helm Tiller into your cluster, the `tiller` service account + will be created with `cluster-admin` privileges in the `gitlab-managed-apps` + namespace. This service account will be added to the installed Helm Tiller and will + be used by Helm to install and run [GitLab managed applications](#installing-applications). + Helm Tiller will also create additional service accounts and other resources for each + installed application. Consult the documentation of the Helm charts for each application + for details. + +If you are [adding an existing Kubernetes cluster](#adding-an-existing-kubernetes-cluster), +ensure the token of the account has administrator privileges for the cluster. + +The following sections summarize which resources will be created on ABAC/RBAC clusters. + +### Attribute-based access control (ABAC) + +| Name | Kind | Details | Created when | +| --- | --- | --- | --- | +| `gitlab` | `ServiceAccount` | `default` namespace | Creating a new GKE Cluster | +| `gitlab-token` | `Secret` | Token for `gitlab` ServiceAccount | Creating a new GKE Cluster | +| `tiller` | `ServiceAccount` | `gitlab-managed-apps` namespace | Installing Helm Tiller | +| `tiller-admin` | `ClusterRoleBinding` | `cluster-admin` roleRef | Installing Helm Tiller | +| Project namespace | `ServiceAccount` | Uses namespace of Project | Creating/Adding a new GKE Cluster | +| Project namespace | `Secret` | Token for project ServiceAccount | Creating/Adding a new GKE Cluster | + +### Role-based access control (RBAC) + +| Name | Kind | Details | Created when | +| --- | --- | --- | --- | +| `gitlab` | `ServiceAccount` | `default` namespace | Creating a new GKE Cluster | +| `gitlab-admin` | `ClusterRoleBinding` | [`cluster-admin`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) roleRef | Creating a new GKE Cluster | +| `gitlab-token` | `Secret` | Token for `gitlab` ServiceAccount | Creating a new GKE Cluster | +| `tiller` | `ServiceAccount` | `gitlab-managed-apps` namespace | Installing Helm Tiller | +| `tiller-admin` | `ClusterRoleBinding` | `cluster-admin` roleRef | Installing Helm Tiller | +| Project namespace | `ServiceAccount` | Uses namespace of Project | Creating/Adding a new GKE Cluster | +| Project namespace | `Secret` | Token for project ServiceAccount | Creating/Adding a new GKE Cluster | +| Project namespace | `RoleBinding` | [`edit`](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles) roleRef | Creating/Adding a new GKE Cluster | ### Security of GitLab Runners @@ -387,12 +390,16 @@ GitLab CI/CD build environment. | Variable | Description | | -------- | ----------- | | `KUBE_URL` | Equal to the API URL. | -| `KUBE_TOKEN` | The Kubernetes token. | +| `KUBE_TOKEN` | The Kubernetes token of the [project service account](#access-controls). | | `KUBE_NAMESPACE` | The Kubernetes namespace is auto-generated if not specified. The default value is `<project_name>-<project_id>`. You can overwrite it to use different one if needed, otherwise the `KUBE_NAMESPACE` variable will receive the default value. | -| `KUBE_CA_PEM_FILE` | Only present if a custom CA bundle was specified. Path to a file containing PEM data. | -| `KUBE_CA_PEM` | (**deprecated**) Only if a custom CA bundle was specified. Raw PEM data. | +| `KUBE_CA_PEM_FILE` | Path to a file containing PEM data. Only present if a custom CA bundle was specified. | +| `KUBE_CA_PEM` | (**deprecated**) Raw PEM data. Only if a custom CA bundle was specified. | | `KUBECONFIG` | Path to a file containing `kubeconfig` for this deployment. CA bundle would be embedded if specified. | +NOTE: **NOTE:** +Prior to GitLab 11.5, `KUBE_TOKEN` was the Kubernetes token of the main +service account of the cluster integration. + ## Enabling or disabling the Kubernetes cluster integration After you have successfully added your cluster information, you can enable the diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 3182ffb27b9..3d66b092143 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3591,6 +3591,9 @@ msgstr "" msgid "Last edited by %{name}" msgstr "" +msgid "Last reply by" +msgstr "" + msgid "Last update" msgstr "" @@ -4192,6 +4195,9 @@ msgstr "" msgid "Notes|Are you sure you want to cancel creating this comment?" msgstr "" +msgid "Notes|Collapse replies" +msgstr "" + msgid "Notes|Show all activity" msgstr "" @@ -7568,6 +7574,11 @@ msgstr "" msgid "remove due date" msgstr "" +msgid "reply" +msgid_plural "replies" +msgstr[0] "" +msgstr[1] "" + msgid "source" msgstr "" diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb index 27a88534258..7f959441dac 100644 --- a/qa/qa/git/repository.rb +++ b/qa/qa/git/repository.rb @@ -109,6 +109,28 @@ module QA known_hosts_file.close(true) end + def push_with_git_protocol(version, file_name, file_content, commit_message = 'Initial commit') + self.git_protocol = version + add_file(file_name, file_content) + commit(commit_message) + push_changes + + fetch_supported_git_protocol + end + + def git_protocol=(value) + raise ArgumentError, "Please specify the protocol you would like to use: 0, 1, or 2" unless %w[0 1 2].include?(value.to_s) + + run("git config protocol.version #{value}") + end + + def fetch_supported_git_protocol + # ls-remote is one command known to respond to Git protocol v2 so we use + # it to get output including the version reported via Git tracing + output = run("git ls-remote #{uri}", "GIT_TRACE_PACKET=1") + output[/git< version (\d+)/, 1] || 'unknown' + end + private attr_reader :uri, :username, :password, :known_hosts_file, :private_key_file @@ -117,8 +139,8 @@ module QA !private_key_file.nil? end - def run(command_str) - command = [env_vars, command_str, '2>&1'].compact.join(' ') + def run(command_str, *extra_env) + command = [env_vars, *extra_env, command_str, '2>&1'].compact.join(' ') Runtime::Logger.debug "Git: command=[#{command}]" output, _ = Open3.capture2(command) diff --git a/qa/qa/page/project/issue/show.rb b/qa/qa/page/project/issue/show.rb index de18b9cefa6..23def93c7dd 100644 --- a/qa/qa/page/project/issue/show.rb +++ b/qa/qa/page/project/issue/show.rb @@ -37,6 +37,11 @@ module QA def select_comments_only_filter click_element :discussion_filter + all_elements(:filter_options)[1].click + end + + def select_history_only_filter + click_element :discussion_filter all_elements(:filter_options).last.click end diff --git a/qa/qa/runtime/env.rb b/qa/qa/runtime/env.rb index c7052a9f300..c4500f9be90 100644 --- a/qa/qa/runtime/env.rb +++ b/qa/qa/runtime/env.rb @@ -7,6 +7,16 @@ module QA attr_writer :personal_access_token + # The environment variables used to indicate if the environment under test + # supports the given feature + SUPPORTED_FEATURES = { + git_protocol_v2: 'QA_CAN_TEST_GIT_PROTOCOL_V2' + }.freeze + + def supported_features + SUPPORTED_FEATURES + end + def debug? enabled?(ENV['QA_DEBUG'], default: false) end @@ -104,6 +114,15 @@ module QA raise ArgumentError, "Please provide GITHUB_ACCESS_TOKEN" end + # Returns true if there is an environment variable that indicates that + # the feature is supported in the environment under test. + # All features are supported by default. + def can_test?(feature) + raise ArgumentError, %Q(Unknown feature "#{feature}") unless SUPPORTED_FEATURES.include? feature + + enabled?(ENV[SUPPORTED_FEATURES[feature]], default: true) + end + private def enabled?(value, default: true) diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb index f5002c8032f..7145b950b6c 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb @@ -31,6 +31,7 @@ module QA create_issue Page::Project::Issue::Show.perform do |show| + show.select_all_activities_filter show.comment('See attached banana for scale', attachment: file_to_attach) show.refresh diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/filter_issue_comments_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/filter_issue_comments_spec.rb index 83603f1cda7..ac34f72bb8f 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/filter_issue_comments_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/filter_issue_comments_spec.rb @@ -27,6 +27,11 @@ module QA expect(show_page).to have_content("made the issue confidential") expect(show_page).to have_content("My own comment") + + show_page.select_history_only_filter + + expect(show_page).to have_content("made the issue confidential") + expect(show_page).not_to have_content("My own comment") end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/protocol_v2_push_ssh_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/protocol_v2_push_ssh_spec.rb new file mode 100644 index 00000000000..bc88e6450f5 --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/repository/protocol_v2_push_ssh_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module QA + context 'Create' do + describe 'Push over SSH using Git protocol version 2', :requires_git_protocol_v2 do + # Note: If you run this test against GDK make sure you've enabled sshd and + # enabled setting the Git protocol by adding `AcceptEnv GIT_PROTOCOL` to + # `sshd_config` + # See: https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/run_qa_against_gdk.md + + let(:key_title) { "key for ssh tests #{Time.now.to_f}" } + let(:ssh_key) do + Factory::Resource::SSHKey.fabricate! do |resource| + resource.title = key_title + end + end + + def login + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) + end + + around do |example| + # Create an SSH key to be used with Git + login + ssh_key + + example.run + + # Remove the SSH key + login + Page::Main::Menu.perform(&:go_to_profile_settings) + Page::Profile::Menu.perform(&:click_ssh_keys) + Page::Profile::SSHKeys.perform do |ssh_keys| + ssh_keys.remove_key(key_title) + end + end + + it 'user pushes to the repository' do + # Create a project to push to + project = Factory::Resource::Project.fabricate! do |project| + project.name = 'git-protocol-project' + end + + file_name = 'README.md' + file_content = 'Test Git protocol v2' + git_protocol = '2' + git_protocol_reported = nil + + # Use Git to clone the project, push a file to it, and then check the + # supported Git protocol + Git::Repository.perform do |repository| + username = 'GitLab QA' + email = 'root@gitlab.com' + + repository.uri = project.repository_ssh_location.uri + + begin + repository.use_ssh_key(ssh_key) + repository.clone + repository.configure_identity(username, email) + + git_protocol_reported = repository.push_with_git_protocol( + git_protocol, + file_name, + file_content) + ensure + repository.delete_ssh_key + end + end + + project.visit! + Page::Project::Show.perform(&:wait_for_push) + + # Check that the push worked + expect(page).to have_content(file_name) + expect(page).to have_content(file_content) + + # And check that the correct Git protocol was used + expect(git_protocol_reported).to eq(git_protocol) + end + end + end +end diff --git a/qa/qa/specs/runner.rb b/qa/qa/specs/runner.rb index ad397c13f0c..1bd8101c36d 100644 --- a/qa/qa/specs/runner.rb +++ b/qa/qa/specs/runner.rb @@ -25,6 +25,10 @@ module QA args.push(%w[--tag ~skip_signup_disabled]) if QA::Runtime::Env.signup_disabled? + QA::Runtime::Env.supported_features.each_key do |key| + args.push(["--tag", "~requires_#{key}"]) unless QA::Runtime::Env.can_test? key + end + args.push(options) args.push(DEFAULT_TEST_PATH_ARGS) unless options.any? { |opt| opt =~ %r{/features/} } diff --git a/qa/spec/git/repository_spec.rb b/qa/spec/git/repository_spec.rb index c629f802aa4..faa154c78da 100644 --- a/qa/spec/git/repository_spec.rb +++ b/qa/spec/git/repository_spec.rb @@ -26,6 +26,36 @@ describe QA::Git::Repository do end end + describe '#git_protocol=' do + [0, 1, 2].each do |version| + it "configures git to use protocol version #{version}" do + expect(repository).to receive(:run).with("git config protocol.version #{version}") + repository.git_protocol = version + end + end + + it 'raises an error if the version is unsupported' do + expect { repository.git_protocol = 'foo' }.to raise_error(ArgumentError, "Please specify the protocol you would like to use: 0, 1, or 2") + end + end + + describe '#fetch_supported_git_protocol' do + it "reports the detected version" do + expect(repository).to receive(:run).and_return("packet: git< version 2") + expect(repository.fetch_supported_git_protocol).to eq('2') + end + + it 'reports unknown if version is unknown' do + expect(repository).to receive(:run).and_return("packet: git< version -1") + expect(repository.fetch_supported_git_protocol).to eq('unknown') + end + + it 'reports unknown if content does not identify a version' do + expect(repository).to receive(:run).and_return("foo") + expect(repository.fetch_supported_git_protocol).to eq('unknown') + end + end + def cd_empty_temp_directory tmp_dir = 'tmp/git-repository-spec/' FileUtils.rm_rf(tmp_dir) if ::File.exist?(tmp_dir) diff --git a/qa/spec/runtime/env_spec.rb b/qa/spec/runtime/env_spec.rb index c59c415c148..ded51d5bb7c 100644 --- a/qa/spec/runtime/env_spec.rb +++ b/qa/spec/runtime/env_spec.rb @@ -3,49 +3,62 @@ describe QA::Runtime::Env do include Support::StubENV - shared_examples 'boolean method' do |method, env_key, default| + shared_examples 'boolean method' do |**kwargs| + it_behaves_like 'boolean method with parameter', kwargs + end + + shared_examples 'boolean method with parameter' do |method:, param: nil, env_key:, default:| context 'when there is an env variable set' do it 'returns false when falsey values specified' do stub_env(env_key, 'false') - expect(described_class.public_send(method)).to be_falsey + expect(described_class.public_send(method, *param)).to be_falsey stub_env(env_key, 'no') - expect(described_class.public_send(method)).to be_falsey + expect(described_class.public_send(method, *param)).to be_falsey stub_env(env_key, '0') - expect(described_class.public_send(method)).to be_falsey + expect(described_class.public_send(method, *param)).to be_falsey end it 'returns true when anything else specified' do stub_env(env_key, 'true') - expect(described_class.public_send(method)).to be_truthy + expect(described_class.public_send(method, *param)).to be_truthy stub_env(env_key, '1') - expect(described_class.public_send(method)).to be_truthy + expect(described_class.public_send(method, *param)).to be_truthy stub_env(env_key, 'anything') - expect(described_class.public_send(method)).to be_truthy + expect(described_class.public_send(method, *param)).to be_truthy end end context 'when there is no env variable set' do it "returns the default, #{default}" do stub_env(env_key, nil) - expect(described_class.public_send(method)).to be(default) + expect(described_class.public_send(method, *param)).to be(default) end end end describe '.signup_disabled?' do - it_behaves_like 'boolean method', :signup_disabled?, 'SIGNUP_DISABLED', false + it_behaves_like 'boolean method', + method: :signup_disabled?, + env_key: 'SIGNUP_DISABLED', + default: false end describe '.debug?' do - it_behaves_like 'boolean method', :debug?, 'QA_DEBUG', false + it_behaves_like 'boolean method', + method: :debug?, + env_key: 'QA_DEBUG', + default: false end describe '.chrome_headless?' do - it_behaves_like 'boolean method', :chrome_headless?, 'CHROME_HEADLESS', true + it_behaves_like 'boolean method', + method: :chrome_headless?, + env_key: 'CHROME_HEADLESS', + default: true end describe '.running_in_ci?' do @@ -182,4 +195,16 @@ describe QA::Runtime::Env do expect(described_class.log_destination).to eq('path/to_file') end end + + describe '.can_test?' do + it_behaves_like 'boolean method with parameter', + method: :can_test?, + param: :git_protocol_v2, + env_key: 'QA_CAN_TEST_GIT_PROTOCOL_V2', + default: true + + it 'raises ArgumentError if feature is unknown' do + expect { described_class.can_test? :foo }.to raise_error(ArgumentError, 'Unknown feature "foo"') + end + end end diff --git a/qa/spec/specs/runner_spec.rb b/qa/spec/specs/runner_spec.rb index 9ddaf7ab1b3..741821ddf8c 100644 --- a/qa/spec/specs/runner_spec.rb +++ b/qa/spec/specs/runner_spec.rb @@ -76,6 +76,20 @@ describe QA::Specs::Runner do end end + context 'when git protocol v2 is not supported' do + before do + allow(QA::Runtime::Env).to receive(:can_test?).with(:git_protocol_v2).and_return(false) + end + + subject { described_class.new } + + it 'it includes default args and excludes the requires_git_protocol_v2 tag' do + expect_rspec_runner_arguments(['--tag', '~orchestrated', '--tag', '~requires_git_protocol_v2', *described_class::DEFAULT_TEST_PATH_ARGS]) + + subject.perform + end + end + def expect_rspec_runner_arguments(arguments) expect(RSpec::Core::Runner).to receive(:run) .with(arguments, $stderr, $stdout) diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index b447e79b0df..81cb3e1f74d 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -3,6 +3,7 @@ import createStore from '~/notes/stores'; import noteableDiscussion from '~/notes/components/noteable_discussion.vue'; import '~/behaviors/markdown/render_gfm'; import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; +import mockDiffFile from '../../diffs/mock_data/diff_file'; const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; @@ -33,9 +34,20 @@ describe('noteable_discussion component', () => { expect(vm.$el.querySelector('.user-avatar-link')).not.toBeNull(); }); + it('should not render discussion header for non diff discussions', () => { + expect(vm.$el.querySelector('.discussion-header')).toBeNull(); + }); + it('should render discussion header', () => { - expect(vm.$el.querySelector('.discussion-header')).not.toBeNull(); - expect(vm.$el.querySelector('.notes').children.length).toEqual(discussionMock.notes.length); + const discussion = { ...discussionMock }; + discussion.diff_file = mockDiffFile; + discussion.diff_discussion = true; + const diffDiscussionVm = new Component({ + store, + propsData: { discussion }, + }).$mount(); + + expect(diffDiscussionVm.$el.querySelector('.discussion-header')).not.toBeNull(); }); describe('actions', () => { diff --git a/spec/javascripts/notes/components/toggle_replies_widget_spec.js b/spec/javascripts/notes/components/toggle_replies_widget_spec.js new file mode 100644 index 00000000000..2ead8cc6e6a --- /dev/null +++ b/spec/javascripts/notes/components/toggle_replies_widget_spec.js @@ -0,0 +1,78 @@ +import Vue from 'vue'; +import toggleRepliesWidget from '~/notes/components/toggle_replies_widget.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import { note } from '../mock_data'; + +const deepCloneObject = obj => JSON.parse(JSON.stringify(obj)); + +describe('toggle replies widget for notes', () => { + let vm; + let ToggleRepliesWidget; + const noteFromOtherUser = deepCloneObject(note); + noteFromOtherUser.author.username = 'fatihacet'; + + const noteFromAnotherUser = deepCloneObject(note); + noteFromAnotherUser.author.username = 'mgreiling'; + noteFromAnotherUser.author.name = 'Mike Greiling'; + + const replies = [note, note, note, noteFromOtherUser, noteFromAnotherUser]; + + beforeEach(() => { + ToggleRepliesWidget = Vue.extend(toggleRepliesWidget); + }); + + afterEach(() => { + vm.$destroy(); + }); + + describe('collapsed state', () => { + beforeEach(() => { + vm = mountComponent(ToggleRepliesWidget, { + replies, + collapsed: true, + }); + }); + + it('should render the collapsed', () => { + const vmTextContent = vm.$el.textContent.replace(/\s\s+/g, ' '); + + expect(vm.$el.classList.contains('collapsed')).toEqual(true); + expect(vm.$el.querySelectorAll('.user-avatar-link').length).toEqual(3); + expect(vm.$el.querySelector('time')).not.toBeNull(); + expect(vmTextContent).toContain('5 replies'); + expect(vmTextContent).toContain(`Last reply by ${noteFromAnotherUser.author.name}`); + }); + + it('should emit toggle event when the replies text clicked', () => { + const spy = spyOn(vm, '$emit'); + + vm.$el.querySelector('.js-replies-text').click(); + + expect(spy).toHaveBeenCalledWith('toggle'); + }); + }); + + describe('expanded state', () => { + beforeEach(() => { + vm = mountComponent(ToggleRepliesWidget, { + replies, + collapsed: false, + }); + }); + + it('should render expanded state', () => { + const vmTextContent = vm.$el.textContent.replace(/\s\s+/g, ' '); + + expect(vm.$el.querySelector('.collapse-replies-btn')).not.toBeNull(); + expect(vmTextContent).toContain('Collapse replies'); + }); + + it('should emit toggle event when the collapse replies text called', () => { + const spy = spyOn(vm, '$emit'); + + vm.$el.querySelector('.js-collapse-replies').click(); + + expect(spy).toHaveBeenCalledWith('toggle'); + }); + }); +}); diff --git a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb index 65f073b2df3..87ab81d8169 100644 --- a/spec/lib/gitlab/import_export/attribute_configuration_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_configuration_spec.rb @@ -23,15 +23,23 @@ describe 'Import/Export attribute configuration' do let(:safe_attributes_file) { 'spec/lib/gitlab/import_export/safe_model_attributes.yml' } let(:safe_model_attributes) { YAML.load_file(safe_attributes_file) } + let(:ee_safe_attributes_file) { 'ee/spec/lib/gitlab/import_export/safe_model_attributes.yml' } + let(:ee_safe_model_attributes) { File.exist?(ee_safe_attributes_file) ? YAML.load_file(ee_safe_attributes_file) : {} } + it 'has no new columns' do relation_names.each do |relation_name| relation_class = relation_class_for_name(relation_name) relation_attributes = relation_class.new.attributes.keys - expect(safe_model_attributes[relation_class.to_s]).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes" - current_attributes = parsed_attributes(relation_name, relation_attributes) - safe_attributes = safe_model_attributes[relation_class.to_s] + safe_attributes = safe_model_attributes[relation_class.to_s].dup || [] + + ee_safe_model_attributes[relation_class.to_s].to_a.each do |attribute| + safe_attributes << attribute + end + + expect(safe_attributes).not_to be_nil, "Expected exported class #{relation_class} to exist in safe_model_attributes" + new_attributes = current_attributes - safe_attributes expect(new_attributes).to be_empty, failure_message(relation_class.to_s, new_attributes) @@ -43,6 +51,7 @@ describe 'Import/Export attribute configuration' do It looks like #{relation_class}, which is exported using the project Import/Export, has new attributes: #{new_attributes.join(',')} Please add the attribute(s) to SAFE_MODEL_ATTRIBUTES if you consider this can be exported. + #{"If the model/associations are EE-specific, use `#{File.expand_path(ee_safe_attributes_file)}`.\n" if ee_safe_model_attributes.any?} Otherwise, please blacklist the attribute(s) in IMPORT_EXPORT_CONFIG by adding it to its correspondent model in the +excluded_attributes+ section. diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index 80604395adf..18cf08f0b9e 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -150,17 +150,25 @@ shared_examples 'discussion comments' do |resource_name| end if resource_name == 'merge request' - let(:note_id) { find("#{comments_selector} .note", match: :first)['data-note-id'] } + let(:note_id) { find("#{comments_selector} .note:first-child", match: :first)['data-note-id'] } + let(:reply_id) { find("#{comments_selector} .note:last-child", match: :first)['data-note-id'] } it 'shows resolved discussion when toggled' do + find("#{comments_selector} .js-vue-discussion-reply").click + find("#{comments_selector} .note-textarea").send_keys('a') + + click_button "Comment" + wait_for_requests + click_button "Resolve discussion" + wait_for_requests expect(page).to have_selector(".note-row-#{note_id}", visible: true) refresh - click_button "Toggle discussion" + click_button "1 reply" - expect(page).to have_selector(".note-row-#{note_id}", visible: true) + expect(page).to have_selector(".note-row-#{reply_id}", visible: true) end end end diff --git a/spec/workers/stuck_import_jobs_worker_spec.rb b/spec/workers/stuck_import_jobs_worker_spec.rb index 2169c14218b..e94d2be9850 100644 --- a/spec/workers/stuck_import_jobs_worker_spec.rb +++ b/spec/workers/stuck_import_jobs_worker_spec.rb @@ -8,29 +8,29 @@ describe StuckImportJobsWorker do context 'when the import status was already updated' do before do allow(Gitlab::SidekiqStatus).to receive(:completed_jids) do - project.import_start - project.import_finish + import_state.start + import_state.finish - [project.import_jid] + [import_state.jid] end end it 'does not mark the project as failed' do worker.perform - expect(project.reload.import_status).to eq('finished') + expect(import_state.reload.status).to eq('finished') end end context 'when the import status was not updated' do before do - allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([project.import_jid]) + allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([import_state.jid]) end it 'marks the project as failed' do worker.perform - expect(project.reload.import_status).to eq('failed') + expect(import_state.reload.status).to eq('failed') end end end @@ -41,27 +41,27 @@ describe StuckImportJobsWorker do end it 'does not mark the project as failed' do - expect { worker.perform }.not_to change { project.reload.import_status } + expect { worker.perform }.not_to change { import_state.reload.status } end end end describe 'with scheduled import_status' do it_behaves_like 'project import job detection' do - let(:project) { create(:project, :import_scheduled) } + let(:import_state) { create(:project, :import_scheduled).import_state } before do - project.import_state.update(jid: '123') + import_state.update(jid: '123') end end end describe 'with started import_status' do it_behaves_like 'project import job detection' do - let(:project) { create(:project, :import_started) } + let(:import_state) { create(:project, :import_started).import_state } before do - project.import_state.update(jid: '123') + import_state.update(jid: '123') end end end |