diff options
76 files changed, 1264 insertions, 326 deletions
diff --git a/app/assets/javascripts/autosave.js b/app/assets/javascripts/autosave.js index 4d2d4db7c0e..0f28bd233ac 100644 --- a/app/assets/javascripts/autosave.js +++ b/app/assets/javascripts/autosave.js @@ -1,8 +1,9 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-param-reassign, quotes, prefer-template, no-var, one-var, no-unused-vars, one-var-declaration-per-line, no-void, consistent-return, no-empty, max-len */ +/* eslint-disable no-param-reassign, prefer-template, no-var, no-void, consistent-return */ + import AccessorUtilities from './lib/utils/accessor'; -window.Autosave = (function() { - function Autosave(field, key, resource) { +export default class Autosave { + constructor(field, key, resource) { this.field = field; this.isLocalStorageAvailable = AccessorUtilities.isLocalStorageAccessSafe(); this.resource = resource; @@ -12,14 +13,10 @@ window.Autosave = (function() { this.key = 'autosave/' + key; this.field.data('autosave', this); this.restore(); - this.field.on('input', (function(_this) { - return function() { - return _this.save(); - }; - })(this)); + this.field.on('input', () => this.save()); } - Autosave.prototype.restore = function() { + restore() { var text; if (!this.isLocalStorageAvailable) return; @@ -40,9 +37,9 @@ window.Autosave = (function() { field.dispatchEvent(event); } } - }; + } - Autosave.prototype.save = function() { + save() { var text; text = this.field.val(); @@ -51,15 +48,11 @@ window.Autosave = (function() { } return this.reset(); - }; + } - Autosave.prototype.reset = function() { + reset() { if (!this.isLocalStorageAvailable) return; return window.localStorage.removeItem(this.key); - }; - - return Autosave; -})(); - -export default window.Autosave; + } +} diff --git a/app/assets/javascripts/boards/filtered_search_boards.js b/app/assets/javascripts/boards/filtered_search_boards.js index 3f083655f95..184665f395c 100644 --- a/app/assets/javascripts/boards/filtered_search_boards.js +++ b/app/assets/javascripts/boards/filtered_search_boards.js @@ -11,7 +11,8 @@ export default class FilteredSearchBoards extends gl.FilteredSearchManager { // Issue boards is slightly different, we handle all the requests async // instead or reloading the page, we just re-fire the list ajax requests this.isHandledAsync = true; - this.cantEdit = cantEdit; + this.cantEdit = cantEdit.filter(i => typeof i === 'string'); + this.cantEditWithValue = cantEdit.filter(i => typeof i === 'object'); } updateObject(path) { @@ -42,7 +43,9 @@ export default class FilteredSearchBoards extends gl.FilteredSearchManager { this.filteredSearchInput.dispatchEvent(new Event('input')); } - canEdit(tokenName) { - return this.cantEdit.indexOf(tokenName) === -1; + canEdit(tokenName, tokenValue) { + if (this.cantEdit.includes(tokenName)) return false; + return this.cantEditWithValue.findIndex(token => token.name === tokenName && + token.value === tokenValue) === -1; } } diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index ea82958e80d..798d7e0d147 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -14,16 +14,18 @@ gl.issueBoards.BoardsStore = { }, state: {}, detail: { - issue: {} + issue: {}, }, moving: { issue: {}, - list: {} + list: {}, }, create () { this.state.lists = []; this.filter.path = getUrlParamsArray().join('&'); - this.detail = { issue: {} }; + this.detail = { + issue: {}, + }; }, addList (listObj, defaultAvatar) { const list = new List(listObj, defaultAvatar); diff --git a/app/assets/javascripts/filtered_search/dropdown_utils.js b/app/assets/javascripts/filtered_search/dropdown_utils.js index 8d711e3213c..cf8a9b0402b 100644 --- a/app/assets/javascripts/filtered_search/dropdown_utils.js +++ b/app/assets/javascripts/filtered_search/dropdown_utils.js @@ -147,6 +147,16 @@ class DropdownUtils { return dataValue !== null; } + static getVisualTokenValues(visualToken) { + const tokenName = visualToken && visualToken.querySelector('.name').textContent.trim(); + let tokenValue = visualToken && visualToken.querySelector('.value') && visualToken.querySelector('.value').textContent.trim(); + if (tokenName === 'label' && tokenValue) { + // remove leading symbol and wrapping quotes + tokenValue = tokenValue.replace(/^~("|')?(.*)/, '$2').replace(/("|')$/, ''); + } + return { tokenName, tokenValue }; + } + // Determines the full search query (visual tokens + input) static getSearchQuery(untilInput = false) { const container = FilteredSearchContainer.container; diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index 7b233842d5a..69c57f923b6 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -185,8 +185,8 @@ class FilteredSearchManager { if (e.keyCode === 8 || e.keyCode === 46) { const { lastVisualToken } = gl.FilteredSearchVisualTokens.getLastVisualTokenBeforeInput(); - const sanitizedTokenName = lastVisualToken && lastVisualToken.querySelector('.name').textContent.trim(); - const canEdit = sanitizedTokenName && this.canEdit && this.canEdit(sanitizedTokenName); + const { tokenName, tokenValue } = gl.DropdownUtils.getVisualTokenValues(lastVisualToken); + const canEdit = tokenName && this.canEdit && this.canEdit(tokenName, tokenValue); if (this.filteredSearchInput.value === '' && lastVisualToken && canEdit) { this.filteredSearchInput.value = gl.FilteredSearchVisualTokens.getLastTokenPartial(); gl.FilteredSearchVisualTokens.removeLastTokenPartial(); @@ -336,8 +336,8 @@ class FilteredSearchManager { let canClearToken = t.classList.contains('js-visual-token'); if (canClearToken) { - const tokenKey = t.querySelector('.name').textContent.trim(); - canClearToken = this.canEdit && this.canEdit(tokenKey); + const { tokenName, tokenValue } = gl.DropdownUtils.getVisualTokenValues(t); + canClearToken = this.canEdit && this.canEdit(tokenName, tokenValue); } if (canClearToken) { @@ -469,7 +469,7 @@ class FilteredSearchManager { } hasFilteredSearch = true; - const canEdit = this.canEdit && this.canEdit(sanitizedKey); + const canEdit = this.canEdit && this.canEdit(sanitizedKey, sanitizedValue); gl.FilteredSearchVisualTokens.addFilterVisualToken( sanitizedKey, `${symbol}${quotationsToUse}${sanitizedValue}${quotationsToUse}`, diff --git a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js index d2f92929b8a..6139e81fe6d 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -38,21 +38,14 @@ class FilteredSearchVisualTokens { } static createVisualTokenElementHTML(canEdit = true) { - let removeTokenMarkup = ''; - if (canEdit) { - removeTokenMarkup = ` - <div class="remove-token" role="button"> - <i class="fa fa-close"></i> - </div> - `; - } - return ` - <div class="selectable" role="button"> + <div class="${canEdit ? 'selectable' : 'hidden'}" role="button"> <div class="name"></div> <div class="value-container"> <div class="value"></div> - ${removeTokenMarkup} + <div class="remove-token" role="button"> + <i class="fa fa-close"></i> + </div> </div> </div> `; diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index cd2562bc6a9..26dd946984d 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -1,8 +1,8 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-use-before-define, no-useless-escape, no-new, quotes, object-shorthand, no-unused-vars, comma-dangle, no-alert, consistent-return, no-else-return, prefer-template, one-var, one-var-declaration-per-line, curly, max-len */ /* global GitLab */ -/* global Autosave */ import Pikaday from 'pikaday'; +import Autosave from './autosave'; import UsersSelect from './users_select'; import GfmAutoComplete from './gfm_auto_complete'; import ZenMode from './zen_mode'; diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 84602cf9207..1e52963b1dd 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -8,7 +8,7 @@ import CreateLabelDropdown from './create_label'; (function() { this.LabelsSelect = (function() { - function LabelsSelect(els) { + function LabelsSelect(els, options = {}) { var _this, $els; _this = this; @@ -58,6 +58,7 @@ import CreateLabelDropdown from './create_label'; labelHTMLTemplate = _.template('<% _.each(labels, function(label){ %> <a href="<%- ["",issueURLSplit[1], issueURLSplit[2],""].join("/") %>issues?label_name[]=<%- encodeURIComponent(label.title) %>"> <span class="label has-tooltip color-label" title="<%- label.description %>" style="background-color: <%- label.color %>; color: <%- label.text_color %>;"> <%- label.title %> </span> </a> <% }); %>'); labelNoneHTMLTemplate = '<span class="no-value">None</span>'; } + const handleClick = options.handleClick; $sidebarLabelTooltip.tooltip(); @@ -316,9 +317,9 @@ import CreateLabelDropdown from './create_label'; }, multiSelect: $dropdown.hasClass('js-multiselect'), vue: $dropdown.hasClass('js-issue-board-sidebar'), - clicked: function(options) { - const { $el, e, isMarking } = options; - const label = options.selectedObj; + clicked: function(clickEvent) { + const { $el, e, isMarking } = clickEvent; + const label = clickEvent.selectedObj; var isIssueIndex, isMRIndex, page, boardsModel; var fadeOutLoader = () => { @@ -391,6 +392,10 @@ import CreateLabelDropdown from './create_label'; .then(fadeOutLoader) .catch(fadeOutLoader); } + else if (handleClick) { + e.preventDefault(); + handleClick(label); + } else { if ($dropdown.hasClass('js-multiselect')) { diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 38403fdaf6e..7f045338417 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -41,7 +41,6 @@ import './behaviors/'; import './activities'; import './admin'; import './aside'; -import './autosave'; import loadAwardsHandler from './awards_handler'; import bp from './breakpoints'; import './commits'; diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index e7d5325a509..74e5a4f1cea 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -5,7 +5,7 @@ import _ from 'underscore'; (function() { this.MilestoneSelect = (function() { - function MilestoneSelect(currentProject, els) { + function MilestoneSelect(currentProject, els, options = {}) { var _this, $els; if (currentProject != null) { _this = this; @@ -136,19 +136,26 @@ import _ from 'underscore'; }, opened: function(e) { const $el = $(e.currentTarget); - if ($dropdown.hasClass('js-issue-board-sidebar')) { + if ($dropdown.hasClass('js-issue-board-sidebar') || options.handleClick) { selectedMilestone = $dropdown[0].dataset.selected || selectedMilestoneDefault; } $('a.is-active', $el).removeClass('is-active'); $(`[data-milestone-id="${selectedMilestone}"] > a`, $el).addClass('is-active'); }, vue: $dropdown.hasClass('js-issue-board-sidebar'), - clicked: function(options) { - const { $el, e } = options; - let selected = options.selectedObj; + clicked: function(clickEvent) { + const { $el, e } = clickEvent; + let selected = clickEvent.selectedObj; var data, isIssueIndex, isMRIndex, isSelecting, page, boardsStore; if (!selected) return; + + if (options.handleClick) { + e.preventDefault(); + options.handleClick(selected); + return; + } + page = $('body').attr('data-page'); isIssueIndex = page === 'projects:issues:index'; isMRIndex = (page === page && page === 'projects:merge_requests:index'); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 5a6868be444..ab101a56db8 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -5,7 +5,7 @@ default-case, prefer-template, consistent-return, no-alert, no-return-assign, no-param-reassign, prefer-arrow-callback, no-else-return, comma-dangle, no-new, brace-style, no-lonely-if, vars-on-top, no-unused-vars, no-sequences, no-shadow, newline-per-chained-call, no-useless-escape, class-methods-use-this */ -/* global Autosave */ + /* global ResolveService */ /* global mrRefreshWidgetUrl */ @@ -20,7 +20,7 @@ import Flash from './flash'; import CommentTypeToggle from './comment_type_toggle'; import GLForm from './gl_form'; import loadAwardsHandler from './awards_handler'; -import './autosave'; +import Autosave from './autosave'; import TaskList from './task_list'; import { ajaxPost, isInViewport, getPagePath, scrollToElement, isMetaKey } from './lib/utils/common_utils'; import imageDiffHelper from './image_diff/helpers/index'; diff --git a/app/assets/javascripts/notes/components/issue_comment_form.vue b/app/assets/javascripts/notes/components/issue_comment_form.vue index 2ce52e4538a..ad384a1cc36 100644 --- a/app/assets/javascripts/notes/components/issue_comment_form.vue +++ b/app/assets/javascripts/notes/components/issue_comment_form.vue @@ -1,10 +1,9 @@ <script> - /* global Autosave */ import { mapActions, mapGetters } from 'vuex'; import _ from 'underscore'; import autosize from 'vendor/autosize'; import Flash from '../../flash'; - import '../../autosave'; + import Autosave from '../../autosave'; import TaskList from '../../task_list'; import * as constants from '../constants'; import eventHub from '../event_hub'; diff --git a/app/assets/javascripts/notes/mixins/autosave.js b/app/assets/javascripts/notes/mixins/autosave.js index 5843b97f225..a008171beda 100644 --- a/app/assets/javascripts/notes/mixins/autosave.js +++ b/app/assets/javascripts/notes/mixins/autosave.js @@ -1,5 +1,4 @@ -/* globals Autosave */ -import '../../autosave'; +import Autosave from '../../autosave'; export default { methods: { diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index a0883b32593..759cc9925f4 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -6,7 +6,7 @@ import _ from 'underscore'; // TODO: remove eventHub hack after code splitting refactor window.emitSidebarEvent = window.emitSidebarEvent || $.noop; -function UsersSelect(currentUser, els) { +function UsersSelect(currentUser, els, options = {}) { var $els; this.users = this.users.bind(this); this.user = this.user.bind(this); @@ -20,6 +20,8 @@ function UsersSelect(currentUser, els) { } } + const { handleClick } = options; + $els = $(els); if (!els) { @@ -442,6 +444,9 @@ function UsersSelect(currentUser, els) { } if ($el.closest('.add-issues-modal').length) { gl.issueBoards.ModalStore.store.filter[$dropdown.data('field-name')] = user.id; + } else if (handleClick) { + e.preventDefault(); + handleClick(user, isMarking); } else if ($dropdown.hasClass('js-filter-submit') && (isIssueIndex || isMRIndex)) { return Issuable.filterResults($dropdown.closest('form')); } else if ($dropdown.hasClass('js-filter-submit')) { diff --git a/app/assets/javascripts/vue_shared/components/loading_icon.vue b/app/assets/javascripts/vue_shared/components/loading_icon.vue index 15581d5c2a0..494fe4468d9 100644 --- a/app/assets/javascripts/vue_shared/components/loading_icon.vue +++ b/app/assets/javascripts/vue_shared/components/loading_icon.vue @@ -18,6 +18,12 @@ required: false, default: false, }, + + class: { + type: String, + required: false, + default: '', + }, }, computed: { @@ -25,7 +31,7 @@ return this.inline ? 'span' : 'div'; }, cssClass() { - return `fa-${this.size}x`; + return `fa-${this.size}x ${this.class}`.trim(); }, }, }; diff --git a/app/assets/javascripts/vue_shared/components/popup_dialog.vue b/app/assets/javascripts/vue_shared/components/popup_dialog.vue index 9e8c10bdc1a..fc6421fecb9 100644 --- a/app/assets/javascripts/vue_shared/components/popup_dialog.vue +++ b/app/assets/javascripts/vue_shared/components/popup_dialog.vue @@ -5,17 +5,27 @@ export default { props: { title: { type: String, - required: true, + required: false, }, text: { type: String, required: false, }, + hideFooter: { + type: Boolean, + required: false, + default: false, + }, kind: { type: String, required: false, default: 'primary', }, + modalDialogClass: { + type: String, + required: false, + default: '', + }, closeKind: { type: String, required: false, @@ -30,6 +40,11 @@ export default { type: String, required: true, }, + submitDisabled: { + type: Boolean, + required: false, + default: false, + }, }, computed: { @@ -57,43 +72,57 @@ export default { </script> <template> -<div - class="modal popup-dialog" - role="dialog" - tabindex="-1"> - <div class="modal-dialog" role="document"> - <div class="modal-content"> - <div class="modal-header"> - <button type="button" - class="close" - @click="close" - aria-label="Close"> - <span aria-hidden="true">×</span> - </button> - <h4 class="modal-title">{{this.title}}</h4> - </div> - <div class="modal-body"> - <slot name="body" :text="text"> - <p>{{text}}</p> - </slot> - </div> - <div class="modal-footer"> - <button - type="button" - class="btn" - :class="btnCancelKindClass" - @click="close"> - {{ closeButtonLabel }} - </button> - <button - type="button" - class="btn" - :class="btnKindClass" - @click="emitSubmit(true)"> - {{ primaryButtonLabel }} - </button> +<div class="modal-open"> + <div + class="modal popup-dialog" + role="dialog" + tabindex="-1" + > + <div + :class="modalDialogClass" + class="modal-dialog" + role="document" + > + <div class="modal-content"> + <div class="modal-header"> + <slot name="header"> + <h4 class="modal-title pull-left"> + {{this.title}} + </h4> + <button + type="button" + class="close pull-right" + @click="close" + aria-label="Close" + > + <span aria-hidden="true">×</span> + </button> + </slot> + </div> + <div class="modal-body"> + <slot name="body" :text="text"> + <p>{{this.text}}</p> + </slot> + </div> + <div class="modal-footer" v-if="!hideFooter"> + <button + type="button" + class="btn pull-left" + :class="btnCancelKindClass" + @click="close"> + {{ closeButtonLabel }} + </button> + <button + type="button" + class="btn pull-right" + :class="btnKindClass" + @click="emitSubmit(true)"> + {{ primaryButtonLabel }} + </button> + </div> </div> </div> </div> + <div class="modal-backdrop fade in" /> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue index 95898d54cf7..dc32e783258 100644 --- a/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue +++ b/app/assets/javascripts/vue_shared/components/user_avatar/user_avatar_link.vue @@ -12,12 +12,14 @@ :img-alt="tooltipText" :img-size="20" :tooltip-text="tooltipText" - tooltip-placement="top" + :tooltip-placement="top" + :username="username" /> */ import userAvatarImage from './user_avatar_image.vue'; +import tooltip from '../../directives/tooltip'; export default { name: 'UserAvatarLink', @@ -60,6 +62,22 @@ export default { required: false, default: 'top', }, + username: { + type: String, + required: false, + default: '', + }, + }, + computed: { + shouldShowUsername() { + return this.username.length > 0; + }, + avatarTooltipText() { + return this.shouldShowUsername ? '' : this.tooltipText; + }, + }, + directives: { + tooltip, }, }; </script> @@ -73,8 +91,13 @@ export default { :img-alt="imgAlt" :css-classes="imgCssClasses" :size="imgSize" - :tooltip-text="tooltipText" + :tooltip-text="avatarTooltipText" + :tooltip-placement="tooltipPlacement" + /><span + v-if="shouldShowUsername" + v-tooltip + :title="tooltipText" :tooltip-placement="tooltipPlacement" - /> + >{{username}}</span> </a> </template> diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 96f9dda26c4..1cfd7ef01a8 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -4,6 +4,9 @@ .cred { color: $common-red; } .cgreen { color: $common-green; } .cdark { color: $common-gray-dark; } +.text-secondary { + color: $gl-text-color-secondary; +} /** COMMON CLASSES **/ .prepend-top-0 { margin-top: 0; } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index a9d804e735d..63697fd38a7 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -37,6 +37,7 @@ .dropdown-menu-nav { @include set-visible; display: block; + min-height: 40px; @media (max-width: $screen-xs-max) { width: 100%; diff --git a/app/assets/stylesheets/framework/modal.scss b/app/assets/stylesheets/framework/modal.scss index 1cebd02df48..d218fb6d702 100644 --- a/app/assets/stylesheets/framework/modal.scss +++ b/app/assets/stylesheets/framework/modal.scss @@ -42,3 +42,11 @@ body.modal-open { width: 98%; } } + +.modal.popup-dialog { + display: block; +} + +.modal-body { + background-color: $modal-body-bg; +} diff --git a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss index 3ea77eb7a43..a23131e0818 100644 --- a/app/assets/stylesheets/framework/tw_bootstrap_variables.scss +++ b/app/assets/stylesheets/framework/tw_bootstrap_variables.scss @@ -164,3 +164,36 @@ $pre-border-color: $border-color; $table-bg-accent: $gray-light; $zindex-popover: 900; + +//== Modals +// +//## + +//** Padding applied to the modal body +$modal-inner-padding: $gl-padding; + +//** Padding applied to the modal title +$modal-title-padding: $gl-padding; +//** Modal title line-height +// $modal-title-line-height: $line-height-base + +//** Background color of modal content area +$modal-content-bg: $gray-light; +$modal-body-bg: $white-light; +//** Modal content border color +// $modal-content-border-color: rgba(0,0,0,.2) +//** Modal content border color **for IE8** +// $modal-content-fallback-border-color: #999 + +//** Modal backdrop background color +// $modal-backdrop-bg: #000 +//** Modal backdrop opacity +// $modal-backdrop-opacity: .5 +//** Modal header border color +// $modal-header-border-color: #e5e5e5 +//** Modal footer border color +// $modal-footer-border-color: $modal-header-border-color + +// $modal-lg: 900px +// $modal-md: 600px +// $modal-sm: 300px diff --git a/app/assets/stylesheets/pages/repo.scss b/app/assets/stylesheets/pages/repo.scss index 6a363b1710e..e8c7f8a8fc0 100644 --- a/app/assets/stylesheets/pages/repo.scss +++ b/app/assets/stylesheets/pages/repo.scss @@ -7,19 +7,6 @@ background: $black-transparent; } -.modal.popup-dialog { - display: block; - background-color: $black-transparent; - z-index: 2100; - - @media (min-width: $screen-md-min) { - .modal-dialog { - width: 600px; - margin: 30px auto; - } - } -} - .project-refs-form, .project-refs-target-form { display: inline-block; diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 80ab681ed87..bc0948cd3fb 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -10,7 +10,7 @@ class ConfirmationsController < Devise::ConfirmationsController users_almost_there_path end - def after_confirmation_path_for(_resource_name, resource) + def after_confirmation_path_for(resource_name, resource) # incoming resource can either be a :user or an :email if signed_in?(:user) after_sign_in(resource) diff --git a/app/controllers/projects/branches_controller.rb b/app/controllers/projects/branches_controller.rb index 7f03ce07dec..f28df83d5a5 100644 --- a/app/controllers/projects/branches_controller.rb +++ b/app/controllers/projects/branches_controller.rb @@ -15,6 +15,8 @@ class Projects::BranchesController < Projects::ApplicationController respond_to do |format| format.html do @refs_pipelines = @project.pipelines.latest_successful_for_refs(@branches.map(&:name)) + @merged_branch_names = + repository.merged_branch_names(@branches.map(&:name)) # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37429 Gitlab::GitalyClient.allow_n_plus_1_calls do @max_commits = @branches.reduce(0) do |memo, branch| diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index b7a108a0ebd..fe1334c0cfe 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -16,7 +16,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :authorize_create_issue!, only: [:new, :create] # Allow modify issue - before_action :authorize_update_issue!, only: [:update, :move] + before_action :authorize_update_issue!, only: [:edit, :update, :move] # Allow create a new branch and empty WIP merge request from current issue before_action :authorize_create_merge_request!, only: [:create_merge_request] @@ -63,6 +63,10 @@ class Projects::IssuesController < Projects::ApplicationController respond_with(@issue) end + def edit + respond_with(@issue) + end + def show @noteable = @issue @note = @project.notes.new(noteable: @issue) @@ -122,6 +126,10 @@ class Projects::IssuesController < Projects::ApplicationController @issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue) respond_to do |format| + format.html do + recaptcha_check_with_fallback { render :edit } + end + format.json do render_issue_json end diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 7112c6ee470..c4a621160af 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -20,17 +20,6 @@ module BoardsHelper project_issues_path(@project) end - def current_board_json - board = @board || @boards.first - - board.to_json( - only: [:id, :name, :milestone_id], - include: { - milestone: { only: [:title] } - } - ) - end - def board_base_url project_boards_path(@project) end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index cf3ce3c9e54..ca65e81f27a 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -249,9 +249,7 @@ module Ci end def commit - @commit ||= project.commit(sha) - rescue - nil + @commit ||= project.commit_by(oid: sha) end def branch? diff --git a/app/models/email.rb b/app/models/email.rb index 384f38f2db7..2da8b050149 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -14,6 +14,8 @@ class Email < ActiveRecord::Base devise :confirmable self.reconfirmable = false # currently email can't be changed, no need to reconfirm + delegate :username, to: :user + def email=(value) write_attribute(:email, value.downcase.strip) end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b0a0c753c09..d45b9c805a4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -396,6 +396,10 @@ class MergeRequest < ActiveRecord::Base end def merge_ongoing? + # While the MergeRequest is locked, it should present itself as 'merge ongoing'. + # The unlocking process is handled by StuckMergeJobsWorker scheduled in Cron. + return true if locked? + !!merge_jid && !merged? && Gitlab::SidekiqStatus.running?(merge_jid) end diff --git a/app/models/project.rb b/app/models/project.rb index 4689b588906..7185b4d44fc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -540,6 +540,10 @@ class Project < ActiveRecord::Base repository.commit(ref) end + def commit_by(oid:) + repository.commit_by(oid: oid) + end + # ref can't be HEAD, can only be branch/tag name or SHA def latest_successful_builds_for(ref = default_branch) latest_pipeline = pipelines.latest_successful_for(ref) @@ -553,7 +557,7 @@ class Project < ActiveRecord::Base def merge_base_commit(first_commit_id, second_commit_id) sha = repository.merge_base(first_commit_id, second_commit_id) - repository.commit(sha) if sha + commit_by(oid: sha) if sha end def saved? diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index 8ba07173c74..5c0b3338a62 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -153,7 +153,10 @@ class KubernetesService < DeploymentService end def default_namespace - "#{project.path}-#{project.id}" if project.present? + return unless project + + slug = "#{project.path}-#{project.id}".downcase + slug.gsub(/[^-a-z0-9]/, '-').gsub(/^-+/, '') end def build_kubeclient!(api_path: 'api', api_version: 'v1') diff --git a/app/models/repository.rb b/app/models/repository.rb index 7497b69f674..44a1e9ce529 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -76,6 +76,7 @@ class Repository @full_path = full_path @disk_path = disk_path || full_path @project = project + @commit_cache = {} end def ==(other) @@ -103,18 +104,17 @@ class Repository def commit(ref = 'HEAD') return nil unless exists? + return ref if ref.is_a?(::Commit) - commit = - if ref.is_a?(Gitlab::Git::Commit) - ref - else - Gitlab::Git::Commit.find(raw_repository, ref) - end + find_commit(ref) + end - commit = ::Commit.new(commit, @project) if commit - commit - rescue Rugged::OdbError, Rugged::TreeError - nil + # Finding a commit by the passed SHA + # Also takes care of caching, based on the SHA + def commit_by(oid:) + return @commit_cache[oid] if @commit_cache.key?(oid) + + @commit_cache[oid] = find_commit(oid) end def commits(ref, path: nil, limit: nil, offset: nil, skip_merges: false, after: nil, before: nil) @@ -231,7 +231,7 @@ class Repository # branches or tags, but we want to keep some of these commits around, for # example if they have comments or CI builds. def keep_around(sha) - return unless sha && commit(sha) + return unless sha && commit_by(oid: sha) return if kept_around?(sha) @@ -902,18 +902,27 @@ class Repository end end - def merged_to_root_ref?(branch_name) - branch_commit = commit(branch_name) - root_ref_commit = commit(root_ref) + def merged_to_root_ref?(branch_or_name, pre_loaded_merged_branches = nil) + branch = Gitlab::Git::Branch.find(self, branch_or_name) + + if branch + root_ref_sha = commit(root_ref).sha + same_head = branch.target == root_ref_sha + merged = + if pre_loaded_merged_branches + pre_loaded_merged_branches.include?(branch.name) + else + ancestor?(branch.target, root_ref_sha) + end - if branch_commit - same_head = branch_commit.id == root_ref_commit.id - !same_head && ancestor?(branch_commit.id, root_ref_commit.id) + !same_head && merged else nil end end + delegate :merged_branch_names, to: :raw_repository + def merge_base(first_commit_id, second_commit_id) first_commit_id = commit(first_commit_id).try(:id) || first_commit_id second_commit_id = commit(second_commit_id).try(:id) || second_commit_id @@ -1064,6 +1073,18 @@ class Repository private + # TODO Generice finder, later split this on finders by Ref or Oid + # gitlab-org/gitlab-ce#39239 + def find_commit(oid_or_ref) + commit = if oid_or_ref.is_a?(Gitlab::Git::Commit) + oid_or_ref + else + Gitlab::Git::Commit.find(raw_repository, oid_or_ref) + end + + ::Commit.new(commit, @project) if commit + end + def blob_data_at(sha, path) blob = blob_at(sha, path) return unless blob @@ -1102,12 +1123,12 @@ class Repository def last_commit_for_path_by_gitaly(sha, path) c = raw_repository.gitaly_commit_client.last_commit_for_path(sha, path) - commit(c) + commit_by(oid: c) end def last_commit_for_path_by_rugged(sha, path) sha = last_commit_id_for_path_by_shelling_out(sha, path) - commit(sha) + commit_by(oid: sha) end def last_commit_id_for_path_by_shelling_out(sha, path) diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 8c5821aa870..156e7b2f078 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -82,16 +82,9 @@ module MergeRequests @merge_request.can_remove_source_branch?(branch_deletion_user) end - # Logs merge error message and cleans `MergeRequest#merge_jid`. - # def handle_merge_error(log_message:, save_message_on_model: false) Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") - - if save_message_on_model - @merge_request.update(merge_error: log_message, merge_jid: nil) - else - clean_merge_jid - end + @merge_request.update(merge_error: log_message) if save_message_on_model end def merge_request_info diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index a1c2f8d0180..5d275967821 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -83,7 +83,7 @@ class SystemHooksService project_id: model.id, owner_name: owner.name, owner_email: owner.respond_to?(:email) ? owner.email : "", - project_visibility: Project.visibility_levels.key(model.visibility_level_value).downcase + project_visibility: model.visibility.downcase } end diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml index 49101d1efa4..6e02ae6c9cc 100644 --- a/app/views/projects/branches/_branch.html.haml +++ b/app/views/projects/branches/_branch.html.haml @@ -1,3 +1,4 @@ +- merged = local_assigns.fetch(:merged, false) - commit = @repository.commit(branch.dereferenced_target) - bar_graph_width_factor = @max_commits > 0 ? 100.0/@max_commits : 0 - diverging_commit_counts = @repository.diverging_commit_counts(branch) @@ -12,7 +13,7 @@ - if branch.name == @repository.root_ref %span.label.label-primary default - - elsif @repository.merged_to_root_ref? branch.name + - elsif merged %span.label.label-info.has-tooltip{ title: s_('Branches|Merged into %{default_branch}') % { default_branch: @repository.root_ref } } = s_('Branches|merged') @@ -47,7 +48,7 @@ target: "#modal-delete-branch", delete_path: project_branch_path(@project, branch.name), branch_name: branch.name, - is_merged: ("true" if @repository.merged_to_root_ref?(branch.name)) } } + is_merged: ("true" if merged) } } = icon("trash-o") - else %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip disabled", diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 7d9645d79e6..aade310236e 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -38,7 +38,7 @@ - if @branches.any? %ul.content-list.all-branches - @branches.each do |branch| - = render "projects/branches/branch", branch: branch + = render "projects/branches/branch", branch: branch, merged: @repository.merged_to_root_ref?(branch, @merged_branch_names) = paginate @branches, theme: 'gitlab' - else .nothing-here-block diff --git a/app/views/projects/issues/edit.html.haml b/app/views/projects/issues/edit.html.haml new file mode 100644 index 00000000000..1b7d878c38c --- /dev/null +++ b/app/views/projects/issues/edit.html.haml @@ -0,0 +1,7 @@ +- page_title "Edit", "#{@issue.title} (#{@issue.to_reference})", "Issues" + +%h3.page-title + Edit Issue ##{@issue.iid} +%hr + += render "form" diff --git a/app/workers/stuck_merge_jobs_worker.rb b/app/workers/stuck_merge_jobs_worker.rb index 7843179d77c..a396c0f27b2 100644 --- a/app/workers/stuck_merge_jobs_worker.rb +++ b/app/workers/stuck_merge_jobs_worker.rb @@ -23,7 +23,7 @@ class StuckMergeJobsWorker merge_requests = MergeRequest.where(id: completed_ids) merge_requests.where.not(merge_commit_sha: nil).update_all(state: :merged) - merge_requests.where(merge_commit_sha: nil).update_all(state: :opened) + merge_requests.where(merge_commit_sha: nil).update_all(state: :opened, merge_jid: nil) Rails.logger.info("Updated state of locked merge jobs. JIDs: #{completed_jids.join(', ')}") end diff --git a/changelogs/unreleased/39366-email-confirmation-fails.yml b/changelogs/unreleased/39366-email-confirmation-fails.yml new file mode 100644 index 00000000000..a5568670c70 --- /dev/null +++ b/changelogs/unreleased/39366-email-confirmation-fails.yml @@ -0,0 +1,5 @@ +--- +title: 'Fix bug preventing secondary emails from being confirmed' +merge_request: 15010 +author: +type: fixed diff --git a/changelogs/unreleased/39441-bring-edit-form-back.yml b/changelogs/unreleased/39441-bring-edit-form-back.yml new file mode 100644 index 00000000000..025417e4da9 --- /dev/null +++ b/changelogs/unreleased/39441-bring-edit-form-back.yml @@ -0,0 +1,5 @@ +--- +title: Fix editing issue description in mobile view +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/bvl-fix-system-hook-project-visibility.yml b/changelogs/unreleased/bvl-fix-system-hook-project-visibility.yml new file mode 100644 index 00000000000..a17ed51c9b8 --- /dev/null +++ b/changelogs/unreleased/bvl-fix-system-hook-project-visibility.yml @@ -0,0 +1,5 @@ +--- +title: Use the correct visibility attribute for projects in system hooks +merge_request: 15065 +author: +type: fixed diff --git a/changelogs/unreleased/make-merge-jid-handling-less-stateful.yml b/changelogs/unreleased/make-merge-jid-handling-less-stateful.yml new file mode 100644 index 00000000000..fe945e822fd --- /dev/null +++ b/changelogs/unreleased/make-merge-jid-handling-less-stateful.yml @@ -0,0 +1,5 @@ +--- +title: Fix widget of locked merge requests not being presented +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/mr-14642.yml b/changelogs/unreleased/mr-14642.yml new file mode 100644 index 00000000000..048cc79e323 --- /dev/null +++ b/changelogs/unreleased/mr-14642.yml @@ -0,0 +1,6 @@ +--- +title: Auto Devops kubernetes default namespace is now correctly built out of gitlab + project group-name +merge_request: 14642 +author: Mircea Danila Dumitrescu +type: fixed diff --git a/changelogs/unreleased/use-git-branch-merged.yml b/changelogs/unreleased/use-git-branch-merged.yml new file mode 100644 index 00000000000..24ec226250c --- /dev/null +++ b/changelogs/unreleased/use-git-branch-merged.yml @@ -0,0 +1,5 @@ +--- +title: Improve branch listing page performance +merge_request: 14729 +author: +type: performance diff --git a/changelogs/unreleased/zj-commit-cache.yml b/changelogs/unreleased/zj-commit-cache.yml new file mode 100644 index 00000000000..e3afe0ea7ef --- /dev/null +++ b/changelogs/unreleased/zj-commit-cache.yml @@ -0,0 +1,5 @@ +--- +title: Cache commits fetched from the repository +merge_request: +author: +type: performance diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 6baae20d16a..11d5e077a36 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -20,7 +20,7 @@ it, the client IP needs to be [included in a whitelist][whitelist]. Currently the embedded Prometheus server is not automatically configured to collect metrics from this endpoint. We recommend setting up another Prometheus server, because the embedded server configuration is overwritten once every -[reconfigure of GitLab][reconfigure]. In the future this will not be required. +[reconfigure of GitLab][reconfigure]. In the future this will not be required. ## Metrics available @@ -45,6 +45,8 @@ In this experimental phase, only a few metrics are available: | redis_ping_success | Gauge | 9.4 | Whether or not the last redis ping succeeded | | redis_ping_latency_seconds | Gauge | 9.4 | Round trip time of the redis ping | | user_session_logins_total | Counter | 9.4 | Counter of how many users have logged in | +| filesystem_circuitbreaker_latency_seconds | Histogram | 9.5 | Latency of the stat check the circuitbreaker uses to probe a shard | +| filesystem_circuitbreaker | Gauge | 9.5 | Wether or not the circuit for a certain shard is broken or not | ## Metrics shared directory diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md new file mode 100644 index 00000000000..932a44f65e4 --- /dev/null +++ b/doc/development/ee_features.md @@ -0,0 +1,382 @@ +# Guidelines for implementing Enterprise Edition feature + +- **Write the code and the tests.**: As with any code, EE features should have + good test coverage to prevent regressions. +- **Write documentation.**: Add documentation to the `doc/` directory. Describe + the feature and include screenshots, if applicable. +- **Submit a MR to the `www-gitlab-com` project.**: Add the new feature to the + [EE features list][ee-features-list]. + +## Act as CE when unlicensed + +Since the implementation of [GitLab CE features to work with unlicensed EE instance][ee-as-ce] +GitLab Enterprise Edition should work like GitLab Community Edition +when no license is active. So EE features always should be guarded by +`project.feature_available?` or `group.feature_available?` (or +`License.feature_available?` if it is a system-wide feature). + +CE specs should remain untouched as much as possible and extra specs +should be added for EE. Licensed features can be stubbed using the +spec helper `stub_licensed_features` in `EE::LicenseHelpers`. + +[ee-as-ce]: https://gitlab.com/gitlab-org/gitlab-ee/issues/2500 + +## Separation of EE code + +We want a [single code base][] eventually, but before we reach the goal, +we still need to merge changes from GitLab CE to EE. To help us get there, +we should make sure that we no longer edit CE files in place in order to +implement EE features. + +Instead, all EE codes should be put inside the `ee/` top-level directory, and +tests should be put inside `spec/ee/`. We don't use `ee/spec` for now due to +technical limitation. The rest of codes should be as close as to the CE files. + +[single code base]: https://gitlab.com/gitlab-org/gitlab-ee/issues/2952#note_41016454 + +### EE-only features + +If the feature being developed is not present in any form in CE, we don't +need to put the codes under `EE` namespace. For example, an EE model could +go into: `ee/app/models/awesome.rb` using `Awesome` as the class name. This +is applied not only to models. Here's a list of other examples: + +- `ee/app/controllers/foos_controller.rb` +- `ee/app/finders/foos_finder.rb` +- `ee/app/helpers/foos_helper.rb` +- `ee/app/mailers/foos_mailer.rb` +- `ee/app/models/foo.rb` +- `ee/app/policies/foo_policy.rb` +- `ee/app/serializers/foo_entity.rb` +- `ee/app/serializers/foo_serializer.rb` +- `ee/app/services/foo/create_service.rb` +- `ee/app/validators/foo_attr_validator.rb` +- `ee/app/workers/foo_worker.rb` + +### EE features based on CE features + +For features that build on existing CE features, write a module in the +`EE` namespace and `prepend` it in the CE class. This makes conflicts +less likely to happen during CE to EE merges because only one line is +added to the CE class - the `prepend` line. + +Since the module would require an `EE` namespace, the file should also be +put in an `ee/` sub-directory. For example, we want to extend the user model +in EE, so we have a module called `::EE::User` put inside +`ee/app/models/ee/user.rb`. + +This is also not just applied to models. Here's a list of other examples: + +- `ee/app/controllers/ee/foos_controller.rb` +- `ee/app/finders/ee/foos_finder.rb` +- `ee/app/helpers/ee/foos_helper.rb` +- `ee/app/mailers/ee/foos_mailer.rb` +- `ee/app/models/ee/foo.rb` +- `ee/app/policies/ee/foo_policy.rb` +- `ee/app/serializers/ee/foo_entity.rb` +- `ee/app/serializers/ee/foo_serializer.rb` +- `ee/app/services/ee/foo/create_service.rb` +- `ee/app/validators/ee/foo_attr_validator.rb` +- `ee/app/workers/ee/foo_worker.rb` + +#### Overriding CE methods + +To override a method present in the CE codebase, use `prepend`. It +lets you override a method in a class with a method from a module, while +still having access the class's implementation with `super`. + +There are a few gotchas with it: + +- you should always add a `raise NotImplementedError unless defined?(super)` + guard clause in the "overrider" method to ensure that if the method gets + renamed in CE, the EE override won't be silently forgotten. +- when the "overrider" would add a line in the middle of the CE + implementation, you should refactor the CE method and split it in + smaller methods. Or create a "hook" method that is empty in CE, + and with the EE-specific implementation in EE. +- when the original implementation contains a guard clause (e.g. + `return unless condition`), we cannot easily extend the behaviour by + overriding the method, because we can't know when the overridden method + (i.e. calling `super` in the overriding method) would want to stop early. + In this case, we shouldn't just override it, but update the original method + to make it call the other method we want to extend, like a [template method + pattern](https://en.wikipedia.org/wiki/Template_method_pattern). + For example, given this base: + ``` ruby + class Base + def execute + return unless enabled? + + # ... + # ... + end + end + ``` + Instead of just overriding `Base#execute`, we should update it and extract + the behaviour into another method: + ``` ruby + class Base + def execute + return unless enabled? + + do_something + end + + private + + def do_something + # ... + # ... + end + end + ``` + Then we're free to override that `do_something` without worrying about the + guards: + ``` ruby + module EE::Base + def do_something + # Follow the above pattern to call super and extend it + end + end + ``` + This would require updating CE first, or make sure this is back ported to CE. + +When prepending, place them in the `ee/` specific sub-directory, and +wrap class or module in `module EE` to avoid naming conflicts. + +For example to override the CE implementation of +`ApplicationController#after_sign_out_path_for`: + +```ruby +def after_sign_out_path_for(resource) + current_application_settings.after_sign_out_path.presence || new_user_session_path +end +``` + +Instead of modifying the method in place, you should add `prepend` to +the existing file: + +```ruby +class ApplicationController < ActionController::Base + prepend EE::ApplicationController + # ... + + def after_sign_out_path_for(resource) + current_application_settings.after_sign_out_path.presence || new_user_session_path + end + + # ... +end +``` + +And create a new file in the `ee/` sub-directory with the altered +implementation: + +```ruby +module EE + class ApplicationController + def after_sign_out_path_for(resource) + raise NotImplementedError unless defined?(super) + + if Gitlab::Geo.secondary? + Gitlab::Geo.primary_node.oauth_logout_url(@geo_logout_state) + else + super + end + end + end +end +``` + +#### Use self-descriptive wrapper methods + +When it's not possible/logical to modify the implementation of a +method. Wrap it in a self-descriptive method and use that method. + +For example, in CE only an `admin` is allowed to access all private +projects/groups, but in EE also an `auditor` has full private +access. It would be incorrect to override the implementation of +`User#admin?`, so instead add a method `full_private_access?` to +`app/models/users.rb`. The implementation in CE will be: + +```ruby +def full_private_access? + admin? +end +``` + +In EE, the implementation `ee/app/models/ee/users.rb` would be: + +```ruby +def full_private_access? + raise NotImplementedError unless defined?(super) + super || auditor? +end +``` + +In `lib/gitlab/visibility_level.rb` this method is used to return the +allowed visibilty levels: + +```ruby +def levels_for_user(user = nil) + if user.full_private_access? + [PRIVATE, INTERNAL, PUBLIC] + elsif # ... +end +``` + +See [CE MR][ce-mr-full-private] and [EE MR][ee-mr-full-private] for +full implementation details. + +[ce-mr-full-private]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12373 +[ee-mr-full-private]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2199 + +### Code in `app/controllers/` + +In controllers, the most common type of conflict is with `before_action` that +has a list of actions in CE but EE adds some actions to that list. + +The same problem often occurs for `params.require` / `params.permit` calls. + +**Mitigations** + +Separate CE and EE actions/keywords. For instance for `params.require` in +`ProjectsController`: + +```ruby +def project_params + params.require(:project).permit(project_params_attributes) +end + +# Always returns an array of symbols, created however best fits the use case. +# It _should_ be sorted alphabetically. +def project_params_attributes + %i[ + description + name + path + ] +end + +``` + +In the `EE::ProjectsController` module: + +```ruby +def project_params_attributes + super + project_params_attributes_ee +end + +def project_params_attributes_ee + %i[ + approvals_before_merge + approver_group_ids + approver_ids + ... + ] +end +``` + +### Code in `app/models/` + +EE-specific models should `extend EE::Model`. + +For example, if EE has a specific `Tanuki` model, you would +place it in `ee/app/models/ee/tanuki.rb`. + +### Code in `app/views/` + +It's a very frequent problem that EE is adding some specific view code in a CE +view. For instance the approval code in the project's settings page. + +**Mitigations** + +Blocks of code that are EE-specific should be moved to partials. This +avoids conflicts with big chunks of HAML code that that are not fun to +resolve when you add the indentation to the equation. + +EE-specific views should be placed in `ee/app/views/ee/`, using extra +sub-directories if appropriate. + +### Code in `lib/` + +Place EE-specific logic in the top-level `EE` module namespace. Namespace the +class beneath the `EE` module just as you would normally. + +For example, if CE has LDAP classes in `lib/gitlab/ldap/` then you would place +EE-specific LDAP classes in `ee/lib/ee/gitlab/ldap`. + +### Code in `spec/` + +When you're testing EE-only features, avoid adding examples to the +existing CE specs. Also do no change existing CE examples, since they +should remain working as-is when EE is running without a license. + +Instead place EE specs in the `spec/ee/spec` folder. + +## JavaScript code in `assets/javascripts/` + +To separate EE-specific JS-files we can also move the files into an `ee` folder. + +For example there can be an +`app/assets/javascripts/protected_branches/protected_branches_bundle.js` and an +EE counterpart +`ee/app/assets/javascripts/protected_branches/protected_branches_bundle.js`. + +That way we can create a separate webpack bundle in `webpack.config.js`: + +```javascript + protected_branches: '~/protected_branches', + ee_protected_branches: 'ee/protected_branches/protected_branches_bundle.js', +``` + +With the separate bundle in place, we can decide which bundle to load inside the +view, using the `page_specific_javascript_bundle_tag` helper. + +```haml +- content_for :page_specific_javascripts do + = page_specific_javascript_bundle_tag('protected_branches') +``` + +## SCSS code in `assets/stylesheets` + +To separate EE-specific styles in SCSS files, if a component you're adding styles for +is limited to only EE, it is better to have a separate SCSS file in appropriate directory +within `app/assets/stylesheets`. + +In some cases, this is not entirely possible or creating dedicated SCSS file is an overkill, +e.g. a text style of some component is different for EE. In such cases, +styles are usually kept in stylesheet that is common for both CE and EE, and it is wise +to isolate such ruleset from rest of CE rules (along with adding comment describing the same) +to avoid conflicts during CE to EE merge. + +#### Bad +```scss +.section-body { + .section-title { + background: $gl-header-color; + } + + &.ee-section-body { + .section-title { + background: $gl-header-color-cyan; + } + } +} +``` + +#### Good +```scss +.section-body { + .section-title { + background: $gl-header-color; + } +} + +/* EE-specific styles */ +.section-body.ee-section-body { + .section-title { + background: $gl-header-color-cyan; + } +} +``` diff --git a/doc/gitlab-basics/add-merge-request.md b/doc/gitlab-basics/add-merge-request.md index bf01fe51dc3..5cc014419ad 100644 --- a/doc/gitlab-basics/add-merge-request.md +++ b/doc/gitlab-basics/add-merge-request.md @@ -3,31 +3,28 @@ Merge requests are useful to integrate separate changes that you've made to a project, on different branches. This is a brief guide on how to create a merge request. For more information, check the -[merge requests documentation](../user/project/merge_requests.md). +[merge requests documentation](../user/project/merge_requests/index.md). --- 1. Before you start, you should have already [created a branch](create-branch.md) and [pushed your changes](basic-git-commands.md) to GitLab. - -1. You can then go to the project where you'd like to merge your changes and - click on the **Merge requests** tab. - -  - +1. Go to the project where you'd like to merge your changes and click on the + **Merge requests** tab. 1. Click on **New merge request** on the right side of the screen. - -  - -1. Select a source branch and click on the **Compare branches and continue** button. +1. From there on, you have the option to select the source branch and the target + branch you'd like to compare to. The default target project is the upstream + repository, but you can choose to compare across any of its forks.  +1. When ready, click on the **Compare branches and continue** button. 1. At a minimum, add a title and a description to your merge request. Optionally, select a user to review your merge request and to accept or close it. You may also select a milestone and labels.  -1. When ready, click on the **Submit merge request** button. Your merge request - will be ready to be approved and published. +1. When ready, click on the **Submit merge request** button. + +Your merge request will be ready to be approved and merged. diff --git a/doc/gitlab-basics/img/merge_request_new.png b/doc/gitlab-basics/img/merge_request_new.png Binary files differdeleted file mode 100644 index 6fcd7bebada..00000000000 --- a/doc/gitlab-basics/img/merge_request_new.png +++ /dev/null diff --git a/doc/gitlab-basics/img/merge_request_select_branch.png b/doc/gitlab-basics/img/merge_request_select_branch.png Binary files differindex 9f6b93943a9..57ea0e65f34 100644 --- a/doc/gitlab-basics/img/merge_request_select_branch.png +++ b/doc/gitlab-basics/img/merge_request_select_branch.png diff --git a/doc/gitlab-basics/img/project_navbar.png b/doc/gitlab-basics/img/project_navbar.png Binary files differdeleted file mode 100644 index be6f38ede32..00000000000 --- a/doc/gitlab-basics/img/project_navbar.png +++ /dev/null diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index e4c09b2b507..54c3e20d61d 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -136,44 +136,54 @@ In the example below we use Amazon S3 for storage, but Fog also lets you use for AWS, Google, OpenStack Swift, Rackspace and Aliyun as well. A local driver is [also available](#uploading-to-locally-mounted-shares). -For omnibus packages, add the following to `/etc/gitlab/gitlab.rb`: +#### Using Amazon S3 -```ruby -gitlab_rails['backup_upload_connection'] = { - 'provider' => 'AWS', - 'region' => 'eu-west-1', - 'aws_access_key_id' => 'AKIAKIAKI', - 'aws_secret_access_key' => 'secret123' - # If using an IAM Profile, don't configure aws_access_key_id & aws_secret_access_key - # 'use_iam_profile' => true -} -gitlab_rails['backup_upload_remote_directory'] = 'my.s3.bucket' -``` +For Omnibus GitLab packages: + +1. Add the following to `/etc/gitlab/gitlab.rb`: + + ```ruby + gitlab_rails['backup_upload_connection'] = { + 'provider' => 'AWS', + 'region' => 'eu-west-1', + 'aws_access_key_id' => 'AKIAKIAKI', + 'aws_secret_access_key' => 'secret123' + # If using an IAM Profile, don't configure aws_access_key_id & aws_secret_access_key + # 'use_iam_profile' => true + } + gitlab_rails['backup_upload_remote_directory'] = 'my.s3.bucket' + ``` + +1. [Reconfigure GitLab] for the changes to take effect -Make sure to run `sudo gitlab-ctl reconfigure` after editing `/etc/gitlab/gitlab.rb` to reflect the changes. +--- For installations from source: -```yaml - backup: - # snip - upload: - # Fog storage connection settings, see http://fog.io/storage/ . - connection: - provider: AWS - region: eu-west-1 - aws_access_key_id: AKIAKIAKI - aws_secret_access_key: 'secret123' - # If using an IAM Profile, leave aws_access_key_id & aws_secret_access_key empty - # ie. aws_access_key_id: '' - # use_iam_profile: 'true' - # The remote 'directory' to store your backups. For S3, this would be the bucket name. - remote_directory: 'my.s3.bucket' - # Turns on AWS Server-Side Encryption with Amazon S3-Managed Keys for backups, this is optional - # encryption: 'AES256' - # Specifies Amazon S3 storage class to use for backups, this is optional - # storage_class: 'STANDARD' -``` +1. Edit `home/git/gitlab/config/gitlab.yml`: + + ```yaml + backup: + # snip + upload: + # Fog storage connection settings, see http://fog.io/storage/ . + connection: + provider: AWS + region: eu-west-1 + aws_access_key_id: AKIAKIAKI + aws_secret_access_key: 'secret123' + # If using an IAM Profile, leave aws_access_key_id & aws_secret_access_key empty + # ie. aws_access_key_id: '' + # use_iam_profile: 'true' + # The remote 'directory' to store your backups. For S3, this would be the bucket name. + remote_directory: 'my.s3.bucket' + # Turns on AWS Server-Side Encryption with Amazon S3-Managed Keys for backups, this is optional + # encryption: 'AES256' + # Specifies Amazon S3 storage class to use for backups, this is optional + # storage_class: 'STANDARD' + ``` + +1. [Restart GitLab] for the changes to take effect If you are uploading your backups to S3 you will probably want to create a new IAM user with restricted access rights. To give the upload user access only for @@ -226,6 +236,50 @@ with the name of your bucket: } ``` +#### Using Google Cloud Storage + +If you want to use Google Cloud Storage to save backups, you'll have to create +an access key from the Google console first: + +1. Go to the storage settings page https://console.cloud.google.com/storage/settings +1. Select "Interoperability" and create an access key +1. Make note of the "Access Key" and "Secret" and replace them in the + configurations below +1. Make sure you already have a bucket created + +For Omnibus GitLab packages: + +1. Edit `/etc/gitlab/gitlab.rb`: + + ```ruby + gitlab_rails['backup_upload_connection'] = { + 'provider' => 'Google', + 'google_storage_access_key_id' => 'Access Key', + 'google_storage_secret_access_key' => 'Secret' + } + gitlab_rails['backup_upload_remote_directory'] = 'my.google.bucket' + ``` + +1. [Reconfigure GitLab] for the changes to take effect + +--- + +For installations from source: + +1. Edit `home/git/gitlab/config/gitlab.yml`: + + ```yaml + backup: + upload: + connection: + provider: 'Google' + google_storage_access_key_id: 'Access Key' + google_storage_secret_access_key: 'Secret' + remote_directory: 'my.google.bucket' + ``` + +1. [Restart GitLab] for the changes to take effect + ### Uploading to locally mounted shares You may also send backups to a mounted share (`NFS` / `CIFS` / `SMB` / etc.) by @@ -554,3 +608,6 @@ The rake task runs this as the `gitlab` user which does not have the superuser a Those objects have no influence on the database backup/restore but they give this annoying warning. For more information see similar questions on postgresql issue tracker[here](http://www.postgresql.org/message-id/201110220712.30886.adrian.klaver@gmail.com) and [here](http://www.postgresql.org/message-id/2039.1177339749@sss.pgh.pa.us) as well as [stack overflow](http://stackoverflow.com/questions/4368789/error-must-be-owner-of-language-plpgsql). + +[reconfigure GitLab]: ../administration/restart_gitlab.md#omnibus-gitlab-reconfigure +[restart GitLab]: ../administration/restart_gitlab.md#installations-from-source diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb index c53882787f1..3487e099381 100644 --- a/lib/gitlab/git/branch.rb +++ b/lib/gitlab/git/branch.rb @@ -3,6 +3,14 @@ module Gitlab module Git class Branch < Ref + def self.find(repo, branch_name) + if branch_name.is_a?(Gitlab::Git::Branch) + branch_name + else + repo.find_branch(branch_name) + end + end + def initialize(repository, name, target, target_commit) super(repository, name, target, target_commit) end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 1957c254c28..23ae37ff71e 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -72,7 +72,8 @@ module Gitlab decorate(repo, commit) if commit rescue Rugged::ReferenceError, Rugged::InvalidError, Rugged::ObjectError, - Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository + Gitlab::Git::CommandError, Gitlab::Git::Repository::NoRepository, + Rugged::OdbError, Rugged::TreeError nil end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 408616d174b..fc8af38d4d9 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -511,6 +511,10 @@ module Gitlab gitaly_commit_client.ancestor?(from, to) end + def merged_branch_names(branch_names = []) + Set.new(git_merged_branch_names(branch_names)) + end + # Return an array of Diff objects that represent the diff # between +from+ and +to+. See Diff::filter_diff_options for the allowed # diff options. The +options+ hash can also include :break_rewrites to @@ -1190,6 +1194,13 @@ module Gitlab sort_branches(branches, sort_by) end + def git_merged_branch_names(branch_names = []) + lines = run_git(['branch', '--merged', root_ref] + branch_names) + .first.lines + + lines.map(&:strip) + end + def log_using_shell?(options) options[:path].present? || options[:disable_walk] || diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index e7b2f52a552..f01d5c96fc8 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -8,6 +8,7 @@ module Gitlab { name: name, email: email, message: message } end end + PageBlob = Struct.new(:name) def self.default_ref 'master' @@ -80,7 +81,15 @@ module Gitlab end def preview_slug(title, format) - gollum_wiki.preview_page(title, '', format).url_path + # Adapted from gollum gem (Gollum::Wiki#preview_page) to avoid + # using Rugged through a Gollum::Wiki instance + page_class = Gollum::Page + page = page_class.new(nil) + ext = page_class.format_to_ext(format.to_sym) + name = page_class.cname(title) + '.' + ext + blob = PageBlob.new(name) + page.populate(blob) + page.url_path end private diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 6f48f091a20..aecdfb50759 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -557,6 +557,29 @@ describe Projects::IssuesController do end end end + + describe 'GET #edit' do + it_behaves_like 'restricted action', success: 200 + + def go(id:) + get :edit, + namespace_id: project.namespace.to_param, + project_id: project, + id: id + end + end + + describe 'PUT #update' do + it_behaves_like 'restricted action', success: 302 + + def go(id:) + put :update, + namespace_id: project.namespace.to_param, + project_id: project, + id: id, + issue: { title: 'New title' } + end + end end describe 'POST #create' do diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index f1ad5dd84ff..1604a2da485 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -46,7 +46,7 @@ describe Projects::PipelinesController do context 'when performing gitaly calls', :request_store do it 'limits the Gitaly requests' do - expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(10) + expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(8) end end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index c2b59239af9..cf38066dedc 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -119,7 +119,7 @@ FactoryGirl.define do finished_at nil end - factory :ci_build_tag do + trait :tag do tag true end diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 8ce470fc288..2db6f9a2982 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -218,15 +218,54 @@ describe 'New/edit issue', :js do context 'edit issue' do before do - visit project_issue_path(project, issue) - page.within('.content .issuable-actions') do - click_on 'Edit' + visit edit_project_issue_path(project, issue) + end + + it 'allows user to update issue' do + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user.id.to_s) + expect(find('input[name="issue[milestone_id]"]', visible: false).value).to match(milestone.id.to_s) + expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible + + page.within '.js-user-search' do + expect(page).to have_content user.name + end + + page.within '.js-milestone-select' do + expect(page).to have_content milestone.title + end + + click_button 'Labels' + page.within '.dropdown-menu-labels' do + click_link label.title + click_link label2.title + end + page.within '.js-label-select' do + expect(page).to have_content label.title + end + expect(page.all('input[name="issue[label_ids][]"]', visible: false)[1].value).to match(label.id.to_s) + expect(page.all('input[name="issue[label_ids][]"]', visible: false)[2].value).to match(label2.id.to_s) + + click_button 'Save changes' + + page.within '.issuable-sidebar' do + page.within '.assignee' do + expect(page).to have_content user.name + end + + page.within '.milestone' do + expect(page).to have_content milestone.title + end + + page.within '.labels' do + expect(page).to have_content label.title + expect(page).to have_content label2.title + end end end it 'description has autocomplete' do - find_field('issue-description').native.send_keys('') - fill_in 'issue-description', with: '@' + find('#issue_description').native.send_keys('') + fill_in 'issue_description', with: '@' expect(page).to have_selector('.atwho-view') end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 25e99774575..d4fd3a50008 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Issues', :js do +describe 'Issues' do include DropzoneHelper include IssueHelpers include SortingHelper @@ -24,15 +24,109 @@ describe 'Issues', :js do end before do - visit project_issue_path(project, issue) - page.within('.content .issuable-actions') do - find('.issuable-edit').click - end - find('.issue-details .content-block .js-zen-enter').click + visit edit_project_issue_path(project, issue) + find('.js-zen-enter').click end it 'opens new issue popup' do - expect(page).to have_content(issue.description) + expect(page).to have_content("Issue ##{issue.iid}") + end + end + + describe 'Editing issue assignee' do + let!(:issue) do + create(:issue, + author: user, + assignees: [user], + project: project) + end + + it 'allows user to select unassigned', :js do + visit edit_project_issue_path(project, issue) + + expect(page).to have_content "Assignee #{user.name}" + + first('.js-user-search').click + click_link 'Unassigned' + + click_button 'Save changes' + + page.within('.assignee') do + expect(page).to have_content 'No assignee - assign yourself' + end + + expect(issue.reload.assignees).to be_empty + end + end + + describe 'due date', :js do + context 'on new form' do + before do + visit new_project_issue_path(project) + end + + it 'saves with due date' do + date = Date.today.at_beginning_of_month + + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' + find('#issuable-due-date').click + + page.within '.pika-single' do + click_button date.day + end + + expect(find('#issuable-due-date').value).to eq date.to_s + + click_button 'Submit issue' + + page.within '.issuable-sidebar' do + expect(page).to have_content date.to_s(:medium) + end + end + end + + context 'on edit form' do + let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) } + + before do + visit edit_project_issue_path(project, issue) + end + + it 'saves with due date' do + date = Date.today.at_beginning_of_month + + expect(find('#issuable-due-date').value).to eq date.to_s + + date = date.tomorrow + + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' + find('#issuable-due-date').click + + page.within '.pika-single' do + click_button date.day + end + + expect(find('#issuable-due-date').value).to eq date.to_s + + click_button 'Save changes' + + page.within '.issuable-sidebar' do + expect(page).to have_content date.to_s(:medium) + end + end + + it 'warns about version conflict' do + issue.update(title: "New title") + + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' + + click_button 'Save changes' + + expect(page).to have_content 'Someone edited the issue the same time you did' + end end end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index d70cf1527e7..a7928857b7d 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -181,6 +181,21 @@ describe "Internal Project Access" do it { is_expected.to be_denied_for(:visitor) } end + describe "GET /:project_path/issues/:id/edit" do + let(:issue) { create(:issue, project: project) } + subject { edit_project_issue_path(project, issue) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_allowed_for(:developer).of(project) } + it { is_expected.to be_allowed_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:external) } + it { is_expected.to be_denied_for(:visitor) } + end + describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index ea130606545..a4396b20afd 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -181,6 +181,21 @@ describe "Private Project Access" do it { is_expected.to be_denied_for(:visitor) } end + describe "GET /:project_path/issues/:id/edit" do + let(:issue) { create(:issue, project: project) } + subject { edit_project_issue_path(project, issue) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_allowed_for(:developer).of(project) } + it { is_expected.to be_allowed_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:external) } + it { is_expected.to be_denied_for(:visitor) } + end + describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index d15f5af66c9..fccdeb0e5b7 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -394,6 +394,21 @@ describe "Public Project Access" do it { is_expected.to be_allowed_for(:visitor) } end + describe "GET /:project_path/issues/:id/edit" do + let(:issue) { create(:issue, project: project) } + subject { edit_project_issue_path(project, issue) } + + it { is_expected.to be_allowed_for(:admin) } + it { is_expected.to be_allowed_for(:owner).of(project) } + it { is_expected.to be_allowed_for(:master).of(project) } + it { is_expected.to be_allowed_for(:developer).of(project) } + it { is_expected.to be_allowed_for(:reporter).of(project) } + it { is_expected.to be_denied_for(:guest).of(project) } + it { is_expected.to be_denied_for(:user) } + it { is_expected.to be_denied_for(:external) } + it { is_expected.to be_denied_for(:visitor) } + end + describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } diff --git a/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js b/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js index 52e450e9ba5..8450ad9dbcb 100644 --- a/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js +++ b/spec/javascripts/vue_shared/components/user_avatar/user_avatar_link_spec.js @@ -11,6 +11,7 @@ describe('User Avatar Link Component', function () { imgCssClasses: 'myextraavatarclass', tooltipText: 'tooltip text', tooltipPlacement: 'bottom', + username: 'username', }; const UserAvatarLinkComponent = Vue.extend(UserAvatarLink); @@ -47,4 +48,42 @@ describe('User Avatar Link Component', function () { expect(this.userAvatarLink[key]).toBeDefined(); }); }); + + describe('no username', function () { + beforeEach(function (done) { + this.userAvatarLink.username = ''; + + Vue.nextTick(done); + }); + + it('should only render image tag in link', function () { + const childElements = this.userAvatarLink.$el.childNodes; + expect(childElements[0].tagName).toBe('IMG'); + + // Vue will render the hidden component as <!----> + expect(childElements[1].tagName).toBeUndefined(); + }); + + it('should render avatar image tooltip', function () { + expect(this.userAvatarLink.$el.querySelector('img').dataset.originalTitle).toEqual(this.propsData.tooltipText); + }); + }); + + describe('username', function () { + it('should not render avatar image tooltip', function () { + expect(this.userAvatarLink.$el.querySelector('img').dataset.originalTitle).toEqual(''); + }); + + it('should render username prop in <span>', function () { + expect(this.userAvatarLink.$el.querySelector('span').innerText.trim()).toEqual(this.propsData.username); + }); + + it('should render text tooltip for <span>', function () { + expect(this.userAvatarLink.$el.querySelector('span').dataset.originalTitle).toEqual(this.propsData.tooltipText); + }); + + it('should render text tooltip placement for <span>', function () { + expect(this.userAvatarLink.$el.querySelector('span').getAttribute('tooltip-placement')).toEqual(this.propsData.tooltipPlacement); + }); + }); }); diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 318a7b7a332..708870060e7 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -7,6 +7,38 @@ describe Gitlab::Git::Branch, seed_helper: true do it { is_expected.to be_kind_of Array } + describe '.find' do + subject { described_class.find(repository, branch) } + + before do + allow(repository).to receive(:find_branch).with(branch) + .and_call_original + end + + context 'when finding branch via branch name' do + let(:branch) { 'master' } + + it 'returns a branch object' do + expect(subject).to be_a(described_class) + expect(subject.name).to eq(branch) + + expect(repository).to have_received(:find_branch).with(branch) + end + end + + context 'when the branch is already a branch' do + let(:commit) { repository.commit('master') } + let(:branch) { described_class.new(repository, 'master', commit.sha, commit) } + + it 'returns a branch object' do + expect(subject).to be_a(described_class) + expect(subject).to eq(branch) + + expect(repository).not_to have_received(:find_branch).with(branch) + end + end + end + describe '#size' do subject { super().size } it { is_expected.to eq(SeedRepo::Repo::BRANCHES.size) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index e3d320631bc..b161d25b96c 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1135,6 +1135,32 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#merged_branch_names' do + context 'when branch names are passed' do + it 'only returns the names we are asking' do + names = repository.merged_branch_names(%w[merge-test]) + + expect(names).to contain_exactly('merge-test') + end + + it 'does not return unmerged branch names' do + names = repository.merged_branch_names(%w[feature]) + + expect(names).to be_empty + end + end + + context 'when no branch names are specified' do + it 'returns all merged branch names' do + names = repository.merged_branch_names + + expect(names).to include('merge-test') + expect(names).to include('fix-mode') + expect(names).not_to include('feature') + end + end + end + describe "#ls_files" do let(:master_file_paths) { repository.ls_files("master") } let(:not_existed_branch) { repository.ls_files("not_existed_branch") } diff --git a/spec/models/email_spec.rb b/spec/models/email_spec.rb index b32dd31ae6d..47eb0717c0c 100644 --- a/spec/models/email_spec.rb +++ b/spec/models/email_spec.rb @@ -40,4 +40,12 @@ describe Email do expect(user.emails.confirmed.count).to eq 1 end end + + describe 'delegation' do + let(:user) { create(:user) } + + it 'delegates to :user' do + expect(build(:email, user: user).username).to eq user.username + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 73e038b61ed..476a2697605 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -86,7 +86,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do - project.repository.raw_repository.delete_branch(subject.target_branch) + project.repository.rm_branch(subject.author, subject.target_branch) end it 'returns nil' do @@ -1388,7 +1388,7 @@ describe MergeRequest do context 'when the target branch does not exist' do before do - subject.project.repository.raw_repository.delete_branch(subject.target_branch) + subject.project.repository.rm_branch(subject.author, subject.target_branch) end it 'returns nil' do @@ -1460,6 +1460,12 @@ describe MergeRequest do end describe '#merge_ongoing?' do + it 'returns true when the merge request is locked' do + merge_request = build_stubbed(:merge_request, state: :locked) + + expect(merge_request.merge_ongoing?).to be(true) + end + it 'returns true when merge_id, MR is not merged and it has no running job' do merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo') allow(Gitlab::SidekiqStatus).to receive(:running?).with('foo') { true } diff --git a/spec/models/project_services/kubernetes_service_spec.rb b/spec/models/project_services/kubernetes_service_spec.rb index 2298dcab55f..00de536a18b 100644 --- a/spec/models/project_services/kubernetes_service_spec.rb +++ b/spec/models/project_services/kubernetes_service_spec.rb @@ -99,45 +99,34 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do describe '#actual_namespace' do subject { service.actual_namespace } - it "returns the default namespace" do - is_expected.to eq(service.send(:default_namespace)) - end - - context 'when namespace is specified' do - before do - service.namespace = 'my-namespace' + shared_examples 'a correctly formatted namespace' do + it 'returns a valid Kubernetes namespace name' do + expect(subject).to match(Gitlab::Regex.kubernetes_namespace_regex) + expect(subject).to eq(expected_namespace) end + end - it "returns the user-namespace" do - is_expected.to eq('my-namespace') - end + it_behaves_like 'a correctly formatted namespace' do + let(:expected_namespace) { service.send(:default_namespace) } end - context 'when service is not assigned to project' do + context 'when the project path contains forbidden characters' do before do - service.project = nil + project.path = '-a_Strange.Path--forSure' end - it "does not return namespace" do - is_expected.to be_nil + it_behaves_like 'a correctly formatted namespace' do + let(:expected_namespace) { "a-strange-path--forsure-#{project.id}" } end end - end - - describe '#actual_namespace' do - subject { service.actual_namespace } - - it "returns the default namespace" do - is_expected.to eq(service.send(:default_namespace)) - end context 'when namespace is specified' do before do service.namespace = 'my-namespace' end - it "returns the user-namespace" do - is_expected.to eq('my-namespace') + it_behaves_like 'a correctly formatted namespace' do + let(:expected_namespace) { 'my-namespace' } end end @@ -146,7 +135,7 @@ describe KubernetesService, :use_clean_rails_memory_store_caching do service.project = nil end - it "does not return namespace" do + it 'does not return namespace' do is_expected.to be_nil end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 874368ada67..d7c07676911 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -299,6 +299,24 @@ describe Repository do it { is_expected.to be_falsey } end + + context 'when pre-loaded merged branches are provided' do + using RSpec::Parameterized::TableSyntax + + where(:branch, :pre_loaded, :expected) do + 'not-merged-branch' | ['branch-merged'] | false + 'branch-merged' | ['not-merged-branch'] | false + 'branch-merged' | ['branch-merged'] | true + 'not-merged-branch' | ['not-merged-branch'] | false + 'master' | ['master'] | false + end + + with_them do + subject { repository.merged_to_root_ref?(branch, pre_loaded) } + + it { is_expected.to eq(expected) } + end + end end describe '#can_be_merged?' do @@ -2260,4 +2278,24 @@ describe Repository do end end end + + describe 'commit cache' do + set(:project) { create(:project, :repository) } + + it 'caches based on SHA' do + # Gets the commit oid, and warms the cache + oid = project.commit.id + + expect(Gitlab::Git::Commit).not_to receive(:find).once + + project.commit_by(oid: oid) + end + + it 'caches nil values' do + expect(Gitlab::Git::Commit).to receive(:find).once + + project.commit_by(oid: '1' * 40) + project.commit_by(oid: '1' * 40) + end + end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 5fd76fce7df..47f4ccd4887 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -385,7 +385,7 @@ describe API::Runner do end context 'when job is made for tag' do - let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job) { create(:ci_build, :tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } it 'sets branch as ref_type' do request_job @@ -436,8 +436,8 @@ describe API::Runner do end context 'when project and pipeline have multiple jobs' do - let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } - let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } + let!(:job) { create(:ci_build, :tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build, :tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } before do @@ -458,7 +458,7 @@ describe API::Runner do end context 'when pipeline have jobs with artifacts' do - let!(:job) { create(:ci_build_tag, :artifacts, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job) { create(:ci_build, :tag, :artifacts, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } before do @@ -478,8 +478,8 @@ describe API::Runner do end context 'when explicit dependencies are defined' do - let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } - let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } + let!(:job) { create(:ci_build, :tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build, :tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } let!(:test_job) do create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy', stage: 'deploy', stage_idx: 1, @@ -502,8 +502,8 @@ describe API::Runner do end context 'when dependencies is an empty array' do - let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } - let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } + let!(:job) { create(:ci_build, :tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build, :tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } let!(:empty_dependencies_job) do create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'empty_dependencies_job', stage: 'deploy', stage_idx: 1, diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index d1043f99b5a..ac196e92601 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -12,55 +12,6 @@ describe MergeRequests::MergeService do end describe '#execute' do - context 'MergeRequest#merge_jid' do - let(:service) do - described_class.new(project, user, commit_message: 'Awesome message') - end - - before do - merge_request.update_column(:merge_jid, 'hash-123') - end - - it 'is cleaned when no error is raised' do - service.execute(merge_request) - - expect(merge_request.reload.merge_jid).to be_nil - end - - it 'is cleaned when expected error is raised' do - allow(service).to receive(:commit).and_raise(described_class::MergeError) - - service.execute(merge_request) - - expect(merge_request.reload.merge_jid).to be_nil - end - - it 'is cleaned when merge request is not mergeable' do - allow(merge_request).to receive(:mergeable?).and_return(false) - - service.execute(merge_request) - - expect(merge_request.reload.merge_jid).to be_nil - end - - it 'is cleaned when no source is found' do - allow(merge_request).to receive(:diff_head_sha).and_return(nil) - - service.execute(merge_request) - - expect(merge_request.reload.merge_jid).to be_nil - end - - it 'is not cleaned when unexpected error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') - allow(service).to receive(:commit).and_raise(StandardError) - - expect { service.execute(merge_request) }.to raise_error(StandardError) - - expect(merge_request.reload.merge_jid).to be_present - end - end - context 'valid params' do let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 8b5d9187785..8f7aea533dc 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -63,6 +63,12 @@ describe SystemHooksService do :group_id, :user_id, :user_username, :user_name, :user_email, :group_access ) end + + it 'includes the correct project visibility level' do + data = event_data(project, :create) + + expect(data[:project_visibility]).to eq('private') + end end context 'event names' do diff --git a/spec/support/update_invalid_issuable.rb b/spec/support/update_invalid_issuable.rb index 50a1d4a56e2..1490287681b 100644 --- a/spec/support/update_invalid_issuable.rb +++ b/spec/support/update_invalid_issuable.rb @@ -25,13 +25,11 @@ shared_examples 'update invalid issuable' do |klass| .and_raise(ActiveRecord::StaleObjectError.new(issuable, :save)) end - if klass == MergeRequest - it 'renders edit when format is html' do - put :update, params + it 'renders edit when format is html' do + put :update, params - expect(response).to render_template(:edit) - expect(assigns[:conflict]).to be_truthy - end + expect(response).to render_template(:edit) + expect(assigns[:conflict]).to be_truthy end it 'renders json error message when format is json' do @@ -44,17 +42,16 @@ shared_examples 'update invalid issuable' do |klass| end end - if klass == MergeRequest - context 'when updating an invalid issuable' do - before do - params[:merge_request][:title] = "" - end + context 'when updating an invalid issuable' do + before do + key = klass == Issue ? :issue : :merge_request + params[key][:title] = "" + end - it 'renders edit when merge request is invalid' do - put :update, params + it 'renders edit when merge request is invalid' do + put :update, params - expect(response).to render_template(:edit) - end + expect(response).to render_template(:edit) end end end diff --git a/spec/workers/stuck_merge_jobs_worker_spec.rb b/spec/workers/stuck_merge_jobs_worker_spec.rb index a5ad78393c9..f8b55e873df 100644 --- a/spec/workers/stuck_merge_jobs_worker_spec.rb +++ b/spec/workers/stuck_merge_jobs_worker_spec.rb @@ -12,8 +12,13 @@ describe StuckMergeJobsWorker do worker.perform - expect(mr_with_sha.reload).to be_merged - expect(mr_without_sha.reload).to be_opened + mr_with_sha.reload + mr_without_sha.reload + + expect(mr_with_sha).to be_merged + expect(mr_without_sha).to be_opened + expect(mr_with_sha.merge_jid).to be_present + expect(mr_without_sha.merge_jid).to be_nil end it 'updates merge request to opened when locked but has not been merged' do |
