diff options
93 files changed, 1285 insertions, 815 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 057e2d6e0dc..b366ae6f069 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -171,7 +171,7 @@ Assigning a team label makes sure issues get the attention of the appropriate people. The current team labels are ~Build, ~"CI/CD", ~Discussion, ~Documentation, ~Edge, -~Geo, ~Gitaly, ~Platform, ~Prometheus, ~Release, and ~"UX". +~Geo, ~Gitaly, ~Platform, ~Monitoring, ~Release, and ~"UX". The descriptions on the [labels page][labels-page] explain what falls under the responsibility of each team. diff --git a/app/assets/javascripts/commit_merge_requests.js b/app/assets/javascripts/commit_merge_requests.js new file mode 100644 index 00000000000..f76c9b7e690 --- /dev/null +++ b/app/assets/javascripts/commit_merge_requests.js @@ -0,0 +1,73 @@ +/* global Flash */ + +import axios from './lib/utils/axios_utils'; +import { n__, s__ } from './locale'; + +export function getHeaderText(childElementCount, mergeRequestCount) { + if (childElementCount === 0) { + return `${mergeRequestCount} ${n__('merge request', 'merge requests', mergeRequestCount)}`; + } + return ','; +} + +export function createHeader(childElementCount, mergeRequestCount) { + const headerText = getHeaderText(childElementCount, mergeRequestCount); + + return $('<span />', { + class: 'append-right-5', + text: headerText, + }); +} + +export function createLink(mergeRequest) { + return $('<a />', { + class: 'append-right-5', + href: mergeRequest.path, + text: `!${mergeRequest.iid}`, + }); +} + +export function createTitle(mergeRequest) { + return $('<span />', { + text: mergeRequest.title, + }); +} + +export function createItem(mergeRequest) { + const $item = $('<span />'); + const $link = createLink(mergeRequest); + const $title = createTitle(mergeRequest); + $item.append($link); + $item.append($title); + + return $item; +} + +export function createContent(mergeRequests) { + const $content = $('<span />'); + + if (mergeRequests.length === 0) { + $content.text(s__('Commits|No related merge requests found')); + } else { + mergeRequests.forEach((mergeRequest) => { + const $header = createHeader($content.children().length, mergeRequests.length); + const $item = createItem(mergeRequest); + $content.append($header); + $content.append($item); + }); + } + + return $content; +} + +export function fetchCommitMergeRequests() { + const $container = $('.merge-requests'); + + axios.get($container.data('projectCommitPath')) + .then((response) => { + const $content = createContent(response.data); + + $container.html($content); + }) + .catch(() => Flash(s__('Commits|An error occurred while fetching merge requests data.'))); +} diff --git a/app/assets/javascripts/protected_tags/protected_tag_dropdown.js b/app/assets/javascripts/create_item_dropdown.js index a0224213aa0..c3eceb285f5 100644 --- a/app/assets/javascripts/protected_tags/protected_tag_dropdown.js +++ b/app/assets/javascripts/create_item_dropdown.js @@ -1,6 +1,6 @@ import _ from 'underscore'; -export default class ProtectedTagDropdown { +export default class CreateItemDropdown { /** * @param {Object} options containing * `$dropdown` target element @@ -8,11 +8,14 @@ export default class ProtectedTagDropdown { * $dropdown must be an element created using `dropdown_tag()` rails helper */ constructor(options) { - this.onSelect = options.onSelect; + this.defaultToggleLabel = options.defaultToggleLabel; + this.fieldName = options.fieldName; + this.onSelect = options.onSelect || (() => {}); + this.getDataOption = options.getData; this.$dropdown = options.$dropdown; this.$dropdownContainer = this.$dropdown.parent(); this.$dropdownFooter = this.$dropdownContainer.find('.dropdown-footer'); - this.$protectedTag = this.$dropdownContainer.find('.js-create-new-protected-tag'); + this.$createButton = this.$dropdownContainer.find('.js-dropdown-create-new-item'); this.buildDropdown(); this.bindEvents(); @@ -23,7 +26,7 @@ export default class ProtectedTagDropdown { buildDropdown() { this.$dropdown.glDropdown({ - data: this.getProtectedTags.bind(this), + data: this.getData.bind(this), filterable: true, remote: false, search: { @@ -31,14 +34,14 @@ export default class ProtectedTagDropdown { }, selectable: true, toggleLabel(selected) { - return (selected && 'id' in selected) ? selected.title : 'Protected Tag'; + return (selected && 'id' in selected) ? selected.title : this.defaultToggleLabel; }, - fieldName: 'protected_tag[name]', - text(protectedTag) { - return _.escape(protectedTag.title); + fieldName: this.fieldName, + text(item) { + return _.escape(item.title); }, - id(protectedTag) { - return _.escape(protectedTag.id); + id(item) { + return _.escape(item.id); }, onFilter: this.toggleCreateNewButton.bind(this), clicked: (options) => { @@ -49,37 +52,37 @@ export default class ProtectedTagDropdown { } bindEvents() { - this.$protectedTag.on('click', this.onClickCreateWildcard.bind(this)); + this.$createButton.on('click', this.onClickCreateWildcard.bind(this)); } onClickCreateWildcard(e) { + e.preventDefault(); + + // Refresh the dropdown's data, which ends up calling `getData` this.$dropdown.data('glDropdown').remote.execute(); this.$dropdown.data('glDropdown').selectRowAtIndex(); - e.preventDefault(); } - getProtectedTags(term, callback) { - if (this.selectedTag) { - callback(gon.open_tags.concat(this.selectedTag)); - } else { - callback(gon.open_tags); - } + getData(term, callback) { + this.getDataOption(term, (data = []) => { + callback(data.concat(this.selectedItem || [])); + }); } - toggleCreateNewButton(tagName) { - if (tagName) { - this.selectedTag = { - title: tagName, - id: tagName, - text: tagName, + toggleCreateNewButton(item) { + if (item) { + this.selectedItem = { + title: item, + id: item, + text: item, }; this.$dropdownContainer - .find('.js-create-new-protected-tag code') - .text(tagName); + .find('.js-dropdown-create-new-item code') + .text(item); } - this.toggleFooter(!tagName); + this.toggleFooter(!item); } toggleFooter(toggleState) { diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 6880784d7d0..1b03741882e 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -1,12 +1,10 @@ /* eslint-disable func-names, space-before-function-paren, no-var, prefer-arrow-callback, wrap-iife, no-shadow, consistent-return, one-var, one-var-declaration-per-line, camelcase, default-case, no-new, quotes, no-duplicate-case, no-case-declarations, no-fallthrough, max-len */ import { s__ } from './locale'; import projectSelect from './project_select'; -import IssuableIndex from './issuable_index'; import Milestone from './milestone'; import IssuableForm from './issuable_form'; import LabelsSelect from './labels_select'; import MilestoneSelect from './milestone_select'; -import NewBranchForm from './new_branch_form'; import NotificationsForm from './notifications_form'; import notificationsDropdown from './notifications_dropdown'; import groupAvatar from './group_avatar'; @@ -18,21 +16,17 @@ import projectAvatar from './project_avatar'; import MergeRequest from './merge_request'; import Compare from './compare'; import initCompareAutocomplete from './compare_autocomplete'; -import ProjectFindFile from './project_find_file'; import ProjectNew from './project_new'; -import projectImport from './project_import'; import Labels from './labels'; import LabelManager from './label_manager'; import Sidebar from './right_sidebar'; import IssuableTemplateSelectors from './templates/issuable_template_selectors'; import Flash from './flash'; import CommitsList from './commits'; -import Issue from './issue'; import BindInOut from './behaviors/bind_in_out'; import SecretValues from './behaviors/secret_values'; import Group from './group'; import ProjectsList from './projects_list'; -import setupProjectEdit from './project_edit'; import MiniPipelineGraph from './mini_pipeline_graph_dropdown'; import UserCallout from './user_callout'; import ShortcutsWiki from './shortcuts_wiki'; @@ -58,7 +52,6 @@ import GlFieldErrors from './gl_field_errors'; import GLForm from './gl_form'; import Shortcuts from './shortcuts'; import ShortcutsNavigation from './shortcuts_navigation'; -import ShortcutsFindFile from './shortcuts_find_file'; import ShortcutsIssuable from './shortcuts_issuable'; import U2FAuthenticate from './u2f/authenticate'; import Members from './members'; @@ -68,6 +61,7 @@ import Diff from './diff'; import ProjectLabelSubscription from './project_label_subscription'; import SearchAutocomplete from './search_autocomplete'; import Activities from './activities'; +import { fetchCommitMergeRequests } from './commit_merge_requests'; (function() { var Dispatcher; @@ -121,22 +115,22 @@ import Activities from './activities'; shortcut_handler = true; break; case 'projects:merge_requests:index': + import('./pages/projects/merge_requests/index') + .then(callDefault) + .catch(fail); + shortcut_handler = true; + break; case 'projects:issues:index': - if (filteredSearchEnabled) { - const filteredSearchManager = new gl.FilteredSearchManager(page === 'projects:issues:index' ? 'issues' : 'merge_requests'); - filteredSearchManager.setup(); - } - const pagePrefix = page === 'projects:merge_requests:index' ? 'merge_request_' : 'issue_'; - new IssuableIndex(pagePrefix); - - shortcut_handler = new ShortcutsNavigation(); - new UsersSelect(); + import('./pages/projects/issues/index') + .then(callDefault) + .catch(fail); + shortcut_handler = true; break; case 'projects:issues:show': - new Issue(); - shortcut_handler = new ShortcutsIssuable(); - new ZenMode(); - initIssuableSidebar(); + import('./pages/projects/issues/show') + .then(callDefault) + .catch(fail); + shortcut_handler = true; break; case 'dashboard:milestones:index': import('./pages/dashboard/milestones/index') @@ -212,8 +206,14 @@ import Activities from './activities'; initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight - paddingTop); break; case 'projects:branches:new': + import('./pages/projects/branches/new') + .then(callDefault) + .catch(fail); + break; case 'projects:branches:create': - new NewBranchForm($('.js-create-branch-form'), JSON.parse(document.getElementById('availableRefs').innerHTML)); + import('./pages/projects/branches/new') + .then(callDefault) + .catch(fail); break; case 'projects:branches:index': import('./pages/projects/branches/index') @@ -221,13 +221,16 @@ import Activities from './activities'; .catch(fail); break; case 'projects:issues:new': + import('./pages/projects/issues/new') + .then(callDefault) + .catch(fail); + shortcut_handler = true; + break; case 'projects:issues:edit': - shortcut_handler = new ShortcutsNavigation(); - new GLForm($('.issue-form'), true); - new IssuableForm($('.issue-form')); - new LabelsSelect(); - new MilestoneSelect(); - new IssuableTemplateSelectors(); + import('./pages/projects/issues/edit') + .then(callDefault) + .catch(fail); + shortcut_handler = true; break; case 'projects:merge_requests:creations:new': const mrNewCompareNode = document.querySelector('.js-merge-request-new-compare'); @@ -271,11 +274,24 @@ import Activities from './activities'; new ZenMode(); break; case 'snippets:new': + import('./pages/snippets/new') + .then(callDefault) + .catch(fail); + break; case 'snippets:edit': + import('./pages/snippets/edit') + .then(callDefault) + .catch(fail); + break; case 'snippets:create': + import('./pages/snippets/new') + .then(callDefault) + .catch(fail); + break; case 'snippets:update': - new GLForm($('.snippet-form'), false); - new ZenMode(); + import('./pages/snippets/edit') + .then(callDefault) + .catch(fail); break; case 'projects:releases:edit': new ZenMode(); @@ -311,6 +327,7 @@ import Activities from './activities'; const stickyBarPaddingTop = 16; initChangesDropdown(document.querySelector('.navbar-gitlab').offsetHeight - stickyBarPaddingTop); $('.commit-info.branches').load(document.querySelector('.js-commit-box').dataset.commitPath); + fetchCommitMergeRequests(); break; case 'projects:commit:pipelines': new MiniPipelineGraph({ @@ -345,12 +362,14 @@ import Activities from './activities'; }); break; case 'projects:edit': - setupProjectEdit(); - // Initialize expandable settings panels - initSettingsPanels(); + import('./pages/projects/edit') + .then(callDefault) + .catch(fail); break; case 'projects:imports:show': - projectImport(); + import('./pages/projects/imports/show') + .then(callDefault) + .catch(fail); break; case 'projects:pipelines:new': case 'projects:pipelines:create': @@ -419,13 +438,9 @@ import Activities from './activities'; }); break; case 'projects:find_file:show': - const findElement = document.querySelector('.js-file-finder'); - const projectFindFile = new ProjectFindFile($(".file-finder-holder"), { - url: findElement.dataset.fileFindUrl, - treeUrl: findElement.dataset.findTreeUrl, - blobUrlTemplate: findElement.dataset.blobUrlTemplate, - }); - new ShortcutsFindFile(projectFindFile); + import('./pages/projects/find_file/show') + .then(callDefault) + .catch(fail); shortcut_handler = true; break; case 'projects:blob:show': @@ -442,12 +457,24 @@ import Activities from './activities'; break; case 'groups:labels:new': case 'groups:labels:edit': + new Labels(); + break; case 'projects:labels:new': + import('./pages/projects/labels/new') + .then(callDefault) + .catch(fail); + break; case 'projects:labels:edit': - new Labels(); + import('./pages/projects/labels/edit') + .then(callDefault) + .catch(fail); break; - case 'groups:labels:index': case 'projects:labels:index': + import('./pages/projects/labels/index') + .then(callDefault) + .catch(fail); + break; + case 'groups:labels:index': if ($('.prioritized-labels').length) { new LabelManager(); } @@ -467,7 +494,7 @@ import Activities from './activities'; shortcut_handler = true; break; case 'projects:forks:new': - import(/* webpackChunkName: 'project_fork' */ './project_fork') + import('./pages/projects/forks/new') .then(callDefault) .catch(fail); break; @@ -524,7 +551,9 @@ import Activities from './activities'; import('./pages/admin/conversational_development_index/show').then(m => m.default()).catch(fail); break; case 'snippets:show': - import('./pages/snippets/show').then(m => m.default()).catch(fail); + import('./pages/snippets/show') + .then(callDefault) + .catch(fail); break; case 'import:fogbugz:new_user_map': import('./pages/import/fogbugz/new_user_map').then(m => m.default()).catch(fail); diff --git a/app/assets/javascripts/init_labels.js b/app/assets/javascripts/init_labels.js new file mode 100644 index 00000000000..5f20055510f --- /dev/null +++ b/app/assets/javascripts/init_labels.js @@ -0,0 +1,18 @@ +import LabelManager from './label_manager'; +import GroupLabelSubscription from './group_label_subscription'; +import ProjectLabelSubscription from './project_label_subscription'; + +export default () => { + if ($('.prioritized-labels').length) { + new LabelManager(); // eslint-disable-line no-new + } + $('.label-subscription').each((i, el) => { + const $el = $(el); + + if ($el.find('.dropdown-group-label').length) { + new GroupLabelSubscription($el); // eslint-disable-line no-new + } else { + new ProjectLabelSubscription($el); // eslint-disable-line no-new + } + }); +}; diff --git a/app/assets/javascripts/pages/projects/branches/new/index.js b/app/assets/javascripts/pages/projects/branches/new/index.js new file mode 100644 index 00000000000..ae5e033e97e --- /dev/null +++ b/app/assets/javascripts/pages/projects/branches/new/index.js @@ -0,0 +1,3 @@ +import NewBranchForm from '~/new_branch_form'; + +export default () => new NewBranchForm($('.js-create-branch-form'), JSON.parse(document.getElementById('availableRefs').innerHTML)); diff --git a/app/assets/javascripts/pages/projects/edit/index.js b/app/assets/javascripts/pages/projects/edit/index.js new file mode 100644 index 00000000000..7f662ef6b6a --- /dev/null +++ b/app/assets/javascripts/pages/projects/edit/index.js @@ -0,0 +1,8 @@ +import initSettingsPanels from '~/settings_panels'; +import setupProjectEdit from '~/project_edit'; + +export default () => { + setupProjectEdit(); + // Initialize expandable settings panels + initSettingsPanels(); +}; diff --git a/app/assets/javascripts/pages/projects/find_file/show/index.js b/app/assets/javascripts/pages/projects/find_file/show/index.js new file mode 100644 index 00000000000..42bde0ff779 --- /dev/null +++ b/app/assets/javascripts/pages/projects/find_file/show/index.js @@ -0,0 +1,12 @@ +import ProjectFindFile from '~/project_find_file'; +import ShortcutsFindFile from '~/shortcuts_find_file'; + +export default () => { + const findElement = document.querySelector('.js-file-finder'); + const projectFindFile = new ProjectFindFile($('.file-finder-holder'), { + url: findElement.dataset.fileFindUrl, + treeUrl: findElement.dataset.findTreeUrl, + blobUrlTemplate: findElement.dataset.blobUrlTemplate, + }); + new ShortcutsFindFile(projectFindFile); // eslint-disable-line no-new +}; diff --git a/app/assets/javascripts/pages/projects/forks/new/index.js b/app/assets/javascripts/pages/projects/forks/new/index.js new file mode 100644 index 00000000000..7825eb01949 --- /dev/null +++ b/app/assets/javascripts/pages/projects/forks/new/index.js @@ -0,0 +1,5 @@ +import ProjectFork from '~/project_fork'; + +export default () => { + new ProjectFork(); // eslint-disable-line no-new +}; diff --git a/app/assets/javascripts/pages/projects/imports/show/index.js b/app/assets/javascripts/pages/projects/imports/show/index.js new file mode 100644 index 00000000000..378f7b3f38b --- /dev/null +++ b/app/assets/javascripts/pages/projects/imports/show/index.js @@ -0,0 +1,5 @@ +import ProjectImport from '~/project_import'; + +export default () => { + new ProjectImport(); // eslint-disable-line no-new +}; diff --git a/app/assets/javascripts/pages/projects/issues/edit/index.js b/app/assets/javascripts/pages/projects/issues/edit/index.js new file mode 100644 index 00000000000..7f27f379d8c --- /dev/null +++ b/app/assets/javascripts/pages/projects/issues/edit/index.js @@ -0,0 +1,5 @@ +import initForm from '../form'; + +export default () => { + initForm(); +}; diff --git a/app/assets/javascripts/pages/projects/issues/form.js b/app/assets/javascripts/pages/projects/issues/form.js new file mode 100644 index 00000000000..5c7daf84738 --- /dev/null +++ b/app/assets/javascripts/pages/projects/issues/form.js @@ -0,0 +1,16 @@ +/* eslint-disable no-new */ +import GLForm from '~/gl_form'; +import IssuableForm from '~/issuable_form'; +import LabelsSelect from '~/labels_select'; +import MilestoneSelect from '~/milestone_select'; +import ShortcutsNavigation from '~/shortcuts_navigation'; +import IssuableTemplateSelectors from '~/templates/issuable_template_selectors'; + +export default () => { + new ShortcutsNavigation(); + new GLForm($('.issue-form'), true); + new IssuableForm($('.issue-form')); + new LabelsSelect(); + new MilestoneSelect(); + new IssuableTemplateSelectors(); +}; diff --git a/app/assets/javascripts/pages/projects/issues/index/index.js b/app/assets/javascripts/pages/projects/issues/index/index.js new file mode 100644 index 00000000000..fd395a45f00 --- /dev/null +++ b/app/assets/javascripts/pages/projects/issues/index/index.js @@ -0,0 +1,18 @@ + +/* eslint-disable no-new */ + +import IssuableIndex from '~/issuable_index'; +import ShortcutsNavigation from '~/shortcuts_navigation'; +import UsersSelect from '~/users_select'; + +export default () => { + const filteredSearchEnabled = gl.FilteredSearchManager && document.querySelector('.filtered-search'); + if (filteredSearchEnabled) { + const filteredSearchManager = new gl.FilteredSearchManager('issues'); + filteredSearchManager.setup(); + } + new IssuableIndex('issue_'); + + new ShortcutsNavigation(); + new UsersSelect(); +}; diff --git a/app/assets/javascripts/pages/projects/issues/new/index.js b/app/assets/javascripts/pages/projects/issues/new/index.js new file mode 100644 index 00000000000..7f27f379d8c --- /dev/null +++ b/app/assets/javascripts/pages/projects/issues/new/index.js @@ -0,0 +1,5 @@ +import initForm from '../form'; + +export default () => { + initForm(); +}; diff --git a/app/assets/javascripts/pages/projects/issues/show/index.js b/app/assets/javascripts/pages/projects/issues/show/index.js new file mode 100644 index 00000000000..48ed8fb2243 --- /dev/null +++ b/app/assets/javascripts/pages/projects/issues/show/index.js @@ -0,0 +1,13 @@ + +/* eslint-disable no-new */ +import initIssuableSidebar from '~/init_issuable_sidebar'; +import Issue from '~/issue'; +import ShortcutsIssuable from '~/shortcuts_issuable'; +import ZenMode from '~/zen_mode'; + +export default () => { + new Issue(); + new ShortcutsIssuable(); + new ZenMode(); + initIssuableSidebar(); +}; diff --git a/app/assets/javascripts/pages/projects/labels/edit/index.js b/app/assets/javascripts/pages/projects/labels/edit/index.js new file mode 100644 index 00000000000..72c5e4744ac --- /dev/null +++ b/app/assets/javascripts/pages/projects/labels/edit/index.js @@ -0,0 +1,3 @@ +import Labels from '~/labels'; + +export default () => new Labels(); diff --git a/app/assets/javascripts/pages/projects/labels/index/index.js b/app/assets/javascripts/pages/projects/labels/index/index.js new file mode 100644 index 00000000000..018345fa112 --- /dev/null +++ b/app/assets/javascripts/pages/projects/labels/index/index.js @@ -0,0 +1,3 @@ +import initLabels from '~/init_labels'; + +export default initLabels; diff --git a/app/assets/javascripts/pages/projects/labels/new/index.js b/app/assets/javascripts/pages/projects/labels/new/index.js new file mode 100644 index 00000000000..72c5e4744ac --- /dev/null +++ b/app/assets/javascripts/pages/projects/labels/new/index.js @@ -0,0 +1,3 @@ +import Labels from '~/labels'; + +export default () => new Labels(); diff --git a/app/assets/javascripts/pages/projects/merge_requests/index/index.js b/app/assets/javascripts/pages/projects/merge_requests/index/index.js new file mode 100644 index 00000000000..a52bea03aa2 --- /dev/null +++ b/app/assets/javascripts/pages/projects/merge_requests/index/index.js @@ -0,0 +1,16 @@ +import IssuableIndex from '~/issuable_index'; +import ShortcutsNavigation from '~/shortcuts_navigation'; +import UsersSelect from '~/users_select'; + +export default () => { + const filteredSearchEnabled = gl.FilteredSearchManager && document.querySelector('.filtered-search'); + + if (filteredSearchEnabled) { + const filteredSearchManager = new gl.FilteredSearchManager('merge_requests'); + filteredSearchManager.setup(); + } + + new IssuableIndex('merge_request_'); // eslint-disable-line no-new + new ShortcutsNavigation(); // eslint-disable-line no-new + new UsersSelect(); // eslint-disable-line no-new +}; diff --git a/app/assets/javascripts/pages/snippets/edit/index.js b/app/assets/javascripts/pages/snippets/edit/index.js new file mode 100644 index 00000000000..9c664b5f1ff --- /dev/null +++ b/app/assets/javascripts/pages/snippets/edit/index.js @@ -0,0 +1,3 @@ +import form from '../form'; + +export default form; diff --git a/app/assets/javascripts/pages/snippets/form.js b/app/assets/javascripts/pages/snippets/form.js new file mode 100644 index 00000000000..f996d3cd74e --- /dev/null +++ b/app/assets/javascripts/pages/snippets/form.js @@ -0,0 +1,7 @@ +import GLForm from '~/gl_form'; +import ZenMode from '~/zen_mode'; + +export default () => { + new GLForm($('.snippet-form'), false); // eslint-disable-line no-new + new ZenMode(); // eslint-disable-line no-new +}; diff --git a/app/assets/javascripts/pages/snippets/new/index.js b/app/assets/javascripts/pages/snippets/new/index.js new file mode 100644 index 00000000000..9c664b5f1ff --- /dev/null +++ b/app/assets/javascripts/pages/snippets/new/index.js @@ -0,0 +1,3 @@ +import form from '../form'; + +export default form; diff --git a/app/assets/javascripts/projects_dropdown/index.js b/app/assets/javascripts/projects_dropdown/index.js index 2660da3c558..e78ebce2923 100644 --- a/app/assets/javascripts/projects_dropdown/index.js +++ b/app/assets/javascripts/projects_dropdown/index.js @@ -19,11 +19,8 @@ document.addEventListener('DOMContentLoaded', () => { return; } - $(navEl).on('show.bs.dropdown', (e) => { - const dropdownEl = $(e.currentTarget).find('.projects-dropdown-menu'); - dropdownEl.one('transitionend', () => { - eventHub.$emit('dropdownOpen'); - }); + $(navEl).on('shown.bs.dropdown', () => { + eventHub.$emit('dropdownOpen'); }); // eslint-disable-next-line no-new diff --git a/app/assets/javascripts/protected_branches/protected_branch_create.js b/app/assets/javascripts/protected_branches/protected_branch_create.js index 0a9fdb074e5..2948baeab11 100644 --- a/app/assets/javascripts/protected_branches/protected_branch_create.js +++ b/app/assets/javascripts/protected_branches/protected_branch_create.js @@ -1,6 +1,6 @@ import _ from 'underscore'; import ProtectedBranchAccessDropdown from './protected_branch_access_dropdown'; -import ProtectedBranchDropdown from './protected_branch_dropdown'; +import CreateItemDropdown from '../create_item_dropdown'; import AccessorUtilities from '../lib/utils/accessor'; const PB_LOCAL_STORAGE_KEY = 'protected-branches-defaults'; @@ -35,10 +35,12 @@ export default class ProtectedBranchCreate { onSelect: this.onSelectCallback, }); - // Protected branch dropdown - this.protectedBranchDropdown = new ProtectedBranchDropdown({ + this.createItemDropdown = new CreateItemDropdown({ $dropdown: $protectedBranchDropdown, + defaultToggleLabel: 'Protected Branch', + fieldName: 'protected_branch[name]', onSelect: this.onSelectCallback, + getData: ProtectedBranchCreate.getProtectedBranches, }); this.loadPreviousSelection($allowedToMergeDropdown.data('glDropdown'), $allowedToPushDropdown.data('glDropdown')); @@ -60,6 +62,10 @@ export default class ProtectedBranchCreate { this.$form.find('input[type="submit"]').attr('disabled', completedForm); } + static getProtectedBranches(term, callback) { + callback(gon.open_branches); + } + loadPreviousSelection(mergeDropdown, pushDropdown) { let mergeIndex = 0; let pushIndex = 0; diff --git a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js b/app/assets/javascripts/protected_branches/protected_branch_dropdown.js deleted file mode 100644 index 678882a8d2c..00000000000 --- a/app/assets/javascripts/protected_branches/protected_branch_dropdown.js +++ /dev/null @@ -1,90 +0,0 @@ -import _ from 'underscore'; - -export default class ProtectedBranchDropdown { - /** - * @param {Object} options containing - * `$dropdown` target element - * `onSelect` event callback - * $dropdown must be an element created using `dropdown_branch()` rails helper - */ - constructor(options) { - this.onSelect = options.onSelect; - this.$dropdown = options.$dropdown; - this.$dropdownContainer = this.$dropdown.parent(); - this.$dropdownFooter = this.$dropdownContainer.find('.dropdown-footer'); - this.$protectedBranch = this.$dropdownContainer.find('.js-create-new-protected-branch'); - - this.buildDropdown(); - this.bindEvents(); - - // Hide footer - this.toggleFooter(true); - } - - buildDropdown() { - this.$dropdown.glDropdown({ - data: this.getProtectedBranches.bind(this), - filterable: true, - remote: false, - search: { - fields: ['title'], - }, - selectable: true, - toggleLabel(selected) { - return (selected && 'id' in selected) ? selected.title : 'Protected Branch'; - }, - fieldName: 'protected_branch[name]', - text(protectedBranch) { - return _.escape(protectedBranch.title); - }, - id(protectedBranch) { - return _.escape(protectedBranch.id); - }, - onFilter: this.toggleCreateNewButton.bind(this), - clicked: (options) => { - options.e.preventDefault(); - this.onSelect(); - }, - }); - } - - bindEvents() { - this.$protectedBranch.on('click', this.onClickCreateWildcard.bind(this)); - } - - onClickCreateWildcard(e) { - e.preventDefault(); - - // Refresh the dropdown's data, which ends up calling `getProtectedBranches` - this.$dropdown.data('glDropdown').remote.execute(); - this.$dropdown.data('glDropdown').selectRowAtIndex(); - } - - getProtectedBranches(term, callback) { - if (this.selectedBranch) { - callback(gon.open_branches.concat(this.selectedBranch)); - } else { - callback(gon.open_branches); - } - } - - toggleCreateNewButton(branchName) { - if (branchName) { - this.selectedBranch = { - title: branchName, - id: branchName, - text: branchName, - }; - - this.$dropdownContainer - .find('.js-create-new-protected-branch code') - .text(branchName); - } - - this.toggleFooter(!branchName); - } - - toggleFooter(toggleState) { - this.$dropdownFooter.toggleClass('hidden', toggleState); - } -} diff --git a/app/assets/javascripts/protected_tags/protected_tag_create.js b/app/assets/javascripts/protected_tags/protected_tag_create.js index 91bd140bd12..d1e4a75c17b 100644 --- a/app/assets/javascripts/protected_tags/protected_tag_create.js +++ b/app/assets/javascripts/protected_tags/protected_tag_create.js @@ -1,5 +1,5 @@ import ProtectedTagAccessDropdown from './protected_tag_access_dropdown'; -import ProtectedTagDropdown from './protected_tag_dropdown'; +import CreateItemDropdown from '../create_item_dropdown'; export default class ProtectedTagCreate { constructor() { @@ -24,9 +24,12 @@ export default class ProtectedTagCreate { $allowedToCreateDropdown.data('glDropdown').selectRowAtIndex(0); // Protected tag dropdown - this.protectedTagDropdown = new ProtectedTagDropdown({ + this.createItemDropdown = new CreateItemDropdown({ $dropdown: this.$form.find('.js-protected-tag-select'), + defaultToggleLabel: 'Protected Tag', + fieldName: 'protected_tag[name]', onSelect: this.onSelectCallback, + getData: ProtectedTagCreate.getProtectedTags, }); } @@ -38,4 +41,8 @@ export default class ProtectedTagCreate { this.$form.find('input[type="submit"]').attr('disabled', !($tagInput.val() && $allowedToCreateInput.length)); } + + static getProtectedTags(term, callback) { + callback(gon.open_tags); + } } diff --git a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue index 7226076a8fc..d69d100a26c 100644 --- a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue +++ b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue @@ -1,12 +1,22 @@ <script> - /* eslint-disable vue/require-default-prop */ - import { __ } from '../../../locale'; + import { __ } from '~/locale'; + import icon from '~/vue_shared/components/icon.vue'; + import toggleButton from '~/vue_shared/components/toggle_button.vue'; + import tooltip from '~/vue_shared/directives/tooltip'; import eventHub from '../../event_hub'; - import loadingButton from '../../../vue_shared/components/loading_button.vue'; + + const ICON_ON = 'notifications'; + const ICON_OFF = 'notifications-off'; + const LABEL_ON = __('Notifications on'); + const LABEL_OFF = __('Notifications off'); export default { + directives: { + tooltip, + }, components: { - loadingButton, + icon, + toggleButton, }, props: { loading: { @@ -17,22 +27,23 @@ subscribed: { type: Boolean, required: false, + default: null, }, id: { type: Number, required: false, + default: null, }, }, computed: { - buttonLabel() { - let label; - if (this.subscribed === false) { - label = __('Subscribe'); - } else if (this.subscribed === true) { - label = __('Unsubscribe'); - } - - return label; + showLoadingState() { + return this.subscribed === null; + }, + notificationIcon() { + return this.subscribed ? ICON_ON : ICON_OFF; + }, + notificationTooltip() { + return this.subscribed ? LABEL_ON : LABEL_OFF; }, }, methods: { @@ -46,21 +57,29 @@ <template> <div> <div class="sidebar-collapsed-icon"> - <i - class="fa fa-rss" - aria-hidden="true" + <span + v-tooltip + :title="notificationTooltip" + data-container="body" + data-placement="left" > - </i> + <icon + :name="notificationIcon" + :size="16" + aria-hidden="true" + class="sidebar-item-icon is-active" + /> + </span> </div> <span class="issuable-header-text hide-collapsed pull-left"> {{ __('Notifications') }} </span> - <loading-button - ref="loadingButton" - class="btn btn-default pull-right hide-collapsed js-issuable-subscribe-button" - :loading="loading" - :label="buttonLabel" - @click="toggleSubscription" + <toggle-button + ref="toggleButton" + class="pull-right hide-collapsed js-issuable-subscribe-button" + :is-loading="showLoadingState" + :value="subscribed" + @change="toggleSubscription" /> </div> </template> diff --git a/app/assets/javascripts/vue_shared/components/toggle_button.vue b/app/assets/javascripts/vue_shared/components/toggle_button.vue index 2b12718ae96..09031d3ffa1 100644 --- a/app/assets/javascripts/vue_shared/components/toggle_button.vue +++ b/app/assets/javascripts/vue_shared/components/toggle_button.vue @@ -23,11 +23,12 @@ name: { type: String, required: false, - default: '', + default: null, }, value: { type: Boolean, - required: true, + required: false, + default: null, }, disabledInput: { type: Boolean, @@ -61,6 +62,7 @@ <template> <label class="toggle-wrapper"> <input + v-if="name" type="hidden" :name="name" :value="value" diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index d1b3754d4ef..1d2303a3a2b 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -666,6 +666,16 @@ } } +.dropdown-create-new-item-button { + @include dropdown-link; + + width: 100%; + background-color: transparent; + border: 0; + text-align: left; + text-overflow: ellipsis; +} + .dropdown-loading { position: absolute; top: 0; diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index ad160f37641..3b7256f3000 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -104,7 +104,10 @@ img { height: 28px; - margin-right: 8px; + + + .logo-text { + margin-left: 8px; + } } &.wrap { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index ae9a8b0182c..759719a72da 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -162,10 +162,6 @@ border: 0; } - span { - display: inline-block; - } - .select2-container span { margin-top: 0; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 61a76d0387a..bf41005b6d5 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -895,17 +895,6 @@ pre.light-well { } } -.create-new-protected-branch-button, -.create-new-protected-tag-button { - @include dropdown-link; - - width: 100%; - background-color: transparent; - border: 0; - text-align: left; - text-overflow: ellipsis; -} - .protected-branches-list, .protected-tags-list { margin-bottom: 30px; diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 2e7344b1cad..effb484ef0f 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -12,7 +12,7 @@ class Projects::CommitController < Projects::ApplicationController before_action :authorize_download_code! before_action :authorize_read_pipeline!, only: [:pipelines] before_action :commit - before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines] + before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines, :merge_requests] before_action :define_note_vars, only: [:show, :diff_for_path] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] @@ -52,6 +52,18 @@ class Projects::CommitController < Projects::ApplicationController end end + def merge_requests + @merge_requests = @commit.merge_requests.map do |mr| + { iid: mr.iid, path: merge_request_path(mr), title: mr.title } + end + + respond_to do |format| + format.json do + render json: @merge_requests.to_json + end + end + end + def branches # branch_names_contains/tag_names_contains can take a long time when there are thousands of # branches/tags - each `git branch --contains xxx` request can consume a cpu core. diff --git a/app/models/commit.rb b/app/models/commit.rb index 39d7f5b159d..21904c87f01 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -238,6 +238,10 @@ class Commit notes.includes(:author) end + def merge_requests + @merge_requests ||= project.merge_requests.by_commit_sha(sha) + end + def method_missing(method, *args, &block) @raw.__send__(method, *args, &block) # rubocop:disable GitlabSecurity/PublicSend end @@ -342,10 +346,11 @@ class Commit @merged_merge_request_hash[current_user] end - def has_been_reverted?(current_user, noteable = self) + def has_been_reverted?(current_user, notes_association = nil) ext = all_references(current_user) + notes_association ||= notes_with_associations - noteable.notes_with_associations.system.each do |note| + notes_association.system.each do |note| note.all_references(current_user, extractor: ext) end diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb index b6c7b6735b9..7c236369793 100644 --- a/app/models/concerns/resolvable_discussion.rb +++ b/app/models/concerns/resolvable_discussion.rb @@ -1,5 +1,6 @@ module ResolvableDiscussion extend ActiveSupport::Concern + include ::Gitlab::Utils::StrongMemoize included do # A number of properties of this `Discussion`, like `first_note` and `resolvable?`, are memoized. @@ -31,27 +32,37 @@ module ResolvableDiscussion end def resolvable? - @resolvable ||= potentially_resolvable? && notes.any?(&:resolvable?) + strong_memoize(:resolvable) do + potentially_resolvable? && notes.any?(&:resolvable?) + end end def resolved? - @resolved ||= resolvable? && notes.none?(&:to_be_resolved?) + strong_memoize(:resolved) do + resolvable? && notes.none?(&:to_be_resolved?) + end end def first_note - @first_note ||= notes.first + strong_memoize(:first_note) do + notes.first + end end def first_note_to_resolve return unless resolvable? - @first_note_to_resolve ||= notes.find(&:to_be_resolved?) # rubocop:disable Gitlab/ModuleWithInstanceVariables + strong_memoize(:first_note_to_resolve) do + notes.find(&:to_be_resolved?) + end end def last_resolved_note return unless resolved? - @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last # rubocop:disable Gitlab/ModuleWithInstanceVariables + strong_memoize(:last_resolved_note) do + resolved_notes.sort_by(&:resolved_at).last + end end def resolved_notes @@ -93,8 +104,8 @@ module ResolvableDiscussion # Set the notes array to the updated notes @notes = notes_relation.fresh.to_a # rubocop:disable Gitlab/ModuleWithInstanceVariables - self.class.memoized_values.each do |var| - instance_variable_set(:"@#{var}", nil) + self.class.memoized_values.each do |name| + clear_memoization(name) end end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2669d2a6ff3..8028ff3875b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -140,7 +140,9 @@ class MergeRequest < ActiveRecord::Base scope :merged, -> { with_state(:merged) } scope :closed_and_merged, -> { with_states(:closed, :merged) } scope :from_source_branches, ->(branches) { where(source_branch: branches) } - + scope :by_commit_sha, ->(sha) do + where('EXISTS (?)', MergeRequestDiff.select(1).where('merge_requests.latest_merge_request_diff_id = merge_request_diffs.id').by_commit_sha(sha)).reorder(nil) + end scope :join_project, -> { joins(:target_project) } scope :references_project, -> { references(:target_project) } scope :assigned, -> { where("assignee_id IS NOT NULL") } @@ -982,7 +984,16 @@ class MergeRequest < ActiveRecord::Base end def can_be_reverted?(current_user) - merge_commit && !merge_commit.has_been_reverted?(current_user, self) + return false unless merge_commit + + merged_at = metrics&.merged_at + notes_association = notes_with_associations + + if merged_at + notes_association = notes_association.where('created_at > ?', merged_at) + end + + !merge_commit.has_been_reverted?(current_user, notes_association) end def can_be_cherry_picked? diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index afab72930c1..69a846da9be 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -28,6 +28,9 @@ class MergeRequestDiff < ActiveRecord::Base end scope :viewable, -> { without_state(:empty) } + scope :by_commit_sha, ->(sha) do + joins(:merge_request_diff_commits).where(merge_request_diff_commits: { sha: sha }).reorder(nil) + end scope :recent, -> { order(id: :desc).limit(100) } diff --git a/app/models/project.rb b/app/models/project.rb index 029f2da2e4e..7ab7df4fdcd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -20,6 +20,7 @@ class Project < ActiveRecord::Base include GroupDescendant include Gitlab::SQL::Pattern include DeploymentPlatform + include ::Gitlab::Utils::StrongMemoize extend Gitlab::ConfigHelper extend Gitlab::CurrentSettings @@ -993,9 +994,13 @@ class Project < ActiveRecord::Base end def repo_exists? - @repo_exists ||= repository.exists? - rescue - @repo_exists = false + strong_memoize(:repo_exists) do + begin + repository.exists? + rescue + false + end + end end def root_ref?(branch) diff --git a/app/models/repository.rb b/app/models/repository.rb index d27212b2058..2ffd9558ebc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -831,13 +831,12 @@ class Repository end def can_be_merged?(source_sha, target_branch) - our_commit = rugged.branches[target_branch].target - their_commit = rugged.lookup(source_sha) - - if our_commit && their_commit - !rugged.merge_commits(our_commit, their_commit).conflicts? - else - false + raw_repository.gitaly_migrate(:can_be_merged) do |is_enabled| + if is_enabled + gitaly_can_be_merged?(source_sha, find_branch(target_branch).target) + else + rugged_can_be_merged?(source_sha, target_branch) + end end end @@ -895,15 +894,18 @@ class Repository 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 = ancestor?(branch.target, @root_ref_sha) + same_head = branch.target == root_ref_sha + merged = ancestor?(branch.target, root_ref_sha) !same_head && merged else nil end end + def root_ref_sha + @root_ref_sha ||= commit(root_ref).sha + end + delegate :merged_branch_names, to: :raw_repository def merge_base(first_commit_id, second_commit_id) @@ -1129,6 +1131,14 @@ class Repository Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', Gitlab::GlRepository.gl_repository(project, is_wiki)) end + def gitaly_can_be_merged?(their_commit, our_commit) + !raw_repository.gitaly_conflicts_client(our_commit, their_commit).conflicts? + end + + def rugged_can_be_merged?(their_commit, our_commit) + !rugged.merge_commits(our_commit, their_commit).conflicts? + end + def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) ref ||= root_ref diff --git a/app/models/user.rb b/app/models/user.rb index 4484ee9ff4c..09aa5a7b318 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -53,7 +53,10 @@ class User < ActiveRecord::Base serialize :otp_backup_codes, JSON # rubocop:disable Cop/ActiveRecordSerialize devise :lockable, :recoverable, :rememberable, :trackable, - :validatable, :omniauthable, :confirmable, :registerable + :validatable, :omniauthable, :confirmable, :registerable + + BLOCKED_MESSAGE = "Your account has been blocked. Please contact your GitLab " \ + "administrator if you think this is an error.".freeze # Override Devise::Models::Trackable#update_tracked_fields! # to limit database writes to at most once every hour @@ -217,8 +220,7 @@ class User < ActiveRecord::Base end def inactive_message - "Your account has been blocked. Please contact your GitLab " \ - "administrator if you think this is an error." + BLOCKED_MESSAGE end end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 690918b4a00..af6d77ef5e8 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -41,8 +41,11 @@ class SystemHooksService when User data.merge!(user_data(model)) - if event == :rename + case event + when :rename data[:old_username] = model.username_was + when :failed_login + data[:state] = model.state end when ProjectMember data.merge!(project_member_data(model)) diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index a24516355bf..509f559c120 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -4,6 +4,34 @@ %div{ class: container_class } .admin-dashboard.prepend-top-default .row + .col-sm-4 + .info-well.dark-well + .well-segment.well-centered + = link_to admin_projects_path do + %h3.text-center + Projects: + = number_with_delimiter(Project.cached_count) + %hr + = link_to('New project', new_project_path, class: "btn btn-new") + .col-sm-4 + .info-well.dark-well + .well-segment.well-centered + = link_to admin_users_path do + %h3.text-center + Users: + = number_with_delimiter(User.count) + %hr + = link_to 'New user', new_admin_user_path, class: "btn btn-new" + .col-sm-4 + .info-well.dark-well + .well-segment.well-centered + = link_to admin_groups_path do + %h3.text-center + Groups: + = number_with_delimiter(Group.count) + %hr + = link_to 'New group', new_admin_group_path, class: "btn btn-new" + .row .col-md-4 .info-well .well-segment.admin-well.admin-well-statistics @@ -136,34 +164,6 @@ %span.pull-right = Gitlab::Database.version .row - .col-sm-4 - .info-well.dark-well - .well-segment.well-centered - = link_to admin_projects_path do - %h3.text-center - Projects: - = number_with_delimiter(Project.cached_count) - %hr - = link_to('New project', new_project_path, class: "btn btn-new") - .col-sm-4 - .info-well.dark-well - .well-segment.well-centered - = link_to admin_users_path do - %h3.text-center - Users: - = number_with_delimiter(User.count) - %hr - = link_to 'New user', new_admin_user_path, class: "btn btn-new" - .col-sm-4 - .info-well.dark-well - .well-segment.well-centered - = link_to admin_groups_path do - %h3.text-center - Groups: - = number_with_delimiter(Group.count) - %hr - = link_to 'New group', new_admin_group_path, class: "btn btn-new" - .row .col-md-4 .info-well .well-segment.admin-well diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 46727811be4..e7fc83a8d04 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -6,8 +6,10 @@ %h1.title = link_to root_path, title: 'Dashboard', id: 'logo' do = brand_header_logo - %span.logo-text.hidden-xs - = brand_header_logo_type + - logo_text = brand_header_logo_type + - if logo_text.present? + %span.logo-text.hidden-xs + = logo_text - if current_user = render "layouts/nav/dashboard" diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index 0c27b09f7b1..96aae06a9df 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -1,5 +1,5 @@ -- issues = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute -- merge_requests = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute +- issues_count = IssuesFinder.new(current_user, group_id: @group.id, state: 'opened').execute.count +- merge_requests_count = MergeRequestsFinder.new(current_user, group_id: @group.id, state: 'opened', non_archived: true).execute.count .nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll @@ -39,14 +39,14 @@ = sprite_icon('issues') %span.nav-item-name Issues - %span.badge.count= number_with_delimiter(issues.count) + %span.badge.count= number_with_delimiter(issues_count) %ul.sidebar-sub-level-items = nav_link(path: ['groups#issues', 'labels#index', 'milestones#index'], html_options: { class: "fly-out-top-item" } ) do = link_to issues_group_path(@group) do %strong.fly-out-top-item-name #{ _('Issues') } - %span.badge.count.issue_counter.fly-out-badge= number_with_delimiter(issues.count) + %span.badge.count.issue_counter.fly-out-badge= number_with_delimiter(issues_count) %li.divider.fly-out-top-item = nav_link(path: 'groups#issues', html_options: { class: 'home' }) do = link_to issues_group_path(@group), title: 'List' do @@ -69,13 +69,13 @@ = sprite_icon('git-merge') %span.nav-item-name Merge Requests - %span.badge.count= number_with_delimiter(merge_requests.count) + %span.badge.count= number_with_delimiter(merge_requests_count) %ul.sidebar-sub-level-items.is-fly-out-only = nav_link(path: 'groups#merge_requests', html_options: { class: "fly-out-top-item" } ) do = link_to merge_requests_group_path(@group) do %strong.fly-out-top-item-name #{ _('Merge Requests') } - %span.badge.count.merge_counter.js-merge-counter.fly-out-badge= number_with_delimiter(merge_requests.count) + %span.badge.count.merge_counter.js-merge-counter.fly-out-badge= number_with_delimiter(merge_requests_count) = nav_link(path: 'group_members#index') do = link_to group_group_members_path(@group) do .nav-icon-container diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 09934c09865..461129a3e0e 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -64,6 +64,12 @@ .commit-info.branches %i.fa.fa-spinner.fa-spin + .well-segment.merge-request-info + .icon-container + = custom_icon('mr_bold') + %span.commit-info.merge-requests{ 'data-project-commit-path' => merge_requests_project_commit_path(@project, @commit.id, format: :json) } + = icon('spinner spin') + - if @commit.last_pipeline - last_pipeline = @commit.last_pipeline .well-segment.pipeline-info diff --git a/app/views/projects/protected_branches/shared/_dropdown.html.haml b/app/views/projects/protected_branches/shared/_dropdown.html.haml index 6e9c473494e..74435236808 100644 --- a/app/views/projects/protected_branches/shared/_dropdown.html.haml +++ b/app/views/projects/protected_branches/shared/_dropdown.html.haml @@ -10,6 +10,6 @@ %ul.dropdown-footer-list %li - %button{ class: "create-new-protected-branch-button js-create-new-protected-branch", title: "New Protected Branch" } + %button{ class: "dropdown-create-new-item-button js-dropdown-create-new-item", title: "New Protected Branch" } Create wildcard %code diff --git a/app/views/projects/protected_tags/shared/_dropdown.html.haml b/app/views/projects/protected_tags/shared/_dropdown.html.haml index 9b6923210f7..f0d7dcccd36 100644 --- a/app/views/projects/protected_tags/shared/_dropdown.html.haml +++ b/app/views/projects/protected_tags/shared/_dropdown.html.haml @@ -10,6 +10,6 @@ %ul.dropdown-footer-list %li - %button{ class: "create-new-protected-tag-button js-create-new-protected-tag", title: "New Protected Tag" } + %button{ class: "dropdown-create-new-item-button js-dropdown-create-new-item", title: "New Protected Tag" } Create wildcard %code diff --git a/changelogs/unreleased/40492-update-admin-dashboard-content-order.yml b/changelogs/unreleased/40492-update-admin-dashboard-content-order.yml new file mode 100644 index 00000000000..2416b15b6d5 --- /dev/null +++ b/changelogs/unreleased/40492-update-admin-dashboard-content-order.yml @@ -0,0 +1,5 @@ +--- +title: Move row containing Projects, Users and Groups count to the top in admin dashboard +merge_request: 16421 +author: +type: changed diff --git a/changelogs/unreleased/41546-count-query-for-issues-and-mrs-runs-twice-on-group-index.yml b/changelogs/unreleased/41546-count-query-for-issues-and-mrs-runs-twice-on-group-index.yml new file mode 100644 index 00000000000..7e42dc20ae8 --- /dev/null +++ b/changelogs/unreleased/41546-count-query-for-issues-and-mrs-runs-twice-on-group-index.yml @@ -0,0 +1,5 @@ +--- +title: Fix double query execution on groups page +merge_request: 16314 +author: +type: performance diff --git a/changelogs/unreleased/41731-predicate-memoization.yml b/changelogs/unreleased/41731-predicate-memoization.yml new file mode 100644 index 00000000000..110f78063f4 --- /dev/null +++ b/changelogs/unreleased/41731-predicate-memoization.yml @@ -0,0 +1,5 @@ +--- +title: Properly memoize some predicate methods +merge_request: 16329 +author: +type: performance diff --git a/changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml b/changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml new file mode 100644 index 00000000000..146ae12afbd --- /dev/null +++ b/changelogs/unreleased/41807-15665-consistently-502s-because-it-fetches-every-commit.yml @@ -0,0 +1,6 @@ +--- +title: Speed up loading merged merge requests when they contained a lot of commits + before merging +merge_request: 16320 +author: +type: performance diff --git a/changelogs/unreleased/display-mr-in-commit-page.yml b/changelogs/unreleased/display-mr-in-commit-page.yml new file mode 100644 index 00000000000..a9224c00b66 --- /dev/null +++ b/changelogs/unreleased/display-mr-in-commit-page.yml @@ -0,0 +1,5 @@ +--- +title: Add link on commit page to merge request that introduced that commit +merge_request: 13713 +author: Hiroyuki Sato +type: added diff --git a/changelogs/unreleased/fix_gitlab-ce-41891.yml b/changelogs/unreleased/fix_gitlab-ce-41891.yml new file mode 100644 index 00000000000..56bdc1a7c32 --- /dev/null +++ b/changelogs/unreleased/fix_gitlab-ce-41891.yml @@ -0,0 +1,5 @@ +--- +title: 'Fix custom header logo design nitpick: Remove unneeded margin on empty logo text' +merge_request: 16383 +author: Markus Doits +type: fixed diff --git a/changelogs/unreleased/fj-41598-fixing-request-mime-type.yml b/changelogs/unreleased/fj-41598-fixing-request-mime-type.yml new file mode 100644 index 00000000000..85e4d78b2df --- /dev/null +++ b/changelogs/unreleased/fj-41598-fixing-request-mime-type.yml @@ -0,0 +1,5 @@ +--- +title: Fixing rack request mime type when using rack attack +merge_request: 16427 +author: +type: fixed diff --git a/changelogs/unreleased/sh-log-when-user-blocked.yml b/changelogs/unreleased/sh-log-when-user-blocked.yml new file mode 100644 index 00000000000..9abf2017514 --- /dev/null +++ b/changelogs/unreleased/sh-log-when-user-blocked.yml @@ -0,0 +1,5 @@ +--- +title: Log and send a system hook if a blocked user attempts to login +merge_request: +author: +type: added diff --git a/config/initializers/warden.rb b/config/initializers/warden.rb index 3d83fb92d56..ee034d21eae 100644 --- a/config/initializers/warden.rb +++ b/config/initializers/warden.rb @@ -2,4 +2,8 @@ Rails.application.configure do |config| Warden::Manager.after_set_user do |user, auth, opts| Gitlab::Auth::UniqueIpsLimiter.limit_user!(user) end + + Warden::Manager.before_failure do |env, opts| + Gitlab::Auth::BlockedUserTracker.log_if_user_blocked(env) + end end diff --git a/config/routes/project.rb b/config/routes/project.rb index bdf4b199c0a..43ada9ba145 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -50,6 +50,7 @@ constraints(ProjectUrlConstrainer.new) do post :revert post :cherry_pick get :diff_for_path + get :merge_requests end end diff --git a/db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb b/db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb new file mode 100644 index 00000000000..1b360b231a8 --- /dev/null +++ b/db/migrate/20170827123848_add_index_on_merge_request_diff_commit_sha.rb @@ -0,0 +1,17 @@ +# rubocop:disable RemoveIndex + +class AddIndexOnMergeRequestDiffCommitSha < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_request_diff_commits, :sha, length: Gitlab::Database.mysql? ? 20 : nil + end + + def down + remove_index :merge_request_diff_commits, :sha if index_exists? :merge_request_diff_commits, :sha + end +end diff --git a/db/schema.rb b/db/schema.rb index 8a6db61250b..f47accca21a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1013,6 +1013,7 @@ ActiveRecord::Schema.define(version: 20180105212544) do end add_index "merge_request_diff_commits", ["merge_request_diff_id", "relative_order"], name: "index_merge_request_diff_commits_on_mr_diff_id_and_order", unique: true, using: :btree + add_index "merge_request_diff_commits", ["sha"], name: "index_merge_request_diff_commits_on_sha", using: :btree create_table "merge_request_diff_files", id: false, force: :cascade do |t| t.integer "merge_request_diff_id", null: false diff --git a/doc/articles/runner_autoscale_aws/index.md b/doc/articles/runner_autoscale_aws/index.md index 9d4c4a57ce5..e2667aebc5f 100644 --- a/doc/articles/runner_autoscale_aws/index.md +++ b/doc/articles/runner_autoscale_aws/index.md @@ -1,410 +1 @@ ---- -last_updated: 2017-11-24 ---- - -> **[Article Type](../../development/writing_documentation.html#types-of-technical-articles):** Admin guide || -> **Level:** intermediary || -> **Author:** [Achilleas Pipinellis](https://gitlab.com/axil) || -> **Publication date:** 2017/11/24 - -# Autoscaling GitLab Runner on AWS - -One of the biggest advantages of GitLab Runner is its ability to automatically -spin up and down VMs to make sure your builds get processed immediately. It's a -great feature, and if used correctly, it can be extremely useful in situations -where you don't use your Runners 24/7 and want to have a cost-effective and -scalable solution. - -## Introduction - -In this tutorial, we'll explore how to properly configure a GitLab Runner in -AWS that will serve as the bastion where it will spawn new Docker machines on -demand. - -In addition, we'll make use of [Amazon's EC2 Spot instances][spot] which will -greatly reduce the costs of the Runner instances while still using quite -powerful autoscaling machines. - -## Prerequisites - -NOTE: **Note:** -A familiarity with Amazon Web Services (AWS) is required as this is where most -of the configuration will take place. - -Your GitLab instance is going to need to talk to the Runners over the network, -and that is something you need think about when configuring any AWS security -groups or when setting up your DNS configuration. - -For example, you can keep the EC2 resources segmented away from public traffic -in a different VPC to better strengthen your network security. Your environment -is likely different, so consider what works best for your situation. - -### AWS security groups - -Docker Machine will attempt to use a -[default security group](https://docs.docker.com/machine/drivers/aws/#security-group) -with rules for port `2376`, which is required for communication with the Docker -daemon. Instead of relying on Docker, you can create a security group with the -rules you need and provide that in the Runner options as we will -[see below](#the-runners-machine-section). This way, you can customize it to your -liking ahead of time based on your networking environment. - -### AWS credentials - -You'll need an [AWS Access Key](https://docs.aws.amazon.com/general/latest/gr/managing-aws-access-keys.html) -tied to a user with permission to scale (EC2) and update the cache (via S3). -Create a new user with [policies](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/iam-policies-for-amazon-ec2.html) -for EC2 (AmazonEC2FullAccess) and S3 (AmazonS3FullAccess). To be more secure, -you can disable console login for that user. Keep the tab open or copy paste the -security credentials in an editor as we'll use them later during the -[Runner configuration](#the-runners-machine-section). - -## Prepare the bastion instance - -The first step is to install GitLab Runner in an EC2 instance that will serve -as the bastion that spawns new machines. This doesn't have to be a powerful -machine since it will not run any jobs itself, a `t2.micro` instance will do. -This machine will be a dedicated host since we need it always up and running, -thus it will be the only standard cost. - -NOTE: **Note:** -For the bastion instance, choose a distribution that both Docker and GitLab -Runner support, for example either Ubuntu, Debian, CentOS or RHEL will work fine. - -Install the prerequisites: - -1. Log in to your server -1. [Install GitLab Runner from the official GitLab repository](https://docs.gitlab.com/runner/install/linux-repository.html) -1. [Install Docker](https://docs.docker.com/engine/installation/#server) -1. [Install Docker Machine](https://docs.docker.com/machine/install-machine/) - -Now that the Runner is installed, it's time to register it. - -## Registering the GitLab Runner - -Before configuring the GitLab Runner, you need to first register it, so that -it connects with your GitLab instance: - -1. [Obtain a Runner token](../../ci/runners/README.md) -1. [Register the Runner](https://docs.gitlab.com/runner/register/index.html#gnu-linux) -1. When asked the executor type, enter `docker+machine` - -You can now move on to the most important part, configuring the GitLab Runner. - -TIP: **Tip:** -If you want every user in your instance to be able to use the autoscaled Runners, -register the Runner as a shared one. - -## Configuring the GitLab Runner - -Now that the Runner is registered, you need to edit its configuration file and -add the required options for the AWS machine driver. - -Let's first break it down to pieces. - -### The global section - -In the global section, you can define the limit of the jobs that can be run -concurrently across all Runners (`concurrent`). This heavily depends on your -needs, like how many users your Runners will accommodate, how much time your -builds take, etc. You can start with something low like `10`, and increase or -decrease its value going forward. - -The `check_interval` option defines how often the Runner should check GitLab -for new jobs, in seconds. - -Example: - -```toml -concurrent = 10 -check_interval = 0 -``` - -[Read more](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-global-section) -about all the options you can use. - -### The `runners` section - -From the `[[runners]]` section, the most important part is the `executor` which -must be set to `docker+machine`. Most of those settings are taken care of when -you register the Runner for the first time. - -`limit` sets the maximum number of machines (running and idle) that this Runner -will spawn. For more info check the [relationship between `limit`, `concurrent` -and `IdleCount`](https://docs.gitlab.com/runner/configuration/autoscale.html#how-concurrent-limit-and-idlecount-generate-the-upper-limit-of-running-machines). - -Example: - -```toml -[[runners]] - name = "gitlab-aws-autoscaler" - url = "<URL of your GitLab instance>" - token = "<Runner's token>" - executor = "docker+machine" - limit = 20 -``` - -[Read more](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-section) -about all the options you can use under `[[runners]]`. - -### The `runners.docker` section - -In the `[runners.docker]` section you can define the default Docker image to -be used by the child Runners if it's not defined in [`.gitlab-ci.yml`](../../ci/yaml/README.md). -By using `privileged = true`, all Runners will be able to run -[Docker in Docker](../../ci/docker/using_docker_build.md#use-docker-in-docker-executor) -which is useful if you plan to build your own Docker images via GitLab CI/CD. - -Next, we use `disable_cache = true` to disable the Docker executor's inner -cache mechanism since we will use the distributed cache mode as described -in the following section. - -Example: - -```toml - [runners.docker] - image = "alpine" - privileged = true - disable_cache = true -``` - -[Read more](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-docker-section) -about all the options you can use under `[runners.docker]`. - -### The `runners.cache` section - -To speed up your jobs, GitLab Runner provides a cache mechanism where selected -directories and/or files are saved and shared between subsequent jobs. -While not required for this setup, it is recommended to use the distributed cache -mechanism that GitLab Runner provides. Since new instances will be created on -demand, it is essential to have a common place where the cache is stored. - -In the following example, we use Amazon S3: - -```toml - [runners.cache] - Type = "s3" - ServerAddress = "s3.amazonaws.com" - AccessKey = "<your AWS Access Key ID>" - SecretKey = "<your AWS Secret Access Key>" - BucketName = "<the bucket where your cache should be kept>" - BucketLocation = "us-east-1" - Shared = true -``` - -Here's some more info to further explore the cache mechanism: - -- [Reference for `runners.cache`](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-cache-section) -- [Deploying and using a cache server for GitLab Runner](https://docs.gitlab.com/runner/configuration/autoscale.html#distributed-runners-caching) -- [How cache works](../../ci/yaml/README.md#cache) - -### The `runners.machine` section - -This is the most important part of the configuration and it's the one that -tells GitLab Runner how and when to spawn new or remove old Docker Machine -instances. - -We will focus on the AWS machine options, for the rest of the settings read -about the: - -- [Autoscaling algorithm and the parameters it's based on](https://docs.gitlab.com/runner/configuration/autoscale.html#autoscaling-algorithm-and-parameters) - depends on the needs of your organization -- [Off peak time configuration](https://docs.gitlab.com/runner/configuration/autoscale.html#off-peak-time-mode-configuration) - useful when there are regular time periods in your organization when no work is done, for example weekends - -Here's an example of the `runners.machine` section: - -```toml - [runners.machine] - IdleCount = 1 - IdleTime = 1800 - MaxBuilds = 10 - OffPeakPeriods = [ - "* * 0-9,18-23 * * mon-fri *", - "* * * * * sat,sun *" - ] - OffPeakIdleCount = 0 - OffPeakIdleTime = 1200 - MachineDriver = "amazonec2" - MachineName = "gitlab-docker-machine-%s" - MachineOptions = [ - "amazonec2-access-key=XXXX", - "amazonec2-secret-key=XXXX", - "amazonec2-region=us-central-1", - "amazonec2-vpc-id=vpc-xxxxx", - "amazonec2-subnet-id=subnet-xxxxx", - "amazonec2-use-private-address=true", - "amazonec2-tags=runner-manager-name,gitlab-aws-autoscaler,gitlab,true,gitlab-runner-autoscale,true", - "amazonec2-security-group=docker-machine-scaler", - "amazonec2-instance-type=m4.2xlarge", - ] -``` - -The Docker Machine driver is set to `amazonec2` and the machine name has a -standard prefix followed by `%s` (required) that is replaced by the ID of the -child Runner: `gitlab-docker-machine-%s`. - -Now, depending on your AWS infrastructure, there are many options you can set up -under `MachineOptions`. Below you can see the most common ones. - -| Machine option | Description | -| -------------- | ----------- | -| `amazonec2-access-key=XXXX` | The AWS access key of the user that has permissions to create EC2 instances, see [AWS credentials](#aws-credentials). | -| `amazonec2-secret-key=XXXX` | The AWS secret key of the user that has permissions to create EC2 instances, see [AWS credentials](#aws-credentials). | -| `amazonec2-region=eu-central-1` | The region to use when launching the instance. You can omit this entirely and the default `us-east-1` will be used. | -| `amazonec2-vpc-id=vpc-xxxxx` | Your [VPC ID](https://docs.docker.com/machine/drivers/aws/#vpc-id) to launch the instance in. | -| `amazonec2-subnet-id=subnet-xxxx` | The AWS VPC subnet ID. | -| `amazonec2-use-private-address=true` | Use the private IP address of Docker Machines, but still create a public IP address. Useful to keep the traffic internal and avoid extra costs.| -| `amazonec2-tags=runner-manager-name,gitlab-aws-autoscaler,gitlab,true,gitlab-runner-autoscale,true` | AWS extra tag key-value pairs, useful to identify the instances on the AWS console. The "Name" tag is set to the machine name by default. We set the "runner-manager-name" to match the Runner name set in `[[runners]]`, so that we can filter all the EC2 instances created by a specific manager setup. Read more about [using tags in AWS](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html). | -| `amazonec2-security-group=docker-machine-scaler` | AWS VPC security group name, see [AWS security groups](#aws-security-groups). | -| `amazonec2-instance-type=m4.2xlarge` | The instance type that the child Runners will run on. | - -TIP: **Tip:** -Under `MachineOptions` you can add anything that the [AWS Docker Machine driver -supports](https://docs.docker.com/machine/drivers/aws/#options). You are highly -encouraged to read Docker's docs as your infrastructure setup may warrant -different options to be applied. - -NOTE: **Note:** -The child instances will use by default Ubuntu 16.04 unless you choose a -different AMI ID by setting `amazonec2-ami`. - -NOTE: **Note:** -If you specify `amazonec2-private-address-only=true` as one of the machine -options, your EC2 instance won't get assigned a public IP. This is ok if your -VPC is configured correctly with an Internet Gateway (IGW) and routing is fine, -but it’s something to consider if you've got a more complex configuration. Read -more in [Docker docs about VPC connectivity](https://docs.docker.com/machine/drivers/aws/#vpc-connectivity). - -[Read more](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-machine-section) -about all the options you can use under `[runners.machine]`. - -### Getting it all together - -Here's the full example of `/etc/gitlab-runner/config.toml`: - -```toml -concurrent = 10 -check_interval = 0 - -[[runners]] - name = "gitlab-aws-autoscaler" - url = "<URL of your GitLab instance>" - token = "<Runner's token>" - executor = "docker+machine" - limit = 20 - [runners.docker] - image = "alpine" - privileged = true - disable_cache = true - [runners.cache] - Type = "s3" - ServerAddress = "s3.amazonaws.com" - AccessKey = "<your AWS Access Key ID>" - SecretKey = "<your AWS Secret Access Key>" - BucketName = "<the bucket where your cache should be kept>" - BucketLocation = "us-east-1" - Shared = true - [runners.machine] - IdleCount = 1 - IdleTime = 1800 - MaxBuilds = 100 - OffPeakPeriods = [ - "* * 0-9,18-23 * * mon-fri *", - "* * * * * sat,sun *" - ] - OffPeakIdleCount = 0 - OffPeakIdleTime = 1200 - MachineDriver = "amazonec2" - MachineName = "gitlab-docker-machine-%s" - MachineOptions = [ - "amazonec2-access-key=XXXX", - "amazonec2-secret-key=XXXX", - "amazonec2-region=us-central-1", - "amazonec2-vpc-id=vpc-xxxxx", - "amazonec2-subnet-id=subnet-xxxxx", - "amazonec2-use-private-address=true", - "amazonec2-tags=runner-manager-name,gitlab-aws-autoscaler,gitlab,true,gitlab-runner-autoscale,true", - "amazonec2-security-group=docker-machine-scaler", - "amazonec2-instance-type=m4.2xlarge", - ] -``` - -## Cutting down costs with Amazon EC2 Spot instances - -As [described by][spot] Amazon: - -> -Amazon EC2 Spot instances allow you to bid on spare Amazon EC2 computing capacity. -Since Spot instances are often available at a discount compared to On-Demand -pricing, you can significantly reduce the cost of running your applications, -grow your application’s compute capacity and throughput for the same budget, -and enable new types of cloud computing applications. - -In addition to the [`runners.machine`](#the-runners-machine-section) options -you picked above, in `/etc/gitlab-runner/config.toml` under the `MachineOptions` -section, add the following: - -```toml - MachineOptions = [ - "amazonec2-request-spot-instance=true", - "amazonec2-spot-price=0.03", - "amazonec2-block-duration-minutes=60" - ] -``` - -With this configuration, Docker Machines are created on Spot instances with a -maximum bid price of $0.03 per hour and the duration of the Spot instance is -capped at 60 minutes. The `0.03` number mentioned above is just an example, so -be sure to check on the current pricing based on the region you picked. - -To learn more about Amazon EC2 Spot instances, visit the following links: - -- https://aws.amazon.com/ec2/spot/ -- https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/spot-requests.html -- https://aws.amazon.com/blogs/aws/focusing-on-spot-instances-lets-talk-about-best-practices/ - -### Caveats of Spot instances - -While Spot instances is a great way to use unused resources and minimize the -costs of your infrastructure, you must be aware of the implications. - -Running CI jobs on Spot instances may increase the failure rates because of the -Spot instances pricing model. If the price exceeds your bid, the existing Spot -instances will be immediately terminated and all your jobs on that host will fail. - -As a consequence, the auto-scale Runner would fail to create new machines while -it will continue to request new instances. This eventually will make 60 requests -and then AWS won't accept any more. Then once the Spot price is acceptable, you -are locked out for a bit because the call amount limit is exceeded. - -If you encounter that case, you can use the following command in the bastion -machine to see the Docker Machines state: - -```sh -docker-machine ls -q --filter state=Error --format "{{.NAME}}" -``` - -NOTE: **Note:** -There are some issues regarding making GitLab Runner gracefully handle Spot -price changes, and there are reports of `docker-machine` attempting to -continually remove a Docker Machine. GitLab has provided patches for both cases -in the upstream project. For more information, see issues -[#2771](https://gitlab.com/gitlab-org/gitlab-runner/issues/2771) and -[#2772](https://gitlab.com/gitlab-org/gitlab-runner/issues/2772). - -## Conclusion - -In this guide we learned how to install and configure a GitLab Runner in -autoscale mode on AWS. - -Using the autoscale feature of GitLab Runner can save you both time and money. -Using the Spot instances that AWS provides can save you even more, but you must -be aware of the implications. As long as your bid is high enough, there shouldn't -be an issue. - -You can read the following use cases from which this tutorial was (heavily) -influenced: - -- [HumanGeo - Scaling GitLab CI](http://blog.thehumangeo.com/gitlab-autoscale-runners.html) -- [subtrakt Health - Autoscale GitLab CI Runners and save 90% on EC2 costs](https://substrakthealth.com/news/gitlab-ci-cost-savings/) - -[spot]: https://aws.amazon.com/ec2/spot/ +This document was moved to [another location](https://docs.gitlab.com/runner/configuration/runner_autoscale_aws/index.html). diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 32464cbb259..ae0b5c0a2ba 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -93,7 +93,7 @@ be an array or a multi-line string. > Introduced in GitLab 8.7 and requires Gitlab Runner v1.2 `after_script` is used to define the command that will be run after for all -jobs. This has to be an array or a multi-line string. +jobs, including failed ones. This has to be an array or a multi-line string. > **Note:** The `before_script` and the main `script` are concatenated and run in a single context/container. diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index f2a9b1d769b..8c8501bcc23 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -11,6 +11,7 @@ Your GitLab instance can perform HTTP POST requests on the following events: - `user_remove_from_team` - `user_create` - `user_destroy` +- `user_failed_login` - `user_rename` - `key_create` - `key_destroy` @@ -22,6 +23,8 @@ Your GitLab instance can perform HTTP POST requests on the following events: The triggers for most of these are self-explanatory, but `project_update` and `project_rename` deserve some clarification: `project_update` is fired any time an attribute of a project is changed (name, description, tags, etc.) *unless* the `path` attribute is also changed. In that case, a `project_rename` is triggered instead (so that, for instance, if all you care about is the repo URL, you can just listen for `project_rename`). +`user_failed_login` is sent whenever a **blocked** user attempts to login and denied access. + System hooks can be used, e.g. for logging or changing information in a LDAP server. > **Note:** @@ -196,6 +199,23 @@ Please refer to `group_rename` and `user_rename` for that case. } ``` +**User failed login:** + +```json +{ + "event_name": "user_failed_login", + "created_at": "2017-10-03T06:08:48Z", + "updated_at": "2018-01-15T04:52:06Z", + "name": "John Smith", + "email": "user4@example.com", + "user_id": 26, + "username": "user4", + "state": "blocked" +} +``` + +If the user is blocked via LDAP, `state` will be `ldap_blocked`. + **User renamed:** ```json diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index d6cfa524a3a..819354bb780 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -164,7 +164,7 @@ Feature: Project Issues Given project "Shop" have "Release 0.4" open issue When I visit issue page "Release 0.4" Then I should see that I am subscribed - When I click button "Unsubscribe" + When I click the subscription toggle Then I should see that I am unsubscribed @javascript diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 3843374678c..3cd26bb429b 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -21,20 +21,20 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps step 'I should see that I am subscribed' do wait_for_requests - expect(find('.js-issuable-subscribe-button span')).to have_content 'Unsubscribe' + expect(find('.js-issuable-subscribe-button')).to have_css 'button.is-checked' end step 'I should see that I am unsubscribed' do wait_for_requests - expect(find('.js-issuable-subscribe-button span')).to have_content 'Subscribe' + expect(find('.js-issuable-subscribe-button')).to have_css 'button:not(.is-checked)' end step 'I click link "Closed"' do find('.issues-state-filters [data-state="closed"] span', text: 'Closed').click end - step 'I click button "Unsubscribe"' do - click_on "Unsubscribe" + step 'I click the subscription toggle' do + find('.js-issuable-subscribe-button button').click end step 'I should see "Release 0.3" in issues' do diff --git a/lib/api/circuit_breakers.rb b/lib/api/circuit_breakers.rb index 598c76f6168..c13154dc0ec 100644 --- a/lib/api/circuit_breakers.rb +++ b/lib/api/circuit_breakers.rb @@ -17,11 +17,11 @@ module API end def storage_health - @failing_storage_health ||= Gitlab::Git::Storage::Health.for_all_storages + @storage_health ||= Gitlab::Git::Storage::Health.for_all_storages end end - desc 'Get all failing git storages' do + desc 'Get all git storages' do detail 'This feature was introduced in GitLab 9.5' success Entities::RepositoryStorageHealth end diff --git a/lib/gitlab/auth/blocked_user_tracker.rb b/lib/gitlab/auth/blocked_user_tracker.rb new file mode 100644 index 00000000000..dae03a179e4 --- /dev/null +++ b/lib/gitlab/auth/blocked_user_tracker.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true +module Gitlab + module Auth + class BlockedUserTracker + ACTIVE_RECORD_REQUEST_PARAMS = 'action_dispatch.request.request_parameters' + + def self.log_if_user_blocked(env) + message = env.dig('warden.options', :message) + + # Devise calls User#active_for_authentication? on the User model and then + # throws an exception to Warden with User#inactive_message: + # https://github.com/plataformatec/devise/blob/v4.2.1/lib/devise/hooks/activatable.rb#L8 + # + # Since Warden doesn't pass the user record to the failure handler, we + # need to do a database lookup with the username. We can limit the + # lookups to happen when the user was blocked by checking the inactive + # message passed along by Warden. + return unless message == User::BLOCKED_MESSAGE + + login = env.dig(ACTIVE_RECORD_REQUEST_PARAMS, 'user', 'login') + + return unless login.present? + + user = User.by_login(login) + + return unless user&.blocked? + + Gitlab::AppLogger.info("Failed login for blocked user: user=#{user.username} ip=#{env['REMOTE_ADDR']}") + SystemHooksService.new.execute_hooks_for(user, :failed_login) + + true + rescue TypeError + end + end + end +end diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb index b4114a3ac96..cf02030c577 100644 --- a/lib/gitlab/auth/user_auth_finders.rb +++ b/lib/gitlab/auth/user_auth_finders.rb @@ -96,9 +96,7 @@ module Gitlab end def ensure_action_dispatch_request(request) - return request if request.is_a?(ActionDispatch::Request) - - ActionDispatch::Request.new(request.env) + ActionDispatch::Request.new(request.env.dup) end def current_request diff --git a/lib/gitlab/background_migration/prepare_untracked_uploads.rb b/lib/gitlab/background_migration/prepare_untracked_uploads.rb index 476c46341ae..4e0121ca34d 100644 --- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb +++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb @@ -7,6 +7,7 @@ module Gitlab class PrepareUntrackedUploads # rubocop:disable Metrics/ClassLength # For bulk_queue_background_migration_jobs_by_range include Database::MigrationHelpers + include ::Gitlab::Utils::StrongMemoize FIND_BATCH_SIZE = 500 RELATIVE_UPLOAD_DIR = "uploads".freeze @@ -142,7 +143,9 @@ module Gitlab end def postgresql? - @postgresql ||= Gitlab::Database.postgresql? + strong_memoize(:postgresql) do + Gitlab::Database.postgresql? + end end def can_bulk_insert_and_ignore_duplicates? @@ -150,8 +153,9 @@ module Gitlab end def postgresql_pre_9_5? - @postgresql_pre_9_5 ||= postgresql? && - Gitlab::Database.version.to_f < 9.5 + strong_memoize(:postgresql_pre_9_5) do + postgresql? && Gitlab::Database.version.to_f < 9.5 + end end def schedule_populate_untracked_uploads_jobs diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb index 85b79362196..c0c666dfb7b 100644 --- a/lib/gitlab/bare_repository_import/repository.rb +++ b/lib/gitlab/bare_repository_import/repository.rb @@ -1,6 +1,8 @@ module Gitlab module BareRepositoryImport class Repository + include ::Gitlab::Utils::StrongMemoize + attr_reader :group_path, :project_name, :repo_path def initialize(root_path, repo_path) @@ -41,11 +43,15 @@ module Gitlab private def wiki? - @wiki ||= repo_path.end_with?('.wiki.git') + strong_memoize(:wiki) do + repo_path.end_with?('.wiki.git') + end end def hashed? - @hashed ||= repo_relative_path.include?('@hashed') + strong_memoize(:hashed) do + repo_relative_path.include?('@hashed') + end end def repo_relative_path diff --git a/lib/gitlab/ci/pipeline/chain/skip.rb b/lib/gitlab/ci/pipeline/chain/skip.rb index 9a72de87bab..32cbb7ca6af 100644 --- a/lib/gitlab/ci/pipeline/chain/skip.rb +++ b/lib/gitlab/ci/pipeline/chain/skip.rb @@ -3,6 +3,8 @@ module Gitlab module Pipeline module Chain class Skip < Chain::Base + include ::Gitlab::Utils::StrongMemoize + SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i def perform! @@ -24,7 +26,9 @@ module Gitlab def commit_message_skips_ci? return false unless @pipeline.git_commit_message - @skipped ||= !!(@pipeline.git_commit_message =~ SKIP_PATTERN) + strong_memoize(:commit_message_skips_ci) do + !!(@pipeline.git_commit_message =~ SKIP_PATTERN) + end end end end diff --git a/lib/gitlab/ci/stage/seed.rb b/lib/gitlab/ci/stage/seed.rb index bc97aa63b02..f33c87f554d 100644 --- a/lib/gitlab/ci/stage/seed.rb +++ b/lib/gitlab/ci/stage/seed.rb @@ -2,6 +2,8 @@ module Gitlab module Ci module Stage class Seed + include ::Gitlab::Utils::StrongMemoize + attr_reader :pipeline delegate :project, to: :pipeline @@ -50,7 +52,9 @@ module Gitlab private def protected_ref? - @protected_ref ||= project.protected_for?(pipeline.ref) + strong_memoize(:protected_ref) do + project.protected_for?(pipeline.ref) + end end end end diff --git a/lib/gitlab/git/conflict/resolver.rb b/lib/gitlab/git/conflict/resolver.rb index 74c9874d590..07b7e811a34 100644 --- a/lib/gitlab/git/conflict/resolver.rb +++ b/lib/gitlab/git/conflict/resolver.rb @@ -15,7 +15,7 @@ module Gitlab @conflicts ||= begin @target_repository.gitaly_migrate(:conflicts_list_conflict_files) do |is_enabled| if is_enabled - gitaly_conflicts_client(@target_repository).list_conflict_files + gitaly_conflicts_client(@target_repository).list_conflict_files.to_a else rugged_list_conflict_files end diff --git a/lib/gitlab/gitaly_client/conflict_files_stitcher.rb b/lib/gitlab/gitaly_client/conflict_files_stitcher.rb new file mode 100644 index 00000000000..97c13d1fdb0 --- /dev/null +++ b/lib/gitlab/gitaly_client/conflict_files_stitcher.rb @@ -0,0 +1,47 @@ +module Gitlab + module GitalyClient + class ConflictFilesStitcher + include Enumerable + + def initialize(rpc_response) + @rpc_response = rpc_response + end + + def each + current_file = nil + + @rpc_response.each do |msg| + msg.files.each do |gitaly_file| + if gitaly_file.header + yield current_file if current_file + + current_file = file_from_gitaly_header(gitaly_file.header) + else + current_file.content << gitaly_file.content + end + end + end + + yield current_file if current_file + end + + private + + def file_from_gitaly_header(header) + Gitlab::Git::Conflict::File.new( + Gitlab::GitalyClient::Util.git_repository(header.repository), + header.commit_oid, + conflict_from_gitaly_file_header(header), + '' + ) + end + + def conflict_from_gitaly_file_header(header) + { + ours: { path: header.our_path, mode: header.our_mode }, + theirs: { path: header.their_path } + } + end + end + end +end diff --git a/lib/gitlab/gitaly_client/conflicts_service.rb b/lib/gitlab/gitaly_client/conflicts_service.rb index 40f032cf873..2565d537aff 100644 --- a/lib/gitlab/gitaly_client/conflicts_service.rb +++ b/lib/gitlab/gitaly_client/conflicts_service.rb @@ -20,7 +20,11 @@ module Gitlab ) response = GitalyClient.call(@repository.storage, :conflicts_service, :list_conflict_files, request) - files_from_response(response).to_a + GitalyClient::ConflictFilesStitcher.new(response) + end + + def conflicts? + list_conflict_files.any? end def resolve_conflicts(target_repository, resolution, source_branch, target_branch) @@ -58,38 +62,6 @@ module Gitlab user: Gitlab::Git::User.from_gitlab(resolution.user).to_gitaly ) end - - def files_from_response(response) - files = [] - - response.each do |msg| - msg.files.each do |gitaly_file| - if gitaly_file.header - files << file_from_gitaly_header(gitaly_file.header) - else - files.last.content << gitaly_file.content - end - end - end - - files - end - - def file_from_gitaly_header(header) - Gitlab::Git::Conflict::File.new( - Gitlab::GitalyClient::Util.git_repository(header.repository), - header.commit_oid, - conflict_from_gitaly_file_header(header), - '' - ) - end - - def conflict_from_gitaly_file_header(header) - { - ours: { path: header.our_path, mode: header.our_mode }, - theirs: { path: header.their_path } - } - end end end end diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index 5da9befa08e..4f160e4a447 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -14,6 +14,8 @@ module Gitlab # puts label.name # end class Client + include ::Gitlab::Utils::StrongMemoize + attr_reader :octokit # A single page of data and the corresponding page number. @@ -173,7 +175,9 @@ module Gitlab end def rate_limiting_enabled? - @rate_limiting_enabled ||= api_endpoint.include?('.github.com') + strong_memoize(:rate_limiting_enabled) do + api_endpoint.include?('.github.com') + end end def api_endpoint diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index d9a5af09f08..f357488ac61 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -16,8 +16,10 @@ module Gitlab def can_do_action?(action) return false unless can_access_git? - @permission_cache ||= {} - @permission_cache[action] ||= user.can?(action, project) + permission_cache[action] = + permission_cache.fetch(action) do + user.can?(action, project) + end end def cannot_do_action?(action) @@ -88,6 +90,10 @@ module Gitlab private + def permission_cache + @permission_cache ||= {} + end + def can_access_git? user && user.can?(:access_git) end diff --git a/rubocop/cop/gitlab/predicate_memoization.rb b/rubocop/cop/gitlab/predicate_memoization.rb new file mode 100644 index 00000000000..3c25d61d087 --- /dev/null +++ b/rubocop/cop/gitlab/predicate_memoization.rb @@ -0,0 +1,39 @@ +module RuboCop + module Cop + module Gitlab + class PredicateMemoization < RuboCop::Cop::Cop + MSG = <<~EOL.freeze + Avoid using `@value ||= query` inside predicate methods in order to + properly memoize `false` or `nil` values. + https://docs.gitlab.com/ee/development/utilities.html#strongmemoize + EOL + + def on_def(node) + return unless predicate_method?(node) + + select_offenses(node).each do |offense| + add_offense(offense, location: :expression) + end + end + + private + + def predicate_method?(node) + node.method_name.to_s.end_with?('?') + end + + def or_ivar_assignment?(or_assignment) + lhs = or_assignment.each_child_node.first + + lhs.ivasgn_type? + end + + def select_offenses(node) + node.each_descendant(:or_asgn).select do |or_assignment| + or_ivar_assignment?(or_assignment) + end + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 57af87f7fb9..9110237c538 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,4 +1,5 @@ require_relative 'cop/gitlab/module_with_instance_variables' +require_relative 'cop/gitlab/predicate_memoization' require_relative 'cop/include_sidekiq_worker' require_relative 'cop/line_break_around_conditional_block' require_relative 'cop/migration/add_column' diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index 205900615c4..b2dbfcd0031 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -334,14 +334,14 @@ describe 'Issue Boards', :js do wait_for_requests page.within('.subscriptions') do - click_button 'Subscribe' + find('.js-issuable-subscribe-button button:not(.is-checked)').click wait_for_requests - expect(page).to have_content('Unsubscribe') + expect(page).to have_css('.js-issuable-subscribe-button button.is-checked') end end - it 'has "Unsubscribe" button when already subscribed' do + it 'has checked subscription toggle when already subscribed' do create(:subscription, user: user, project: project, subscribable: issue2, subscribed: true) visit project_board_path(project, board) wait_for_requests @@ -350,10 +350,10 @@ describe 'Issue Boards', :js do wait_for_requests page.within('.subscriptions') do - click_button 'Unsubscribe' + find('.js-issuable-subscribe-button button.is-checked').click wait_for_requests - expect(page).to have_content('Subscribe') + expect(page).to have_css('.js-issuable-subscribe-button button:not(.is-checked)') end 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 index 4ca435491cb..f55eb5c6664 100644 --- a/spec/features/projects/merge_requests/user_manages_subscription_spec.rb +++ b/spec/features/projects/merge_requests/user_manages_subscription_spec.rb @@ -13,20 +13,18 @@ describe 'User manages subscription', :js do end it 'toggles subscription' do - subscribe_button = find('.js-issuable-subscribe-button') + page.within('.js-issuable-subscribe-button') do + expect(page).to have_css 'button:not(.is-checked)' + find('button:not(.is-checked)').click - expect(subscribe_button).to have_content('Subscribe') + wait_for_requests - click_on('Subscribe') + expect(page).to have_css 'button.is-checked' + find('button.is-checked').click - wait_for_requests + wait_for_requests - expect(subscribe_button).to have_content('Unsubscribe') - - click_on('Unsubscribe') - - wait_for_requests - - expect(subscribe_button).to have_content('Subscribe') + expect(page).to have_css 'button:not(.is-checked)' + end end end diff --git a/spec/javascripts/commit_merge_requests_spec.js b/spec/javascripts/commit_merge_requests_spec.js new file mode 100644 index 00000000000..3466ef51ea8 --- /dev/null +++ b/spec/javascripts/commit_merge_requests_spec.js @@ -0,0 +1,60 @@ +import * as CommitMergeRequests from '~/commit_merge_requests'; + +describe('CommitMergeRequests', () => { + describe('createContent', () => { + it('should return created content', () => { + const content1 = CommitMergeRequests.createContent([{ iid: 1, path: '/path1', title: 'foo' }, { iid: 2, path: '/path2', title: 'baz' }])[0]; + expect(content1.tagName).toEqual('SPAN'); + expect(content1.childElementCount).toEqual(4); + + const content2 = CommitMergeRequests.createContent([])[0]; + expect(content2.tagName).toEqual('SPAN'); + expect(content2.childElementCount).toEqual(0); + expect(content2.innerText).toEqual('No related merge requests found'); + }); + }); + + describe('getHeaderText', () => { + it('should return header text', () => { + expect(CommitMergeRequests.getHeaderText(0, 1)).toEqual('1 merge request'); + expect(CommitMergeRequests.getHeaderText(0, 2)).toEqual('2 merge requests'); + expect(CommitMergeRequests.getHeaderText(1, 1)).toEqual(','); + expect(CommitMergeRequests.getHeaderText(1, 2)).toEqual(','); + }); + }); + + describe('createHeader', () => { + it('should return created header', () => { + const header = CommitMergeRequests.createHeader(0, 1)[0]; + expect(header.tagName).toEqual('SPAN'); + expect(header.innerText).toEqual('1 merge request'); + }); + }); + + describe('createItem', () => { + it('should return created item', () => { + const item = CommitMergeRequests.createItem({ iid: 1, path: '/path', title: 'foo' })[0]; + expect(item.tagName).toEqual('SPAN'); + expect(item.childElementCount).toEqual(2); + expect(item.children[0].tagName).toEqual('A'); + expect(item.children[1].tagName).toEqual('SPAN'); + }); + }); + + describe('createLink', () => { + it('should return created link', () => { + const link = CommitMergeRequests.createLink({ iid: 1, path: '/path', title: 'foo' })[0]; + expect(link.tagName).toEqual('A'); + expect(link.href).toMatch(/\/path$/); + expect(link.innerText).toEqual('!1'); + }); + }); + + describe('createTitle', () => { + it('should return created title', () => { + const title = CommitMergeRequests.createTitle({ iid: 1, path: '/path', title: 'foo' })[0]; + expect(title.tagName).toEqual('SPAN'); + expect(title.innerText).toEqual('foo'); + }); + }); +}); diff --git a/spec/javascripts/sidebar/subscriptions_spec.js b/spec/javascripts/sidebar/subscriptions_spec.js index 9b33dd02fb9..79db05f04ed 100644 --- a/spec/javascripts/sidebar/subscriptions_spec.js +++ b/spec/javascripts/sidebar/subscriptions_spec.js @@ -20,23 +20,23 @@ describe('Subscriptions', function () { subscribed: undefined, }); - expect(vm.$refs.loadingButton.loading).toBe(true); - expect(vm.$refs.loadingButton.label).toBeUndefined(); + expect(vm.$refs.toggleButton.isLoading).toBe(true); + expect(vm.$refs.toggleButton.$el.querySelector('.project-feature-toggle')).toHaveClass('is-loading'); }); - it('has "Subscribe" text when currently not subscribed', () => { + it('is toggled "off" when currently not subscribed', () => { vm = mountComponent(Subscriptions, { subscribed: false, }); - expect(vm.$refs.loadingButton.label).toBe('Subscribe'); + expect(vm.$refs.toggleButton.$el.querySelector('.project-feature-toggle')).not.toHaveClass('is-checked'); }); - it('has "Unsubscribe" text when currently not subscribed', () => { + it('is toggled "on" when currently subscribed', () => { vm = mountComponent(Subscriptions, { subscribed: true, }); - expect(vm.$refs.loadingButton.label).toBe('Unsubscribe'); + expect(vm.$refs.toggleButton.$el.querySelector('.project-feature-toggle')).toHaveClass('is-checked'); }); }); diff --git a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb new file mode 100644 index 00000000000..726a3c1c83a --- /dev/null +++ b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Gitlab::Auth::BlockedUserTracker do + set(:user) { create(:user) } + + describe '.log_if_user_blocked' do + it 'does not log if user failed to login due to undefined reason' do + expect_any_instance_of(SystemHooksService).not_to receive(:execute_hooks_for) + + expect(described_class.log_if_user_blocked({})).to be_nil + end + + it 'gracefully handles malformed environment variables' do + env = { 'warden.options' => 'test' } + + expect(described_class.log_if_user_blocked(env)).to be_nil + end + + context 'failed login due to blocked user' do + let(:env) do + { + 'warden.options' => { message: User::BLOCKED_MESSAGE }, + described_class::ACTIVE_RECORD_REQUEST_PARAMS => { 'user' => { 'login' => user.username } } + } + end + + subject { described_class.log_if_user_blocked(env) } + + before do + expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login) + end + + it 'logs a blocked user' do + user.block! + + expect(subject).to be_truthy + end + + it 'logs a blocked user by e-mail' do + user.block! + env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email + + expect(subject).to be_truthy + end + + it 'logs a LDAP blocked user' do + user.ldap_block! + + expect(subject).to be_truthy + end + end + end +end diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb index 4637816570c..2733eef6611 100644 --- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -76,6 +76,16 @@ describe Gitlab::Auth::UserAuthFinders do expect(find_user_from_rss_token).to be_nil end end + + context 'when the request format is empty' do + it 'the method call does not modify the original value' do + env['action_dispatch.request.formats'] = nil + + find_user_from_rss_token + + expect(env['action_dispatch.request.formats']).to be_nil + end + end end describe '#find_user_from_access_token' do diff --git a/spec/lib/gitlab/gitaly_client/conflict_files_stitcher_spec.rb b/spec/lib/gitlab/gitaly_client/conflict_files_stitcher_spec.rb new file mode 100644 index 00000000000..1c933410bd5 --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/conflict_files_stitcher_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::ConflictFilesStitcher do + describe 'enumeration' do + it 'combines segregated ConflictFile messages together' do + target_project = create(:project, :repository) + target_repository = target_project.repository.raw + target_gitaly_repository = target_repository.gitaly_repository + + our_path_1 = 'our/path/1' + their_path_1 = 'their/path/1' + our_mode_1 = 0744 + commit_oid_1 = 'f00' + content_1 = 'content of the first file' + + our_path_2 = 'our/path/2' + their_path_2 = 'their/path/2' + our_mode_2 = 0600 + commit_oid_2 = 'ba7' + content_2 = 'content of the second file' + + header_1 = double(repository: target_gitaly_repository, commit_oid: commit_oid_1, + our_path: our_path_1, their_path: their_path_1, our_mode: our_mode_1) + header_2 = double(repository: target_gitaly_repository, commit_oid: commit_oid_2, + our_path: our_path_2, their_path: their_path_2, our_mode: our_mode_2) + + messages = [ + double(files: [double(header: header_1), double(header: nil, content: content_1[0..5])]), + double(files: [double(header: nil, content: content_1[6..-1])]), + double(files: [double(header: header_2)]), + double(files: [double(header: nil, content: content_2[0..5]), double(header: nil, content: content_2[6..10])]), + double(files: [double(header: nil, content: content_2[11..-1])]) + ] + + conflict_files = described_class.new(messages).to_a + + expect(conflict_files.size).to be(2) + + expect(conflict_files[0].content).to eq(content_1) + expect(conflict_files[0].their_path).to eq(their_path_1) + expect(conflict_files[0].our_path).to eq(our_path_1) + expect(conflict_files[0].our_mode).to be(our_mode_1) + expect(conflict_files[0].repository).to eq(target_repository) + expect(conflict_files[0].commit_oid).to eq(commit_oid_1) + + expect(conflict_files[1].content).to eq(content_2) + expect(conflict_files[1].their_path).to eq(their_path_2) + expect(conflict_files[1].our_path).to eq(our_path_2) + expect(conflict_files[1].our_mode).to be(our_mode_2) + expect(conflict_files[1].repository).to eq(target_repository) + expect(conflict_files[1].commit_oid).to eq(commit_oid_2) + end + end +end diff --git a/spec/lib/gitlab/gitaly_client/conflicts_service_spec.rb b/spec/lib/gitlab/gitaly_client/conflicts_service_spec.rb index b9641de7eda..e4fe01a671f 100644 --- a/spec/lib/gitlab/gitaly_client/conflicts_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/conflicts_service_spec.rb @@ -19,41 +19,12 @@ describe Gitlab::GitalyClient::ConflictsService do their_commit_oid: their_commit_oid ) end - let(:our_path) { 'our/path' } - let(:their_path) { 'their/path' } - let(:our_mode) { 0744 } - let(:header) do - double(repository: target_gitaly_repository, commit_oid: our_commit_oid, - our_path: our_path, our_mode: 0744, their_path: their_path) - end - let(:response) do - [ - double(files: [double(header: header), double(content: 'foo', header: nil)]), - double(files: [double(content: 'bar', header: nil)]) - ] - end - let(:file) { subject[0] } - - subject { client.list_conflict_files } it 'sends an RPC request' do expect_any_instance_of(Gitaly::ConflictsService::Stub).to receive(:list_conflict_files) - .with(request, kind_of(Hash)).and_return([]) - - subject - end - - it 'forms a Gitlab::Git::ConflictFile collection from the response' do - allow_any_instance_of(Gitaly::ConflictsService::Stub).to receive(:list_conflict_files) - .with(request, kind_of(Hash)).and_return(response) + .with(request, kind_of(Hash)).and_return([].to_enum) - expect(subject.size).to be(1) - expect(file.content).to eq('foobar') - expect(file.their_path).to eq(their_path) - expect(file.our_path).to eq(our_path) - expect(file.our_mode).to be(our_mode) - expect(file.repository).to eq(target_repository) - expect(file.commit_oid).to eq(our_commit_oid) + client.list_conflict_files end end diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index 38829773599..f2efcd9d0e9 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -151,11 +151,11 @@ describe CommitRange do .with(commit1, user) .and_return(true) - expect(commit1.has_been_reverted?(user, issue)).to eq(true) + expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(true) end - it 'returns false a commit has not been reverted' do - expect(commit1.has_been_reverted?(user, issue)).to eq(false) + it 'returns false if the commit has not been reverted' do + expect(commit1.has_been_reverted?(user, issue.notes_with_associations)).to eq(false) end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 817254c7d1e..d3826417762 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -513,4 +513,17 @@ eos expect(described_class.valid_hash?('a' * 41)).to be false end end + + describe '#merge_requests' do + let!(:project) { create(:project, :repository) } + let!(:merge_request1) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } + let!(:merge_request2) { create(:merge_request, source_project: project, source_branch: 'merged-target', target_branch: 'feature') } + let(:commit1) { merge_request1.merge_request_diff.commits.last } + let(:commit2) { merge_request1.merge_request_diff.commits.first } + + it 'returns merge_requests that introduced that commit' do + expect(commit1.merge_requests).to eq([merge_request1, merge_request2]) + expect(commit2.merge_requests).to eq([merge_request1]) + end + end end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index d556004eccf..b4249d72fc8 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -15,6 +15,28 @@ describe MergeRequestDiff do it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } end + describe '.by_commit_sha' do + subject(:by_commit_sha) { described_class.by_commit_sha(sha) } + + let!(:merge_request) { create(:merge_request, :with_diffs) } + + context 'with sha contained in' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns merge request diffs' do + expect(by_commit_sha).to eq([merge_request.merge_request_diff]) + end + end + + context 'with sha not contained in' do + let(:sha) { 'b83d6e3' } + + it 'returns empty result' do + expect(by_commit_sha).to be_empty + end + end + end + describe '#latest' do let!(:mr) { create(:merge_request, :with_diffs) } let!(:first_diff) { mr.merge_request_diff } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8ff82c4f791..c76f32b3989 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -87,6 +87,39 @@ describe MergeRequest do it { is_expected.to respond_to(:merge_when_pipeline_succeeds) } end + describe '.by_commit_sha' do + subject(:by_commit_sha) { described_class.by_commit_sha(sha) } + + let!(:merge_request) { create(:merge_request, :with_diffs) } + + context 'with sha contained in latest merge request diff' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns merge requests' do + expect(by_commit_sha).to eq([merge_request]) + end + end + + context 'with sha contained not in latest merge request diff' do + let(:sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' } + + it 'returns empty requests' do + latest_merge_request_diff = merge_request.merge_request_diffs.create + latest_merge_request_diff.merge_request_diff_commits.where(sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0').delete_all + + expect(by_commit_sha).to be_empty + end + end + + context 'with sha not contained in' do + let(:sha) { 'b83d6e3' } + + it 'returns empty result' do + expect(by_commit_sha).to be_empty + end + end + end + describe '.in_projects' do it 'returns the merge requests for a set of projects' do expect(described_class.in_projects(Project.all)).to eq([subject]) @@ -1030,6 +1063,83 @@ describe MergeRequest do end end + describe '#can_be_reverted?' do + context 'when there is no merged_at for the MR' do + before do + subject.metrics.update!(merged_at: nil) + end + + it 'returns false' do + expect(subject.can_be_reverted?(nil)).to be_falsey + end + end + + context 'when there is no merge_commit for the MR' do + before do + subject.metrics.update!(merged_at: Time.now.utc) + end + + it 'returns false' do + expect(subject.can_be_reverted?(nil)).to be_falsey + end + end + + context 'when the MR has been merged' do + before do + MergeRequests::MergeService + .new(subject.target_project, subject.author) + .execute(subject) + end + + context 'when there is no revert commit' do + it 'returns true' do + expect(subject.can_be_reverted?(nil)).to be_truthy + end + end + + context 'when there is a revert commit' do + let(:current_user) { subject.author } + let(:branch) { subject.target_branch } + let(:project) { subject.target_project } + + let(:revert_commit_id) do + params = { + commit: subject.merge_commit, + branch_name: branch, + start_branch: branch + } + + Commits::RevertService.new(project, current_user, params).execute[:result] + end + + before do + project.add_master(current_user) + + ProcessCommitWorker.new.perform(project.id, + current_user.id, + project.commit(revert_commit_id).to_hash, + project.default_branch == branch) + end + + context 'when the revert commit is mentioned in a note after the MR was merged' do + it 'returns false' do + expect(subject.can_be_reverted?(current_user)).to be_falsey + end + end + + context 'when the revert commit is mentioned in a note before the MR was merged' do + before do + subject.notes.last.update!(created_at: subject.metrics.merged_at - 1.second) + end + + it 'returns true' do + expect(subject.can_be_reverted?(current_user)).to be_truthy + end + end + end + end + end + describe '#participants' do let(:project) { create(:project, :public) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index edd981752d9..f3456e5b354 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -358,28 +358,38 @@ describe Repository do end describe '#can_be_merged?' do - context 'mergeable branches' do - subject { repository.can_be_merged?('0b4bc9a49b562e85de7cc9e834518ea6828729b9', 'master') } + shared_examples 'can be merged' do + context 'mergeable branches' do + subject { repository.can_be_merged?('0b4bc9a49b562e85de7cc9e834518ea6828729b9', 'master') } - it { is_expected.to be_truthy } - end + it { is_expected.to be_truthy } + end - context 'non-mergeable branches' do - subject { repository.can_be_merged?('bb5206fee213d983da88c47f9cf4cc6caf9c66dc', 'feature') } + context 'non-mergeable branches' do + subject { repository.can_be_merged?('bb5206fee213d983da88c47f9cf4cc6caf9c66dc', 'feature') } - it { is_expected.to be_falsey } - end + it { is_expected.to be_falsey } + end - context 'non merged branch' do - subject { repository.merged_to_root_ref?('fix') } + context 'non merged branch' do + subject { repository.merged_to_root_ref?('fix') } - it { is_expected.to be_falsey } + it { is_expected.to be_falsey } + end + + context 'non existent branch' do + subject { repository.merged_to_root_ref?('non_existent_branch') } + + it { is_expected.to be_nil } + end end - context 'non existent branch' do - subject { repository.merged_to_root_ref?('non_existent_branch') } + context 'when Gitaly can_be_merged feature is enabled' do + it_behaves_like 'can be merged' + end - it { is_expected.to be_nil } + context 'when Gitaly can_be_merged feature is disabled', :disable_gitaly do + it_behaves_like 'can be merged' end end diff --git a/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb b/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb new file mode 100644 index 00000000000..21fc4584654 --- /dev/null +++ b/spec/rubocop/cop/gitlab/predicate_memoization_spec.rb @@ -0,0 +1,100 @@ +require 'spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/predicate_memoization' + +describe RuboCop::Cop::Gitlab::PredicateMemoization do + include CopHelper + + subject(:cop) { described_class.new } + + shared_examples('registering offense') do |options| + let(:offending_lines) { options[:offending_lines] } + + it 'registers an offense when a predicate method is memoizing via ivar' do + inspect_source(source) + + aggregate_failures do + expect(cop.offenses.size).to eq(offending_lines.size) + expect(cop.offenses.map(&:line)).to eq(offending_lines) + end + end + end + + shared_examples('not registering offense') do + it 'does not register offenses' do + inspect_source(source) + + expect(cop.offenses).to be_empty + end + end + + context 'when source is a predicate method memoizing via ivar' do + it_behaves_like 'registering offense', offending_lines: [3] do + let(:source) do + <<~RUBY + class C + def really? + @really ||= true + end + end + RUBY + end + end + + it_behaves_like 'registering offense', offending_lines: [4] do + let(:source) do + <<~RUBY + class C + def really? + value = true + @really ||= value + end + end + RUBY + end + end + end + + context 'when source is a predicate method using ivar with assignment' do + it_behaves_like 'not registering offense' do + let(:source) do + <<~RUBY + class C + def really? + @really = true + end + end + RUBY + end + end + end + + context 'when source is a predicate method using local with ||=' do + it_behaves_like 'not registering offense' do + let(:source) do + <<~RUBY + class C + def really? + really ||= true + end + end + RUBY + end + end + end + + context 'when source is a regular method memoizing via ivar' do + it_behaves_like 'not registering offense' do + let(:source) do + <<~RUBY + class C + def really + @really ||= true + end + end + RUBY + end + end + end +end diff --git a/spec/services/system_hooks_service_spec.rb b/spec/services/system_hooks_service_spec.rb index 46cd10cdc12..c40cd5b7548 100644 --- a/spec/services/system_hooks_service_spec.rb +++ b/spec/services/system_hooks_service_spec.rb @@ -105,12 +105,25 @@ describe SystemHooksService do expect(data[:old_username]).to eq(user.username_was) end end + + context 'user_failed_login' do + it 'contains state of user' do + user.ldap_block! + + data = event_data(user, :failed_login) + + expect(data).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :state) + expect(data[:username]).to eq(user.username) + expect(data[:state]).to eq('ldap_blocked') + end + end end context 'event names' do it { expect(event_name(user, :create)).to eq "user_create" } it { expect(event_name(user, :destroy)).to eq "user_destroy" } it { expect(event_name(user, :rename)).to eq 'user_rename' } + it { expect(event_name(user, :failed_login)).to eq 'user_failed_login' } it { expect(event_name(project, :create)).to eq "project_create" } it { expect(event_name(project, :destroy)).to eq "project_destroy" } it { expect(event_name(project, :rename)).to eq "project_rename" } |