diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2017-10-06 20:28:49 +0100 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2017-10-06 20:28:49 +0100 |
commit | 32ad31a16a6af4957193b35010965a6cedc8946c (patch) | |
tree | d9293d3993d12d34d2f3ab460dfaae997c75be06 | |
parent | 60e9ba21e9cec68629d39a554747607f3049fd88 (diff) | |
parent | 2cf5dca8f80cdefeb8932bf80417f52f289668c8 (diff) | |
download | gitlab-ce-fl-38869-linked-tabs.tar.gz |
Merge branch 'master' into fl-38869-linked-tabsfl-38869-linked-tabs
* master: (136 commits)
Ensure we set SUITE_FLAKY_RSPEC_REPORT_PATH to nil in RspecFlaky::Listener spec
Gitaly feature flag metadata
Add delete issue docs
Adjust tooltips to adhere to 8px grid and make them more readable
Fix JS lock issue specs
Improve redirect uri state and fix all remaining tests
Add 1000+ counters (instead of inifnite) to jobs controller
Use short path project_clusters_url
Change Clusters to Cluster in sidebar
Security fix: redirection in google_api/authorizations_controller
UX review
Fix sidebar title Fix fixture
Fix margins in edit form
Use utc for time comparision
Good old dangling comma
Fix wording in the js messages
Resetting of active Line + setting it for the async display functions
Add some empty spaces
Fix failing spec
Refactor discussion lock docs
...
195 files changed, 5187 insertions, 1163 deletions
diff --git a/app/assets/images/auth_buttons/signin_with_google.png b/app/assets/images/auth_buttons/signin_with_google.png Binary files differnew file mode 100644 index 00000000000..b1327b4f7b4 --- /dev/null +++ b/app/assets/images/auth_buttons/signin_with_google.png diff --git a/app/assets/javascripts/clusters.js b/app/assets/javascripts/clusters.js new file mode 100644 index 00000000000..50dbeb06362 --- /dev/null +++ b/app/assets/javascripts/clusters.js @@ -0,0 +1,112 @@ +/* globals Flash */ +import Visibility from 'visibilityjs'; +import axios from 'axios'; +import Poll from './lib/utils/poll'; +import { s__ } from './locale'; +import './flash'; + +/** + * Cluster page has 2 separate parts: + * Toggle button + * + * - Polling status while creating or scheduled + * -- Update status area with the response result + */ + +class ClusterService { + constructor(options = {}) { + this.options = options; + } + fetchData() { + return axios.get(this.options.endpoint); + } +} + +export default class Clusters { + constructor() { + const dataset = document.querySelector('.js-edit-cluster-form').dataset; + + this.state = { + statusPath: dataset.statusPath, + clusterStatus: dataset.clusterStatus, + clusterStatusReason: dataset.clusterStatusReason, + toggleStatus: dataset.toggleStatus, + }; + + this.service = new ClusterService({ endpoint: this.state.statusPath }); + this.toggleButton = document.querySelector('.js-toggle-cluster'); + this.toggleInput = document.querySelector('.js-toggle-input'); + this.errorContainer = document.querySelector('.js-cluster-error'); + this.successContainer = document.querySelector('.js-cluster-success'); + this.creatingContainer = document.querySelector('.js-cluster-creating'); + this.errorReasonContainer = this.errorContainer.querySelector('.js-error-reason'); + + this.toggleButton.addEventListener('click', this.toggle.bind(this)); + + if (this.state.clusterStatus !== 'created') { + this.updateContainer(this.state.clusterStatus, this.state.clusterStatusReason); + } + + if (this.state.statusPath) { + this.initPolling(); + } + } + + toggle() { + this.toggleButton.classList.toggle('checked'); + this.toggleInput.setAttribute('value', this.toggleButton.classList.contains('checked').toString()); + } + + initPolling() { + this.poll = new Poll({ + resource: this.service, + method: 'fetchData', + successCallback: (data) => { + const { status, status_reason } = data.data; + this.updateContainer(status, status_reason); + }, + errorCallback: () => { + Flash(s__('ClusterIntegration|Something went wrong on our end.')); + }, + }); + + if (!Visibility.hidden()) { + this.poll.makeRequest(); + } else { + this.service.fetchData(); + } + + Visibility.change(() => { + if (!Visibility.hidden()) { + this.poll.restart(); + } else { + this.poll.stop(); + } + }); + } + + hideAll() { + this.errorContainer.classList.add('hidden'); + this.successContainer.classList.add('hidden'); + this.creatingContainer.classList.add('hidden'); + } + + updateContainer(status, error) { + this.hideAll(); + switch (status) { + case 'created': + this.successContainer.classList.remove('hidden'); + break; + case 'errored': + this.errorContainer.classList.remove('hidden'); + this.errorReasonContainer.textContent = error; + break; + case 'scheduled': + case 'creating': + this.creatingContainer.classList.remove('hidden'); + break; + default: + this.hideAll(); + } + } +} diff --git a/app/assets/javascripts/diff.js b/app/assets/javascripts/diff.js index ae8338f5fd2..9ddfdb6ae21 100644 --- a/app/assets/javascripts/diff.js +++ b/app/assets/javascripts/diff.js @@ -17,7 +17,8 @@ class Diff { } }); - FilesCommentButton.init($diffFile); + const tab = document.getElementById('diffs'); + if (!tab || (tab && tab.dataset && tab.dataset.isLocked !== '')) FilesCommentButton.init($diffFile); $diffFile.each((index, file) => new gl.ImageFile(file)); diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index bbaa4e4d91e..e4e7cae540e 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -525,6 +525,11 @@ import { ajaxGet, convertPermissionToBoolean } from './lib/utils/common_utils'; case 'admin:impersonation_tokens:index': new gl.DueDateSelectors(); break; + case 'projects:clusters:show': + import(/* webpackChunkName: "clusters" */ './clusters') + .then(cluster => new cluster.default()) // eslint-disable-line new-cap + .catch(() => {}); + break; } switch (path[0]) { case 'sessions': diff --git a/app/assets/javascripts/line_highlighter.js b/app/assets/javascripts/line_highlighter.js index a16d00b5cef..a75d1a4b8d0 100644 --- a/app/assets/javascripts/line_highlighter.js +++ b/app/assets/javascripts/line_highlighter.js @@ -54,12 +54,14 @@ LineHighlighter.prototype.bindEvents = function() { $fileHolder.on('highlight:line', this.highlightHash); }; -LineHighlighter.prototype.highlightHash = function() { - var range; +LineHighlighter.prototype.highlightHash = function(newHash) { + let range; + if (newHash && typeof newHash === 'string') this._hash = newHash; + + this.clearHighlight(); if (this._hash !== '') { range = this.hashToRange(this._hash); - if (range[0]) { this.highlightRange(range); const lineSelector = `#L${range[0]}`; diff --git a/app/assets/javascripts/notes/components/issue_comment_form.vue b/app/assets/javascripts/notes/components/issue_comment_form.vue index 1a7da84a424..ab8516296a8 100644 --- a/app/assets/javascripts/notes/components/issue_comment_form.vue +++ b/app/assets/javascripts/notes/components/issue_comment_form.vue @@ -7,10 +7,12 @@ import TaskList from '../../task_list'; import * as constants from '../constants'; import eventHub from '../event_hub'; - import confidentialIssue from '../../vue_shared/components/issue/confidential_issue_warning.vue'; + import issueWarning from '../../vue_shared/components/issue/issue_warning.vue'; import issueNoteSignedOutWidget from './issue_note_signed_out_widget.vue'; + import issueDiscussionLockedWidget from './issue_discussion_locked_widget.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; + import issuableStateMixin from '../mixins/issuable_state'; export default { name: 'issueCommentForm', @@ -26,8 +28,9 @@ }; }, components: { - confidentialIssue, + issueWarning, issueNoteSignedOutWidget, + issueDiscussionLockedWidget, markdownField, userAvatarLink, }, @@ -55,6 +58,9 @@ isIssueOpen() { return this.issueState === constants.OPENED || this.issueState === constants.REOPENED; }, + canCreateNote() { + return this.getIssueData.current_user.can_create_note; + }, issueActionButtonTitle() { if (this.note.length) { const actionText = this.isIssueOpen ? 'close' : 'reopen'; @@ -90,9 +96,6 @@ endpoint() { return this.getIssueData.create_note_path; }, - isConfidentialIssue() { - return this.getIssueData.confidential; - }, }, methods: { ...mapActions([ @@ -220,6 +223,9 @@ }); }, }, + mixins: [ + issuableStateMixin, + ], mounted() { // jQuery is needed here because it is a custom event being dispatched with jQuery. $(document).on('issuable:change', (e, isClosed) => { @@ -235,6 +241,7 @@ <template> <div> <issue-note-signed-out-widget v-if="!isLoggedIn" /> + <issue-discussion-locked-widget v-else-if="!canCreateNote" /> <ul v-else class="notes notes-form timeline"> @@ -253,15 +260,22 @@ <div class="timeline-content timeline-content-form"> <form ref="commentForm" - class="new-note js-quick-submit common-note-form gfm-form js-main-target-form"> - <confidentialIssue v-if="isConfidentialIssue" /> + class="new-note js-quick-submit common-note-form gfm-form js-main-target-form" + > + <div class="error-alert"></div> + + <issue-warning + v-if="hasWarning(getIssueData)" + :is-locked="isLocked(getIssueData)" + :is-confidential="isConfidential(getIssueData)" + /> + <markdown-field :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" :quick-actions-docs-path="quickActionsDocsPath" :add-spacing-classes="false" - :is-confidential-issue="isConfidentialIssue" ref="markdownField"> <textarea id="note-body" diff --git a/app/assets/javascripts/notes/components/issue_discussion_locked_widget.vue b/app/assets/javascripts/notes/components/issue_discussion_locked_widget.vue new file mode 100644 index 00000000000..e73ec2aaf71 --- /dev/null +++ b/app/assets/javascripts/notes/components/issue_discussion_locked_widget.vue @@ -0,0 +1,19 @@ +<script> + export default { + computed: { + lockIcon() { + return gl.utils.spriteIcon('lock'); + }, + }, + }; + +</script> + +<template> + <div class="disabled-comment text-center"> + <span class="issuable-note-warning"> + <span class="icon" v-html="lockIcon"></span> + <span>This issue is locked. Only <b>project members</b> can comment.</span> + </span> + </div> +</template> diff --git a/app/assets/javascripts/notes/components/issue_note_form.vue b/app/assets/javascripts/notes/components/issue_note_form.vue index 626c0f2ce18..e2539d6b89d 100644 --- a/app/assets/javascripts/notes/components/issue_note_form.vue +++ b/app/assets/javascripts/notes/components/issue_note_form.vue @@ -1,8 +1,9 @@ <script> import { mapGetters } from 'vuex'; import eventHub from '../event_hub'; - import confidentialIssue from '../../vue_shared/components/issue/confidential_issue_warning.vue'; + import issueWarning from '../../vue_shared/components/issue/issue_warning.vue'; import markdownField from '../../vue_shared/components/markdown/field.vue'; + import issuableStateMixin from '../mixins/issuable_state'; export default { name: 'issueNoteForm', @@ -39,12 +40,13 @@ }; }, components: { - confidentialIssue, + issueWarning, markdownField, }, computed: { ...mapGetters([ 'getDiscussionLastNote', + 'getIssueData', 'getIssueDataByProp', 'getNotesDataByProp', 'getUserDataByProp', @@ -67,9 +69,6 @@ isDisabled() { return !this.note.length || this.isSubmitting; }, - isConfidentialIssue() { - return this.getIssueDataByProp('confidential'); - }, }, methods: { handleUpdate() { @@ -95,6 +94,9 @@ this.$emit('cancelFormEdition', shouldConfirm, this.noteBody !== this.note); }, }, + mixins: [ + issuableStateMixin, + ], mounted() { this.$refs.textarea.focus(); }, @@ -125,7 +127,13 @@ <div class="flash-container timeline-content"></div> <form class="edit-note common-note-form js-quick-submit gfm-form"> - <confidentialIssue v-if="isConfidentialIssue" /> + + <issue-warning + v-if="hasWarning(getIssueData)" + :is-locked="isLocked(getIssueData)" + :is-confidential="isConfidential(getIssueData)" + /> + <markdown-field :markdown-preview-path="markdownPreviewPath" :markdown-docs-path="markdownDocsPath" diff --git a/app/assets/javascripts/notes/mixins/issuable_state.js b/app/assets/javascripts/notes/mixins/issuable_state.js new file mode 100644 index 00000000000..97f3ea0d5de --- /dev/null +++ b/app/assets/javascripts/notes/mixins/issuable_state.js @@ -0,0 +1,15 @@ +export default { + methods: { + isConfidential(issue) { + return !!issue.confidential; + }, + + isLocked(issue) { + return !!issue.discussion_locked; + }, + + hasWarning(issue) { + return this.isConfidential(issue) || this.isLocked(issue); + }, + }, +}; diff --git a/app/assets/javascripts/repo/components/repo_editor.vue b/app/assets/javascripts/repo/components/repo_editor.vue index 96d6a75bb61..02d9c775046 100644 --- a/app/assets/javascripts/repo/components/repo_editor.vue +++ b/app/assets/javascripts/repo/components/repo_editor.vue @@ -63,12 +63,7 @@ const RepoEditor = { const lineNumber = e.target.position.lineNumber; if (e.target.element.classList.contains('line-numbers')) { location.hash = `L${lineNumber}`; - Store.activeLine = lineNumber; - - Helper.monacoInstance.setPosition({ - lineNumber: this.activeLine, - column: 1, - }); + Store.setActiveLine(lineNumber); } }, }, @@ -101,6 +96,15 @@ const RepoEditor = { this.setupEditor(); } }, + + activeLine() { + if (Helper.monacoInstance) { + Helper.monacoInstance.setPosition({ + lineNumber: this.activeLine, + column: 1, + }); + } + }, }, computed: { shouldHideEditor() { diff --git a/app/assets/javascripts/repo/components/repo_preview.vue b/app/assets/javascripts/repo/components/repo_preview.vue index 2fe369a4693..a87bef6084a 100644 --- a/app/assets/javascripts/repo/components/repo_preview.vue +++ b/app/assets/javascripts/repo/components/repo_preview.vue @@ -14,6 +14,11 @@ export default { highlightFile() { $(this.$el).find('.file-content').syntaxHighlight(); }, + highlightLine() { + if (Store.activeLine > -1) { + this.lineHighlighter.highlightHash(`#L${Store.activeLine}`); + } + }, }, mounted() { this.highlightFile(); @@ -26,8 +31,12 @@ export default { html() { this.$nextTick(() => { this.highlightFile(); + this.highlightLine(); }); }, + activeLine() { + this.highlightLine(); + }, }, }; </script> diff --git a/app/assets/javascripts/repo/components/repo_sidebar.vue b/app/assets/javascripts/repo/components/repo_sidebar.vue index 685f6ff806f..e0f3c33003a 100644 --- a/app/assets/javascripts/repo/components/repo_sidebar.vue +++ b/app/assets/javascripts/repo/components/repo_sidebar.vue @@ -18,22 +18,40 @@ export default { }, created() { - this.addPopEventListener(); + window.addEventListener('popstate', this.checkHistory); + }, + destroyed() { + window.removeEventListener('popstate', this.checkHistory); }, data: () => Store, methods: { - addPopEventListener() { - window.addEventListener('popstate', () => { - if (location.href.indexOf('#') > -1) return; - this.linkClicked({ + checkHistory() { + let selectedFile = this.files.find(file => location.pathname.indexOf(file.url) > -1); + if (!selectedFile) { + // Maybe it is not in the current tree but in the opened tabs + selectedFile = Helper.getFileFromPath(location.pathname); + } + + let lineNumber = null; + if (location.hash.indexOf('#L') > -1) lineNumber = Number(location.hash.substr(2)); + + if (selectedFile) { + if (selectedFile.url !== this.activeFile.url) { + this.fileClicked(selectedFile, lineNumber); + } else { + Store.setActiveLine(lineNumber); + } + } else { + // Not opened at all lets open new tab + this.fileClicked({ url: location.href, - }); - }); + }, lineNumber); + } }, - fileClicked(clickedFile) { + fileClicked(clickedFile, lineNumber) { let file = clickedFile; if (file.loading) return; file.loading = true; @@ -41,17 +59,20 @@ export default { if (file.type === 'tree' && file.opened) { file = Store.removeChildFilesOfTree(file); file.loading = false; + Store.setActiveLine(lineNumber); } else { const openFile = Helper.getFileFromPath(file.url); if (openFile) { file.loading = false; Store.setActiveFiles(openFile); + Store.setActiveLine(lineNumber); } else { Service.url = file.url; Helper.getContent(file) .then(() => { file.loading = false; Helper.scrollTabsRight(); + Store.setActiveLine(lineNumber); }) .catch(Helper.loadingError); } diff --git a/app/assets/javascripts/repo/helpers/repo_helper.js b/app/assets/javascripts/repo/helpers/repo_helper.js index ac59a2bed23..7483f8bc305 100644 --- a/app/assets/javascripts/repo/helpers/repo_helper.js +++ b/app/assets/javascripts/repo/helpers/repo_helper.js @@ -254,7 +254,9 @@ const RepoHelper = { RepoHelper.key = RepoHelper.genKey(); - history.pushState({ key: RepoHelper.key }, '', url); + if (document.location.pathname !== url) { + history.pushState({ key: RepoHelper.key }, '', url); + } if (title) { document.title = title; diff --git a/app/assets/javascripts/repo/stores/repo_store.js b/app/assets/javascripts/repo/stores/repo_store.js index 9a4fc40bc69..93b39cff27e 100644 --- a/app/assets/javascripts/repo/stores/repo_store.js +++ b/app/assets/javascripts/repo/stores/repo_store.js @@ -26,7 +26,7 @@ const RepoStore = { }, activeFile: Helper.getDefaultActiveFile(), activeFileIndex: 0, - activeLine: 0, + activeLine: -1, activeFileLabel: 'Raw', files: [], isCommitable: false, @@ -85,6 +85,7 @@ const RepoStore = { if (!file.loading) Helper.updateHistoryEntry(file.url, file.pageTitle || file.name); RepoStore.binary = file.binary; + RepoStore.setActiveLine(-1); }, setFileActivity(file, openedFile, i) { @@ -101,6 +102,10 @@ const RepoStore = { RepoStore.activeFileIndex = i; }, + setActiveLine(activeLine) { + if (!isNaN(activeLine)) RepoStore.activeLine = activeLine; + }, + setActiveToRaw() { RepoStore.activeFile.raw = false; // can't get vue to listen to raw for some reason so RepoStore for now. diff --git a/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue b/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue index 8e7abdbffef..f2b1099a678 100644 --- a/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue +++ b/app/assets/javascripts/sidebar/components/confidential/confidential_issue_sidebar.vue @@ -47,9 +47,9 @@ export default { </script> <template> - <div class="block confidentiality"> + <div class="block issuable-sidebar-item confidentiality"> <div class="sidebar-collapsed-icon"> - <i class="fa" :class="faEye" aria-hidden="true" data-hidden="true"></i> + <i class="fa" :class="faEye" aria-hidden="true"></i> </div> <div class="title hide-collapsed"> Confidentiality @@ -62,19 +62,19 @@ export default { Edit </a> </div> - <div class="value confidential-value hide-collapsed"> + <div class="value sidebar-item-value hide-collapsed"> <editForm v-if="edit" :toggle-form="toggleForm" :is-confidential="isConfidential" :update-confidential-attribute="updateConfidentialAttribute" /> - <div v-if="!isConfidential" class="no-value confidential-value"> - <i class="fa fa-eye is-not-confidential"></i> + <div v-if="!isConfidential" class="no-value sidebar-item-value"> + <i class="fa fa-eye sidebar-item-icon"></i> Not confidential </div> - <div v-else class="value confidential-value hide-collapsed"> - <i aria-hidden="true" data-hidden="true" class="fa fa-eye-slash is-confidential"></i> + <div v-else class="value sidebar-item-value hide-collapsed"> + <i aria-hidden="true" class="fa fa-eye-slash sidebar-item-icon is-active"></i> This issue is confidential </div> </div> diff --git a/app/assets/javascripts/sidebar/components/confidential/edit_form.vue b/app/assets/javascripts/sidebar/components/confidential/edit_form.vue index d578b663a54..dd17b5abd46 100644 --- a/app/assets/javascripts/sidebar/components/confidential/edit_form.vue +++ b/app/assets/javascripts/sidebar/components/confidential/edit_form.vue @@ -2,9 +2,6 @@ import editFormButtons from './edit_form_buttons.vue'; export default { - components: { - editFormButtons, - }, props: { isConfidential: { required: true, @@ -19,12 +16,16 @@ export default { type: Function, }, }, + + components: { + editFormButtons, + }, }; </script> <template> <div class="dropdown open"> - <div class="dropdown-menu confidential-warning-message"> + <div class="dropdown-menu sidebar-item-warning-message"> <div> <p v-if="!isConfidential"> You are going to turn on the confidentiality. This means that only team members with diff --git a/app/assets/javascripts/sidebar/components/confidential/edit_form_buttons.vue b/app/assets/javascripts/sidebar/components/confidential/edit_form_buttons.vue index 97af4a3f505..7ed0619ee6b 100644 --- a/app/assets/javascripts/sidebar/components/confidential/edit_form_buttons.vue +++ b/app/assets/javascripts/sidebar/components/confidential/edit_form_buttons.vue @@ -15,7 +15,7 @@ export default { }, }, computed: { - onOrOff() { + toggleButtonText() { return this.isConfidential ? 'Turn Off' : 'Turn On'; }, updateConfidentialBool() { @@ -26,7 +26,7 @@ export default { </script> <template> - <div class="confidential-warning-message-actions"> + <div class="sidebar-item-warning-message-actions"> <button type="button" class="btn btn-default append-right-10" @@ -39,7 +39,7 @@ export default { class="btn btn-close" @click.prevent="updateConfidentialAttribute(updateConfidentialBool)" > - {{ onOrOff }} + {{ toggleButtonText }} </button> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/lock/edit_form.vue b/app/assets/javascripts/sidebar/components/lock/edit_form.vue new file mode 100644 index 00000000000..c7a6edc7c70 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/lock/edit_form.vue @@ -0,0 +1,61 @@ +<script> +import editFormButtons from './edit_form_buttons.vue'; +import issuableMixin from '../../../vue_shared/mixins/issuable'; + +export default { + props: { + isLocked: { + required: true, + type: Boolean, + }, + + toggleForm: { + required: true, + type: Function, + }, + + updateLockedAttribute: { + required: true, + type: Function, + }, + + issuableType: { + required: true, + type: String, + }, + }, + + mixins: [ + issuableMixin, + ], + + components: { + editFormButtons, + }, +}; +</script> + +<template> + <div class="dropdown open"> + <div class="dropdown-menu sidebar-item-warning-message"> + <p class="text" v-if="isLocked"> + Unlock this {{ issuableDisplayName(issuableType) }}? + <strong>Everyone</strong> + will be able to comment. + </p> + + <p class="text" v-else> + Lock this {{ issuableDisplayName(issuableType) }}? + Only + <strong>project members</strong> + will be able to comment. + </p> + + <edit-form-buttons + :is-locked="isLocked" + :toggle-form="toggleForm" + :update-locked-attribute="updateLockedAttribute" + /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/components/lock/edit_form_buttons.vue b/app/assets/javascripts/sidebar/components/lock/edit_form_buttons.vue new file mode 100644 index 00000000000..c3a553a7605 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/lock/edit_form_buttons.vue @@ -0,0 +1,50 @@ +<script> +export default { + props: { + isLocked: { + required: true, + type: Boolean, + }, + + toggleForm: { + required: true, + type: Function, + }, + + updateLockedAttribute: { + required: true, + type: Function, + }, + }, + + computed: { + buttonText() { + return this.isLocked ? this.__('Unlock') : this.__('Lock'); + }, + + toggleLock() { + return !this.isLocked; + }, + }, +}; +</script> + +<template> + <div class="sidebar-item-warning-message-actions"> + <button + type="button" + class="btn btn-default append-right-10" + @click="toggleForm" + > + {{ __('Cancel') }} + </button> + + <button + type="button" + class="btn btn-close" + @click.prevent="updateLockedAttribute(toggleLock)" + > + {{ buttonText }} + </button> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue b/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue new file mode 100644 index 00000000000..c4b2900e020 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/lock/lock_issue_sidebar.vue @@ -0,0 +1,120 @@ +<script> +/* global Flash */ +import editForm from './edit_form.vue'; +import issuableMixin from '../../../vue_shared/mixins/issuable'; + +export default { + props: { + isLocked: { + required: true, + type: Boolean, + }, + + isEditable: { + required: true, + type: Boolean, + }, + + mediator: { + required: true, + type: Object, + validator(mediatorObject) { + return mediatorObject.service && mediatorObject.service.update && mediatorObject.store; + }, + }, + + issuableType: { + required: true, + type: String, + }, + }, + + mixins: [ + issuableMixin, + ], + + components: { + editForm, + }, + + computed: { + lockIconClass() { + return this.isLocked ? 'fa-lock' : 'fa-unlock'; + }, + + isLockDialogOpen() { + return this.mediator.store.isLockDialogOpen; + }, + }, + + methods: { + toggleForm() { + this.mediator.store.isLockDialogOpen = !this.mediator.store.isLockDialogOpen; + }, + + updateLockedAttribute(locked) { + this.mediator.service.update(this.issuableType, { + discussion_locked: locked, + }) + .then(() => location.reload()) + .catch(() => Flash(this.__(`Something went wrong trying to change the locked state of this ${this.issuableDisplayName(this.issuableType)}`))); + }, + }, +}; +</script> + +<template> + <div class="block issuable-sidebar-item lock"> + <div class="sidebar-collapsed-icon"> + <i + class="fa" + :class="lockIconClass" + aria-hidden="true" + ></i> + </div> + + <div class="title hide-collapsed"> + Lock {{issuableDisplayName(issuableType) }} + <button + v-if="isEditable" + class="pull-right lock-edit btn btn-blank" + type="button" + @click.prevent="toggleForm" + > + {{ __('Edit') }} + </button> + </div> + + <div class="value sidebar-item-value hide-collapsed"> + <edit-form + v-if="isLockDialogOpen" + :toggle-form="toggleForm" + :is-locked="isLocked" + :update-locked-attribute="updateLockedAttribute" + :issuable-type="issuableType" + /> + + <div + v-if="isLocked" + class="value sidebar-item-value" + > + <i + aria-hidden="true" + class="fa fa-lock sidebar-item-icon is-active" + ></i> + {{ __('Locked') }} + </div> + + <div + v-else + class="no-value sidebar-item-value hide-collapsed" + > + <i + aria-hidden="true" + class="fa fa-unlock sidebar-item-icon" + ></i> + {{ __('Unlocked') }} + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/sidebar_bundle.js b/app/assets/javascripts/sidebar/sidebar_bundle.js index 3d8972050a9..09b9d75c02d 100644 --- a/app/assets/javascripts/sidebar/sidebar_bundle.js +++ b/app/assets/javascripts/sidebar/sidebar_bundle.js @@ -1,46 +1,76 @@ import Vue from 'vue'; -import sidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; -import sidebarAssignees from './components/assignees/sidebar_assignees'; -import confidential from './components/confidential/confidential_issue_sidebar.vue'; +import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; +import SidebarAssignees from './components/assignees/sidebar_assignees'; +import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; import SidebarMoveIssue from './lib/sidebar_move_issue'; +import LockIssueSidebar from './components/lock/lock_issue_sidebar.vue'; +import Translate from '../vue_shared/translate'; import Mediator from './sidebar_mediator'; +Vue.use(Translate); + +function mountConfidentialComponent(mediator) { + const el = document.getElementById('js-confidential-entry-point'); + + if (!el) return; + + const dataNode = document.getElementById('js-confidential-issue-data'); + const initialData = JSON.parse(dataNode.innerHTML); + + const ConfidentialComp = Vue.extend(ConfidentialIssueSidebar); + + new ConfidentialComp({ + propsData: { + isConfidential: initialData.is_confidential, + isEditable: initialData.is_editable, + service: mediator.service, + }, + }).$mount(el); +} + +function mountLockComponent(mediator) { + const el = document.getElementById('js-lock-entry-point'); + + if (!el) return; + + const dataNode = document.getElementById('js-lock-issue-data'); + const initialData = JSON.parse(dataNode.innerHTML); + + const LockComp = Vue.extend(LockIssueSidebar); + + new LockComp({ + propsData: { + isLocked: initialData.is_locked, + isEditable: initialData.is_editable, + mediator, + issuableType: gl.utils.isInIssuePage() ? 'issue' : 'merge_request', + }, + }).$mount(el); +} + function domContentLoaded() { const sidebarOptions = JSON.parse(document.querySelector('.js-sidebar-options').innerHTML); const mediator = new Mediator(sidebarOptions); mediator.fetch(); - const sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees'); - const confidentialEl = document.querySelector('#js-confidential-entry-point'); + const sidebarAssigneesEl = document.getElementById('js-vue-sidebar-assignees'); // Only create the sidebarAssignees vue app if it is found in the DOM // We currently do not use sidebarAssignees for the MR page if (sidebarAssigneesEl) { - new Vue(sidebarAssignees).$mount(sidebarAssigneesEl); + new Vue(SidebarAssignees).$mount(sidebarAssigneesEl); } - if (confidentialEl) { - const dataNode = document.getElementById('js-confidential-issue-data'); - const initialData = JSON.parse(dataNode.innerHTML); + mountConfidentialComponent(mediator); + mountLockComponent(mediator); - const ConfidentialComp = Vue.extend(confidential); - - new ConfidentialComp({ - propsData: { - isConfidential: initialData.is_confidential, - isEditable: initialData.is_editable, - service: mediator.service, - }, - }).$mount(confidentialEl); - - new SidebarMoveIssue( - mediator, - $('.js-move-issue'), - $('.js-move-issue-confirmation-button'), - ).init(); - } + new SidebarMoveIssue( + mediator, + $('.js-move-issue'), + $('.js-move-issue-confirmation-button'), + ).init(); - new Vue(sidebarTimeTracking).$mount('#issuable-time-tracker'); + new Vue(SidebarTimeTracking).$mount('#issuable-time-tracker'); } document.addEventListener('DOMContentLoaded', domContentLoaded); diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index cc04a2a3fcf..d5d04103f3f 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -15,6 +15,7 @@ export default class SidebarStore { }; this.autocompleteProjects = []; this.moveToProjectId = 0; + this.isLockDialogOpen = false; SidebarStore.singleton = this; } diff --git a/app/assets/javascripts/vue_shared/components/issue/confidential_issue_warning.vue b/app/assets/javascripts/vue_shared/components/issue/confidential_issue_warning.vue deleted file mode 100644 index 397d16331d5..00000000000 --- a/app/assets/javascripts/vue_shared/components/issue/confidential_issue_warning.vue +++ /dev/null @@ -1,16 +0,0 @@ -<script> - export default { - name: 'confidentialIssueWarning', - }; -</script> -<template> - <div class="confidential-issue-warning"> - <i - aria-hidden="true" - class="fa fa-eye-slash"> - </i> - <span> - This is a confidential issue. Your comment will not be visible to the public. - </span> - </div> -</template> diff --git a/app/assets/javascripts/vue_shared/components/issue/issue_warning.vue b/app/assets/javascripts/vue_shared/components/issue/issue_warning.vue new file mode 100644 index 00000000000..16c0a8efcd2 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/issue/issue_warning.vue @@ -0,0 +1,55 @@ +<script> + export default { + props: { + isLocked: { + type: Boolean, + default: false, + required: false, + }, + + isConfidential: { + type: Boolean, + default: false, + required: false, + }, + }, + + computed: { + iconClass() { + return { + 'fa-eye-slash': this.isConfidential, + 'fa-lock': this.isLocked, + }; + }, + + isLockedAndConfidential() { + return this.isConfidential && this.isLocked; + }, + }, + }; +</script> +<template> + <div class="issuable-note-warning"> + <i + aria-hidden="true" + class="fa icon" + :class="iconClass" + v-if="!isLockedAndConfidential" + ></i> + + <span v-if="isLockedAndConfidential"> + {{ __('This issue is confidential and locked.') }} + {{ __('People without permission will never get a notification and won\'t be able to comment.') }} + </span> + + <span v-else-if="isConfidential"> + {{ __('This is a confidential issue.') }} + {{ __('Your comment will not be visible to the public.') }} + </span> + + <span v-else-if="isLocked"> + {{ __('This issue is locked.') }} + {{ __('Only project members can comment.') }} + </span> + </div> +</template> diff --git a/app/assets/javascripts/vue_shared/mixins/issuable.js b/app/assets/javascripts/vue_shared/mixins/issuable.js new file mode 100644 index 00000000000..263361587e0 --- /dev/null +++ b/app/assets/javascripts/vue_shared/mixins/issuable.js @@ -0,0 +1,9 @@ +export default { + methods: { + issuableDisplayName(issuableType) { + const displayName = issuableType.replace(/_/, ' '); + + return this.__ ? this.__(displayName) : displayName; + }, + }, +}; diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index 74b846217bb..e8037c77aab 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -40,6 +40,7 @@ @import "framework/tables"; @import "framework/notes"; @import "framework/timeline"; +@import "framework/tooltips"; @import "framework/typography"; @import "framework/zen"; @import "framework/blank"; diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index d178bc17462..c77160a678b 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -381,7 +381,11 @@ background: transparent; border: 0; + &:hover, + &:active, &:focus { outline: 0; + background: transparent; + box-shadow: none; } } diff --git a/app/assets/stylesheets/framework/gitlab-theme.scss b/app/assets/stylesheets/framework/gitlab-theme.scss index 6b69e8018be..a6bdcf46aa7 100644 --- a/app/assets/stylesheets/framework/gitlab-theme.scss +++ b/app/assets/stylesheets/framework/gitlab-theme.scss @@ -95,7 +95,7 @@ } } - .title { + .navbar .title { > a { &:hover, &:focus { diff --git a/app/assets/stylesheets/framework/tooltips.scss b/app/assets/stylesheets/framework/tooltips.scss new file mode 100644 index 00000000000..93baf73cb78 --- /dev/null +++ b/app/assets/stylesheets/framework/tooltips.scss @@ -0,0 +1,7 @@ +.tooltip-inner { + font-size: $tooltip-font-size; + border-radius: $border-radius-default; + line-height: 16px; + font-weight: $gl-font-weight-normal; + padding: $gl-btn-padding; +} diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 60260355765..5ab40947da9 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -203,6 +203,11 @@ $code_font_size: 12px; $code_line_height: 1.6; /* + * Tooltips + */ +$tooltip-font-size: 12px; + +/* * Padding */ $gl-padding: 16px; @@ -700,3 +705,9 @@ Project Templates Icons $rails: #c00; $node: #353535; $java: #70ad51; + +/* +Issuable warning +*/ +$issuable-warning-size: 24px; +$issuable-warning-icon-margin: 4px; diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss new file mode 100644 index 00000000000..5538e46a6c4 --- /dev/null +++ b/app/assets/stylesheets/pages/clusters.scss @@ -0,0 +1,9 @@ +.edit-cluster-form { + .clipboard-addon { + background-color: $white-light; + } + + .alert-block { + margin-bottom: 20px; + } +} diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 7eb28354e6d..db3b7e89d7b 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -5,27 +5,29 @@ margin-right: auto; } -.is-confidential { +.issuable-warning-icon { color: $orange-600; background-color: $orange-100; border-radius: $border-radius-default; padding: 5px; - margin: 0 3px 0 -4px; + margin: 0 $btn-side-margin 0 0; + width: $issuable-warning-size; + height: $issuable-warning-size; + text-align: center; + + &:first-of-type { + margin-right: $issuable-warning-icon-margin; + } } -.is-not-confidential { +.sidebar-item-icon { border-radius: $border-radius-default; padding: 5px; margin: 0 3px 0 -4px; -} - -.confidentiality { - .is-not-confidential { - margin: auto; - } - .is-confidential { - margin: auto; + &.is-active { + color: $orange-600; + background-color: $orange-50; } } diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 74d9acb5490..420bca9ece5 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -101,7 +101,7 @@ } } -.confidential-issue-warning { +.issuable-note-warning { color: $orange-600; background-color: $orange-100; border-radius: $border-radius-default $border-radius-default 0 0; @@ -110,28 +110,52 @@ padding: 3px 12px; margin: auto; align-items: center; + + .icon { + margin-right: $issuable-warning-icon-margin; + } +} + +.disabled-comment .issuable-note-warning { + border: none; + border-radius: $label-border-radius; + padding-top: $gl-vert-padding; + padding-bottom: $gl-vert-padding; + + .icon svg { + position: relative; + top: 2px; + margin-right: $btn-xs-side-margin; + width: $gl-font-size; + height: $gl-font-size; + fill: $orange-600; + } } -.confidential-value { +.sidebar-item-value { .fa { background-color: inherit; } } -.confidential-warning-message { +.sidebar-item-warning-message { line-height: 1.5; padding: 16px; - .confidential-warning-message-actions { + .text { + color: $text-color; + } + + .sidebar-item-warning-message-actions { display: flex; - button { + .btn { flex-grow: 1; } } } -.confidential-issue-warning + .md-area { +.issuable-note-warning + .md-area { border-top-left-radius: 0; border-top-right-radius: 0; } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 46d31e41ada..925fe4513ee 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -703,6 +703,12 @@ ul.notes { color: $note-disabled-comment-color; padding: 90px 0; + &.discussion-locked { + border: none; + background-color: $white-light; + } + + a { color: $gl-link-color; } diff --git a/app/controllers/google_api/authorizations_controller.rb b/app/controllers/google_api/authorizations_controller.rb new file mode 100644 index 00000000000..5551057ff55 --- /dev/null +++ b/app/controllers/google_api/authorizations_controller.rb @@ -0,0 +1,29 @@ +module GoogleApi + class AuthorizationsController < ApplicationController + def callback + token, expires_at = GoogleApi::CloudPlatform::Client + .new(nil, callback_google_api_auth_url) + .get_token(params[:code]) + + session[GoogleApi::CloudPlatform::Client.session_key_for_token] = token + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = + expires_at.to_s + + state_redirect_uri = redirect_uri_from_session_key(params[:state]) + + if state_redirect_uri + redirect_to state_redirect_uri + else + redirect_to root_path + end + end + + private + + def redirect_uri_from_session_key(state) + key = GoogleApi::CloudPlatform::Client + .session_key_for_redirect_uri(params[:state]) + session[key] if key + end + end +end diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb new file mode 100644 index 00000000000..03019b0becc --- /dev/null +++ b/app/controllers/projects/clusters_controller.rb @@ -0,0 +1,136 @@ +class Projects::ClustersController < Projects::ApplicationController + before_action :cluster, except: [:login, :index, :new, :create] + before_action :authorize_read_cluster! + before_action :authorize_create_cluster!, only: [:new, :create] + before_action :authorize_google_api, only: [:new, :create] + before_action :authorize_update_cluster!, only: [:update] + before_action :authorize_admin_cluster!, only: [:destroy] + + def index + if project.cluster + redirect_to project_cluster_path(project, project.cluster) + else + redirect_to new_project_cluster_path(project) + end + end + + def login + begin + state = generate_session_key_redirect(namespace_project_clusters_url.to_s) + + @authorize_url = GoogleApi::CloudPlatform::Client.new( + nil, callback_google_api_auth_url, + state: state).authorize_url + rescue GoogleApi::Auth::ConfigMissingError + # no-op + end + end + + def new + @cluster = project.build_cluster + end + + def create + @cluster = Ci::CreateClusterService + .new(project, current_user, create_params) + .execute(token_in_session) + + if @cluster.persisted? + redirect_to project_cluster_path(project, @cluster) + else + render :new + end + end + + def status + respond_to do |format| + format.json do + Gitlab::PollingInterval.set_header(response, interval: 10_000) + + render json: ClusterSerializer + .new(project: @project, current_user: @current_user) + .represent_status(@cluster) + end + end + end + + def show + end + + def update + Ci::UpdateClusterService + .new(project, current_user, update_params) + .execute(cluster) + + if cluster.valid? + flash[:notice] = "Cluster was successfully updated." + redirect_to project_cluster_path(project, project.cluster) + else + render :show + end + end + + def destroy + if cluster.destroy + flash[:notice] = "Cluster integration was successfully removed." + redirect_to project_clusters_path(project), status: 302 + else + flash[:notice] = "Cluster integration was not removed." + render :show + end + end + + private + + def cluster + @cluster ||= project.cluster.present(current_user: current_user) + end + + def create_params + params.require(:cluster).permit( + :gcp_project_id, + :gcp_cluster_zone, + :gcp_cluster_name, + :gcp_cluster_size, + :gcp_machine_type, + :project_namespace, + :enabled) + end + + def update_params + params.require(:cluster).permit( + :project_namespace, + :enabled) + end + + def authorize_google_api + unless GoogleApi::CloudPlatform::Client.new(token_in_session, nil) + .validate_token(expires_at_in_session) + redirect_to action: 'login' + end + end + + def token_in_session + @token_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_token] + end + + def expires_at_in_session + @expires_at_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] + end + + def generate_session_key_redirect(uri) + GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key| + session[key] = uri + end + end + + def authorize_update_cluster! + access_denied! unless can?(current_user, :update_cluster, cluster) + end + + def authorize_admin_cluster! + access_denied! unless can?(current_user, :admin_cluster, cluster) + end +end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index ee6e6f80cdd..b7a108a0ebd 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -278,6 +278,7 @@ class Projects::IssuesController < Projects::ApplicationController state_event task_num lock_version + discussion_locked ] + [{ label_ids: [], assignee_ids: [] }] end diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 96abdac91b6..1b985ea9763 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -11,7 +11,7 @@ class Projects::JobsController < Projects::ApplicationController def index @scope = params[:scope] @all_builds = project.builds.relevant - @builds = @all_builds.order('created_at DESC') + @builds = @all_builds.order('ci_builds.id DESC') @builds = case @scope when 'pending' diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 6602b204fcb..eb7d7bf374c 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -34,6 +34,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont :target_project_id, :task_num, :title, + :discussion_locked, label_ids: [] ] diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 41a13f6f577..ef7d047b1ad 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -66,7 +66,16 @@ class Projects::NotesController < Projects::ApplicationController params.merge(last_fetched_at: last_fetched_at) end + def authorize_admin_note! + return access_denied! unless can?(current_user, :admin_note, note) + end + def authorize_resolve_note! return access_denied! unless can?(current_user, :resolve_note, note) end + + def authorize_create_note! + return unless noteable.lockable? + access_denied! unless can?(current_user, :create_note, noteable) + end end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index ce028195e51..c219aa3d6a9 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -130,8 +130,12 @@ module NotesHelper end def can_create_note? + issuable = @issue || @merge_request + if @snippet.is_a?(PersonalSnippet) can?(current_user, :comment_personal_snippet, @snippet) + elsif issuable + can?(current_user, :create_note, issuable) else can?(current_user, :create_note, @project) end diff --git a/app/helpers/numbers_helper.rb b/app/helpers/numbers_helper.rb new file mode 100644 index 00000000000..45bd3606076 --- /dev/null +++ b/app/helpers/numbers_helper.rb @@ -0,0 +1,11 @@ +module NumbersHelper + def limited_counter_with_delimiter(resource, **options) + limit = options.fetch(:limit, 1000).to_i + count = resource.limit(limit + 1).count(:all) + if count > limit + number_with_delimiter(count - 1, options) + '+' + else + number_with_delimiter(count, options) + end + end +end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 4c0cce54527..20e050195ea 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -293,6 +293,7 @@ module ProjectsHelper snippets: :read_project_snippet, settings: :admin_project, builds: :read_build, + clusters: :read_cluster, labels: :read_label, issues: :read_issue, project_members: :read_project_member, diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index d7eaf6ce24d..00fe67d6ffb 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -19,7 +19,9 @@ module SystemNoteHelper 'discussion' => 'comment', 'moved' => 'arrow-right', 'outdated' => 'pencil', - 'duplicate' => 'issue-duplicate' + 'duplicate' => 'issue-duplicate', + 'locked' => 'lock', + 'unlocked' => 'lock-open' }.freeze def system_note_icon_name(note) diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index 1c4ddabcad5..5d75b2aa6a3 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -74,4 +74,8 @@ module Noteable def discussions_can_be_resolved_by?(user) discussions_to_be_resolved.all? { |discussion| discussion.can_resolve?(user) } end + + def lockable? + [MergeRequest, Issue].include?(self.class) + end end diff --git a/app/models/gcp/cluster.rb b/app/models/gcp/cluster.rb new file mode 100644 index 00000000000..18bd6a6dcb4 --- /dev/null +++ b/app/models/gcp/cluster.rb @@ -0,0 +1,113 @@ +module Gcp + class Cluster < ActiveRecord::Base + extend Gitlab::Gcp::Model + include Presentable + + belongs_to :project, inverse_of: :cluster + belongs_to :user + belongs_to :service + + default_value_for :gcp_cluster_zone, 'us-central1-a' + default_value_for :gcp_cluster_size, 3 + default_value_for :gcp_machine_type, 'n1-standard-4' + + attr_encrypted :password, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + attr_encrypted :kubernetes_token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + attr_encrypted :gcp_token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + + validates :gcp_project_id, + length: 1..63, + format: { + with: Gitlab::Regex.kubernetes_namespace_regex, + message: Gitlab::Regex.kubernetes_namespace_regex_message + } + + validates :gcp_cluster_name, + length: 1..63, + format: { + with: Gitlab::Regex.kubernetes_namespace_regex, + message: Gitlab::Regex.kubernetes_namespace_regex_message + } + + validates :gcp_cluster_zone, presence: true + + validates :gcp_cluster_size, + presence: true, + numericality: { + only_integer: true, + greater_than: 0 + } + + validates :project_namespace, + allow_blank: true, + length: 1..63, + format: { + with: Gitlab::Regex.kubernetes_namespace_regex, + message: Gitlab::Regex.kubernetes_namespace_regex_message + } + + # if we do not do status transition we prevent change + validate :restrict_modification, on: :update, unless: :status_changed? + + state_machine :status, initial: :scheduled do + state :scheduled, value: 1 + state :creating, value: 2 + state :created, value: 3 + state :errored, value: 4 + + event :make_creating do + transition any - [:creating] => :creating + end + + event :make_created do + transition any - [:created] => :created + end + + event :make_errored do + transition any - [:errored] => :errored + end + + before_transition any => [:errored, :created] do |cluster| + cluster.gcp_token = nil + cluster.gcp_operation_id = nil + end + + before_transition any => [:errored] do |cluster, transition| + status_reason = transition.args.first + cluster.status_reason = status_reason if status_reason + end + end + + def project_namespace_placeholder + "#{project.path}-#{project.id}" + end + + def on_creation? + scheduled? || creating? + end + + def api_url + 'https://' + endpoint if endpoint + end + + def restrict_modification + if on_creation? + errors.add(:base, "cannot modify during creation") + return false + end + + true + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 5f80195028a..e51e70f01b7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -165,6 +165,7 @@ class Project < ActiveRecord::Base has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true has_one :project_feature, inverse_of: :project has_one :statistics, class_name: 'ProjectStatistics' + has_one :cluster, class_name: 'Gcp::Cluster', inverse_of: :project # Container repositories need to remove data from the container registry, # which is not managed by the DB. Hence we're still using dependent: :destroy diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 0b33e45473b..1f9f8d7286b 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -2,7 +2,7 @@ class SystemNoteMetadata < ActiveRecord::Base ICON_TYPES = %w[ commit description merge confidential visible label assignee cross_reference title time_tracking branch milestone discussion task moved - opened closed merged duplicate + opened closed merged duplicate locked unlocked outdated ].freeze diff --git a/app/policies/gcp/cluster_policy.rb b/app/policies/gcp/cluster_policy.rb new file mode 100644 index 00000000000..e77173ea6e1 --- /dev/null +++ b/app/policies/gcp/cluster_policy.rb @@ -0,0 +1,12 @@ +module Gcp + class ClusterPolicy < BasePolicy + alias_method :cluster, :subject + + delegate { @subject.project } + + rule { can?(:master_access) }.policy do + enable :update_cluster + enable :admin_cluster + end + end +end diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb index daf6fa9e18a..f0aa16d2ecf 100644 --- a/app/policies/issuable_policy.rb +++ b/app/policies/issuable_policy.rb @@ -1,6 +1,10 @@ class IssuablePolicy < BasePolicy delegate { @subject.project } + condition(:locked, scope: :subject, score: 0) { @subject.discussion_locked? } + + condition(:is_project_member) { @user && @subject.project && @subject.project.team.member?(@user) } + desc "User is the assignee or author" condition(:assignee_or_author) do @user && @subject.assignee_or_author?(@user) @@ -12,4 +16,12 @@ class IssuablePolicy < BasePolicy enable :read_merge_request enable :update_merge_request end + + rule { locked & ~is_project_member }.policy do + prevent :create_note + prevent :update_note + prevent :admin_note + prevent :resolve_note + prevent :edit_note + end end diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb index 20cd51cfb99..d4cb5a77e63 100644 --- a/app/policies/note_policy.rb +++ b/app/policies/note_policy.rb @@ -1,5 +1,6 @@ class NotePolicy < BasePolicy delegate { @subject.project } + delegate { @subject.noteable if @subject.noteable.lockable? } condition(:is_author) { @user && @subject.author == @user } condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? } @@ -8,6 +9,7 @@ class NotePolicy < BasePolicy condition(:editable, scope: :subject) { @subject.editable? } rule { ~editable | anonymous }.prevent :edit_note + rule { is_author | admin }.enable :edit_note rule { can?(:master_access) }.enable :edit_note diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index b7b5bd34189..f599eab42f2 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -193,6 +193,8 @@ class ProjectPolicy < BasePolicy enable :admin_pages enable :read_pages enable :update_pages + enable :read_cluster + enable :create_cluster end rule { can?(:public_user_access) }.policy do diff --git a/app/presenters/gcp/cluster_presenter.rb b/app/presenters/gcp/cluster_presenter.rb new file mode 100644 index 00000000000..f7908f92a37 --- /dev/null +++ b/app/presenters/gcp/cluster_presenter.rb @@ -0,0 +1,9 @@ +module Gcp + class ClusterPresenter < Gitlab::View::Presenter::Delegated + presents :cluster + + def gke_cluster_url + "https://console.cloud.google.com/kubernetes/clusters/details/#{gcp_cluster_zone}/#{gcp_cluster_name}" + end + end +end diff --git a/app/serializers/cluster_entity.rb b/app/serializers/cluster_entity.rb new file mode 100644 index 00000000000..08a113c4d8a --- /dev/null +++ b/app/serializers/cluster_entity.rb @@ -0,0 +1,6 @@ +class ClusterEntity < Grape::Entity + include RequestAwareEntity + + expose :status_name, as: :status + expose :status_reason +end diff --git a/app/serializers/cluster_serializer.rb b/app/serializers/cluster_serializer.rb new file mode 100644 index 00000000000..2c87202a105 --- /dev/null +++ b/app/serializers/cluster_serializer.rb @@ -0,0 +1,7 @@ +class ClusterSerializer < BaseSerializer + entity ClusterEntity + + def represent_status(resource) + represent(resource, { only: [:status, :status_reason] }) + end +end diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb index 0d6feb78173..10d3ad0214b 100644 --- a/app/serializers/issue_entity.rb +++ b/app/serializers/issue_entity.rb @@ -3,6 +3,7 @@ class IssueEntity < IssuableEntity expose :branch_name expose :confidential + expose :discussion_locked expose :assignees, using: API::Entities::UserBasic expose :due_date expose :moved_to_id @@ -14,7 +15,7 @@ class IssueEntity < IssuableEntity expose :current_user do expose :can_create_note do |issue| - can?(request.current_user, :create_note, issue.project) + can?(request.current_user, :create_note, issue) end expose :can_update do |issue| diff --git a/app/services/ci/create_cluster_service.rb b/app/services/ci/create_cluster_service.rb new file mode 100644 index 00000000000..f7ee0e468e2 --- /dev/null +++ b/app/services/ci/create_cluster_service.rb @@ -0,0 +1,15 @@ +module Ci + class CreateClusterService < BaseService + def execute(access_token) + params['gcp_machine_type'] ||= GoogleApi::CloudPlatform::Client::DEFAULT_MACHINE_TYPE + + cluster_params = + params.merge(user: current_user, + gcp_token: access_token) + + project.create_cluster(cluster_params).tap do |cluster| + ClusterProvisionWorker.perform_async(cluster.id) if cluster.persisted? + end + end + end +end diff --git a/app/services/ci/fetch_gcp_operation_service.rb b/app/services/ci/fetch_gcp_operation_service.rb new file mode 100644 index 00000000000..0b68e4d6ea9 --- /dev/null +++ b/app/services/ci/fetch_gcp_operation_service.rb @@ -0,0 +1,17 @@ +module Ci + class FetchGcpOperationService + def execute(cluster) + api_client = + GoogleApi::CloudPlatform::Client.new(cluster.gcp_token, nil) + + operation = api_client.projects_zones_operations( + cluster.gcp_project_id, + cluster.gcp_cluster_zone, + cluster.gcp_operation_id) + + yield(operation) if block_given? + rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e + return cluster.make_errored!("Failed to request to CloudPlatform; #{e.message}") + end + end +end diff --git a/app/services/ci/fetch_kubernetes_token_service.rb b/app/services/ci/fetch_kubernetes_token_service.rb new file mode 100644 index 00000000000..44da87cb00c --- /dev/null +++ b/app/services/ci/fetch_kubernetes_token_service.rb @@ -0,0 +1,72 @@ +## +# TODO: +# Almost components in this class were copied from app/models/project_services/kubernetes_service.rb +# We should dry up those classes not to repeat the same code. +# Maybe we should have a special facility (e.g. lib/kubernetes_api) to maintain all Kubernetes API caller. +module Ci + class FetchKubernetesTokenService + attr_reader :api_url, :ca_pem, :username, :password + + def initialize(api_url, ca_pem, username, password) + @api_url = api_url + @ca_pem = ca_pem + @username = username + @password = password + end + + def execute + read_secrets.each do |secret| + name = secret.dig('metadata', 'name') + if /default-token/ =~ name + token_base64 = secret.dig('data', 'token') + return Base64.decode64(token_base64) if token_base64 + end + end + + nil + end + + private + + def read_secrets + kubeclient = build_kubeclient! + + kubeclient.get_secrets.as_json + rescue KubeException => err + raise err unless err.error_code == 404 + [] + end + + def build_kubeclient!(api_path: 'api', api_version: 'v1') + raise "Incomplete settings" unless api_url && username && password + + ::Kubeclient::Client.new( + join_api_url(api_path), + api_version, + auth_options: { username: username, password: password }, + ssl_options: kubeclient_ssl_options, + http_proxy_uri: ENV['http_proxy'] + ) + end + + def join_api_url(api_path) + url = URI.parse(api_url) + prefix = url.path.sub(%r{/+\z}, '') + + url.path = [prefix, api_path].join("/") + + url.to_s + end + + def kubeclient_ssl_options + opts = { verify_ssl: OpenSSL::SSL::VERIFY_PEER } + + if ca_pem.present? + opts[:cert_store] = OpenSSL::X509::Store.new + opts[:cert_store].add_cert(OpenSSL::X509::Certificate.new(ca_pem)) + end + + opts + end + end +end diff --git a/app/services/ci/finalize_cluster_creation_service.rb b/app/services/ci/finalize_cluster_creation_service.rb new file mode 100644 index 00000000000..347875c5697 --- /dev/null +++ b/app/services/ci/finalize_cluster_creation_service.rb @@ -0,0 +1,33 @@ +module Ci + class FinalizeClusterCreationService + def execute(cluster) + api_client = + GoogleApi::CloudPlatform::Client.new(cluster.gcp_token, nil) + + begin + gke_cluster = api_client.projects_zones_clusters_get( + cluster.gcp_project_id, + cluster.gcp_cluster_zone, + cluster.gcp_cluster_name) + rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e + return cluster.make_errored!("Failed to request to CloudPlatform; #{e.message}") + end + + endpoint = gke_cluster.endpoint + api_url = 'https://' + endpoint + ca_cert = Base64.decode64(gke_cluster.master_auth.cluster_ca_certificate) + username = gke_cluster.master_auth.username + password = gke_cluster.master_auth.password + + kubernetes_token = Ci::FetchKubernetesTokenService.new( + api_url, ca_cert, username, password).execute + + unless kubernetes_token + return cluster.make_errored!('Failed to get a default token of kubernetes') + end + + Ci::IntegrateClusterService.new.execute( + cluster, endpoint, ca_cert, kubernetes_token, username, password) + end + end +end diff --git a/app/services/ci/integrate_cluster_service.rb b/app/services/ci/integrate_cluster_service.rb new file mode 100644 index 00000000000..d123ce8d26b --- /dev/null +++ b/app/services/ci/integrate_cluster_service.rb @@ -0,0 +1,26 @@ +module Ci + class IntegrateClusterService + def execute(cluster, endpoint, ca_cert, token, username, password) + Gcp::Cluster.transaction do + cluster.update!( + enabled: true, + endpoint: endpoint, + ca_cert: ca_cert, + kubernetes_token: token, + username: username, + password: password, + service: cluster.project.find_or_initialize_service('kubernetes'), + status_event: :make_created) + + cluster.service.update!( + active: true, + api_url: cluster.api_url, + ca_pem: ca_cert, + namespace: cluster.project_namespace, + token: token) + end + rescue ActiveRecord::RecordInvalid => e + cluster.make_errored!("Failed to integrate cluster into kubernetes_service: #{e.message}") + end + end +end diff --git a/app/services/ci/provision_cluster_service.rb b/app/services/ci/provision_cluster_service.rb new file mode 100644 index 00000000000..52d80b01813 --- /dev/null +++ b/app/services/ci/provision_cluster_service.rb @@ -0,0 +1,36 @@ +module Ci + class ProvisionClusterService + def execute(cluster) + api_client = + GoogleApi::CloudPlatform::Client.new(cluster.gcp_token, nil) + + begin + operation = api_client.projects_zones_clusters_create( + cluster.gcp_project_id, + cluster.gcp_cluster_zone, + cluster.gcp_cluster_name, + cluster.gcp_cluster_size, + machine_type: cluster.gcp_machine_type) + rescue Google::Apis::ServerError, Google::Apis::ClientError, Google::Apis::AuthorizationError => e + return cluster.make_errored!("Failed to request to CloudPlatform; #{e.message}") + end + + unless operation.status == 'RUNNING' || operation.status == 'PENDING' + return cluster.make_errored!("Operation status is unexpected; #{operation.status_message}") + end + + cluster.gcp_operation_id = api_client.parse_operation_id(operation.self_link) + + unless cluster.gcp_operation_id + return cluster.make_errored!('Can not find operation_id from self_link') + end + + if cluster.make_creating + WaitForClusterCreationWorker.perform_in( + WaitForClusterCreationWorker::INITIAL_INTERVAL, cluster.id) + else + return cluster.make_errored!("Failed to update cluster record; #{cluster.errors}") + end + end + end +end diff --git a/app/services/ci/update_cluster_service.rb b/app/services/ci/update_cluster_service.rb new file mode 100644 index 00000000000..70d88fca660 --- /dev/null +++ b/app/services/ci/update_cluster_service.rb @@ -0,0 +1,22 @@ +module Ci + class UpdateClusterService < BaseService + def execute(cluster) + Gcp::Cluster.transaction do + cluster.update!(params) + + if params['enabled'] == 'true' + cluster.service.update!( + active: true, + api_url: cluster.api_url, + ca_pem: cluster.ca_cert, + namespace: cluster.project_namespace, + token: cluster.kubernetes_token) + else + cluster.service.update!(active: false) + end + end + rescue ActiveRecord::RecordInvalid => e + cluster.errors.add(:base, e.message) + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 12604e7eb5d..f83ece7098f 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -43,6 +43,10 @@ class IssuableBaseService < BaseService SystemNoteService.change_time_spent(issuable, issuable.project, current_user) end + def create_discussion_lock_note(issuable) + SystemNoteService.discussion_lock(issuable, current_user) + end + def filter_params(issuable) ability_name = :"admin_#{issuable.to_ability_name}" @@ -57,6 +61,7 @@ class IssuableBaseService < BaseService params.delete(:due_date) params.delete(:canonical_issue_id) params.delete(:project) + params.delete(:discussion_locked) end filter_assignee(issuable) @@ -236,6 +241,7 @@ class IssuableBaseService < BaseService handle_common_system_notes(issuable, old_labels: old_labels) end + change_discussion_lock(issuable) handle_changes( issuable, old_labels: old_labels, @@ -294,6 +300,12 @@ class IssuableBaseService < BaseService end end + def change_discussion_lock(issuable) + if issuable.previous_changes.include?('discussion_locked') + create_discussion_lock_note(issuable) + end + end + def toggle_award(issuable) award = params.delete(:emoji_award) if award diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 1f66a2668f9..7b32e215c7f 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -591,6 +591,13 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) end + def discussion_lock(issuable, author) + action = issuable.discussion_locked? ? 'locked' : 'unlocked' + body = "#{action} this issue" + + create_note(NoteSummary.new(issuable, issuable.project, author, body, action: action)) + end + private def notes_for_mentioner(mentioner, noteable, notes) diff --git a/app/views/admin/jobs/index.html.haml b/app/views/admin/jobs/index.html.haml index 0310498ae54..7066ed12b95 100644 --- a/app/views/admin/jobs/index.html.haml +++ b/app/views/admin/jobs/index.html.haml @@ -3,7 +3,7 @@ %div{ class: container_class } - .top-area + .top-area.scrolling-tabs-container.inner-page-scroll-tabs - build_path_proc = ->(scope) { admin_jobs_path(scope: scope) } = render "shared/builds/tabs", build_path_proc: build_path_proc, all_builds: @all_builds, scope: @scope diff --git a/app/views/admin/projects/index.html.haml b/app/views/admin/projects/index.html.haml index 3f202fbf4fe..4d8754afdd2 100644 --- a/app/views/admin/projects/index.html.haml +++ b/app/views/admin/projects/index.html.haml @@ -4,7 +4,7 @@ %div{ class: container_class } - .top-area + .top-area.scrolling-tabs-container.inner-page-scroll-tabs .prepend-top-default .search-holder = render 'shared/projects/search_form', autofocus: true, icon: true diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 8765b814405..759d6ff68ea 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -146,7 +146,7 @@ = number_with_delimiter(@project.open_merge_requests_count) - if project_nav_tab? :pipelines - = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts]) do + = nav_link(controller: [:pipelines, :builds, :jobs, :pipeline_schedules, :environments, :artifacts, :clusters]) do = link_to project_pipelines_path(@project), class: 'shortcuts-pipelines' do .nav-icon-container = sprite_icon('pipeline') @@ -189,6 +189,12 @@ %span Charts + - if project_nav_tab? :clusters + = nav_link(controller: :clusters) do + = link_to project_clusters_path(@project), title: 'Cluster', class: 'shortcuts-cluster' do + %span + Cluster + - if project_nav_tab? :wiki = nav_link(controller: :wikis) do = link_to get_project_wiki_path(@project), class: 'shortcuts-wiki' do diff --git a/app/views/projects/_md_preview.html.haml b/app/views/projects/_md_preview.html.haml index 71424593f2e..770608eddff 100644 --- a/app/views/projects/_md_preview.html.haml +++ b/app/views/projects/_md_preview.html.haml @@ -1,5 +1,12 @@ - referenced_users = local_assigns.fetch(:referenced_users, nil) +- if defined?(@merge_request) && @merge_request.discussion_locked? + .issuable-note-warning + = icon('lock', class: 'icon') + %span + = _('This merge request is locked.') + = _('Only project members can comment.') + .md-area .md-header %ul.nav-links.clearfix diff --git a/app/views/projects/clusters/_form.html.haml b/app/views/projects/clusters/_form.html.haml new file mode 100644 index 00000000000..371cdb1e403 --- /dev/null +++ b/app/views/projects/clusters/_form.html.haml @@ -0,0 +1,37 @@ +.row + .col-sm-8.col-sm-offset-4 + %p + - link_to_help_page = link_to(s_('ClusterIntegration|help page'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Read our %{link_to_help_page} on cluster integration.').html_safe % { link_to_help_page: link_to_help_page} + + = form_for [@project.namespace.becomes(Namespace), @project, @cluster] do |field| + = form_errors(@cluster) + .form-group + = field.label :gcp_cluster_name, s_('ClusterIntegration|Cluster name') + = field.text_field :gcp_cluster_name, class: 'form-control' + + .form-group + = field.label :gcp_project_id, s_('ClusterIntegration|Google Cloud Platform project ID') + = link_to(s_('ClusterIntegration|See your projects'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') + = field.text_field :gcp_project_id, class: 'form-control' + + .form-group + = field.label :gcp_cluster_zone, s_('ClusterIntegration|Zone') + = link_to(s_('ClusterIntegration|See zones'), 'https://cloud.google.com/compute/docs/regions-zones/regions-zones', target: '_blank', rel: 'noopener noreferrer') + = field.text_field :gcp_cluster_zone, class: 'form-control', placeholder: 'us-central1-a' + + .form-group + = field.label :gcp_cluster_size, s_('ClusterIntegration|Number of nodes') + = field.text_field :gcp_cluster_size, class: 'form-control', placeholder: '3' + + .form-group + = field.label :gcp_machine_type, s_('ClusterIntegration|Machine type') + = link_to(s_('ClusterIntegration|See machine types'), 'https://cloud.google.com/compute/docs/machine-types', target: '_blank', rel: 'noopener noreferrer') + = field.text_field :gcp_machine_type, class: 'form-control', placeholder: 'n1-standard-4' + + .form-group + = field.label :project_namespace, s_('ClusterIntegration|Project namespace (optional, unique)') + = field.text_field :project_namespace, class: 'form-control', placeholder: @cluster.project_namespace_placeholder + + .form-group + = field.submit s_('ClusterIntegration|Create cluster'), class: 'btn btn-save' diff --git a/app/views/projects/clusters/_header.html.haml b/app/views/projects/clusters/_header.html.haml new file mode 100644 index 00000000000..0134d46491c --- /dev/null +++ b/app/views/projects/clusters/_header.html.haml @@ -0,0 +1,14 @@ +%h4.prepend-top-0 + = s_('ClusterIntegration|Create new cluster on Google Container Engine') +%p + = s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:') +%ul + %li + - link_to_container_engine = link_to(s_('ClusterIntegration|access to Google Container Engine'), 'https://console.cloud.google.com', target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Your account must have %{link_to_container_engine}').html_safe % { link_to_container_engine: link_to_container_engine } + %li + - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/container-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements } + %li + - link_to_container_project = link_to(s_('ClusterIntegration|Google Container Engine project'), target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|A %{link_to_container_project} must have been created under this account').html_safe % { link_to_container_project: link_to_container_project } diff --git a/app/views/projects/clusters/_sidebar.html.haml b/app/views/projects/clusters/_sidebar.html.haml new file mode 100644 index 00000000000..761879db32b --- /dev/null +++ b/app/views/projects/clusters/_sidebar.html.haml @@ -0,0 +1,7 @@ +%h4.prepend-top-0 + = s_('ClusterIntegration|Cluster integration') +%p + = s_('ClusterIntegration|With a cluster associated to this project, you can use review apps, deploy your applications, run your pipelines, and much more in an easy way.') +%p + - link = link_to(s_('ClusterIntegration|cluster'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Learn more about %{link_to_documentation}').html_safe % { link_to_documentation: link } diff --git a/app/views/projects/clusters/login.html.haml b/app/views/projects/clusters/login.html.haml new file mode 100644 index 00000000000..ae132672b7e --- /dev/null +++ b/app/views/projects/clusters/login.html.haml @@ -0,0 +1,16 @@ +- breadcrumb_title "Cluster" +- page_title _("Login") + +.row.prepend-top-default + .col-sm-4 + = render 'sidebar' + .col-sm-8 + = render 'header' +.row + .col-sm-8.col-sm-offset-4.signin-with-google + - if @authorize_url + = link_to @authorize_url do + = image_tag('auth_buttons/signin_with_google.png') + - else + - link = link_to(s_('ClusterIntegration|properly configured'), help_page_path("integration/google"), target: '_blank', rel: 'noopener noreferrer') + = s_('Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_to_documentation: link } diff --git a/app/views/projects/clusters/new.html.haml b/app/views/projects/clusters/new.html.haml new file mode 100644 index 00000000000..c538d41ffad --- /dev/null +++ b/app/views/projects/clusters/new.html.haml @@ -0,0 +1,9 @@ +- breadcrumb_title "Cluster" +- page_title _("New Cluster") + +.row.prepend-top-default + .col-sm-4 + = render 'sidebar' + .col-sm-8 + = render 'header' += render 'form' diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml new file mode 100644 index 00000000000..aee6f904a62 --- /dev/null +++ b/app/views/projects/clusters/show.html.haml @@ -0,0 +1,70 @@ +- breadcrumb_title "Cluster" +- page_title _("Cluster") + +- status_path = status_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster.id, format: :json) if can?(current_user, :admin_cluster, @cluster) && @cluster.on_creation? +.row.prepend-top-default.edit-cluster-form.js-edit-cluster-form{ data: { status_path: status_path, + toggle_status: @cluster.enabled? ? 'true': 'false', + cluster_status: @cluster.status_name, + cluster_status_reason: @cluster.status_reason } } + .col-sm-4 + = render 'sidebar' + .col-sm-8 + %label.append-bottom-10{ for: 'enable-cluster-integration' } + = s_('ClusterIntegration|Enable cluster integration') + %p + - if @cluster.enabled? + - if can?(current_user, :update_cluster, @cluster) + = s_('ClusterIntegration|Cluster integration is enabled for this project. Disabling this integration will not affect your cluster, it will only temporarily turn off GitLab\'s connection to it.') + - else + = s_('ClusterIntegration|Cluster integration is enabled for this project.') + - else + = s_('ClusterIntegration|Cluster integration is disabled for this project.') + + = form_for [@project.namespace.becomes(Namespace), @project, @cluster] do |field| + = form_errors(@cluster) + .form-group.append-bottom-20 + %label.append-bottom-10 + = field.hidden_field :enabled, { class: 'js-toggle-input'} + + %button{ type: 'button', + class: "js-toggle-cluster project-feature-toggle #{'checked' unless !@cluster.enabled?} #{'disabled' unless can?(current_user, :update_cluster, @cluster)}", + 'aria-label': s_('ClusterIntegration|Toggle Cluster'), + disabled: !can?(current_user, :update_cluster, @cluster), + data: { 'enabled-text': 'Enabled', 'disabled-text': 'Disabled' } } + + - if can?(current_user, :update_cluster, @cluster) + .form-group + = field.submit s_('ClusterIntegration|Save'), class: 'btn btn-success' + + - if can?(current_user, :admin_cluster, @cluster) + %label.append-bottom-10{ for: 'google-container-engine' } + = s_('ClusterIntegration|Google Container Engine') + %p + - link_gke = link_to(s_('ClusterIntegration|Google Container Engine'), @cluster.gke_cluster_url, target: '_blank', rel: 'noopener noreferrer') + = s_('ClusterIntegration|Manage your cluster by visiting %{link_gke}').html_safe % { link_gke: link_gke } + + .hidden.js-cluster-error.alert.alert-danger.alert-block{ role: 'alert' } + = s_('ClusterIntegration|Something went wrong while creating your cluster on Google Container Engine') + %p.js-error-reason + + .hidden.js-cluster-creating.alert.alert-info.alert-block{ role: 'alert' } + = s_('ClusterIntegration|Cluster is being created on Google Container Engine...') + + .hidden.js-cluster-success.alert.alert-success.alert-block{ role: 'alert' } + = s_('ClusterIntegration|Cluster was successfully created on Google Container Engine') + + .form_group.append-bottom-20 + %label.append-bottom-10{ for: 'cluter-name' } + = s_('ClusterIntegration|Cluster name') + .input-group + %input.form-control.cluster-name{ value: @cluster.gcp_cluster_name, disabled: true } + %span.input-group-addon.clipboard-addon + = clipboard_button(text: @cluster.gcp_cluster_name, title: s_('ClusterIntegration|Copy cluster name')) + + - if can?(current_user, :admin_cluster, @cluster) + .well.form_group + %label.text-danger + = s_('ClusterIntegration|Remove cluster integration') + %p + = s_('ClusterIntegration|Removing cluster integration will remove the cluster configuration you have added to this project. It will not delete your project.') + = link_to(s_('ClusterIntegration|Remove integration'), namespace_project_cluster_path(@project.namespace, @project, @cluster.id), method: :delete, class: 'btn btn-danger', data: { confirm: "Are you sure you want to remove cluster integration from this project? This will not delete your cluster on Google Container Engine"}) diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index fbaf88356bf..b9fec8af4d7 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -27,7 +27,9 @@ .issuable-meta - if @issue.confidential - = icon('eye-slash', class: 'is-confidential') + = icon('eye-slash', class: 'issuable-warning-icon') + - if @issue.discussion_locked? + = icon('lock', class: 'issuable-warning-icon') = issuable_meta(@issue, @project, "Issue") .issuable-actions.js-issuable-actions diff --git a/app/views/projects/jobs/index.html.haml b/app/views/projects/jobs/index.html.haml index 4a238b99b58..9963cc93633 100644 --- a/app/views/projects/jobs/index.html.haml +++ b/app/views/projects/jobs/index.html.haml @@ -8,7 +8,7 @@ .nav-controls - if can?(current_user, :update_build, @project) - - if @all_builds.running_or_pending.any? + - if @all_builds.running_or_pending.limit(1).any? = link_to 'Cancel running', cancel_all_project_jobs_path(@project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post diff --git a/app/views/projects/merge_requests/_mr_title.html.haml b/app/views/projects/merge_requests/_mr_title.html.haml index 9ff85c2ee4c..cb723fe6a18 100644 --- a/app/views/projects/merge_requests/_mr_title.html.haml +++ b/app/views/projects/merge_requests/_mr_title.html.haml @@ -15,6 +15,8 @@ = icon('angle-double-left') .issuable-meta + - if @merge_request.discussion_locked? + = icon('lock', class: 'issuable-warning-icon') = issuable_meta(@merge_request, @project, "Merge request") .issuable-actions.js-issuable-actions diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index d3742f3e4be..d88e3d794d3 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -83,7 +83,7 @@ #pipelines.pipelines.tab-pane - if @pipelines.any? = render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request) - #diffs.diffs.tab-pane + #diffs.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked? } } -# This tab is always loaded via AJAX .mr-loading-status diff --git a/app/views/shared/builds/_tabs.html.haml b/app/views/shared/builds/_tabs.html.haml index 3baa956b910..639f28cc210 100644 --- a/app/views/shared/builds/_tabs.html.haml +++ b/app/views/shared/builds/_tabs.html.haml @@ -3,22 +3,22 @@ = link_to build_path_proc.call(nil) do All %span.badge.js-totalbuilds-count - = number_with_delimiter(all_builds.count(:id)) + = limited_counter_with_delimiter(all_builds) %li{ class: active_when(scope == 'pending') }> = link_to build_path_proc.call('pending') do Pending %span.badge - = number_with_delimiter(all_builds.pending.count(:id)) + = limited_counter_with_delimiter(all_builds.pending) %li{ class: active_when(scope == 'running') }> = link_to build_path_proc.call('running') do Running %span.badge - = number_with_delimiter(all_builds.running.count(:id)) + = limited_counter_with_delimiter(all_builds.running) %li{ class: active_when(scope == 'finished') }> = link_to build_path_proc.call('finished') do Finished %span.badge - = number_with_delimiter(all_builds.finished.count(:id)) + = limited_counter_with_delimiter(all_builds.finished) diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 674f13ddb23..7b7411b1e23 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -119,6 +119,10 @@ %script#js-confidential-issue-data{ type: "application/json" }= { is_confidential: @issue.confidential, is_editable: can_edit_issuable }.to_json.html_safe #js-confidential-entry-point + - if issuable.has_attribute?(:discussion_locked) + %script#js-lock-issue-data{ type: "application/json" }= { is_locked: issuable.discussion_locked?, is_editable: can_edit_issuable }.to_json.html_safe + #js-lock-entry-point + = render "shared/issuable/participants", participants: issuable.participants(current_user) - if current_user - subscribed = issuable.subscribed?(current_user, @project) diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml index e3e86709b8f..c6e18108c7a 100644 --- a/app/views/shared/notes/_notes_with_form.html.haml +++ b/app/views/shared/notes/_notes_with_form.html.haml @@ -1,3 +1,6 @@ +- issuable = @issue || @merge_request +- discussion_locked = issuable&.discussion_locked? + %ul#notes-list.notes.main-notes-list.timeline = render "shared/notes/notes" @@ -21,5 +24,14 @@ or = link_to "sign in", new_session_path(:user, redirect_to_referer: 'yes'), class: 'js-sign-in-link' to comment - +- elsif discussion_locked + .disabled-comment.text-center.prepend-top-default + %span.issuable-note-warning + %span.icon= sprite_icon('lock', size: 14) + %span + This + = issuable.class.to_s.titleize.downcase + is locked. Only + %b project members + can comment. %script.js-notes-data{ type: "application/json" }= initial_notes_data(autocomplete).to_json.html_safe diff --git a/app/workers/cluster_provision_worker.rb b/app/workers/cluster_provision_worker.rb new file mode 100644 index 00000000000..63300b58a25 --- /dev/null +++ b/app/workers/cluster_provision_worker.rb @@ -0,0 +1,10 @@ +class ClusterProvisionWorker + include Sidekiq::Worker + include ClusterQueue + + def perform(cluster_id) + Gcp::Cluster.find_by_id(cluster_id).try do |cluster| + Ci::ProvisionClusterService.new.execute(cluster) + end + end +end diff --git a/app/workers/concerns/cluster_queue.rb b/app/workers/concerns/cluster_queue.rb new file mode 100644 index 00000000000..a5074d13220 --- /dev/null +++ b/app/workers/concerns/cluster_queue.rb @@ -0,0 +1,10 @@ +## +# Concern for setting Sidekiq settings for the various Gcp clusters workers. +# +module ClusterQueue + extend ActiveSupport::Concern + + included do + sidekiq_options queue: :gcp_cluster + end +end diff --git a/app/workers/wait_for_cluster_creation_worker.rb b/app/workers/wait_for_cluster_creation_worker.rb new file mode 100644 index 00000000000..5aa3bbdaa9d --- /dev/null +++ b/app/workers/wait_for_cluster_creation_worker.rb @@ -0,0 +1,27 @@ +class WaitForClusterCreationWorker + include Sidekiq::Worker + include ClusterQueue + + INITIAL_INTERVAL = 2.minutes + EAGER_INTERVAL = 10.seconds + TIMEOUT = 20.minutes + + def perform(cluster_id) + Gcp::Cluster.find_by_id(cluster_id).try do |cluster| + Ci::FetchGcpOperationService.new.execute(cluster) do |operation| + case operation.status + when 'RUNNING' + if TIMEOUT < Time.now.utc - operation.start_time.to_time.utc + return cluster.make_errored!("Cluster creation time exceeds timeout; #{TIMEOUT}") + end + + WaitForClusterCreationWorker.perform_in(EAGER_INTERVAL, cluster.id) + when 'DONE' + Ci::FinalizeClusterCreationService.new.execute(cluster) + else + return cluster.make_errored!("Unexpected operation status; #{operation.status} #{operation.status_message}") + end + end + end + end +end diff --git a/changelogs/unreleased/18608-lock-issues.yml b/changelogs/unreleased/18608-lock-issues.yml new file mode 100644 index 00000000000..7d907f744f6 --- /dev/null +++ b/changelogs/unreleased/18608-lock-issues.yml @@ -0,0 +1,4 @@ +title: Discussion lock for issues and merge requests +merge_request: +author: +type: added diff --git a/changelogs/unreleased/38775-scrollable-tabs-on-admin.yml b/changelogs/unreleased/38775-scrollable-tabs-on-admin.yml new file mode 100644 index 00000000000..65a66714bcb --- /dev/null +++ b/changelogs/unreleased/38775-scrollable-tabs-on-admin.yml @@ -0,0 +1,5 @@ +--- +title: Make tabs on top scrollable on admin dashboard +merge_request: 14685 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/add-1000-plus-counters-for-jobs-page.yml b/changelogs/unreleased/add-1000-plus-counters-for-jobs-page.yml new file mode 100644 index 00000000000..5f5a61406da --- /dev/null +++ b/changelogs/unreleased/add-1000-plus-counters-for-jobs-page.yml @@ -0,0 +1,5 @@ +--- +title: Add 1000+ counters to job page +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/adjusting-tooltips.yml b/changelogs/unreleased/adjusting-tooltips.yml new file mode 100644 index 00000000000..726b75caecd --- /dev/null +++ b/changelogs/unreleased/adjusting-tooltips.yml @@ -0,0 +1,5 @@ +--- +title: Adjust tooltips to adhere to 8px grid and make them more readable +merge_request: +author: +type: changed diff --git a/changelogs/unreleased/content-title-link-hover-bg.yml b/changelogs/unreleased/content-title-link-hover-bg.yml new file mode 100644 index 00000000000..c4c31c2ad06 --- /dev/null +++ b/changelogs/unreleased/content-title-link-hover-bg.yml @@ -0,0 +1,5 @@ +--- +title: Fixed navbar title colors leaking out of the navbar +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/feature-sm-35954-create-kubernetes-cluster-on-gke-from-k8s-service.yml b/changelogs/unreleased/feature-sm-35954-create-kubernetes-cluster-on-gke-from-k8s-service.yml new file mode 100644 index 00000000000..14b35b6daee --- /dev/null +++ b/changelogs/unreleased/feature-sm-35954-create-kubernetes-cluster-on-gke-from-k8s-service.yml @@ -0,0 +1,5 @@ +--- +title: Create Kubernetes cluster on GKE from k8s service +merge_request: 14470 +author: +type: added diff --git a/changelogs/unreleased/gitaly_feature_flag_metadata.yml b/changelogs/unreleased/gitaly_feature_flag_metadata.yml new file mode 100644 index 00000000000..58e42ef9324 --- /dev/null +++ b/changelogs/unreleased/gitaly_feature_flag_metadata.yml @@ -0,0 +1,5 @@ +--- +title: Add client and call site metadata to Gitaly calls for better traceability +merge_request: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14332 +author: +type: added diff --git a/changelogs/unreleased/jobs-sort-by-id.yml b/changelogs/unreleased/jobs-sort-by-id.yml new file mode 100644 index 00000000000..ec2c3a17b74 --- /dev/null +++ b/changelogs/unreleased/jobs-sort-by-id.yml @@ -0,0 +1,5 @@ +--- +title: Sort JobsController by id, not created_at +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/replace_project_merge_requests-feature.yml b/changelogs/unreleased/replace_project_merge_requests-feature.yml new file mode 100644 index 00000000000..082c922a32b --- /dev/null +++ b/changelogs/unreleased/replace_project_merge_requests-feature.yml @@ -0,0 +1,5 @@ +--- +title: Replace the 'project/merge_requests.feature' spinach test with an rspec analog +merge_request: 14621 +author: Vitaliy @blackst0ne Klachkov +type: other diff --git a/config/routes.rb b/config/routes.rb index 5683725c8a2..405bfcc2d8e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -87,6 +87,7 @@ Rails.application.routes.draw do resources :issues, module: :boards, only: [:index, :update] end + draw :google_api draw :import draw :uploads draw :explore diff --git a/config/routes/google_api.rb b/config/routes/google_api.rb new file mode 100644 index 00000000000..3fb236d3d51 --- /dev/null +++ b/config/routes/google_api.rb @@ -0,0 +1,5 @@ +namespace :google_api do + resource :auth, only: [], controller: :authorizations do + match :callback, via: [:get, :post] + end +end diff --git a/config/routes/project.rb b/config/routes/project.rb index 70d7673250c..7f0e056c884 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -183,6 +183,16 @@ constraints(ProjectUrlConstrainer.new) do end end + resources :clusters, except: [:edit] do + collection do + get :login + end + + member do + get :status, format: :json + end + end + resources :environments, except: [:destroy] do member do post :stop diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 8235e3853dc..e2bb766ee47 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -62,5 +62,6 @@ - [update_user_activity, 1] - [propagate_service_template, 1] - [background_migration, 1] + - [gcp_cluster, 1] - [project_migrate_hashed_storage, 1] - [storage_migrator, 1] diff --git a/db/migrate/20170815221154_add_discussion_locked_to_issuable.rb b/db/migrate/20170815221154_add_discussion_locked_to_issuable.rb new file mode 100644 index 00000000000..5bd777c53a0 --- /dev/null +++ b/db/migrate/20170815221154_add_discussion_locked_to_issuable.rb @@ -0,0 +1,13 @@ +class AddDiscussionLockedToIssuable < ActiveRecord::Migration + DOWNTIME = false + + def up + add_column(:merge_requests, :discussion_locked, :boolean) + add_column(:issues, :discussion_locked, :boolean) + end + + def down + remove_column(:merge_requests, :discussion_locked) + remove_column(:issues, :discussion_locked) + end +end diff --git a/db/migrate/20170924094327_create_gcp_clusters.rb b/db/migrate/20170924094327_create_gcp_clusters.rb new file mode 100644 index 00000000000..657dddcbbc4 --- /dev/null +++ b/db/migrate/20170924094327_create_gcp_clusters.rb @@ -0,0 +1,45 @@ +class CreateGcpClusters < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :gcp_clusters do |t| + # Order columns by best align scheme + t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade } + t.references :user, foreign_key: { on_delete: :nullify } + t.references :service, foreign_key: { on_delete: :nullify } + t.integer :status + t.integer :gcp_cluster_size, null: false + + # Timestamps + t.datetime_with_timezone :created_at, null: false + t.datetime_with_timezone :updated_at, null: false + + # Enable/disable + t.boolean :enabled, default: true + + # General + t.text :status_reason + + # k8s integration specific + t.string :project_namespace + + # Cluster details + t.string :endpoint + t.text :ca_cert + t.text :encrypted_kubernetes_token + t.string :encrypted_kubernetes_token_iv + t.string :username + t.text :encrypted_password + t.string :encrypted_password_iv + + # GKE + t.string :gcp_project_id, null: false + t.string :gcp_cluster_zone, null: false + t.string :gcp_cluster_name, null: false + t.string :gcp_machine_type + t.string :gcp_operation_id + t.text :encrypted_gcp_token + t.string :encrypted_gcp_token_iv + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 46b0ac03418..a71ab36b839 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -580,6 +580,35 @@ ActiveRecord::Schema.define(version: 20171005130944) do add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree + create_table "gcp_clusters", force: :cascade do |t| + t.integer "project_id", null: false + t.integer "user_id" + t.integer "service_id" + t.integer "status" + t.integer "gcp_cluster_size", null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + t.boolean "enabled", default: true + t.text "status_reason" + t.string "project_namespace" + t.string "endpoint" + t.text "ca_cert" + t.text "encrypted_kubernetes_token" + t.string "encrypted_kubernetes_token_iv" + t.string "username" + t.text "encrypted_password" + t.string "encrypted_password_iv" + t.string "gcp_project_id", null: false + t.string "gcp_cluster_zone", null: false + t.string "gcp_cluster_name", null: false + t.string "gcp_machine_type" + t.string "gcp_operation_id" + t.text "encrypted_gcp_token" + t.string "encrypted_gcp_token_iv" + end + + add_index "gcp_clusters", ["project_id"], name: "index_gcp_clusters_on_project_id", unique: true, using: :btree + create_table "gpg_key_subkeys", force: :cascade do |t| t.integer "gpg_key_id", null: false t.binary "keyid" @@ -677,6 +706,7 @@ ActiveRecord::Schema.define(version: 20171005130944) do t.integer "cached_markdown_version" t.datetime "last_edited_at" t.integer "last_edited_by_id" + t.boolean "discussion_locked" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -900,6 +930,7 @@ ActiveRecord::Schema.define(version: 20171005130944) do t.integer "head_pipeline_id" t.boolean "ref_fetched" t.string "merge_jid" + t.boolean "discussion_locked" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree @@ -1739,6 +1770,9 @@ ActiveRecord::Schema.define(version: 20171005130944) do add_foreign_key "events", "projects", on_delete: :cascade add_foreign_key "events", "users", column: "author_id", name: "fk_edfd187b6f", on_delete: :cascade add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade + add_foreign_key "gcp_clusters", "projects", on_delete: :cascade + add_foreign_key "gcp_clusters", "services", on_delete: :nullify + add_foreign_key "gcp_clusters", "users", on_delete: :nullify add_foreign_key "gpg_key_subkeys", "gpg_keys", on_delete: :cascade add_foreign_key "gpg_keys", "users", on_delete: :cascade add_foreign_key "gpg_signatures", "gpg_key_subkeys", on_delete: :nullify diff --git a/doc/api/issues.md b/doc/api/issues.md index cd2cfe8e430..ec8ff3cd3f3 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -110,7 +110,8 @@ Example response: "human_time_estimate": null, "human_total_time_spent": null }, - "confidential": false + "confidential": false, + "discussion_locked": false } ] ``` @@ -216,7 +217,8 @@ Example response: "human_time_estimate": null, "human_total_time_spent": null }, - "confidential": false + "confidential": false, + "discussion_locked": false } ] ``` @@ -323,7 +325,8 @@ Example response: "human_time_estimate": null, "human_total_time_spent": null }, - "confidential": false + "confidential": false, + "discussion_locked": false } ] ``` @@ -407,6 +410,7 @@ Example response: "human_total_time_spent": null }, "confidential": false, + "discussion_locked": false, "_links": { "self": "http://example.com/api/v4/projects/1/issues/2", "notes": "http://example.com/api/v4/projects/1/issues/2/notes", @@ -482,6 +486,7 @@ Example response: "human_total_time_spent": null }, "confidential": false, + "discussion_locked": false, "_links": { "self": "http://example.com/api/v4/projects/1/issues/2", "notes": "http://example.com/api/v4/projects/1/issues/2/notes", @@ -515,6 +520,8 @@ PUT /projects/:id/issues/:issue_iid | `state_event` | string | no | The state event of an issue. Set `close` to close the issue and `reopen` to reopen it | | `updated_at` | string | no | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) | | `due_date` | string | no | Date time string in the format YEAR-MONTH-DAY, e.g. `2016-03-11` | +| `discussion_locked` | boolean | no | Flag indicating if the issue's discussion is locked. If the discussion is locked only project members can add or edit comments. | + ```bash curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/4/issues/85?state_event=close @@ -558,6 +565,7 @@ Example response: "human_total_time_spent": null }, "confidential": false, + "discussion_locked": false, "_links": { "self": "http://example.com/api/v4/projects/1/issues/2", "notes": "http://example.com/api/v4/projects/1/issues/2/notes", @@ -657,6 +665,7 @@ Example response: "human_total_time_spent": null }, "confidential": false, + "discussion_locked": false, "_links": { "self": "http://example.com/api/v4/projects/1/issues/2", "notes": "http://example.com/api/v4/projects/1/issues/2/notes", @@ -735,6 +744,7 @@ Example response: "human_total_time_spent": null }, "confidential": false, + "discussion_locked": false, "_links": { "self": "http://example.com/api/v4/projects/1/issues/2", "notes": "http://example.com/api/v4/projects/1/issues/2/notes", @@ -765,6 +775,44 @@ POST /projects/:id/issues/:issue_iid/unsubscribe curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/93/unsubscribe ``` +Example response: + +```json +{ + "id": 93, + "iid": 12, + "project_id": 5, + "title": "Incidunt et rerum ea expedita iure quibusdam.", + "description": "Et cumque architecto sed aut ipsam.", + "state": "opened", + "created_at": "2016-04-05T21:41:45.217Z", + "updated_at": "2016-04-07T13:02:37.905Z", + "labels": [], + "milestone": null, + "assignee": { + "name": "Edwardo Grady", + "username": "keyon", + "id": 21, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/3e6f06a86cf27fa8b56f3f74f7615987?s=80&d=identicon", + "web_url": "https://gitlab.example.com/keyon" + }, + "author": { + "name": "Vivian Hermann", + "username": "orville", + "id": 11, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", + "web_url": "https://gitlab.example.com/orville" + }, + "subscribed": false, + "due_date": null, + "web_url": "http://example.com/example/example/issues/12", + "confidential": false, + "discussion_locked": false +} +``` + ## Create a todo Manually creates a todo for the current user on an issue. If @@ -857,7 +905,8 @@ Example response: "downvotes": 0, "due_date": null, "web_url": "http://example.com/example/example/issues/110", - "confidential": false + "confidential": false, + "discussion_locked": false }, "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/issues/10", "body": "Vel voluptas atque dicta mollitia adipisci qui at.", diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 63a5dd1445c..50a971102fb 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -201,6 +201,7 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -276,6 +277,7 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -387,6 +389,7 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -480,6 +483,7 @@ POST /projects/:id/merge_requests "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -509,6 +513,7 @@ PUT /projects/:id/merge_requests/:merge_request_iid | `labels` | string | no | Labels for MR as a comma-separated list | | `milestone_id` | integer | no | The ID of a milestone | | `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | +| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. | Must include at least one non-required attribute from above. @@ -563,6 +568,7 @@ Must include at least one non-required attribute from above. "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -667,6 +673,7 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -743,6 +750,7 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", + "discussion_locked": false, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1037,7 +1045,8 @@ Example response: "id": 14, "state": "active", "avatar_url": "http://www.gravatar.com/avatar/a7fa515d53450023c83d62986d0658a8?s=80&d=identicon", - "web_url": "https://gitlab.example.com/francisca" + "web_url": "https://gitlab.example.com/francisca", + "discussion_locked": false }, "assignee": { "name": "Dr. Gabrielle Strosin", diff --git a/doc/user/discussions/img/discussion_lock_system_notes.png b/doc/user/discussions/img/discussion_lock_system_notes.png Binary files differnew file mode 100644 index 00000000000..8e8e8e0bc3d --- /dev/null +++ b/doc/user/discussions/img/discussion_lock_system_notes.png diff --git a/doc/user/discussions/img/lock_form_member.png b/doc/user/discussions/img/lock_form_member.png Binary files differnew file mode 100644 index 00000000000..01c6308d24c --- /dev/null +++ b/doc/user/discussions/img/lock_form_member.png diff --git a/doc/user/discussions/img/lock_form_non_member.png b/doc/user/discussions/img/lock_form_non_member.png Binary files differnew file mode 100644 index 00000000000..3bb70b69580 --- /dev/null +++ b/doc/user/discussions/img/lock_form_non_member.png diff --git a/doc/user/discussions/img/turn_off_lock.png b/doc/user/discussions/img/turn_off_lock.png Binary files differnew file mode 100644 index 00000000000..dd05b398a8b --- /dev/null +++ b/doc/user/discussions/img/turn_off_lock.png diff --git a/doc/user/discussions/img/turn_on_lock.png b/doc/user/discussions/img/turn_on_lock.png Binary files differnew file mode 100644 index 00000000000..9597da4e14d --- /dev/null +++ b/doc/user/discussions/img/turn_on_lock.png diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index efea99eb120..ab46befb91c 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -153,12 +153,52 @@ comments in greater detail. ![Discussion comment](img/discussion_comment.png) +## Locking discussions + +> [Introduced][ce-14531] in GitLab 10.1. + +There might be some cases where a discussion is better off if it's locked down. +For example: + +- Discussions that are several years old and the issue/merge request is closed, + but people continue to try to resurrect the discussion. +- Discussions where someone or a group of people are trolling, are abusive, or + in-general are causing the discussion to be unproductive. + +In locked discussions, only team members can write new comments and edit the old +ones. + +To lock or unlock a discussion, you need to have at least Master [permissions]: + +1. Find the "Lock" section in the sidebar and click **Edit** +1. In the dialog that will appear, you can choose to turn on or turn off the + discussion lock +1. Optionally, leave a comment to explain your reasoning behind that action + +| Turn off discussion lock | Turn on discussion lock | +| :-----------: | :----------: | +| ![Turn off discussion lock](img/turn_off_lock.png) | ![Turn on discussion lock](img/turn_on_lock.png) | + +Every change is indicated by a system note in the issue's or merge request's +comments. + +![Discussion lock system notes](img/discussion_lock_system_notes.png) + +Once an issue or merge request is locked, project members can see the indicator +in the comment area, whereas non project members can only see the information +that the discussion is locked. + +| Team member | Not a member | +| :-----------: | :----------: | +| ![Comment form member](img/lock_form_member.png) | ![Comment form non-member](img/lock_form_non_member.png) | + [ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022 [ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125 [ce-7527]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7527 [ce-7180]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7180 [ce-8266]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8266 [ce-14053]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14053 +[ce-14531]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14531 [resolve-discussion-button]: img/resolve_discussion_button.png [resolve-comment-button]: img/resolve_comment_button.png [discussion-view]: img/discussion_view.png diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 44ee994a26b..be72d42625a 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -25,6 +25,7 @@ The following table depicts the various user permission levels in a project. | Create confidential issue | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | | View confidential issues | (✓) [^2] | ✓ | ✓ | ✓ | ✓ | | Leave comments | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | +| Lock comments | | | | ✓ | ✓ | | See a list of jobs | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | See a job log | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | | Download and browse job artifacts | ✓ [^3] | ✓ | ✓ | ✓ | ✓ | @@ -71,6 +72,7 @@ The following table depicts the various user permission levels in a project. | Switch visibility level | | | | | ✓ | | Transfer project to another namespace | | | | | ✓ | | Remove project | | | | | ✓ | +| Delete issues | | | | | ✓ | | Force push to protected branches [^4] | | | | | | | Remove protected branches [^4] | | | | | | | Remove pages | | | | | ✓ | diff --git a/doc/user/project/issues/deleting_issues.md b/doc/user/project/issues/deleting_issues.md new file mode 100644 index 00000000000..d7442104c53 --- /dev/null +++ b/doc/user/project/issues/deleting_issues.md @@ -0,0 +1,11 @@ +# Deleting Issues + +> [Introduced][ce-2982] in GitLab 8.6 + +Please read through the [GitLab Issue Documentation](index.md) for an overview on GitLab Issues. + +You can delete an issue by editing it and clicking on the delete button. + +![delete issue - button](img/delete_issue.png) + +>**Note:** Only [project owners](../../permissions.md) can delete issues.
\ No newline at end of file diff --git a/doc/user/project/issues/img/delete_issue.png b/doc/user/project/issues/img/delete_issue.png Binary files differnew file mode 100644 index 00000000000..a356f52044e --- /dev/null +++ b/doc/user/project/issues/img/delete_issue.png diff --git a/doc/user/project/issues/index.md b/doc/user/project/issues/index.md index 0f187946a4a..3e81dcb78c6 100644 --- a/doc/user/project/issues/index.md +++ b/doc/user/project/issues/index.md @@ -90,6 +90,10 @@ Learn distinct ways to [close issues](closing_issues.md) in GitLab. Read through the [documentation on moving issues](moving_issues.md). +## Deleting issues + +Read through the [documentation on deleting issues](deleting_issues.md) + ## Create a merge request from an issue Learn more about it on the [GitLab Issues Functionalities documentation](issues_functionalities.md#18-new-merge-request). diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature deleted file mode 100644 index 349fa2663a7..00000000000 --- a/features/project/merge_requests.feature +++ /dev/null @@ -1,324 +0,0 @@ -@project_merge_requests -Feature: Project Merge Requests - Background: - Given I sign in as a user - And I own project "Shop" - And project "Shop" have "Bug NS-04" open merge request - And project "Shop" have "Feature NS-03" closed merge request - And I visit project "Shop" merge requests page - - Scenario: I should see open merge requests - Then I should see "Bug NS-04" in merge requests - And I should not see "Feature NS-03" in merge requests - - Scenario: I should see CI status for merge requests - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - Given "Bug NS-05" has CI status - When I visit project "Shop" merge requests page - Then I should see merge request "Bug NS-05" with CI status - - Scenario: I should not see target branch name when it is project's default branch - Then I should see "Bug NS-04" in merge requests - And I should not see "master" branch - - Scenario: I should see target branch when it is different from default - Given project "Shop" have "Bug NS-06" open merge request - When I visit project "Shop" merge requests page - Then I should see "feature_conflict" branch - - @javascript - Scenario: I should not see the numbers of diverged commits if the branch is rebased on the target - Given project "Shop" have "Bug NS-07" open merge request with rebased branch - When I visit merge request page "Bug NS-07" - Then I should not see the diverged commits count - - @javascript - Scenario: I should see the numbers of diverged commits if the branch diverged from the target - Given project "Shop" have "Bug NS-08" open merge request with diverged branch - When I visit merge request page "Bug NS-08" - Then I should see the diverged commits count - - @javascript - Scenario: I should see rejected merge requests - Given I click link "Closed" - Then I should see "Feature NS-03" in merge requests - And I should not see "Bug NS-04" in merge requests - - @javascript - Scenario: I should see all merge requests - Given I click link "All" - Then I should see "Feature NS-03" in merge requests - And I should see "Bug NS-04" in merge requests - - @javascript - Scenario: I visit an open merge request page - Given I click link "Bug NS-04" - Then I should see merge request "Bug NS-04" - - @javascript - Scenario: I visit a merged merge request page - Given project "Shop" have "Feature NS-05" merged merge request - And I click link "Merged" - And I click link "Feature NS-05" - Then I should see merge request "Feature NS-05" - - @javascript - Scenario: I close merge request page - Given I click link "Bug NS-04" - And I click link "Close" - Then I should see closed merge request "Bug NS-04" - - @javascript - Scenario: I reopen merge request page - Given I click link "Bug NS-04" - And I click link "Close" - Then I should see closed merge request "Bug NS-04" - When I click link "Reopen" - Then I should see reopened merge request "Bug NS-04" - - @javascript - Scenario: I submit new unassigned merge request - Given I click link "New Merge Request" - And I submit new merge request "Wiki Feature" - Then I should see merge request "Wiki Feature" - - @javascript - Scenario: I comment on a merge request - Given I visit merge request page "Bug NS-04" - And I leave a comment like "XML attached" - Then I should see comment "XML attached" - - @javascript - Scenario: Visiting Merge Requests after being sorted the list - Given I visit project "Shop" merge requests page - And I sort the list by "Last updated" - And I visit my project's home page - And I visit project "Shop" merge requests page - Then The list should be sorted by "Last updated" - - @javascript - Scenario: Visiting Merge Requests from a different Project after sorting - Given I visit project "Shop" merge requests page - And I sort the list by "Last updated" - And I visit dashboard merge requests page - Then The list should be sorted by "Last updated" - - @javascript - Scenario: Sort merge requests by upvotes - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And project "Shop" have "Bug NS-06" open merge request - And merge request "Bug NS-04" have 2 upvotes and 1 downvote - And merge request "Bug NS-06" have 1 upvote and 2 downvotes - And I sort the list by "Popularity" - Then The list should be sorted by "Popularity" - - @javascript - Scenario: I comment on a merge request diff - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is wrong" on diff - And I switch to the merge request's comments tab - Then I should see a discussion has started on diff - And I should see a badge of "1" next to the discussion link - - @javascript - Scenario: I see a new comment on merge request diff from another user in the discussion tab - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And user "John Doe" leaves a comment like "Line is wrong" on diff - Then I should see a discussion by user "John Doe" has started on diff - And I should see a badge of "1" next to the discussion link - - @javascript - Scenario: I edit a comment on a merge request diff - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is wrong" on diff - And I change the comment "Line is wrong" to "Typo, please fix" on diff - Then I should not see a diff comment saying "Line is wrong" - And I should see a diff comment saying "Typo, please fix" - - @javascript - Scenario: I delete a comment on a merge request diff - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is wrong" on diff - And I should see a badge of "1" next to the discussion link - And I delete the comment "Line is wrong" on diff - And I click on the Discussion tab - Then I should not see any discussion - And I should see a badge of "0" next to the discussion link - - @javascript - Scenario: I comment on a line of a commit in merge request - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the commit in the merge request - And I leave a comment like "Line is wrong" on diff in commit - And I switch to the merge request's comments tab - Then I should see a discussion has started on commit diff - - @javascript - Scenario: I comment on a commit in merge request - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the commit in the merge request - And I leave a comment on the diff page in commit - And I switch to the merge request's comments tab - Then I should see a discussion has started on commit - - @javascript - Scenario: I accept merge request with custom commit message - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And merge request "Bug NS-05" is mergeable - And I visit merge request page "Bug NS-05" - And merge request is mergeable - Then I modify merge commit message - And I accept this merge request - Then I should see merged request - - # Markdown - - @javascript - Scenario: Headers inside the description should have ids generated for them. - When I visit merge request page "Bug NS-04" - Then Header "Description header" should have correct id and link - - @javascript - Scenario: Headers inside comments should not have ids generated for them. - Given I visit merge request page "Bug NS-04" - And I leave a comment with a header containing "Comment with a header" - Then The comment with the header should not have an ID - - # Toggling inline comments - - @javascript - Scenario: I hide comments on a merge request diff with comments in a single file - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is wrong" on line 39 of the third file - And I click link "Hide inline discussion" of the third file - Then I should not see a comment like "Line is wrong here" in the third file - - @javascript - Scenario: I show comments on a merge request diff with comments in a single file - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is wrong" on line 39 of the third file - Then I should see a comment like "Line is wrong" in the third file - - @javascript - Scenario: I hide comments on a merge request diff with comments in multiple files - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is correct" on line 12 of the second file - And I leave a comment like "Line is wrong" on line 39 of the third file - And I click link "Hide inline discussion" of the third file - Then I should not see a comment like "Line is wrong here" in the third file - And I should still see a comment like "Line is correct" in the second file - - @javascript - Scenario: I show comments on a merge request diff with comments in multiple files - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is correct" on line 12 of the second file - And I leave a comment like "Line is wrong" on line 39 of the third file - And I click link "Hide inline discussion" of the third file - And I click link "Show inline discussion" of the third file - Then I should see a comment like "Line is wrong" in the third file - And I should still see a comment like "Line is correct" in the second file - - @javascript - Scenario: I unfold diff - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I unfold diff - Then I should see additional file lines - - @javascript - Scenario: I unfold diff in Side-by-Side view - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I click Side-by-side Diff tab - And I unfold diff - Then I should see additional file lines - - @javascript - Scenario: I show comments on a merge request side-by-side diff with comments in multiple files - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - And I leave a comment like "Line is correct" on line 12 of the second file - And I leave a comment like "Line is wrong" on line 39 of the third file - And I click Side-by-side Diff tab - Then I should see comments on the side-by-side diff page - - @javascript - Scenario: I view diffs on a merge request - Given project "Shop" have "Bug NS-05" open merge request with diffs inside - And I visit merge request page "Bug NS-05" - And I click on the Changes tab - Then I should see the proper Inline and Side-by-side links - - # Description preview - - @javascript - Scenario: I can't preview without text - Given I visit merge request page "Bug NS-04" - And I click link "Edit" for the merge request - And I haven't written any description text - Then The Markdown preview tab should say there is nothing to do - - @javascript - Scenario: I can preview with text - Given I visit merge request page "Bug NS-04" - And I click link "Edit" for the merge request - And I write a description like ":+1: Nice" - Then The Markdown preview tab should display rendered Markdown - - @javascript - Scenario: I preview a merge request description - Given I visit merge request page "Bug NS-04" - And I click link "Edit" for the merge request - And I preview a description text like "Bug fixed :smile:" - Then I should see the Markdown preview - And I should not see the Markdown text field - - @javascript - Scenario: I can edit after preview - Given I visit merge request page "Bug NS-04" - And I click link "Edit" for the merge request - And I preview a description text like "Bug fixed :smile:" - Then I should see the Markdown write tab - - @javascript - Scenario: I can unsubscribe from merge request - Given I visit merge request page "Bug NS-04" - Then I should see that I am subscribed - When I click button "Unsubscribe" - Then I should see that I am unsubscribed - - @javascript - Scenario: I can change the target branch - Given I visit merge request page "Bug NS-04" - And I click link "Edit" for the merge request - When I click the "Target branch" dropdown - And I select a new target branch - Then I should see new target branch changes - - @javascript - Scenario: I can close merge request after commenting - Given I visit merge request page "Bug NS-04" - And I leave a comment like "XML attached" - Then I should see comment "XML attached" - And I click link "Close" - Then I should see closed merge request "Bug NS-04" diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb deleted file mode 100644 index dde918e3d41..00000000000 --- a/features/steps/project/merge_requests.rb +++ /dev/null @@ -1,632 +0,0 @@ -class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps - include SharedAuthentication - include SharedIssuable - include SharedProject - include SharedNote - include SharedPaths - include SharedMarkdown - include SharedDiffNote - include SharedUser - include WaitForRequests - - after do - wait_for_requests if javascript_test? - end - - step 'I click link "New Merge Request"' do - page.within '.nav-controls' do - page.has_link?('New Merge Request') ? click_link("New Merge Request") : click_link('New merge request') - end - end - - step 'I click link "Bug NS-04"' do - click_link "Bug NS-04" - end - - step 'I click link "Feature NS-05"' do - click_link "Feature NS-05" - end - - step 'I click link "All"' do - find('.issues-state-filters [data-state="all"] span', text: 'All').click - # Waits for load - expect(find('.issues-state-filters > .active')).to have_content 'All' - end - - step 'I click link "Merged"' do - find('#state-merged').trigger('click') - end - - step 'I click link "Closed"' do - find('.issues-state-filters [data-state="closed"] span', text: 'Closed').click - end - - step 'I should see merge request "Wiki Feature"' do - page.within '.merge-request' do - expect(page).to have_content "Wiki Feature" - end - wait_for_requests - end - - step 'I should see closed merge request "Bug NS-04"' do - expect(page).to have_content "Bug NS-04" - expect(page).to have_content "Closed by" - wait_for_requests - end - - step 'I should see merge request "Bug NS-04"' do - expect(page).to have_content "Bug NS-04" - wait_for_requests - end - - step 'I should see merge request "Feature NS-05"' do - expect(page).to have_content "Feature NS-05" - wait_for_requests - end - - step 'I should not see "master" branch' do - expect(find('.issuable-info')).not_to have_content "master" - end - - step 'I should see "feature_conflict" branch' do - expect(page).to have_content "feature_conflict" - end - - step 'I should see "Bug NS-04" in merge requests' do - expect(page).to have_content "Bug NS-04" - end - - step 'I should see "Feature NS-03" in merge requests' do - expect(page).to have_content "Feature NS-03" - end - - step 'I should not see "Feature NS-03" in merge requests' do - expect(page).not_to have_content "Feature NS-03" - end - - step 'I should not see "Bug NS-04" in merge requests' do - expect(page).not_to have_content "Bug NS-04" - end - - step 'I should see that I am subscribed' do - expect(find('.issuable-subscribe-button span')).to have_content 'Unsubscribe' - end - - step 'I should see that I am unsubscribed' do - expect(find('.issuable-subscribe-button span')).to have_content 'Subscribe' - end - - step 'I click button "Unsubscribe"' do - click_on "Unsubscribe" - wait_for_requests - end - - step 'I click link "Close"' do - first(:css, '.close-mr-link').click - end - - step 'I submit new merge request "Wiki Feature"' do - find('.js-source-branch').click - find('.dropdown-source-branch .dropdown-content a', text: 'fix').click - - find('.js-target-branch').click - first('.dropdown-target-branch .dropdown-content a', text: 'feature').click - - click_button "Compare branches" - fill_in "merge_request_title", with: "Wiki Feature" - click_button "Submit merge request" - end - - step 'project "Shop" have "Bug NS-04" open merge request' do - create(:merge_request, - title: "Bug NS-04", - source_project: project, - target_project: project, - source_branch: 'fix', - target_branch: 'merge-test', - author: project.users.first, - description: "# Description header" - ) - end - - step 'project "Shop" have "Bug NS-06" open merge request' do - create(:merge_request, - title: "Bug NS-06", - source_project: project, - target_project: project, - source_branch: 'fix', - target_branch: 'feature_conflict', - author: project.users.first, - description: "# Description header" - ) - end - - step 'project "Shop" have "Bug NS-05" open merge request with diffs inside' do - create(:merge_request_with_diffs, - title: "Bug NS-05", - source_project: project, - target_project: project, - author: project.users.first, - source_branch: 'merge-test') - end - - step 'project "Shop" have "Feature NS-05" merged merge request' do - create(:merged_merge_request, - title: "Feature NS-05", - source_project: project, - target_project: project, - author: project.users.first) - end - - step 'project "Shop" have "Bug NS-07" open merge request with rebased branch' do - create(:merge_request, :rebased, - title: "Bug NS-07", - source_project: project, - target_project: project, - author: project.users.first) - end - - step 'project "Shop" have "Bug NS-08" open merge request with diverged branch' do - create(:merge_request, :diverged, - title: "Bug NS-08", - source_project: project, - target_project: project, - author: project.users.first) - end - - step 'project "Shop" have "Feature NS-03" closed merge request' do - create(:closed_merge_request, - title: "Feature NS-03", - source_project: project, - target_project: project, - author: project.users.first) - end - - step 'project "Community" has "Bug CO-01" open merge request with diffs inside' do - project = Project.find_by(name: "Community") - create(:merge_request_with_diffs, - title: "Bug CO-01", - source_project: project, - target_project: project, - author: project.users.first) - end - - step 'merge request "Bug NS-04" have 2 upvotes and 1 downvote' do - merge_request = MergeRequest.find_by(title: 'Bug NS-04') - create_list(:award_emoji, 2, awardable: merge_request) - create(:award_emoji, :downvote, awardable: merge_request) - end - - step 'merge request "Bug NS-06" have 1 upvote and 2 downvotes' do - awardable = MergeRequest.find_by(title: 'Bug NS-06') - create(:award_emoji, awardable: awardable) - create_list(:award_emoji, 2, :downvote, awardable: awardable) - end - - step 'The list should be sorted by "Least popular"' do - page.within '.mr-list' do - page.within 'li.merge-request:nth-child(1)' do - expect(page).to have_content 'Bug NS-06' - expect(page).to have_content '1 2' - end - - page.within 'li.merge-request:nth-child(2)' do - expect(page).to have_content 'Bug NS-04' - expect(page).to have_content '2 1' - end - - page.within 'li.merge-request:nth-child(3)' do - expect(page).to have_content 'Bug NS-05' - expect(page).not_to have_content '0 0' - end - end - end - - step 'The list should be sorted by "Popularity"' do - page.within '.mr-list' do - page.within 'li.merge-request:nth-child(1)' do - expect(page).to have_content 'Bug NS-04' - expect(page).to have_content '2 1' - end - - page.within 'li.merge-request:nth-child(2)' do - expect(page).to have_content 'Bug NS-06' - expect(page).to have_content '1 2' - end - - page.within 'li.merge-request:nth-child(3)' do - expect(page).to have_content 'Bug NS-05' - expect(page).not_to have_content '0 0' - end - end - end - - step 'I click on the Changes tab' do - page.within '.merge-request-tabs' do - click_link 'Changes' - end - - # Waits for load - expect(page).to have_css('.tab-content #diffs.active') - end - - step 'I should see the proper Inline and Side-by-side links' do - expect(page).to have_css('#parallel-diff-btn', count: 1) - expect(page).to have_css('#inline-diff-btn', count: 1) - end - - step 'I switch to the merge request\'s comments tab' do - visit project_merge_request_path(project, merge_request) - end - - step 'I click on the commit in the merge request' do - page.within '.merge-request-tabs' do - click_link 'Commits' - end - - page.within '.commits' do - click_link Commit.truncate_sha(sample_commit.id) - end - end - - step 'I leave a comment on the diff page' do - init_diff_note - leave_comment "One comment to rule them all" - end - - step 'I leave a comment on the diff page in commit' do - click_diff_line(sample_commit.line_code) - leave_comment "One comment to rule them all" - end - - step 'I leave a comment like "Line is wrong" on diff' do - init_diff_note - leave_comment "Line is wrong" - end - - step 'user "John Doe" leaves a comment like "Line is wrong" on diff' do - mr = MergeRequest.find_by(title: "Bug NS-05") - create(:diff_note_on_merge_request, project: project, - noteable: mr, - author: user_exists("John Doe"), - note: 'Line is wrong') - end - - step 'I leave a comment like "Line is wrong" on diff in commit' do - click_diff_line(sample_commit.line_code) - leave_comment "Line is wrong" - end - - step 'I change the comment "Line is wrong" to "Typo, please fix" on diff' do - page.within('.diff-file:nth-of-type(5) .note') do - find('.js-note-edit').click - - page.within('.current-note-edit-form', visible: true) do - fill_in 'note_note', with: 'Typo, please fix' - click_button 'Save comment' - end - - expect(page).not_to have_button 'Save comment', disabled: true, visible: true - end - end - - step 'I should not see a diff comment saying "Line is wrong"' do - page.within('.diff-file:nth-of-type(5) .note') do - expect(page).not_to have_visible_content 'Line is wrong' - end - end - - step 'I should see a diff comment saying "Typo, please fix"' do - page.within('.diff-file:nth-of-type(5) .note') do - expect(page).to have_visible_content 'Typo, please fix' - end - end - - step 'I delete the comment "Line is wrong" on diff' do - page.within('.diff-file:nth-of-type(5) .note') do - find('.more-actions').click - find('.more-actions .dropdown-menu li', match: :first) - - find('.js-note-delete').click - end - end - - step 'I click on the Discussion tab' do - page.within '.merge-request-tabs' do - find('.notes-tab').trigger('click') - end - - # Waits for load - expect(page).to have_css('.tab-content #notes.active') - end - - step 'I should not see any discussion' do - expect(page).not_to have_css('.notes .discussion') - end - - step 'I should see a discussion has started on diff' do - page.within(".notes .discussion") do - page.should have_content "#{current_user.name} #{current_user.to_reference} started a discussion" - page.should have_content sample_commit.line_code_path - page.should have_content "Line is wrong" - end - end - - step 'I should see a discussion by user "John Doe" has started on diff' do - # Trigger a refresh of notes - execute_script("$(document).trigger('visibilitychange');") - wait_for_requests - page.within(".notes .discussion") do - page.should have_content "#{user_exists("John Doe").name} #{user_exists("John Doe").to_reference} started a discussion" - page.should have_content sample_commit.line_code_path - page.should have_content "Line is wrong" - end - end - - step 'I should see a badge of "1" next to the discussion link' do - expect_discussion_badge_to_have_counter("1") - wait_for_requests - end - - step 'I should see a badge of "0" next to the discussion link' do - expect_discussion_badge_to_have_counter("0") - wait_for_requests - end - - step 'I should see a discussion has started on commit diff' do - page.within(".notes .discussion") do - page.should have_content "#{current_user.name} #{current_user.to_reference} started a discussion on commit" - page.should have_content sample_commit.line_code_path - page.should have_content "Line is wrong" - wait_for_requests - end - end - - step 'I should see a discussion has started on commit' do - page.within(".notes .discussion") do - page.should have_content "#{current_user.name} #{current_user.to_reference} started a discussion on commit" - page.should have_content "One comment to rule them all" - wait_for_requests - end - end - - step 'merge request is mergeable' do - expect(page).to have_button 'Merge' - end - - step 'I modify merge commit message' do - click_button "Modify commit message" - fill_in 'Commit message', with: 'wow such merge' - end - - step 'merge request "Bug NS-05" is mergeable' do - merge_request.mark_as_mergeable - end - - step 'I accept this merge request' do - page.within '.mr-state-widget' do - click_button "Merge" - end - end - - step 'I should see merged request' do - page.within '.status-box' do - expect(page).to have_content "Merged" - wait_for_requests - end - end - - step 'I click link "Reopen"' do - first(:css, '.reopen-mr-link').trigger('click') - end - - step 'I should see reopened merge request "Bug NS-04"' do - page.within '.status-box' do - expect(page).to have_content "Open" - end - wait_for_requests - end - - step 'I click link "Hide inline discussion" of the third file' do - page.within '.files>div:nth-child(3)' do - find('.js-toggle-diff-comments').trigger('click') - end - end - - step 'I click link "Show inline discussion" of the third file' do - page.within '.files>div:nth-child(3)' do - find('.js-toggle-diff-comments').trigger('click') - end - end - - step 'I should not see a comment like "Line is wrong" in the third file' do - page.within '.files>div:nth-child(3)' do - expect(page).not_to have_visible_content "Line is wrong" - end - end - - step 'I should see a comment like "Line is wrong" in the third file' do - page.within '.files>div:nth-child(3) .note-body > .note-text' do - expect(page).to have_visible_content "Line is wrong" - wait_for_requests - end - end - - step 'I should not see a comment like "Line is wrong here" in the third file' do - page.within '.files>div:nth-child(3)' do - expect(page).not_to have_visible_content "Line is wrong here" - end - end - - step 'I should see a comment like "Line is wrong here" in the third file' do - page.within '.files>div:nth-child(3) .note-body > .note-text' do - expect(page).to have_visible_content "Line is wrong here" - end - end - - step 'I leave a comment like "Line is correct" on line 12 of the second file' do - init_diff_note_first_file - - page.within(".js-discussion-note-form") do - fill_in "note_note", with: "Line is correct" - click_button "Comment" - end - - wait_for_requests - - page.within ".files>div:nth-child(2) .note-body > .note-text" do - expect(page).to have_content "Line is correct" - end - end - - step 'I leave a comment like "Line is wrong" on line 39 of the third file' do - init_diff_note_second_file - - page.within(".js-discussion-note-form") do - fill_in "note_note", with: "Line is wrong on here" - click_button "Comment" - end - - wait_for_requests - end - - step 'I should still see a comment like "Line is correct" in the second file' do - page.within '.files>div:nth-child(2) .note-body > .note-text' do - expect(page).to have_visible_content "Line is correct" - end - end - - step 'I unfold diff' do - expect(page).to have_css('.js-unfold') - - first('.js-unfold').click - end - - step 'I should see additional file lines' do - expect(first('.text-file')).to have_content('.bundle') - end - - step 'I click Side-by-side Diff tab' do - find('a', text: 'Side-by-side').trigger('click') - - # Waits for load - expect(page).to have_css('.parallel') - end - - step 'I should see comments on the side-by-side diff page' do - page.within '.files>div:nth-child(2) .parallel .note-body > .note-text' do - expect(page).to have_visible_content "Line is correct" - wait_for_requests - end - end - - step 'I fill in merge request search with "Fe"' do - fill_in 'issuable_search', with: "Fe" - page.within '.merge-requests-holder' do - find('.merge-request') - end - end - - step 'I click the "Target branch" dropdown' do - expect(page).to have_content('Target branch') - first('.target_branch').click - end - - step 'I select a new target branch' do - select "feature", from: "merge_request_target_branch" - click_button 'Save' - end - - step 'I should see new target branch changes' do - expect(page).to have_content 'Request to merge fix into feature' - expect(page).to have_content 'changed target branch from merge-test to feature' - wait_for_requests - end - - step 'I click on "Email Patches"' do - click_link "Email Patches" - end - - step 'I click on "Plain Diff"' do - click_link "Plain Diff" - end - - step 'I should see a patch diff' do - expect(page).to have_content('diff --git') - end - - step '"Bug NS-05" has CI status' do - project = merge_request.source_project - project.enable_ci - - pipeline = - create(:ci_pipeline, - project: project, - sha: merge_request.diff_head_sha, - ref: merge_request.source_branch, - head_pipeline_of: merge_request) - - create :ci_build, pipeline: pipeline - end - - step 'I should see merge request "Bug NS-05" with CI status' do - page.within ".mr-list" do - expect(page).to have_link "Pipeline: pending" - end - end - - step 'I should see the diverged commits count' do - page.within ".mr-source-target" do - expect(page).to have_content /([0-9]+ commits behind)/ - end - - wait_for_requests - end - - step 'I should not see the diverged commits count' do - page.within ".mr-source-target" do - expect(page).not_to have_content /([0-9]+ commit[s]? behind)/ - end - - wait_for_requests - end - - def merge_request - @merge_request ||= MergeRequest.find_by!(title: "Bug NS-05") - end - - def init_diff_note - click_diff_line(sample_commit.line_code) - end - - def leave_comment(message) - page.within(".js-discussion-note-form", visible: true) do - fill_in "note_note", with: message - click_button "Comment" - end - - wait_for_requests - - page.within(".notes_holder", visible: true) do - expect(page).to have_content message - end - end - - def init_diff_note_first_file - click_diff_line(sample_compare.changes[0][:line_code]) - end - - def init_diff_note_second_file - click_diff_line(sample_compare.changes[1][:line_code]) - end - - def have_visible_content(text) - have_css("*", text: text, visible: true) - end - - def expect_discussion_badge_to_have_counter(value) - page.within(".notes-tab .badge") do - page.should have_content value - end - end -end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 86dec3ca9f1..5f0bad14839 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -368,6 +368,7 @@ module API end expose :due_date expose :confidential + expose :discussion_locked expose :web_url do |issue, options| Gitlab::UrlBuilder.build(issue) @@ -464,6 +465,7 @@ module API expose :diff_head_sha, as: :sha expose :merge_commit_sha expose :user_notes_count + expose :discussion_locked expose :should_remove_source_branch?, as: :should_remove_source_branch expose :force_remove_source_branch?, as: :force_remove_source_branch diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 1729df2aad0..0df41dcc903 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -48,6 +48,7 @@ module API optional :labels, type: String, desc: 'Comma-separated list of label names' optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential' + optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked" end params :issue_params do @@ -193,7 +194,7 @@ module API desc: 'Date time when the issue was updated. Available only for admins and project owners.' optional :state_event, type: String, values: %w[reopen close], desc: 'State of the issue' use :issue_params - at_least_one_of :title, :description, :assignee_ids, :assignee_id, :milestone_id, + at_least_one_of :title, :description, :assignee_ids, :assignee_id, :milestone_id, :discussion_locked, :labels, :created_at, :due_date, :confidential, :state_event end put ':id/issues/:issue_iid' do diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 828bc48383e..be843ec8251 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -214,12 +214,14 @@ module API :remove_source_branch, :state_event, :target_branch, - :title + :title, + :discussion_locked ] optional :title, type: String, allow_blank: false, desc: 'The title of the merge request' optional :target_branch, type: String, allow_blank: false, desc: 'The target branch' optional :state_event, type: String, values: %w[close reopen], desc: 'Status of the merge request' + optional :discussion_locked, type: Boolean, desc: 'Whether the MR discussion is locked' use :optional_params at_least_one_of(*at_least_one_of_ce) diff --git a/lib/api/notes.rb b/lib/api/notes.rb index d6e7203adaf..0b9ab4eeb05 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -78,6 +78,8 @@ module API } if can?(current_user, noteable_read_ability_name(noteable), noteable) + authorize! :create_note, noteable + if params[:created_at] && (current_user.admin? || user_project.owner == current_user) opts[:created_at] = params[:created_at] end diff --git a/lib/gitlab/gcp/model.rb b/lib/gitlab/gcp/model.rb new file mode 100644 index 00000000000..195391f0e3c --- /dev/null +++ b/lib/gitlab/gcp/model.rb @@ -0,0 +1,13 @@ +module Gitlab + module Gcp + module Model + def table_name_prefix + "gcp_" + end + + def model_name + @model_name ||= ActiveModel::Name.new(self, nil, self.name.split("::").last) + end + end + end +end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 87b300dcf7e..cf36106e23d 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -28,6 +28,7 @@ module Gitlab SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze MAXIMUM_GITALY_CALLS = 30 + CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze MUTEX = Mutex.new private_constant :MUTEX @@ -79,7 +80,16 @@ module Gitlab def self.request_metadata(storage) encoded_token = Base64.strict_encode64(token(storage).to_s) - { metadata: { 'authorization' => "Bearer #{encoded_token}" } } + metadata = { + 'authorization' => "Bearer #{encoded_token}", + 'client_name' => CLIENT_NAME + } + + feature_stack = Thread.current[:gitaly_feature_stack] + feature = feature_stack && feature_stack[0] + metadata['call_site'] = feature.to_s if feature + + { metadata: metadata } end def self.token(storage) @@ -137,7 +147,14 @@ module Gitlab Gitlab::Metrics.measure(metric_name) do # Some migrate calls wrap other migrate calls allow_n_plus_1_calls do - yield is_enabled + feature_stack = Thread.current[:gitaly_feature_stack] ||= [] + feature_stack.unshift(feature) + begin + yield is_enabled + ensure + feature_stack.shift + Thread.current[:gitaly_feature_stack] = nil if feature_stack.empty? + end end end end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 2171c6c7bbb..dec8b4c5acd 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -53,6 +53,7 @@ project_tree: - :auto_devops - :triggers - :pipeline_schedules + - :cluster - :services - :hooks - protected_branches: diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 380b336395d..a76cf1addc0 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -8,6 +8,8 @@ module Gitlab triggers: 'Ci::Trigger', pipeline_schedules: 'Ci::PipelineSchedule', builds: 'Ci::Build', + cluster: 'Gcp::Cluster', + clusters: 'Gcp::Cluster', hooks: 'ProjectHook', merge_access_levels: 'ProtectedBranch::MergeAccessLevel', push_access_levels: 'ProtectedBranch::PushAccessLevel', diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 7c02c9c5c48..e68160c8faf 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -33,6 +33,7 @@ module Gitlab explore favicon.ico files + google_api groups health_check help diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 6857038dba8..3f3ba77d47f 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -48,6 +48,7 @@ module Gitlab deploy_keys: DeployKey.count, deployments: Deployment.count, environments: ::Environment.count, + gcp_clusters: ::Gcp::Cluster.count, in_review_folder: ::Environment.in_review_folder.count, groups: Group.count, issues: Issue.count, diff --git a/lib/google_api/auth.rb b/lib/google_api/auth.rb new file mode 100644 index 00000000000..99a82c849e0 --- /dev/null +++ b/lib/google_api/auth.rb @@ -0,0 +1,54 @@ +module GoogleApi + class Auth + attr_reader :access_token, :redirect_uri, :state + + ConfigMissingError = Class.new(StandardError) + + def initialize(access_token, redirect_uri, state: nil) + @access_token = access_token + @redirect_uri = redirect_uri + @state = state + end + + def authorize_url + client.auth_code.authorize_url( + redirect_uri: redirect_uri, + scope: scope, + state: state # This is used for arbitary redirection + ) + end + + def get_token(code) + ret = client.auth_code.get_token(code, redirect_uri: redirect_uri) + return ret.token, ret.expires_at + end + + protected + + def scope + raise NotImplementedError + end + + private + + def config + Gitlab.config.omniauth.providers.find { |provider| provider.name == "google_oauth2" } + end + + def client + return @client if defined?(@client) + + unless config + raise ConfigMissingError + end + + @client = ::OAuth2::Client.new( + config.app_id, + config.app_secret, + site: 'https://accounts.google.com', + token_url: '/o/oauth2/token', + authorize_url: '/o/oauth2/auth' + ) + end + end +end diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb new file mode 100644 index 00000000000..a440a3e3562 --- /dev/null +++ b/lib/google_api/cloud_platform/client.rb @@ -0,0 +1,88 @@ +require 'google/apis/container_v1' + +module GoogleApi + module CloudPlatform + class Client < GoogleApi::Auth + DEFAULT_MACHINE_TYPE = 'n1-standard-1'.freeze + SCOPE = 'https://www.googleapis.com/auth/cloud-platform'.freeze + LEAST_TOKEN_LIFE_TIME = 10.minutes + + class << self + def session_key_for_token + :cloud_platform_access_token + end + + def session_key_for_expires_at + :cloud_platform_expires_at + end + + def new_session_key_for_redirect_uri + SecureRandom.hex.tap do |state| + yield session_key_for_redirect_uri(state) + end + end + + def session_key_for_redirect_uri(state) + "cloud_platform_second_redirect_uri_#{state}" + end + end + + def scope + SCOPE + end + + def validate_token(expires_at) + return false unless access_token + return false unless expires_at + + # Making sure that the token will have been still alive during the cluster creation. + return false if token_life_time(expires_at) < LEAST_TOKEN_LIFE_TIME + + true + end + + def projects_zones_clusters_get(project_id, zone, cluster_id) + service = Google::Apis::ContainerV1::ContainerService.new + service.authorization = access_token + + service.get_zone_cluster(project_id, zone, cluster_id) + end + + def projects_zones_clusters_create(project_id, zone, cluster_name, cluster_size, machine_type:) + service = Google::Apis::ContainerV1::ContainerService.new + service.authorization = access_token + + request_body = Google::Apis::ContainerV1::CreateClusterRequest.new( + { + "cluster": { + "name": cluster_name, + "initial_node_count": cluster_size, + "node_config": { + "machine_type": machine_type + } + } + } ) + + service.create_cluster(project_id, zone, request_body) + end + + def projects_zones_operations(project_id, zone, operation_id) + service = Google::Apis::ContainerV1::ContainerService.new + service.authorization = access_token + + service.get_zone_operation(project_id, zone, operation_id) + end + + def parse_operation_id(self_link) + m = self_link.match(%r{projects/.*/zones/.*/operations/(.*)}) + m[1] if m + end + + private + + def token_life_time(expires_at) + DateTime.strptime(expires_at, '%s').to_time.utc - Time.now.utc + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 99f773e6bca..7569af9d175 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2017-10-03 16:06-0400\n" -"PO-Revision-Date: 2017-10-03 16:06-0400\n" +"POT-Creation-Date: 2017-10-04 23:47+0100\n" +"PO-Revision-Date: 2017-10-04 23:47+0100\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" @@ -367,6 +367,129 @@ msgstr "" msgid "Clone repository" msgstr "" +msgid "ClusterIntegration|A %{link_to_container_project} must have been created under this account" +msgstr "" + +msgid "ClusterIntegration|Cluster integration" +msgstr "" + +msgid "ClusterIntegration|Cluster integration is disabled for this project." +msgstr "" + +msgid "ClusterIntegration|Cluster integration is enabled for this project." +msgstr "" + +msgid "ClusterIntegration|Cluster integration is enabled for this project. Disabling this integration will not affect your cluster, it will only temporarily turn off GitLab's connection to it." +msgstr "" + +msgid "ClusterIntegration|Cluster is being created on Google Container Engine..." +msgstr "" + +msgid "ClusterIntegration|Cluster name" +msgstr "" + +msgid "ClusterIntegration|Cluster was successfully created on Google Container Engine" +msgstr "" + +msgid "ClusterIntegration|Copy cluster name" +msgstr "" + +msgid "ClusterIntegration|Create cluster" +msgstr "" + +msgid "ClusterIntegration|Create new cluster on Google Container Engine" +msgstr "" + +msgid "ClusterIntegration|Enable cluster integration" +msgstr "" + +msgid "ClusterIntegration|Google Cloud Platform project ID" +msgstr "" + +msgid "ClusterIntegration|Google Container Engine" +msgstr "" + +msgid "ClusterIntegration|Google Container Engine project" +msgstr "" + +msgid "ClusterIntegration|Google Container Engine" +msgstr "" + +msgid "ClusterIntegration|Learn more about %{link_to_documentation}" +msgstr "" + +msgid "ClusterIntegration|See machine types" +msgstr "" + +msgid "ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters" +msgstr "" + +msgid "ClusterIntegration|Manage your cluster by visiting %{link_gke}" +msgstr "" + +msgid "ClusterIntegration|Number of nodes" +msgstr "" + +msgid "ClusterIntegration|Project namespace (optional, unique)" +msgstr "" + +msgid "ClusterIntegration|Remove cluster integration" +msgstr "" + +msgid "ClusterIntegration|Remove integration" +msgstr "" + +msgid "ClusterIntegration|Removing cluster integration will remove the cluster configuration you have added to this project. It will not delete your project." +msgstr "" + +msgid "ClusterIntegration|Save changes" +msgstr "" + +msgid "ClusterIntegration|See your projects" +msgstr "" + +msgid "ClusterIntegration|See zones" +msgstr "" + +msgid "ClusterIntegration|Something went wrong on our end." +msgstr "" + +msgid "ClusterIntegration|Something went wrong while creating your cluster on Google Container Engine." +msgstr "" + +msgid "ClusterIntegration|Please make sure that your Google account meets the following requirements:" +msgstr "" + +msgid "ClusterIntegration|Toggle Cluster" +msgstr "" + +msgid "ClusterIntegration|Read our %{link_to_help_page} on cluster integration." +msgstr "" + +msgid "ClusterIntegration|With a cluster associated to this project, you can use review apps, deploy your applications, run your pipelines, and much more in an easy way." +msgstr "" + +msgid "ClusterIntegration|Your account must have %{link_to_container_engine}" +msgstr "" + +msgid "ClusterIntegration|Zone" +msgstr "" + +msgid "ClusterIntegration|access to Google Container Engine" +msgstr "" + +msgid "ClusterIntegration|cluster" +msgstr "" + +msgid "ClusterIntegration|help page" +msgstr "" + +msgid "ClusterIntegration|meets the requirements" +msgstr "" + +msgid "ClusterIntegration|properly configured" +msgstr "" + msgid "Comments" msgstr "" @@ -640,6 +763,9 @@ msgstr "" msgid "GoToYourFork|Fork" msgstr "" +msgid "Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service." +msgstr "" + msgid "GroupSettings|Prevent sharing a project within %{group} with other groups" msgstr "" diff --git a/spec/controllers/google_api/authorizations_controller_spec.rb b/spec/controllers/google_api/authorizations_controller_spec.rb new file mode 100644 index 00000000000..80d553f0f34 --- /dev/null +++ b/spec/controllers/google_api/authorizations_controller_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe GoogleApi::AuthorizationsController do + describe 'GET|POST #callback' do + let(:user) { create(:user) } + let(:token) { 'token' } + let(:expires_at) { 1.hour.since.strftime('%s') } + + subject { get :callback, code: 'xxx', state: @state } + + before do + sign_in(user) + + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:get_token).and_return([token, expires_at]) + end + + it 'sets token and expires_at in session' do + subject + + expect(session[GoogleApi::CloudPlatform::Client.session_key_for_token]) + .to eq(token) + expect(session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at]) + .to eq(expires_at) + end + + context 'when redirect uri key is stored in state' do + set(:project) { create(:project) } + let(:redirect_uri) { project_clusters_url(project).to_s } + + before do + @state = GoogleApi::CloudPlatform::Client + .new_session_key_for_redirect_uri do |key| + session[key] = redirect_uri + end + end + + it 'redirects to the URL stored in state param' do + expect(subject).to redirect_to(redirect_uri) + end + end + + context 'when redirection url is not stored in state' do + it 'redirects to root_path' do + expect(subject).to redirect_to(root_path) + end + end + end +end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb new file mode 100644 index 00000000000..7985028d73b --- /dev/null +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -0,0 +1,308 @@ +require 'spec_helper' + +describe Projects::ClustersController do + set(:user) { create(:user) } + set(:project) { create(:project) } + let(:role) { :master } + + before do + project.team << [user, role] + + sign_in(user) + end + + describe 'GET index' do + subject do + get :index, namespace_id: project.namespace, + project_id: project + end + + context 'when cluster is already created' do + let!(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + + it 'redirects to show a cluster' do + subject + + expect(response).to redirect_to(project_cluster_path(project, cluster)) + end + end + + context 'when we do not have cluster' do + it 'redirects to create a cluster' do + subject + + expect(response).to redirect_to(new_project_cluster_path(project)) + end + end + end + + describe 'GET login' do + render_views + + subject do + get :login, namespace_id: project.namespace, + project_id: project + end + + context 'when we do have omniauth configured' do + it 'shows login button' do + subject + + expect(response.body).to include('auth_buttons/signin_with_google') + end + end + + context 'when we do not have omniauth configured' do + before do + stub_omniauth_setting(providers: []) + end + + it 'shows notice message' do + subject + + expect(response.body).to include('Ask your GitLab administrator if you want to use this service.') + end + end + end + + shared_examples 'requires to login' do + it 'redirects to create a cluster' do + subject + + expect(response).to redirect_to(login_project_clusters_path(project)) + end + end + + describe 'GET new' do + render_views + + subject do + get :new, namespace_id: project.namespace, + project_id: project + end + + context 'when logged' do + before do + make_logged_in + end + + it 'shows a creation form' do + subject + + expect(response.body).to include('Create cluster') + end + end + + context 'when not logged' do + it_behaves_like 'requires to login' + end + end + + describe 'POST create' do + subject do + post :create, params.merge(namespace_id: project.namespace, + project_id: project) + end + + context 'when not logged' do + let(:params) { {} } + + it_behaves_like 'requires to login' + end + + context 'when logged in' do + before do + make_logged_in + end + + context 'when all required parameters are set' do + let(:params) do + { + cluster: { + gcp_cluster_name: 'new-cluster', + gcp_project_id: '111' + } + } + end + + before do + expect(ClusterProvisionWorker).to receive(:perform_async) { } + end + + it 'creates a new cluster' do + expect { subject }.to change { Gcp::Cluster.count } + + expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + end + end + + context 'when not all required parameters are set' do + render_views + + let(:params) do + { + cluster: { + project_namespace: 'some namespace' + } + } + end + + it 'shows an error message' do + expect { subject }.not_to change { Gcp::Cluster.count } + + expect(response).to render_template(:new) + end + end + end + end + + describe 'GET status' do + let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + + subject do + get :status, namespace_id: project.namespace, + project_id: project, + id: cluster, + format: :json + end + + it "responds with matching schema" do + subject + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('cluster_status') + end + end + + describe 'GET show' do + render_views + + let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + + subject do + get :show, namespace_id: project.namespace, + project_id: project, + id: cluster + end + + context 'when logged as master' do + it "allows to update cluster" do + subject + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Save") + end + + it "allows remove integration" do + subject + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Remove integration") + end + end + + context 'when logged as developer' do + let(:role) { :developer } + + it "does not allow to access page" do + subject + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'PUT update' do + render_views + + let(:service) { project.build_kubernetes_service } + let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project, service: service) } + let(:params) { {} } + + subject do + put :update, params.merge(namespace_id: project.namespace, + project_id: project, + id: cluster) + end + + context 'when logged as master' do + context 'when valid params are used' do + let(:params) do + { + cluster: { enabled: false } + } + end + + it "redirects back to show page" do + subject + + expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + expect(flash[:notice]).to eq('Cluster was successfully updated.') + end + end + + context 'when invalid params are used' do + let(:params) do + { + cluster: { project_namespace: 'my Namespace 321321321 #' } + } + end + + it "rejects changes" do + subject + + expect(response).to have_http_status(:ok) + expect(response).to render_template(:show) + end + end + end + + context 'when logged as developer' do + let(:role) { :developer } + + it "does not allow to update cluster" do + subject + + expect(response).to have_http_status(:not_found) + end + end + end + + describe 'delete update' do + let(:cluster) { create(:gcp_cluster, :created_on_gke, project: project) } + + subject do + delete :destroy, namespace_id: project.namespace, + project_id: project, + id: cluster + end + + context 'when logged as master' do + it "redirects back to clusters list" do + subject + + expect(response).to redirect_to(project_clusters_path(project)) + expect(flash[:notice]).to eq('Cluster integration was successfully removed.') + end + end + + context 'when logged as developer' do + let(:role) { :developer } + + it "does not allow to destroy cluster" do + subject + + expect(response).to have_http_status(:not_found) + end + end + end + + def make_logged_in + session[GoogleApi::CloudPlatform::Client.session_key_for_token] = '1234' + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = in_hour.to_i.to_s + end + + def in_hour + Time.now + 1.hour + end +end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index c0337f96fc6..e3114e5c06e 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -266,6 +266,56 @@ describe Projects::NotesController do end end end + + context 'when the merge request discussion is locked' do + before do + project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + merge_request.update_attribute(:discussion_locked, true) + end + + context 'when a noteable is not found' do + it 'returns 404 status' do + request_params[:note][:noteable_id] = 9999 + post :create, request_params.merge(format: :json) + + expect(response).to have_http_status(404) + end + end + + context 'when a user is a team member' do + it 'returns 302 status for html' do + post :create, request_params + + expect(response).to have_http_status(302) + end + + it 'returns 200 status for json' do + post :create, request_params.merge(format: :json) + + expect(response).to have_http_status(200) + end + + it 'creates a new note' do + expect { post :create, request_params }.to change { Note.count }.by(1) + end + end + + context 'when a user is not a team member' do + before do + project.project_member(user).destroy + end + + it 'returns 404 status' do + post :create, request_params + + expect(response).to have_http_status(404) + end + + it 'does not create a new note' do + expect { post :create, request_params }.not_to change { Note.count } + end + end + end end describe 'DELETE destroy' do diff --git a/spec/factories/gcp/cluster.rb b/spec/factories/gcp/cluster.rb new file mode 100644 index 00000000000..630e40da888 --- /dev/null +++ b/spec/factories/gcp/cluster.rb @@ -0,0 +1,38 @@ +FactoryGirl.define do + factory :gcp_cluster, class: Gcp::Cluster do + project + user + enabled true + gcp_project_id 'gcp-project-12345' + gcp_cluster_name 'test-cluster' + gcp_cluster_zone 'us-central1-a' + gcp_cluster_size 1 + gcp_machine_type 'n1-standard-4' + + trait :with_kubernetes_service do + after(:create) do |cluster, evaluator| + create(:kubernetes_service, project: cluster.project).tap do |service| + cluster.update(service: service) + end + end + end + + trait :custom_project_namespace do + project_namespace 'sample-app' + end + + trait :created_on_gke do + status_event :make_created + endpoint '111.111.111.111' + ca_cert 'xxxxxx' + kubernetes_token 'xxxxxx' + username 'xxxxxx' + password 'xxxxxx' + end + + trait :errored do + status_event :make_errored + status_reason 'general error' + end + end +end diff --git a/spec/features/issuables/discussion_lock_spec.rb b/spec/features/issuables/discussion_lock_spec.rb new file mode 100644 index 00000000000..7ea29ff252b --- /dev/null +++ b/spec/features/issuables/discussion_lock_spec.rb @@ -0,0 +1,106 @@ +require 'spec_helper' + +describe 'Discussion Lock', :js do + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project, author: user) } + let(:project) { create(:project, :public) } + + before do + sign_in(user) + end + + context 'when a user is a team member' do + before do + project.add_developer(user) + end + + context 'when the discussion is unlocked' do + it 'the user can lock the issue' do + visit project_issue_path(project, issue) + + expect(find('.issuable-sidebar')).to have_content('Unlocked') + + page.within('.issuable-sidebar') do + find('.lock-edit').click + click_button('Lock') + end + + expect(find('#notes')).to have_content('locked this issue') + end + end + + context 'when the discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) + end + + it 'the user can unlock the issue' do + expect(find('.issuable-sidebar')).to have_content('Locked') + + page.within('.issuable-sidebar') do + find('.lock-edit').click + click_button('Unlock') + end + + expect(find('#notes')).to have_content('unlocked this issue') + expect(find('.issuable-sidebar')).to have_content('Unlocked') + end + + it 'the user can create a comment' do + page.within('#notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('div#notes')).to have_content('Some new comment') + end + end + end + + context 'when a user is not a team member' do + context 'when the discussion is unlocked' do + before do + visit project_issue_path(project, issue) + end + + it 'the user can not lock the issue' do + expect(find('.issuable-sidebar')).to have_content('Unlocked') + expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit') + end + + it 'the user can create a comment' do + page.within('#notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('div#notes')).to have_content('Some new comment') + end + end + + context 'when the discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + visit project_issue_path(project, issue) + end + + it 'the user can not unlock the issue' do + expect(find('.issuable-sidebar')).to have_content('Locked') + expect(find('.issuable-sidebar')).not_to have_selector('.lock-edit') + end + + it 'the user can not create a comment' do + page.within('#notes') do + expect(page).not_to have_selector('js-main-target-form') + expect(page.find('.disabled-comment')) + .to have_content('This issue is locked. Only project members can comment.') + end + end + end + end +end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index aa8cf3b013c..e2db1442d90 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -609,14 +609,14 @@ describe 'Issues', :js do visit project_issue_path(project, issue) - expect(page).to have_css('.confidential-issue-warning') - expect(page).to have_css('.is-confidential') - expect(page).not_to have_css('.is-not-confidential') + expect(page).to have_css('.issuable-note-warning') + expect(find('.issuable-sidebar-item.confidentiality')).to have_css('.is-active') + expect(find('.issuable-sidebar-item.confidentiality')).not_to have_css('.not-active') find('.confidential-edit').click - expect(page).to have_css('.confidential-warning-message') + expect(page).to have_css('.sidebar-item-warning-message') - within('.confidential-warning-message') do + within('.sidebar-item-warning-message') do find('.btn-close').click end @@ -624,7 +624,7 @@ describe 'Issues', :js do visit project_issue_path(project, issue) - expect(page).not_to have_css('.is-confidential') + expect(page).not_to have_css('.is-active') end end end diff --git a/spec/features/merge_requests/discussion_lock_spec.rb b/spec/features/merge_requests/discussion_lock_spec.rb new file mode 100644 index 00000000000..7bbd3b1e69e --- /dev/null +++ b/spec/features/merge_requests/discussion_lock_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'Discussion Lock', :js do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, source_project: project, author: user) } + let(:project) { create(:project, :public, :repository) } + + before do + sign_in(user) + end + + context 'when the discussion is locked' do + before do + merge_request.update_attribute(:discussion_locked, true) + end + + context 'when a user is a team member' do + before do + project.add_developer(user) + visit project_merge_request_path(project, merge_request) + end + + it 'the user can create a comment' do + page.within('.issuable-discussion #notes .js-main-target-form') do + fill_in 'note[note]', with: 'Some new comment' + click_button 'Comment' + end + + wait_for_requests + + expect(find('.issuable-discussion #notes')).to have_content('Some new comment') + end + end + + context 'when a user is not a team member' do + before do + visit project_merge_request_path(project, merge_request) + end + + it 'the user can not create a comment' do + page.within('.issuable-discussion #notes') do + expect(page).not_to have_selector('js-main-target-form') + expect(page.find('.disabled-comment')) + .to have_content('This merge request is locked. Only project members can comment.') + end + end + end + end +end diff --git a/spec/features/merge_requests/user_posts_diff_notes_spec.rb b/spec/features/merge_requests/user_posts_diff_notes_spec.rb index 2fb6d0b965f..e1ca4fe186c 100644 --- a/spec/features/merge_requests/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_requests/user_posts_diff_notes_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'Merge requests > User posts diff notes', :js do + include MergeRequestDiffHelpers + let(:user) { create(:user) } let(:merge_request) { create(:merge_request) } let(:project) { merge_request.source_project } @@ -244,36 +246,6 @@ feature 'Merge requests > User posts diff notes', :js do expect(line[:num]).not_to have_css comment_button_class end - def get_line_components(line_holder, diff_side = nil) - if diff_side.nil? - get_inline_line_components(line_holder) - else - get_parallel_line_components(line_holder, diff_side) - end - end - - def get_inline_line_components(line_holder) - { content: line_holder.find('.line_content', match: :first), num: line_holder.find('.diff-line-num', match: :first) } - end - - def get_parallel_line_components(line_holder, diff_side = nil) - side_index = diff_side == 'left' ? 0 : 1 - # Wait for `.line_content` - line_holder.find('.line_content', match: :first) - # Wait for `.diff-line-num` - line_holder.find('.diff-line-num', match: :first) - { content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] } - end - - def click_diff_line(line_holder, diff_side = nil) - line = get_line_components(line_holder, diff_side) - line[:content].hover - - expect(line[:num]).to have_css comment_button_class - - line[:num].find(comment_button_class).trigger 'click' - end - def write_comment_on_line(line_holder, diff_side) click_diff_line(line_holder, diff_side) diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb new file mode 100644 index 00000000000..810f2c39b43 --- /dev/null +++ b/spec/features/projects/clusters_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' + +feature 'Clusters', :js do + let!(:project) { create(:project, :repository) } + let!(:user) { create(:user) } + + before do + project.add_master(user) + gitlab_sign_in(user) + end + + context 'when user has signed in Google' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:validate_token).and_return(true) + end + + context 'when user does not have a cluster and visits cluster index page' do + before do + visit project_clusters_path(project) + end + + it 'user sees a new page' do + expect(page).to have_button('Create cluster') + end + + context 'when user filled form with valid parameters' do + before do + double.tap do |dbl| + allow(dbl).to receive(:status).and_return('RUNNING') + allow(dbl).to receive(:self_link) + .and_return('projects/gcp-project-12345/zones/us-central1-a/operations/ope-123') + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_create).and_return(dbl) + end + + allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) + + fill_in 'cluster_gcp_project_id', with: 'gcp-project-123' + fill_in 'cluster_gcp_cluster_name', with: 'dev-cluster' + click_button 'Create cluster' + end + + it 'user sees a cluster details page and creation status' do + expect(page).to have_content('Cluster is being created on Google Container Engine...') + + Gcp::Cluster.last.make_created! + + expect(page).to have_content('Cluster was successfully created on Google Container Engine') + end + end + + context 'when user filled form with invalid parameters' do + before do + click_button 'Create cluster' + end + + it 'user sees a validation error' do + expect(page).to have_css('#error_explanation') + end + end + end + + context 'when user has a cluster and visits cluster index page' do + let!(:cluster) { create(:gcp_cluster, :created_on_gke, :with_kubernetes_service, project: project) } + + before do + visit project_clusters_path(project) + end + + it 'user sees an cluster details page' do + expect(page).to have_button('Save') + expect(page.find(:css, '.cluster-name').value).to eq(cluster.gcp_cluster_name) + end + + context 'when user disables the cluster' do + before do + page.find(:css, '.js-toggle-cluster').click + click_button 'Save' + end + + it 'user sees the succeccful message' do + expect(page).to have_content('Cluster was successfully updated.') + end + end + + context 'when user destory the cluster' do + before do + page.accept_confirm do + click_link 'Remove integration' + end + end + + it 'user sees creation form with the succeccful message' do + expect(page).to have_content('Cluster integration was successfully removed.') + expect(page).to have_button('Create cluster') + end + end + end + end + + context 'when user has not signed in Google' do + before do + visit project_clusters_path(project) + end + + it 'user sees a login page' do + expect(page).to have_css('.signin-with-google') + end + end +end diff --git a/spec/features/projects/merge_requests/user_accepts_merge_request_spec.rb b/spec/features/projects/merge_requests/user_accepts_merge_request_spec.rb index 6c0b5e279d5..c35ba2d7016 100644 --- a/spec/features/projects/merge_requests/user_accepts_merge_request_spec.rb +++ b/spec/features/projects/merge_requests/user_accepts_merge_request_spec.rb @@ -62,4 +62,23 @@ describe 'User accepts a merge request', :js do wait_for_requests end end + + context 'when modifying the merge commit message' do + before do + merge_request.mark_as_mergeable + + visit(merge_request_path(merge_request)) + end + + it 'accepts a merge request' do + click_button('Modify commit message') + fill_in('Commit message', with: 'wow such merge') + + click_button('Merge') + + page.within('.status-box') do + expect(page).to have_content('Merged') + end + end + end end diff --git a/spec/features/projects/merge_requests/user_closes_merge_request_spec.rb b/spec/features/projects/merge_requests/user_closes_merge_request_spec.rb new file mode 100644 index 00000000000..b257f447439 --- /dev/null +++ b/spec/features/projects/merge_requests/user_closes_merge_request_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe 'User closes a merge requests', :js do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(merge_request_path(merge_request)) + end + + it 'closes a merge request' do + click_link('Close merge request', match: :first) + + expect(page).to have_content(merge_request.title) + expect(page).to have_content('Closed by') + end +end diff --git a/spec/features/projects/merge_requests/user_comments_on_commit_spec.rb b/spec/features/projects/merge_requests/user_comments_on_commit_spec.rb new file mode 100644 index 00000000000..0a952cfc2a9 --- /dev/null +++ b/spec/features/projects/merge_requests/user_comments_on_commit_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe 'User comments on a commit', :js do + include MergeRequestDiffHelpers + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(project_commit_path(project, sample_commit.id)) + end + + include_examples 'comment on merge request file' +end diff --git a/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb b/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb new file mode 100644 index 00000000000..f34302f25f8 --- /dev/null +++ b/spec/features/projects/merge_requests/user_comments_on_diff_spec.rb @@ -0,0 +1,172 @@ +require 'spec_helper' + +describe 'User comments on a diff', :js do + include MergeRequestDiffHelpers + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') + end + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(diffs_project_merge_request_path(project, merge_request)) + end + + context 'when viewing comments' do + context 'when toggling inline comments' do + context 'in a single file' do + it 'hides a comment' do + click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: 'Line is wrong') + click_button('Comment') + end + + page.within('.files > div:nth-child(3)') do + expect(page).to have_content('Line is wrong') + + find('.js-toggle-diff-comments').trigger('click') + + expect(page).not_to have_content('Line is wrong') + end + end + end + + context 'in multiple files' do + it 'toggles comments' do + click_diff_line(find("[id='#{sample_compare.changes[0][:line_code]}']")) + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: 'Line is correct') + click_button('Comment') + end + + wait_for_requests + + page.within('.files > div:nth-child(2) .note-body > .note-text') do + expect(page).to have_content('Line is correct') + end + + click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) + + page.within('.js-discussion-note-form') do + fill_in('note_note', with: 'Line is wrong') + click_button('Comment') + end + + wait_for_requests + + # Hide the comment. + page.within('.files > div:nth-child(3)') do + find('.js-toggle-diff-comments').trigger('click') + + expect(page).not_to have_content('Line is wrong') + end + + # At this moment a user should see only one comment. + # The other one should be hidden. + page.within('.files > div:nth-child(2) .note-body > .note-text') do + expect(page).to have_content('Line is correct') + end + + # Show the comment. + page.within('.files > div:nth-child(3)') do + find('.js-toggle-diff-comments').trigger('click') + end + + # Now both the comments should be shown. + page.within('.files > div:nth-child(3) .note-body > .note-text') do + expect(page).to have_content('Line is wrong') + end + + page.within('.files > div:nth-child(2) .note-body > .note-text') do + expect(page).to have_content('Line is correct') + end + + # Check the same comments in the side-by-side view. + click_link('Side-by-side') + + wait_for_requests + + page.within('.files > div:nth-child(3) .parallel .note-body > .note-text') do + expect(page).to have_content('Line is wrong') + end + + page.within('.files > div:nth-child(2) .parallel .note-body > .note-text') do + expect(page).to have_content('Line is correct') + end + end + end + end + end + + context 'when adding comments' do + include_examples 'comment on merge request file' + end + + context 'when editing comments' do + it 'edits a comment' do + click_diff_line(find("[id='#{sample_commit.line_code}']")) + + page.within('.js-discussion-note-form') do + fill_in(:note_note, with: 'Line is wrong') + click_button('Comment') + end + + page.within('.diff-file:nth-of-type(5) .note') do + find('.js-note-edit').click + + page.within('.current-note-edit-form') do + fill_in('note_note', with: 'Typo, please fix') + click_button('Save comment') + end + + expect(page).not_to have_button('Save comment', disabled: true) + end + + page.within('.diff-file:nth-of-type(5) .note') do + expect(page).to have_content('Typo, please fix').and have_no_content('Line is wrong') + end + end + end + + context 'when deleting comments' do + it 'deletes a comment' do + click_diff_line(find("[id='#{sample_commit.line_code}']")) + + page.within('.js-discussion-note-form') do + fill_in(:note_note, with: 'Line is wrong') + click_button('Comment') + end + + page.within('.notes-tab .badge') do + expect(page).to have_content('1') + end + + page.within('.diff-file:nth-of-type(5) .note') do + find('.more-actions').click + find('.more-actions .dropdown-menu li', match: :first) + + find('.js-note-delete').click + end + + page.within('.merge-request-tabs') do + find('.notes-tab').trigger('click') + end + + wait_for_requests + + expect(page).not_to have_css('.notes .discussion') + + page.within('.notes-tab .badge') do + expect(page).to have_content('0') + end + end + end +end diff --git a/spec/features/projects/merge_requests/user_comments_on_merge_request_spec.rb b/spec/features/projects/merge_requests/user_comments_on_merge_request_spec.rb new file mode 100644 index 00000000000..2eb652147ce --- /dev/null +++ b/spec/features/projects/merge_requests/user_comments_on_merge_request_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe 'User comments on a merge request', :js do + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(merge_request_path(merge_request)) + end + + it 'adds a comment' do + page.within('.js-main-target-form') do + fill_in(:note_note, with: '# Comment with a header') + click_button('Comment') + end + + wait_for_requests + + page.within('.note') do + expect(page).to have_content('Comment with a header') + expect(page).not_to have_css('#comment-with-a-header') + end + end + + it 'loads new comment' do + # Add new comment in background in order to check + # if it's going to be loaded automatically for current user. + create(:diff_note_on_merge_request, project: project, noteable: merge_request, author: user, note: 'Line is wrong') + + # Trigger a refresh of notes. + execute_script("$(document).trigger('visibilitychange');") + wait_for_requests + + page.within('.notes .discussion') do + expect(page).to have_content("#{user.name} #{user.to_reference} started a discussion") + expect(page).to have_content(sample_commit.line_code_path) + expect(page).to have_content('Line is wrong') + end + + page.within('.notes-tab .badge') do + expect(page).to have_content('1') + end + end +end diff --git a/spec/features/projects/merge_requests/user_creates_merge_request_spec.rb b/spec/features/projects/merge_requests/user_creates_merge_request_spec.rb new file mode 100644 index 00000000000..f285c6c8783 --- /dev/null +++ b/spec/features/projects/merge_requests/user_creates_merge_request_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe 'User creates a merge request', :js do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(project_new_merge_request_path(project)) + end + + it 'creates a merge request' do + find('.js-source-branch').click + click_link('fix') + + find('.js-target-branch').click + click_link('feature') + + click_button('Compare branches') + + fill_in('merge_request_title', with: 'Wiki Feature') + click_button('Submit merge request') + + page.within('.merge-request') do + expect(page).to have_content('Wiki Feature') + end + + wait_for_requests + end +end diff --git a/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb b/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb new file mode 100644 index 00000000000..f6e3997383f --- /dev/null +++ b/spec/features/projects/merge_requests/user_edits_merge_request_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe 'User edits a merge request', :js do + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(edit_project_merge_request_path(project, merge_request)) + end + + it 'changes the target branch' do + expect(page).to have_content('Target branch') + + first('.target_branch').click + select('merge-test', from: 'merge_request_target_branch', visible: false) + click_button('Save changes') + + expect(page).to have_content("Request to merge #{merge_request.source_branch} into merge-test") + expect(page).to have_content("changed target branch from #{merge_request.target_branch} to merge-test") + end +end diff --git a/spec/features/projects/merge_requests/user_manages_subscription_spec.rb b/spec/features/projects/merge_requests/user_manages_subscription_spec.rb new file mode 100644 index 00000000000..30a80f8e652 --- /dev/null +++ b/spec/features/projects/merge_requests/user_manages_subscription_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe 'User manages subscription', :js do + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(merge_request_path(merge_request)) + end + + it 'toggles subscription' do + subscribe_button = find('.issuable-subscribe-button span') + + expect(subscribe_button).to have_content('Subscribe') + + click_on('Subscribe') + + wait_for_requests + + expect(subscribe_button).to have_content('Unsubscribe') + + click_on('Unsubscribe') + + wait_for_requests + + expect(subscribe_button).to have_content('Subscribe') + end +end diff --git a/spec/features/projects/merge_requests/user_reopens_merge_request_spec.rb b/spec/features/projects/merge_requests/user_reopens_merge_request_spec.rb new file mode 100644 index 00000000000..ba3c9789da1 --- /dev/null +++ b/spec/features/projects/merge_requests/user_reopens_merge_request_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe 'User reopens a merge requests', :js do + let(:project) { create(:project, :public, :repository) } + let!(:merge_request) { create(:closed_merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(merge_request_path(merge_request)) + end + + it 'reopens a merge request' do + click_link('Reopen merge request', match: :first) + + page.within('.status-box') do + expect(page).to have_content('Open') + end + end +end diff --git a/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb new file mode 100644 index 00000000000..d8d9f7e2a8c --- /dev/null +++ b/spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe 'User sorts merge requests' do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let!(:merge_request2) do + create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') + end + let(:project) { create(:project, :public, :repository) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(project_merge_requests_path(project)) + end + + it 'keeps the sort option' do + find('button.dropdown-toggle').click + + page.within('.content ul.dropdown-menu.dropdown-menu-align-right li') do + click_link('Last updated') + end + + visit(merge_requests_dashboard_path(assignee_id: user.id)) + + expect(find('.issues-filters')).to have_content('Last updated') + + visit(project_merge_requests_path(project)) + + expect(find('.issues-filters')).to have_content('Last updated') + end + + context 'when merge requests have awards' do + before do + create_list(:award_emoji, 2, awardable: merge_request) + create(:award_emoji, :downvote, awardable: merge_request) + + create(:award_emoji, awardable: merge_request2) + create_list(:award_emoji, 2, :downvote, awardable: merge_request2) + end + + it 'sorts by popularity' do + find('button.dropdown-toggle').click + + page.within('.content ul.dropdown-menu.dropdown-menu-align-right li') do + click_link('Popularity') + end + + page.within('.mr-list') do + page.within('li.merge-request:nth-child(1)') do + expect(page).to have_content(merge_request.title) + expect(page).to have_content('2 1') + end + + page.within('li.merge-request:nth-child(2)') do + expect(page).to have_content(merge_request2.title) + expect(page).to have_content('1 2') + end + end + end + end +end diff --git a/spec/features/projects/merge_requests/user_views_all_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_views_all_merge_requests_spec.rb new file mode 100644 index 00000000000..6c695bd7aa9 --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_all_merge_requests_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'User views all merge requests' do + let!(:closed_merge_request) { create(:closed_merge_request, source_project: project, target_project: project) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :public) } + + before do + visit(project_merge_requests_path(project, state: :all)) + end + + it 'shows all merge requests' do + expect(page).to have_content(merge_request.title).and have_content(closed_merge_request.title) + end +end diff --git a/spec/features/projects/merge_requests/user_views_closed_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_views_closed_merge_requests_spec.rb new file mode 100644 index 00000000000..853809fe87a --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_closed_merge_requests_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'User views closed merge requests' do + let!(:closed_merge_request) { create(:closed_merge_request, source_project: project, target_project: project) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :public) } + + before do + visit(project_merge_requests_path(project, state: :closed)) + end + + it 'shows closed merge requests' do + expect(page).to have_content(closed_merge_request.title).and have_no_content(merge_request.title) + end +end diff --git a/spec/features/projects/merge_requests/user_views_diffs_spec.rb b/spec/features/projects/merge_requests/user_views_diffs_spec.rb new file mode 100644 index 00000000000..295eb02b625 --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_diffs_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe 'User views diffs', :js do + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') + end + let(:project) { create(:project, :public, :repository) } + + before do + visit(diffs_project_merge_request_path(project, merge_request)) + + wait_for_requests + end + + shared_examples 'unfold diffs' do + it 'unfolds diffs' do + first('.js-unfold').click + + expect(first('.text-file')).to have_content('.bundle') + end + end + + it 'shows diffs' do + expect(page).to have_css('.tab-content #diffs.active') + expect(page).to have_css('#parallel-diff-btn', count: 1) + expect(page).to have_css('#inline-diff-btn', count: 1) + end + + context 'when in the inline view' do + include_examples 'unfold diffs' + end + + context 'when in the side-by-side view' do + before do + click_link('Side-by-side') + + wait_for_requests + end + + it 'shows diffs in parallel' do + expect(page).to have_css('.parallel') + end + + include_examples 'unfold diffs' + end +end diff --git a/spec/features/projects/merge_requests/user_views_merged_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_views_merged_merge_requests_spec.rb new file mode 100644 index 00000000000..eb012694f1e --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_merged_merge_requests_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'User views merged merge requests' do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let!(:merged_merge_request) { create(:merged_merge_request, source_project: project, target_project: project) } + let(:project) { create(:project, :public) } + + before do + visit(project_merge_requests_path(project, state: :merged)) + end + + it 'shows merged merge requests' do + expect(page).to have_content(merged_merge_request.title).and have_no_content(merge_request.title) + end +end diff --git a/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb b/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb new file mode 100644 index 00000000000..8970cf54457 --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_open_merge_request_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +describe 'User views an open merge request' do + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project, description: '# Description header') + end + + context 'when a merge request does not have repository' do + let(:project) { create(:project, :public) } + + before do + visit(merge_request_path(merge_request)) + end + + it 'renders both the title and the description' do + node = find('.wiki h1 a#user-content-description-header') + expect(node[:href]).to end_with('#description-header') + + # Work around a weird Capybara behavior where calling `parent` on a node + # returns the whole document, not the node's actual parent element + expect(find(:xpath, "#{node.path}/..").text).to eq(merge_request.description[2..-1]) + + expect(page).to have_content(merge_request.title).and have_content(merge_request.description) + end + end + + context 'when a merge request has repository', :js do + let(:project) { create(:project, :public, :repository) } + + context 'when rendering description preview' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + + visit(edit_project_merge_request_path(project, merge_request)) + end + + it 'renders empty description preview' do + find('.gfm-form').fill_in(:merge_request_description, with: '') + + page.within('.gfm-form') do + click_link('Preview') + + expect(find('.js-md-preview')).to have_content('Nothing to preview.') + end + end + + it 'renders description preview' do + find('.gfm-form').fill_in(:merge_request_description, with: ':+1: Nice') + + page.within('.gfm-form') do + click_link('Preview') + + expect(find('.js-md-preview')).to have_css('gl-emoji') + end + + expect(find('.gfm-form')).to have_css('.js-md-preview').and have_link('Write') + expect(find('#merge_request_description', visible: false)).not_to be_visible + end + end + + context 'when the branch is rebased on the target' do + let(:merge_request) { create(:merge_request, :rebased, source_project: project, target_project: project) } + + before do + visit(merge_request_path(merge_request)) + end + + it 'does not show diverged commits count' do + page.within('.mr-source-target') do + expect(page).not_to have_content(/([0-9]+ commit[s]? behind)/) + end + end + end + + context 'when the branch is diverged on the target' do + let(:merge_request) { create(:merge_request, :diverged, source_project: project, target_project: project) } + + before do + visit(merge_request_path(merge_request)) + end + + it 'shows diverged commits count' do + page.within('.mr-source-target') do + expect(page).to have_content(/([0-9]+ commits behind)/) + end + end + end + end +end diff --git a/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb new file mode 100644 index 00000000000..07b8c1ef479 --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' + +describe 'User views open merge requests' do + let(:project) { create(:project, :public, :repository) } + + context "when the target branch is the project's default branch" do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let!(:closed_merge_request) { create(:closed_merge_request, source_project: project, target_project: project) } + + before do + visit(project_merge_requests_path(project)) + end + + it 'shows open merge requests' do + expect(page).to have_content(merge_request.title).and have_no_content(closed_merge_request.title) + end + + it 'does not show target branch name' do + expect(page).to have_content(merge_request.title) + expect(find('.issuable-info')).not_to have_content(project.default_branch) + end + end + + context "when the target branch is different from the project's default branch" do + let!(:merge_request) do + create(:merge_request, + source_project: project, + target_project: project, + source_branch: 'fix', + target_branch: 'feature_conflict') + end + + before do + visit(project_merge_requests_path(project)) + end + + it 'shows target branch name' do + expect(page).to have_content(merge_request.target_branch) + end + end + + context 'when a merge request has pipelines' do + let!(:build) { create :ci_build, pipeline: pipeline } + + let(:merge_request) do + create(:merge_request_with_diffs, + source_project: project, + target_project: project, + source_branch: 'merge-test') + end + + let(:pipeline) do + create(:ci_pipeline, + project: project, + sha: merge_request.diff_head_sha, + ref: merge_request.source_branch, + head_pipeline_of: merge_request) + end + + before do + project.enable_ci + + visit(project_merge_requests_path(project)) + end + + it 'shows pipeline status' do + page.within('.mr-list') do + expect(page).to have_link('Pipeline: pending') + end + end + end +end diff --git a/spec/fixtures/api/schemas/cluster_status.json b/spec/fixtures/api/schemas/cluster_status.json new file mode 100644 index 00000000000..1f255a17881 --- /dev/null +++ b/spec/fixtures/api/schemas/cluster_status.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required" : [ + "status" + ], + "properties" : { + "status": { "type": "string" }, + "status_reason": { "type": ["string", "null"] } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/public_api/v4/issues.json b/spec/fixtures/api/schemas/public_api/v4/issues.json index 03c422ab023..5c08dbc3b96 100644 --- a/spec/fixtures/api/schemas/public_api/v4/issues.json +++ b/spec/fixtures/api/schemas/public_api/v4/issues.json @@ -9,6 +9,7 @@ "title": { "type": "string" }, "description": { "type": ["string", "null"] }, "state": { "type": "string" }, + "discussion_locked": { "type": ["boolean", "null"] }, "closed_at": { "type": "date" }, "created_at": { "type": "date" }, "updated_at": { "type": "date" }, diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index 31b3f4ba946..5828be5255b 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -72,6 +72,7 @@ "user_notes_count": { "type": "integer" }, "should_remove_source_branch": { "type": ["boolean", "null"] }, "force_remove_source_branch": { "type": ["boolean", "null"] }, + "discussion_locked": { "type": ["boolean", "null"] }, "web_url": { "type": "uri" }, "time_stats": { "time_estimate": { "type": "integer" }, diff --git a/spec/javascripts/clusters_spec.js b/spec/javascripts/clusters_spec.js new file mode 100644 index 00000000000..eb1cd6eb804 --- /dev/null +++ b/spec/javascripts/clusters_spec.js @@ -0,0 +1,79 @@ +import Clusters from '~/clusters'; + +describe('Clusters', () => { + let cluster; + preloadFixtures('clusters/show_cluster.html.raw'); + + beforeEach(() => { + loadFixtures('clusters/show_cluster.html.raw'); + cluster = new Clusters(); + }); + + describe('toggle', () => { + it('should update the button and the input field on click', () => { + cluster.toggleButton.click(); + + expect( + cluster.toggleButton.classList, + ).not.toContain('checked'); + + expect( + cluster.toggleInput.getAttribute('value'), + ).toEqual('false'); + }); + }); + + describe('updateContainer', () => { + describe('when creating cluster', () => { + it('should show the creating container', () => { + cluster.updateContainer('creating'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeFalsy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeTruthy(); + }); + }); + + describe('when cluster is created', () => { + it('should show the success container', () => { + cluster.updateContainer('created'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeFalsy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeTruthy(); + }); + }); + + describe('when cluster has error', () => { + it('should show the error container', () => { + cluster.updateContainer('errored', 'this is an error'); + + expect( + cluster.creatingContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.successContainer.classList.contains('hidden'), + ).toBeTruthy(); + expect( + cluster.errorContainer.classList.contains('hidden'), + ).toBeFalsy(); + + expect( + cluster.errorReasonContainer.textContent, + ).toContain('this is an error'); + }); + }); + }); +}); diff --git a/spec/javascripts/fixtures/clusters.rb b/spec/javascripts/fixtures/clusters.rb new file mode 100644 index 00000000000..5774f36f026 --- /dev/null +++ b/spec/javascripts/fixtures/clusters.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe Projects::ClustersController, '(JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} + let(:project) { create(:project, :repository, namespace: namespace) } + let(:cluster) { project.create_cluster!(gcp_cluster_name: "gke-test-creation-1", gcp_project_id: 'gitlab-internal-153318', gcp_cluster_zone: 'us-central1-a', gcp_cluster_size: '1', project_namespace: 'aaa', gcp_machine_type: 'n1-standard-1')} + + render_views + + before(:all) do + clean_frontend_fixtures('clusters/') + end + + before do + sign_in(admin) + end + + after do + remove_repository(project) + end + + it 'clusters/show_cluster.html.raw' do |example| + get :show, + namespace_id: project.namespace.to_param, + project_id: project, + id: cluster + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end +end diff --git a/spec/javascripts/repo/components/repo_file_spec.js b/spec/javascripts/repo/components/repo_file_spec.js index f15633bd8b9..620b604f404 100644 --- a/spec/javascripts/repo/components/repo_file_spec.js +++ b/spec/javascripts/repo/components/repo_file_spec.js @@ -29,15 +29,17 @@ describe('RepoFile', () => { }).$mount(); } - beforeEach(() => { - spyOn(repoFile.mixins[0].methods, 'timeFormated').and.returnValue(updated); - }); - it('renders link, icon, name and last commit details', () => { - const vm = createComponent({ - file, - activeFile, + const RepoFile = Vue.extend(repoFile); + const vm = new RepoFile({ + propsData: { + file, + activeFile, + }, }); + spyOn(vm, 'timeFormated').and.returnValue(updated); + vm.$mount(); + const name = vm.$el.querySelector('.repo-file-name'); const fileIcon = vm.$el.querySelector('.file-icon'); diff --git a/spec/javascripts/repo/components/repo_sidebar_spec.js b/spec/javascripts/repo/components/repo_sidebar_spec.js index 23c10ea022e..35d2b37ac2a 100644 --- a/spec/javascripts/repo/components/repo_sidebar_spec.js +++ b/spec/javascripts/repo/components/repo_sidebar_spec.js @@ -5,18 +5,26 @@ import RepoStore from '~/repo/stores/repo_store'; import repoSidebar from '~/repo/components/repo_sidebar.vue'; describe('RepoSidebar', () => { + let vm; + function createComponent() { const RepoSidebar = Vue.extend(repoSidebar); return new RepoSidebar().$mount(); } + afterEach(() => { + vm.$destroy(); + }); + it('renders a sidebar', () => { RepoStore.files = [{ id: 0, }]; RepoStore.openedFiles = []; - const vm = createComponent(); + RepoStore.isRoot = false; + + vm = createComponent(); const thead = vm.$el.querySelector('thead'); const tbody = vm.$el.querySelector('tbody'); @@ -35,7 +43,7 @@ describe('RepoSidebar', () => { RepoStore.openedFiles = [{ id: 0, }]; - const vm = createComponent(); + vm = createComponent(); expect(vm.$el.classList.contains('sidebar-mini')).toBeTruthy(); expect(vm.$el.querySelector('thead')).toBeFalsy(); @@ -47,7 +55,7 @@ describe('RepoSidebar', () => { tree: true, }; RepoStore.files = []; - const vm = createComponent(); + vm = createComponent(); expect(vm.$el.querySelectorAll('tbody .loading-file').length).toEqual(5); }); @@ -57,7 +65,7 @@ describe('RepoSidebar', () => { id: 0, }]; RepoStore.isRoot = true; - const vm = createComponent(); + vm = createComponent(); expect(vm.$el.querySelector('tbody .prev-directory')).toBeTruthy(); }); @@ -72,7 +80,7 @@ describe('RepoSidebar', () => { }; RepoStore.files = [file1]; RepoStore.isRoot = true; - const vm = createComponent(); + vm = createComponent(); vm.fileClicked(file1); @@ -87,7 +95,7 @@ describe('RepoSidebar', () => { spyOn(Helper, 'getFileFromPath').and.returnValue(file); spyOn(RepoStore, 'setActiveFiles'); - const vm = createComponent(); + vm = createComponent(); vm.fileClicked(file); expect(RepoStore.setActiveFiles).toHaveBeenCalledWith(file); @@ -103,7 +111,7 @@ describe('RepoSidebar', () => { }; RepoStore.files = [file1]; RepoStore.isRoot = true; - const vm = createComponent(); + vm = createComponent(); vm.fileClicked(file1); @@ -114,12 +122,48 @@ describe('RepoSidebar', () => { describe('goToPreviousDirectoryClicked', () => { it('should hide files in directory if already open', () => { const prevUrl = 'foo/bar'; - const vm = createComponent(); + vm = createComponent(); vm.goToPreviousDirectoryClicked(prevUrl); expect(RepoService.url).toEqual(prevUrl); }); }); + + describe('back button', () => { + const file1 = { + id: 1, + url: 'file1', + }; + const file2 = { + id: 2, + url: 'file2', + }; + RepoStore.files = [file1, file2]; + RepoStore.openedFiles = [file1, file2]; + RepoStore.isRoot = true; + + vm = createComponent(); + vm.fileClicked(file1); + + it('render previous file when using back button', () => { + spyOn(Helper, 'getContent').and.callThrough(); + + vm.fileClicked(file2); + expect(Helper.getContent).toHaveBeenCalledWith(file2); + Helper.getContent.calls.reset(); + + history.pushState({ + key: Math.random(), + }, '', file1.url); + const popEvent = document.createEvent('Event'); + popEvent.initEvent('popstate', true, true); + window.dispatchEvent(popEvent); + + expect(Helper.getContent.calls.mostRecent().args[0].url).toContain(file1.url); + + window.history.pushState({}, null, '/'); + }); + }); }); }); diff --git a/spec/javascripts/sidebar/lock/edit_form_buttons_spec.js b/spec/javascripts/sidebar/lock/edit_form_buttons_spec.js new file mode 100644 index 00000000000..b0ea8ae0206 --- /dev/null +++ b/spec/javascripts/sidebar/lock/edit_form_buttons_spec.js @@ -0,0 +1,36 @@ +import Vue from 'vue'; +import editFormButtons from '~/sidebar/components/lock/edit_form_buttons.vue'; +import mountComponent from '../../helpers/vue_mount_component_helper'; + +describe('EditFormButtons', () => { + let vm1; + let vm2; + + beforeEach(() => { + const Component = Vue.extend(editFormButtons); + const toggleForm = () => { }; + const updateLockedAttribute = () => { }; + + vm1 = mountComponent(Component, { + isLocked: true, + toggleForm, + updateLockedAttribute, + }); + + vm2 = mountComponent(Component, { + isLocked: false, + toggleForm, + updateLockedAttribute, + }); + }); + + it('renders unlock or lock text based on locked state', () => { + expect( + vm1.$el.innerHTML.includes('Unlock'), + ).toBe(true); + + expect( + vm2.$el.innerHTML.includes('Lock'), + ).toBe(true); + }); +}); diff --git a/spec/javascripts/sidebar/lock/edit_form_spec.js b/spec/javascripts/sidebar/lock/edit_form_spec.js new file mode 100644 index 00000000000..7abd6997a18 --- /dev/null +++ b/spec/javascripts/sidebar/lock/edit_form_spec.js @@ -0,0 +1,41 @@ +import Vue from 'vue'; +import editForm from '~/sidebar/components/lock/edit_form.vue'; + +describe('EditForm', () => { + let vm1; + let vm2; + + beforeEach(() => { + const Component = Vue.extend(editForm); + const toggleForm = () => { }; + const updateLockedAttribute = () => { }; + + vm1 = new Component({ + propsData: { + isLocked: true, + toggleForm, + updateLockedAttribute, + issuableType: 'issue', + }, + }).$mount(); + + vm2 = new Component({ + propsData: { + isLocked: false, + toggleForm, + updateLockedAttribute, + issuableType: 'merge_request', + }, + }).$mount(); + }); + + it('renders on the appropriate warning text', () => { + expect( + vm1.$el.innerHTML.includes('Unlock this issue?'), + ).toBe(true); + + expect( + vm2.$el.innerHTML.includes('Lock this merge request?'), + ).toBe(true); + }); +}); diff --git a/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js b/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js new file mode 100644 index 00000000000..696fca516bc --- /dev/null +++ b/spec/javascripts/sidebar/lock/lock_issue_sidebar_spec.js @@ -0,0 +1,71 @@ +import Vue from 'vue'; +import lockIssueSidebar from '~/sidebar/components/lock/lock_issue_sidebar.vue'; + +describe('LockIssueSidebar', () => { + let vm1; + let vm2; + + beforeEach(() => { + const Component = Vue.extend(lockIssueSidebar); + + const mediator = { + service: { + update: Promise.resolve(true), + }, + + store: { + isLockDialogOpen: false, + }, + }; + + vm1 = new Component({ + propsData: { + isLocked: true, + isEditable: true, + mediator, + issuableType: 'issue', + }, + }).$mount(); + + vm2 = new Component({ + propsData: { + isLocked: false, + isEditable: false, + mediator, + issuableType: 'merge_request', + }, + }).$mount(); + }); + + it('shows if locked and/or editable', () => { + expect( + vm1.$el.innerHTML.includes('Edit'), + ).toBe(true); + + expect( + vm1.$el.innerHTML.includes('Locked'), + ).toBe(true); + + expect( + vm2.$el.innerHTML.includes('Unlocked'), + ).toBe(true); + }); + + it('displays the edit form when editable', (done) => { + expect(vm1.isLockDialogOpen).toBe(false); + + vm1.$el.querySelector('.lock-edit').click(); + + expect(vm1.isLockDialogOpen).toBe(true); + + vm1.$nextTick(() => { + expect( + vm1.$el + .innerHTML + .includes('Unlock this issue?'), + ).toBe(true); + + done(); + }); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js b/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js index b63633c03b8..e667b4b3677 100644 --- a/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js +++ b/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js @@ -31,6 +31,7 @@ describe('MRWidgetService', () => { }); it('should have methods defined', () => { + window.history.pushState({}, null, '/'); const service = new MRWidgetService(mr); expect(service.merge()).toBeDefined(); diff --git a/spec/javascripts/vue_shared/components/issue/confidential_issue_warning_spec.js b/spec/javascripts/vue_shared/components/issue/confidential_issue_warning_spec.js deleted file mode 100644 index 6df08f3ebe7..00000000000 --- a/spec/javascripts/vue_shared/components/issue/confidential_issue_warning_spec.js +++ /dev/null @@ -1,20 +0,0 @@ -import Vue from 'vue'; -import confidentialIssue from '~/vue_shared/components/issue/confidential_issue_warning.vue'; - -describe('Confidential Issue Warning Component', () => { - let vm; - - beforeEach(() => { - const Component = Vue.extend(confidentialIssue); - vm = new Component().$mount(); - }); - - afterEach(() => { - vm.$destroy(); - }); - - it('should render confidential issue warning information', () => { - expect(vm.$el.querySelector('i').className).toEqual('fa fa-eye-slash'); - expect(vm.$el.querySelector('span').textContent.trim()).toEqual('This is a confidential issue. Your comment will not be visible to the public.'); - }); -}); diff --git a/spec/javascripts/vue_shared/components/issue/issue_warning_spec.js b/spec/javascripts/vue_shared/components/issue/issue_warning_spec.js new file mode 100644 index 00000000000..2cf4d8e00ed --- /dev/null +++ b/spec/javascripts/vue_shared/components/issue/issue_warning_spec.js @@ -0,0 +1,46 @@ +import Vue from 'vue'; +import issueWarning from '~/vue_shared/components/issue/issue_warning.vue'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +const IssueWarning = Vue.extend(issueWarning); + +function formatWarning(string) { + // Replace newlines with a space then replace multiple spaces with one space + return string.trim().replace(/\n/g, ' ').replace(/\s\s+/g, ' '); +} + +describe('Issue Warning Component', () => { + describe('isLocked', () => { + it('should render locked issue warning information', () => { + const vm = mountComponent(IssueWarning, { + isLocked: true, + }); + + expect(vm.$el.querySelector('i').className).toEqual('fa icon fa-lock'); + expect(formatWarning(vm.$el.querySelector('span').textContent)).toEqual('This issue is locked. Only project members can comment.'); + }); + }); + + describe('isConfidential', () => { + it('should render confidential issue warning information', () => { + const vm = mountComponent(IssueWarning, { + isConfidential: true, + }); + + expect(vm.$el.querySelector('i').className).toEqual('fa icon fa-eye-slash'); + expect(formatWarning(vm.$el.querySelector('span').textContent)).toEqual('This is a confidential issue. Your comment will not be visible to the public.'); + }); + }); + + describe('isLocked and isConfidential', () => { + it('should render locked and confidential issue warning information', () => { + const vm = mountComponent(IssueWarning, { + isLocked: true, + isConfidential: true, + }); + + expect(vm.$el.querySelector('i')).toBeFalsy(); + expect(formatWarning(vm.$el.querySelector('span').textContent)).toEqual('This issue is confidential and locked. People without permission will never get a notification and won\'t be able to comment.'); + }); + }); +}); diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 3fb8edeb701..ec425fd2803 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -147,6 +147,10 @@ deploy_keys: - user - deploy_keys_projects - projects +cluster: +- project +- user +- service services: - project - service_hook @@ -177,6 +181,7 @@ project: - tag_taggings - tags - chat_services +- cluster - creator - group - namespace diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 48938346577..80d92b2e6a3 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -25,6 +25,7 @@ Issue: - relative_position - last_edited_at - last_edited_by_id +- discussion_locked Event: - id - target_type @@ -168,6 +169,7 @@ MergeRequest: - last_edited_at - last_edited_by_id - head_pipeline_id +- discussion_locked MergeRequestDiff: - id - state @@ -311,6 +313,32 @@ Ci::PipelineSchedule: - deleted_at - created_at - updated_at +Gcp::Cluster: +- id +- project_id +- user_id +- service_id +- enabled +- status +- status_reason +- project_namespace +- endpoint +- ca_cert +- encrypted_kubernetes_token +- encrypted_kubernetes_token_iv +- username +- encrypted_password +- encrypted_password_iv +- gcp_project_id +- gcp_cluster_zone +- gcp_cluster_name +- gcp_cluster_size +- gcp_machine_type +- gcp_operation_id +- encrypted_gcp_token +- encrypted_gcp_token_iv +- created_at +- updated_at DeployKey: - id - user_id diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index ee152872acc..777e9c8e21d 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -60,6 +60,7 @@ describe Gitlab::UsageData do deploy_keys deployments environments + gcp_clusters in_review_folder groups issues diff --git a/spec/lib/google_api/auth_spec.rb b/spec/lib/google_api/auth_spec.rb new file mode 100644 index 00000000000..87a3f43274f --- /dev/null +++ b/spec/lib/google_api/auth_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe GoogleApi::Auth do + let(:redirect_uri) { 'http://localhost:3000/google_api/authorizations/callback' } + let(:redirect_to) { 'http://localhost:3000/namaspace/project/clusters' } + + let(:client) do + GoogleApi::CloudPlatform::Client + .new(nil, redirect_uri, state: redirect_to) + end + + describe '#authorize_url' do + subject { client.authorize_url } + + it 'returns authorize_url' do + is_expected.to start_with('https://accounts.google.com/o/oauth2') + is_expected.to include(URI.encode(redirect_uri, URI::PATTERN::RESERVED)) + is_expected.to include(URI.encode(redirect_to, URI::PATTERN::RESERVED)) + end + end + + describe '#get_token' do + let(:token) do + double.tap do |dbl| + allow(dbl).to receive(:token).and_return('token') + allow(dbl).to receive(:expires_at).and_return('expires_at') + end + end + + before do + allow_any_instance_of(OAuth2::Strategy::AuthCode) + .to receive(:get_token).and_return(token) + end + + it 'returns token and expires_at' do + token, expires_at = client.get_token('xxx') + expect(token).to eq('token') + expect(expires_at).to eq('expires_at') + end + end +end diff --git a/spec/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb new file mode 100644 index 00000000000..acc5bd1da35 --- /dev/null +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -0,0 +1,128 @@ +require 'spec_helper' + +describe GoogleApi::CloudPlatform::Client do + let(:token) { 'token' } + let(:client) { described_class.new(token, nil) } + + describe '.session_key_for_redirect_uri' do + let(:state) { 'random_string' } + + subject { described_class.session_key_for_redirect_uri(state) } + + it 'creates a new session key' do + is_expected.to eq('cloud_platform_second_redirect_uri_random_string') + end + end + + describe '.new_session_key_for_redirect_uri' do + it 'generates a new session key' do + expect { |b| described_class.new_session_key_for_redirect_uri(&b) } + .to yield_with_args(String) + end + end + + describe '#validate_token' do + subject { client.validate_token(expires_at) } + + let(:expires_at) { 1.hour.since.utc.strftime('%s') } + + context 'when token is nil' do + let(:token) { nil } + + it { is_expected.to be_falsy } + end + + context 'when expires_at is nil' do + let(:expires_at) { nil } + + it { is_expected.to be_falsy } + end + + context 'when expires in 1 hour' do + it { is_expected.to be_truthy } + end + + context 'when expires in 10 minutes' do + let(:expires_at) { 5.minutes.since.utc.strftime('%s') } + + it { is_expected.to be_falsy } + end + end + + describe '#projects_zones_clusters_get' do + subject { client.projects_zones_clusters_get(spy, spy, spy) } + let(:gke_cluster) { double } + + before do + allow_any_instance_of(Google::Apis::ContainerV1::ContainerService) + .to receive(:get_zone_cluster).and_return(gke_cluster) + end + + it { is_expected.to eq(gke_cluster) } + end + + describe '#projects_zones_clusters_create' do + subject do + client.projects_zones_clusters_create( + spy, spy, cluster_name, cluster_size, machine_type: machine_type) + end + + let(:cluster_name) { 'test-cluster' } + let(:cluster_size) { 1 } + let(:machine_type) { 'n1-standard-4' } + let(:operation) { double } + + before do + allow_any_instance_of(Google::Apis::ContainerV1::ContainerService) + .to receive(:create_cluster).and_return(operation) + end + + it { is_expected.to eq(operation) } + + it 'sets corresponded parameters' do + expect_any_instance_of(Google::Apis::ContainerV1::CreateClusterRequest) + .to receive(:initialize).with( + { + "cluster": { + "name": cluster_name, + "initial_node_count": cluster_size, + "node_config": { + "machine_type": machine_type + } + } + } ) + + subject + end + end + + describe '#projects_zones_operations' do + subject { client.projects_zones_operations(spy, spy, spy) } + let(:operation) { double } + + before do + allow_any_instance_of(Google::Apis::ContainerV1::ContainerService) + .to receive(:get_zone_operation).and_return(operation) + end + + it { is_expected.to eq(operation) } + end + + describe '#parse_operation_id' do + subject { client.parse_operation_id(self_link) } + + context 'when expected url' do + let(:self_link) do + 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123' + end + + it { is_expected.to eq('ope-123') } + end + + context 'when unexpected url' do + let(:self_link) { '???' } + + it { is_expected.to be_nil } + end + end +end diff --git a/spec/lib/rspec_flaky/listener_spec.rb b/spec/lib/rspec_flaky/listener_spec.rb index 7590ea9576d..bfb7648b486 100644 --- a/spec/lib/rspec_flaky/listener_spec.rb +++ b/spec/lib/rspec_flaky/listener_spec.rb @@ -45,6 +45,7 @@ describe RspecFlaky::Listener, :aggregate_failures do # Stub these env variables otherwise specs don't behave the same on the CI stub_env('CI_PROJECT_URL', nil) stub_env('CI_JOB_ID', nil) + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', nil) end describe '#initialize' do diff --git a/spec/models/gcp/cluster_spec.rb b/spec/models/gcp/cluster_spec.rb new file mode 100644 index 00000000000..350fbc257d9 --- /dev/null +++ b/spec/models/gcp/cluster_spec.rb @@ -0,0 +1,240 @@ +require 'spec_helper' + +describe Gcp::Cluster do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:user) } + it { is_expected.to belong_to(:service) } + + it { is_expected.to validate_presence_of(:gcp_cluster_zone) } + + describe '#default_value_for' do + let(:cluster) { described_class.new } + + it { expect(cluster.gcp_cluster_zone).to eq('us-central1-a') } + it { expect(cluster.gcp_cluster_size).to eq(3) } + it { expect(cluster.gcp_machine_type).to eq('n1-standard-4') } + end + + describe '#validates' do + subject { cluster.valid? } + + context 'when validates gcp_project_id' do + let(:cluster) { build(:gcp_cluster, gcp_project_id: gcp_project_id) } + + context 'when valid' do + let(:gcp_project_id) { 'gcp-project-12345' } + + it { is_expected.to be_truthy } + end + + context 'when empty' do + let(:gcp_project_id) { '' } + + it { is_expected.to be_falsey } + end + + context 'when too long' do + let(:gcp_project_id) { 'A' * 64 } + + it { is_expected.to be_falsey } + end + + context 'when includes abnormal character' do + let(:gcp_project_id) { '!!!!!!' } + + it { is_expected.to be_falsey } + end + end + + context 'when validates gcp_cluster_name' do + let(:cluster) { build(:gcp_cluster, gcp_cluster_name: gcp_cluster_name) } + + context 'when valid' do + let(:gcp_cluster_name) { 'test-cluster' } + + it { is_expected.to be_truthy } + end + + context 'when empty' do + let(:gcp_cluster_name) { '' } + + it { is_expected.to be_falsey } + end + + context 'when too long' do + let(:gcp_cluster_name) { 'A' * 64 } + + it { is_expected.to be_falsey } + end + + context 'when includes abnormal character' do + let(:gcp_cluster_name) { '!!!!!!' } + + it { is_expected.to be_falsey } + end + end + + context 'when validates gcp_cluster_size' do + let(:cluster) { build(:gcp_cluster, gcp_cluster_size: gcp_cluster_size) } + + context 'when valid' do + let(:gcp_cluster_size) { 1 } + + it { is_expected.to be_truthy } + end + + context 'when zero' do + let(:gcp_cluster_size) { 0 } + + it { is_expected.to be_falsey } + end + end + + context 'when validates project_namespace' do + let(:cluster) { build(:gcp_cluster, project_namespace: project_namespace) } + + context 'when valid' do + let(:project_namespace) { 'default-namespace' } + + it { is_expected.to be_truthy } + end + + context 'when empty' do + let(:project_namespace) { '' } + + it { is_expected.to be_truthy } + end + + context 'when too long' do + let(:project_namespace) { 'A' * 64 } + + it { is_expected.to be_falsey } + end + + context 'when includes abnormal character' do + let(:project_namespace) { '!!!!!!' } + + it { is_expected.to be_falsey } + end + end + + context 'when validates restrict_modification' do + let(:cluster) { create(:gcp_cluster) } + + before do + cluster.make_creating! + end + + context 'when created' do + before do + cluster.make_created! + end + + it { is_expected.to be_truthy } + end + + context 'when creating' do + it { is_expected.to be_falsey } + end + end + end + + describe '#state_machine' do + let(:cluster) { build(:gcp_cluster) } + + context 'when transits to created state' do + before do + cluster.gcp_token = 'tmp' + cluster.gcp_operation_id = 'tmp' + cluster.make_created! + end + + it 'nullify gcp_token and gcp_operation_id' do + expect(cluster.gcp_token).to be_nil + expect(cluster.gcp_operation_id).to be_nil + expect(cluster).to be_created + end + end + + context 'when transits to errored state' do + let(:reason) { 'something wrong' } + + before do + cluster.make_errored!(reason) + end + + it 'sets status_reason' do + expect(cluster.status_reason).to eq(reason) + expect(cluster).to be_errored + end + end + end + + describe '#project_namespace_placeholder' do + subject { cluster.project_namespace_placeholder } + + let(:cluster) { create(:gcp_cluster) } + + it 'returns a placeholder' do + is_expected.to eq("#{cluster.project.path}-#{cluster.project.id}") + end + end + + describe '#on_creation?' do + subject { cluster.on_creation? } + + let(:cluster) { create(:gcp_cluster) } + + context 'when status is creating' do + before do + cluster.make_creating! + end + + it { is_expected.to be_truthy } + end + + context 'when status is created' do + before do + cluster.make_created! + end + + it { is_expected.to be_falsey } + end + end + + describe '#api_url' do + subject { cluster.api_url } + + let(:cluster) { create(:gcp_cluster, :created_on_gke) } + let(:api_url) { 'https://' + cluster.endpoint } + + it { is_expected.to eq(api_url) } + end + + describe '#restrict_modification' do + subject { cluster.restrict_modification } + + let(:cluster) { create(:gcp_cluster) } + + context 'when status is created' do + before do + cluster.make_created! + end + + it { is_expected.to be_truthy } + end + + context 'when status is creating' do + before do + cluster.make_creating! + end + + it { is_expected.to be_falsey } + + it 'sets error' do + is_expected.to be_falsey + expect(cluster.errors).not_to be_empty + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 3d6a79e0649..9b470e79a76 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -76,6 +76,7 @@ describe Project do it { is_expected.to have_many(:uploads).dependent(:destroy) } it { is_expected.to have_many(:pipeline_schedules) } it { is_expected.to have_many(:members_and_requesters) } + it { is_expected.to have_one(:cluster) } context 'after initialized' do it "has a project_feature" do diff --git a/spec/policies/gcp/cluster_policy_spec.rb b/spec/policies/gcp/cluster_policy_spec.rb new file mode 100644 index 00000000000..e213aa3d557 --- /dev/null +++ b/spec/policies/gcp/cluster_policy_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Gcp::ClusterPolicy, :models do + set(:project) { create(:project) } + set(:cluster) { create(:gcp_cluster, project: project) } + let(:user) { create(:user) } + let(:policy) { described_class.new(user, cluster) } + + describe 'rules' do + context 'when developer' do + before do + project.add_developer(user) + end + + it { expect(policy).to be_disallowed :update_cluster } + it { expect(policy).to be_disallowed :admin_cluster } + end + + context 'when master' do + before do + project.add_master(user) + end + + it { expect(policy).to be_allowed :update_cluster } + it { expect(policy).to be_allowed :admin_cluster } + end + end +end diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb new file mode 100644 index 00000000000..2cf669e8191 --- /dev/null +++ b/spec/policies/issuable_policy_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe IssuablePolicy, models: true do + describe '#rules' do + context 'when discussion is locked for the issuable' do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project, discussion_locked: true) } + let(:policies) { described_class.new(user, issue) } + + context 'when the user is not a project member' do + it 'can not create a note' do + expect(policies).to be_disallowed(:create_note) + end + end + + context 'when the user is a project member' do + before do + project.add_guest(user) + end + + it 'can create a note' do + expect(policies).to be_allowed(:create_note) + end + end + end + end +end diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb new file mode 100644 index 00000000000..58d36a2c84e --- /dev/null +++ b/spec/policies/note_policy_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe NotePolicy, mdoels: true do + describe '#rules' do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + + def policies(noteable = nil) + return @policies if @policies + + noteable ||= issue + note = create(:note, noteable: noteable, author: user, project: project) + + @policies = described_class.new(user, note) + end + + context 'when the project is public' do + context 'when the note author is not a project member' do + it 'can edit a note' do + expect(policies).to be_allowed(:update_note) + expect(policies).to be_allowed(:admin_note) + expect(policies).to be_allowed(:resolve_note) + expect(policies).to be_allowed(:read_note) + end + end + + context 'when the noteable is a snippet' do + it 'can edit note' do + policies = policies(create(:project_snippet, project: project)) + + expect(policies).to be_allowed(:update_note) + expect(policies).to be_allowed(:admin_note) + expect(policies).to be_allowed(:resolve_note) + expect(policies).to be_allowed(:read_note) + end + end + + context 'when a discussion is locked' do + before do + issue.update_attribute(:discussion_locked, true) + end + + context 'when the note author is a project member' do + before do + project.add_developer(user) + end + + it 'can edit a note' do + expect(policies).to be_allowed(:update_note) + expect(policies).to be_allowed(:admin_note) + expect(policies).to be_allowed(:resolve_note) + expect(policies).to be_allowed(:read_note) + end + end + + context 'when the note author is not a project member' do + it 'can not edit a note' do + expect(policies).to be_disallowed(:update_note) + expect(policies).to be_disallowed(:admin_note) + expect(policies).to be_disallowed(:resolve_note) + end + + it 'can read a note' do + expect(policies).to be_allowed(:read_note) + end + end + end + end + end +end diff --git a/spec/presenters/gcp/cluster_presenter_spec.rb b/spec/presenters/gcp/cluster_presenter_spec.rb new file mode 100644 index 00000000000..8d86dc31582 --- /dev/null +++ b/spec/presenters/gcp/cluster_presenter_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Gcp::ClusterPresenter do + let(:project) { create(:project) } + let(:cluster) { create(:gcp_cluster, project: project) } + + subject(:presenter) do + described_class.new(cluster) + end + + it 'inherits from Gitlab::View::Presenter::Delegated' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) + end + + describe '#initialize' do + it 'takes a cluster and optional params' do + expect { presenter }.not_to raise_error + end + + it 'exposes cluster' do + expect(presenter.cluster).to eq(cluster) + end + + it 'forwards missing methods to cluster' do + expect(presenter.gcp_cluster_zone).to eq(cluster.gcp_cluster_zone) + end + end + + describe '#gke_cluster_url' do + subject { described_class.new(cluster).gke_cluster_url } + + it { is_expected.to include(cluster.gcp_cluster_zone) } + it { is_expected.to include(cluster.gcp_cluster_name) } + end +end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index f5882c0c74a..fb440fa551c 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -302,6 +302,40 @@ describe API::Notes do expect(private_issue.notes.reload).to be_empty end end + + context 'when the merge request discussion is locked' do + before do + merge_request.update_attribute(:discussion_locked, true) + end + + context 'when a user is a team member' do + subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user), body: 'Hi!' } + + it 'returns 200 status' do + subject + + expect(response).to have_http_status(201) + end + + it 'creates a new note' do + expect { subject }.to change { Note.count }.by(1) + end + end + + context 'when a user is not a team member' do + subject { post api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", private_user), body: 'Hi!' } + + it 'returns 403 status' do + subject + + expect(response).to have_http_status(403) + end + + it 'does not create a new note' do + expect { subject }.not_to change { Note.count } + end + end + end end describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do diff --git a/spec/serializers/cluster_entity_spec.rb b/spec/serializers/cluster_entity_spec.rb new file mode 100644 index 00000000000..2c7f49974f1 --- /dev/null +++ b/spec/serializers/cluster_entity_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe ClusterEntity do + set(:cluster) { create(:gcp_cluster, :errored) } + let(:request) { double('request') } + + let(:entity) do + described_class.new(cluster) + end + + describe '#as_json' do + subject { entity.as_json } + + it 'contains status' do + expect(subject[:status]).to eq(:errored) + end + + it 'contains status reason' do + expect(subject[:status_reason]).to eq('general error') + end + end +end diff --git a/spec/serializers/cluster_serializer_spec.rb b/spec/serializers/cluster_serializer_spec.rb new file mode 100644 index 00000000000..1ac6784d28f --- /dev/null +++ b/spec/serializers/cluster_serializer_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe ClusterSerializer do + let(:serializer) do + described_class.new + end + + describe '#represent_status' do + subject { serializer.represent_status(resource) } + + context 'when represents only status' do + let(:resource) { create(:gcp_cluster, :errored) } + + it 'serializes only status' do + expect(subject.keys).to contain_exactly(:status, :status_reason) + end + end + end +end diff --git a/spec/services/ci/create_cluster_service_spec.rb b/spec/services/ci/create_cluster_service_spec.rb new file mode 100644 index 00000000000..6e7398fbffa --- /dev/null +++ b/spec/services/ci/create_cluster_service_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Ci::CreateClusterService do + describe '#execute' do + let(:access_token) { 'xxx' } + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:result) { described_class.new(project, user, params).execute(access_token) } + + context 'when correct params' do + let(:params) do + { + gcp_project_id: 'gcp-project', + gcp_cluster_name: 'test-cluster', + gcp_cluster_zone: 'us-central1-a', + gcp_cluster_size: 1 + } + end + + it 'creates a cluster object' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { result }.to change { Gcp::Cluster.count }.by(1) + expect(result.gcp_project_id).to eq('gcp-project') + expect(result.gcp_cluster_name).to eq('test-cluster') + expect(result.gcp_cluster_zone).to eq('us-central1-a') + expect(result.gcp_cluster_size).to eq(1) + expect(result.gcp_token).to eq(access_token) + end + end + + context 'when invalid params' do + let(:params) do + { + gcp_project_id: 'gcp-project', + gcp_cluster_name: 'test-cluster', + gcp_cluster_zone: 'us-central1-a', + gcp_cluster_size: 'ABC' + } + end + + it 'returns an error' do + expect(ClusterProvisionWorker).not_to receive(:perform_async) + expect { result }.to change { Gcp::Cluster.count }.by(0) + end + end + end +end diff --git a/spec/services/ci/fetch_gcp_operation_service_spec.rb b/spec/services/ci/fetch_gcp_operation_service_spec.rb new file mode 100644 index 00000000000..7792979c5cb --- /dev/null +++ b/spec/services/ci/fetch_gcp_operation_service_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' +require 'google/apis' + +describe Ci::FetchGcpOperationService do + describe '#execute' do + let(:cluster) { create(:gcp_cluster) } + let(:operation) { double } + + context 'when suceeded' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_operations).and_return(operation) + end + + it 'fetch the gcp operaion' do + expect { |b| described_class.new.execute(cluster, &b) } + .to yield_with_args(operation) + end + end + + context 'when raises an error' do + let(:error) { Google::Apis::ServerError.new('a') } + + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_operations).and_raise(error) + end + + it 'sets an error to cluster object' do + expect { |b| described_class.new.execute(cluster, &b) } + .not_to yield_with_args + expect(cluster.reload).to be_errored + end + end + end +end diff --git a/spec/services/ci/fetch_kubernetes_token_service_spec.rb b/spec/services/ci/fetch_kubernetes_token_service_spec.rb new file mode 100644 index 00000000000..1d05c9671a9 --- /dev/null +++ b/spec/services/ci/fetch_kubernetes_token_service_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' + +describe Ci::FetchKubernetesTokenService do + describe '#execute' do + subject { described_class.new(api_url, ca_pem, username, password).execute } + + let(:api_url) { 'http://111.111.111.111' } + let(:ca_pem) { '' } + let(:username) { 'admin' } + let(:password) { 'xxx' } + + context 'when params correct' do + let(:token) { 'xxx.token.xxx' } + + let(:secrets_json) do + [ + { + 'metadata': { + name: metadata_name + }, + 'data': { + 'token': Base64.encode64(token) + } + } + ] + end + + before do + allow_any_instance_of(Kubeclient::Client) + .to receive(:get_secrets).and_return(secrets_json) + end + + context 'when default-token exists' do + let(:metadata_name) { 'default-token-123' } + + it { is_expected.to eq(token) } + end + + context 'when default-token does not exist' do + let(:metadata_name) { 'another-token-123' } + + it { is_expected.to be_nil } + end + end + + context 'when api_url is nil' do + let(:api_url) { nil } + + it { expect { subject }.to raise_error("Incomplete settings") } + end + + context 'when username is nil' do + let(:username) { nil } + + it { expect { subject }.to raise_error("Incomplete settings") } + end + + context 'when password is nil' do + let(:password) { nil } + + it { expect { subject }.to raise_error("Incomplete settings") } + end + end +end diff --git a/spec/services/ci/finalize_cluster_creation_service_spec.rb b/spec/services/ci/finalize_cluster_creation_service_spec.rb new file mode 100644 index 00000000000..def3709fdb4 --- /dev/null +++ b/spec/services/ci/finalize_cluster_creation_service_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe Ci::FinalizeClusterCreationService do + describe '#execute' do + let(:cluster) { create(:gcp_cluster) } + let(:result) { described_class.new.execute(cluster) } + + context 'when suceeded to get cluster from api' do + let(:gke_cluster) { double } + + before do + allow(gke_cluster).to receive(:endpoint).and_return('111.111.111.111') + allow(gke_cluster).to receive(:master_auth).and_return(spy) + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_get).and_return(gke_cluster) + end + + context 'when suceeded to get kubernetes token' do + let(:kubernetes_token) { 'abc' } + + before do + allow_any_instance_of(Ci::FetchKubernetesTokenService) + .to receive(:execute).and_return(kubernetes_token) + end + + it 'executes integration cluster' do + expect_any_instance_of(Ci::IntegrateClusterService).to receive(:execute) + described_class.new.execute(cluster) + end + end + + context 'when failed to get kubernetes token' do + before do + allow_any_instance_of(Ci::FetchKubernetesTokenService) + .to receive(:execute).and_return(nil) + end + + it 'sets an error to cluster object' do + described_class.new.execute(cluster) + + expect(cluster.reload).to be_errored + end + end + end + + context 'when failed to get cluster from api' do + let(:error) { Google::Apis::ServerError.new('a') } + + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_get).and_raise(error) + end + + it 'sets an error to cluster object' do + described_class.new.execute(cluster) + + expect(cluster.reload).to be_errored + end + end + end +end diff --git a/spec/services/ci/integrate_cluster_service_spec.rb b/spec/services/ci/integrate_cluster_service_spec.rb new file mode 100644 index 00000000000..3a79c205bd1 --- /dev/null +++ b/spec/services/ci/integrate_cluster_service_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Ci::IntegrateClusterService do + describe '#execute' do + let(:cluster) { create(:gcp_cluster, :custom_project_namespace) } + let(:endpoint) { '123.123.123.123' } + let(:ca_cert) { 'ca_cert_xxx' } + let(:token) { 'token_xxx' } + let(:username) { 'username_xxx' } + let(:password) { 'password_xxx' } + + before do + described_class + .new.execute(cluster, endpoint, ca_cert, token, username, password) + + cluster.reload + end + + context 'when correct params' do + it 'creates a cluster object' do + expect(cluster.endpoint).to eq(endpoint) + expect(cluster.ca_cert).to eq(ca_cert) + expect(cluster.kubernetes_token).to eq(token) + expect(cluster.username).to eq(username) + expect(cluster.password).to eq(password) + expect(cluster.service.active).to be_truthy + expect(cluster.service.api_url).to eq(cluster.api_url) + expect(cluster.service.ca_pem).to eq(ca_cert) + expect(cluster.service.namespace).to eq(cluster.project_namespace) + expect(cluster.service.token).to eq(token) + end + end + + context 'when invalid params' do + let(:endpoint) { nil } + + it 'sets an error to cluster object' do + expect(cluster).to be_errored + end + end + end +end diff --git a/spec/services/ci/provision_cluster_service_spec.rb b/spec/services/ci/provision_cluster_service_spec.rb new file mode 100644 index 00000000000..5ce5c788314 --- /dev/null +++ b/spec/services/ci/provision_cluster_service_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +describe Ci::ProvisionClusterService do + describe '#execute' do + let(:cluster) { create(:gcp_cluster) } + let(:operation) { spy } + + shared_examples 'error' do + it 'sets an error to cluster object' do + described_class.new.execute(cluster) + + expect(cluster.reload).to be_errored + end + end + + context 'when suceeded to request provision' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_create).and_return(operation) + end + + context 'when operation status is RUNNING' do + before do + allow(operation).to receive(:status).and_return('RUNNING') + end + + context 'when suceeded to parse gcp operation id' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:parse_operation_id).and_return('operation-123') + end + + context 'when cluster status is scheduled' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:parse_operation_id).and_return('operation-123') + end + + it 'schedules a worker for status minitoring' do + expect(WaitForClusterCreationWorker).to receive(:perform_in) + + described_class.new.execute(cluster) + end + end + + context 'when cluster status is creating' do + before do + cluster.make_creating! + end + + it_behaves_like 'error' + end + end + + context 'when failed to parse gcp operation id' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:parse_operation_id).and_return(nil) + end + + it_behaves_like 'error' + end + end + + context 'when operation status is others' do + before do + allow(operation).to receive(:status).and_return('others') + end + + it_behaves_like 'error' + end + end + + context 'when failed to request provision' do + let(:error) { Google::Apis::ServerError.new('a') } + + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_create).and_raise(error) + end + + it_behaves_like 'error' + end + end +end diff --git a/spec/services/ci/update_cluster_service_spec.rb b/spec/services/ci/update_cluster_service_spec.rb new file mode 100644 index 00000000000..a289385b88f --- /dev/null +++ b/spec/services/ci/update_cluster_service_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Ci::UpdateClusterService do + describe '#execute' do + let(:cluster) { create(:gcp_cluster, :created_on_gke, :with_kubernetes_service) } + + before do + described_class.new(cluster.project, cluster.user, params).execute(cluster) + + cluster.reload + end + + context 'when correct params' do + context 'when enabled is true' do + let(:params) { { 'enabled' => 'true' } } + + it 'enables cluster and overwrite kubernetes service' do + expect(cluster.enabled).to be_truthy + expect(cluster.service.active).to be_truthy + expect(cluster.service.api_url).to eq(cluster.api_url) + expect(cluster.service.ca_pem).to eq(cluster.ca_cert) + expect(cluster.service.namespace).to eq(cluster.project_namespace) + expect(cluster.service.token).to eq(cluster.kubernetes_token) + end + end + + context 'when enabled is false' do + let(:params) { { 'enabled' => 'false' } } + + it 'disables cluster and kubernetes service' do + expect(cluster.enabled).to be_falsy + expect(cluster.service.active).to be_falsy + end + end + end + end +end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index a8a8aeed1bd..bdbe0a353fb 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -48,7 +48,8 @@ describe Issues::UpdateService, :mailer do assignee_ids: [user2.id], state_event: 'close', label_ids: [label.id], - due_date: Date.tomorrow + due_date: Date.tomorrow, + discussion_locked: true } end @@ -62,6 +63,7 @@ describe Issues::UpdateService, :mailer do expect(issue).to be_closed expect(issue.labels).to match_array [label] expect(issue.due_date).to eq Date.tomorrow + expect(issue.discussion_locked).to be_truthy end it 'refreshes the number of open issues when the issue is made confidential', :use_clean_rails_memory_store_caching do @@ -110,6 +112,7 @@ describe Issues::UpdateService, :mailer do expect(issue.labels).to be_empty expect(issue.milestone).to be_nil expect(issue.due_date).to be_nil + expect(issue.discussion_locked).to be_falsey end end @@ -148,6 +151,13 @@ describe Issues::UpdateService, :mailer do expect(note).not_to be_nil expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**' end + + it 'creates system note about discussion lock' do + note = find_note('locked this issue') + + expect(note).not_to be_nil + expect(note.note).to eq 'locked this issue' + end end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 681feee61d1..b11a1b31f32 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -49,7 +49,8 @@ describe MergeRequests::UpdateService, :mailer do state_event: 'close', label_ids: [label.id], target_branch: 'target', - force_remove_source_branch: '1' + force_remove_source_branch: '1', + discussion_locked: true } end @@ -73,6 +74,7 @@ describe MergeRequests::UpdateService, :mailer do expect(@merge_request.labels.first.title).to eq(label.name) expect(@merge_request.target_branch).to eq('target') expect(@merge_request.merge_params['force_remove_source_branch']).to eq('1') + expect(@merge_request.discussion_locked).to be_truthy end it 'executes hooks with update action' do @@ -123,6 +125,13 @@ describe MergeRequests::UpdateService, :mailer do expect(note.note).to eq 'changed target branch from `master` to `target`' end + it 'creates system note about discussion lock' do + note = find_note('locked this issue') + + expect(note).not_to be_nil + expect(note.note).to eq 'locked this issue' + end + context 'when not including source branch removal options' do before do opts.delete(:force_remove_source_branch) diff --git a/spec/support/helpers/merge_request_diff_helpers.rb b/spec/support/helpers/merge_request_diff_helpers.rb new file mode 100644 index 00000000000..fd22e384b1b --- /dev/null +++ b/spec/support/helpers/merge_request_diff_helpers.rb @@ -0,0 +1,28 @@ +module MergeRequestDiffHelpers + def click_diff_line(line_holder, diff_side = nil) + line = get_line_components(line_holder, diff_side) + line[:content].hover + line[:num].find('.add-diff-note').trigger('click') + end + + def get_line_components(line_holder, diff_side = nil) + if diff_side.nil? + get_inline_line_components(line_holder) + else + get_parallel_line_components(line_holder, diff_side) + end + end + + def get_inline_line_components(line_holder) + { content: line_holder.find('.line_content', match: :first), num: line_holder.find('.diff-line-num', match: :first) } + end + + def get_parallel_line_components(line_holder, diff_side = nil) + side_index = diff_side == 'left' ? 0 : 1 + # Wait for `.line_content` + line_holder.find('.line_content', match: :first) + # Wait for `.diff-line-num` + line_holder.find('.diff-line-num', match: :first) + { content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] } + end +end diff --git a/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb b/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb new file mode 100644 index 00000000000..221926aaf7e --- /dev/null +++ b/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb @@ -0,0 +1,28 @@ +shared_examples 'comment on merge request file' do + it 'adds a comment' do + click_diff_line(find("[id='#{sample_commit.line_code}']")) + + page.within('.js-discussion-note-form') do + fill_in(:note_note, with: 'Line is wrong') + click_button('Comment') + end + + wait_for_requests + + page.within('.notes_holder') do + expect(page).to have_content('Line is wrong') + end + + visit(merge_request_path(merge_request)) + + page.within('.notes .discussion') do + expect(page).to have_content("#{user.name} #{user.to_reference} started a discussion") + expect(page).to have_content(sample_commit.line_code_path) + expect(page).to have_content('Line is wrong') + end + + page.within('.notes-tab .badge') do + expect(page).to have_content('1') + end + end +end diff --git a/spec/workers/cluster_provision_worker_spec.rb b/spec/workers/cluster_provision_worker_spec.rb new file mode 100644 index 00000000000..11f208289db --- /dev/null +++ b/spec/workers/cluster_provision_worker_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe ClusterProvisionWorker do + describe '#perform' do + context 'when cluster exists' do + let(:cluster) { create(:gcp_cluster) } + + it 'provision a cluster' do + expect_any_instance_of(Ci::ProvisionClusterService).to receive(:execute) + + described_class.new.perform(cluster.id) + end + end + + context 'when cluster does not exist' do + it 'does not provision a cluster' do + expect_any_instance_of(Ci::ProvisionClusterService).not_to receive(:execute) + + described_class.new.perform(123) + end + end + end +end diff --git a/spec/workers/concerns/cluster_queue_spec.rb b/spec/workers/concerns/cluster_queue_spec.rb new file mode 100644 index 00000000000..1050651fa51 --- /dev/null +++ b/spec/workers/concerns/cluster_queue_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe ClusterQueue do + let(:worker) do + Class.new do + include Sidekiq::Worker + include ClusterQueue + end + end + + it 'sets a default pipelines queue automatically' do + expect(worker.sidekiq_options['queue']) + .to eq :gcp_cluster + end +end diff --git a/spec/workers/wait_for_cluster_creation_worker_spec.rb b/spec/workers/wait_for_cluster_creation_worker_spec.rb new file mode 100644 index 00000000000..dcd4a3b9aec --- /dev/null +++ b/spec/workers/wait_for_cluster_creation_worker_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe WaitForClusterCreationWorker do + describe '#perform' do + context 'when cluster exists' do + let(:cluster) { create(:gcp_cluster) } + let(:operation) { double } + + before do + allow(operation).to receive(:status).and_return(status) + allow(operation).to receive(:start_time).and_return(1.minute.ago) + allow(operation).to receive(:status_message).and_return('error') + allow_any_instance_of(Ci::FetchGcpOperationService).to receive(:execute).and_yield(operation) + end + + context 'when operation status is RUNNING' do + let(:status) { 'RUNNING' } + + it 'reschedules worker' do + expect(described_class).to receive(:perform_in) + + described_class.new.perform(cluster.id) + end + + context 'when operation timeout' do + before do + allow(operation).to receive(:start_time).and_return(30.minutes.ago.utc) + end + + it 'sets an error message on cluster' do + described_class.new.perform(cluster.id) + + expect(cluster.reload).to be_errored + end + end + end + + context 'when operation status is DONE' do + let(:status) { 'DONE' } + + it 'finalizes cluster creation' do + expect_any_instance_of(Ci::FinalizeClusterCreationService).to receive(:execute) + + described_class.new.perform(cluster.id) + end + end + + context 'when operation status is others' do + let(:status) { 'others' } + + it 'sets an error message on cluster' do + described_class.new.perform(cluster.id) + + expect(cluster.reload).to be_errored + end + end + end + + context 'when cluster does not exist' do + it 'does not provision a cluster' do + expect_any_instance_of(Ci::FetchGcpOperationService).not_to receive(:execute) + + described_class.new.perform(1234) + end + end + end +end |