diff options
108 files changed, 2013 insertions, 1904 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index a9a7f3fec01..8b27ad70f93 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.107.0 +0.109.0 @@ -418,7 +418,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.102.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.103.0', require: 'gitaly' gem 'grpc', '~> 1.11.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed diff --git a/Gemfile.lock b/Gemfile.lock index 8281c1eff9a..13f9212c637 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -282,7 +282,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.102.0) + gitaly-proto (0.103.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1041,7 +1041,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.102.0) + gitaly-proto (~> 0.103.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 52388f17c7c..3161e4653ee 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -285,7 +285,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.102.0) + gitaly-proto (0.103.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1051,7 +1051,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.102.0) + gitaly-proto (~> 0.103.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/PROCESS.md b/PROCESS.md index a46fd8c25b4..1cc5b44aa67 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -197,24 +197,7 @@ to. For example: If you think a merge request should go into an RC or patch even though it does not meet these requirements, you can ask for an exception to be made. -Go to [Release tasks issue tracker](https://gitlab.com/gitlab-org/release/tasks/issues/new) and create an issue -using the `Exception-request` issue template. - -**Do not** set the relevant `Pick into X.Y` label (see above) before request an -exception; this should be done after the exception is approved. - -You can find who is who on the [team page](https://about.gitlab.com/team/). - -Whether an exception is made is determined by weighing the benefit and urgency of the change -(how important it is to the company that this is released _right now_ instead of in a month) -against the potential negative impact -(things breaking without enough time to comfortably find and fix them before the release on the 22nd). -When in doubt, we err on the side of _not_ cherry-picking. - -For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement -(e.g. adding a database index or adding `includes` to a query), but not for a new feature, no matter how relatively small or thoroughly tested. - -All MRs which have had exceptions granted must be merged by the 15th. +Check [this guide](https://gitlab.com/gitlab-org/release/docs/blob/master/general/exception-request/process.md) about how to open an exception request before opening one. ### Regressions diff --git a/app/assets/javascripts/boards/components/board_card.vue b/app/assets/javascripts/boards/components/board_card.vue index b7d3574bc80..0398102ad02 100644 --- a/app/assets/javascripts/boards/components/board_card.vue +++ b/app/assets/javascripts/boards/components/board_card.vue @@ -1,78 +1,78 @@ <script> -/* eslint-disable vue/require-default-prop */ -import './issue_card_inner'; -import eventHub from '../eventhub'; + /* eslint-disable vue/require-default-prop */ + import IssueCardInner from './issue_card_inner.vue'; + import eventHub from '../eventhub'; -const Store = gl.issueBoards.BoardsStore; + const Store = gl.issueBoards.BoardsStore; -export default { - name: 'BoardsIssueCard', - components: { - 'issue-card-inner': gl.issueBoards.IssueCardInner, - }, - props: { - list: { - type: Object, - default: () => ({}), + export default { + name: 'BoardsIssueCard', + components: { + IssueCardInner, }, - issue: { - type: Object, - default: () => ({}), + props: { + list: { + type: Object, + default: () => ({}), + }, + issue: { + type: Object, + default: () => ({}), + }, + issueLinkBase: { + type: String, + default: '', + }, + disabled: { + type: Boolean, + default: false, + }, + index: { + type: Number, + default: 0, + }, + rootPath: { + type: String, + default: '', + }, + groupId: { + type: Number, + }, }, - issueLinkBase: { - type: String, - default: '', + data() { + return { + showDetail: false, + detailIssue: Store.detail, + }; }, - disabled: { - type: Boolean, - default: false, + computed: { + issueDetailVisible() { + return this.detailIssue.issue && this.detailIssue.issue.id === this.issue.id; + }, }, - index: { - type: Number, - default: 0, - }, - rootPath: { - type: String, - default: '', - }, - groupId: { - type: Number, - }, - }, - data() { - return { - showDetail: false, - detailIssue: Store.detail, - }; - }, - computed: { - issueDetailVisible() { - return this.detailIssue.issue && this.detailIssue.issue.id === this.issue.id; - }, - }, - methods: { - mouseDown() { - this.showDetail = true; - }, - mouseMove() { - this.showDetail = false; - }, - showIssue(e) { - if (e.target.classList.contains('js-no-trigger')) return; - - if (this.showDetail) { + methods: { + mouseDown() { + this.showDetail = true; + }, + mouseMove() { this.showDetail = false; + }, + showIssue(e) { + if (e.target.classList.contains('js-no-trigger')) return; + + if (this.showDetail) { + this.showDetail = false; - if (Store.detail.issue && Store.detail.issue.id === this.issue.id) { - eventHub.$emit('clearDetailIssue'); - } else { - eventHub.$emit('newDetailIssue', this.issue); - Store.detail.list = this.list; + if (Store.detail.issue && Store.detail.issue.id === this.issue.id) { + eventHub.$emit('clearDetailIssue'); + } else { + eventHub.$emit('newDetailIssue', this.issue); + Store.detail.list = this.list; + } } - } + }, }, - }, -}; + }; </script> <template> diff --git a/app/assets/javascripts/boards/components/issue_card_inner.js b/app/assets/javascripts/boards/components/issue_card_inner.js deleted file mode 100644 index f7d7b910e2f..00000000000 --- a/app/assets/javascripts/boards/components/issue_card_inner.js +++ /dev/null @@ -1,196 +0,0 @@ -import $ from 'jquery'; -import Vue from 'vue'; -import UserAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; -import eventHub from '../eventhub'; - -const Store = gl.issueBoards.BoardsStore; - -window.gl = window.gl || {}; -window.gl.issueBoards = window.gl.issueBoards || {}; - -gl.issueBoards.IssueCardInner = Vue.extend({ - components: { - UserAvatarLink, - }, - props: { - issue: { - type: Object, - required: true, - }, - issueLinkBase: { - type: String, - required: true, - }, - list: { - type: Object, - required: false, - default: () => ({}), - }, - rootPath: { - type: String, - required: true, - }, - updateFilters: { - type: Boolean, - required: false, - default: false, - }, - groupId: { - type: Number, - required: false, - default: null, - }, - }, - data() { - return { - limitBeforeCounter: 3, - maxRender: 4, - maxCounter: 99, - }; - }, - computed: { - numberOverLimit() { - return this.issue.assignees.length - this.limitBeforeCounter; - }, - assigneeCounterTooltip() { - return `${this.assigneeCounterLabel} more`; - }, - assigneeCounterLabel() { - if (this.numberOverLimit > this.maxCounter) { - return `${this.maxCounter}+`; - } - - return `+${this.numberOverLimit}`; - }, - shouldRenderCounter() { - if (this.issue.assignees.length <= this.maxRender) { - return false; - } - - return this.issue.assignees.length > this.numberOverLimit; - }, - issueId() { - if (this.issue.iid) { - return `#${this.issue.iid}`; - } - return false; - }, - showLabelFooter() { - return this.issue.labels.find(l => this.showLabel(l)) !== undefined; - }, - }, - methods: { - isIndexLessThanlimit(index) { - return index < this.limitBeforeCounter; - }, - shouldRenderAssignee(index) { - // Eg. maxRender is 4, - // Render up to all 4 assignees if there are only 4 assigness - // Otherwise render up to the limitBeforeCounter - if (this.issue.assignees.length <= this.maxRender) { - return index < this.maxRender; - } - - return index < this.limitBeforeCounter; - }, - assigneeUrl(assignee) { - return `${this.rootPath}${assignee.username}`; - }, - assigneeUrlTitle(assignee) { - return `Assigned to ${assignee.name}`; - }, - avatarUrlTitle(assignee) { - return `Avatar for ${assignee.name}`; - }, - showLabel(label) { - if (!label.id) return false; - return true; - }, - filterByLabel(label, e) { - if (!this.updateFilters) return; - - const filterPath = gl.issueBoards.BoardsStore.filter.path.split('&'); - const labelTitle = encodeURIComponent(label.title); - const param = `label_name[]=${labelTitle}`; - const labelIndex = filterPath.indexOf(param); - $(e.currentTarget).tooltip('hide'); - - if (labelIndex === -1) { - filterPath.push(param); - } else { - filterPath.splice(labelIndex, 1); - } - - gl.issueBoards.BoardsStore.filter.path = filterPath.join('&'); - - Store.updateFiltersUrl(); - - eventHub.$emit('updateTokens'); - }, - labelStyle(label) { - return { - backgroundColor: label.color, - color: label.textColor, - }; - }, - }, - template: ` - <div> - <div class="board-card-header"> - <h4 class="board-card-title"> - <i - class="fa fa-eye-slash confidential-icon" - v-if="issue.confidential" - aria-hidden="true" - /> - <a - class="js-no-trigger" - :href="issue.path" - :title="issue.title">{{ issue.title }}</a> - <span - class="board-card-number" - v-if="issueId" - > - {{ issue.referencePath }} - </span> - </h4> - <div class="board-card-assignee"> - <user-avatar-link - v-for="(assignee, index) in issue.assignees" - :key="assignee.id" - v-if="shouldRenderAssignee(index)" - class="js-no-trigger" - :link-href="assigneeUrl(assignee)" - :img-alt="avatarUrlTitle(assignee)" - :img-src="assignee.avatar" - :tooltip-text="assigneeUrlTitle(assignee)" - tooltip-placement="bottom" - /> - <span - class="avatar-counter has-tooltip" - :title="assigneeCounterTooltip" - v-if="shouldRenderCounter" - > - {{ assigneeCounterLabel }} - </span> - </div> - </div> - <div - class="board-card-footer" - v-if="showLabelFooter" - > - <button - class="badge color-label has-tooltip" - v-for="label in issue.labels" - type="button" - v-if="showLabel(label)" - @click="filterByLabel(label, $event)" - :style="labelStyle(label)" - :title="label.description" - data-container="body"> - {{ label.title }} - </button> - </div> - </div> - `, -}); diff --git a/app/assets/javascripts/boards/components/issue_card_inner.vue b/app/assets/javascripts/boards/components/issue_card_inner.vue new file mode 100644 index 00000000000..a0b7fe81a4a --- /dev/null +++ b/app/assets/javascripts/boards/components/issue_card_inner.vue @@ -0,0 +1,196 @@ +<script> + import $ from 'jquery'; + import UserAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; + import eventHub from '../eventhub'; + + const Store = gl.issueBoards.BoardsStore; + + export default { + components: { + UserAvatarLink, + }, + props: { + issue: { + type: Object, + required: true, + }, + issueLinkBase: { + type: String, + required: true, + }, + list: { + type: Object, + required: false, + default: () => ({}), + }, + rootPath: { + type: String, + required: true, + }, + updateFilters: { + type: Boolean, + required: false, + default: false, + }, + groupId: { + type: Number, + required: false, + default: null, + }, + }, + data() { + return { + limitBeforeCounter: 3, + maxRender: 4, + maxCounter: 99, + }; + }, + computed: { + numberOverLimit() { + return this.issue.assignees.length - this.limitBeforeCounter; + }, + assigneeCounterTooltip() { + return `${this.assigneeCounterLabel} more`; + }, + assigneeCounterLabel() { + if (this.numberOverLimit > this.maxCounter) { + return `${this.maxCounter}+`; + } + + return `+${this.numberOverLimit}`; + }, + shouldRenderCounter() { + if (this.issue.assignees.length <= this.maxRender) { + return false; + } + + return this.issue.assignees.length > this.numberOverLimit; + }, + issueId() { + if (this.issue.iid) { + return `#${this.issue.iid}`; + } + return false; + }, + showLabelFooter() { + return this.issue.labels.find(l => this.showLabel(l)) !== undefined; + }, + }, + methods: { + isIndexLessThanlimit(index) { + return index < this.limitBeforeCounter; + }, + shouldRenderAssignee(index) { + // Eg. maxRender is 4, + // Render up to all 4 assignees if there are only 4 assigness + // Otherwise render up to the limitBeforeCounter + if (this.issue.assignees.length <= this.maxRender) { + return index < this.maxRender; + } + + return index < this.limitBeforeCounter; + }, + assigneeUrl(assignee) { + return `${this.rootPath}${assignee.username}`; + }, + assigneeUrlTitle(assignee) { + return `Assigned to ${assignee.name}`; + }, + avatarUrlTitle(assignee) { + return `Avatar for ${assignee.name}`; + }, + showLabel(label) { + if (!label.id) return false; + return true; + }, + filterByLabel(label, e) { + if (!this.updateFilters) return; + + const filterPath = gl.issueBoards.BoardsStore.filter.path.split('&'); + const labelTitle = encodeURIComponent(label.title); + const param = `label_name[]=${labelTitle}`; + const labelIndex = filterPath.indexOf(param); + $(e.currentTarget).tooltip('hide'); + + if (labelIndex === -1) { + filterPath.push(param); + } else { + filterPath.splice(labelIndex, 1); + } + + gl.issueBoards.BoardsStore.filter.path = filterPath.join('&'); + + Store.updateFiltersUrl(); + + eventHub.$emit('updateTokens'); + }, + labelStyle(label) { + return { + backgroundColor: label.color, + color: label.textColor, + }; + }, + }, + }; +</script> +<template> + <div> + <div class="board-card-header"> + <h4 class="board-card-title"> + <i + v-if="issue.confidential" + class="fa fa-eye-slash confidential-icon" + aria-hidden="true" + ></i> + <a + :href="issue.path" + :title="issue.title" + class="js-no-trigger">{{ issue.title }}</a> + <span + v-if="issueId" + class="board-card-number" + > + {{ issue.referencePath }} + </span> + </h4> + <div class="board-card-assignee"> + <user-avatar-link + v-for="(assignee, index) in issue.assignees" + v-if="shouldRenderAssignee(index)" + :key="assignee.id" + :link-href="assigneeUrl(assignee)" + :img-alt="avatarUrlTitle(assignee)" + :img-src="assignee.avatar" + :tooltip-text="assigneeUrlTitle(assignee)" + class="js-no-trigger" + tooltip-placement="bottom" + /> + <span + v-if="shouldRenderCounter" + :title="assigneeCounterTooltip" + class="avatar-counter has-tooltip" + > + {{ assigneeCounterLabel }} + </span> + </div> + </div> + <div + v-if="showLabelFooter" + class="board-card-footer" + > + <button + v-for="label in issue.labels" + v-if="showLabel(label)" + :key="label.id" + :style="labelStyle(label)" + :title="label.description" + class="badge color-label has-tooltip" + type="button" + data-container="body" + @click="filterByLabel(label, $event)" + > + {{ label.title }} + </button> + </div> + </div> +</template> diff --git a/app/assets/javascripts/boards/components/modal/header.js b/app/assets/javascripts/boards/components/modal/header.js deleted file mode 100644 index cc9848058ca..00000000000 --- a/app/assets/javascripts/boards/components/modal/header.js +++ /dev/null @@ -1,79 +0,0 @@ -import Vue from 'vue'; -import modalFilters from './filters'; -import modalTabs from './tabs.vue'; -import ModalStore from '../../stores/modal_store'; -import modalMixin from '../../mixins/modal_mixins'; - -gl.issueBoards.ModalHeader = Vue.extend({ - components: { - modalTabs, - modalFilters, - }, - mixins: [modalMixin], - props: { - projectId: { - type: Number, - required: true, - }, - milestonePath: { - type: String, - required: true, - }, - labelPath: { - type: String, - required: true, - }, - }, - data() { - return ModalStore.store; - }, - computed: { - selectAllText() { - if (ModalStore.selectedCount() !== this.issues.length || this.issues.length === 0) { - return 'Select all'; - } - - return 'Deselect all'; - }, - showSearch() { - return this.activeTab === 'all' && !this.loading && this.issuesCount > 0; - }, - }, - methods: { - toggleAll() { - this.$refs.selectAllBtn.blur(); - - ModalStore.toggleAll(); - }, - }, - template: ` - <div> - <header class="add-issues-header form-actions"> - <h2> - Add issues - <button - type="button" - class="close" - data-dismiss="modal" - aria-label="Close" - @click="toggleModal(false)"> - <span aria-hidden="true">×</span> - </button> - </h2> - </header> - <modal-tabs v-if="!loading && issuesCount > 0"></modal-tabs> - <div - class="add-issues-search append-bottom-10" - v-if="showSearch"> - <modal-filters :store="filter" /> - <button - type="button" - class="btn btn-success btn-inverted prepend-left-10" - ref="selectAllBtn" - @click="toggleAll"> - {{ selectAllText }} - </button> - </div> - </div> - `, -}); diff --git a/app/assets/javascripts/boards/components/modal/header.vue b/app/assets/javascripts/boards/components/modal/header.vue new file mode 100644 index 00000000000..979fb4d7199 --- /dev/null +++ b/app/assets/javascripts/boards/components/modal/header.vue @@ -0,0 +1,82 @@ +<script> + import ModalFilters from './filters'; + import ModalTabs from './tabs.vue'; + import ModalStore from '../../stores/modal_store'; + import modalMixin from '../../mixins/modal_mixins'; + + export default { + components: { + ModalTabs, + ModalFilters, + }, + mixins: [modalMixin], + props: { + projectId: { + type: Number, + required: true, + }, + milestonePath: { + type: String, + required: true, + }, + labelPath: { + type: String, + required: true, + }, + }, + data() { + return ModalStore.store; + }, + computed: { + selectAllText() { + if (ModalStore.selectedCount() !== this.issues.length || this.issues.length === 0) { + return 'Select all'; + } + + return 'Deselect all'; + }, + showSearch() { + return this.activeTab === 'all' && !this.loading && this.issuesCount > 0; + }, + }, + methods: { + toggleAll() { + this.$refs.selectAllBtn.blur(); + + ModalStore.toggleAll(); + }, + }, + }; +</script> +<template> + <div> + <header class="add-issues-header form-actions"> + <h2> + Add issues + <button + type="button" + class="close" + data-dismiss="modal" + aria-label="Close" + @click="toggleModal(false)" + > + <span aria-hidden="true">×</span> + </button> + </h2> + </header> + <modal-tabs v-if="!loading && issuesCount > 0"/> + <div + v-if="showSearch" + class="add-issues-search append-bottom-10"> + <modal-filters :store="filter" /> + <button + ref="selectAllBtn" + type="button" + class="btn btn-success btn-inverted prepend-left-10" + @click="toggleAll" + > + {{ selectAllText }} + </button> + </div> + </div> +</template> diff --git a/app/assets/javascripts/boards/components/modal/index.js b/app/assets/javascripts/boards/components/modal/index.js deleted file mode 100644 index 983061f52ae..00000000000 --- a/app/assets/javascripts/boards/components/modal/index.js +++ /dev/null @@ -1,171 +0,0 @@ -/* global ListIssue */ - -import Vue from 'vue'; -import queryData from '~/boards/utils/query_data'; -import loadingIcon from '~/vue_shared/components/loading_icon.vue'; -import './header'; -import './list'; -import ModalFooter from './footer.vue'; -import EmptyState from './empty_state.vue'; -import ModalStore from '../../stores/modal_store'; - -gl.issueBoards.IssuesModal = Vue.extend({ - components: { - EmptyState, - 'modal-header': gl.issueBoards.ModalHeader, - 'modal-list': gl.issueBoards.ModalList, - ModalFooter, - loadingIcon, - }, - props: { - newIssuePath: { - type: String, - required: true, - }, - emptyStateSvg: { - type: String, - required: true, - }, - issueLinkBase: { - type: String, - required: true, - }, - rootPath: { - type: String, - required: true, - }, - projectId: { - type: Number, - required: true, - }, - milestonePath: { - type: String, - required: true, - }, - labelPath: { - type: String, - required: true, - }, - }, - data() { - return ModalStore.store; - }, - computed: { - showList() { - if (this.activeTab === 'selected') { - return this.selectedIssues.length > 0; - } - - return this.issuesCount > 0; - }, - showEmptyState() { - if (!this.loading && this.issuesCount === 0) { - return true; - } - - return this.activeTab === 'selected' && this.selectedIssues.length === 0; - }, - }, - watch: { - page() { - this.loadIssues(); - }, - showAddIssuesModal() { - if (this.showAddIssuesModal && !this.issues.length) { - this.loading = true; - const loadingDone = () => { - this.loading = false; - }; - - this.loadIssues() - .then(loadingDone) - .catch(loadingDone); - } else if (!this.showAddIssuesModal) { - this.issues = []; - this.selectedIssues = []; - this.issuesCount = false; - } - }, - filter: { - handler() { - if (this.$el.tagName) { - this.page = 1; - this.filterLoading = true; - const loadingDone = () => { - this.filterLoading = false; - }; - - this.loadIssues(true) - .then(loadingDone) - .catch(loadingDone); - } - }, - deep: true, - }, - }, - created() { - this.page = 1; - }, - methods: { - loadIssues(clearIssues = false) { - if (!this.showAddIssuesModal) return false; - - return gl.boardService.getBacklog(queryData(this.filter.path, { - page: this.page, - per: this.perPage, - })) - .then(res => res.data) - .then((data) => { - if (clearIssues) { - this.issues = []; - } - - data.issues.forEach((issueObj) => { - const issue = new ListIssue(issueObj); - const foundSelectedIssue = ModalStore.findSelectedIssue(issue); - issue.selected = !!foundSelectedIssue; - - this.issues.push(issue); - }); - - this.loadingNewPage = false; - - if (!this.issuesCount) { - this.issuesCount = data.size; - } - }).catch(() => { - // TODO: handle request error - }); - }, - }, - template: ` - <div - class="add-issues-modal" - v-if="showAddIssuesModal"> - <div class="add-issues-container"> - <modal-header - :project-id="projectId" - :milestone-path="milestonePath" - :label-path="labelPath"> - </modal-header> - <modal-list - :issue-link-base="issueLinkBase" - :root-path="rootPath" - :empty-state-svg="emptyStateSvg" - v-if="!loading && showList && !filterLoading"></modal-list> - <empty-state - v-if="showEmptyState" - :new-issue-path="newIssuePath" - :empty-state-svg="emptyStateSvg"></empty-state> - <section - class="add-issues-list text-center" - v-if="loading || filterLoading"> - <div class="add-issues-list-loading"> - <loading-icon /> - </div> - </section> - <modal-footer></modal-footer> - </div> - </div> - `, -}); diff --git a/app/assets/javascripts/boards/components/modal/index.vue b/app/assets/javascripts/boards/components/modal/index.vue new file mode 100644 index 00000000000..33e72a6782e --- /dev/null +++ b/app/assets/javascripts/boards/components/modal/index.vue @@ -0,0 +1,178 @@ +<script> + /* global ListIssue */ + import queryData from '~/boards/utils/query_data'; + import loadingIcon from '~/vue_shared/components/loading_icon.vue'; + import ModalHeader from './header.vue'; + import ModalList from './list.vue'; + import ModalFooter from './footer.vue'; + import EmptyState from './empty_state.vue'; + import ModalStore from '../../stores/modal_store'; + + export default { + components: { + EmptyState, + ModalHeader, + ModalList, + ModalFooter, + loadingIcon, + }, + props: { + newIssuePath: { + type: String, + required: true, + }, + emptyStateSvg: { + type: String, + required: true, + }, + issueLinkBase: { + type: String, + required: true, + }, + rootPath: { + type: String, + required: true, + }, + projectId: { + type: Number, + required: true, + }, + milestonePath: { + type: String, + required: true, + }, + labelPath: { + type: String, + required: true, + }, + }, + data() { + return ModalStore.store; + }, + computed: { + showList() { + if (this.activeTab === 'selected') { + return this.selectedIssues.length > 0; + } + + return this.issuesCount > 0; + }, + showEmptyState() { + if (!this.loading && this.issuesCount === 0) { + return true; + } + + return this.activeTab === 'selected' && this.selectedIssues.length === 0; + }, + }, + watch: { + page() { + this.loadIssues(); + }, + showAddIssuesModal() { + if (this.showAddIssuesModal && !this.issues.length) { + this.loading = true; + const loadingDone = () => { + this.loading = false; + }; + + this.loadIssues() + .then(loadingDone) + .catch(loadingDone); + } else if (!this.showAddIssuesModal) { + this.issues = []; + this.selectedIssues = []; + this.issuesCount = false; + } + }, + filter: { + handler() { + if (this.$el.tagName) { + this.page = 1; + this.filterLoading = true; + const loadingDone = () => { + this.filterLoading = false; + }; + + this.loadIssues(true) + .then(loadingDone) + .catch(loadingDone); + } + }, + deep: true, + }, + }, + created() { + this.page = 1; + }, + methods: { + loadIssues(clearIssues = false) { + if (!this.showAddIssuesModal) return false; + + return gl.boardService + .getBacklog( + queryData(this.filter.path, { + page: this.page, + per: this.perPage, + }), + ) + .then(res => res.data) + .then(data => { + if (clearIssues) { + this.issues = []; + } + + data.issues.forEach(issueObj => { + const issue = new ListIssue(issueObj); + const foundSelectedIssue = ModalStore.findSelectedIssue(issue); + issue.selected = !!foundSelectedIssue; + + this.issues.push(issue); + }); + + this.loadingNewPage = false; + + if (!this.issuesCount) { + this.issuesCount = data.size; + } + }) + .catch(() => { + // TODO: handle request error + }); + }, + }, + }; +</script> +<template> + <div + v-if="showAddIssuesModal" + class="add-issues-modal"> + <div class="add-issues-container"> + <modal-header + :project-id="projectId" + :milestone-path="milestonePath" + :label-path="labelPath" + /> + <modal-list + v-if="!loading && showList && !filterLoading" + :issue-link-base="issueLinkBase" + :root-path="rootPath" + :empty-state-svg="emptyStateSvg" + /> + <empty-state + v-if="showEmptyState" + :new-issue-path="newIssuePath" + :empty-state-svg="emptyStateSvg" + /> + <section + v-if="loading || filterLoading" + class="add-issues-list text-center" + > + <div class="add-issues-list-loading"> + <loading-icon /> + </div> + </section> + <modal-footer/> + </div> + </div> +</template> diff --git a/app/assets/javascripts/boards/components/modal/list.js b/app/assets/javascripts/boards/components/modal/list.js deleted file mode 100644 index 11061c72a7b..00000000000 --- a/app/assets/javascripts/boards/components/modal/list.js +++ /dev/null @@ -1,159 +0,0 @@ -import Vue from 'vue'; -import bp from '../../../breakpoints'; -import ModalStore from '../../stores/modal_store'; - -gl.issueBoards.ModalList = Vue.extend({ - components: { - 'issue-card-inner': gl.issueBoards.IssueCardInner, - }, - props: { - issueLinkBase: { - type: String, - required: true, - }, - rootPath: { - type: String, - required: true, - }, - emptyStateSvg: { - type: String, - required: true, - }, - }, - data() { - return ModalStore.store; - }, - computed: { - loopIssues() { - if (this.activeTab === 'all') { - return this.issues; - } - - return this.selectedIssues; - }, - groupedIssues() { - const groups = []; - this.loopIssues.forEach((issue, i) => { - const index = i % this.columns; - - if (!groups[index]) { - groups.push([]); - } - - groups[index].push(issue); - }); - - return groups; - }, - }, - watch: { - activeTab() { - if (this.activeTab === 'all') { - ModalStore.purgeUnselectedIssues(); - } - }, - }, - mounted() { - this.scrollHandlerWrapper = this.scrollHandler.bind(this); - this.setColumnCountWrapper = this.setColumnCount.bind(this); - this.setColumnCount(); - - this.$refs.list.addEventListener('scroll', this.scrollHandlerWrapper); - window.addEventListener('resize', this.setColumnCountWrapper); - }, - beforeDestroy() { - this.$refs.list.removeEventListener('scroll', this.scrollHandlerWrapper); - window.removeEventListener('resize', this.setColumnCountWrapper); - }, - methods: { - scrollHandler() { - const currentPage = Math.floor(this.issues.length / this.perPage); - - if ( - this.scrollTop() > this.scrollHeight() - 100 && - !this.loadingNewPage && - currentPage === this.page - ) { - this.loadingNewPage = true; - this.page += 1; - } - }, - toggleIssue(e, issue) { - if (e.target.tagName !== 'A') { - ModalStore.toggleIssue(issue); - } - }, - listHeight() { - return this.$refs.list.getBoundingClientRect().height; - }, - scrollHeight() { - return this.$refs.list.scrollHeight; - }, - scrollTop() { - return this.$refs.list.scrollTop + this.listHeight(); - }, - showIssue(issue) { - if (this.activeTab === 'all') return true; - - const index = ModalStore.selectedIssueIndex(issue); - - return index !== -1; - }, - setColumnCount() { - const breakpoint = bp.getBreakpointSize(); - - if (breakpoint === 'lg' || breakpoint === 'md') { - this.columns = 3; - } else if (breakpoint === 'sm') { - this.columns = 2; - } else { - this.columns = 1; - } - }, - }, - template: ` - <section - class="add-issues-list add-issues-list-columns" - ref="list"> - <div - class="empty-state add-issues-empty-state-filter text-center" - v-if="issuesCount > 0 && issues.length === 0"> - <div - class="svg-content"> - <img :src="emptyStateSvg"/> - </div> - <div class="text-content"> - <h4> - There are no issues to show. - </h4> - </div> - </div> - <div - v-for="group in groupedIssues" - class="add-issues-list-column"> - <div - v-for="issue in group" - v-if="showIssue(issue)" - class="board-card-parent"> - <div - class="board-card" - :class="{ 'is-active': issue.selected }" - @click="toggleIssue($event, issue)"> - <issue-card-inner - :issue="issue" - :issue-link-base="issueLinkBase" - :root-path="rootPath"> - </issue-card-inner> - <span - :aria-label="'Issue #' + issue.id + ' selected'" - aria-checked="true" - v-if="issue.selected" - class="issue-card-selected text-center"> - <i class="fa fa-check"></i> - </span> - </div> - </div> - </div> - </section> - `, -}); diff --git a/app/assets/javascripts/boards/components/modal/list.vue b/app/assets/javascripts/boards/components/modal/list.vue new file mode 100644 index 00000000000..02ac36d7367 --- /dev/null +++ b/app/assets/javascripts/boards/components/modal/list.vue @@ -0,0 +1,162 @@ +<script> + import bp from '../../../breakpoints'; + import ModalStore from '../../stores/modal_store'; + import IssueCardInner from '../issue_card_inner.vue'; + + export default { + components: { + IssueCardInner, + }, + props: { + issueLinkBase: { + type: String, + required: true, + }, + rootPath: { + type: String, + required: true, + }, + emptyStateSvg: { + type: String, + required: true, + }, + }, + data() { + return ModalStore.store; + }, + computed: { + loopIssues() { + if (this.activeTab === 'all') { + return this.issues; + } + + return this.selectedIssues; + }, + groupedIssues() { + const groups = []; + this.loopIssues.forEach((issue, i) => { + const index = i % this.columns; + + if (!groups[index]) { + groups.push([]); + } + + groups[index].push(issue); + }); + + return groups; + }, + }, + watch: { + activeTab() { + if (this.activeTab === 'all') { + ModalStore.purgeUnselectedIssues(); + } + }, + }, + mounted() { + this.scrollHandlerWrapper = this.scrollHandler.bind(this); + this.setColumnCountWrapper = this.setColumnCount.bind(this); + this.setColumnCount(); + + this.$refs.list.addEventListener('scroll', this.scrollHandlerWrapper); + window.addEventListener('resize', this.setColumnCountWrapper); + }, + beforeDestroy() { + this.$refs.list.removeEventListener('scroll', this.scrollHandlerWrapper); + window.removeEventListener('resize', this.setColumnCountWrapper); + }, + methods: { + scrollHandler() { + const currentPage = Math.floor(this.issues.length / this.perPage); + + if ( + this.scrollTop() > this.scrollHeight() - 100 && + !this.loadingNewPage && + currentPage === this.page + ) { + this.loadingNewPage = true; + this.page += 1; + } + }, + toggleIssue(e, issue) { + if (e.target.tagName !== 'A') { + ModalStore.toggleIssue(issue); + } + }, + listHeight() { + return this.$refs.list.getBoundingClientRect().height; + }, + scrollHeight() { + return this.$refs.list.scrollHeight; + }, + scrollTop() { + return this.$refs.list.scrollTop + this.listHeight(); + }, + showIssue(issue) { + if (this.activeTab === 'all') return true; + + const index = ModalStore.selectedIssueIndex(issue); + + return index !== -1; + }, + setColumnCount() { + const breakpoint = bp.getBreakpointSize(); + + if (breakpoint === 'lg' || breakpoint === 'md') { + this.columns = 3; + } else if (breakpoint === 'sm') { + this.columns = 2; + } else { + this.columns = 1; + } + }, + }, + }; +</script> +<template> + <section + ref="list" + class="add-issues-list add-issues-list-columns"> + <div + v-if="issuesCount > 0 && issues.length === 0" + class="empty-state add-issues-empty-state-filter text-center"> + <div + class="svg-content"> + <img :src="emptyStateSvg" /> + </div> + <div class="text-content"> + <h4> + There are no issues to show. + </h4> + </div> + </div> + <div + v-for="(group, index) in groupedIssues" + :key="index" + class="add-issues-list-column"> + <div + v-for="issue in group" + v-if="showIssue(issue)" + :key="issue.id" + class="board-card-parent"> + <div + :class="{ 'is-active': issue.selected }" + class="board-card" + @click="toggleIssue($event, issue)"> + <issue-card-inner + :issue="issue" + :issue-link-base="issueLinkBase" + :root-path="rootPath"/> + <span + v-if="issue.selected" + :aria-label="'Issue #' + issue.id + ' selected'" + aria-checked="true" + class="issue-card-selected text-center"> + <i class="fa fa-check"></i> + </span> + </div> + </div> + </div> + </section> +</template> diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index 751a66f89c6..200d1923635 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -25,7 +25,7 @@ import './filters/due_date_filters'; import './components/board'; import './components/board_sidebar'; import './components/new_list_dropdown'; -import './components/modal/index'; +import BoardAddIssuesModal from './components/modal/index.vue'; import '~/vue_shared/vue_resource_interceptor'; // eslint-disable-line import/first export default () => { @@ -49,7 +49,7 @@ export default () => { components: { 'board': gl.issueBoards.Board, 'board-sidebar': gl.issueBoards.BoardSidebar, - 'board-add-issues-modal': gl.issueBoards.IssuesModal, + BoardAddIssuesModal, }, data: { state: Store.state, 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 8999fd2ac96..a74ea4bfaaf 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -4,14 +4,7 @@ import { s__ } from '~/locale'; import { mapState, mapGetters, mapActions } from 'vuex'; import Icon from '~/vue_shared/components/icon.vue'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, - LINE_POSITION_RIGHT, - UNFOLD_COUNT, -} from '../constants'; +import { LINE_POSITION_RIGHT, UNFOLD_COUNT } from '../constants'; import * as utils from '../store/utils'; export default { @@ -63,6 +56,21 @@ export default { required: false, default: false, }, + isMatchLine: { + type: Boolean, + required: false, + default: false, + }, + isMetaLine: { + type: Boolean, + required: false, + default: false, + }, + isContextLine: { + type: Boolean, + required: false, + default: false, + }, }, computed: { ...mapState({ @@ -70,15 +78,6 @@ export default { diffFiles: state => state.diffs.diffFiles, }), ...mapGetters(['isLoggedIn', 'discussionsByLineCode']), - isMatchLine() { - return this.lineType === MATCH_LINE_TYPE; - }, - isContextLine() { - return this.lineType === CONTEXT_LINE_TYPE; - }, - isMetaLine() { - return this.lineType === OLD_NO_NEW_LINE_TYPE || this.lineType === NEW_NO_NEW_LINE_TYPE; - }, lineHref() { return this.lineCode ? `#${this.lineCode}` : '#'; }, @@ -109,9 +108,9 @@ export default { }, }, methods: { - ...mapActions(['loadMoreLines']), + ...mapActions(['loadMoreLines', 'showCommentForm']), handleCommentButton() { - this.$emit('showCommentForm', { lineCode: this.lineCode }); + this.showCommentForm({ lineCode: this.lineCode }); }, handleLoadMoreLines() { if (this.isRequesting) { diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue new file mode 100644 index 00000000000..68fe6787f9b --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -0,0 +1,131 @@ +<script> +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, + NEW_NO_NEW_LINE_TYPE, + LINE_HOVER_CLASS_NAME, + LINE_UNFOLD_CLASS_NAME, +} from '../constants'; + +export default { + components: { + DiffLineGutterContent, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFile: { + type: Object, + required: true, + }, + showCommentButton: { + type: Boolean, + required: false, + default: false, + }, + linePosition: { + type: String, + required: false, + default: '', + }, + lineType: { + type: String, + required: false, + default: '', + }, + isContentLine: { + type: Boolean, + required: false, + default: false, + }, + isBottom: { + type: Boolean, + required: false, + default: false, + }, + isHover: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + ...mapGetters(['isLoggedIn', 'isInlineView']), + normalizedLine() { + if (this.isInlineView) { + return this.line; + } + + return this.lineType === OLD_LINE_TYPE ? this.line.left : this.line.right; + }, + isMatchLine() { + return this.normalizedLine.type === MATCH_LINE_TYPE; + }, + isContextLine() { + return this.normalizedLine.type === CONTEXT_LINE_TYPE; + }, + isMetaLine() { + return ( + this.normalizedLine.type === OLD_NO_NEW_LINE_TYPE || + this.normalizedLine.type === NEW_NO_NEW_LINE_TYPE || + this.normalizedLine.type === EMPTY_CELL_TYPE + ); + }, + classNameMap() { + const { type } = this.normalizedLine; + + return { + [type]: type, + [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine, + [LINE_HOVER_CLASS_NAME]: + this.isLoggedIn && + this.isHover && + !this.isMatchLine && + !this.isContextLine && + !this.isMetaLine, + }; + }, + lineNumber() { + const { lineType, normalizedLine } = this; + + return lineType === OLD_LINE_TYPE ? normalizedLine.oldLine : normalizedLine.newLine; + }, + }, +}; +</script> + +<template> + <td + v-if="isContentLine" + :class="lineType" + class="line_content" + v-html="normalizedLine.richText" + > + </td> + <td + v-else + :class="classNameMap" + > + <diff-line-gutter-content + :file-hash="diffFile.fileHash" + :line-type="normalizedLine.type" + :line-code="normalizedLine.lineCode" + :line-position="linePosition" + :line-number="lineNumber" + :meta-data="normalizedLine.metaData" + :show-comment-button="showCommentButton" + :context-lines-path="diffFile.contextLinesPath" + :is-bottom="isBottom" + :is-match-line="isMatchLine" + :is-context-line="isContentLine" + :is-meta-line="isMetaLine" + /> + </td> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_table_row.vue b/app/assets/javascripts/diffs/components/diff_table_row.vue new file mode 100644 index 00000000000..8716fdaf44d --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_table_row.vue @@ -0,0 +1,191 @@ +<script> +import $ from 'jquery'; +import { mapGetters } from 'vuex'; +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, + LINE_POSITION_LEFT, + LINE_POSITION_RIGHT, +} from '../constants'; + +export default { + components: { + DiffTableCell, + }, + props: { + diffFile: { + type: Object, + required: true, + }, + line: { + type: Object, + required: true, + }, + isBottom: { + type: Boolean, + required: false, + default: false, + }, + }, + data() { + return { + isHover: false, + isLeftHover: false, + isRightHover: false, + }; + }, + computed: { + ...mapGetters(['isInlineView', 'isParallelView']), + isContextLine() { + return this.line.left + ? this.line.left.type === CONTEXT_LINE_TYPE + : 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, + }; + }, + inlineRowId() { + const { lineCode, oldLine, newLine } = this.line; + + return lineCode || `${this.diffFile.fileHash}_${oldLine}_${newLine}`; + }, + parallelViewLeftLineType() { + if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) { + return OLD_NO_NEW_LINE_TYPE; + } + + return this.line.left.type; + }, + }, + created() { + this.newLineType = NEW_LINE_TYPE; + this.oldLineType = OLD_LINE_TYPE; + this.linePositionLeft = LINE_POSITION_LEFT; + this.linePositionRight = LINE_POSITION_RIGHT; + }, + methods: { + handleMouseMove(e) { + const isHover = e.type === 'mouseover'; + + if (this.isInlineView) { + this.isHover = isHover; + } else { + const hoveringCell = e.target.closest('td'); + const allCellsInHoveringRow = Array.from(e.currentTarget.children); + const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell); + + if (hoverIndex >= 2) { + this.isRightHover = isHover; + } else { + this.isLeftHover = isHover; + } + } + }, + // Prevent text selecting on both sides of parallel diff view + // Backport of the same code from legacy diff notes. + handleParallelLineMouseDown(e) { + const line = $(e.currentTarget); + const table = line.closest('table'); + + table.removeClass('left-side-selected right-side-selected'); + const [lineClass] = ['left-side', 'right-side'].filter(name => line.hasClass(name)); + + if (lineClass) { + table.addClass(`${lineClass}-selected`); + } + }, + }, +}; +</script> + +<template> + <tr + v-if="isInlineView" + :id="inlineRowId" + :class="classNameMap" + class="line_holder" + @mouseover="handleMouseMove" + @mouseout="handleMouseMove" + > + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="oldLineType" + :is-bottom="isBottom" + :is-hover="isHover" + :show-comment-button="true" + class="diff-line-num old_line" + /> + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="newLineType" + :is-bottom="isBottom" + :is-hover="isHover" + class="diff-line-num new_line" + /> + <diff-table-cell + :class="line.type" + :diff-file="diffFile" + :line="line" + :is-content-line="true" + /> + </tr> + + <tr + v-else + :class="classNameMap" + class="line_holder" + @mouseover="handleMouseMove" + @mouseout="handleMouseMove" + > + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="oldLineType" + :line-position="linePositionLeft" + :is-bottom="isBottom" + :is-hover="isLeftHover" + :show-comment-button="true" + class="diff-line-num old_line" + /> + <diff-table-cell + :id="line.left.lineCode" + :diff-file="diffFile" + :line="line" + :is-content-line="true" + :line-type="parallelViewLeftLineType" + class="line_content parallel left-side" + @mousedown.native="handleParallelLineMouseDown" + /> + <diff-table-cell + :diff-file="diffFile" + :line="line" + :line-type="newLineType" + :line-position="linePositionRight" + :is-bottom="isBottom" + :is-hover="isRightHover" + :show-comment-button="true" + class="diff-line-num new_line" + /> + <diff-table-cell + :id="line.right.lineCode" + :diff-file="diffFile" + :line="line" + :is-content-line="true" + :line-type="line.right.type" + class="line_content parallel right-side" + @mousedown.native="handleParallelLineMouseDown" + /> + </tr> +</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue new file mode 100644 index 00000000000..0e935f1d68e --- /dev/null +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -0,0 +1,82 @@ +<script> +import { mapState, mapGetters } from 'vuex'; +import diffDiscussions from './diff_discussions.vue'; +import diffLineNoteForm from './diff_line_note_form.vue'; + +export default { + components: { + diffDiscussions, + diffLineNoteForm, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + lineIndex: { + type: Number, + required: true, + }, + }, + computed: { + ...mapState({ + diffLineCommentForms: state => state.diffs.diffLineCommentForms, + }), + ...mapGetters(['discussionsByLineCode']), + isDiscussionExpanded() { + if (!this.discussions.length) { + return false; + } + + return this.discussions.every(discussion => discussion.expanded); + }, + hasCommentForm() { + return this.diffLineCommentForms[this.line.lineCode]; + }, + discussions() { + return this.discussionsByLineCode[this.line.lineCode] || []; + }, + shouldRender() { + return this.isDiscussionExpanded || this.hasCommentForm; + }, + className() { + return this.discussions.length ? '' : 'js-temp-notes-holder'; + }, + }, +}; +</script> + +<template> + <tr + v-if="shouldRender" + :class="className" + class="notes_holder" + > + <td + class="notes_line" + colspan="2" + ></td> + <td class="notes_content"> + <div class="content"> + <diff-discussions + :discussions="discussions" + /> + <diff-line-note-form + v-if="diffLineCommentForms[line.lineCode]" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line" + :note-target-line="diffLines[lineIndex]" + /> + </div> + </td> + </tr> +</template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 21376117bef..e72f85df77a 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -1,34 +1,12 @@ <script> import diffContentMixin from '../mixins/diff_content'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, - LINE_HOVER_CLASS_NAME, - LINE_UNFOLD_CLASS_NAME, -} from '../constants'; +import inlineDiffCommentRow from './inline_diff_comment_row.vue'; export default { - mixins: [diffContentMixin], - methods: { - handleMouse(lineCode, isOver) { - this.hoveredLineCode = isOver ? lineCode : null; - }, - getLineClass(line) { - const isSameLine = this.hoveredLineCode && this.hoveredLineCode === line.lineCode; - const isMatchLine = line.type === MATCH_LINE_TYPE; - const isContextLine = line.type === CONTEXT_LINE_TYPE; - const isMetaLine = line.type === OLD_NO_NEW_LINE_TYPE || line.type === NEW_NO_NEW_LINE_TYPE; - - return { - [line.type]: line.type, - [LINE_UNFOLD_CLASS_NAME]: isMatchLine, - [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && isSameLine && !isMatchLine && !isContextLine && !isMetaLine, - }; - }, + components: { + inlineDiffCommentRow, }, + mixins: [diffContentMixin], }; </script> @@ -41,76 +19,19 @@ export default { <template v-for="(line, index) in normalizedDiffLines" > - <tr - :id="line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`" + <diff-table-row + :diff-file="diffFile" + :line="line" + :is-bottom="index + 1 === diffLinesLength" :key="line.lineCode" - :class="getRowClass(line)" - class="line_holder" - @mouseover="handleMouse(line.lineCode, true)" - @mouseout="handleMouse(line.lineCode, false)" - > - <td - :class="getLineClass(line)" - class="diff-line-num old_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.type" - :line-code="line.lineCode" - :line-number="line.oldLine" - :meta-data="line.metaData" - :show-comment-button="true" - :context-lines-path="diffFile.contextLinesPath" - :is-bottom="index + 1 === diffLinesLength" - @showCommentForm="handleShowCommentForm" - /> - </td> - <td - :class="getLineClass(line)" - class="diff-line-num new_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.type" - :line-code="line.lineCode" - :line-number="line.newLine" - :meta-data="line.metaData" - :is-bottom="index + 1 === diffLinesLength" - :context-lines-path="diffFile.contextLinesPath" - /> - </td> - <td - :class="line.type" - class="line_content" - v-html="line.richText" - > - </td> - </tr> - <tr - v-if="isDiscussionExpanded(line.lineCode) || diffLineCommentForms[line.lineCode]" + /> + <inline-diff-comment-row + :diff-file="diffFile" + :diff-lines="normalizedDiffLines" + :line="line" + :line-index="index" :key="index" - :class="discussionsByLineCode[line.lineCode] ? '' : 'js-temp-notes-holder'" - class="notes_holder" - > - <td - class="notes_line" - colspan="2" - ></td> - <td class="notes_content"> - <div class="content"> - <diff-discussions - :discussions="discussionsByLineCode[line.lineCode] || []" - /> - <diff-line-note-form - v-if="diffLineCommentForms[line.lineCode]" - :diff-file="diffFile" - :diff-lines="diffLines" - :line="line" - :note-target-line="diffLines[index]" - /> - </div> - </td> - </tr> + /> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue new file mode 100644 index 00000000000..5f33ec7a3c2 --- /dev/null +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -0,0 +1,129 @@ +<script> +import { mapState, mapGetters } from 'vuex'; +import diffDiscussions from './diff_discussions.vue'; +import diffLineNoteForm from './diff_line_note_form.vue'; + +export default { + components: { + diffDiscussions, + diffLineNoteForm, + }, + props: { + line: { + type: Object, + required: true, + }, + diffFile: { + type: Object, + required: true, + }, + diffLines: { + type: Array, + required: true, + }, + lineIndex: { + type: Number, + required: true, + }, + }, + computed: { + ...mapState({ + diffLineCommentForms: state => state.diffs.diffLineCommentForms, + }), + ...mapGetters(['discussionsByLineCode']), + leftLineCode() { + return this.line.left.lineCode; + }, + rightLineCode() { + return this.line.right.lineCode; + }, + hasDiscussion() { + const discussions = this.discussionsByLineCode; + + return discussions[this.leftLineCode] || discussions[this.rightLineCode]; + }, + hasExpandedDiscussionOnLeft() { + const discussions = this.discussionsByLineCode[this.leftLineCode]; + + return discussions ? discussions.every(discussion => discussion.expanded) : false; + }, + hasExpandedDiscussionOnRight() { + const discussions = this.discussionsByLineCode[this.rightLineCode]; + + return discussions ? discussions.every(discussion => discussion.expanded) : false; + }, + hasAnyExpandedDiscussion() { + return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; + }, + shouldRenderDiscussionsRow() { + const hasDiscussion = this.hasDiscussion && this.hasAnyExpandedDiscussion; + const hasCommentFormOnLeft = this.diffLineCommentForms[this.leftLineCode]; + const hasCommentFormOnRight = this.diffLineCommentForms[this.rightLineCode]; + + return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight; + }, + shouldRenderDiscussionsOnLeft() { + return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft; + }, + shouldRenderDiscussionsOnRight() { + return ( + this.discussionsByLineCode[this.rightLineCode] && + this.hasExpandedDiscussionOnRight && + this.line.right.type + ); + }, + className() { + return this.hasDiscussion ? '' : 'js-temp-notes-holder'; + }, + }, +}; +</script> + +<template> + <tr + v-if="shouldRenderDiscussionsRow" + :class="className" + class="notes_holder" + > + <td class="notes_line old"></td> + <td class="notes_content parallel old"> + <div + v-if="shouldRenderDiscussionsOnLeft" + class="content" + > + <diff-discussions + :discussions="discussionsByLineCode[leftLineCode]" + /> + </div> + <diff-line-note-form + v-if="diffLineCommentForms[leftLineCode] && + diffLineCommentForms[leftLineCode]" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line.left" + :note-target-line="diffLines[lineIndex].left" + position="left" + /> + </td> + <td class="notes_line new"></td> + <td class="notes_content parallel new"> + <div + v-if="shouldRenderDiscussionsOnRight" + class="content" + > + <diff-discussions + :discussions="discussionsByLineCode[rightLineCode]" + /> + </div> + <diff-line-note-form + v-if="diffLineCommentForms[rightLineCode] && + diffLineCommentForms[rightLineCode] && line.right.type" + :diff-file="diffFile" + :diff-lines="diffLines" + :line="line.right" + :note-target-line="diffLines[lineIndex].right" + position="right" + /> + </td> + </tr> +</template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 60edbcbbda8..ed92b4ee249 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -1,17 +1,12 @@ <script> import diffContentMixin from '../mixins/diff_content'; -import { - EMPTY_CELL_TYPE, - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, - LINE_HOVER_CLASS_NAME, - LINE_UNFOLD_CLASS_NAME, - LINE_POSITION_RIGHT, -} from '../constants'; +import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; +import { EMPTY_CELL_TYPE } from '../constants'; export default { + components: { + parallelDiffCommentRow, + }, mixins: [diffContentMixin], computed: { parallelDiffLines() { @@ -26,77 +21,6 @@ export default { }); }, }, - methods: { - hasDiscussion(line) { - const discussions = this.discussionsByLineCode; - const hasDiscussion = discussions[line.left.lineCode] || discussions[line.right.lineCode]; - - return hasDiscussion; - }, - getClassName(line, position) { - const { type, lineCode } = line[position]; - const isMatchLine = type === MATCH_LINE_TYPE; - const isContextLine = !isMatchLine && type !== EMPTY_CELL_TYPE && type !== CONTEXT_LINE_TYPE; - const isMetaLine = type === OLD_NO_NEW_LINE_TYPE || type === NEW_NO_NEW_LINE_TYPE; - const isSameLine = this.hoveredLineCode && this.hoveredLineCode === lineCode; - const isSameSection = position === this.hoveredSection; - - return { - [type]: type, - [LINE_UNFOLD_CLASS_NAME]: isMatchLine, - [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && isContextLine && isSameLine && isSameSection && !isMetaLine, - }; - }, - handleMouse(e, line, isHover) { - if (isHover) { - const cell = e.target.closest('td'); - - if (this.$refs.leftLines.indexOf(cell) > -1) { - this.hoveredLineCode = line.left.lineCode; - this.hoveredSection = 'left'; - } else if (this.$refs.rightLines.indexOf(cell) > -1) { - this.hoveredLineCode = line.right.lineCode; - this.hoveredSection = 'right'; - } - } else { - this.hoveredLineCode = null; - this.hoveredSection = null; - } - }, - shouldRenderDiscussionsRow(line) { - const hasDiscussion = this.hasDiscussion(line) && this.hasAnyExpandedDiscussion(line); - const hasCommentFormOnLeft = this.diffLineCommentForms[line.left.lineCode]; - const hasCommentFormOnRight = this.diffLineCommentForms[line.right.lineCode]; - - return hasDiscussion || hasCommentFormOnLeft || hasCommentFormOnRight; - }, - shouldRenderDiscussions(line, position) { - const { lineCode } = line[position]; - let render = this.discussionsByLineCode[lineCode] && this.isDiscussionExpanded(lineCode); - - // Avoid rendering context line discussions on the right side in parallel view - if (position === LINE_POSITION_RIGHT) { - render = render && line.right.type; - } - - return render; - }, - hasAnyExpandedDiscussion(line) { - const isLeftExpanded = this.isDiscussionExpanded(line.left.lineCode); - const isRightExpanded = this.isDiscussionExpanded(line.right.lineCode); - - return isLeftExpanded || isRightExpanded; - }, - getLineCode(line, side) { - const { lineCode } = side; - if (lineCode) { - return lineCode; - } - - return `${this.fileHash}_${line.left.oldLine}_${line.right.newLine}`; - }, - }, }; </script> @@ -104,119 +28,26 @@ export default { <div :class="userColorScheme" :data-commit-id="commitId" - class="code diff-wrap-lines js-syntax-highlight text-file"> + class="code diff-wrap-lines js-syntax-highlight text-file" + > <table> <tbody> <template v-for="(line, index) in parallelDiffLines" > - <tr + <diff-table-row + :diff-file="diffFile" + :line="line" + :is-bottom="index + 1 === diffLinesLength" :key="index" - :class="getRowClass(line)" - class="line_holder parallel" - @mouseover="handleMouse($event, line, true)" - @mouseout="handleMouse($event, line, false)" - > - <td - ref="leftLines" - :class="getClassName(line, 'left')" - class="diff-line-num old_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.left.type" - :line-code="line.left.lineCode" - :line-number="line.left.oldLine" - :meta-data="line.left.metaData" - :show-comment-button="true" - :context-lines-path="diffFile.contextLinesPath" - :is-bottom="index + 1 === diffLinesLength" - line-position="left" - @showCommentForm="handleShowCommentForm" - /> - </td> - <td - ref="leftLines" - :class="getClassName(line, 'left')" - :id="getLineCode(line, line.left)" - class="line_content parallel left-side" - v-html="line.left.richText" - > - </td> - <td - ref="rightLines" - :class="getClassName(line, 'right')" - class="diff-line-num new_line" - > - <diff-line-gutter-content - :file-hash="fileHash" - :line-type="line.right.type" - :line-code="line.right.lineCode" - :line-number="line.right.newLine" - :meta-data="line.right.metaData" - :show-comment-button="true" - :context-lines-path="diffFile.contextLinesPath" - :is-bottom="index + 1 === diffLinesLength" - line-position="right" - @showCommentForm="handleShowCommentForm" - /> - </td> - <td - ref="rightLines" - :class="getClassName(line, 'right')" - :id="getLineCode(line, line.right)" - class="line_content parallel right-side" - v-html="line.right.richText" - > - </td> - </tr> - <tr - v-if="shouldRenderDiscussionsRow(line)" + /> + <parallel-diff-comment-row :key="line.left.lineCode || line.right.lineCode" - :class="hasDiscussion(line) ? '' : 'js-temp-notes-holder'" - class="notes_holder" - > - <td class="notes_line old"></td> - <td class="notes_content parallel old"> - <div - v-if="shouldRenderDiscussions(line, 'left')" - class="content" - > - <diff-discussions - :discussions="discussionsByLineCode[line.left.lineCode]" - /> - </div> - <diff-line-note-form - v-if="diffLineCommentForms[line.left.lineCode] && - diffLineCommentForms[line.left.lineCode]" - :diff-file="diffFile" - :diff-lines="diffLines" - :line="line.left" - :note-target-line="diffLines[index].left" - position="left" - /> - </td> - <td class="notes_line new"></td> - <td class="notes_content parallel new"> - <div - v-if="shouldRenderDiscussions(line, 'right')" - class="content" - > - <diff-discussions - :discussions="discussionsByLineCode[line.right.lineCode]" - /> - </div> - <diff-line-note-form - v-if="diffLineCommentForms[line.right.lineCode] && - diffLineCommentForms[line.right.lineCode] && line.right.type" - :diff-file="diffFile" - :diff-lines="diffLines" - :line="line.right" - :note-target-line="diffLines[index].right" - position="right" - /> - </td> - </tr> + :line="line" + :diff-file="diffFile" + :diff-lines="parallelDiffLines" + :line-index="index" + /> </template> </tbody> </table> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index d314f08e60e..2fa8367f528 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -14,6 +14,8 @@ export const TEXT_DIFF_POSITION_TYPE = 'text'; export const LINE_POSITION_LEFT = 'left'; export const LINE_POSITION_RIGHT = 'right'; +export const LINE_SIDE_LEFT = 'left-side'; +export const LINE_SIDE_RIGHT = 'right-side'; export const DIFF_VIEW_COOKIE_NAME = 'diff_view'; export const LINE_HOVER_CLASS_NAME = 'is-over'; diff --git a/app/assets/javascripts/diffs/mixins/diff_content.js b/app/assets/javascripts/diffs/mixins/diff_content.js index bef06ad2b52..ebb511d3a7e 100644 --- a/app/assets/javascripts/diffs/mixins/diff_content.js +++ b/app/assets/javascripts/diffs/mixins/diff_content.js @@ -1,9 +1,9 @@ -import { mapState, mapGetters, mapActions } from 'vuex'; +import { mapGetters } from 'vuex'; import diffDiscussions from '../components/diff_discussions.vue'; import diffLineGutterContent from '../components/diff_line_gutter_content.vue'; import diffLineNoteForm from '../components/diff_line_note_form.vue'; +import diffTableRow from '../components/diff_table_row.vue'; import { trimFirstCharOfLineContent } from '../store/utils'; -import { CONTEXT_LINE_TYPE, CONTEXT_LINE_CLASS_NAME } from '../constants'; export default { props: { @@ -16,22 +16,14 @@ export default { required: true, }, }, - data() { - return { - hoveredLineCode: null, - hoveredSection: null, - }; - }, components: { diffDiscussions, + diffTableRow, diffLineNoteForm, diffLineGutterContent, }, computed: { - ...mapState({ - diffLineCommentForms: state => state.diffs.diffLineCommentForms, - }), - ...mapGetters(['discussionsByLineCode', 'isLoggedIn', 'commit']), + ...mapGetters(['commit']), commitId() { return this.commit && this.commit.id; }, @@ -41,15 +33,15 @@ export default { normalizedDiffLines() { return this.diffLines.map(line => { if (line.richText) { - return this.trimFirstChar(line); + return trimFirstCharOfLineContent(line); } if (line.left) { - Object.assign(line, { left: this.trimFirstChar(line.left) }); + Object.assign(line, { left: trimFirstCharOfLineContent(line.left) }); } if (line.right) { - Object.assign(line, { right: this.trimFirstChar(line.right) }); + Object.assign(line, { right: trimFirstCharOfLineContent(line.right) }); } return line; @@ -62,28 +54,4 @@ export default { return this.diffFile.fileHash; }, }, - methods: { - ...mapActions(['showCommentForm', 'cancelCommentForm']), - getRowClass(line) { - const isContextLine = line.left - ? line.left.type === CONTEXT_LINE_TYPE - : line.type === CONTEXT_LINE_TYPE; - - return { - [line.type]: line.type, - [CONTEXT_LINE_CLASS_NAME]: isContextLine, - }; - }, - trimFirstChar(line) { - return trimFirstCharOfLineContent(line); - }, - handleShowCommentForm(params) { - this.showCommentForm({ lineCode: params.lineCode }); - }, - isDiscussionExpanded(lineCode) { - const discussions = this.discussionsByLineCode[lineCode]; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; - }, - }, }; diff --git a/app/assets/javascripts/protected_branches/protected_branch_create.js b/app/assets/javascripts/protected_branches/protected_branch_create.js index 7c61c070a35..b601b19e7be 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_create.js +++ b/app/assets/javascripts/protected_branches/protected_branch_create.js @@ -1,11 +1,8 @@ import $ from 'jquery'; -import _ from 'underscore'; import ProtectedBranchAccessDropdown from './protected_branch_access_dropdown'; import CreateItemDropdown from '../create_item_dropdown'; import AccessorUtilities from '../lib/utils/accessor'; -const PB_LOCAL_STORAGE_KEY = 'protected-branches-defaults'; - export default class ProtectedBranchCreate { constructor() { this.$form = $('.js-new-protected-branch'); @@ -43,8 +40,6 @@ export default class ProtectedBranchCreate { onSelect: this.onSelectCallback, getData: ProtectedBranchCreate.getProtectedBranches, }); - - this.loadPreviousSelection($allowedToMergeDropdown.data('glDropdown'), $allowedToPushDropdown.data('glDropdown')); } // This will run after clicked callback @@ -59,39 +54,10 @@ export default class ProtectedBranchCreate { $allowedToPushInput.length ); - this.savePreviousSelection($allowedToMergeInput.val(), $allowedToPushInput.val()); this.$form.find('input[type="submit"]').prop('disabled', completedForm); } static getProtectedBranches(term, callback) { callback(gon.open_branches); } - - loadPreviousSelection(mergeDropdown, pushDropdown) { - let mergeIndex = 0; - let pushIndex = 0; - if (this.isLocalStorageAvailable) { - const savedDefaults = JSON.parse(window.localStorage.getItem(PB_LOCAL_STORAGE_KEY)); - if (savedDefaults != null) { - mergeIndex = _.findLastIndex(mergeDropdown.fullData.roles, { - id: parseInt(savedDefaults.mergeSelection, 0), - }); - pushIndex = _.findLastIndex(pushDropdown.fullData.roles, { - id: parseInt(savedDefaults.pushSelection, 0), - }); - } - } - mergeDropdown.selectRowAtIndex(mergeIndex); - pushDropdown.selectRowAtIndex(pushIndex); - } - - savePreviousSelection(mergeSelection, pushSelection) { - if (this.isLocalStorageAvailable) { - const branchDefaults = { - mergeSelection, - pushSelection, - }; - window.localStorage.setItem(PB_LOCAL_STORAGE_KEY, JSON.stringify(branchDefaults)); - } - } } diff --git a/app/assets/stylesheets/framework/wells.scss b/app/assets/stylesheets/framework/wells.scss index 514fac82b1e..161943766d4 100644 --- a/app/assets/stylesheets/framework/wells.scss +++ b/app/assets/stylesheets/framework/wells.scss @@ -93,6 +93,10 @@ font-size: 12px; } } + + svg { + vertical-align: text-top; + } } .light-well { diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 8cc5252648d..90a5250c247 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/app/assets/stylesheets/highlight/white_base.scss @@ -102,7 +102,9 @@ pre.code, // Diff line .line_holder { - &.match .line_content { + &.match .line_content, + .new-nonewline.line_content, + .old-nonewline.line_content { @include matchLine; } diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index e54f372344d..3fedd5bfb29 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -8,7 +8,6 @@ class HealthController < ActionController::Base Gitlab::HealthChecks::Redis::CacheCheck, Gitlab::HealthChecks::Redis::QueuesCheck, Gitlab::HealthChecks::Redis::SharedStateCheck, - Gitlab::HealthChecks::FsShardsCheck, Gitlab::HealthChecks::GitalyCheck ].freeze diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f5d94ad96a1..0190aa90763 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -270,7 +270,7 @@ module ApplicationHelper { members: members_project_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]), issues: issues_project_autocomplete_sources_path(object), - merge_requests: merge_requests_project_autocomplete_sources_path(object), + mergeRequests: merge_requests_project_autocomplete_sources_path(object), labels: labels_project_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]), milestones: milestones_project_autocomplete_sources_path(object), commands: commands_project_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]) diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 5b160ffba67..7606d68ff29 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -6,9 +6,9 @@ module MergeRequests # class PostMergeService < MergeRequests::BaseService def execute(merge_request) + merge_request.mark_as_merged close_issues(merge_request) todo_service.merge_merge_request(merge_request, current_user) - merge_request.mark_as_merged create_event(merge_request) create_note(merge_request) notification_service.merge_mr(merge_request, current_user) diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index c0083cd6afd..5b4bc86b9ba 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -18,10 +18,18 @@ module MergeRequests return false end + log_prefix = "#{self.class.name} info (#{merge_request.to_reference(full: true)}):" + + Gitlab::GitLogger.info("#{log_prefix} rebase started") + rebase_sha = repository.rebase(current_user, merge_request) + Gitlab::GitLogger.info("#{log_prefix} rebased to #{rebase_sha}") + merge_request.update_attributes(rebase_commit_sha: rebase_sha) + Gitlab::GitLogger.info("#{log_prefix} rebase SHA saved: #{rebase_sha}") + true rescue => e log_error(REBASE_ERROR, save_message_on_model: true) diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index 236e9fe8c44..51ff9eff5e4 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -6,7 +6,8 @@ class MetricsService Gitlab::HealthChecks::Redis::RedisCheck, Gitlab::HealthChecks::Redis::CacheCheck, Gitlab::HealthChecks::Redis::QueuesCheck, - Gitlab::HealthChecks::Redis::SharedStateCheck + Gitlab::HealthChecks::Redis::SharedStateCheck, + Gitlab::HealthChecks::GitalyCheck ].freeze def prometheus_metrics_text diff --git a/app/views/projects/environments/_external_url.html.haml b/app/views/projects/environments/_external_url.html.haml index a82ef5ee5bb..a264252e095 100644 --- a/app/views/projects/environments/_external_url.html.haml +++ b/app/views/projects/environments/_external_url.html.haml @@ -1,4 +1,4 @@ - if environment.external_url && can?(current_user, :read_environment, environment) = link_to environment.external_url, target: '_blank', rel: 'noopener noreferrer', class: 'btn external-url' do - = icon('external-link') + = sprite_icon('external-link') View deployment diff --git a/app/views/projects/environments/_metrics_button.html.haml b/app/views/projects/environments/_metrics_button.html.haml index b4102fcf103..a4b27575095 100644 --- a/app/views/projects/environments/_metrics_button.html.haml +++ b/app/views/projects/environments/_metrics_button.html.haml @@ -3,5 +3,5 @@ - return unless can?(current_user, :read_environment, environment) = link_to environment_metrics_path(environment), title: 'See metrics', class: 'btn metrics-button' do - = icon('area-chart') + = sprite_icon('chart') Monitoring diff --git a/app/views/projects/environments/_terminal_button.html.haml b/app/views/projects/environments/_terminal_button.html.haml index a6201bdbc42..38bc087664b 100644 --- a/app/views/projects/environments/_terminal_button.html.haml +++ b/app/views/projects/environments/_terminal_button.html.haml @@ -1,3 +1,3 @@ - if environment.has_terminals? && can?(current_user, :admin_environment, @project) = link_to terminal_project_environment_path(@project, environment), class: 'btn terminal-button' do - = icon('terminal') + = sprite_icon('terminal') diff --git a/app/views/projects/environments/terminal.html.haml b/app/views/projects/environments/terminal.html.haml index 6ec4ff56552..5b680189bc8 100644 --- a/app/views/projects/environments/terminal.html.haml +++ b/app/views/projects/environments/terminal.html.haml @@ -16,7 +16,7 @@ .nav-controls - if @environment.external_url.present? = link_to @environment.external_url, class: 'btn btn-default', target: '_blank', rel: 'noopener noreferrer nofollow' do - = icon('external-link') + = sprite_icon('external-link') = render 'projects/deployments/actions', deployment: @environment.last_deployment .terminal-container{ class: container_class } diff --git a/app/views/projects/services/mattermost_slash_commands/_detailed_help.html.haml b/app/views/projects/services/mattermost_slash_commands/_detailed_help.html.haml index 209b9c71390..9314804c5dd 100644 --- a/app/views/projects/services/mattermost_slash_commands/_detailed_help.html.haml +++ b/app/views/projects/services/mattermost_slash_commands/_detailed_help.html.haml @@ -6,13 +6,13 @@ 1. = link_to 'https://docs.mattermost.com/developer/slash-commands.html#enabling-custom-commands', target: '_blank', rel: 'noopener noreferrer nofollow' do Enable custom slash commands - = icon('external-link') + = sprite_icon('external-link', size: 16) on your Mattermost installation %li 2. = link_to 'https://docs.mattermost.com/developer/slash-commands.html#set-up-a-custom-command', target: '_blank', rel: 'noopener noreferrer nofollow' do Add a slash command - = icon('external-link') + = sprite_icon('external-link', size: 16) in your Mattermost team with these options: %hr diff --git a/app/views/projects/services/mattermost_slash_commands/_help.html.haml b/app/views/projects/services/mattermost_slash_commands/_help.html.haml index b20614dc88f..f51dd581d29 100644 --- a/app/views/projects/services/mattermost_slash_commands/_help.html.haml +++ b/app/views/projects/services/mattermost_slash_commands/_help.html.haml @@ -7,7 +7,7 @@ project by entering slash commands in Mattermost. = link_to help_page_path('user/project/integrations/mattermost_slash_commands.md'), target: '_blank' do View documentation - = icon('external-link') + = sprite_icon('external-link', size: 16) %p.inline See list of available commands in Mattermost after setting up this service, by entering diff --git a/app/views/projects/services/slack_slash_commands/_help.html.haml b/app/views/projects/services/slack_slash_commands/_help.html.haml index 9d045d84b52..f25d2ecdfb1 100644 --- a/app/views/projects/services/slack_slash_commands/_help.html.haml +++ b/app/views/projects/services/slack_slash_commands/_help.html.haml @@ -8,7 +8,7 @@ project by entering slash commands in Slack. = link_to help_page_path('user/project/integrations/slack_slash_commands.md'), target: '_blank' do View documentation - = icon('external-link') + = sprite_icon('external-link', size: 16) %p.inline See list of available commands in Slack after setting up this service, by entering @@ -20,7 +20,7 @@ 1. = link_to 'https://my.slack.com/services/new/slash-commands', target: '_blank', rel: 'noreferrer noopener nofollow' do Add a slash command - = icon('external-link') + = sprite_icon('external-link', size: 16) in your Slack team with these options: %hr diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 026f756582d..d06f51b1828 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -11,7 +11,7 @@ - cronjob:remove_old_web_hook_logs - cronjob:remove_unreferenced_lfs_objects - cronjob:repository_archive_cache -- cronjob:repository_check_batch +- cronjob:repository_check_dispatch - cronjob:requests_profiles - cronjob:schedule_update_user_activity - cronjob:stuck_ci_jobs @@ -71,6 +71,7 @@ - pipeline_processing:update_head_pipeline_for_merge_request - repository_check:repository_check_clear +- repository_check:repository_check_batch - repository_check:repository_check_single_repository - default diff --git a/app/workers/ci/archive_traces_cron_worker.rb b/app/workers/ci/archive_traces_cron_worker.rb index 2ac65f41f4e..7016edde698 100644 --- a/app/workers/ci/archive_traces_cron_worker.rb +++ b/app/workers/ci/archive_traces_cron_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Ci class ArchiveTracesCronWorker include ApplicationWorker diff --git a/app/workers/ci/build_trace_chunk_flush_worker.rb b/app/workers/ci/build_trace_chunk_flush_worker.rb index 218d6688bd9..6376c6d32cf 100644 --- a/app/workers/ci/build_trace_chunk_flush_worker.rb +++ b/app/workers/ci/build_trace_chunk_flush_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Ci class BuildTraceChunkFlushWorker include ApplicationWorker diff --git a/app/workers/concerns/application_worker.rb b/app/workers/concerns/application_worker.rb index 37586e161c9..bb06e31641d 100644 --- a/app/workers/concerns/application_worker.rb +++ b/app/workers/concerns/application_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Sidekiq::Worker.extend ActiveSupport::Concern module ApplicationWorker diff --git a/app/workers/concerns/cluster_applications.rb b/app/workers/concerns/cluster_applications.rb index 24ecaa0b52f..9758a1ceb0e 100644 --- a/app/workers/concerns/cluster_applications.rb +++ b/app/workers/concerns/cluster_applications.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ClusterApplications extend ActiveSupport::Concern diff --git a/app/workers/concerns/cluster_queue.rb b/app/workers/concerns/cluster_queue.rb index 24b9f145220..e44b40c36c9 100644 --- a/app/workers/concerns/cluster_queue.rb +++ b/app/workers/concerns/cluster_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + ## # Concern for setting Sidekiq settings for the various Gcp clusters workers. # diff --git a/app/workers/concerns/cronjob_queue.rb b/app/workers/concerns/cronjob_queue.rb index b6581779f6a..0683b229381 100644 --- a/app/workers/concerns/cronjob_queue.rb +++ b/app/workers/concerns/cronjob_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Concern that sets various Sidekiq settings for workers executed using a # cronjob. module CronjobQueue diff --git a/app/workers/concerns/each_shard_worker.rb b/app/workers/concerns/each_shard_worker.rb new file mode 100644 index 00000000000..d0a728fb495 --- /dev/null +++ b/app/workers/concerns/each_shard_worker.rb @@ -0,0 +1,31 @@ +module EachShardWorker + extend ActiveSupport::Concern + include ::Gitlab::Utils::StrongMemoize + + def each_eligible_shard + Gitlab::ShardHealthCache.update(eligible_shard_names) + + eligible_shard_names.each do |shard_name| + yield shard_name + end + end + + # override when you want to filter out some shards + def eligible_shard_names + healthy_shard_names + end + + def healthy_shard_names + strong_memoize(:healthy_shard_names) do + healthy_ready_shards.map { |result| result.labels[:shard] } + end + end + + def healthy_ready_shards + ready_shards.select(&:success) + end + + def ready_shards + Gitlab::HealthChecks::GitalyCheck.readiness + end +end diff --git a/app/workers/concerns/exception_backtrace.rb b/app/workers/concerns/exception_backtrace.rb index ea0f1f8d19b..37c9eaba0d7 100644 --- a/app/workers/concerns/exception_backtrace.rb +++ b/app/workers/concerns/exception_backtrace.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Concern for enabling a few lines of exception backtraces in Sidekiq module ExceptionBacktrace extend ActiveSupport::Concern diff --git a/app/workers/concerns/gitlab/github_import/queue.rb b/app/workers/concerns/gitlab/github_import/queue.rb index 22c2ce458e8..59b621f16ab 100644 --- a/app/workers/concerns/gitlab/github_import/queue.rb +++ b/app/workers/concerns/gitlab/github_import/queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Gitlab module GithubImport module Queue diff --git a/app/workers/concerns/mail_scheduler_queue.rb b/app/workers/concerns/mail_scheduler_queue.rb index f3e9680d756..c051151e973 100644 --- a/app/workers/concerns/mail_scheduler_queue.rb +++ b/app/workers/concerns/mail_scheduler_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module MailSchedulerQueue extend ActiveSupport::Concern diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index 526ed0bad07..7735dec5e6b 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module NewIssuable attr_reader :issuable, :user diff --git a/app/workers/concerns/object_storage_queue.rb b/app/workers/concerns/object_storage_queue.rb index a80f473a6d4..8650eed213a 100644 --- a/app/workers/concerns/object_storage_queue.rb +++ b/app/workers/concerns/object_storage_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Concern for setting Sidekiq settings for the various GitLab ObjectStorage workers. module ObjectStorageQueue extend ActiveSupport::Concern diff --git a/app/workers/concerns/pipeline_background_queue.rb b/app/workers/concerns/pipeline_background_queue.rb index 8bf43de6b26..bbb8ad0c982 100644 --- a/app/workers/concerns/pipeline_background_queue.rb +++ b/app/workers/concerns/pipeline_background_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + ## # Concern for setting Sidekiq settings for the low priority CI pipeline workers. # diff --git a/app/workers/concerns/pipeline_queue.rb b/app/workers/concerns/pipeline_queue.rb index e77093a6902..3aaed4669e5 100644 --- a/app/workers/concerns/pipeline_queue.rb +++ b/app/workers/concerns/pipeline_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + ## # Concern for setting Sidekiq settings for the various CI pipeline workers. # diff --git a/app/workers/concerns/project_import_options.rb b/app/workers/concerns/project_import_options.rb index ef23990ad97..22bdf441d6b 100644 --- a/app/workers/concerns/project_import_options.rb +++ b/app/workers/concerns/project_import_options.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ProjectImportOptions extend ActiveSupport::Concern diff --git a/app/workers/concerns/project_start_import.rb b/app/workers/concerns/project_start_import.rb index 4e55a1ee3d6..46a133db2a1 100644 --- a/app/workers/concerns/project_start_import.rb +++ b/app/workers/concerns/project_start_import.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Used in EE by mirroring module ProjectStartImport def start(project) diff --git a/app/workers/concerns/repository_check_queue.rb b/app/workers/concerns/repository_check_queue.rb index 43fb66c31b0..216d67e5dbc 100644 --- a/app/workers/concerns/repository_check_queue.rb +++ b/app/workers/concerns/repository_check_queue.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Concern for setting Sidekiq settings for the various repository check workers. module RepositoryCheckQueue extend ActiveSupport::Concern diff --git a/app/workers/concerns/waitable_worker.rb b/app/workers/concerns/waitable_worker.rb index 48ebe862248..d85bc7d1660 100644 --- a/app/workers/concerns/waitable_worker.rb +++ b/app/workers/concerns/waitable_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module WaitableWorker extend ActiveSupport::Concern diff --git a/app/workers/mail_scheduler/issue_due_worker.rb b/app/workers/mail_scheduler/issue_due_worker.rb index 54285884a52..8794ad7a82c 100644 --- a/app/workers/mail_scheduler/issue_due_worker.rb +++ b/app/workers/mail_scheduler/issue_due_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module MailScheduler class IssueDueWorker include ApplicationWorker diff --git a/app/workers/mail_scheduler/notification_service_worker.rb b/app/workers/mail_scheduler/notification_service_worker.rb index 7cfe0aa0df1..4726e416182 100644 --- a/app/workers/mail_scheduler/notification_service_worker.rb +++ b/app/workers/mail_scheduler/notification_service_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'active_job/arguments' module MailScheduler diff --git a/app/workers/object_storage/background_move_worker.rb b/app/workers/object_storage/background_move_worker.rb index 9c4d72e0ecf..8dff65e46e3 100644 --- a/app/workers/object_storage/background_move_worker.rb +++ b/app/workers/object_storage/background_move_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ObjectStorage class BackgroundMoveWorker include ApplicationWorker diff --git a/app/workers/repository_check/batch_worker.rb b/app/workers/repository_check/batch_worker.rb index 72f0a9b0619..051382a08a9 100644 --- a/app/workers/repository_check/batch_worker.rb +++ b/app/workers/repository_check/batch_worker.rb @@ -1,13 +1,20 @@ +# frozen_string_literal: true + module RepositoryCheck class BatchWorker include ApplicationWorker - include CronjobQueue + include RepositoryCheckQueue RUN_TIME = 3600 BATCH_SIZE = 10_000 - def perform + attr_reader :shard_name + + def perform(shard_name) + @shard_name = shard_name + return unless Gitlab::CurrentSettings.repository_checks_enabled + return unless Gitlab::ShardHealthCache.healthy_shard?(shard_name) start = Time.now @@ -37,18 +44,22 @@ module RepositoryCheck end def never_checked_project_ids(batch_size) - Project.where(last_repository_check_at: nil) + projects_on_shard.where(last_repository_check_at: nil) .where('created_at < ?', 24.hours.ago) .limit(batch_size).pluck(:id) end def old_checked_project_ids(batch_size) - Project.where.not(last_repository_check_at: nil) + projects_on_shard.where.not(last_repository_check_at: nil) .where('last_repository_check_at < ?', 1.month.ago) .reorder(last_repository_check_at: :asc) .limit(batch_size).pluck(:id) end + def projects_on_shard + Project.where(repository_storage: shard_name) + end + def try_obtain_lease(id) # Use a 24-hour timeout because on servers/projects where 'git fsck' is # super slow we definitely do not want to run it twice in parallel. diff --git a/app/workers/repository_check/clear_worker.rb b/app/workers/repository_check/clear_worker.rb index 97b89dc3db5..81e1a4b63bb 100644 --- a/app/workers/repository_check/clear_worker.rb +++ b/app/workers/repository_check/clear_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module RepositoryCheck class ClearWorker include ApplicationWorker diff --git a/app/workers/repository_check/dispatch_worker.rb b/app/workers/repository_check/dispatch_worker.rb new file mode 100644 index 00000000000..891a273afd7 --- /dev/null +++ b/app/workers/repository_check/dispatch_worker.rb @@ -0,0 +1,15 @@ +module RepositoryCheck + class DispatchWorker + include ApplicationWorker + include CronjobQueue + include ::EachShardWorker + + def perform + return unless Gitlab::CurrentSettings.repository_checks_enabled + + each_eligible_shard do |shard_name| + RepositoryCheck::BatchWorker.perform_async(shard_name) + end + end + end +end diff --git a/app/workers/repository_check/single_repository_worker.rb b/app/workers/repository_check/single_repository_worker.rb index 3cffb8b14e4..f44e5693b25 100644 --- a/app/workers/repository_check/single_repository_worker.rb +++ b/app/workers/repository_check/single_repository_worker.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module RepositoryCheck class SingleRepositoryWorker include ApplicationWorker diff --git a/changelogs/unreleased/19439-api-file-sha56-and-head.yml b/changelogs/unreleased/19439-api-file-sha56-and-head.yml new file mode 100644 index 00000000000..4bc1e560631 --- /dev/null +++ b/changelogs/unreleased/19439-api-file-sha56-and-head.yml @@ -0,0 +1,5 @@ +--- +title: Add SHA256 and HEAD on File API +merge_request: 19439 +author: ahmet2mir +type: added diff --git a/changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml b/changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml new file mode 100644 index 00000000000..7d42d971022 --- /dev/null +++ b/changelogs/unreleased/46546-do-not-pre-select-previous-user-s-when-creating-protected-branches.yml @@ -0,0 +1,5 @@ +--- +title: CE port gitlab-ee!6112 +merge_request: 19714 +author: +type: other diff --git a/changelogs/unreleased/48528-fix-mr-autocompletion.yml b/changelogs/unreleased/48528-fix-mr-autocompletion.yml new file mode 100644 index 00000000000..ac44f878d1d --- /dev/null +++ b/changelogs/unreleased/48528-fix-mr-autocompletion.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken '!' support to autocomplete MRs in GFM fields +merge_request: 20204 +author: +type: fixed diff --git a/changelogs/unreleased/add-more-rebase-logging.yml b/changelogs/unreleased/add-more-rebase-logging.yml new file mode 100644 index 00000000000..a7d1c3aa664 --- /dev/null +++ b/changelogs/unreleased/add-more-rebase-logging.yml @@ -0,0 +1,5 @@ +--- +title: Add more detailed logging to githost.log when rebasing +merge_request: +author: +type: other diff --git a/changelogs/unreleased/frozen-string-enable-app-workers-2.yml b/changelogs/unreleased/frozen-string-enable-app-workers-2.yml new file mode 100644 index 00000000000..81de6899d76 --- /dev/null +++ b/changelogs/unreleased/frozen-string-enable-app-workers-2.yml @@ -0,0 +1,5 @@ +--- +title: Finish enabling frozen string for app/workers/*.rb +merge_request: 20197 +author: gfyoung +type: other diff --git a/changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml b/changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml new file mode 100644 index 00000000000..2049afc3d44 --- /dev/null +++ b/changelogs/unreleased/osw-mark-as-merged-as-first-post-merge-action.yml @@ -0,0 +1,5 @@ +--- +title: Mark MR as merged regardless of errors when closing issues +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/tc-repo-check-per-shard.yml b/changelogs/unreleased/tc-repo-check-per-shard.yml new file mode 100644 index 00000000000..227b6b0b93b --- /dev/null +++ b/changelogs/unreleased/tc-repo-check-per-shard.yml @@ -0,0 +1,5 @@ +--- +title: Run repository checks in parallel for each shard +merge_request: 20179 +author: +type: added diff --git a/changelogs/unreleased/update-environments-nav-controls.yml b/changelogs/unreleased/update-environments-nav-controls.yml new file mode 100644 index 00000000000..639dadd0cdf --- /dev/null +++ b/changelogs/unreleased/update-environments-nav-controls.yml @@ -0,0 +1,5 @@ +--- +title: Update environments nav controls icons +merge_request: 20199 +author: George Tsiolis +type: changed diff --git a/changelogs/unreleased/update-integrations-external-link-icons.yml b/changelogs/unreleased/update-integrations-external-link-icons.yml new file mode 100644 index 00000000000..9972744bd00 --- /dev/null +++ b/changelogs/unreleased/update-integrations-external-link-icons.yml @@ -0,0 +1,5 @@ +--- +title: Update integrations external link icons +merge_request: 20205 +author: George Tsiolis +type: changed diff --git a/changelogs/unreleased/zj-gitaly-read-write-check.yml b/changelogs/unreleased/zj-gitaly-read-write-check.yml new file mode 100644 index 00000000000..43951d20e8f --- /dev/null +++ b/changelogs/unreleased/zj-gitaly-read-write-check.yml @@ -0,0 +1,5 @@ +--- +title: Gitaly metrics check for read/writeability +merge_request: 20022 +author: +type: other diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 489dc8840e5..e0779112850 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -33,7 +33,7 @@ production: &base port: 80 # Set to 443 if using HTTPS, see installation.md#using-https for additional HTTPS configuration details https: false # Set to true if using HTTPS, see installation.md#using-https for additional HTTPS configuration details - # Uncommment this line below if your ssh host is different from HTTP/HTTPS one + # Uncomment this line below if your ssh host is different from HTTP/HTTPS one # (you'd obviously need to replace ssh.host_example.com with your own host). # Otherwise, ssh host will be set to the `host:` value above # ssh_host: ssh.host_example.com diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 3d3448cb4d6..550647ae1c6 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -279,7 +279,7 @@ Settings.cron_jobs['expire_build_artifacts_worker']['cron'] ||= '50 * * * *' Settings.cron_jobs['expire_build_artifacts_worker']['job_class'] = 'ExpireBuildArtifactsWorker' Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *' -Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::BatchWorker' +Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::DispatchWorker' Settings.cron_jobs['admin_email_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['admin_email_worker']['cron'] ||= '0 0 * * 0' Settings.cron_jobs['admin_email_worker']['job_class'] = 'AdminEmailWorker' @@ -394,7 +394,7 @@ repositories_storages = Settings.repositories.storages.values repository_downloads_path = Settings.gitlab['repository_downloads_path'].to_s.gsub(%r{/$}, '') repository_downloads_full_path = File.expand_path(repository_downloads_path, Settings.gitlab['user_home']) -# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1237 +# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1255 Gitlab::GitalyClient::StorageSettings.allow_disk_access do if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs.legacy_disk_path.gsub(%r{/$}, '')) } Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index ff6865608f0..bf9e5a50382 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -2,20 +2,6 @@ def storage_name_valid?(name) !!(name =~ /\A[a-zA-Z0-9\-_]+\z/) end -def find_parent_path(name, path) - parent = Pathname.new(path).realpath.parent - Gitlab.config.repositories.storages.detect do |n, rs| - name != n && Pathname.new(rs.legacy_disk_path).realpath == parent - end -rescue Errno::EIO, Errno::ENOENT => e - warning = "WARNING: couldn't verify #{path} (#{name}). "\ - "If this is an external storage, it might be offline." - message = "#{warning}\n#{e.message}" - Rails.logger.error("#{message}\n\t" + e.backtrace.join("\n\t")) - - nil -end - def storage_validation_error(message) raise "#{message}. Please fix this in your gitlab.yml before starting GitLab." end @@ -37,17 +23,4 @@ def validate_storages_config end end -# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1237 -def validate_storages_paths - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab.config.repositories.storages.each do |name, repository_storage| - parent_name, _parent_path = find_parent_path(name, repository_storage.legacy_disk_path) - if parent_name - storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") - end - end - end -end - validate_storages_config -validate_storages_paths unless Rails.env.test? || ENV['SKIP_STORAGE_VALIDATION'] == 'true' diff --git a/doc/administration/repository_storage_types.md b/doc/administration/repository_storage_types.md index 39bd19ac851..087fe729b28 100644 --- a/doc/administration/repository_storage_types.md +++ b/doc/administration/repository_storage_types.md @@ -82,6 +82,46 @@ To migrate your existing projects to the new storage type, check the specific [rake tasks]: raketasks/storage.md#migrate-existing-projects-to-hashed-storage [storage-paths]: repository_storage_types.md +#### Rollback + +There is no automated rollback implemented. Below are the steps required to rollback +from each storage migration. + +The rollback has to be performed in the reverse order. To get into "Legacy" state, +you need to rollback Attachments first, then Project. + +Also note that if Geo is enabled, after the migration was triggered, an event is generated +to replicate the operation on any Secondary node. That means the on disk changes will also +need to be performed on these nodes as well. Database changes will propagate without issues. + +You must make sure the migration event was already processed or otherwise it may migrate +the files back to Hashed state again. + +##### Attachments + +To rollback single Attachment migration, rename `aa/bb/abcdef1234567890...` folder back to `namespace/project`. + +Both folder names can be generated by the `FileUploader.absolute_base_dir(project)`, you +just need to switch the version from the `project` back to the previous one. + +```ruby +project.storage_version +# => 2 + +FileUploader.absolute_base_dir(project) +# => "/opt/gitlab/embedded/service/gitlab-rails/public/uploads/@hashed/d4/73/d4735e3a265e16eee03f59718b9b5d03019c07d8b6c51f90da3a666eec13ab35" + +project.storage_version = 1 + +FileUploader.absolute_base_dir(project) +# => "/opt/gitlab/embedded/service/gitlab-rails/public/uploads/gitlab/gitlab-shell-renamed" +``` + +##### Project + +To rollback single Project migration, move `@hashed/aa/bb/aabbcdef1234567890abcdef.git` and `@hashed/aa/bb/aabbcdef1234567890abcdef.wiki.git` +back to `namespace/project.git` and `namespace/project.wiki.git` respectively and switch the version from the `project` back to `null`. + ### Hashed Storage coverage We are incrementally moving every storable object in GitLab to the Hashed @@ -100,6 +140,30 @@ which is true for CI Cache and LFS Objects. | Pages | Yes | No | - | - | | Docker Registry | Yes | No | - | - | | CI Build Logs | No | No | - | - | -| CI Artifacts | No | No | Yes (Premium) | - | +| CI Artifacts | No | No | Yes | 9.4 / 10.6 | | CI Cache | No | No | Yes | - | -| LFS Objects | Yes | No | Yes (Premium) | - | +| LFS Objects | Yes | Similar | Yes | 10.0 / 10.7 | + +#### Implementation Details + +##### Avatars + +Each file is stored in a folder with its `id` from the database. The filename is always `avatar.png` for user avatars. +When avatar is replaced, `Upload` model is destroyed and a new one takes place with different `id`. + +##### CI Artifacts + +CI Artifacts are S3 compatible since **9.4** (GitLab Premium), and available in GitLab Core since **10.6**. + +##### LFS Objects + +LFS Objects implements a similar storage pattern using 2 chars, 2 level folders, following git own implementation: + +```ruby +"shared/lfs-objects/#{oid[0..1}/#{oid[2..3]}/#{oid[4..-1]}" + +# Based on object `oid`: `8909029eb962194cfb326259411b22ae3f4a814b5be4f80651735aeef9f3229c`, path will be: +"shared/lfs-objects/89/09/029eb962194cfb326259411b22ae3f4a814b5be4f80651735aeef9f3229c" +``` + +They are also S3 compatible since **10.0** (GitLab Premium), and available in GitLab Core since **10.7**. diff --git a/doc/api/repository_files.md b/doc/api/repository_files.md index c29dc22e12d..49fb9bc141d 100644 --- a/doc/api/repository_files.md +++ b/doc/api/repository_files.md @@ -27,6 +27,7 @@ Example response: "size": 1476, "encoding": "base64", "content": "IyA9PSBTY2hlbWEgSW5mb3...", + "content_sha256": "4c294617b60715c1d218e61164a3abd4808a4284cbc30e6728a01ad9aada4481", "ref": "master", "blob_id": "79f7bbd25901e8334750839545a9bd021f0e4c83", "commit_id": "d5a3ff139356ce33e37e73add446f16869741b50", @@ -39,6 +40,36 @@ Parameters: - `file_path` (required) - Url encoded full path to new file. Ex. lib%2Fclass%2Erb - `ref` (required) - The name of branch, tag or commit +NOTE: **Note:** +`blob_id` is the blob sha, see [repositories - Get a blob from repository](repositories.md#get-a-blob-from-repository) + +In addition to the `GET` method, you can also use `HEAD` to get just file metadata. + +``` +HEAD /projects/:id/repository/files/:file_path +``` + +```bash +curl --head --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v4/projects/13083/repository/files/app%2Fmodels%2Fkey%2Erb?ref=master' +``` + +Example response: + +```text +HTTP/1.1 200 OK +... +X-Gitlab-Blob-Id: 79f7bbd25901e8334750839545a9bd021f0e4c83 +X-Gitlab-Commit-Id: d5a3ff139356ce33e37e73add446f16869741b50 +X-Gitlab-Content-Sha256: 4c294617b60715c1d218e61164a3abd4808a4284cbc30e6728a01ad9aada4481 +X-Gitlab-Encoding: base64 +X-Gitlab-File-Name: key.rb +X-Gitlab-File-Path: app/models/key.rb +X-Gitlab-Last-Commit-Id: 570e7b2abdd848b95f2f578043fc23bd6f6fd24d +X-Gitlab-Ref: master +X-Gitlab-Size: 1476 +... +``` + ## Get raw file from repository ``` @@ -54,6 +85,9 @@ Parameters: - `file_path` (required) - Url encoded full path to new file. Ex. lib%2Fclass%2Erb - `ref` (required) - The name of branch, tag or commit +NOTE: **Note:** +Like [Get file from repository](repository_files.md#get-file-from-repository) you can use `HEAD` to get just file metadata. + ## Create new file in repository ``` diff --git a/lib/api/files.rb b/lib/api/files.rb index 1598d3c00b8..29d7489bd7c 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -5,6 +5,8 @@ module API # Prevents returning plain/text responses for files with .txt extension after_validation { content_type "application/json" } + helpers ::API::Helpers::HeadersHelpers + helpers do def commit_params(attrs) { @@ -40,6 +42,20 @@ module API } end + def blob_data + { + file_name: @blob.name, + file_path: @blob.path, + size: @blob.size, + encoding: "base64", + content_sha256: Digest::SHA256.hexdigest(@blob.data), + ref: params[:ref], + blob_id: @blob.id, + commit_id: @commit.id, + last_commit_id: @repo.last_commit_id_for_path(@commit.sha, params[:file_path]) + } + end + params :simple_file_params do requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.' @@ -61,6 +77,17 @@ module API requires :id, type: String, desc: 'The project ID' end resource :projects, requirements: FILE_ENDPOINT_REQUIREMENTS do + desc 'Get raw file metadata from repository' + params do + requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' + requires :ref, type: String, desc: 'The name of branch, tag or commit' + end + head ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do + assign_file_vars! + + set_http_headers(blob_data) + end + desc 'Get raw file contents from the repository' params do requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' @@ -69,9 +96,22 @@ module API get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do assign_file_vars! + set_http_headers(blob_data) + send_git_blob @repo, @blob end + desc 'Get file metadata from repository' + params do + requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' + requires :ref, type: String, desc: 'The name of branch, tag or commit' + end + head ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do + assign_file_vars! + + set_http_headers(blob_data) + end + desc 'Get a file from the repository' params do requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' @@ -80,17 +120,11 @@ module API get ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do assign_file_vars! - { - file_name: @blob.name, - file_path: @blob.path, - size: @blob.size, - encoding: "base64", - content: Base64.strict_encode64(@blob.data), - ref: params[:ref], - blob_id: @blob.id, - commit_id: @commit.id, - last_commit_id: @repo.last_commit_id_for_path(@commit.sha, params[:file_path]) - } + data = blob_data + + set_http_headers(data) + + data.merge(content: Base64.strict_encode64(@blob.data)) end desc 'Create new file in repository' diff --git a/lib/api/helpers/headers_helpers.rb b/lib/api/helpers/headers_helpers.rb new file mode 100644 index 00000000000..cde51fccc62 --- /dev/null +++ b/lib/api/helpers/headers_helpers.rb @@ -0,0 +1,11 @@ +module API + module Helpers + module HeadersHelpers + def set_http_headers(header_data) + header_data.each do |key, value| + header "X-Gitlab-#{key.to_s.split('_').collect(&:capitalize).join('-')}", value + end + end + end + end +end diff --git a/lib/gitaly/server.rb b/lib/gitaly/server.rb index 605e93022e7..2760211fee8 100644 --- a/lib/gitaly/server.rb +++ b/lib/gitaly/server.rb @@ -22,6 +22,18 @@ module Gitaly server_version == Gitlab::GitalyClient.expected_server_version end + def read_writeable? + readable? && writeable? + end + + def readable? + storage_status&.readable + end + + def writeable? + storage_status&.writeable + end + def address Gitlab::GitalyClient.address(@storage) rescue RuntimeError => e @@ -30,13 +42,17 @@ module Gitaly private + def storage_status + @storage_status ||= info.storage_statuses.find { |s| s.storage_name == storage } + end + def info @info ||= begin Gitlab::GitalyClient::ServerService.new(@storage).info rescue GRPC::Unavailable, GRPC::GRPC::DeadlineExceeded # This will show the server as being out of date - Gitaly::ServerInfoResponse.new(git_version: '', server_version: '') + Gitaly::ServerInfoResponse.new(git_version: '', server_version: '', storage_statuses: []) end end end diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb deleted file mode 100644 index 050fe7a5173..00000000000 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ /dev/null @@ -1,169 +0,0 @@ -module Gitlab - module HealthChecks - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/1218 - class FsShardsCheck - extend BaseAbstractCheck - RANDOM_STRING = SecureRandom.hex(1000).freeze - COMMAND_TIMEOUT = '1'.freeze - TIMEOUT_EXECUTABLE = 'timeout'.freeze - - class << self - def readiness - repository_storages.map do |storage_name| - begin - if !storage_circuitbreaker_test(storage_name) - HealthChecks::Result.new(false, 'circuitbreaker tripped', shard: storage_name) - elsif !storage_stat_test(storage_name) - HealthChecks::Result.new(false, 'cannot stat storage', shard: storage_name) - else - with_temp_file(storage_name) do |tmp_file_path| - if !storage_write_test(tmp_file_path) - HealthChecks::Result.new(false, 'cannot write to storage', shard: storage_name) - elsif !storage_read_test(tmp_file_path) - HealthChecks::Result.new(false, 'cannot read from storage', shard: storage_name) - else - HealthChecks::Result.new(true, nil, shard: storage_name) - end - end - end - rescue RuntimeError => ex - message = "unexpected error #{ex} when checking storage #{storage_name}" - Rails.logger.error(message) - HealthChecks::Result.new(false, message, shard: storage_name) - end - end - end - - def metrics - repository_storages.flat_map do |storage_name| - [ - storage_stat_metrics(storage_name), - storage_write_metrics(storage_name), - storage_read_metrics(storage_name), - storage_circuitbreaker_metrics(storage_name) - ].flatten - end - end - - private - - def operation_metrics(ok_metric, latency_metric, **labels) - result, elapsed = yield - [ - metric(latency_metric, elapsed, **labels), - metric(ok_metric, result ? 1 : 0, **labels) - ] - rescue RuntimeError => ex - Rails.logger.error("unexpected error #{ex} when checking #{ok_metric}") - [metric(ok_metric, 0, **labels)] - end - - def repository_storages - storages_paths.keys - end - - def storages_paths - Gitlab.config.repositories.storages - end - - def exec_with_timeout(cmd_args, *args, &block) - Gitlab::Popen.popen([TIMEOUT_EXECUTABLE, COMMAND_TIMEOUT].concat(cmd_args), *args, &block) - end - - def with_temp_file(storage_name) - temp_file_path = Dir::Tmpname.create(%w(fs_shards_check +deleted), storage_path(storage_name)) { |path| path } - yield temp_file_path - ensure - delete_test_file(temp_file_path) - end - - def storage_path(storage_name) - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - storages_paths[storage_name]&.legacy_disk_path - end - end - - # All below test methods use shell commands to perform actions on storage volumes. - # In case a storage volume have connectivity problems causing pure Ruby IO operation to wait indefinitely, - # we can rely on shell commands to be terminated once `timeout` kills them. - # - # However we also fallback to pure Ruby file operations in case a specific shell command is missing - # so we are still able to perform healthchecks and gather metrics from such system. - - def delete_test_file(tmp_path) - _, status = exec_with_timeout(%W{ rm -f #{tmp_path} }) - status.zero? - rescue Errno::ENOENT - File.delete(tmp_path) rescue Errno::ENOENT - end - - def storage_stat_test(storage_name) - stat_path = File.join(storage_path(storage_name), '.') - begin - _, status = exec_with_timeout(%W{ stat #{stat_path} }) - status.zero? - rescue Errno::ENOENT - File.exist?(stat_path) && File::Stat.new(stat_path).readable? - end - end - - def storage_write_test(tmp_path) - _, status = exec_with_timeout(%W{ tee #{tmp_path} }) do |stdin| - stdin.write(RANDOM_STRING) - end - status.zero? - rescue Errno::ENOENT - written_bytes = File.write(tmp_path, RANDOM_STRING) rescue Errno::ENOENT - written_bytes == RANDOM_STRING.length - end - - def storage_read_test(tmp_path) - _, status = exec_with_timeout(%W{ diff #{tmp_path} - }) do |stdin| - stdin.write(RANDOM_STRING) - end - status.zero? - rescue Errno::ENOENT - file_contents = File.read(tmp_path) rescue Errno::ENOENT - file_contents == RANDOM_STRING - end - - def storage_circuitbreaker_test(storage_name) - Gitlab::Git::Storage::CircuitBreaker.build(storage_name).perform { "OK" } - rescue Gitlab::Git::Storage::Inaccessible - nil - end - - def storage_stat_metrics(storage_name) - operation_metrics(:filesystem_accessible, :filesystem_access_latency_seconds, shard: storage_name) do - with_timing { storage_stat_test(storage_name) } - end - end - - def storage_write_metrics(storage_name) - operation_metrics(:filesystem_writable, :filesystem_write_latency_seconds, shard: storage_name) do - with_temp_file(storage_name) do |tmp_file_path| - with_timing { storage_write_test(tmp_file_path) } - end - end - end - - def storage_read_metrics(storage_name) - operation_metrics(:filesystem_readable, :filesystem_read_latency_seconds, shard: storage_name) do - with_temp_file(storage_name) do |tmp_file_path| - storage_write_test(tmp_file_path) # writes data used by read test - with_timing { storage_read_test(tmp_file_path) } - end - end - end - - def storage_circuitbreaker_metrics(storage_name) - operation_metrics(:filesystem_circuitbreaker, - :filesystem_circuitbreaker_latency_seconds, - shard: storage_name) do - with_timing { storage_circuitbreaker_test(storage_name) } - end - end - end - end - end -end diff --git a/lib/gitlab/health_checks/gitaly_check.rb b/lib/gitlab/health_checks/gitaly_check.rb index 11416c002e3..1f623e0b6ec 100644 --- a/lib/gitlab/health_checks/gitaly_check.rb +++ b/lib/gitlab/health_checks/gitaly_check.rb @@ -13,14 +13,14 @@ module Gitlab end def metrics - repository_storages.flat_map do |storage_name| - result, elapsed = with_timing { check(storage_name) } - labels = { shard: storage_name } + Gitaly::Server.all.flat_map do |server| + result, elapsed = with_timing { server.read_writeable? } + labels = { shard: server.storage } [ - metric("#{metric_prefix}_success", successful?(result) ? 1 : 0, **labels), + metric("#{metric_prefix}_success", result ? 1 : 0, **labels), metric("#{metric_prefix}_latency_seconds", elapsed, **labels) - ].flatten + ] end end @@ -36,10 +36,6 @@ module Gitlab METRIC_PREFIX end - def successful?(result) - result[:success] - end - def repository_storages storages.keys end diff --git a/lib/gitlab/shard_health_cache.rb b/lib/gitlab/shard_health_cache.rb new file mode 100644 index 00000000000..3f03f46d4b1 --- /dev/null +++ b/lib/gitlab/shard_health_cache.rb @@ -0,0 +1,41 @@ +module Gitlab + class ShardHealthCache + HEALTHY_SHARDS_KEY = 'gitlab-healthy-shards'.freeze + HEALTHY_SHARDS_TIMEOUT = 300 + + # Clears the Redis set storing the list of healthy shards + def self.clear + Gitlab::Redis::Cache.with { |redis| redis.del(HEALTHY_SHARDS_KEY) } + end + + # Updates the list of healthy shards using a Redis set + # + # shards - An array of shard names to store + def self.update(shards) + Gitlab::Redis::Cache.with do |redis| + redis.multi do |m| + m.del(HEALTHY_SHARDS_KEY) + shards.each { |shard_name| m.sadd(HEALTHY_SHARDS_KEY, shard_name) } + m.expire(HEALTHY_SHARDS_KEY, HEALTHY_SHARDS_TIMEOUT) + end + end + end + + # Returns an array of strings of healthy shards + def self.cached_healthy_shards + Gitlab::Redis::Cache.with { |redis| redis.smembers(HEALTHY_SHARDS_KEY) } + end + + # Checks whether the given shard name is in the list of healthy shards. + # + # shard_name - The string to check + def self.healthy_shard?(shard_name) + Gitlab::Redis::Cache.with { |redis| redis.sismember(HEALTHY_SHARDS_KEY, shard_name) } + end + + # Returns the number of healthy shards in the Redis set + def self.healthy_shard_count + Gitlab::Redis::Cache.with { |redis| redis.scard(HEALTHY_SHARDS_KEY) } + end + end +end diff --git a/lib/gitlab/slash_commands/presenters/access.rb b/lib/gitlab/slash_commands/presenters/access.rb index 1a817eb735b..81f7cd3ffe8 100644 --- a/lib/gitlab/slash_commands/presenters/access.rb +++ b/lib/gitlab/slash_commands/presenters/access.rb @@ -15,7 +15,7 @@ module Gitlab if @resource ":wave: Hi there! Before I do anything for you, please [connect your GitLab account](#{@resource})." else - ":sweat_smile: Couldn't identify you, nor can I autorize you!" + ":sweat_smile: Couldn't identify you, nor can I authorize you!" end ephemeral_response(text: message) diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index 542eddc2d16..d800ad7c187 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -69,8 +69,7 @@ describe HealthController do expect(json_response['cache_check']['status']).to eq('ok') expect(json_response['queues_check']['status']).to eq('ok') expect(json_response['shared_state_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['labels']['shard']).to eq('default') + expect(json_response['gitaly_check']['status']).to eq('ok') end end @@ -122,7 +121,6 @@ describe HealthController do expect(json_response['cache_check']['status']).to eq('ok') expect(json_response['queues_check']['status']).to eq('ok') expect(json_response['shared_state_check']['status']).to eq('ok') - expect(json_response['fs_shards_check']['status']).to eq('ok') end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 9e8a37171ec..7376841fac8 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -59,6 +59,13 @@ describe MetricsController do expect(response.body).to match(/^redis_shared_state_ping_latency_seconds [0-9\.]+$/) end + it 'returns Gitaly metrics' do + get :index + + expect(response.body).to match(/^gitaly_health_check_success{shard="default"} 1$/) + expect(response.body).to match(/^gitaly_health_check_latency_seconds{shard="default"} [0-9\.]+$/) + end + context 'prometheus metrics are disabled' do before do allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(false) diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 4c0f9971425..39bd4af6cd0 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -60,33 +60,6 @@ feature 'Protected Branches', :js do expect(page).to have_content('No branches to show') end end - - describe "Saved defaults" do - it "keeps the allowed to merge and push dropdowns defaults based on the previous selection" do - visit project_protected_branches_path(project) - form = '.js-new-protected-branch' - - within form do - find(".js-allowed-to-merge").click - wait_for_requests - click_link 'No one' - find(".js-allowed-to-push").click - wait_for_requests - click_link 'Developers + Maintainers' - end - - visit project_protected_branches_path(project) - - within form do - page.within(".js-allowed-to-merge") do - expect(page.find(".dropdown-toggle-text")).to have_content("No one") - end - page.within(".js-allowed-to-push") do - expect(page.find(".dropdown-toggle-text")).to have_content("Developers + Maintainers") - end - end - end - end end context 'logged in as admin' do @@ -97,6 +70,7 @@ feature 'Protected Branches', :js do describe "explicit protected branches" do it "allows creating explicit protected branches" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -110,6 +84,7 @@ feature 'Protected Branches', :js do project.repository.add_branch(admin, 'some-branch', commit.id) visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -118,6 +93,7 @@ feature 'Protected Branches', :js do it "displays an error message if the named branch does not exist" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('some-branch') click_on "Protect" @@ -128,6 +104,7 @@ feature 'Protected Branches', :js do describe "wildcard protected branches" do it "allows creating protected branches with a wildcard" do visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('*-stable') click_on "Protect" @@ -141,6 +118,7 @@ feature 'Protected Branches', :js do project.repository.add_branch(admin, 'staging-stable', 'master') visit project_protected_branches_path(project) + set_defaults set_protected_branch_name('*-stable') click_on "Protect" @@ -157,6 +135,7 @@ feature 'Protected Branches', :js do visit project_protected_branches_path(project) set_protected_branch_name('*-stable') + set_defaults click_on "Protect" visit project_protected_branches_path(project) @@ -180,4 +159,18 @@ feature 'Protected Branches', :js do find(".dropdown-input-field").set(branch_name) click_on("Create wildcard #{branch_name}") end + + def set_defaults + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 593b2ca1825..14297a1a544 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -157,7 +157,7 @@ describe ApplicationHelper do let(:noteable_type) { Issue } it 'returns paths for autocomplete_sources_controller' do sources = helper.autocomplete_data_sources(project, noteable_type) - expect(sources.keys).to match_array([:members, :issues, :merge_requests, :labels, :milestones, :commands]) + expect(sources.keys).to match_array([:members, :issues, :mergeRequests, :labels, :milestones, :commands]) sources.keys.each do |key| expect(sources[key]).not_to be_nil end diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 8d9dc092547..f96e5a2133f 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -44,49 +44,6 @@ describe '6_validations' do end end - describe 'validate_storages_paths' do - context 'with correct settings' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/d')) - end - - it 'passes through' do - expect { validate_storages_paths }.not_to raise_error - end - end - - context 'with nested storage paths' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c/d')) - end - - it 'throws an error' do - expect { validate_storages_paths }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.') - end - end - - context 'with similar but un-nested storage paths' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c'), 'bar' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/paths/a/b/c2')) - end - - it 'passes through' do - expect { validate_storages_paths }.not_to raise_error - end - end - - describe 'inaccessible storage' do - before do - mock_storages('foo' => Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/tests/a/path/that/does/not/exist')) - end - - it 'passes through with a warning' do - expect(Rails.logger).to receive(:error) - expect { validate_storages_paths }.not_to raise_error - end - end - end - def mock_storages(storages) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) end diff --git a/spec/javascripts/boards/issue_card_spec.js b/spec/javascripts/boards/issue_card_spec.js index 05acf903933..72e961c8a10 100644 --- a/spec/javascripts/boards/issue_card_spec.js +++ b/spec/javascripts/boards/issue_card_spec.js @@ -9,7 +9,7 @@ import '~/vue_shared/models/assignee'; import '~/boards/models/issue'; import '~/boards/models/list'; import '~/boards/stores/boards_store'; -import '~/boards/components/issue_card_inner'; +import IssueCardInner from '~/boards/components/issue_card_inner.vue'; import { listObj } from './mock_data'; describe('Issue card component', () => { @@ -48,7 +48,7 @@ describe('Issue card component', () => { component = new Vue({ el: document.querySelector('.test-container'), components: { - 'issue-card': gl.issueBoards.IssueCardInner, + 'issue-card': IssueCardInner, }, data() { return { diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js index cce10c4083c..2d136a63c52 100644 --- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js @@ -2,12 +2,6 @@ import Vue from 'vue'; import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue'; import store from '~/mr_notes/stores'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import { - MATCH_LINE_TYPE, - CONTEXT_LINE_TYPE, - OLD_NO_NEW_LINE_TYPE, - NEW_NO_NEW_LINE_TYPE, -} from '~/diffs/constants'; import discussionsMockData from '../mock_data/diff_discussions'; import diffFileMockData from '../mock_data/diff_file'; @@ -31,45 +25,6 @@ describe('DiffLineGutterContent', () => { }; describe('computed', () => { - describe('isMatchLine', () => { - it('should return true for match line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isMatchLine).toEqual(true); - }); - - it('should return false for non-match line type', () => { - const component = createComponent({ lineType: CONTEXT_LINE_TYPE }); - expect(component.isMatchLine).toEqual(false); - }); - }); - - describe('isContextLine', () => { - it('should return true for context line type', () => { - const component = createComponent({ lineType: CONTEXT_LINE_TYPE }); - expect(component.isContextLine).toEqual(true); - }); - - it('should return false for non-context line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isContextLine).toEqual(false); - }); - }); - - describe('isMetaLine', () => { - it('should return true for meta line type', () => { - const component = createComponent({ lineType: NEW_NO_NEW_LINE_TYPE }); - expect(component.isMetaLine).toEqual(true); - - const component2 = createComponent({ lineType: OLD_NO_NEW_LINE_TYPE }); - expect(component2.isMetaLine).toEqual(true); - }); - - it('should return false for non-meta line type', () => { - const component = createComponent({ lineType: MATCH_LINE_TYPE }); - expect(component.isMetaLine).toEqual(false); - }); - }); - describe('lineHref', () => { it('should prepend # to lineCode', () => { const lineCode = 'LC_42'; @@ -109,7 +64,7 @@ describe('DiffLineGutterContent', () => { describe('template', () => { it('should render three dots for context lines', () => { const component = createComponent({ - lineType: MATCH_LINE_TYPE, + isMatchLine: true, }); expect(component.$el.querySelector('span').classList.contains('context-cell')).toEqual(true); diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js index 0d5a3576204..e1adf60962e 100644 --- a/spec/javascripts/diffs/components/inline_diff_view_spec.js +++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js @@ -1,7 +1,6 @@ import Vue from 'vue'; import InlineDiffView from '~/diffs/components/inline_diff_view.vue'; import store from '~/mr_notes/stores'; -import * as constants from '~/diffs/constants'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffFileMockData from '../mock_data/diff_file'; import discussionsMockData from '../mock_data/diff_discussions'; @@ -14,58 +13,13 @@ describe('InlineDiffView', () => { beforeEach(() => { const diffFile = getDiffFileMock(); + store.dispatch('setInlineDiffViewType'); component = createComponentWithStore(Vue.extend(InlineDiffView), store, { diffFile, diffLines: diffFile.highlightedDiffLines, }).$mount(); }); - describe('methods', () => { - describe('handleMouse', () => { - it('should set hoveredLineCode', () => { - expect(component.hoveredLineCode).toEqual(null); - - component.handleMouse('lineCode1', true); - expect(component.hoveredLineCode).toEqual('lineCode1'); - - component.handleMouse('lineCode1', false); - expect(component.hoveredLineCode).toEqual(null); - }); - }); - - describe('getLineClass', () => { - it('should return line class object', () => { - const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants; - const { MATCH_LINE_TYPE, NEW_LINE_TYPE } = constants; - - expect(component.getLineClass(component.diffLines[0])).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: false, - }); - - component.handleMouse(component.diffLines[0].lineCode, true); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, - }); - - expect(component.getLineClass(component.diffLines[0])).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: true, - }); - - expect(component.getLineClass(component.diffLines[5])).toEqual({ - [MATCH_LINE_TYPE]: MATCH_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: true, - [LINE_HOVER_CLASS_NAME]: false, - }); - }); - }); - }); - describe('template', () => { it('should have rendered diff lines', () => { const el = component.$el; @@ -89,23 +43,5 @@ describe('InlineDiffView', () => { done(); }); }); - - it('should render new discussion forms', done => { - const el = component.$el; - const lines = getDiffFileMock().highlightedDiffLines; - - component.handleShowCommentForm({ lineCode: lines[0].lineCode }); - component.handleShowCommentForm({ lineCode: lines[1].lineCode }); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2); - expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true); - expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true); - - store.state.diffs.diffLineCommentForms = {}; - - done(); - }); - }); }); }); diff --git a/spec/javascripts/diffs/components/parallel_diff_view_spec.js b/spec/javascripts/diffs/components/parallel_diff_view_spec.js index cab533217c0..165e4b69b6c 100644 --- a/spec/javascripts/diffs/components/parallel_diff_view_spec.js +++ b/spec/javascripts/diffs/components/parallel_diff_view_spec.js @@ -4,12 +4,10 @@ import store from '~/mr_notes/stores'; import * as constants from '~/diffs/constants'; import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; import diffFileMockData from '../mock_data/diff_file'; -import discussionsMockData from '../mock_data/diff_discussions'; describe('ParallelDiffView', () => { let component; const getDiffFileMock = () => Object.assign({}, diffFileMockData); - const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; beforeEach(() => { const diffFile = getDiffFileMock(); @@ -28,197 +26,4 @@ describe('ParallelDiffView', () => { }); }); }); - - describe('methods', () => { - describe('hasDiscussion', () => { - it('it should return true if there is a discussion either for left or right section', () => { - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { line_42: true }; - }, - }); - - expect(component.hasDiscussion({ left: {}, right: {} })).toEqual(undefined); - expect(component.hasDiscussion({ left: { lineCode: 'line_42' }, right: {} })).toEqual(true); - expect(component.hasDiscussion({ left: {}, right: { lineCode: 'line_42' } })).toEqual(true); - }); - }); - - describe('getClassName', () => { - it('should return line class object', () => { - const { LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME } = constants; - const { MATCH_LINE_TYPE, NEW_LINE_TYPE, LINE_POSITION_RIGHT } = constants; - - expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: false, - }); - - const eventMock = { - target: component.$refs.rightLines[1], - }; - - component.handleMouse(eventMock, component.diffLines[1], true); - Object.defineProperty(component, 'isLoggedIn', { - get() { - return true; - }, - }); - - expect(component.getClassName(component.diffLines[1], LINE_POSITION_RIGHT)).toEqual({ - [NEW_LINE_TYPE]: NEW_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: false, - [LINE_HOVER_CLASS_NAME]: true, - }); - - expect(component.getClassName(component.diffLines[5], LINE_POSITION_RIGHT)).toEqual({ - [MATCH_LINE_TYPE]: MATCH_LINE_TYPE, - [LINE_UNFOLD_CLASS_NAME]: true, - [LINE_HOVER_CLASS_NAME]: false, - }); - }); - }); - - describe('handleMouse', () => { - it('should set hovered line code and line section to null when isHover is false', () => { - const rightLineEventMock = { target: component.$refs.rightLines[1] }; - expect(component.hoveredLineCode).toEqual(null); - expect(component.hoveredSection).toEqual(null); - - component.handleMouse(rightLineEventMock, null, false); - expect(component.hoveredLineCode).toEqual(null); - expect(component.hoveredSection).toEqual(null); - }); - - it('should set hovered line code and line section for right section', () => { - const rightLineEventMock = { target: component.$refs.rightLines[1] }; - component.handleMouse(rightLineEventMock, component.diffLines[1], true); - expect(component.hoveredLineCode).toEqual(component.diffLines[1].right.lineCode); - expect(component.hoveredSection).toEqual(constants.LINE_POSITION_RIGHT); - }); - - it('should set hovered line code and line section for left section', () => { - const leftLineEventMock = { target: component.$refs.leftLines[2] }; - component.handleMouse(leftLineEventMock, component.diffLines[2], true); - expect(component.hoveredLineCode).toEqual(component.diffLines[2].left.lineCode); - expect(component.hoveredSection).toEqual(constants.LINE_POSITION_LEFT); - }); - }); - - describe('shouldRenderDiscussions', () => { - it('should return true if there is a discussion on left side and it is expanded', () => { - const line = { left: { lineCode: 'lineCode1' } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(true); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.left.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual(true); - expect(component.isDiscussionExpanded).toHaveBeenCalledWith(line.left.lineCode); - }); - - it('should return false if there is a discussion on left side but it is collapsed', () => { - const line = { left: { lineCode: 'lineCode1' } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(false); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.left.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_LEFT)).toEqual( - false, - ); - }); - - it('should return false for discussions on the right side if there is no line type', () => { - const CUSTOM_RIGHT_LINE_TYPE = 'CUSTOM_RIGHT_LINE_TYPE'; - const line = { right: { lineCode: 'lineCode1', type: CUSTOM_RIGHT_LINE_TYPE } }; - spyOn(component, 'isDiscussionExpanded').and.returnValue(true); - Object.defineProperty(component, 'discussionsByLineCode', { - get() { - return { - [line.right.lineCode]: true, - }; - }, - }); - - expect(component.shouldRenderDiscussions(line, constants.LINE_POSITION_RIGHT)).toEqual( - CUSTOM_RIGHT_LINE_TYPE, - ); - }); - }); - - describe('hasAnyExpandedDiscussion', () => { - const LINE_CODE_LEFT = 'LINE_CODE_LEFT'; - const LINE_CODE_RIGHT = 'LINE_CODE_RIGHT'; - - it('should return true if there is a discussion either on the left or the right side', () => { - const mockLineOne = { - right: { lineCode: LINE_CODE_RIGHT }, - left: {}, - }; - const mockLineTwo = { - left: { lineCode: LINE_CODE_LEFT }, - right: {}, - }; - - spyOn(component, 'isDiscussionExpanded').and.callFake(lc => lc === LINE_CODE_RIGHT); - expect(component.hasAnyExpandedDiscussion(mockLineOne)).toEqual(true); - expect(component.hasAnyExpandedDiscussion(mockLineTwo)).toEqual(false); - }); - }); - }); - - describe('template', () => { - it('should have rendered diff lines', () => { - const el = component.$el; - - expect(el.querySelectorAll('tr.line_holder.parallel').length).toEqual(6); - expect(el.querySelectorAll('td.empty-cell').length).toEqual(4); - expect(el.querySelectorAll('td.line_content.parallel.right-side').length).toEqual(6); - expect(el.querySelectorAll('td.line_content.parallel.left-side').length).toEqual(6); - expect(el.querySelectorAll('td.match').length).toEqual(4); - expect(el.textContent.indexOf('Bad dates') > -1).toEqual(true); - }); - - it('should render discussions', done => { - const el = component.$el; - component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.notes_holder').length).toEqual(1); - expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(5); - expect(el.innerText.indexOf('comment 5') > -1).toEqual(true); - component.$store.dispatch('setInitialNotes', []); - - done(); - }); - }); - - it('should render new discussion forms', done => { - const el = component.$el; - const lines = getDiffFileMock().parallelDiffLines; - - component.handleShowCommentForm({ lineCode: lines[0].lineCode }); - component.handleShowCommentForm({ lineCode: lines[1].lineCode }); - - Vue.nextTick(() => { - expect(el.querySelectorAll('.js-vue-markdown-field').length).toEqual(2); - expect(el.querySelectorAll('tr')[1].classList.contains('notes_holder')).toEqual(true); - expect(el.querySelectorAll('tr')[3].classList.contains('notes_holder')).toEqual(true); - - store.state.diffs.diffLineCommentForms = {}; - - done(); - }); - }); - }); }); diff --git a/spec/lib/gitaly/server_spec.rb b/spec/lib/gitaly/server_spec.rb index ed5d56e91d4..09bf21b5946 100644 --- a/spec/lib/gitaly/server_spec.rb +++ b/spec/lib/gitaly/server_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitaly::Server do + let(:server) { described_class.new('default') } + describe '.all' do let(:storages) { Gitlab.config.repositories.storages } @@ -17,6 +19,38 @@ describe Gitaly::Server do it { is_expected.to respond_to(:up_to_date?) } it { is_expected.to respond_to(:address) } + describe 'readable?' do + context 'when the storage is readable' do + it 'returns true' do + expect(server).to be_readable + end + end + + context 'when the storage is not readable' do + let(:server) { described_class.new('broken') } + + it 'returns false' do + expect(server).not_to be_readable + end + end + end + + describe 'writeable?' do + context 'when the storage is writeable' do + it 'returns true' do + expect(server).to be_writeable + end + end + + context 'when the storage is not writeable' do + let(:server) { described_class.new('broken') } + + it 'returns false' do + expect(server).not_to be_writeable + end + end + end + describe 'request memoization' do context 'when requesting multiple properties', :request_store do it 'uses memoization for the info request' do diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb deleted file mode 100644 index 9dcf272d25e..00000000000 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ /dev/null @@ -1,200 +0,0 @@ -require 'spec_helper' - -describe Gitlab::HealthChecks::FsShardsCheck do - def command_exists?(command) - _, status = Gitlab::Popen.popen(%W{ #{command} 1 echo }) - status.zero? - rescue Errno::ENOENT - false - end - - def timeout_command - @timeout_command ||= - if command_exists?('timeout') - 'timeout' - elsif command_exists?('gtimeout') - 'gtimeout' - else - '' - end - end - - let(:metric_class) { Gitlab::HealthChecks::Metric } - let(:result_class) { Gitlab::HealthChecks::Result } - let(:repository_storages) { ['default'] } - let(:tmp_dir) { Dir.mktmpdir } - - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => tmp_dir) - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:repository_storages) { repository_storages } - allow(described_class).to receive(:storages_paths) { storages_paths } - stub_const('Gitlab::HealthChecks::FsShardsCheck::TIMEOUT_EXECUTABLE', timeout_command) - end - - after do - FileUtils.remove_entry_secure(tmp_dir) if Dir.exist?(tmp_dir) - end - - shared_examples 'filesystem checks' do - describe '#readiness' do - subject { described_class.readiness } - - context 'storage has a tripped circuitbreaker', :broken_storage do - let(:repository_storages) { ['broken'] } - let(:storages_paths) do - Gitlab.config.repositories.storages - end - - it { is_expected.to include(result_class.new(false, 'circuitbreaker tripped', shard: 'broken')) } - end - - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist') - }.with_indifferent_access - end - - before do - allow(described_class).to receive(:storage_circuitbreaker_test) { true } - end - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) } - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it { is_expected.to include(result_class.new(true, nil, shard: 'default')) } - - it 'cleans up files used for testing' do - expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original - - expect { subject }.not_to change(Dir.entries(tmp_dir), :count) - end - - context 'read test fails' do - before do - allow(described_class).to receive(:storage_read_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: 'default')) } - end - - context 'write test fails' do - before do - allow(described_class).to receive(:storage_write_test).with(any_args).and_return(false) - end - - it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: 'default')) } - end - end - end - - describe '#metrics' do - context 'storage points to not existing folder' do - let(:storages_paths) do - { - default: Gitlab::GitalyClient::StorageSettings.new('path' => 'tmp/this/path/doesnt/exist') - }.with_indifferent_access - end - - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_circuitbreaker_latency_seconds, value: be >= 0)) - end - end - - context 'storage points to directory that has both read and write rights' do - before do - FileUtils.chmod_R(0755, tmp_dir) - end - - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 1)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_circuitbreaker_latency_seconds, value: be >= 0)) - end - - it 'cleans up files used for metrics' do - expect { described_class.metrics }.not_to change(Dir.entries(tmp_dir), :count) - end - end - end - end - - context 'when timeout kills fs checks' do - before do - stub_const('Gitlab::HealthChecks::FsShardsCheck::COMMAND_TIMEOUT', '1') - - allow(described_class).to receive(:exec_with_timeout).and_wrap_original { |m| m.call(%w(sleep 60)) } - FileUtils.chmod_R(0755, tmp_dir) - end - - describe '#readiness' do - subject { described_class.readiness } - - it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) } - end - - describe '#metrics' do - it 'provides metrics' do - metrics = described_class.metrics - - expect(metrics).to all(have_attributes(labels: { shard: 'default' })) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) - expect(metrics).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) - end - end - end - - context 'when popen always finds required binaries' do - before do - allow(described_class).to receive(:exec_with_timeout).and_wrap_original do |method, *args, &block| - begin - method.call(*args, &block) - rescue RuntimeError, Errno::ENOENT - raise 'expected not to happen' - end - end - - stub_const('Gitlab::HealthChecks::FsShardsCheck::COMMAND_TIMEOUT', '10') - end - - it_behaves_like 'filesystem checks' - end - - context 'when popen never finds required binaries' do - before do - allow(Gitlab::Popen).to receive(:popen).and_raise(Errno::ENOENT) - end - - it_behaves_like 'filesystem checks' - end -end diff --git a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb index 724beefff69..4912cd48761 100644 --- a/spec/lib/gitlab/health_checks/gitaly_check_spec.rb +++ b/spec/lib/gitlab/health_checks/gitaly_check_spec.rb @@ -30,13 +30,14 @@ describe Gitlab::HealthChecks::GitalyCheck do describe '#metrics' do subject { described_class.metrics } + let(:server) { double(storage: 'default', read_writeable?: up) } before do - expect(Gitlab::GitalyClient::HealthCheckService).to receive(:new).and_return(gitaly_check) + allow(Gitaly::Server).to receive(:new).and_return(server) end context 'Gitaly server is up' do - let(:gitaly_check) { double(check: { success: true }) } + let(:up) { true } it 'provides metrics' do expect(subject).to all(have_attributes(labels: { shard: 'default' })) @@ -46,7 +47,7 @@ describe Gitlab::HealthChecks::GitalyCheck do end context 'Gitaly server is down' do - let(:gitaly_check) { double(check: { success: false, message: 'Connection refused' }) } + let(:up) { false } it 'provides metrics' do expect(subject).to include(an_object_having_attributes(name: 'gitaly_health_check_success', value: 0)) diff --git a/spec/lib/gitlab/shard_health_cache_spec.rb b/spec/lib/gitlab/shard_health_cache_spec.rb new file mode 100644 index 00000000000..e1a69261939 --- /dev/null +++ b/spec/lib/gitlab/shard_health_cache_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe Gitlab::ShardHealthCache, :clean_gitlab_redis_cache do + let(:shards) { %w(foo bar) } + + before do + described_class.update(shards) + end + + describe '.clear' do + it 'leaves no shards around' do + described_class.clear + + expect(described_class.healthy_shard_count).to eq(0) + end + end + + describe '.update' do + it 'returns the healthy shards' do + expect(described_class.cached_healthy_shards).to match_array(shards) + end + + it 'replaces the existing set' do + new_set = %w(test me more) + described_class.update(new_set) + + expect(described_class.cached_healthy_shards).to match_array(new_set) + end + end + + describe '.healthy_shard_count' do + it 'returns the healthy shard count' do + expect(described_class.healthy_shard_count).to eq(2) + end + + it 'returns 0 if no shards are available' do + described_class.update([]) + + expect(described_class.healthy_shard_count).to eq(0) + end + end + + describe '.healthy_shard?' do + it 'returns true for a healthy shard' do + expect(described_class.healthy_shard?('foo')).to be_truthy + end + + it 'returns false for an unknown shard' do + expect(described_class.healthy_shard?('unknown')).to be_falsey + end + end +end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index d8fdfd6dee1..4bc5d3ee899 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -21,6 +21,89 @@ describe API::Files do "/projects/#{project.id}/repository/files/#{file_path}" end + describe "HEAD /projects/:id/repository/files/:file_path" do + shared_examples_for 'repository files' do + it 'returns file attributes in headers' do + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(200) + expect(response.headers['X-Gitlab-File-Path']).to eq(CGI.unescape(file_path)) + expect(response.headers['X-Gitlab-File-Name']).to eq('popen.rb') + expect(response.headers['X-Gitlab-Last-Commit-Id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + expect(response.headers['X-Gitlab-Content-Sha256']).to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') + end + + it 'returns file by commit sha' do + # This file is deleted on HEAD + file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" + params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9" + + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(200) + expect(response.headers['X-Gitlab-File-Name']).to eq('commit.js.coffee') + expect(response.headers['X-Gitlab-Content-Sha256']).to eq('08785f04375b47f81f46e68cc125d5ef368aa20576ddb53f91f4d83f1d04b929') + end + + context 'when mandatory params are not given' do + it "responds with a 400 status" do + head api(route("any%2Ffile"), current_user) + + expect(response).to have_gitlab_http_status(400) + end + end + + context 'when file_path does not exist' do + it "responds with a 404 status" do + params[:ref] = 'master' + + head api(route('app%2Fmodels%2Fapplication%2Erb'), current_user), params + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when file_path does not exist' do + include_context 'disabled repository' + + it "responds with a 403 status" do + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(403) + end + end + end + + context 'when unauthenticated', 'and project is public' do + it_behaves_like 'repository files' do + let(:project) { create(:project, :public, :repository) } + let(:current_user) { nil } + end + end + + context 'when unauthenticated', 'and project is private' do + it "responds with a 404 status" do + current_user = nil + + head api(route(file_path), current_user), params + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when authenticated', 'as a developer' do + it_behaves_like 'repository files' do + let(:current_user) { user } + end + end + + context 'when authenticated', 'as a guest' do + it_behaves_like '403 response' do + let(:request) { head api(route(file_path), guest), params } + end + end + end + describe "GET /projects/:id/repository/files/:file_path" do shared_examples_for 'repository files' do it 'returns file attributes as json' do @@ -30,6 +113,7 @@ describe API::Files do expect(json_response['file_path']).to eq(CGI.unescape(file_path)) expect(json_response['file_name']).to eq('popen.rb') expect(json_response['last_commit_id']).to eq('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') + expect(json_response['content_sha256']).to eq('c440cd09bae50c4632cc58638ad33c6aa375b6109d811e76a9cc3a613c1e8887') expect(Base64.decode64(json_response['content']).lines.first).to eq("require 'fileutils'\n") end @@ -51,6 +135,7 @@ describe API::Files do expect(response).to have_gitlab_http_status(200) expect(json_response['file_name']).to eq('commit.js.coffee') + expect(json_response['content_sha256']).to eq('08785f04375b47f81f46e68cc125d5ef368aa20576ddb53f91f4d83f1d04b929') expect(Base64.decode64(json_response['content']).lines.first).to eq("class Commit\n") end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index a15d60aafe0..95eff029f98 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1679,7 +1679,7 @@ describe API::Issues do let!(:user_agent_detail) { create(:user_agent_detail, subject: issue) } context 'when unauthenticated' do - it "returns unautorized" do + it "returns unauthorized" do get api("/projects/#{project.id}/issues/#{issue.iid}/user_agent_detail") expect(response).to have_gitlab_http_status(401) @@ -1695,7 +1695,7 @@ describe API::Issues do expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) end - it "returns unautorized for non-admin users" do + it "returns unauthorized for non-admin users" do get api("/projects/#{project.id}/issues/#{issue.iid}/user_agent_detail", user) expect(response).to have_gitlab_http_status(403) diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 4a2289ca137..a3b5e8c6223 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -25,7 +25,7 @@ describe API::ProjectSnippets do expect(response).to have_gitlab_http_status(404) end - it "returns unautorized for non-admin users" do + it "returns unauthorized for non-admin users" do get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/user_agent_detail", user) expect(response).to have_gitlab_http_status(403) diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index c5456977b60..6da769cb3ed 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -314,7 +314,7 @@ describe API::Snippets do expect(json_response['akismet_submitted']).to eq(user_agent_detail.submitted) end - it "returns unautorized for non-admin users" do + it "returns unauthorized for non-admin users" do get api("/snippets/#{snippet.id}/user_agent_detail", user) expect(response).to have_gitlab_http_status(403) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index 790ecce8ded..46e4e3559dc 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -47,5 +47,18 @@ describe MergeRequests::PostMergeService do expect(diff_removal_service).to have_received(:execute) end + + it 'marks MR as merged regardless of errors when closing issues' do + merge_request.update(target_branch: 'foo') + allow(project).to receive(:default_branch).and_return('foo') + + issue = create(:issue, project: project) + allow(merge_request).to receive(:closes_issues).and_return([issue]) + allow_any_instance_of(Issues::CloseService).to receive(:execute).with(issue, commit: merge_request).and_raise + + expect { described_class.new(project, user, {}).execute(merge_request) }.to raise_error + + expect(merge_request.reload).to be_merged + end end end diff --git a/spec/support/shared_examples/features/protected_branches_access_control_ce.rb b/spec/support/shared_examples/features/protected_branches_access_control_ce.rb index 5241c0fa6f1..a8f2c2e7a5a 100644 --- a/spec/support/shared_examples/features/protected_branches_access_control_ce.rb +++ b/spec/support/shared_examples/features/protected_branches_access_control_ce.rb @@ -5,6 +5,12 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + within('.js-new-protected-branch') do allowed_to_push_button = find(".js-allowed-to-push") @@ -25,6 +31,18 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) @@ -59,6 +77,12 @@ shared_examples "protected branches > access control > CE" do end end + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) @@ -70,6 +94,18 @@ shared_examples "protected branches > access control > CE" do set_protected_branch_name('master') + find(".js-allowed-to-merge").click + within('.qa-allowed-to-merge-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + + find(".js-allowed-to-push").click + within('.qa-allowed-to-push-dropdown') do + expect(first("li")).to have_content("Roles") + find(:link, 'No one').click + end + click_on "Protect" expect(ProtectedBranch.count).to eq(1) diff --git a/spec/workers/repository_check/batch_worker_spec.rb b/spec/workers/repository_check/batch_worker_spec.rb index 6cd27d2fafb..6bc551be9ad 100644 --- a/spec/workers/repository_check/batch_worker_spec.rb +++ b/spec/workers/repository_check/batch_worker_spec.rb @@ -1,14 +1,19 @@ require 'spec_helper' describe RepositoryCheck::BatchWorker do + let(:shard_name) { 'default' } subject { described_class.new } + before do + Gitlab::ShardHealthCache.update([shard_name]) + end + it 'prefers projects that have never been checked' do projects = create_list(:project, 3, created_at: 1.week.ago) projects[0].update_column(:last_repository_check_at, 4.months.ago) projects[2].update_column(:last_repository_check_at, 3.months.ago) - expect(subject.perform).to eq(projects.values_at(1, 0, 2).map(&:id)) + expect(subject.perform(shard_name)).to eq(projects.values_at(1, 0, 2).map(&:id)) end it 'sorts projects by last_repository_check_at' do @@ -17,7 +22,7 @@ describe RepositoryCheck::BatchWorker do projects[1].update_column(:last_repository_check_at, 4.months.ago) projects[2].update_column(:last_repository_check_at, 3.months.ago) - expect(subject.perform).to eq(projects.values_at(1, 2, 0).map(&:id)) + expect(subject.perform(shard_name)).to eq(projects.values_at(1, 2, 0).map(&:id)) end it 'excludes projects that were checked recently' do @@ -26,7 +31,14 @@ describe RepositoryCheck::BatchWorker do projects[1].update_column(:last_repository_check_at, 2.months.ago) projects[2].update_column(:last_repository_check_at, 3.days.ago) - expect(subject.perform).to eq([projects[1].id]) + expect(subject.perform(shard_name)).to eq([projects[1].id]) + end + + it 'excludes projects on another shard' do + projects = create_list(:project, 2, created_at: 1.week.ago) + projects[0].update_column(:repository_storage, 'other') + + expect(subject.perform(shard_name)).to eq([projects[1].id]) end it 'does nothing when repository checks are disabled' do @@ -34,13 +46,20 @@ describe RepositoryCheck::BatchWorker do stub_application_setting(repository_checks_enabled: false) - expect(subject.perform).to eq(nil) + expect(subject.perform(shard_name)).to eq(nil) + end + + it 'does nothing when shard is unhealthy' do + shard_name = 'broken' + create(:project, created_at: 1.week.ago, repository_storage: shard_name) + + expect(subject.perform(shard_name)).to eq(nil) end it 'skips projects created less than 24 hours ago' do project = create(:project) project.update_column(:created_at, 23.hours.ago) - expect(subject.perform).to eq([]) + expect(subject.perform(shard_name)).to eq([]) end end diff --git a/spec/workers/repository_check/dispatch_worker_spec.rb b/spec/workers/repository_check/dispatch_worker_spec.rb new file mode 100644 index 00000000000..20a4f1f5344 --- /dev/null +++ b/spec/workers/repository_check/dispatch_worker_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe RepositoryCheck::DispatchWorker do + subject { described_class.new } + + it 'does nothing when repository checks are disabled' do + stub_application_setting(repository_checks_enabled: false) + + expect(RepositoryCheck::BatchWorker).not_to receive(:perform_async) + + subject.perform + end + + it 'dispatches work to RepositoryCheck::BatchWorker' do + expect(RepositoryCheck::BatchWorker).to receive(:perform_async).at_least(:once) + + subject.perform + end + + context 'with unhealthy shard' do + let(:default_shard_name) { 'default' } + let(:unhealthy_shard_name) { 'unhealthy' } + let(:default_shard) { Gitlab::HealthChecks::Result.new(true, nil, shard: default_shard_name) } + let(:unhealthy_shard) { Gitlab::HealthChecks::Result.new(false, '14:Connect Failed', shard: unhealthy_shard_name) } + + before do + allow(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness).and_return([default_shard, unhealthy_shard]) + end + + it 'only triggers RepositoryCheck::BatchWorker for healthy shards' do + expect(RepositoryCheck::BatchWorker).to receive(:perform_async).with('default') + + subject.perform + end + end +end |